From 29a85cfe949c428afe362c2b8dd2e54b8e497ee7 Mon Sep 17 00:00:00 2001 From: David Kim Date: Fri, 29 Oct 2021 20:33:29 +1030 Subject: [PATCH] Remove deprecated WIP from GraphQL We deprecated WIP in favor of Draft in 13.12. We are removing it completely now. Changelog: removed --- .../components/states/work_in_progress.vue | 32 +++++------ .../queries/get_state.query.graphql | 2 +- ...ress.query.graphql => draft.query.graphql} | 0 .../queries/toggle_draft.mutation.graphql | 10 ++++ .../queries/toggle_wip.mutation.graphql | 10 ---- .../stores/get_state_key.js | 4 +- .../stores/mr_widget_store.js | 4 +- .../stores/state_maps.js | 6 +- .../mutations/merge_requests/set_wip.rb | 35 ------------ app/graphql/types/merge_request_type.rb | 3 - app/graphql/types/mutation_type.rb | 3 - doc/api/graphql/reference/index.md | 26 --------- .../components/states/mr_widget_wip_spec.js | 8 +-- .../stores/get_state_key_spec.js | 8 +-- .../mutations/merge_requests/set_wip_spec.rb | 55 ------------------- spec/graphql/types/merge_request_type_spec.rb | 2 +- spec/graphql/types/mutation_type_spec.rb | 8 --- .../{set_wip_spec.rb => set_draft_spec.rb} | 8 +-- 18 files changed, 47 insertions(+), 177 deletions(-) rename app/assets/javascripts/vue_merge_request_widget/queries/states/{work_in_progress.query.graphql => draft.query.graphql} (100%) create mode 100644 app/assets/javascripts/vue_merge_request_widget/queries/toggle_draft.mutation.graphql delete mode 100644 app/assets/javascripts/vue_merge_request_widget/queries/toggle_wip.mutation.graphql delete mode 100644 app/graphql/mutations/merge_requests/set_wip.rb delete mode 100644 spec/graphql/mutations/merge_requests/set_wip_spec.rb rename spec/requests/api/graphql/mutations/merge_requests/{set_wip_spec.rb => set_draft_spec.rb} (91%) diff --git a/app/assets/javascripts/vue_merge_request_widget/components/states/work_in_progress.vue b/app/assets/javascripts/vue_merge_request_widget/components/states/work_in_progress.vue index 790870ee4c6fa2..fa4f8b76cb980f 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/states/work_in_progress.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/states/work_in_progress.vue @@ -10,8 +10,8 @@ import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import eventHub from '../../event_hub'; import mergeRequestQueryVariablesMixin from '../../mixins/merge_request_query_variables'; import getStateQuery from '../../queries/get_state.query.graphql'; -import workInProgressQuery from '../../queries/states/work_in_progress.query.graphql'; -import removeWipMutation from '../../queries/toggle_wip.mutation.graphql'; +import draftQuery from '../../queries/states/draft.query.graphql'; +import removeDraftMutation from '../../queries/toggle_draft.mutation.graphql'; import StatusIcon from '../mr_widget_status_icon.vue'; export default { @@ -23,7 +23,7 @@ export default { mixins: [glFeatureFlagMixin(), mergeRequestQueryVariablesMixin], apollo: { userPermissions: { - query: workInProgressQuery, + query: draftQuery, skip() { return !this.glFeatures.mergeRequestWidgetGraphql; }, @@ -53,25 +53,25 @@ export default { }, }, methods: { - removeWipMutation() { + removeDraftMutation() { const { mergeRequestQueryVariables } = this; this.isMakingRequest = true; this.$apollo .mutate({ - mutation: removeWipMutation, + mutation: removeDraftMutation, variables: { ...mergeRequestQueryVariables, - wip: false, + draft: false, }, update( store, { data: { - mergeRequestSetWip: { + mergeRequestSetDraft: { errors, - mergeRequest: { mergeableDiscussionsState, workInProgress, title }, + mergeRequest: { mergeableDiscussionsState, draft, title }, }, }, }, @@ -91,7 +91,7 @@ export default { const data = produce(sourceData, (draftState) => { draftState.project.mergeRequest.mergeableDiscussionsState = mergeableDiscussionsState; - draftState.project.mergeRequest.workInProgress = workInProgress; + draftState.project.mergeRequest.draft = draft; draftState.project.mergeRequest.title = title; }); @@ -104,14 +104,14 @@ export default { optimisticResponse: { // eslint-disable-next-line @gitlab/require-i18n-strings __typename: 'Mutation', - mergeRequestSetWip: { + mergeRequestSetDraft: { __typename: 'MergeRequestSetWipPayload', errors: [], mergeRequest: { __typename: 'MergeRequest', mergeableDiscussionsState: true, title: this.mr.title, - workInProgress: false, + draft: false, }, }, }, @@ -119,7 +119,7 @@ export default { .then( ({ data: { - mergeRequestSetWip: { + mergeRequestSetDraft: { mergeRequest: { title }, }, }, @@ -137,9 +137,9 @@ export default { this.isMakingRequest = false; }); }, - handleRemoveWIP() { + handleRemoveDraft() { if (this.glFeatures.mergeRequestWidgetGraphql) { - this.removeWipMutation(); + this.removeDraftMutation(); } else { this.isMakingRequest = true; this.service @@ -178,8 +178,8 @@ export default { size="small" :disabled="isMakingRequest" :loading="isMakingRequest" - class="js-remove-wip gl-ml-3" - @click="handleRemoveWIP" + class="js-remove-draft gl-ml-3" + @click="handleRemoveDraft" > {{ s__('mrWidget|Mark as ready') }} diff --git a/app/assets/javascripts/vue_merge_request_widget/queries/get_state.query.graphql b/app/assets/javascripts/vue_merge_request_widget/queries/get_state.query.graphql index 871aa880b36bf2..bfb1517be81d23 100644 --- a/app/assets/javascripts/vue_merge_request_widget/queries/get_state.query.graphql +++ b/app/assets/javascripts/vue_merge_request_widget/queries/get_state.query.graphql @@ -23,7 +23,7 @@ query getState($projectPath: ID!, $iid: String!) { userPermissions { canMerge } - workInProgress + draft } } } diff --git a/app/assets/javascripts/vue_merge_request_widget/queries/states/work_in_progress.query.graphql b/app/assets/javascripts/vue_merge_request_widget/queries/states/draft.query.graphql similarity index 100% rename from app/assets/javascripts/vue_merge_request_widget/queries/states/work_in_progress.query.graphql rename to app/assets/javascripts/vue_merge_request_widget/queries/states/draft.query.graphql diff --git a/app/assets/javascripts/vue_merge_request_widget/queries/toggle_draft.mutation.graphql b/app/assets/javascripts/vue_merge_request_widget/queries/toggle_draft.mutation.graphql new file mode 100644 index 00000000000000..200fb1b7ca527c --- /dev/null +++ b/app/assets/javascripts/vue_merge_request_widget/queries/toggle_draft.mutation.graphql @@ -0,0 +1,10 @@ +mutation toggleDraftStatus($projectPath: ID!, $iid: String!, $draft: Boolean!) { + mergeRequestSetDraft(input: { projectPath: $projectPath, iid: $iid, draft: $draft }) { + mergeRequest { + mergeableDiscussionsState + title + draft + } + errors + } +} diff --git a/app/assets/javascripts/vue_merge_request_widget/queries/toggle_wip.mutation.graphql b/app/assets/javascripts/vue_merge_request_widget/queries/toggle_wip.mutation.graphql deleted file mode 100644 index cfaa198d5167bd..00000000000000 --- a/app/assets/javascripts/vue_merge_request_widget/queries/toggle_wip.mutation.graphql +++ /dev/null @@ -1,10 +0,0 @@ -mutation toggleWIPStatus($projectPath: ID!, $iid: String!, $wip: Boolean!) { - mergeRequestSetWip(input: { projectPath: $projectPath, iid: $iid, wip: $wip }) { - mergeRequest { - mergeableDiscussionsState - title - workInProgress - } - errors - } -} diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/get_state_key.js b/app/assets/javascripts/vue_merge_request_widget/stores/get_state_key.js index d52371032eb9ea..2ae4f4da2f303f 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/get_state_key.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/get_state_key.js @@ -17,8 +17,8 @@ export default function deviseState() { return stateKey.rebase; } else if (this.onlyAllowMergeIfPipelineSucceeds && this.isPipelineFailed) { return stateKey.pipelineFailed; - } else if (this.workInProgress) { - return stateKey.workInProgress; + } else if (this.draft) { + return stateKey.draft; } else if (this.hasMergeableDiscussionsState && !this.autoMergeEnabled) { return stateKey.unresolvedDiscussions; } else if (this.isPipelineBlocked) { diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js index 6628225cd46ada..10a2907c81ab3b 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js @@ -164,7 +164,7 @@ export default class MergeRequestStore { this.projectArchived = data.project_archived; this.isSHAMismatch = this.sha !== data.diff_head_sha; this.shouldBeRebased = Boolean(data.should_be_rebased); - this.workInProgress = data.work_in_progress; + this.draft = data.draft; } const currentUser = data.current_user; @@ -207,7 +207,7 @@ export default class MergeRequestStore { this.isPipelineFailed = this.ciStatus === 'failed' || this.ciStatus === 'canceled'; this.isSHAMismatch = this.sha !== mergeRequest.diffHeadSha; this.shouldBeRebased = mergeRequest.shouldBeRebased; - this.workInProgress = mergeRequest.workInProgress; + this.draft = mergeRequest.draft; this.mergeRequestState = mergeRequest.state; this.setState(); diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js b/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js index 4cb23407a74461..9dfeaee905cc80 100644 --- a/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js +++ b/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js @@ -4,7 +4,7 @@ export const stateToComponentMap = { merging: 'mr-widget-merging', conflicts: 'mr-widget-conflicts', missingBranch: 'mr-widget-missing-branch', - workInProgress: 'mr-widget-wip', + draft: 'mr-widget-wip', readyToMerge: 'mr-widget-ready-to-merge', nothingToMerge: 'mr-widget-nothing-to-merge', notAllowedToMerge: 'mr-widget-not-allowed', @@ -24,7 +24,7 @@ export const stateToComponentMap = { export const statesToShowHelpWidget = [ 'merging', 'conflicts', - 'workInProgress', + 'draft', 'readyToMerge', 'checking', 'unresolvedDiscussions', @@ -40,7 +40,7 @@ export const stateKey = { nothingToMerge: 'nothingToMerge', checking: 'checking', conflicts: 'conflicts', - workInProgress: 'workInProgress', + draft: 'draft', pipelineFailed: 'pipelineFailed', unresolvedDiscussions: 'unresolvedDiscussions', pipelineBlocked: 'pipelineBlocked', diff --git a/app/graphql/mutations/merge_requests/set_wip.rb b/app/graphql/mutations/merge_requests/set_wip.rb deleted file mode 100644 index 9b6b67d4b4fbd8..00000000000000 --- a/app/graphql/mutations/merge_requests/set_wip.rb +++ /dev/null @@ -1,35 +0,0 @@ -# frozen_string_literal: true - -module Mutations - module MergeRequests - class SetWip < Base - graphql_name 'MergeRequestSetWip' - - argument :wip, - GraphQL::Types::Boolean, - required: true, - description: <<~DESC - Whether or not to set the merge request as a draft. - DESC - - def resolve(project_path:, iid:, wip: nil) - merge_request = authorized_find!(project_path: project_path, iid: iid) - project = merge_request.project - - ::MergeRequests::UpdateService.new(project: project, current_user: current_user, params: { wip_event: wip_event(merge_request, wip) }) - .execute(merge_request) - - { - merge_request: merge_request, - errors: errors_on_object(merge_request) - } - end - - private - - def wip_event(merge_request, wip) - wip ? 'wip' : 'unwip' - end - end - end -end diff --git a/app/graphql/types/merge_request_type.rb b/app/graphql/types/merge_request_type.rb index 004ac3644874d2..a8a96b0ccef31e 100644 --- a/app/graphql/types/merge_request_type.rb +++ b/app/graphql/types/merge_request_type.rb @@ -53,9 +53,6 @@ class MergeRequestType < BaseObject description: 'Indicates if the source branch is protected.' field :target_branch, GraphQL::Types::String, null: false, description: 'Target branch of the merge request.' - field :work_in_progress, GraphQL::Types::Boolean, 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::Types::Boolean, method: :draft?, null: false, description: 'Indicates if the merge request is a draft.' field :merge_when_pipeline_succeeds, GraphQL::Types::Boolean, null: true, diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index 7be995a40b2171..5366283e55a8a1 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -65,9 +65,6 @@ class MutationType < BaseObject mount_mutation Mutations::MergeRequests::SetLocked mount_mutation Mutations::MergeRequests::SetMilestone mount_mutation Mutations::MergeRequests::SetSubscription - 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 diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 9694fd84fd4e10..6d4de0ec1b4afa 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -3493,31 +3493,6 @@ Input type: `MergeRequestSetSubscriptionInput` | `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | | `mergeRequest` | [`MergeRequest`](#mergerequest) | Merge request after mutation. | -### `Mutation.mergeRequestSetWip` - -WARNING: -**Deprecated** in 13.12. -Use mergeRequestSetDraft. - -Input type: `MergeRequestSetWipInput` - -#### Arguments - -| Name | Type | Description | -| ---- | ---- | ----------- | -| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | -| `iid` | [`String!`](#string) | IID of the merge request to mutate. | -| `projectPath` | [`ID!`](#id) | Project the merge request to mutate is in. | -| `wip` | [`Boolean!`](#boolean) | Whether or not to set the merge request as a draft. | - -#### Fields - -| Name | Type | Description | -| ---- | ---- | ----------- | -| `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | -| `errors` | [`[String!]!`](#string) | Errors encountered during execution of the mutation. | -| `mergeRequest` | [`MergeRequest`](#mergerequest) | Merge request after mutation. | - ### `Mutation.mergeRequestUpdate` Update attributes of a merge request. @@ -11509,7 +11484,6 @@ Maven metadata. | `userNotesCount` | [`Int`](#int) | User notes count of the merge request. | | `userPermissions` | [`MergeRequestPermissions!`](#mergerequestpermissions) | Permissions for the current user on the resource. | | `webUrl` | [`String`](#string) | Web URL of the merge request. | -| `workInProgress` **{warning-solid}** | [`Boolean!`](#boolean) | **Deprecated** in 13.12. Use `draft`. | #### Fields with arguments diff --git a/spec/frontend/vue_mr_widget/components/states/mr_widget_wip_spec.js b/spec/frontend/vue_mr_widget/components/states/mr_widget_wip_spec.js index be15e4df66d477..0fb0d5b0b68dd4 100644 --- a/spec/frontend/vue_mr_widget/components/states/mr_widget_wip_spec.js +++ b/spec/frontend/vue_mr_widget/components/states/mr_widget_wip_spec.js @@ -46,7 +46,7 @@ describe('Wip', () => { is_new_mr_data: true, }; - describe('handleRemoveWIP', () => { + describe('handleRemoveDraft', () => { it('should make a request to service and handle response', (done) => { const vm = createComponent(); @@ -59,7 +59,7 @@ describe('Wip', () => { }), ); - vm.handleRemoveWIP(); + vm.handleRemoveDraft(); setImmediate(() => { expect(vm.isMakingRequest).toBeTruthy(); expect(eventHub.$emit).toHaveBeenCalledWith('UpdateWidgetData', mrObj); @@ -84,7 +84,7 @@ describe('Wip', () => { expect(el.innerText).toContain('This merge request is still a draft.'); expect(el.querySelector('button').getAttribute('disabled')).toBeTruthy(); expect(el.querySelector('button').innerText).toContain('Merge'); - expect(el.querySelector('.js-remove-wip').innerText.replace(/\s\s+/g, ' ')).toContain( + expect(el.querySelector('.js-remove-draft').innerText.replace(/\s\s+/g, ' ')).toContain( 'Mark as ready', ); }); @@ -93,7 +93,7 @@ describe('Wip', () => { vm.mr.removeWIPPath = ''; Vue.nextTick(() => { - expect(el.querySelector('.js-remove-wip')).toEqual(null); + expect(el.querySelector('.js-remove-draft')).toEqual(null); done(); }); }); diff --git a/spec/frontend/vue_mr_widget/stores/get_state_key_spec.js b/spec/frontend/vue_mr_widget/stores/get_state_key_spec.js index 4d0e5e71f27c7f..fc760f5c5be027 100644 --- a/spec/frontend/vue_mr_widget/stores/get_state_key_spec.js +++ b/spec/frontend/vue_mr_widget/stores/get_state_key_spec.js @@ -15,7 +15,7 @@ describe('getStateKey', () => { branchMissing: false, commitsCount: 2, hasConflicts: false, - workInProgress: false, + draft: false, }; const bound = getStateKey.bind(context); @@ -49,9 +49,9 @@ describe('getStateKey', () => { expect(bound()).toEqual('unresolvedDiscussions'); - context.workInProgress = true; + context.draft = true; - expect(bound()).toEqual('workInProgress'); + expect(bound()).toEqual('draft'); context.onlyAllowMergeIfPipelineSucceeds = true; context.isPipelineFailed = true; @@ -99,7 +99,7 @@ describe('getStateKey', () => { branchMissing: false, commitsCount: 2, hasConflicts: false, - workInProgress: false, + draft: false, }; const bound = getStateKey.bind(context); diff --git a/spec/graphql/mutations/merge_requests/set_wip_spec.rb b/spec/graphql/mutations/merge_requests/set_wip_spec.rb deleted file mode 100644 index fae9c4f7fe0a89..00000000000000 --- a/spec/graphql/mutations/merge_requests/set_wip_spec.rb +++ /dev/null @@ -1,55 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Mutations::MergeRequests::SetWip 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(:wip) { true } - let(:mutated_merge_request) { subject[:merge_request] } - - subject { mutation.resolve(project_path: merge_request.project.full_path, iid: merge_request.iid, wip: wip) } - - 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 wip' do - expect(mutated_merge_request).to eq(merge_request) - expect(mutated_merge_request).to be_work_in_progress - 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 wip as false' do - let(:wip) { false } - - it 'removes `wip` from the title' do - merge_request.update!(title: "WIP: working on it") - - expect(mutated_merge_request).not_to be_work_in_progress - end - - it 'does not do anything if the title did not start with wip' do - expect(mutated_merge_request).not_to be_work_in_progress - end - end - end - end -end diff --git a/spec/graphql/types/merge_request_type_spec.rb b/spec/graphql/types/merge_request_type_spec.rb index bc3ccb0d9ba025..b17b7c32289e1b 100644 --- a/spec/graphql/types/merge_request_type_spec.rb +++ b/spec/graphql/types/merge_request_type_spec.rb @@ -18,7 +18,7 @@ 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 draft merge_when_pipeline_succeeds diff_head_sha + target_branch 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 diff --git a/spec/graphql/types/mutation_type_spec.rb b/spec/graphql/types/mutation_type_spec.rb index c1a5c93c85b95f..95d835c88cf65c 100644 --- a/spec/graphql/types/mutation_type_spec.rb +++ b/spec/graphql/types/mutation_type_spec.rb @@ -3,14 +3,6 @@ require 'spec_helper' RSpec.describe Types::MutationType do - 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 diff --git a/spec/requests/api/graphql/mutations/merge_requests/set_wip_spec.rb b/spec/requests/api/graphql/mutations/merge_requests/set_draft_spec.rb similarity index 91% rename from spec/requests/api/graphql/mutations/merge_requests/set_wip_spec.rb rename to spec/requests/api/graphql/mutations/merge_requests/set_draft_spec.rb index 2143abd3031cdd..bea2365eaa6745 100644 --- a/spec/requests/api/graphql/mutations/merge_requests/set_wip_spec.rb +++ b/spec/requests/api/graphql/mutations/merge_requests/set_draft_spec.rb @@ -8,14 +8,14 @@ let(:current_user) { create(:user) } let(:merge_request) { create(:merge_request) } let(:project) { merge_request.project } - let(:input) { { wip: true } } + let(:input) { { draft: true } } let(:mutation) do variables = { project_path: project.full_path, iid: merge_request.iid.to_s } - graphql_mutation(:merge_request_set_wip, variables.merge(input), + graphql_mutation(:merge_request_set_draft, variables.merge(input), <<-QL.strip_heredoc clientMutationId errors @@ -28,7 +28,7 @@ end def mutation_response - graphql_mutation_response(:merge_request_set_wip) + graphql_mutation_response(:merge_request_set_draft) end before do @@ -58,7 +58,7 @@ def mutation_response end context 'when passing Draft false as input' do - let(:input) { { wip: false } } + let(:input) { { draft: false } } it 'does not do anything if the merge reqeust was not marked draft' do post_graphql_mutation(mutation, current_user: current_user) -- GitLab