Skip to content

Commit 9c44b3c

Browse files
AIlkivAIlkiv
committed
Optimization method FormsService::canSubmit
Signed-off-by: ailkiv <a.ilkiv.ye@gmail.com>
1 parent 8c4a6b2 commit 9c44b3c

5 files changed

Lines changed: 41 additions & 73 deletions

File tree

‎lib/Controller/ApiController.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1161,7 +1161,9 @@ public function insertSubmission(int $formId, array $answers, string $shareHash
11611161

11621162
// Ensure the form is unique if needed.
11631163
// If we can not submit anymore then the submission must be unique
1164-
if (!$this->formsService->canSubmit($form) && !$this->submissionService->isUniqueSubmission($submission)) {
1164+
if (!$this->formsService->canSubmit($form)
1165+
&& $this->submissionMapper->hasFormSubmissionsByUser($form->getId(), $this->currentUser->getUID(), true)
1166+
) {
11651167
$this->submissionMapper->delete($submission);
11661168
throw new OCSForbiddenException('Already submitted');
11671169
}

‎lib/Db/SubmissionMapper.php‎

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -86,44 +86,48 @@ public function findById(int $id): Submission {
8686
}
8787

8888
/**
89-
* @param int $formId
90-
* @throws DoesNotExistException if not found
91-
* @return array
89+
* Checks if the specified user has submissions for the specified form
90+
* @param int $formId ID of the form
91+
* @param string $userId UID of the user
92+
* @param bool $checkMultipleFormSubmissions if true, we check if there is more than one submissions, if false, at least one submissions
93+
* @return bool
9294
*/
93-
public function findParticipantsByForm(int $formId): array {
95+
public function hasFormSubmissionsByUser(
96+
int $formId,
97+
string $userId,
98+
bool $checkMultipleFormSubmissions
99+
): bool {
100+
$requireCountFormSubmissions = $checkMultipleFormSubmissions ? 2 : 1;
101+
94102
$qb = $this->db->getQueryBuilder();
95103

96-
$qb->select('user_id')
104+
$query = $qb->select('id')
97105
->from($this->getTableName())
98106
->where(
99107
$qb->expr()->eq('form_id', $qb->createNamedParameter($formId, IQueryBuilder::PARAM_INT))
100-
);
101-
102-
$submissionEntities = $this->findEntities($qb);
103-
104-
// From array of submissionEntities produce array of userIds.
105-
$userIds = array_map(function ($submissionEntity) {
106-
return $submissionEntity->getUserId();
107-
}, $submissionEntities);
108+
)
109+
->andWhere(
110+
$qb->expr()->eq('user_id', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR))
111+
)
112+
->setMaxResults($requireCountFormSubmissions);
113+
$result = $query->executeQuery();
114+
$rows = $result->fetchAll();
115+
$result->closeCursor();
108116

109-
return $userIds;
117+
return count($rows) === $requireCountFormSubmissions;
110118
}
111119

112120
/**
113-
* Count submissions by form and optionally also by userId
114-
* @param int $formId ID of the form to count submissions for
115-
* @param string|null $userId optionally limit submissions to the one of that user
121+
* Count submissions by form
122+
* @param int $formId ID of the form to count submissions
116123
* @throws \Exception
117124
*/
118-
public function countSubmissions(int $formId, ?string $userId = null): int {
125+
public function countSubmissions(int $formId): int {
119126
$qb = $this->db->getQueryBuilder();
120127

121128
$query = $qb->select($qb->func()->count('*', 'num_submissions'))
122129
->from($this->getTableName())
123130
->where($qb->expr()->eq('form_id', $qb->createNamedParameter($formId, IQueryBuilder::PARAM_INT)));
124-
if (!is_null($userId)) {
125-
$query->andWhere($qb->expr()->eq('user_id', $qb->createNamedParameter($userId, IQueryBuilder::PARAM_STR)));
126-
}
127131

128132
$result = $query->executeQuery();
129133
$row = $result->fetch();

‎lib/Service/FormsService.php‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -333,15 +333,12 @@ public function canSubmit(Form $form): bool {
333333
if ($this->currentUser->getUID() === $form->getOwnerId()) {
334334
return true;
335335
}
336-
337336
// Refuse access, if SubmitMultiple is not set and user already has taken part.
338-
if (!$form->getSubmitMultiple()) {
339-
$participants = $this->submissionMapper->findParticipantsByForm($form->getId());
340-
foreach ($participants as $participant) {
341-
if ($participant === $this->currentUser->getUID()) {
342-
return false;
343-
}
344-
}
337+
if (
338+
!$form->getSubmitMultiple()
339+
&& $this->submissionMapper->hasFormSubmissionsByUser($form->getId(), $this->currentUser->getUID(), false)
340+
) {
341+
return false;
345342
}
346343

347344
return true;

‎tests/Unit/Service/FormsServiceTest.php‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -802,25 +802,25 @@ public function dataCanSubmit() {
802802
'allowFormOwner' => [
803803
'ownerId' => 'currentUser',
804804
'submitMultiple' => false,
805-
'participantsArray' => ['currentUser'],
805+
'hasFormSubmissionsByUser' => true,
806806
'expected' => true
807807
],
808808
'submitMultipleGood' => [
809809
'ownerId' => 'someUser',
810810
'submitMultiple' => false,
811-
'participantsArray' => ['notCurrentUser'],
811+
'hasFormSubmissionsByUser' => false,
812812
'expected' => true
813813
],
814814
'submitMultipleNotGood' => [
815815
'ownerId' => 'someUser',
816816
'submitMultiple' => false,
817-
'participantsArray' => ['notCurrentUser', 'currentUser'],
817+
'hasFormSubmissionsByUser' => true,
818818
'expected' => false
819819
],
820820
'submitMultiple' => [
821821
'ownerId' => 'someUser',
822822
'submitMultiple' => true,
823-
'participantsArray' => ['currentUser'],
823+
'hasFormSubmissionsByUser' => true,
824824
'expected' => true
825825
]
826826
];
@@ -830,10 +830,10 @@ public function dataCanSubmit() {
830830
*
831831
* @param string $ownerId
832832
* @param bool $submitMultiple
833-
* @param array $participantsArray
833+
* @param bool $hasFormSubmissionsByUser
834834
* @param bool $expected
835835
*/
836-
public function testCanSubmit(string $ownerId, bool $submitMultiple, array $participantsArray, bool $expected) {
836+
public function testCanSubmit(string $ownerId, bool $submitMultiple, bool $hasFormSubmissionsByUser, bool $expected) {
837837
$form = new Form();
838838
$form->setId(42);
839839
$form->setAccess([
@@ -844,9 +844,9 @@ public function testCanSubmit(string $ownerId, bool $submitMultiple, array $part
844844
$form->setSubmitMultiple($submitMultiple);
845845

846846
$this->submissionMapper->expects($this->any())
847-
->method('findParticipantsByForm')
847+
->method('hasFormSubmissionsByUser')
848848
->with(42)
849-
->willReturn($participantsArray);
849+
->willReturn($hasFormSubmissionsByUser);
850850

851851
$this->assertEquals($expected, $this->formsService->canSubmit($form));
852852
}

‎tests/Unit/Service/SubmissionServiceTest.php‎

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -153,41 +153,6 @@ public function setUp(): void {
153153
);
154154
}
155155

156-
/**
157-
* @dataProvider dataIsUniqueSubmission
158-
*/
159-
public function testIsUniqueSubmission(array $submissionData, int $numberOfSubmissions, bool $expected) {
160-
$this->submissionMapper->method('countSubmissions')
161-
->with($submissionData['formId'], $submissionData['userId'])
162-
->willReturn($numberOfSubmissions);
163-
164-
$submission = Submission::fromParams($submissionData);
165-
$this->assertEquals($expected, $this->submissionService->isUniqueSubmission($submission));
166-
}
167-
168-
public function dataIsUniqueSubmission() {
169-
return [
170-
[
171-
'submissionData' => [
172-
'id' => 1,
173-
'userId' => 'user',
174-
'formId' => 1,
175-
],
176-
'numberOfSubmissions' => 1,
177-
'expected' => true,
178-
],
179-
[
180-
'submissionData' => [
181-
'id' => 3,
182-
'userId' => 'user',
183-
'formId' => 1,
184-
],
185-
'numberOfSubmissions' => 2,
186-
'expected' => false,
187-
],
188-
];
189-
}
190-
191156
public function testGetSubmissions() {
192157
$submission_1 = new Submission();
193158
$submission_1->setId(42);

0 commit comments

Comments
 (0)