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

Eager loading countries when creating a new zone #3649

Merged

Conversation

softr8
Copy link
Contributor

@softr8 softr8 commented Jun 1, 2020

Zones can be created at country or state level, and the
select box that renders the state, it needs the country name
as well, to build this, it calls the mthod state_with_country
and it causes a query n+1, eager loading their countries
improves dramatically the response time when rendering the
new form.

Before

Started GET "/admin/zones/new"
....
Rendered backend/app/views/spree/admin/zones/_state_members.html.erb (Duration: 10392.6ms | Allocations: 10016775)
...
Completed 200 OK in 11247ms (Views: 10958.5ms | ActiveRecord: 257.5ms | Allocations: 11049597)

After

Started GET "/admin/zones/new"
....
Rendered backend/app/views/spree/admin/zones/_state_members.html.erb (Duration: 237.7ms | Allocations: 179137)
...
Completed 200 OK in 1268ms (Views: 1034.5ms | ActiveRecord: 108.2ms | Allocations: 1278890)

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have updated Guides and README accordingly to this change (if needed)
  • I have added tests to cover this change (if needed)
  • I have attached screenshots to this PR for visual changes (if needed)

Zones can be created at country or state level, and the
select box that renders the state, it needs the country name
as well, to build this, it calls the mthod state_with_country
and it causes a query n+1, eager loading their countries
improves dramatically the response time when rendering the
new form
Copy link
Member

@jarednorman jarednorman left a comment

Choose a reason for hiding this comment

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

Looks good!

Copy link
Member

@spaghetticode spaghetticode left a comment

Choose a reason for hiding this comment

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

@softr8 thanks for the speed improvement 🏎️💨

@spaghetticode spaghetticode merged commit 42a096f into solidusio:master Jun 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants