refinerycms icon indicating copy to clipboard operation
refinerycms copied to clipboard

url method causing n+1 queries

Open evenreven opened this issue 2 years ago • 0 comments

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.

evenreven avatar May 24 '22 11:05 evenreven