From 1bae251f068a3be5601205f65c4ba63e1109f18e Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Tue, 31 Dec 2024 12:08:51 +0100 Subject: [PATCH 01/44] patch for AllowEdit for Forms5 Signed-off-by: Timotheus Pokorra --- docs/DataStructure.md | 2 + lib/Controller/ApiController.php | 195 +++++++++++++++++- lib/Db/AnswerMapper.php | 20 ++ lib/Db/Form.php | 5 + lib/Db/SubmissionMapper.php | 23 +++ lib/FormsMigrator.php | 1 + .../Version030300Date20230815000000.php | 59 ++++++ lib/Service/FormsService.php | 51 ++++- src/components/Questions/QuestionDropdown.vue | 2 +- .../SidebarTabs/SettingsSidebarTab.vue | 35 +++- src/mixins/ViewsMixin.js | 8 +- src/views/Submit.vue | 71 ++++++- 12 files changed, 451 insertions(+), 21 deletions(-) create mode 100644 lib/Migration/Version030300Date20230815000000.php 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..ef6ed82c1 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,64 @@ 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 + * @throws OCSForbiddenException + */ + #[CORS()] + #[NoAdminRequired()] + #[NoCSRFRequired()] + #[PublicPage()] + #[ApiRoute(verb: 'POST', url: Constants::API_BASE . 'forms/{formId}/submissions/{submissionId}', requirements: Constants::API_V3_REQUIREMENTS)] + 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, + ]); + + list($form, $questions) = $this->checkAndPrepareSubmission($formId, $answers, $shareHash); + + // if edit is allowed get existing submission of this user + if ($form->getAllowEdit() && $this->currentUser) { + try { + $submission = $this->submissionMapper->findByFormAndUser($form->getId(), $this->currentUser->getUID()); + } catch (DoesNotExistException $e) { + throw new OCSBadRequestException('Cannot update a non existing submission'); + } + } else { + throw new OCSBadRequestException('Can only update if AllowEdit is set'); + } + + $submission->setTimestamp(time()); + $this->submissionMapper->update($submission); + + // 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(); + } + /** * Delete a specific submission * @@ -1520,14 +1579,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, $submissionId, array $question, array $answerArray, bool $update) { + // get stored answers for this question + $storedAnswers = []; + if ($update) { + $storedAnswers = $this->answerMapper->findBySubmissionAndQuestion($submissionId, $question['id']); + } + + $newAnswerTexts = array(); + foreach ($answerArray as $answer) { $answerEntity = new Answer(); $answerEntity->setSubmissionId($submissionId); @@ -1544,6 +1612,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,18 +1658,40 @@ private function storeAnswersForQuestion(Form $form, $submissionId, array $quest $file->move($folder->getPath() . '/' . $name); $answerText = $name; + + $answerEntity->setText($answerText); + $this->answerMapper->insert($answerEntity); + if ($uploadedFile) { + $this->uploadedFileMapper->delete($uploadedFile); + } } 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); + } + } } } } @@ -1616,6 +1733,64 @@ private function loadFormForSubmission(int $formId, string $shareHash): Form { return $form; } + /** + * check a submission and return some required data objects + * + * @param int $formId the form id + * @param array $answers [question_id => arrayOfString] + * @param string $shareHash public share-hash -> Necessary to submit on public link-shares. + * @return array + * @throws OCSBadRequestException + * @throws OCSForbiddenException + */ + private function checkAndPrepareSubmission(int $formId, array $answers, string $shareHash = ''): array { + try { + $form = $this->formMapper->findById($formId); + $questions = $this->formsService->getQuestions($formId); + } catch (IMapperException $e) { + $this->logger->debug('Could not find form'); + throw new OCSBadRequestException(); + } + + // Does the user have access to the form (Either by logged in user, or by providing public share-hash.) + try { + $isPublicShare = false; + + // If hash given, find the corresponding share & check if hash corresponds to given formId. + if ($shareHash !== '') { + // public by legacy Link + if (isset($form->getAccess()['legacyLink']) && $shareHash === $form->getHash()) { + $isPublicShare = true; + } + + // Public link share + $share = $this->shareMapper->findPublicShareByHash($shareHash); + if ($share->getFormId() === $formId) { + $isPublicShare = true; + } + } + } catch (DoesNotExistException $e) { + // $isPublicShare already false. + } finally { + // Now forbid, if no public share and no direct share. + if (!$isPublicShare && !$this->formsService->hasUserAccess($form)) { + throw new OCSForbiddenException('Not allowed to access this form'); + } + } + + // Not allowed if form has expired. + if ($this->formsService->hasFormExpired($form)) { + throw new OCSForbiddenException('This form is no longer taking answers'); + } + + // Is the submission valid + if (!$this->submissionService->validateSubmission($questions, $answers, $form->getOwnerId())) { + throw new OCSBadRequestException('At least one submitted answer is not valid'); + } + + return array($form, $questions); + } + /** * Helper that retrieves a form if the current user is allowed to edit it * This throws an exception in case either the form is not found or permissions are missing. 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..d7ad906ca 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', 'bool'); $this->addType('showExpiration', 'boolean'); $this->addType('lastUpdated', 'integer'); $this->addType('state', 'integer'); @@ -160,6 +164,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/Version030300Date20230815000000.php b/lib/Migration/Version030300Date20230815000000.php new file mode 100644 index 000000000..8106f3565 --- /dev/null +++ b/lib/Migration/Version030300Date20230815000000.php @@ -0,0 +1,59 @@ + + * + * @author Timotheus Pokorra + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Forms\Migration; + +use Closure; +use OCP\DB\ISchemaWrapper; +use OCP\DB\Types; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +class Version030300Date20230815000000 extends SimpleMigrationStep { + + /** + * @param IOutput $output + * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` + * @param array $options + * @return null|ISchemaWrapper + */ + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + $table = $schema->getTable('forms_v2_forms'); + + if (!$table->hasColumn('allow_edit')) { + $table->addColumn('allow_edit', Types::BOOLEAN, [ + 'notnull' => false, + 'default' => 0, + ]); + + return $schema; + } + + return null; + } +} \ No newline at end of file diff --git a/lib/Service/FormsService.php b/lib/Service/FormsService.php index e07664009..c01d214c0 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; @@ -54,6 +55,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 +98,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] = array(); + } + $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 +221,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) { + // do nothing + } catch (MultipleObjectsReturnedException $e) { + // do nothing + } + } + $result['shares'] = $this->getShares($form->getId()); // Append permissions for current user. @@ -351,9 +398,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/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..548477bc5 100644 --- a/src/components/SidebarTabs/SettingsSidebarTab.vue +++ b/src/components/SidebarTabs/SettingsSidebarTab.vue @@ -21,6 +21,13 @@ @update:checked="onSubmitMultipleChange"> {{ t('forms', 'Allow multiple responses per person') }} + + {{ t('forms', 'Allow editing and deleting responses per person') }} + {{ t('forms', 'Submit') }} + + + {{ t('forms', 'Delete') }} + @@ -207,6 +218,7 @@ import IconCancelSvg from '@mdi/svg/svg/cancel.svg?raw' import IconCheckSvg from '@mdi/svg/svg/check.svg?raw' import IconRefreshSvg from '@mdi/svg/svg/refresh.svg?raw' import IconSendSvg from '@mdi/svg/svg/send.svg?raw' +import IconDeleteSvg from '@mdi/svg/svg/delete.svg?raw' import { FormState } from '../models/FormStates.ts' import answerTypes from '../models/AnswerTypes.js' @@ -284,6 +296,7 @@ export default { IconCheckSvg, IconRefreshSvg, IconSendSvg, + IconDeleteSvg, maxStringLengths: loadState('forms', 'maxStringLengths'), } @@ -694,15 +707,25 @@ export default { this.loading = true try { - await axios.post( - generateOcsUrl('apps/forms/api/v3/forms/{id}/submissions', { + if (this.newSubmission === false) { + await axios.post( + generateOcsUrl('apps/forms/api/v3/forms/{id}/submissions/{submissionId}', { + id: this.form.id, + submissionId: this.submissionId + }), + { + answers: this.answers, + shareHash: this.shareHash, + }) + } else { + await axios.post(generateOcsUrl('apps/forms/api/v3/forms/{id}/submissions', { id: this.form.id, }), { answers: this.answers, shareHash: this.shareHash, - }, - ) + }) + } this.submitForm = true this.success = true this.deleteFormFieldFromLocalStorage() @@ -724,11 +747,44 @@ export default { this.resetData() }, + /** + * Delete the submission + */ + async onDeleteSubmission() { + if (!confirm(t('forms', 'Are you sure you want to delete your response?'))) { + return + } + + this.loading = true + + try { + if (this.newSubmission === false) { + await axios.delete(generateOcsUrl('apps/forms/api/v3/forms/{id}/submissions/{submissionId}', { + id: this.form.id, + submissionId: this.submissionId + })) + } else { + throw new Error('cannot delete new submission') + } + this.success = true + this.submitForm = true + this.success = true + this.deleteFormFieldFromLocalStorage() + emit('forms:last-updated:set', this.form.id) + } catch (error) { + logger.error('Error while deleting the form submission', { error }) + showError(t('forms', 'There was an error deleting the form submission')) + } finally { + this.loading = false + } + }, + /** * Reset View-Data */ resetData() { this.answers = {} + this.newSubmission = true this.loading = false this.showConfirmLeaveDialog = false this.showClearFormDialog = false @@ -833,6 +889,13 @@ export default { margin-block-end: 160px; padding-inline-start: 20px; } + + .delete-button { + float:right; + background-color: red; + margin: 5px; + padding-inline-start: 20px; + } } } From f59571dfb84764b164bdcf30c8f6a748bba99f47 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Tue, 31 Dec 2024 12:58:24 +0100 Subject: [PATCH 02/44] drop DeleteSubmission, and make Clear button work for AllowEdit Signed-off-by: Timotheus Pokorra --- lib/Controller/ApiController.php | 39 +++++++++++++---------- src/views/Submit.vue | 53 +------------------------------- 2 files changed, 24 insertions(+), 68 deletions(-) diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index ef6ed82c1..555b66323 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -1347,17 +1347,24 @@ public function updateSubmission(int $formId, int $submissionId, array $answers, $submission->setTimestamp(time()); $this->submissionMapper->update($submission); - // 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; + if (empty($answers)) { + // Clear Answers + foreach ($questions as $question) { + $this->storeAnswersForQuestion($form, $submission->getId(), $question, array(''), 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]; + $question = $questions[$questionIndex]; - $this->storeAnswersForQuestion($form, $submission->getId(), $question, $answerArray, true); + $this->storeAnswersForQuestion($form, $submission->getId(), $question, $answerArray, true); + } } //Create Activity @@ -1682,15 +1689,15 @@ private function storeAnswersForQuestion(Form $form, $submissionId, array $quest $this->answerMapper->insert($answerEntity); } } + } - 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); - } + 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); } } } diff --git a/src/views/Submit.vue b/src/views/Submit.vue index 9def3b04d..985021b4f 100644 --- a/src/views/Submit.vue +++ b/src/views/Submit.vue @@ -138,17 +138,6 @@ {{ t('forms', 'Submit') }} - - - {{ t('forms', 'Delete') }} - @@ -707,7 +696,7 @@ export default { this.loading = true try { - if (this.newSubmission === false) { + if (this.submissionId) { await axios.post( generateOcsUrl('apps/forms/api/v3/forms/{id}/submissions/{submissionId}', { id: this.form.id, @@ -747,44 +736,11 @@ export default { this.resetData() }, - /** - * Delete the submission - */ - async onDeleteSubmission() { - if (!confirm(t('forms', 'Are you sure you want to delete your response?'))) { - return - } - - this.loading = true - - try { - if (this.newSubmission === false) { - await axios.delete(generateOcsUrl('apps/forms/api/v3/forms/{id}/submissions/{submissionId}', { - id: this.form.id, - submissionId: this.submissionId - })) - } else { - throw new Error('cannot delete new submission') - } - this.success = true - this.submitForm = true - this.success = true - this.deleteFormFieldFromLocalStorage() - emit('forms:last-updated:set', this.form.id) - } catch (error) { - logger.error('Error while deleting the form submission', { error }) - showError(t('forms', 'There was an error deleting the form submission')) - } finally { - this.loading = false - } - }, - /** * Reset View-Data */ resetData() { this.answers = {} - this.newSubmission = true this.loading = false this.showConfirmLeaveDialog = false this.showClearFormDialog = false @@ -889,13 +845,6 @@ export default { margin-block-end: 160px; padding-inline-start: 20px; } - - .delete-button { - float:right; - background-color: red; - margin: 5px; - padding-inline-start: 20px; - } } } From d6047c418d48b00b9f8eceaedfe596304bff011b Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Tue, 31 Dec 2024 14:43:20 +0100 Subject: [PATCH 03/44] fix php-cs lint issues Signed-off-by: Timotheus Pokorra --- lib/Controller/ApiController.php | 16 ++++++++-------- .../Version030300Date20230815000000.php | 2 +- lib/Service/FormsService.php | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index 555b66323..fc32ec652 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -1331,7 +1331,7 @@ public function updateSubmission(int $formId, int $submissionId, array $answers, 'shareHash' => $shareHash, ]); - list($form, $questions) = $this->checkAndPrepareSubmission($formId, $answers, $shareHash); + [$form, $questions] = $this->checkAndPrepareSubmission($formId, $answers, $shareHash); // if edit is allowed get existing submission of this user if ($form->getAllowEdit() && $this->currentUser) { @@ -1350,7 +1350,7 @@ public function updateSubmission(int $formId, int $submissionId, array $answers, if (empty($answers)) { // Clear Answers foreach ($questions as $question) { - $this->storeAnswersForQuestion($form, $submission->getId(), $question, array(''), true); + $this->storeAnswersForQuestion($form, $submission->getId(), $question, ['']), true); } } else { // Process Answers @@ -1601,7 +1601,7 @@ private function storeAnswersForQuestion(Form $form, $submissionId, array $quest $storedAnswers = $this->answerMapper->findBySubmissionAndQuestion($submissionId, $question['id']); } - $newAnswerTexts = array(); + $newAnswerTexts = []; foreach ($answerArray as $answer) { $answerEntity = new Answer(); @@ -1627,7 +1627,7 @@ private function storeAnswersForQuestion(Form $form, $submissionId, array $quest // has this answer already been stored? $foundAnswer = false; - foreach($storedAnswers as $storedAnswer) { + foreach ($storedAnswers as $storedAnswer) { if ($storedAnswer->getText() == $answerText) { // nothing to be changed $foundAnswer = true; @@ -1635,7 +1635,7 @@ private function storeAnswersForQuestion(Form $form, $submissionId, array $quest } } if (!$foundAnswer) { - if ($answerText === "") { + if ($answerText === '') { continue; } // need to add answer @@ -1679,7 +1679,7 @@ private function storeAnswersForQuestion(Form $form, $submissionId, array $quest $answerEntity->setText($answerText); $this->answerMapper->update($answerEntity); } else { - if ($answerText === "") { + if ($answerText === '') { continue; } $answerEntity = new Answer(); @@ -1693,7 +1693,7 @@ private function storeAnswersForQuestion(Form $form, $submissionId, array $quest if (in_array($question['type'], Constants::ANSWER_TYPES_PREDEFINED)) { // drop all answers that are not in new set of answers - foreach($storedAnswers as $storedAnswer) { + foreach ($storedAnswers as $storedAnswer) { $questionId = $storedAnswer->getQuestionId(); if (empty($newAnswerTexts[$questionId]) || !in_array($storedAnswer->getText(), $newAnswerTexts[$questionId])) { @@ -1795,7 +1795,7 @@ private function checkAndPrepareSubmission(int $formId, array $answers, string $ throw new OCSBadRequestException('At least one submitted answer is not valid'); } - return array($form, $questions); + return [$form, $questions]; } /** diff --git a/lib/Migration/Version030300Date20230815000000.php b/lib/Migration/Version030300Date20230815000000.php index 8106f3565..20be89e46 100644 --- a/lib/Migration/Version030300Date20230815000000.php +++ b/lib/Migration/Version030300Date20230815000000.php @@ -56,4 +56,4 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt return null; } -} \ No newline at end of file +} diff --git a/lib/Service/FormsService.php b/lib/Service/FormsService.php index c01d214c0..de588525a 100644 --- a/lib/Service/FormsService.php +++ b/lib/Service/FormsService.php @@ -106,7 +106,7 @@ private function getAnswers(int $formId, int $submissionId, string $userId): arr $answer = $answerEntity->read(); $questionId = $answer['questionId']; if (!array_key_exists($questionId, $answerList)) { - $answerList[$questionId] = array(); + $answerList[$questionId] = []; } $options = $this->getOptions($answer['questionId']); if (!empty($options)) { From 92a5702eea003cf62060146efd48fbc07f403e84 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Tue, 31 Dec 2024 14:53:03 +0100 Subject: [PATCH 04/44] small fixes Signed-off-by: Timotheus Pokorra --- lib/Controller/ApiController.php | 2 +- .../Version030300Date20230815000000.php | 21 ++----------------- 2 files changed, 3 insertions(+), 20 deletions(-) diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index fc32ec652..31b4b83ed 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -1350,7 +1350,7 @@ public function updateSubmission(int $formId, int $submissionId, array $answers, if (empty($answers)) { // Clear Answers foreach ($questions as $question) { - $this->storeAnswersForQuestion($form, $submission->getId(), $question, ['']), true); + $this->storeAnswersForQuestion($form, $submission->getId(), $question, [''], true); } } else { // Process Answers diff --git a/lib/Migration/Version030300Date20230815000000.php b/lib/Migration/Version030300Date20230815000000.php index 20be89e46..aa496968b 100644 --- a/lib/Migration/Version030300Date20230815000000.php +++ b/lib/Migration/Version030300Date20230815000000.php @@ -3,25 +3,8 @@ declare(strict_types=1); /** - * @copyright Copyright (c) 2023 Timotheus Pokorra - * - * @author Timotheus Pokorra - * - * @license GNU AGPL version 3 or any later version - * - * This program is free software: you can redistribute it and/or modify - * it under the terms of the GNU Affero General Public License as - * published by the Free Software Foundation, either version 3 of the - * License, or (at your option) any later version. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Affero General Public License for more details. - * - * You should have received a copy of the GNU Affero General Public License - * along with this program. If not, see . - * + * SPDX-FileCopyrightText: 2023 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later */ namespace OCA\Forms\Migration; From a03bffb9fe3f7ccf064d7d03ae6474416e275fce Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Tue, 31 Dec 2024 15:02:14 +0100 Subject: [PATCH 05/44] run prettier Signed-off-by: Timotheus Pokorra --- .../SidebarTabs/SettingsSidebarTab.vue | 10 ++++--- src/views/Submit.vue | 28 +++++++++++-------- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/src/components/SidebarTabs/SettingsSidebarTab.vue b/src/components/SidebarTabs/SettingsSidebarTab.vue index 548477bc5..9552c088d 100644 --- a/src/components/SidebarTabs/SettingsSidebarTab.vue +++ b/src/components/SidebarTabs/SettingsSidebarTab.vue @@ -21,7 +21,8 @@ @update:checked="onSubmitMultipleChange"> {{ t('forms', 'Allow multiple responses per person') }} - Date: Tue, 31 Dec 2024 15:09:24 +0100 Subject: [PATCH 06/44] more fixes Signed-off-by: Timotheus Pokorra --- lib/Controller/ApiController.php | 7 ++++--- lib/Db/Form.php | 2 +- lib/Service/FormsService.php | 4 +--- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index 31b4b83ed..9cf44e30b 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -1668,9 +1668,6 @@ private function storeAnswersForQuestion(Form $form, $submissionId, array $quest $answerEntity->setText($answerText); $this->answerMapper->insert($answerEntity); - if ($uploadedFile) { - $this->uploadedFileMapper->delete($uploadedFile); - } } else { $answerText = $answer; // Not a multiple-question, answerText is given answer @@ -1689,6 +1686,10 @@ private function storeAnswersForQuestion(Form $form, $submissionId, array $quest $this->answerMapper->insert($answerEntity); } } + + if ($uploadedFile) { + $this->uploadedFileMapper->delete($uploadedFile); + } } if (in_array($question['type'], Constants::ANSWER_TYPES_PREDEFINED)) { diff --git a/lib/Db/Form.php b/lib/Db/Form.php index d7ad906ca..6a4008feb 100644 --- a/lib/Db/Form.php +++ b/lib/Db/Form.php @@ -74,7 +74,7 @@ public function __construct() { $this->addType('expires', 'integer'); $this->addType('isAnonymous', 'boolean'); $this->addType('submitMultiple', 'boolean'); - $this->addType('allowEdit', 'bool'); + $this->addType('allowEdit', 'boolean'); $this->addType('showExpiration', 'boolean'); $this->addType('lastUpdated', 'integer'); $this->addType('state', 'integer'); diff --git a/lib/Service/FormsService.php b/lib/Service/FormsService.php index de588525a..082ed99a3 100644 --- a/lib/Service/FormsService.php +++ b/lib/Service/FormsService.php @@ -234,9 +234,7 @@ public function getForm(Form $form): array { $result['submissionId'] = $submissionEntity->getId(); } } catch (DoesNotExistException $e) { - // do nothing - } catch (MultipleObjectsReturnedException $e) { - // do nothing + //handle silently } } From 907aa20c4b259ed83644dec9b220f1aba2c219f0 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Fri, 3 Jan 2025 07:36:20 +0100 Subject: [PATCH 07/44] fix existing tests by adding AllowEdit Signed-off-by: Timotheus Pokorra --- tests/Unit/Controller/ApiControllerTest.php | 1 + tests/Unit/FormsMigratorTest.php | 4 +++- tests/Unit/Service/FormsServiceTest.php | 14 +++++++++++++- 3 files changed, 17 insertions(+), 2 deletions(-) diff --git a/tests/Unit/Controller/ApiControllerTest.php b/tests/Unit/Controller/ApiControllerTest.php index 20bf8e2f4..83414a57f 100644 --- a/tests/Unit/Controller/ApiControllerTest.php +++ b/tests/Unit/Controller/ApiControllerTest.php @@ -399,6 +399,7 @@ public function dataTestCreateNewForm() { 'submissionMessage' => null, 'fileId' => null, 'fileFormat' => null, + 'allowEdit' => null, ]] ]; } 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..4814834ef 100644 --- a/tests/Unit/Service/FormsServiceTest.php +++ b/tests/Unit/Service/FormsServiceTest.php @@ -42,6 +42,7 @@ function microtime(bool|float $asFloat = false) { use OCA\Forms\Db\ShareMapper; use OCA\Forms\Db\Submission; use OCA\Forms\Db\SubmissionMapper; +use OCA\Forms\Db\AnswerMapper; use OCA\Forms\Service\CirclesService; use OCA\Forms\Service\ConfigService; use OCA\Forms\Service\FormsService; @@ -85,6 +86,9 @@ class FormsServiceTest extends TestCase { /** @var SubmissionMapper|MockObject */ private $submissionMapper; + /** @var AnswerMapper|MockObject */ + private $answerMapper; + /** @var ConfigService|MockObject */ private $configService; @@ -114,6 +118,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 +152,7 @@ public function setUp(): void { $this->questionMapper, $this->shareMapper, $this->submissionMapper, + $this->answerMapper, $this->configService, $this->groupManager, $this->userManager, @@ -243,7 +249,8 @@ public function dataGetForm() { 'submissionMessage' => null, 'fileId' => null, 'fileFormat' => null, - 'permissions' => Constants::PERMISSION_ALL + 'permissions' => Constants::PERMISSION_ALL, + 'allowEdit' => false, ]] ]; } @@ -456,6 +463,7 @@ public function dataGetPublicForm() { 'submit' ], 'submissionMessage' => null, + 'allowEdit' => false, ]] ]; } @@ -622,6 +630,7 @@ public function testGetPermissions_NotLoggedIn() { $this->questionMapper, $this->shareMapper, $this->submissionMapper, + $this->answerMapper, $this->configService, $this->groupManager, $this->userManager, @@ -861,6 +870,7 @@ public function testPublicCanSubmit() { $this->questionMapper, $this->shareMapper, $this->submissionMapper, + $this->answerMapper, $this->configService, $this->groupManager, $this->userManager, @@ -972,6 +982,7 @@ public function testHasUserAccess_NotLoggedIn() { $this->questionMapper, $this->shareMapper, $this->submissionMapper, + $this->answerMapper, $this->configService, $this->groupManager, $this->userManager, @@ -1203,6 +1214,7 @@ public function testNotifyNewSubmission($shares, $shareNotifications) { $this->questionMapper, $this->shareMapper, $this->submissionMapper, + $this->answerMapper, $this->configService, $this->groupManager, $this->userManager, From 52cebbf24384db07a6fb800c5d6f2c9b347b8cb4 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Fri, 3 Jan 2025 07:44:57 +0100 Subject: [PATCH 08/44] fix lint issue Signed-off-by: Timotheus Pokorra --- tests/Unit/Service/FormsServiceTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Unit/Service/FormsServiceTest.php b/tests/Unit/Service/FormsServiceTest.php index 4814834ef..609d36d34 100644 --- a/tests/Unit/Service/FormsServiceTest.php +++ b/tests/Unit/Service/FormsServiceTest.php @@ -32,6 +32,7 @@ function microtime(bool|float $asFloat = false) { 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\Option; @@ -42,7 +43,6 @@ function microtime(bool|float $asFloat = false) { use OCA\Forms\Db\ShareMapper; use OCA\Forms\Db\Submission; use OCA\Forms\Db\SubmissionMapper; -use OCA\Forms\Db\AnswerMapper; use OCA\Forms\Service\CirclesService; use OCA\Forms\Service\ConfigService; use OCA\Forms\Service\FormsService; From 1b72d9d5ed0ca889b38226c0a7162d683e64e1af Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Fri, 3 Jan 2025 07:47:19 +0100 Subject: [PATCH 09/44] fix existing tests by adding AllowEdit Signed-off-by: Timotheus Pokorra --- tests/Integration/Api/ApiV3Test.php | 2 ++ tests/Unit/Controller/ApiControllerTest.php | 4 ++++ tests/Unit/Service/FormsServiceTest.php | 4 ++++ 3 files changed, 10 insertions(+) diff --git a/tests/Integration/Api/ApiV3Test.php b/tests/Integration/Api/ApiV3Test.php index bd14b8f5a..b36818836 100644 --- a/tests/Integration/Api/ApiV3Test.php +++ b/tests/Integration/Api/ApiV3Test.php @@ -366,6 +366,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 +426,7 @@ public function dataGetFullForm() { 'state' => 0, 'isAnonymous' => false, 'submitMultiple' => false, + 'allowEdit' => false, 'showExpiration' => false, 'lastUpdated' => 123456789, 'canSubmit' => true, diff --git a/tests/Unit/Controller/ApiControllerTest.php b/tests/Unit/Controller/ApiControllerTest.php index 83414a57f..05f5a46f4 100644 --- a/tests/Unit/Controller/ApiControllerTest.php +++ b/tests/Unit/Controller/ApiControllerTest.php @@ -394,6 +394,7 @@ public function dataTestCreateNewForm() { 'expires' => 0, 'isAnonymous' => false, 'submitMultiple' => false, + 'allowEdit' => false, 'showExpiration' => false, 'lastUpdated' => 123456789, 'submissionMessage' => null, @@ -486,6 +487,7 @@ public function dataCloneForm() { 'expires' => 0, 'isAnonymous' => false, 'submitMultiple' => false, + 'allowEdit' => false, 'showExpiration' => false ], 'new' => [ @@ -501,6 +503,7 @@ public function dataCloneForm() { 'expires' => 0, 'isAnonymous' => false, 'submitMultiple' => false, + 'allowEdit' => false, 'showExpiration' => false ] ] @@ -938,6 +941,7 @@ public function dataTestDeletePermission() { 'expires' => 0, 'isAnonymous' => false, 'submitMultiple' => false, + 'allowEdit' => false, 'showExpiration' => false ], ] diff --git a/tests/Unit/Service/FormsServiceTest.php b/tests/Unit/Service/FormsServiceTest.php index 609d36d34..25f69f829 100644 --- a/tests/Unit/Service/FormsServiceTest.php +++ b/tests/Unit/Service/FormsServiceTest.php @@ -786,24 +786,28 @@ 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 ] From f210694991e173a9fca359f213493fc358099146 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Fri, 3 Jan 2025 07:56:20 +0100 Subject: [PATCH 10/44] fix testCanSubmit with AllowEdit=false Signed-off-by: Timotheus Pokorra --- tests/Unit/Service/FormsServiceTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/Unit/Service/FormsServiceTest.php b/tests/Unit/Service/FormsServiceTest.php index 25f69f829..8fe5dea68 100644 --- a/tests/Unit/Service/FormsServiceTest.php +++ b/tests/Unit/Service/FormsServiceTest.php @@ -821,7 +821,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([ @@ -830,6 +830,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') From b17e1b62365226bceb56036b47db3be9aeae9a30 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Fri, 3 Jan 2025 08:10:05 +0100 Subject: [PATCH 11/44] extend testCanSubmit by allowEditGood and allowEditNotGood Signed-off-by: Timotheus Pokorra --- tests/Unit/Service/FormsServiceTest.php | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tests/Unit/Service/FormsServiceTest.php b/tests/Unit/Service/FormsServiceTest.php index 8fe5dea68..730e97396 100644 --- a/tests/Unit/Service/FormsServiceTest.php +++ b/tests/Unit/Service/FormsServiceTest.php @@ -810,7 +810,21 @@ public function dataCanSubmit() { '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 + ], ]; } /** From 287af92aa77833bb51ca1c23967d6ee0b442e27f Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Sat, 4 Jan 2025 01:02:34 +0100 Subject: [PATCH 12/44] add unit test testUpdateSubmission_answers Signed-off-by: Timotheus Pokorra --- lib/Controller/ApiController.php | 6 +- tests/Unit/Controller/ApiControllerTest.php | 70 ++++++++++++++++++++- 2 files changed, 74 insertions(+), 2 deletions(-) diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index 9cf44e30b..8a5ccbed7 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -1344,6 +1344,10 @@ public function updateSubmission(int $formId, int $submissionId, array $answers, throw new OCSBadRequestException('Can only update if AllowEdit is set'); } + if ($submissionId != $submission->getId()) { + throw new OCSBadRequestException('Can only update your own submissions'); + } + $submission->setTimestamp(time()); $this->submissionMapper->update($submission); @@ -1594,7 +1598,7 @@ public function uploadFiles(int $formId, int $questionId, string $shareHash = '' * @param string[]|array $answerArray * @param bool $update */ - private function storeAnswersForQuestion(Form $form, $submissionId, array $question, array $answerArray, bool $update) { + private function storeAnswersForQuestion(Form $form, int $submissionId, array $question, array $answerArray, bool $update) { // get stored answers for this question $storedAnswers = []; if ($update) { diff --git a/tests/Unit/Controller/ApiControllerTest.php b/tests/Unit/Controller/ApiControllerTest.php index 05f5a46f4..5de62aa36 100644 --- a/tests/Unit/Controller/ApiControllerTest.php +++ b/tests/Unit/Controller/ApiControllerTest.php @@ -634,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'); @@ -682,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) @@ -836,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); From 80915d3eced2dcdfcca2e3ac3f05421c6de04c0f Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Sat, 4 Jan 2025 08:06:37 +0100 Subject: [PATCH 13/44] add unit test testGetFormWithAnswers for AllowEdit Signed-off-by: Timotheus Pokorra --- tests/Unit/Service/FormsServiceTest.php | 184 ++++++++++++++++++++++++ 1 file changed, 184 insertions(+) diff --git a/tests/Unit/Service/FormsServiceTest.php b/tests/Unit/Service/FormsServiceTest.php index 730e97396..50709e9ad 100644 --- a/tests/Unit/Service/FormsServiceTest.php +++ b/tests/Unit/Service/FormsServiceTest.php @@ -32,6 +32,7 @@ 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; @@ -43,6 +44,7 @@ function microtime(bool|float $asFloat = false) { use OCA\Forms\Db\ShareMapper; use OCA\Forms\Db\Submission; use OCA\Forms\Db\SubmissionMapper; +use OCA\Forms\Service\AnswerService; use OCA\Forms\Service\CirclesService; use OCA\Forms\Service\ConfigService; use OCA\Forms\Service\FormsService; @@ -352,6 +354,188 @@ public function testGetForm(array $expected) { $this->assertEquals($expected, $this->formsService->getForm($form)); } + public function dataGetFormWithAnswers() { + return [ + // Just the full form with Answers + '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 testGetFormWithAnswers(array $expected) { + // The form, with AllowEdit + $form = new Form(); + $form->setId(42); + $form->setState(0); // default => 0 means active + $form->setHash('abcdefg'); + $form->setTitle('Form 1'); + $form->setDescription('Description Text'); + $form->setOwnerId('currentUser'); + $form->setCreated(123456789); + $form->setAccess([ + 'permitAllUsers' => false, + 'showToAllUsers' => false, + ]); + $form->setExpires(0); + $form->setIsAnonymous(false); + $form->setSubmitMultiple(false); + $form->setAllowEdit(true); + $form->setShowExpiration(false); + $form->setLastUpdated(123456789); + + $submission = new Submission(); + $submission->setId(12); + + // Questions + $question1 = new Question(); + $question1->setId(1); + $question1->setFormId(42); + $question1->setOrder(1); + $question1->setType('dropdown'); + $question1->setIsRequired(false); + $question1->setExtraSettings([ + 'shuffleOptions' => true + ]); + $question1->setText('Question 1'); + $question1->setDescription('This is our first question.'); + $question2 = new Question(); + $question2->setId(2); + $question2->setFormId(42); + $question2->setOrder(2); + $question2->setType('short'); + $question2->setIsRequired(true); + $question2->setText('Question 2'); + $question2->setDescription(''); + $question2->setName('city'); + $question2->setExtraSettings([]); + $this->questionMapper->expects($this->once()) + ->method('findByForm') + ->with(42) + ->willReturn([$question1, $question2]); + + // Options + $option1 = new Option(); + $option1->setId(1); + $option1->setQuestionId(1); + $option1->setText('Option 1'); + $option2 = new Option(); + $option2->setId(2); + $option2->setQuestionId(1); + $option2->setText('Option 2'); + $this->optionMapper->expects($this->any()) + ->method('findByQuestion') + ->with(1) + ->willReturn([$option1, $option2]); + + $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 ]; + + $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)); + } + public function dataGetPartialForm() { return [ 'onePartialOwnedForm' => [[ From 1233d44072cd6b65237abec3474bd5140420be55 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Sat, 4 Jan 2025 08:24:19 +0100 Subject: [PATCH 14/44] add unit test testGetFormAllowEditWithoutAnswers Signed-off-by: Timotheus Pokorra --- tests/Unit/Service/FormsServiceTest.php | 101 +++++++++++++++++++++++- 1 file changed, 98 insertions(+), 3 deletions(-) diff --git a/tests/Unit/Service/FormsServiceTest.php b/tests/Unit/Service/FormsServiceTest.php index 50709e9ad..ad425d472 100644 --- a/tests/Unit/Service/FormsServiceTest.php +++ b/tests/Unit/Service/FormsServiceTest.php @@ -44,7 +44,6 @@ function microtime(bool|float $asFloat = false) { use OCA\Forms\Db\ShareMapper; use OCA\Forms\Db\Submission; use OCA\Forms\Db\SubmissionMapper; -use OCA\Forms\Service\AnswerService; use OCA\Forms\Service\CirclesService; use OCA\Forms\Service\ConfigService; use OCA\Forms\Service\FormsService; @@ -356,7 +355,7 @@ public function testGetForm(array $expected) { public function dataGetFormWithAnswers() { return [ - // Just the full form with Answers + // Just the full form with answers for AllowEdit 'one-full-form-with-answers' => [[ 'id' => 42, 'state' => 0, @@ -439,7 +438,7 @@ public function dataGetFormWithAnswers() { * * @param array $expected */ - public function testGetFormWithAnswers(array $expected) { + public function testGetFormAllowEditWithAnswers(array $expected) { // The form, with AllowEdit $form = new Form(); $form->setId(42); @@ -536,6 +535,102 @@ public function testGetFormWithAnswers(array $expected) { $this->assertEquals($expected, $this->formsService->getForm($form)); } + /** + * @dataProvider dataGetFormWithAnswers + * + * @param array $expected + */ + public function testGetFormAllowEditWithoutAnswers(array $expected) { + // drop the answers for this test + unset($expected['answers']); + unset($expected['newSubmission']); + unset($expected['submissionId']); + + // The form, with AllowEdit + $form = new Form(); + $form->setId(42); + $form->setState(0); // default => 0 means active + $form->setHash('abcdefg'); + $form->setTitle('Form 1'); + $form->setDescription('Description Text'); + $form->setOwnerId('currentUser'); + $form->setCreated(123456789); + $form->setAccess([ + 'permitAllUsers' => false, + 'showToAllUsers' => false, + ]); + $form->setExpires(0); + $form->setIsAnonymous(false); + $form->setSubmitMultiple(false); + $form->setAllowEdit(true); + $form->setShowExpiration(false); + $form->setLastUpdated(123456789); + + $submission = new Submission(); + $submission->setId(12); + + // Questions + $question1 = new Question(); + $question1->setId(1); + $question1->setFormId(42); + $question1->setOrder(1); + $question1->setType('dropdown'); + $question1->setIsRequired(false); + $question1->setExtraSettings([ + 'shuffleOptions' => true + ]); + $question1->setText('Question 1'); + $question1->setDescription('This is our first question.'); + $question2 = new Question(); + $question2->setId(2); + $question2->setFormId(42); + $question2->setOrder(2); + $question2->setType('short'); + $question2->setIsRequired(true); + $question2->setText('Question 2'); + $question2->setDescription(''); + $question2->setName('city'); + $question2->setExtraSettings([]); + $this->questionMapper->expects($this->once()) + ->method('findByForm') + ->with(42) + ->willReturn([$question1, $question2]); + + // Options + $option1 = new Option(); + $option1->setId(1); + $option1->setQuestionId(1); + $option1->setText('Option 1'); + $option2 = new Option(); + $option2->setId(2); + $option2->setQuestionId(1); + $option2->setText('Option 2'); + $this->optionMapper->expects($this->any()) + ->method('findByQuestion') + ->with(1) + ->willReturn([$option1, $option2]); + + $answers = [ ]; + + $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)); + } + public function dataGetPartialForm() { return [ 'onePartialOwnedForm' => [[ From 3e86676fbfe16d6b08af591ef3a69b5404bf4146 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Sat, 4 Jan 2025 08:58:29 +0100 Subject: [PATCH 15/44] improve unit tests testGetFormAllowEditWithoutAnswers Signed-off-by: Timotheus Pokorra --- tests/Unit/Service/FormsServiceTest.php | 193 +++++------------------- 1 file changed, 40 insertions(+), 153 deletions(-) diff --git a/tests/Unit/Service/FormsServiceTest.php b/tests/Unit/Service/FormsServiceTest.php index ad425d472..51bbc360f 100644 --- a/tests/Unit/Service/FormsServiceTest.php +++ b/tests/Unit/Service/FormsServiceTest.php @@ -256,12 +256,7 @@ public function dataGetForm() { ]; } - /** - * @dataProvider dataGetForm - * - * @param array $expected - */ - public function testGetForm(array $expected) { + private function prepareFormTest(bool $withUserMock) { // The form $form = new Form(); $form->setId(42); @@ -283,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(); @@ -339,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) @@ -439,83 +460,15 @@ public function dataGetFormWithAnswers() { * @param array $expected */ public function testGetFormAllowEditWithAnswers(array $expected) { + [$form, $user, $share, $answers] = $this->prepareFormTest(false); + // The form, with AllowEdit - $form = new Form(); - $form->setId(42); - $form->setState(0); // default => 0 means active - $form->setHash('abcdefg'); - $form->setTitle('Form 1'); - $form->setDescription('Description Text'); - $form->setOwnerId('currentUser'); - $form->setCreated(123456789); - $form->setAccess([ - 'permitAllUsers' => false, - 'showToAllUsers' => false, - ]); - $form->setExpires(0); - $form->setIsAnonymous(false); $form->setSubmitMultiple(false); $form->setAllowEdit(true); - $form->setShowExpiration(false); - $form->setLastUpdated(123456789); $submission = new Submission(); $submission->setId(12); - // Questions - $question1 = new Question(); - $question1->setId(1); - $question1->setFormId(42); - $question1->setOrder(1); - $question1->setType('dropdown'); - $question1->setIsRequired(false); - $question1->setExtraSettings([ - 'shuffleOptions' => true - ]); - $question1->setText('Question 1'); - $question1->setDescription('This is our first question.'); - $question2 = new Question(); - $question2->setId(2); - $question2->setFormId(42); - $question2->setOrder(2); - $question2->setType('short'); - $question2->setIsRequired(true); - $question2->setText('Question 2'); - $question2->setDescription(''); - $question2->setName('city'); - $question2->setExtraSettings([]); - $this->questionMapper->expects($this->once()) - ->method('findByForm') - ->with(42) - ->willReturn([$question1, $question2]); - - // Options - $option1 = new Option(); - $option1->setId(1); - $option1->setQuestionId(1); - $option1->setText('Option 1'); - $option2 = new Option(); - $option2->setId(2); - $option2->setQuestionId(1); - $option2->setText('Option 2'); - $this->optionMapper->expects($this->any()) - ->method('findByQuestion') - ->with(1) - ->willReturn([$option1, $option2]); - - $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 ]; - $this->submissionMapper->expects($this->once()) ->method('findByFormAndUser') ->with(42, 'currentUser') @@ -540,87 +493,21 @@ public function testGetFormAllowEditWithAnswers(array $expected) { * * @param array $expected */ - public function testGetFormAllowEditWithoutAnswers(array $expected) { - // drop the answers for this test + 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 = new Form(); - $form->setId(42); - $form->setState(0); // default => 0 means active - $form->setHash('abcdefg'); - $form->setTitle('Form 1'); - $form->setDescription('Description Text'); - $form->setOwnerId('currentUser'); - $form->setCreated(123456789); - $form->setAccess([ - 'permitAllUsers' => false, - 'showToAllUsers' => false, - ]); - $form->setExpires(0); - $form->setIsAnonymous(false); $form->setSubmitMultiple(false); $form->setAllowEdit(true); - $form->setShowExpiration(false); - $form->setLastUpdated(123456789); - - $submission = new Submission(); - $submission->setId(12); - - // Questions - $question1 = new Question(); - $question1->setId(1); - $question1->setFormId(42); - $question1->setOrder(1); - $question1->setType('dropdown'); - $question1->setIsRequired(false); - $question1->setExtraSettings([ - 'shuffleOptions' => true - ]); - $question1->setText('Question 1'); - $question1->setDescription('This is our first question.'); - $question2 = new Question(); - $question2->setId(2); - $question2->setFormId(42); - $question2->setOrder(2); - $question2->setType('short'); - $question2->setIsRequired(true); - $question2->setText('Question 2'); - $question2->setDescription(''); - $question2->setName('city'); - $question2->setExtraSettings([]); - $this->questionMapper->expects($this->once()) - ->method('findByForm') - ->with(42) - ->willReturn([$question1, $question2]); - - // Options - $option1 = new Option(); - $option1->setId(1); - $option1->setQuestionId(1); - $option1->setText('Option 1'); - $option2 = new Option(); - $option2->setId(2); - $option2->setQuestionId(1); - $option2->setText('Option 2'); - $this->optionMapper->expects($this->any()) - ->method('findByQuestion') - ->with(1) - ->willReturn([$option1, $option2]); - - $answers = [ ]; $this->submissionMapper->expects($this->once()) ->method('findByFormAndUser') ->with(42, 'currentUser') - ->willReturn($submission); - - $this->answerMapper->expects($this->once()) - ->method('findBySubmission') - ->with(12) - ->willReturn($answers); + ->willThrowException(new DoesNotExistException('Test exception')); $this->submissionMapper->expects($this->once()) ->method('countSubmissions') From f42eaa249fee24dc484cf1eb780ddd10a4f59492 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Sat, 4 Jan 2025 09:17:32 +0100 Subject: [PATCH 16/44] use existing function loadFormForSubmission instead of new function checkAndPrepareSubmission Signed-off-by: Timotheus Pokorra --- lib/Controller/ApiController.php | 70 +++++--------------------------- 1 file changed, 11 insertions(+), 59 deletions(-) diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index 8a5ccbed7..af86d3717 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -1331,7 +1331,17 @@ public function updateSubmission(int $formId, int $submissionId, array $answers, 'shareHash' => $shareHash, ]); - [$form, $questions] = $this->checkAndPrepareSubmission($formId, $answers, $shareHash); + $form = $this->loadFormForSubmission($formId, $shareHash); + + $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'); + } // if edit is allowed get existing submission of this user if ($form->getAllowEdit() && $this->currentUser) { @@ -1745,64 +1755,6 @@ private function loadFormForSubmission(int $formId, string $shareHash): Form { return $form; } - /** - * check a submission and return some required data objects - * - * @param int $formId the form id - * @param array $answers [question_id => arrayOfString] - * @param string $shareHash public share-hash -> Necessary to submit on public link-shares. - * @return array - * @throws OCSBadRequestException - * @throws OCSForbiddenException - */ - private function checkAndPrepareSubmission(int $formId, array $answers, string $shareHash = ''): array { - try { - $form = $this->formMapper->findById($formId); - $questions = $this->formsService->getQuestions($formId); - } catch (IMapperException $e) { - $this->logger->debug('Could not find form'); - throw new OCSBadRequestException(); - } - - // Does the user have access to the form (Either by logged in user, or by providing public share-hash.) - try { - $isPublicShare = false; - - // If hash given, find the corresponding share & check if hash corresponds to given formId. - if ($shareHash !== '') { - // public by legacy Link - if (isset($form->getAccess()['legacyLink']) && $shareHash === $form->getHash()) { - $isPublicShare = true; - } - - // Public link share - $share = $this->shareMapper->findPublicShareByHash($shareHash); - if ($share->getFormId() === $formId) { - $isPublicShare = true; - } - } - } catch (DoesNotExistException $e) { - // $isPublicShare already false. - } finally { - // Now forbid, if no public share and no direct share. - if (!$isPublicShare && !$this->formsService->hasUserAccess($form)) { - throw new OCSForbiddenException('Not allowed to access this form'); - } - } - - // Not allowed if form has expired. - if ($this->formsService->hasFormExpired($form)) { - throw new OCSForbiddenException('This form is no longer taking answers'); - } - - // Is the submission valid - if (!$this->submissionService->validateSubmission($questions, $answers, $form->getOwnerId())) { - throw new OCSBadRequestException('At least one submitted answer is not valid'); - } - - return [$form, $questions]; - } - /** * Helper that retrieves a form if the current user is allowed to edit it * This throws an exception in case either the form is not found or permissions are missing. From b1442d01920e70def18ff0a062c00ccd1c400d37 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Sat, 4 Jan 2025 11:10:14 +0100 Subject: [PATCH 17/44] new function canDeleteSubmission with Tests. improve tests for canDeleteResults Signed-off-by: Timotheus Pokorra --- lib/Service/FormsService.php | 22 +++++++ tests/Unit/Controller/ApiControllerTest.php | 4 +- tests/Unit/Service/FormsServiceTest.php | 67 ++++++++++++++++++++- 3 files changed, 90 insertions(+), 3 deletions(-) diff --git a/lib/Service/FormsService.php b/lib/Service/FormsService.php index 082ed99a3..0d5ec2706 100644 --- a/lib/Service/FormsService.php +++ b/lib/Service/FormsService.php @@ -379,6 +379,28 @@ public function canDeleteResults(Form $form): bool { return !$this->isFormArchived($form); } + /** + * Can the current user delete own submission + * + * @param Form $form + * @param Submission $submission + * @return boolean + */ + public function canDeleteSubmission(Form $form, Submission $submission): bool { + + // Do not allow deleting results on archived forms + if ($this->isFormArchived($form)) { + return false; + } + + // if AllowEdit then the current user can delete own submission + if ($form->getAllowEdit() && $submission->getUserId() == $this->currentUser->getUID()) { + return true; + } + + return false; + } + /** * Can the user submit a form * diff --git a/tests/Unit/Controller/ApiControllerTest.php b/tests/Unit/Controller/ApiControllerTest.php index 5de62aa36..d9b1c43b4 100644 --- a/tests/Unit/Controller/ApiControllerTest.php +++ b/tests/Unit/Controller/ApiControllerTest.php @@ -951,7 +951,7 @@ public function testDeleteSubmissionNoPermission($submissionData, $formData) { $this->formsService ->expects($this->once()) - ->method('canDeleteResults') + ->method('canDeleteSubmission') ->with($form) ->willReturn(false); @@ -978,7 +978,7 @@ public function testDeleteSubmission($submissionData, $formData) { $this->formsService ->expects($this->once()) - ->method('canDeleteResults') + ->method('canDeleteSubmission') ->with($form) ->willReturn(true); diff --git a/tests/Unit/Service/FormsServiceTest.php b/tests/Unit/Service/FormsServiceTest.php index 51bbc360f..0e66c5280 100644 --- a/tests/Unit/Service/FormsServiceTest.php +++ b/tests/Unit/Service/FormsServiceTest.php @@ -885,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' => [[ @@ -894,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' => [ @@ -908,6 +917,7 @@ public function dataCanDeleteResults() { 'type' => 0, 'permissions' => [Constants::PERMISSION_SUBMIT, Constants::PERMISSION_RESULTS], ]], + 'formArchived' => false, 'expected' => false ] ]; @@ -917,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); @@ -927,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) { @@ -947,6 +959,59 @@ public function testCanDeleteResults(string $ownerId, array $sharesArray, bool $ $this->assertEquals($expected, $this->formsService->canDeleteResults($form)); } + public function dataCanDeleteSubmission() { + return [ + 'disallowNoAllowEdit' => [ + 'formArchived' => false, + 'submissionUserId' => 'currentUser', + 'allowEdit' => false, + 'expected' => false + ], + 'disallowArchivedForm' => [ + 'formArchived' => true, + 'submissionUserId' => 'currentUser', + 'allowEdit' => false, + 'expected' => false + ], + 'allowAllowEdit' => [ + 'formArchived' => false, + 'submissionUserId' => 'currentUser', + 'allowEdit' => true, + 'expected' => true + ], + 'disallowAllowEditOtherUser' => [ + 'formArchived' => false, + 'submissionUserId' => 'otherUser', + 'allowEdit' => true, + 'expected' => false + ], + ]; + } + /** + * @dataProvider dataCanDeleteSubmission + * + * @param bool $formArchived + * @param string $submissionUserId, + * @param bool $allowEdit + * @param bool $expected + */ + public function testCanDeleteSubmission(bool $formArchived, string $submissionUserId, bool $allowEdit, bool $expected) { + $form = new Form(); + $form->setId(42); + $form->setAccess([ + 'permitAllUsers' => false, + 'showToAllUsers' => false, + ]); + $form->setState($formArchived?Constants::FORM_STATE_ARCHIVED:Constants::FORM_STATE_ACTIVE); + $form->setAllowEdit($allowEdit); + + $submission = new Submission(); + $submission->setFormId(42); + $submission->setUserId($submissionUserId); + + $this->assertEquals($expected, $this->formsService->canDeleteSubmission($form, $submission)); + } + public function dataCanSubmit() { return [ 'allowFormOwner' => [ From bead1643fbca23a60f46dce8df223c7aa6de9ad6 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Sat, 4 Jan 2025 11:29:08 +0100 Subject: [PATCH 18/44] drop function canDeleteSubmission. it is unrelated to this PR Signed-off-by: Timotheus Pokorra --- lib/Service/FormsService.php | 22 --------- tests/Unit/Controller/ApiControllerTest.php | 4 +- tests/Unit/Service/FormsServiceTest.php | 53 --------------------- 3 files changed, 2 insertions(+), 77 deletions(-) diff --git a/lib/Service/FormsService.php b/lib/Service/FormsService.php index 0d5ec2706..082ed99a3 100644 --- a/lib/Service/FormsService.php +++ b/lib/Service/FormsService.php @@ -379,28 +379,6 @@ public function canDeleteResults(Form $form): bool { return !$this->isFormArchived($form); } - /** - * Can the current user delete own submission - * - * @param Form $form - * @param Submission $submission - * @return boolean - */ - public function canDeleteSubmission(Form $form, Submission $submission): bool { - - // Do not allow deleting results on archived forms - if ($this->isFormArchived($form)) { - return false; - } - - // if AllowEdit then the current user can delete own submission - if ($form->getAllowEdit() && $submission->getUserId() == $this->currentUser->getUID()) { - return true; - } - - return false; - } - /** * Can the user submit a form * diff --git a/tests/Unit/Controller/ApiControllerTest.php b/tests/Unit/Controller/ApiControllerTest.php index d9b1c43b4..5de62aa36 100644 --- a/tests/Unit/Controller/ApiControllerTest.php +++ b/tests/Unit/Controller/ApiControllerTest.php @@ -951,7 +951,7 @@ public function testDeleteSubmissionNoPermission($submissionData, $formData) { $this->formsService ->expects($this->once()) - ->method('canDeleteSubmission') + ->method('canDeleteResults') ->with($form) ->willReturn(false); @@ -978,7 +978,7 @@ public function testDeleteSubmission($submissionData, $formData) { $this->formsService ->expects($this->once()) - ->method('canDeleteSubmission') + ->method('canDeleteResults') ->with($form) ->willReturn(true); diff --git a/tests/Unit/Service/FormsServiceTest.php b/tests/Unit/Service/FormsServiceTest.php index 0e66c5280..db6556a31 100644 --- a/tests/Unit/Service/FormsServiceTest.php +++ b/tests/Unit/Service/FormsServiceTest.php @@ -959,59 +959,6 @@ public function testCanDeleteResults(string $ownerId, array $sharesArray, bool $ $this->assertEquals($expected, $this->formsService->canDeleteResults($form)); } - public function dataCanDeleteSubmission() { - return [ - 'disallowNoAllowEdit' => [ - 'formArchived' => false, - 'submissionUserId' => 'currentUser', - 'allowEdit' => false, - 'expected' => false - ], - 'disallowArchivedForm' => [ - 'formArchived' => true, - 'submissionUserId' => 'currentUser', - 'allowEdit' => false, - 'expected' => false - ], - 'allowAllowEdit' => [ - 'formArchived' => false, - 'submissionUserId' => 'currentUser', - 'allowEdit' => true, - 'expected' => true - ], - 'disallowAllowEditOtherUser' => [ - 'formArchived' => false, - 'submissionUserId' => 'otherUser', - 'allowEdit' => true, - 'expected' => false - ], - ]; - } - /** - * @dataProvider dataCanDeleteSubmission - * - * @param bool $formArchived - * @param string $submissionUserId, - * @param bool $allowEdit - * @param bool $expected - */ - public function testCanDeleteSubmission(bool $formArchived, string $submissionUserId, bool $allowEdit, bool $expected) { - $form = new Form(); - $form->setId(42); - $form->setAccess([ - 'permitAllUsers' => false, - 'showToAllUsers' => false, - ]); - $form->setState($formArchived?Constants::FORM_STATE_ARCHIVED:Constants::FORM_STATE_ACTIVE); - $form->setAllowEdit($allowEdit); - - $submission = new Submission(); - $submission->setFormId(42); - $submission->setUserId($submissionUserId); - - $this->assertEquals($expected, $this->formsService->canDeleteSubmission($form, $submission)); - } - public function dataCanSubmit() { return [ 'allowFormOwner' => [ From 6c5c1e39a4c1ebcea45e7973df4044adc1d3ae6f Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Sat, 4 Jan 2025 11:41:38 +0100 Subject: [PATCH 19/44] adjust label for AllowEdit Signed-off-by: Timotheus Pokorra --- src/components/SidebarTabs/SettingsSidebarTab.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/SidebarTabs/SettingsSidebarTab.vue b/src/components/SidebarTabs/SettingsSidebarTab.vue index 9552c088d..aa05bd3a8 100644 --- a/src/components/SidebarTabs/SettingsSidebarTab.vue +++ b/src/components/SidebarTabs/SettingsSidebarTab.vue @@ -27,7 +27,7 @@ type="switch" :disabled="disableAllowEdit" @update:checked="onAllowEditChange"> - {{ t('forms', 'Allow editing and deleting responses per person') }} + {{ t('forms', 'Allow editing responses per person') }} Date: Thu, 9 Jan 2025 19:55:30 +0100 Subject: [PATCH 20/44] rename migration to Version 5 and current date Signed-off-by: Timotheus Pokorra --- ...20230815000000.php => Version050000Date20250109201500.php} | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename lib/Migration/{Version030300Date20230815000000.php => Version050000Date20250109201500.php} (86%) diff --git a/lib/Migration/Version030300Date20230815000000.php b/lib/Migration/Version050000Date20250109201500.php similarity index 86% rename from lib/Migration/Version030300Date20230815000000.php rename to lib/Migration/Version050000Date20250109201500.php index aa496968b..c7406798b 100644 --- a/lib/Migration/Version030300Date20230815000000.php +++ b/lib/Migration/Version050000Date20250109201500.php @@ -3,7 +3,7 @@ declare(strict_types=1); /** - * SPDX-FileCopyrightText: 2023 Nextcloud GmbH and Nextcloud contributors + * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors * SPDX-License-Identifier: AGPL-3.0-or-later */ @@ -15,7 +15,7 @@ use OCP\Migration\IOutput; use OCP\Migration\SimpleMigrationStep; -class Version030300Date20230815000000 extends SimpleMigrationStep { +class Version050000Date20250109201500 extends SimpleMigrationStep { /** * @param IOutput $output From 804f8a58b54bbc0eb9ccbf2760d082420d29578e Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Thu, 9 Jan 2025 19:56:54 +0100 Subject: [PATCH 21/44] drop IconDeleteSvg from Submit.vue Signed-off-by: Timotheus Pokorra --- src/views/Submit.vue | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/views/Submit.vue b/src/views/Submit.vue index 4069efc14..ec7487d9e 100644 --- a/src/views/Submit.vue +++ b/src/views/Submit.vue @@ -207,7 +207,6 @@ import IconCancelSvg from '@mdi/svg/svg/cancel.svg?raw' import IconCheckSvg from '@mdi/svg/svg/check.svg?raw' import IconRefreshSvg from '@mdi/svg/svg/refresh.svg?raw' import IconSendSvg from '@mdi/svg/svg/send.svg?raw' -import IconDeleteSvg from '@mdi/svg/svg/delete.svg?raw' import { FormState } from '../models/FormStates.ts' import answerTypes from '../models/AnswerTypes.js' @@ -285,7 +284,6 @@ export default { IconCheckSvg, IconRefreshSvg, IconSendSvg, - IconDeleteSvg, maxStringLengths: loadState('forms', 'maxStringLengths'), } From eda710f3eaa707d6748b262f3f739b4bbdd42eac Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Thu, 9 Jan 2025 20:00:48 +0100 Subject: [PATCH 22/44] fix error messages with multiple ors Signed-off-by: Timotheus Pokorra --- src/components/SidebarTabs/SettingsSidebarTab.vue | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/SidebarTabs/SettingsSidebarTab.vue b/src/components/SidebarTabs/SettingsSidebarTab.vue index aa05bd3a8..5959b333e 100644 --- a/src/components/SidebarTabs/SettingsSidebarTab.vue +++ b/src/components/SidebarTabs/SettingsSidebarTab.vue @@ -203,7 +203,7 @@ export default { if (this.disableSubmitMultiple) { return t( 'forms', - 'This can not be controlled, if the form has a public link or stores responses anonymously, or the response can be edited.', + 'This can not be controlled, if the form has a public link, stores responses anonymously, or the response can be edited.', ) } return '' @@ -212,7 +212,7 @@ export default { if (this.disableAllowEdit) { return t( 'forms', - 'This can not be controlled, if the form has a public link or stores responses anonymously, or multiple responses are allowed.', + 'This can not be controlled, if the form has a public link, stores responses anonymously, or multiple responses are allowed.', ) } return '' From 6e89f10afc8674cd4a6d4fe17353ab565393355c Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Thu, 9 Jan 2025 20:02:46 +0100 Subject: [PATCH 23/44] use PUT for updating submission Signed-off-by: Timotheus Pokorra --- lib/Controller/ApiController.php | 2 +- src/views/Submit.vue | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index af86d3717..9aa53bfde 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -1323,7 +1323,7 @@ public function newSubmission(int $formId, array $answers, string $shareHash = ' #[NoAdminRequired()] #[NoCSRFRequired()] #[PublicPage()] - #[ApiRoute(verb: 'POST', url: Constants::API_BASE . 'forms/{formId}/submissions/{submissionId}', requirements: Constants::API_V3_REQUIREMENTS)] + #[ApiRoute(verb: 'PUT', url: Constants::API_BASE . 'forms/{formId}/submissions/{submissionId}', requirements: Constants::API_V3_REQUIREMENTS)] 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, diff --git a/src/views/Submit.vue b/src/views/Submit.vue index ec7487d9e..3b8a04c17 100644 --- a/src/views/Submit.vue +++ b/src/views/Submit.vue @@ -695,7 +695,7 @@ export default { try { if (this.submissionId) { - await axios.post( + await axios.put( generateOcsUrl( 'apps/forms/api/v3/forms/{id}/submissions/{submissionId}', { From 9e202eb9307c3eae0c2579fb1af1adbc26eaea0e Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Thu, 9 Jan 2025 20:09:35 +0100 Subject: [PATCH 24/44] fail silently for MultipleObjectsReturnedException from findByFormAndUser Signed-off-by: Timotheus Pokorra --- lib/Service/FormsService.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/Service/FormsService.php b/lib/Service/FormsService.php index 082ed99a3..5b90b7679 100644 --- a/lib/Service/FormsService.php +++ b/lib/Service/FormsService.php @@ -235,6 +235,8 @@ public function getForm(Form $form): array { } } catch (DoesNotExistException $e) { //handle silently + } catch (MultipleObjectsReturnedException $e) { + //handle silently } } From 45b098b03e851e6b8647ed645075c9b21fbf9bd3 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Thu, 9 Jan 2025 20:12:30 +0100 Subject: [PATCH 25/44] updateSubmission: first check if editing is allowed and by this user Signed-off-by: Timotheus Pokorra --- lib/Controller/ApiController.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index 9aa53bfde..e0c4add01 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -1333,6 +1333,10 @@ public function updateSubmission(int $formId, int $submissionId, array $answers, $form = $this->loadFormForSubmission($formId, $shareHash); + if (!($form->getAllowEdit() && $this->currentUser)) { + 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()); @@ -1343,15 +1347,11 @@ public function updateSubmission(int $formId, int $submissionId, array $answers, throw new OCSBadRequestException('At least one submitted answer is not valid'); } - // if edit is allowed get existing submission of this user - if ($form->getAllowEdit() && $this->currentUser) { - try { - $submission = $this->submissionMapper->findByFormAndUser($form->getId(), $this->currentUser->getUID()); - } catch (DoesNotExistException $e) { - throw new OCSBadRequestException('Cannot update a non existing submission'); - } - } else { - throw new OCSBadRequestException('Can only update if AllowEdit is set'); + // 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()) { From ff54b5b3e5eb7eb2e3c6b055dcf1d97565589d59 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Thu, 9 Jan 2025 20:26:52 +0100 Subject: [PATCH 26/44] add integration test testUpdateSubmission Signed-off-by: Timotheus Pokorra --- tests/Integration/Api/ApiV3Test.php | 82 +++++++++++++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/tests/Integration/Api/ApiV3Test.php b/tests/Integration/Api/ApiV3Test.php index b36818836..b84d52ef5 100644 --- a/tests/Integration/Api/ApiV3Test.php +++ b/tests/Integration/Api/ApiV3Test.php @@ -41,6 +41,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 +165,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 +203,7 @@ private function setTestForms() { 'state' => 0, 'is_anonymous' => false, 'submit_multiple' => false, + 'allow_edit' => true, 'show_expiration' => false, 'last_updated' => 123456789, 'submission_message' => '', @@ -1349,6 +1352,85 @@ public function testNewSubmission() { ], $data['submissions'][0]); } + /** + * @dataProvider dataNewSubmission + */ + public function testUpdateSubmission() { + + $uploadedFileResponse = $this->http->request('PUT', + "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->http->request('PUT', "api/v3/forms/{$this->testForms[0]['id']}/submissions/{$this->testForms[0]['submissions'][0]['id']}", [ + 'json' => [ + 'answers' => [ + $this->testForms[0]['questions'][0]['id'] => ['ShortAnswer!2'], + $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' => 'test', + 'userDisplayName' => 'Test Displayname', + '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); From e10c3121ad36c22adcb0fc35e1a82cd9f00301f2 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Thu, 9 Jan 2025 20:27:59 +0100 Subject: [PATCH 27/44] fix missing MultipleObjectsReturnedException for FormsService Signed-off-by: Timotheus Pokorra --- lib/Service/FormsService.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/Service/FormsService.php b/lib/Service/FormsService.php index 5b90b7679..14ff18028 100644 --- a/lib/Service/FormsService.php +++ b/lib/Service/FormsService.php @@ -23,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; From 11da4d10ebf4b8b8d6f2a4d86661fb26fea36765 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Tue, 21 Jan 2025 22:35:43 +0100 Subject: [PATCH 28/44] testUpdateSubmission: POST files instead of PUT Signed-off-by: Timotheus Pokorra --- tests/Integration/Api/ApiV3Test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Integration/Api/ApiV3Test.php b/tests/Integration/Api/ApiV3Test.php index b84d52ef5..d7cccaefb 100644 --- a/tests/Integration/Api/ApiV3Test.php +++ b/tests/Integration/Api/ApiV3Test.php @@ -1357,7 +1357,7 @@ public function testNewSubmission() { */ public function testUpdateSubmission() { - $uploadedFileResponse = $this->http->request('PUT', + $uploadedFileResponse = $this->http->request('POST', "api/v3/forms/{$this->testForms[0]['id']}/submissions/files/{$this->testForms[0]['questions'][2]['id']}", [ 'multipart' => [ From 6080f538411052caa268c67fa47b5f3cb9d90351 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Tue, 21 Jan 2025 22:47:42 +0100 Subject: [PATCH 29/44] fixes for Psalm: add allowEdit Signed-off-by: Timotheus Pokorra --- lib/Db/Form.php | 1 + lib/ResponseDefinitions.php | 1 + 2 files changed, 2 insertions(+) diff --git a/lib/Db/Form.php b/lib/Db/Form.php index 6a4008feb..cdd630c4b 100644 --- a/lib/Db/Form.php +++ b/lib/Db/Form.php @@ -144,6 +144,7 @@ public function setAccess(array $access) { * expires: int, * isAnonymous: bool, * submitMultiple: bool, + * allowEdit: bool, * showExpiration: bool, * lastUpdated: int, * submissionMessage: ?string, diff --git a/lib/ResponseDefinitions.php b/lib/ResponseDefinitions.php index c13808147..2cbbd4881 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, From ab8ce5f4db7d600d298cb3b49882e984e552b4c0 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Tue, 21 Jan 2025 22:51:40 +0100 Subject: [PATCH 30/44] fix ApiRoute for updateSubmission Signed-off-by: Timotheus Pokorra --- lib/Controller/ApiController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index e0c4add01..b4a68922e 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -1323,7 +1323,7 @@ public function newSubmission(int $formId, array $answers, string $shareHash = ' #[NoAdminRequired()] #[NoCSRFRequired()] #[PublicPage()] - #[ApiRoute(verb: 'PUT', url: Constants::API_BASE . 'forms/{formId}/submissions/{submissionId}', requirements: Constants::API_V3_REQUIREMENTS)] + #[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, From f0f59bdd2bad035fca645bc7a03987bda1457683 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Tue, 21 Jan 2025 22:54:37 +0100 Subject: [PATCH 31/44] fix RespectAdminSettingsTest Signed-off-by: Timotheus Pokorra --- tests/Integration/Api/RespectAdminSettingsTest.php | 1 + 1 file changed, 1 insertion(+) 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, ], ]; } From 949885924a84b03256e8ad50f8090dd593a150a2 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Tue, 21 Jan 2025 23:06:45 +0100 Subject: [PATCH 32/44] fix openapi errors Signed-off-by: Timotheus Pokorra --- lib/Controller/ApiController.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index b4a68922e..086e85c13 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -1313,11 +1313,11 @@ public function newSubmission(int $formId, array $answers, string $shareHash = ' * * @param int $formId the form id * @param int $submissionId the submission id - * @param array $answers [question_id => arrayOfString] + * @param array> $answers [question_id => arrayOfString] * @param string $shareHash public share-hash -> Necessary to submit on public link-shares. - * @return DataResponse - * @throws OCSBadRequestException - * @throws OCSForbiddenException + * @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 */ #[CORS()] #[NoAdminRequired()] @@ -1355,7 +1355,7 @@ public function updateSubmission(int $formId, int $submissionId, array $answers, } if ($submissionId != $submission->getId()) { - throw new OCSBadRequestException('Can only update your own submissions'); + throw new OCSForbiddenException('Can only update your own submissions'); } $submission->setTimestamp(time()); @@ -1384,7 +1384,7 @@ public function updateSubmission(int $formId, int $submissionId, array $answers, //Create Activity $this->formsService->notifyNewSubmission($form, $submission); - return new DataResponse(); + return new DataResponse($submissionId); } /** From 8d0586ece7ab412a779f0dd6bbb64374bbe814a7 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Tue, 21 Jan 2025 23:14:08 +0100 Subject: [PATCH 33/44] another fix for openapi Signed-off-by: Timotheus Pokorra --- lib/Controller/ApiController.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index 086e85c13..e2ba97161 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -1318,6 +1318,8 @@ public function newSubmission(int $formId, array $answers, string $shareHash = ' * @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()] From c763ecd71d0789a8f2c13eb8b1e1ae30618f80d8 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Tue, 21 Jan 2025 23:19:59 +0100 Subject: [PATCH 34/44] extend FormsForm for psalm Signed-off-by: Timotheus Pokorra --- lib/ResponseDefinitions.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/ResponseDefinitions.php b/lib/ResponseDefinitions.php index 2cbbd4881..af1717cd4 100644 --- a/lib/ResponseDefinitions.php +++ b/lib/ResponseDefinitions.php @@ -120,6 +120,9 @@ * shares: list, * submissionCount?: int, * submissionMessage: ?string, + * answers?: list, + * newSubmission?: bool, + * submissionId?: int, * } * * @psalm-type FormsUploadedFile = array{ From 62ec61d6b0cb7fa282359813e56283ea0c7c16fb Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Tue, 21 Jan 2025 23:23:38 +0100 Subject: [PATCH 35/44] update openapi.json Signed-off-by: Timotheus Pokorra --- openapi.json | 181 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 181 insertions(+) diff --git a/openapi.json b/openapi.json index 9637fa107..ba508822c 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": "array", + "items": { + "$ref": "#/components/schemas/Answer" + } + }, + "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", From 384e551e80022fa9eb7d691f9fafe11f4e847225 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Tue, 21 Jan 2025 23:27:00 +0100 Subject: [PATCH 36/44] another fix for FormsForm Signed-off-by: Timotheus Pokorra --- lib/ResponseDefinitions.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ResponseDefinitions.php b/lib/ResponseDefinitions.php index af1717cd4..31c5c16d9 100644 --- a/lib/ResponseDefinitions.php +++ b/lib/ResponseDefinitions.php @@ -120,7 +120,7 @@ * shares: list, * submissionCount?: int, * submissionMessage: ?string, - * answers?: list, + * answers?: non-empty-array, * newSubmission?: bool, * submissionId?: int, * } From b740d47f843a049129e995cad9e88bb5d418fe53 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Tue, 21 Jan 2025 23:31:48 +0100 Subject: [PATCH 37/44] another fix for FormsForm Signed-off-by: Timotheus Pokorra --- lib/ResponseDefinitions.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ResponseDefinitions.php b/lib/ResponseDefinitions.php index 31c5c16d9..af1717cd4 100644 --- a/lib/ResponseDefinitions.php +++ b/lib/ResponseDefinitions.php @@ -120,7 +120,7 @@ * shares: list, * submissionCount?: int, * submissionMessage: ?string, - * answers?: non-empty-array, + * answers?: list, * newSubmission?: bool, * submissionId?: int, * } From f4fcdd2ecdd64c9c16466831c29080448ac5e837 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Tue, 21 Jan 2025 23:39:06 +0100 Subject: [PATCH 38/44] another attempt to fix openapi issues Signed-off-by: Timotheus Pokorra --- lib/ResponseDefinitions.php | 2 +- openapi.json | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/ResponseDefinitions.php b/lib/ResponseDefinitions.php index af1717cd4..0ebde5954 100644 --- a/lib/ResponseDefinitions.php +++ b/lib/ResponseDefinitions.php @@ -120,7 +120,7 @@ * shares: list, * submissionCount?: int, * submissionMessage: ?string, - * answers?: list, + * answers?: array, * newSubmission?: bool, * submissionId?: int, * } diff --git a/openapi.json b/openapi.json index ba508822c..2c45536c4 100644 --- a/openapi.json +++ b/openapi.json @@ -210,9 +210,9 @@ "nullable": true }, "answers": { - "type": "array", - "items": { - "$ref": "#/components/schemas/Answer" + "type": "object", + "additionalProperties": { + "type": "object" } }, "newSubmission": { From 8fb85d1438d681b44afbdfd317ed239e1c8d9628 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Wed, 22 Jan 2025 11:24:05 +0100 Subject: [PATCH 39/44] simplify check in updateSubmission Signed-off-by: Timotheus Pokorra --- lib/Controller/ApiController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index e2ba97161..6a6ecf4f3 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -1335,7 +1335,7 @@ public function updateSubmission(int $formId, int $submissionId, array $answers, $form = $this->loadFormForSubmission($formId, $shareHash); - if (!($form->getAllowEdit() && $this->currentUser)) { + if (!$form->getAllowEdit()) { throw new OCSBadRequestException('Can only update if AllowEdit is set'); } From 4588f833c3ce1b3fcb701274b5a47f1478263e7c Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Wed, 22 Jan 2025 11:36:21 +0100 Subject: [PATCH 40/44] testUpdateSubmission: set AllowEdit for form Signed-off-by: Timotheus Pokorra --- tests/Integration/Api/ApiV3Test.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/Integration/Api/ApiV3Test.php b/tests/Integration/Api/ApiV3Test.php index d7cccaefb..9564e2f9a 100644 --- a/tests/Integration/Api/ApiV3Test.php +++ b/tests/Integration/Api/ApiV3Test.php @@ -1357,6 +1357,14 @@ public function testNewSubmission() { */ public function testUpdateSubmission() { + $resp = $this->http->request('PATCH', "api/v3/forms/{$this->testForms[0]['id']}", [ + 'json' => [ + 'keyValuePairs' => [ + 'AllowEdit' => true, + ] + ] + ]); + $uploadedFileResponse = $this->http->request('POST', "api/v3/forms/{$this->testForms[0]['id']}/submissions/files/{$this->testForms[0]['questions'][2]['id']}", [ From 49979c37e79de758d09d3759d06912a99cdcad7f Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Wed, 22 Jan 2025 11:57:11 +0100 Subject: [PATCH 41/44] testUpdateSubmission: use different http client for user1 Signed-off-by: Timotheus Pokorra --- tests/Integration/Api/ApiV3Test.php | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/tests/Integration/Api/ApiV3Test.php b/tests/Integration/Api/ApiV3Test.php index 9564e2f9a..8c101bcad 100644 --- a/tests/Integration/Api/ApiV3Test.php +++ b/tests/Integration/Api/ApiV3Test.php @@ -19,6 +19,8 @@ class ApiV3Test extends IntegrationBase { /** @var GuzzleHttp\Client */ private $http; + /** @var GuzzleHttp\Client */ + private $httpUser1; protected array $users = [ 'test' => 'Test user', @@ -255,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', 'test'], + 'headers' => [ + 'OCS-ApiRequest' => 'true', + 'Accept' => 'application/json' + ], + ]); } public function tearDown(): void { @@ -1365,7 +1377,7 @@ public function testUpdateSubmission() { ] ]); - $uploadedFileResponse = $this->http->request('POST', + $uploadedFileResponse = $this->httpUser1->request('POST', "api/v3/forms/{$this->testForms[0]['id']}/submissions/files/{$this->testForms[0]['questions'][2]['id']}", [ 'multipart' => [ @@ -1380,7 +1392,7 @@ public function testUpdateSubmission() { $data = $this->OcsResponse2Data($uploadedFileResponse); $uploadedFileId = $data[0]['uploadedFileId']; - $resp = $this->http->request('PUT', "api/v3/forms/{$this->testForms[0]['id']}/submissions/{$this->testForms[0]['submissions'][0]['id']}", [ + $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'] => ['ShortAnswer!2'], From 0f98d4bdadc3d5c26b230f119df98e3be0c34712 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Wed, 22 Jan 2025 12:02:47 +0100 Subject: [PATCH 42/44] fix password for user1 Signed-off-by: Timotheus Pokorra --- tests/Integration/Api/ApiV3Test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Integration/Api/ApiV3Test.php b/tests/Integration/Api/ApiV3Test.php index 8c101bcad..bd8509c91 100644 --- a/tests/Integration/Api/ApiV3Test.php +++ b/tests/Integration/Api/ApiV3Test.php @@ -261,7 +261,7 @@ public function setUp(): void { // Set up http Client for user user1 $this->httpUser1 = new Client([ 'base_uri' => 'http://localhost:8080/ocs/v2.php/apps/forms/', - 'auth' => ['user1', 'test'], + 'auth' => ['user1', 'user1'], 'headers' => [ 'OCS-ApiRequest' => 'true', 'Accept' => 'application/json' From 3596f8f801ddac1fa56c9a3bd17857ce0da73202 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Wed, 22 Jan 2025 12:07:57 +0100 Subject: [PATCH 43/44] fix testUpdateSubmission Signed-off-by: Timotheus Pokorra --- tests/Integration/Api/ApiV3Test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Integration/Api/ApiV3Test.php b/tests/Integration/Api/ApiV3Test.php index bd8509c91..ad3b3c241 100644 --- a/tests/Integration/Api/ApiV3Test.php +++ b/tests/Integration/Api/ApiV3Test.php @@ -1395,7 +1395,7 @@ public function testUpdateSubmission() { $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'] => ['ShortAnswer!2'], + $this->testForms[0]['questions'][0]['id'] => ['ShortAnswer2!'], $this->testForms[0]['questions'][1]['id'] => [ $this->testForms[0]['questions'][1]['options'][1]['id'] ], From f9913cf1d8ea0151a8fd687c8d5a551e961b6be8 Mon Sep 17 00:00:00 2001 From: Timotheus Pokorra Date: Wed, 22 Jan 2025 12:17:20 +0100 Subject: [PATCH 44/44] fix testUpdateSubmission Signed-off-by: Timotheus Pokorra --- tests/Integration/Api/ApiV3Test.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Integration/Api/ApiV3Test.php b/tests/Integration/Api/ApiV3Test.php index ad3b3c241..f705e4e7c 100644 --- a/tests/Integration/Api/ApiV3Test.php +++ b/tests/Integration/Api/ApiV3Test.php @@ -1429,8 +1429,8 @@ public function testUpdateSubmission() { unset($data['submissions'][0]['timestamp']); $this->assertEquals([ - 'userId' => 'test', - 'userDisplayName' => 'Test Displayname', + 'userId' => 'user1', + 'userDisplayName' => 'User No. 1', 'formId' => $this->testForms[0]['id'], 'answers' => [ [