Merge branch 'ph/343325/attentionRequiredGraphqlAPI' into 'master'

Added GraphQL API mutation for requiring attention

See merge request gitlab-org/gitlab!72990

app/graphql/mutations/merge_requests/attention_required.rb

+# frozen_string_literal: true
+
+module Mutations
+  module MergeRequests
+    class AttentionRequired < Base
+      graphql_name 'MergeRequestAttentionRequired'
+
+      argument :user_id, ::Types::GlobalIDType[::User],
+               loads: Types::UserType,
+               required: true,
+               description: <<~DESC
+                            User ID for the user that has their 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
+
+        {
+          merge_request: merge_request,
+          errors: Array(result[:message])
+        }
+      end
+    end
+  end
+end

app/graphql/types/mutation_type.rb

@@ -69,6 +69,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::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,6 +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'
     end
   end
 

app/models/merge_request.rb

@@ -1919,6 +1919,10 @@ class MergeRequest < ApplicationRecord
     true
   end
 
+  def find_assignee(user)
+    merge_request_assignees.find_by(user_id: user.id)
+  end
+
   def find_reviewer(user)
     merge_request_reviewers.find_by(user_id: user.id)
   end

app/models/todo.rb

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

app/services/merge_requests/attention_required_service.rb

+# frozen_string_literal: true
+
+module MergeRequests
+  class AttentionRequiredService < MergeRequests::BaseService
+    attr_accessor :merge_request, :user
+
+    def initialize(project:, current_user:, merge_request:, user:)
+      super(project: project, current_user: current_user)
+
+      @merge_request = merge_request
+      @user = user
+    end
+
+    def execute
+      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)
+
+        notity_user
+
+        success
+      else
+        error("User is not a reviewer or assignee of the merge request")
+      end
+    end
+
+    private
+
+    def notity_user
+      todo_service.create_attention_required_todo(merge_request, current_user, user)
+    end
+
+    def assignee
+      merge_request.find_assignee(user)
+    end
+
+    def reviewer
+      merge_request.find_reviewer(user)
+    end
+  end
+end

app/services/todo_service.rb

@@ -217,6 +217,11 @@ 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)
+    create_todos(users, attributes)
+  end
+
   private
 
   def create_todos(users, attributes)

doc/api/graphql/reference/index.md

@@ -3228,6 +3228,29 @@ 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`

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe 'Setting attention required for reviewer' do
+  include GraphqlHelpers
+
+  let(:current_user) { create(:user) }
+  let(:merge_request) { create(:merge_request, reviewers: [user]) }
+  let(:project) { merge_request.project }
+  let(:user) { create(:user) }
+  let(:input) { { user_id: global_id_of(user) } }
+
+  let(:mutation) do
+    variables = {
+      project_path: project.full_path,
+      iid: merge_request.iid.to_s
+    }
+    graphql_mutation(:merge_request_attention_required, variables.merge(input),
+                     <<-QL.strip_heredoc
+                       clientMutationId
+                       errors
+    QL
+    )
+  end
+
+  def mutation_response
+    graphql_mutation_response(:merge_request_attention_required)
+  end
+
+  def mutation_errors
+    mutation_response['errors']
+  end
+
+  before do
+    project.add_developer(current_user)
+    project.add_developer(user)
+  end
+
+  it 'returns an error if the user is not allowed to update the merge request' do
+    post_graphql_mutation(mutation, current_user: create(:user))
+
+    expect(graphql_errors).not_to be_empty
+  end
+
+  describe 'reviewer does not exist' do
+    let(:input) { { user_id: global_id_of(create(:user)) } }
+
+    it 'returns an error' do
+      post_graphql_mutation(mutation, current_user: current_user)
+
+      expect(response).to have_gitlab_http_status(:success)
+      expect(mutation_errors).not_to be_empty
+    end
+  end
+
+  describe 'reviewer exists' do
+    it 'does not return an error' do
+      post_graphql_mutation(mutation, current_user: current_user)
+
+      expect(response).to have_gitlab_http_status(:success)
+      expect(mutation_errors).to be_empty
+    end
+  end
+end

spec/services/merge_requests/attention_required_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe MergeRequests::AttentionRequiredService do
+  let(:current_user) { create(:user) }
+  let(:user) { create(:user) }
+  let(:assignee_user) { create(:user) }
+  let(:merge_request) { create(:merge_request, reviewers: [user], assignees: [assignee_user]) }
+  let(:reviewer) { merge_request.find_reviewer(user) }
+  let(:assignee) { merge_request.find_assignee(assignee_user) }
+  let(:project) { merge_request.project }
+  let(:service) { described_class.new(project: project, current_user: current_user, merge_request: merge_request, user: user) }
+  let(:result) { service.execute }
+  let(:todo_service) { spy('todo service') }
+
+  before do
+    allow(service).to receive(:todo_service).and_return(todo_service)
+
+    project.add_developer(current_user)
+    project.add_developer(user)
+  end
+
+  describe '#execute' do
+    context 'invalid permissions' do
+      let(:service) { described_class.new(project: project, current_user: create(:user), merge_request: merge_request, user: user) }
+
+      it 'returns an error' do
+        expect(result[:status]).to eq :error
+      end
+    end
+
+    context 'reviewer does not exist' do
+      let(:service) { described_class.new(project: project, current_user: current_user, merge_request: merge_request, user: create(:user)) }
+
+      it 'returns an error' do
+        expect(result[:status]).to eq :error
+      end
+    end
+
+    context 'reviewer exists' do
+      it 'returns success' do
+        expect(result[:status]).to eq :success
+      end
+
+      it 'updates reviewers state' do
+        service.execute
+        reviewer.reload
+
+        expect(reviewer.state).to eq 'attention_required'
+      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)
+
+        service.execute
+      end
+    end
+
+    context 'assignee exists' do
+      let(:service) { described_class.new(project: project, current_user: current_user, merge_request: merge_request, user: assignee_user) }
+
+      it 'returns success' do
+        expect(result[:status]).to eq :success
+      end
+
+      it 'updates assignees state' do
+        service.execute
+        assignee.reload
+
+        expect(assignee.state).to eq 'attention_required'
+      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)
+
+        service.execute
+      end
+    end
+
+    context 'assignee is the same as reviewer' do
+      let(:merge_request) { create(:merge_request, reviewers: [user], assignees: [user]) }
+      let(:service) { described_class.new(project: project, current_user: current_user, merge_request: merge_request, user: user) }
+      let(:assignee) { merge_request.find_assignee(user) }
+
+      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'
+      end
+    end
+  end
+end

spec/services/todo_service_spec.rb

@@ -1218,6 +1218,17 @@ RSpec.describe TodoService do
     end
   end
 
+  describe '#create_attention_required_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)
+
+      should_create_todo(user: user, target: target, action: Todo::ATTENTION_REQUIRED)
+    end
+  end
+
   def should_create_todo(attributes = {})
     attributes.reverse_merge!(
       project: project,