Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5894] Fix VolunteersController#edit N+1 issues #6068

Merged
merged 1 commit into from
Oct 13, 2024

Conversation

ElisaRmz
Copy link
Collaborator

@ElisaRmz ElisaRmz commented Oct 4, 2024

What github issue is this PR for, if any?

Resolves #5894

What changed, and why?

When editing a Volunteer, an N+1 query occurred.
After the changes, it doesn't happen an the code is faster.

How is this tested? (please write tests!) 💖💪

N+1 queries are tested with Bullet gem, a message is shown in the console

Add to your query

Screenshots please :)

Previous queries
Antes
Actual queries
Captura de pantalla 2024-10-04 a las 16 15 12

@github-actions github-actions bot added the ruby Pull requests that update Ruby code label Oct 4, 2024
@ElisaRmz ElisaRmz assigned ElisaRmz and unassigned ElisaRmz Oct 7, 2024
Copy link
Collaborator

@elasticspoon elasticspoon left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@@ -5,14 +5,14 @@ def grouped_options_for_assigning_case(volunteer)
[
[
"Not Assigned",
CasaCase
CasaCase.eager_load([:assigned_volunteers])
Copy link
Collaborator

Choose a reason for hiding this comment

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

did you choose eager_load over includes on purpose? includes will choose to use eager_load or preload. It doesn't matter that much but just curious

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In this case, I followed the recommendation of Bullet gem.
Firstly, include was suggested, but when added a new recommendation appeared with eager_load. (I didn't take a new screenshot)

@ElisaRmz ElisaRmz marked this pull request as ready for review October 8, 2024 11:20
@ElisaRmz
Copy link
Collaborator Author

ElisaRmz commented Oct 9, 2024

@elasticspoon, I have a question: Can I merge it once it's approved, or is this only done by project owners?

@elasticspoon
Copy link
Collaborator

@elasticspoon, I have a question: Can I merge it once it's approved, or is this only done by project owners?

Feel free to merge yourself. Or you can wait for me to do it.

@elasticspoon elasticspoon merged commit 4f68eb6 into main Oct 13, 2024
18 checks passed
@elasticspoon elasticspoon deleted the n+1_volunteers_controller_edit branch October 13, 2024 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ruby Pull requests that update Ruby code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fix VolunteersController#edit N+1 issues
2 participants