Skip to content

Commit

Permalink
Refactor form creation and update logic
Browse files Browse the repository at this point in the history 
This commit refactors the form creation and update logic in the `ApiController` class. It removes the unnecessary setting of the `created` and `lastUpdated` timestamps in the `Form` entity, as these values are now automatically set in the `FormMapper` class. This improves code readability and reduces redundancy.

The changes also include updates to the `FormMapper` class, where the `insert` and `update` methods now automatically set the `created` and `lastUpdated` timestamps respectively.

Signed-off-by: Christian Hartmann <[email protected]>
  • Loading branch information
@Chartman123
Chartman123 committed Sep 26, 2024
1 parent b40ef76 commit 9ad8d3d
Show file tree
Hide file tree
Showing 10 changed files with 62 additions and 116 deletions.
66 changes: 23 additions & 43 deletions lib/Controller/ApiController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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('');
Expand All @@ -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 {
Expand All @@ -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');
Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -507,6 +502,8 @@ public function newQuestion(int $formId, ?string $type = null, string $text = ''
}
}

$this->formMapper->update($form);

Check warning on line 505 in lib/Controller/ApiController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/ApiController.php#L505 

Added line #L505 was not covered by tests

return new DataResponse($response);
}

Expand Down Expand Up @@ -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);

Check warning on line 576 in lib/Controller/ApiController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/ApiController.php#L576 

Added line #L576 was not covered by tests

return new DataResponse($question->getId());
}
Expand Down Expand Up @@ -633,7 +629,7 @@ public function deleteQuestion(int $formId, int $questionId): DataResponse {
}
}

$this->formsService->setLastUpdatedTimestamp($formId);
$this->formMapper->update($form);

Check warning on line 632 in lib/Controller/ApiController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/ApiController.php#L632 

Added line #L632 was not covered by tests

return new DataResponse($questionId);
}
Expand Down Expand Up @@ -722,7 +718,7 @@ public function reorderQuestions(int $formId, array $newOrder): DataResponse {
];
}

$this->formsService->setLastUpdatedTimestamp($formId);
$this->formMapper->update($form);

Check warning on line 721 in lib/Controller/ApiController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/ApiController.php#L721 

Added line #L721 was not covered by tests

return new DataResponse($response);
}
Expand Down Expand Up @@ -785,7 +781,7 @@ public function newOption(int $formId, int $questionId, array $optionTexts): Dat
}
}

$this->formsService->setLastUpdatedTimestamp($formId);
$this->formMapper->update($form);

Check warning on line 784 in lib/Controller/ApiController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/ApiController.php#L784 

Added line #L784 was not covered by tests

return new DataResponse($addedOptions);
}
Expand Down Expand Up @@ -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);

Check warning on line 849 in lib/Controller/ApiController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/ApiController.php#L849 

Added line #L849 was not covered by tests

return new DataResponse($option->getId());
}
Expand Down Expand Up @@ -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);

Check warning on line 892 in lib/Controller/ApiController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/ApiController.php#L892 

Added line #L892 was not covered by tests

return new DataResponse($optionId);
}
Expand Down Expand Up @@ -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);

Check warning on line 991 in lib/Controller/ApiController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/ApiController.php#L991 

Added line #L991 was not covered by tests

return new DataResponse($formId);
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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('');
Expand All @@ -1431,7 +1423,6 @@ public function newFormLegacy(): DataResponse {
$form->setShowExpiration(false);
$form->setExpires(0);
$form->setIsAnonymous(false);
$form->setLastUpdated(time());

$this->formMapper->insert($form);

Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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);

Check warning on line 1660 in lib/Controller/ApiController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/ApiController.php#L1660 

Added line #L1660 was not covered by tests

return new DataResponse($response);
}
Expand Down Expand Up @@ -1756,7 +1746,7 @@ public function reorderQuestionsLegacy(int $formId, array $newOrder): DataRespon
];
}

$this->formsService->setLastUpdatedTimestamp($formId);
$this->formMapper->update($form);

Check warning on line 1749 in lib/Controller/ApiController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/ApiController.php#L1749 

Added line #L1749 was not covered by tests

return new DataResponse($response);
}
Expand Down Expand Up @@ -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);

Check warning on line 1814 in lib/Controller/ApiController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/ApiController.php#L1814 

Added line #L1814 was not covered by tests

return new DataResponse($question->getId());
}
Expand Down Expand Up @@ -1874,7 +1863,7 @@ public function deleteQuestionLegacy(int $id): DataResponse {
}
}

$this->formsService->setLastUpdatedTimestamp($form->getId());
$this->formMapper->update($form);

Check warning on line 1866 in lib/Controller/ApiController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/ApiController.php#L1866 

Added line #L1866 was not covered by tests

return new DataResponse($id);
}
Expand Down Expand Up @@ -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);

Check warning on line 1973 in lib/Controller/ApiController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/ApiController.php#L1973 

Added line #L1973 was not covered by tests

return new DataResponse($option->read());
}
Expand Down Expand Up @@ -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);

Check warning on line 2033 in lib/Controller/ApiController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/ApiController.php#L2033 

Added line #L2033 was not covered by tests

return new DataResponse($option->getId());
}
Expand Down Expand Up @@ -2084,8 +2071,7 @@ public function deleteOptionLegacy(int $id): DataResponse {
}

$this->optionMapper->delete($option);

$this->formsService->setLastUpdatedTimestamp($form->getId());
$this->formMapper->update($form);

Check warning on line 2074 in lib/Controller/ApiController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/ApiController.php#L2074 

Added line #L2074 was not covered by tests

return new DataResponse($id);
}
Expand Down Expand Up @@ -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);

Check warning on line 2332 in lib/Controller/ApiController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/ApiController.php#L2332 

Added line #L2332 was not covered by tests

//Create Activity
$this->formsService->notifyNewSubmission($form, $submission);
Expand Down Expand Up @@ -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);

Check warning on line 2384 in lib/Controller/ApiController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/ApiController.php#L2384 

Added line #L2384 was not covered by tests

return new DataResponse($id);
}
Expand Down Expand Up @@ -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);

Check warning on line 2420 in lib/Controller/ApiController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/ApiController.php#L2420 

Added line #L2420 was not covered by tests

return new DataResponse($formId);
}
Expand Down Expand Up @@ -2532,8 +2516,6 @@ public function unlinkFileLegacy(string $hash): DataResponse {

$this->formMapper->update($form);

$this->formsService->setLastUpdatedTimestamp($form->getId());

return new DataResponse($hash);
}

Expand Down Expand Up @@ -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,
Expand Down
20 changes: 8 additions & 12 deletions lib/Controller/ShareApiController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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());
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -458,12 +456,11 @@ public function newShareLegacy(int $formId, int $shareType, string $shareWith =

/** @var Share */
$share = $this->shareMapper->insert($share);
$this->formMapper->update($form);

Check warning on line 459 in lib/Controller/ShareApiController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/ShareApiController.php#L459 

Added line #L459 was not covered by tests

// 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);
Expand Down Expand Up @@ -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);

Check warning on line 501 in lib/Controller/ShareApiController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/ShareApiController.php#L500-L501 

Added lines #L500 - L501 were not covered by tests

return new DataResponse($id);
}
Expand Down Expand Up @@ -587,7 +583,7 @@ public function updateShareLegacy(int $id, array $keyValuePairs): DataResponse {
}
}

$this->formsService->setLastUpdatedTimestamp($form->getId());
$this->formMapper->update($form);

Check warning on line 586 in lib/Controller/ShareApiController.php

View check run for this annotation

Codecov / codecov/patch

lib/Controller/ShareApiController.php#L586 

Added line #L586 was not covered by tests

return new DataResponse($formShare->getId());
}
Expand Down
25 changes: 25 additions & 0 deletions lib/Db/FormMapper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);

Check warning on line 64 in lib/Db/FormMapper.php

View check run for this annotation

Codecov / codecov/patch

lib/Db/FormMapper.php#L61-L64 

Added lines #L61 - L64 were not covered by tests
}

/**
* @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);

Check warning on line 75 in lib/Db/FormMapper.php

View check run for this annotation

Codecov / codecov/patch

lib/Db/FormMapper.php#L73-L75 

Added lines #L73 - L75 were not covered by tests
}

/**
* @param int $id
* @return Form
Expand Down
1 change: 0 additions & 1 deletion lib/Db/OptionMapper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
Loading

0 comments on commit 9ad8d3d

Please sign in to comment.