Refactor assignee and reviewer state to allow nil values

Refactors all the `attention_required` variable names to be
`attention_requested` to better match the feature name.

Changes the AttentionRequest service to be a ToggleAttentionRequested
service which will toggle between `attention_requested` and `nil`

This also hooks it up all on the frontend so that the assignees
and reviewers in the sidebar send the correct GraphQL mutation
when the attention request button is clicked.

Changelog: changed

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

app/assets/javascripts/sidebar/components/assignees/assignees.vue

@@ -39,8 +39,8 @@ export default {
     assignSelf() {
       this.$emit('assign-self');
     },
-    toggleAttentionRequired(data) {
-      this.$emit('toggle-attention-required', data);
+    toggleAttentionRequested(data) {
+      this.$emit('toggle-attention-requested', data);
     },
   },
 };
@@ -65,7 +65,7 @@ export default {
         v-else
         :users="sortedAssigness"
         :issuable-type="issuableType"
-        @toggle-attention-required="toggleAttentionRequired"
+        @toggle-attention-requested="toggleAttentionRequested"
       />
     </div>
   </div>

app/assets/javascripts/sidebar/components/assignees/issuable_assignees.vue

@@ -33,8 +33,8 @@ export default {
     },
   },
   methods: {
-    toggleAttentionRequired(data) {
-      this.$emit('toggle-attention-required', data);
+    toggleAttentionRequested(data) {
+      this.$emit('toggle-attention-requested', data);
     },
   },
 };
@@ -66,7 +66,7 @@ export default {
       :users="users"
       :issuable-type="issuableType"
       class="gl-text-gray-800 gl-mt-2 hide-collapsed"
-      @toggle-attention-required="toggleAttentionRequired"
+      @toggle-attention-requested="toggleAttentionRequested"
     />
   </div>
 </template>

app/assets/javascripts/sidebar/components/assignees/sidebar_assignees.vue

@@ -125,8 +125,8 @@ export default {
         availability: this.assigneeAvailabilityStatus[username] || '',
       }));
     },
-    toggleAttentionRequired(data) {
-      this.mediator.toggleAttentionRequired('assignee', data);
+    toggleAttentionRequested(data) {
+      this.mediator.toggleAttentionRequested('assignee', data);
     },
   },
 };
@@ -155,7 +155,7 @@ export default {
       :editable="store.editable"
       :issuable-type="issuableType"
       @assign-self="assignSelf"
-      @toggle-attention-required="toggleAttentionRequired"
+      @toggle-attention-requested="toggleAttentionRequested"
     />
   </div>
 </template>

app/assets/javascripts/sidebar/components/assignees/uncollapsed_assignee_list.vue

@@ -2,7 +2,7 @@
 import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import { IssuableType } from '~/issue_show/constants';
 import { __, sprintf } from '~/locale';
-import AttentionRequiredToggle from '../attention_required_toggle.vue';
+import AttentionRequestedToggle from '../attention_requested_toggle.vue';
 import AssigneeAvatarLink from './assignee_avatar_link.vue';
 import UserNameWithStatus from './user_name_with_status.vue';
 
@@ -10,7 +10,7 @@ const DEFAULT_RENDER_COUNT = 5;
 
 export default {
   components: {
-    AttentionRequiredToggle,
+    AttentionRequestedToggle,
     AssigneeAvatarLink,
     UserNameWithStatus,
   },
@@ -82,8 +82,8 @@ export default {
       }
       return u?.status?.availability || '';
     },
-    toggleAttentionRequired(data) {
-      this.$emit('toggle-attention-required', data);
+    toggleAttentionRequested(data) {
+      this.$emit('toggle-attention-requested', data);
     },
   },
 };
@@ -113,11 +113,11 @@ export default {
         }"
         class="gl-display-inline-block"
       >
-        <attention-required-toggle
+        <attention-requested-toggle
           v-if="showVerticalList && user.can_update_merge_request"
           :user="user"
           type="assignee"
-          @toggle-attention-required="toggleAttentionRequired"
+          @toggle-attention-requested="toggleAttentionRequested"
         />
         <assignee-avatar-link
           :user="user"

app/assets/javascripts/sidebar/components/attention_required_toggle.vue → app/assets/javascripts/sidebar/components/attention_requested_toggle.vue

@@ -5,9 +5,9 @@ import { BV_HIDE_TOOLTIP } from '~/lib/utils/constants';
 
 export default {
   i18n: {
-    attentionRequiredReviewer: __('Request attention to review'),
-    attentionRequiredAssignee: __('Request attention'),
-    removeAttentionRequired: __('Remove attention request'),
+    attentionRequestedReviewer: __('Request attention to review'),
+    attentionRequestedAssignee: __('Request attention'),
+    removeAttentionRequested: __('Remove attention request'),
   },
   components: {
     GlButton,
@@ -32,13 +32,13 @@ export default {
   },
   computed: {
     tooltipTitle() {
-      if (this.user.attention_required) {
-        return this.$options.i18n.removeAttentionRequired;
+      if (this.user.attention_requested) {
+        return this.$options.i18n.removeAttentionRequested;
       }
 
       return this.type === 'reviewer'
-        ? this.$options.i18n.attentionRequiredReviewer
-        : this.$options.i18n.attentionRequiredAssignee;
+        ? this.$options.i18n.attentionRequestedReviewer
+        : this.$options.i18n.attentionRequestedAssignee;
     },
   },
   methods: {
@@ -47,7 +47,7 @@ export default {
 
       this.$root.$emit(BV_HIDE_TOOLTIP);
       this.loading = true;
-      this.$emit('toggle-attention-required', {
+      this.$emit('toggle-attention-requested', {
         user: this.user,
         callback: this.toggleAttentionRequiredComplete,
       });
@@ -63,8 +63,8 @@ export default {
   <span v-gl-tooltip.left.viewport="tooltipTitle">
     <gl-button
       :loading="loading"
-      :variant="user.attention_required ? 'warning' : 'default'"
-      :icon="user.attention_required ? 'star' : 'star-o'"
+      :variant="user.attention_requested ? 'warning' : 'default'"
+      :icon="user.attention_requested ? 'star' : 'star-o'"
       :aria-label="tooltipTitle"
       size="small"
       category="tertiary"

app/assets/javascripts/sidebar/components/reviewers/reviewers.vue

@@ -49,8 +49,8 @@ export default {
     requestReview(data) {
       this.$emit('request-review', data);
     },
-    toggleAttentionRequired(data) {
-      this.$emit('toggle-attention-required', data);
+    toggleAttentionRequested(data) {
+      this.$emit('toggle-attention-requested', data);
     },
   },
 };
@@ -73,7 +73,7 @@ export default {
         :root-path="rootPath"
         :issuable-type="issuableType"
         @request-review="requestReview"
-        @toggle-attention-required="toggleAttentionRequired"
+        @toggle-attention-requested="toggleAttentionRequested"
       />
     </div>
   </div>

app/assets/javascripts/sidebar/components/reviewers/sidebar_reviewers.vue

@@ -88,8 +88,8 @@ export default {
     requestReview(data) {
       this.mediator.requestReview(data);
     },
-    toggleAttentionRequired(data) {
-      this.mediator.toggleAttentionRequired('reviewer', data);
+    toggleAttentionRequested(data) {
+      this.mediator.toggleAttentionRequested('reviewer', data);
     },
   },
 };
@@ -109,7 +109,7 @@ export default {
       :editable="store.editable"
       :issuable-type="issuableType"
       @request-review="requestReview"
-      @toggle-attention-required="toggleAttentionRequired"
+      @toggle-attention-requested="toggleAttentionRequested"
     />
   </div>
 </template>

app/assets/javascripts/sidebar/components/reviewers/uncollapsed_reviewer_list.vue

@@ -2,7 +2,7 @@
 import { GlButton, GlTooltipDirective, GlIcon } from '@gitlab/ui';
 import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
 import { __, sprintf, s__ } from '~/locale';
-import AttentionRequiredToggle from '../attention_required_toggle.vue';
+import AttentionRequestedToggle from '../attention_requested_toggle.vue';
 import ReviewerAvatarLink from './reviewer_avatar_link.vue';
 
 const LOADING_STATE = 'loading';
@@ -16,7 +16,7 @@ export default {
     GlButton,
     GlIcon,
     ReviewerAvatarLink,
-    AttentionRequiredToggle,
+    AttentionRequestedToggle,
   },
   directives: {
     GlTooltip: GlTooltipDirective,
@@ -80,8 +80,8 @@ export default {
         this.loadingStates[userId] = null;
       }
     },
-    toggleAttentionRequired(data) {
-      this.$emit('toggle-attention-required', data);
+    toggleAttentionRequested(data) {
+      this.$emit('toggle-attention-requested', data);
     },
   },
   LOADING_STATE,
@@ -97,11 +97,11 @@ export default {
       :class="{ 'gl-mb-3': index !== users.length - 1 }"
       data-testid="reviewer"
     >
-      <attention-required-toggle
+      <attention-requested-toggle
         v-if="glFeatures.mrAttentionRequests && user.can_update_merge_request"
         :user="user"
         type="reviewer"
-        @toggle-attention-required="toggleAttentionRequired"
+        @toggle-attention-requested="toggleAttentionRequested"
       />
       <reviewer-avatar-link :user="user" :root-path="rootPath" :issuable-type="issuableType">
         <div class="gl-ml-3 gl-line-height-normal gl-display-grid">

app/assets/javascripts/sidebar/queries/attention_required.mutation.graphql

-mutation mergeRequestAttentionRequired($projectPath: ID!, $iid: String!, $userId: ID!) {
-  mergeRequestAttentionRequired(input: { projectPath: $projectPath, iid: $iid, userId: $userId }) {
-    errors
-  }
-}

app/assets/javascripts/sidebar/queries/toggle_attention_requested.mutation.graphql

+mutation mergeRequestToggleAttentionRequested($projectPath: ID!, $iid: String!, $userId: ID!) {
+  mergeRequestToggleAttentionRequested(
+    input: { projectPath: $projectPath, iid: $iid, userId: $userId }
+  ) {
+    errors
+  }
+}

app/assets/javascripts/sidebar/services/sidebar_service.js

@@ -5,7 +5,7 @@ import createGqClient, { fetchPolicies } from '~/lib/graphql';
 import axios from '~/lib/utils/axios_utils';
 import reviewerRereviewMutation from '../queries/reviewer_rereview.mutation.graphql';
 import sidebarDetailsMRQuery from '../queries/sidebarDetailsMR.query.graphql';
-import attentionRequiredMutation from '../queries/attention_required.mutation.graphql';
+import toggleAttentionRequestedMutation from '../queries/toggle_attention_requested.mutation.graphql';
 
 const queries = {
   merge_request: sidebarDetailsMRQuery,
@@ -92,9 +92,9 @@ export default class SidebarService {
     });
   }
 
-  attentionRequired(userId) {
+  toggleAttentionRequested(userId) {
     return gqClient.mutate({
-      mutation: attentionRequiredMutation,
+      mutation: toggleAttentionRequestedMutation,
       variables: {
         userId: convertToGraphQLId(TYPE_USER, `${userId}`),
         projectPath: this.fullPath,

app/assets/javascripts/sidebar/sidebar_mediator.js

@@ -63,30 +63,27 @@ export default class SidebarMediator {
       .catch(() => callback(userId, false));
   }
 
-  async toggleAttentionRequired(type, { user, callback }) {
+  async toggleAttentionRequested(type, { user, callback }) {
     try {
       const isReviewer = type === 'reviewer';
       const reviewerOrAssignee = isReviewer
         ? this.store.findReviewer(user)
         : this.store.findAssignee(user);
 
-      if (reviewerOrAssignee.attention_required) {
+      await this.service.toggleAttentionRequested(user.id);
+
+      if (reviewerOrAssignee.attention_requested) {
         toast(
           sprintf(__('Removed attention request from @%{username}'), {
             username: user.username,
           }),
         );
       } else {
-        await this.service.attentionRequired(user.id);
-
         toast(sprintf(__('Requested attention from @%{username}'), { username: user.username }));
       }
 
-      if (isReviewer) {
-        this.store.updateReviewer(user.id, 'attention_required');
-      } else {
-        this.store.updateAssignee(user.id, 'attention_required');
-      }
+      this.store.updateReviewer(user.id, 'attention_requested');
+      this.store.updateAssignee(user.id, 'attention_requested');
 
       callback();
     } catch (error) {

app/graphql/mutations/merge_requests/attention_required.rb → app/graphql/mutations/merge_requests/toggle_attention_requested.rb

@@ -2,20 +2,20 @@
 
 module Mutations
   module MergeRequests
-    class AttentionRequired < Base
-      graphql_name 'MergeRequestAttentionRequired'
+    class ToggleAttentionRequested < Base
+      graphql_name 'MergeRequestToggleAttentionRequested'
 
       argument :user_id, ::Types::GlobalIDType[::User],
                loads: Types::UserType,
                required: true,
                description: <<~DESC
-                            User ID for the user that has their attention requested.
+                            User ID for the user to toggle attention requested.
                DESC
 
       def resolve(project_path:, iid:, user:)
         merge_request = authorized_find!(project_path: project_path, iid: iid)
 
-        result = ::MergeRequests::AttentionRequiredService.new(project: merge_request.project, current_user: current_user, merge_request: merge_request, user: user).execute
+        result = ::MergeRequests::ToggleAttentionRequestedService.new(project: merge_request.project, current_user: current_user, merge_request: merge_request, user: user).execute
 
         {
           merge_request: merge_request,

app/graphql/types/mutation_type.rb

@@ -68,7 +68,7 @@ module Types
     mount_mutation Mutations::MergeRequests::SetDraft, calls_gitaly: true
     mount_mutation Mutations::MergeRequests::SetAssignees
     mount_mutation Mutations::MergeRequests::ReviewerRereview
-    mount_mutation Mutations::MergeRequests::AttentionRequired, feature_flag: :mr_attention_requests
+    mount_mutation Mutations::MergeRequests::ToggleAttentionRequested, feature_flag: :mr_attention_requests
     mount_mutation Mutations::Metrics::Dashboard::Annotations::Create
     mount_mutation Mutations::Metrics::Dashboard::Annotations::Delete
     mount_mutation Mutations::Notes::Create::Note, calls_gitaly: true

app/helpers/todos_helper.rb

@@ -24,7 +24,7 @@ module TodosHelper
     when Todo::UNMERGEABLE then 'Could not merge'
     when Todo::DIRECTLY_ADDRESSED then "directly addressed #{todo_action_subject(todo)} on"
     when Todo::MERGE_TRAIN_REMOVED then "Removed from Merge Train:"
-    when Todo::ATTENTION_REQUIRED then 'requested your attention on'
+    when Todo::ATTENTION_REQUESTED then 'requested your attention on'
     end
   end
 

app/models/concerns/merge_request_reviewer_state.rb

@@ -7,7 +7,7 @@ module MergeRequestReviewerState
     enum state: {
       unreviewed: 0,
       reviewed: 1,
-      attention_required: 2
+      attention_requested: 2
     }
 
     validates :state,
@@ -18,7 +18,7 @@ module MergeRequestReviewerState
 
     def set_state
       if Feature.enabled?(:mr_attention_requests, self.merge_request&.project, default_enabled: :yaml)
-        self.state = :attention_required
+        self.state = :attention_requested
       end
     end
   end

app/models/merge_request.rb

@@ -1945,7 +1945,7 @@ class MergeRequest < ApplicationRecord
     end
   end
 
-  def attention_required_enabled?
+  def attention_requested_enabled?
     Feature.enabled?(:mr_attention_requests, project, default_enabled: :yaml)
   end
 

app/models/todo.rb

@@ -18,7 +18,7 @@ class Todo < ApplicationRecord
   DIRECTLY_ADDRESSED  = 7
   MERGE_TRAIN_REMOVED = 8 # This is an EE-only feature
   REVIEW_REQUESTED    = 9
-  ATTENTION_REQUIRED  = 10
+  ATTENTION_REQUESTED = 10
 
   ACTION_NAMES = {
     ASSIGNED => :assigned,
@@ -30,7 +30,7 @@ class Todo < ApplicationRecord
     UNMERGEABLE => :unmergeable,
     DIRECTLY_ADDRESSED => :directly_addressed,
     MERGE_TRAIN_REMOVED => :merge_train_removed,
-    ATTENTION_REQUIRED => :attention_required
+    ATTENTION_REQUESTED => :attention_requested
   }.freeze
 
   belongs_to :author, class_name: "User"
@@ -191,8 +191,8 @@ class Todo < ApplicationRecord
     action == REVIEW_REQUESTED
   end
 
-  def attention_required?
-    action == ATTENTION_REQUIRED
+  def attention_requested?
+    action == ATTENTION_REQUESTED
   end
 
   def merge_train_removed?

app/serializers/merge_request_user_entity.rb

@@ -20,8 +20,8 @@ class MergeRequestUserEntity < ::API::Entities::UserBasic
     find_reviewer_or_assignee(user, options)&.reviewed?
   end
 
-  expose :attention_required, if: satisfies(:present?, :allows_reviewers?, :attention_required_enabled?) do |user, options|
-    find_reviewer_or_assignee(user, options)&.attention_required?
+  expose :attention_requested, if: satisfies(:present?, :allows_reviewers?, :attention_requested_enabled?) do |user, options|
+    find_reviewer_or_assignee(user, options)&.attention_requested?
   end
 
   expose :approved, if: satisfies(:present?) do |user, options|

app/services/merge_requests/attention_required_service.rb → app/services/merge_requests/toggle_attention_requested_service.rb

 # frozen_string_literal: true
 
 module MergeRequests
-  class AttentionRequiredService < MergeRequests::BaseService
+  class ToggleAttentionRequestedService < MergeRequests::BaseService
     attr_accessor :merge_request, :user
 
     def initialize(project:, current_user:, merge_request:, user:)
@@ -15,10 +15,12 @@ module MergeRequests
       return error("Invalid permissions") unless can?(current_user, :update_merge_request, merge_request)
 
       if reviewer || assignee
-        reviewer&.update(state: :attention_required)
-        assignee&.update(state: :attention_required)
+        update_state(reviewer)
+        update_state(assignee)
 
-        notity_user
+        if reviewer&.attention_requested? || assignee&.attention_requested?
+          notity_user
+        end
 
         success
       else
@@ -29,7 +31,7 @@ module MergeRequests
     private
 
     def notity_user
-      todo_service.create_attention_required_todo(merge_request, current_user, user)
+      todo_service.create_attention_requested_todo(merge_request, current_user, user)
     end
 
     def assignee
@@ -39,5 +41,9 @@ module MergeRequests
     def reviewer
       merge_request.find_reviewer(user)
     end
+
+    def update_state(reviewer_or_assignee)
+      reviewer_or_assignee&.update(state: reviewer_or_assignee&.attention_requested? ? :reviewed : :attention_requested)
+    end
   end
 end

app/services/todo_service.rb

@@ -217,8 +217,8 @@ class TodoService
     create_todos(reviewers, attributes)
   end
 
-  def create_attention_required_todo(target, author, users)
-    attributes = attributes_for_todo(target.project, target, author, Todo::ATTENTION_REQUIRED)
+  def create_attention_requested_todo(target, author, users)
+    attributes = attributes_for_todo(target.project, target, author, Todo::ATTENTION_REQUESTED)
     create_todos(users, attributes)
   end
 

doc/api/graphql/reference/index.md

@@ -3313,29 +3313,6 @@ Input type: `MergeRequestAcceptInput`
 | <a id="mutationmergerequestaccepterrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
 | <a id="mutationmergerequestacceptmergerequest"></a>`mergeRequest` | [`MergeRequest`](#mergerequest) | Merge request after mutation. |
 
-### `Mutation.mergeRequestAttentionRequired`
-
-Available only when feature flag `mr_attention_requests` is enabled. This flag is disabled by default, because the feature is experimental and is subject to change without notice.
-
-Input type: `MergeRequestAttentionRequiredInput`
-
-#### Arguments
-
-| Name | Type | Description |
-| ---- | ---- | ----------- |
-| <a id="mutationmergerequestattentionrequiredclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
-| <a id="mutationmergerequestattentionrequirediid"></a>`iid` | [`String!`](#string) | IID of the merge request to mutate. |
-| <a id="mutationmergerequestattentionrequiredprojectpath"></a>`projectPath` | [`ID!`](#id) | Project the merge request to mutate is in. |
-| <a id="mutationmergerequestattentionrequireduserid"></a>`userId` | [`UserID!`](#userid) | User ID for the user that has their attention requested. |
-
-#### Fields
-
-| Name | Type | Description |
-| ---- | ---- | ----------- |
-| <a id="mutationmergerequestattentionrequiredclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
-| <a id="mutationmergerequestattentionrequirederrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
-| <a id="mutationmergerequestattentionrequiredmergerequest"></a>`mergeRequest` | [`MergeRequest`](#mergerequest) | Merge request after mutation. |
-
 ### `Mutation.mergeRequestCreate`
 
 Input type: `MergeRequestCreateInput`
@@ -3509,6 +3486,29 @@ Input type: `MergeRequestSetSubscriptionInput`
 | <a id="mutationmergerequestsetsubscriptionerrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
 | <a id="mutationmergerequestsetsubscriptionmergerequest"></a>`mergeRequest` | [`MergeRequest`](#mergerequest) | Merge request after mutation. |
 
+### `Mutation.mergeRequestToggleAttentionRequested`
+
+Available only when feature flag `mr_attention_requests` is enabled. This flag is disabled by default, because the feature is experimental and is subject to change without notice.
+
+Input type: `MergeRequestToggleAttentionRequestedInput`
+
+#### Arguments
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="mutationmergerequesttoggleattentionrequestedclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
+| <a id="mutationmergerequesttoggleattentionrequestediid"></a>`iid` | [`String!`](#string) | IID of the merge request to mutate. |
+| <a id="mutationmergerequesttoggleattentionrequestedprojectpath"></a>`projectPath` | [`ID!`](#id) | Project the merge request to mutate is in. |
+| <a id="mutationmergerequesttoggleattentionrequesteduserid"></a>`userId` | [`UserID!`](#userid) | User ID for the user to toggle attention requested. |
+
+#### Fields
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| <a id="mutationmergerequesttoggleattentionrequestedclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
+| <a id="mutationmergerequesttoggleattentionrequestederrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
+| <a id="mutationmergerequesttoggleattentionrequestedmergerequest"></a>`mergeRequest` | [`MergeRequest`](#mergerequest) | Merge request after mutation. |
+
 ### `Mutation.mergeRequestUpdate`
 
 Update attributes of a merge request.
@@ -16455,7 +16455,7 @@ State of a review of a GitLab merge request.
 
 | Value | Description |
 | ----- | ----------- |
-| <a id="mergerequestreviewstateattention_required"></a>`ATTENTION_REQUIRED` | The merge request is attention_required. |
+| <a id="mergerequestreviewstateattention_requested"></a>`ATTENTION_REQUESTED` | The merge request is attention_requested. |
 | <a id="mergerequestreviewstatereviewed"></a>`REVIEWED` | The merge request is reviewed. |
 | <a id="mergerequestreviewstateunreviewed"></a>`UNREVIEWED` | The merge request is unreviewed. |
 

spec/frontend/sidebar/components/attention_required_toggle_spec.js

 import { GlButton } from '@gitlab/ui';
 import { mount } from '@vue/test-utils';
-import AttentionRequiredToggle from '~/sidebar/components/attention_required_toggle.vue';
+import AttentionRequestedToggle from '~/sidebar/components/attention_requested_toggle.vue';
 
 let wrapper;
 
 function factory(propsData = {}) {
-  wrapper = mount(AttentionRequiredToggle, { propsData });
+  wrapper = mount(AttentionRequestedToggle, { propsData });
 }
 
 const findToggle = () => wrapper.findComponent(GlButton);
@@ -16,52 +16,52 @@ describe('Attention require toggle', () => {
   });
 
   it('renders button', () => {
-    factory({ type: 'reviewer', user: { attention_required: false } });
+    factory({ type: 'reviewer', user: { attention_requested: false } });
 
     expect(findToggle().exists()).toBe(true);
   });
 
   it.each`
-    attentionRequired | icon
-    ${true}           | ${'star'}
-    ${false}          | ${'star-o'}
+    attentionRequested | icon
+    ${true}            | ${'star'}
+    ${false}           | ${'star-o'}
   `(
-    'renders $icon icon when attention_required is $attentionRequired',
-    ({ attentionRequired, icon }) => {
-      factory({ type: 'reviewer', user: { attention_required: attentionRequired } });
+    'renders $icon icon when attention_requested is $attentionRequested',
+    ({ attentionRequested, icon }) => {
+      factory({ type: 'reviewer', user: { attention_requested: attentionRequested } });
 
       expect(findToggle().props('icon')).toBe(icon);
     },
   );
 
   it.each`
-    attentionRequired | variant
-    ${true}           | ${'warning'}
-    ${false}          | ${'default'}
+    attentionRequested | variant
+    ${true}            | ${'warning'}
+    ${false}           | ${'default'}
   `(
-    'renders button with variant $variant when attention_required is $attentionRequired',
-    ({ attentionRequired, variant }) => {
-      factory({ type: 'reviewer', user: { attention_required: attentionRequired } });
+    'renders button with variant $variant when attention_requested is $attentionRequested',
+    ({ attentionRequested, variant }) => {
+      factory({ type: 'reviewer', user: { attention_requested: attentionRequested } });
 
       expect(findToggle().props('variant')).toBe(variant);
     },
   );
 
-  it('emits toggle-attention-required on click', async () => {
-    factory({ type: 'reviewer', user: { attention_required: true } });
+  it('emits toggle-attention-requested on click', async () => {
+    factory({ type: 'reviewer', user: { attention_requested: true } });
 
     await findToggle().trigger('click');
 
-    expect(wrapper.emitted('toggle-attention-required')[0]).toEqual([
+    expect(wrapper.emitted('toggle-attention-requested')[0]).toEqual([
       {
-        user: { attention_required: true },
+        user: { attention_requested: true },
         callback: expect.anything(),
       },
     ]);
   });
 
   it('sets loading on click', async () => {
-    factory({ type: 'reviewer', user: { attention_required: true } });
+    factory({ type: 'reviewer', user: { attention_requested: true } });
 
     await findToggle().trigger('click');
 
@@ -69,14 +69,14 @@ describe('Attention require toggle', () => {
   });
 
   it.each`
-    type          | attentionRequired | tooltip
-    ${'reviewer'} | ${true}           | ${AttentionRequiredToggle.i18n.removeAttentionRequired}
-    ${'reviewer'} | ${false}          | ${AttentionRequiredToggle.i18n.attentionRequiredReviewer}
-    ${'assignee'} | ${false}          | ${AttentionRequiredToggle.i18n.attentionRequiredAssignee}
+    type          | attentionRequested | tooltip
+    ${'reviewer'} | ${true}            | ${AttentionRequestedToggle.i18n.removeAttentionRequested}
+    ${'reviewer'} | ${false}           | ${AttentionRequestedToggle.i18n.attentionRequestedReviewer}
+    ${'assignee'} | ${false}           | ${AttentionRequestedToggle.i18n.attentionRequestedAssignee}
   `(
-    'sets tooltip as $tooltip when attention_required is $attentionRequired and type is $type',
-    ({ type, attentionRequired, tooltip }) => {
-      factory({ type, user: { attention_required: attentionRequired } });
+    'sets tooltip as $tooltip when attention_requested is $attentionRequested and type is $type',
+    ({ type, attentionRequested, tooltip }) => {
+      factory({ type, user: { attention_requested: attentionRequested } });
 
       expect(findToggle().attributes('aria-label')).toBe(tooltip);
     },

spec/frontend/sidebar/components/reviewers/uncollapsed_reviewer_list_spec.js

 import { shallowMount } from '@vue/test-utils';
 import { TEST_HOST } from 'helpers/test_constants';
-import AttentionRequiredToggle from '~/sidebar/components/attention_required_toggle.vue';
+import AttentionRequestedToggle from '~/sidebar/components/attention_requested_toggle.vue';
 import ReviewerAvatarLink from '~/sidebar/components/reviewers/reviewer_avatar_link.vue';
 import UncollapsedReviewerList from '~/sidebar/components/reviewers/uncollapsed_reviewer_list.vue';
 import userDataMock from '../../user_data_mock';
@@ -121,11 +121,11 @@ describe('UncollapsedReviewerList component', () => {
     expect(wrapper.findAll('[data-testid="re-request-button"]').length).toBe(0);
   });
 
-  it('emits toggle-attention-required', () => {
+  it('emits toggle-attention-requested', () => {
     createComponent({ users: [userDataMock()] }, { mrAttentionRequests: true });
 
-    wrapper.find(AttentionRequiredToggle).vm.$emit('toggle-attention-required', 'data');
+    wrapper.find(AttentionRequestedToggle).vm.$emit('toggle-attention-requested', 'data');
 
-    expect(wrapper.emitted('toggle-attention-required')[0]).toEqual(['data']);
+    expect(wrapper.emitted('toggle-attention-requested')[0]).toEqual(['data']);
   });
 });

spec/frontend/sidebar/sidebar_mediator_spec.js

@@ -119,19 +119,19 @@ describe('Sidebar mediator', () => {
     });
   });
 
-  describe('toggleAttentionRequired', () => {
+  describe('toggleAttentionRequested', () => {
     let attentionRequiredService;
 
     beforeEach(() => {
       attentionRequiredService = jest
-        .spyOn(mediator.service, 'attentionRequired')
+        .spyOn(mediator.service, 'toggleAttentionRequested')
         .mockResolvedValue();
     });
 
     it('calls attentionRequired service method', async () => {
-      mediator.store.reviewers = [{ id: 1, attention_required: false, username: 'root' }];
+      mediator.store.reviewers = [{ id: 1, attention_requested: false, username: 'root' }];
 
-      await mediator.toggleAttentionRequired('reviewer', {
+      await mediator.toggleAttentionRequested('reviewer', {
         user: { id: 1, username: 'root' },
         callback: jest.fn(),
       });
@@ -145,23 +145,23 @@ describe('Sidebar mediator', () => {
     `('finds $type', ({ type, method }) => {
       const methodSpy = jest.spyOn(mediator.store, method);
 
-      mediator.toggleAttentionRequired(type, { user: { id: 1 }, callback: jest.fn() });
+      mediator.toggleAttentionRequested(type, { user: { id: 1 }, callback: jest.fn() });
 
       expect(methodSpy).toHaveBeenCalledWith({ id: 1 });
     });
 
     it.each`
-      attentionRequired | toastMessage
-      ${true}           | ${'Removed attention request from @root'}
-      ${false}          | ${'Requested attention from @root'}
+      attentionRequested | toastMessage
+      ${true}            | ${'Removed attention request from @root'}
+      ${false}           | ${'Requested attention from @root'}
     `(
-      'it creates toast $toastMessage when attention_required is $attentionRequired',
-      async ({ attentionRequired, toastMessage }) => {
+      'it creates toast $toastMessage when attention_requested is $attentionRequested',
+      async ({ attentionRequested, toastMessage }) => {
         mediator.store.reviewers = [
-          { id: 1, attention_required: attentionRequired, username: 'root' },
+          { id: 1, attention_requested: attentionRequested, username: 'root' },
         ];
 
-        await mediator.toggleAttentionRequired('reviewer', {
+        await mediator.toggleAttentionRequested('reviewer', {
           user: { id: 1, username: 'root' },
           callback: jest.fn(),
         });

spec/graphql/types/merge_request_review_state_enum_spec.rb

@@ -13,9 +13,9 @@ RSpec.describe GitlabSchema.types['MergeRequestReviewState'] do
         description: 'The merge request is unreviewed.',
         value: 'unreviewed'
       ),
-      'ATTENTION_REQUIRED' => have_attributes(
-        description: 'The merge request is attention_required.',
-        value: 'attention_required'
+      'ATTENTION_REQUESTED' => have_attributes(
+        description: 'The merge request is attention_requested.',
+        value: 'attention_requested'
       )
     )
   end

spec/graphql/types/user_merge_request_interaction_type_spec.rb

@@ -78,7 +78,7 @@ RSpec.describe GitlabSchema.types['UserMergeRequestInteraction'] do
         merge_request.reviewers << user
       end
 
-      it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['ATTENTION_REQUIRED'].value) }
+      it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['ATTENTION_REQUESTED'].value) }
 
       it 'implies not reviewed' do
         expect(resolve(:reviewed)).to be false

spec/models/users/merge_request_interaction_spec.rb

@@ -61,7 +61,7 @@ RSpec.describe ::Users::MergeRequestInteraction do
         merge_request.reviewers << user
       end
 
-      it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['ATTENTION_REQUIRED'].value) }
+      it { is_expected.to eq(Types::MergeRequestReviewStateEnum.values['ATTENTION_REQUESTED'].value) }
 
       it 'implies not reviewed' do
         expect(interaction).not_to be_reviewed

spec/requests/api/graphql/mutations/merge_requests/attention_required_spec.rb → spec/requests/api/graphql/mutations/merge_requests/update_reviewer_state_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe 'Setting attention required for reviewer' do
+RSpec.describe 'Toggle attention requested for reviewer' do
   include GraphqlHelpers
 
   let(:current_user) { create(:user) }
@@ -16,7 +16,7 @@ RSpec.describe 'Setting attention required for reviewer' do
       project_path: project.full_path,
       iid: merge_request.iid.to_s
     }
-    graphql_mutation(:merge_request_attention_required, variables.merge(input),
+    graphql_mutation(:merge_request_toggle_attention_requested, variables.merge(input),
                      <<-QL.strip_heredoc
                        clientMutationId
                        errors
@@ -25,7 +25,7 @@ RSpec.describe 'Setting attention required for reviewer' do
   end
 
   def mutation_response
-    graphql_mutation_response(:merge_request_attention_required)
+    graphql_mutation_response(:merge_request_toggle_attention_requested)
   end
 
   def mutation_errors

spec/requests/api/graphql/project/merge_request_spec.rb

@@ -347,7 +347,7 @@ RSpec.describe 'getting merge request information nested in a project' do
         expect(interaction_data).to contain_exactly a_hash_including(
           'canMerge' => false,
           'canUpdate' => can_update,
-          'reviewState' => attention_required,
+          'reviewState' => attention_requested,
           'reviewed' => false,
           'approved' => false
         )
@@ -380,8 +380,8 @@ RSpec.describe 'getting merge request information nested in a project' do
     describe 'scalability' do
       let_it_be(:other_users) { create_list(:user, 3) }
 
-      let(:attention_required) do
-        { 'reviewState' => 'ATTENTION_REQUIRED' }
+      let(:attention_requested) do
+        { 'reviewState' => 'ATTENTION_REQUESTED' }
       end
 
       let(:reviewed) do
@@ -413,9 +413,9 @@ RSpec.describe 'getting merge request information nested in a project' do
           expect { post_graphql(query) }.not_to exceed_query_limit(baseline)
 
           expect(interaction_data).to contain_exactly(
-            include(attention_required),
-            include(attention_required),
-            include(attention_required),
+            include(attention_requested),
+            include(attention_requested),
+            include(attention_requested),
             include(reviewed)
           )
         end
@@ -444,7 +444,7 @@ RSpec.describe 'getting merge request information nested in a project' do
 
   it_behaves_like 'when requesting information about MR interactions' do
     let(:field) { :reviewers }
-    let(:attention_required) { 'ATTENTION_REQUIRED' }
+    let(:attention_requested) { 'ATTENTION_REQUESTED' }
     let(:can_update) { false }
 
     def assign_user(user)
@@ -454,7 +454,7 @@ RSpec.describe 'getting merge request information nested in a project' do
 
   it_behaves_like 'when requesting information about MR interactions' do
     let(:field) { :assignees }
-    let(:attention_required) { nil }
+    let(:attention_requested) { nil }
     let(:can_update) { true } # assignees can update MRs
 
     def assign_user(user)

spec/serializers/merge_request_user_entity_spec.rb

@@ -19,7 +19,7 @@ RSpec.describe MergeRequestUserEntity do
       is_expected.to include(
         :id, :name, :username, :state, :avatar_url, :web_url,
         :can_merge, :can_update_merge_request, :reviewed, :approved,
-        :attention_required
+        :attention_requested
       )
     end
 
@@ -57,8 +57,8 @@ RSpec.describe MergeRequestUserEntity do
       end
     end
 
-    context 'attention_required' do
-      it { is_expected.to include(attention_required: true ) }
+    context 'attention_requested' do
+      it { is_expected.to include(attention_requested: true ) }
     end
 
     describe 'performance' do

spec/services/merge_requests/attention_required_service_spec.rb → spec/services/merge_requests/toggle_attention_requested_service_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe MergeRequests::AttentionRequiredService do
+RSpec.describe MergeRequests::ToggleAttentionRequestedService do
   let(:current_user) { create(:user) }
   let(:user) { create(:user) }
   let(:assignee_user) { create(:user) }
@@ -39,6 +39,10 @@ RSpec.describe MergeRequests::AttentionRequiredService do
     end
 
     context 'reviewer exists' do
+      before do
+        reviewer.update!(state: :reviewed)
+      end
+
       it 'returns success' do
         expect(result[:status]).to eq :success
       end
@@ -47,11 +51,11 @@ RSpec.describe MergeRequests::AttentionRequiredService do
         service.execute
         reviewer.reload
 
-        expect(reviewer.state).to eq 'attention_required'
+        expect(reviewer.state).to eq 'attention_requested'
       end
 
       it 'creates a new todo for the reviewer' do
-        expect(todo_service).to receive(:create_attention_required_todo).with(merge_request, current_user, user)
+        expect(todo_service).to receive(:create_attention_requested_todo).with(merge_request, current_user, user)
 
         service.execute
       end
@@ -60,6 +64,10 @@ RSpec.describe MergeRequests::AttentionRequiredService do
     context 'assignee exists' do
       let(:service) { described_class.new(project: project, current_user: current_user, merge_request: merge_request, user: assignee_user) }
 
+      before do
+        assignee.update!(state: :reviewed)
+      end
+
       it 'returns success' do
         expect(result[:status]).to eq :success
       end
@@ -68,11 +76,11 @@ RSpec.describe MergeRequests::AttentionRequiredService do
         service.execute
         assignee.reload
 
-        expect(assignee.state).to eq 'attention_required'
+        expect(assignee.state).to eq 'attention_requested'
       end
 
       it 'creates a new todo for the reviewer' do
-        expect(todo_service).to receive(:create_attention_required_todo).with(merge_request, current_user, assignee_user)
+        expect(todo_service).to receive(:create_attention_requested_todo).with(merge_request, current_user, assignee_user)
 
         service.execute
       end
@@ -83,13 +91,37 @@ RSpec.describe MergeRequests::AttentionRequiredService do
       let(:service) { described_class.new(project: project, current_user: current_user, merge_request: merge_request, user: user) }
       let(:assignee) { merge_request.find_assignee(user) }
 
+      before do
+        reviewer.update!(state: :reviewed)
+        assignee.update!(state: :reviewed)
+      end
+
       it 'updates reviewers and assignees state' do
         service.execute
         reviewer.reload
         assignee.reload
 
-        expect(reviewer.state).to eq 'attention_required'
-        expect(assignee.state).to eq 'attention_required'
+        expect(reviewer.state).to eq 'attention_requested'
+        expect(assignee.state).to eq 'attention_requested'
+      end
+    end
+
+    context 'state is attention_requested' do
+      before do
+        reviewer.update!(state: :attention_requested)
+      end
+
+      it 'toggles state to reviewed' do
+        service.execute
+        reviewer.reload
+
+        expect(reviewer.state).to eq "reviewed"
+      end
+
+      it 'does not create a new todo for the reviewer' do
+        expect(todo_service).not_to receive(:create_attention_requested_todo).with(merge_request, current_user, assignee_user)
+
+        service.execute
       end
     end
   end

spec/services/todo_service_spec.rb

@@ -1218,14 +1218,14 @@ RSpec.describe TodoService do
     end
   end
 
-  describe '#create_attention_required_todo' do
+  describe '#create_attention_requested_todo' do
     let(:target) { create(:merge_request, author: author, source_project: project) }
     let(:user) { create(:user) }
 
     it 'creates a todo for user' do
-      service.create_attention_required_todo(target, author, user)
+      service.create_attention_requested_todo(target, author, user)
 
-      should_create_todo(user: user, target: target, action: Todo::ATTENTION_REQUIRED)
+      should_create_todo(user: user, target: target, action: Todo::ATTENTION_REQUESTED)
     end
   end
 

spec/support/shared_examples/models/reviewer_state_shared_examples.rb

@@ -10,6 +10,6 @@ RSpec.shared_examples 'having reviewer state' do
   end
 
   describe 'mr_attention_requests feature flag is enabled' do
-    it { is_expected.to have_attributes(state: 'attention_required') }
+    it { is_expected.to have_attributes(state: 'attention_requested') }
   end
 end