diff --git a/docs/DataStructure.md b/docs/DataStructure.md index d0e9acfa3..a1ef88f46 100644 --- a/docs/DataStructure.md +++ b/docs/DataStructure.md @@ -1,5 +1,5 @@ @@ -240,6 +240,7 @@ Currently supported Question-Types are: | `linearscale` | A linear or Likert scale question where you choose an option that best fits your opinion | | `color` | A color answer, hex string representation (e. g. `#123456`) | | `ranking` | Using pre-defined options, the user ranks them from most to least preferred. Needs at least one option available. Answers are stored in ranked order (one answer row per option). | +| `conditional` | A conditional branching question with a trigger question and multiple branches containing subquestions | ## Extra Settings @@ -282,3 +283,188 @@ Supported option types and their intended usage. | `column` | Represents a column header when rendering options in a grid-style question. | `row` and `column` option types are primarily used together with `grid` question types to build a two-dimensional selection matrix. `choice` is the default for normal option lists. +| `triggerType` | `conditional` | string | [See trigger types](#conditional-trigger-types) | The type of trigger question (dropdown, multiple_unique, etc.) | +| `branches` | `conditional` | Array | Array of [Branch objects](#branch-object) | The branches with conditions and subquestions | + +## Conditional Questions + +Conditional questions enable branching logic in forms. A trigger question determines which branch of subquestions appears based on the respondent's answer. + +### Question Properties for Subquestions + +Subquestions (questions belonging to a conditional question's branch) have additional properties: + +| Property | Type | Description | +| ---------------- | ------- | ---------------------------------------------------------------------- | +| parentQuestionId | Integer | The ID of the parent conditional question (null for regular questions) | +| branchId | String | The ID of the branch this subquestion belongs to | + +### Conditional Trigger Types + +Supported trigger types for conditional questions: + +| Trigger Type | Condition Type | Description | +| ----------------- | ---------------------------------------------------------------------------------------------- | --------------------------------------------- | +| `multiple_unique` | `option_selected`, `option_not_selected` | Radio buttons - single option selection | +| `dropdown` | `option_selected`, `option_not_selected` | Dropdown - single option selection | +| `multiple` | `option_selected`, `option_not_selected`, `options_combination` | Checkbox - option or combination matching | +| `short` | `string_equals`, `string_not_equals`, `string_contains`, `string_not_contains`, `regex` | Short text - string or regex matching | +| `long` | `string_contains`, `string_not_contains`, `regex` | Long text - string or regex matching | +| `linearscale` | `value_equals`, `value_not_equals`, `value_range`, `value_min`, `value_max` | Linear scale - numeric value matching | +| `date` | `date_equals`, `date_before`, `date_after`, `date_range` | Date - exact or range matching (`YYYY-MM-DD`) | +| `time` | `time_equals`, `time_before`, `time_after`, `time_range` | Time - exact or range matching (`HH:mm`) | +| `color` | `value_equals` | Color - exact value matching | +| `file` | `file_uploaded`, `file_not_uploaded`, `filename_contains`, `filename_equals`, `filename_regex` | File - upload status or filename matching | + +### Branch Object + +A branch defines conditions and subquestions that appear when those conditions are met. + +| Property | Type | Description | +| ------------ | ---------------------------------------- | ---------------------------------------------- | +| id | String | Unique identifier for the branch | +| conditions | Array of [Conditions](#condition-object) | Conditions that must be met to show the branch | +| subQuestions | Array of [Questions](#question) | Questions shown when conditions are met | + +```json +{ + "id": "branch-1705587600000", + "conditions": [{ "type": "option_selected", "optionId": 42 }], + "subQuestions": [ + { + "id": 101, + "formId": 3, + "order": 1, + "type": "short", + "text": "Please provide details", + "parentQuestionId": 100, + "branchId": "branch-1705587600000" + } + ] +} +``` + +### Condition Object + +Conditions determine when a branch is activated. The structure depends on the trigger type. + +#### option_selected (for dropdown, multiple_unique) + +```json +{ "type": "option_selected", "optionId": 42 } +``` + +#### options_combination (for multiple/checkboxes) + +```json +{ "type": "options_combination", "optionIds": [42, 43] } +``` + +All options in `optionIds` must be selected for the branch to activate (AND logic). + +#### string_equals (for short text) + +```json +{ "type": "string_equals", "value": "yes" } +``` + +#### string_contains (for short, long text) + +```json +{ "type": "string_contains", "value": "keyword" } +``` + +#### regex (for short, long text) + +```json +{ "type": "regex", "value": "^yes.*" } +``` + +#### value_equals (for color) + +```json +{ "type": "value_equals", "value": "#ff0000" } +``` + +#### value_range (for linearscale, time) + +```json +{ "type": "value_range", "min": 3, "max": 5 } +``` + +#### date_range (for date) + +```json +{ "type": "date_range", "min": "2024-01-01", "max": "2024-12-31" } +``` + +#### time_range (for time) + +```json +{ "type": "time_range", "min": "09:00", "max": "17:00" } +``` + +#### file_uploaded (for file) + +```json +{ "type": "file_uploaded", "fileUploaded": true } +``` + +### Conditional Question Example + +A complete conditional question structure: + +```json +{ + "id": 100, + "formId": 3, + "order": 1, + "type": "conditional", + "isRequired": true, + "text": "Do you have any dietary restrictions?", + "options": [ + { "id": 42, "questionId": 100, "order": 1, "text": "Yes" }, + { "id": 43, "questionId": 100, "order": 2, "text": "No" } + ], + "extraSettings": { + "triggerType": "dropdown", + "branches": [ + { + "id": "branch-yes", + "conditions": [{ "type": "option_selected", "optionId": 42 }], + "subQuestions": [ + { + "id": 101, + "formId": 3, + "order": 1, + "type": "long", + "text": "Please describe your dietary restrictions", + "parentQuestionId": 100, + "branchId": "branch-yes" + } + ] + } + ] + } +} +``` + +### Conditional Answer Structure + +When submitting or storing conditional question answers, the structure differs from regular questions: + +```json +{ + "100": { + "trigger": ["42"], + "subQuestions": { + "101": ["Vegetarian, no nuts"] + } + } +} +``` + +| Property | Type | Description | +| ------------ | --------------------------- | --------------------------------------------------- | +| trigger | Array of strings | Answer values for the trigger question | +| subQuestions | Object (questionId → Array) | Map of subquestion IDs to their answer value arrays | diff --git a/lib/Constants.php b/lib/Constants.php index 3e465cd0c..aabae35bf 100644 --- a/lib/Constants.php +++ b/lib/Constants.php @@ -1,7 +1,7 @@ ['string'], + 'branches' => ['array'], + ]; + + /** + * Condition types for conditional questions + */ + public const CONDITION_TYPE_OPTION_SELECTED = 'option_selected'; + public const CONDITION_TYPE_OPTIONS_COMBINATION = 'options_combination'; + public const CONDITION_TYPE_STRING_EQUALS = 'string_equals'; + public const CONDITION_TYPE_STRING_CONTAINS = 'string_contains'; + public const CONDITION_TYPE_REGEX = 'regex'; + public const CONDITION_TYPE_VALUE_EQUALS = 'value_equals'; + public const CONDITION_TYPE_VALUE_NOT_EQUALS = 'value_not_equals'; + public const CONDITION_TYPE_VALUE_RANGE = 'value_range'; + public const CONDITION_TYPE_VALUE_MIN = 'value_min'; + public const CONDITION_TYPE_VALUE_MAX = 'value_max'; + public const CONDITION_TYPE_DATE_RANGE = 'date_range'; + public const CONDITION_TYPE_FILE_UPLOADED = 'file_uploaded'; + + /** + * Trigger types allowed for conditional questions + * Maps each trigger type to its supported condition types + */ + public const CONDITIONAL_TRIGGER_TYPES = [ + self::ANSWER_TYPE_MULTIPLEUNIQUE => [self::CONDITION_TYPE_OPTION_SELECTED], + self::ANSWER_TYPE_DROPDOWN => [self::CONDITION_TYPE_OPTION_SELECTED], + self::ANSWER_TYPE_MULTIPLE => [self::CONDITION_TYPE_OPTIONS_COMBINATION], + self::ANSWER_TYPE_SHORT => [self::CONDITION_TYPE_STRING_EQUALS, self::CONDITION_TYPE_STRING_CONTAINS, self::CONDITION_TYPE_REGEX], + self::ANSWER_TYPE_LONG => [self::CONDITION_TYPE_STRING_CONTAINS, self::CONDITION_TYPE_REGEX], + self::ANSWER_TYPE_LINEARSCALE => [self::CONDITION_TYPE_VALUE_EQUALS, self::CONDITION_TYPE_VALUE_NOT_EQUALS, self::CONDITION_TYPE_VALUE_RANGE, self::CONDITION_TYPE_VALUE_MIN, self::CONDITION_TYPE_VALUE_MAX], + self::ANSWER_TYPE_DATE => [self::CONDITION_TYPE_DATE_RANGE], + self::ANSWER_TYPE_DATETIME => [self::CONDITION_TYPE_DATE_RANGE], + self::ANSWER_TYPE_TIME => [self::CONDITION_TYPE_DATE_RANGE], + self::ANSWER_TYPE_COLOR => [self::CONDITION_TYPE_VALUE_EQUALS], + self::ANSWER_TYPE_FILE => [self::CONDITION_TYPE_FILE_UPLOADED], + ]; + /** * !! Keep in sync with src/mixins/ShareTypes.js !! */ diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index 14bf7ea96..cb56ae208 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -1,7 +1,7 @@ setConfirmationEmailQuestionId($newQuestion->getId()); } - // Get Options, set new QuestionId, reinsert - $options = $this->optionMapper->findByQuestion($oldQuestion->getId()); - foreach ($options as $oldOption) { - $optionData = $oldOption->read(); + $copiedOptions = $this->copyOptions($oldQuestion->getId(), $newQuestion->getId()); - unset($optionData['id']); - $optionData['questionId'] = $newQuestion->getId(); - $newOption = Option::fromParams($optionData); - $this->optionMapper->insert($newOption); + if ($newQuestion->getType() === Constants::ANSWER_TYPE_CONDITIONAL) { + $this->copyConditionalBranches($oldQuestion, $newQuestion, $copiedOptions); } } @@ -530,6 +525,8 @@ public function getQuestion(int $formId, int $questionId): DataResponse { * @param FormsQuestionGridCellType $subtype the new question subtype * @param string $text the new question title * @param ?int $fromId (optional) id of the question that should be cloned + * @param ?int $parentQuestionId (optional) id of the parent conditional question (for subquestions) + * @param ?string $branchId (optional) branch id within the parent conditional question * @param ?int $position (optional) the position of the new question * @return DataResponse * @throws OCSBadRequestException Invalid type @@ -545,7 +542,7 @@ public function getQuestion(int $formId, int $questionId): DataResponse { #[NoAdminRequired()] #[BruteForceProtection(action: 'form')] #[ApiRoute(verb: 'POST', url: '/api/v3/forms/{formId}/questions')] - public function newQuestion(int $formId, ?string $type = null, ?string $subtype = null, string $text = '', ?int $fromId = null, ?int $position = null): DataResponse { + public function newQuestion(int $formId, ?string $type = null, ?string $subtype = null, string $text = '', ?int $fromId = null, ?int $parentQuestionId = null, ?string $branchId = null, ?int $position = null): DataResponse { $form = $this->formsService->getFormIfAllowed($formId, Constants::PERMISSION_EDIT); $this->formsService->obtainFormLock($form); @@ -555,10 +552,12 @@ public function newQuestion(int $formId, ?string $type = null, ?string $subtype } if ($fromId === null) { - $this->logger->debug('Adding new question: formId: {formId}, type: {type}, text: {text}', [ + $this->logger->debug('Adding new question: formId: {formId}, type: {type}, text: {text}, parentQuestionId: {parentQuestionId}, branchId: {branchId}', [ 'formId' => $formId, 'type' => $type, 'text' => $text, + 'parentQuestionId' => $parentQuestionId, + 'branchId' => $branchId, ]); if (array_search($type, Constants::ANSWER_TYPES) === false) { @@ -572,8 +571,54 @@ public function newQuestion(int $formId, ?string $type = null, ?string $subtype throw new OCSBadRequestException('Datetime question type no longer supported'); } + // Block creation of nested conditional questions (conditional within conditional) + if ($type === Constants::ANSWER_TYPE_CONDITIONAL && $parentQuestionId !== null) { + $this->logger->debug('Nested conditional questions are not supported'); + throw new OCSBadRequestException('Nested conditional questions are not supported'); + } + + // Validate parent question if creating a subquestion + if ($parentQuestionId !== null) { + try { + $parentQuestion = $this->questionMapper->findById($parentQuestionId); + } catch (IMapperException $e) { + $this->logger->debug('Could not find parent question'); + throw new OCSNotFoundException('Could not find parent question'); + } + + // Parent must be a conditional question + if ($parentQuestion->getType() !== Constants::ANSWER_TYPE_CONDITIONAL) { + $this->logger->debug('Parent question must be a conditional question'); + throw new OCSBadRequestException('Parent question must be a conditional question'); + } + + // Parent must belong to the same form + if ($parentQuestion->getFormId() !== $formId) { + $this->logger->debug('Parent question does not belong to this form'); + throw new OCSBadRequestException('Parent question does not belong to this form'); + } + + // branchId is required when parentQuestionId is set + if (empty($branchId)) { + $this->logger->debug('branchId is required when creating a subquestion'); + throw new OCSBadRequestException('branchId is required when creating a subquestion'); + } + + // The branch must exist on the parent question + $branchIds = array_column($parentQuestion->getExtraSettings()['branches'] ?? [], 'id'); + if (!in_array($branchId, $branchIds, true)) { + $this->logger->debug('Branch does not exist on the parent question'); + throw new OCSBadRequestException('Branch does not exist on the parent question'); + } + } + // Retrieve all active questions sorted by Order. - $questions = $this->questionMapper->findByForm($formId); + // For subquestions, get order within the branch + if ($parentQuestionId !== null) { + $questions = $this->questionMapper->findByBranch($parentQuestionId, $branchId); + } else { + $questions = $this->questionMapper->findByForm($formId); + } if ($position !== null) { $position = $this->shiftQuestionsForInsert($questions, $position); @@ -598,6 +643,12 @@ public function newQuestion(int $formId, ?string $type = null, ?string $subtype $question->setIsRequired(false); $question->setExtraSettings($subtype ? ['questionType' => $subtype] : []); + // Set parent question and branch for subquestions + if ($parentQuestionId !== null) { + $question->setParentQuestionId($parentQuestionId); + $question->setBranchId($branchId); + } + $question = $this->questionMapper->insert($question); $response = $this->formsService->getQuestion($question->getId()); @@ -617,13 +668,16 @@ public function newQuestion(int $formId, ?string $type = null, ?string $subtype if ($sourceQuestion->getFormId() !== $formId) { throw new OCSBadRequestException('Question doesn\'t belong to given form'); } - $sourceOptions = $this->optionMapper->findByQuestion($fromId); } catch (IMapperException) { $this->logger->debug('Could not find question'); throw new OCSNotFoundException('Could not find question'); } - $allQuestions = $this->questionMapper->findByForm($formId); + // Subquestions are ordered within their branch + $sourceParentId = $sourceQuestion->getParentQuestionId(); + $allQuestions = $sourceParentId !== null + ? $this->questionMapper->findByBranch($sourceParentId, (string)$sourceQuestion->getBranchId()) + : $this->questionMapper->findByForm($formId); $questionData = $sourceQuestion->read(); unset($questionData['id']); @@ -638,19 +692,18 @@ public function newQuestion(int $formId, ?string $type = null, ?string $subtype $newQuestion = Question::fromParams($questionData); $this->questionMapper->insert($newQuestion); - $response = $newQuestion->read(); - $response['options'] = []; - $response['accept'] = []; - - foreach ($sourceOptions as $sourceOption) { - $optionData = $sourceOption->read(); - - unset($optionData['id']); - $optionData['questionId'] = $newQuestion->getId(); - $newOption = Option::fromParams($optionData); - $insertedOption = $this->optionMapper->insert($newOption); + $copiedOptions = $this->copyOptions($fromId, $newQuestion->getId()); - $response['options'][] = $insertedOption->read(); + if ($newQuestion->getType() === Constants::ANSWER_TYPE_CONDITIONAL) { + $this->copyConditionalBranches($sourceQuestion, $newQuestion, $copiedOptions); + $response = $this->formsService->getQuestion($newQuestion->getId()); + if ($response === null) { + throw new OCSException('Failed to copy question'); + } + } else { + $response = $newQuestion->read(); + $response['options'] = array_values(array_map(static fn (Option $option) => $option->read(), $copiedOptions)); + $response['accept'] = []; } } @@ -673,7 +726,9 @@ public function newQuestion(int $formId, ?string $type = null, ?string $subtype * @return DataResponse * @throws OCSBadRequestException Question doesn\'t belong to given Form * @throws OCSBadRequestException Invalid extraSettings, will not update. + * @throws OCSBadRequestException Not allowed to change the type of or into a conditional question * @throws OCSForbiddenException Empty keyValuePairs, will not update + * @throws OCSForbiddenException Not allowed to update `parentQuestionId`, `branchId`, `options` or `accept` * @throws OCSForbiddenException Not allowed to update `id` or `formId` * @throws OCSForbiddenException Please use reorderQuestions() to change order * @throws OCSForbiddenException This form is archived and can not be modified @@ -727,6 +782,23 @@ public function updateQuestion(int $formId, int $questionId, array $keyValuePair throw new OCSForbiddenException('Not allowed to update \'id\' or \'formId\''); } + // The place of a subquestion is fixed on creation, options and accept have their own handling + foreach (['parentQuestionId', 'branchId', 'options', 'accept'] as $key) { + if (array_key_exists($key, $keyValuePairs)) { + $this->logger->debug('Not allowed to update \'{key}\'', ['key' => $key]); + throw new OCSForbiddenException('Not allowed to update \'' . $key . '\''); + } + } + + // Conditional questions can not be changed into another type and vice versa, + // the subquestions would be lost or end up nested + if (array_key_exists('type', $keyValuePairs) + && $keyValuePairs['type'] !== $question->getType() + && ($keyValuePairs['type'] === Constants::ANSWER_TYPE_CONDITIONAL || $question->getType() === Constants::ANSWER_TYPE_CONDITIONAL)) { + $this->logger->debug('Not allowed to change the type of or into a conditional question'); + throw new OCSBadRequestException('Not allowed to change the type of or into a conditional question'); + } + // Don't allow to reorder here if (array_key_exists('order', $keyValuePairs)) { $this->logger->debug('Key \'order\' is not allowed on updateQuestion. Please use reorderQuestions() to change order.'); @@ -742,6 +814,10 @@ public function updateQuestion(int $formId, int $questionId, array $keyValuePair $form->setConfirmationEmailQuestionId(null); } + if ($question->getType() === Constants::ANSWER_TYPE_CONDITIONAL && isset($keyValuePairs['extraSettings']['branches'])) { + $keyValuePairs['extraSettings'] = $this->updateConditionalBranches($question, $keyValuePairs['extraSettings']); + } + // Create QuestionEntity with given Params & Id. $question = Question::fromParams($keyValuePairs); $question->setId($questionId); @@ -798,6 +874,15 @@ public function deleteQuestion(int $formId, int $questionId): DataResponse { // Store Order of deleted Question $deletedOrder = $question->getOrder(); + // If this is a conditional question, also soft-delete its subquestions + if ($question->getType() === Constants::ANSWER_TYPE_CONDITIONAL) { + $subQuestions = $this->questionMapper->findByParentQuestion($questionId); + foreach ($subQuestions as $subQuestion) { + $subQuestion->setOrder(0); + $this->questionMapper->update($subQuestion); + } + } + // Mark question as deleted $question->setOrder(0); $this->questionMapper->update($question); @@ -810,8 +895,11 @@ public function deleteQuestion(int $formId, int $questionId): DataResponse { ]); } - // Update all question-order > deleted order. - $formQuestions = $this->questionMapper->findByForm($formId); + // Update all question-order > deleted order, subquestions are ordered within their branch + $parentQuestionId = $question->getParentQuestionId(); + $formQuestions = $parentQuestionId !== null + ? $this->questionMapper->findByBranch($parentQuestionId, (string)$question->getBranchId()) + : $this->questionMapper->findByForm($formId); foreach ($formQuestions as $question) { $questionOrder = $question->getOrder(); if ($questionOrder > $deletedOrder) { @@ -929,6 +1017,125 @@ public function reorderQuestions(int $formId, array $newOrder): DataResponse { return new DataResponse($response); } + /** + * Updates the Order of all Questions in a Branch of a conditional + * + * @param int $formId Id of the form to reorder + * @param string $branchId Id of the branch. + * @param int $parentQuestionId Id of the parent trigger question. + * @param list $newOrder Array of Question-Ids in new order. + * @return DataResponse, array{}> + * @throws OCSBadRequestException The given array contains duplicates + * @throws OCSBadRequestException The length of the given array does not match the number of stored questions + * @throws OCSBadRequestException Question doesn't belong to given Form + * @throws OCSBadRequestException One question has already been marked as deleted + * @throws OCSForbiddenException This form is archived and can not be modified + * @throws OCSForbiddenException User has no permissions to get this form + * @throws OCSNotFoundException Could not find form + * @throws OCSNotFoundException Could not find question + * + * 200: the question ids of the given form in the new order + */ + #[CORS()] + #[NoAdminRequired()] + #[BruteForceProtection(action: 'form')] + #[ApiRoute(verb: 'PATCH', url: '/api/v3/forms/{formId}/subquestions')] + public function reorderSubQuestions(int $formId, string $branchId, int $parentQuestionId, array $newOrder): DataResponse { + $this->logger->debug('Reordering Sub-Questions on Form {formId}, Parent-Question {parentQuestionId}, Branch {branchId} as Question-Ids {newOrder}', [ + 'formId' => $formId, + 'parentQuestionId' => $parentQuestionId, + 'branchId' => $branchId, + 'newOrder' => $newOrder + ]); + + $form = $this->formsService->getFormIfAllowed($formId, Constants::PERMISSION_EDIT); + $this->formsService->obtainFormLock($form); + + if ($this->formsService->isFormArchived($form)) { + $this->logger->debug('This form is archived and can not be modified'); + throw new OCSForbiddenException('This form is archived and can not be modified'); + } + + // Check if array contains duplicates + if (array_unique($newOrder) !== $newOrder) { + $this->logger->debug('The given array contains duplicates'); + throw new OCSBadRequestException('The given array contains duplicates'); + } + + // Check if all questions are given in Array. + $questions = $this->questionMapper->findByBranch($parentQuestionId, $branchId); + if (sizeof($questions) !== sizeof($newOrder)) { + $this->logger->debug('The length of the given array does not match the number of stored questions'); + throw new OCSBadRequestException('The length of the given array does not match the number of stored questions'); + } + + $questions = []; // Clear Array of Entities + $response = []; // Array of ['questionId' => ['order' => newOrder]] + + // Store array of Question-Entities and check the Questions FormId & old Order. + foreach ($newOrder as $arrayKey => $questionId) { + try { + $questions[$arrayKey] = $this->questionMapper->findById($questionId); + } catch (IMapperException $e) { + $this->logger->debug('Could not find question {questionId}', [ + 'questionId' => $questionId + ]); + throw new OCSNotFoundException('Could not find question'); + } + + // Abort if a question is not part of the Form. + if ($questions[$arrayKey]->getFormId() !== $formId) { + $this->logger->debug('This Question is not part of the given form: {questionId}', [ + 'questionId' => $questionId + ]); + throw new OCSBadRequestException('Question doesn\'t belong to given Form'); + } + // Abort if a question is not part of the Parent Question. + if ($questions[$arrayKey]->getParentQuestionId() !== $parentQuestionId) { + $this->logger->debug('This Question is not part of the given parent question: {parentQuestionId}', [ + 'parentQuestionId' => $parentQuestionId + ]); + throw new OCSBadRequestException('Question doesn\'t belong to given Parent Question'); + } + + // Abort if a question is not part of the Branch. + if ($questions[$arrayKey]->getBranchId() !== $branchId) { + $this->logger->debug('This Question is not part of the given branch: {branchId}', [ + 'branchId' => $branchId + ]); + throw new OCSBadRequestException('Question doesn\'t belong to given Branch'); + } + + // Abort if a question is already marked as deleted (order==0) + $oldOrder = $questions[$arrayKey]->getOrder(); + if ($oldOrder === 0) { + $this->logger->debug('This question has already been marked as deleted: Id: {questionId}', [ + 'questionId' => $questions[$arrayKey]->getId() + ]); + throw new OCSBadRequestException('One question has already been marked as deleted'); + } + + // Only set order, if it changed. + if ($oldOrder !== $arrayKey + 1) { + // Set Order. ArrayKey counts from zero, order counts from 1. + $questions[$arrayKey]->setOrder($arrayKey + 1); + } + } + + // Write to Database + foreach ($questions as $question) { + $this->questionMapper->update($question); + + $response[(string)$question->getId()] = [ + 'order' => $question->getOrder() + ]; + } + + $this->formMapper->update($form); + + return new DataResponse($response); + } + // Options /** @@ -1865,6 +2072,83 @@ public function uploadFiles(int $formId, int $questionId, string $shareHash = '' // private functions + /** + * Copy the options of a question to another question + * + * @return array the inserted options, keyed by the id of the option they were copied from + */ + private function copyOptions(int $fromQuestionId, int $toQuestionId): array { + $copiedOptions = []; + foreach ($this->optionMapper->findByQuestion($fromQuestionId) as $option) { + $optionData = $option->read(); + unset($optionData['id']); + $optionData['questionId'] = $toQuestionId; + $copiedOptions[$option->getId()] = $this->optionMapper->insert(Option::fromParams($optionData)); + } + return $copiedOptions; + } + + /** + * Copy the subquestions of a conditional question and point the branch conditions of the copy to its own options + * + * @param array $copiedOptions the options of the copy, keyed by the id of the option they were copied from + */ + private function copyConditionalBranches(Question $source, Question $copy, array $copiedOptions): void { + $mapOptionId = static fn ($optionId) => isset($copiedOptions[$optionId]) ? $copiedOptions[$optionId]->getId() : $optionId; + + $extraSettings = $copy->getExtraSettings(); + foreach ($extraSettings['branches'] ?? [] as $branchIndex => $branch) { + // Older versions stored a snapshot of the subquestions here, the rows are copied below + unset($extraSettings['branches'][$branchIndex]['subQuestions']); + foreach ($branch['conditions'] ?? [] as $conditionIndex => $condition) { + if (isset($condition['optionId'])) { + $extraSettings['branches'][$branchIndex]['conditions'][$conditionIndex]['optionId'] = $mapOptionId($condition['optionId']); + } + if (isset($condition['optionIds']) && is_array($condition['optionIds'])) { + $extraSettings['branches'][$branchIndex]['conditions'][$conditionIndex]['optionIds'] = array_map($mapOptionId, $condition['optionIds']); + } + } + } + $copy->setExtraSettings($extraSettings); + $this->questionMapper->update($copy); + + foreach ($this->questionMapper->findByParentQuestion($source->getId()) as $subQuestion) { + if ($subQuestion->getFormId() !== $source->getFormId()) { + continue; + } + $subQuestionData = $subQuestion->read(); + unset($subQuestionData['id']); + $subQuestionData['formId'] = $copy->getFormId(); + $subQuestionData['parentQuestionId'] = $copy->getId(); + $newSubQuestion = Question::fromParams($subQuestionData); + $this->questionMapper->insert($newSubQuestion); + $this->copyOptions($subQuestion->getId(), $newSubQuestion->getId()); + } + } + + /** + * Drop subquestion snapshots from the branches and delete the subquestions of removed branches + * + * @param array $extraSettings the new extra settings of the conditional question + * @return array the extra settings to store + */ + private function updateConditionalBranches(Question $question, array $extraSettings): array { + $branchIds = []; + foreach ($extraSettings['branches'] as $branchIndex => $branch) { + unset($extraSettings['branches'][$branchIndex]['subQuestions']); + $branchIds[] = $branch['id'] ?? null; + } + + foreach ($this->questionMapper->findByParentQuestion($question->getId()) as $subQuestion) { + if (!in_array($subQuestion->getBranchId(), $branchIds, true)) { + $subQuestion->setOrder(0); + $this->questionMapper->update($subQuestion); + } + } + + return $extraSettings; + } + /** * Insert answers for a question * @@ -1890,13 +2174,18 @@ private function storeAnswersForQuestion(Form $form, $submissionId, array $quest return; } + // Special handling for conditional questions + if ($question['type'] === Constants::ANSWER_TYPE_CONDITIONAL) { + $this->storeConditionalAnswer($form, $submissionId, $question, $answerArray); + return; + } + foreach ($answerArray as $answer) { $answerEntity = new Answer(); $answerEntity->setSubmissionId($submissionId); $answerEntity->setQuestionId($question['id']); $answerText = ''; - $uploadedFile = null; // Are we using answer ids as values if (in_array($question['type'], Constants::ANSWER_TYPES_PREDEFINED) && $question['type'] !== Constants::ANSWER_TYPE_LINEARSCALE) { // Search corresponding option, skip processing if not found @@ -1907,28 +2196,7 @@ private function storeAnswersForQuestion(Form $form, $submissionId, array $quest $answerText = str_replace(Constants::QUESTION_EXTRASETTINGS_OTHER_PREFIX, '', $answer); } } elseif ($question['type'] === Constants::ANSWER_TYPE_FILE) { - $uploadedFile = $this->uploadedFileMapper->getForSubmission( - (int)$answer['uploadedFileId'], - $form->getId(), - $question['id'], - $answer['uploadToken'] ?? '', - ); - $answerEntity->setFileId($uploadedFile->getFileId()); - - $userFolder = $this->rootFolder->getUserFolder($form->getOwnerId()); - $path = $this->formsService->getUploadedFilePath($form, $submissionId, $question['id'], $question['name'], $question['text']); - - if ($userFolder->nodeExists($path)) { - $folder = $userFolder->get($path); - } else { - $folder = $userFolder->newFolder($path); - } - /** @var Folder $folder */ - $file = $userFolder->getById($uploadedFile->getFileId())[0]; - $name = $folder->getNonExistingName($file->getName()); - $file->move($folder->getPath() . '/' . $name); - - $answerText = $name; + $answerText = $this->storeFileAnswer($form, $submissionId, $question, $answer, $answerEntity); } else { $answerText = $answer; // Not a multiple-question, answerText is given answer } @@ -1939,8 +2207,71 @@ private function storeAnswersForQuestion(Form $form, $submissionId, array $quest $answerEntity->setText($answerText); $this->answerMapper->insert($answerEntity); - if ($uploadedFile) { - $this->uploadedFileMapper->delete($uploadedFile); + } + } + + /** + * Store a file answer by moving the uploaded file to the submission folder. + * + * @param array $question The file question + * @param array{uploadedFileId: int|string, fileName?: string, uploadToken?: string} $answer The uploaded file answer + */ + private function storeFileAnswer(Form $form, int $submissionId, array $question, array $answer, Answer $answerEntity): string { + $uploadedFile = $this->uploadedFileMapper->getForSubmission( + (int)$answer['uploadedFileId'], + $form->getId(), + $question['id'], + $answer['uploadToken'] ?? '', + ); + $answerEntity->setFileId($uploadedFile->getFileId()); + + $userFolder = $this->rootFolder->getUserFolder($form->getOwnerId()); + $path = $this->formsService->getUploadedFilePath($form, $submissionId, $question['id'], $question['name'], $question['text']); + + if ($userFolder->nodeExists($path)) { + $folder = $userFolder->get($path); + } else { + $folder = $userFolder->newFolder($path); + } + /** @var Folder $folder */ + + $files = $userFolder->getById($uploadedFile->getFileId()); + if (empty($files)) { + throw new OCSBadRequestException('Uploaded file not found in storage.'); + } + $file = $files[0]; + $name = $folder->getNonExistingName($file->getName()); + $file->move($folder->getPath() . '/' . $name); + $this->uploadedFileMapper->delete($uploadedFile); + + return $name; + } + + /** + * Store answers for a conditional question + * + * Conditional answers have the structure: + * - trigger: array of trigger answer values (option IDs for predefined types, or text) + * - subQuestions: array of subquestion answers keyed by subquestion ID + * + * @param Form $form + * @param int $submissionId + * @param array $question The conditional question + * @param array $answerData The conditional answer data + */ + private function storeConditionalAnswer(Form $form, int $submissionId, array $question, array $answerData): void { + [$triggerAnswers, $subQuestionAnswers] = SubmissionService::splitConditionalAnswer($answerData); + + // The trigger is stored like a regular question of the trigger type + $this->storeAnswersForQuestion($form, $submissionId, SubmissionService::getTriggerQuestion($question), $triggerAnswers); + + // Store subquestion answers only for the branches the trigger activated + foreach ($this->submissionService->getActiveBranches($question, $triggerAnswers) as $branch) { + foreach ($branch['subQuestions'] ?? [] as $subQuestion) { + $subAnswers = $subQuestionAnswers[$subQuestion['id']] ?? []; + if (!empty($subAnswers)) { + $this->storeAnswersForQuestion($form, $submissionId, $subQuestion, (array)$subAnswers); + } } } } diff --git a/lib/Db/Question.php b/lib/Db/Question.php index 05355b95f..da99ce3df 100644 --- a/lib/Db/Question.php +++ b/lib/Db/Question.php @@ -3,7 +3,7 @@ declare(strict_types=1); /** - * SPDX-FileCopyrightText: 2019 Nextcloud GmbH and Nextcloud contributors + * SPDX-FileCopyrightText: 2019-2026 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: AGPL-3.0-or-later */ @@ -21,9 +21,9 @@ * @method void setOrder(integer $value) * @psalm-method FormsQuestionType getType() * @method string getType() - * @psalm-method 'date'|'datetime'|'dropdown'|'file'|'long'|'multiple'|'multiple_unique'|'short'|'time' getType() + * @psalm-method 'color'|'conditional'|'date'|'datetime'|'dropdown'|'file'|'long'|'multiple'|'multiple_unique'|'short'|'time' getType() * @method void setType(string $value) - * @psalm-method void setType('date'|'datetime'|'dropdown'|'file'|'long'|'multiple'|'multiple_unique'|'short'|'time' $value) + * @psalm-method void setType('color'|'conditional'|'date'|'datetime'|'dropdown'|'file'|'long'|'multiple'|'multiple_unique'|'short'|'time' $value) * @method bool getIsRequired() * @method void setIsRequired(bool $value) * @method string getText() @@ -32,6 +32,10 @@ * @method void setDescription(string $value) * @method string getName() * @method void setName(string $value) + * @method ?int getParentQuestionId() + * @method void setParentQuestionId(?int $value) + * @method ?string getBranchId() + * @method void setBranchId(?string $value) */ class Question extends Entity { protected $formId; @@ -42,6 +46,8 @@ class Question extends Entity { protected $name; protected $description; protected $extraSettingsJson; + protected $parentQuestionId; + protected $branchId; public function __construct() { $this->addType('formId', 'integer'); @@ -51,6 +57,8 @@ public function __construct() { $this->addType('text', 'string'); $this->addType('description', 'string'); $this->addType('name', 'string'); + $this->addType('parentQuestionId', 'integer'); + $this->addType('branchId', 'string'); } /** @@ -85,6 +93,8 @@ public function setExtraSettings(array $extraSettings): void { * name: string, * description: string, * extraSettings: FormsQuestionExtraSettings, + * parentQuestionId: ?int, + * branchId: ?string, * } */ public function read(): array { @@ -98,6 +108,8 @@ public function read(): array { 'name' => (string)$this->getName(), 'description' => (string)$this->getDescription(), 'extraSettings' => $this->getExtraSettings(), + 'parentQuestionId' => $this->getParentQuestionId(), + 'branchId' => $this->getBranchId(), ]; } diff --git a/lib/Db/QuestionMapper.php b/lib/Db/QuestionMapper.php index c705ac3c1..9284d1a06 100644 --- a/lib/Db/QuestionMapper.php +++ b/lib/Db/QuestionMapper.php @@ -1,7 +1,7 @@ db->getQueryBuilder(); $qb->select('*') @@ -37,6 +37,13 @@ public function findByForm(int $formId, bool $loadDeleted = false): array { $qb->expr()->eq('form_id', $qb->createNamedParameter($formId, IQueryBuilder::PARAM_INT)) ); + if (!$loadSubquestions) { + // Only load top-level questions (not subquestions of conditional questions) + $qb->andWhere( + $qb->expr()->isNull('parent_question_id') + ); + } + if (!$loadDeleted) { // Don't load questions, that are marked as deleted (marked by order==0). $qb->andWhere( @@ -50,6 +57,60 @@ public function findByForm(int $formId, bool $loadDeleted = false): array { return $this->findEntities($qb); } + /** + * Find subquestions belonging to a parent conditional question + * + * @param int $parentQuestionId The ID of the parent conditional question + * @param bool $loadDeleted Whether to include soft-deleted questions + * @return Question[] + */ + public function findByParentQuestion(int $parentQuestionId, bool $loadDeleted = false): array { + $qb = $this->db->getQueryBuilder(); + + $qb->select('*') + ->from($this->getTableName()) + ->where( + $qb->expr()->eq('parent_question_id', $qb->createNamedParameter($parentQuestionId, IQueryBuilder::PARAM_INT)) + ); + + if (!$loadDeleted) { + $qb->andWhere( + $qb->expr()->neq('order', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)) + ); + } + + // Sort by order within the conditional question + $qb->orderBy('order'); + + return $this->findEntities($qb); + } + + /** + * Find subquestions belonging to a specific branch of a conditional question + * + * @param int $parentQuestionId The ID of the parent conditional question + * @param string $branchId The branch identifier + * @return Question[] + */ + public function findByBranch(int $parentQuestionId, string $branchId): array { + $qb = $this->db->getQueryBuilder(); + + $qb->select('*') + ->from($this->getTableName()) + ->where( + $qb->expr()->eq('parent_question_id', $qb->createNamedParameter($parentQuestionId, IQueryBuilder::PARAM_INT)) + ) + ->andWhere( + $qb->expr()->eq('branch_id', $qb->createNamedParameter($branchId)) + ) + ->andWhere( + $qb->expr()->neq('order', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)) + ) + ->orderBy('order'); + + return $this->findEntities($qb); + } + /** * @param int $formId */ diff --git a/lib/Migration/Version050300Date20260118000000.php b/lib/Migration/Version050300Date20260118000000.php new file mode 100644 index 000000000..fd562c211 --- /dev/null +++ b/lib/Migration/Version050300Date20260118000000.php @@ -0,0 +1,60 @@ +getTable('forms_v2_questions'); + + // Add parent_question_id column - references parent conditional question + if (!$table->hasColumn('parent_question_id')) { + $table->addColumn('parent_question_id', Types::INTEGER, [ + 'notnull' => false, + 'default' => null, + 'comment' => 'ID of parent conditional question, null for top-level questions', + ]); + } + + // Add branch_id column - identifies which branch this subquestion belongs to + if (!$table->hasColumn('branch_id')) { + $table->addColumn('branch_id', Types::STRING, [ + 'notnull' => false, + 'length' => 64, + 'default' => null, + 'comment' => 'Branch identifier within parent conditional question', + ]); + } + + // Add index for efficient lookup of subquestions by parent + if (!$table->hasIndex('forms_v2_q_parent_idx')) { + $table->addIndex(['parent_question_id'], 'forms_v2_q_parent_idx'); + } + + return $schema; + } +} diff --git a/lib/ResponseDefinitions.php b/lib/ResponseDefinitions.php index 15ed97324..120921de4 100644 --- a/lib/ResponseDefinitions.php +++ b/lib/ResponseDefinitions.php @@ -42,9 +42,11 @@ * validationRegex?: string, * validationType?: string, * questionType?: string, + * triggerType?: string, + * branches?: list>, * } * - * @psalm-type FormsQuestionType = "dropdown"|"multiple"|"multiple_unique"|"date"|"time"|"short"|"long"|"file"|"datetime"|"grid" + * @psalm-type FormsQuestionType = "dropdown"|"multiple"|"multiple_unique"|"date"|"time"|"short"|"long"|"file"|"datetime"|"grid"|"color"|"conditional" * @psalm-type FormsQuestionGridCellType = "checkbox"|"number"|"radio" * * @psalm-type FormsQuestion = array{ @@ -59,6 +61,8 @@ * extraSettings: FormsQuestionExtraSettings|\stdClass, * options: list, * accept: list, + * parentQuestionId?: ?int, + * branchId?: ?string, * } * * @psalm-type FormsAnswer = array{ diff --git a/lib/Service/FormsService.php b/lib/Service/FormsService.php index 197ababbc..4df01d926 100644 --- a/lib/Service/FormsService.php +++ b/lib/Service/FormsService.php @@ -1,7 +1,7 @@ + * @param integer $formId + * @return list */ - private function getOptions(int $questionId): array { - $optionList = []; + public function getQuestions(int $formId): array { try { - $optionEntities = $this->optionMapper->findByQuestion($questionId); - foreach ($optionEntities as $optionEntity) { - $optionList[] = $optionEntity->read(); - } + // Top-level questions and subquestions are loaded at once and sorted by order + $questionEntities = $this->questionMapper->findByForm($formId, false, true); } catch (DoesNotExistException) { - //handle silently - } finally { - return $optionList; + return []; + } + + $optionsByQuestion = $this->getOptionsByQuestion($questionEntities); + $subQuestionsByParent = []; + foreach ($questionEntities as $questionEntity) { + if ($questionEntity->getParentQuestionId() !== null) { + $subQuestionsByParent[$questionEntity->getParentQuestionId()][] = $questionEntity; + } + } + + $questionList = []; + foreach ($questionEntities as $questionEntity) { + if ($questionEntity->getParentQuestionId() === null) { + $questionList[] = $this->formatQuestion($questionEntity, $optionsByQuestion, $subQuestionsByParent[$questionEntity->getId()] ?? []); + } } + return $questionList; } /** - * Load questions corresponding to form + * Load the options of the given questions at once * - * @param integer $formId - * @return list + * @param Question[] $questionEntities + * @return array> the options keyed by question id */ - public function getQuestions(int $formId): array { - $questionList = []; - try { - $questionEntities = $this->questionMapper->findByForm($formId); + private function getOptionsByQuestion(array $questionEntities): array { + $questionIds = array_map(static fn (Question $question) => $question->getId(), $questionEntities); + $optionsByQuestion = []; + foreach ($this->optionMapper->findByQuestions($questionIds) as $optionEntity) { + $optionsByQuestion[$optionEntity->getQuestionId()][] = $optionEntity->read(); + } + return $optionsByQuestion; + } - $questionIds = array_map(static fn (Question $question) => $question->getId(), $questionEntities); - $optionsByQuestion = []; - foreach ($this->optionMapper->findByQuestions($questionIds) as $optionEntity) { - $optionsByQuestion[$optionEntity->getQuestionId()][] = $optionEntity; + /** + * Build the API representation of a question + * + * @param array> $optionsByQuestion the options keyed by question id + * @param Question[] $subQuestionEntities the subquestions, if the question is a conditional question + * @return FormsQuestion + */ + private function formatQuestion(Question $questionEntity, array $optionsByQuestion, array $subQuestionEntities = []): array { + $question = $questionEntity->read(); + $question['options'] = $optionsByQuestion[$question['id']] ?? []; + $question['accept'] = []; + if ($question['type'] === Constants::ANSWER_TYPE_FILE) { + if ($question['extraSettings']['allowedFileTypes'] ?? null) { + $question['accept'] = array_map(fn (string $fileType) => str_contains($fileType, '/') ? $fileType : $fileType . '/*', $question['extraSettings']['allowedFileTypes']); } - foreach ($questionEntities as $questionEntity) { - $question = $questionEntity->read(); - $question['options'] = array_map( - static fn (Option $option) => $option->read(), - $optionsByQuestion[$question['id']] ?? [] - ); - $question['accept'] = []; - if ($question['type'] === Constants::ANSWER_TYPE_FILE) { - if ($question['extraSettings']['allowedFileTypes'] ?? null) { - $question['accept'] = array_map(fn (string $fileType) => str_contains($fileType, '/') ? $fileType : $fileType . '/*', $question['extraSettings']['allowedFileTypes']); - } - - if ($question['extraSettings']['allowedFileExtensions'] ?? null) { - foreach ($question['extraSettings']['allowedFileExtensions'] as $extension) { - $question['accept'][] = '.' . $extension; - } - } + if ($question['extraSettings']['allowedFileExtensions'] ?? null) { + foreach ($question['extraSettings']['allowedFileExtensions'] as $extension) { + $question['accept'][] = '.' . $extension; } + } + } - // Set `isRequired` if minimum options limit is set - if ($question['type'] === Constants::ANSWER_TYPE_MULTIPLE && ($question['extraSettings']['optionsLimitMin'] ?? 0) > 0) { - $question['isRequired'] = true; - } + // Set `isRequired` if minimum options limit is set + if ($question['type'] === Constants::ANSWER_TYPE_MULTIPLE && ($question['extraSettings']['optionsLimitMin'] ?? 0) > 0) { + $question['isRequired'] = true; + } - $questionList[] = $question; + // Attach the subquestions of conditional questions to their branches + if ($question['type'] === Constants::ANSWER_TYPE_CONDITIONAL && is_array($question['extraSettings']['branches'] ?? null)) { + $subQuestionsByBranch = []; + foreach ($subQuestionEntities as $subQuestionEntity) { + // Subquestions always belong to the form of their parent + if ($subQuestionEntity->getFormId() === $questionEntity->getFormId()) { + $subQuestionsByBranch[$subQuestionEntity->getBranchId()][] = $this->formatQuestion($subQuestionEntity, $optionsByQuestion); + } + } + foreach ($question['extraSettings']['branches'] as $index => $branch) { + $question['extraSettings']['branches'][$index]['subQuestions'] = $subQuestionsByBranch[$branch['id'] ?? ''] ?? []; } - } catch (DoesNotExistException) { - //handle silently - } finally { - return $questionList; } + + return $question; } /** @@ -164,24 +184,16 @@ public function getQuestions(int $formId): array { public function getQuestion(int $questionId): ?array { try { $questionEntity = $this->questionMapper->findById($questionId); - $question = $questionEntity->read(); - $question['options'] = $this->getOptions($question['id']); - $question['accept'] = []; - if ($question['type'] === Constants::ANSWER_TYPE_FILE) { - if ($question['extraSettings']['allowedFileTypes'] ?? null) { - $question['accept'] = array_map(fn (string $fileType) => str_contains($fileType, '/') ? $fileType : $fileType . '/*', $question['extraSettings']['allowedFileTypes']); - } - - if ($question['extraSettings']['allowedFileExtensions'] ?? null) { - foreach ($question['extraSettings']['allowedFileExtensions'] as $extension) { - $question['accept'][] = '.' . $extension; - } - } - } - return $question; } catch (DoesNotExistException) { return null; } + + $subQuestionEntities = $questionEntity->getType() === Constants::ANSWER_TYPE_CONDITIONAL + ? $this->questionMapper->findByParentQuestion($questionId) + : []; + $optionsByQuestion = $this->getOptionsByQuestion(array_merge([$questionEntity], $subQuestionEntities)); + + return $this->formatQuestion($questionEntity, $optionsByQuestion, $subQuestionEntities); } /** @@ -852,6 +864,64 @@ public function notifyNewSubmission(Form $form, Submission $submission): void { $this->confirmationEmailService->send($form, $submission); } + /** + * Check that a regular expression is enclosed in slashes, only uses modifiers supported by PHP *and* JS and compiles + */ + private function isValidRegex(string $regex): bool { + // general pattern of a valid regex + $VALID_REGEX = '/^\/(.+)\/([smi]{0,3})$/'; + // pattern to look for unescaped slashes + $REGEX_UNESCAPED_SLASHES = '/(?<=(^|[^\\]))(\\\\)*\//'; + + $matches = []; + // only pattern with delimiters and supported modifiers (by PHP *and* JS) + if (@preg_match($VALID_REGEX, $regex, $matches) !== 1) { + return false; + } + + // We use slashes as delimters, so unescaped slashes within the pattern are **not** allowed + if (@preg_match($REGEX_UNESCAPED_SLASHES, $matches[1]) === 1) { + return false; + } + + // Try to compile the given pattern, `preg_match` will return false if the pattern is invalid + return @preg_match($regex, 'some string') !== false; + } + + /** + * Check the conditions of a conditional question branch against its trigger type + * + * @param string $triggerType the type of the trigger question + * @param array $conditions the conditions of the branch + */ + private function areBranchConditionsValid(string $triggerType, array $conditions): bool { + $allowedTypes = Constants::CONDITIONAL_TRIGGER_TYPES[$triggerType] ?? []; + foreach ($conditions as $condition) { + if (!is_array($condition)) { + return false; + } + if (isset($condition['type']) && !in_array($condition['type'], $allowedTypes, true)) { + return false; + } + if (isset($condition['optionId']) && !is_int($condition['optionId'])) { + return false; + } + if (isset($condition['optionIds']) + && (!is_array($condition['optionIds']) || array_filter($condition['optionIds'], static fn ($id) => !is_int($id)) !== [])) { + return false; + } + foreach (['value', 'min', 'max'] as $key) { + if (isset($condition[$key]) && !is_scalar($condition[$key])) { + return false; + } + } + if (($condition['type'] ?? null) === Constants::CONDITION_TYPE_REGEX && !$this->isValidRegex((string)($condition['value'] ?? ''))) { + return false; + } + } + return true; + } + /** * Return shares of a form shared with given user * @@ -900,19 +970,28 @@ public function areExtraSettingsValid(array $extraSettings, string $questionType return true; } - // Ensure only allowed keys are set - $allowed = match ($questionType) { + // Map allowed options to answer type + $extraSettingsMap = [ Constants::ANSWER_TYPE_DROPDOWN => Constants::EXTRA_SETTINGS_DROPDOWN, - Constants::ANSWER_TYPE_MULTIPLE, Constants::ANSWER_TYPE_MULTIPLEUNIQUE => Constants::EXTRA_SETTINGS_MULTIPLE, + Constants::ANSWER_TYPE_MULTIPLE => Constants::EXTRA_SETTINGS_MULTIPLE, + Constants::ANSWER_TYPE_MULTIPLEUNIQUE => Constants::EXTRA_SETTINGS_MULTIPLE, Constants::ANSWER_TYPE_SHORT => Constants::EXTRA_SETTINGS_SHORT, Constants::ANSWER_TYPE_FILE => Constants::EXTRA_SETTINGS_FILE, Constants::ANSWER_TYPE_DATE => Constants::EXTRA_SETTINGS_DATE, Constants::ANSWER_TYPE_GRID => Constants::EXTRA_SETTINGS_GRID, - Constants::ANSWER_TYPE_RANKING => Constants::EXTRA_SETTINGS_RANKING, Constants::ANSWER_TYPE_TIME => Constants::EXTRA_SETTINGS_TIME, Constants::ANSWER_TYPE_LINEARSCALE => Constants::EXTRA_SETTINGS_LINEARSCALE, - default => [], - }; + Constants::ANSWER_TYPE_RANKING => Constants::EXTRA_SETTINGS_RANKING, + Constants::ANSWER_TYPE_CONDITIONAL => Constants::EXTRA_SETTINGS_CONDITIONAL, + ]; + + // Get triggertype if is conditional + $triggerType = $extraSettings['triggerType'] ?? null; + if (is_string($triggerType) && isset($extraSettingsMap[$triggerType])) { + $extraSettingsMap[Constants::ANSWER_TYPE_CONDITIONAL] += $extraSettingsMap[$triggerType]; + } + // Ensure only allowed keys are set + $allowed = $extraSettingsMap[$questionType] ?? []; // Number of keys in extraSettings but not in allowed (but not the other way round) $diff = array_diff(array_keys($extraSettings), array_keys($allowed)); if (count($diff) > 0) { @@ -1022,35 +1101,62 @@ public function areExtraSettingsValid(array $extraSettings, string $questionType return true; } - // general pattern of a valid regex - $VALID_REGEX = '/^\/(.+)\/([smi]{0,3})$/'; - // pattern to look for unescaped slashes - $REGEX_UNESCAPED_SLASHES = '/(?<=(^|[^\\\\]))(\\\\\\\\)*\\//'; + if (!$this->isValidRegex($extraSettings['validationRegex'])) { + return false; + } + } + + // Special handling of linear scale validation + } elseif ($questionType === Constants::ANSWER_TYPE_LINEARSCALE) { + // Ensure limits are sane + if (isset($extraSettings['optionsLowest']) && ($extraSettings['optionsLowest'] < 0 || $extraSettings['optionsLowest'] > 1) + || isset($extraSettings['optionsHighest']) && ($extraSettings['optionsHighest'] < 2 || $extraSettings['optionsHighest'] > 10)) { + return false; + } + } elseif ($questionType === Constants::ANSWER_TYPE_CONDITIONAL) { + // Validate conditional question settings + if (!isset($extraSettings['triggerType']) || !is_string($extraSettings['triggerType'])) { + return false; + } - $matches = []; - // only pattern with delimiters and supported modifiers (by PHP *and* JS) - if (@preg_match($VALID_REGEX, $extraSettings['validationRegex'], $matches) !== 1) { + // Validate trigger type is a valid question type (and not nested conditional) + if (!array_key_exists($extraSettings['triggerType'], Constants::CONDITIONAL_TRIGGER_TYPES)) { + return false; + } + + // Branches are required for conditional questions + if (!isset($extraSettings['branches']) || !is_array($extraSettings['branches'])) { + return false; + } + + // Validate branches structure + foreach ($extraSettings['branches'] as $branch) { + // Each branch must have an id + if (!isset($branch['id']) || !is_string($branch['id'])) { return false; } - // We use slashes as delimters, so unescaped slashes within the pattern are **not** allowed - if (@preg_match($REGEX_UNESCAPED_SLASHES, $matches[1]) === 1) { + // Branches must have conditions array + if (!isset($branch['conditions']) || !is_array($branch['conditions'])) { return false; } - // Try to compile the given pattern, `preg_match` will return false if the pattern is invalid - if (@preg_match($extraSettings['validationRegex'], 'some string') === false) { + if (!$this->areBranchConditionsValid($extraSettings['triggerType'], $branch['conditions'])) { return false; } } - // Special handling of linear scale validation - } elseif ($questionType === Constants::ANSWER_TYPE_LINEARSCALE) { - // Ensure limits are sane - if (isset($extraSettings['optionsLowest']) && ($extraSettings['optionsLowest'] < 0 || $extraSettings['optionsLowest'] > 1) - || isset($extraSettings['optionsHighest']) && ($extraSettings['optionsHighest'] < 2 || $extraSettings['optionsHighest'] > 10)) { - return false; + $diff = array_diff(array_keys($extraSettings), array_keys(Constants::EXTRA_SETTINGS_CONDITIONAL)); + // Handle options of triggerQuestion + if (count($diff) > 0 && isset($triggerType) && $triggerType !== '') { + $diffDict = []; + foreach ($diff as $k) { + $diffDict[$k] = $extraSettings[$k]; + } + $valid = self::areExtraSettingsValid($diffDict, $triggerType); + return $valid; } + } return true; } diff --git a/lib/Service/SubmissionService.php b/lib/Service/SubmissionService.php index 278fb2828..b5065421e 100644 --- a/lib/Service/SubmissionService.php +++ b/lib/Service/SubmissionService.php @@ -1,7 +1,7 @@ questionMapper->findByForm($form->getId()); + $questions = $this->orderQuestionsForExport($this->questionMapper->findByForm($form->getId(), false, true)); $defaultTimeZone = $this->config->getSystemValueString('default_timezone', 'UTC'); if (!$this->currentUser) { @@ -591,7 +591,7 @@ private function escapeCSV(string $value): string { * @param array $answers Array of the submitted answers * @param string $formOwnerId Owner of the form * @param int $formId Id of the form being submitted - * @throw \InvalidArgumentException if validation failed + * @throws \InvalidArgumentException if validation failed */ public function validateSubmission(array $questions, array $answers, string $formOwnerId, int $formId): void { // Check by questions @@ -599,6 +599,20 @@ public function validateSubmission(array $questions, array $answers, string $for $questionId = $question['id']; $questionAnswered = array_key_exists($questionId, $answers); + // Special handling for conditional questions + if ($question['type'] === Constants::ANSWER_TYPE_CONDITIONAL) { + // Check if required conditional has any answer + if ($question['isRequired'] && !$questionAnswered) { + throw new \InvalidArgumentException(sprintf('Question "%s" is required.', $question['text'])); + } + + // If answered, validate the conditional structure + if ($questionAnswered) { + $this->validateConditionalQuestion($question, $answers[$questionId], $formOwnerId, $formId); + } + continue; + } + // Check if all required questions have an answer if ($question['isRequired'] && (!$questionAnswered @@ -959,4 +973,325 @@ private function setCellValue(Worksheet $activeWorksheet, int $column, int $row, $activeWorksheet->getCell([$column, $row]) ->setValueExplicit($value, DataType::TYPE_STRING); } + + /** + * Put the subquestions of each conditional question right after it, in branch order, + * and leave out subquestions of branches that no longer exist + * + * @param Question[] $questions top-level questions and subquestions, sorted by order + * @return list + */ + private function orderQuestionsForExport(array $questions): array { + $subQuestions = []; + foreach ($questions as $question) { + if ($question->getParentQuestionId() !== null) { + $subQuestions[$question->getParentQuestionId()][(string)$question->getBranchId()][] = $question; + } + } + + $ordered = []; + foreach ($questions as $question) { + if ($question->getParentQuestionId() !== null) { + continue; + } + $ordered[] = $question; + if ($question->getType() === Constants::ANSWER_TYPE_CONDITIONAL) { + foreach ($question->getExtraSettings()['branches'] ?? [] as $branch) { + array_push($ordered, ...($subQuestions[$question->getId()][(string)($branch['id'] ?? '')] ?? [])); + } + } + } + return $ordered; + } + + /** + * Validate conditional question answers + * + * Conditional questions have a special answer structure: + * - trigger: array of trigger answer values + * - subQuestions: array of subquestion answers keyed by subquestion ID + * + * @param array $question The conditional question + * @param array $answerData The answer data for the conditional question + * @param string $formOwnerId Owner of the form + * @param int $formId Id of the form being submitted + * @throws \InvalidArgumentException if validation failed + */ + private function validateConditionalQuestion(array $question, array $answerData, string $formOwnerId, int $formId): void { + if (empty($question['extraSettings']['triggerType'])) { + throw new \InvalidArgumentException(sprintf('Conditional question "%s" is missing trigger type configuration.', $question['text'])); + } + + [$triggerAnswer, $subQuestionAnswers] = self::splitConditionalAnswer($answerData); + + // The trigger gets the same checks as a regular question of the trigger type + $triggerQuestion = self::getTriggerQuestion($question); + $this->validateSubmission([$triggerQuestion], $triggerAnswer === [] ? [] : [$triggerQuestion['id'] => $triggerAnswer], $formOwnerId, $formId); + + // Only the subquestions of the active branches may be answered, with the full per-question rules + $subQuestions = []; + foreach ($this->getActiveBranches($question, $triggerAnswer) as $branch) { + array_push($subQuestions, ...($branch['subQuestions'] ?? [])); + } + $this->validateSubmission($subQuestions, $subQuestionAnswers, $formOwnerId, $formId); + } + + /** + * Split the answer of a conditional question into the trigger answer and the subquestion answers + * + * @param array $answerData `{trigger: [...], subQuestions: {id: [...]}}`, or just the trigger answer + * @return array{0: array, 1: array} the trigger answer and the subquestion answers keyed by subquestion id + */ + public static function splitConditionalAnswer(array $answerData): array { + if (!array_key_exists('trigger', $answerData) && !array_key_exists('subQuestions', $answerData)) { + return [$answerData, []]; + } + + $triggerAnswer = $answerData['trigger'] ?? []; + $subQuestionAnswers = $answerData['subQuestions'] ?? []; + if (!is_array($triggerAnswer) || !is_array($subQuestionAnswers)) { + throw new \InvalidArgumentException('Invalid answer for conditional question.'); + } + foreach ($subQuestionAnswers as $subQuestionAnswer) { + if (!is_array($subQuestionAnswer)) { + throw new \InvalidArgumentException('Invalid answer for conditional question.'); + } + } + + return [$triggerAnswer, $subQuestionAnswers]; + } + + /** + * Get the trigger of a conditional question as a regular question of the trigger type + * + * The trigger shares id, options and the remaining extra settings with the conditional question. + * + * @param array $question The conditional question + * @return array The trigger question + */ + public static function getTriggerQuestion(array $question): array { + $extraSettings = $question['extraSettings'] ?? []; + $triggerType = $extraSettings['triggerType'] ?? ''; + unset($extraSettings['triggerType'], $extraSettings['branches']); + + return [ + ...$question, + 'type' => $triggerType, + 'extraSettings' => $extraSettings, + ]; + } + + /** + * Resolve the branch a conditional question's trigger answer activates + * + * @param array $question The conditional question + * @param array $triggerAnswer The trigger answer values + * @return array The active branches or empty array if none matches + */ + public function getActiveBranches(array $question, array $triggerAnswer): array { + $extraSettings = $question['extraSettings'] ?? []; + return $this->findActiveBranches( + $extraSettings['triggerType'] ?? '', + $triggerAnswer, + $extraSettings['branches'] ?? [], + $question['options'] ?? [], + ); + } + + /** + * Find the active branches based on trigger answer + * + * @param string $triggerType The type of the trigger question + * @param array $triggerAnswer The trigger answer values + * @param array $branches The available branches + * @param array $options The options for the trigger question + * @return array The active branches or empty array if none matches + */ + private function findActiveBranches(string $triggerType, array $triggerAnswer, array $branches, array $options): array { + return array_filter($branches, function ($branch) use ($triggerType, $triggerAnswer) { + $conditions = $branch['conditions'] ?? []; + if (empty($conditions)) { + return false; + } + return $this->evaluateBranchConditions($triggerType, $triggerAnswer, $conditions); + }); + } + + /** + * Evaluate if branch conditions match the trigger answer + * + * @param string $triggerType The type of the trigger question + * @param array $triggerAnswer The trigger answer values + * @param array $conditions The conditions to evaluate + * @return bool True if conditions match + */ + private function evaluateBranchConditions(string $triggerType, array $triggerAnswer, array $conditions): bool { + // Option ids arrive as numbers or strings depending on the trigger component + $selectedOptionIds = array_map('strval', array_filter($triggerAnswer, 'is_scalar')); + + // An unanswered trigger activates no branch, except "no file uploaded" for file triggers + if ($triggerType !== Constants::ANSWER_TYPE_FILE && ($triggerAnswer === [] || ($triggerAnswer[0] ?? '') === '')) { + return false; + } + + switch ($triggerType) { + case Constants::ANSWER_TYPE_MULTIPLEUNIQUE: + case Constants::ANSWER_TYPE_DROPDOWN: + // Single select: check if selected option matches any condition + foreach ($conditions as $condition) { + $optionId = $condition['optionId'] ?? null; + if (is_scalar($optionId) && in_array((string)$optionId, $selectedOptionIds, true)) { + return true; + } + } + return false; + case Constants::ANSWER_TYPE_MULTIPLE: + // Multi-select: the branch is active if all option ids of any of its conditions are selected + foreach ($conditions as $condition) { + $optionIds = $condition['optionIds'] ?? []; + if (empty($optionIds) || !is_array($optionIds)) { + continue; + } + if (array_diff(array_map('strval', $optionIds), $selectedOptionIds) === []) { + return true; + } + } + return false; + case Constants::ANSWER_TYPE_SHORT: + case Constants::ANSWER_TYPE_LONG: + // Text-based: evaluate regex/string conditions + $text = is_scalar($triggerAnswer[0] ?? null) ? (string)$triggerAnswer[0] : ''; + foreach ($conditions as $condition) { + $type = $condition['type'] ?? Constants::CONDITION_TYPE_STRING_CONTAINS; + $value = $condition['value'] ?? ''; + + switch ($type) { + case Constants::CONDITION_TYPE_STRING_EQUALS: + if ($text === $value) { + return true; + } + break; + case Constants::CONDITION_TYPE_STRING_CONTAINS: + if (str_contains($text, $value)) { + return true; + } + break; + case Constants::CONDITION_TYPE_REGEX: + if ($this->safeRegexMatch($value, $text)) { + return true; + } + break; + } + } + return false; + case Constants::ANSWER_TYPE_LINEARSCALE: + $numValue = (float)($triggerAnswer[0] ?? 0); + foreach ($conditions as $condition) { + $type = $condition['type'] ?? Constants::CONDITION_TYPE_VALUE_EQUALS; + if ($type === Constants::CONDITION_TYPE_VALUE_EQUALS) { + if ($numValue == (float)($condition['value'] ?? 0)) { + return true; + } + } elseif ($type === Constants::CONDITION_TYPE_VALUE_NOT_EQUALS) { + if ($numValue != (float)($condition['value'] ?? 0)) { + return true; + } + } elseif ($type === Constants::CONDITION_TYPE_VALUE_RANGE) { + $min = $condition['min'] ?? -PHP_FLOAT_MAX; + $max = $condition['max'] ?? PHP_FLOAT_MAX; + if ($numValue >= $min && $numValue <= $max) { + return true; + } + } elseif ($type === Constants::CONDITION_TYPE_VALUE_MIN) { + $min = $condition['min'] ?? -PHP_FLOAT_MAX; + if ($numValue >= $min) { + return true; + } + } elseif ($type === Constants::CONDITION_TYPE_VALUE_MAX) { + $max = $condition['max'] ?? PHP_FLOAT_MAX; + if ($numValue <= $max) { + return true; + } + } + } + return false; + case Constants::ANSWER_TYPE_COLOR: + $colorValue = is_scalar($triggerAnswer[0] ?? null) ? (string)$triggerAnswer[0] : ''; + foreach ($conditions as $condition) { + if (strcasecmp($colorValue, $condition['value'] ?? '') === 0) { + return true; + } + } + return false; + case Constants::ANSWER_TYPE_FILE: + $hasFile = !empty($triggerAnswer); + foreach ($conditions as $condition) { + if (($condition['fileUploaded'] ?? true) === $hasFile) { + return true; + } + } + return false; + case Constants::ANSWER_TYPE_DATE: + case Constants::ANSWER_TYPE_DATETIME: + case Constants::ANSWER_TYPE_TIME: + // Date range conditions + $dateValue = $triggerAnswer[0] ?? ''; + if (empty($dateValue)) { + return false; + } + $format = Constants::ANSWER_PHPDATETIME_FORMAT[$triggerType] ?? 'Y-m-d'; + $date = \DateTime::createFromFormat('!' . $format, $dateValue); + if (!$date) { + return false; + } + + foreach ($conditions as $condition) { + $min = isset($condition['min']) ? \DateTime::createFromFormat('!' . $format, $condition['min']) : null; + $max = isset($condition['max']) ? \DateTime::createFromFormat('!' . $format, $condition['max']) : null; + + $inRange = true; + if ($min && $date < $min) { + $inRange = false; + } + if ($max && $date > $max) { + $inRange = false; + } + if ($inRange) { + return true; + } + } + return false; + default: + return false; + } + } + + /** + * Safely execute a regex match with validation to prevent ReDoS attacks + * + * @param string $pattern The regex pattern to match + * @param string $subject The string to match against + * @return bool True if the pattern matches, false otherwise + */ + private function safeRegexMatch(string $pattern, string $subject): bool { + if (empty($pattern) || strlen($subject) > 10000) { + return false; + } + + // Validate regex syntax + if (@preg_match($pattern, '') === false) { + return false; + } + + // Set backtrack limit to prevent catastrophic backtracking + $previousLimit = ini_get('pcre.backtrack_limit'); + ini_set('pcre.backtrack_limit', '10000'); + + try { + $result = @preg_match($pattern, $subject); + return $result === 1; + } finally { + ini_set('pcre.backtrack_limit', $previousLimit); + } + } } diff --git a/openapi.json b/openapi.json index 4336a8c92..dee58049a 100644 --- a/openapi.json +++ b/openapi.json @@ -484,6 +484,15 @@ "items": { "type": "string" } + }, + "parentQuestionId": { + "type": "integer", + "format": "int64", + "nullable": true + }, + "branchId": { + "type": "string", + "nullable": true } } }, @@ -595,6 +604,18 @@ }, "questionType": { "type": "string" + }, + "triggerType": { + "type": "string" + }, + "branches": { + "type": "array", + "items": { + "type": "object", + "additionalProperties": { + "type": "object" + } + } } } }, @@ -618,7 +639,9 @@ "long", "file", "datetime", - "grid" + "grid", + "color", + "conditional" ] }, "Share": { @@ -1713,6 +1736,19 @@ "default": null, "description": "(optional) id of the question that should be cloned" }, + "parentQuestionId": { + "type": "integer", + "format": "int64", + "nullable": true, + "default": null, + "description": "(optional) id of the parent conditional question (for subquestions)" + }, + "branchId": { + "type": "string", + "nullable": true, + "default": null, + "description": "(optional) branch id within the parent conditional question" + }, "position": { "type": "integer", "format": "int64", @@ -2691,6 +2727,224 @@ } } }, + "/ocs/v2.php/apps/forms/api/v3/forms/{formId}/subquestions": { + "patch": { + "operationId": "api-reorder-sub-questions", + "summary": "Updates the Order of all Questions in a Branch of a conditional", + "description": "This endpoint allows CORS requests", + "tags": [ + "api" + ], + "security": [ + { + "basic_auth": [] + } + ], + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "branchId", + "parentQuestionId", + "newOrder" + ], + "properties": { + "branchId": { + "type": "string", + "description": "Id of the branch." + }, + "parentQuestionId": { + "type": "integer", + "format": "int64", + "description": "Id of the parent trigger question." + }, + "newOrder": { + "type": "array", + "description": "Array of Question-Ids in new order.", + "items": { + "type": "integer", + "format": "int64" + } + } + } + } + } + } + }, + "parameters": [ + { + "name": "formId", + "in": "path", + "description": "Id of the form to reorder", + "required": true, + "schema": { + "type": "integer", + "format": "int64" + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "the question ids of the given form in the new order", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "object", + "additionalProperties": { + "$ref": "#/components/schemas/Order" + } + } + } + } + } + } + } + } + }, + "400": { + "description": "One question has already been marked as deleted", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + }, + "403": { + "description": "User has no permissions to get this form", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + }, + "404": { + "description": "Could not find question", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + }, + "401": { + "description": "Current user is not logged in", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + } + } + } + }, "/ocs/v2.php/apps/forms/api/v3/forms/{formId}/questions/{questionId}/options": { "post": { "operationId": "api-new-option", diff --git a/playwright/e2e/conditional-question.spec.ts b/playwright/e2e/conditional-question.spec.ts new file mode 100644 index 000000000..4ea37a722 --- /dev/null +++ b/playwright/e2e/conditional-question.spec.ts @@ -0,0 +1,154 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import type { APIRequestContext } from '@playwright/test' + +import { expect, mergeTests } from '@playwright/test' +import { test as randomUserTest } from '../support/fixtures/random-user.ts' +import { test as submitTest } from '../support/fixtures/submit.ts' + +const test = mergeTests(randomUserTest, submitTest) + +/** + * Call the Forms OCS API as the logged in user + * + * @param request the request context of the page + * @param method the HTTP method + * @param path the path below `api/v3` + * @param data the JSON body + */ +async function api( + request: APIRequestContext, + method: string, + path: string, + data?: unknown, +) { + const response = await request.fetch(`../ocs/v2.php/apps/forms/api/v3${path}`, { + method, + data, + headers: { 'OCS-APIRequest': 'true', Accept: 'application/json' }, + }) + expect(response.ok()).toBeTruthy() + return (await response.json()).ocs.data +} + +test.describe('Conditional questions', () => { + let formId: number + let hash: string + + // A dropdown conditional with one subquestion per answer, followed by a regular question + test.beforeEach(async ({ page }) => { + const request = page.request + ;({ id: formId, hash } = await api(request, 'POST', '/forms')) + await api(request, 'PATCH', `/forms/${formId}`, { + keyValuePairs: { title: 'Conditional form' }, + }) + + const conditional = await api( + request, + 'POST', + `/forms/${formId}/questions`, + { + type: 'conditional', + text: 'Do you have dietary restrictions?', + }, + ) + const [yes, no] = await api( + request, + 'POST', + `/forms/${formId}/questions/${conditional.id}/options`, + { optionTexts: ['Yes', 'No'] }, + ) + await api(request, 'PATCH', `/forms/${formId}/questions/${conditional.id}`, { + keyValuePairs: { + extraSettings: { + triggerType: 'dropdown', + branches: [ + { id: 'branch-yes', conditions: [{ optionId: yes.id }] }, + { id: 'branch-no', conditions: [{ optionId: no.id }] }, + ], + }, + }, + }) + for (const [branchId, text] of [ + ['branch-yes', 'Which ones?'], + ['branch-no', 'Why not?'], + ]) { + await api(request, 'POST', `/forms/${formId}/questions`, { + type: 'short', + text, + parentQuestionId: conditional.id, + branchId, + }) + } + await api(request, 'POST', `/forms/${formId}/questions`, { + type: 'short', + text: 'Favourite word?', + }) + + await page.goto(`apps/forms/${hash}/submit`) + }) + + test('Only the subquestions of the chosen answer are shown and stored', async ({ + page, + submitView, + }) => { + await expect(page.getByText('Which ones?')).toBeHidden() + + await submitView.selectDropdown('Do you have dietary restrictions?', 'Yes') + const subQuestion = page.getByRole('listitem', { + name: 'Question number 1.1', + }) + await expect(subQuestion.getByRole('textbox')).toHaveAccessibleName( + 'Which ones?', + ) + await subQuestion.getByRole('textbox').fill('nuts') + + // The regular question keeps its own number and label + await expect( + page + .getByRole('listitem', { name: 'Question number 2' }) + .getByRole('textbox'), + ).toHaveAccessibleName('Favourite word?') + + // Changing the answer replaces the subquestions and drops the answer of the other branch + await submitView.selectDropdown('Do you have dietary restrictions?', 'No') + await expect(page.getByText('Which ones?')).toBeHidden() + await page + .getByRole('listitem', { name: 'Question number 1.1' }) + .getByRole('textbox') + .fill('no reason') + + await submitView.submit() + await expect(submitView.successMessage).toBeVisible() + + const { submissions } = await api( + page.request, + 'GET', + `/forms/${formId}/submissions`, + ) + expect(submissions[0].answers.map((answer) => answer.text).sort()).toEqual([ + 'No', + 'no reason', + ]) + }) + + test('Element ids stay unique when subquestions are shown', async ({ + page, + submitView, + }) => { + await submitView.selectDropdown('Do you have dietary restrictions?', 'Yes') + await expect(page.getByText('Which ones?')).toBeVisible() + + const duplicateIds = await page.evaluate(() => { + const counts: Record = {} + document.querySelectorAll('main [id]').forEach((element) => { + counts[element.id] = (counts[element.id] ?? 0) + 1 + }) + return Object.keys(counts).filter((id) => counts[id] > 1) + }) + expect(duplicateIds).toEqual([]) + }) +}) diff --git a/src/components/Questions/BranchConditionEditor.vue b/src/components/Questions/BranchConditionEditor.vue new file mode 100644 index 000000000..ae4027c7e --- /dev/null +++ b/src/components/Questions/BranchConditionEditor.vue @@ -0,0 +1,602 @@ + + + + + + + diff --git a/src/components/Questions/Question.vue b/src/components/Questions/Question.vue index 353fad731..1bf83c4e9 100644 --- a/src/components/Questions/Question.vue +++ b/src/components/Questions/Question.vue @@ -4,16 +4,22 @@ --> @@ -98,6 +107,7 @@ {{ t('forms', 'Technical name') }} - + {{ t('forms', 'Copy question') }} - + @@ -123,7 +136,7 @@