Mutations should return the mutated resource

This updates our todo mutations to follow our best-practices, which
state that mutations should return the mutated resource.

app/graphql/mutations/base_mutation.rb

@@ -7,6 +7,8 @@ module Mutations
 
     ERROR_MESSAGE = 'You cannot perform write operations on a read-only instance'
 
+    field_class ::Types::BaseField
+
     field :errors, [GraphQL::STRING_TYPE],
           null: false,
           description: 'Errors encountered during execution of the mutation.'

app/graphql/mutations/todos/mark_all_done.rb

@@ -10,8 +10,13 @@ module Mutations
       field :updated_ids,
             [GraphQL::ID_TYPE],
             null: false,
+            deprecated: { reason: 'Use todos', milestone: '13.2' },
             description: 'Ids of the updated todos'
 
+      field :todos, [::Types::TodoType],
+            null: false,
+            description: 'Updated todos'
+
       def resolve
         authorize!(current_user)
 
@@ -19,6 +24,7 @@ module Mutations
 
         {
           updated_ids: map_to_global_ids(updated_ids),
+          todos: Todo.id_in(updated_ids),
           errors: []
         }
       end

app/graphql/mutations/todos/restore_many.rb

@@ -14,7 +14,12 @@ module Mutations
 
       field :updated_ids, [GraphQL::ID_TYPE],
             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' }
+
+      field :todos, [::Types::TodoType],
+            null: false,
+            description: 'Updated todos'
 
       def resolve(ids:)
         check_update_amount_limit!(ids)
@@ -24,6 +29,7 @@ module Mutations
 
         {
             updated_ids: gids_of(updated_ids),
+            todos: Todo.id_in(updated_ids),
             errors: errors_on_objects(todos)
         }
       end

changelogs/unreleased/ajk-todos-return-resource.yml

+---
+title: Todo Mutations should return the mutated todos
+merge_request: 35998
+author:
+type: added

doc/api/graphql/reference/gitlab_schema.graphql

@@ -12824,9 +12824,14 @@ type TodoRestoreManyPayload {
   errors: [String!]!
 
   """
-  The ids of the updated todo items
+  Updated todos
   """
-  updatedIds: [ID!]!
+  todos: [Todo!]!
+
+  """
+  The ids of the updated todo items. Deprecated in 13.2: Use todos
+  """
+  updatedIds: [ID!]! @deprecated(reason: "Use todos. Deprecated in 13.2")
 }
 
 """
@@ -12906,9 +12911,14 @@ type TodosMarkAllDonePayload {
   errors: [String!]!
 
   """
-  Ids of the updated todos
+  Updated todos
+  """
+  todos: [Todo!]!
+
+  """
+  Ids of the updated todos. Deprecated in 13.2: Use todos
   """
-  updatedIds: [ID!]!
+  updatedIds: [ID!]! @deprecated(reason: "Use todos. Deprecated in 13.2")
 }
 
 """

doc/api/graphql/reference/gitlab_schema.json

@@ -37956,9 +37956,35 @@
               "isDeprecated": false,
               "deprecationReason": null
             },
+            {
+              "name": "todos",
+              "description": "Updated todos",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "NON_NULL",
+                "name": null,
+                "ofType": {
+                  "kind": "LIST",
+                  "name": null,
+                  "ofType": {
+                    "kind": "NON_NULL",
+                    "name": null,
+                    "ofType": {
+                      "kind": "OBJECT",
+                      "name": "Todo",
+                      "ofType": null
+                    }
+                  }
+                }
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
             {
               "name": "updatedIds",
-              "description": "The ids of the updated todo items",
+              "description": "The ids of the updated todo items. Deprecated in 13.2: Use todos",
               "args": [
 
               ],
@@ -37979,8 +38005,8 @@
                   }
                 }
               },
-              "isDeprecated": false,
-              "deprecationReason": null
+              "isDeprecated": true,
+              "deprecationReason": "Use todos. Deprecated in 13.2"
             }
           ],
           "inputFields": null,
@@ -38191,9 +38217,35 @@
               "isDeprecated": false,
               "deprecationReason": null
             },
+            {
+              "name": "todos",
+              "description": "Updated todos",
+              "args": [
+
+              ],
+              "type": {
+                "kind": "NON_NULL",
+                "name": null,
+                "ofType": {
+                  "kind": "LIST",
+                  "name": null,
+                  "ofType": {
+                    "kind": "NON_NULL",
+                    "name": null,
+                    "ofType": {
+                      "kind": "OBJECT",
+                      "name": "Todo",
+                      "ofType": null
+                    }
+                  }
+                }
+              },
+              "isDeprecated": false,
+              "deprecationReason": null
+            },
             {
               "name": "updatedIds",
-              "description": "Ids of the updated todos",
+              "description": "Ids of the updated todos. Deprecated in 13.2: Use todos",
               "args": [
 
               ],
@@ -38214,8 +38266,8 @@
                   }
                 }
               },
-              "isDeprecated": false,
-              "deprecationReason": null
+              "isDeprecated": true,
+              "deprecationReason": "Use todos. Deprecated in 13.2"
             }
           ],
           "inputFields": null,

doc/api/graphql/reference/index.md

@@ -1931,7 +1931,8 @@ 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. |
-| `updatedIds` | ID! => Array | The ids of the updated todo items |
+| `todos` | Todo! => Array | Updated todos |
+| `updatedIds` **{warning-solid}** | ID! => Array | **Deprecated:** Use todos. Deprecated in 13.2 |
 
 ## TodoRestorePayload
 
@@ -1951,7 +1952,8 @@ 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. |
-| `updatedIds` | ID! => Array | Ids of the updated todos |
+| `todos` | Todo! => Array | Updated todos |
+| `updatedIds` **{warning-solid}** | ID! => Array | **Deprecated:** Use todos. Deprecated in 13.2 |
 
 ## ToggleAwardEmojiPayload
 

spec/graphql/mutations/todos/mark_all_done_spec.rb

@@ -21,7 +21,7 @@ RSpec.describe Mutations::Todos::MarkAllDone do
 
   describe '#resolve' do
     it 'marks all pending todos as done' do
-      updated_todo_ids = mutation_for(current_user).resolve.dig(:updated_ids)
+      updated_todo_ids, todos = mutation_for(current_user).resolve.values_at(:updated_ids, :todos)
 
       expect(todo1.reload.state).to eq('done')
       expect(todo2.reload.state).to eq('done')
@@ -29,6 +29,7 @@ RSpec.describe Mutations::Todos::MarkAllDone do
       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(todos).to contain_exactly(todo1, todo3)
     end
 
     it 'behaves as expected if there are no todos for the requesting user' do

spec/graphql/mutations/todos/restore_many_spec.rb

@@ -25,6 +25,8 @@ RSpec.describe Mutations::Todos::RestoreMany do
       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)
     end
 
     it 'handles a todo which is already pending as expected' do
@@ -33,6 +35,7 @@ RSpec.describe Mutations::Todos::RestoreMany do
       expect_states_were_not_changed
 
       expect(result[:updated_ids]).to eq([])
+      expect(result[:todos]).to be_empty
     end
 
     it 'ignores requests for todos which do not belong to the current user' do
@@ -56,6 +59,7 @@ RSpec.describe Mutations::Todos::RestoreMany do
 
       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(result[:todos]).to contain_exactly(todo1, todo4)
 
       expect(todo1.reload.state).to eq('pending')
       expect(todo2.reload.state).to eq('pending')