Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: handle union type response #62

Merged
merged 2 commits into from
Oct 13, 2024

Conversation

chrstnmcf
Copy link
Contributor

@chrstnmcf chrstnmcf commented Sep 30, 2024

The graphql-composer was not able to handle union type return types so when a query/mutation had a union type response (e.g. UpdateAuthorAddressResponse) while collecting the queries the graphql composer tried to access for example the field UpdateAuthorAddressResponse.id instead of Author.id which does not exist.

updateAuthorAddress(authorId: $authorId, address: $address) {
  __typename
  ...on Author {
    id name { firstName lastName }
  }
  ... on NotFoundError {
    message
  }
}

So I added a check if the return type is a union type and if this is the case it passes the type name of the fragment to collectNestedQueries so that when collecting the nested queries for fragments, it knows the actual field type name.

Also in order to correctly build the queries again in the end a new fragment property was added to the query node selection so that when building the query for a union type response it wraps the selection with the fragment (e.g. ... on Author { ... }.

I added some test cases for this and this solution worked perfectly fine for our use cases so I hope I haven't missed anything.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina merged commit ecc5a2e into platformatic:main Oct 13, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants