Merge pull request #1536 from basecamp/timeline-refactor

Events timeline: move some helpers to a column presenter

app/helpers/events_helper.rb

@@ -1,74 +1,22 @@
 module EventsHelper
-  def event_columns(event_type, day_timeline)
-    case event_type
-    when "added"
-      events = day_timeline.events.where(action: [ "card_published", "card_reopened" ])
-      full_events_count = events.count
-      {
-        title: event_column_title("Added", full_events_count, day_timeline.day),
-        index: 1,
-        events: events.limit(100).load,
-        full_events_count: full_events_count
-      }
-    when "closed"
-      events = day_timeline.events.where(action: "card_closed")
-      full_events_count = events.count
-      {
-        title: event_column_title("Closed", full_events_count, day_timeline.day),
-        index: 3,
-        events: events.limit(100).load,
-        full_events_count: full_events_count
-      }
+  def event_action_icon(event)
+    case event.action
+    when "card_assigned"
+      "assigned"
+    when "card_unassigned"
+      "minus"
+    when "comment_created"
+      "comment"
+    when "card_title_changed"
+      "rename"
+    when "card_board_changed"
+      "move"
     else
-      events = day_timeline.events.where.not(action: [ "card_published", "card_closed", "card_reopened" ])
-      full_events_count = events.count
-      {
-        title: event_column_title("Updated", full_events_count, day_timeline.day),
-        index: 2,
-        events: events.limit(100).load,
-        full_events_count: full_events_count
-      }
+      "person"
     end
   end
 
-  private
-    def event_column_title(base_title, count, day)
-      date_tag = local_datetime_tag day, style: :agoorweekday
-      parts = [ base_title, date_tag ]
-      parts << tag.span("(#{count})", class: "font-weight-normal") if count > 0
-      safe_join(parts, " ")
-    end
-
-    def event_column(event)
-      case event.action
-      when "card_closed"
-        3
-      when "card_published", "card_reopened"
-        1
-      else
-        2
-      end
-    end
-
-    def event_cluster_tag(hour, col, &)
-      row = 25 - hour
-      tag.div class: "events__time-block", style: "grid-area: #{row}/#{col}", &
-    end
-
-    def event_action_icon(event)
-      case event.action
-      when "card_assigned"
-        "assigned"
-      when "card_unassigned"
-        "minus"
-      when "comment_created"
-        "comment"
-      when "card_title_changed"
-        "rename"
-      when "card_board_changed"
-        "move"
-      else
-        "person"
-      end
-    end
+  def events_at_hour_container(column, hour, &block)
+    tag.div class: "events__time-block", style: "grid-area: #{25 - hour}/#{column.index}", &block
+  end
 end

app/models/user/day_timeline.rb

@@ -29,6 +29,18 @@ class User::DayTimeline
     day.yesterday.beginning_of_day
   end
 
+  def added_column
+    @added_column ||= build_column("Added", 1, events.where(action: %w[card_published card_reopened]))
+  end
+
+  def updated_column
+    @updated_column ||= build_column("Updated", 2, events.where.not(action: %w[card_published card_closed card_reopened]))
+  end
+
+  def closed_column
+    @closed_column ||= build_column("Closed", 3, events.where(action: "card_closed"))
+  end
+
   def cache_key
     ActiveSupport::Cache.expand_cache_key [ user, filter, day.to_date, events ], "day-timeline"
   end
@@ -70,6 +82,10 @@ class User::DayTimeline
       filtered_events.where(created_at: ...day.beginning_of_day).chronologically.last
     end
 
+    def build_column(base_title, index, events)
+      Column.new(self, base_title, index, events)
+    end
+
     def window
       day.all_day
     end

app/models/user/day_timeline/column.rb

@@ -0,0 +1,42 @@
+class User::DayTimeline::Column
+  include ActionView::Helpers::TagHelper, ActionView::Helpers::OutputSafetyHelper, TimeHelper
+
+  attr_reader :index
+
+  def initialize(day_timeline, base_title, index, events)
+    @day_timeline = day_timeline
+    @base_title = base_title
+    @index = index
+    @events = events
+  end
+
+  def title
+    date_tag = local_datetime_tag(day_timeline.day, style: :agoorweekday)
+    parts = [ base_title, date_tag ]
+    parts << tag.span("(#{full_events_count})", class: "font-weight-normal") if full_events_count > 0
+    safe_join(parts, " ")
+  end
+
+  def events_by_hour
+    limited_events.group_by { it.created_at.hour }
+  end
+
+  def has_more_events?
+    limited_events.count < full_events_count
+  end
+
+  def hidden_events_count
+    full_events_count - limited_events.count
+  end
+
+  private
+    attr_reader :base_title, :day_timeline, :events
+
+    def limited_events
+      @limited_events ||= events.limit(100).load
+    end
+
+    def full_events_count
+      @full_events_count ||= events.count
+    end
+end

app/views/events/_day_timeline.html.erb

@@ -1,8 +0,0 @@
-<% day_timeline.events.group_by { it.created_at.hour }.each do |hour, events_grouped_by_hour| %>
-  <% events_grouped_by_hour.group_by { event_column(it) }.each do |column, events| %>
-    <%= event_cluster_tag(hour, column) do %>
-      <%= render partial: "events/event", collection: events %>
-      <%= local_datetime_tag events.first.created_at, class: "event__timestamp txt-small translucent" %>
-    <% end %>
-  <% end %>
-<% end %>

app/views/events/day_timeline/_column.html.erb

@@ -1,17 +1,15 @@
-<% column = event_columns(event_type, day_timeline) %>
-
 <div class="events__column">
-  <h3 class="events__column-header"><%= column[:title] %></h3>
-  <% column[:events].group_by { it.created_at.hour }.each do |hour, events| %>
-    <%= event_cluster_tag(hour, column[:index]) do %>
+  <h3 class="events__column-header"><%= column.title %></h3>
+  <% column.events_by_hour.each do |hour, events| %>
+    <%= events_at_hour_container(column, hour) do %>
       <%= render partial: "events/event", collection: events, cached: true %>
       <%= local_datetime_tag events.first.created_at, class: "event__timestamp txt-small translucent" %>
     <% end %>
   <% end %>
 
-  <% if column[:events].count < column[:full_events_count] %>
-    <div class="events__column-footer fill-highlight txt-x-small border-radius" style="grid-column-start: <%= column[:index] %>">
-      Showing the 100 most recent (<%= column[:full_events_count] - column[:events].count %> are hidden)
+  <% if column.has_more_events? %>
+    <div class="events__column-footer fill-highlight txt-x-small border-radius" style="grid-column-start: <%= column.index %>">
+      Showing the 100 most recent (<%= column.hidden_events_count %> are hidden)
     </div>
   <% end %>
 </div>

app/views/events/day_timeline/_columns.html.erb

@@ -1,9 +1,9 @@
 <% cache [ day_timeline.events ] do %>
   <% if day_timeline.has_activity? %>
     <div class="events__columns">
-      <%= render "events/day_timeline/column", event_type: "added", day_timeline: day_timeline %>
-      <%= render "events/day_timeline/column", event_type: "updated", day_timeline: day_timeline %>
-      <%= render "events/day_timeline/column", event_type: "closed", day_timeline: day_timeline %>
+      <%= render "events/day_timeline/column", column: day_timeline.added_column %>
+      <%= render "events/day_timeline/column", column: day_timeline.updated_column %>
+      <%= render "events/day_timeline/column", column: day_timeline.closed_column %>
     </div>
   <% end %>
 <% end %>

app/views/events/event/_layout.html.erb

@@ -18,7 +18,7 @@
       </span>
     </h4>
 
-    <% if event_column(event) == 2 %>
+    <% if event.action.in?(%w[ card_closed card_published card_reopened ]) %>
       <span class="event__icon flex-item-justify-end txt-reversed">
         <%= icon_tag event_action_icon(event) %>
       </span>