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 ra-data-graphql-simple creates incorrect query when schema contains a non-null list #9371

Merged
merged 3 commits into from
Oct 24, 2023
Merged

Fix ra-data-graphql-simple creates incorrect query when schema contains a non-null list #9371

merged 3 commits into from
Oct 24, 2023

Conversation

dricholm
Copy link
Contributor

Fixes #9370, since I couldn't provide a sandbox environment due to GraphQL endpoint requirement. Unit tests should help illustrate the case.

Nullability and lists can be nested, which was incorrectly handled previously. A scenario:

  • Have an argument with both list and item non-nullable with any scalar, e.g. ID
  • ra-data-graphql-simple generates: [ID!] (item is non-nullable, while list is nullable), however it should be [ID!]!
  • Above result causes an error on server due to incorrect type.

Moved logic to a new file and added additional unit tests.

- Handle nested lists and non-null types.
Copy link
Contributor

@mchaffotte mchaffotte left a comment

Choose a reason for hiding this comment

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

Nice catch

packages/ra-data-graphql-simple/src/buildGqlQuery.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@slax57 slax57 left a comment

Choose a reason for hiding this comment

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

Your fix looks great, thanks!

You simply have 2 linter warnings left in buildGqlQuery.ts, due to unused imports.
Can you fix them?

@dricholm
Copy link
Contributor Author

Your fix looks great, thanks!

You simply have 2 linter warnings left in buildGqlQuery.ts, due to unused imports. Can you fix them?

Sure, thanks for the notice. Removed them.

Copy link
Contributor

@slax57 slax57 left a comment

Choose a reason for hiding this comment

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

Thanks!

@slax57 slax57 added this to the 4.15.2 milestone Oct 24, 2023
@slax57 slax57 merged commit 1b2b556 into marmelab:master Oct 24, 2023
10 checks passed
@slax57 slax57 changed the title fix(ra-data-graphql-simple): Handle nested types Fix ra-data-graphql-simple creates incorrect query when schema contains a non-null list Oct 24, 2023
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.

Incorrect handling of lists and non-nullability in arguments by ra-data-graphql-simple
3 participants