Merge branch 'ph/removeMergeRequestReviewerState' into 'master'

Toggle assignee and reviewer attention requested state

See merge request gitlab-org/gitlab!74176

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.
@@ -16474,7 +16474,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