Split user bots between CE and EE

Introduce a `UserBotTypeEnums` module,
which allows split bot types between CE and EE.

Move "support bot" and "visual review bot" back to EE.

app/models/user.rb

@@ -59,11 +59,7 @@ class User < ApplicationRecord
 
   MINIMUM_INACTIVE_DAYS = 180
 
-  enum bot_type: {
-    support_bot: 1,
-    alert_bot: 2,
-    visual_review_bot: 3
-  }
+  enum bot_type: ::UserBotTypeEnums.bots
 
   # Override Devise::Models::Trackable#update_tracked_fields!
   # to limit database writes to at most once every hour
@@ -615,24 +611,6 @@ class User < ApplicationRecord
       end
     end
 
-    def support_bot
-      email_pattern = "support%s@#{Settings.gitlab.host}"
-
-      unique_internal(where(bot_type: :support_bot), 'support-bot', email_pattern) do |u|
-        u.bio = 'The GitLab support bot used for Service Desk'
-        u.name = 'GitLab Support Bot'
-      end
-    end
-
-    def visual_review_bot
-      email_pattern = "visual_review%s@#{Settings.gitlab.host}"
-
-      unique_internal(where(bot_type: :visual_review_bot), 'visual-review-bot', email_pattern) do |u|
-        u.bio = 'The Gitlab Visual Review feedback bot'
-        u.name = 'Gitlab Visual Review Bot'
-      end
-    end
-
     # Return true if there is only single non-internal user in the deployment,
     # ghost user is ignored.
     def single_user?

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,15 +44,9 @@ class BasePolicy < DeclarativePolicy::Base
     ::Gitlab::ExternalAuthorization.perform_check?
   end
 
-  with_options scope: :user, score: 0
-  condition(:support_bot) { @user&.support_bot? }
-
   with_options scope: :user, score: 0
   condition(:alert_bot) { @user&.alert_bot? }
 
-  with_options scope: :user, score: 0
-  condition(:visual_review_bot) { @user&.visual_review_bot? }
-
   rule { external_authorization_enabled & ~can?(:read_all_resources) }.policy do
     prevent :read_cross_project
   end

app/policies/concerns/policy_actor.rb

@@ -34,17 +34,9 @@ module PolicyActor
     false
   end
 
-  def support_bot?
-    false
-  end
-
   def alert_bot?
     false
   end
-
-  def visual_review_bot?
-    false
-  end
 end
 
 PolicyActor.prepend_if_ee('EE::PolicyActor')

ee/app/models/ee/user.rb

@@ -87,6 +87,24 @@ module EE
     class_methods do
       extend ::Gitlab::Utils::Override
 
+      def support_bot
+        email_pattern = "support%s@#{Settings.gitlab.host}"
+
+        unique_internal(where(bot_type: :support_bot), 'support-bot', email_pattern) do |u|
+          u.bio = 'The GitLab support bot used for Service Desk'
+          u.name = 'GitLab Support Bot'
+        end
+      end
+
+      def visual_review_bot
+        email_pattern = "visual_review%s@#{Settings.gitlab.host}"
+
+        unique_internal(where(bot_type: :visual_review_bot), 'visual-review-bot', email_pattern) do |u|
+          u.bio = 'The Gitlab Visual Review feedback bot'
+          u.name = 'Gitlab Visual Review Bot'
+        end
+      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%')

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

@@ -8,6 +8,12 @@ module EE
       with_scope :user
       condition(:auditor, score: 0) { @user&.auditor? }
 
+      with_scope :user
+      condition(:support_bot, score: 0) { @user&.support_bot? }
+
+      with_scope :user
+      condition(:visual_review_bot, score: 0) { @user&.visual_review_bot? }
+
       with_scope :global
       condition(:license_block) { License.block_changes? }
 

ee/app/policies/ee/policy_actor.rb

@@ -5,5 +5,13 @@ module EE
     def auditor?
       false
     end
+
+    def support_bot?
+      false
+    end
+
+    def visual_review_bot?
+      false
+    end
   end
 end

ee/spec/models/user_spec.rb

@@ -583,6 +583,37 @@ describe User do
     end
   end
 
+  describe 'internal methods' do
+    let!(:user) { create(:user) }
+    let!(:ghost) { described_class.ghost }
+    let!(:support_bot) { described_class.support_bot }
+    let!(:alert_bot) { described_class.alert_bot }
+    let!(:visual_review_bot) { described_class.visual_review_bot }
+    let!(:non_internal) { [user] }
+    let!(:internal) { [ghost, support_bot, alert_bot, visual_review_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(support_bot.bot?).to eq(true)
+        expect(alert_bot.bot?).to eq(true)
+        expect(visual_review_bot.bot?).to eq(true)
+      end
+    end
+  end
+
   describe '#using_license_seat?' do
     let(:user) { create(:user) }
 

spec/factories/users.rb

@@ -24,7 +24,7 @@ FactoryBot.define do
     end
 
     trait :bot do
-      bot_type { User.bot_types[:support_bot] }
+      bot_type { User.bot_types[:alert_bot] }
     end
 
     trait :external do

spec/models/user_spec.rb

@@ -4130,11 +4130,9 @@ describe User, :do_not_mock_admin_mode do
   describe 'internal methods' do
     let!(:user) { create(:user) }
     let!(:ghost) { described_class.ghost }
-    let!(:support_bot) { described_class.support_bot }
     let!(:alert_bot) { described_class.alert_bot }
-    let!(:visual_review_bot) { described_class.visual_review_bot }
     let!(:non_internal) { [user] }
-    let!(:internal) { [ghost, support_bot, alert_bot, visual_review_bot] }
+    let!(:internal) { [ghost, alert_bot] }
 
     it 'returns non internal users' do
       expect(described_class.internal).to eq(internal)
@@ -4151,9 +4149,7 @@ describe User, :do_not_mock_admin_mode do
         expect(user.bot?).to eq(false)
         expect(ghost.bot?).to eq(false)
 
-        expect(support_bot.bot?).to eq(true)
         expect(alert_bot.bot?).to eq(true)
-        expect(visual_review_bot.bot?).to eq(true)
       end
     end
   end