Commit 6ada7514 authored by Mark Chao's avatar Mark Chao

Merge branch '356069-deprecate-iteraion-updates' into 'master'

Deprecate updating an iteration's attributes via GraphQL

See merge request gitlab-org/gitlab!83349
parents 5d06a064 14e438b7
...@@ -4962,11 +4962,11 @@ Input type: `UpdateIterationInput` ...@@ -4962,11 +4962,11 @@ Input type: `UpdateIterationInput`
| ---- | ---- | ----------- | | ---- | ---- | ----------- |
| <a id="mutationupdateiterationclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | | <a id="mutationupdateiterationclientmutationid"></a>`clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. |
| <a id="mutationupdateiterationdescription"></a>`description` | [`String`](#string) | Description of the iteration. | | <a id="mutationupdateiterationdescription"></a>`description` | [`String`](#string) | Description of the iteration. |
| <a id="mutationupdateiterationduedate"></a>`dueDate` | [`String`](#string) | End date of the iteration. | | <a id="mutationupdateiterationduedate"></a>`dueDate` **{warning-solid}** | [`String`](#string) | **Deprecated:** Manual iteration updates are deprecated, only `description` updates will be allowed in the future. Deprecated in 14.10. |
| <a id="mutationupdateiterationgrouppath"></a>`groupPath` | [`ID!`](#id) | Group of the iteration. | | <a id="mutationupdateiterationgrouppath"></a>`groupPath` | [`ID!`](#id) | Group of the iteration. |
| <a id="mutationupdateiterationid"></a>`id` | [`ID!`](#id) | Global ID of the iteration. | | <a id="mutationupdateiterationid"></a>`id` | [`ID!`](#id) | Global ID of the iteration. |
| <a id="mutationupdateiterationstartdate"></a>`startDate` | [`String`](#string) | Start date of the iteration. | | <a id="mutationupdateiterationstartdate"></a>`startDate` **{warning-solid}** | [`String`](#string) | **Deprecated:** Manual iteration updates are deprecated, only `description` updates will be allowed in the future. Deprecated in 14.10. |
| <a id="mutationupdateiterationtitle"></a>`title` | [`String`](#string) | Title of the iteration. | | <a id="mutationupdateiterationtitle"></a>`title` **{warning-solid}** | [`String`](#string) | **Deprecated:** Manual iteration updates are deprecated, only `description` updates will be allowed in the future. Deprecated in 14.10. |
#### Fields #### Fields
...@@ -5,6 +5,8 @@ module Mutations ...@@ -5,6 +5,8 @@ module Mutations
class Update < BaseMutation class Update < BaseMutation
graphql_name 'UpdateIteration' graphql_name 'UpdateIteration'
ITERATION_DEPRECATION_MESSAGE = 'Manual iteration updates are deprecated, only `description` updates will be allowed in the future'
include Mutations::ResolvesGroup include Mutations::ResolvesGroup
include ResolvesProject include ResolvesProject
...@@ -29,7 +31,8 @@ module Mutations ...@@ -29,7 +31,8 @@ module Mutations
argument :title, argument :title,
GraphQL::Types::String, GraphQL::Types::String,
required: false, required: false,
description: 'Title of the iteration.' description: 'Title of the iteration.',
deprecated: { reason: ITERATION_DEPRECATION_MESSAGE, milestone: '14.10' }
argument :description, argument :description,
GraphQL::Types::String, GraphQL::Types::String,
...@@ -39,12 +42,14 @@ module Mutations ...@@ -39,12 +42,14 @@ module Mutations
argument :start_date, argument :start_date,
GraphQL::Types::String, GraphQL::Types::String,
required: false, required: false,
description: 'Start date of the iteration.' description: 'Start date of the iteration.',
deprecated: { reason: ITERATION_DEPRECATION_MESSAGE, milestone: '14.10' }
argument :due_date, argument :due_date,
GraphQL::Types::String, GraphQL::Types::String,
required: false, required: false,
description: 'End date of the iteration.' description: 'End date of the iteration.',
deprecated: { reason: ITERATION_DEPRECATION_MESSAGE, milestone: '14.10' }
def resolve(args) def resolve(args)
validate_arguments!(args) validate_arguments!(args)
...@@ -54,6 +59,8 @@ module Mutations ...@@ -54,6 +59,8 @@ module Mutations
validate_title_argument!(parent, args) validate_title_argument!(parent, args)
iteration = authorized_find!(parent: parent, id: args[:id]) iteration = authorized_find!(parent: parent, id: args[:id])
validate_allowed_attributes_if_automatic!(iteration, args)
response = ::Iterations::UpdateService.new(parent, current_user, args).execute(iteration) response = ::Iterations::UpdateService.new(parent, current_user, args).execute(iteration)
response_object = response.success? ? response.payload[:iteration] : nil response_object = response.success? ? response.payload[:iteration] : nil
...@@ -67,6 +74,17 @@ module Mutations ...@@ -67,6 +74,17 @@ module Mutations
private private
# Raising an error only if automatic as that would mean the cadence was created with the
# iteration_cadences feature flag enabled
def validate_allowed_attributes_if_automatic!(iteration, args)
return unless iteration.iterations_cadence.automatic?
deprecated_arguments = [:title, :start_date, :due_date]
return if (deprecated_arguments & args.keys).empty?
raise Gitlab::Graphql::Errors::ArgumentError, ITERATION_DEPRECATION_MESSAGE
end
def find_object(parent:, id:) def find_object(parent:, id:)
params = { parent: parent, id: id } params = { parent: parent, id: id }
......
...@@ -7,9 +7,10 @@ RSpec.describe 'Updating an Iteration' do ...@@ -7,9 +7,10 @@ RSpec.describe 'Updating an Iteration' do
let_it_be(:current_user) { create(:user) } let_it_be(:current_user) { create(:user) }
let_it_be(:group) { create(:group) } let_it_be(:group) { create(:group) }
let_it_be(:cadence) { create(:iterations_cadence, group: group) } let_it_be(:cadence) { build(:iterations_cadence, group: group, automatic: false).tap { |cadence| cadence.save!(validate: false) } }
let_it_be(:iteration) { create(:iteration, group: group, iterations_cadence: cadence) } let_it_be(:iteration) { create(:iteration, group: group, iterations_cadence: cadence) }
let(:subject_iteration) { iteration }
let(:start_date) { 1.day.from_now.strftime('%F') } let(:start_date) { 1.day.from_now.strftime('%F') }
let(:end_date) { 5.days.from_now.strftime('%F') } let(:end_date) { 5.days.from_now.strftime('%F') }
let(:attributes) do let(:attributes) do
...@@ -22,7 +23,7 @@ RSpec.describe 'Updating an Iteration' do ...@@ -22,7 +23,7 @@ RSpec.describe 'Updating an Iteration' do
end end
let(:mutation) do let(:mutation) do
params = { group_path: group.full_path, id: iteration.to_global_id.to_s }.merge(attributes) params = { group_path: group.full_path, id: subject_iteration.to_global_id.to_s }.merge(attributes)
graphql_mutation(:update_iteration, params) graphql_mutation(:update_iteration, params)
end end
...@@ -40,7 +41,7 @@ RSpec.describe 'Updating an Iteration' do ...@@ -40,7 +41,7 @@ RSpec.describe 'Updating an Iteration' do
it_behaves_like 'a mutation that returns a top-level access error' it_behaves_like 'a mutation that returns a top-level access error'
it 'does not update iteration' do it 'does not update iteration' do
expect { post_graphql_mutation(mutation, current_user: current_user) }.not_to change(iteration, :title) expect { post_graphql_mutation(mutation, current_user: current_user) }.not_to change(subject_iteration, :title)
end end
end end
...@@ -77,10 +78,46 @@ RSpec.describe 'Updating an Iteration' do ...@@ -77,10 +78,46 @@ RSpec.describe 'Updating an Iteration' do
expect(iteration_hash['dueDate'].to_date).to eq(end_date.to_date) expect(iteration_hash['dueDate'].to_date).to eq(end_date.to_date)
# Let's also check that the object was updated properly # Let's also check that the object was updated properly
iteration.reload subject_iteration.reload
expect(iteration.description).to eq('some description') expect(subject_iteration.description).to eq('some description')
expect(iteration.start_date).to eq(start_date.to_date) expect(subject_iteration.start_date).to eq(start_date.to_date)
expect(iteration.due_date).to eq(end_date.to_date) expect(subject_iteration.due_date).to eq(end_date.to_date)
end
context 'when updating attributes on an automatic cadence' do
let_it_be(:automatic_cadence) { create(:iterations_cadence, group: group) }
let_it_be(:legacy_iteration) { create(:iteration, group: group, iterations_cadence: automatic_cadence) }
let(:subject_iteration) { legacy_iteration }
context 'when updating deprecated attributes' do
using RSpec::Parameterized::TableSyntax
where(:argument, :argument_value) do
:title | 'updated title'
:start_date | 1.week.ago.to_date.to_s
:due_date | 1.week.from_now.to_date.to_s
end
with_them do
let(:attributes) { { argument => argument_value } }
it_behaves_like 'a mutation that returns top-level errors',
errors: ['Manual iteration updates are deprecated, only `description` updates will be allowed in the future']
end
end
context 'when updating description' do
let(:attributes) { { description: 'updated description' } }
it 'allows updating the description of an iteration' do
expect do
post_graphql_mutation(mutation, current_user: current_user)
subject_iteration.reload
end.to change(subject_iteration, :description).to('updated description')
end
end
end end
context 'when updating title' do context 'when updating title' do
...@@ -104,7 +141,7 @@ RSpec.describe 'Updating an Iteration' do ...@@ -104,7 +141,7 @@ RSpec.describe 'Updating an Iteration' do
post_graphql_mutation(mutation, current_user: current_user) post_graphql_mutation(mutation, current_user: current_user)
expect(mutation_response['iteration']['title']).to eq(expected_title) expect(mutation_response['iteration']['title']).to eq(expected_title)
expect(iteration.reload.title).to eq(expected_title) expect(subject_iteration.reload.title).to eq(expected_title)
end end
end end
end end
...@@ -136,8 +173,8 @@ RSpec.describe 'Updating an Iteration' do ...@@ -136,8 +173,8 @@ RSpec.describe 'Updating an Iteration' do
expect(iteration_hash['startDate'].to_date).to eq(start_date.to_date) expect(iteration_hash['startDate'].to_date).to eq(start_date.to_date)
# Let's also check that the object was updated properly # Let's also check that the object was updated properly
iteration.reload subject_iteration.reload
expect(iteration.start_date).to eq(start_date.to_date) expect(subject_iteration.start_date).to eq(start_date.to_date)
end end
context 'when another iteration with given dates overlap' do context 'when another iteration with given dates overlap' do
...@@ -160,14 +197,14 @@ RSpec.describe 'Updating an Iteration' do ...@@ -160,14 +197,14 @@ RSpec.describe 'Updating an Iteration' do
end end
context 'when given a raw model id (backward compatibility)' do context 'when given a raw model id (backward compatibility)' do
let(:attributes) { { id: iteration.id, title: 'title' } } let(:attributes) { { id: subject_iteration.id, title: 'title' } }
it 'updates the iteration' do it 'updates the iteration' do
post_graphql_mutation(mutation, current_user: current_user) post_graphql_mutation(mutation, current_user: current_user)
iteration_hash = mutation_response['iteration'] iteration_hash = mutation_response['iteration']
expect(iteration_hash['title']).to eq('title') expect(iteration_hash['title']).to eq('title')
expect(iteration.reload.title).to eq('title') expect(subject_iteration.reload.title).to eq('title')
end end
end end
...@@ -178,7 +215,7 @@ RSpec.describe 'Updating an Iteration' do ...@@ -178,7 +215,7 @@ RSpec.describe 'Updating an Iteration' do
errors: ['The list of iteration attributes is empty'] errors: ['The list of iteration attributes is empty']
it 'does not update the iteration' do it 'does not update the iteration' do
expect { post_graphql_mutation(mutation, current_user: current_user) }.not_to change(iteration, :title) expect { post_graphql_mutation(mutation, current_user: current_user) }.not_to change(subject_iteration, :title)
end 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