diff --git a/app/models/broadcast_message.rb b/app/models/broadcast_message.rb index 3692bcc680d..fdc5a2adea0 100644 --- a/app/models/broadcast_message.rb +++ b/app/models/broadcast_message.rb @@ -19,11 +19,21 @@ class BroadcastMessage < ActiveRecord::Base after_commit :flush_redis_cache def self.current - Rails.cache.fetch(CACHE_KEY) do - where('ends_at > :now AND starts_at <= :now', now: Time.zone.now) - .reorder(id: :asc) - .to_a - end + messages = Rails.cache.fetch(CACHE_KEY) { current_and_future_messages.to_a } + + return messages if messages.empty? + + 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 def active? @@ -38,6 +48,18 @@ class BroadcastMessage < ActiveRecord::Base ends_at < Time.zone.now 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 Rails.cache.delete(CACHE_KEY) end diff --git a/changelogs/unreleased/fix-broadcast-message-caching.yml b/changelogs/unreleased/fix-broadcast-message-caching.yml new file mode 100644 index 00000000000..58ec1766cfd --- /dev/null +++ b/changelogs/unreleased/fix-broadcast-message-caching.yml @@ -0,0 +1,5 @@ +--- +title: Fix caching of future broadcast messages +merge_request: +author: +type: fixed diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb index 3369aef1d3e..461e754dc1f 100644 --- a/spec/models/broadcast_message_spec.rb +++ b/spec/models/broadcast_message_spec.rb @@ -53,6 +53,29 @@ describe BroadcastMessage do 2.times { described_class.current } 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 describe '#active?' do