Skip to content

url method causing n+1 queries #3515

@evenreven

Description

@evenreven

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions