Merge branch 'ajk-globalid-todos' into 'master'

[GraphQL] Todos: use Global-ID scalar

See merge request gitlab-org/gitlab!36112

app/graphql/mutations/todos/base.rb

@@ -11,16 +11,6 @@ module Mutations
         id = ::Types::GlobalIDType[::Todo].coerce_isolated_input(id)
         GitlabSchema.find_by_gid(id)
       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

app/graphql/mutations/todos/mark_all_done.rb

@@ -8,7 +8,7 @@ module Mutations
       authorize :update_user
 
       field :updated_ids,
-            [GraphQL::ID_TYPE],
+            [::Types::GlobalIDType[::Todo]],
             null: false,
             deprecated: { reason: 'Use todos', milestone: '13.2' },
             description: 'Ids of the updated todos'
@@ -23,7 +23,7 @@ module Mutations
         updated_ids = mark_all_todos_done
 
         {
-          updated_ids: map_to_global_ids(updated_ids),
+          updated_ids: updated_ids,
           todos: Todo.id_in(updated_ids),
           errors: []
         }

app/graphql/mutations/todos/restore_many.rb

@@ -12,7 +12,7 @@ module Mutations
                required: true,
                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,
             description: 'The ids of the updated todo items',
             deprecated: { reason: 'Use todos', milestone: '13.2' }
@@ -28,7 +28,7 @@ module Mutations
         updated_ids = restore(todos)
 
         {
-            updated_ids: gids_of(updated_ids),
+            updated_ids: updated_ids,
             todos: Todo.id_in(updated_ids),
             errors: errors_on_objects(todos)
         }
@@ -36,10 +36,6 @@ module Mutations
 
       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)
         ids.map do |gid|
           # TODO: remove this line when the compatibility layer is removed

doc/api/graphql/reference/gitlab_schema.graphql

@@ -20841,7 +20841,7 @@ type TodoRestoreManyPayload {
   """
   The ids of the updated todo items. Deprecated in 13.2: Use todos
   """
-  updatedIds: [ID!]! @deprecated(reason: "Use todos. Deprecated in 13.2")
+  updatedIds: [TodoID!]! @deprecated(reason: "Use todos. Deprecated in 13.2")
 }
 
 """
@@ -20938,7 +20938,7 @@ type TodosMarkAllDonePayload {
   """
   Ids of the updated todos. Deprecated in 13.2: Use todos
   """
-  updatedIds: [ID!]! @deprecated(reason: "Use todos. Deprecated in 13.2")
+  updatedIds: [TodoID!]! @deprecated(reason: "Use todos. Deprecated in 13.2")
 }
 
 """

doc/api/graphql/reference/gitlab_schema.json

@@ -60657,7 +60657,7 @@
                     "name": null,
                     "ofType": {
                       "kind": "SCALAR",
-                      "name": "ID",
+                      "name": "TodoID",
                       "ofType": null
                     }
                   }
@@ -60934,7 +60934,7 @@
                     "name": null,
                     "ofType": {
                       "kind": "SCALAR",
-                      "name": "ID",
+                      "name": "TodoID",
                       "ofType": null
                     }
                   }

doc/api/graphql/reference/index.md

@@ -3077,7 +3077,7 @@ Autogenerated return type of TodoRestoreMany.
 | `clientMutationId` | String | A unique identifier for the client performing the mutation. |
 | `errors` | String! => Array | Errors encountered during execution of the mutation. |
 | `todos` | Todo! => Array | Updated todos |
-| `updatedIds` **{warning-solid}** | ID! => Array | **Deprecated:** Use todos. Deprecated in 13.2 |
+| `updatedIds` **{warning-solid}** | TodoID! => Array | **Deprecated:** Use todos. Deprecated in 13.2 |
 
 ### TodoRestorePayload
 
@@ -3098,7 +3098,7 @@ Autogenerated return type of TodosMarkAllDone.
 | `clientMutationId` | String | A unique identifier for the client performing the mutation. |
 | `errors` | String! => Array | Errors encountered during execution of the mutation. |
 | `todos` | Todo! => Array | Updated todos |
-| `updatedIds` **{warning-solid}** | ID! => Array | **Deprecated:** Use todos. Deprecated in 13.2 |
+| `updatedIds` **{warning-solid}** | TodoID! => Array | **Deprecated:** Use todos. Deprecated in 13.2 |
 
 ### ToggleAwardEmojiPayload
 

spec/graphql/mutations/todos/mark_all_done_spec.rb

@@ -28,7 +28,7 @@ RSpec.describe Mutations::Todos::MarkAllDone do
       expect(todo3.reload.state).to eq('done')
       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)
     end
 

spec/graphql/mutations/todos/restore_many_spec.rb

@@ -24,11 +24,11 @@ RSpec.describe Mutations::Todos::RestoreMany do
       expect(todo2.reload.state).to eq('pending')
       expect(other_user_todo.reload.state).to eq('done')
 
-      todo_ids = result[:updated_ids]
-      expect(todo_ids.size).to eq(1)
-      expect(todo_ids.first).to eq(todo1.to_global_id.to_s)
-
-      expect(result[:todos]).to contain_exactly(todo1)
+      expect(result).to match(
+        errors: be_empty,
+        updated_ids: contain_exactly(todo1.id),
+        todos: contain_exactly(todo1)
+      )
     end
 
     it 'handles a todo which is already pending as expected' do
@@ -36,8 +36,11 @@ RSpec.describe Mutations::Todos::RestoreMany do
 
       expect_states_were_not_changed
 
-      expect(result[:updated_ids]).to eq([])
-      expect(result[:todos]).to be_empty
+      expect(result).to match(
+        errors: be_empty,
+        updated_ids: be_empty,
+        todos: be_empty
+      )
     end
 
     it 'ignores requests for todos which do not belong to the current user' do
@@ -61,7 +64,7 @@ RSpec.describe Mutations::Todos::RestoreMany do
       expect(result[:updated_ids].size).to eq(2)
 
       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(todo1.reload.state).to eq('pending')

spec/requests/api/graphql/mutations/todos/restore_many_spec.rb

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