refactor: extract events feed query from events controller into query object

app/controllers/events_controller.rb

@@ -14,7 +14,7 @@ def upcoming
     # (mirrors workshops#show).
     return if performed?
 
-    @events, @pagy = fetch_upcoming_events
+    @events, @pagy = EventsFeedQuery.call(upcoming: true, page: requested_page, request:)
   end
 
   def past
@@ -29,7 +29,7 @@ def past
     # key alone cannot see them — the short expiry bounds that staleness.
     return if @past_events_page_html
 
-    @past_events, @pagy = fetch_past_events
+    @past_events, @pagy = EventsFeedQuery.call(upcoming: false, page: requested_page, request:)
     # Only mint a key for a page that exists: bogus page numbers would
     # otherwise flood the cache store with garbage keys, and Solid Cache
     # evicts oldest entries globally when over max_size.
@@ -111,112 +111,4 @@ def find_or_create_invitation(event, member, role)
   def set_event
     @event = Event.find_by!(slug: params[:event_id])
   end
-
-  def fetch_upcoming_events
-    result = paginated_events(upcoming: true)
-    return [{}, nil] if result.nil?
-
-    rows, pagy = result
-    events = load_events(rows)
-    grouped = events.group_by(&:date)
-    decorated = grouped.transform_values { |items| EventPresenter.decorate_collection(items) }
-
-    [decorated, pagy]
-  end
-
-  def fetch_past_events
-    result = paginated_events(upcoming: false)
-    return [{}, nil] if result.nil?
-
-    rows, pagy = result
-    events = load_events(rows)
-    grouped = events.group_by(&:date)
-    decorated = grouped.transform_values { |items| EventPresenter.decorate_collection(items) }
-
-    [decorated, pagy]
-  end
-
-  # Builds a UNION ALL query across Workshops, Meetings, and Events,
-  # paginates at the database level, and returns (id, event_type) pairs
-  # for the current page. Only the 20 visible rows come back from the DB.
-  def paginated_events(upcoming:)
-    now = Time.zone.now
-    page = requested_page
-    direction = upcoming ? 'ASC' : 'DESC'
-    comparator = upcoming ? :gteq : :lt
-
-    # Pure Arel subqueries — no ActiveRecord relation bind params to leak
-    w = Arel::Table.new(:workshops)
-    ch = Arel::Table.new(:chapters)
-    ws = Arel::SelectManager.new(w)
-    ws.project(w[:id], w[:date_and_time], Arel.sql("'Workshop' AS event_type"))
-    ws.join(ch).on(w[:chapter_id].eq(ch[:id]))
-    ws.where(ch[:active].eq(true).and(w[:date_and_time].public_send(comparator, now)))
-
-    m = Arel::Table.new(:meetings)
-    ms = Arel::SelectManager.new(m)
-    ms.project(m[:id], m[:date_and_time], Arel.sql("'Meeting' AS event_type"))
-    ms.where(m[:date_and_time].public_send(comparator, now))
-
-    e = Arel::Table.new(:events)
-    es = Arel::SelectManager.new(e)
-    es.project(e[:id], e[:date_and_time], Arel.sql("'Event' AS event_type"))
-    es.where(e[:date_and_time].public_send(comparator, now))
-
-    union = Arel::Nodes::UnionAll.new(
-      Arel::Nodes::UnionAll.new(ws, ms), es
-    )
-
-    # COUNT at DB level — single query
-    count_query = Arel::SelectManager.new
-    count_query.from(union.as('events'))
-    count_query.project(Arel.star.count)
-    total = ActiveRecord::Base.connection.select_value(count_query.to_sql).to_i
-    return nil if total.zero?
-
-    pagy_opts = { count: total, page:, limit: 20, request: }
-    pagy_opts[:request] = Pagy::Request.new(pagy_opts)
-    pagy = Pagy::Offset.new(**pagy_opts)
-
-    # Only 20 rows leave the database
-    pagination_query = Arel::SelectManager.new
-    pagination_query.from(union.as('events'))
-    pagination_query.project(:id, :event_type)
-    pagination_query.order(Arel.sql("date_and_time #{direction}"))
-    pagination_query.skip(pagy.offset).take(pagy.limit)
-
-    rows = ActiveRecord::Base.connection.select_all(pagination_query.to_sql)
-    [rows, pagy]
-  end
-
-  # Loads full ActiveRecord objects for the paginated (id, event_type) pairs
-  # with eager loading, preserving the UNION order.
-  def load_events(rows)
-    grouped = rows.each_with_object({}) do |row, hash|
-      (hash[row['event_type']] ||= []) << row['id'].to_i
-    end
-
-    # workshop_host must be preloaded: eager_loading it alongside :sponsors
-    # routes both through the workshop_sponsors table in one join, and the
-    # host scope then binds to the wrong table alias — the host comes back
-    # nil or points at a non-host sponsor. Same pattern as Listable.most_recent.
-    workshops = Workshop.eager_load(:sponsors, :organisers,
-                                    chapter: { permissions: :members })
-                        .preload(workshop_host: :sponsor)
-                        .where(id: grouped['Workshop'])
-                        .to_a.index_by(&:id)
-    meetings = Meeting.eager_load(:venue, :organisers).where(id: grouped['Meeting'])
-                      .to_a.index_by(&:id)
-    events = Event.eager_load(:venue, :sponsors, :organisers)
-                  .where(id: grouped['Event'])
-                  .to_a.index_by(&:id)
-
-    rows.filter_map do |row|
-      case row['event_type']
-      when 'Workshop' then workshops[row['id'].to_i]
-      when 'Meeting' then meetings[row['id'].to_i]
-      when 'Event' then events[row['id'].to_i]
-      end
-    end
-  end
 end

app/queries/event_feed_union.rb

@@ -0,0 +1,50 @@
+class EventFeedUnion
+  def self.call(upcoming:)
+    new.call(upcoming:)
+  end
+
+  def call(upcoming:)
+    build_event_feed_union(upcoming:)
+  end
+
+  private
+
+  def build_event_feed_union(upcoming:)
+    now = Time.zone.now
+    comparator = upcoming ? :gteq : :lt
+
+    # Pure Arel subqueries — no ActiveRecord relation bind params to leak
+    workshop_subquery = workshop_feed_subquery(comparator:, now:)
+    meeting_subquery = meeting_feed_subquery(comparator:, now:)
+    event_subquery = event_feed_subquery(comparator:, now:)
+
+    Arel::Nodes::UnionAll.new(
+      Arel::Nodes::UnionAll.new(workshop_subquery, meeting_subquery), event_subquery
+    )
+  end
+
+  def workshop_feed_subquery(comparator:, now:)
+    workshops = Arel::Table.new(:workshops)
+    chapters = Arel::Table.new(:chapters)
+
+    query = timed_feed_subquery(table: workshops, type: 'Workshop', comparator:, now:)
+    query.join(chapters).on(workshops[:chapter_id].eq(chapters[:id]))
+    query.where(chapters[:active].eq(true).and(workshops[:date_and_time].public_send(comparator, now)))
+    query
+  end
+
+  def timed_feed_subquery(table:, type:, comparator:, now:)
+    query = Arel::SelectManager.new(table)
+    query.project(table[:id], table[:date_and_time], Arel.sql("'#{type}' AS event_type"))
+    query.where(table[:date_and_time].public_send(comparator, now))
+    query
+  end
+
+  def meeting_feed_subquery(comparator:, now:)
+    timed_feed_subquery(table: Arel::Table.new(:meetings), type: 'Meeting', comparator:, now:)
+  end
+
+  def event_feed_subquery(comparator:, now:)
+    timed_feed_subquery(table: Arel::Table.new(:events), type: 'Event', comparator:, now:)
+  end
+end

app/queries/events_feed_query.rb

@@ -0,0 +1,120 @@
+require 'pagy/classes/request'
+
+class EventsFeedQuery
+  def self.call(...)
+    new(...).call
+  end
+
+  def initialize(upcoming:, page:, request:)
+    @upcoming = upcoming
+    @page = page
+    @request = request
+  end
+
+  def call
+    fetch_events(upcoming: @upcoming)
+  end
+
+  private
+
+  # Builds a UNION ALL query across Workshops, Meetings, and Events,
+  # paginates at the database level, and returns (id, event_type) pairs
+  # for the current page. Only the 20 visible rows come back from the DB.
+  def paginated_events(upcoming:)
+    direction = upcoming ? 'ASC' : 'DESC'
+    union = EventFeedUnion.call(upcoming:)
+    total = count_query(union:)
+    return nil if total.zero?
+
+    pagy_opts = { count: total, page: @page, limit: 20, request: @request }
+    pagy_opts[:request] = Pagy::Request.new(pagy_opts)
+    pagy = Pagy::Offset.new(**pagy_opts)
+
+    # Only 20 rows leave the database
+    pagination_query = pagination_query(union:, pagy:, direction:)
+
+    rows = ActiveRecord::Base.connection.select_all(pagination_query.to_sql)
+    [rows, pagy]
+  end
+
+  # Loads full ActiveRecord objects for the paginated (id, event_type) pairs
+  # with eager loading, preserving the UNION order.
+  def load_events(rows)
+    grouped = rows.each_with_object({}) do |row, hash|
+      (hash[row['event_type']] ||= []) << row['id'].to_i
+    end
+
+    # workshop_host must be preloaded: eager_loading it alongside :sponsors
+    # routes both through the workshop_sponsors table in one join, and the
+    # host scope then binds to the wrong table alias — the host comes back
+    # nil or points at a non-host sponsor. Same pattern as Listable.most_recent.
+    workshops = load_workshops_by_id(grouped:)
+    meetings = load_meetings_by_id(grouped:)
+    events = load_events_by_id(grouped:)
+
+    resolve_events_in_order(rows:, workshops:, meetings:, events:)
+  end
+
+  def fetch_events(upcoming:)
+    result = paginated_events(upcoming:)
+    return [{}, nil] if result.nil?
+
+    rows, pagy = result
+    events = load_events(rows)
+    grouped = events.group_by(&:date)
+    decorated = grouped.transform_values { |items| EventPresenter.decorate_collection(items) }
+
+    [decorated, pagy]
+  end
+
+  def count_query(union:)
+    # COUNT at DB level — single query
+    count_query = Arel::SelectManager.new
+    count_query.from(union.as('events'))
+    count_query.project(Arel.star.count)
+    ActiveRecord::Base.connection.select_value(count_query.to_sql).to_i
+  end
+
+  def pagination_query(union:, pagy:, direction:)
+    pagination_query = Arel::SelectManager.new
+    pagination_query.from(union.as('events'))
+    pagination_query.project(:id, :event_type)
+    pagination_query.order(Arel.sql("date_and_time #{direction}"))
+    pagination_query.skip(pagy.offset).take(pagy.limit)
+    pagination_query
+  end
+
+  def load_workshops_by_id(grouped:)
+    Workshop
+      .eager_load(:sponsors, :organisers, chapter: { permissions: :members })
+      .preload(workshop_host: :sponsor)
+      .where(id: grouped['Workshop'])
+      .to_a
+      .index_by(&:id)
+  end
+
+  def load_meetings_by_id(grouped:)
+    Meeting
+      .eager_load(:venue, :organisers).where(id: grouped['Meeting'])
+      .to_a
+      .index_by(&:id)
+  end
+
+  def load_events_by_id(grouped:)
+    Event
+      .eager_load(:venue, :sponsors, :organisers)
+      .where(id: grouped['Event'])
+      .to_a
+      .index_by(&:id)
+  end
+
+  def resolve_events_in_order(rows:, workshops:, meetings:, events:)
+    rows.filter_map do |row|
+      case row['event_type']
+      when 'Workshop' then workshops[row['id'].to_i]
+      when 'Meeting' then meetings[row['id'].to_i]
+      when 'Event' then events[row['id'].to_i]
+      end
+    end
+  end
+end

spec/controllers/events_controller_spec.rb

@@ -1,22 +1,6 @@
 require 'rails_helper'
 
 RSpec.describe EventsController do
-  describe '#fetch_upcoming_events' do
-    context 'when no events exist' do
-      it 'returns empty hash and nil pagy' do
-        expect(controller.send(:fetch_upcoming_events)).to eq([{}, nil])
-      end
-    end
-  end
-
-  describe '#fetch_past_events' do
-    context 'when no events exist' do
-      it 'returns empty hash and nil pagy' do
-        expect(controller.send(:fetch_past_events)).to eq([{}, nil])
-      end
-    end
-  end
-
   describe 'GET #student' do
     let(:member) { Fabricate(:member) }
     let(:event) { Fabricate(:event) }
@@ -244,65 +228,4 @@
       expect(response).to have_http_status(:ok)
     end
   end
-
-  describe '#load_events' do
-    let(:chapter) { Fabricate(:chapter) }
-    let(:host_sponsor) { Fabricate(:sponsor, name: 'Zulu GmbH') }
-    let(:extra_sponsor) { Fabricate(:sponsor, name: 'Alpha AG') }
-
-    def rows_for(workshops)
-      workshops.map { |w| { 'event_type' => 'Workshop', 'id' => w.id } }
-    end
-
-    it 'loads the host sponsor as workshop_host for every workshop' do
-      workshops = Fabricate.times(6, :workshop_no_sponsor, chapter:)
-      workshops.each do |workshop|
-        Fabricate(:workshop_sponsor, workshop:, sponsor: host_sponsor, host: true)
-        Fabricate(:workshop_sponsor, workshop:, sponsor: extra_sponsor, host: false)
-      end
-
-      loaded = controller.send(:load_events, rows_for(workshops))
-
-      expect(loaded.map { |e| e.workshop_host&.sponsor&.id }).to all(eq(host_sponsor.id))
-    end
-
-    it 'lists all sponsors of a workshop' do
-      workshop = Fabricate(:workshop_no_sponsor, chapter:)
-      Fabricate(:workshop_sponsor, workshop:, sponsor: host_sponsor, host: true)
-      Fabricate(:workshop_sponsor, workshop:, sponsor: extra_sponsor, host: false)
-
-      loaded = controller.send(:load_events, rows_for([workshop]))
-
-      expect(loaded.first.sponsors).to contain_exactly(host_sponsor, extra_sponsor)
-    end
-
-    it 'renders sponsors and organisers in name order' do
-      workshop = Fabricate(:workshop_no_sponsor, chapter:)
-      Fabricate(:workshop_sponsor, workshop:, sponsor: host_sponsor, host: true)
-      Fabricate(:workshop_sponsor, workshop:, sponsor: extra_sponsor, host: false)
-      anton = Fabricate(:member, name: 'Anton')
-      zora = Fabricate(:member, name: 'Zora')
-      anton.add_role :organiser, workshop
-      zora.add_role :organiser, workshop
-
-      loaded = controller.send(:load_events, rows_for([workshop]))
-      presenter = EventPresenter.decorate(loaded.first)
-
-      expect(presenter.sponsors.map(&:name)).to eq(['Alpha AG', 'Zulu GmbH'])
-      expect(presenter.organisers.map(&:name)).to eq(%w[Anton Zora])
-    end
-
-    it 'sorts organisers with blank names without raising' do
-      workshop = Fabricate(:workshop_no_sponsor, chapter:)
-      anton = Fabricate(:member, name: 'Anton')
-      anton.update_column(:name, nil)
-      anton.add_role :organiser, workshop
-      Fabricate(:member, name: 'Zora').add_role :organiser, workshop
-
-      loaded = controller.send(:load_events, rows_for([workshop]))
-      presenter = EventPresenter.decorate(loaded.first)
-
-      expect(presenter.organisers.map(&:name)).to eq([nil, 'Zora'])
-    end
-  end
 end