From 5421000f7ff4c7cd47f289a79ba4b61f42cf31e2 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Mon, 18 Dec 2023 21:45:30 +0100 Subject: [PATCH] fix: Adjust API version, fix API test and make code naming consistent Signed-off-by: Ferdinand Thiessen --- appinfo/routes.php | 64 ++++++++++----------- docs/API.md | 15 +++++ lib/Controller/ApiController.php | 9 +-- src/components/Questions/Question.vue | 8 +-- src/mixins/QuestionMixin.js | 8 +-- src/views/Create.vue | 10 ++-- tests/Integration/Api/ApiV2Test.php | 29 ++++------ tests/Unit/Controller/ApiControllerTest.php | 18 ++++++ 8 files changed, 93 insertions(+), 68 deletions(-) diff --git a/appinfo/routes.php b/appinfo/routes.php index 51df82a45..f93510885 100644 --- a/appinfo/routes.php +++ b/appinfo/routes.php @@ -73,7 +73,7 @@ 'verb' => 'OPTIONS', 'requirements' => [ 'path' => '.+', - 'apiVersion' => 'v2(\.[1-2])?' + 'apiVersion' => 'v2(\.[1-3])?' ] ], @@ -83,7 +83,7 @@ 'url' => '/api/{apiVersion}/forms', 'verb' => 'GET', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-2])?' + 'apiVersion' => 'v2(\.[1-3])?' ] ], [ @@ -91,7 +91,7 @@ 'url' => '/api/{apiVersion}/form', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-2])?' + 'apiVersion' => 'v2(\.[1-3])?' ] ], [ @@ -99,7 +99,7 @@ 'url' => '/api/{apiVersion}/form/{id}', 'verb' => 'GET', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-2])?' + 'apiVersion' => 'v2(\.[1-3])?' ] ], [ @@ -107,7 +107,7 @@ 'url' => '/api/{apiVersion}/form/clone/{id}', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-2])?' + 'apiVersion' => 'v2(\.[1-3])?' ] ], // TODO: Remove POST in next API release @@ -116,7 +116,7 @@ 'url' => '/api/{apiVersion}/form/update', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-2])?' + 'apiVersion' => 'v2(\.[1-3])?' ] ], [ @@ -124,7 +124,7 @@ 'url' => '/api/{apiVersion}/form/update', 'verb' => 'PATCH', 'requirements' => [ - 'apiVersion' => 'v2.2' + 'apiVersion' => 'v2\.[2-3]' ] ], [ @@ -132,7 +132,7 @@ 'url' => '/api/{apiVersion}/form/transfer', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2.2' + 'apiVersion' => 'v2\.[2-3]' ] ], [ @@ -140,7 +140,7 @@ 'url' => '/api/{apiVersion}/form/{id}', 'verb' => 'DELETE', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-2])?' + 'apiVersion' => 'v2(\.[1-3])?' ] ], [ @@ -148,7 +148,7 @@ 'url' => '/api/{apiVersion}/partial_form/{hash}', 'verb' => 'GET', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-2])?' + 'apiVersion' => 'v2(\.[1-3])?' ] ], [ @@ -156,7 +156,7 @@ 'url' => '/api/{apiVersion}/shared_forms', 'verb' => 'GET', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-2])?' + 'apiVersion' => 'v2(\.[1-3])?' ] ], @@ -166,7 +166,7 @@ 'url' => '/api/{apiVersion}/question', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-2])?' + 'apiVersion' => 'v2(\.[1-3])?' ] ], // TODO: Remove POST in next API release @@ -175,7 +175,7 @@ 'url' => '/api/{apiVersion}/question/update', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-2])?' + 'apiVersion' => 'v2(\.[1-3])?' ] ], [ @@ -183,7 +183,7 @@ 'url' => '/api/{apiVersion}/question/update', 'verb' => 'PATCH', 'requirements' => [ - 'apiVersion' => 'v2.2' + 'apiVersion' => 'v2\.[2-3]' ] ], // TODO: Remove POST in next API release @@ -192,7 +192,7 @@ 'url' => '/api/{apiVersion}/question/reorder', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-2])?' + 'apiVersion' => 'v2(\.[1-3])?' ] ], [ @@ -200,7 +200,7 @@ 'url' => '/api/{apiVersion}/question/reorder', 'verb' => 'PUT', 'requirements' => [ - 'apiVersion' => 'v2.2' + 'apiVersion' => 'v2\.[2-3]' ] ], [ @@ -208,15 +208,15 @@ 'url' => '/api/{apiVersion}/question/{id}', 'verb' => 'DELETE', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-2])?' + 'apiVersion' => 'v2(\.[1-3])?' ] ], [ - 'name' => 'api#duplicateQuestion', + 'name' => 'api#cloneQuestion', 'url' => '/api/{apiVersion}/question/clone/{id}', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2.1' + 'apiVersion' => 'v2.3' ] ], @@ -226,7 +226,7 @@ 'url' => '/api/{apiVersion}/option', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-2])?' + 'apiVersion' => 'v2(\.[1-3])?' ] ], // TODO: Remove POST in next API release @@ -235,7 +235,7 @@ 'url' => '/api/{apiVersion}/option/update', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-2])?' + 'apiVersion' => 'v2(\.[1-3])?' ] ], [ @@ -243,7 +243,7 @@ 'url' => '/api/{apiVersion}/option/update', 'verb' => 'PATCH', 'requirements' => [ - 'apiVersion' => 'v2.2' + 'apiVersion' => 'v2\.[2-3]' ] ], [ @@ -251,7 +251,7 @@ 'url' => '/api/{apiVersion}/option/{id}', 'verb' => 'DELETE', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-2])?' + 'apiVersion' => 'v2(\.[1-3])?' ] ], @@ -261,7 +261,7 @@ 'url' => '/api/{apiVersion}/share', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-2])?' + 'apiVersion' => 'v2(\.[1-3])?' ] ], [ @@ -269,7 +269,7 @@ 'url' => '/api/{apiVersion}/share/{id}', 'verb' => 'DELETE', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-2])?' + 'apiVersion' => 'v2(\.[1-3])?' ] ], // TODO: Remove POST in next API release @@ -286,7 +286,7 @@ 'url' => '/api/{apiVersion}/share/update', 'verb' => 'PATCH', 'requirements' => [ - 'apiVersion' => 'v2.2' + 'apiVersion' => 'v2\.[2-3]' ] ], @@ -296,7 +296,7 @@ 'url' => '/api/{apiVersion}/submissions/{hash}', 'verb' => 'GET', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-2])?' + 'apiVersion' => 'v2(\.[1-3])?' ] ], [ @@ -304,7 +304,7 @@ 'url' => '/api/{apiVersion}/submissions/export/{hash}', 'verb' => 'GET', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-2])?' + 'apiVersion' => 'v2(\.[1-3])?' ] ], [ @@ -312,7 +312,7 @@ 'url' => '/api/{apiVersion}/submissions/export', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-2])?' + 'apiVersion' => 'v2(\.[1-3])?' ] ], [ @@ -320,7 +320,7 @@ 'url' => '/api/{apiVersion}/submissions/{formId}', 'verb' => 'DELETE', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-2])?' + 'apiVersion' => 'v2(\.[1-3])?' ] ], [ @@ -328,7 +328,7 @@ 'url' => '/api/{apiVersion}/submission/insert', 'verb' => 'POST', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-2])?' + 'apiVersion' => 'v2(\.[1-3])?' ] ], [ @@ -336,7 +336,7 @@ 'url' => '/api/{apiVersion}/submission/{id}', 'verb' => 'DELETE', 'requirements' => [ - 'apiVersion' => 'v2(\.[1-2])?' + 'apiVersion' => 'v2(\.[1-3])?' ] ], ] diff --git a/docs/API.md b/docs/API.md index 4de86d4de..1c489c06b 100644 --- a/docs/API.md +++ b/docs/API.md @@ -29,6 +29,8 @@ This file contains the API-Documentation. For more information on the returned D ### Other API changes - In API version 2.1 the endpoint `/api/v2.1/share/update` was added to update a Share +- In API version 2.2 the endpoint `/api/v2.2/form/transfer` was added to transfer ownership of a form +- In API version 2.3 the endpoint `/api/v2.3/question/clone` was added to clone a question ## Form Endpoints ### List owned Forms @@ -329,6 +331,19 @@ Reorders all Questions of a single form "data": 4 ``` +### Clone a question +Creates a clone of a question with all its options. +- Endpoint: `/api/v2.3/question/clone/{id}` +- Url-Parameter: + | Parameter | Type | Description | + |-----------|---------|-------------| + | _id_ | Integer | ID of the question to clone | +- Method: `POST` +- Response: Returns cloned question object with the new ID set. +``` +See section 'Create a new question'. +``` + ## Option Endpoints Contains only manipulative question-endpoints. To retrieve options, request the full form data. diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index 252481f28..b255ca7ca 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -50,6 +50,7 @@ use OCP\AppFramework\OCS\OCSBadRequestException; use OCP\AppFramework\OCS\OCSException; use OCP\AppFramework\OCS\OCSForbiddenException; +use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\AppFramework\OCSController; use OCP\Files\NotFoundException; use OCP\Files\NotPermittedException; @@ -717,14 +718,14 @@ public function deleteQuestion(int $id): DataResponse { * @CORS * @NoAdminRequired * - * Duplicate a question + * Clone a question * * @param int $id the question id * @return DataResponse * @throws OCSBadRequestException|OCSForbiddenException */ - public function duplicateQuestion(int $id): DataResponse { - $this->logger->debug('Question to be duplicated: {id}', [ + public function cloneQuestion(int $id): DataResponse { + $this->logger->debug('Question to be cloned: {id}', [ 'id' => $id ]); @@ -734,7 +735,7 @@ public function duplicateQuestion(int $id): DataResponse { $form = $this->formMapper->findById($sourceQuestion->getFormId()); } catch (IMapperException $e) { $this->logger->debug('Could not find form or question'); - throw new OCSBadRequestException('Could not find form or question'); + throw new OCSNotFoundException('Could not find form or question'); } if ($form->getOwnerId() !== $this->currentUser->getUID()) { diff --git a/src/components/Questions/Question.vue b/src/components/Questions/Question.vue index d14c898d3..81584df54 100644 --- a/src/components/Questions/Question.vue +++ b/src/components/Questions/Question.vue @@ -100,7 +100,7 @@ {{ t('forms', 'Technical name') }} - + @@ -315,10 +315,10 @@ export default { }, /** - * Duplicate this question + * Clone this question */ - onDuplicate() { - this.$emit('duplicate') + onClone() { + this.$emit('clone') }, }, } diff --git a/src/mixins/QuestionMixin.js b/src/mixins/QuestionMixin.js index 4b4fda839..31a386664 100644 --- a/src/mixins/QuestionMixin.js +++ b/src/mixins/QuestionMixin.js @@ -201,8 +201,8 @@ export default { */ commonListeners() { return { + clone: this.onClone, delete: this.onDelete, - duplicate: this.onDuplicate, 'update:text': this.onTitleChange, 'update:description': this.onDescriptionChange, 'update:isRequired': this.onRequiredChange, @@ -292,10 +292,10 @@ export default { }, /** - * Duplicate this question. + * Clone this question. */ - onDuplicate() { - this.$emit('duplicate') + onClone() { + this.$emit('clone') }, /** diff --git a/src/views/Create.vue b/src/views/Create.vue index e23a2deb6..471ab2996 100644 --- a/src/views/Create.vue +++ b/src/views/Create.vue @@ -96,8 +96,8 @@ :index="index + 1" :max-string-lengths="maxStringLengths" v-bind.sync="form.questions[index]" + @clone="cloneQuestion(question)" @delete="deleteQuestion(question)" - @duplicate="duplicateQuestion(question)" @move-down="onMoveDown(index)" @move-up="onMoveUp(index)" /> @@ -395,15 +395,15 @@ export default { }, /** - * Duplicate a question + * Clone a question * - * @param {number} id the question id to duplicate in the current form + * @param {number} id the question id to clone in the current form */ - async duplicateQuestion({ id }) { + async cloneQuestion({ id }) { this.isLoadingQuestions = true try { - const response = await axios.post(generateOcsUrl('apps/forms/api/v2.1/question/clone/{id}', { id })) + const response = await axios.post(generateOcsUrl('apps/forms/api/v2.3/question/clone/{id}', { id })) const question = OcsResponse2Data(response) this.form.questions.push(Object.assign({ diff --git a/tests/Integration/Api/ApiV2Test.php b/tests/Integration/Api/ApiV2Test.php index f14fb97b3..2cb5bd66d 100644 --- a/tests/Integration/Api/ApiV2Test.php +++ b/tests/Integration/Api/ApiV2Test.php @@ -930,29 +930,20 @@ public function testDeleteQuestion(array $fullFormExpected) { $this->testGetFullForm($fullFormExpected); } - public function dataDuplicateQuestion() { - $fullFormExpected = $this->dataGetFullForm()['getFullForm']['expected']; - array_splice($fullFormExpected['questions'][1]['options'], 0, 1); - - return [ - 'duplicateQuestion' => [ - 'fullFormExpected' => $fullFormExpected - ] - ]; - } - - /** - * @dataProvider dataDuplicateQuestion - * @param array $fullFormExpected - */ - public function testDuplicateQuestion(array $fullFormExpected) { - $resp = $this->http->request('POST', "api/v2/question/{$this->testForms[0]['questions'][0]['id']}"); + public function testCloneQuestion() { + $resp = $this->http->request('POST', 'api/v2.3/question/clone/' . $this->testForms[0]['questions'][0]['id']); $data = $this->OcsResponse2Data($resp); + $this->testForms[0]['questions'][] = $data; $this->assertEquals(200, $resp->getStatusCode()); - $this->assertEquals($this->testForms[0]['questions'][count($this->testForms[0]['questions'])]['id'], $data); + $this->assertNotEquals($data['id'], $this->testForms[0]['questions'][0]['id']); - $this->testGetFullForm($fullFormExpected); + $copy = $this->testForms[0]['questions'][0]; + unset($copy['id']); + unset($copy['order']); + foreach ($copy as $key => $value) { + $this->assertEquals($value, $data[$key]); + } } public function dataCreateNewOption() { diff --git a/tests/Unit/Controller/ApiControllerTest.php b/tests/Unit/Controller/ApiControllerTest.php index 4a02e76e5..47de63b8c 100644 --- a/tests/Unit/Controller/ApiControllerTest.php +++ b/tests/Unit/Controller/ApiControllerTest.php @@ -51,6 +51,7 @@ function time($expected = null) { use OCA\Forms\Db\Form; use OCA\Forms\Db\FormMapper; use OCA\Forms\Db\OptionMapper; +use OCA\Forms\Db\Question; use OCA\Forms\Db\QuestionMapper; use OCA\Forms\Db\ShareMapper; use OCA\Forms\Db\Submission; @@ -59,10 +60,12 @@ function time($expected = null) { use OCA\Forms\Service\FormsService; use OCA\Forms\Service\SubmissionService; use OCA\Forms\Tests\Unit\MockedMapperException; +use OCP\AppFramework\Db\IMapperException; use OCP\AppFramework\Http\DataDownloadResponse; use OCP\AppFramework\Http\DataResponse; use OCP\AppFramework\OCS\OCSBadRequestException; use OCP\AppFramework\OCS\OCSForbiddenException; +use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\IL10N; use OCP\IRequest; use OCP\IUser; @@ -565,6 +568,21 @@ private function formAccess(bool $hasUserAccess = true, bool $hasFormExpired = f ->willReturn($canSubmit); } + public function testCloneQuestion_notFound() { + $this->questionMapper->method('findById')->with(42)->willThrowException($this->createMock(IMapperException::class)); + $this->expectException(OCSNotFoundException::class); + $this->apiController->cloneQuestion(42); + } + + public function testCloneQuestion_noPermission() { + $form = Form::fromParams(['ownerId' => 'otherUser']); + $question = Question::fromParams(['formId' => 1]); + $this->questionMapper->method('findById')->with(42)->willReturn($question); + $this->formMapper->method('findById')->with(1)->willReturn($form); + $this->expectException(OCSForbiddenException::class); + $this->apiController->cloneQuestion(42); + } + public function testInsertSubmission_answers() { $form = new Form(); $form->setId(1);