Serve /events/past from a page-level fragment on cache hits

app/controllers/events_controller.rb

@@ -22,7 +22,18 @@ def past
     fresh_when(latest, etag: latest)
     return if performed?
 
+    key = past_page_cache_key(latest)
+    @past_events_page_html = read_fragment(key)
+    # A hit skips the COUNT, eager loads, decoration and render; the view
+    # outputs the stored page. Deletions do not bump MAX(updated_at), so the
+    # key alone cannot see them — the short expiry bounds that staleness.
+    return if @past_events_page_html
+
     @past_events, @pagy = fetch_past_events
+    # 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.
+    @past_events_cache_key = key if @pagy.nil? || requested_page <= @pagy.pages
   end
 
   def show
@@ -59,6 +70,17 @@ def rsvp
 
   private
 
+  def requested_page
+    # Clamp to >= 1 (mirrors Pagy's own resolve_page); .to_s also handles array params
+    [1, params[:page].to_s.to_i].max
+  end
+
+  def past_page_cache_key(latest)
+    # .to_f: a raw Time in a cache key is stringified with subsecond precision
+    # that differs between the write and the read (see sitemaps/show.xml.builder).
+    [:events_past_page, I18n.locale, requested_page, latest.to_f]
+  end
+
   def latest_model_updated
     sql = <<~SQL.squish
       SELECT MAX(latest) FROM (
@@ -119,8 +141,7 @@ def fetch_past_events
   # for the current page. Only the 20 visible rows come back from the DB.
   def paginated_events(upcoming:)
     now = Time.zone.now
-    # Clamp to >= 1 (mirrors Pagy's own resolve_page); .to_s also handles array params
-    page = [1, params[:page].to_s.to_i].max
+    page = requested_page
     direction = upcoming ? 'ASC' : 'DESC'
     comparator = upcoming ? :gteq : :lt
 

app/views/events/past.html.haml

@@ -1,12 +1,18 @@
 - title 'Past Events'
 
-.container{'data-test': 'past-events'}
-  .row
-    .col
-      - if @past_events.any?
-        %h3.mb-4 Past Events
-        = render partial: 'events', locals: { grouped_events: @past_events }
+- if @past_events_page_html
+  -# haml-lint:disable UnescapedHtml -- trusted: rendered by this template
+  != @past_events_page_html
+- else
+  - cache_if(@past_events_cache_key.present?, @past_events_cache_key,
+             skip_digest: true, expires_in: 30.minutes) do
+    .container{ 'data-test': 'past-events' }
+      .row
+        .col
+          - if @past_events.any?
+            %h3.mb-4 Past Events
+            = render partial: 'events', locals: { grouped_events: @past_events }
 
-- if @pagy
-  .container.mt-4
-    = render partial: 'shared/pagination', locals: { pagy: @pagy, model: 'event' }
+    - if @pagy
+      .container.mt-4
+        = render partial: 'shared/pagination', locals: { pagy: @pagy, model: 'event' }

spec/requests/events_past_page_fragment_spec.rb

@@ -0,0 +1,89 @@
+require 'rails_helper'
+
+RSpec.describe 'Past events page fragment' do
+  let(:fragment_store) { ActiveSupport::Cache::MemoryStore.new }
+
+  around do |example|
+    old_perform_caching = ActionController::Base.perform_caching
+    old_cache_store = ActionController::Base.cache_store
+
+    ActionController::Base.perform_caching = true
+    ActionController::Base.cache_store = fragment_store
+
+    example.run
+  ensure
+    ActionController::Base.perform_caching = old_perform_caching
+    ActionController::Base.cache_store = old_cache_store
+  end
+
+  def count_queries
+    queries = 0
+    subscriber = ActiveSupport::Notifications.subscribe('sql.active_record') { queries += 1 }
+    yield
+    queries
+  ensure
+    ActiveSupport::Notifications.unsubscribe(subscriber)
+  end
+
+  before do
+    Fabricate(:event, date_and_time: 2.weeks.ago, name: 'Ancient workshop')
+    Fabricate(:event, date_and_time: 3.weeks.ago, name: 'Older workshop')
+  end
+
+  it 'stores the rendered page as a single fragment' do
+    get '/events/past'
+
+    fragment_keys = fragment_store.instance_variable_get(:@data).keys
+    expect(fragment_keys.grep(/events_past_page/)).to be_present
+  end
+
+  it 'serves repeat visits from the fragment without the fetch pipeline' do
+    first_visit_queries = count_queries { get '/events/past' }
+    repeat_visit_queries = count_queries { get '/events/past' }
+
+    expect(response).to have_http_status(:ok)
+    expect(response.body).to include('Ancient workshop')
+    expect(repeat_visit_queries).to be < first_visit_queries
+  end
+
+  it 'clamps invalid page params onto the page 1 fragment' do
+    get '/events/past', params: { page: 0 }
+    clamped_keys = fragment_store.instance_variable_get(:@data).keys
+
+    get '/events/past'
+
+    fragment_keys = fragment_store.instance_variable_get(:@data).keys
+    expect(fragment_keys).to eq(clamped_keys)
+  end
+
+  it 'does not persist fragments for pages beyond the last page' do
+    get '/events/past', params: { page: 999_999 }
+
+    fragment_keys = fragment_store.instance_variable_get(:@data).keys
+    expect(fragment_keys.grep(/events_past_page/)).to be_empty
+  end
+
+  it 'serves fresh content once a tracked table row changes' do
+    get '/events/past'
+    stale_body = response.body
+
+    Fabricate(:event, date_and_time: 4.weeks.ago, name: 'Brand new past event')
+
+    get '/events/past'
+
+    expect(response.body).not_to eq(stale_body)
+    expect(response.body).to include('Brand new past event')
+  end
+
+  it 'expires the fragment so a deleted event self-heals within the horizon' do
+    get '/events/past'
+    deleted = Event.find_by(name: 'Ancient workshop')
+    deleted.destroy
+
+    travel_to(31.minutes.from_now) do
+      get '/events/past'
+
+      expect(response.body).not_to include('Ancient workshop')
+    end
+  end
+end