Paginate the timeline to allow it to render in a reasonable amount of time. In order to avoid sorting large numbers of objects in order to get the right order of combined events across the two model types, get the overall order first from the database and then populate the slice we're interested in.

3 files changed, 96 insertions, 32 deletions

diff --git a/app/controllers/admin_general_controller.rb b/app/controllers/admin_general_controller.rb
index 2c961dfc5..7e8498d8a 100644
--- a/app/controllers/admin_general_controller.rb
+++ b/app/controllers/admin_general_controller.rb
@@ -61,16 +61,59 @@ class AdminGeneralController < AdminController
             @events_title = "Events, all time"
             date_back_to = Time.now - 1000.years
         end
-        @events = InfoRequestEvent.find(:all, :order => "created_at desc, id desc",
-                :conditions => ["created_at > ? ", date_back_to.getutc])
-        @public_body_history = PublicBody.versioned_class.find(:all, :order => "updated_at desc, id desc",
-                :conditions => ["updated_at > ? ", date_back_to.getutc])
-        for pbh in @public_body_history
-            pbh.created_at = pbh.updated_at
+
+        # Get an array of event attributes within the timespan in the format
+        # [id, type_of_model, event_timestamp]
+        # Note that the relevent date for InfoRequestEvents is creation, but
+        # for PublicBodyVersions is update thoughout
+        connection = InfoRequestEvent.connection
+        timestamps = connection.select_rows("SELECT id,'InfoRequestEvent',
+                                                    created_at AS timestamp
+                                             FROM info_request_events
+                                             WHERE created_at > '#{date_back_to.getutc}'
+                                             UNION
+                                             SELECT id, 'PublicBodyVersion',
+                                                  updated_at AS timestamp
+                                             FROM #{PublicBody.versioned_class.table_name}
+                                             WHERE updated_at > '#{date_back_to.getutc}'
+                                             ORDER by timestamp desc")
+        @events = WillPaginate::Collection.create((params[:page] or 1), 100) do |pager|
+            # create a hash for each model type being returned
+            info_request_event_ids = {}
+            public_body_version_ids = {}
+            # get the relevant slice from the paginator
+            timestamps.slice(pager.offset, pager.per_page).each_with_index do |event, index|
+                # for each event in the slice, add an item to the hash for the model type
+                # whose key is the model id, and value is the position in the slice
+                if event[1] == 'InfoRequestEvent'
+                    info_request_event_ids[event[0].to_i] = index
+                else
+                    public_body_version_ids[event[0].to_i] = index
+                end
+            end
+            # get all the models in the slice, eagerly loading the associations we use in the view
+            public_body_versions = PublicBody.versioned_class.find(:all,
+                                          :conditions => ['id in (?)', public_body_version_ids.keys],
+                                          :include => [ { :public_body => :translations }])
+            info_request_events = InfoRequestEvent.find(:all,
+                                          :conditions => ['id in (?)', info_request_event_ids.keys],
+                                          :include => [:info_request])
+            @events = []
+            # drop the models into a combined array, ordered by their position in the timestamp slice
+            public_body_versions.each do |version|
+                @events[public_body_version_ids[version.id]] = [version, version.updated_at]
+            end
+            info_request_events.each do |event|
+                @events[info_request_event_ids[event.id]] = [event, event.created_at]
+            end
+
+            # inject the result array into the paginated collection:
+            pager.replace(@events)
+
+            # set the total entries for the page to the overall number of results
+            pager.total_entries = timestamps.size
         end
-        @events += @public_body_history
 
-        @events.sort! { |a,b| b.created_at <=> a.created_at }
     end
 
     def stats
diff --git a/app/views/admin_general/timeline.rhtml b/app/views/admin_general/timeline.rhtml
index eecab4823..e84539970 100644
--- a/app/views/admin_general/timeline.rhtml
+++ b/app/views/admin_general/timeline.rhtml
@@ -11,25 +11,25 @@
 | <a href="?all=1">All time</a></p>
 
 <% last_date = nil %>
-<% for event in @events %>
-    <% if last_date != event.created_at.to_date %>
+<% for event, event_at in @events %>
+    <% if last_date != event_at.to_date %>
         <% if last_date.nil? %>
             <p>
         <% end %>
-        <h3><%= simple_date(event.created_at) %></h3>
+        <h3><%= simple_date(event_at) %></h3>
         <p>
     <% else %>
         <br>
     <% end %>
-    <% last_date = event.created_at.to_date %>
-    
-    <%= simple_time(event.created_at) %>
+    <% last_date = event_at.to_date %>
 
-    <% if event.class.to_s == 'InfoRequestEvent' %>
+    <%= simple_time(event_at) %>
+
+    <% if event.is_a? InfoRequestEvent %>
         <%= request_both_links(event.info_request) %>
         <% if event.event_type == 'edit' %>
             was edited by administrator <strong><%=h event.params[:editor] %></strong>.
-            <% for p in ['title', 'prominence', 'described_state', 'awaiting_description'] 
+            <% for p in ['title', 'prominence', 'described_state', 'awaiting_description']
                     if event.params[p.to_sym] != event.params[('old_'+p).to_sym]
                         %> Changed <%=p%> from '<%=h event.params[('old_'+p).to_sym]%>' to '<%=h event.params[p.to_sym] %>'. <%
                     end
@@ -39,7 +39,7 @@
             <% outgoing_message = OutgoingMessage.find(event.params[:outgoing_message_id].to_i) %>
             had outgoing message edited by administrator <strong><%=h event.params[:editor] %></strong>.
             <% if outgoing_message %>
-                <% for p in ['body'] 
+                <% for p in ['body']
                         if event.params[p.to_sym] != event.params[('old_'+p).to_sym]
                             %> Changed <%=p%> from '<%=h event.params[('old_'+p).to_sym]%>' to '<%=h event.params[p.to_sym] %>'. <%
                         end
@@ -52,7 +52,7 @@
             <% comment = Comment.find(event.params[:comment_id].to_i) %>
             had annotation edited by administrator <strong><%=h event.params[:editor] %></strong>.
             <% if comment %>
-                <% for p in ['body'] 
+                <% for p in ['body']
                         if event.params[p.to_sym] != event.params[('old_'+p).to_sym]
                             %> Changed <%=p%> from '<%=h event.params[('old_'+p).to_sym]%>' to '<%=h event.params[p.to_sym] %>'. <%
                         end
@@ -71,7 +71,7 @@
             had incoming message redelivered to another request by administrator <strong><%=h event.params[:editor] %></strong>.
          <% elsif event.event_type == 'response' %>
             <% incoming_message = event.incoming_message %>
-            received 
+            received
             <%= link_to 'a response', main_url(incoming_message_url(incoming_message)) %>
             from <%=h event.info_request.public_body.name %>.
         <% elsif event.event_type == 'sent' %>
@@ -95,5 +95,5 @@
 <% if not @events.empty? %>
 </p>
 <% end %>
-
+<%= will_paginate(@events) %>
 
diff --git a/spec/controllers/admin_general_controller_spec.rb b/spec/controllers/admin_general_controller_spec.rb
index 820d1e7f3..dc1eb0d97 100644
--- a/spec/controllers/admin_general_controller_spec.rb
+++ b/spec/controllers/admin_general_controller_spec.rb
@@ -1,18 +1,39 @@
 require File.expand_path(File.dirname(__FILE__) + '/../spec_helper')
 
-describe AdminGeneralController, "when viewing front page of admin interface" do
-    integrate_views
-    before { basic_auth_login @request }
-  
-    it "should render the front page" do
-        get :index, :suppress_redirect => 1
-        response.should render_template('index')
-    end
+describe AdminGeneralController do
+
+    describe "when viewing front page of admin interface" do
+
+        integrate_views
+        before { basic_auth_login @request }
+
+        it "should render the front page" do
+            get :index, :suppress_redirect => 1
+            response.should render_template('index')
+        end
+
+        it "should redirect to include trailing slash" do
+            get :index
+            response.should redirect_to(:controller => 'admin_general',
+                                        :action => 'index')
+        end
 
-    it "should redirect to include trailing slash" do
-        get :index
-        response.should redirect_to(:controller => 'admin_general',
-                                    :action => 'index')
     end
 
+    describe 'when viewing the timeline' do
+
+        it 'should assign an array of events in order of descending date to the view' do
+            get :timeline, :all => 1
+            previous_event = nil
+            previous_event_at = nil
+            assigns[:events].each do |event, event_at|
+                if previous_event
+                    (event_at <= previous_event_at).should be_true
+                end
+                previous_event = event
+                previous_event_at = event_at
+            end
+        end
+
+    end
 end