Commit f9e189f6 authored by Simon Knox's avatar Simon Knox Committed by Ash McKenzie

Accept a global id for updateIteration mutation

Also accept a global id when querying the `iterations`
field

But maintain backward compatibility by accpeting
a raw model id
parent 2baf662c
......@@ -23,8 +23,6 @@ FactoryBot/InlineAssociation:
Graphql/IDType:
Exclude:
- 'ee/app/graphql/ee/mutations/issues/update.rb'
- 'ee/app/graphql/mutations/iterations/update.rb'
- 'ee/app/graphql/resolvers/iterations_resolver.rb'
- 'app/graphql/mutations/boards/issues/issue_move_list.rb'
- 'app/graphql/mutations/issues/update.rb'
- 'app/graphql/mutations/metrics/dashboard/annotations/delete.rb'
......
......@@ -9084,17 +9084,17 @@ type Group {
first: Int
"""
The ID of the Iteration to look up
Global ID of the Iteration to look up.
"""
id: ID
"""
The internal ID of the Iteration to look up
Internal ID of the Iteration to look up.
"""
iid: ID
"""
Whether to include ancestor iterations. Defaults to true
Whether to include ancestor iterations. Defaults to true.
"""
includeAncestors: Boolean
......@@ -9111,7 +9111,7 @@ type Group {
startDate: Time
"""
Filter iterations by state
Filter iterations by state.
"""
state: IterationState
......@@ -9121,7 +9121,7 @@ type Group {
timeframe: Timeframe
"""
Fuzzy search by title
Fuzzy search by title.
"""
title: String
): IterationConnection
......@@ -15765,17 +15765,17 @@ type Project {
first: Int
"""
The ID of the Iteration to look up
Global ID of the Iteration to look up.
"""
id: ID
"""
The internal ID of the Iteration to look up
Internal ID of the Iteration to look up.
"""
iid: ID
"""
Whether to include ancestor iterations. Defaults to true
Whether to include ancestor iterations. Defaults to true.
"""
includeAncestors: Boolean
......@@ -15792,7 +15792,7 @@ type Project {
startDate: Time
"""
Filter iterations by state
Filter iterations by state.
"""
state: IterationState
......@@ -15802,7 +15802,7 @@ type Project {
timeframe: Timeframe
"""
Fuzzy search by title
Fuzzy search by title.
"""
title: String
): IterationConnection
......@@ -22434,32 +22434,32 @@ input UpdateIterationInput {
clientMutationId: String
"""
The description of the iteration
Description of the iteration.
"""
description: String
"""
The end date of the iteration
End date of the iteration.
"""
dueDate: String
"""
The group of the iteration
Group of the iteration.
"""
groupPath: ID!
"""
The id of the iteration
Global ID of the iteration.
"""
id: ID!
"""
The start date of the iteration
Start date of the iteration.
"""
startDate: String
"""
The title of the iteration
Title of the iteration.
"""
title: String
}
......@@ -22479,7 +22479,7 @@ type UpdateIterationPayload {
errors: [String!]!
"""
The updated iteration
Updated iteration.
"""
iteration: Iteration
}
......
......@@ -24922,7 +24922,7 @@
},
{
"name": "state",
"description": "Filter iterations by state",
"description": "Filter iterations by state.",
"type": {
"kind": "ENUM",
"name": "IterationState",
......@@ -24932,7 +24932,7 @@
},
{
"name": "title",
"description": "Fuzzy search by title",
"description": "Fuzzy search by title.",
"type": {
"kind": "SCALAR",
"name": "String",
......@@ -24942,7 +24942,7 @@
},
{
"name": "id",
"description": "The ID of the Iteration to look up",
"description": "Global ID of the Iteration to look up.",
"type": {
"kind": "SCALAR",
"name": "ID",
......@@ -24952,7 +24952,7 @@
},
{
"name": "iid",
"description": "The internal ID of the Iteration to look up",
"description": "Internal ID of the Iteration to look up.",
"type": {
"kind": "SCALAR",
"name": "ID",
......@@ -24962,7 +24962,7 @@
},
{
"name": "includeAncestors",
"description": "Whether to include ancestor iterations. Defaults to true",
"description": "Whether to include ancestor iterations. Defaults to true.",
"type": {
"kind": "SCALAR",
"name": "Boolean",
......@@ -46092,7 +46092,7 @@
},
{
"name": "state",
"description": "Filter iterations by state",
"description": "Filter iterations by state.",
"type": {
"kind": "ENUM",
"name": "IterationState",
......@@ -46102,7 +46102,7 @@
},
{
"name": "title",
"description": "Fuzzy search by title",
"description": "Fuzzy search by title.",
"type": {
"kind": "SCALAR",
"name": "String",
......@@ -46112,7 +46112,7 @@
},
{
"name": "id",
"description": "The ID of the Iteration to look up",
"description": "Global ID of the Iteration to look up.",
"type": {
"kind": "SCALAR",
"name": "ID",
......@@ -46122,7 +46122,7 @@
},
{
"name": "iid",
"description": "The internal ID of the Iteration to look up",
"description": "Internal ID of the Iteration to look up.",
"type": {
"kind": "SCALAR",
"name": "ID",
......@@ -46132,7 +46132,7 @@
},
{
"name": "includeAncestors",
"description": "Whether to include ancestor iterations. Defaults to true",
"description": "Whether to include ancestor iterations. Defaults to true.",
"type": {
"kind": "SCALAR",
"name": "Boolean",
......@@ -65094,7 +65094,7 @@
"inputFields": [
{
"name": "groupPath",
"description": "The group of the iteration",
"description": "Group of the iteration.",
"type": {
"kind": "NON_NULL",
"name": null,
......@@ -65108,7 +65108,7 @@
},
{
"name": "id",
"description": "The id of the iteration",
"description": "Global ID of the iteration.",
"type": {
"kind": "NON_NULL",
"name": null,
......@@ -65122,7 +65122,7 @@
},
{
"name": "title",
"description": "The title of the iteration",
"description": "Title of the iteration.",
"type": {
"kind": "SCALAR",
"name": "String",
......@@ -65132,7 +65132,7 @@
},
{
"name": "description",
"description": "The description of the iteration",
"description": "Description of the iteration.",
"type": {
"kind": "SCALAR",
"name": "String",
......@@ -65142,7 +65142,7 @@
},
{
"name": "startDate",
"description": "The start date of the iteration",
"description": "Start date of the iteration.",
"type": {
"kind": "SCALAR",
"name": "String",
......@@ -65152,7 +65152,7 @@
},
{
"name": "dueDate",
"description": "The end date of the iteration",
"description": "End date of the iteration.",
"type": {
"kind": "SCALAR",
"name": "String",
......@@ -65222,7 +65222,7 @@
},
{
"name": "iteration",
"description": "The updated iteration",
"description": "Updated iteration.",
"args": [
],
......@@ -3363,7 +3363,7 @@ Autogenerated return type of UpdateIteration.
| ----- | ---- | ----------- |
| `clientMutationId` | String | A unique identifier for the client performing the mutation. |
| `errors` | String! => Array | Errors encountered during execution of the mutation. |
| `iteration` | Iteration | The updated iteration |
| `iteration` | Iteration | Updated iteration. |
### UpdateNotePayload
......
......@@ -4,7 +4,6 @@ import { deprecatedCreateFlash as createFlash } from '~/flash';
import { visitUrl } from '~/lib/utils/url_utility';
import { __ } from '~/locale';
import MarkdownField from '~/vue_shared/components/markdown/field.vue';
import { getIdFromGraphQLId } from '~/graphql_shared/utils';
import createIteration from '../queries/create_iteration.mutation.graphql';
import updateIteration from '../queries/update_iteration.mutation.graphql';
import DueDateSelectors from '~/due_date_select';
......@@ -109,7 +108,7 @@ export default {
variables: {
input: {
...this.variables.input,
id: getIdFromGraphQLId(this.iteration.id),
id: this.iteration.id,
},
},
})
......
......@@ -13,39 +13,42 @@ module Mutations
field :iteration,
Types::IterationType,
null: true,
description: 'The updated iteration'
description: 'Updated iteration.'
argument :group_path, GraphQL::ID_TYPE,
required: true,
description: "The group of the iteration"
description: 'Group of the iteration.'
# rubocop:disable Graphql/IDType
argument :id,
GraphQL::ID_TYPE,
required: true,
description: 'The id of the iteration'
description: 'Global ID of the iteration.'
# rubocop:enable Graphql/IDType
argument :title,
GraphQL::STRING_TYPE,
required: false,
description: 'The title of the iteration'
description: 'Title of the iteration.'
argument :description,
GraphQL::STRING_TYPE,
required: false,
description: 'The description of the iteration'
description: 'Description of the iteration.'
argument :start_date,
GraphQL::STRING_TYPE,
required: false,
description: 'The start date of the iteration'
description: 'Start date of the iteration.'
argument :due_date,
GraphQL::STRING_TYPE,
required: false,
description: 'The end date of the iteration'
description: 'End date of the iteration.'
def resolve(args)
validate_arguments!(args)
args[:id] = id_from_args(args)
parent = resolve_group(full_path: args[:group_path]).try(:sync)
iteration = authorized_find!(parent: parent, id: args[:id])
......@@ -64,12 +67,21 @@ module Mutations
private
def find_object(parent:, id:)
::Resolvers::IterationsResolver.new(object: parent, context: context, field: nil).resolve(id: id).items.first
::Resolvers::IterationsResolver.new(object: parent, context: context, field: nil)
.resolve(id: id).items.first
end
def validate_arguments!(args)
raise Gitlab::Graphql::Errors::ArgumentError, 'The list of iteration attributes is empty' if args.except(:group_path, :id).empty?
end
# Originally accepted a raw model id. Now accept a gid, but allow a raw id
# for backward compatibility
def id_from_args(args)
GitlabSchema.parse_gid(args[:id], expected_type: ::Iteration)
rescue Gitlab::Graphql::Errors::ArgumentError
::Gitlab::GlobalId.as_global_id(args[:id].to_i, model_name: 'Iteration')
end
end
end
end
......@@ -7,27 +7,32 @@ module Resolvers
argument :state, Types::IterationStateEnum,
required: false,
description: 'Filter iterations by state'
description: 'Filter iterations by state.'
argument :title, GraphQL::STRING_TYPE,
required: false,
description: 'Fuzzy search by title'
description: 'Fuzzy search by title.'
# rubocop:disable Graphql/IDType
argument :id, GraphQL::ID_TYPE,
required: false,
description: 'The ID of the Iteration to look up'
description: 'Global ID of the Iteration to look up.'
# rubocop:enable Graphql/IDType
argument :iid, GraphQL::ID_TYPE,
required: false,
description: 'The internal ID of the Iteration to look up'
description: 'Internal ID of the Iteration to look up.'
argument :include_ancestors, GraphQL::BOOLEAN_TYPE,
required: false,
description: 'Whether to include ancestor iterations. Defaults to true'
description: 'Whether to include ancestor iterations. Defaults to true.'
type Types::IterationType, null: true
type Types::IterationType.connection_type, null: true
def resolve(**args)
validate_timeframe_params!(args)
authorize!
args[:id] = id_from_args(args)
args[:include_ancestors] = true if args[:include_ancestors].nil? && args[:iid].nil?
iterations = IterationsFinder.new(context[:current_user], iterations_finder_params(args)).execute
......@@ -58,5 +63,15 @@ module Resolvers
def authorize!
Ability.allowed?(context[:current_user], :read_iteration, parent) || raise_resource_not_available_error!
end
# Originally accepted a raw model id. Now accept a gid, but allow a raw id
# for backward compatibility
def id_from_args(args)
return unless args[:id].present?
GitlabSchema.parse_gid(args[:id], expected_type: ::Iteration).model_id
rescue Gitlab::Graphql::Errors::ArgumentError
args[:id]
end
end
end
......@@ -186,7 +186,7 @@ describe('Iteration Form', () => {
variables: {
input: {
groupPath,
id,
id: iteration.id,
title,
description,
startDate,
......
......@@ -43,13 +43,23 @@ RSpec.describe Resolvers::IterationsResolver do
start_date = now
end_date = start_date + 1.hour
search = 'wow'
id = 1
id = '1'
iid = 2
params = { id: id, iid: iid, group_ids: group.id, state: 'closed', start_date: start_date, end_date: end_date, search_title: search }
expect(IterationsFinder).to receive(:new).with(current_user, params).and_call_original
resolve_group_iterations(start_date: start_date, end_date: end_date, state: 'closed', title: search, id: id, iid: iid)
resolve_group_iterations(start_date: start_date, end_date: end_date, state: 'closed', title: search, id: 'gid://gitlab/Iteration/1', iid: iid)
end
it 'accepts a raw model id for backward compatibility' do
id = 1
iid = 2
params = { id: id, iid: iid, group_ids: group.id, state: 'all', start_date: nil, end_date: nil, search_title: nil }
expect(IterationsFinder).to receive(:new).with(current_user, params).and_call_original
resolve_group_iterations(id: id, iid: iid)
end
end
......
......@@ -50,6 +50,7 @@ RSpec.describe 'Querying an Iteration' do
let_it_be(:project_iteration) { create(:iteration, :skip_project_validation, project: project) }
shared_examples 'scoped path' do
let(:queried_iteration_id) { queried_iteration.to_global_id.to_s }
let(:iteration_nodes) do
nodes = <<~NODES
nodes {
......@@ -58,7 +59,7 @@ RSpec.describe 'Querying an Iteration' do
}
NODES
query_graphql_field('iterations', { id: queried_iteration.id }, nodes)
query_graphql_field('iterations', { id: queried_iteration_id }, nodes)
end
before_all do
......@@ -68,6 +69,14 @@ RSpec.describe 'Querying an Iteration' do
specify do
expect(subject).to include('scopedPath' => expected_scope_path, 'scopedUrl' => expected_scope_url)
end
context 'when given a raw model id (backward compatibility)' do
let(:queried_iteration_id) { queried_iteration.id }
specify do
expect(subject).to include('scopedPath' => expected_scope_path, 'scopedUrl' => expected_scope_url)
end
end
end
context 'inside a project context' do
......
......@@ -21,7 +21,7 @@ RSpec.describe 'Updating an Iteration' do
end
let(:mutation) do
params = { group_path: group.full_path, id: iteration.id }.merge(attributes)
params = { group_path: group.full_path, id: iteration.to_global_id.to_s }.merge(attributes)
graphql_mutation(:update_iteration, params)
end
......@@ -53,9 +53,12 @@ RSpec.describe 'Updating an Iteration' do
stub_licensed_features(iterations: false)
end
it_behaves_like 'a mutation that returns top-level errors',
errors: ['The resource that you are attempting to access does not '\
'exist or you don\'t have permission to perform this action']
it_behaves_like 'a mutation that returns top-level errors' do
let(:match_errors) do
include('The resource that you are attempting to access does not '\
'exist or you don\'t have permission to perform this action')
end
end
end
context 'when iterations are enabled' do
......@@ -92,6 +95,18 @@ RSpec.describe 'Updating an Iteration' do
end
end
context 'when given a raw model id (backward compatibility)' do
let(:attributes) { { id: iteration.id, title: 'title' } }
it 'updates the iteration' do
post_graphql_mutation(mutation, current_user: current_user)
iteration_hash = mutation_response['iteration']
expect(iteration_hash['title']).to eq('title')
expect(iteration.reload.title).to eq('title')
end
end
context 'when the list of attributes is empty' do
let(:attributes) { {} }
......
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