From 75cdc7460f789569d0997a9812ca1985b3676b86 Mon Sep 17 00:00:00 2001
From: Philip Arndt <parndt@gmail.com>
Date: Mon, 14 Mar 2011 16:57:23 +1300
Subject: [PATCH] Improved performance in menu rendering when there are very
 many pages by first querying the translations table and then using those
 results in the pages table rather than the other way around. There are still
 too many queries in the logfile for my liking so this has a way to come.

---
 core/app/views/shared/_menu.html.erb        |  3 +--
 core/app/views/shared/_menu_branch.html.erb |  6 +++++-
 core/lib/refinery/application_controller.rb |  2 +-
 pages/app/models/page.rb                    | 12 ++++++------
 4 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/core/app/views/shared/_menu.html.erb b/core/app/views/shared/_menu.html.erb
index 1dd3be81bc..cd96007f3e 100644
--- a/core/app/views/shared/_menu.html.erb
+++ b/core/app/views/shared/_menu.html.erb
@@ -11,7 +11,6 @@
       selected_item = collection.detect{|page| selected_page?(page)}
       selected_item = @page if selected_item.nil?
     end
-    sibling_count = roots.length - 1
 -%>
     <nav id='<%= dom_id %>' class='<%= %W(#{css} clearfix).join(' ') %>'>
       <ul>
@@ -19,7 +18,7 @@
                    :collection => roots,
                    :locals => {
                      :hide_children => hide_children,
-                     :sibling_count => sibling_count,
+                     :sibling_count => (roots.length - 1),
                      :collection => collection,
                      :selected_item => selected_item,
                      :apply_css => true #if you don't care about class='first' class='last' or class='selected' set apply_css to false for speed.
diff --git a/core/app/views/shared/_menu_branch.html.erb b/core/app/views/shared/_menu_branch.html.erb
index c01a235a49..379f5a0dd4 100644
--- a/core/app/views/shared/_menu_branch.html.erb
+++ b/core/app/views/shared/_menu_branch.html.erb
@@ -5,7 +5,11 @@
   dom_id = ("id='item_#{menu_branch_counter}'" if menu_branch.parent_id.nil? and menu_branch.title.present?)
 
   hide_children = (defined?(hide_children) && hide_children)
-  children = (hide_children || !menu_branch.has_descendants?) ? [] : collection.select { |p| p.parent_id == menu_branch.id }
+  children = if (hide_children || !menu_branch.has_descendants?)
+    []
+  else
+    collection.select { |p| p.parent_id == menu_branch.id }
+  end
 -%>
 <li<%= ['', css, dom_id].compact.join(' ').gsub(/\ *$/, '') %>>
   <%= link_to menu_branch.title, menu_branch.url -%>
diff --git a/core/lib/refinery/application_controller.rb b/core/lib/refinery/application_controller.rb
index 5cea32305d..737c4f6b43 100644
--- a/core/lib/refinery/application_controller.rb
+++ b/core/lib/refinery/application_controller.rb
@@ -87,7 +87,7 @@ def login?
 
       # get all the pages to be displayed in the site menu.
       def find_pages_for_menu
-        @menu_pages = Page.live.in_menu.order('lft ASC')
+        @menu_pages = Page.in_menu.live.order('lft ASC')
       end
 
       # use a different model for the meta information.
diff --git a/pages/app/models/page.rb b/pages/app/models/page.rb
index fef89ee35a..86b4b7e076 100644
--- a/pages/app/models/page.rb
+++ b/pages/app/models/page.rb
@@ -32,14 +32,14 @@ class Page < ActiveRecord::Base
 
   scope :live, where(:draft => false)
 
-  # shows all pages with :show_in_menu set to true, but it also
+  # Shows all pages with :show_in_menu set to true, but it also
   # rejects any page that has not been translated to the current locale.
+  # This works using a query against the translated content first and then
+  # using all of the page_ids we further filter against this model's table.
   scope :in_menu, lambda {
-    pages = Arel::Table.new(Page.table_name)
-    translations = Arel::Table.new(Page.translations_table_name)
-
-    includes(:translations).where(:show_in_menu => true).where(
-      translations[:locale].eq(Globalize.locale)).where(pages[:id].eq(translations[:page_id]))
+    includes(:translations).where(Page.arel_table[:id].in(
+      Page::Translation.where(:locale => Globalize.locale).map(&:page_id)
+    )).where(:show_in_menu => true)
   }
 
   # when a dialog pops up to link to a page, how many pages per page should there be