Commit bcee7231 authored by Phil Hughes's avatar Phil Hughes

Added GraphQL API mutation for requiring attention

Adds the GraphQL mutation that will mark the assignee
or reviewer as requiring attention.
Creates a todo item when the users attention is required.

https://gitlab.com/gitlab-org/gitlab/-/issues/343325
parent 2397a248
# 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
......@@ -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
......
......@@ -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
......
......@@ -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
......
......@@ -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
......
# 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
......@@ -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)
......
......@@ -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`
......
# 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
# 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
......@@ -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,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment