From bc973baa3e61b231dd0a433175fa788a1cbcd9c3 Mon Sep 17 00:00:00 2001 From: notmd <33456881+notmd@users.noreply.github.com> Date: Wed, 22 Feb 2023 06:19:26 +0700 Subject: [PATCH] implement language as state instead of cookie (#1785) close #1365 close #1662 close #1692 close #1530 - Detect language from URL instead of cookie. Steps to verify this. - In tab 1, choose a task in language en. - Open tab 2, change the language to a different one, and complete 1 task. - Back to tab 1 to finish the pending task. Verify that the next task and the UI are still in English. This change is really **critical**, so be careful when deploying to production, better to test it in staging with our community first --- website/.eslintrc.json | 6 +-- .../e2e/tasks/no_tasks_available.cy.ts | 2 +- website/cypress/support/commands.ts | 2 +- .../components/Messages/LabelFlagGroup.tsx | 5 +-- website/src/hooks/locale/useCurrentLocale.ts | 3 ++ website/src/hooks/tasks/useGenericTaskAPI.tsx | 16 ++++--- website/src/lib/languages.ts | 23 +++++++++- website/src/lib/routes.ts | 42 +++++++++++++++++++ website/src/lib/users.ts | 26 ------------ website/src/pages/api/available_tasks.ts | 9 ++-- website/src/pages/api/messages/index.ts | 4 +- website/src/pages/api/new_task/[task_type].ts | 11 ++--- website/src/pages/api/update_task.ts | 16 +++---- website/src/pages/dashboard.tsx | 22 +++------- website/src/pages/messages/index.tsx | 14 +++---- 15 files changed, 111 insertions(+), 90 deletions(-) create mode 100644 website/src/hooks/locale/useCurrentLocale.ts diff --git a/website/.eslintrc.json b/website/.eslintrc.json index efe8a0c461..8fbf78960f 100644 --- a/website/.eslintrc.json +++ b/website/.eslintrc.json @@ -1,10 +1,6 @@ { "root": true, - "extends": [ - "eslint:recommended", - "plugin:@typescript-eslint/recommended", - "next/core-web-vitals" - ], + "extends": ["eslint:recommended", "plugin:@typescript-eslint/recommended", "next/core-web-vitals"], "rules": { "unused-imports/no-unused-imports": "warn", "simple-import-sort/imports": "warn", diff --git a/website/cypress/e2e/tasks/no_tasks_available.cy.ts b/website/cypress/e2e/tasks/no_tasks_available.cy.ts index 27c33b48da..64fdd3a680 100644 --- a/website/cypress/e2e/tasks/no_tasks_available.cy.ts +++ b/website/cypress/e2e/tasks/no_tasks_available.cy.ts @@ -4,7 +4,7 @@ describe("no tasks available", () => { cy.intercept( { method: "GET", - url: "/api/new_task/prompter_reply", + url: "/api/new_task/prompter_reply?lang=en", }, { statusCode: 500, diff --git a/website/cypress/support/commands.ts b/website/cypress/support/commands.ts index 4eecf14617..813fc718a6 100644 --- a/website/cypress/support/commands.ts +++ b/website/cypress/support/commands.ts @@ -51,7 +51,7 @@ Cypress.Commands.add("signInUsingEmailedLink", (emailAddress) => { // we do a GET to this url to force the python backend to add an entry for our user // in the database, otherwise the tos acceptance will error with 404 user not found // then we accept the tos - cy.request("GET", "/api/available_tasks").then(() => cy.request("POST", "/api/tos", {})); + cy.request("GET", "/api/available_tasks?lang=en").then(() => cy.request("POST", "/api/tos", {})); }); }); diff --git a/website/src/components/Messages/LabelFlagGroup.tsx b/website/src/components/Messages/LabelFlagGroup.tsx index 7598627eec..fa2a71ab4c 100644 --- a/website/src/components/Messages/LabelFlagGroup.tsx +++ b/website/src/components/Messages/LabelFlagGroup.tsx @@ -1,6 +1,6 @@ import { Button, Flex, Tooltip } from "@chakra-ui/react"; import { useTranslation } from "next-i18next"; -import { useCookies } from "react-cookie"; +import { useCurrentLocale } from "src/hooks/locale/useCurrentLocale"; import { getTypeSafei18nKey } from "src/lib/i18n"; import { getLocaleDisplayName } from "src/lib/languages"; @@ -20,8 +20,7 @@ export const LabelFlagGroup = ({ onChange, }: LabelFlagGroupProps) => { const { t } = useTranslation("labelling"); - const [cookies] = useCookies(["NEXT_LOCALE"]); - const currentLanguage = cookies["NEXT_LOCALE"]; + const currentLanguage = useCurrentLocale(); const expectedLanguageName = getLocaleDisplayName(expectedLanguage, currentLanguage); return ( diff --git a/website/src/hooks/locale/useCurrentLocale.ts b/website/src/hooks/locale/useCurrentLocale.ts new file mode 100644 index 0000000000..2f714ce71f --- /dev/null +++ b/website/src/hooks/locale/useCurrentLocale.ts @@ -0,0 +1,3 @@ +import { useRouter } from "next/router"; + +export const useCurrentLocale = () => useRouter().locale || "en"; diff --git a/website/src/hooks/tasks/useGenericTaskAPI.tsx b/website/src/hooks/tasks/useGenericTaskAPI.tsx index f16b081a8b..2f9ea0b4cd 100644 --- a/website/src/hooks/tasks/useGenericTaskAPI.tsx +++ b/website/src/hooks/tasks/useGenericTaskAPI.tsx @@ -1,27 +1,31 @@ import { useCallback, useState } from "react"; import { TaskInfos } from "src/components/Tasks/TaskTypes"; import { get, post } from "src/lib/api"; +import { API_ROUTES } from "src/lib/routes"; import { TaskApiHook } from "src/types/Hooks"; import { BaseTask, ServerTaskResponse, TaskResponse, TaskType as TaskTypeEnum } from "src/types/Task"; import { AllTaskReplies } from "src/types/TaskResponses"; import useSWRImmutable from "swr/immutable"; import useSWRMutation from "swr/mutation"; +import { useCurrentLocale } from "../locale/useCurrentLocale"; + export const useGenericTaskAPI = ( taskType: TaskTypeEnum ): TaskApiHook => { const [response, setResponse] = useState>({ taskAvailability: "AWAITING_INITIAL" }); - + const locale = useCurrentLocale(); // Note: We use isValidating to indicate we are loading because it signals eash load, not just the first one. const { isValidating: isLoading, mutate: requestNewTask } = useSWRImmutable>( - "/api/new_task/" + taskType, + API_ROUTES.NEW_TASK(taskType, { lang: locale }), get, { onSuccess: (taskResponse) => { setResponse({ ...taskResponse, taskAvailability: "AVAILABLE", - taskInfo: TaskInfos.find((taskType) => taskType.type === taskResponse.task.type), + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + taskInfo: TaskInfos.find((taskType) => taskType.type === taskResponse.task.type)!, }); }, onError: () => { @@ -33,7 +37,7 @@ export const useGenericTaskAPI = { requestNewTask(); }, @@ -63,9 +67,9 @@ export const useGenericTaskAPI = { +export const getLocaleDisplayName = (locale: string, displayLocale?: string) => { // Intl defaults to English for locales that are not oficially translated if (missingDisplayNamesForLocales[locale]) { return missingDisplayNamesForLocales[locale]; @@ -15,3 +19,20 @@ export const getLocaleDisplayName = (locale: string, displayLocale = undefined) // Return the Titlecased version of the language name. return displayName.charAt(0).toLocaleUpperCase() + displayName.slice(1); }; + +export const getLanguageFromRequest = (req: NextApiRequest) => { + const body = req.method === "GET" ? req.query : req.body; + const lang = body["lang"]; + + if (!lang || typeof lang !== "string") { + throw new OasstError({ + message: "Invalid language", + httpStatusCode: -1, + errorCode: -1, + path: req.url!, + method: req.method!, + }); + } + + return lang; +}; diff --git a/website/src/lib/routes.ts b/website/src/lib/routes.ts index 34c79caf80..c6fee4d288 100644 --- a/website/src/lib/routes.ts +++ b/website/src/lib/routes.ts @@ -1,4 +1,46 @@ +import { TaskType } from "src/types/Task"; + +export type RouteQuery = Record; + +export const stringifyQuery = (query: RouteQuery | undefined) => { + if (!query) { + return ""; + } + + const filteredQuery = Object.fromEntries(Object.entries(query).filter(([, value]) => value !== undefined)) as Record< + string, + string + >; + + return new URLSearchParams(filteredQuery).toString(); +}; + +const createRoute = (path: string, query?: RouteQuery) => { + if (!query) { + return path; + } + + return `${path}?${stringifyQuery(query)}`; +}; + export const ROUTES = { ADMIN_MESSAGE_DETAIL: (id: string) => `/admin/messages/${id}`, MESSAGE_DETAIL: (id: string) => `/messages/${id}`, }; + +export type QueryWithLang = T extends undefined + ? { lang: string } + : T & { lang: string }; + +const withLang = + (path: string, q?: T) => + (query: QueryWithLang) => { + return createRoute(path, { ...q, ...query }); + }; + +export const API_ROUTES = { + NEW_TASK: (type: TaskType, query: QueryWithLang) => createRoute(`/api/new_task/${type}`, query), + UPDATE_TASK: "/api/update_task", + AVAILABLE_TASK: withLang("/api/available_tasks"), + RECENT_MESSAGES: withLang("/api/messages"), +}; diff --git a/website/src/lib/users.ts b/website/src/lib/users.ts index 3c032647bf..8462729872 100644 --- a/website/src/lib/users.ts +++ b/website/src/lib/users.ts @@ -1,32 +1,6 @@ -import parser from "accept-language-parser"; -import type { NextApiRequest } from "next"; -import { i18n } from "src/../next-i18next.config"; import prisma from "src/lib/prismadb"; import type { BackendUserCore } from "src/types/Users"; -const LOCALE_SET = new Set(i18n.locales); - -/** - * Returns the most appropriate user language using the following priority: - * - * 1. The `NEXT_LOCALE` cookie which is set by the client side and will be in - * the set of supported locales. - * 2. The `accept-language` header if it contains a supported locale as set by - * the i18n module. - * 3. "en" as a final fallback. - */ -export const getUserLanguage = (req: NextApiRequest): string => { - const cookieLanguage = req.cookies["NEXT_LOCALE"]; - if (cookieLanguage) { - return cookieLanguage; - } - const headerLanguages = parser.parse(req.headers["accept-language"]); - if (headerLanguages.length > 0 && LOCALE_SET.has(headerLanguages[0].code)) { - return headerLanguages[0].code; - } - return "en"; -}; - /** * Returns a `BackendUserCore` that can be used for interacting with the Backend service. * diff --git a/website/src/pages/api/available_tasks.ts b/website/src/pages/api/available_tasks.ts index 218e3864e5..b23ee95dbb 100644 --- a/website/src/pages/api/available_tasks.ts +++ b/website/src/pages/api/available_tasks.ts @@ -1,12 +1,13 @@ import { withoutRole } from "src/lib/auth"; +import { getLanguageFromRequest } from "src/lib/languages"; import { createApiClientFromUser } from "src/lib/oasst_client_factory"; -import { getBackendUserCore, getUserLanguage } from "src/lib/users"; +import { getBackendUserCore } from "src/lib/users"; const handler = withoutRole("banned", async (req, res, token) => { const user = await getBackendUserCore(token.sub); - const oasstApiClient = createApiClientFromUser(user); - const userLanguage = getUserLanguage(req); - const availableTasks = await oasstApiClient.fetch_available_tasks(user, userLanguage); + const oasstApiClient = createApiClientFromUser(user!); + const userLanguage = getLanguageFromRequest(req); + const availableTasks = await oasstApiClient.fetch_available_tasks(user!, userLanguage); res.status(200).json(availableTasks); }); diff --git a/website/src/pages/api/messages/index.ts b/website/src/pages/api/messages/index.ts index 978ed3ffd6..4d67cdd33d 100644 --- a/website/src/pages/api/messages/index.ts +++ b/website/src/pages/api/messages/index.ts @@ -1,10 +1,10 @@ import { withoutRole } from "src/lib/auth"; +import { getLanguageFromRequest } from "src/lib/languages"; import { createApiClient } from "src/lib/oasst_client_factory"; -import { getUserLanguage } from "src/lib/users"; const handler = withoutRole("banned", async (req, res, token) => { const client = await createApiClient(token); - const userLanguage = getUserLanguage(req); + const userLanguage = getLanguageFromRequest(req); const messages = await client.fetch_recent_messages(userLanguage); res.status(200).json(messages); }); diff --git a/website/src/pages/api/new_task/[task_type].ts b/website/src/pages/api/new_task/[task_type].ts index 08c0e42bfd..5cfdcdd42f 100644 --- a/website/src/pages/api/new_task/[task_type].ts +++ b/website/src/pages/api/new_task/[task_type].ts @@ -1,9 +1,10 @@ import { withoutRole } from "src/lib/auth"; import { ERROR_CODES } from "src/lib/constants"; +import { getLanguageFromRequest } from "src/lib/languages"; import { OasstError } from "src/lib/oasst_api_client"; import { createApiClientFromUser } from "src/lib/oasst_client_factory"; import prisma from "src/lib/prismadb"; -import { getBackendUserCore, getUserLanguage } from "src/lib/users"; +import { getBackendUserCore } from "src/lib/users"; /** * Returns a new task created from the Task Backend. We do a few things here: @@ -16,16 +17,16 @@ import { getBackendUserCore, getUserLanguage } from "src/lib/users"; const handler = withoutRole("banned", async (req, res, token) => { // Fetch the new task. const { task_type } = req.query; - const userLanguage = getUserLanguage(req); + const lang = getLanguageFromRequest(req); const user = await getBackendUserCore(token.sub); - const oasstApiClient = createApiClientFromUser(user); + const oasstApiClient = createApiClientFromUser(user!); let task; try { - task = await oasstApiClient.fetchTask(task_type as string, user, userLanguage); + task = await oasstApiClient.fetchTask(task_type as string, user!, lang); } catch (err) { if (err instanceof OasstError && err.errorCode === ERROR_CODES.TASK_REQUESTED_TYPE_NOT_AVAILABLE) { - res.status(503).json({}); + res.status(503).json(err); } else { console.error(err); res.status(500).json(err); diff --git a/website/src/pages/api/update_task.ts b/website/src/pages/api/update_task.ts index 59920b1359..fee3768efe 100644 --- a/website/src/pages/api/update_task.ts +++ b/website/src/pages/api/update_task.ts @@ -1,8 +1,9 @@ import { Prisma } from "@prisma/client"; import { withoutRole } from "src/lib/auth"; +import { getLanguageFromRequest } from "src/lib/languages"; import { createApiClient } from "src/lib/oasst_client_factory"; import prisma from "src/lib/prismadb"; -import { getBackendUserCore, getUserLanguage } from "src/lib/users"; +import { getBackendUserCore } from "src/lib/users"; /** * Stores the task interaction with the Task Backend and then returns the next task generated. @@ -18,6 +19,8 @@ const handler = withoutRole("banned", async (req, res, token) => { // Parse out the local task ID and the interaction contents. const { id: frontendId, content, update_type } = req.body; + const lang = getLanguageFromRequest(req); + // do in parallel since they are independent const [_, registeredTask, oasstApiClient] = await Promise.all([ // Record that the user has done meaningful work and is no longer new. @@ -46,18 +49,9 @@ const handler = withoutRole("banned", async (req, res, token) => { }); const user = await getBackendUserCore(token.sub); - const userLanguage = getUserLanguage(req); let newTask; try { - newTask = await oasstApiClient.interactTask( - update_type, - taskId, - frontendId, - interaction.id, - content, - user, - userLanguage - ); + newTask = await oasstApiClient.interactTask(update_type, taskId, frontendId, interaction.id, content, user!, lang); } catch (err) { console.error(JSON.stringify(err)); return res.status(500).json(err); diff --git a/website/src/pages/dashboard.tsx b/website/src/pages/dashboard.tsx index 55382b7fa7..c3ea14f8ea 100644 --- a/website/src/pages/dashboard.tsx +++ b/website/src/pages/dashboard.tsx @@ -1,34 +1,24 @@ import { Flex } from "@chakra-ui/react"; import Head from "next/head"; import { useTranslation } from "next-i18next"; -import { useEffect, useMemo, useState } from "react"; +import { useMemo } from "react"; import { LeaderboardWidget, TaskOption, WelcomeCard } from "src/components/Dashboard"; import { getDashboardLayout } from "src/components/Layout"; import { get } from "src/lib/api"; import { AvailableTasks, TaskCategory } from "src/types/Task"; export { getDefaultStaticProps as getStaticProps } from "src/lib/default_static_props"; import { TaskCategoryItem } from "src/components/Dashboard/TaskOption"; +import { useCurrentLocale } from "src/hooks/locale/useCurrentLocale"; +import { API_ROUTES } from "src/lib/routes"; import useSWR from "swr"; const Dashboard = () => { - const { - t, - i18n: { language }, - } = useTranslation(["dashboard", "common", "tasks"]); - const [activeLang, setLang] = useState(null); - const { data, mutate: fetchTasks } = useSWR("/api/available_tasks", get, { + const { t } = useTranslation(["dashboard", "common", "tasks"]); + const lang = useCurrentLocale(); + const { data } = useSWR(API_ROUTES.AVAILABLE_TASK({ lang }), get, { refreshInterval: 2 * 60 * 1000, //2 minutes - revalidateOnMount: false, // triggered in the hook below }); - useEffect(() => { - // re-fetch tasks if the language has changed - if (activeLang !== language) { - setLang(language); - fetchTasks(); - } - }, [activeLang, setLang, language, fetchTasks]); - const availableTaskTypes = useMemo(() => { const taskTypes = filterAvailableTasks(data ?? {}); return { [TaskCategory.Random]: taskTypes }; diff --git a/website/src/pages/messages/index.tsx b/website/src/pages/messages/index.tsx index c8bc4d64b9..af83a568b4 100644 --- a/website/src/pages/messages/index.tsx +++ b/website/src/pages/messages/index.tsx @@ -1,29 +1,25 @@ import { Box, CircularProgress, SimpleGrid, Text, useColorModeValue } from "@chakra-ui/react"; import Head from "next/head"; import { useTranslation } from "next-i18next"; -import { useEffect, useState } from "react"; -import { useCookies } from "react-cookie"; import { getDashboardLayout } from "src/components/Layout"; import { MessageConversation } from "src/components/Messages/MessageConversation"; import { get } from "src/lib/api"; import useSWRImmutable from "swr/immutable"; export { getDefaultStaticProps as getStaticProps } from "src/lib/default_static_props"; +import { useCurrentLocale } from "src/hooks/locale/useCurrentLocale"; import { getLocaleDisplayName } from "src/lib/languages"; +import { API_ROUTES } from "src/lib/routes"; const MessagesDashboard = () => { const { t } = useTranslation(["message"]); const boxBgColor = useColorModeValue("white", "gray.800"); const boxAccentColor = useColorModeValue("gray.200", "gray.900"); - const { data: messages } = useSWRImmutable("/api/messages", get, { revalidateOnMount: true }); + const lang = useCurrentLocale(); + const { data: messages } = useSWRImmutable(API_ROUTES.RECENT_MESSAGES({ lang }), get, { revalidateOnMount: true }); const { data: userMessages } = useSWRImmutable(`/api/messages/user`, get, { revalidateOnMount: true }); - const [cookies] = useCookies(["NEXT_LOCALE"]); - const [currentLanguage, setCurrentLanguage] = useState("en"); - - useEffect(() => { - setCurrentLanguage(cookies["NEXT_LOCALE"] || "en"); - }, [cookies]); + const currentLanguage = useCurrentLocale(); return ( <>