2.3.1 changes previous mutation behaviour re optional arguments

[sporto]

Describe the bug

Previously input object will include all arguments in the hash (even if not present in the request).
Now they only include arguments that are present.

Versions

graphql version 2.3.1

Steps to reproduce

Given an input object like

class SomeInput < Types::BaseInputObject
  argument :a, String, required: true
  argument :b, String, required: false
end

When b is not in the request.

In 2.3.0 the mutation receives:

{
  :a => "xyz"
  :b => nil
}

In 2.3.1 the mutation receives

{
  :a => "xyz"
}

Expected behavior

Unsure if this intended, maybe keep previous behaviour.

This change breaks mutations that expect to find all attributes. e.g. when using DryStruct to automatically convert the hash into something else.

[rmosolgo]

Woah... I didn't change this on purpose, but I don't think it ever should have sent :b => nil if :b wasn't present in the request and :b didn't have a default value.

But I wonder what changed that 😖 ...

[dgodd]

I'm not sure if this is related, but in 2.3.1 (but not 2.3.0) the data for prepare on on array based input argument has switched from being an array of the prepared objects, to an array of the input type. I can prepare an example and a seperate issue if this is unrelated.

[rmosolgo]

Hey @dgodd, thanks for reporting that. I just shipped 2.3.2 to Rubygems with a fix for that issue (#4933).

@sporto, I'm sorry for the weird change but I think the new behavior is the right one: if the argument doesn't have a client-provided value and it doesn't have a default value, then it won't be present in the hash. You could get the old behavior by adding default_value: nil to your schema. (That would make it b: Int = null in GraphQL, too.)

I added a spec for this specific case in 549fe88 so I can keep a better eye on it in the future.

If you find anything further that suggests something should be fixed here, or if you have trouble working around this change, please let me know and we can look for other things to address it!

[sporto]

@rmosolgo thanks

However, not sure what has been done here, you said you fixed the issue, but then said that this behaviour (not included those keys) is the right one. What does fixing the issue means?

I upgraded to 2.3.2 but odly it didn't break our code, I was expecting it to break as per 2.3.1 (by having those keys not present)

I think this spec is not quite right
549fe88

See

def result(values:)
          "[a: #{values[:a].inspect}, #{values.key?(:a)}], [b: #{values[:b].inspect}, #{values.key?([:b])}], [c: #{values[:c].inspect}, #{values.key?(:c)}]"
        end

This will always return false:

values.key?([:b])

#4938

[rmosolgo]

Hey @sporto, sorry for the confusion. @dgodd had reported a different issue, which I fixed. I wasn't able to replicate your issue so I didn't do anything special to fix it.

However, if you found that 2.3.2 works for you, then maybe you were affected by the same bug. Where you using a list of input objects in your query? That's what my previous fix was for.

[sporto]

@rmosolgo thanks, yes we are using lists of input objects, that was probably the initial issue I saw