From 4c34be70e32524250295e556fe38d4b9ae15e4cb Mon Sep 17 00:00:00 2001
From: imand3r <ianderson@gitlab.com>
Date: Wed, 5 Mar 2025 23:47:50 +0000
Subject: [PATCH] Refactor Users::AutoBanService to use Users::BanService
---
app/services/users/auto_ban_service.rb | 32 +++++++++++--------
.../identity_verification_shared_examples.rb | 2 +-
spec/services/users/auto_ban_service_spec.rb | 4 +--
3 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/app/services/users/auto_ban_service.rb b/app/services/users/auto_ban_service.rb
index caadc92551e64..56bf31a3d7821 100644
--- a/app/services/users/auto_ban_service.rb
+++ b/app/services/users/auto_ban_service.rb
@@ -1,34 +1,40 @@
# frozen_string_literal: true
module Users
- class AutoBanService < BaseService
+ class AutoBanService
+ Error = Class.new(StandardError)
+
def initialize(user:, reason:)
@user = user
@reason = reason
end
def execute
- if user.ban
- record_custom_attribute
- ban_duplicate_users
- success
- else
- messages = user.errors.full_messages
- error(messages.uniq.join('. '))
- end
+ ban_user
end
def execute!
- user.ban!
- record_custom_attribute
- ban_duplicate_users
- success
+ result = ban_user
+
+ raise Error, result[:message] if result[:status] == :error
end
private
attr_reader :user, :reason
+ def ban_user
+ result = ::Users::BanService.new(admin_bot).execute(user)
+
+ record_custom_attribute if result[:status] == :success
+
+ result
+ end
+
+ def admin_bot
+ Users::Internal.admin_bot
+ end
+
def ban_duplicate_users
AntiAbuse::BanDuplicateUsersWorker.perform_async(user.id)
end
diff --git a/ee/spec/support/shared_examples/requests/identity_verification_shared_examples.rb b/ee/spec/support/shared_examples/requests/identity_verification_shared_examples.rb
index 99370ab73b5ad..5e77da90561fc 100644
--- a/ee/spec/support/shared_examples/requests/identity_verification_shared_examples.rb
+++ b/ee/spec/support/shared_examples/requests/identity_verification_shared_examples.rb
@@ -68,7 +68,7 @@ def mock_rate_limit(limit_name, method, result, scope: nil)
do_request
- expect(Gitlab::AppLogger).to have_received(:info).with(a_hash_including(logger_args))
+ expect(Gitlab::AppLogger).to have_received(:info).with(hash_including(logger_args))
tracking_args = {
category: message,
diff --git a/spec/services/users/auto_ban_service_spec.rb b/spec/services/users/auto_ban_service_spec.rb
index e40f28eabeb49..52409a6f522cf 100644
--- a/spec/services/users/auto_ban_service_spec.rb
+++ b/spec/services/users/auto_ban_service_spec.rb
@@ -51,7 +51,7 @@
response = execute
expect(response[:status]).to eq(:error)
- expect(response[:message]).to match('State cannot transition via "ban"')
+ expect(response[:message]).to match('You cannot ban blocked users.')
end
it 'does not modify the BannedUser record or user state' do
@@ -76,7 +76,7 @@
end
it 'raises an error and does not ban the user', :aggregate_failures do
- expect { execute! }.to raise_error(StateMachines::InvalidTransition)
+ expect { execute! }.to raise_error(described_class::Error)
.and not_change { Users::BannedUser.count }
.and not_change { user.state }
end
--
GitLab