Merge branch 'move-alerting-to-core-move-alert-bot' into 'master'

Move User.alert_bot out of EE

Closes #200006

See merge request gitlab-org/gitlab!23867

app/models/user.rb

@@ -59,6 +59,8 @@ class User < ApplicationRecord
 
   MINIMUM_INACTIVE_DAYS = 180
 
+  enum bot_type: ::UserBotTypeEnums.bots
+
   # Override Devise::Models::Trackable#update_tracked_fields!
   # to limit database writes to at most once every hour
   # rubocop: disable CodeReuse/ServiceClass
@@ -322,6 +324,8 @@ class User < ApplicationRecord
   scope :with_emails, -> { preload(:emails) }
   scope :with_dashboard, -> (dashboard) { where(dashboard: dashboard) }
   scope :with_public_profile, -> { where(private_profile: false) }
+  scope :bots, -> { where.not(bot_type: nil) }
+  scope :humans, -> { where(bot_type: nil) }
 
   scope :with_expiring_and_not_notified_personal_access_tokens, ->(at) do
     where('EXISTS (?)',
@@ -598,6 +602,15 @@ class User < ApplicationRecord
       end
     end
 
+    def alert_bot
+      email_pattern = "alert%s@#{Settings.gitlab.host}"
+
+      unique_internal(where(bot_type: :alert_bot), 'alert-bot', email_pattern) do |u|
+        u.bio = 'The GitLab alert bot'
+        u.name = 'GitLab Alert Bot'
+      end
+    end
+
     # Return true if there is only single non-internal user in the deployment,
     # ghost user is ignored.
     def single_user?
@@ -613,16 +626,20 @@ class User < ApplicationRecord
     username
   end
 
+  def bot?
+    bot_type.present?
+  end
+
   def internal?
-    ghost?
+    ghost? || bot?
   end
 
   def self.internal
-    where(ghost: true)
+    where(ghost: true).or(bots)
   end
 
   def self.non_internal
-    without_ghosts
+    without_ghosts.humans
   end
 
   #

app/models/user_bot_type_enums.rb

+# frozen_string_literal: true
+
+module UserBotTypeEnums
+  def self.bots
+    # When adding a new key, please ensure you are not conflicting with EE-only keys in app/models/user_bot_types_enums.rb
+    {
+      alert_bot: 2
+    }
+  end
+end
+
+UserBotTypeEnums.prepend_if_ee('EE::UserBotTypeEnums')

app/policies/base_policy.rb

@@ -44,6 +44,9 @@ class BasePolicy < DeclarativePolicy::Base
     ::Gitlab::ExternalAuthorization.perform_check?
   end
 
+  with_options scope: :user, score: 0
+  condition(:alert_bot) { @user&.alert_bot? }
+
   rule { external_authorization_enabled & ~can?(:read_all_resources) }.policy do
     prevent :read_cross_project
   end

app/policies/concerns/policy_actor.rb

@@ -33,6 +33,10 @@ module PolicyActor
   def can_create_group
     false
   end
+
+  def alert_bot?
+    false
+  end
 end
 
 PolicyActor.prepend_if_ee('EE::PolicyActor')

app/policies/project_policy.rb

@@ -515,6 +515,8 @@ class ProjectPolicy < BasePolicy
   end
 
   def lookup_access_level!
+    return ::Gitlab::Access::REPORTER if alert_bot?
+
     # NOTE: max_member_access has its own cache
     project.team.max_member_access(@user.id)
   end

ee/app/models/ee/user.rb

@@ -70,9 +70,6 @@ module EE
         joins(:identities).where(identities: { provider: provider })
       end
 
-      scope :bots, -> { where.not(bot_type: nil) }
-      scope :humans, -> { where(bot_type: nil) }
-
       scope :with_invalid_expires_at_tokens, ->(expiration_date) do
         where(id: ::PersonalAccessToken.with_invalid_expires_at(expiration_date).select(:user_id))
       end
@@ -85,12 +82,6 @@ module EE
       # Note: When adding an option, it's value MUST equal to the last value + 1.
       enum group_view: { details: 1, security_dashboard: 2 }, _prefix: true
       scope :group_view_details, -> { where('group_view = ? OR group_view IS NULL', group_view[:details]) }
-
-      enum bot_type: {
-        support_bot: 1,
-        alert_bot: 2,
-        visual_review_bot: 3
-      }
     end
 
     class_methods do
@@ -105,15 +96,6 @@ module EE
         end
       end
 
-      def alert_bot
-        email_pattern = "alert%s@#{Settings.gitlab.host}"
-
-        unique_internal(where(bot_type: :alert_bot), 'alert-bot', email_pattern) do |u|
-          u.bio = 'The GitLab alert bot'
-          u.name = 'GitLab Alert Bot'
-        end
-      end
-
       def visual_review_bot
         email_pattern = "visual_review%s@#{Settings.gitlab.host}"
 
@@ -123,16 +105,6 @@ module EE
         end
       end
 
-      override :internal
-      def internal
-        super.or(bots)
-      end
-
-      override :non_internal
-      def non_internal
-        super.humans
-      end
-
       def non_ldap
         joins('LEFT JOIN identities ON identities.user_id = users.id')
           .where('identities.provider IS NULL OR identities.provider NOT LIKE ?', 'ldap%')
@@ -334,18 +306,6 @@ module EE
       super
     end
 
-    override :internal?
-    def internal?
-      super || bot?
-    end
-
-    def bot?
-      return bot_type.present? if has_attribute?(:bot_type)
-
-      # Some older *migration* specs utilize this removed column
-      read_attribute(:support_bot)
-    end
-
     protected
 
     override :password_required?

ee/app/models/ee/user_bot_type_enums.rb

+# frozen_string_literal: true
+
+module EE
+  module UserBotTypeEnums
+    extend ActiveSupport::Concern
+
+    class_methods do
+      extend ::Gitlab::Utils::Override
+
+      override :bots
+      def bots
+        # When adding a new key, please ensure you are not redefining a key that already exists in app/models/user_bot_types_enums.rb
+        super.merge(
+          support_bot: 1,
+          visual_review_bot: 3
+        )
+      end
+    end
+  end
+end

ee/app/policies/ee/base_policy.rb

@@ -11,9 +11,6 @@ module EE
       with_scope :user
       condition(:support_bot, score: 0) { @user&.support_bot? }
 
-      with_scope :user
-      condition(:alert_bot, score: 0) { @user&.alert_bot? }
-
       with_scope :user
       condition(:visual_review_bot, score: 0) { @user&.visual_review_bot? }
 

ee/app/policies/ee/policy_actor.rb

@@ -10,10 +10,6 @@ module EE
       false
     end
 
-    def alert_bot?
-      false
-    end
-
     def visual_review_bot?
       false
     end

ee/app/policies/ee/project_policy.rb

@@ -316,7 +316,6 @@ module EE
     override :lookup_access_level!
     def lookup_access_level!
       return ::Gitlab::Access::NO_ACCESS if needs_new_sso_session?
-      return ::Gitlab::Access::REPORTER if alert_bot?
       return ::Gitlab::Access::REPORTER if support_bot? && service_desk_enabled?
       return ::Gitlab::Access::NO_ACCESS if visual_review_bot?
 

ee/spec/factories/users.rb

@@ -9,10 +9,6 @@ FactoryBot.modify do
     trait :group_managed do
       association :managing_group, factory: :group
     end
-
-    trait :bot do
-      bot_type { User.bot_types[:support_bot] }
-    end
   end
 
   factory :omniauth_user do

ee/spec/models/user_spec.rb

@@ -74,16 +74,6 @@ describe User do
       end
     end
 
-    describe 'bots & humans' do
-      it 'returns corresponding users' do
-        human = create(:user)
-        bot = create(:user, :bot)
-
-        expect(described_class.humans).to match_array([human])
-        expect(described_class.bots).to match_array([bot])
-      end
-    end
-
     describe 'with_invalid_expires_at_tokens' do
       it 'only includes users with invalid tokens' do
         valid_pat = create(:personal_access_token, expires_at: 7.days.from_now)

ee/spec/policies/project_policy_spec.rb

@@ -1021,18 +1021,6 @@ describe ProjectPolicy do
     end
   end
 
-  context 'alert bot' do
-    let(:current_user) { User.alert_bot }
-
-    it { is_expected.to be_allowed(:reporter_access) }
-
-    context 'within a private project' do
-      let(:project) { create(:project, :private) }
-
-      it { is_expected.to be_allowed(:admin_issue) }
-    end
-  end
-
   context 'support bot' do
     let(:current_user) { User.support_bot }
 

spec/factories/users.rb

@@ -23,6 +23,10 @@ FactoryBot.define do
       after(:build) { |user, _| user.block! }
     end
 
+    trait :bot do
+      bot_type { User.bot_types[:alert_bot] }
+    end
+
     trait :external do
       external { true }
     end

spec/models/user_spec.rb

@@ -4126,4 +4126,41 @@ describe User, :do_not_mock_admin_mode do
       end
     end
   end
+
+  describe 'internal methods' do
+    let_it_be(:user) { create(:user) }
+    let!(:ghost) { described_class.ghost }
+    let!(:alert_bot) { described_class.alert_bot }
+    let!(:non_internal) { [user] }
+    let!(:internal) { [ghost, alert_bot] }
+
+    it 'returns non internal users' do
+      expect(described_class.internal).to eq(internal)
+      expect(internal.all?(&:internal?)).to eq(true)
+    end
+
+    it 'returns internal users' do
+      expect(described_class.non_internal).to eq(non_internal)
+      expect(non_internal.all?(&:internal?)).to eq(false)
+    end
+
+    describe '#bot?' do
+      it 'marks bot users' do
+        expect(user.bot?).to eq(false)
+        expect(ghost.bot?).to eq(false)
+
+        expect(alert_bot.bot?).to eq(true)
+      end
+    end
+  end
+
+  describe 'bots & humans' do
+    it 'returns corresponding users' do
+      human = create(:user)
+      bot = create(:user, :bot)
+
+      expect(described_class.humans).to match_array([human])
+      expect(described_class.bots).to match_array([bot])
+    end
+  end
 end

spec/policies/project_policy_spec.rb

@@ -559,4 +559,18 @@ describe ProjectPolicy do
       end
     end
   end
+
+  context 'alert bot' do
+    let(:current_user) { User.alert_bot }
+
+    subject { described_class.new(current_user, project) }
+
+    it { is_expected.to be_allowed(:reporter_access) }
+
+    context 'within a private project' do
+      let(:project) { create(:project, :private) }
+
+      it { is_expected.to be_allowed(:admin_issue) }
+    end
+  end
 end