Merge pull request #5480 from Shopify/ravangen.invalid-empty-selection-on-union-type

Pass `resolved_type` into `legacy_invalid_empty_selections_on_union`

Author: rmosolgo

lib/graphql/schema.rb

@@ -1708,7 +1708,7 @@ def did_you_mean(new_dym = NOT_CONFIGURED)
       # If you need to support previous, non-spec behavior which allowed selecting union fields
       # but *not* selecting any fields on that union, set this to `true` to continue allowing that behavior.
       #
-      # If this is `true`, then {.legacy_invalid_empty_selections_on_union} will be called with {Query} objects
+      # If this is `true`, then {.legacy_invalid_empty_selections_on_union_with_type} will be called with {Query} objects
       # with that kind of selections. You must implement that method
       # @param new_value [Boolean]
       # @return [true, false, nil]
@@ -1724,18 +1724,35 @@ def allow_legacy_invalid_empty_selections_on_union(new_value = NOT_CONFIGURED)
         end
       end
 
+      # This method is called during validation when a previously-allowed, but non-spec
+      # query is encountered where a union field has no child selections on it.
+      #
+      # If `legacy_invalid_empty_selections_on_union_with_type` is overridden, this method will not be called.
+      #
+      # You should implement this method or `legacy_invalid_empty_selections_on_union_with_type`
+      # to log the violation so that you can contact clients and notify them about changing their queries.
+      # Then return a suitable value to tell GraphQL-Ruby how to continue.
+      # @param query [GraphQL::Query]
+      # @return [:return_validation_error] Let GraphQL-Ruby return the (new) normal validation error for this query
+      # @return [String] A validation error to return for this query
+      # @return [nil] Don't send the client an error, continue the legacy behavior (allow this query to execute)
+      def legacy_invalid_empty_selections_on_union(query)
+        raise "Implement `def self.legacy_invalid_empty_selections_on_union_with_type(query, type)` or `def self.legacy_invalid_empty_selections_on_union(query)` to handle this scenario"
+      end
+
       # This method is called during validation when a previously-allowed, but non-spec
       # query is encountered where a union field has no child selections on it.
       #
       # You should implement this method to log the violation so that you can contact clients
       # and notify them about changing their queries. Then return a suitable value to
       # tell GraphQL-Ruby how to continue.
       # @param query [GraphQL::Query]
+      # @param type1 [Module] A GraphQL type definition
       # @return [:return_validation_error] Let GraphQL-Ruby return the (new) normal validation error for this query
       # @return [String] A validation error to return for this query
       # @return [nil] Don't send the client an error, continue the legacy behavior (allow this query to execute)
-      def legacy_invalid_empty_selections_on_union(query)
-        raise "Implement `def self.legacy_invalid_empty_selections_on_union(query)` to handle this scenario"
+      def legacy_invalid_empty_selections_on_union_with_type(query, type)
+        legacy_invalid_empty_selections_on_union(query)
       end
 
       # This setting controls how GraphQL-Ruby handles overlapping selections on scalar types when the types

lib/graphql/static_validation/rules/fields_have_appropriate_selections.rb

@@ -49,7 +49,7 @@ def validate_field_selections(ast_node, resolved_type)
           if !resolved_type.kind.fields?
             case @schema.allow_legacy_invalid_empty_selections_on_union
             when true
-              legacy_invalid_empty_selection_result = @schema.legacy_invalid_empty_selections_on_union(@context.query)
+              legacy_invalid_empty_selection_result = @schema.legacy_invalid_empty_selections_on_union_with_type(@context.query, resolved_type)
               case legacy_invalid_empty_selection_result
               when :return_validation_error
                 # keep `return_validation_error = true`
@@ -61,7 +61,7 @@ def validate_field_selections(ast_node, resolved_type)
                 return_validation_error = false
                 legacy_invalid_empty_selection_result = nil
               else
-                raise GraphQL::InvariantError, "Unexpected return value from legacy_invalid_empty_selections_on_union, must be `:return_validation_error`, String, or nil (got: #{legacy_invalid_empty_selection_result.inspect})"
+                raise GraphQL::InvariantError, "Unexpected return value from legacy_invalid_empty_selections_on_union_with_type, must be `:return_validation_error`, String, or nil (got: #{legacy_invalid_empty_selection_result.inspect})"
               end
             when false
               # pass -- error below

spec/graphql/static_validation/rules/fields_have_appropriate_selections_spec.rb

@@ -90,6 +90,21 @@
 
   describe "selections on unions" do
     let(:query_string) { "{ searchDairy }"}
+    describe "When the schema has custom handling with type to return the message" do
+      let(:schema) { Class.new(Dummy::Schema) {
+          allow_legacy_invalid_empty_selections_on_union(true)
+          def self.legacy_invalid_empty_selections_on_union_with_type(query, type)
+            :return_validation_error
+          end
+        }
+      }
+
+      it "returns the default message" do
+        expected_err = "Field must have selections (field 'searchDairy' returns DairyProduct but has no selections. Did you mean 'searchDairy { ... }'?)"
+        assert_includes(errors.map { |e| e["message"] }, expected_err)
+      end
+    end
+
     describe "When the schema has custom handling to return the message" do
       let(:schema) { Class.new(Dummy::Schema) {
           allow_legacy_invalid_empty_selections_on_union(true)
@@ -105,6 +120,21 @@ def self.legacy_invalid_empty_selections_on_union(query)
       end
     end
 
+    describe "When the schema has custom handling with type to return a custom message" do
+      let(:schema) { Class.new(Dummy::Schema) {
+          allow_legacy_invalid_empty_selections_on_union(true)
+          def self.legacy_invalid_empty_selections_on_union_with_type(query, type)
+            "Boo, hiss!"
+          end
+        }
+      }
+
+      it "returns the custom message" do
+        expected_err = "Boo, hiss!"
+        assert_includes(errors.map { |e| e["message"] }, expected_err)
+      end
+    end
+
     describe "When the schema has custom handling to return a custom message" do
       let(:schema) { Class.new(Dummy::Schema) {
           allow_legacy_invalid_empty_selections_on_union(true)
@@ -120,6 +150,20 @@ def self.legacy_invalid_empty_selections_on_union(query)
       end
     end
 
+    describe "When the schema has custom handling with type to allow the query" do
+      let(:schema) { Class.new(Dummy::Schema) {
+          allow_legacy_invalid_empty_selections_on_union(true)
+          def self.legacy_invalid_empty_selections_on_union_with_type(query, type)
+            nil
+          end
+        }
+      }
+
+      it "returns no errors" do
+        assert_equal [], errors
+      end
+    end
+
     describe "When the schema has custom handling to allow the query" do
       let(:schema) { Class.new(Dummy::Schema) {
           allow_legacy_invalid_empty_selections_on_union(true)