Merge branch '5371-cant-change-timeframe-on-admin-timeline-when-filte…

…ring' into develop

app/controllers/admin_general_controller.rb

@@ -5,6 +5,7 @@
 # Email: [email protected]; WWW: http://www.mysociety.org/
 
 class AdminGeneralController < AdminController
+  include AdminGeneralTimelineHelper
 
   def index
     # Tasks to do
@@ -170,46 +171,11 @@ def debug
 
   private
 
-  def get_date_back_to_utc
-    date_back_to = if params[:hour]
-      Time.zone.now - 1.hour
-    elsif params[:day]
-      Time.zone.now - 1.day
-    elsif params[:week]
-      Time.zone.now - 1.week
-    elsif params[:month]
-      Time.zone.now - 1.month
-    elsif params[:all]
-      Time.zone.now - 1000.years
-    else
-      Time.zone.now - 2.days
-    end
-    date_back_to.getutc
-  end
-
-  def get_event_type
-    if params[:event_type] == 'authority_change'
-      'Authority changes'
-    elsif params[:event_type] == 'info_request_event'
-      'Request events'
-    else
-      "Events"
-    end
-  end
-
   def get_events_title
-    title = if params[:hour]
-      "#{get_event_type} in last hour"
-    elsif params[:day]
-      "#{get_event_type} in last day"
-    elsif params[:week]
-      "#{get_event_type} in last week"
-    elsif params[:month]
-      "#{get_event_type} in last month"
-    elsif params[:all]
-      "#{get_event_type}, all time"
+    if current_time_filter == 'All time'
+      "#{current_event_type}, all time"
     else
-      "#{get_event_type} in last two days"
+      "#{current_event_type} in the last #{current_time_filter.downcase}"
     end
   end
 
@@ -219,26 +185,26 @@ def get_timestamps
     # Note that the relevant date for InfoRequestEvents is creation, but
     # for PublicBodyVersions is update throughout
     connection = InfoRequestEvent.connection
-    authority_change_clause = "SELECT id, 'PublicBodyVersion',
-                                      updated_at AS timestamp
-                               FROM #{PublicBody.versioned_class.table_name}
-                               WHERE updated_at > '#{get_date_back_to_utc}'"
-
-    info_request_event_clause = "SELECT id, 'InfoRequestEvent',
-                                        created_at AS timestamp
-                                 FROM info_request_events
-                                 WHERE created_at > '#{get_date_back_to_utc}'"
-
-    timestamps = if params[:event_type] == 'authority_change'
-      connection.select_rows("#{authority_change_clause}
-                              ORDER by timestamp desc")
-    elsif params[:event_type] == 'info_request_event'
-      connection.select_rows("#{info_request_event_clause}
-                              ORDER by timestamp desc")
+
+    authority_change_scope = PublicBody.versioned_class.
+      select("id, 'PublicBodyVersion', updated_at AS timestamp").
+      where(updated_at: start_date...).
+      order(timestamp: :desc)
+
+    info_request_event_scope = InfoRequestEvent.
+      select("id, 'InfoRequestEvent', created_at AS timestamp").
+      where(created_at: start_date...).
+      order(timestamp: :desc)
+
+    case params[:event_type]
+    when 'authority_change'
+      connection.select_rows(authority_change_scope.to_sql)
+    when 'info_request_event'
+      connection.select_rows(info_request_event_scope.to_sql)
     else
-      connection.select_rows("#{info_request_event_clause}
+      connection.select_rows("#{info_request_event_scope.unscope(:order).to_sql}
                               UNION
-                              #{authority_change_clause}
+                              #{authority_change_scope.unscope(:order).to_sql}
                               ORDER by timestamp desc")
     end
   end

app/helpers/admin_general_timeline_helper.rb

@@ -0,0 +1,36 @@
+##
+# Helpers for rendering timeline filter buttons. Also used in the controller for
+# generating the queries to load events.
+#
+module AdminGeneralTimelineHelper
+  def start_date
+    params[:start_date]&.to_datetime || 2.days.ago
+  end
+
+  def time_filters
+    {
+      'Hour' => 1.hour.ago,
+      'Day' => 1.day.ago,
+      '2 days' => 2.days.ago,
+      'Week' => 1.week.ago,
+      'Month' => 1.month.ago,
+      'All time' => Time.utc(1970, 1, 1)
+    }
+  end
+
+  def current_time_filter
+    time_filters.min_by { |_, time| (time - start_date).abs }.first
+  end
+
+  def event_types
+    {
+      authority_change: 'Authority changes',
+      info_request_event: 'Request events',
+      all: 'All events'
+    }
+  end
+
+  def current_event_type
+    event_types[params[:event_type]&.to_sym] || event_types[:all]
+  end
+end

app/views/admin_general/timeline.html.erb

@@ -1,23 +1,18 @@
 <% @title = "Timeline" %>
 <div class="btn-toolbar">
   <div class="btn-group">
-    <%= link_to "Hour", admin_timeline_path(:hour => 1), :class => "btn" %>
-    <%= link_to "Day", admin_timeline_path(:day => 1), :class => "btn" %>
-    <%= link_to "2 days", admin_timeline_path, :class => "btn" %>
-    <%= link_to "Week", admin_timeline_path(:week => 1), :class => "btn" %>
-    <%= link_to "Month", admin_timeline_path(:month => 1), :class => "btn" %>
-    <%= link_to "All time", admin_timeline_path(:all => 1), :class => "btn" %>
+    <% time_filters.each do |time_label, date| %>
+      <%= link_to time_label,
+                  admin_timeline_path(start_date: date&.utc&.iso8601, event_type: params[:event_type]),
+                  class: "btn #{'active' if current_time_filter == time_label}" %>
+    <% end %>
   </div>
   <div class="btn-group">
-    <%= link_to 'Authority changes',
-                { event_type: 'authority_change' },
-                class: 'btn' %>
-    <%= link_to 'Request events',
-                { event_type: 'info_request_event' },
-                class: 'btn' %>
-    <%= link_to 'All',
-                { event_type: nil },
-                class: 'btn' %>
+    <% event_types.each do |event_type, event_label| %>
+      <%= link_to event_label,
+                  admin_timeline_path(start_date: params[:start_date], event_type: event_type),
+                  class: "btn #{'active' if current_event_type == event_label}" %>
+    <% end %>
   </div>
 </div>
 <div class="row">

spec/controllers/admin_general_controller_spec.rb

@@ -339,16 +339,25 @@
     end
 
     it 'sets the title appropriately' do
-      expect(assigns[:events_title]).to eq("Events, all time")
+      expect(assigns[:events_title]).to eq("All events in the last 2 days")
+    end
+
+    context 'when start_date is set' do
+
+      before do
+        get :timeline, params: { all: 1, start_date: Time.utc(1970, 1, 1) }
+      end
+
+      it 'sets the title appropriately' do
+        expect(assigns[:events_title]).to eq("All events, all time")
+      end
+
     end
 
     context 'when event_type is info_request_event' do
 
       before do
-        get :timeline, params: {
-                         all: 1,
-                         event_type: 'info_request_event'
-                       }
+        get :timeline, params: { all: 1, event_type: 'info_request_event' }
       end
 
       it 'assigns an array of info request events in order of descending
@@ -358,7 +367,9 @@
       end
 
       it 'sets the title appropriately' do
-        expect(assigns[:events_title]).to eq("Request events, all time")
+        expect(assigns[:events_title]).to eq(
+          "Request events in the last 2 days"
+        )
       end
     end
 
@@ -375,7 +386,9 @@
       end
 
       it 'sets the title appropriately' do
-        expect(assigns[:events_title]).to eq("Authority changes, all time")
+        expect(assigns[:events_title]).to eq(
+          "Authority changes in the last 2 days"
+        )
       end
 
     end