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

MPDX-8309 Send user's language to GraphQL server #1095

Merged
merged 7 commits into from
Sep 27, 2024
Merged

MPDX-8309 Send user's language to GraphQL server #1095

merged 7 commits into from
Sep 27, 2024

Conversation

dr-bizz
Copy link
Contributor

@dr-bizz dr-bizz commented Sep 25, 2024

Description

In this PR, I send the user's language to the GraphQL API.

Client-side GraphQL
In my first commit, I change the client-side GraphQL to send the user's language to the API.
This includes grabbing the user's language preferences and adding it to the Next Auth session so we can use it throughout the site.

Server-side GraphQL
In the second commit, I ensure the server-side is also sending the user's language to the API. (Not proxy like I put in my commit message)

Show message to users to re-login
In the third commit I add a message to users asking them to logout and log back in after changing their language to ensure the language is fully updated. This is because we only get to set the NextAuth session on login.

Checklist:

  • I have given my PR a title with the format "MPDX-(JIRA#) (summary sentence max 80 chars)"
  • I have applied the appropriate labels. (Add the label "On Staging" to get the branch automatically merged into staging.)
  • I have requested a review from another person on the project

Copy link
Contributor

github-actions bot commented Sep 25, 2024

Bundle sizes [mpdx-react]

Compared against e4cb967

No significant changes found

new ApolloLink((operation, forward) => {
const languageHeader = language ? { 'Accept-Language': language } : {};
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't the server automatically honor the user's language preference? This code looks like it uses the language preference and defaults to the accept header: https://github.com/CruGlobal/mpdx_api/blob/7eff744504fe52358a0a975cb5f36ee6b5f1e5ad/app/controllers/graphql_controller.rb#L59-L68

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Doesn't the server automatically honor the user's language preference? This code looks like it uses the language preference and defaults to the accept header:

Yes, it does, but by default, GraphQL sends the en-US value as the Accept-Language if it is missing.

Copy link
Contributor

@canac canac Sep 26, 2024

Choose a reason for hiding this comment

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

Sorry, I misread it. The server uses the Accept-Language header first and falls back to the user's preference. I feel like we should ask the server people to switch that. In the meantime, can you just set Accept-Language to an empty string? When I do that, translated constants are coming through. I think that would be a simpler solution and it won't require the user to log out to update their language.

operation.setContext(({ headers }) => ({
headers: {
...headers,
...languageHeader,
Authorization: `Bearer ${apiToken}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

This worked for me

Suggested change
Authorization: `Bearer ${apiToken}`,
Authorization: `Bearer ${apiToken}`,
'Accept-Language': '',

@dr-bizz dr-bizz requested a review from canac September 26, 2024 18:22
Copy link
Contributor

@canac canac left a comment

Choose a reason for hiding this comment

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

Great work! I assume we've tested that this won't break code that expects the constants to be in English?

@dr-bizz dr-bizz added the Preview Environment Add this label to create an Amplify Preview label Sep 26, 2024
Copy link
Contributor

Preview branch generated at https://MPDX-8309.d3dytjb8adxkk5.amplifyapp.com

@dr-bizz dr-bizz merged commit 48d1bf9 into main Sep 27, 2024
20 checks passed
@dr-bizz dr-bizz deleted the MPDX-8309 branch September 27, 2024 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Preview Environment Add this label to create an Amplify Preview
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants