From 9cf4299bfbe8a3ade2a7e0c3ced27792d33ea305 Mon Sep 17 00:00:00 2001 From: Christian Hartmann Date: Mon, 23 Sep 2024 18:41:51 +0200 Subject: [PATCH] Adjust code to API v3 and use reorder logic from Questions Signed-off-by: Christian Hartmann --- appinfo/routes.php | 385 +++++++++------------------- docs/API.md | 29 +-- docs/API_v3.md | 42 ++- lib/Controller/ApiController.php | 137 ++++++---- lib/Db/Option.php | 4 +- tests/Integration/Api/ApiV2Test.php | 4 - tests/Integration/Api/ApiV3Test.php | 12 +- 7 files changed, 259 insertions(+), 354 deletions(-) diff --git a/appinfo/routes.php b/appinfo/routes.php index 6ed8db7a7..bf9caf49b 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -84,7 +84,7 @@ // ['name' => 'api#getOption', 'url' => $apiBase . 'forms/{formId}/questions/{questionId}/options/{optionId}', 'verb' => 'GET', 'requirements' => $requirements_v3], ['name' => 'api#updateOption', 'url' => $apiBase . 'forms/{formId}/questions/{questionId}/options/{optionId}', 'verb' => 'PATCH', 'requirements' => $requirements_v3], ['name' => 'api#deleteOption', 'url' => $apiBase . 'forms/{formId}/questions/{questionId}/options/{optionId}', 'verb' => 'DELETE', 'requirements' => $requirements_v3], - // ['name' => 'api#reorderOptions', 'url' => $apiBase . 'forms/{formId}/questions/{questionId}/options', 'verb' => 'PATCH', 'requirements' => $requirements_v3], + ['name' => 'api#reorderOptions', 'url' => $apiBase . 'forms/{formId}/questions/{questionId}/options', 'verb' => 'PATCH', 'requirements' => $requirements_v3], // Shares // ['name' => 'shareApi#getUserShares', 'url' => $apiBase . 'shares', 'verb' => 'GET', 'requirements' => $requirements_v3], @@ -106,284 +106,135 @@ // Legacy v2 routes (TODO: remove with Forms v5) // Forms - [ - 'name' => 'api#getForms', - 'url' => '/api/{apiVersion}/forms', - 'verb' => 'GET', - 'requirements' => [ - 'apiVersion' => 'v2(\.[1-5])?' - ] - ], - [ - 'name' => 'api#newForm', - 'url' => '/api/{apiVersion}/form', - 'verb' => 'POST', - 'requirements' => [ - 'apiVersion' => 'v2(\.[1-5])?' - ] - ], - [ - 'name' => 'api#getForm', - 'url' => '/api/{apiVersion}/form/{id}', - 'verb' => 'GET', - 'requirements' => [ - 'apiVersion' => 'v2(\.[1-5])?' - ] - ], - [ - 'name' => 'api#cloneForm', - 'url' => '/api/{apiVersion}/form/clone/{id}', - 'verb' => 'POST', - 'requirements' => [ - 'apiVersion' => 'v2(\.[1-5])?' - ] - ], - // TODO: Remove POST in next API release - [ - 'name' => 'api#updateForm', - 'url' => '/api/{apiVersion}/form/update', - 'verb' => 'POST', - 'requirements' => [ - 'apiVersion' => 'v2(\.[1-5])?' - ] - ], - [ - 'name' => 'api#updateForm', - 'url' => '/api/{apiVersion}/form/update', - 'verb' => 'PATCH', - 'requirements' => [ - 'apiVersion' => 'v2\.[2-5]' - ] - ], - [ - 'name' => 'api#transferOwner', - 'url' => '/api/{apiVersion}/form/transfer', - 'verb' => 'POST', - 'requirements' => [ - 'apiVersion' => 'v2\.[2-5]' - ] - ], - [ - 'name' => 'api#deleteForm', - 'url' => '/api/{apiVersion}/form/{id}', - 'verb' => 'DELETE', - 'requirements' => [ - 'apiVersion' => 'v2(\.[1-5])?' - ] - ], - [ - 'name' => 'api#getPartialForm', - 'url' => '/api/{apiVersion}/partial_form/{hash}', - 'verb' => 'GET', - 'requirements' => [ - 'apiVersion' => 'v2(\.[1-5])?' - ] - ], - [ - 'name' => 'api#getSharedForms', - 'url' => '/api/{apiVersion}/shared_forms', - 'verb' => 'GET', - 'requirements' => [ - 'apiVersion' => 'v2(\.[1-5])?' - ] - ], + ['name' => 'api#getFormsLegacy', 'url' => $apiBase . 'forms', 'verb' => 'GET', 'requirements' => [ + 'apiVersion' => 'v2(\.[1-5])?' + ]], + ['name' => 'api#newFormLegacy', 'url' => $apiBase . 'form', 'verb' => 'POST', 'requirements' => [ + 'apiVersion_path' => 'v2(\.[1-5])?' + ]], + ['name' => 'api#getFormLegacy', 'url' => $apiBase . 'form/{id}', 'verb' => 'GET', 'requirements' => [ + 'apiVersion_path' => 'v2(\.[1-5])?', + 'id' => '\d+' + ]], + ['name' => 'api#cloneFormLegacy', 'url' => $apiBase . 'form/clone/{id}', 'verb' => 'POST', 'requirements' => [ + 'apiVersion' => 'v2(\.[1-5])?', + 'id' => '\d+' + ]], + ['name' => 'api#updateFormLegacy', 'url' => $apiBase . 'form/update', 'verb' => 'POST', 'requirements' => [ + 'apiVersion' => 'v2(\.[1-5])?' + ]], + ['name' => 'api#updateFormLegacy', 'url' => $apiBase . 'form/update', 'verb' => 'PATCH', 'requirements' => [ + 'apiVersion' => 'v2\.[2-5]' + ]], + ['name' => 'api#transferOwnerLegacy', 'url' => $apiBase . 'form/transfer', 'verb' => 'POST', 'requirements' => [ + 'apiVersion' => 'v2\.[2-5]' + ]], + ['name' => 'api#deleteFormLegacy', 'url' => $apiBase . 'form/{id}', 'verb' => 'DELETE', 'requirements' => [ + 'apiVersion' => 'v2(\.[1-5])?', + 'id' => '\d+' + ]], + ['name' => 'api#getPartialFormLegacy', 'url' => $apiBase . 'partial_form/{hash}', 'verb' => 'GET', 'requirements' => [ + 'apiVersion' => 'v2(\.[1-5])?', + 'hash' => '[a-zA-Z0-9]{16}' + ]], + ['name' => 'api#getSharedFormsLegacy', 'url' => $apiBase . 'shared_forms', 'verb' => 'GET', 'requirements' => [ + 'apiVersion' => 'v2(\.[1-5])?' + ]], // Questions - [ - 'name' => 'api#newQuestion', - 'url' => '/api/{apiVersion}/question', - 'verb' => 'POST', - 'requirements' => [ - 'apiVersion' => 'v2(\.[1-5])?' - ] - ], + ['name' => 'api#newQuestionLegacy', 'url' => $apiBase . 'question', 'verb' => 'POST', 'requirements' => [ + 'apiVersion' => 'v2(\.[1-5])?' + ]], // TODO: Remove POST in next API release - [ - 'name' => 'api#updateQuestion', - 'url' => '/api/{apiVersion}/question/update', - 'verb' => 'POST', - 'requirements' => [ - 'apiVersion' => 'v2(\.[1-5])?' - ] - ], - [ - 'name' => 'api#updateQuestion', - 'url' => '/api/{apiVersion}/question/update', - 'verb' => 'PATCH', - 'requirements' => [ - 'apiVersion' => 'v2\.[2-5]' - ] - ], + ['name' => 'api#updateQuestionLegacy', 'url' => $apiBase . 'question/update', 'verb' => 'POST', 'requirements' => [ + 'apiVersion' => 'v2(\.[1-5])?' + ]], + ['name' => 'api#updateQuestionLegacy', 'url' => $apiBase . 'question/update', 'verb' => 'PATCH', 'requirements' => [ + 'apiVersion' => 'v2\.[2-5]' + ]], // TODO: Remove POST in next API release - [ - 'name' => 'api#reorderQuestions', - 'url' => '/api/{apiVersion}/question/reorder', - 'verb' => 'POST', - 'requirements' => [ - 'apiVersion' => 'v2(\.[1-5])?' - ] - ], - [ - 'name' => 'api#reorderQuestions', - 'url' => '/api/{apiVersion}/question/reorder', - 'verb' => 'PUT', - 'requirements' => [ - 'apiVersion' => 'v2\.[2-5]' - ] - ], - [ - 'name' => 'api#deleteQuestion', - 'url' => '/api/{apiVersion}/question/{id}', - 'verb' => 'DELETE', - 'requirements' => [ - 'apiVersion' => 'v2(\.[1-5])?' - ] - ], - [ - 'name' => 'api#cloneQuestion', - 'url' => '/api/{apiVersion}/question/clone/{id}', - 'verb' => 'POST', - 'requirements' => [ - 'apiVersion' => 'v2\.[3-5]' - ] - ], + ['name' => 'api#reorderQuestionsLegacy', 'url' => $apiBase . 'question/reorder', 'verb' => 'POST', 'requirements' => [ + 'apiVersion' => 'v2(\.[1-5])?' + ]], + ['name' => 'api#reorderQuestionsLegacy', 'url' => $apiBase . 'question/reorder', 'verb' => 'PUT', 'requirements' => [ + 'apiVersion' => 'v2\.[2-5]' + ]], + ['name' => 'api#deleteQuestionLegacy', 'url' => $apiBase . 'question/{id}', 'verb' => 'DELETE', 'requirements' => [ + 'apiVersion' => 'v2(\.[1-5])?', + 'id' => '\d+' + ]], + ['name' => 'api#cloneQuestionLegacy', 'url' => $apiBase . 'question/clone/{id}', 'verb' => 'POST', 'requirements' => [ + 'apiVersion' => 'v2\.[3-5]', + 'id' => '\d+' + ]], // Options - [ - 'name' => 'api#newOption', - 'url' => '/api/{apiVersion}/option', - 'verb' => 'POST', - 'requirements' => [ - 'apiVersion' => 'v2(\.[1-5])?' - ] - ], + ['name' => 'api#newOptionLegacy', 'url' => $apiBase . 'option', 'verb' => 'POST', 'requirements' => [ + 'apiVersion' => 'v2(\.[1-5])?' + ]], // TODO: Remove POST in next API release - [ - 'name' => 'api#updateOption', - 'url' => '/api/{apiVersion}/option/update', - 'verb' => 'POST', - 'requirements' => [ - 'apiVersion' => 'v2(\.[1-5])?' - ] - ], - [ - 'name' => 'api#updateOption', - 'url' => '/api/{apiVersion}/option/update', - 'verb' => 'PATCH', - 'requirements' => [ - 'apiVersion' => 'v2\.[2-5]' - ] - ], - [ - 'name' => 'api#deleteOption', - 'url' => '/api/{apiVersion}/option/{id}', - 'verb' => 'DELETE', - 'requirements' => [ - 'apiVersion' => 'v2(\.[1-5])?' - ] - ], + ['name' => 'api#updateOptionLegacy', 'url' => $apiBase . 'option/update', 'verb' => 'POST', 'requirements' => [ + 'apiVersion' => 'v2(\.[1-5])?' + ]], + ['name' => 'api#updateOptionLegacy', 'url' => $apiBase . 'option/update', 'verb' => 'PATCH', 'requirements' => [ + 'apiVersion' => 'v2\.[2-5]' + ]], + ['name' => 'api#deleteOptionLegacy', 'url' => $apiBase . 'option/{id}', 'verb' => 'DELETE', 'requirements' => [ + 'apiVersion' => 'v2(\.[1-5])?', + 'id' => '\d+' + ]], // Shares - [ - 'name' => 'shareApi#newShare', - 'url' => '/api/{apiVersion}/share', - 'verb' => 'POST', - 'requirements' => [ - 'apiVersion' => 'v2(\.[1-5])?' - ] - ], - [ - 'name' => 'shareApi#deleteShare', - 'url' => '/api/{apiVersion}/share/{id}', - 'verb' => 'DELETE', - 'requirements' => [ - 'apiVersion' => 'v2(\.[1-5])?' - ] - ], + ['name' => 'shareApi#newShareLegacy', 'url' => $apiBase . 'share', 'verb' => 'POST', 'requirements' => [ + 'apiVersion' => 'v2(\.[1-5])?' + ]], + ['name' => 'shareApi#deleteShareLegacy', 'url' => $apiBase . 'share/{id}', 'verb' => 'DELETE', 'requirements' => [ + 'apiVersion' => 'v2(\.[1-5])?', + 'id' => '\d+' + ]], // TODO: Remove POST in next API release - [ - 'name' => 'shareApi#updateShare', - 'url' => '/api/{apiVersion}/share/update', - 'verb' => 'POST', - 'requirements' => [ - 'apiVersion' => 'v2\.[1-5]' - ] - ], - [ - 'name' => 'shareApi#updateShare', - 'url' => '/api/{apiVersion}/share/update', - 'verb' => 'PATCH', - 'requirements' => [ - 'apiVersion' => 'v2\.[2-5]' - ] - ], + ['name' => 'shareApi#updateShareLegacy', 'url' => $apiBase . 'share/update', 'verb' => 'POST', 'requirements' => [ + 'apiVersion' => 'v2\.[1-5]' + ]], + ['name' => 'shareApi#updateShareLegacy', 'url' => $apiBase . 'share/update', 'verb' => 'PATCH', 'requirements' => [ + 'apiVersion' => 'v2\.[2-5]' + ]], // Submissions - [ - 'name' => 'api#getSubmissions', - 'url' => '/api/{apiVersion}/submissions/{hash}', - 'verb' => 'GET', - 'requirements' => [ - 'apiVersion' => 'v2(\.[1-5])?' - ] - ], - [ - 'name' => 'api#exportSubmissions', - 'url' => '/api/{apiVersion}/submissions/export/{hash}', - 'verb' => 'GET', - 'requirements' => [ - 'apiVersion' => 'v2(\.[1-5])?' - ] - ], - [ - 'name' => 'api#exportSubmissionsToCloud', - 'url' => '/api/{apiVersion}/submissions/export', - 'verb' => 'POST', - 'requirements' => [ - 'apiVersion' => 'v2(\.[1-5])?' - ] - ], - [ - 'name' => 'api#deleteAllSubmissions', - 'url' => '/api/{apiVersion}/submissions/{formId}', - 'verb' => 'DELETE', - 'requirements' => [ - 'apiVersion' => 'v2(\.[1-5])?' - ] - ], - [ - 'name' => 'api#insertSubmission', - 'url' => '/api/{apiVersion}/submission/insert', - 'verb' => 'POST', - 'requirements' => [ - 'apiVersion' => 'v2(\.[1-5])?' - ] - ], - [ - 'name' => 'api#deleteSubmission', - 'url' => '/api/{apiVersion}/submission/{id}', - 'verb' => 'DELETE', - 'requirements' => [ - 'apiVersion' => 'v2(\.[1-5])?' - ] - ], + ['name' => 'api#getSubmissionsLegacy', 'url' => $apiBase . 'submissions/{hash}', 'verb' => 'GET', 'requirements' => [ + 'apiVersion' => 'v2(\.[1-5])?', + 'hash' => '[a-zA-Z0-9]{16}' + ]], + ['name' => 'api#exportSubmissionsLegacy', 'url' => $apiBase . 'submissions/export/{hash}', 'verb' => 'GET', 'requirements' => [ + 'apiVersion' => 'v2(\.[1-5])?', + 'hash' => '[a-zA-Z0-9]{16}' + ]], + ['name' => 'api#exportSubmissionsToCloudLegacy', 'url' => $apiBase . 'submissions/export', 'verb' => 'POST', 'requirements' => [ + 'apiVersion' => 'v2(\.[1-5])?' + ]], + ['name' => 'api#deleteAllSubmissionsLegacy', 'url' => $apiBase . 'submissions/{formId}', 'verb' => 'DELETE', 'requirements' => [ + 'apiVersion' => 'v2(\.[1-5])?', + 'formId' => '\d+' + ]], + ['name' => 'api#uploadFilesLegacy', 'url' => $apiBase . 'uploadFiles/{formId}/{questionId}', 'verb' => 'POST', 'requirements' => [ + 'apiVersion' => 'v2.5', + 'formId' => '\d+', + 'questionId' => '\d+' + ]], + ['name' => 'api#insertSubmissionLegacy', 'url' => $apiBase . 'submission/insert', 'verb' => 'POST', 'requirements' => [ + 'apiVersion' => 'v2(\.[1-5])?' + ]], + ['name' => 'api#deleteSubmissionLegacy', 'url' => $apiBase . 'submission/{id}', 'verb' => 'DELETE', 'requirements' => [ + 'apiVersion' => 'v2(\.[1-5])?', + 'id' => '\d+' + ]], // Submissions linking with file in cloud - [ - 'name' => 'api#linkFile', - 'url' => '/api/{apiVersion}/form/link/{fileFormat}', - 'verb' => 'POST', - 'requirements' => [ - 'apiVersion' => 'v2.[4-5]', - 'fileFormat' => 'csv|ods|xlsx' - ] - ], - [ - 'name' => 'api#unlinkFile', - 'url' => '/api/{apiVersion}/form/unlink', - 'verb' => 'POST', - 'requirements' => [ - 'apiVersion' => 'v2.[4-5]', - ] - ] + ['name' => 'api#linkFileLegacy', 'url' => $apiBase . 'form/link/{fileFormat}', 'verb' => 'POST', 'requirements' => [ + 'apiVersion' => 'v2.[4-5]', + 'fileFormat' => 'csv|ods|xlsx' + ]], + ['name' => 'api#unlinkFileLegacy', 'url' => $apiBase . 'form/unlink', 'verb' => 'POST', 'requirements' => [ + 'apiVersion' => 'v2.[4-5]', + ]] ] ]; diff --git a/docs/API.md b/docs/API.md index 1fb3ecd60..ca09a6ea5 100644 --- a/docs/API.md +++ b/docs/API.md @@ -41,6 +41,8 @@ This file contains the API-Documentation. For more information on the returned D - In API version 2.5 the following endpoints were introduced: - `POST /api/2.5/uploadFiles/{formId}/{questionId}` to upload files to answer before form submitting +- In API version 2.5 the following change was made: + - Options now contain a property `order` - In API version 2.4 the following endpoints were introduced: - `POST /api/2.4/form/link/{fileFormat}` to link form to a file - `POST /api/2.4/form/unlink` to unlink form from a file @@ -196,12 +198,14 @@ Returns the full-depth object of the requested form (without submissions). { "id": 1, "questionId": 1, - "text": "Option 1" + "text": "Option 1", + "order": null }, { "id": 2, "questionId": 1, - "text": "Option 2" + "text": "Option 2", + "order": null } ], "accept": [], @@ -489,18 +493,6 @@ Update a single or all properties of an option-object ``` "data": 7 ``` -### Reorder options -- Endpoint: `/api/v2.5/question/{id}/options` -- Url-Parameter: - | Parameter | Type | Description | - |-----------|---------|-------------| - | _id_ | Integer | ID of the question to reorder options for | -- Parameters: - | Parameter | Type | Description | - |-----------|---------|-------------| - | _order_ | Array | Ordered array of option IDs, the options will be reordered according to their position in this array | -- Method: `PATCH` -- Response: **Status-Code OK**. ### Delete an option @@ -647,17 +639,20 @@ Get all Submissions to a Form { "id": 1, "questionId": 1, - "text": "Option 1" + "text": "Option 1", + "order": null }, { "id": 27, "questionId": 1, - "text": "Option 2" + "text": "Option 2", + "order": null }, { "id": 30, "questionId": 1, - "text": "Option 3" + "text": "Option 3", + "order": null } ], "extraSettings": {} diff --git a/docs/API_v3.md b/docs/API_v3.md index 831ef6630..5098ea460 100644 --- a/docs/API_v3.md +++ b/docs/API_v3.md @@ -46,6 +46,7 @@ This file contains the API-Documentation. For more information on the returned D - `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 - `POST /api/v3/forms/{formId}/questions/{questionId}/options` does now accept more options at once + - `PATCH /api/v3/forms/{formId}/questions/{questionId}/options/reorder` to reorder the options - `POST /api/v3/forms/{formId}/submissions/files/{questionId}` to upload a file to a file question before submitting the form - In API version 2.5 the following endpoints were introduced: - `POST /api/v2.5/uploadFiles/{formId}/{questionId}` to upload files to answer before form submitting @@ -178,12 +179,14 @@ Returns the full-depth object of the requested form (without submissions). { "id": 1, "questionId": 1, - "text": "Option 1" + "text": "Option 1", + "order": null }, { "id": 2, "questionId": 1, - "text": "Option 2" + "text": "Option 2", + "order": null } ], "accept": [], @@ -304,12 +307,14 @@ Returns the questions and options of the given form (without submissions). { "id": 1, "questionId": 1, - "text": "Option 1" + "text": "Option 1", + "order": null }, { "id": 2, "questionId": 1, - "text": "Option 2" + "text": "Option 2", + "order": null } ], "accept": [], @@ -385,12 +390,14 @@ Returns the requested question and options of the given form (without submission { "id": 1, "questionId": 1, - "text": "Option 1" + "text": "Option 1", + "order": null }, { "id": 2, "questionId": 1, "text": "Option 2" + "order": null } ], "accept": [], @@ -549,6 +556,22 @@ Update a single or all properties of an option-object "data": 7 ``` +### Reorder options + +- Endpoint: `/api/v3/forms/{formId}/questions/{questionId}/options/reorder` +- Method: `PATCH` +- Url-Parameter: + | Parameter | Type | Description | + |-----------|---------|-------------| + | _formId_ | Integer | ID of the form containing the question and option | + | _questionId_ | Integer | ID of the question, the new option will belong to | +- Parameters: + | Parameter | Type | Description | + |-----------|---------|-------------| + | _newOrder_ | Array | Array of **all** option IDs, ordered in the desired order | +- Restrictions: The Array **must** contain all option IDs corresponding to the specified question and **must not** contain any duplicates. +- Response: Array of optionIds and their corresponding order. + ## Sharing Endpoints ### Add a new Share @@ -685,17 +708,20 @@ Get all Submissions to a Form { "id": 1, "questionId": 1, - "text": "Option 1" + "text": "Option 1", + "order": null }, { "id": 27, "questionId": 1, - "text": "Option 2" + "text": "Option 2", + "order": null }, { "id": 30, "questionId": 1, - "text": "Option 3" + "text": "Option 3", + "order": null } ], "extraSettings": {} diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index 87b52ad37..9f965fa11 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -894,6 +894,90 @@ public function deleteOption(int $formId, int $questionId, int $optionId): DataR return new DataResponse($optionId); } + /** + * Reorder options for a given question + * @param int $formId id of form + * @param int $questionId id of question + * @param Array $newOrder Order to use + */ + public function reorderOptions(int $formId, int $questionId, array $newOrder) { + $form = $this->getFormIfAllowed($formId); + if ($this->formsService->isFormArchived($form)) { + $this->logger->debug('This form is archived and can not be modified'); + throw new OCSForbiddenException(); + } + + try { + $question = $this->questionMapper->findById($questionId); + } catch (IMapperException $e) { + $this->logger->debug('Could not find form or question', ['exception' => $e]); + throw new OCSNotFoundException('Could not find form or question'); + } + + if ($question->getFormId() !== $formId) { + $this->logger->debug('The given question id doesn\'t match the form.'); + throw new OCSBadRequestException(); + } + + // 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'); + } + + $options = $this->optionMapper->findByQuestion($questionId); + + if (sizeof($options) !== sizeof($newOrder)) { + $this->logger->debug('The length of the given array does not match the number of stored options'); + throw new OCSBadRequestException('The length of the given array does not match the number of stored options'); + } + + $options = []; // Clear Array of Entities + $response = []; // Array of ['optionId' => ['order' => newOrder]] + + // Store array of Option entities and check the Options questionId & old order. + foreach ($newOrder as $arrayKey => $optionId) { + try { + $options[$arrayKey] = $this->optionMapper->findById($optionId); + } catch (IMapperException $e) { + $this->logger->debug('Could not find option. Id: {optionId}', [ + 'optionId' => $optionId + ]); + throw new OCSBadRequestException(); + } + + // Abort if a question is not part of the Form. + if ($options[$arrayKey]->getQuestionId() !== $questionId) { + $this->logger->debug('This Option is not part of the given Question: formId: {formId}', [ + 'formId' => $formId + ]); + throw new OCSBadRequestException(); + } + + // Abort if a question is already marked as deleted (order==0) + $oldOrder = $options[$arrayKey]->getOrder(); + + // Only set order, if it changed. + if ($oldOrder !== $arrayKey + 1) { + // Set Order. ArrayKey counts from zero, order counts from 1. + $options[$arrayKey]->setOrder($arrayKey + 1); + } + } + + // Write to Database + foreach ($options as $option) { + $this->optionMapper->update($option); + + $response[$option->getId()] = [ + 'order' => $option->getOrder() + ]; + } + + $this->formMapper->update($form); + + return new DataResponse($response); + } + // Submissions /** @@ -1940,7 +2024,7 @@ public function cloneQuestionLegacy(int $id): DataResponse { * @throws OCSBadRequestException * @throws OCSForbiddenException */ - public function newOptionLegacy(int $questionId, string $text, int|null $order = null): DataResponse { + public function newOptionLegacy(int $questionId, string $text): DataResponse { $this->logger->debug('Adding new option: questionId: {questionId}, text: {text}', [ 'questionId' => $questionId, 'text' => $text, @@ -1968,9 +2052,6 @@ public function newOptionLegacy(int $questionId, string $text, int|null $order = $option->setQuestionId($questionId); $option->setText($text); - if ($order !== null) { - $option->setOrder($order); - } $option = $this->optionMapper->insert($option); $this->formMapper->update($form); @@ -2074,59 +2155,11 @@ public function deleteOptionLegacy(int $id): DataResponse { } $this->optionMapper->delete($option); - - // Reorder the remaining options - $options = array_values($this->optionMapper->findByQuestion($question->getId())); - foreach ($options as $order => $option) { - $option->setOrder($order); - $this->optionMapper->update($option); - } $this->formMapper->update($form); return new DataResponse($id); } - /** - * Reorder options for a given question - * @param int $id The question ID - * @param int[] $order Order to use - */ - public function reorderOptions(int $id, array $order) { - try { - /** @var int[] */ - $order = array_flip(array_values($order)); - } catch (\Error $e) { - throw new OCSBadRequestException('Invalid order parameter'); - } - - try { - $question = $this->questionMapper->findById($id); - $form = $this->formMapper->findById($question->getFormId()); - } catch (IMapperException $e) { - $this->logger->debug('Could not find form or question', ['exception' => $e]); - throw new OCSNotFoundException('Could not find question'); - } - - if ($form->getOwnerId() !== $this->currentUser->getUID()) { - $this->logger->debug('This form is not owned by the current user'); - throw new OCSForbiddenException(); - } - - if ($this->formsService->isFormArchived($form)) { - $this->logger->debug('This form is archived and can not be modified'); - throw new OCSForbiddenException(); - } - - $options = $this->optionMapper->findByQuestion($id); - - foreach ($options as $option) { - $option->setOrder($order[$option->getId()] ?? 0); - $this->optionMapper->update($option); - } - - return new DataResponse([]); - } - /** * @CORS * @NoAdminRequired diff --git a/lib/Db/Option.php b/lib/Db/Option.php index be6d2060d..01aaac6e0 100644 --- a/lib/Db/Option.php +++ b/lib/Db/Option.php @@ -40,8 +40,8 @@ class Option extends Entity { // For 32bit PHP long integers, like IDs, are represented by floats protected int|float|null $questionId; - protected string|null $text; - protected int|null $order; + protected ?string $text; + protected ?int $order; /** * Option constructor. diff --git a/tests/Integration/Api/ApiV2Test.php b/tests/Integration/Api/ApiV2Test.php index 1abe34697..0b56b2174 100644 --- a/tests/Integration/Api/ApiV2Test.php +++ b/tests/Integration/Api/ApiV2Test.php @@ -980,10 +980,6 @@ public function dataDeleteOption() { $fullFormExpected = $this->dataGetFullForm()['getFullForm']['expected']; array_splice($fullFormExpected['questions'][1]['options'], 0, 1); - // Now the other options are reordered - $fullFormExpected['questions'][1]['options'][0]['order'] = 0; - $fullFormExpected['questions'][1]['options'][1]['order'] = 1; - return [ 'deleteOption' => [ 'fullFormExpected' => $fullFormExpected diff --git a/tests/Integration/Api/ApiV3Test.php b/tests/Integration/Api/ApiV3Test.php index 73c310bdf..157662927 100644 --- a/tests/Integration/Api/ApiV3Test.php +++ b/tests/Integration/Api/ApiV3Test.php @@ -464,13 +464,16 @@ public function dataGetFullForm() { 'order' => 2, 'options' => [ [ - 'text' => 'Option 1' + 'text' => 'Option 1', + 'order' => null, ], [ - 'text' => 'Option 2' + 'text' => 'Option 2', + 'order' => null, ], [ - 'text' => '' + 'text' => '', + 'order' => null, ] ], 'accept' => [], @@ -870,7 +873,8 @@ public function dataCreateNewOption() { 'newOption' => [ 'expected' => [ // 'questionId' => Done dynamically below. - 'text' => 'A new Option.' + 'text' => 'A new Option.', + 'order' => null, ] ] ];