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

exclude error ressources from graphql #5848

Merged
merged 2 commits into from
Oct 4, 2023

Conversation

jotwea
Copy link
Contributor

@jotwea jotwea commented Sep 22, 2023

Q A
Branch? 3.2
Tickets Closes api-platform/api-platform#2515 references #4994
License MIT
Doc PR none

I noticed that PR #5433 introduced side effects on the GraphQL side which lead to an invalid schema.
{ "errors": [ { "message": "Names must only contain [_a-zA-Z0-9] but \"hydra:title\" does not.", "extensions": {}, "stack": "GraphQLError: Names must only contain [_a-zA-Z0-9] but \"hydra:title\" does not." } ] }

This PR just excludes the Error resources for graphql
see #4994 (comment)

@soyuka
Copy link
Member

soyuka commented Sep 28, 2023

can't we introduce a graphql error operation instead?

@soyuka
Copy link
Member

soyuka commented Sep 28, 2023

I'd like a little bit more informations about this, also a test covering the issue would be nice not sure if that's the best option

@jotwea
Copy link
Contributor Author

jotwea commented Sep 28, 2023

At the moment the Graphql API is broken - the schema is invalid because hydra:title and hydra:description are no proper field names. You can see this error when you open the GraphiQL IDE. This PR is meant to be a simple fix that the schema will be valid again.

I do not know much about this error resource and cannot judge if this is relevant for GraphQL and how to implement this properly. Feel free to work on that solution. But still I would vote to merge this PR before as a quick fix.

@soyuka
Copy link
Member

soyuka commented Sep 28, 2023

we need a regression test and to fix serialization groups indeed! I'll look into that.

@soyuka
Copy link
Member

soyuka commented Oct 4, 2023

We investigated and I now understand better what's going on... I'm trying to find a way to test this.

@jotwea
Copy link
Contributor Author

jotwea commented Oct 4, 2023

Thx a lot

@soyuka soyuka merged commit 2080936 into api-platform:main Oct 4, 2023
42 of 43 checks passed
@soyuka
Copy link
Member

soyuka commented Oct 4, 2023

Thanks @jotwea !

@jotwea jotwea deleted the error-resource-graphql-fix branch October 13, 2023 12:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

hydra:title and hydra:description fields are not GraphQL valid names
3 participants