From 4f818f6f81a58aa6d38471ee394063852a60fef9 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Tue, 10 Nov 2026 08:40:10 +0100 Subject: [PATCH] fix(api): treat deleted questions as not found when deleting or copying them A deleted question keeps its row with order 0. Deleting it again moved every question after order 0 one position up, so the first question of the form got order 0 and was deleted as well. Copying it failed when the form had no other questions, and otherwise brought the deleted question back as a copy. Assisted-by: Claude Code:claude-opus-5-5 Signed-off-by: Andy Scherzinger --- lib/Controller/ApiController.php | 7 ++++ tests/Unit/Controller/ApiControllerTest.php | 39 +++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index e81475f50..16ac98a72 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -579,6 +579,9 @@ public function newQuestion(int $formId, ?string $type = null, ?string $subtype if ($sourceQuestion->getFormId() !== $formId) { throw new OCSBadRequestException('Question doesn\'t belong to given form'); } + if ($sourceQuestion->getOrder() === 0) { + throw new OCSNotFoundException('Could not find question'); + } $sourceOptions = $this->optionMapper->findByQuestion($fromId); } catch (IMapperException) { $this->logger->debug('Could not find question'); @@ -757,6 +760,10 @@ public function deleteQuestion(int $formId, int $questionId): DataResponse { throw new OCSBadRequestException('Question doesn\'t belong to given Form'); } + if ($question->getOrder() === 0) { + throw new OCSNotFoundException('Could not find question'); + } + // Store Order of deleted Question $deletedOrder = $question->getOrder(); diff --git a/tests/Unit/Controller/ApiControllerTest.php b/tests/Unit/Controller/ApiControllerTest.php index dd5b870d4..6b9af0231 100644 --- a/tests/Unit/Controller/ApiControllerTest.php +++ b/tests/Unit/Controller/ApiControllerTest.php @@ -1381,6 +1381,45 @@ public function testDeleteQuestionClearsConfirmationEmailQuestionIdWhenDeletingS $this->assertNull($form->getConfirmationEmailQuestionId()); } + public function testDeleteQuestionRejectsDeletedQuestion(): void { + $form = new Form(); + $form->setId(1); + $form->setOwnerId('currentUser'); + + $question = new Question(); + $question->setId(7); + $question->setFormId(1); + $question->setOrder(0); + + $this->formsService->method('getFormIfAllowed')->with(1, Constants::PERMISSION_EDIT)->willReturn($form); + $this->formsService->method('isFormArchived')->with($form)->willReturn(false); + $this->questionMapper->method('findById')->with(7)->willReturn($question); + $this->questionMapper->expects($this->never())->method('update'); + $this->formMapper->expects($this->never())->method('update'); + + $this->expectException(OCSNotFoundException::class); + $this->apiController->deleteQuestion(1, 7); + } + + public function testCloneQuestionRejectsDeletedQuestion(): void { + $form = new Form(); + $form->setId(1); + $form->setOwnerId('currentUser'); + + $question = new Question(); + $question->setId(7); + $question->setFormId(1); + $question->setOrder(0); + + $this->formsService->method('getFormIfAllowed')->with(1, Constants::PERMISSION_EDIT)->willReturn($form); + $this->formsService->method('isFormArchived')->with($form)->willReturn(false); + $this->questionMapper->method('findById')->with(7)->willReturn($question); + $this->questionMapper->expects($this->never())->method('insert'); + + $this->expectException(OCSNotFoundException::class); + $this->apiController->newQuestion(1, null, null, '', 7); + } + public function testCloneFormWithConfirmationEmailQuestionId(): void { $this->configService->method('canCreateForms')->willReturn(true);