From 789f6231f7060c61b87992da9c3df0e831d192b8 Mon Sep 17 00:00:00 2001
From: eugielimpin <elimpin@gitlab.com>
Date: Wed, 2 Feb 2022 12:16:16 +0800
Subject: [PATCH 01/20] Create role_targeted_broadcast_messages feature flag

---
 .../development/role_targeted_broadcast_messages.yml      | 8 ++++++++
 1 file changed, 8 insertions(+)
 create mode 100644 config/feature_flags/development/role_targeted_broadcast_messages.yml

diff --git a/config/feature_flags/development/role_targeted_broadcast_messages.yml b/config/feature_flags/development/role_targeted_broadcast_messages.yml
new file mode 100644
index 00000000000000..723cab1abbb35b
--- /dev/null
+++ b/config/feature_flags/development/role_targeted_broadcast_messages.yml
@@ -0,0 +1,8 @@
+---
+name: role_targeted_broadcast_messages
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/77498
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/351736
+milestone: '14.8'
+type: development
+group: group::activation
+default_enabled: false
-- 
GitLab


From 4dd3fcc008cc46479bd9650ef2a933e2fd0f3487 Mon Sep 17 00:00:00 2001
From: eugielimpin <elimpin@gitlab.com>
Date: Fri, 28 Jan 2022 16:19:43 +0800
Subject: [PATCH 02/20] Allow broadcast messages to be targeted to the current
 user's role

Changelog: added
---
 app/models/broadcast_message.rb               | 38 +++++++++++++----
 ...get_access_levels_to_broadcast_messages.rb |  7 ++++
 db/schema_migrations/20220128081329           |  1 +
 db/structure.sql                              |  3 +-
 spec/models/broadcast_message_spec.rb         | 42 +++++++++++++++++--
 5 files changed, 79 insertions(+), 12 deletions(-)
 create mode 100644 db/migrate/20220128081329_add_target_access_levels_to_broadcast_messages.rb
 create mode 100644 db/schema_migrations/20220128081329

diff --git a/app/models/broadcast_message.rb b/app/models/broadcast_message.rb
index 1ee5c081840477..5b7703849595cc 100644
--- a/app/models/broadcast_message.rb
+++ b/app/models/broadcast_message.rb
@@ -4,12 +4,21 @@ class BroadcastMessage < ApplicationRecord
   include CacheMarkdownField
   include Sortable
 
+  ALLOWED_TARGET_ACCESS_LEVELS = [
+    Gitlab::Access::GUEST,
+    Gitlab::Access::REPORTER,
+    Gitlab::Access::DEVELOPER,
+    Gitlab::Access::MAINTAINER,
+    Gitlab::Access::OWNER
+  ].freeze
+
   cache_markdown_field :message, pipeline: :broadcast_message, whitelisted: true
 
   validates :message,   presence: true
   validates :starts_at, presence: true
   validates :ends_at,   presence: true
   validates :broadcast_type, presence: true
+  validates :target_access_levels, inclusion: { in: ALLOWED_TARGET_ACCESS_LEVELS }
 
   validates :color, allow_blank: true, color: true
   validates :font,  allow_blank: true, color: true
@@ -29,20 +38,20 @@ class BroadcastMessage < ApplicationRecord
   }
 
   class << self
-    def current_banner_messages(current_path = nil)
-      fetch_messages BANNER_CACHE_KEY, current_path do
+    def current_banner_messages(current_path = nil, user_access_level = nil)
+      fetch_messages BANNER_CACHE_KEY, current_path, user_access_level do
         current_and_future_messages.banner
       end
     end
 
-    def current_notification_messages(current_path = nil)
-      fetch_messages NOTIFICATION_CACHE_KEY, current_path do
+    def current_notification_messages(current_path = nil, user_access_level = nil)
+      fetch_messages NOTIFICATION_CACHE_KEY, current_path, user_access_level do
         current_and_future_messages.notification
       end
     end
 
-    def current(current_path = nil)
-      fetch_messages CACHE_KEY, current_path do
+    def current(current_path = nil, user_access_level = nil)
+      fetch_messages CACHE_KEY, current_path, user_access_level do
         current_and_future_messages
       end
     end
@@ -63,7 +72,7 @@ def cache_expires_in
 
     private
 
-    def fetch_messages(cache_key, current_path)
+    def fetch_messages(cache_key, current_path, user_access_level)
       messages = cache.fetch(cache_key, as: BroadcastMessage, expires_in: cache_expires_in) do
         yield
       end
@@ -74,7 +83,13 @@ def fetch_messages(cache_key, current_path)
       # displaying we'll refresh the cache so we don't need to keep filtering.
       cache.expire(cache_key) if now_or_future != messages
 
-      now_or_future.select(&:now?).select { |message| message.matches_current_path(current_path) }
+      messages = now_or_future.select(&:now?)
+      messages = messages.select do |message|
+        message.matches_current_path(current_path)
+      end
+      messages.select do |message|
+        message.matches_current_user_access_level(user_access_level)
+      end
     end
   end
 
@@ -102,6 +117,13 @@ def now_or_future?
     now? || future?
   end
 
+  def matches_current_user_access_level(user_access_level)
+    return true if target_access_levels.empty?
+    return false if user_access_level.blank?
+
+    target_access_levels.include? user_access_level
+  end
+
   def matches_current_path(current_path)
     return false if current_path.blank? && target_path.present?
     return true if current_path.blank? || target_path.blank?
diff --git a/db/migrate/20220128081329_add_target_access_levels_to_broadcast_messages.rb b/db/migrate/20220128081329_add_target_access_levels_to_broadcast_messages.rb
new file mode 100644
index 00000000000000..5958895ede8687
--- /dev/null
+++ b/db/migrate/20220128081329_add_target_access_levels_to_broadcast_messages.rb
@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class AddTargetAccessLevelsToBroadcastMessages < Gitlab::Database::Migration[1.0]
+  def change
+    add_column :broadcast_messages, :target_access_levels, :integer, array: true, null: false, default: []
+  end
+end
diff --git a/db/schema_migrations/20220128081329 b/db/schema_migrations/20220128081329
new file mode 100644
index 00000000000000..765b4c3a519282
--- /dev/null
+++ b/db/schema_migrations/20220128081329
@@ -0,0 +1 @@
+6e273d5b92595ae6054b0665b4ff446fb2bed24ff1aab122537833dc8f4d9ab8
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 34866dcff4cb31..c158165b44dde1 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -11364,7 +11364,8 @@ CREATE TABLE broadcast_messages (
     cached_markdown_version integer,
     target_path character varying(255),
     broadcast_type smallint DEFAULT 1 NOT NULL,
-    dismissable boolean
+    dismissable boolean,
+    target_access_levels integer[] DEFAULT '{}'::integer[] NOT NULL
 );
 
 CREATE SEQUENCE broadcast_messages_id_seq
diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb
index d981189c6f133d..8d9030ad5d1815 100644
--- a/spec/models/broadcast_message_spec.rb
+++ b/spec/models/broadcast_message_spec.rb
@@ -23,6 +23,8 @@
 
     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(BroadcastMessage::ALLOWED_TARGET_ACCESS_LEVELS) }
   end
 
   shared_examples 'time constrainted' do |broadcast_type|
@@ -175,12 +177,44 @@
     end
   end
 
+  shared_examples "matches with user access level" do |broadcast_type|
+    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(nil, nil)).to include(message)
+      end
+    end
+
+    context 'when target_access_levels is not empty' do
+      let_it_be(:target_access_levels) { [Gitlab::Access::GUEST] }
+      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(nil, nil)).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
+
   describe '.current', :use_clean_rails_memory_store_caching do
-    subject { -> (path = nil) { described_class.current(path) } }
+    subject { -> (path = nil, user_access_level = nil) { described_class.current(path, user_access_level) } }
 
     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 'returns both types' do
       banner_message = create(:broadcast_message, broadcast_type: :banner)
@@ -191,11 +225,12 @@
   end
 
   describe '.current_banner_messages', :use_clean_rails_memory_store_caching do
-    subject { -> (path = nil) { described_class.current_banner_messages(path) } }
+    subject { -> (path = nil, user_access_level = nil) { described_class.current_banner_messages(path, user_access_level) } }
 
     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 'only returns banners' do
       banner_message = create(:broadcast_message, broadcast_type: :banner)
@@ -206,11 +241,12 @@
   end
 
   describe '.current_notification_messages', :use_clean_rails_memory_store_caching do
-    subject { -> (path = nil) { described_class.current_notification_messages(path) } }
+    subject { -> (path = nil, user_access_level = nil) { described_class.current_notification_messages(path, user_access_level) } }
 
     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 'only returns notifications' do
       notification_message = create(:broadcast_message, broadcast_type: :notification)
-- 
GitLab


From a4bd0f7d2f693b4fdd3f17913821bd4a3883804d Mon Sep 17 00:00:00 2001
From: eugielimpin <elimpin@gitlab.com>
Date: Thu, 27 Jan 2022 15:47:21 +0800
Subject: [PATCH 03/20] Add role multi-select to broadcast messages form

---
 .../admin/broadcast_messages_controller.rb           |  2 +-
 app/helpers/broadcast_messages_helper.rb             |  6 ++++++
 app/views/admin/broadcast_messages/_form.html.haml   |  8 ++++++++
 locale/gitlab.pot                                    | 12 ++++++------
 4 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/app/controllers/admin/broadcast_messages_controller.rb b/app/controllers/admin/broadcast_messages_controller.rb
index 4660b0bfbb0920..ef843a84e6ccbb 100644
--- a/app/controllers/admin/broadcast_messages_controller.rb
+++ b/app/controllers/admin/broadcast_messages_controller.rb
@@ -65,6 +65,6 @@ def broadcast_message_params
       target_path
       broadcast_type
       dismissable
-    ))
+    ), target_access_levels: []).reverse_merge!(target_access_levels: [])
   end
 end
diff --git a/app/helpers/broadcast_messages_helper.rb b/app/helpers/broadcast_messages_helper.rb
index 881e11b10ea42b..e2e739398b0477 100644
--- a/app/helpers/broadcast_messages_helper.rb
+++ b/app/helpers/broadcast_messages_helper.rb
@@ -61,4 +61,10 @@ def render_broadcast_message(broadcast_message)
   def broadcast_type_options
     BroadcastMessage.broadcast_types.keys.map { |w| [w.humanize, w] }
   end
+
+  def target_access_level_options
+    BroadcastMessage::ALLOWED_TARGET_ACCESS_LEVELS.map do |access_level|
+      [Gitlab::Access.human_access(access_level), access_level]
+    end
+  end
 end
diff --git a/app/views/admin/broadcast_messages/_form.html.haml b/app/views/admin/broadcast_messages/_form.html.haml
index b68c22b6942bc0..814eee694a7cbd 100644
--- a/app/views/admin/broadcast_messages/_form.html.haml
+++ b/app/views/admin/broadcast_messages/_form.html.haml
@@ -55,6 +55,14 @@
       = f.check_box :dismissable
       = f.label :dismissable do
         = _('Allow users to dismiss the broadcast message')
+  - if Feature.enabled?(:role_targeted_broadcast_messages)
+    .form-group.row
+      .col-sm-2.col-form-label
+        = f.label :target_access_levels, _('Target roles')
+      .col-sm-10
+        = f.select :target_access_levels, target_access_level_options, { include_hidden: false }, multiple: true, class: 'form-control'
+        .form-text.text-muted
+          = _('The broadcast message displays only to users in projects and groups who have these roles.')
   .form-group.row.js-toggle-colors-container.toggle-colors.hide
     .col-sm-2.col-form-label
       = f.label :font, _("Font Color")
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 8769f0fcede5be..a0e7e4387355b3 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -28,15 +28,9 @@ msgstr ""
 msgid " Please sign in."
 msgstr ""
 
-msgid " Target Path"
-msgstr ""
-
 msgid " Try to %{action} this file again."
 msgstr ""
 
-msgid " Type"
-msgstr ""
-
 msgid " You need to do this before %{grace_period_deadline}."
 msgstr ""
 
@@ -35666,6 +35660,9 @@ msgstr ""
 msgid "Target branch"
 msgstr ""
 
+msgid "Target roles"
+msgstr ""
+
 msgid "Target-Branch"
 msgstr ""
 
@@ -36154,6 +36151,9 @@ msgstr ""
 msgid "The branch or tag does not exist"
 msgstr ""
 
+msgid "The broadcast message displays only to users in projects and groups who have these roles"
+msgstr ""
+
 msgid "The character highlighter helps you keep the subject line to %{titleLength} characters and wrap the body at %{bodyLength} so they are readable in git."
 msgstr ""
 
-- 
GitLab


From 3ff536b154a8010166c81e4bcdcf35e533ef3806 Mon Sep 17 00:00:00 2001
From: eugielimpin <elimpin@gitlab.com>
Date: Wed, 2 Feb 2022 14:18:03 +0800
Subject: [PATCH 04/20] Add Group.find_by_url method

---
 app/models/group.rb       | 13 +++++++++++++
 spec/models/group_spec.rb | 40 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/app/models/group.rb b/app/models/group.rb
index 53da70f47e558e..367576e5afba51 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -225,6 +225,19 @@ def get_ids_by_ids_or_paths(ids, paths)
       by_ids_or_paths(ids, paths).pluck(:id)
     end
 
+    def find_by_url(url)
+      match = Rails.application.routes.recognize_path(url)
+
+      group_id = match[:id] || match[:group_id]
+
+      return if match[:unmatched_route].present?
+      return if group_id.blank?
+
+      find_by_full_path(group_id)
+    rescue ActionController::RoutingError
+      nil
+    end
+
     private
 
     def public_to_user_arel(user)
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 4bc4df02c249e2..77dcc950f51e3a 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -2861,6 +2861,7 @@ def setup_group_members(group)
       expect(group.crm_enabled?).to be_truthy
     end
   end
+
   describe '.get_ids_by_ids_or_paths' do
     let(:group_path) { 'group_path' }
     let!(:group) { create(:group, path: group_path) }
@@ -2881,6 +2882,45 @@ def setup_group_members(group)
     end
   end
 
+  describe '.find_by_url' do
+    include Gitlab::Routing.url_helpers
+
+    subject { described_class.find_by_url(url) }
+
+    let_it_be(:group) { create(:group) }
+
+    context 'url is recognised as a group url' do
+      let(:url) { group_url(group) }
+
+      it { is_expected.to eq(group) }
+
+      context 'when id param is :group_id' do
+        let(:url) { group_settings_ci_cd_url(group) }
+
+        it { is_expected.to eq(group) }
+      end
+    end
+
+    context 'when path detection throws an error' do
+      let(:url) { 'https://example.com' }
+
+      before do
+        allow(Rails.application.routes).to receive(:recognize_path).with(url) { raise ActionController::RoutingError, 'test' }
+      end
+
+      it 'returns nil' do
+        expect { subject }.not_to raise_error(ActionController::RoutingError)
+        expect(subject).to be_nil
+      end
+    end
+
+    context 'url is not a group url' do
+      let(:url) { 'https://example.com/unknown' }
+
+      it { is_expected.to be_nil }
+    end
+  end
+
   describe '#shared_with_group_links_visible_to_user' do
     let_it_be(:admin) { create :admin }
     let_it_be(:normal_user) { create :user }
-- 
GitLab


From 84eba1f9c6188e89982f336ffad51712fe7336b1 Mon Sep 17 00:00:00 2001
From: eugielimpin <elimpin@gitlab.com>
Date: Wed, 2 Feb 2022 14:18:36 +0800
Subject: [PATCH 05/20] Update Project.find_by_url method

---
 app/models/project.rb       |  7 +++++--
 spec/models/project_spec.rb | 14 ++++++++++++--
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/app/models/project.rb b/app/models/project.rb
index 512c6ac1acb36d..d31df146ae8ada 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -847,10 +847,13 @@ def find_by_url(url)
 
       match = Rails.application.routes.recognize_path(url)
 
+      namespace_id = match[:namespace_id]
+      project_id = match[:id] || match[:project_id]
+
       return if match[:unmatched_route].present?
-      return if match[:namespace_id].blank? || match[:id].blank?
+      return if namespace_id.blank? || project_id.blank?
 
-      find_by_full_path(match.values_at(:namespace_id, :id).join("/"))
+      find_by_full_path([namespace_id, project_id].join("/"))
     rescue ActionController::RoutingError, URI::InvalidURIError
       nil
     end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index f24e5a42e95f28..aac8ab34f79169 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1971,6 +1971,8 @@ def has_external_wiki
   end
 
   describe '.find_by_url' do
+    include Gitlab::Routing.url_helpers
+
     subject { described_class.find_by_url(url) }
 
     let_it_be(:project) { create(:project) }
@@ -1980,13 +1982,21 @@ def has_external_wiki
     end
 
     context 'url is internal' do
-      let(:url) { "https://#{Gitlab.config.gitlab.host}/#{path}" }
+      let(:url) { "https://#{Gitlab.config.gitlab.host}#{path}" }
 
       context 'path is recognised as a project path' do
-        let(:path) { project.full_path }
+        let(:path) { project_path(project) }
 
         it { is_expected.to eq(project) }
 
+        context 'when project id param in path is :project_id' do
+          subject { described_class.find_by_url(url) }
+
+          let(:path) { project_ci_pipeline_editor_path(project) }
+
+          it { is_expected.to eq(project) }
+        end
+
         it 'returns nil if the path detection throws an error' do
           expect(Rails.application.routes).to receive(:recognize_path).with(url) { raise ActionController::RoutingError, 'test' }
 
-- 
GitLab


From 9b1e2bb4f4461b195b38d9dc498321653a5345fa Mon Sep 17 00:00:00 2001
From: eugielimpin <elimpin@gitlab.com>
Date: Tue, 1 Feb 2022 15:48:15 +0800
Subject: [PATCH 06/20] Fetch broadcast messages taking into account user's
 access level

---
 app/helpers/broadcast_messages_helper.rb      |  35 +++++-
 .../helpers/broadcast_messages_helper_spec.rb | 105 +++++++++++++++++-
 2 files changed, 133 insertions(+), 7 deletions(-)

diff --git a/app/helpers/broadcast_messages_helper.rb b/app/helpers/broadcast_messages_helper.rb
index e2e739398b0477..bbee7bf862adb2 100644
--- a/app/helpers/broadcast_messages_helper.rb
+++ b/app/helpers/broadcast_messages_helper.rb
@@ -1,14 +1,16 @@
 # frozen_string_literal: true
 
 module BroadcastMessagesHelper
+  include Gitlab::Utils::StrongMemoize
+
   def current_broadcast_banner_messages
-    BroadcastMessage.current_banner_messages(request.path).select do |message|
+    BroadcastMessage.current_banner_messages(request.path, user_access_level).select do |message|
       cookies["hide_broadcast_message_#{message.id}"].blank?
     end
   end
 
   def current_broadcast_notification_message
-    not_hidden_messages = BroadcastMessage.current_notification_messages(request.path).select do |message|
+    not_hidden_messages = BroadcastMessage.current_notification_messages(request.path, user_access_level).select do |message|
       cookies["hide_broadcast_message_#{message.id}"].blank?
     end
     not_hidden_messages.last
@@ -67,4 +69,33 @@ def target_access_level_options
       [Gitlab::Access.human_access(access_level), access_level]
     end
   end
+
+  private
+
+  # User's access level for the current project/group/sub-group page the user
+  # is currently in
+  def user_access_level
+    return if Feature.disabled?(:role_targeted_broadcast_messages)
+    return unless current_user.present?
+
+    strong_memoize(:user_access_level) do
+      if path_project
+        next path_project.team&.max_member_access(current_user.id)
+      end
+
+      path_group&.max_member_access_for_user(current_user)
+    end
+  end
+
+  def path_group
+    strong_memoize(:path_group) do
+      Group.find_by_url(request.url)
+    end
+  end
+
+  def path_project
+    strong_memoize(:path_project) do
+      Project.find_by_url(request.url)
+    end
+  end
 end
diff --git a/spec/helpers/broadcast_messages_helper_spec.rb b/spec/helpers/broadcast_messages_helper_spec.rb
index 3e8cbdf89a07ad..6c68e7a601f506 100644
--- a/spec/helpers/broadcast_messages_helper_spec.rb
+++ b/spec/helpers/broadcast_messages_helper_spec.rb
@@ -3,6 +3,91 @@
 require 'spec_helper'
 
 RSpec.describe BroadcastMessagesHelper do
+  include Gitlab::Routing.url_helpers
+
+  let_it_be(:user) { create(:user) }
+
+  before do
+    allow(helper).to receive(:current_user).and_return(user)
+  end
+
+  shared_examples 'returns role-targeted broadcast message when in project, group, or sub-group URL' do
+    let(:feature_flag_state) { true }
+
+    before do
+      stub_feature_flags(role_targeted_broadcast_messages: feature_flag_state)
+    end
+
+    context 'when in a project page' do
+      let(:project) { create(:project) }
+
+      before do
+        # Project.find_by_url checks that the host is equal to Gitlab.config.gitlab.host
+        stub_config_setting(host: 'test.host')
+
+        project.add_developer(user)
+        allow(helper).to receive(:request) { double(:Request, url: project_url(project), path: project_path(project)) }
+      end
+
+      it { is_expected.to eq message }
+
+      context 'when feature flag is disabled' do
+        let(:feature_flag_state) { false }
+
+        it { is_expected.to be_nil }
+      end
+    end
+
+    context 'when in a group page' do
+      let(:group) { create(:group) }
+
+      before do
+        group.add_developer(user)
+        allow(helper).to receive(:request) { double(:Request, url: group_url(group), path: group_path(group)) }
+      end
+
+      it { is_expected.to eq message }
+
+      context 'when feature flag is disabled' do
+        let(:feature_flag_state) { false }
+
+        it { is_expected.to be_nil }
+      end
+    end
+
+    context 'when in a sub-group page' do
+      let(:parent_group) { create(:group) }
+      let(:group) { create(:group, parent: parent_group) }
+
+      before do
+        group.add_developer(user)
+        allow(helper).to receive(:request) { double(:Request, url: group_url(group), path: group_path(group)) }
+      end
+
+      it { is_expected.to eq message }
+
+      context 'when feature flag is disabled' do
+        let(:feature_flag_state) { false }
+
+        it { is_expected.to be_nil }
+      end
+    end
+
+    context 'when not in a project, group, or sub-group page' do
+      before do
+        allow(helper).to receive(:request) { double(:Request, url: root_url, path: root_path) }
+      end
+
+      it { is_expected.to be_nil }
+
+      context 'when feature flag is disabled' do
+        let(:feature_flag_state) { false }
+
+        it { is_expected.to be_nil }
+      end
+    end
+  end
+
   describe 'current_broadcast_notification_message' do
     subject { helper.current_broadcast_notification_message }
 
@@ -24,16 +109,26 @@
     context 'without broadcast notification messages' do
       it { is_expected.to be_nil }
     end
+
+    describe 'user access level targeted messages' do
+      let!(:message) { create(:broadcast_message, broadcast_type: 'notification', starts_at: Time.now, target_access_levels: [Gitlab::Access::DEVELOPER]) }
+
+      include_examples 'returns role-targeted broadcast message when in project, group, or sub-group URL'
+    end
   end
 
-  describe 'broadcast_message' do
-    let_it_be(:user) { create(:user) }
+  describe 'current_broadcast_banner_messages' do
+    describe 'user access level targeted messages' do
+      let!(:message) { create(:broadcast_message, broadcast_type: 'banner', starts_at: Time.now, target_access_levels: [Gitlab::Access::DEVELOPER]) }
 
-    let(:current_broadcast_message) { BroadcastMessage.new(message: 'Current Message') }
+      subject { helper.current_broadcast_banner_messages.first }
 
-    before do
-      allow(helper).to receive(:current_user).and_return(user)
+      include_examples 'returns role-targeted broadcast message when in project, group, or sub-group URL'
     end
+  end
+
+  describe 'broadcast_message' do
+    let(:current_broadcast_message) { BroadcastMessage.new(message: 'Current Message') }
 
     it 'returns nil when no current message' do
       expect(helper.broadcast_message(nil)).to be_nil
-- 
GitLab


From da922200b742a25addce8c0dea0b6b2dfebefdd7 Mon Sep 17 00:00:00 2001
From: eugielimpin <elimpin@gitlab.com>
Date: Wed, 2 Feb 2022 14:20:41 +0800
Subject: [PATCH 07/20] Display broadcast messages target access levels value

---
 app/helpers/broadcast_messages_helper.rb      |  6 ++++
 .../admin/broadcast_messages/index.html.haml  | 12 +++++--
 locale/gitlab.pot                             |  8 ++---
 .../index.html.haml_spec.rb                   | 36 +++++++++++++++++++
 4 files changed, 55 insertions(+), 7 deletions(-)
 create mode 100644 spec/views/admin/broadcast_messages/index.html.haml_spec.rb

diff --git a/app/helpers/broadcast_messages_helper.rb b/app/helpers/broadcast_messages_helper.rb
index bbee7bf862adb2..241d563cdf55b1 100644
--- a/app/helpers/broadcast_messages_helper.rb
+++ b/app/helpers/broadcast_messages_helper.rb
@@ -70,6 +70,12 @@ def target_access_level_options
     end
   end
 
+  def target_access_levels_display(access_levels)
+    access_levels.map do |access_level|
+      Gitlab::Access.human_access(access_level)
+    end.join(', ')
+  end
+
   private
 
   # User's access level for the current project/group/sub-group page the user
diff --git a/app/views/admin/broadcast_messages/index.html.haml b/app/views/admin/broadcast_messages/index.html.haml
index 3f07bea78405c5..8b657eda0c0161 100644
--- a/app/views/admin/broadcast_messages/index.html.haml
+++ b/app/views/admin/broadcast_messages/index.html.haml
@@ -1,10 +1,11 @@
 - breadcrumb_title _("Messages")
 - page_title _("Broadcast Messages")
+- targeted_broadcast_messages_enabled = Feature.enabled?(:role_targeted_broadcast_messages)
 
 %h3.page-title
   = _('Broadcast Messages')
 %p.light
-  = _('Broadcast messages are displayed for every user and can be used to notify users about scheduled maintenance, recent upgrades and more.')
+  = _('Use banners and notifications to notify your users about scheduled maintenance, recent upgrades, and more.')
 
 = render 'form'
 
@@ -19,8 +20,10 @@
           %th= _('Preview')
           %th= _('Starts')
           %th= _('Ends')
-          %th= _(' Target Path')
-          %th= _(' Type')
+          - if targeted_broadcast_messages_enabled
+            %th= _('Target roles')
+          %th= _('Target Path')
+          %th= _('Type')
           %th &nbsp;
       %tbody
         - @broadcast_messages.each do |message|
@@ -33,6 +36,9 @@
               = message.starts_at
             %td
               = message.ends_at
+            - if targeted_broadcast_messages_enabled
+              %td
+                = target_access_levels_display(message.target_access_levels)
             %td
               = message.target_path
             %td
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index a0e7e4387355b3..76254c89a79d18 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -6112,9 +6112,6 @@ msgstr ""
 msgid "Broadcast Messages"
 msgstr ""
 
-msgid "Broadcast messages are displayed for every user and can be used to notify users about scheduled maintenance, recent upgrades and more."
-msgstr ""
-
 msgid "Browse Directory"
 msgstr ""
 
@@ -36151,7 +36148,7 @@ msgstr ""
 msgid "The branch or tag does not exist"
 msgstr ""
 
-msgid "The broadcast message displays only to users in projects and groups who have these roles"
+msgid "The broadcast message displays only to users in projects and groups who have these roles."
 msgstr ""
 
 msgid "The character highlighter helps you keep the subject line to %{titleLength} characters and wrap the body at %{bodyLength} so they are readable in git."
@@ -39399,6 +39396,9 @@ msgstr ""
 msgid "Use authorized_keys file to authenticate SSH keys"
 msgstr ""
 
+msgid "Use banners and notifications to notify your users about scheduled maintenance, recent upgrades, and more."
+msgstr ""
+
 msgid "Use cURL"
 msgstr ""
 
diff --git a/spec/views/admin/broadcast_messages/index.html.haml_spec.rb b/spec/views/admin/broadcast_messages/index.html.haml_spec.rb
new file mode 100644
index 00000000000000..00153f316ded13
--- /dev/null
+++ b/spec/views/admin/broadcast_messages/index.html.haml_spec.rb
@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'admin/broadcast_messages/index' do
+  describe 'Target roles select and table column' do
+    let(:feature_flag_state) { true }
+
+    let!(:message) { create(:broadcast_message, broadcast_type: 'banner', target_access_levels: [Gitlab::Access::GUEST, Gitlab::Access::DEVELOPER]) }
+
+    before do
+      assign(:broadcast_messages, BroadcastMessage.page(1))
+      assign(:broadcast_message, BroadcastMessage.new)
+
+      stub_feature_flags(role_targeted_broadcast_messages: feature_flag_state)
+
+      render
+    end
+
+    it 'rendered' do
+      expect(rendered).to have_content('Target roles')
+      expect(rendered).to have_content('Owner')
+      expect(rendered).to have_content('Guest, Developer')
+    end
+
+    context 'when feature flag is off' do
+      let(:feature_flag_state) { false }
+
+      it 'is not rendered' do
+        expect(rendered).not_to have_content('Target roles')
+        expect(rendered).not_to have_content('Owner')
+        expect(rendered).not_to have_content('Guest, Developer')
+      end
+    end
+  end
+end
-- 
GitLab


From fde303d522a308dd26d7f58507343a2d82245734 Mon Sep 17 00:00:00 2001
From: eugielimpin <elimpin@gitlab.com>
Date: Wed, 2 Feb 2022 14:21:32 +0800
Subject: [PATCH 08/20] Update broadcast_messages feature specs

---
 .../admin/admin_broadcast_messages_spec.rb    | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/spec/features/admin/admin_broadcast_messages_spec.rb b/spec/features/admin/admin_broadcast_messages_spec.rb
index 476dd4469bc633..aaa1f08c84fd6a 100644
--- a/spec/features/admin/admin_broadcast_messages_spec.rb
+++ b/spec/features/admin/admin_broadcast_messages_spec.rb
@@ -7,7 +7,12 @@
     admin = create(:admin)
     sign_in(admin)
     gitlab_enable_admin_mode_sign_in(admin)
-    create(:broadcast_message, :expired, message: 'Migration to new server')
+    create(
+      :broadcast_message,
+      :expired,
+      message: 'Migration to new server',
+      target_access_levels: [Gitlab::Access::DEVELOPER]
+    )
     visit admin_broadcast_messages_path
   end
 
@@ -21,10 +26,13 @@
     fill_in 'broadcast_message_target_path', with: '*/user_onboarded'
     fill_in 'broadcast_message_font', with: '#b94a48'
     select Date.today.next_year.year, from: 'broadcast_message_ends_at_1i'
+    select 'Guest', from: 'broadcast_message_target_access_levels'
+    select 'Owner', from: 'broadcast_message_target_access_levels'
     click_button 'Add broadcast message'
 
     expect(current_path).to eq admin_broadcast_messages_path
     expect(page).to have_content 'Application update from 4:00 CST to 5:00 CST'
+    expect(page).to have_content 'Guest, Owner'
     expect(page).to have_content '*/user_onboarded'
     expect(page).to have_selector 'strong', text: '4:00 CST to 5:00 CST'
     expect(page).to have_selector %(div[style="background-color: #f2dede; color: #b94a48"])
@@ -35,10 +43,14 @@
     fill_in 'broadcast_message_target_path', with: '*/user_onboarded'
     select 'Notification', from: 'broadcast_message_broadcast_type'
     select Date.today.next_year.year, from: 'broadcast_message_ends_at_1i'
+    select 'Reporter', from: 'broadcast_message_target_access_levels'
+    select 'Developer', from: 'broadcast_message_target_access_levels'
+    select 'Maintainer', from: 'broadcast_message_target_access_levels'
     click_button 'Add broadcast message'
 
     expect(current_path).to eq admin_broadcast_messages_path
     expect(page).to have_content 'Application update from 4:00 CST to 5:00 CST'
+    expect(page).to have_content 'Reporter, Developer, Maintainer'
     expect(page).to have_content '*/user_onboarded'
     expect(page).to have_content 'Notification'
     expect(page).to have_selector 'strong', text: '4:00 CST to 5:00 CST'
@@ -47,10 +59,15 @@
   it 'edit an existing broadcast message' do
     click_link 'Edit'
     fill_in 'broadcast_message_message', with: 'Application update RIGHT NOW'
+    select 'Reporter', from: 'broadcast_message_target_access_levels'
     click_button 'Update broadcast message'
 
     expect(current_path).to eq admin_broadcast_messages_path
     expect(page).to have_content 'Application update RIGHT NOW'
+
+    page.within('.table-responsive') do
+      expect(page).to have_content 'Reporter, Developer'
+    end
   end
 
   it 'remove an existing broadcast message' do
-- 
GitLab


From 80415d68bf59dfe9ba130fd4341390c72b92c1a1 Mon Sep 17 00:00:00 2001
From: eugielimpin <elimpin@gitlab.com>
Date: Thu, 10 Feb 2022 11:00:48 +0800
Subject: [PATCH 09/20] Rename method that returns a boolean to end with a
 question mark

---
 app/models/broadcast_message.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/models/broadcast_message.rb b/app/models/broadcast_message.rb
index 5b7703849595cc..334c2d25aec6ec 100644
--- a/app/models/broadcast_message.rb
+++ b/app/models/broadcast_message.rb
@@ -88,7 +88,7 @@ def fetch_messages(cache_key, current_path, user_access_level)
         message.matches_current_path(current_path)
       end
       messages.select do |message|
-        message.matches_current_user_access_level(user_access_level)
+        message.matches_current_user_access_level?(user_access_level)
       end
     end
   end
@@ -117,7 +117,7 @@ def now_or_future?
     now? || future?
   end
 
-  def matches_current_user_access_level(user_access_level)
+  def matches_current_user_access_level?(user_access_level)
     return true if target_access_levels.empty?
     return false if user_access_level.blank?
 
-- 
GitLab


From 03f65ca3940b8293e4599761ac9fb1e3dba52bb4 Mon Sep 17 00:00:00 2001
From: eugielimpin <elimpin@gitlab.com>
Date: Fri, 11 Feb 2022 13:14:34 +0800
Subject: [PATCH 10/20] Read feature flag default_enabled value from YAML
 definition file

---
 app/helpers/broadcast_messages_helper.rb           | 2 +-
 app/views/admin/broadcast_messages/_form.html.haml | 2 +-
 app/views/admin/broadcast_messages/index.html.haml | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/helpers/broadcast_messages_helper.rb b/app/helpers/broadcast_messages_helper.rb
index 241d563cdf55b1..6d4680f2fdc914 100644
--- a/app/helpers/broadcast_messages_helper.rb
+++ b/app/helpers/broadcast_messages_helper.rb
@@ -81,7 +81,7 @@ def target_access_levels_display(access_levels)
   # User's access level for the current project/group/sub-group page the user
   # is currently in
   def user_access_level
-    return if Feature.disabled?(:role_targeted_broadcast_messages)
+    return if Feature.disabled?(:role_targeted_broadcast_messages, default_enabled: :yaml)
     return unless current_user.present?
 
     strong_memoize(:user_access_level) do
diff --git a/app/views/admin/broadcast_messages/_form.html.haml b/app/views/admin/broadcast_messages/_form.html.haml
index 814eee694a7cbd..d81ebb8a2bbbe9 100644
--- a/app/views/admin/broadcast_messages/_form.html.haml
+++ b/app/views/admin/broadcast_messages/_form.html.haml
@@ -55,7 +55,7 @@
       = f.check_box :dismissable
       = f.label :dismissable do
         = _('Allow users to dismiss the broadcast message')
-  - if Feature.enabled?(:role_targeted_broadcast_messages)
+  - if Feature.enabled?(:role_targeted_broadcast_messages, default_enabled: :yaml)
     .form-group.row
       .col-sm-2.col-form-label
         = f.label :target_access_levels, _('Target roles')
diff --git a/app/views/admin/broadcast_messages/index.html.haml b/app/views/admin/broadcast_messages/index.html.haml
index 8b657eda0c0161..54c2a9d525015f 100644
--- a/app/views/admin/broadcast_messages/index.html.haml
+++ b/app/views/admin/broadcast_messages/index.html.haml
@@ -1,6 +1,6 @@
 - breadcrumb_title _("Messages")
 - page_title _("Broadcast Messages")
-- targeted_broadcast_messages_enabled = Feature.enabled?(:role_targeted_broadcast_messages)
+- targeted_broadcast_messages_enabled = Feature.enabled?(:role_targeted_broadcast_messages, default_enabled: :yaml)
 
 %h3.page-title
   = _('Broadcast Messages')
-- 
GitLab


From 570b0e6c9e0dd1733b917ecade74acdb3161dd8d Mon Sep 17 00:00:00 2001
From: eugielimpin <elimpin@gitlab.com>
Date: Fri, 11 Feb 2022 13:22:18 +0800
Subject: [PATCH 11/20] Remove redundant conditional

`target_access_levels.include? user_access_level` does the same thing as
`return false if user_access_level.blank?` since `target_access_levels`
is validate to guarantee not to include `nil`.
---
 app/models/broadcast_message.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/app/models/broadcast_message.rb b/app/models/broadcast_message.rb
index 334c2d25aec6ec..57914de65b9ccb 100644
--- a/app/models/broadcast_message.rb
+++ b/app/models/broadcast_message.rb
@@ -119,7 +119,6 @@ def now_or_future?
 
   def matches_current_user_access_level?(user_access_level)
     return true if target_access_levels.empty?
-    return false if user_access_level.blank?
 
     target_access_levels.include? user_access_level
   end
-- 
GitLab


From ca45dcbb8f0e29f1d2cbda2278215e87acc1304d Mon Sep 17 00:00:00 2001
From: eugielimpin <elimpin@gitlab.com>
Date: Fri, 11 Feb 2022 14:35:52 +0800
Subject: [PATCH 12/20] Change method signature to use keyword arguments

---
 app/helpers/broadcast_messages_helper.rb | 10 ++++++++--
 app/models/broadcast_message.rb          |  6 +++---
 app/services/post_receive_service.rb     |  2 +-
 spec/models/broadcast_message_spec.rb    | 18 +++++++++++++++---
 4 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/app/helpers/broadcast_messages_helper.rb b/app/helpers/broadcast_messages_helper.rb
index 6d4680f2fdc914..afe63f21d75325 100644
--- a/app/helpers/broadcast_messages_helper.rb
+++ b/app/helpers/broadcast_messages_helper.rb
@@ -4,13 +4,19 @@ module BroadcastMessagesHelper
   include Gitlab::Utils::StrongMemoize
 
   def current_broadcast_banner_messages
-    BroadcastMessage.current_banner_messages(request.path, user_access_level).select do |message|
+    BroadcastMessage.current_banner_messages(
+      current_path: request.path,
+      user_access_level: user_access_level
+    ).select do |message|
       cookies["hide_broadcast_message_#{message.id}"].blank?
     end
   end
 
   def current_broadcast_notification_message
-    not_hidden_messages = BroadcastMessage.current_notification_messages(request.path, user_access_level).select do |message|
+    not_hidden_messages = BroadcastMessage.current_notification_messages(
+      current_path: request.path,
+      user_access_level: user_access_level
+    ).select do |message|
       cookies["hide_broadcast_message_#{message.id}"].blank?
     end
     not_hidden_messages.last
diff --git a/app/models/broadcast_message.rb b/app/models/broadcast_message.rb
index 57914de65b9ccb..6cbf1e4e787fb9 100644
--- a/app/models/broadcast_message.rb
+++ b/app/models/broadcast_message.rb
@@ -38,19 +38,19 @@ class BroadcastMessage < ApplicationRecord
   }
 
   class << self
-    def current_banner_messages(current_path = nil, user_access_level = nil)
+    def current_banner_messages(current_path: nil, user_access_level: nil)
       fetch_messages BANNER_CACHE_KEY, current_path, user_access_level do
         current_and_future_messages.banner
       end
     end
 
-    def current_notification_messages(current_path = nil, user_access_level = nil)
+    def current_notification_messages(current_path: nil, user_access_level: nil)
       fetch_messages NOTIFICATION_CACHE_KEY, current_path, user_access_level do
         current_and_future_messages.notification
       end
     end
 
-    def current(current_path = nil, user_access_level = nil)
+    def current(current_path: nil, user_access_level: nil)
       fetch_messages CACHE_KEY, current_path, user_access_level do
         current_and_future_messages
       end
diff --git a/app/services/post_receive_service.rb b/app/services/post_receive_service.rb
index f5638b0aa40126..15c978e6763ada 100644
--- a/app/services/post_receive_service.rb
+++ b/app/services/post_receive_service.rb
@@ -86,7 +86,7 @@ def broadcast_message
     banner = nil
 
     if project
-      scoped_messages = BroadcastMessage.current_banner_messages(project.full_path).select do |message|
+      scoped_messages = BroadcastMessage.current_banner_messages(current_path: project.full_path).select do |message|
         message.target_path.present? && message.matches_current_path(project.full_path)
       end
 
diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb
index 8d9030ad5d1815..c29bf333067a0f 100644
--- a/spec/models/broadcast_message_spec.rb
+++ b/spec/models/broadcast_message_spec.rb
@@ -209,7 +209,11 @@
   end
 
   describe '.current', :use_clean_rails_memory_store_caching do
-    subject { -> (path = nil, user_access_level = nil) { described_class.current(path, user_access_level) } }
+    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
@@ -225,7 +229,11 @@
   end
 
   describe '.current_banner_messages', :use_clean_rails_memory_store_caching do
-    subject { -> (path = nil, user_access_level = nil) { described_class.current_banner_messages(path, user_access_level) } }
+    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
@@ -241,7 +249,11 @@
   end
 
   describe '.current_notification_messages', :use_clean_rails_memory_store_caching do
-    subject { -> (path = nil, user_access_level = nil) { described_class.current_notification_messages(path, user_access_level) } }
+    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
-- 
GitLab


From 38d2abd69cc929a6229db5e1b38334b023be2798 Mon Sep 17 00:00:00 2001
From: eugielimpin <elimpin@gitlab.com>
Date: Fri, 11 Feb 2022 14:51:53 +0800
Subject: [PATCH 13/20] Use select! (with bang) instead of select

---
 app/models/broadcast_message.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/models/broadcast_message.rb b/app/models/broadcast_message.rb
index 6cbf1e4e787fb9..3131ad2f0af6c8 100644
--- a/app/models/broadcast_message.rb
+++ b/app/models/broadcast_message.rb
@@ -84,10 +84,10 @@ def fetch_messages(cache_key, current_path, user_access_level)
       cache.expire(cache_key) if now_or_future != messages
 
       messages = now_or_future.select(&:now?)
-      messages = messages.select do |message|
+      messages.select! do |message|
         message.matches_current_path(current_path)
       end
-      messages.select do |message|
+      messages.select! do |message|
         message.matches_current_user_access_level?(user_access_level)
       end
     end
-- 
GitLab


From 3fee90a1de61371b91290da31dc58bbafb23e9a5 Mon Sep 17 00:00:00 2001
From: eugielimpin <elimpin@gitlab.com>
Date: Fri, 11 Feb 2022 14:53:48 +0800
Subject: [PATCH 14/20] Filter by user_access_level first

Regex operations made by matches_current_path are more expensive than
the Array#include? called by matches_current_user_access_level?
---
 app/models/broadcast_message.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/models/broadcast_message.rb b/app/models/broadcast_message.rb
index 3131ad2f0af6c8..1e5b804d665b21 100644
--- a/app/models/broadcast_message.rb
+++ b/app/models/broadcast_message.rb
@@ -85,10 +85,10 @@ def fetch_messages(cache_key, current_path, user_access_level)
 
       messages = now_or_future.select(&:now?)
       messages.select! do |message|
-        message.matches_current_path(current_path)
+        message.matches_current_user_access_level?(user_access_level)
       end
       messages.select! do |message|
-        message.matches_current_user_access_level?(user_access_level)
+        message.matches_current_path(current_path)
       end
     end
   end
-- 
GitLab


From f9e0734cafc83345d32199b0c228e1549ca8cebf Mon Sep 17 00:00:00 2001
From: eugielimpin <elimpin@gitlab.com>
Date: Fri, 11 Feb 2022 14:59:57 +0800
Subject: [PATCH 15/20] Make private method name more specific

Private methods in helpers are still accessible in views. To minimize
possibility of name clashing we rename the method to be very specific.
---
 app/helpers/broadcast_messages_helper.rb | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/app/helpers/broadcast_messages_helper.rb b/app/helpers/broadcast_messages_helper.rb
index afe63f21d75325..ddac6828bfdc51 100644
--- a/app/helpers/broadcast_messages_helper.rb
+++ b/app/helpers/broadcast_messages_helper.rb
@@ -6,7 +6,7 @@ module BroadcastMessagesHelper
   def current_broadcast_banner_messages
     BroadcastMessage.current_banner_messages(
       current_path: request.path,
-      user_access_level: user_access_level
+      user_access_level: current_user_access_level_for_project_or_group
     ).select do |message|
       cookies["hide_broadcast_message_#{message.id}"].blank?
     end
@@ -15,7 +15,7 @@ def current_broadcast_banner_messages
   def current_broadcast_notification_message
     not_hidden_messages = BroadcastMessage.current_notification_messages(
       current_path: request.path,
-      user_access_level: user_access_level
+      user_access_level: current_user_access_level_for_project_or_group
     ).select do |message|
       cookies["hide_broadcast_message_#{message.id}"].blank?
     end
@@ -84,13 +84,11 @@ def target_access_levels_display(access_levels)
 
   private
 
-  # User's access level for the current project/group/sub-group page the user
-  # is currently in
-  def user_access_level
+  def current_user_access_level_for_project_or_group
     return if Feature.disabled?(:role_targeted_broadcast_messages, default_enabled: :yaml)
     return unless current_user.present?
 
-    strong_memoize(:user_access_level) do
+    strong_memoize(:current_user_access_level_for_project_or_group) do
       if path_project
         next path_project.team&.max_member_access(current_user.id)
       end
-- 
GitLab


From 87d3e20c1320782847ac51d2fdb9160cec1637d5 Mon Sep 17 00:00:00 2001
From: eugielimpin <elimpin@gitlab.com>
Date: Fri, 11 Feb 2022 15:12:51 +0800
Subject: [PATCH 16/20] Revert to using select instead of select! (with bang)

select! (with bang) apparently returns nil if there were no changes made
to the array so we can't use it as the last method call.
---
 app/models/broadcast_message.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/app/models/broadcast_message.rb b/app/models/broadcast_message.rb
index 1e5b804d665b21..90fde5f83853b1 100644
--- a/app/models/broadcast_message.rb
+++ b/app/models/broadcast_message.rb
@@ -84,10 +84,10 @@ def fetch_messages(cache_key, current_path, user_access_level)
       cache.expire(cache_key) if now_or_future != messages
 
       messages = now_or_future.select(&:now?)
-      messages.select! do |message|
+      messages = messages.select do |message|
         message.matches_current_user_access_level?(user_access_level)
       end
-      messages.select! do |message|
+      messages.select do |message|
         message.matches_current_path(current_path)
       end
     end
-- 
GitLab


From b5c74e85a094843e795065a794c588622c8b0092 Mon Sep 17 00:00:00 2001
From: eugielimpin <elimpin@gitlab.com>
Date: Mon, 14 Feb 2022 17:59:20 +0800
Subject: [PATCH 17/20] Get project or group from Project:: or Groups::
 ApplicationController

---
 app/helpers/broadcast_messages_helper.rb      | 20 +++---------
 .../helpers/broadcast_messages_helper_spec.rb | 32 ++++---------------
 2 files changed, 10 insertions(+), 42 deletions(-)

diff --git a/app/helpers/broadcast_messages_helper.rb b/app/helpers/broadcast_messages_helper.rb
index ddac6828bfdc51..95e68c7e3cfc74 100644
--- a/app/helpers/broadcast_messages_helper.rb
+++ b/app/helpers/broadcast_messages_helper.rb
@@ -89,23 +89,11 @@ def current_user_access_level_for_project_or_group
     return unless current_user.present?
 
     strong_memoize(:current_user_access_level_for_project_or_group) do
-      if path_project
-        next path_project.team&.max_member_access(current_user.id)
+      if controller.is_a? Projects::ApplicationController
+        @project&.team&.max_member_access(current_user.id)
+      elsif controller.is_a? Groups::ApplicationController
+        @group&.max_member_access_for_user(current_user)
       end
-
-      path_group&.max_member_access_for_user(current_user)
-    end
-  end
-
-  def path_group
-    strong_memoize(:path_group) do
-      Group.find_by_url(request.url)
-    end
-  end
-
-  def path_project
-    strong_memoize(:path_project) do
-      Project.find_by_url(request.url)
     end
   end
 end
diff --git a/spec/helpers/broadcast_messages_helper_spec.rb b/spec/helpers/broadcast_messages_helper_spec.rb
index 6c68e7a601f506..4c3f08b85b4b44 100644
--- a/spec/helpers/broadcast_messages_helper_spec.rb
+++ b/spec/helpers/broadcast_messages_helper_spec.rb
@@ -16,17 +16,17 @@
 
     before do
       stub_feature_flags(role_targeted_broadcast_messages: feature_flag_state)
+      allow(helper).to receive(:cookies) { {} }
     end
 
     context 'when in a project page' do
       let(:project) { create(:project) }
 
       before do
-        # Project.find_by_url checks that the host is equal to Gitlab.config.gitlab.host
-        stub_config_setting(host: 'test.host')
-
         project.add_developer(user)
-        allow(helper).to receive(:request) { double(:Request, url: project_url(project), path: project_path(project)) }
+
+        assign(:project, project)
+        allow(helper).to receive(:controller) { ProjectsController.new }
       end
 
       it { is_expected.to eq message }
@@ -43,25 +43,9 @@
 
       before do
         group.add_developer(user)
-        allow(helper).to receive(:request) { double(:Request, url: group_url(group), path: group_path(group)) }
-      end
-
-      it { is_expected.to eq message }
-
-      context 'when feature flag is disabled' do
-        let(:feature_flag_state) { false }
-
-        it { is_expected.to be_nil }
-      end
-    end
 
-    context 'when in a sub-group page' do
-      let(:parent_group) { create(:group) }
-      let(:group) { create(:group, parent: parent_group) }
-
-      before do
-        group.add_developer(user)
-        allow(helper).to receive(:request) { double(:Request, url: group_url(group), path: group_path(group)) }
+        assign(:group, group)
+        allow(helper).to receive(:controller) { GroupsController.new }
       end
 
       it { is_expected.to eq message }
@@ -74,10 +58,6 @@
     end
 
     context 'when not in a project, group, or sub-group page' do
-      before do
-        allow(helper).to receive(:request) { double(:Request, url: root_url, path: root_path) }
-      end
-
       it { is_expected.to be_nil }
 
       context 'when feature flag is disabled' do
-- 
GitLab


From d295b2f6c1f6c6ce22b10bdde0c739d165e8c875 Mon Sep 17 00:00:00 2001
From: eugielimpin <elimpin@gitlab.com>
Date: Mon, 14 Feb 2022 17:56:37 +0800
Subject: [PATCH 18/20] Revert updates to Project.find_by_url

---
 app/models/project.rb       |  7 ++-----
 spec/models/project_spec.rb | 14 ++------------
 2 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/app/models/project.rb b/app/models/project.rb
index d31df146ae8ada..512c6ac1acb36d 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -847,13 +847,10 @@ def find_by_url(url)
 
       match = Rails.application.routes.recognize_path(url)
 
-      namespace_id = match[:namespace_id]
-      project_id = match[:id] || match[:project_id]
-
       return if match[:unmatched_route].present?
-      return if namespace_id.blank? || project_id.blank?
+      return if match[:namespace_id].blank? || match[:id].blank?
 
-      find_by_full_path([namespace_id, project_id].join("/"))
+      find_by_full_path(match.values_at(:namespace_id, :id).join("/"))
     rescue ActionController::RoutingError, URI::InvalidURIError
       nil
     end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index aac8ab34f79169..f24e5a42e95f28 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1971,8 +1971,6 @@ def has_external_wiki
   end
 
   describe '.find_by_url' do
-    include Gitlab::Routing.url_helpers
-
     subject { described_class.find_by_url(url) }
 
     let_it_be(:project) { create(:project) }
@@ -1982,21 +1980,13 @@ def has_external_wiki
     end
 
     context 'url is internal' do
-      let(:url) { "https://#{Gitlab.config.gitlab.host}#{path}" }
+      let(:url) { "https://#{Gitlab.config.gitlab.host}/#{path}" }
 
       context 'path is recognised as a project path' do
-        let(:path) { project_path(project) }
+        let(:path) { project.full_path }
 
         it { is_expected.to eq(project) }
 
-        context 'when project id param in path is :project_id' do
-          subject { described_class.find_by_url(url) }
-
-          let(:path) { project_ci_pipeline_editor_path(project) }
-
-          it { is_expected.to eq(project) }
-        end
-
         it 'returns nil if the path detection throws an error' do
           expect(Rails.application.routes).to receive(:recognize_path).with(url) { raise ActionController::RoutingError, 'test' }
 
-- 
GitLab


From 15a6393d29547539e8938d84a61a1d852a7147af Mon Sep 17 00:00:00 2001
From: eugielimpin <elimpin@gitlab.com>
Date: Mon, 14 Feb 2022 17:57:24 +0800
Subject: [PATCH 19/20] Remove Group.find_by_url class method

---
 app/models/group.rb       | 13 -------------
 spec/models/group_spec.rb | 40 ---------------------------------------
 2 files changed, 53 deletions(-)

diff --git a/app/models/group.rb b/app/models/group.rb
index 367576e5afba51..53da70f47e558e 100644
--- a/app/models/group.rb
+++ b/app/models/group.rb
@@ -225,19 +225,6 @@ def get_ids_by_ids_or_paths(ids, paths)
       by_ids_or_paths(ids, paths).pluck(:id)
     end
 
-    def find_by_url(url)
-      match = Rails.application.routes.recognize_path(url)
-
-      group_id = match[:id] || match[:group_id]
-
-      return if match[:unmatched_route].present?
-      return if group_id.blank?
-
-      find_by_full_path(group_id)
-    rescue ActionController::RoutingError
-      nil
-    end
-
     private
 
     def public_to_user_arel(user)
diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb
index 77dcc950f51e3a..4bc4df02c249e2 100644
--- a/spec/models/group_spec.rb
+++ b/spec/models/group_spec.rb
@@ -2861,7 +2861,6 @@ def setup_group_members(group)
       expect(group.crm_enabled?).to be_truthy
     end
   end
-
   describe '.get_ids_by_ids_or_paths' do
     let(:group_path) { 'group_path' }
     let!(:group) { create(:group, path: group_path) }
@@ -2882,45 +2881,6 @@ def setup_group_members(group)
     end
   end
 
-  describe '.find_by_url' do
-    include Gitlab::Routing.url_helpers
-
-    subject { described_class.find_by_url(url) }
-
-    let_it_be(:group) { create(:group) }
-
-    context 'url is recognised as a group url' do
-      let(:url) { group_url(group) }
-
-      it { is_expected.to eq(group) }
-
-      context 'when id param is :group_id' do
-        let(:url) { group_settings_ci_cd_url(group) }
-
-        it { is_expected.to eq(group) }
-      end
-    end
-
-    context 'when path detection throws an error' do
-      let(:url) { 'https://example.com' }
-
-      before do
-        allow(Rails.application.routes).to receive(:recognize_path).with(url) { raise ActionController::RoutingError, 'test' }
-      end
-
-      it 'returns nil' do
-        expect { subject }.not_to raise_error(ActionController::RoutingError)
-        expect(subject).to be_nil
-      end
-    end
-
-    context 'url is not a group url' do
-      let(:url) { 'https://example.com/unknown' }
-
-      it { is_expected.to be_nil }
-    end
-  end
-
   describe '#shared_with_group_links_visible_to_user' do
     let_it_be(:admin) { create :admin }
     let_it_be(:normal_user) { create :user }
-- 
GitLab


From d893fcc895a62a02a3cb254337fd9be0ba1254a3 Mon Sep 17 00:00:00 2001
From: Mark Chao <mchao@gitlab.com>
Date: Tue, 15 Feb 2022 03:13:16 +0000
Subject: [PATCH 20/20] Use let_it_be to reduce DB query

---
 spec/helpers/broadcast_messages_helper_spec.rb            | 8 ++++----
 spec/models/broadcast_message_spec.rb                     | 2 +-
 .../admin/broadcast_messages/index.html.haml_spec.rb      | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/spec/helpers/broadcast_messages_helper_spec.rb b/spec/helpers/broadcast_messages_helper_spec.rb
index 4c3f08b85b4b44..e721a3fdc9573c 100644
--- a/spec/helpers/broadcast_messages_helper_spec.rb
+++ b/spec/helpers/broadcast_messages_helper_spec.rb
@@ -20,7 +20,7 @@
     end
 
     context 'when in a project page' do
-      let(:project) { create(:project) }
+      let_it_be(:project) { create(:project) }
 
       before do
         project.add_developer(user)
@@ -39,7 +39,7 @@
     end
 
     context 'when in a group page' do
-      let(:group) { create(:group) }
+      let_it_be(:group) { create(:group) }
 
       before do
         group.add_developer(user)
@@ -91,7 +91,7 @@
     end
 
     describe 'user access level targeted messages' do
-      let!(:message) { create(:broadcast_message, broadcast_type: 'notification', starts_at: Time.now, target_access_levels: [Gitlab::Access::DEVELOPER]) }
+      let_it_be(:message) { create(:broadcast_message, broadcast_type: 'notification', starts_at: Time.now, target_access_levels: [Gitlab::Access::DEVELOPER]) }
 
       include_examples 'returns role-targeted broadcast message when in project, group, or sub-group URL'
     end
@@ -99,7 +99,7 @@
 
   describe 'current_broadcast_banner_messages' do
     describe 'user access level targeted messages' do
-      let!(:message) { create(:broadcast_message, broadcast_type: 'banner', starts_at: Time.now, target_access_levels: [Gitlab::Access::DEVELOPER]) }
+      let_it_be(:message) { create(:broadcast_message, broadcast_type: 'banner', starts_at: Time.now, target_access_levels: [Gitlab::Access::DEVELOPER]) }
 
       subject { helper.current_broadcast_banner_messages.first }
 
diff --git a/spec/models/broadcast_message_spec.rb b/spec/models/broadcast_message_spec.rb
index c29bf333067a0f..3a072cfe2ecb85 100644
--- a/spec/models/broadcast_message_spec.rb
+++ b/spec/models/broadcast_message_spec.rb
@@ -24,7 +24,7 @@
     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(BroadcastMessage::ALLOWED_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|
diff --git a/spec/views/admin/broadcast_messages/index.html.haml_spec.rb b/spec/views/admin/broadcast_messages/index.html.haml_spec.rb
index 00153f316ded13..e1dc76428df6fd 100644
--- a/spec/views/admin/broadcast_messages/index.html.haml_spec.rb
+++ b/spec/views/admin/broadcast_messages/index.html.haml_spec.rb
@@ -6,7 +6,7 @@
   describe 'Target roles select and table column' do
     let(:feature_flag_state) { true }
 
-    let!(:message) { create(:broadcast_message, broadcast_type: 'banner', target_access_levels: [Gitlab::Access::GUEST, Gitlab::Access::DEVELOPER]) }
+    let_it_be(:message) { create(:broadcast_message, broadcast_type: 'banner', target_access_levels: [Gitlab::Access::GUEST, Gitlab::Access::DEVELOPER]) }
 
     before do
       assign(:broadcast_messages, BroadcastMessage.page(1))
-- 
GitLab