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 GraphQL schema localization. #17041

Merged
merged 22 commits into from
Nov 26, 2024

Conversation

gvkries
Copy link
Contributor

@gvkries gvkries commented Nov 20, 2024

The GraphQL schema is cached the first time it is being build and only one instance and language is being cached at all. This means that the schema language depends on the first request and does not reflect the currents user language at all.
I have now added a per-culture cache of the schema.

Additionally, parts of the schema are not localized at all. You may end up with a mixture of different languages in the schema. I added localization to different parts of the schema, to make this more complete.

Fixes #2765.

Copy link
Contributor

This pull request has merge conflicts. Please resolve those before requesting a review.

@hishamco
Copy link
Member

Fix conflict

@gvkries gvkries requested a review from MikeAlhayek November 25, 2024 12:17
Copy link
Member

@MikeAlhayek MikeAlhayek left a comment

Choose a reason for hiding this comment

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

@gvkries some minor suggestion

Comment on lines 107 to 110
Field<StringGraphType>("fileName").Description(S["the file name of the media file item"]).Resolve(x => x.Source.FileName);
Field<StringGraphType>("path").Description(S["the path of the media file item"]).Resolve(x => x.Source.Path);
Field<StringGraphType>("url").Description(S["the url name of the media file item"]).Resolve(x => x.Source.Url);
Field<StringGraphType>("mediaText").Description(S["the media text of the file item"]).Resolve(x => x.Source.MediaText);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Field<StringGraphType>("fileName").Description(S["the file name of the media file item"]).Resolve(x => x.Source.FileName);
Field<StringGraphType>("path").Description(S["the path of the media file item"]).Resolve(x => x.Source.Path);
Field<StringGraphType>("url").Description(S["the url name of the media file item"]).Resolve(x => x.Source.Url);
Field<StringGraphType>("mediaText").Description(S["the media text of the file item"]).Resolve(x => x.Source.MediaText);
Field<StringGraphType>("fileName").Description(S["the file name of the media file item"]).Resolve(x => x.Source.FileName);
Field<StringGraphType>("path")
.Description(S["the path of the media file item"])
.Resolve(x => x.Source.Path);
Field<StringGraphType>("url")
.Description(S["the url name of the media file item"])
.Resolve(x => x.Source.Url);
Field<StringGraphType>("mediaText")
.Description(S["the media text of the file item"])
.Resolve(x => x.Source.MediaText);

@MikeAlhayek MikeAlhayek merged commit 2c1a310 into OrchardCMS:main Nov 26, 2024
6 checks passed
@sebastienros
Copy link
Member

Very well done. You navigated most of the pitfalls of localization we can have in OC.

@gvkries
Copy link
Contributor Author

gvkries commented Nov 26, 2024

Very well done. You navigated most of the pitfalls of localization we can have in OC.

With the help of @MikeAlhayek 🤗

@gvkries gvkries deleted the gvkries/graphql-localization branch November 26, 2024 19:05
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.

[GraphQL] Cache schema by culture
4 participants