diff --git a/docs/DataStructure.md b/docs/DataStructure.md index f83d5bb71..d037ff724 100644 --- a/docs/DataStructure.md +++ b/docs/DataStructure.md @@ -27,6 +27,7 @@ This document describes the Object-Structure, that is used within the Forms App | isAnonymous | Boolean | | If Answers will be stored anonymously | | state | Integer | [Form state](#form-state) | The state of the form | | submitMultiple | Boolean | | If users are allowed to submit multiple times to the form | +| allowEdit | Boolean | | If users are allowed to edit or delete their response | | showExpiration | Boolean | | If the expiration date will be shown on the form | | canSubmit | Boolean | | If the user can Submit to the form, i.e. calculated information out of `submitMultiple` and existing submissions. | | permissions | Array of [Permissions](#permissions) | Array of permissions regarding the form | @@ -46,6 +47,7 @@ This document describes the Object-Structure, that is used within the Forms App "expires": 0, "isAnonymous": false, "submitMultiple": true, + "allowEdit": false, "showExpiration": false, "canSubmit": true, "permissions": [ diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index 4072dedc1..6a6ecf4f3 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -170,6 +170,7 @@ public function newForm(?int $fromId = null): DataResponse { 'showToAllUsers' => false, ]); $form->setSubmitMultiple(false); + $form->setAllowEdit(false); $form->setShowExpiration(false); $form->setExpires(0); $form->setIsAnonymous(false); @@ -1292,7 +1293,7 @@ public function newSubmission(int $formId, array $answers, string $shareHash = ' continue; } - $this->storeAnswersForQuestion($form, $submission->getId(), $questions[$questionIndex], $answerArray); + $this->storeAnswersForQuestion($form, $submission->getId(), $questions[$questionIndex], $answerArray, false); } $this->formMapper->update($form); @@ -1307,6 +1308,87 @@ public function newSubmission(int $formId, array $answers, string $shareHash = ' return new DataResponse(null, Http::STATUS_CREATED); } + /** + * Update an existing submission + * + * @param int $formId the form id + * @param int $submissionId the submission id + * @param array> $answers [question_id => arrayOfString] + * @param string $shareHash public share-hash -> Necessary to submit on public link-shares. + * @return DataResponse + * @throws OCSBadRequestException Can only update submission if AllowEdit is set and the answers are valid + * @throws OCSForbiddenException Can only update your own submission + * + * 200: the id of the updated submission + */ + #[CORS()] + #[NoAdminRequired()] + #[NoCSRFRequired()] + #[PublicPage()] + #[ApiRoute(verb: 'PUT', url: '/api/v3/forms/{formId}/submissions/{submissionId}')] + public function updateSubmission(int $formId, int $submissionId, array $answers, string $shareHash = ''): DataResponse { + $this->logger->debug('Updating submission: formId: {formId}, answers: {answers}, shareHash: {shareHash}', [ + 'formId' => $formId, + 'answers' => $answers, + 'shareHash' => $shareHash, + ]); + + $form = $this->loadFormForSubmission($formId, $shareHash); + + if (!$form->getAllowEdit()) { + throw new OCSBadRequestException('Can only update if AllowEdit is set'); + } + + $questions = $this->formsService->getQuestions($formId); + // Is the submission valid + $isSubmissionValid = $this->submissionService->validateSubmission($questions, $answers, $form->getOwnerId()); + if (is_string($isSubmissionValid)) { + throw new OCSBadRequestException($isSubmissionValid); + } + if ($isSubmissionValid === false) { + throw new OCSBadRequestException('At least one submitted answer is not valid'); + } + + // get existing submission of this user + try { + $submission = $this->submissionMapper->findByFormAndUser($form->getId(), $this->currentUser->getUID()); + } catch (DoesNotExistException $e) { + throw new OCSBadRequestException('Cannot update a non existing submission'); + } + + if ($submissionId != $submission->getId()) { + throw new OCSForbiddenException('Can only update your own submissions'); + } + + $submission->setTimestamp(time()); + $this->submissionMapper->update($submission); + + if (empty($answers)) { + // Clear Answers + foreach ($questions as $question) { + $this->storeAnswersForQuestion($form, $submission->getId(), $question, [''], true); + } + } else { + // Process Answers + foreach ($answers as $questionId => $answerArray) { + // Search corresponding Question, skip processing if not found + $questionIndex = array_search($questionId, array_column($questions, 'id')); + if ($questionIndex === false) { + continue; + } + + $question = $questions[$questionIndex]; + + $this->storeAnswersForQuestion($form, $submission->getId(), $question, $answerArray, true); + } + } + + //Create Activity + $this->formsService->notifyNewSubmission($form, $submission); + + return new DataResponse($submissionId); + } + /** * Delete a specific submission * @@ -1520,14 +1602,23 @@ public function uploadFiles(int $formId, int $questionId, string $shareHash = '' // private functions /** - * Insert answers for a question + * Insert or update answers for a question * * @param Form $form * @param int $submissionId * @param array $question * @param string[]|array $answerArray + * @param bool $update */ - private function storeAnswersForQuestion(Form $form, $submissionId, array $question, array $answerArray) { + private function storeAnswersForQuestion(Form $form, int $submissionId, array $question, array $answerArray, bool $update) { + // get stored answers for this question + $storedAnswers = []; + if ($update) { + $storedAnswers = $this->answerMapper->findBySubmissionAndQuestion($submissionId, $question['id']); + } + + $newAnswerTexts = []; + foreach ($answerArray as $answer) { $answerEntity = new Answer(); $answerEntity->setSubmissionId($submissionId); @@ -1544,6 +1635,33 @@ private function storeAnswersForQuestion(Form $form, $submissionId, array $quest } elseif (!empty($question['extraSettings']['allowOtherAnswer']) && strpos($answer, Constants::QUESTION_EXTRASETTINGS_OTHER_PREFIX) === 0) { $answerText = str_replace(Constants::QUESTION_EXTRASETTINGS_OTHER_PREFIX, '', $answer); } + + if (!array_key_exists($question['id'], $newAnswerTexts)) { + $newAnswerTexts[$question['id']] = []; + } + $newAnswerTexts[$question['id']][] = $answerText; + + // has this answer already been stored? + $foundAnswer = false; + foreach ($storedAnswers as $storedAnswer) { + if ($storedAnswer->getText() == $answerText) { + // nothing to be changed + $foundAnswer = true; + break; + } + } + if (!$foundAnswer) { + if ($answerText === '') { + continue; + } + // need to add answer + $answerEntity = new Answer(); + $answerEntity->setSubmissionId($submissionId); + $answerEntity->setQuestionId($question['id']); + $answerEntity->setText($answerText); + $this->answerMapper->insert($answerEntity); + } + } elseif ($question['type'] === Constants::ANSWER_TYPE_FILE) { $uploadedFile = $this->uploadedFileMapper->getByUploadedFileId($answer['uploadedFileId']); $answerEntity->setFileId($uploadedFile->getFileId()); @@ -1563,20 +1681,43 @@ private function storeAnswersForQuestion(Form $form, $submissionId, array $quest $file->move($folder->getPath() . '/' . $name); $answerText = $name; + + $answerEntity->setText($answerText); + $this->answerMapper->insert($answerEntity); } else { $answerText = $answer; // Not a multiple-question, answerText is given answer - } - if ($answerText === '') { - continue; + if (!empty($storedAnswers)) { + $answerEntity = $storedAnswers[0]; + $answerEntity->setText($answerText); + $this->answerMapper->update($answerEntity); + } else { + if ($answerText === '') { + continue; + } + $answerEntity = new Answer(); + $answerEntity->setSubmissionId($submissionId); + $answerEntity->setQuestionId($question['id']); + $answerEntity->setText($answerText); + $this->answerMapper->insert($answerEntity); + } } - $answerEntity->setText($answerText); - $this->answerMapper->insert($answerEntity); if ($uploadedFile) { $this->uploadedFileMapper->delete($uploadedFile); } } + + if (in_array($question['type'], Constants::ANSWER_TYPES_PREDEFINED)) { + // drop all answers that are not in new set of answers + foreach ($storedAnswers as $storedAnswer) { + $questionId = $storedAnswer->getQuestionId(); + + if (empty($newAnswerTexts[$questionId]) || !in_array($storedAnswer->getText(), $newAnswerTexts[$questionId])) { + $this->answerMapper->delete($storedAnswer); + } + } + } } private function loadFormForSubmission(int $formId, string $shareHash): Form { diff --git a/lib/Db/AnswerMapper.php b/lib/Db/AnswerMapper.php index 2420f6b34..a04674609 100644 --- a/lib/Db/AnswerMapper.php +++ b/lib/Db/AnswerMapper.php @@ -41,6 +41,26 @@ public function findBySubmission(int $submissionId): array { return $this->findEntities($qb); } + /** + * @param int $submissionId + * @param int $questionId + * @throws \OCP\AppFramework\Db\DoesNotExistException if not found + * @return Answer[] + */ + + public function findBySubmissionAndQuestion(int $submissionId, int $questionId): array { + $qb = $this->db->getQueryBuilder(); + + $qb->select('*') + ->from($this->getTableName()) + ->where( + $qb->expr()->eq('submission_id', $qb->createNamedParameter($submissionId, IQueryBuilder::PARAM_INT)), + $qb->expr()->eq('question_id', $qb->createNamedParameter($questionId, IQueryBuilder::PARAM_INT)) + ); + + return $this->findEntities($qb); + } + /** * @param int $submissionId */ diff --git a/lib/Db/Form.php b/lib/Db/Form.php index ffb45d987..cdd630c4b 100644 --- a/lib/Db/Form.php +++ b/lib/Db/Form.php @@ -35,6 +35,8 @@ * @method void setIsAnonymous(bool $value) * @method int getSubmitMultiple() * @method void setSubmitMultiple(bool $value) + * @method int getAllowEdit() + * @method void setAllowEdit(bool $value) * @method int getShowExpiration() * @method void setShowExpiration(bool $value) * @method int getLastUpdated() @@ -58,6 +60,7 @@ class Form extends Entity { protected $expires; protected $isAnonymous; protected $submitMultiple; + protected $allowEdit; protected $showExpiration; protected $submissionMessage; protected $lastUpdated; @@ -71,6 +74,7 @@ public function __construct() { $this->addType('expires', 'integer'); $this->addType('isAnonymous', 'boolean'); $this->addType('submitMultiple', 'boolean'); + $this->addType('allowEdit', 'boolean'); $this->addType('showExpiration', 'boolean'); $this->addType('lastUpdated', 'integer'); $this->addType('state', 'integer'); @@ -140,6 +144,7 @@ public function setAccess(array $access) { * expires: int, * isAnonymous: bool, * submitMultiple: bool, + * allowEdit: bool, * showExpiration: bool, * lastUpdated: int, * submissionMessage: ?string, @@ -160,6 +165,7 @@ public function read() { 'expires' => (int)$this->getExpires(), 'isAnonymous' => (bool)$this->getIsAnonymous(), 'submitMultiple' => (bool)$this->getSubmitMultiple(), + 'allowEdit' => (bool)$this->getAllowEdit(), 'showExpiration' => (bool)$this->getShowExpiration(), 'lastUpdated' => (int)$this->getLastUpdated(), 'submissionMessage' => $this->getSubmissionMessage(), diff --git a/lib/Db/SubmissionMapper.php b/lib/Db/SubmissionMapper.php index a6c5d8031..5f198c682 100644 --- a/lib/Db/SubmissionMapper.php +++ b/lib/Db/SubmissionMapper.php @@ -47,6 +47,29 @@ public function findByForm(int $formId): array { return $this->findEntities($qb); } + /** + * @param int $formId + * @param string $userId + * + * @return Submission + * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException if more than one result + * @throws \OCP\AppFramework\Db\DoesNotExistException if not found + */ + public function findByFormAndUser(int $formId, string $userId): Submission { + $qb = $this->db->getQueryBuilder(); + + $qb->select('*') + ->from($this->getTableName()) + ->where( + $qb->expr()->eq('form_id', $qb->createNamedParameter($formId, IQueryBuilder::PARAM_INT)), + $qb->expr()->eq('user_id', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)) + ) + //Newest submissions first + ->orderBy('timestamp', 'DESC'); + + return $this->findEntity($qb); + } + /** * @param int $id * @return Submission diff --git a/lib/FormsMigrator.php b/lib/FormsMigrator.php index 4a56c3c61..791f97009 100644 --- a/lib/FormsMigrator.php +++ b/lib/FormsMigrator.php @@ -146,6 +146,7 @@ public function import(IUser $user, IImportSource $importSource, OutputInterface $form->setExpires($formData['expires']); $form->setIsAnonymous($formData['isAnonymous']); $form->setSubmitMultiple($formData['submitMultiple']); + $form->setAllowEdit($formData['allowEdit']); $form->setShowExpiration($formData['showExpiration']); $this->formMapper->insert($form); diff --git a/lib/Migration/Version050000Date20250109201500.php b/lib/Migration/Version050000Date20250109201500.php new file mode 100644 index 000000000..c7406798b --- /dev/null +++ b/lib/Migration/Version050000Date20250109201500.php @@ -0,0 +1,42 @@ +getTable('forms_v2_forms'); + + if (!$table->hasColumn('allow_edit')) { + $table->addColumn('allow_edit', Types::BOOLEAN, [ + 'notnull' => false, + 'default' => 0, + ]); + + return $schema; + } + + return null; + } +} diff --git a/lib/ResponseDefinitions.php b/lib/ResponseDefinitions.php index c13808147..0ebde5954 100644 --- a/lib/ResponseDefinitions.php +++ b/lib/ResponseDefinitions.php @@ -111,6 +111,7 @@ * isAnonymous: bool, * lastUpdated: int, * submitMultiple: bool, + * allowEdit: bool, * showExpiration: bool, * canSubmit: bool, * permissions: list, @@ -119,6 +120,9 @@ * shares: list, * submissionCount?: int, * submissionMessage: ?string, + * answers?: array, + * newSubmission?: bool, + * submissionId?: int, * } * * @psalm-type FormsUploadedFile = array{ diff --git a/lib/Service/FormsService.php b/lib/Service/FormsService.php index e07664009..14ff18028 100644 --- a/lib/Service/FormsService.php +++ b/lib/Service/FormsService.php @@ -9,6 +9,7 @@ use OCA\Forms\Activity\ActivityManager; use OCA\Forms\Constants; +use OCA\Forms\Db\AnswerMapper; use OCA\Forms\Db\Form; use OCA\Forms\Db\FormMapper; use OCA\Forms\Db\OptionMapper; @@ -22,6 +23,7 @@ use OCA\Forms\ResponseDefinitions; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\IMapperException; +use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\EventDispatcher\IEventDispatcher; use OCP\Files\IRootFolder; use OCP\Files\NotFoundException; @@ -54,6 +56,7 @@ public function __construct( private QuestionMapper $questionMapper, private ShareMapper $shareMapper, private SubmissionMapper $submissionMapper, + private AnswerMapper $answerMapper, private ConfigService $configService, private IGroupManager $groupManager, private IUserManager $userManager, @@ -96,6 +99,32 @@ private function getOptions(int $questionId): array { } } + private function getAnswers(int $formId, int $submissionId, string $userId): array { + + $answerList = []; + $answerEntities = $this->answerMapper->findBySubmission($submissionId); + foreach ($answerEntities as $answerEntity) { + $answer = $answerEntity->read(); + $questionId = $answer['questionId']; + if (!array_key_exists($questionId, $answerList)) { + $answerList[$questionId] = []; + } + $options = $this->getOptions($answer['questionId']); + if (!empty($options)) { + // match option text to option index + foreach ($options as $option) { + if ($option['text'] == $answer['text']) { + $answerList[$questionId][] = strval($option['id']); + } + } + } else { + // copy the text + $answerList[$questionId][] = $answer['text']; + } + } + return $answerList; + } + /** * Load questions corresponding to form * @@ -193,6 +222,25 @@ public function getShares(int $formId): array { public function getForm(Form $form): array { $result = $form->read(); $result['questions'] = $this->getQuestions($form->getId()); + + // add previous submission if there is one by this user for this form + if ($this->currentUser->getUID() && $form->getAllowEdit()) { + $submissionEntity = null; + try { + $submissionEntity = $this->submissionMapper->findByFormAndUser($form->getId(), $this->currentUser->getUID()); + $answers = $this->getAnswers($form->getId(), $submissionEntity->getId(), $this->currentUser->getUID()); + if (!empty($answers)) { + $result['answers'] = $answers; + $result['newSubmission'] = false; + $result['submissionId'] = $submissionEntity->getId(); + } + } catch (DoesNotExistException $e) { + //handle silently + } catch (MultipleObjectsReturnedException $e) { + //handle silently + } + } + $result['shares'] = $this->getShares($form->getId()); // Append permissions for current user. @@ -351,9 +399,9 @@ public function canSubmit(Form $form): bool { return true; } - // Refuse access, if SubmitMultiple is not set and user already has taken part. + // Refuse access, if SubmitMultiple is not set and AllowEdit is not set and user already has taken part. if ( - !$form->getSubmitMultiple() && + !$form->getSubmitMultiple() && !$form->getAllowEdit() && $this->submissionMapper->hasFormSubmissionsByUser($form, $this->currentUser->getUID()) ) { return false; diff --git a/openapi.json b/openapi.json index 9637fa107..2c45536c4 100644 --- a/openapi.json +++ b/openapi.json @@ -105,6 +105,7 @@ "isAnonymous", "lastUpdated", "submitMultiple", + "allowEdit", "showExpiration", "canSubmit", "permissions", @@ -164,6 +165,9 @@ "submitMultiple": { "type": "boolean" }, + "allowEdit": { + "type": "boolean" + }, "showExpiration": { "type": "boolean" }, @@ -204,6 +208,19 @@ "submissionMessage": { "type": "string", "nullable": true + }, + "answers": { + "type": "object", + "additionalProperties": { + "type": "object" + } + }, + "newSubmission": { + "type": "boolean" + }, + "submissionId": { + "type": "integer", + "format": "int64" } } }, @@ -3381,6 +3398,170 @@ } }, "/ocs/v2.php/apps/forms/api/v3/forms/{formId}/submissions/{submissionId}": { + "put": { + "operationId": "api-update-submission", + "summary": "Update an existing submission", + "tags": [ + "api" + ], + "security": [ + {}, + { + "basic_auth": [] + } + ], + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "answers" + ], + "properties": { + "answers": { + "type": "object", + "description": "[question_id => arrayOfString]", + "additionalProperties": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "shareHash": { + "type": "string", + "default": "", + "description": "public share-hash -> Necessary to submit on public link-shares." + } + } + } + } + } + }, + "parameters": [ + { + "name": "formId", + "in": "path", + "description": "the form id", + "required": true, + "schema": { + "type": "integer", + "format": "int64" + } + }, + { + "name": "submissionId", + "in": "path", + "description": "the submission id", + "required": true, + "schema": { + "type": "integer", + "format": "int64" + } + }, + { + "name": "OCS-APIRequest", + "in": "header", + "description": "Required to be true for the API request to pass", + "required": true, + "schema": { + "type": "boolean", + "default": true + } + } + ], + "responses": { + "200": { + "description": "the id of the updated submission", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": { + "type": "integer", + "format": "int64" + } + } + } + } + } + } + } + }, + "400": { + "description": "Can only update submission if AllowEdit is set and the answers are valid", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + }, + "403": { + "description": "Can only update your own submission", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": [ + "ocs" + ], + "properties": { + "ocs": { + "type": "object", + "required": [ + "meta", + "data" + ], + "properties": { + "meta": { + "$ref": "#/components/schemas/OCSMeta" + }, + "data": {} + } + } + } + } + } + } + } + } + }, "delete": { "operationId": "api-delete-submission", "summary": "Delete a specific submission", diff --git a/src/components/Questions/QuestionDropdown.vue b/src/components/Questions/QuestionDropdown.vue index f9f25d221..80f209aa3 100644 --- a/src/components/Questions/QuestionDropdown.vue +++ b/src/components/Questions/QuestionDropdown.vue @@ -162,7 +162,7 @@ export default { } const selected = this.values.map((id) => - this.options.find((option) => option.id === id), + this.options.find((option) => option.id === parseInt(id)), ) return this.isMultiple ? selected : selected[0] diff --git a/src/components/SidebarTabs/SettingsSidebarTab.vue b/src/components/SidebarTabs/SettingsSidebarTab.vue index 29f92f102..5959b333e 100644 --- a/src/components/SidebarTabs/SettingsSidebarTab.vue +++ b/src/components/SidebarTabs/SettingsSidebarTab.vue @@ -21,6 +21,14 @@ @update:checked="onSubmitMultipleChange"> {{ t('forms', 'Allow multiple responses per person') }} + + {{ t('forms', 'Allow editing responses per person') }} + 'Test user', @@ -41,6 +43,7 @@ private function setTestForms() { 'state' => 0, 'is_anonymous' => false, 'submit_multiple' => false, + 'allow_edit' => true, 'show_expiration' => false, 'last_updated' => 123456789, 'submission_message' => 'Back to website', @@ -164,6 +167,7 @@ private function setTestForms() { 'state' => 0, 'is_anonymous' => false, 'submit_multiple' => false, + 'allow_edit' => true, 'show_expiration' => false, 'last_updated' => 123456789, 'submission_message' => '', @@ -201,6 +205,7 @@ private function setTestForms() { 'state' => 0, 'is_anonymous' => false, 'submit_multiple' => false, + 'allow_edit' => true, 'show_expiration' => false, 'last_updated' => 123456789, 'submission_message' => '', @@ -252,6 +257,16 @@ public function setUp(): void { 'Accept' => 'application/json' ], ]); + + // Set up http Client for user user1 + $this->httpUser1 = new Client([ + 'base_uri' => 'http://localhost:8080/ocs/v2.php/apps/forms/', + 'auth' => ['user1', 'user1'], + 'headers' => [ + 'OCS-ApiRequest' => 'true', + 'Accept' => 'application/json' + ], + ]); } public function tearDown(): void { @@ -366,6 +381,7 @@ public function dataGetNewForm() { 'state' => 0, 'isAnonymous' => false, 'submitMultiple' => false, + 'allowEdit' => false, 'showExpiration' => false, // 'lastUpdated' => time() can not be checked exactly 'canSubmit' => true, @@ -425,6 +441,7 @@ public function dataGetFullForm() { 'state' => 0, 'isAnonymous' => false, 'submitMultiple' => false, + 'allowEdit' => false, 'showExpiration' => false, 'lastUpdated' => 123456789, 'canSubmit' => true, @@ -1347,6 +1364,93 @@ public function testNewSubmission() { ], $data['submissions'][0]); } + /** + * @dataProvider dataNewSubmission + */ + public function testUpdateSubmission() { + + $resp = $this->http->request('PATCH', "api/v3/forms/{$this->testForms[0]['id']}", [ + 'json' => [ + 'keyValuePairs' => [ + 'AllowEdit' => true, + ] + ] + ]); + + $uploadedFileResponse = $this->httpUser1->request('POST', + "api/v3/forms/{$this->testForms[0]['id']}/submissions/files/{$this->testForms[0]['questions'][2]['id']}", + [ + 'multipart' => [ + [ + 'name' => 'files[]', + 'contents' => 'hello world2', + 'filename' => 'test2.txt' + ] + ] + ]); + + $data = $this->OcsResponse2Data($uploadedFileResponse); + $uploadedFileId = $data[0]['uploadedFileId']; + + $resp = $this->httpUser1->request('PUT', "api/v3/forms/{$this->testForms[0]['id']}/submissions/{$this->testForms[0]['submissions'][0]['id']}", [ + 'json' => [ + 'answers' => [ + $this->testForms[0]['questions'][0]['id'] => ['ShortAnswer2!'], + $this->testForms[0]['questions'][1]['id'] => [ + $this->testForms[0]['questions'][1]['options'][1]['id'] + ], + $this->testForms[0]['questions'][2]['id'] => [['uploadedFileId' => $uploadedFileId]] + ] + ] + ]); + $data = $this->OcsResponse2Data($resp); + + $this->assertEquals(200, $resp->getStatusCode()); + + // Check stored submissions + $resp = $this->http->request('GET', "api/v3/forms/{$this->testForms[0]['id']}/submissions"); + $data = $this->OcsResponse2Data($resp); + + // Check Ids + foreach ($data['submissions'][0]['answers'] as $aIndex => $answer) { + $this->assertEquals($data['submissions'][0]['id'], $answer['submissionId']); + unset($data['submissions'][0]['answers'][$aIndex]['id']); + unset($data['submissions'][0]['answers'][$aIndex]['submissionId']); + + if (isset($answer['fileId'])) { + $this->assertIsNumeric($answer['fileId'], 'fileId should be numeric.'); + $this->assertGreaterThan(0, $answer['fileId'], 'fileId should be greater than 0.'); + unset($data['submissions'][0]['answers'][$aIndex]['fileId']); + } + } + unset($data['submissions'][0]['id']); + // Check general behaviour of timestamp (Insert in the last 10 seconds) + $this->assertTrue(time() - $data['submissions'][0]['timestamp'] < 10); + unset($data['submissions'][0]['timestamp']); + + $this->assertEquals([ + 'userId' => 'user1', + 'userDisplayName' => 'User No. 1', + 'formId' => $this->testForms[0]['id'], + 'answers' => [ + [ + 'questionId' => $this->testForms[0]['questions'][0]['id'], + 'text' => 'ShortAnswer2!', + 'fileId' => null, + ], + [ + 'questionId' => $this->testForms[0]['questions'][1]['id'], + 'text' => 'Option 2', + 'fileId' => null, + ], + [ + 'questionId' => $this->testForms[0]['questions'][2]['id'], + 'text' => 'test2.txt', + ], + ] + ], $data['submissions'][0]); + } + public function dataDeleteSingleSubmission() { $submissionsExpected = $this->dataGetSubmissions()['getSubmissions']['expected']; array_splice($submissionsExpected['submissions'], 0, 1); diff --git a/tests/Integration/Api/RespectAdminSettingsTest.php b/tests/Integration/Api/RespectAdminSettingsTest.php index 622fe2a38..fbb127543 100644 --- a/tests/Integration/Api/RespectAdminSettingsTest.php +++ b/tests/Integration/Api/RespectAdminSettingsTest.php @@ -131,6 +131,7 @@ private static function sharedTestForms(): array { ], 'canSubmit' => true, 'submissionCount' => 0, + 'allowEdit' => false, ], ]; } diff --git a/tests/Unit/Controller/ApiControllerTest.php b/tests/Unit/Controller/ApiControllerTest.php index 20bf8e2f4..5de62aa36 100644 --- a/tests/Unit/Controller/ApiControllerTest.php +++ b/tests/Unit/Controller/ApiControllerTest.php @@ -394,11 +394,13 @@ public function dataTestCreateNewForm() { 'expires' => 0, 'isAnonymous' => false, 'submitMultiple' => false, + 'allowEdit' => false, 'showExpiration' => false, 'lastUpdated' => 123456789, 'submissionMessage' => null, 'fileId' => null, 'fileFormat' => null, + 'allowEdit' => null, ]] ]; } @@ -485,6 +487,7 @@ public function dataCloneForm() { 'expires' => 0, 'isAnonymous' => false, 'submitMultiple' => false, + 'allowEdit' => false, 'showExpiration' => false ], 'new' => [ @@ -500,6 +503,7 @@ public function dataCloneForm() { 'expires' => 0, 'isAnonymous' => false, 'submitMultiple' => false, + 'allowEdit' => false, 'showExpiration' => false ] ] @@ -630,7 +634,10 @@ public function testUploadFiles() { $this->apiController->uploadFiles(1, 10, ''); } - public function testNewSubmission_answers() { + /** + * Values for the mock objects for the following methods: testNewSubmission_answers, testUpdateSubmission_answers. + */ + public function dataForSubmission_answers() { $form = new Form(); $form->setId(1); $form->setHash('hash'); @@ -678,6 +685,16 @@ public function testNewSubmission_answers() { 5 => ['ignore unknown question'], ]; + return [ + 'test' => [$form, $questions, $answers], + ]; + } + + /** + * @dataProvider dataForSubmission_answers() + */ + public function testNewSubmission_answers($form, $questions, $answers) { + $this->formMapper->expects($this->once()) ->method('findById') ->with(1) @@ -832,6 +849,61 @@ public function testNewSubmission_validateSubmission() { $this->apiController->newSubmission(1, [], ''); } + /** + * @dataProvider dataForSubmission_answers() + */ + public function testUpdateSubmission_answers($form, $questions, $answers) { + + $form->setAllowEdit(true); + $submission = new Submission(); + $submission->setId(12); + + $this->formMapper->expects($this->once()) + ->method('findById') + ->with(1) + ->willReturn($form); + + $this->formsService->expects($this->once()) + ->method('getQuestions') + ->with(1) + ->willReturn($questions); + + $this->formAccess(); + + $this->submissionService + ->method('validateSubmission') + ->willReturn(true); + + $this->submissionMapper->expects($this->once()) + ->method('findByFormAndUser') + ->with(1, 'currentUser') + ->willReturn($submission); + + $userFolder = $this->createMock(Folder::class); + $userFolder->expects($this->once()) + ->method('nodeExists') + ->willReturn(true); + + $file = $this->createMock(File::class); + + $userFolder->expects($this->once()) + ->method('getById') + ->willReturn([$file]); + + $folder = $this->createMock(Folder::class); + + $userFolder->expects($this->once()) + ->method('get') + ->willReturn($folder); + + $this->storage->expects($this->once()) + ->method('getUserFolder') + ->with('admin') + ->willReturn($userFolder); + + $this->apiController->updateSubmission(1, 12, $answers, ''); + } + public function testDeleteSubmissionNotFound() { $exception = $this->createMock(MapperException::class); @@ -937,6 +1009,7 @@ public function dataTestDeletePermission() { 'expires' => 0, 'isAnonymous' => false, 'submitMultiple' => false, + 'allowEdit' => false, 'showExpiration' => false ], ] diff --git a/tests/Unit/FormsMigratorTest.php b/tests/Unit/FormsMigratorTest.php index 5dfd16c42..c477a7139 100644 --- a/tests/Unit/FormsMigratorTest.php +++ b/tests/Unit/FormsMigratorTest.php @@ -108,6 +108,7 @@ public function dataExport() { "state": 0, "isAnonymous": false, "submitMultiple": false, + "allowEdit": false, "showExpiration": false, "lastUpdated": 123456789, "submissionMessage": "Back to website", @@ -179,6 +180,7 @@ public function testExport(string $expectedJson) { $form->setExpires(0); $form->setIsAnonymous(false); $form->setSubmitMultiple(false); + $form->setAllowEdit(false); $form->setShowExpiration(false); $form->setLastUpdated(123456789); $form->setSubmissionMessage('Back to website'); @@ -251,7 +253,7 @@ public function testExport(string $expectedJson) { public function dataImport() { return [ 'exactlyOneOfEach' => [ - '$inputJson' => '[{"title":"Link","description":"","created":1646251830,"access":{"permitAllUsers":false,"showToAllUsers":false},"expires":0,"state":0,"isAnonymous":false,"submitMultiple":false,"showExpiration":false,"lastUpdated":123456789,"questions":[{"id":14,"order":2,"type":"multiple","isRequired":false,"text":"checkbox","description":"huhu","extraSettings":{},"options":[{"text":"ans1"}]}],"submissions":[{"userId":"anyUser@localhost","timestamp":1651354059,"answers":[{"questionId":14,"text":"ans1"}]}]}]' + '$inputJson' => '[{"title":"Link","description":"","created":1646251830,"access":{"permitAllUsers":false,"showToAllUsers":false},"expires":0,"state":0,"isAnonymous":false,"submitMultiple":false,"allowEdit":false,"showExpiration":false,"lastUpdated":123456789,"questions":[{"id":14,"order":2,"type":"multiple","isRequired":false,"text":"checkbox","description":"huhu","extraSettings":{},"options":[{"text":"ans1"}]}],"submissions":[{"userId":"anyUser@localhost","timestamp":1651354059,"answers":[{"questionId":14,"text":"ans1"}]}]}]' ] ]; } diff --git a/tests/Unit/Service/FormsServiceTest.php b/tests/Unit/Service/FormsServiceTest.php index a5b06b752..db6556a31 100644 --- a/tests/Unit/Service/FormsServiceTest.php +++ b/tests/Unit/Service/FormsServiceTest.php @@ -32,6 +32,8 @@ function microtime(bool|float $asFloat = false) { use OCA\Forms\Activity\ActivityManager; use OCA\Forms\Constants; +use OCA\Forms\Db\Answer; +use OCA\Forms\Db\AnswerMapper; use OCA\Forms\Db\Form; use OCA\Forms\Db\FormMapper; use OCA\Forms\Db\Option; @@ -85,6 +87,9 @@ class FormsServiceTest extends TestCase { /** @var SubmissionMapper|MockObject */ private $submissionMapper; + /** @var AnswerMapper|MockObject */ + private $answerMapper; + /** @var ConfigService|MockObject */ private $configService; @@ -114,6 +119,7 @@ public function setUp(): void { $this->questionMapper = $this->createMock(QuestionMapper::class); $this->shareMapper = $this->createMock(ShareMapper::class); $this->submissionMapper = $this->createMock(SubmissionMapper::class); + $this->answerMapper = $this->createMock(AnswerMapper::class); $this->configService = $this->createMock(ConfigService::class); $this->groupManager = $this->createMock(IGroupManager::class); @@ -147,6 +153,7 @@ public function setUp(): void { $this->questionMapper, $this->shareMapper, $this->submissionMapper, + $this->answerMapper, $this->configService, $this->groupManager, $this->userManager, @@ -243,17 +250,13 @@ public function dataGetForm() { 'submissionMessage' => null, 'fileId' => null, 'fileFormat' => null, - 'permissions' => Constants::PERMISSION_ALL + 'permissions' => Constants::PERMISSION_ALL, + 'allowEdit' => false, ]] ]; } - /** - * @dataProvider dataGetForm - * - * @param array $expected - */ - public function testGetForm(array $expected) { + private function prepareFormTest(bool $withUserMock) { // The form $form = new Form(); $form->setId(42); @@ -275,13 +278,15 @@ public function testGetForm(array $expected) { // User & Group Formatting $user = $this->createMock(IUser::class); - $user->expects($this->once()) - ->method('getDisplayName') - ->willReturn('Some User'); - $this->userManager->expects($this->once()) - ->method('get') - ->with('someUser') - ->willReturn($user); + if ($withUserMock) { + $user->expects($this->once()) + ->method('getDisplayName') + ->willReturn('Some User'); + $this->userManager->expects($this->once()) + ->method('get') + ->with('someUser') + ->willReturn($user); + } // Questions $question1 = new Question(); @@ -331,6 +336,30 @@ public function testGetForm(array $expected) { $share->setShareWith('someUser'); $share->setPermissions([Constants::PERMISSION_SUBMIT]); + $answerEntity = new Answer(); + $answerEntity->setId(1); + $answerEntity->setSubmissionId(12); + $answerEntity->setFileId(112); + $answerEntity->setQuestionId(1); + $answerEntity->setText('Option 1'); + $answer2Entity = new Answer(); + $answer2Entity->setId(2); + $answer2Entity->setSubmissionId(12); + $answer2Entity->setQuestionId(2); + $answer2Entity->setText('London'); + $answers = [ $answerEntity, $answer2Entity ]; + + return [$form, $user, $share, $answers]; + } + + /** + * @dataProvider dataGetForm + * + * @param array $expected + */ + public function testGetForm(array $expected) { + [$form, $user, $share, $answers] = $this->prepareFormTest(true); + $this->shareMapper->expects($this->any()) ->method('findByForm') ->with(42) @@ -345,6 +374,150 @@ public function testGetForm(array $expected) { $this->assertEquals($expected, $this->formsService->getForm($form)); } + public function dataGetFormWithAnswers() { + return [ + // Just the full form with answers for AllowEdit + 'one-full-form-with-answers' => [[ + 'id' => 42, + 'state' => 0, + 'hash' => 'abcdefg', + 'title' => 'Form 1', + 'description' => 'Description Text', + 'ownerId' => 'currentUser', + 'created' => 123456789, + 'access' => [ + 'permitAllUsers' => false, + 'showToAllUsers' => false, + ], + 'expires' => 0, + 'isAnonymous' => false, + 'submitMultiple' => false, + 'showExpiration' => false, + 'lastUpdated' => 123456789, + 'canSubmit' => true, + 'submissionCount' => 123, + 'questions' => [ + [ + 'id' => 1, + 'formId' => 42, + 'order' => 1, + 'type' => 'dropdown', + 'isRequired' => false, + 'extraSettings' => ['shuffleOptions' => true], + 'text' => 'Question 1', + 'description' => 'This is our first question.', + 'name' => '', + 'options' => [ + [ + 'id' => 1, + 'questionId' => 1, + 'text' => 'Option 1', + 'order' => null, + ], + [ + 'id' => 2, + 'questionId' => 1, + 'text' => 'Option 2', + 'order' => null, + ] + ], + 'accept' => [], + ], + [ + 'id' => 2, + 'formId' => 42, + 'order' => 2, + 'type' => 'short', + 'isRequired' => true, + 'extraSettings' => [], + 'text' => 'Question 2', + 'description' => '', + 'name' => 'city', + 'options' => [], + 'accept' => [], + ] + ], + 'shares' => [ + ], + 'answers' => [ + 1 => [ 0 => '1' ], + 2 => [ 0 => 'London' ], + ], + 'submissionMessage' => null, + 'fileId' => null, + 'fileFormat' => null, + 'permissions' => Constants::PERMISSION_ALL, + 'allowEdit' => true, + 'newSubmission' => false, + 'submissionId' => 12, + ]] + ]; + } + + /** + * @dataProvider dataGetFormWithAnswers + * + * @param array $expected + */ + public function testGetFormAllowEditWithAnswers(array $expected) { + [$form, $user, $share, $answers] = $this->prepareFormTest(false); + + // The form, with AllowEdit + $form->setSubmitMultiple(false); + $form->setAllowEdit(true); + + $submission = new Submission(); + $submission->setId(12); + + $this->submissionMapper->expects($this->once()) + ->method('findByFormAndUser') + ->with(42, 'currentUser') + ->willReturn($submission); + + $this->answerMapper->expects($this->once()) + ->method('findBySubmission') + ->with(12) + ->willReturn($answers); + + $this->submissionMapper->expects($this->once()) + ->method('countSubmissions') + ->with(42) + ->willReturn(123); + + // Run the test + $this->assertEquals($expected, $this->formsService->getForm($form)); + } + + /** + * @dataProvider dataGetFormWithAnswers + * + * @param array $expected + */ + public function testGetFormAllowEditWithoutSubmission(array $expected) { + // drop the submissions and answers for this test + unset($expected['answers']); + unset($expected['newSubmission']); + unset($expected['submissionId']); + [$form, $user, $share, $answers] = $this->prepareFormTest(false); + + // The form, with AllowEdit + $form->setSubmitMultiple(false); + $form->setAllowEdit(true); + + $this->submissionMapper->expects($this->once()) + ->method('findByFormAndUser') + ->with(42, 'currentUser') + ->willThrowException(new DoesNotExistException('Test exception')); + + $this->submissionMapper->expects($this->once()) + ->method('countSubmissions') + ->with(42) + ->willReturn(123); + + // Run the test + $this->assertEquals($expected, $this->formsService->getForm($form)); + } + public function dataGetPartialForm() { return [ 'onePartialOwnedForm' => [[ @@ -456,6 +629,7 @@ public function dataGetPublicForm() { 'submit' ], 'submissionMessage' => null, + 'allowEdit' => false, ]] ]; } @@ -622,6 +796,7 @@ public function testGetPermissions_NotLoggedIn() { $this->questionMapper, $this->shareMapper, $this->submissionMapper, + $this->answerMapper, $this->configService, $this->groupManager, $this->userManager, @@ -710,8 +885,15 @@ public function dataCanDeleteResults() { 'allowFormOwner' => [ 'ownerId' => 'currentUser', 'sharesArray' => [], + 'formArchived' => false, 'expected' => true ], + 'disallowArchivedForm' => [ + 'ownerId' => 'currentUser', + 'sharesArray' => [], + 'formArchived' => true, + 'expected' => false + ], 'allowShared' => [ 'ownerId' => 'someUser', 'sharesArray' => [[ @@ -719,11 +901,13 @@ public function dataCanDeleteResults() { 'type' => 0, 'permissions' => [Constants::PERMISSION_SUBMIT, Constants::PERMISSION_RESULTS, Constants::PERMISSION_RESULTS_DELETE], ]], + 'formArchived' => false, 'expected' => true ], 'disallowNotowned' => [ 'ownerId' => 'someUser', 'sharesArray' => [], + 'formArchived' => false, 'expected' => false ], 'allowNotShared' => [ @@ -733,6 +917,7 @@ public function dataCanDeleteResults() { 'type' => 0, 'permissions' => [Constants::PERMISSION_SUBMIT, Constants::PERMISSION_RESULTS], ]], + 'formArchived' => false, 'expected' => false ] ]; @@ -742,9 +927,10 @@ public function dataCanDeleteResults() { * * @param string $ownerId * @param array $sharesArray + * @param bool $formArchived * @param bool $expected */ - public function testCanDeleteResults(string $ownerId, array $sharesArray, bool $expected) { + public function testCanDeleteResults(string $ownerId, array $sharesArray, bool $formArchived, bool $expected) { $form = new Form(); $form->setId(42); $form->setOwnerId($ownerId); @@ -752,6 +938,7 @@ public function testCanDeleteResults(string $ownerId, array $sharesArray, bool $ 'permitAllUsers' => false, 'showToAllUsers' => false, ]); + $form->setState($formArchived?Constants::FORM_STATE_ARCHIVED:Constants::FORM_STATE_ACTIVE); $shares = []; foreach ($sharesArray as $id => $share) { @@ -777,27 +964,45 @@ public function dataCanSubmit() { 'allowFormOwner' => [ 'ownerId' => 'currentUser', 'submitMultiple' => false, + 'allowEdit' => false, 'hasFormSubmissionsByUser' => true, 'expected' => true ], 'submitMultipleGood' => [ 'ownerId' => 'someUser', 'submitMultiple' => false, + 'allowEdit' => false, 'hasFormSubmissionsByUser' => false, 'expected' => true ], 'submitMultipleNotGood' => [ 'ownerId' => 'someUser', 'submitMultiple' => false, + 'allowEdit' => false, 'hasFormSubmissionsByUser' => true, 'expected' => false ], 'submitMultiple' => [ 'ownerId' => 'someUser', 'submitMultiple' => true, + 'allowEdit' => false, 'hasFormSubmissionsByUser' => true, 'expected' => true - ] + ], + 'allowEditGood' => [ + 'ownerId' => 'someUser', + 'submitMultiple' => false, + 'allowEdit' => true, + 'hasFormSubmissionsByUser' => true, + 'expected' => true + ], + 'allowEditNotGood' => [ + 'ownerId' => 'someUser', + 'submitMultiple' => false, + 'allowEdit' => false, + 'hasFormSubmissionsByUser' => true, + 'expected' => false + ], ]; } /** @@ -808,7 +1013,7 @@ public function dataCanSubmit() { * @param bool $hasFormSubmissionsByUser * @param bool $expected */ - public function testCanSubmit(string $ownerId, bool $submitMultiple, bool $hasFormSubmissionsByUser, bool $expected) { + public function testCanSubmit(string $ownerId, bool $submitMultiple, bool $allowEdit, bool $hasFormSubmissionsByUser, bool $expected) { $form = new Form(); $form->setId(42); $form->setAccess([ @@ -817,6 +1022,7 @@ public function testCanSubmit(string $ownerId, bool $submitMultiple, bool $hasFo ]); $form->setOwnerId($ownerId); $form->setSubmitMultiple($submitMultiple); + $form->setAllowEdit($allowEdit); $this->submissionMapper->expects($this->any()) ->method('hasFormSubmissionsByUser') @@ -861,6 +1067,7 @@ public function testPublicCanSubmit() { $this->questionMapper, $this->shareMapper, $this->submissionMapper, + $this->answerMapper, $this->configService, $this->groupManager, $this->userManager, @@ -972,6 +1179,7 @@ public function testHasUserAccess_NotLoggedIn() { $this->questionMapper, $this->shareMapper, $this->submissionMapper, + $this->answerMapper, $this->configService, $this->groupManager, $this->userManager, @@ -1203,6 +1411,7 @@ public function testNotifyNewSubmission($shares, $shareNotifications) { $this->questionMapper, $this->shareMapper, $this->submissionMapper, + $this->answerMapper, $this->configService, $this->groupManager, $this->userManager,