From 422f3801aefbf463f587577c08bdf4955a07edce Mon Sep 17 00:00:00 2001
From: Morgan Roderick <morgan@roderick.dk>
Date: Wed, 7 Oct 2026 21:19:00 +0200
Subject: [PATCH] fix(chapters): cache the chapters sidebar with a 10-minute
 TTL

The sidebar fragment was cached under a fixed key while the Chapter
callbacks deleted a different key, so the homepage kept rendering
deactivated chapters for any write path, including direct database
updates. Cache the fragment under a versioned key with a 10-minute TTL
instead: every write path shows up within the TTL, and a warm cache hit
runs no chapter query.

Fixes #2993
---
 .../chapters_sidebar_component.html.erb       |  2 +-
 app/models/chapter.rb                         |  7 --
 spec/models/chapter_spec.rb                   | 24 -------
 .../requests/chapters_sidebar_caching_spec.rb | 67 +++++++++++++++++++
 4 files changed, 68 insertions(+), 32 deletions(-)
 create mode 100644 spec/requests/chapters_sidebar_caching_spec.rb

diff --git a/app/components/chapters_sidebar_component.html.erb b/app/components/chapters_sidebar_component.html.erb
index df5116ca4..66c22d549 100644
--- a/app/components/chapters_sidebar_component.html.erb
+++ b/app/components/chapters_sidebar_component.html.erb
@@ -1,4 +1,4 @@
-<% cache "chapters-sidebar" do %>
+<% cache ['chapters-sidebar', title, I18n.locale, :v2], skip_digest: true, expires_in: 10.minutes do %>
   <% if title %>
     <h3><%= title %></h3>
   <% end %>
diff --git a/app/models/chapter.rb b/app/models/chapter.rb
index 7a5b5cb90..70822e516 100644
--- a/app/models/chapter.rb
+++ b/app/models/chapter.rb
@@ -17,9 +17,6 @@ class Chapter < ApplicationRecord
   has_many :feedbacks, through: :workshops
 
   before_save :set_slug
-  after_update_commit :expire_chapters_sidebar_cache
-  after_create_commit :expire_chapters_sidebar_cache
-  after_destroy_commit :expire_chapters_sidebar_cache
 
   scope :active, -> { where(active: true) }
 
@@ -65,10 +62,6 @@ def members_for_group(name)
     members.where(groups: { name: }).distinct
   end
 
-  def expire_chapters_sidebar_cache
-    Rails.cache.delete('chapters-sidebar')
-  end
-
   def time_zone_exists
     return unless time_zone && ActiveSupport::TimeZone[time_zone].nil?
 
diff --git a/spec/models/chapter_spec.rb b/spec/models/chapter_spec.rb
index 4736591ef..6de8df507 100644
--- a/spec/models/chapter_spec.rb
+++ b/spec/models/chapter_spec.rb
@@ -44,30 +44,6 @@
     end
   end
 
-  context 'with cache expiration' do
-    let(:cache_key) { 'chapters-sidebar' }
-
-    it 'expires cache when chapter is created' do
-      Rails.cache.write(cache_key, 'cached content')
-      Fabricate(:chapter)
-      expect(Rails.cache.read(cache_key)).to be_nil
-    end
-
-    it 'expires cache when chapter is updated' do
-      Rails.cache.write(cache_key, 'cached content')
-      chapter = Fabricate(:chapter)
-      chapter.update!(name: 'Updated Name')
-      expect(Rails.cache.read(cache_key)).to be_nil
-    end
-
-    it 'expires cache when chapter is destroyed' do
-      Rails.cache.write(cache_key, 'cached content')
-      chapter = Fabricate(:chapter)
-      chapter.destroy
-      expect(Rails.cache.read(cache_key)).to be_nil
-    end
-  end
-
   describe '#students' do
     let(:chapter) { Fabricate(:chapter) }
     let(:student_group) { Fabricate(:group, chapter:, name: 'Students') }
diff --git a/spec/requests/chapters_sidebar_caching_spec.rb b/spec/requests/chapters_sidebar_caching_spec.rb
new file mode 100644
index 000000000..dc79e4055
--- /dev/null
+++ b/spec/requests/chapters_sidebar_caching_spec.rb
@@ -0,0 +1,67 @@
+require 'rails_helper'
+
+RSpec.describe 'Homepage chapters sidebar 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
+
+  before do
+    Fabricate(:chapter, name: 'Newcastle', slug: 'newcastle')
+    Fabricate(:chapter, name: 'London', slug: 'london')
+  end
+
+  def count_sidebar_queries
+    queries = 0
+    subscriber = ActiveSupport::Notifications.subscribe('sql.active_record') do |_name, _start, _finish, _id, payload|
+      queries += 1 if payload[:sql].match?(/"chapters"\."active"/)
+    end
+    yield
+    queries
+  ensure
+    ActiveSupport::Notifications.unsubscribe(subscriber)
+  end
+
+  it 'stores the sidebar as a fragment' do
+    get '/'
+
+    fragment_keys = fragment_store.instance_variable_get(:@data).keys
+    expect(fragment_keys.grep(/chapters-sidebar/)).to be_present
+  end
+
+  it 'runs no sidebar query on a warm fragment' do
+    get '/'
+
+    warm_queries = count_sidebar_queries { get '/' }
+
+    expect(response).to have_http_status(:ok)
+    expect(warm_queries).to be_zero
+  end
+
+  it 'drops a chapter deactivated by a direct database update within the TTL' do
+    get '/'
+    expect(response.body).to include('Newcastle')
+
+    Chapter.where(slug: 'newcastle').update_all(active: false)
+
+    get '/'
+    expect(response.body).to include('Newcastle')
+
+    travel_to(11.minutes.from_now) do
+      get '/'
+
+      expect(response.body).to include('London')
+      expect(response.body).not_to include('Newcastle')
+    end
+  end
+end