Some disjoint fields of the same union type are discarded about join-monster HOT 7 CLOSED

The the problem was that the two disjoint fields where coming from the db in different objects.
So the latter overrode the first one, that is why you were alway having just one of those.

Now I check if we have a result that is an object and a new piece comes, we just merge it.

{ author: { capitalizedLastName: "..." } } + { author: { email: "..." } } = { author: { capitalizedLastName: "...", email: "..." } }

in the end I was able to fix both modes with same approach in different places:

from join-monster.

@Grundlefleck I'd love your input in the solution I've proposed #519

And If you can tell me if you have a test for sqlBatch or how did you know it would not work there.

Thanks for such a detailed issue, investigation, pull request and patience.

from join-monster.

I added this test 06f2a26 and it passes in both batch and join modes.

I also tested that on master. with same results.

Note that there is an extra object, I believe disjoint fields need more work.

We could merge my PR that make multiple disjoint fields work in join mode which solves @Grundlefleck

Then work in PRs addressing:

• multiple disjoint in batch mode
• remove extra object in single disjoint in batch mode
• remove extra object in single disjoint in join mode

from join-monster.

fixed by #519

from join-monster.

Hi @nicoabie thanks for looking into it.

@Grundlefleck I'd love your input in the solution I've proposed #519

I confess I don't really understand how it works, but probably it's because I can't visualise the runtime types when moveProp is called. Is the strategy to merge (via Object.assign) the field representing the unioned type? So in the example test case, the author field would be like: { capitalizedLastName: "...", email: "..." } and that would be passed back?

And If you can tell me if you have a test for sqlBatch or how did you know it would not work there.

Only from running e.g. npm run test which would execute the same tests using STRATEGY=batch. I didn't even consider batch mode until I ran the test suite... which is a sign of a great test suite! 😄

Thanks for such a detailed issue, investigation, pull request and patience.

No, thank you.

from join-monster.

we just merge it.

Gotcha. I had in my head that the resultant object needed to match the fields being requested, and that any extra fields not requested in the GQL query input might trip some validation.

Not sure where I got that from, but if it's not a concern then making all fields available is way simpler than trying to wrangle a call to resolveType().

from join-monster.

Yes, It will match exactly the fields requested. The thing is that the fields came is different objects. That's all :)

from join-monster.