Commit 04cbb3b9 authored by Alex Kalderimis's avatar Alex Kalderimis Committed by Alex Kalderimis

To-Dos: use GlobalID scalar

This uses the new GlobalID scalar type which provides the following
advantages:

- more declarative, intentional code: readers of our schema and queries
  can more easily see what values are valid.
- more robust resolution and mutation. We get validations for free by
  using these types, particularly if we use the parameterized variants.
- We can remove some manual casting for fields
parent 509feeef
...@@ -11,16 +11,6 @@ module Mutations ...@@ -11,16 +11,6 @@ module Mutations
id = ::Types::GlobalIDType[::Todo].coerce_isolated_input(id) id = ::Types::GlobalIDType[::Todo].coerce_isolated_input(id)
GitlabSchema.find_by_gid(id) GitlabSchema.find_by_gid(id)
end end
def map_to_global_ids(ids)
return [] if ids.blank?
ids.map { |id| to_global_id(id) }
end
def to_global_id(id)
Gitlab::GlobalId.as_global_id(id, model_name: Todo.name).to_s
end
end end
end end
end end
...@@ -8,7 +8,7 @@ module Mutations ...@@ -8,7 +8,7 @@ module Mutations
authorize :update_user authorize :update_user
field :updated_ids, field :updated_ids,
[GraphQL::ID_TYPE], [::Types::GlobalIDType[::Todo]],
null: false, null: false,
deprecated: { reason: 'Use todos', milestone: '13.2' }, deprecated: { reason: 'Use todos', milestone: '13.2' },
description: 'Ids of the updated todos' description: 'Ids of the updated todos'
...@@ -23,7 +23,7 @@ module Mutations ...@@ -23,7 +23,7 @@ module Mutations
updated_ids = mark_all_todos_done updated_ids = mark_all_todos_done
{ {
updated_ids: map_to_global_ids(updated_ids), updated_ids: updated_ids,
todos: Todo.id_in(updated_ids), todos: Todo.id_in(updated_ids),
errors: [] errors: []
} }
......
...@@ -12,7 +12,7 @@ module Mutations ...@@ -12,7 +12,7 @@ module Mutations
required: true, required: true,
description: 'The global ids of the todos to restore (a maximum of 50 is supported at once)' description: 'The global ids of the todos to restore (a maximum of 50 is supported at once)'
field :updated_ids, [GraphQL::ID_TYPE], field :updated_ids, [::Types::GlobalIDType[Todo]],
null: false, null: false,
description: 'The ids of the updated todo items', description: 'The ids of the updated todo items',
deprecated: { reason: 'Use todos', milestone: '13.2' } deprecated: { reason: 'Use todos', milestone: '13.2' }
...@@ -28,7 +28,7 @@ module Mutations ...@@ -28,7 +28,7 @@ module Mutations
updated_ids = restore(todos) updated_ids = restore(todos)
{ {
updated_ids: gids_of(updated_ids), updated_ids: updated_ids,
todos: Todo.id_in(updated_ids), todos: Todo.id_in(updated_ids),
errors: errors_on_objects(todos) errors: errors_on_objects(todos)
} }
...@@ -36,10 +36,6 @@ module Mutations ...@@ -36,10 +36,6 @@ module Mutations
private private
def gids_of(ids)
ids.map { |id| Gitlab::GlobalId.as_global_id(id, model_name: Todo.name).to_s }
end
def model_ids_of(ids) def model_ids_of(ids)
ids.map do |gid| ids.map do |gid|
# TODO: remove this line when the compatibility layer is removed # TODO: remove this line when the compatibility layer is removed
......
...@@ -28,7 +28,7 @@ RSpec.describe Mutations::Todos::MarkAllDone do ...@@ -28,7 +28,7 @@ RSpec.describe Mutations::Todos::MarkAllDone do
expect(todo3.reload.state).to eq('done') expect(todo3.reload.state).to eq('done')
expect(other_user_todo.reload.state).to eq('pending') expect(other_user_todo.reload.state).to eq('pending')
expect(updated_todo_ids).to contain_exactly(global_id_of(todo1), global_id_of(todo3)) expect(updated_todo_ids).to contain_exactly(todo1.id, todo3.id)
expect(todos).to contain_exactly(todo1, todo3) expect(todos).to contain_exactly(todo1, todo3)
end end
......
...@@ -24,11 +24,11 @@ RSpec.describe Mutations::Todos::RestoreMany do ...@@ -24,11 +24,11 @@ RSpec.describe Mutations::Todos::RestoreMany do
expect(todo2.reload.state).to eq('pending') expect(todo2.reload.state).to eq('pending')
expect(other_user_todo.reload.state).to eq('done') expect(other_user_todo.reload.state).to eq('done')
todo_ids = result[:updated_ids] expect(result).to match(
expect(todo_ids.size).to eq(1) errors: be_empty,
expect(todo_ids.first).to eq(todo1.to_global_id.to_s) updated_ids: contain_exactly(todo1.id),
todos: contain_exactly(todo1)
expect(result[:todos]).to contain_exactly(todo1) )
end end
it 'handles a todo which is already pending as expected' do it 'handles a todo which is already pending as expected' do
...@@ -36,8 +36,11 @@ RSpec.describe Mutations::Todos::RestoreMany do ...@@ -36,8 +36,11 @@ RSpec.describe Mutations::Todos::RestoreMany do
expect_states_were_not_changed expect_states_were_not_changed
expect(result[:updated_ids]).to eq([]) expect(result).to match(
expect(result[:todos]).to be_empty errors: be_empty,
updated_ids: be_empty,
todos: be_empty
)
end end
it 'ignores requests for todos which do not belong to the current user' do it 'ignores requests for todos which do not belong to the current user' do
...@@ -61,7 +64,7 @@ RSpec.describe Mutations::Todos::RestoreMany do ...@@ -61,7 +64,7 @@ RSpec.describe Mutations::Todos::RestoreMany do
expect(result[:updated_ids].size).to eq(2) expect(result[:updated_ids].size).to eq(2)
returned_todo_ids = result[:updated_ids] returned_todo_ids = result[:updated_ids]
expect(returned_todo_ids).to contain_exactly(todo1.to_global_id.to_s, todo4.to_global_id.to_s) expect(returned_todo_ids).to contain_exactly(todo1.id, todo4.id)
expect(result[:todos]).to contain_exactly(todo1, todo4) expect(result[:todos]).to contain_exactly(todo1, todo4)
expect(todo1.reload.state).to eq('pending') expect(todo1.reload.state).to eq('pending')
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe 'Restoring many Todos' do
include GraphqlHelpers
let_it_be(:current_user) { create(:user) }
let_it_be(:author) { create(:user) }
let_it_be(:other_user) { create(:user) }
let_it_be(:todo1) { create(:todo, user: current_user, author: author, state: :done) }
let_it_be(:todo2) { create(:todo, user: current_user, author: author, state: :done) }
let_it_be(:other_user_todo) { create(:todo, user: other_user, author: author, state: :done) }
let(:input_ids) { [todo1, todo2].map { |obj| global_id_of(obj) } }
let(:input) { { ids: input_ids } }
let(:mutation) do
graphql_mutation(:todo_restore_many, input,
<<-QL.strip_heredoc
clientMutationId
errors
updatedIds
todos {
id
state
}
QL
)
end
def mutation_response
graphql_mutation_response(:todo_restore_many)
end
it 'restores many todos' do
post_graphql_mutation(mutation, current_user: current_user)
expect(todo1.reload.state).to eq('pending')
expect(todo2.reload.state).to eq('pending')
expect(other_user_todo.reload.state).to eq('done')
expect(mutation_response).to include(
'errors' => be_empty,
'updatedIds' => match_array(input_ids),
'todos' => contain_exactly(
{ 'id' => global_id_of(todo1), 'state' => 'pending' },
{ 'id' => global_id_of(todo2), 'state' => 'pending' }
)
)
end
context 'when using an invalid gid' do
let(:input_ids) { [global_id_of(author)] }
let(:invalid_gid_error) { /does not represent an instance of #{todo1.class}/ }
it 'contains the expected error' do
post_graphql_mutation(mutation, current_user: current_user)
errors = json_response['errors']
expect(errors).not_to be_blank
expect(errors.first['message']).to match(invalid_gid_error)
expect(todo1.reload.state).to eq('done')
expect(todo2.reload.state).to eq('done')
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