Merge branch '321892-graphql-updates-to-remove-wip-in-favor-of-draft' into 'master'

Update GraphQL to deprecate WIP in favor of Draft

See merge request gitlab-org/gitlab!60803

app/graphql/mutations/merge_requests/set_draft.rb

+# 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

app/graphql/types/merge_request_type.rb

@@ -54,6 +54,9 @@ module Types
     field :target_branch, GraphQL::STRING_TYPE, null: false,
           description: 'Target branch of the merge request.'
     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.'
     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).'

app/graphql/types/mutation_type.rb

@@ -52,7 +52,10 @@ module Types
     mount_mutation Mutations::MergeRequests::SetLocked
     mount_mutation Mutations::MergeRequests::SetMilestone
     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::ReviewerRereview
     mount_mutation Mutations::Metrics::Dashboard::Annotations::Create

changelogs/unreleased/321892-graphql-updates-to-remove-wip-in-favor-of-draft.yml

+---
+title: Deprecate SetWip GraphQL mutation and add SetDraft mutation
+merge_request: 60803
+author:
+type: deprecated

doc/api/graphql/reference/index.md

@@ -2781,6 +2781,27 @@ Input type: `MergeRequestSetAssigneesInput`
 | <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. |
 
+### `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`
 
 Input type: `MergeRequestSetLabelsInput`
@@ -2868,6 +2889,10 @@ Input type: `MergeRequestSetSubscriptionInput`
 
 ### `Mutation.mergeRequestSetWip`
 
+WARNING:
+**Deprecated** in 13.12.
+Use mergeRequestSetDraft.
+
 Input type: `MergeRequestSetWipInput`
 
 #### Arguments
@@ -9668,6 +9693,7 @@ Maven metadata.
 | <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="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="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. |
@@ -9723,7 +9749,7 @@ Maven metadata.
 | <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="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
 

doc/development/api_graphql_styleguide.md

@@ -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
 service, for example `UpdateMergeRequestService`.
 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
 that too many fine-grained mutations can lead to organizational challenges in maintainability, code
@@ -1281,7 +1281,7 @@ end
 [input type](https://graphql.org/learn/schema/#input-types).
 
 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
 [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,
          required: true,
          description: "The IID of the merge request to mutate."
 
-argument :wip,
+argument :draft,
          GraphQL::BOOLEAN_TYPE,
          required: false,
          description: <<~DESC
-                      Whether or not to set the merge request as a WIP.
-                      If not passed, the value will be toggled.
-                      DESC
+           Whether or not to set the merge request as a draft.
+         DESC
 ```
 
 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`.
 
 ### Object identifier arguments
@@ -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
 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:
 
 ```ruby
@@ -1379,13 +1378,13 @@ module Types
 
     graphql_name "Mutation"
 
-    mount_mutation Mutations::MergeRequests::SetWip
+    mount_mutation Mutations::MergeRequests::SetDraft
   end
 end
 ```
 
-Generates a field called `mergeRequestSetWip` that
-`Mutations::MergeRequests::SetWip` to be resolved.
+Generates a field called `mergeRequestSetDraft` that
+`Mutations::MergeRequests::SetDraft` to be resolved.
 
 ### Authorizing resources
 
@@ -1395,8 +1394,8 @@ To authorize resources inside a mutation, we first provide the required
 ```ruby
 module Mutations
   module MergeRequests
-    class SetWip < Base
-      graphql_name 'MergeRequestSetWip'
+    class SetDraft < Base
+      graphql_name 'MergeRequestSetDraft'
 
       authorize :update_merge_request
     end

spec/graphql/mutations/merge_requests/set_draft_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Mutations::MergeRequests::SetDraft do
+  let_it_be(:merge_request) { create(:merge_request) }
+  let_it_be(: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 if/when 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

spec/graphql/types/merge_request_type_spec.rb

@@ -16,7 +16,7 @@ RSpec.describe GitlabSchema.types['MergeRequest'] do
       notes discussions user_permissions id iid title title_html description
       description_html state created_at updated_at source_project target_project
       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
       diff_refs diff_stats diff_stats_summary
       force_remove_source_branch merge_status in_progress_merge_commit_sha

spec/graphql/types/mutation_type_spec.rb

@@ -3,8 +3,16 @@
 require 'spec_helper'
 
 RSpec.describe Types::MutationType do
-  it 'is expected to have the MergeRequestSetWip' do
-    expect(described_class).to have_graphql_mutation(Mutations::MergeRequests::SetWip)
+  it 'is expected to have the deprecated MergeRequestSetWip' do
+    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
 
   describe 'deprecated and aliased mutations' 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.arguments).to eq(canonical_field.resolver.arguments) }
     end
+  end
 
-    def get_field(name)
-      described_class.fields[GraphqlHelpers.fieldnamerize(name)]
-    end
+  def get_field(name)
+    described_class.fields[GraphqlHelpers.fieldnamerize(name)]
   end
 end