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
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions pages/_app.page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@
const GraphQLProviders: React.FC<{
children: React.ReactNode;
}> = ({ children = null }) => {
const { apiToken } = useRequiredSession();
const client = useMemo(() => makeClient(apiToken), [apiToken]);
const { apiToken, language } = useRequiredSession();
const client = useMemo(() => makeClient(apiToken, language), [apiToken]);

Check warning on line 61 in pages/_app.page.tsx

View check run for this annotation

Codecov / codecov/patch

pages/_app.page.tsx#L60-L61

Added lines #L60 - L61 were not covered by tests

return (
<ApolloProvider client={client}>
Expand Down
12 changes: 11 additions & 1 deletion pages/api/auth/[...nextauth].page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ declare module 'next-auth' {
userID: string;
impersonating?: boolean;
impersonatorApiToken?: string;
language?: string | null;
};
}

Expand All @@ -40,6 +41,7 @@ declare module 'next-auth' {
userID?: string;
impersonating?: boolean;
impersonatorApiToken?: string;
language?: string | null;
}
}

Expand All @@ -51,6 +53,7 @@ declare module 'next-auth/jwt' {
userID?: string;
impersonating?: boolean;
impersonatorApiToken?: string;
language?: string | null;
}
}

Expand Down Expand Up @@ -165,6 +168,7 @@ const Auth = (req: NextApiRequest, res: NextApiResponse): Promise<void> => {
const handleSettingUserInfo = async (
access_token: string,
userId: string,
language?: string | null,
) => {
const { user: userInfo, cookies } = setUserInfo(
access_token,
Expand All @@ -175,6 +179,7 @@ const Auth = (req: NextApiRequest, res: NextApiResponse): Promise<void> => {
user.userID = userInfo.userID;
user.impersonating = userInfo.impersonating;
user.impersonatorApiToken = userInfo.impersonatorApiToken;
user.language = language;
if (cookies) {
res.setHeader('Set-Cookie', cookies);
}
Expand All @@ -198,6 +203,7 @@ const Auth = (req: NextApiRequest, res: NextApiResponse): Promise<void> => {
await handleSettingUserInfo(
data.apiOauthSignIn.token,
data.apiOauthSignIn.user.id,
data.apiOauthSignIn.user.preferences?.locale,
);
return true;
}
Expand All @@ -217,6 +223,7 @@ const Auth = (req: NextApiRequest, res: NextApiResponse): Promise<void> => {
await handleSettingUserInfo(
data.oktaSignIn.token,
data.oktaSignIn.user.id,
data.oktaSignIn.user.preferences?.locale,
);
return true;
}
Expand All @@ -241,13 +248,15 @@ const Auth = (req: NextApiRequest, res: NextApiResponse): Promise<void> => {
userID: user.userID,
impersonating: user.impersonating,
impersonatorApiToken: user.impersonatorApiToken,
language: user.language,
};
} else {
return token;
}
},
session: ({ session, token }) => {
const { admin, developer, apiToken, userID, impersonating } = token;
const { admin, developer, apiToken, userID, impersonating, language } =
token;

// Check the expiration of the API token JWT without verifying its signature
// Throwing an exception here will cause a redirect to the login page
Expand All @@ -264,6 +273,7 @@ const Auth = (req: NextApiRequest, res: NextApiResponse): Promise<void> => {
apiToken,
userID,
impersonating,
language,
},
};
},
Expand Down
7 changes: 7 additions & 0 deletions pages/api/auth/apiOauthSignIn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ export type ApiOauthSignInMutation = { __typename?: 'Mutation' } & {
user?: Types.Maybe<
{ __typename?: 'User' } & Pick<Types.User, 'id'> & {
name: Types.User['firstName'];
} & {
preferences?: Types.Maybe<
{ __typename?: 'Preference' } & Pick<Types.Preference, 'locale'>
>;
}
>;
}
Expand All @@ -40,6 +44,9 @@ export const ApiOauthSignInDocument = gql`
user {
id
name: firstName
preferences {
locale
}
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions pages/api/auth/oktaSignIn.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ mutation oktaSignIn($accessToken: String!) {
user {
id
name: firstName
preferences {
locale
}
}
}
}
2 changes: 1 addition & 1 deletion pages/api/handoff.page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ export const returnRedirectUrl = async (
: jwtToken?.apiToken;

if (apiToken && jwtToken?.userID && req.query.auth !== 'true') {
const ssrClient = makeSsrClient(apiToken);
const ssrClient = makeSsrClient(apiToken, jwtToken.language);
const response = await ssrClient.query<
GetDefaultAccountQuery,
GetDefaultAccountQueryVariables
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,16 @@ export const LanguageAccordion: React.FC<LanguageAccordionProps> = ({
enqueueSnackbar(t('Saved successfully.'), {
variant: 'success',
});

enqueueSnackbar(
dr-bizz marked this conversation as resolved.
Show resolved Hide resolved
t(
'Please log out and log back in again to ensure all language changes are fully applied.',
),
{
variant: 'warning',
persist: true,
},
);
},
onError: () => {
enqueueSnackbar(t('Saving failed.'), {
Expand Down
4 changes: 2 additions & 2 deletions src/lib/apollo/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@
});
}

const makeClient = (apiToken: string) => {
const makeClient = (apiToken: string, language?: string | null) => {

Check warning on line 28 in src/lib/apollo/client.ts

View check run for this annotation

Codecov / codecov/patch

src/lib/apollo/client.ts#L28

Added line #L28 was not covered by tests
const client = new ApolloClient({
link: from([
makeAuthLink(apiToken),
makeAuthLink(apiToken, language),
onError(({ graphQLErrors, networkError }) => {
// Don't show sign out and display errors on the login page because the user won't be logged in
if (graphQLErrors && window.location.pathname !== '/login') {
Expand Down
4 changes: 3 additions & 1 deletion src/lib/apollo/link.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,13 @@ export const batchLink = split(
restProxyHttpLink,
);

export const makeAuthLink = (apiToken: string) =>
export const makeAuthLink = (apiToken: string, language?: string | null) =>
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': '',

},
}));
Expand Down
3 changes: 2 additions & 1 deletion src/lib/apollo/ssrClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@

const makeSsrClient = (
apiToken: string | null,
language?: string | null,
): ApolloClient<NormalizedCacheObject> => {
const links: ApolloLink[] = [];
if (apiToken !== null) {
links.push(makeAuthLink(apiToken));
links.push(makeAuthLink(apiToken, language));

Check warning on line 31 in src/lib/apollo/ssrClient.ts

View check run for this annotation

Codecov / codecov/patch

src/lib/apollo/ssrClient.ts#L31

Added line #L31 was not covered by tests
}
if (isRollBarEnabled) {
links.push(serverErrorLink);
Expand Down
Loading