Skip to content

Optimization method FormsService::canSubmit - #2225

Merged
Chartman123 merged 1 commit into
nextcloud:mainfrom
AIlkiv:optimization-formsService-canSubmit
Jul 10, 2024
Merged

Optimization method FormsService::canSubmit#2225
Chartman123 merged 1 commit into
nextcloud:mainfrom
AIlkiv:optimization-formsService-canSubmit

Conversation

@AIlkiv

@AIlkiv AIlkiv commented Jun 27, 2024

Copy link
Copy Markdown
Collaborator

Currently, to check if a user has filled out the form, all entities are retrieved, and then two loops are performed at the PHP. My version does the same thing but with a single simple query.

@codecov

codecov Bot commented Jun 27, 2024

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 52.63158% with 9 lines in your changes missing coverage. Please review.

Project coverage is 46.03%. Comparing base (f6c1b8a) to head (a0b26df).
Report is 394 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2225      +/-   ##
============================================
+ Coverage     45.95%   46.03%   +0.08%     
- Complexity      797      800       +3     
============================================
  Files            65       66       +1     
  Lines          3066     3065       -1     
============================================
+ Hits           1409     1411       +2     
+ Misses         1657     1654       -3     

@Chartman123 Chartman123 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one :) Just a little comment about the new name for the function :)

@susnux what do you think?

And it would be nice if you could add some more tests to satisfy the Codecov checks 馃槈

Comment thread lib/Db/SubmissionMapper.php Outdated
@Chartman123 Chartman123 added 3. to review Waiting for reviews performances Performances issues and optimisations labels Jun 27, 2024
@Chartman123 Chartman123 added this to the 4.3 milestone Jun 27, 2024
@AIlkiv
AIlkiv force-pushed the optimization-formsService-canSubmit branch 2 times, most recently from 9c44b3c to b3d13cc Compare June 27, 2024 17:22
@AIlkiv
AIlkiv requested a review from Chartman123 June 28, 2024 09:09
@Chartman123
Chartman123 requested a review from susnux July 2, 2024 12:31

@susnux susnux left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally nice! Some notes :)

(and we should add a composite index on "formId userId"

Comment thread lib/Db/SubmissionMapper.php Outdated
Comment thread lib/Db/SubmissionMapper.php Outdated
Comment thread lib/Db/SubmissionMapper.php
@AIlkiv
AIlkiv force-pushed the optimization-formsService-canSubmit branch 3 times, most recently from 7d667ff to 548075a Compare July 9, 2024 08:39
Comment thread lib/Db/SubmissionMapper.php
@AIlkiv
AIlkiv force-pushed the optimization-formsService-canSubmit branch from 548075a to d252a41 Compare July 9, 2024 08:53
@AIlkiv
AIlkiv requested review from Chartman123 and susnux July 9, 2024 13:43
@Chartman123

Copy link
Copy Markdown
Collaborator

@AIlkiv could you please rebase on current main?

@Chartman123 Chartman123 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some small comments, but rest seems ok to me :)

Comment thread lib/Controller/ApiController.php Outdated
Comment thread lib/Db/SubmissionMapper.php
Comment thread lib/Migration/Version040300Date20240708163401.php Outdated
Comment thread lib/Service/FormsService.php
Comment thread lib/Service/FormsService.php Outdated
@AIlkiv
AIlkiv force-pushed the optimization-formsService-canSubmit branch 2 times, most recently from 2421d7c to 0aa2748 Compare July 10, 2024 05:22
@AIlkiv
AIlkiv requested a review from Chartman123 July 10, 2024 05:27
@AIlkiv
AIlkiv force-pushed the optimization-formsService-canSubmit branch from 0aa2748 to c1e789a Compare July 10, 2024 06:36
Signed-off-by: ailkiv <a.ilkiv.ye@gmail.com>
@AIlkiv
AIlkiv force-pushed the optimization-formsService-canSubmit branch from c1e789a to a0b26df Compare July 10, 2024 06:52
@Chartman123
Chartman123 merged commit 96e6790 into nextcloud:main Jul 10, 2024
@github-actions

Copy link
Copy Markdown
Contributor

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews feedback-requested performances Performances issues and optimisations

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants