From 33240ecca20fdd72fd53a173f4c56f7e6e953e0f Mon Sep 17 00:00:00 2001
From: Eulyeon Ko <5961404-euko@users.noreply.gitlab.com>
Date: Thu, 1 Feb 2024 00:34:05 +0000
Subject: [PATCH] Revert "Merge branch '431288-user-unassignment-note' into
'master'"
This reverts merge request !143045
---
app/services/members/destroy_service.rb | 4 +--
.../members/unassign_issuables_service.rb | 34 ++-----------------
.../update_assignees_service.rb | 10 +-----
ee/app/services/ee/members/destroy_service.rb | 4 +--
.../unassign_issuables_service_spec.rb | 16 ++++-----
.../update_assignees_service_spec.rb | 10 ------
6 files changed, 15 insertions(+), 63 deletions(-)
diff --git a/app/services/members/destroy_service.rb b/app/services/members/destroy_service.rb
index 29a6c9995f9b8..14cdb83b4e5d6 100644
--- a/app/services/members/destroy_service.rb
+++ b/app/services/members/destroy_service.rb
@@ -47,10 +47,10 @@ def mark_as_recursive_call
def enqueue_jobs_that_needs_to_be_run_only_once_per_hierarchy(member, unassign_issuables)
return if recursive_call?
- enqueue_cleanup_jobs_once_per_hierarchy(member, unassign_issuables)
+ enqueue_cleanup_jobs_once_per_heirarchy(member, unassign_issuables)
end
- def enqueue_cleanup_jobs_once_per_hierarchy(member, unassign_issuables)
+ def enqueue_cleanup_jobs_once_per_heirarchy(member, unassign_issuables)
enqueue_delete_todos(member)
enqueue_unassign_issuables(member) if unassign_issuables
end
diff --git a/app/services/members/unassign_issuables_service.rb b/app/services/members/unassign_issuables_service.rb
index 90fcd547fe431..95e07deb761c4 100644
--- a/app/services/members/unassign_issuables_service.rb
+++ b/app/services/members/unassign_issuables_service.rb
@@ -14,40 +14,10 @@ def execute
project_ids = entity.is_a?(Group) ? entity.all_projects.select(:id) : [entity.id]
- unassign_from_issues(project_ids)
- unassign_from_merge_requests(project_ids)
+ user.issue_assignees.on_issues(Issue.in_projects(project_ids).select(:id)).delete_all
+ user.merge_request_assignees.in_projects(project_ids).delete_all
user.invalidate_cache_counts
end
-
- private
-
- def unassign_from_issues(project_ids)
- user.issue_assignees.on_issues(Issue.in_projects(project_ids)).select(:issue_id).each do |assignee|
- issue = Issue.find(assignee.issue_id)
-
- Issues::UpdateService.new(
- container: issue.project,
- current_user: user,
- params: { assignee_ids: new_assignee_ids(issue) }
- ).execute(issue)
- end
- end
-
- def unassign_from_merge_requests(project_ids)
- user.merge_request_assignees.in_projects(project_ids).select(:merge_request_id).each do |assignee|
- merge_request = MergeRequest.find(assignee.merge_request_id)
-
- ::MergeRequests::UpdateAssigneesService.new(
- project: merge_request.project,
- current_user: user,
- params: { assignee_ids: new_assignee_ids(merge_request), skip_authorization: true }
- ).execute(merge_request)
- end
- end
-
- def new_assignee_ids(issuable)
- issuable.assignees.map(&:id) - [user.id]
- end
end
end
diff --git a/app/services/merge_requests/update_assignees_service.rb b/app/services/merge_requests/update_assignees_service.rb
index 32a47d9f2d8cb..c1e970740b97f 100644
--- a/app/services/merge_requests/update_assignees_service.rb
+++ b/app/services/merge_requests/update_assignees_service.rb
@@ -2,18 +2,12 @@
module MergeRequests
class UpdateAssigneesService < UpdateService
- def initialize(project:, current_user: nil, params: {})
- super
-
- @skip_authorization = @params.delete(:skip_authorization) || false
- end
-
# a stripped down service that only does what it must to update the
# assignees, and knows that it does not have to check for other updates.
# This saves a lot of queries for irrelevant things that cannot possibly
# change in the execution of this service.
def execute(merge_request)
- return merge_request unless current_user&.can?(:set_merge_request_metadata, merge_request) || skip_authorization
+ return merge_request unless current_user&.can?(:set_merge_request_metadata, merge_request)
old_assignees = merge_request.assignees.to_a
old_ids = old_assignees.map(&:id)
@@ -39,8 +33,6 @@ def execute(merge_request)
private
- attr_reader :skip_authorization
-
def assignee_ids
filter_sentinel_values(params.fetch(:assignee_ids)).first(1)
end
diff --git a/ee/app/services/ee/members/destroy_service.rb b/ee/app/services/ee/members/destroy_service.rb
index 55866cac57de8..1d9cd62eccc61 100644
--- a/ee/app/services/ee/members/destroy_service.rb
+++ b/ee/app/services/ee/members/destroy_service.rb
@@ -97,8 +97,8 @@ def cleanup_escalation_rules(member)
::IncidentManagement::EscalationRules::DestroyService.new(escalation_rules: rules, user: member.user).execute
end
- override :enqueue_cleanup_jobs_once_per_hierarchy
- def enqueue_cleanup_jobs_once_per_hierarchy(member, unassign_issuables)
+ override :enqueue_cleanup_jobs_once_per_heirarchy
+ def enqueue_cleanup_jobs_once_per_heirarchy(member, unassign_issuables)
super
enqueue_cleanup_add_on_seat_assignments(member)
diff --git a/spec/services/members/unassign_issuables_service_spec.rb b/spec/services/members/unassign_issuables_service_spec.rb
index dc6f93f7be09d..9623cef868b5e 100644
--- a/spec/services/members/unassign_issuables_service_spec.rb
+++ b/spec/services/members/unassign_issuables_service_spec.rb
@@ -14,14 +14,14 @@
describe '#execute' do
RSpec.shared_examples 'un-assigning issuables' do |issue_count, mr_count, open_issue_count, open_mr_count|
- # :sidekiq_inline is used b/c unlike issues, assignee changes for MRs get handled asynchronously.
- it 'removes issuable assignments', :sidekiq_inline, :aggregate_failures do
- expect { subject }
- .to change { user.assigned_issues.count }.from(issue_count).to(0)
- .and change { user.assigned_merge_requests.count }.from(mr_count).to(0)
- .and change { Note.where('note ILIKE ?', '%unassigned%').count }.by(
- user.assigned_issues.count + user.assigned_merge_requests.count
- )
+ it 'removes issuable assignments', :aggregate_failures do
+ expect(user.assigned_issues.count).to eq(issue_count)
+ expect(user.assigned_merge_requests.count).to eq(mr_count)
+
+ subject
+
+ expect(user.assigned_issues.count).to eq(0)
+ expect(user.assigned_merge_requests.count).to eq(0)
end
it 'invalidates user cache', :aggregate_failures, :clean_gitlab_redis_cache do
diff --git a/spec/services/merge_requests/update_assignees_service_spec.rb b/spec/services/merge_requests/update_assignees_service_spec.rb
index 2d7d7ad3ea15a..f9b0a993b06fb 100644
--- a/spec/services/merge_requests/update_assignees_service_spec.rb
+++ b/spec/services/merge_requests/update_assignees_service_spec.rb
@@ -143,16 +143,6 @@ def update_merge_request
expect { update_merge_request }
.not_to change { merge_request.reload.assignees.to_a }
end
-
- context 'when skip_authorization is set' do
- let(:opts) { { assignee_ids: [user2.id], skip_authorization: true } }
-
- it 'updates the MR assignees' do
- expect { update_merge_request }
- .to change { merge_request.reload.assignees }
- .from([user3]).to([user2])
- end
- end
end
end
end
--
GitLab