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

url method causing n+1 queries #3515

Open
evenreven opened this issue May 24, 2022 · 0 comments
Open

url method causing n+1 queries #3515

evenreven opened this issue May 24, 2022 · 0 comments

Comments

@evenreven
Copy link
Contributor

evenreven commented May 24, 2022

I have thousands of pages in my app, but my global menu is - by design - only showing around 40 pages (depth: 0 and depth: 1) in a simple nested structure. Up until now I've used a menu presenter that inherits from the Refinery menu presenter, but I've now tried to remove the presenter and do this instead (kinda messy, but works):

# app/controllers/application_controller.rb
class ApplicationController < ActionController::Base
  before_action :load_menu_pages
  protect_from_forgery with: :null_session

  def load_menu_pages
    menu_pages = Refinery::Page.includes(:parent, :translations).live.where(show_in_menu: true, depth: 0..1).order(:rgt)
    @top_menu_pages = menu_pages.where(depth: 0)
    @menu_pages = menu_pages
  end
end

Then call a simple partial from the application layout itself:

<%# app/views/application/_menu.html.erb %>
<% @top_menu_pages.each do |top| %>
  <li class="submenu"><%= link_to top.title, url_for(top) %>
    <ul>
      <% @menu_pages.each do |single| %>
        <li class="menu-item"><%= link_to single.title, url_for(single) if single.parent == top %></li>
      <% end %>
    </ul>
  </li>
<% end %>

My presenter could probably have been tuned, but it was using 2000 sql queries for each request to build the menu*, and for such a simple structure a presenter seemed like overkill anyway (also, I don't really like the presenter pattern). The above setup instead uses a modest 5 sql queries.

So far, so good. But url_for(page) creates links like /pages/studies and /pages/dance instead of nested slugs like /studies and /studies/dance, which is a problem. So I tried to use the url method on the iteration itself (i.e. single.url instead of url_for(single)), and it works, but it introduced an N+1 problem and I now have 41 queries (one for each menu_page).

Is it possible to avoid this while keeping pretty nested urls? Is there a better method to use or anything that the controller can eager load that alleviates this?

*obviously I use caching here, but invalidation is way more expensive than it has to be.

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

No branches or pull requests

1 participant