diff --git a/docs/API_v3.md b/docs/API_v3.md index e2ffc2ece..4b7ac188f 100644 --- a/docs/API_v3.md +++ b/docs/API_v3.md @@ -37,7 +37,7 @@ Some requests succeed with their usual status, and their payload keeps its shape | 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) | +| `removed-jumps` | Inner list of question ids, e.g. `(3 7)` | Updating, reordering and copying questions; deleting options; changing the bounds of a linear scale | Jumps to the question itself or to an earlier question, jumps of the deleted option, or jumps for values outside the scale, were removed from the `extraSettings` of these questions, see [Jumps](DataStructure.md#jumps) | ``` X-Forms-Adjusted: removed-jumps=(3 7) @@ -62,7 +62,7 @@ X-Forms-Adjusted: removed-jumps=(3 7) ### 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. +- Questions can jump to a later question or to the end of the form with the extra settings `goTo` (per answer, on dropdown, radio and linear scale 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, deleting an option removes its jumps, and narrowing a linear scale removes the jumps of the values that are gone. 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 @@ -466,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. 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." +- 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, and jumps of a linear scale for values outside its bounds, 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 diff --git a/docs/DataStructure.md b/docs/DataStructure.md index 81c6abe24..83ad7fc76 100644 --- a/docs/DataStructure.md +++ b/docs/DataStructure.md @@ -270,16 +270,16 @@ 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 | +| `goTo` | `dropdown, linearscale, multiple_unique` | Array | see [Jumps](#jumps) | Where to go next for an answer | | `next` | `color, date, file, grid, long, multiple, ranking, short, time` | 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. +- `goTo` is a list of jumps per answer: `{"optionId": 12, "target": 34}` for an option, `{"other": true, "target": "submit"}` for the answer "Other", `{"value": 4, "target": 34}` for a value of a linear scale. There is at most one jump per answer. A `value` is an integer from 0 to 10, the range of all scales. When a question is updated, or its form copied or imported, the server removes the jumps for values outside the scale of the question; on update the response header `X-Forms-Adjusted` then holds `removed-jumps` with the id of the question. - `next` is one target after the whole question; on a checkbox question it applies whichever options are checked. -- 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 question has either jumps per answer or one jump after the question, depending on its type: `goTo` only on `dropdown`, `linearscale` 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. diff --git a/lib/Constants.php b/lib/Constants.php index 59d1c3fb5..e08c91494 100644 --- a/lib/Constants.php +++ b/lib/Constants.php @@ -237,6 +237,7 @@ class Constants { public const QUESTION_TYPES_WITH_GOTO = [ self::ANSWER_TYPE_DROPDOWN, + self::ANSWER_TYPE_LINEARSCALE, self::ANSWER_TYPE_MULTIPLEUNIQUE, ]; diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index 9def9bdfa..f975ba213 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -699,7 +699,7 @@ public function newQuestion(int $formId, ?string $type = null, ?string $subtype * 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 the question itself or to an earlier question, and jumps of a linear scale for values outside its bounds, 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 @@ -782,7 +782,8 @@ public function updateQuestion(int $formId, int $questionId, array $keyValuePair $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); + $removedJumps = FormsService::removeJumpsOutsideScale($updatedQuestion); + $removedJumps = in_array($updatedQuestion, FormsService::removeBackwardJumps($formQuestions), true) || $removedJumps; $keyValuePairs['extraSettings'] = $updatedQuestion->getExtraSettings(); } diff --git a/lib/ResponseDefinitions.php b/lib/ResponseDefinitions.php index 74950b4bd..23fb8696c 100644 --- a/lib/ResponseDefinitions.php +++ b/lib/ResponseDefinitions.php @@ -27,7 +27,7 @@ * dateMax?: string|int, * dateMin?: string|int, * dateRange?: bool, - * goTo?: list, + * goTo?: list, target: int|string}>, * maxAllowedFilesCount?: int, * maxFileSize?: int, * next?: int|string, diff --git a/lib/Service/FormsService.php b/lib/Service/FormsService.php index a7671db16..4b8eda3d4 100644 --- a/lib/Service/FormsService.php +++ b/lib/Service/FormsService.php @@ -1078,7 +1078,7 @@ private static function isJumpTarget(mixed $target): bool { } /** - * Whether the value is a list of jumps per answer, each with one answer key and a target, at most one per answer + * Whether the value is a list of jumps per answer, each with one answer key (`value` from 0 to 10 on linear scales, else `optionId` or `other`) and a target, at most one per answer * * @param mixed $goTo the value of `goTo` * @param string $questionType the question type @@ -1094,7 +1094,12 @@ private static function isGoToValid(mixed $goTo, string $questionType): bool { return false; } - if (is_int($jump['optionId'] ?? null) && $jump['optionId'] > 0) { + if ($questionType === Constants::ANSWER_TYPE_LINEARSCALE) { + if (!is_int($jump['value'] ?? null) || $jump['value'] < 0 || $jump['value'] > 10) { + return false; + } + $answer = 'value:' . $jump['value']; + } elseif (is_int($jump['optionId'] ?? null) && $jump['optionId'] > 0) { $answer = 'option:' . $jump['optionId']; } elseif (($jump['other'] ?? null) === true) { $answer = 'other'; @@ -1215,6 +1220,7 @@ public static function mapJumpsOfCopies(array $questions, array $questionIdMap, $extraSettings['goTo'] = $goTo; } $question->setExtraSettings($extraSettings); + self::removeJumpsOutsideScale($question); $copiesWithJumps[] = $question; } } @@ -1261,6 +1267,30 @@ public static function removeBackwardJumps(array $questions): array { return $changed; } + /** + * Remove the jumps of a linear scale for values outside its bounds + * + * @param Question $question the question + * @return bool whether jumps were removed, with the new extra settings set + */ + public static function removeJumpsOutsideScale(Question $question): bool { + $extraSettings = $question->getExtraSettings(); + if ($question->getType() !== Constants::ANSWER_TYPE_LINEARSCALE || !is_array($extraSettings['goTo'] ?? null)) { + return false; + } + + $lowest = $extraSettings['optionsLowest'] ?? 1; + $highest = $extraSettings['optionsHighest'] ?? 5; + $goTo = array_values(array_filter($extraSettings['goTo'], static fn (mixed $jump): bool => !is_int(is_array($jump) ? ($jump['value'] ?? null) : null) || ($jump['value'] >= $lowest && $jump['value'] <= $highest))); + if (count($goTo) === count($extraSettings['goTo'])) { + return false; + } + + $extraSettings['goTo'] = $goTo; + $question->setExtraSettings($extraSettings); + return true; + } + /** * Get list of forms * diff --git a/lib/Service/SubmissionService.php b/lib/Service/SubmissionService.php index f82387d46..4d9f8cde3 100644 --- a/lib/Service/SubmissionService.php +++ b/lib/Service/SubmissionService.php @@ -871,7 +871,8 @@ private static function getJumpTarget(array $question, mixed $answer): string|in 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))) { + || (($jump['other'] ?? false) === true && self::isOtherAnswer($question, $value)) + || (isset($jump['value']) && ctype_digit((string)$value) && (int)$value === $jump['value'])) { $target = $jump['target'] ?? null; return is_int($target) || is_string($target) ? $target : null; } diff --git a/openapi.json b/openapi.json index 3433195fd..20cecd878 100644 --- a/openapi.json +++ b/openapi.json @@ -545,6 +545,12 @@ "other": { "type": "boolean" }, + "value": { + "type": "integer", + "format": "int64", + "minimum": 0, + "maximum": 10 + }, "target": { "oneOf": [ { @@ -2378,7 +2384,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": "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", + "description": "Jumps to the question itself or to an earlier question, and jumps of a linear scale for values outside its bounds, 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" ], diff --git a/playwright/e2e/jump-issues.spec.ts b/playwright/e2e/jump-issues.spec.ts index 87a3bd98b..e36888b9b 100644 --- a/playwright/e2e/jump-issues.spec.ts +++ b/playwright/e2e/jump-issues.spec.ts @@ -236,3 +236,28 @@ test('jump issues: a jump of the answer "Other" breaks while "Other" is off', () ), ).toEqual([2]) }) + +test('jump issues: a required scale whose values all jump past a question makes it unreachable', () => { + const scale = (goTo: Array<{ value: number; target: number }>) => + ({ + id: 1, + text: 'Question 1', + type: 'linearscale', + isRequired: true, + extraSettings: { optionsLowest: 1, optionsHighest: 2, goTo }, + }) as FormsQuestion + expect( + issues([scale([{ value: 1, target: 3 }]), short(2), short(3)]).unreachable, + ).toEqual([]) + expect( + issues([ + scale([ + { value: 1, target: 3 }, + { value: 2, target: 3 }, + { value: 3, target: 2 }, + ]), + short(2), + short(3), + ]).unreachable, + ).toEqual([2]) +}) diff --git a/playwright/e2e/jumps.spec.ts b/playwright/e2e/jumps.spec.ts index 91819cfe8..c6cf93119 100644 --- a/playwright/e2e/jumps.spec.ts +++ b/playwright/e2e/jumps.spec.ts @@ -159,4 +159,71 @@ test.describe('Jumps', () => { await submitView.checkRadio('Do you have a pet?', 'Yes') await expect(submitView.getQuestion('Comments')).toBeVisible() }) + + test('A value of a linear scale jumps past a question, set in the editor', async ({ + page, + submitView, + }) => { + const form = await api(page.request, 'POST', '/forms') + await api(page.request, 'PATCH', `/forms/${form.id}`, { + keyValuePairs: { title: 'Scale form' }, + }) + const scale = await api( + page.request, + 'POST', + `/forms/${form.id}/questions`, + { + type: 'linearscale', + text: 'How do you like it?', + }, + ) + await api(page.request, 'POST', `/forms/${form.id}/questions`, { + type: 'short', + text: 'What can we improve?', + }) + const comments = await api( + page.request, + 'POST', + `/forms/${form.id}/questions`, + { + type: 'short', + text: 'Comments', + }, + ) + 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 "5 - Strongly agree" 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/${scale.id}`, + ) + expect(stored.extraSettings.goTo).toEqual([ + { value: 5, target: comments.id }, + ]) + }).toPass() + + await page.goto(`apps/forms/${form.hash}/submit`) + await submitView.checkRadio('How do you like it?', '5') + await expect(submitView.getQuestion('What can we improve?')).toBeHidden() + await submitView.checkRadio('How do you like it?', '4') + await expect(submitView.getQuestion('What can we improve?')).toBeVisible() + }) }) diff --git a/src/components/Questions/QuestionLinearScale.vue b/src/components/Questions/QuestionLinearScale.vue index 059df0f8c..a5c1b5023 100644 --- a/src/components/Questions/QuestionLinearScale.vue +++ b/src/components/Questions/QuestionLinearScale.vue @@ -128,6 +128,21 @@ {{ optionsLabelHighest }} +
    +
  • + {{ getValueText(option) }} + +
    • +
@@ -142,18 +157,21 @@ import NcActionInput from '@nextcloud/vue/components/NcActionInput' import NcCheckboxRadioSwitch from '@nextcloud/vue/components/NcCheckboxRadioSwitch' import NcIconSvgWrapper from '@nextcloud/vue/components/NcIconSvgWrapper' import NcTextArea from '@nextcloud/vue/components/NcTextArea' +import JumpTargetSelect from './JumpTargetSelect.vue' import Question from './Question.vue' import { QUESTION_EMITS, QUESTION_PROPS, useQuestion, } from '../../composables/useQuestion.ts' +import { getJumps } from '../../utils/Jumps.ts' type LinearScaleExtraSettings = { optionsLowest?: number | null optionsHighest?: number | null optionsLabelLowest?: string | null optionsLabelHighest?: string | null + goTo?: unknown } export default defineComponent({ @@ -164,6 +182,7 @@ export default defineComponent({ NcActionInput, NcCheckboxRadioSwitch, NcTextArea, + JumpTargetSelect, Question, }, @@ -187,6 +206,27 @@ export default defineComponent({ const defaultLowestLabel = t('forms', 'Strongly disagree') const defaultHighestLabel = t('forms', 'Strongly agree') + /** + * The jumps within new bounds of the scale, to save together with the bounds + * + * @param lowest the lowest value of the scale + * @param highest the highest value of the scale + */ + const jumpsWithin = ( + lowest: number, + highest: number, + ): Record => { + const goTo = getJumps(extraSettings.value) + const kept = goTo.filter( + (jump) => + jump.value === undefined + || (jump.value >= lowest && jump.value <= highest), + ) + return kept.length === goTo.length + ? {} + : { goTo: kept.length > 0 ? kept : undefined } + } + const optionsLowest = computed({ get: () => { return extraSettings.value.optionsLowest ?? 1 @@ -194,6 +234,7 @@ export default defineComponent({ set: (value: number) => { question.onExtraSettingsChange({ optionsLowest: value === 1 ? null : value, + ...jumpsWithin(value, extraSettings.value.optionsHighest ?? 5), }) }, }) @@ -205,6 +246,7 @@ export default defineComponent({ set: (value: number) => { question.onExtraSettingsChange({ optionsHighest: value === 5 ? null : value, + ...jumpsWithin(extraSettings.value.optionsLowest ?? 1, value), }) }, }) @@ -241,6 +283,28 @@ export default defineComponent({ const questionValues = computed(() => values.value) + /** + * A scale value with the label of its end of the scale, e.g. "1 - Strongly disagree" + * + * @param option the value + */ + const getValueText = (option: number): string => { + const label = + option === optionsLowest.value + ? optionsLabelLowest.value + : option === optionsHighest.value + ? optionsLabelHighest.value + : '' + return label + ? t( + 'forms', + '{value} - {label}', + { value: option, label }, + { escape: false, sanitize: false }, + ) + : String(option) + } + /** * ID for the label for the lowest option */ @@ -336,6 +400,7 @@ export default defineComponent({ lowest, highest, scaleOptions, + getValueText, isUnique, questionValues, labelId, @@ -357,6 +422,23 @@ export default defineComponent({