Merge branch 'revert-bb6773d2' into 'master'

Revert "Merge branch '431288-user-unassignment-note' into 'master'"

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/143451



Merged-by: Eulyeon Ko <5961404-euko@users.noreply.gitlab.com>
Approved-by: Jessie Young <jessieyoung@gitlab.com>

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

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

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

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)

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

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