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 %>
<%= title %>
<% 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