From 783f3c63db5cef165c105d7ff151ba46cb52ad4f Mon Sep 17 00:00:00 2001 From: Alex Watt Date: Mon, 21 Oct 2024 15:49:01 -0400 Subject: [PATCH 1/2] Update validation message for one_of --- .../schema/validator/required_validator.rb | 29 ++++++++++++++++-- spec/graphql/schema/argument_spec.rb | 2 +- .../validator/required_validator_spec.rb | 30 +++++++++---------- spec/graphql/schema/validator_spec.rb | 4 +-- 4 files changed, 44 insertions(+), 21 deletions(-) diff --git a/lib/graphql/schema/validator/required_validator.rb b/lib/graphql/schema/validator/required_validator.rb index f939b15c1c..5ddf90e418 100644 --- a/lib/graphql/schema/validator/required_validator.rb +++ b/lib/graphql/schema/validator/required_validator.rb @@ -37,7 +37,7 @@ class Validator class RequiredValidator < Validator # @param one_of [Symbol, Array] An argument, or a list of arguments, that represents a valid set of inputs for this field # @param message [String] - def initialize(one_of: nil, argument: nil, message: "%{validated} has the wrong arguments", **default_options) + def initialize(one_of: nil, argument: nil, message: nil, **default_options) @one_of = if one_of one_of elsif argument @@ -49,7 +49,7 @@ def initialize(one_of: nil, argument: nil, message: "%{validated} has the wrong super(**default_options) end - def validate(_object, _context, value) + def validate(_object, context, value) matched_conditions = 0 if !value.nil? @@ -73,9 +73,32 @@ def validate(_object, _context, value) if matched_conditions == 1 nil # OK else - @message + @message || build_message(context) end end + + def build_message(context) + argument_definitions = @validated.arguments(context).values + required_names = @one_of.map do |arg_keyword| + if arg_keyword.is_a?(Array) + names = arg_keyword.map { |arg| arg_keyword_to_grapqhl_name(argument_definitions, arg) } + "(" + names.join(" and ") + ")" + else + arg_keyword_to_grapqhl_name(argument_definitions, arg_keyword) + end + end + + if required_names.size == 1 + "%{validated} must include the following argument: #{required_names.first}." + else + "%{validated} must include exactly one of the following arguments: #{required_names.join(", ")}." + end + end + + def arg_keyword_to_grapqhl_name(argument_definitions, arg_keyword) + argument_definition = argument_definitions.find { |defn| defn.keyword == arg_keyword } + argument_definition.graphql_name + end end end end diff --git a/spec/graphql/schema/argument_spec.rb b/spec/graphql/schema/argument_spec.rb index cc2079c6b2..2e8c88189d 100644 --- a/spec/graphql/schema/argument_spec.rb +++ b/spec/graphql/schema/argument_spec.rb @@ -604,7 +604,7 @@ def echo(str:) res = RequiredNullableSchema.execute('{ echo(str: null) }') assert_nil res["data"].fetch("echo") res = RequiredNullableSchema.execute('{ echo }') - assert_equal ["echo has the wrong arguments"], res["errors"].map { |e| e["message"] } + assert_equal ["echo must include the following argument: str."], res["errors"].map { |e| e["message"] } end end diff --git a/spec/graphql/schema/validator/required_validator_spec.rb b/spec/graphql/schema/validator/required_validator_spec.rb index b59813ac84..ea896ddcb3 100644 --- a/spec/graphql/schema/validator/required_validator_spec.rb +++ b/spec/graphql/schema/validator/required_validator_spec.rb @@ -9,10 +9,10 @@ { config: { one_of: [:a, :b] }, cases: [ - { query: "{ validated: multiValidated(a: 1, b: 2) }", result: nil, error_messages: ["multiValidated has the wrong arguments"] }, - { query: "{ validated: multiValidated(a: 1, b: 2, c: 3) }", result: nil, error_messages: ["multiValidated has the wrong arguments"] }, - { query: "{ validated: multiValidated }", result: nil, error_messages: ["multiValidated has the wrong arguments"] }, - { query: "{ validated: multiValidated(c: 3) }", result: nil, error_messages: ["multiValidated has the wrong arguments"] }, + { query: "{ validated: multiValidated(a: 1, b: 2) }", result: nil, error_messages: ["multiValidated must include exactly one of the following arguments: a, b."] }, + { query: "{ validated: multiValidated(a: 1, b: 2, c: 3) }", result: nil, error_messages: ["multiValidated must include exactly one of the following arguments: a, b."] }, + { query: "{ validated: multiValidated }", result: nil, error_messages: ["multiValidated must include exactly one of the following arguments: a, b."] }, + { query: "{ validated: multiValidated(c: 3) }", result: nil, error_messages: ["multiValidated must include exactly one of the following arguments: a, b."] }, { query: "{ validated: multiValidated(a: 1) }", result: 1, error_messages: [] }, { query: "{ validated: multiValidated(a: 1, c: 3) }", result: 4, error_messages: [] }, { query: "{ validated: multiValidated(b: 2) }", result: 2, error_messages: [] }, @@ -24,10 +24,10 @@ cases: [ { query: "{ validated: multiValidated(a: 1) }", result: 1, error_messages: [] }, { query: "{ validated: multiValidated(b: 2, c: 3) }", result: 5, error_messages: [] }, - { query: "{ validated: multiValidated }", result: nil, error_messages: ["multiValidated has the wrong arguments"] }, - { query: "{ validated: multiValidated(a: 1, b: 2, c: 3) }", result: nil, error_messages: ["multiValidated has the wrong arguments"] }, - { query: "{ validated: multiValidated(c: 3) }", result: nil, error_messages: ["multiValidated has the wrong arguments"] }, - { query: "{ validated: multiValidated(b: 2) }", result: nil, error_messages: ["multiValidated has the wrong arguments"] }, + { query: "{ validated: multiValidated }", result: nil, error_messages: ["multiValidated must include exactly one of the following arguments: a, (b and c)."] }, + { query: "{ validated: multiValidated(a: 1, b: 2, c: 3) }", result: nil, error_messages: ["multiValidated must include exactly one of the following arguments: a, (b and c)."] }, + { query: "{ validated: multiValidated(c: 3) }", result: nil, error_messages: ["multiValidated must include exactly one of the following arguments: a, (b and c)."] }, + { query: "{ validated: multiValidated(b: 2) }", result: nil, error_messages: ["multiValidated must include exactly one of the following arguments: a, (b and c)."] }, ] }, { @@ -36,9 +36,9 @@ cases: [ { query: "{ validated: validatedInput(input: { a: 1 }) }", result: 1, error_messages: [] }, { query: "{ validated: validatedInput(input: { b: 2, c: 3 }) }", result: 5, error_messages: [] }, - { query: "{ validated: validatedInput(input: { a: 1, b: 2, c: 3 }) }", result: nil, error_messages: ["ValidatedInput has the wrong arguments"] }, - { query: "{ validated: validatedInput(input: { c: 3 }) }", result: nil, error_messages: ["ValidatedInput has the wrong arguments"] }, - { query: "{ validated: validatedInput(input: { b: 2 }) }", result: nil, error_messages: ["ValidatedInput has the wrong arguments"] }, + { query: "{ validated: validatedInput(input: { a: 1, b: 2, c: 3 }) }", result: nil, error_messages: ["ValidatedInput must include exactly one of the following arguments: a, (b and c)."] }, + { query: "{ validated: validatedInput(input: { c: 3 }) }", result: nil, error_messages: ["ValidatedInput must include exactly one of the following arguments: a, (b and c)."] }, + { query: "{ validated: validatedInput(input: { b: 2 }) }", result: nil, error_messages: ["ValidatedInput must include exactly one of the following arguments: a, (b and c)."] }, ] }, { @@ -47,10 +47,10 @@ cases: [ { query: "{ validated: validatedResolver(a: 1) }", result: 1, error_messages: [] }, { query: "{ validated: validatedResolver(b: 2, c: 3) }", result: 5, error_messages: [] }, - { query: "{ validated: validatedResolver(a: 1, b: 2, c: 3) }", result: nil, error_messages: ["validatedResolver has the wrong arguments"] }, - { query: "{ validated: validatedResolver(c: 3) }", result: nil, error_messages: ["validatedResolver has the wrong arguments"] }, - { query: "{ validated: validatedResolver(b: 2) }", result: nil, error_messages: ["validatedResolver has the wrong arguments"] }, - { query: "{ validated: validatedResolver }", result: nil, error_messages: ["validatedResolver has the wrong arguments"] }, + { query: "{ validated: validatedResolver(a: 1, b: 2, c: 3) }", result: nil, error_messages: ["validatedResolver must include exactly one of the following arguments: a, (b and c)."] }, + { query: "{ validated: validatedResolver(c: 3) }", result: nil, error_messages: ["validatedResolver must include exactly one of the following arguments: a, (b and c)."] }, + { query: "{ validated: validatedResolver(b: 2) }", result: nil, error_messages: ["validatedResolver must include exactly one of the following arguments: a, (b and c)."] }, + { query: "{ validated: validatedResolver }", result: nil, error_messages: ["validatedResolver must include exactly one of the following arguments: a, (b and c)."] }, ] }, { diff --git a/spec/graphql/schema/validator_spec.rb b/spec/graphql/schema/validator_spec.rb index 4998486325..fc021d41da 100644 --- a/spec/graphql/schema/validator_spec.rb +++ b/spec/graphql/schema/validator_spec.rb @@ -195,7 +195,7 @@ def int_input(input:) res = ValidationInheritanceSchema.execute("{ intInput(input: { int: 1, otherInt: 2 }) }") assert_nil res["data"]["intInput"] - assert_equal ["IntInput has the wrong arguments"], res["errors"].map { |e| e["message"] } + assert_equal ["IntInput must include exactly one of the following arguments: int, otherInt."], res["errors"].map { |e| e["message"] } end it "works with resolvers" do @@ -204,7 +204,7 @@ def int_input(input:) res = ValidationInheritanceSchema.execute("{ int(int: 1, otherInt: 2) }") assert_nil res["data"]["int"] - assert_equal ["int has the wrong arguments"], res["errors"].map { |e| e["message"] } + assert_equal ["int must include exactly one of the following arguments: int, otherInt."], res["errors"].map { |e| e["message"] } end end end From 8e7cdc3510b32cad8e83d3717b9aedef591f1aea Mon Sep 17 00:00:00 2001 From: Alex Watt Date: Mon, 21 Oct 2024 16:14:57 -0400 Subject: [PATCH 2/2] Update docstring --- lib/graphql/schema/validator/required_validator.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/graphql/schema/validator/required_validator.rb b/lib/graphql/schema/validator/required_validator.rb index 5ddf90e418..d5afbb2da1 100644 --- a/lib/graphql/schema/validator/required_validator.rb +++ b/lib/graphql/schema/validator/required_validator.rb @@ -35,7 +35,8 @@ class Validator # end # class RequiredValidator < Validator - # @param one_of [Symbol, Array] An argument, or a list of arguments, that represents a valid set of inputs for this field + # @param one_of [Array] A list of arguments, exactly one of which is required for this field + # @param argument [Symbol] An argument that is required for this field # @param message [String] def initialize(one_of: nil, argument: nil, message: nil, **default_options) @one_of = if one_of