feat: add maximum submissions limit for forms#3199
feat: add maximum submissions limit for forms#3199LAfricain wants to merge 1 commit intonextcloud:mainfrom
Conversation
Chartman123
left a comment
Chartman123
left a comment
There was a problem hiding this comment.
@LAfricain thanks for your PR. I just had a quick look at your code and didn't test it yet in my instance. Already a few comments. :)
Chartman123
added
enhancement
Chartman123
left a comment
Chartman123
left a comment
There was a problem hiding this comment.
Some more comments :) Please have a look at the failing workflows, too.
|
Hello @Chartman123, |
|
@LAfricain sorry, I don't have much spare time at the moment :) Please fix the remaining workflow issues |
It's Ok, no rush. I didn't realize I had to fix the errors myself; I still need to get used to the tool. It's done now. |
|
@LAfricain yes, you have to fix them yourself (or use the npm scripts and php scripts). Environments like VSCode also can assist you with that :) |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
@LAfricain it's getting better :) only the DCO (sign-off your commits) and OpenAPI missing... Could you then please also squash all your commits into a single one? |
LAfricain
force-pushed
the
maxsubmissions
branch
from
4e655a8 to
0f256fc
Compare
|
Oh, please don't merge the main branch into this PR, always use |
LAfricain
force-pushed
the
maxsubmissions
branch
2 times, most recently
from
d1c6101 to
05bfd72
Compare
Chartman123
force-pushed
the
maxsubmissions
branch
from
05bfd72 to
06294e7
Compare
Chartman123
left a comment
Chartman123
left a comment
There was a problem hiding this comment.
a few more comments that came up during test
| // Check if max submissions limit is reached | ||
| $maxSubmissions = $form->getMaxSubmissions(); | ||
| if ($maxSubmissions !== null && $this->submissionMapper->countSubmissions($form->getId()) >= $maxSubmissions) { | ||
| return false; | ||
| } | ||
|
|
There was a problem hiding this comment.
This check fails once the submission reaches the limit as this part of the code is called after inserting the submission.
@susnux do you think a single check for maxSubmissions is enough in ApiController or do we need the double check here as well? If so, we need to change the condition from >= to >
Add the ability to limit the number of responses a form can receive. When the limit is reached, the form is automatically closed and displays a dedicated message instead of accepting new submissions. - Add max_submissions column to forms_v2_forms table (migration) - Add maxSubmissions property to Form entity - Check submission limit in FormsService::canSubmit() - Add limit enforcement in ApiController::newSubmission() - Add isMaxSubmissionsReached flag in form API response - Update FormsForm psalm type in ResponseDefinitions - Add limit settings UI in SettingsSidebarTab - Display dedicated 'Form is full' message in Submit view - Update openapi.json - Update unit and integration tests Closes nextcloud#596 Signed-off-by: lafricain79 <lafricain79@gmail.com>
LAfricain
force-pushed
the
maxsubmissions
branch
from
043a49d to
4246abe
Compare
2 participants
Add the ability to limit the number of responses a form can receive. When the limit is reached, the form is automatically closed and displays a dedicated message instead of accepting new submissions.
Closes #596