diff options
Diffstat (limited to 'spec/models/broadcast_message_spec.rb')
-rw-r--r-- | spec/models/broadcast_message_spec.rb | 150 |
1 files changed, 141 insertions, 9 deletions
diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb index d981189c6f1..b0bfdabe13c 100644 --- a/spec/models/broadcast_message_spec.rb +++ b/spec/models/broadcast_message_spec.rb @@ -23,6 +23,8 @@ RSpec.describe BroadcastMessage do it { is_expected.to allow_value(1).for(:broadcast_type) } it { is_expected.not_to allow_value(nil).for(:broadcast_type) } + it { is_expected.not_to allow_value(nil).for(:target_access_levels) } + it { is_expected.to validate_inclusion_of(:target_access_levels).in_array(described_class::ALLOWED_TARGET_ACCESS_LEVELS) } end shared_examples 'time constrainted' do |broadcast_type| @@ -60,7 +62,7 @@ RSpec.describe BroadcastMessage do subject.call - Timecop.travel(3.weeks) do + travel_to(3.weeks.from_now) do subject.call end end @@ -71,7 +73,7 @@ RSpec.describe BroadcastMessage do expect(subject.call).to match_array([message]) expect(described_class.cache).to receive(:expire).and_call_original - Timecop.travel(1.week) do + travel_to(1.week.from_now) do 2.times { expect(subject.call).to be_empty } end end @@ -94,7 +96,7 @@ RSpec.describe BroadcastMessage do expect(subject.call.length).to eq(1) - Timecop.travel(future.starts_at) do + travel_to(future.starts_at + 1.second) do expect(subject.call.length).to eq(2) end end @@ -175,12 +177,112 @@ RSpec.describe BroadcastMessage do end end + shared_examples "matches with user access level" do |broadcast_type| + let_it_be(:target_access_levels) { [Gitlab::Access::GUEST] } + + let(:feature_flag_state) { true } + + before do + stub_feature_flags(role_targeted_broadcast_messages: feature_flag_state) + end + + context 'when feature flag is disabled' do + let(:feature_flag_state) { false } + + context 'when message is role-targeted' do + let_it_be(:message) { create(:broadcast_message, target_access_levels: target_access_levels, broadcast_type: broadcast_type) } + + it 'does not return the message' do + expect(subject.call(nil, Gitlab::Access::GUEST)).to be_empty + end + end + + context 'when message is not role-targeted' do + let_it_be(:message) { create(:broadcast_message, target_access_levels: [], broadcast_type: broadcast_type) } + + it 'returns the message' do + expect(subject.call(nil, Gitlab::Access::GUEST)).to include(message) + end + end + end + + context 'when target_access_levels is empty' do + let_it_be(:message) { create(:broadcast_message, target_access_levels: [], broadcast_type: broadcast_type) } + + it 'returns the message if user access level is not nil' do + expect(subject.call(nil, Gitlab::Access::MINIMAL_ACCESS)).to include(message) + end + + it 'returns the message if user access level is nil' do + expect(subject.call).to include(message) + end + end + + context 'when target_access_levels is not empty' do + let_it_be(:message) { create(:broadcast_message, target_access_levels: target_access_levels, broadcast_type: broadcast_type) } + + it "does not return the message if user access level is nil" do + expect(subject.call).to be_empty + end + + it "returns the message if user access level is in target_access_levels" do + expect(subject.call(nil, Gitlab::Access::GUEST)).to include(message) + end + + it "does not return the message if user access level is not in target_access_levels" do + expect(subject.call(nil, Gitlab::Access::MINIMAL_ACCESS)).to be_empty + end + end + end + + shared_examples "handles stale cache data gracefully" do + # Regression test for https://gitlab.com/gitlab-org/gitlab/-/issues/353076 + context 'when cache returns stale data (e.g. nil target_access_levels)' do + let(:message) { build(:broadcast_message, :banner, target_access_levels: nil) } + let(:cache) { Gitlab::JsonCache.new } + + before do + cache.write(described_class::BANNER_CACHE_KEY, [message]) + allow(BroadcastMessage).to receive(:cache) { cache } + end + + it 'does not raise error (e.g. NoMethodError from nil.empty?)' do + expect { subject.call }.not_to raise_error + end + + context 'when feature flag is disabled' do + it 'does not raise error (e.g. NoMethodError from nil.empty?)' do + stub_feature_flags(role_targeted_broadcast_messages: false) + + expect { subject.call }.not_to raise_error + end + end + end + end + describe '.current', :use_clean_rails_memory_store_caching do - subject { -> (path = nil) { described_class.current(path) } } + subject do + -> (path = nil, user_access_level = nil) do + described_class.current(current_path: path, user_access_level: user_access_level) + end + end it_behaves_like 'time constrainted', :banner it_behaves_like 'message cache', :banner it_behaves_like 'matches with current path', :banner + it_behaves_like 'matches with user access level', :banner + it_behaves_like 'handles stale cache data gracefully' + + context 'when message is from cache' do + before do + subject.call + end + + it_behaves_like 'matches with current path', :banner + it_behaves_like 'matches with user access level', :banner + it_behaves_like 'matches with current path', :notification + it_behaves_like 'matches with user access level', :notification + end it 'returns both types' do banner_message = create(:broadcast_message, broadcast_type: :banner) @@ -191,11 +293,26 @@ RSpec.describe BroadcastMessage do end describe '.current_banner_messages', :use_clean_rails_memory_store_caching do - subject { -> (path = nil) { described_class.current_banner_messages(path) } } + subject do + -> (path = nil, user_access_level = nil) do + described_class.current_banner_messages(current_path: path, user_access_level: user_access_level) + end + end it_behaves_like 'time constrainted', :banner it_behaves_like 'message cache', :banner it_behaves_like 'matches with current path', :banner + it_behaves_like 'matches with user access level', :banner + it_behaves_like 'handles stale cache data gracefully' + + context 'when message is from cache' do + before do + subject.call + end + + it_behaves_like 'matches with current path', :banner + it_behaves_like 'matches with user access level', :banner + end it 'only returns banners' do banner_message = create(:broadcast_message, broadcast_type: :banner) @@ -206,11 +323,26 @@ RSpec.describe BroadcastMessage do end describe '.current_notification_messages', :use_clean_rails_memory_store_caching do - subject { -> (path = nil) { described_class.current_notification_messages(path) } } + subject do + -> (path = nil, user_access_level = nil) do + described_class.current_notification_messages(current_path: path, user_access_level: user_access_level) + end + end it_behaves_like 'time constrainted', :notification it_behaves_like 'message cache', :notification it_behaves_like 'matches with current path', :notification + it_behaves_like 'matches with user access level', :notification + it_behaves_like 'handles stale cache data gracefully' + + context 'when message is from cache' do + before do + subject.call + end + + it_behaves_like 'matches with current path', :notification + it_behaves_like 'matches with user access level', :notification + end it 'only returns notifications' do notification_message = create(:broadcast_message, broadcast_type: :notification) @@ -286,9 +418,9 @@ RSpec.describe BroadcastMessage do it 'flushes the Redis cache' do message = create(:broadcast_message) - expect(Rails.cache).to receive(:delete).with(described_class::CACHE_KEY) - expect(Rails.cache).to receive(:delete).with(described_class::BANNER_CACHE_KEY) - expect(Rails.cache).to receive(:delete).with(described_class::NOTIFICATION_CACHE_KEY) + expect(Rails.cache).to receive(:delete).with("#{described_class::CACHE_KEY}:#{Gitlab.revision}") + expect(Rails.cache).to receive(:delete).with("#{described_class::BANNER_CACHE_KEY}:#{Gitlab.revision}") + expect(Rails.cache).to receive(:delete).with("#{described_class::NOTIFICATION_CACHE_KEY}:#{Gitlab.revision}") message.flush_redis_cache end |