Commit 27a7ae44 authored by David Kim's avatar David Kim

Deprecate SetWip GraphQL mutation and add SetDraft mutation

Changelog: deprecated
parent 2268d9f2
# frozen_string_literal: true
module Mutations
module MergeRequests
class SetDraft < Base
graphql_name 'MergeRequestSetDraft'
argument :draft,
GraphQL::BOOLEAN_TYPE,
required: true,
description: <<~DESC
Whether or not to set the merge request as a draft.
DESC
def resolve(project_path:, iid:, draft: nil)
merge_request = authorized_find!(project_path: project_path, iid: iid)
project = merge_request.project
::MergeRequests::UpdateService.new(project, current_user, wip_event: wip_event(draft))
.execute(merge_request)
{
merge_request: merge_request,
errors: errors_on_object(merge_request)
}
end
private
def wip_event(wip)
wip ? 'wip' : 'unwip'
end
end
end
end
...@@ -54,6 +54,9 @@ module Types ...@@ -54,6 +54,9 @@ module Types
field :target_branch, GraphQL::STRING_TYPE, null: false, field :target_branch, GraphQL::STRING_TYPE, null: false,
description: 'Target branch of the merge request.' description: 'Target branch of the merge request.'
field :work_in_progress, GraphQL::BOOLEAN_TYPE, method: :work_in_progress?, null: false, field :work_in_progress, GraphQL::BOOLEAN_TYPE, method: :work_in_progress?, null: false,
deprecated: { reason: 'Use `draft`', milestone: '13.12' },
description: 'Indicates if the merge request is a draft.'
field :draft, GraphQL::BOOLEAN_TYPE, method: :draft?, null: false,
description: 'Indicates if the merge request is a draft.' description: 'Indicates if the merge request is a draft.'
field :merge_when_pipeline_succeeds, GraphQL::BOOLEAN_TYPE, null: true, field :merge_when_pipeline_succeeds, GraphQL::BOOLEAN_TYPE, null: true,
description: 'Indicates if the merge has been set to be merged when its pipeline succeeds (MWPS).' description: 'Indicates if the merge has been set to be merged when its pipeline succeeds (MWPS).'
......
...@@ -52,7 +52,10 @@ module Types ...@@ -52,7 +52,10 @@ module Types
mount_mutation Mutations::MergeRequests::SetLocked mount_mutation Mutations::MergeRequests::SetLocked
mount_mutation Mutations::MergeRequests::SetMilestone mount_mutation Mutations::MergeRequests::SetMilestone
mount_mutation Mutations::MergeRequests::SetSubscription mount_mutation Mutations::MergeRequests::SetSubscription
mount_mutation Mutations::MergeRequests::SetWip, calls_gitaly: true mount_mutation Mutations::MergeRequests::SetWip,
calls_gitaly: true,
deprecated: { reason: 'Use mergeRequestSetDraft', milestone: '13.12' }
mount_mutation Mutations::MergeRequests::SetDraft, calls_gitaly: true
mount_mutation Mutations::MergeRequests::SetAssignees mount_mutation Mutations::MergeRequests::SetAssignees
mount_mutation Mutations::MergeRequests::ReviewerRereview mount_mutation Mutations::MergeRequests::ReviewerRereview
mount_mutation Mutations::Metrics::Dashboard::Annotations::Create mount_mutation Mutations::Metrics::Dashboard::Annotations::Create
......
---
title: Deprecate SetWip GraphQL mutation and add SetDraft mutation
merge_request: 60803
author:
type: deprecated
...@@ -2725,6 +2725,27 @@ Input type: `MergeRequestSetAssigneesInput` ...@@ -2725,6 +2725,27 @@ Input type: `MergeRequestSetAssigneesInput`
| <a id="mutationmergerequestsetassigneeserrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | | <a id="mutationmergerequestsetassigneeserrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
| <a id="mutationmergerequestsetassigneesmergerequest"></a>`mergeRequest` | [`MergeRequest`](#mergerequest) | The merge request after mutation. | | <a id="mutationmergerequestsetassigneesmergerequest"></a>`mergeRequest` | [`MergeRequest`](#mergerequest) | The merge request after mutation. |
### `Mutation.mergeRequestSetDraft`
Input type: `MergeRequestSetDraftInput`
#### Arguments
| Name | Type | Description |
| ---- | ---- | ----------- |
| <a id="mutationmergerequestsetdraftclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
| <a id="mutationmergerequestsetdraftdraft"></a>`draft` | [`Boolean!`](#boolean) | Whether or not to set the merge request as a draft. |
| <a id="mutationmergerequestsetdraftiid"></a>`iid` | [`String!`](#string) | The IID of the merge request to mutate. |
| <a id="mutationmergerequestsetdraftprojectpath"></a>`projectPath` | [`ID!`](#id) | The project the merge request to mutate is in. |
#### Fields
| Name | Type | Description |
| ---- | ---- | ----------- |
| <a id="mutationmergerequestsetdraftclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
| <a id="mutationmergerequestsetdrafterrors"></a>`errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. |
| <a id="mutationmergerequestsetdraftmergerequest"></a>`mergeRequest` | [`MergeRequest`](#mergerequest) | The merge request after mutation. |
### `Mutation.mergeRequestSetLabels` ### `Mutation.mergeRequestSetLabels`
Input type: `MergeRequestSetLabelsInput` Input type: `MergeRequestSetLabelsInput`
...@@ -2812,6 +2833,10 @@ Input type: `MergeRequestSetSubscriptionInput` ...@@ -2812,6 +2833,10 @@ Input type: `MergeRequestSetSubscriptionInput`
### `Mutation.mergeRequestSetWip` ### `Mutation.mergeRequestSetWip`
WARNING:
**Deprecated** in 13.12.
Use mergeRequestSetDraft.
Input type: `MergeRequestSetWipInput` Input type: `MergeRequestSetWipInput`
#### Arguments #### Arguments
...@@ -9538,6 +9563,7 @@ Represents an entry from the Cloud License history. ...@@ -9538,6 +9563,7 @@ Represents an entry from the Cloud License history.
| <a id="mergerequestdiscussions"></a>`discussions` | [`DiscussionConnection!`](#discussionconnection) | All discussions on this noteable. (see [Connections](#connections)) | | <a id="mergerequestdiscussions"></a>`discussions` | [`DiscussionConnection!`](#discussionconnection) | All discussions on this noteable. (see [Connections](#connections)) |
| <a id="mergerequestdivergedfromtargetbranch"></a>`divergedFromTargetBranch` | [`Boolean!`](#boolean) | Indicates if the source branch is behind the target branch. | | <a id="mergerequestdivergedfromtargetbranch"></a>`divergedFromTargetBranch` | [`Boolean!`](#boolean) | Indicates if the source branch is behind the target branch. |
| <a id="mergerequestdownvotes"></a>`downvotes` | [`Int!`](#int) | Number of downvotes for the merge request. | | <a id="mergerequestdownvotes"></a>`downvotes` | [`Int!`](#int) | Number of downvotes for the merge request. |
| <a id="mergerequestdraft"></a>`draft` | [`Boolean!`](#boolean) | Indicates if the merge request is a draft. |
| <a id="mergerequestforceremovesourcebranch"></a>`forceRemoveSourceBranch` | [`Boolean`](#boolean) | Indicates if the project settings will lead to source branch deletion after merge. | | <a id="mergerequestforceremovesourcebranch"></a>`forceRemoveSourceBranch` | [`Boolean`](#boolean) | Indicates if the project settings will lead to source branch deletion after merge. |
| <a id="mergerequesthasci"></a>`hasCi` | [`Boolean!`](#boolean) | Indicates if the merge request has CI. | | <a id="mergerequesthasci"></a>`hasCi` | [`Boolean!`](#boolean) | Indicates if the merge request has CI. |
| <a id="mergerequesthassecurityreports"></a>`hasSecurityReports` | [`Boolean!`](#boolean) | Indicates if the source branch has any security reports. | | <a id="mergerequesthassecurityreports"></a>`hasSecurityReports` | [`Boolean!`](#boolean) | Indicates if the source branch has any security reports. |
...@@ -9593,7 +9619,7 @@ Represents an entry from the Cloud License history. ...@@ -9593,7 +9619,7 @@ Represents an entry from the Cloud License history.
| <a id="mergerequestusernotescount"></a>`userNotesCount` | [`Int`](#int) | User notes count of the merge request. | | <a id="mergerequestusernotescount"></a>`userNotesCount` | [`Int`](#int) | User notes count of the merge request. |
| <a id="mergerequestuserpermissions"></a>`userPermissions` | [`MergeRequestPermissions!`](#mergerequestpermissions) | Permissions for the current user on the resource. | | <a id="mergerequestuserpermissions"></a>`userPermissions` | [`MergeRequestPermissions!`](#mergerequestpermissions) | Permissions for the current user on the resource. |
| <a id="mergerequestweburl"></a>`webUrl` | [`String`](#string) | Web URL of the merge request. | | <a id="mergerequestweburl"></a>`webUrl` | [`String`](#string) | Web URL of the merge request. |
| <a id="mergerequestworkinprogress"></a>`workInProgress` | [`Boolean!`](#boolean) | Indicates if the merge request is a draft. | | <a id="mergerequestworkinprogress"></a>`workInProgress` **{warning-solid}** | [`Boolean!`](#boolean) | **Deprecated** in 13.12. Use `draft`. |
#### Fields with arguments #### Fields with arguments
......
...@@ -1185,7 +1185,7 @@ are returned as the result of the mutation. ...@@ -1185,7 +1185,7 @@ are returned as the result of the mutation.
The service-oriented architecture in GitLab means that most mutations call a Create, Delete, or Update The service-oriented architecture in GitLab means that most mutations call a Create, Delete, or Update
service, for example `UpdateMergeRequestService`. service, for example `UpdateMergeRequestService`.
For Update mutations, you might want to only update one aspect of an object, and thus only need a For Update mutations, you might want to only update one aspect of an object, and thus only need a
_fine-grained_ mutation, for example `MergeRequest::SetWip`. _fine-grained_ mutation, for example `MergeRequest::SetDraft`.
It's acceptable to have both fine-grained mutations and coarse-grained mutations, but be aware It's acceptable to have both fine-grained mutations and coarse-grained mutations, but be aware
that too many fine-grained mutations can lead to organizational challenges in maintainability, code that too many fine-grained mutations can lead to organizational challenges in maintainability, code
...@@ -1281,7 +1281,7 @@ end ...@@ -1281,7 +1281,7 @@ end
[input type](https://graphql.org/learn/schema/#input-types). [input type](https://graphql.org/learn/schema/#input-types).
For example, the For example, the
[`mergeRequestSetWip` mutation](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/graphql/mutations/merge_requests/set_wip.rb) [`mergeRequestSetDraft` mutation](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/graphql/mutations/merge_requests/set_draft.rb)
defines these arguments (some defines these arguments (some
[through inheritance](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/graphql/mutations/merge_requests/base.rb)): [through inheritance](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/graphql/mutations/merge_requests/base.rb)):
...@@ -1294,17 +1294,16 @@ argument :iid, GraphQL::STRING_TYPE, ...@@ -1294,17 +1294,16 @@ argument :iid, GraphQL::STRING_TYPE,
required: true, required: true,
description: "The IID of the merge request to mutate." description: "The IID of the merge request to mutate."
argument :wip, argument :draft,
GraphQL::BOOLEAN_TYPE, GraphQL::BOOLEAN_TYPE,
required: false, required: false,
description: <<~DESC description: <<~DESC
Whether or not to set the merge request as a WIP. Whether or not to set the merge request as a draft.
If not passed, the value will be toggled. DESC
DESC
``` ```
These arguments automatically generate an input type called These arguments automatically generate an input type called
`MergeRequestSetWipInput` with the 3 arguments we specified and the `MergeRequestSetDraftInput` with the 3 arguments we specified and the
`clientMutationId`. `clientMutationId`.
### Object identifier arguments ### Object identifier arguments
...@@ -1341,7 +1340,7 @@ From here, we can call the service that modifies the resource. ...@@ -1341,7 +1340,7 @@ From here, we can call the service that modifies the resource.
The `resolve` method should then return a hash with the same field The `resolve` method should then return a hash with the same field
names as defined on the mutation including an `errors` array. For example, names as defined on the mutation including an `errors` array. For example,
the `Mutations::MergeRequests::SetWip` defines a `merge_request` the `Mutations::MergeRequests::SetDraft` defines a `merge_request`
field: field:
```ruby ```ruby
...@@ -1379,13 +1378,13 @@ module Types ...@@ -1379,13 +1378,13 @@ module Types
graphql_name "Mutation" graphql_name "Mutation"
mount_mutation Mutations::MergeRequests::SetWip mount_mutation Mutations::MergeRequests::SetDraft
end end
end end
``` ```
Generates a field called `mergeRequestSetWip` that Generates a field called `mergeRequestSetDraft` that
`Mutations::MergeRequests::SetWip` to be resolved. `Mutations::MergeRequests::SetDraft` to be resolved.
### Authorizing resources ### Authorizing resources
...@@ -1395,8 +1394,8 @@ To authorize resources inside a mutation, we first provide the required ...@@ -1395,8 +1394,8 @@ To authorize resources inside a mutation, we first provide the required
```ruby ```ruby
module Mutations module Mutations
module MergeRequests module MergeRequests
class SetWip < Base class SetDraft < Base
graphql_name 'MergeRequestSetWip' graphql_name 'MergeRequestSetDraft'
authorize :update_merge_request authorize :update_merge_request
end end
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Mutations::MergeRequests::SetDraft do
let(:merge_request) { create(:merge_request) }
let(:user) { create(:user) }
subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) }
specify { expect(described_class).to require_graphql_authorizations(:update_merge_request) }
describe '#resolve' do
let(:draft) { true }
let(:mutated_merge_request) { subject[:merge_request] }
subject { mutation.resolve(project_path: merge_request.project.full_path, iid: merge_request.iid, draft: draft) }
it_behaves_like 'permission level for merge request mutation is correctly verified'
context 'when the user can update the merge request' do
before do
merge_request.project.add_developer(user)
end
it 'returns the merge request as a draft' do
expect(mutated_merge_request).to eq(merge_request)
expect(mutated_merge_request).to be_draft
expect(subject[:errors]).to be_empty
end
it 'returns errors merge request could not be updated' do
# Make the merge request invalid
merge_request.allow_broken = true
merge_request.update!(source_project: nil)
expect(subject[:errors]).not_to be_empty
end
context 'when passing draft as false' do
let(:draft) { false }
it 'removes `Draft` from the title' do
merge_request.update!(title: "Draft: working on it")
expect(mutated_merge_request).not_to be_draft
end
it 'does not do anything if the title did not start with draft' do
expect(mutated_merge_request).not_to be_draft
end
end
end
end
end
...@@ -16,7 +16,7 @@ RSpec.describe GitlabSchema.types['MergeRequest'] do ...@@ -16,7 +16,7 @@ RSpec.describe GitlabSchema.types['MergeRequest'] do
notes discussions user_permissions id iid title title_html description notes discussions user_permissions id iid title title_html description
description_html state created_at updated_at source_project target_project description_html state created_at updated_at source_project target_project
project project_id source_project_id target_project_id source_branch project project_id source_project_id target_project_id source_branch
target_branch work_in_progress merge_when_pipeline_succeeds diff_head_sha target_branch work_in_progress draft merge_when_pipeline_succeeds diff_head_sha
merge_commit_sha user_notes_count user_discussions_count should_remove_source_branch merge_commit_sha user_notes_count user_discussions_count should_remove_source_branch
diff_refs diff_stats diff_stats_summary diff_refs diff_stats diff_stats_summary
force_remove_source_branch merge_status in_progress_merge_commit_sha force_remove_source_branch merge_status in_progress_merge_commit_sha
......
...@@ -3,8 +3,16 @@ ...@@ -3,8 +3,16 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Types::MutationType do RSpec.describe Types::MutationType do
it 'is expected to have the MergeRequestSetWip' do it 'is expected to have the deprecated MergeRequestSetWip' do
expect(described_class).to have_graphql_mutation(Mutations::MergeRequests::SetWip) field = get_field('MergeRequestSetWip')
expect(field).to be_present
expect(field.deprecation_reason).to be_present
expect(field.resolver).to eq(Mutations::MergeRequests::SetWip)
end
it 'is expected to have the MergeRequestSetDraft' do
expect(described_class).to have_graphql_mutation(Mutations::MergeRequests::SetDraft)
end end
describe 'deprecated and aliased mutations' do describe 'deprecated and aliased mutations' do
...@@ -27,9 +35,9 @@ RSpec.describe Types::MutationType do ...@@ -27,9 +35,9 @@ RSpec.describe Types::MutationType do
it { expect(alias_field.resolver.fields).to eq(canonical_field.resolver.fields) } it { expect(alias_field.resolver.fields).to eq(canonical_field.resolver.fields) }
it { expect(alias_field.resolver.arguments).to eq(canonical_field.resolver.arguments) } it { expect(alias_field.resolver.arguments).to eq(canonical_field.resolver.arguments) }
end end
end
def get_field(name) def get_field(name)
described_class.fields[GraphqlHelpers.fieldnamerize(name)] described_class.fields[GraphqlHelpers.fieldnamerize(name)]
end
end end
end end
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