diff --git a/app/services/members/destroy_service.rb b/app/services/members/destroy_service.rb
index 14cdb83b4e5d64d3f6c3970b3c4eafbf7c20c727..29a6c9995f9b87a0791405f60dbaedc4c2da0ebd 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_heirarchy(member, unassign_issuables)
+ enqueue_cleanup_jobs_once_per_hierarchy(member, unassign_issuables)
end
- def enqueue_cleanup_jobs_once_per_heirarchy(member, unassign_issuables)
+ def enqueue_cleanup_jobs_once_per_hierarchy(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 95e07deb761c4194120e2ba0c860834dae049c8e..90fcd547fe431ddb437de60d49ea337a571c164b 100644
--- a/app/services/members/unassign_issuables_service.rb
+++ b/app/services/members/unassign_issuables_service.rb
@@ -14,10 +14,40 @@ def execute
project_ids = entity.is_a?(Group) ? entity.all_projects.select(:id) : [entity.id]
- user.issue_assignees.on_issues(Issue.in_projects(project_ids).select(:id)).delete_all
- user.merge_request_assignees.in_projects(project_ids).delete_all
+ unassign_from_issues(project_ids)
+ unassign_from_merge_requests(project_ids)
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 c1e970740b97f7b98c31618c07037db8b1da908e..32a47d9f2d8cb2c41e8b16027aa06d02af2ced56 100644
--- a/app/services/merge_requests/update_assignees_service.rb
+++ b/app/services/merge_requests/update_assignees_service.rb
@@ -2,12 +2,18 @@
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)
+ return merge_request unless current_user&.can?(:set_merge_request_metadata, merge_request) || skip_authorization
old_assignees = merge_request.assignees.to_a
old_ids = old_assignees.map(&:id)
@@ -33,6 +39,8 @@ 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 1d9cd62eccc616d1e0f93457d54c8b6617fe764f..55866cac57de8ae25fd05322842f73172a0d06dd 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_heirarchy
- def enqueue_cleanup_jobs_once_per_heirarchy(member, unassign_issuables)
+ override :enqueue_cleanup_jobs_once_per_hierarchy
+ def enqueue_cleanup_jobs_once_per_hierarchy(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 9623cef868b5e8ee37557d195ecfa3f1726dc2a9..dc6f93f7be09d70d18da373013f38ad8ed044737 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|
- 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)
+ # :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
+ )
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 f9b0a993b06fbf26ff9c538da463a936606e7c60..2d7d7ad3ea15a4da1c9313caad79d1caf16f8deb 100644
--- a/spec/services/merge_requests/update_assignees_service_spec.rb
+++ b/spec/services/merge_requests/update_assignees_service_spec.rb
@@ -143,6 +143,16 @@ 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