From 7aba08d0956378115bb425f021ce33f9f3fbc771 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Mon, 22 Apr 2024 01:28:59 +0200 Subject: [PATCH] feat: Allow to reorder options for "multiple" question type Signed-off-by: Ferdinand Thiessen Signed-off-by: Christian Hartmann --- appinfo/routes.php | 70 +++++----- docs/API.md | 73 ++++++----- docs/API_v3.md | 42 ++++-- lib/Controller/ApiController.php | 122 ++++++++++++++++++ lib/Db/Option.php | 19 ++- lib/Db/OptionMapper.php | 14 +- lib/Db/QuestionMapper.php | 6 +- .../Version040300Date20240420155356.php | 68 ++++++++++ tests/Integration/Api/ApiV2Test.php | 23 +++- tests/Integration/Api/ApiV3Test.php | 23 +++- tests/Integration/IntegrationBase.php | 3 +- tests/Unit/Service/FormsServiceTest.php | 6 +- 12 files changed, 363 insertions(+), 106 deletions(-) create mode 100644 lib/Migration/Version040300Date20240420155356.php diff --git a/appinfo/routes.php b/appinfo/routes.php index e255eae8..bf9caf49 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -59,7 +59,7 @@ return [ // CORS Preflight ['name' => 'api#preflightedCors', 'url' => $apiBase . '{path}', 'verb' => 'OPTIONS', 'requirements' => [ 'path' => '.+', - 'apiVersion' => 'v2(\.[1-4])?|v3' + 'apiVersion' => 'v2(\.[1-5])?|v3' ]], // API routes v3 @@ -84,7 +84,7 @@ return [ // ['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], @@ -107,113 +107,113 @@ return [ // Legacy v2 routes (TODO: remove with Forms v5) // Forms ['name' => 'api#getFormsLegacy', 'url' => $apiBase . 'forms', 'verb' => 'GET', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-4])?' + 'apiVersion' => 'v2(\.[1-5])?' ]], ['name' => 'api#newFormLegacy', 'url' => $apiBase . 'form', 'verb' => 'POST', 'requirements' => [ - 'apiVersion_path' => 'v2(\.[1-4])?' + 'apiVersion_path' => 'v2(\.[1-5])?' ]], ['name' => 'api#getFormLegacy', 'url' => $apiBase . 'form/{id}', 'verb' => 'GET', 'requirements' => [ - 'apiVersion_path' => 'v2(\.[1-4])?', + 'apiVersion_path' => 'v2(\.[1-5])?', 'id' => '\d+' ]], ['name' => 'api#cloneFormLegacy', 'url' => $apiBase . 'form/clone/{id}', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-4])?', + 'apiVersion' => 'v2(\.[1-5])?', 'id' => '\d+' ]], ['name' => 'api#updateFormLegacy', 'url' => $apiBase . 'form/update', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-4])?' + 'apiVersion' => 'v2(\.[1-5])?' ]], ['name' => 'api#updateFormLegacy', 'url' => $apiBase . 'form/update', 'verb' => 'PATCH', 'requirements' => [ - 'apiVersion' => 'v2\.[2-4]' + 'apiVersion' => 'v2\.[2-5]' ]], ['name' => 'api#transferOwnerLegacy', 'url' => $apiBase . 'form/transfer', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2\.[2-4]' + 'apiVersion' => 'v2\.[2-5]' ]], ['name' => 'api#deleteFormLegacy', 'url' => $apiBase . 'form/{id}', 'verb' => 'DELETE', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-4])?', + 'apiVersion' => 'v2(\.[1-5])?', 'id' => '\d+' ]], ['name' => 'api#getPartialFormLegacy', 'url' => $apiBase . 'partial_form/{hash}', 'verb' => 'GET', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-4])?', + 'apiVersion' => 'v2(\.[1-5])?', 'hash' => '[a-zA-Z0-9]{16}' ]], ['name' => 'api#getSharedFormsLegacy', 'url' => $apiBase . 'shared_forms', 'verb' => 'GET', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-4])?' + 'apiVersion' => 'v2(\.[1-5])?' ]], // Questions ['name' => 'api#newQuestionLegacy', 'url' => $apiBase . 'question', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-4])?' + 'apiVersion' => 'v2(\.[1-5])?' ]], // TODO: Remove POST in next API release ['name' => 'api#updateQuestionLegacy', 'url' => $apiBase . 'question/update', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-4])?' + 'apiVersion' => 'v2(\.[1-5])?' ]], ['name' => 'api#updateQuestionLegacy', 'url' => $apiBase . 'question/update', 'verb' => 'PATCH', 'requirements' => [ - 'apiVersion' => 'v2\.[2-4]' + 'apiVersion' => 'v2\.[2-5]' ]], // TODO: Remove POST in next API release ['name' => 'api#reorderQuestionsLegacy', 'url' => $apiBase . 'question/reorder', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-4])?' + 'apiVersion' => 'v2(\.[1-5])?' ]], ['name' => 'api#reorderQuestionsLegacy', 'url' => $apiBase . 'question/reorder', 'verb' => 'PUT', 'requirements' => [ - 'apiVersion' => 'v2\.[2-4]' + 'apiVersion' => 'v2\.[2-5]' ]], ['name' => 'api#deleteQuestionLegacy', 'url' => $apiBase . 'question/{id}', 'verb' => 'DELETE', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-4])?', + 'apiVersion' => 'v2(\.[1-5])?', 'id' => '\d+' ]], ['name' => 'api#cloneQuestionLegacy', 'url' => $apiBase . 'question/clone/{id}', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2\.[3-4]', + 'apiVersion' => 'v2\.[3-5]', 'id' => '\d+' ]], // Options ['name' => 'api#newOptionLegacy', 'url' => $apiBase . 'option', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-4])?' + 'apiVersion' => 'v2(\.[1-5])?' ]], // TODO: Remove POST in next API release ['name' => 'api#updateOptionLegacy', 'url' => $apiBase . 'option/update', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-4])?' + 'apiVersion' => 'v2(\.[1-5])?' ]], ['name' => 'api#updateOptionLegacy', 'url' => $apiBase . 'option/update', 'verb' => 'PATCH', 'requirements' => [ - 'apiVersion' => 'v2\.[2-4]' + 'apiVersion' => 'v2\.[2-5]' ]], ['name' => 'api#deleteOptionLegacy', 'url' => $apiBase . 'option/{id}', 'verb' => 'DELETE', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-4])?', + 'apiVersion' => 'v2(\.[1-5])?', 'id' => '\d+' ]], // Shares ['name' => 'shareApi#newShareLegacy', 'url' => $apiBase . 'share', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-4])?' + 'apiVersion' => 'v2(\.[1-5])?' ]], ['name' => 'shareApi#deleteShareLegacy', 'url' => $apiBase . 'share/{id}', 'verb' => 'DELETE', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-4])?', + 'apiVersion' => 'v2(\.[1-5])?', 'id' => '\d+' ]], // TODO: Remove POST in next API release ['name' => 'shareApi#updateShareLegacy', 'url' => $apiBase . 'share/update', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2\.[1-4]' + 'apiVersion' => 'v2\.[1-5]' ]], ['name' => 'shareApi#updateShareLegacy', 'url' => $apiBase . 'share/update', 'verb' => 'PATCH', 'requirements' => [ - 'apiVersion' => 'v2\.[2-4]' + 'apiVersion' => 'v2\.[2-5]' ]], // Submissions ['name' => 'api#getSubmissionsLegacy', 'url' => $apiBase . 'submissions/{hash}', 'verb' => 'GET', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-4])?', + 'apiVersion' => 'v2(\.[1-5])?', 'hash' => '[a-zA-Z0-9]{16}' ]], ['name' => 'api#exportSubmissionsLegacy', 'url' => $apiBase . 'submissions/export/{hash}', 'verb' => 'GET', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-4])?', + 'apiVersion' => 'v2(\.[1-5])?', 'hash' => '[a-zA-Z0-9]{16}' ]], ['name' => 'api#exportSubmissionsToCloudLegacy', 'url' => $apiBase . 'submissions/export', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-4])?' + 'apiVersion' => 'v2(\.[1-5])?' ]], ['name' => 'api#deleteAllSubmissionsLegacy', 'url' => $apiBase . 'submissions/{formId}', 'verb' => 'DELETE', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-4])?', + 'apiVersion' => 'v2(\.[1-5])?', 'formId' => '\d+' ]], ['name' => 'api#uploadFilesLegacy', 'url' => $apiBase . 'uploadFiles/{formId}/{questionId}', 'verb' => 'POST', 'requirements' => [ @@ -222,19 +222,19 @@ return [ 'questionId' => '\d+' ]], ['name' => 'api#insertSubmissionLegacy', 'url' => $apiBase . 'submission/insert', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-4])?' + 'apiVersion' => 'v2(\.[1-5])?' ]], ['name' => 'api#deleteSubmissionLegacy', 'url' => $apiBase . 'submission/{id}', 'verb' => 'DELETE', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-4])?', + 'apiVersion' => 'v2(\.[1-5])?', 'id' => '\d+' ]], // Submissions linking with file in cloud ['name' => 'api#linkFileLegacy', 'url' => $apiBase . 'form/link/{fileFormat}', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2.4', + 'apiVersion' => 'v2.[4-5]', 'fileFormat' => 'csv|ods|xlsx' ]], ['name' => 'api#unlinkFileLegacy', 'url' => $apiBase . 'form/unlink', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2.4', + 'apiVersion' => 'v2.[4-5]', ]] ] ]; diff --git a/docs/API.md b/docs/API.md index 97d22e38..ca09a6ea 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 @@ -58,7 +60,7 @@ This file contains the API-Documentation. For more information on the returned D Returns condensed objects of all Forms beeing owned by the authenticated user. -- Endpoint: `/api/v2.4/forms` +- Endpoint: `/api/v2.5/forms` - Method: `GET` - Parameters: None - Response: Array of condensed Form Objects, sorted as newest first. @@ -98,7 +100,7 @@ Returns condensed objects of all Forms beeing owned by the authenticated user. Returns condensed objects of all Forms, that are shared & shown to the authenticated user and that have not expired yet. -- Endpoint: `/api/v2.4/shared_forms` +- Endpoint: `/api/v2.5/shared_forms` - Method: `GET` - Parameters: None - Response: Array of condensed Form Objects, sorted as newest first, similar to [List owned Forms](#list-owned-forms). @@ -111,7 +113,7 @@ See above, 'List owned forms' Returns a single partial form object, corresponding to owned/shared form-listings. -- Endpoint: `/api/v2.4/partial_form/{hash}` +- Endpoint: `/api/v2.5/partial_form/{hash}` - Method: `GET` - Url-Parameter: | Parameter | Type | Description | @@ -135,7 +137,7 @@ Returns a single partial form object, corresponding to owned/shared form-listing ### Create a new Form -- Endpoint: `/api/v2.4/form` +- Endpoint: `/api/v2.5/form` - Method: `POST` - Parameters: None - Response: The new form object, similar to requesting an existing form. @@ -148,7 +150,7 @@ See next section, 'Request full data of a form' Returns the full-depth object of the requested form (without submissions). -- Endpoint: `/api/v2.4/form/{id}` +- Endpoint: `/api/v2.5/form/{id}` - Url-Parameter: | Parameter | Type | Description | |-----------|---------|-------------| @@ -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": [], @@ -243,7 +247,7 @@ Returns the full-depth object of the requested form (without submissions). Creates a clone of a form (without submissions). -- Endpoint: `/api/v2.4/form/clone/{id}` +- Endpoint: `/api/v2.5/form/clone/{id}` - Url-Parameter: | Parameter | Type | Description | |-----------|---------|-------------| @@ -259,7 +263,7 @@ See section 'Request full data of a form'. Update a single or multiple properties of a form-object. Concerns **only** the Form-Object, properties of Questions, Options and Submissions, as well as their creation or deletion, are handled separately. -- Endpoint: `/api/v2.4/form/update` +- Endpoint: `/api/v2.5/form/update` - Method: `PATCH` - _Method: `POST` deprecated_ - Parameters: @@ -278,7 +282,7 @@ Update a single or multiple properties of a form-object. Concerns **only** the F Transfer the ownership of a form to another user -- Endpoint: `/api/v2.4/form/transfer` +- Endpoint: `/api/v2.5/form/transfer` - Method: `POST` - Parameters: | Parameter | Type | Description | @@ -294,7 +298,7 @@ Transfer the ownership of a form to another user ### Delete a form -- Endpoint: `/api/v2.4/form/{id}` +- Endpoint: `/api/v2.5/form/{id}` - Url-Parameter: | Parameter | Type | Description | |-----------|---------|-------------| @@ -308,7 +312,7 @@ Transfer the ownership of a form to another user ### Link a form to a file -- Endpoint: `/api/v2.4/form/link/{fileFormat}` +- Endpoint: `/api/v2.5/form/link/{fileFormat}` - Url-Parameter: | Parameter | Type | Description | |--------------|---------|--------------| @@ -332,7 +336,7 @@ Transfer the ownership of a form to another user ### Unlink file from form -- Endpoint: `/api/v2.4/form/unlink` +- Endpoint: `/api/v2.5/form/unlink` - Method: `POST` - Parameters: | Parameter | Type | Description | @@ -346,7 +350,7 @@ Contains only manipulative question-endpoints. To retrieve questions, request th ### Create a new question -- Endpoint: `/api/v2.4/question` +- Endpoint: `/api/v2.5/question` - Method: `POST` - Parameters: | Parameter | Type | Optional | Description | @@ -374,7 +378,7 @@ Contains only manipulative question-endpoints. To retrieve questions, request th Update a single or multiple properties of a question-object. -- Endpoint: `/api/v2.4/question/update` +- Endpoint: `/api/v2.5/question/update` - Method: `PATCH` - _Method: `POST` deprecated_ - Parameters: @@ -393,7 +397,7 @@ Update a single or multiple properties of a question-object. Reorders all Questions of a single form -- Endpoint: `/api/v2.4/question/reorder` +- Endpoint: `/api/v2.5/question/reorder` - Method: `PUT` - _Method: `POST` deprecated_ - Parameters: @@ -420,7 +424,7 @@ Reorders all Questions of a single form ### Delete a question -- Endpoint: `/api/v2.4/question/{id}` +- Endpoint: `/api/v2.5/question/{id}` - Url-Parameter: | Parameter | Type | Description | |-----------|---------|-------------| @@ -436,7 +440,7 @@ Reorders all Questions of a single form Creates a clone of a question with all its options. -- Endpoint: `/api/v2.4/question/clone/{id}` +- Endpoint: `/api/v2.5/question/clone/{id}` - Url-Parameter: | Parameter | Type | Description | |-----------|---------|-------------| @@ -454,7 +458,7 @@ Contains only manipulative question-endpoints. To retrieve options, request the ### Create a new Option -- Endpoint: `/api/v2.4/option` +- Endpoint: `/api/v2.5/option` - Method: `POST` - Parameters: | Parameter | Type | Description | @@ -475,7 +479,7 @@ Contains only manipulative question-endpoints. To retrieve options, request the Update a single or all properties of an option-object -- Endpoint: `/api/v2.4/option/update` +- Endpoint: `/api/v2.5/option/update` - Method: `PATCH` - _Method: `POST` deprecated_ - Parameters: @@ -492,7 +496,7 @@ Update a single or all properties of an option-object ### Delete an option -- Endpoint: `/api/v2.4/option/{id}` +- Endpoint: `/api/v2.5/option/{id}` - Url-Parameter: | Parameter | Type | Description | |-----------|---------|-------------| @@ -508,7 +512,7 @@ Update a single or all properties of an option-object ### Add a new Share -- Endpoint: `/api/v2.4/share` +- Endpoint: `/api/v2.5/share` - Method: `POST` - Parameters: | Parameter | Type | Description | @@ -532,7 +536,7 @@ Update a single or all properties of an option-object ### Delete a Share -- Endpoint: `/api/v2.4/share/{id}` +- Endpoint: `/api/v2.5/share/{id}` - Url-Parameter: | Parameter | Type | Description | |-----------|---------|-------------| @@ -546,7 +550,7 @@ Update a single or all properties of an option-object ### Update a Share -- Endpoint: `/api/v2.4/share/update` +- Endpoint: `/api/v2.5/share/update` - Parameters: | Parameter | Type | Description | |------------------|----------|-------------| @@ -569,7 +573,7 @@ Update a single or all properties of an option-object Get all Submissions to a Form -- Endpoint: `/api/v2.4/submissions/{hash}` +- Endpoint: `/api/v2.5/submissions/{hash}` - Url-Parameter: | Parameter | Type | Description | |-----------|---------|-------------| @@ -635,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": {} @@ -668,7 +675,7 @@ Get all Submissions to a Form Returns all submissions to the form in form of a csv-file. -- Endpoint: `/api/v2.4/submissions/export/{hash}` +- Endpoint: `/api/v2.5/submissions/export/{hash}` - Url-Parameter: | Parameter | Type | Description | |--------------|---------|-------------| @@ -687,7 +694,7 @@ Returns all submissions to the form in form of a csv-file. Creates a csv file and stores it to the cloud, resp. Files-App. -- Endpoint: `/api/v2.4/submissions/export` +- Endpoint: `/api/v2.5/submissions/export` - Method: `POST` - Parameters: | Parameter | Type | Description | @@ -705,7 +712,7 @@ Creates a csv file and stores it to the cloud, resp. Files-App. Delete all Submissions to a form -- Endpoint: `/api/v2.4/submissions/{formId}` +- Endpoint: `/api/v2.5/submissions/{formId}` - Url-Parameter: | Parameter | Type | Description | |-----------|---------|-------------| @@ -739,7 +746,7 @@ Upload a files to answer before form submitting Store Submission to Database -- Endpoint: `/api/v2.4/submission/insert` +- Endpoint: `/api/v2.5/submission/insert` - Method: `POST` - Parameters: | Parameter | Type | Description | @@ -771,7 +778,7 @@ Store Submission to Database ### Delete a single Submission -- Endpoint: `/api/v2.4/submission/{id}` +- Endpoint: `/api/v2.5/submission/{id}` - Url-Parameter: | Parameter | Type | Description | |-----------|---------|-------------| diff --git a/docs/API_v3.md b/docs/API_v3.md index 831ef663..5098ea46 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 a4a998cc..f3b0584a 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -764,12 +764,22 @@ class ApiController extends OCSController { throw new OCSBadRequestException(); } + // Retrieve all options sorted by 'order'. Takes the order of the last array-element and adds one. + $options = $this->optionMapper->findByQuestion($questionId); + $lastOption = array_pop($options); + if ($lastOption) { + $optionOrder = $lastOption->getOrder() + 1; + } else { + $optionOrder = 1; + } + $addedOptions = []; foreach ($optionTexts as $text) { $option = new Option(); $option->setQuestionId($questionId); $option->setText($text); + $option->setOrder($optionOrder++); try { $option = $this->optionMapper->insert($option); @@ -889,11 +899,104 @@ class ApiController extends OCSController { } $this->optionMapper->delete($option); + + // Reorder the remaining options + $options = array_values($this->optionMapper->findByQuestion($questionId)); + foreach ($options as $order => $option) { + // Always start order with 1 + $option->setOrder($order + 1); + $this->optionMapper->update($option); + } + $this->formMapper->update($form); 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 /** @@ -1964,10 +2067,20 @@ class ApiController extends OCSController { throw new OCSForbiddenException(); } + // Retrieve all options sorted by 'order'. Takes the order of the last array-element and adds one. + $options = $this->optionMapper->findByQuestion($questionId); + $lastOption = array_pop($options); + if ($lastOption) { + $optionOrder = $lastOption->getOrder() + 1; + } else { + $optionOrder = 1; + } + $option = new Option(); $option->setQuestionId($questionId); $option->setText($text); + $option->setOrder($optionOrder); $option = $this->optionMapper->insert($option); $this->formMapper->update($form); @@ -2071,6 +2184,15 @@ class ApiController extends OCSController { } $this->optionMapper->delete($option); + + // Reorder the remaining options + $options = array_values($this->optionMapper->findByQuestion($option->getQuestionId())); + foreach ($options as $order => $option) { + // Always start order with 1 + $option->setOrder($order + 1); + $this->optionMapper->update($option); + } + $this->formMapper->update($form); return new DataResponse($id); diff --git a/lib/Db/Option.php b/lib/Db/Option.php index debaa5b9..01aaac6e 100644 --- a/lib/Db/Option.php +++ b/lib/Db/Option.php @@ -29,23 +29,29 @@ namespace OCA\Forms\Db; use OCP\AppFramework\Db\Entity; /** - * @method int getQuestionId() - * @method void setQuestionId(int $value) + * @method int|float getQuestionId() + * @method void setQuestionId(int|float $value) * @method string getText() * @method void setText(string $value) + * @method int getOrder(); + * @method void setOrder(int $value) */ class Option extends Entity { - /** @var int */ - protected $questionId; - /** @var string */ - protected $text; + // For 32bit PHP long integers, like IDs, are represented by floats + protected int|float|null $questionId; + protected ?string $text; + protected ?int $order; /** * Option constructor. */ public function __construct() { + $this->questionId = null; + $this->text = null; + $this->order = null; $this->addType('questionId', 'integer'); + $this->addType('order', 'integer'); $this->addType('text', 'string'); } @@ -53,6 +59,7 @@ class Option extends Entity { return [ 'id' => $this->getId(), 'questionId' => $this->getQuestionId(), + 'order' => $this->getOrder(), 'text' => (string)$this->getText(), ]; } diff --git a/lib/Db/OptionMapper.php b/lib/Db/OptionMapper.php index 3881e3f2..eeee22b0 100644 --- a/lib/Db/OptionMapper.php +++ b/lib/Db/OptionMapper.php @@ -27,7 +27,6 @@ declare(strict_types=1); namespace OCA\Forms\Db; -use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\QBMapper; use OCP\IDBConnection; @@ -45,11 +44,10 @@ class OptionMapper extends QBMapper { } /** - * @param int $questionId - * @throws DoesNotExistException if not found + * @param int|float $questionId * @return Option[] */ - public function findByQuestion(int $questionId): array { + public function findByQuestion(int|float $questionId): array { $qb = $this->db->getQueryBuilder(); $qb->select('*') @@ -57,7 +55,8 @@ class OptionMapper extends QBMapper { ->where( $qb->expr()->eq('question_id', $qb->createNamedParameter($questionId)) ) - ->orderBy('id'); + ->orderBy('order') + ->addOrderBy('id'); return $this->findEntities($qb); } @@ -73,7 +72,10 @@ class OptionMapper extends QBMapper { $qb->executeStatement(); } - public function findById(int $optionId): Option { + /** + * @param int|float $optionId The option ID (int but for 32bit systems PHP will use float) + */ + public function findById(int|float $optionId): Option { $qb = $this->db->getQueryBuilder(); $qb->select('*') diff --git a/lib/Db/QuestionMapper.php b/lib/Db/QuestionMapper.php index d6c70722..65ac6a71 100644 --- a/lib/Db/QuestionMapper.php +++ b/lib/Db/QuestionMapper.php @@ -90,7 +90,11 @@ class QuestionMapper extends QBMapper { $qb->executeStatement(); } - public function findById(int $questionId): Question { + /** + * Find Question by its ID + * @param int|float $questionId The question ID (int but for 32bit systems PHP uses float) + */ + public function findById(int|float $questionId): Question { $qb = $this->db->getQueryBuilder(); $qb->select('*') diff --git a/lib/Migration/Version040300Date20240420155356.php b/lib/Migration/Version040300Date20240420155356.php new file mode 100644 index 00000000..aed46ee9 --- /dev/null +++ b/lib/Migration/Version040300Date20240420155356.php @@ -0,0 +1,68 @@ + + * + * @author Ferdinand Thiessen + * + * @license AGPL-3.0-or-later + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Forms\Migration; + +use Closure; +use OCP\DB\ISchemaWrapper; +use OCP\DB\Types; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +/** + * Add "order" column for options + */ +class Version040300Date20240420155356 extends SimpleMigrationStep { + + /** + * @param IOutput $output + * @param Closure(): ISchemaWrapper $schemaClosure + * @param array $options + * @return null|ISchemaWrapper + */ + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + $table = $schema->getTable('forms_v2_options'); + + // Abort if already existing. + if ($table->hasColumn('order')) { + return null; + } + + // Create new column + $table->addColumn('order', Types::INTEGER, [ + 'notnull' => false, + 'default' => null, + 'unsigned' => true, + ]); + + // Add index for better performance + $table->addIndex(['question_id', 'order'], 'forms_options_question_order'); + + return $schema; + } +} diff --git a/tests/Integration/Api/ApiV2Test.php b/tests/Integration/Api/ApiV2Test.php index f435030f..d7a55804 100644 --- a/tests/Integration/Api/ApiV2Test.php +++ b/tests/Integration/Api/ApiV2Test.php @@ -84,13 +84,16 @@ class ApiV2Test extends IntegrationBase { 'order' => 2, 'options' => [ [ - 'text' => 'Option 1' + 'text' => 'Option 1', + 'order' => 1 ], [ - 'text' => 'Option 2' + 'text' => 'Option 2', + 'order' => 2 ], [ - 'text' => '' + 'text' => '', + 'order' => 3 ] ], 'accept' => [], @@ -496,13 +499,16 @@ class ApiV2Test extends IntegrationBase { 'order' => 2, 'options' => [ [ - 'text' => 'Option 1' + 'text' => 'Option 1', + 'order' => 1, ], [ - 'text' => 'Option 2' + 'text' => 'Option 2', + 'order' => 2, ], [ - 'text' => '' + 'text' => '', + 'order' => 3, ] ], 'accept' => [], @@ -906,7 +912,8 @@ class ApiV2Test extends IntegrationBase { 'newOption' => [ 'expected' => [ // 'questionId' => Done dynamically below. - 'text' => 'A new Option.' + 'text' => 'A new Option.', + 'order' => 4, ] ] ]; @@ -995,6 +1002,8 @@ class ApiV2Test extends IntegrationBase { $this->assertEquals($this->testForms[0]['questions'][1]['options'][0]['id'], $data); $fullFormExpected['lastUpdated'] = time(); + $fullFormExpected['questions'][1]['options'][0]['order'] = 1; + $fullFormExpected['questions'][1]['options'][1]['order'] = 2; $this->testGetFullForm($fullFormExpected); } diff --git a/tests/Integration/Api/ApiV3Test.php b/tests/Integration/Api/ApiV3Test.php index 73c310bd..a36079c7 100644 --- a/tests/Integration/Api/ApiV3Test.php +++ b/tests/Integration/Api/ApiV3Test.php @@ -84,13 +84,16 @@ class ApiV3Test extends IntegrationBase { 'order' => 2, 'options' => [ [ - 'text' => 'Option 1' + 'text' => 'Option 1', + 'order' => 1 ], [ - 'text' => 'Option 2' + 'text' => 'Option 2', + 'order' => 2 ], [ - 'text' => '' + 'text' => '', + 'order' => 3 ] ], 'accept' => [], @@ -464,13 +467,16 @@ class ApiV3Test extends IntegrationBase { 'order' => 2, 'options' => [ [ - 'text' => 'Option 1' + 'text' => 'Option 1', + 'order' => 1, ], [ - 'text' => 'Option 2' + 'text' => 'Option 2', + 'order' => 2, ], [ - 'text' => '' + 'text' => '', + 'order' => 3, ] ], 'accept' => [], @@ -870,7 +876,8 @@ class ApiV3Test extends IntegrationBase { 'newOption' => [ 'expected' => [ // 'questionId' => Done dynamically below. - 'text' => 'A new Option.' + 'text' => 'A new Option.', + 'order' => 4, ] ] ]; @@ -957,6 +964,8 @@ class ApiV3Test extends IntegrationBase { $this->assertEquals($this->testForms[0]['questions'][1]['options'][0]['id'], $data); $fullFormExpected['lastUpdated'] = time(); + $fullFormExpected['questions'][1]['options'][0]['order'] = 1; + $fullFormExpected['questions'][1]['options'][1]['order'] = 2; $this->testGetFullForm($fullFormExpected); } diff --git a/tests/Integration/IntegrationBase.php b/tests/Integration/IntegrationBase.php index 5d2e5078..3ad35615 100644 --- a/tests/Integration/IntegrationBase.php +++ b/tests/Integration/IntegrationBase.php @@ -104,7 +104,8 @@ class IntegrationBase extends TestCase { $qb->insert('forms_v2_options') ->values([ 'question_id' => $qb->createNamedParameter($questionId, IQueryBuilder::PARAM_INT), - 'text' => $qb->createNamedParameter($option['text'], IQueryBuilder::PARAM_STR) + 'text' => $qb->createNamedParameter($option['text'], IQueryBuilder::PARAM_STR), + 'order' => $qb->createNamedParameter($option['order'], IQueryBuilder::PARAM_INT) ]); $qb->executeStatement(); $this->testForms[$index]['questions'][$qIndex]['options'][$oIndex]['id'] = $qb->getLastInsertId(); diff --git a/tests/Unit/Service/FormsServiceTest.php b/tests/Unit/Service/FormsServiceTest.php index 987ded79..b58fb81d 100644 --- a/tests/Unit/Service/FormsServiceTest.php +++ b/tests/Unit/Service/FormsServiceTest.php @@ -228,12 +228,14 @@ class FormsServiceTest extends TestCase { [ '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' => [],