Fixes for attention request removal automation

Fixes some bugs where the attention request status
would show as incorrect.

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/348432

app/assets/javascripts/sidebar/mount_sidebar.js

@@ -26,6 +26,7 @@ import trackShowInviteMemberLink from '~/sidebar/track_invite_members';
 import { DropdownVariant } from '~/vue_shared/components/sidebar/labels_select_vue/constants';
 import LabelsSelectWidget from '~/vue_shared/components/sidebar/labels_select_widget/labels_select_root.vue';
 import { LabelType } from '~/vue_shared/components/sidebar/labels_select_widget/constants';
+import eventHub from '~/sidebar/event_hub';
 import Translate from '../vue_shared/translate';
 import SidebarAssignees from './components/assignees/sidebar_assignees.vue';
 import CopyEmailToClipboard from './components/copy_email_to_clipboard.vue';
@@ -600,6 +601,12 @@ export function mountSidebar(mediator, store) {
   mountTimeTrackingComponent();
 
   mountSeverityComponent();
+
+  if (window.gon?.features?.mrAttentionRequests) {
+    eventHub.$on('removeCurrentUserAttentionRequested', () =>
+      mediator.removeCurrentUserAttentionRequested(),
+    );
+  }
 }
 
 export { getSidebarOptions };

app/assets/javascripts/sidebar/sidebar_mediator.js

@@ -30,7 +30,7 @@ export default class SidebarMediator {
     this.store.addAssignee(this.store.currentUser);
   }
 
-  saveAssignees(field) {
+  async saveAssignees(field) {
     const selected = this.store.assignees.map((u) => u.id);
 
     // If there are no ids, that means we have to unassign (which is id = 0)
@@ -38,10 +38,22 @@ export default class SidebarMediator {
     const assignees = selected.length === 0 ? [0] : selected;
     const data = { assignee_ids: assignees };
 
-    return this.service.update(field, data);
+    try {
+      const res = await this.service.update(field, data);
+
+      this.store.overwrite('assignees', res.data.assignees);
+
+      if (res.data.reviewers) {
+        this.store.overwrite('reviewers', res.data.reviewers);
+      }
+
+      return Promise.resolve(res);
+    } catch (e) {
+      return Promise.reject(e);
+    }
   }
 
-  saveReviewers(field) {
+  async saveReviewers(field) {
     const selected = this.store.reviewers.map((u) => u.id);
 
     // If there are no ids, that means we have to unassign (which is id = 0)
@@ -49,7 +61,16 @@ export default class SidebarMediator {
     const reviewers = selected.length === 0 ? [0] : selected;
     const data = { reviewer_ids: reviewers };
 
-    return this.service.update(field, data);
+    try {
+      const res = await this.service.update(field, data);
+
+      this.store.overwrite('reviewers', res.data.reviewers);
+      this.store.overwrite('assignees', res.data.assignees);
+
+      return Promise.resolve(res);
+    } catch (e) {
+      return Promise.reject();
+    }
   }
 
   requestReview({ userId, callback }) {
@@ -63,6 +84,19 @@ export default class SidebarMediator {
       .catch(() => callback(userId, false));
   }
 
+  removeCurrentUserAttentionRequested() {
+    const currentUserId = gon.current_user_id;
+
+    const currentUserReviewer = this.store.findReviewer({ id: currentUserId });
+    const currentUserAssignee = this.store.findAssignee({ id: currentUserId });
+
+    if (currentUserReviewer?.attention_requested || currentUserAssignee?.attention_requested) {
+      // Update current users attention_requested state
+      this.store.updateReviewer(currentUserId, 'attention_requested');
+      this.store.updateAssignee(currentUserId, 'attention_requested');
+    }
+  }
+
   async toggleAttentionRequested(type, { user, callback }) {
     try {
       const isReviewer = type === 'reviewer';
@@ -82,15 +116,7 @@ export default class SidebarMediator {
         const currentUserId = gon.current_user_id;
 
         if (currentUserId !== user.id) {
-          const currentUserReviewerOrAssignee = isReviewer
-            ? this.store.findReviewer({ id: currentUserId })
-            : this.store.findAssignee({ id: currentUserId });
-
-          if (currentUserReviewerOrAssignee?.attention_requested) {
-            // Update current users attention_requested state
-            this.store.updateReviewer(currentUserId, 'attention_requested');
-            this.store.updateAssignee(currentUserId, 'attention_requested');
-          }
+          this.removeCurrentUserAttentionRequested();
         }
 
         toast(sprintf(__('Requested attention from @%{username}'), { username: user.username }));

app/assets/javascripts/sidebar/stores/sidebar_store.js

@@ -98,6 +98,10 @@ export default class SidebarStore {
     }
   }
 
+  overwrite(key, newData) {
+    this[key] = newData;
+  }
+
   findAssignee(findAssignee) {
     return this.assignees.find(({ id }) => id === findAssignee.id);
   }

app/assets/javascripts/vue_merge_request_widget/components/approvals/approvals.vue

@@ -3,6 +3,7 @@ import { GlButton } from '@gitlab/ui';
 import createFlash from '~/flash';
 import { BV_SHOW_MODAL } from '~/lib/utils/constants';
 import { s__ } from '~/locale';
+import sidebarEventHub from '~/sidebar/event_hub';
 import eventHub from '../../event_hub';
 import approvalsMixin from '../../mixins/approvals';
 import MrWidgetContainer from '../mr_widget_container.vue';
@@ -172,6 +173,7 @@ export default {
           this.mr.setApprovals(data);
           eventHub.$emit('MRWidgetUpdateRequested');
           eventHub.$emit('ApprovalUpdated');
+          sidebarEventHub.$emit('removeCurrentUserAttentionRequested');
           this.$emit('updated');
         })
         .catch(errFn)

app/serializers/merge_request_sidebar_basic_entity.rb

@@ -5,5 +5,9 @@ class MergeRequestSidebarBasicEntity < IssuableSidebarBasicEntity
     expose :can_merge do |merge_request|
       merge_request.can_be_merged_by?(current_user)
     end
+
+    expose :can_update_merge_request do |merge_request|
+      current_user.can?(:update_merge_request, merge_request)
+    end
   end
 end

app/services/merge_requests/base_service.rb

@@ -59,8 +59,10 @@ module MergeRequests
       merge_request_activity_counter.track_users_review_requested(users: new_reviewers)
       merge_request_activity_counter.track_reviewers_changed_action(user: current_user)
 
+      unless new_reviewers.include?(current_user)
         remove_attention_requested(merge_request, current_user)
       end
+    end
 
     def cleanup_environments(merge_request)
       Environments::StopService.new(merge_request.source_project, current_user)

app/services/merge_requests/handle_assignees_change_service.rb

@@ -23,8 +23,10 @@ module MergeRequests
 
       execute_assignees_hooks(merge_request, old_assignees) if options[:execute_hooks]
 
+      unless new_assignees.include?(current_user)
         remove_attention_requested(merge_request, current_user)
       end
+    end
 
     private
 

app/services/merge_requests/remove_approval_service.rb

@@ -17,6 +17,7 @@ module MergeRequests
         reset_approvals_cache(merge_request)
         create_note(merge_request)
         merge_request_activity_counter.track_unapprove_mr_action(user: current_user)
+        remove_attention_requested(merge_request, current_user)
       end
 
       success

ee/spec/serializers/merge_request_sidebar_basic_entity_spec.rb

@@ -18,7 +18,7 @@ RSpec.describe MergeRequestSidebarBasicEntity do
 
         expect(entity[:current_user].keys).to contain_exactly(
           :id, :name, :username, :state, :avatar_url, :web_url, :todo,
-          :can_edit, :can_move, :can_admin_label, :can_merge
+          :can_edit, :can_move, :can_admin_label, :can_merge, :can_update_merge_request
         )
       end
     end
@@ -30,7 +30,7 @@ RSpec.describe MergeRequestSidebarBasicEntity do
 
         expect(entity[:current_user].keys).to contain_exactly(
           :id, :name, :username, :state, :avatar_url, :web_url, :todo,
-          :can_edit, :can_move, :can_admin_label, :can_merge
+          :can_edit, :can_move, :can_admin_label, :can_merge, :can_update_merge_request
         )
       end
     end
@@ -42,7 +42,7 @@ RSpec.describe MergeRequestSidebarBasicEntity do
 
         expect(entity[:current_user].keys).to contain_exactly(
           :id, :name, :username, :state, :avatar_url, :web_url, :todo,
-          :can_edit, :can_move, :can_admin_label, :can_merge, :is_gitlab_employee
+          :can_edit, :can_move, :can_admin_label, :can_merge, :is_gitlab_employee, :can_update_merge_request
         )
       end
     end