handle mutation that returns an array

[camnicklaus]

Hey 👋
so, first laid eyes on graphql AND this codebase for the first time last week, so bear that in mind :)
I feel like I'm likely just doing something wrong or have missed some config, however...

Issue:
we have a mutation that returns a list of records
in the schema it looks something like this:
createNewThing(input: CustomInput!): [NewThingRecord]

input CustomInput {
  newThingTypeId: ID!
  newThingCount: Int!
}

and in our handler we have something along these lines:

  options.mutations.createNewThing = (newThings, { input }, _db) => {

    return [newThings.insert(_db.newThingTypesList.find(input.newThingTypeId))]
  }

the problem is that newThings that is passed to the mutation function is undefined here because the returnType name is nested under ofType instead of being top level in /mocks.mutations

the work around we currently have is to just use the _db instance that's also passed but I'm wondering if we're just doing something else wrong.
Maybe it's not common practice to return a list from a mutation?
or maybe I'm just missing some other configuration?

anyway... would love any eyes/input on this.
I struggled a bit to follow the code that gets the returnType, I wonder if maybe some checking could be done earlier in the chain? if I'm not just doing something else dumb that is ;)

[camnicklaus]

thought I'd add this shot in case it's helpful (took while in the process of debugging)

[jneurock]

Hi, thanks for the PR! It looks like you found an issue with looking up the correct return type for mutations. I agree that the code is hard to follow. I imagine all of it is. The add-on was coded rather quickly and definitely needs a lot of work.

The fix you added looks good; however, it does need a test. Let me know if you need some help adding a test, otherwise, have at it.

Thanks!

[camnicklaus]

@jneurock right on, I suppose I could use a hand with a test. maybe just point me in the right direction?
my thought was to create a new acceptance test with a new route in the dummy app, person/new, along with a createPerson mutation etc... but that seems like maybe too much?
I'd like to dig in a little more and find where the return type is pulled in, seems like there might be a better spot to check whether it's top level or not.

[jneurock]

Acceptance test would be fine. That's how all the queries and mutations are currently tested. It seems like you outlined a fine test scenario above.

[camnicklaus]

@jneurock how's this looking?

[jneurock]

Looks great. Thanks for finding/fixing the issue!