From 1236da948a0a658de7026c2ef106c42293a36e48 Mon Sep 17 00:00:00 2001 From: Riccardo Binetti Date: Wed, 31 Jan 2024 22:50:04 +0100 Subject: [PATCH 1/2] improvement: make mutation arguments non-null As discussed in #105 and #110, put this behind an opt-in configuration to avoid breaking existing code. The ID in update mutations is always non-null if non-null mutation arguments are allowed, while input is non-null if it's allowed _and_ there is at least a non-null field in the input. Document the newly added config variable in the getting started guide. --- .../tutorials/getting-started-with-graphql.md | 1 + lib/resource/resource.ex | 38 +++++++++++++++---- 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/documentation/tutorials/getting-started-with-graphql.md b/documentation/tutorials/getting-started-with-graphql.md index ecef21d9..f5c6af27 100644 --- a/documentation/tutorials/getting-started-with-graphql.md +++ b/documentation/tutorials/getting-started-with-graphql.md @@ -21,6 +21,7 @@ in `config/config.exs` ```elixir config :ash_graphql, :default_managed_relationship_type_name_template, :action_name +config :ash_graphql, :allow_non_null_mutation_arguments?, true ``` This won't be necessary after the next major release, where this new configuration will be the default. diff --git a/lib/resource/resource.ex b/lib/resource/resource.ex index c7f60637..569b7baf 100644 --- a/lib/resource/resource.ex +++ b/lib/resource/resource.ex @@ -552,14 +552,14 @@ defmodule AshGraphql.Resource do [] -> [] - _ -> + fields -> [ %Absinthe.Blueprint.Schema.InputValueDefinition{ identifier: :input, module: schema, name: "input", placement: :argument_definition, - type: String.to_atom("#{name}_input") + type: mutation_input_type(name, fields) } ] end @@ -621,14 +621,14 @@ defmodule AshGraphql.Resource do [] -> [] - _ -> + fields -> [ %Absinthe.Blueprint.Schema.InputValueDefinition{ identifier: :input, module: schema, name: "input", placement: :argument_definition, - type: String.to_atom("#{mutation.name}_input") + type: mutation_input_type(mutation.name, fields) } ] end @@ -672,7 +672,7 @@ defmodule AshGraphql.Resource do [] -> mutation_args(mutation, resource, schema) - _ -> + fields -> mutation_args(mutation, resource, schema) ++ [ %Absinthe.Blueprint.Schema.InputValueDefinition{ @@ -680,7 +680,7 @@ defmodule AshGraphql.Resource do module: schema, name: "input", placement: :argument_definition, - type: String.to_atom("#{mutation.name}_input"), + type: mutation_input_type(mutation.name, fields), __reference__: ref(__ENV__) } ] @@ -730,6 +730,12 @@ defmodule AshGraphql.Resource do |> Enum.concat(mutation_read_args(mutation, resource, schema)) end + @allow_non_null_mutation_arguments? Application.compile_env( + :ash_graphql, + :allow_non_null_mutation_arguments?, + false + ) + defp mutation_args(mutation, resource, schema) do [ %Absinthe.Blueprint.Schema.InputValueDefinition{ @@ -737,13 +743,31 @@ defmodule AshGraphql.Resource do module: schema, name: "id", placement: :argument_definition, - type: :id, + type: maybe_wrap_non_null(:id, @allow_non_null_mutation_arguments?), __reference__: ref(__ENV__) } | mutation_read_args(mutation, resource, schema) ] end + # sobelow_skip ["DOS.StringToAtom"] + defp mutation_input_type(mutation_name, mutation_fields) do + any_non_null_field? = + mutation_fields + |> Enum.any?(fn + %Absinthe.Blueprint.Schema.FieldDefinition{ + type: %Absinthe.Blueprint.TypeReference.NonNull{} + } -> + true + + _ -> + false + end) + + String.to_atom("#{mutation_name}_input") + |> maybe_wrap_non_null(any_non_null_field? and @allow_non_null_mutation_arguments?) + end + defp mutation_read_args(%{read_action: read_action}, resource, schema) do read_action = cond do From 182bedcfafff7c8292651f743e5efab47fd49ab7 Mon Sep 17 00:00:00 2001 From: Riccardo Binetti Date: Wed, 31 Jan 2024 23:02:58 +0100 Subject: [PATCH 2/2] chore: enable non-null mutation arguments in tests --- config/config.exs | 1 + test/create_test.exs | 2 +- test/destroy_test.exs | 10 +++++----- test/update_test.exs | 6 +++--- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/config/config.exs b/config/config.exs index ea4325c7..dee34a92 100644 --- a/config/config.exs +++ b/config/config.exs @@ -6,6 +6,7 @@ config :ash, :validate_api_resource_inclusion?, false config :ash, :validate_api_config_inclusion?, false config :ash_graphql, :default_managed_relationship_type_name_template, :action_name +config :ash_graphql, :allow_non_null_mutation_arguments?, true if Mix.env() == :dev do config :git_ops, diff --git a/test/create_test.exs b/test/create_test.exs index 22790bea..3e10dd85 100644 --- a/test/create_test.exs +++ b/test/create_test.exs @@ -376,7 +376,7 @@ defmodule AshGraphql.CreateTest do test "a create with a managed relationship works with many_to_many and [on_lookup: :relate, on_match: :relate]" do resp = """ - mutation CreatePostWithCommentsAndTags($input: CreatePostWithCommentsAndTagsInput) { + mutation CreatePostWithCommentsAndTags($input: CreatePostWithCommentsAndTagsInput!) { createPostWithCommentsAndTags(input: $input) { result{ text diff --git a/test/destroy_test.exs b/test/destroy_test.exs index 52a05177..c101b2d9 100644 --- a/test/destroy_test.exs +++ b/test/destroy_test.exs @@ -14,7 +14,7 @@ defmodule AshGraphql.DestroyTest do resp = """ - mutation DeletePost($id: ID) { + mutation DeletePost($id: ID!) { deletePost(id: $id) { result{ text @@ -42,7 +42,7 @@ defmodule AshGraphql.DestroyTest do resp = """ - mutation ArchivePost($id: ID) { + mutation ArchivePost($id: ID!) { deletePost(id: $id) { result{ text @@ -96,7 +96,7 @@ defmodule AshGraphql.DestroyTest do resp = """ - mutation DeleteWithError($id: ID) { + mutation DeleteWithError($id: ID!) { deletePostWithError(id: $id) { result{ text @@ -128,7 +128,7 @@ defmodule AshGraphql.DestroyTest do test "destroying a non-existent record returns a not found error" do resp = """ - mutation DeletePost($id: ID) { + mutation DeletePost($id: ID!) { deletePost(id: $id) { result{ text @@ -162,7 +162,7 @@ defmodule AshGraphql.DestroyTest do resp = """ - mutation DeletePost($id: ID) { + mutation DeletePost($id: ID!) { deletePostWithError(id: $id) { result{ text diff --git a/test/update_test.exs b/test/update_test.exs index 760219df..d02223f1 100644 --- a/test/update_test.exs +++ b/test/update_test.exs @@ -19,7 +19,7 @@ defmodule AshGraphql.UpdateTest do resp = """ - mutation UpdatePost($id: ID, $input: UpdatePostInput) { + mutation UpdatePost($id: ID!, $input: UpdatePostInput) { updatePost(id: $id, input: $input) { result{ text @@ -209,7 +209,7 @@ defmodule AshGraphql.UpdateTest do resp = """ - mutation UpdatePostConfirm($input: UpdatePostConfirmInput, $id: ID) { + mutation UpdatePostConfirm($input: UpdatePostConfirmInput, $id: ID!) { updatePostConfirm(input: $input, id: $id) { result{ text @@ -253,7 +253,7 @@ defmodule AshGraphql.UpdateTest do resp = """ - mutation UpdatePostConfirm($input: UpdatePostConfirmInput, $id: ID) { + mutation UpdatePostConfirm($input: UpdatePostConfirmInput, $id: ID!) { updatePostConfirm(input: $input, id: $id) { result{ text