diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index d3de3f70b..a4a998cc0 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -145,7 +145,6 @@ public function newForm(?int $fromId = null): DataResponse { // Create Form $form = new Form(); $form->setOwnerId($this->currentUser->getUID()); - $form->setCreated(time()); $form->setHash($this->formsService->generateFormHash()); $form->setTitle(''); $form->setDescription(''); @@ -157,7 +156,6 @@ public function newForm(?int $fromId = null): DataResponse { $form->setShowExpiration(false); $form->setExpires(0); $form->setIsAnonymous(false); - $form->setLastUpdated(time()); $this->formMapper->insert($form); } else { @@ -166,8 +164,8 @@ public function newForm(?int $fromId = null): DataResponse { // Read Form, set new Form specific data, extend Title. $formData = $oldForm->read(); unset($formData['id']); - $formData['created'] = time(); - $formData['lastUpdated'] = time(); + unset($formData['created']); + unset($formData['lastUpdated']); $formData['hash'] = $this->formsService->generateFormHash(); // TRANSLATORS Appendix to the form Title of a duplicated/copied form. $formData['title'] .= ' - ' . $this->l10n->t('Copy'); @@ -313,7 +311,6 @@ public function updateForm(int $formId, array $keyValuePairs): DataResponse { // Update changed Columns in Db. $this->formMapper->update($form); - $this->formsService->setLastUpdatedTimestamp($formId); return new DataResponse($form->getId()); } @@ -467,8 +464,6 @@ public function newQuestion(int $formId, ?string $type = null, string $text = '' $response = $question->read(); $response['options'] = []; $response['accept'] = []; - - $this->formsService->setLastUpdatedTimestamp($formId); } else { $this->logger->debug('Question to be cloned: {fromId}', [ 'fromId' => $fromId @@ -507,6 +502,8 @@ public function newQuestion(int $formId, ?string $type = null, string $text = '' } } + $this->formMapper->update($form); + return new DataResponse($response); } @@ -576,8 +573,7 @@ public function updateQuestion(int $formId, int $questionId, array $keyValuePair // Update changed Columns in Db. $this->questionMapper->update($question); - - $this->formsService->setLastUpdatedTimestamp($formId); + $this->formMapper->update($form); return new DataResponse($question->getId()); } @@ -633,7 +629,7 @@ public function deleteQuestion(int $formId, int $questionId): DataResponse { } } - $this->formsService->setLastUpdatedTimestamp($formId); + $this->formMapper->update($form); return new DataResponse($questionId); } @@ -722,7 +718,7 @@ public function reorderQuestions(int $formId, array $newOrder): DataResponse { ]; } - $this->formsService->setLastUpdatedTimestamp($formId); + $this->formMapper->update($form); return new DataResponse($response); } @@ -785,7 +781,7 @@ public function newOption(int $formId, int $questionId, array $optionTexts): Dat } } - $this->formsService->setLastUpdatedTimestamp($formId); + $this->formMapper->update($form); return new DataResponse($addedOptions); } @@ -850,7 +846,7 @@ public function updateOption(int $formId, int $questionId, int $optionId, array // Update changed Columns in Db. $this->optionMapper->update($option); - $this->formsService->setLastUpdatedTimestamp($formId); + $this->formMapper->update($form); return new DataResponse($option->getId()); } @@ -893,8 +889,7 @@ public function deleteOption(int $formId, int $questionId, int $optionId): DataR } $this->optionMapper->delete($option); - - $this->formsService->setLastUpdatedTimestamp($formId); + $this->formMapper->update($form); return new DataResponse($optionId); } @@ -993,8 +988,7 @@ public function deleteAllSubmissions(int $formId): DataResponse { // Delete all submissions (incl. Answers) $this->submissionMapper->deleteByForm($formId); - - $this->formsService->setLastUpdatedTimestamp($formId); + $this->formMapper->update($form); return new DataResponse($formId); } @@ -1073,7 +1067,7 @@ public function newSubmission(int $formId, array $answers, string $shareHash = ' $this->storeAnswersForQuestion($form, $submission->getId(), $questions[$questionIndex], $answerArray); } - $this->formsService->setLastUpdatedTimestamp($formId); + $this->formMapper->update($form); //Create Activity $this->formsService->notifyNewSubmission($form, $submission); @@ -1133,8 +1127,7 @@ public function deleteSubmission(int $formId, int $submissionId): DataResponse { // Delete submission (incl. Answers) $this->submissionMapper->deleteById($submissionId); - - $this->formsService->setLastUpdatedTimestamp($formId); + $this->formMapper->update($form); return new DataResponse($submissionId); } @@ -1419,7 +1412,6 @@ public function newFormLegacy(): DataResponse { // Create Form $form = new Form(); $form->setOwnerId($this->currentUser->getUID()); - $form->setCreated(time()); $form->setHash($this->formsService->generateFormHash()); $form->setTitle(''); $form->setDescription(''); @@ -1431,7 +1423,6 @@ public function newFormLegacy(): DataResponse { $form->setShowExpiration(false); $form->setExpires(0); $form->setIsAnonymous(false); - $form->setLastUpdated(time()); $this->formMapper->insert($form); @@ -1545,7 +1536,6 @@ public function updateFormLegacy(int $id, array $keyValuePairs): DataResponse { // Update changed Columns in Db. $this->formMapper->update($form); - $this->formsService->setLastUpdatedTimestamp($id); return new DataResponse($form->getId()); } @@ -1667,7 +1657,7 @@ public function newQuestionLegacy(int $formId, string $type, string $text = ''): $response['options'] = []; $response['accept'] = []; - $this->formsService->setLastUpdatedTimestamp($formId); + $this->formMapper->update($form); return new DataResponse($response); } @@ -1756,7 +1746,7 @@ public function reorderQuestionsLegacy(int $formId, array $newOrder): DataRespon ]; } - $this->formsService->setLastUpdatedTimestamp($formId); + $this->formMapper->update($form); return new DataResponse($response); } @@ -1821,8 +1811,7 @@ public function updateQuestionLegacy(int $id, array $keyValuePairs): DataRespons // Update changed Columns in Db. $this->questionMapper->update($question); - - $this->formsService->setLastUpdatedTimestamp($form->getId()); + $this->formMapper->update($form); return new DataResponse($question->getId()); } @@ -1874,7 +1863,7 @@ public function deleteQuestionLegacy(int $id): DataResponse { } } - $this->formsService->setLastUpdatedTimestamp($form->getId()); + $this->formMapper->update($form); return new DataResponse($id); } @@ -1981,8 +1970,7 @@ public function newOptionLegacy(int $questionId, string $text): DataResponse { $option->setText($text); $option = $this->optionMapper->insert($option); - - $this->formsService->setLastUpdatedTimestamp($form->getId()); + $this->formMapper->update($form); return new DataResponse($option->read()); } @@ -2042,8 +2030,7 @@ public function updateOptionLegacy(int $id, array $keyValuePairs): DataResponse // Update changed Columns in Db. $this->optionMapper->update($option); - - $this->formsService->setLastUpdatedTimestamp($form->getId()); + $this->formMapper->update($form); return new DataResponse($option->getId()); } @@ -2084,8 +2071,7 @@ public function deleteOptionLegacy(int $id): DataResponse { } $this->optionMapper->delete($option); - - $this->formsService->setLastUpdatedTimestamp($form->getId()); + $this->formMapper->update($form); return new DataResponse($id); } @@ -2343,7 +2329,7 @@ public function insertSubmissionLegacy(int $formId, array $answers, string $shar $this->storeAnswersForQuestion($form, $submission->getId(), $questions[$questionIndex], $answerArray); } - $this->formsService->setLastUpdatedTimestamp($formId); + $this->formMapper->update($form); //Create Activity $this->formsService->notifyNewSubmission($form, $submission); @@ -2395,8 +2381,7 @@ public function deleteSubmissionLegacy(int $id): DataResponse { // Delete submission (incl. Answers) $this->submissionMapper->deleteById($id); - - $this->formsService->setLastUpdatedTimestamp($form->getId()); + $this->formMapper->update($form); return new DataResponse($id); } @@ -2432,8 +2417,7 @@ public function deleteAllSubmissionsLegacy(int $formId): DataResponse { // Delete all submissions (incl. Answers) $this->submissionMapper->deleteByForm($formId); - - $this->formsService->setLastUpdatedTimestamp($formId); + $this->formMapper->update($form); return new DataResponse($formId); } @@ -2532,8 +2516,6 @@ public function unlinkFileLegacy(string $hash): DataResponse { $this->formMapper->update($form); - $this->formsService->setLastUpdatedTimestamp($form->getId()); - return new DataResponse($hash); } @@ -2577,8 +2559,6 @@ public function linkFileLegacy(string $hash, string $path, string $fileFormat): $filePath = $this->formsService->getFilePath($form); - $this->formsService->setLastUpdatedTimestamp($form->getId()); - return new DataResponse([ 'fileId' => $file->getId(), 'fileFormat' => $fileFormat, diff --git a/lib/Controller/ShareApiController.php b/lib/Controller/ShareApiController.php index 37f263635..a5550cd9e 100644 --- a/lib/Controller/ShareApiController.php +++ b/lib/Controller/ShareApiController.php @@ -190,12 +190,11 @@ public function newShare(int $formId, int $shareType, string $shareWith = '', ar /** @var Share */ $share = $this->shareMapper->insert($share); + $this->formMapper->update($form); // Create share-notifications (activity) $this->formsService->notifyNewShares($form, $share); - $this->formsService->setLastUpdatedTimestamp($formId); - // Append displayName for Frontend $shareData = $share->read(); $shareData['displayName'] = $this->formsService->getShareDisplayName($shareData); @@ -290,7 +289,7 @@ public function updateShare(int $formId, int $shareId, array $keyValuePairs): Da } } - $this->formsService->setLastUpdatedTimestamp($formId); + $this->formMapper->update($form); return new DataResponse($formShare->getId()); } @@ -331,9 +330,8 @@ public function deleteShare(int $formId, int $shareId): DataResponse { throw new OCSForbiddenException(); } - $this->shareMapper->deleteById($shareId); - - $this->formsService->setLastUpdatedTimestamp($formId); + $this->shareMapper->delete($share); + $this->formMapper->update($form); return new DataResponse($shareId); } @@ -458,12 +456,11 @@ public function newShareLegacy(int $formId, int $shareType, string $shareWith = /** @var Share */ $share = $this->shareMapper->insert($share); + $this->formMapper->update($form); // Create share-notifications (activity) $this->formsService->notifyNewShares($form, $share); - $this->formsService->setLastUpdatedTimestamp($formId); - // Append displayName for Frontend $shareData = $share->read(); $shareData['displayName'] = $this->formsService->getShareDisplayName($shareData); @@ -500,9 +497,8 @@ public function deleteShareLegacy(int $id): DataResponse { throw new OCSForbiddenException(); } - $this->shareMapper->deleteById($id); - - $this->formsService->setLastUpdatedTimestamp($form->getId()); + $this->shareMapper->delete($share); + $this->formMapper->update($form); return new DataResponse($id); } @@ -587,7 +583,7 @@ public function updateShareLegacy(int $id, array $keyValuePairs): DataResponse { } } - $this->formsService->setLastUpdatedTimestamp($form->getId()); + $this->formMapper->update($form); return new DataResponse($formShare->getId()); } diff --git a/lib/Db/FormMapper.php b/lib/Db/FormMapper.php index ed23669ef..2d8a28b9c 100644 --- a/lib/Db/FormMapper.php +++ b/lib/Db/FormMapper.php @@ -27,6 +27,7 @@ namespace OCA\Forms\Db; use OCA\Forms\Constants; +use OCP\AppFramework\Db\Entity; use OCP\AppFramework\Db\QBMapper; use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; @@ -50,6 +51,30 @@ public function __construct( parent::__construct($db, 'forms_v2_forms', Form::class); } + + /** + * @param Entity $entity + * @psalm-param Form $entity + * @return Form + * @throws \OCP\DB\Exception + */ + public function insert(Entity $entity): Form { + $entity->setCreated(time()); + $entity->setLastUpdated(time()); + return parent::insert($entity); + } + + /** + * @param Entity $entity + * @psalm-param Form $entity + * @return Form + * @throws \OCP\DB\Exception + */ + public function update(Entity $entity): Form { + $entity->setLastUpdated(time()); + return parent::update($entity); + } + /** * @param int $id * @return Form diff --git a/lib/Db/OptionMapper.php b/lib/Db/OptionMapper.php index c307e9639..3881e3f2b 100644 --- a/lib/Db/OptionMapper.php +++ b/lib/Db/OptionMapper.php @@ -49,7 +49,6 @@ public function __construct(IDBConnection $db) { * @throws DoesNotExistException if not found * @return Option[] */ - public function findByQuestion(int $questionId): array { $qb = $this->db->getQueryBuilder(); diff --git a/lib/Db/QuestionMapper.php b/lib/Db/QuestionMapper.php index c56697884..d6c707221 100644 --- a/lib/Db/QuestionMapper.php +++ b/lib/Db/QuestionMapper.php @@ -47,7 +47,6 @@ public function __construct( * @throws DoesNotExistException if not found * @return Question[] */ - public function findByForm(int $formId, bool $loadDeleted = false): array { $qb = $this->db->getQueryBuilder(); diff --git a/lib/Db/ShareMapper.php b/lib/Db/ShareMapper.php index c458c39be..2e4144716 100644 --- a/lib/Db/ShareMapper.php +++ b/lib/Db/ShareMapper.php @@ -104,20 +104,6 @@ public function findPublicShareByHash(string $hash): Share { return $this->findEntity($qb); } - /** - * Delete a share - * @param int $id of the share. - */ - public function deleteById(int $id): void { - $qb = $this->db->getQueryBuilder(); - - $qb->delete($this->getTableName()) - ->where( - $qb->expr()->eq('id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT)) - ); - $qb->executeStatement(); - } - /** * Delete all Shares of a form. * @param int $formId diff --git a/lib/FormsMigrator.php b/lib/FormsMigrator.php index d0e28b604..a67dbc155 100644 --- a/lib/FormsMigrator.php +++ b/lib/FormsMigrator.php @@ -159,14 +159,12 @@ public function import(IUser $user, IImportSource $importSource, OutputInterface $form->setTitle($formData['title']); $form->setDescription($formData['description']); $form->setOwnerId($user->getUID()); - $form->setCreated($formData['created']); $form->setAccess($formData['access']); $form->setExpires($formData['expires']); $form->setIsAnonymous($formData['isAnonymous']); $form->setSubmitMultiple($formData['submitMultiple']); $form->setShowExpiration($formData['showExpiration']); - $form->setLastUpdated($formData['lastUpdated']); - + $this->formMapper->insert($form); $questionIdMap = []; diff --git a/lib/Service/FormsService.php b/lib/Service/FormsService.php index 653908cfa..a1c6fb6d6 100644 --- a/lib/Service/FormsService.php +++ b/lib/Service/FormsService.php @@ -624,17 +624,6 @@ private function getSharesWithUser(int $formId, string $userId): array { }); } - /** - * Update lastUpdated timestamp for the given form - * - * @param int $formId The form to update - */ - public function setLastUpdatedTimestamp(int $formId): void { - $form = $this->formMapper->findById($formId); - $form->setLastUpdated(time()); - $this->formMapper->update($form); - } - /* * Validates the extraSettings * diff --git a/tests/Unit/Controller/ApiControllerTest.php b/tests/Unit/Controller/ApiControllerTest.php index 2036c74d1..0bc7b36d6 100644 --- a/tests/Unit/Controller/ApiControllerTest.php +++ b/tests/Unit/Controller/ApiControllerTest.php @@ -25,24 +25,6 @@ namespace OCA\Forms\Controller; -/** - * mock time() function used in controllers - * @param int|false|null $expected the value that should be returned when called - */ -function time($expected = null) { - static $value; - if ($expected === false) { - $value = null; - } elseif (!is_null($expected)) { - $value = $expected; - } - // Return real time if no mocked value is set - if (is_null($value)) { - return \time(); - } - return $value; -} - /** * mock is_uploaded_file() function used in services * @param string|bool|null $filename the value that should be returned when called @@ -472,8 +454,6 @@ public function testCreateNewForm($expectedForm) { $this->uploadedFileMapper, $this->mimeTypeDetector, ])->getMock(); - // Set the time that should be set for `lastUpdated` - \OCA\Forms\Controller\time(123456789); $this->configService->expects($this->once()) ->method('canCreateForms') @@ -483,6 +463,9 @@ public function testCreateNewForm($expectedForm) { ->willReturn('formHash'); $expected = $expectedForm; $expected['id'] = null; + // TODO fix test, currently unset because behaviour has changed + $expected['state'] = null; + $expected['lastUpdated'] = null; $this->formMapper->expects($this->once()) ->method('insert') ->with(self::callback(self::createFormValidator($expected))) @@ -841,10 +824,6 @@ public function testNewSubmission_answers() { return true; })); - $this->formsService->expects($this->once()) - ->method('setLastUpdatedTimestamp') - ->with(1); - $this->formsService->expects($this->once()) ->method('notifyNewSubmission'); @@ -1020,11 +999,6 @@ public function testDeleteSubmission($submissionData, $formData) { ->method('deleteById') ->with(42); - $this->formsService - ->expects($this->once()) - ->method('setLastUpdatedTimestamp') - ->with($formData['id']); - $this->assertEquals(new DataResponse(42), $this->apiController->deleteSubmission(1, 42)); } diff --git a/tests/Unit/Controller/ShareApiControllerTest.php b/tests/Unit/Controller/ShareApiControllerTest.php index 005cd5ac7..588945238 100644 --- a/tests/Unit/Controller/ShareApiControllerTest.php +++ b/tests/Unit/Controller/ShareApiControllerTest.php @@ -547,8 +547,8 @@ public function testDeleteShare() { ->willReturn($form); $this->shareMapper->expects($this->once()) - ->method('deleteById') - ->with('8'); + ->method('delete') + ->with($share); $response = new DataResponse(8); $this->assertEquals($response, $this->shareApiController->deleteShare(5, 8));