update question

Signed-off-by: Christian Hartmann <[email protected]>

appinfo/routes.php

@@ -69,7 +69,8 @@
 		// Questions
 		['name' => 'api#getQuestions', 'url' => '/api/{apiVersion}/forms/{id}/questions', 'verb' => 'GET', 'requirements' => $requirements_v3],
 		['name' => 'api#newQuestion', 'url' => '/api/{apiVersion}/forms/{id}/questions', 'verb' => 'POST', 'requirements' => $requirements_v3],
-		# ['name' => 'api#getQuestion', 'url' => '/api/{apiVersion}/questions', 'verb' => 'GET', 'requirements' => $requirements_v3],
+		# ['name' => 'api#getQuestion', 'url' => '/api/{apiVersion}/forms/{id}/questions/{questionId}', 'verb' => 'GET', 'requirements' => $requirements_v3],
+		['name' => 'api#updateQuestion', 'url' => '/api/{apiVersion}/forms/{id}/questions/{questionId}', 'verb' => 'PATCH', 'requirements' => $requirements_v3],
 
 		// Legacy v2 routes (TODO: remove with Forms v5)
 		// Forms
@@ -120,21 +121,13 @@
 		]
 		],
 		// TODO: Remove POST in next API release
-		[
-			'name' => 'api#updateQuestion',
-			'url' => '/api/{apiVersion}/question/update',
-			'verb' => 'POST',
-			'requirements' => [
-				'apiVersion' => 'v2(\.[1-4])?'
-			]
+		['name' => 'api#updateQuestionLegacy', 'url' => '/api/{apiVersion}/question/update', 'verb' => 'POST', 'requirements' => [
+			'apiVersion' => 'v2(\.[1-4])?'
+		]
 		],
-		[
-			'name' => 'api#updateQuestion',
-			'url' => '/api/{apiVersion}/question/update',
-			'verb' => 'PATCH',
-			'requirements' => [
-				'apiVersion' => 'v2\.[2-4]'
-			]
+		['name' => 'api#updateQuestionLegacy', 'url' => '/api/{apiVersion}/question/update', 'verb' => 'PATCH', 'requirements' => [
+			'apiVersion' => 'v2\.[2-4]'
+		]
 		],
 		// TODO: Remove POST in next API release
 		[

lib/Controller/ApiController.php

@@ -494,6 +494,78 @@ public function newQuestion(int $id, ?string $type = null, string $text = '', ?i
 		return new DataResponse($response);
 	}
 
+	/**
+	 * @CORS
+	 * @NoAdminRequired
+	 *
+	 * Writes the given key-value pairs into Database.
+	 * Key 'order' should only be changed by reorderQuestions() and is not allowed here.
+	 *
+	 * @param int $id the form id
+	 * @param int $questionId id of question to update
+	 * @param array $keyValuePairs Array of key=>value pairs to update.
+	 * @return DataResponse
+	 * @throws OCSBadRequestException
+	 * @throws OCSForbiddenException
+	 */
+	public function updateQuestion(int $id, int $questionId, array $keyValuePairs): DataResponse {
+		$this->logger->debug('Updating question: formId: {id}, questionId: {id}, values: {keyValuePairs}', [
+			'id' => $id,
+			'questionId' => $questionId,
+			'keyValuePairs' => $keyValuePairs
+		]);
+
+		try {
+			$question = $this->questionMapper->findById($questionId);
+		} catch (IMapperException $e) {
+			$this->logger->debug('Could not find question');
+			throw new OCSBadRequestException('Could not find question');
+		}
+
+		if ($question->getFormId() !== $id) {
+			throw new OCSBadRequestException('Question doesn\'t belong to given Form');
+		}
+
+		$form = $this->getFormIfAllowed($id);
+		if ($this->formsService->isFormArchived($form)) {
+			$this->logger->debug('This form is archived and can not be modified');
+			throw new OCSForbiddenException();
+		}
+
+		// Don't allow empty array
+		if (sizeof($keyValuePairs) === 0) {
+			$this->logger->info('Empty keyValuePairs, will not update.');
+			throw new OCSForbiddenException();
+		}
+
+		//Don't allow to change id or formId
+		if (key_exists('id', $keyValuePairs) || key_exists('formId', $keyValuePairs)) {
+			$this->logger->debug('Not allowed to update id or formId');
+			throw new OCSForbiddenException();
+		}
+
+		// Don't allow to reorder here
+		if (key_exists('order', $keyValuePairs)) {
+			$this->logger->debug('Key \'order\' is not allowed on updateQuestion. Please use reorderQuestions() to change order.');
+			throw new OCSForbiddenException('Please use reorderQuestions() to change order');
+		}
+
+		if (key_exists('extraSettings', $keyValuePairs) && !$this->formsService->areExtraSettingsValid($keyValuePairs['extraSettings'], $question->getType())) {
+			throw new OCSBadRequestException('Invalid extraSettings, will not update.');
+		}
+
+		// Create QuestionEntity with given Params & Id.
+		$question = Question::fromParams($keyValuePairs);
+		$question->setId($questionId);
+
+		// Update changed Columns in Db.
+		$this->questionMapper->update($question);
+
+		$this->formsService->setLastUpdatedTimestamp($id);
+
+		return new DataResponse($question->getId());
+	}
+
 	/*
 	/* Legacy API v2 methods (TODO: remove with Forms v5)
 	 */
@@ -959,7 +1031,7 @@ public function reorderQuestions(int $formId, array $newOrder): DataResponse {
 	 * @throws OCSBadRequestException
 	 * @throws OCSForbiddenException
 	 */
-	public function updateQuestion(int $id, array $keyValuePairs): DataResponse {
+	public function updateQuestionLegacy(int $id, array $keyValuePairs): DataResponse {
 		$this->logger->debug('Updating question: questionId: {id}, values: {keyValuePairs}', [
 			'id' => $id,
 			'keyValuePairs' => $keyValuePairs

src/mixins/QuestionMixin.js

@@ -363,9 +363,14 @@ export default {
 			try {
 				// TODO: add loading status feedback ?
 				await axios.patch(
-					generateOcsUrl('apps/forms/api/v2.4/question/update'),
+					generateOcsUrl(
+						'apps/forms/api/v3/forms/{id}/questions/{questionId}',
+						{
+							id: this.formId,
+							questionId: this.id,
+						},
+					),
 					{
-						id: this.id,
 						keyValuePairs: {
 							[key]: value,
 						},

tests/Integration/Api/ApiV3Test.php

@@ -787,9 +787,8 @@ public function dataUpdateQuestionProperties() {
 	 * @param array $fullFormExpected
 	 */
 	public function testUpdateQuestionProperties(array $fullFormExpected): void {
-		$resp = $this->http->request('PATCH', 'api/v2.4/question/update', [
+		$resp = $this->http->request('PATCH', "api/v3/forms/{$this->testForms[0]['id']}/questions/{$this->testForms[0]['questions'][0]['id']}", [
 			'json' => [
-				'id' => $this->testForms[0]['questions'][0]['id'],
 				'keyValuePairs' => [
 					'isRequired' => false,
 					'text' => 'Still first Question!'