diff --git a/REUSE.toml b/REUSE.toml index 249413bd9..60bfea4ed 100644 --- a/REUSE.toml +++ b/REUSE.toml @@ -47,6 +47,12 @@ precedence = "aggregate" SPDX-FileCopyrightText = "2019 Nextcloud GmbH and Nextcloud contributors" SPDX-License-Identifier = "AGPL-3.0-or-later" +[[annotations]] +path = ["tests/fixtures/jump-paths.json"] +precedence = "aggregate" +SPDX-FileCopyrightText = "2026 Nextcloud GmbH and Nextcloud contributors" +SPDX-License-Identifier = "AGPL-3.0-or-later" + [[annotations]] path = ["img/*"] precedence = "aggregate" diff --git a/docs/API_v3.md b/docs/API_v3.md index 0cb4a5b2e..e2ffc2ece 100644 --- a/docs/API_v3.md +++ b/docs/API_v3.md @@ -18,6 +18,7 @@ This file contains the API-Documentation. For more information on the returned D - The OCS-Endpoint _always returns_ an object called `ocs`. This contains an object `meta` holding some meta-data, as well as an object `data` holding the actual data. In this document, the response-blocks only show the `data`, if not explicitely stated different. - OpenAPI spec: a machine-readable OpenAPI description is available at `/openapi.json` in the repository root. Prefer it as source-of-truth for exact request/response shapes. +- When the server changes data beyond what a request asked for, it reports this in the response header `X-Forms-Adjusted`, see [Adjustments by the server](#adjustments-by-the-server). ``` "ocs": { @@ -30,6 +31,18 @@ This file contains the API-Documentation. For more information on the returned D } ``` +### Adjustments by the server + +Some requests succeed with their usual status, and their payload keeps its shape, but the server changes data beyond what the request asked for. The response header `X-Forms-Adjusted` then reports this as a [structured field dictionary (RFC 8941)](https://www.rfc-editor.org/rfc/rfc8941#name-dictionaries); the header is absent when nothing was adjusted. `Access-Control-Expose-Headers` makes it readable for cross-origin clients. Clients should ignore keys they do not know, as more keys may follow. + +| Key | Value | Set by | Meaning | +|-----|-------|--------|---------| +| `removed-jumps` | Inner list of question ids, e.g. `(3 7)` | Updating, reordering and copying questions; deleting options | Jumps to the question itself or to an earlier question, or jumps of the deleted option, were removed from the `extraSettings` of these questions, see [Jumps](DataStructure.md#jumps) | + +``` +X-Forms-Adjusted: removed-jumps=(3 7) +``` + ## API changes ### Deprecation info @@ -49,6 +62,7 @@ This file contains the API-Documentation. For more information on the returned D ### Other API changes +- Questions can jump to a later question or to the end of the form with the extra settings `goTo` (per answer, on dropdown and radio questions) or `next` (after the question, on the other types with jumps), never both (see [Jumps](DataStructure.md#jumps)). Jumps to questions of other forms or to options that are not options of the question are rejected, and deleting an option removes its jumps. Submissions and submission updates ignore answers to the questions the jumps skip, and an answer "Other" needs a text. Updating, reordering and copying questions removes jumps to the question itself or to an earlier question; the response header `X-Forms-Adjusted` then lists the questions concerned under `removed-jumps` (see [Adjustments by the server](#adjustments-by-the-server)). Copying or importing a form drops such jumps as well, without the header, as these create a new form. - In API version 3.0 the following endpoints were introduced/changed: - `GET /api/v3/forms/{formId}/questions` to get all questions of a form - `GET /api/v3/forms/{formId}/questions/{questionId}` to get a single question @@ -260,7 +274,7 @@ Creates a clone of a form (without submissions). | Parameter | Type | Description | |-----------|---------|-------------| | _formId_ | Integer | ID of the form to clone | -- Response: Returns the full object of the new form. See [Request full data of a Form](#request-full-data-of-a-form) +- Response: Returns the full object of the new form. See [Request full data of a Form](#request-full-data-of-a-form). The jumps of the copied questions point at the copied questions and options; jumps whose target or option was not copied, jumps that would go to the question itself or to an earlier question, and `goTo` or `next` on a question type without it, are dropped, without the header `X-Forms-Adjusted` (see [Jumps](DataStructure.md#jumps)). ``` See section 'Request full data of a form'. @@ -452,7 +466,7 @@ Update a single or multiple properties of a question-object. |-----------|---------|-------------| | _keyValuePairs_ | Array | Array of key-value pairs to update | - Restrictions: It is **not allowed** to update one of the following key-value pairs: _id, formId, order, type_. -- Response: **Status-Code OK**, as well as the id of the updated question. +- Response: **Status-Code OK**, as well as the id of the updated question. Jumps in `extraSettings` that go to the question itself or to an earlier question are removed before saving (see [Jumps](DataStructure.md#jumps)), and the response header `X-Forms-Adjusted` then holds `removed-jumps=()` (see [Adjustments by the server](#adjustments-by-the-server)). Jumps with an invalid structure, `goTo` or `next` on a question type without it, and ids of questions of other forms or of options that are not options of the question are rejected with **Status-Code Bad Request** "Invalid extraSettings, will not update." ``` "data": 1 @@ -473,7 +487,7 @@ Reorders all Questions of a single form |-----------|---------|-------------| | _newOrder_ | Array | Array of **all** Question-IDs, ordered in the desired order | - Restrictions: The Array **must** contain all Question-IDs corresponding to the specified form and **must not** contain any duplicates. -- Response: Array of questionIDs and their corresponding order. +- Response: Array of questionIDs and their corresponding order. Jumps that would go to the question itself or to an earlier question are removed (see [Jumps](DataStructure.md#jumps)): the questions concerned also carry their new `extraSettings`, and the response header `X-Forms-Adjusted` then lists their ids, e.g. `X-Forms-Adjusted: removed-jumps=(3 7)` (see [Adjustments by the server](#adjustments-by-the-server)). ``` "data": { @@ -519,7 +533,7 @@ Creates a clone of a question with all its options. | Parameter | Type | Optional | Description | |-----------|---------|----------|-------------| | _position_ | Integer | yes | _(optional)_ 1-based position to insert the cloned question. When omitted the clone is appended at the end. | -- Response: Returns cloned question object with the new ID set. +- Response: Returns cloned question object with the new ID set. Jumps that would go to the clone itself or to an earlier question, for example when it is appended at the end, are removed from its `extraSettings` (see [Jumps](DataStructure.md#jumps)), and the response header `X-Forms-Adjusted` then holds `removed-jumps=()` (see [Adjustments by the server](#adjustments-by-the-server)). ``` See section 'Create a new question'. @@ -590,7 +604,7 @@ Update a single or all properties of an option-object | _formId_ | Integer | ID of the form containing the question and option | | _questionId_ | Integer | ID of the question, the new option will belong to | | _optionId_ | Integer | ID of the option to delete | -- Response: **Status-Code OK**, as well as the id of the deleted option. +- Response: **Status-Code OK**, as well as the id of the deleted option. The jumps of the question for this option are removed (see [Jumps](DataStructure.md#jumps)), and the response header `X-Forms-Adjusted` then holds `removed-jumps=()` (see [Adjustments by the server](#adjustments-by-the-server)). ``` "data": 7 @@ -951,6 +965,8 @@ Store Submission to Database - An **array** of values as value --> Even for short Text Answers, wrapped into Array. - For Question-Types with pre-defined answers (`multiple`, `multiple_unique`, `dropdown`), the array contains the corresponding option-IDs. - For File-Uploads, the array contains objects with `uploadedFileId` and `uploadToken` (both from the [Upload a file] endpoint for the same `questionId`). + - Answers to questions that the [jumps](DataStructure.md#jumps) of the answers skip are ignored, and a skipped required question needs no answer. + - An answer "Other" is `system-other-answer:` followed by its text, and the text must not be empty. ``` { diff --git a/docs/DataStructure.md b/docs/DataStructure.md index d0e9acfa3..f9aea0f99 100644 --- a/docs/DataStructure.md +++ b/docs/DataStructure.md @@ -270,6 +270,25 @@ Optional extra settings for some [Question Types](#question-types) | `columns` | `grid` | Array | - | Array of column identifiers / labels for grid questions | | `rows` | `grid` | Array | - | Array of row identifiers / labels for grid questions | | `questionType` | `grid` | String | `checkbox`, `number`, `radio` | Type of cell for grid questions (checkbox, numeric input, or radio) | +| `goTo` | `dropdown, multiple_unique` | Array | see [Jumps](#jumps) | Where to go next for an answer | +| `next` | `color, file, grid, long, ranking, short` | Integer / string | question id, `'submit'` | Where to go after the question, see [Jumps](#jumps) | + +### Jumps + +An answer can lead to a later question or to the end of the form, as in Microsoft and Google Forms. The form stays a flat list of questions. + +- `goTo` is a list of jumps per answer: `{"optionId": 12, "target": 34}` for an option, `{"other": true, "target": "submit"}` for the answer "Other". There is at most one jump per answer. +- `next` is one target after the whole question. +- A question has either jumps per answer or one jump after the question, depending on its type: `goTo` only on `dropdown` and `multiple_unique`, `next` only on the other types listed above. Sending `next` on a question type with `goTo`, or `goTo` on a question type with `next`, is rejected with 400 "Invalid extraSettings". An answer without a `goTo` entry leads to the following question. +- A target is the id of a later question of the same form or `"submit"` for the end of the form, and `optionId` is the id of an option of the question. Ids of questions of other forms and option ids that are not options of the question are rejected as invalid `extraSettings`; ids of deleted questions of the form are accepted. Without a jump the respondent goes to the following question. +- Jumps never go back: the server removes jumps to the question itself or to an earlier question when a question is updated, reordered or copied, and when a form is copied or imported; copying or importing a form also drops `goTo` or `next` on question types without it. Updating, reordering and copying a question still succeed with their usual status, and the payload keeps its shape: a reorder adds the new `extraSettings` of the questions concerned, and a copied question is returned with its reduced `extraSettings`. The response header `X-Forms-Adjusted` lists the questions that lost jumps, e.g. `removed-jumps=(3 7)`, see [Adjustments by the server](API_v3.md#adjustments-by-the-server). +- Deleting an option removes the jumps of the question for that option; the response header `X-Forms-Adjusted` then holds `removed-jumps` with the id of the question. +- Saving a question rejects jumps with an invalid structure with 400, as any invalid `extraSettings`. Copying or importing a form keeps the jumps whose target and option it can map, and drops the others and jumps per answer with an invalid structure. The path walks ignore malformed entries. +- Question types in neither list, such as `datetime`, accept neither `goTo` nor `next` (400). +- A jump of the answer "Other" only matches while `allowOtherAnswer` is on. +- `next` also applies when the question is left unanswered. + +The respondent's path starts at the first question and follows these targets. The first value of an answer decides, and answers to questions the submit view does not show, such as questions without a title, do not count. A target that is not a later question of the form, for example after the target was deleted, counts as the following question, and a jump of an option that no longer exists never matches. Questions off the path are skipped: answers to them are ignored, and their required flag does not apply. ### Option Types diff --git a/lib/Constants.php b/lib/Constants.php index 3e465cd0c..e896a9969 100644 --- a/lib/Constants.php +++ b/lib/Constants.php @@ -233,6 +233,22 @@ class Constants { 'shuffleOptions' => ['boolean'], ]; + public const JUMP_TARGET_SUBMIT = 'submit'; + + public const QUESTION_TYPES_WITH_GOTO = [ + self::ANSWER_TYPE_DROPDOWN, + self::ANSWER_TYPE_MULTIPLEUNIQUE, + ]; + + public const QUESTION_TYPES_WITH_NEXT = [ + self::ANSWER_TYPE_COLOR, + self::ANSWER_TYPE_FILE, + self::ANSWER_TYPE_GRID, + self::ANSWER_TYPE_LONG, + self::ANSWER_TYPE_RANKING, + self::ANSWER_TYPE_SHORT, + ]; + public const EXTRA_SETTINGS_GRID_QUESTION_TYPE = [ self::ANSWER_GRID_TYPE_CHECKBOX, self::ANSWER_GRID_TYPE_NUMBER, diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index 74a7c1011..9def9bdfa 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -64,6 +64,7 @@ * @psalm-import-type FormsOrder from ResponseDefinitions * @psalm-import-type FormsPartialForm from ResponseDefinitions * @psalm-import-type FormsQuestion from ResponseDefinitions + * @psalm-import-type FormsQuestionOrder from ResponseDefinitions * @psalm-import-type FormsQuestionType from ResponseDefinitions * @psalm-import-type FormsQuestionGridCellType from ResponseDefinitions * @psalm-import-type FormsSubmission from ResponseDefinitions @@ -222,6 +223,9 @@ public function newForm(?int $fromId = null): DataResponse { // Get Questions, set new formId, reinsert $questions = $this->questionMapper->findByForm($oldForm->getId()); $oldConfirmationEmailQuestionId = $oldForm->getConfirmationEmailQuestionId(); + $questionIdMap = []; + $optionIdMap = []; + $newQuestions = []; foreach ($questions as $oldQuestion) { $questionData = $oldQuestion->read(); @@ -230,6 +234,8 @@ public function newForm(?int $fromId = null): DataResponse { $questionData['formId'] = $form->getId(); $newQuestion = Question::fromParams($questionData); $this->questionMapper->insert($newQuestion); + $questionIdMap[$oldQuestion->getId()] = $newQuestion->getId(); + $newQuestions[] = $newQuestion; if (isset($oldConfirmationEmailQuestionId) && $oldConfirmationEmailQuestionId === $oldQuestion->getId()) { $form->setConfirmationEmailQuestionId($newQuestion->getId()); @@ -244,9 +250,14 @@ public function newForm(?int $fromId = null): DataResponse { $optionData['questionId'] = $newQuestion->getId(); $newOption = Option::fromParams($optionData); $this->optionMapper->insert($newOption); + $optionIdMap[$oldOption->getId()] = $newOption->getId(); } } + foreach (FormsService::mapJumpsOfCopies($newQuestions, $questionIdMap, $optionIdMap) as $newQuestion) { + $this->questionMapper->update($newQuestion); + } + $this->formMapper->update($form); $this->auditLogger->log(Operation::FormCreated, 'Form %s was created by "%s" as a copy of form %s', [ 'formId' => $form->getId(), @@ -525,13 +536,15 @@ public function getQuestion(int $formId, int $questionId): DataResponse { /** * Add a new question * + * A clone loses its jumps to itself or to an earlier question, and the header `X-Forms-Adjusted` then holds `removed-jumps` with its id + * * @param int $formId the form id * @param FormsQuestionType $type the new question type * @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 $position (optional) the position of the new question - * @return DataResponse + * @return DataResponse * @throws OCSBadRequestException Invalid type * @throws OCSBadRequestException Datetime question type no longer supported * @throws OCSForbiddenException User has no permissions to get this form @@ -554,6 +567,7 @@ public function newQuestion(int $formId, ?string $type = null, ?string $subtype throw new OCSForbiddenException('This form is archived and can not be modified'); } + $headers = []; if ($fromId === null) { $this->logger->debug('Adding new question: formId: {formId}, type: {type}, text: {text}', [ 'formId' => $formId, @@ -644,6 +658,7 @@ public function newQuestion(int $formId, ?string $type = null, ?string $subtype $response = $newQuestion->read(); $response['options'] = []; $response['accept'] = []; + $optionIdMap = []; foreach ($sourceOptions as $sourceOption) { $optionData = $sourceOption->read(); @@ -652,9 +667,23 @@ public function newQuestion(int $formId, ?string $type = null, ?string $subtype $optionData['questionId'] = $newQuestion->getId(); $newOption = Option::fromParams($optionData); $insertedOption = $this->optionMapper->insert($newOption); + $optionIdMap[$sourceOption->getId()] = $insertedOption->getId(); $response['options'][] = $insertedOption->read(); } + + $extraSettings = $newQuestion->getExtraSettings(); + if (array_key_exists('goTo', $extraSettings) || array_key_exists('next', $extraSettings)) { + $newQuestion->setExtraSettings(FormsService::mapJumps($extraSettings, null, $optionIdMap)); + $formQuestions = array_map(static fn (Question $formQuestion): Question => $formQuestion->getId() === $newQuestion->getId() ? $newQuestion : $formQuestion, $this->questionMapper->findByForm($formId)); + if (in_array($newQuestion, FormsService::removeBackwardJumps($formQuestions), true)) { + $headers = self::getRemovedJumpsHeaders([$newQuestion->getId()]); + } + if ($newQuestion->getExtraSettings() !== $extraSettings) { + $this->questionMapper->update($newQuestion); + $response['extraSettings'] = $newQuestion->getExtraSettings(); + } + } } $this->formMapper->update($form); @@ -663,17 +692,20 @@ public function newQuestion(int $formId, ?string $type = null, ?string $subtype $response['extraSettings'] = new \stdClass(); } - return new DataResponse($response, Http::STATUS_CREATED); + return new DataResponse($response, Http::STATUS_CREATED, $headers); } /** * Writes the given key-value pairs into Database * Key `order` should only be changed by reorderQuestions() and is not allowed here * + * Jumps to the question itself or to an earlier question are removed before saving, and the header `X-Forms-Adjusted` then holds `removed-jumps` with the id of the question. + * Jumps to questions of other forms or to options that are not options of the question are rejected as invalid `extraSettings`. + * * @param int $formId the form id * @param int $questionId id of question to update * @param array $keyValuePairs Array of key=>value pairs to update. - * @return DataResponse + * @return DataResponse * @throws OCSBadRequestException Question doesn\'t belong to given Form * @throws OCSBadRequestException Invalid extraSettings, will not update. * @throws OCSForbiddenException Empty keyValuePairs, will not update @@ -740,6 +772,20 @@ public function updateQuestion(int $formId, int $questionId, array $keyValuePair throw new OCSBadRequestException('Invalid extraSettings, will not update.'); } + $removedJumps = false; + if (is_array($keyValuePairs['extraSettings'] ?? null) + && (array_key_exists('goTo', $keyValuePairs['extraSettings']) || array_key_exists('next', $keyValuePairs['extraSettings']))) { + $allQuestions = $this->questionMapper->findByForm($formId, true); + if (!$this->formsService->areJumpsOfForm($keyValuePairs['extraSettings'], $questionId, $allQuestions)) { + throw new OCSBadRequestException('Invalid extraSettings, will not update.'); + } + $updatedQuestion = clone $question; + $updatedQuestion->setExtraSettings($keyValuePairs['extraSettings']); + $formQuestions = array_values(array_map(static fn (Question $formQuestion): Question => $formQuestion->getId() === $questionId ? $updatedQuestion : $formQuestion, array_filter($allQuestions, static fn (Question $formQuestion): bool => $formQuestion->getOrder() !== 0))); + $removedJumps = in_array($updatedQuestion, FormsService::removeBackwardJumps($formQuestions), true); + $keyValuePairs['extraSettings'] = $updatedQuestion->getExtraSettings(); + } + if ($form->getConfirmationEmailQuestionId() === $question->getId() && !$question->isEmailType($keyValuePairs['type'] ?? null, $keyValuePairs['extraSettings'] ?? null)) { $form->setConfirmationEmailQuestionId(null); @@ -753,7 +799,7 @@ public function updateQuestion(int $formId, int $questionId, array $keyValuePair $this->questionMapper->update($question); $this->formMapper->update($form); - return new DataResponse($question->getId()); + return new DataResponse($question->getId(), Http::STATUS_OK, $removedJumps ? self::getRemovedJumpsHeaders([$questionId]) : []); } /** @@ -839,9 +885,11 @@ public function deleteQuestion(int $formId, int $questionId): DataResponse { /** * Updates the Order of all Questions of a Form * + * Jumps that would go to the question itself or to an earlier question are removed: the new extra settings of these questions are returned, and the header `X-Forms-Adjusted` holds `removed-jumps` with their ids + * * @param int $formId Id of the form to reorder * @param list $newOrder Array of Question-Ids in new order. - * @return DataResponse, array{}> + * @return DataResponse, array{X-Forms-Adjusted?: string, Access-Control-Expose-Headers?: string}> * @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 @@ -851,7 +899,7 @@ public function deleteQuestion(int $formId, int $questionId): DataResponse { * @throws OCSNotFoundException Could not find form * @throws OCSNotFoundException Could not find question * - * 200: the question ids of the given form in the new order + * 200: the question ids of the given form in the new order, with the new extra settings of the questions whose jumps were removed */ #[CORS()] #[NoAdminRequired()] @@ -922,6 +970,8 @@ public function reorderQuestions(int $formId, array $newOrder): DataResponse { } } + $changedQuestions = FormsService::removeBackwardJumps($questions); + // Write to Database foreach ($questions as $question) { $this->questionMapper->update($question); @@ -929,11 +979,14 @@ public function reorderQuestions(int $formId, array $newOrder): DataResponse { $response[(string)$question->getId()] = [ 'order' => $question->getOrder() ]; + if (in_array($question, $changedQuestions, true)) { + $response[(string)$question->getId()]['extraSettings'] = $question->getExtraSettings() ?: new \stdClass(); + } } $this->formMapper->update($form); - return new DataResponse($response); + return new DataResponse($response, Http::STATUS_OK, self::getRemovedJumpsHeaders(array_map(static fn (Question $question): int => $question->getId(), $changedQuestions))); } // Options @@ -1099,10 +1152,12 @@ public function updateOption(int $formId, int $questionId, int $optionId, array /** * Delete an option * + * The jumps of the question for this option are removed as well, and the header `X-Forms-Adjusted` then holds `removed-jumps` with the id of the question. + * * @param int $formId id of form * @param int $questionId id of question * @param int $optionId id of option to update - * @return DataResponse Returns the id of the deleted option + * @return DataResponse Returns the id of the deleted option * @throws OCSBadRequestException The given option id doesn't match the question or form * @throws OCSForbiddenException This form is archived and can not be modified * @throws OCSForbiddenException Current user has no permission to edit @@ -1143,6 +1198,18 @@ public function deleteOption(int $formId, int $questionId, int $optionId): DataR $this->optionMapper->delete($option); + $headers = []; + $extraSettings = $question->getExtraSettings(); + if (is_array($extraSettings['goTo'] ?? null)) { + $goTo = array_values(array_filter($extraSettings['goTo'], static fn (mixed $jump): bool => !is_array($jump) || ($jump['optionId'] ?? null) !== $optionId)); + if (count($goTo) !== count($extraSettings['goTo'])) { + $extraSettings['goTo'] = $goTo; + $question->setExtraSettings($extraSettings); + $this->questionMapper->update($question); + $headers = self::getRemovedJumpsHeaders([$questionId]); + } + } + // Reorder the remaining options $options = array_values($this->optionMapper->findByQuestion($questionId)); foreach ($options as $order => $option) { @@ -1153,7 +1220,7 @@ public function deleteOption(int $formId, int $questionId, int $optionId): DataR $this->formMapper->update($form); - return new DataResponse($optionId); + return new DataResponse($optionId, Http::STATUS_OK, $headers); } /** @@ -1482,6 +1549,7 @@ public function newSubmission(int $formId, array $answers, string $shareHash = ' } catch (\InvalidArgumentException $e) { throw new OCSBadRequestException($e->getMessage()); } + $answers = SubmissionService::getAnswersOnPath($questions, $answers); // Create Submission $submission = new Submission(); @@ -1577,6 +1645,7 @@ public function updateSubmission(int $formId, int $submissionId, array $answers) } catch (\InvalidArgumentException $e) { throw new OCSBadRequestException($e->getMessage()); } + $answers = SubmissionService::getAnswersOnPath($questions, $answers); try { $submission = $this->submissionMapper->findById($submissionId); @@ -1872,6 +1941,23 @@ public function uploadFiles(int $formId, int $questionId, string $shareHash = '' // private functions + /** + * The response headers reporting what the server changed beyond the request, readable by cross-origin clients as well: + * `X-Forms-Adjusted` is a structured field dictionary (RFC 8941), here with `removed-jumps` listing the questions whose jumps were removed + * + * @param list $questionIds the ids of the questions whose jumps were removed + * @return array{X-Forms-Adjusted?: string, Access-Control-Expose-Headers?: string} no headers when nothing was adjusted + */ + private static function getRemovedJumpsHeaders(array $questionIds): array { + if ($questionIds === []) { + return []; + } + return [ + 'X-Forms-Adjusted' => 'removed-jumps=(' . implode(' ', $questionIds) . ')', + 'Access-Control-Expose-Headers' => 'X-Forms-Adjusted', + ]; + } + /** * Insert answers for a question * @@ -1910,7 +1996,7 @@ private function storeAnswersForQuestion(Form $form, $submissionId, array $quest $optionIndex = array_search($answer, array_column($question['options'], 'id')); if ($optionIndex !== false) { $answerText = $question['options'][$optionIndex]['text']; - } elseif (!empty($question['extraSettings']['allowOtherAnswer']) && str_starts_with($answer, Constants::QUESTION_EXTRASETTINGS_OTHER_PREFIX)) { + } elseif (SubmissionService::isOtherAnswer($question, $answer)) { $answerText = str_replace(Constants::QUESTION_EXTRASETTINGS_OTHER_PREFIX, '', $answer); } } elseif ($question['type'] === Constants::ANSWER_TYPE_FILE) { diff --git a/lib/FormsMigrator.php b/lib/FormsMigrator.php index 303d42a1d..08bf87aa7 100644 --- a/lib/FormsMigrator.php +++ b/lib/FormsMigrator.php @@ -77,7 +77,6 @@ public function export(IUser $user, IExportDestination $exportDestination, Outpu // Do NOT unset ID of question here, as it is necessary for answers. unset($formData['questions'][$qKey]['formId']); foreach ($question['options'] as $oKey => $option) { - unset($formData['questions'][$qKey]['options'][$oKey]['id']); unset($formData['questions'][$qKey]['options'][$oKey]['questionId']); } } @@ -164,6 +163,8 @@ public function import(IUser $user, IImportSource $importSource, OutputInterface $this->formMapper->insert($form); $questionIdMap = []; + $optionIdMap = []; + $questions = []; foreach ($formData['questions'] as $questionData) { $question = new Question(); $question->setFormId($form->getId()); @@ -178,6 +179,7 @@ public function import(IUser $user, IImportSource $importSource, OutputInterface // Store QuestionId to map Answers. $questionIdMap[$questionData['id']] = $question->getId(); + $questions[] = $question; foreach ($questionData['options'] as $optionData) { $option = new Option(); @@ -185,9 +187,16 @@ public function import(IUser $user, IImportSource $importSource, OutputInterface $option->setText($optionData['text']); $this->optionMapper->insert($option); + if (isset($optionData['id'])) { + $optionIdMap[$optionData['id']] = $option->getId(); + } } } + foreach (FormsService::mapJumpsOfCopies($questions, $questionIdMap, $optionIdMap) as $question) { + $this->questionMapper->update($question); + } + if (($formData['confirmationEmailQuestionId'] ?? null) !== null && isset($questionIdMap[$formData['confirmationEmailQuestionId']])) { $form->setConfirmationEmailQuestionId($questionIdMap[$formData['confirmationEmailQuestionId']]); diff --git a/lib/ResponseDefinitions.php b/lib/ResponseDefinitions.php index 15ed97324..74950b4bd 100644 --- a/lib/ResponseDefinitions.php +++ b/lib/ResponseDefinitions.php @@ -27,8 +27,10 @@ * dateMax?: string|int, * dateMin?: string|int, * dateRange?: bool, + * goTo?: list, * maxAllowedFilesCount?: int, * maxFileSize?: int, + * next?: int|string, * optionsHighest?: 2|3|4|5|6|7|8|9|10, * optionsLabelHighest?: string, * optionsLabelLowest?: string, @@ -44,6 +46,11 @@ * questionType?: string, * } * + * @psalm-type FormsQuestionOrder = array{ + * order: int, + * extraSettings?: FormsQuestionExtraSettings|\stdClass, + * } + * * @psalm-type FormsQuestionType = "dropdown"|"multiple"|"multiple_unique"|"date"|"time"|"short"|"long"|"file"|"datetime"|"grid" * @psalm-type FormsQuestionGridCellType = "checkbox"|"number"|"radio" * diff --git a/lib/Service/FormsService.php b/lib/Service/FormsService.php index 197ababbc..a7671db16 100644 --- a/lib/Service/FormsService.php +++ b/lib/Service/FormsService.php @@ -913,6 +913,12 @@ public function areExtraSettingsValid(array $extraSettings, string $questionType Constants::ANSWER_TYPE_LINEARSCALE => Constants::EXTRA_SETTINGS_LINEARSCALE, default => [], }; + if (in_array($questionType, Constants::QUESTION_TYPES_WITH_NEXT, true)) { + $allowed['next'] = ['integer', 'string']; + } + if (in_array($questionType, Constants::QUESTION_TYPES_WITH_GOTO, true)) { + $allowed['goTo'] = ['array']; + } // 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) { @@ -927,6 +933,13 @@ public function areExtraSettingsValid(array $extraSettings, string $questionType } } + if (array_key_exists('next', $extraSettings) && !self::isJumpTarget($extraSettings['next'])) { + return false; + } + if (array_key_exists('goTo', $extraSettings) && !self::isGoToValid($extraSettings['goTo'], $questionType)) { + return false; + } + // Validate extraSettings for specific question types if ($questionType === Constants::ANSWER_TYPE_DATE) { $format = Constants::ANSWER_PHPDATETIME_FORMAT['date']; @@ -1055,6 +1068,199 @@ public function areExtraSettingsValid(array $extraSettings, string $questionType return true; } + /** + * Whether the value is a jump target: the id of a question or the end of the form + * + * @param mixed $target the value + */ + private static function isJumpTarget(mixed $target): bool { + return (is_int($target) && $target > 0) || $target === Constants::JUMP_TARGET_SUBMIT; + } + + /** + * Whether the value is a list of jumps per answer, each with one answer key and a target, at most one per answer + * + * @param mixed $goTo the value of `goTo` + * @param string $questionType the question type + */ + private static function isGoToValid(mixed $goTo, string $questionType): bool { + if (!is_array($goTo) || !array_is_list($goTo)) { + return false; + } + + $answers = []; + foreach ($goTo as $jump) { + if (!is_array($jump) || count($jump) !== 2 || !self::isJumpTarget($jump['target'] ?? null)) { + return false; + } + + if (is_int($jump['optionId'] ?? null) && $jump['optionId'] > 0) { + $answer = 'option:' . $jump['optionId']; + } elseif (($jump['other'] ?? null) === true) { + $answer = 'other'; + } else { + return false; + } + + if (isset($answers[$answer])) { + return false; + } + $answers[$answer] = true; + } + + return true; + } + + /** + * Whether the jumps only name questions of the form and options of the question. + * Targets are ids of questions of the form, deleted ones included; option ids are ids of options of the question. + * + * @param array $extraSettings the extra settings of the question, with a valid structure + * @param int $questionId the id of the question + * @param list $formQuestions the questions of the form, deleted ones included + */ + public function areJumpsOfForm(array $extraSettings, int $questionId, array $formQuestions): bool { + $goTo = is_array($extraSettings['goTo'] ?? null) ? $extraSettings['goTo'] : []; + $questionIds = array_map(static fn (Question $question): int => $question->getId(), $formQuestions); + foreach ([$extraSettings['next'] ?? null, ...array_column($goTo, 'target')] as $target) { + if (is_int($target) && !in_array($target, $questionIds, true)) { + return false; + } + } + + $jumpOptionIds = array_column($goTo, 'optionId'); + if ($jumpOptionIds === []) { + return true; + } + $optionIds = array_map(static fn (Option $option): int => $option->getId(), $this->optionMapper->findByQuestion($questionId)); + return array_diff($jumpOptionIds, $optionIds) === []; + } + + /** + * The extra settings of a copied question, with its jumps pointing at the copied questions and options. + * Jumps whose target or option has no copy are removed. + * + * @param array $extraSettings the extra settings of the original question + * @param ?array $questionIdMap the ids of the copied questions by original id, `null` keeps the targets + * @param array $optionIdMap the ids of the copied options by original id + * @return array the extra settings of the copy + */ + public static function mapJumps(array $extraSettings, ?array $questionIdMap, array $optionIdMap): array { + $mapTarget = static function (mixed $target) use ($questionIdMap): string|int|null { + if (!is_int($target)) { + return $target === Constants::JUMP_TARGET_SUBMIT ? $target : null; + } + return $questionIdMap === null ? $target : ($questionIdMap[$target] ?? null); + }; + + if (array_key_exists('next', $extraSettings)) { + $next = $mapTarget($extraSettings['next']); + if ($next === null) { + unset($extraSettings['next']); + } else { + $extraSettings['next'] = $next; + } + } + + if (array_key_exists('goTo', $extraSettings)) { + $goTo = []; + foreach (is_array($extraSettings['goTo']) ? $extraSettings['goTo'] : [] as $jump) { + if (!is_array($jump)) { + continue; + } + $jump['target'] = $mapTarget($jump['target'] ?? null); + if (array_key_exists('optionId', $jump)) { + $jump['optionId'] = is_int($jump['optionId']) ? ($optionIdMap[$jump['optionId']] ?? null) : null; + if ($jump['optionId'] === null) { + continue; + } + } + if ($jump['target'] !== null) { + $goTo[] = $jump; + } + } + $extraSettings['goTo'] = $goTo; + } + + return $extraSettings; + } + + /** + * Point the jumps of copied questions at the copied questions and options, and remove the jumps that go to the question itself or to an earlier question, jumps per answer with an invalid structure or for an answer with a jump already, and `goTo` or `next` on question types without it + * + * @param list $questions the copied questions + * @param array $questionIdMap the ids of the copied questions by original id + * @param array $optionIdMap the ids of the copied options by original id + * @return list the copied questions with jumps, with their new extra settings set + */ + public static function mapJumpsOfCopies(array $questions, array $questionIdMap, array $optionIdMap): array { + $copiesWithJumps = []; + foreach ($questions as $question) { + $extraSettings = $question->getExtraSettings(); + if (array_key_exists('goTo', $extraSettings) || array_key_exists('next', $extraSettings)) { + if (!in_array($question->getType(), Constants::QUESTION_TYPES_WITH_GOTO, true)) { + unset($extraSettings['goTo']); + } + if (!in_array($question->getType(), Constants::QUESTION_TYPES_WITH_NEXT, true)) { + unset($extraSettings['next']); + } + $extraSettings = self::mapJumps($extraSettings, $questionIdMap, $optionIdMap); + if (array_key_exists('goTo', $extraSettings)) { + $goTo = []; + foreach ($extraSettings['goTo'] as $jump) { + if (self::isGoToValid([...$goTo, $jump], $question->getType())) { + $goTo[] = $jump; + } + } + $extraSettings['goTo'] = $goTo; + } + $question->setExtraSettings($extraSettings); + $copiesWithJumps[] = $question; + } + } + + if ($copiesWithJumps !== []) { + usort($questions, static fn (Question $a, Question $b): int => $a->getOrder() <=> $b->getOrder()); + self::removeBackwardJumps($questions); + } + + return $copiesWithJumps; + } + + /** + * Remove the jumps that go to the question itself or to an earlier question, keeping the jumps to the end of the form, to later questions and to questions that are not in the list + * + * @param list $questions the questions of the form in their order + * @return list the questions whose jumps were removed, with their new extra settings set + */ + public static function removeBackwardJumps(array $questions): array { + $positions = array_flip(array_map(static fn (Question $question): int => $question->getId(), $questions)); + $changed = []; + + foreach ($questions as $position => $question) { + $isBackward = static fn (mixed $target): bool => is_int($target) && isset($positions[$target]) && $positions[$target] <= $position; + $extraSettings = $question->getExtraSettings(); + $newSettings = $extraSettings; + + if ($isBackward($extraSettings['next'] ?? null)) { + unset($newSettings['next']); + } + if (is_array($extraSettings['goTo'] ?? null)) { + $goTo = array_values(array_filter($extraSettings['goTo'], static fn (mixed $jump): bool => !$isBackward(is_array($jump) ? ($jump['target'] ?? null) : null))); + if (count($goTo) !== count($extraSettings['goTo'])) { + $newSettings['goTo'] = $goTo; + } + } + + if ($newSettings !== $extraSettings) { + $question->setExtraSettings($newSettings); + $changed[] = $question; + } + } + + return $changed; + } + /** * Get list of forms * diff --git a/lib/Service/SubmissionService.php b/lib/Service/SubmissionService.php index dab992070..f82387d46 100644 --- a/lib/Service/SubmissionService.php +++ b/lib/Service/SubmissionService.php @@ -612,9 +612,29 @@ private static function isShownToRespondents(array $question): bool { }; } + /** + * Whether the values of an answer hold anything: a non-empty text, an uploaded file with its token, or a grid row with a value + * + * @param array $values the values of the answer + */ + private static function hasValues(array $values): bool { + return array_filter($values, static function (mixed $value): bool { + if (is_array($value)) { + if (isset($value['uploadedFileId'])) { + return !empty($value['uploadedFileId']) && !empty($value['uploadToken']); + } + + return !empty(array_filter($value, static fn ($subValue): bool => is_array($subValue) ? !empty(array_filter($subValue)) : $subValue !== '')); + } + + return is_scalar($value) && (string)$value !== ''; + }) !== []; + } + /** * Validate all answers against the questions * Required questions the submit view does not show to respondents need no answer + * Only the questions on the path of the respondent are checked, answers to skipped questions are ignored * * @param array $questions Array of the questions of the form * @param array $answers Array of the submitted answers @@ -623,28 +643,18 @@ private static function isShownToRespondents(array $question): bool { * @throw \InvalidArgumentException if validation failed */ public function validateSubmission(array $questions, array $answers, string $formOwnerId, int $formId): void { + $visibleQuestions = self::getQuestionsOnPath($questions, $answers); + // Check by questions - foreach ($questions as $question) { + foreach ($visibleQuestions as $question) { $questionId = $question['id']; $questionAnswered = array_key_exists($questionId, $answers); // Check if all required questions have an answer - if ($question['isRequired'] + if (!empty($question['isRequired']) && self::isShownToRespondents($question) && (!$questionAnswered - || !array_filter($answers[$questionId], static function (string|array $value): bool { - if (is_array($value)) { - // file type - if (isset($value['uploadedFileId'])) { - return !empty($value['uploadedFileId']) && !empty($value['uploadToken']); - } - - // Grid questions - return !empty(array_filter($value, static fn ($subValue): bool => is_array($subValue) ? !empty(array_filter($subValue)) : $subValue !== '')); - } - - return $value !== ''; - }) + || !self::hasValues($answers[$questionId]) || (!empty($question['extraSettings']['allowOtherAnswer']) && !array_filter($answers[$questionId], fn ($value) => $value !== Constants::QUESTION_EXTRASETTINGS_OTHER_PREFIX))) ) { throw new \InvalidArgumentException(sprintf('Question "%s" is required.', $question['text'])); @@ -695,7 +705,7 @@ public function validateSubmission(array $questions, array $answers, string $for } // Check if all answers are within the possible options - if (in_array($question['type'], Constants::ANSWER_TYPES_PREDEFINED) && empty($question['extraSettings']['allowOtherAnswer'])) { + if (in_array($question['type'], Constants::ANSWER_TYPES_PREDEFINED)) { // Normalize option IDs once for consistent comparison (DB may return ints, request may send strings) $optionIds = $this->normalizeOptionIds($question['options'] ?? []); @@ -711,14 +721,16 @@ public function validateSubmission(array $questions, array $answers, string $for // Check if all grid rows, columns and values match the configured grid subtype elseif ($question['type'] === Constants::ANSWER_TYPE_GRID) { $this->validateGridQuestion($question, $answers[$questionId]); + } elseif ($answer === Constants::QUESTION_EXTRASETTINGS_OTHER_PREFIX && self::isOtherAnswer($question, $answer)) { + throw new \InvalidArgumentException(sprintf('The answer "Other" for question "%s" needs a text.', $question['text'])); } // Search corresponding option, return false if non-existent - else { + elseif (!self::isOtherAnswer($question, $answer)) { // Accept numeric strings like "46" from JSON payloads reliably (e.g. with hardening extensions enabled) - $answerId = is_int($answer) ? $answer : (is_string($answer) ? intval(trim($answer)) : null); + $answerId = is_int($answer) ? $answer : (is_string($answer) ? intval($answer) : null); // Reject non-numeric / malformed values early - if ($answerId === null || (string)$answerId !== (string)intval($answerId)) { + if ($answerId === null || (string)$answerId !== (string)$answer) { throw new \InvalidArgumentException(sprintf('Answer "%s" for question "%s" is not a valid option.', is_scalar($answer) ? (string)$answer : gettype($answer), $question['text'])); } @@ -786,10 +798,88 @@ public function validateSubmission(array $questions, array $answers, string $for // Check for excess answers foreach ($answers as $id => $answerArray) { // Search corresponding question, return false if not found - if (!in_array($id, array_column($questions, 'id'))) { - throw new \InvalidArgumentException(sprintf('Answer for non-existent question with ID %d.', $id)); + if (!in_array($id, array_column($questions, 'id'), true)) { + throw new \InvalidArgumentException(sprintf('Answer for non-existent question with ID %s.', $id)); + } + } + } + + /** + * The questions on the path of the respondent: from the first question, each question leads to the target of its + * answer in `goTo`, else to its `next` target, else to the following question, until the end of the form. + * A target that is not a later question of the form counts as the following question. + * + * @param array $questions the questions of the form in their order + * @param array $answers the answers by question id + * @return list the questions on the path, in their order + */ + public static function getVisibleQuestions(array $questions, array $answers): array { + $questions = array_values($questions); + $positions = array_flip(array_column($questions, 'id')); + $visibleQuestions = []; + + $position = 0; + while ($position < count($questions)) { + $question = $questions[$position]; + $visibleQuestions[] = $question; + + $target = self::getJumpTarget($question, $answers[$question['id']] ?? []); + if ($target === Constants::JUMP_TARGET_SUBMIT) { + break; } + $targetPosition = is_int($target) && isset($positions[$target]) ? $positions[$target] : null; + $position = $targetPosition !== null && $targetPosition > $position ? $targetPosition : $position + 1; } + + return $visibleQuestions; + } + + /** + * The answers to the questions on the path of the respondent + * + * @param array $questions the questions of the form in their order + * @param array $answers the answers by question id + * @return array the answers by question id, without the answers to skipped questions + */ + public static function getAnswersOnPath(array $questions, array $answers): array { + return array_intersect_key($answers, array_flip(array_column(self::getQuestionsOnPath($questions, $answers), 'id'))); + } + + /** + * The questions on the path of the respondent, see getVisibleQuestions(), where answers to questions the submit view does not show do not decide the path + * + * @param array $questions the questions of the form in their order + * @param array $answers the answers by question id + * @return list the questions on the path, in their order + */ + private static function getQuestionsOnPath(array $questions, array $answers): array { + $hiddenQuestionIds = array_column(array_filter($questions, static fn (array $question): bool => !self::isShownToRespondents($question)), 'id'); + return self::getVisibleQuestions($questions, array_diff_key($answers, array_flip($hiddenQuestionIds))); + } + + /** + * The jump target of a question for its answer: the target of the `goTo` entry matching the first value of the answer, else `next` on question types with `next` + * + * @param array $question the question + * @param mixed $answer the values of the answer to the question + * @return string|int|null the target, `null` for the following question + */ + private static function getJumpTarget(array $question, mixed $answer): string|int|null { + $extraSettings = $question['extraSettings'] ?? []; + $value = is_array($answer) && array_is_list($answer) ? ($answer[0] ?? null) : null; + + if (is_int($value) || is_float($value) || is_string($value)) { + foreach (is_array($extraSettings['goTo'] ?? null) ? $extraSettings['goTo'] : [] as $jump) { + if ((isset($jump['optionId']) && (string)$jump['optionId'] === (string)$value) + || (($jump['other'] ?? false) === true && self::isOtherAnswer($question, $value))) { + $target = $jump['target'] ?? null; + return is_int($target) || is_string($target) ? $target : null; + } + } + } + + $next = in_array($question['type'] ?? null, Constants::QUESTION_TYPES_WITH_NEXT, true) ? ($extraSettings['next'] ?? null) : null; + return is_int($next) || is_string($next) ? $next : null; } /** @@ -928,6 +1018,18 @@ private function validateGridQuestion(array $question, array $answers): void { } } + /** + * Whether the value is the answer "Other" of a question that allows it + * + * @param array $question the question + * @param mixed $answer a value of the answer + */ + public static function isOtherAnswer(array $question, mixed $answer): bool { + return !empty($question['extraSettings']['allowOtherAnswer']) + && is_string($answer) + && str_starts_with($answer, Constants::QUESTION_EXTRASETTINGS_OTHER_PREFIX); + } + private function normalizeNumericOptionId(mixed $value): ?int { if (is_int($value)) { return $value; diff --git a/openapi.json b/openapi.json index 4336a8c92..3433195fd 100644 --- a/openapi.json +++ b/openapi.json @@ -530,6 +530,35 @@ "dateRange": { "type": "boolean" }, + "goTo": { + "type": "array", + "items": { + "type": "object", + "required": [ + "target" + ], + "properties": { + "optionId": { + "type": "integer", + "format": "int64" + }, + "other": { + "type": "boolean" + }, + "target": { + "oneOf": [ + { + "type": "integer", + "format": "int64" + }, + { + "type": "string" + } + ] + } + } + } + }, "maxAllowedFilesCount": { "type": "integer", "format": "int64" @@ -538,6 +567,17 @@ "type": "integer", "format": "int64" }, + "next": { + "oneOf": [ + { + "type": "integer", + "format": "int64" + }, + { + "type": "string" + } + ] + }, "optionsHighest": { "type": "integer", "format": "int64", @@ -606,6 +646,29 @@ "radio" ] }, + "QuestionOrder": { + "type": "object", + "required": [ + "order" + ], + "properties": { + "order": { + "type": "integer", + "format": "int64" + }, + "extraSettings": { + "anyOf": [ + { + "$ref": "#/components/schemas/QuestionExtraSettings" + }, + { + "type": "object", + "additionalProperties": true + } + ] + } + } + }, "QuestionType": { "type": "string", "enum": [ @@ -1675,7 +1738,7 @@ "post": { "operationId": "api-new-question", "summary": "Add a new question", - "description": "This endpoint allows CORS requests", + "description": "A clone loses its jumps to itself or to an earlier question, and the header `X-Forms-Adjusted` then holds `removed-jumps` with its id\nThis endpoint allows CORS requests", "tags": [ "api" ], @@ -1750,6 +1813,18 @@ "responses": { "201": { "description": "the created question", + "headers": { + "X-Forms-Adjusted": { + "schema": { + "type": "string" + } + }, + "Access-Control-Expose-Headers": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -1895,7 +1970,7 @@ "patch": { "operationId": "api-reorder-questions", "summary": "Updates the Order of all Questions of a Form", - "description": "This endpoint allows CORS requests", + "description": "Jumps that would go to the question itself or to an earlier question are removed: the new extra settings of these questions are returned, and the header `X-Forms-Adjusted` holds `removed-jumps` with their ids\nThis endpoint allows CORS requests", "tags": [ "api" ], @@ -1951,7 +2026,19 @@ ], "responses": { "200": { - "description": "the question ids of the given form in the new order", + "description": "the question ids of the given form in the new order, with the new extra settings of the questions whose jumps were removed", + "headers": { + "X-Forms-Adjusted": { + "schema": { + "type": "string" + } + }, + "Access-Control-Expose-Headers": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -1973,7 +2060,7 @@ "data": { "type": "object", "additionalProperties": { - "$ref": "#/components/schemas/Order" + "$ref": "#/components/schemas/QuestionOrder" } } } @@ -2291,7 +2378,7 @@ "patch": { "operationId": "api-update-question", "summary": "Writes the given key-value pairs into Database Key `order` should only be changed by reorderQuestions() and is not allowed here", - "description": "This endpoint allows CORS requests", + "description": "Jumps to the question itself or to an earlier question are removed before saving, and the header `X-Forms-Adjusted` then holds `removed-jumps` with the id of the question. Jumps to questions of other forms or to options that are not options of the question are rejected as invalid `extraSettings`.\nThis endpoint allows CORS requests", "tags": [ "api" ], @@ -2357,6 +2444,18 @@ "responses": { "200": { "description": "the id of the updated question", + "headers": { + "X-Forms-Adjusted": { + "schema": { + "type": "string" + } + }, + "Access-Control-Expose-Headers": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { @@ -3360,7 +3459,7 @@ "delete": { "operationId": "api-delete-option", "summary": "Delete an option", - "description": "Returns the id of the deleted option\nThis endpoint allows CORS requests", + "description": "The jumps of the question for this option are removed as well, and the header `X-Forms-Adjusted` then holds `removed-jumps` with the id of the question.\nReturns the id of the deleted option\nThis endpoint allows CORS requests", "tags": [ "api" ], @@ -3414,6 +3513,18 @@ "responses": { "200": { "description": "the id of the deleted option", + "headers": { + "X-Forms-Adjusted": { + "schema": { + "type": "string" + } + }, + "Access-Control-Expose-Headers": { + "schema": { + "type": "string" + } + } + }, "content": { "application/json": { "schema": { diff --git a/playwright/e2e/jump-issues.spec.ts b/playwright/e2e/jump-issues.spec.ts new file mode 100644 index 000000000..87a3bd98b --- /dev/null +++ b/playwright/e2e/jump-issues.spec.ts @@ -0,0 +1,238 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import type { FormsQuestion } from '../../src/types/Entities.d.ts' + +import { expect, test } from '@playwright/test' +import { + formatList, + getJumpIssues, + getNewlyBrokenQuestions, +} from '../../src/utils/Jumps.ts' + +/** + * A radio question with the options "Yes" (id 11) and "No" (id 12) + * + * @param id the id of the question + * @param extraSettings the extra settings + * @param isRequired whether the question is required + */ +function radio( + id: number, + extraSettings: FormsQuestion['extraSettings'], + isRequired = false, +): FormsQuestion { + return { + id, + text: `Question ${id}`, + type: 'multiple_unique', + isRequired, + options: [ + { id: 11, questionId: id, text: 'Yes', order: 1 }, + { id: 12, questionId: id, text: 'No', order: 2 }, + ], + extraSettings, + } as FormsQuestion +} + +/** + * A short text question + * + * @param id the id of the question + * @param extraSettings the extra settings + * @param isRequired whether the question is required + */ +function short( + id: number, + extraSettings: FormsQuestion['extraSettings'] = {}, + isRequired = false, +): FormsQuestion { + return { id, text: `Question ${id}`, type: 'short', isRequired, extraSettings } +} + +/** + * The issues as plain values + * + * @param questions the questions of the form + */ +function issues(questions: FormsQuestion[]) { + const { broken, unreachable, skippable } = getJumpIssues(questions) + return { + broken: [...broken], + unreachable: [...unreachable], + skippable: [...skippable], + } +} + +test('jump lists: a language variant such as sr@latin lists as its base language', () => { + expect(formatList(['1', '2', '3'], 'sr@latin')).toBe('1, 2 i 3') + expect(formatList(['1', '2'], 'de-DE')).toBe('1 und 2') +}) + +test('jump issues: a jump over a question lets respondents skip it', () => { + expect( + issues([ + radio(1, { goTo: [{ optionId: 12, target: 3 }] }), + short(2), + short(3), + ]), + ).toEqual({ broken: [], unreachable: [], skippable: [2] }) +}) + +test('jump issues: "End of the form" lets respondents skip all later questions', () => { + expect( + issues([ + radio(1, { goTo: [{ optionId: 12, target: 'submit' }] }), + short(2), + short(3), + ]), + ).toEqual({ broken: [], unreachable: [], skippable: [2, 3] }) +}) + +test('jump issues: a required question whose answers all jump past a question makes it unreachable', () => { + expect( + issues([ + radio( + 1, + { + goTo: [ + { optionId: 11, target: 3 }, + { optionId: 12, target: 'submit' }, + ], + }, + true, + ), + short(2), + short(3), + ]), + ).toEqual({ broken: [], unreachable: [2], skippable: [3] }) +}) + +test('jump issues: an optional question without an answer goes to the following question', () => { + expect( + issues([ + radio(1, { + goTo: [ + { optionId: 11, target: 3 }, + { optionId: 12, target: 3 }, + ], + }), + short(2), + short(3), + ]), + ).toEqual({ broken: [], unreachable: [], skippable: [2] }) +}) + +test('jump issues: "End of the form" after a question makes the later questions unreachable', () => { + expect(issues([short(1, { next: 'submit' }), short(2), short(3)])).toEqual({ + broken: [], + unreachable: [2, 3], + skippable: [], + }) +}) + +test('jump issues: a jump to an earlier or deleted question is broken', () => { + expect( + issues([ + short(1), + radio(2, { goTo: [{ optionId: 12, target: 1 }] }), + short(3, { next: 99 }), + short(4), + ]), + ).toEqual({ broken: [2, 3], unreachable: [], skippable: [] }) +}) + +test('jump issues: a jump that breaks is new even when its question has a broken jump already', () => { + const pet = radio(1, { + goTo: [ + { optionId: 11, target: 99 }, + { optionId: 12, target: 3 }, + ], + }) + const before = getJumpIssues([pet, short(2), short(3)]) + const after = getJumpIssues([short(3), pet, short(2)]) + + expect(getNewlyBrokenQuestions(before, after)).toEqual([1]) + expect(getNewlyBrokenQuestions(before, before)).toEqual([]) +}) + +test('jump issues: a malformed jump is ignored', () => { + expect( + issues([ + radio(1, { + goTo: [null, { optionId: 12, target: [3] }], + } as unknown as FormsQuestion['extraSettings']), + short(2), + short(3), + ]), + ).toEqual({ broken: [], unreachable: [], skippable: [] }) +}) + +test('jump issues: a jump of a deleted option is ignored', () => { + expect( + issues([ + radio(1, { goTo: [{ optionId: 99, target: 3 }] }), + short(2), + short(3), + ]), + ).toEqual({ broken: [], unreachable: [], skippable: [] }) +}) + +test('jump issues: with "Other" allowed, the answer "Other" needs a jump as well to make a question unreachable', () => { + const goTo = [ + { optionId: 11, target: 3 }, + { optionId: 12, target: 3 }, + ] + expect( + issues([ + radio(1, { goTo, allowOtherAnswer: true }, true), + short(2), + short(3), + ]).unreachable, + ).toEqual([]) + expect( + issues([ + radio( + 1, + { + goTo: [...goTo, { other: true, target: 3 }], + allowOtherAnswer: true, + }, + true, + ), + short(2), + short(3), + ]).unreachable, + ).toEqual([2]) +}) + +test('jump issues: the jumps of a question the submit view does not show lead nowhere', () => { + const other = radio( + 1, + { goTo: [{ other: true, target: 3 }], allowOtherAnswer: true }, + true, + ) + expect(issues([{ ...other, options: [] }, short(2), short(3)])).toEqual({ + broken: [], + unreachable: [], + skippable: [], + }) + const untitled = radio(1, { goTo: [{ optionId: 12, target: 3 }] }, true) + expect(issues([{ ...untitled, text: ' ' }, short(2), short(3)])).toEqual({ + broken: [], + unreachable: [], + skippable: [], + }) +}) + +test('jump issues: a jump of the answer "Other" breaks while "Other" is off', () => { + const pet = radio(2, { goTo: [{ other: true, target: 3 }] }) + expect( + getNewlyBrokenQuestions( + getJumpIssues([short(1), pet, short(3)]), + getJumpIssues([short(3), short(1), pet]), + ), + ).toEqual([2]) +}) diff --git a/playwright/e2e/jump-paths.spec.ts b/playwright/e2e/jump-paths.spec.ts new file mode 100644 index 000000000..d69cae596 --- /dev/null +++ b/playwright/e2e/jump-paths.spec.ts @@ -0,0 +1,33 @@ +/** + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import type { FormsQuestion } from '../../src/types/Entities.d.ts' + +import { expect, test } from '@playwright/test' +import { readFileSync } from 'node:fs' +import { getVisibleQuestions } from '../../src/utils/Jumps.ts' + +/** + * Runs the cases of tests/fixtures/jump-paths.json, shared with SubmissionServiceTest::testJumpPaths() + */ +const cases: Array<{ + name: string + questions: FormsQuestion[] + answers: Record + visible: number[] +}> = JSON.parse( + readFileSync( + new URL('../../tests/fixtures/jump-paths.json', import.meta.url), + 'utf-8', + ), +) + +for (const { name, questions, answers, visible } of cases) { + test(`jump path: ${name}`, () => { + expect( + getVisibleQuestions(questions, answers).map((question) => question.id), + ).toEqual(visible) + }) +} diff --git a/playwright/e2e/jumps.spec.ts b/playwright/e2e/jumps.spec.ts new file mode 100644 index 000000000..91819cfe8 --- /dev/null +++ b/playwright/e2e/jumps.spec.ts @@ -0,0 +1,162 @@ +/** + * 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 +} + +/** + * Create a form with a radio question "Do you have a pet?" (Yes, No), a required short question "Which pet?" and a short question "Comments" + * + * @param request the request context of the page + */ +async function createPetForm(request: APIRequestContext) { + const form = await api(request, 'POST', '/forms') + await api(request, 'PATCH', `/forms/${form.id}`, { + keyValuePairs: { title: 'Pet form' }, + }) + const pet = await api(request, 'POST', `/forms/${form.id}/questions`, { + type: 'multiple_unique', + text: 'Do you have a pet?', + }) + const options = await api( + request, + 'POST', + `/forms/${form.id}/questions/${pet.id}/options`, + { optionTexts: ['Yes', 'No'] }, + ) + const which = await api(request, 'POST', `/forms/${form.id}/questions`, { + type: 'short', + text: 'Which pet?', + }) + await api(request, 'PATCH', `/forms/${form.id}/questions/${which.id}`, { + keyValuePairs: { isRequired: true }, + }) + const comments = await api(request, 'POST', `/forms/${form.id}/questions`, { + type: 'short', + text: 'Comments', + }) + return { form, pet, options, which, comments } +} + +test.describe('Jumps', () => { + test('An answer jumps to a later question, set in the editor', async ({ + page, + }) => { + const { form, pet, options, comments } = await createPetForm(page.request) + await page.goto(`apps/forms/${form.hash}/edit`) + + const question = page.getByRole('listitem', { name: 'Question number 1' }) + await question.getByRole('button', { name: 'Actions', exact: true }).click() + await page + .getByRole('checkbox', { name: 'Go to based on answer' }) + .click({ force: true }) + await page.keyboard.press('Escape') + + await question + .getByRole('combobox', { name: 'Question 1: after "No" go to' }) + .click() + await page + .getByRole('option') + .filter({ hasText: 'Question 3: Comments' }) + .click() + + await expect(async () => { + const stored = await api( + page.request, + 'GET', + `/forms/${form.id}/questions/${pet.id}`, + ) + expect(stored.extraSettings.goTo).toEqual([ + { optionId: options[1].id, target: comments.id }, + ]) + }).toPass() + }) + + test('Skipped questions are hidden, not required and not submitted', async ({ + page, + submitView, + }) => { + const { form, pet, options, comments } = await createPetForm(page.request) + await api(page.request, 'PATCH', `/forms/${form.id}/questions/${pet.id}`, { + keyValuePairs: { + extraSettings: { + goTo: [{ optionId: options[1].id, target: comments.id }], + }, + }, + }) + await page.goto(`apps/forms/${form.hash}/submit`) + + await expect(submitView.getQuestion(/Which pet\?/)).toBeVisible() + await submitView.checkRadio('Do you have a pet?', 'Yes') + await submitView.fillText(/Which pet\?/, 'Rex') + await submitView.checkRadio('Do you have a pet?', 'No') + await expect(submitView.getQuestion(/Which pet\?/)).toBeHidden() + await submitView.fillText('Comments', 'None') + + await submitView.submit() + await expect(submitView.successMessage).toBeVisible() + + const { submissions } = await api( + page.request, + 'GET', + `/forms/${form.id}/submissions`, + ) + expect(submissions[0].answers.map((answer) => answer.text).sort()).toEqual([ + 'No', + 'None', + ]) + }) + + test('"End of the form" after a question hides the questions after it', async ({ + page, + submitView, + }) => { + const { form, pet, options } = await createPetForm(page.request) + await api(page.request, 'PATCH', `/forms/${form.id}/questions/${pet.id}`, { + keyValuePairs: { + extraSettings: { + goTo: [{ optionId: options[1].id, target: 'submit' }], + }, + }, + }) + await page.goto(`apps/forms/${form.hash}/submit`) + + await expect(submitView.getQuestion(/Which pet\?/)).toBeVisible() + await expect(submitView.getQuestion('Comments')).toBeVisible() + await submitView.checkRadio('Do you have a pet?', 'No') + await expect(submitView.getQuestion(/Which pet\?/)).toBeHidden() + await expect(submitView.getQuestion('Comments')).toBeHidden() + await submitView.checkRadio('Do you have a pet?', 'Yes') + await expect(submitView.getQuestion('Comments')).toBeVisible() + }) +}) diff --git a/src/components/Questions/AnswerInput.vue b/src/components/Questions/AnswerInput.vue index 9181f22ff..c82f0f962 100644 --- a/src/components/Questions/AnswerInput.vue +++ b/src/components/Questions/AnswerInput.vue @@ -4,7 +4,10 @@ -->