From: Tom Hughes <tom@compton.nu>
Date: Sat, 23 Mar 2013 17:52:56 +0000 (+0000)
Subject: Optimise database reads in DiaryEntry#list
X-Git-Tag: live~5697
X-Git-Url: https://git.openstreetmap.org./rails.git/commitdiff_plain/369d5a0bfd1230f53a656264d5a44c56413483ed

Optimise database reads in DiaryEntry#list
---

diff --git a/app/controllers/diary_entry_controller.rb b/app/controllers/diary_entry_controller.rb
index 777034468..6f1f7240c 100644
--- a/app/controllers/diary_entry_controller.rb
+++ b/app/controllers/diary_entry_controller.rb
@@ -80,36 +80,15 @@ class DiaryEntryController < ApplicationController
 
       if @this_user
         @title = t 'diary_entry.list.user_title', :user => @this_user.display_name
-        @entry_pages, @entries = paginate(:diary_entries,
-                                          :conditions => {
-                                            :user_id => @this_user.id,
-                                            :visible => true
-                                          },
-                                          :order => 'created_at DESC',
-                                          :per_page => 20)
+        @entries = @this_user.diary_entries
       else
         render_unknown_user params[:display_name]
+        return
       end
-    elsif params[:language]
-      @title = t 'diary_entry.list.in_language_title', :language => Language.find(params[:language]).english_name
-      @entry_pages, @entries = paginate(:diary_entries, :include => :user,
-                                        :conditions => {
-                                          :users => { :status => ["active", "confirmed"] },
-                                          :visible => true,
-                                          :language_code => params[:language]
-                                        },
-                                        :order => 'created_at DESC',
-                                        :per_page => 20)
     elsif params[:friends]
       if @user
         @title = t 'diary_entry.list.title_friends'
-        @entry_pages, @entries = paginate(:diary_entries, :include => :user,
-                                          :conditions => {
-                                            :user_id => @user.friend_users,
-                                            :visible => true
-                                          },
-                                          :order => 'created_at DESC',
-                                          :per_page => 20)
+        @entries = DiaryEntry.where(:user_id => @user.friend_users)
       else
           require_user
           return
@@ -117,27 +96,30 @@ class DiaryEntryController < ApplicationController
     elsif params[:nearby]
       if @user
         @title = t 'diary_entry.list.title_nearby'
-        @entry_pages, @entries = paginate(:diary_entries, :include => :user,
-                                          :conditions => {
-                                            :user_id => @user.nearby,
-                                            :visible => true
-                                          },
-                                          :order => 'created_at DESC',
-                                          :per_page => 20)
+        @entries = DiaryEntry.where(:user_id => @user.nearby)
       else
           require_user
           return
       end
     else
-      @title = t 'diary_entry.list.title'
-      @entry_pages, @entries = paginate(:diary_entries, :include => :user,
-                                        :conditions => {
-                                          :users => { :status => ["active", "confirmed"] },
-                                          :visible => true
-                                        },
-                                        :order => 'created_at DESC',
-                                        :per_page => 20)
+      @entries = DiaryEntry.joins(:user).where(:users => { :status => ["active", "confirmed"] })
+      
+      if params[:language]
+        @title = t 'diary_entry.list.in_language_title', :language => Language.find(params[:language]).english_name
+        @entries = @entries.where(:language_code => params[:language])
+      else
+        @title = t 'diary_entry.list.title'
+      end
     end
+
+    @page = (params[:page] || 1).to_i
+    @page_size = 20
+
+    @entries = @entries.visible
+    @entries = @entries.order("created_at DESC")
+    @entries = @entries.offset((@page - 1) * @page_size)
+    @entries = @entries.limit(@page_size)
+    @entries = @entries.includes(:user, :language)
   end
 
   def rss
@@ -151,6 +133,7 @@ class DiaryEntryController < ApplicationController
         @link = "http://#{SERVER_URL}/user/#{user.display_name}/diary"
       else
         render :nothing => true, :status => :not_found
+        return
       end
     else
       @entries = DiaryEntry.joins(:user).where(:users => { :status => ["active", "confirmed"] })
diff --git a/app/views/diary_entry/list.html.erb b/app/views/diary_entry/list.html.erb
index 3353fef79..09d8af2fd 100644
--- a/app/views/diary_entry/list.html.erb
+++ b/app/views/diary_entry/list.html.erb
@@ -28,16 +28,25 @@
 <% else %>
   <h4><%= t 'diary_entry.list.recent_entries' %></h4>
 
-<% if @this_user %>
-  <%= render :partial => 'diary_entry', :collection => @entries %>
-<% else %>
-  <%= render :partial => 'diary_list_entry', :collection => @entries %>
-<% end %>
+  <% if @this_user %>
+    <%= render :partial => 'diary_entry', :collection => @entries %>
+  <% else %>
+    <%= render :partial => 'diary_list_entry', :collection => @entries %>
+  <% end %>
 
-<%= link_to t('diary_entry.list.older_entries'), { :page => @entry_pages.current.next, :language => params[:language] } if @entry_pages.current.next %>
-<% if @entry_pages.current.next and @entry_pages.current.previous %>|<% end %>
-<%= link_to t('diary_entry.list.newer_entries'), { :page => @entry_pages.current.previous, :language => params[:language] } if @entry_pages.current.previous %>
+  <% if @page > 1 -%>
+    <%= link_to t('diary_entry.list.older_entries'), params.merge(:page => @page - 1 ) %>
+  <% else -%>
+    <%= t('diary_entry.list.older_entries') %>
+  <% end -%>
 
+  |
+
+  <% if @entries.size < @page_size -%>
+    <%= t('diary_entry.list.newer_entries') %>
+  <% else -%>
+    <%= link_to t('diary_entry.list.newer_entries'), params.merge(:page => @page + 1) %>
+  <% end -%>
 <% end %>
 
 <% unless params[:friends] or params[:nearby] -%>