Commit e6f20e52 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'fix-broadcast-message-caching' into 'master'

Fix caching of future broadcast messages

Closes #36661

See merge request !13667
parents dd0681d7 e0b589f1
...@@ -19,11 +19,21 @@ class BroadcastMessage < ActiveRecord::Base ...@@ -19,11 +19,21 @@ class BroadcastMessage < ActiveRecord::Base
after_commit :flush_redis_cache after_commit :flush_redis_cache
def self.current def self.current
Rails.cache.fetch(CACHE_KEY) do messages = Rails.cache.fetch(CACHE_KEY) { current_and_future_messages.to_a }
where('ends_at > :now AND starts_at <= :now', now: Time.zone.now)
.reorder(id: :asc) return messages if messages.empty?
.to_a
end now_or_future = messages.select(&:now_or_future?)
# If there are cached entries but none are to be displayed we'll purge the
# cache so we don't keep running this code all the time.
Rails.cache.delete(CACHE_KEY) if now_or_future.empty?
now_or_future.select(&:now?)
end
def self.current_and_future_messages
where('ends_at > :now', now: Time.zone.now).reorder(id: :asc)
end end
def active? def active?
...@@ -38,6 +48,18 @@ class BroadcastMessage < ActiveRecord::Base ...@@ -38,6 +48,18 @@ class BroadcastMessage < ActiveRecord::Base
ends_at < Time.zone.now ends_at < Time.zone.now
end end
def now?
(starts_at..ends_at).cover?(Time.zone.now)
end
def future?
starts_at > Time.zone.now
end
def now_or_future?
now? || future?
end
def flush_redis_cache def flush_redis_cache
Rails.cache.delete(CACHE_KEY) Rails.cache.delete(CACHE_KEY)
end end
......
---
title: Fix caching of future broadcast messages
merge_request:
author:
type: fixed
...@@ -53,6 +53,29 @@ describe BroadcastMessage do ...@@ -53,6 +53,29 @@ describe BroadcastMessage do
2.times { described_class.current } 2.times { described_class.current }
end end
it 'includes messages that need to be displayed in the future' do
create(:broadcast_message)
future = create(
:broadcast_message,
starts_at: Time.now + 10.minutes,
ends_at: Time.now + 20.minutes
)
expect(described_class.current.length).to eq(1)
Timecop.travel(future.starts_at) do
expect(described_class.current.length).to eq(2)
end
end
it 'does not clear the cache if only a future message should be displayed' do
create(:broadcast_message, :future)
expect(Rails.cache).not_to receive(:delete)
expect(described_class.current.length).to eq(0)
end
end end
describe '#active?' do describe '#active?' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment