make question endpoints openapi compatible

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

lib/Controller/ApiController.php

@@ -77,6 +77,8 @@
 /**
  * @psalm-import-type FormsPartialForm from ResponseDefinitions
  * @psalm-import-type FormsForm from ResponseDefinitions
+ * @psalm-import-type FormsQuestion from ResponseDefinitions
+ * @psalm-import-type FormsOption from ResponseDefinitions
  */
 class ApiController extends OCSController {
 	private ?IUser $currentUser;
@@ -367,10 +369,10 @@ public function deleteForm(int $formId): DataResponse {
 	/**
 	 * Read all questions (including options)
 	 *
-	 * @param int $formId FormId
-	 * @return DataResponse
-	 * @throws OCSBadRequestException
-	 * @throws OCSForbiddenException
+	 * @param int $formId the form id
+	 * @return DataResponse<array<FormQuestion>, Http::STATUS_OK, array<>>
+	 * @throws OCSForbiddenException User has no permissions to get this form
+	 * @throws OCSNotFoundException Could not find form
 	 */
 	#[CORS()]
 	#[NoAdminRequired()]
@@ -380,7 +382,7 @@ public function getQuestions(int $formId): DataResponse {
 			$form = $this->formMapper->findById($formId);
 		} catch (IMapperException $e) {
 			$this->logger->debug('Could not find form');
-			throw new OCSBadRequestException();
+			throw new OCSNotFoundException();
 		}
 
 		if (!$this->formsService->hasUserAccess($form)) {
@@ -398,9 +400,10 @@ public function getQuestions(int $formId): DataResponse {
 	 *
 	 * @param int $formId FormId
 	 * @param int $questionId QuestionId
-	 * @return DataResponse
-	 * @throws OCSBadRequestException
-	 * @throws OCSForbiddenException
+	 * @return DataResponse<FormQuestion, Http::STATUS_OK, array<>>
+	 * @throws OCSBadRequestException Question doesn\'t belong to given Form
+	 * @throws OCSForbiddenException User has no permissions to get this form
+	 * @throws OCSNotFoundException Could not find form
 	 */
 	#[CORS()]
 	#[NoAdminRequired()]
@@ -410,7 +413,7 @@ public function getQuestion(int $formId, int $questionId): DataResponse {
 			$form = $this->formMapper->findById($formId);
 		} catch (IMapperException $e) {
 			$this->logger->debug('Could not find form');
-			throw new OCSBadRequestException();
+			throw new OCSNotFoundException();
 		}
 
 		if (!$this->formsService->hasUserAccess($form)) {
@@ -421,7 +424,7 @@ public function getQuestion(int $formId, int $questionId): DataResponse {
 		$question = $this->formsService->getQuestion($questionId);
 
 		if ($question['formId'] !== $formId) {
-			throw new OCSBadRequestException('Question doesn\'t belong to given Form');
+			throw new OCSBadRequestException('Question doesn\'t belong to given form');
 		}
 
 		return new DataResponse($question);
@@ -433,10 +436,14 @@ public function getQuestion(int $formId, int $questionId): DataResponse {
 	 * @param int $formId the form id
 	 * @param string $type the new question type
 	 * @param string $text the new question title
-	 * @param int $fromId (optional) id of the question that should be cloned
-	 * @return DataResponse
-	 * @throws OCSBadRequestException
-	 * @throws OCSForbiddenException
+	 * @param ?int $fromId (optional) id of the question that should be cloned
+	 * @return DataResponse<FormQuestion, Http::STATUS_CREATED, array<>>
+	 * @throws OCSBadRequestException Invalid type
+	 * @throws OCSBadRequestException Datetime question type no longer supported
+	 * @throws OCSForbiddenException User has no permissions to get this form
+	 * @throws OCSForbiddenException This form is archived and can not be modified
+	 * @throws OCSNotFoundException Could not find form
+	 * @throws OCSNotFoundException Could not find question
 	 */
 	#[CORS()]
 	#[NoAdminRequired()]
@@ -530,19 +537,26 @@ public function newQuestion(int $formId, ?string $type = null, string $text = ''
 
 		$this->formMapper->update($form);
 
-		return new DataResponse($response);
+		return new DataResponse($response, Http::STATUS_CREATED);
 	}
 
 	/**
-	 * Writes the given key-value pairs into Database.
-	 * Key 'order' should only be changed by reorderQuestions() and is not allowed here.
+	 * Writes the given key-value pairs into Database
+	 * Key `order` should only be changed by reorderQuestions() and is not allowed here
 	 *
 	 * @param int $formId 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
+	 * @param array<string, mixed> $keyValuePairs Array of key=>value pairs to update.
+	 * @return DataResponse<int id, Http::STATUS_OK, array<>>
+	 * @throws OCSBadRequestException Question doesn\'t belong to given Form
+	 * @throws OCSBadRequestException Invalid extraSettings, will not update.
+	 * @throws OCSForbiddenException Empty keyValuePairs, will not update
+	 * @throws OCSForbiddenException Not allowed to update `id` or `formId`
+	 * @throws OCSForbiddenException Please use reorderQuestions() to change order
+	 * @throws OCSForbiddenException This form is archived and can not be modified
+	 * @throws OCSForbiddenException User has no permissions to get this form
+	 * @throws OCSNotFoundException Could not find form
+	 * @throws OCSNotFoundException Could not find question
 	 */
 	#[CORS()]
 	#[NoAdminRequired()]
@@ -558,7 +572,7 @@ public function updateQuestion(int $formId, int $questionId, array $keyValuePair
 			$question = $this->questionMapper->findById($questionId);
 		} catch (IMapperException $e) {
 			$this->logger->debug('Could not find question');
-			throw new OCSBadRequestException('Could not find question');
+			throw new OCSNotFoundException('Could not find question');
 		}
 
 		if ($question->getFormId() !== $formId) {
@@ -568,19 +582,19 @@ public function updateQuestion(int $formId, int $questionId, array $keyValuePair
 		$form = $this->getFormIfAllowed($formId);
 		if ($this->formsService->isFormArchived($form)) {
 			$this->logger->debug('This form is archived and can not be modified');
-			throw new OCSForbiddenException();
+			throw new OCSForbiddenException('This form is archived and can not be modified');
 		}
 
 		// Don't allow empty array
 		if (sizeof($keyValuePairs) === 0) {
 			$this->logger->info('Empty keyValuePairs, will not update.');
-			throw new OCSForbiddenException();
+			throw new OCSBacRequestException('This form is archived and can not be modified');
 		}
 
 		//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();
+			throw new OCSForbiddenException('Not allowed to update \'id\' or \'formId\'');
 		}
 
 		// Don't allow to reorder here
@@ -609,9 +623,12 @@ public function updateQuestion(int $formId, int $questionId, array $keyValuePair
 	 *
 	 * @param int $formId the form id
 	 * @param int $questionId the question id
-	 * @return DataResponse
-	 * @throws OCSBadRequestException
-	 * @throws OCSForbiddenException
+	 * @return DataResponse<int id, Http::STATUS_OK, array<>>
+	 * @throws OCSBadRequestException Question doesn\'t belong to given Form
+	 * @throws OCSForbiddenException This form is archived and can not be modified
+	 * @throws OCSForbiddenException User has no permissions to get this form
+	 * @throws OCSNotFoundException Could not find form
+	 * @throws OCSNotFoundException Could not find question
 	 */
 	#[CORS()]
 	#[NoAdminRequired()]
@@ -625,7 +642,7 @@ public function deleteQuestion(int $formId, int $questionId): DataResponse {
 			$question = $this->questionMapper->findById($questionId);
 		} catch (IMapperException $e) {
 			$this->logger->debug('Could not find question');
-			throw new OCSBadRequestException('Could not find question');
+			throw new OCSNotFoundException('Could not find question');
 		}
 
 		if ($question->getFormId() !== $formId) {
@@ -635,7 +652,7 @@ public function deleteQuestion(int $formId, int $questionId): DataResponse {
 		$form = $this->getFormIfAllowed($formId);
 		if ($this->formsService->isFormArchived($form)) {
 			$this->logger->debug('This form is archived and can not be modified');
-			throw new OCSForbiddenException();
+			throw new OCSForbiddenException('This form is archived and can not be modified');
 		}
 
 		// Store Order of deleted Question
@@ -661,13 +678,19 @@ public function deleteQuestion(int $formId, int $questionId): DataResponse {
 	}
 
 	/**
-	 * Updates the Order of all Questions of a Form.
+	 * Updates the Order of all Questions of a Form
 	 *
 	 * @param int $formId Id of the form to reorder
-	 * @param array<int, int> $newOrder Array of Question-Ids in new order.
-	 * @return DataResponse
-	 * @throws OCSBadRequestException
-	 * @throws OCSForbiddenException
+	 * @param array<string, int> $newOrder Array of Question-Ids in new order.
+	 * @return DataResponse<array<int, int>, Http::STATUS_OK, array<>>
+	 * @throws OCSBadRequestException The given array contains duplicates
+	 * @throws OCSBadRequestException The length of the given array does not match the number of stored questions
+	 * @throws OCSBadRequestException Question doesn\'t belong to given Form
+	 * @throws OCSBadRequestException One question has already been marked as deleted
+	 * @throws OCSForbiddenException This form is archived and can not be modified
+	 * @throws OCSForbiddenException User has no permissions to get this form
+	 * @throws OCSNotFoundException Could not find form
+	 * @throws OCSNotFoundException Could not find question
 	 */
 	#[CORS()]
 	#[NoAdminRequired()]
@@ -681,7 +704,7 @@ public function reorderQuestions(int $formId, array $newOrder): DataResponse {
 		$form = $this->getFormIfAllowed($formId);
 		if ($this->formsService->isFormArchived($form)) {
 			$this->logger->debug('This form is archived and can not be modified');
-			throw new OCSForbiddenException();
+			throw new OCSForbiddenException('This form is archived and can not be modified');
 		}
 
 		// Check if array contains duplicates
@@ -705,27 +728,27 @@ public function reorderQuestions(int $formId, array $newOrder): DataResponse {
 			try {
 				$questions[$arrayKey] = $this->questionMapper->findById($questionId);
 			} catch (IMapperException $e) {
-				$this->logger->debug('Could not find question. Id: {questionId}', [
+				$this->logger->debug('Could not find question {questionId}', [
 					'questionId' => $questionId
 				]);
-				throw new OCSBadRequestException();
+				throw new OCSNotFoundException('Could not find question');
 			}
 
 			// Abort if a question is not part of the Form.
 			if ($questions[$arrayKey]->getFormId() !== $formId) {
-				$this->logger->debug('This Question is not part of the given Form: questionId: {questionId}', [
+				$this->logger->debug('This Question is not part of the given form: {questionId}', [
 					'questionId' => $questionId
 				]);
-				throw new OCSBadRequestException();
+				throw new OCSBadRequestException('Question doesn\'t belong to given Form');
 			}
 
 			// Abort if a question is already marked as deleted (order==0)
 			$oldOrder = $questions[$arrayKey]->getOrder();
 			if ($oldOrder === 0) {
-				$this->logger->debug('This Question has already been marked as deleted: Id: {questionId}', [
+				$this->logger->debug('This question has already been marked as deleted: Id: {questionId}', [
 					'questionId' => $questions[$arrayKey]->getId()
 				]);
-				throw new OCSBadRequestException();
+				throw new OCSBadRequestException('One question has already been marked as deleted');
 			}
 
 			// Only set order, if it changed.

lib/ResponseDefinitions.php

@@ -43,6 +43,7 @@
  *   "shares": array<FormsShare>,
  *   "submissions": array<FormsSubmission>,
  * }
+ *
  * @psalm-type FormsPartialForm = array{
  *   "id": int,
  *   "hash": string,
@@ -52,6 +53,28 @@
  *   "partial": bool true,
  *   "state": int
  * }
+ *
+ * @psalm-type FormsQuestion = array{
+ *   "id": int,
+ *   "formId": int,
+ *   "order": int,
+ *   "type": string,
+ *   "isRequired": bool,
+ *   "text": string,
+ *   "name": string,
+ *   "options": array<FormsOption>,
+ *   "accept": array<FormsQuestionFileType>,
+ *   "extraSettings": stdClass
+ * }
+ *
+ * @psalm-type FormsOption = array{
+ *   "id": int,
+ *   "questionId": int,
+ *   "text": string,
+ *   "order": ?int
+ * }
+ * 
+ * @psalm-type FormsQuestionFileType = string
  */
 class ResponseDefinitions {
 }