From 7274512c2f408355370d7b36779bd78690b6bbfb Mon Sep 17 00:00:00 2001 From: Keith Stevens Date: Sat, 21 Jan 2023 21:36:37 +0900 Subject: [PATCH 1/5] Implementing core of setting user languages and fetching language specific tasks --- website/next-i18next.config.js | 2 +- website/package-lock.json | 45 +++++++++++++++++++ website/package.json | 2 + website/src/components/Header/Header.tsx | 2 + .../LanguageSelector/LanguageSelector.tsx | 40 +++++++++++++++++ .../src/components/LanguageSelector/index.tsx | 1 + website/src/lib/oasst_api_client.ts | 7 ++- website/src/lib/users.ts | 16 ++++++- website/src/pages/api/new_task/[task_type].ts | 5 ++- website/src/pages/api/update_task.ts | 13 +++++- 10 files changed, 125 insertions(+), 8 deletions(-) create mode 100644 website/src/components/LanguageSelector/LanguageSelector.tsx create mode 100644 website/src/components/LanguageSelector/index.tsx diff --git a/website/next-i18next.config.js b/website/next-i18next.config.js index 7c87a7a4..40c4b14d 100644 --- a/website/next-i18next.config.js +++ b/website/next-i18next.config.js @@ -1,6 +1,6 @@ module.exports = { i18n: { defaultLocale: "en", - locales: ["en"], + locales: ["de", "en", "fr"], }, }; diff --git a/website/package-lock.json b/website/package-lock.json index 06f3c98d..5c5dc795 100644 --- a/website/package-lock.json +++ b/website/package-lock.json @@ -21,6 +21,7 @@ "@next/font": "^13.1.0", "@prisma/client": "^4.7.1", "@tailwindcss/forms": "^0.5.3", + "accept-language-parser": "^1.5.0", "autoprefixer": "^10.4.13", "axios": "^1.2.1", "boolean": "^3.2.0", @@ -38,6 +39,7 @@ "npm": "^9.2.0", "postcss-focus-visible": "^7.1.0", "react": "18.2.0", + "react-cookies": "^0.1.1", "react-dom": "18.2.0", "react-feature-flags": "^1.0.0", "react-hook-form": "^7.42.1", @@ -13616,6 +13618,11 @@ "integrity": "sha512-j2afSsaIENvHZN2B8GOpF566vZ5WVk5opAiMTvWgaQT8DkbOqsTfvNAvHoRGU2zzP8cPoqys+xHTRDWW8L+/BA==", "dev": true }, + "node_modules/accept-language-parser": { + "version": "1.5.0", + "resolved": "https://registry.npmjs.org/accept-language-parser/-/accept-language-parser-1.5.0.tgz", + "integrity": "sha512-QhyTbMLYo0BBGg1aWbeMG4ekWtds/31BrEU+DONOg/7ax23vxpL03Pb7/zBmha2v7vdD3AyzZVWBVGEZxKOXWw==" + }, "node_modules/accepts": { "version": "1.3.8", "resolved": "https://registry.npmjs.org/accepts/-/accepts-1.3.8.tgz", @@ -32466,6 +32473,23 @@ "react": "^15.3.0 || ^16.0.0 || ^17.0.0 || ^18.0.0" } }, + "node_modules/react-cookies": { + "version": "0.1.1", + "resolved": "https://registry.npmjs.org/react-cookies/-/react-cookies-0.1.1.tgz", + "integrity": "sha512-PP75kJ4vtoHuuTdq0TAD3RmlAv7vuDQh9fkC4oDlhntgs9vX1DmREomO0Y1mcQKR9nMZ6/zxoflaMJ3MAmF5KQ==", + "dependencies": { + "cookie": "^0.3.1", + "object-assign": "^4.1.1" + } + }, + "node_modules/react-cookies/node_modules/cookie": { + "version": "0.3.1", + "resolved": "https://registry.npmjs.org/cookie/-/cookie-0.3.1.tgz", + "integrity": "sha512-+IJOX0OqlHCszo2mBUq+SrEbCj6w7Kpffqx60zYbPTFaO4+yYgRjHwcZNpWvaTylDHaV7PPmBHzSecZiMhtPgw==", + "engines": { + "node": ">= 0.6" + } + }, "node_modules/react-docgen": { "version": "5.4.3", "resolved": "https://registry.npmjs.org/react-docgen/-/react-docgen-5.4.3.tgz", @@ -47817,6 +47841,11 @@ "integrity": "sha512-j2afSsaIENvHZN2B8GOpF566vZ5WVk5opAiMTvWgaQT8DkbOqsTfvNAvHoRGU2zzP8cPoqys+xHTRDWW8L+/BA==", "dev": true }, + "accept-language-parser": { + "version": "1.5.0", + "resolved": "https://registry.npmjs.org/accept-language-parser/-/accept-language-parser-1.5.0.tgz", + "integrity": "sha512-QhyTbMLYo0BBGg1aWbeMG4ekWtds/31BrEU+DONOg/7ax23vxpL03Pb7/zBmha2v7vdD3AyzZVWBVGEZxKOXWw==" + }, "accepts": { "version": "1.3.8", "resolved": "https://registry.npmjs.org/accepts/-/accepts-1.3.8.tgz", @@ -61962,6 +61991,22 @@ "@babel/runtime": "^7.12.13" } }, + "react-cookies": { + "version": "0.1.1", + "resolved": "https://registry.npmjs.org/react-cookies/-/react-cookies-0.1.1.tgz", + "integrity": "sha512-PP75kJ4vtoHuuTdq0TAD3RmlAv7vuDQh9fkC4oDlhntgs9vX1DmREomO0Y1mcQKR9nMZ6/zxoflaMJ3MAmF5KQ==", + "requires": { + "cookie": "^0.3.1", + "object-assign": "^4.1.1" + }, + "dependencies": { + "cookie": { + "version": "0.3.1", + "resolved": "https://registry.npmjs.org/cookie/-/cookie-0.3.1.tgz", + "integrity": "sha512-+IJOX0OqlHCszo2mBUq+SrEbCj6w7Kpffqx60zYbPTFaO4+yYgRjHwcZNpWvaTylDHaV7PPmBHzSecZiMhtPgw==" + } + } + }, "react-docgen": { "version": "5.4.3", "resolved": "https://registry.npmjs.org/react-docgen/-/react-docgen-5.4.3.tgz", diff --git a/website/package.json b/website/package.json index 4ae762e4..8866a9e2 100644 --- a/website/package.json +++ b/website/package.json @@ -38,6 +38,7 @@ "@next/font": "^13.1.0", "@prisma/client": "^4.7.1", "@tailwindcss/forms": "^0.5.3", + "accept-language-parser": "^1.5.0", "autoprefixer": "^10.4.13", "axios": "^1.2.1", "boolean": "^3.2.0", @@ -55,6 +56,7 @@ "npm": "^9.2.0", "postcss-focus-visible": "^7.1.0", "react": "18.2.0", + "react-cookies": "^0.1.1", "react-dom": "18.2.0", "react-feature-flags": "^1.0.0", "react-hook-form": "^7.42.1", diff --git a/website/src/components/Header/Header.tsx b/website/src/components/Header/Header.tsx index a1b36123..64614578 100644 --- a/website/src/components/Header/Header.tsx +++ b/website/src/components/Header/Header.tsx @@ -5,6 +5,7 @@ import { useSession } from "next-auth/react"; import { useTranslation } from "next-i18next"; import { Flags } from "react-feature-flags"; import { FaUser } from "react-icons/fa"; +import { LanguageSelector } from "src/components/LanguageSelector"; import { UserMenu } from "./UserMenu"; @@ -45,6 +46,7 @@ export function Header() { FlagTest + diff --git a/website/src/components/LanguageSelector/LanguageSelector.tsx b/website/src/components/LanguageSelector/LanguageSelector.tsx new file mode 100644 index 00000000..e611bf0f --- /dev/null +++ b/website/src/components/LanguageSelector/LanguageSelector.tsx @@ -0,0 +1,40 @@ +import { Select } from "@chakra-ui/react"; +import { useRouter } from "next/router"; +import { useTranslation } from "next-i18next"; +import { useCallback, useMemo, useState } from "react"; +import cookie from "react-cookies"; + +const LanguageSelector = () => { + const router = useRouter(); + const { i18n } = useTranslation(); + + const { language: currentLanguage } = i18n; + const languageNames = useMemo(() => { + return new Intl.DisplayNames([currentLanguage], { + type: "language", + }); + }, [currentLanguage]); + + const languageChanged = useCallback( + async (option) => { + const locale = option.target.value; + cookie.save("NEXT_LOCALE", locale, { path: "/" }); + const path = router.asPath; + return router.push(path, path, { locale }); + }, + [router] + ); + + const locales = router.locales; + return ( + + ); +}; + +export { LanguageSelector }; diff --git a/website/src/components/LanguageSelector/index.tsx b/website/src/components/LanguageSelector/index.tsx new file mode 100644 index 00000000..feb9f322 --- /dev/null +++ b/website/src/components/LanguageSelector/index.tsx @@ -0,0 +1 @@ +export * from "./LanguageSelector"; diff --git a/website/src/lib/oasst_api_client.ts b/website/src/lib/oasst_api_client.ts index 1e74b020..36e3ae33 100644 --- a/website/src/lib/oasst_api_client.ts +++ b/website/src/lib/oasst_api_client.ts @@ -108,10 +108,11 @@ export class OasstApiClient { // TODO return a strongly typed Task? // This method is used to store a task in RegisteredTask.task. // This is a raw Json type, so we can't use it to strongly type the task. - async fetchTask(taskType: string, user: BackendUserCore): Promise { + async fetchTask(taskType: string, user: BackendUserCore, lang: string): Promise { return this.post("/api/v1/tasks/", { type: taskType, user, + lang, }); } @@ -136,7 +137,8 @@ export class OasstApiClient { messageId: string, userMessageId: string, content: object, - user: BackendUserCore + user: BackendUserCore, + lang: string ): Promise { return this.post("/api/v1/tasks/interaction", { type: updateType, @@ -144,6 +146,7 @@ export class OasstApiClient { task_id: taskId, message_id: messageId, user_message_id: userMessageId, + lang, ...content, }); } diff --git a/website/src/lib/users.ts b/website/src/lib/users.ts index 2aa8c708..3dbe5a08 100644 --- a/website/src/lib/users.ts +++ b/website/src/lib/users.ts @@ -1,6 +1,20 @@ +import parser from "accept-language-parser"; +import type { NextApiRequest } from "next"; import prisma from "src/lib/prismadb"; import type { BackendUserCore } from "src/types/Users"; +const getUserLanguage = (req: NextApiRequest) => { + const cookieLanguage = req.cookies["NEXT_LOCALE"]; + if (cookieLanguage) { + return cookieLanguage; + } + const headerLanguages = parser.parse(req.headers["accept-language"]); + if (headerLanguages.length > 0) { + return headerLanguages[0].code; + } + return "en"; +}; + /** * Returns a `BackendUserCore` that can be used for interacting with the Backend service. * @@ -35,4 +49,4 @@ const getBackendUserCore = async (id: string) => { } as BackendUserCore; }; -export { getBackendUserCore }; +export { getBackendUserCore, getUserLanguage }; diff --git a/website/src/pages/api/new_task/[task_type].ts b/website/src/pages/api/new_task/[task_type].ts index c8255b18..360b8faa 100644 --- a/website/src/pages/api/new_task/[task_type].ts +++ b/website/src/pages/api/new_task/[task_type].ts @@ -1,7 +1,7 @@ import { withoutRole } from "src/lib/auth"; import { oasstApiClient } from "src/lib/oasst_api_client"; import prisma from "src/lib/prismadb"; -import { getBackendUserCore } from "src/lib/users"; +import { getBackendUserCore, getUserLanguage } from "src/lib/users"; /** * Returns a new task created from the Task Backend. We do a few things here: @@ -14,11 +14,12 @@ import { getBackendUserCore } 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 user = await getBackendUserCore(token.sub); let task; try { - task = await oasstApiClient.fetchTask(task_type as string, user); + task = await oasstApiClient.fetchTask(task_type as string, user, userLanguage); } catch (err) { 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 c547503a..6f08d640 100644 --- a/website/src/pages/api/update_task.ts +++ b/website/src/pages/api/update_task.ts @@ -2,7 +2,7 @@ import { Prisma } from "@prisma/client"; import { withoutRole } from "src/lib/auth"; import { oasstApiClient } from "src/lib/oasst_api_client"; import prisma from "src/lib/prismadb"; -import { getBackendUserCore } from "src/lib/users"; +import { getBackendUserCore, getUserLanguage } from "src/lib/users"; /** * Stores the task interaction with the Task Backend and then returns the next task generated. @@ -41,9 +41,18 @@ 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); + newTask = await oasstApiClient.interactTask( + update_type, + taskId, + frontendId, + interaction.id, + content, + user, + userLanguage + ); } catch (err) { console.error(JSON.stringify(err)); return res.status(500).json(err); From ab09a3f50fe17242c32c7e82f2b94e409561e73e Mon Sep 17 00:00:00 2001 From: Keith Stevens Date: Sun, 22 Jan 2023 16:13:35 +0900 Subject: [PATCH 2/5] Adding a valid language check --- website/src/lib/users.ts | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/website/src/lib/users.ts b/website/src/lib/users.ts index 3dbe5a08..3f1cb65d 100644 --- a/website/src/lib/users.ts +++ b/website/src/lib/users.ts @@ -3,13 +3,26 @@ import type { NextApiRequest } from "next"; import prisma from "src/lib/prismadb"; import type { BackendUserCore } from "src/types/Users"; +import { i18n } from "src/../next-i18next.config"; + +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. + */ const getUserLanguage = (req: NextApiRequest) => { const cookieLanguage = req.cookies["NEXT_LOCALE"]; if (cookieLanguage) { return cookieLanguage; } const headerLanguages = parser.parse(req.headers["accept-language"]); - if (headerLanguages.length > 0) { + if (headerLanguages.length > 0 && LOCALE_SET.has(headerLanguages[0].code)) { return headerLanguages[0].code; } return "en"; From 79331df366f3a95bb5209b210552189457ab9723 Mon Sep 17 00:00:00 2001 From: Keith Stevens Date: Sun, 22 Jan 2023 16:15:54 +0900 Subject: [PATCH 3/5] Fixing an import order --- website/src/components/LanguageSelector/LanguageSelector.tsx | 2 +- website/src/lib/users.ts | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/website/src/components/LanguageSelector/LanguageSelector.tsx b/website/src/components/LanguageSelector/LanguageSelector.tsx index e611bf0f..37659265 100644 --- a/website/src/components/LanguageSelector/LanguageSelector.tsx +++ b/website/src/components/LanguageSelector/LanguageSelector.tsx @@ -1,7 +1,7 @@ import { Select } from "@chakra-ui/react"; import { useRouter } from "next/router"; import { useTranslation } from "next-i18next"; -import { useCallback, useMemo, useState } from "react"; +import { useCallback, useMemo } from "react"; import cookie from "react-cookies"; const LanguageSelector = () => { diff --git a/website/src/lib/users.ts b/website/src/lib/users.ts index 3f1cb65d..637e93a9 100644 --- a/website/src/lib/users.ts +++ b/website/src/lib/users.ts @@ -1,10 +1,9 @@ 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"; -import { i18n } from "src/../next-i18next.config"; - const LOCALE_SET = new Set(i18n.locales); /** From 28089d9ecf6766086298c4a1034f06046062b9cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20K=C3=B6pf?= Date: Sun, 22 Jan 2023 09:29:21 +0100 Subject: [PATCH 4/5] fix username+auth combo check --- backend/oasst_backend/prompt_repository.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/backend/oasst_backend/prompt_repository.py b/backend/oasst_backend/prompt_repository.py index abf5b721..8c259dda 100644 --- a/backend/oasst_backend/prompt_repository.py +++ b/backend/oasst_backend/prompt_repository.py @@ -693,7 +693,7 @@ class PromptRepository: if user_id: qry = qry.filter(Message.user_id == user_id) if username or auth_method: - if not username and auth_method: + if not (username and auth_method): raise OasstError("Auth method or username missing.", OasstErrorCode.AUTH_AND_USERNAME_REQUIRED) qry = qry.join(User) qry = qry.filter(User.username == username, User.auth_method == auth_method) From c0391a6df9aff9bf83e2baa1b59d0d5478ad434b Mon Sep 17 00:00:00 2001 From: James Melvin Ebenezer Date: Sun, 22 Jan 2023 15:38:02 +0530 Subject: [PATCH 5/5] fix: redundant row updates with no Task id in text_labels table (#876) * fix: redundant row updates with no Task id in text_labels table * fix: review comments incorporated * fix: better error handling and function name * fix: review comments Co-authored-by: James Melvin --- backend/oasst_backend/api/v1/text_labels.py | 16 +++++++++++----- backend/oasst_backend/prompt_repository.py | 15 +++++++++++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/backend/oasst_backend/api/v1/text_labels.py b/backend/oasst_backend/api/v1/text_labels.py index c9afd88c..dc6cc889 100644 --- a/backend/oasst_backend/api/v1/text_labels.py +++ b/backend/oasst_backend/api/v1/text_labels.py @@ -4,8 +4,9 @@ from loguru import logger from oasst_backend.api import deps from oasst_backend.prompt_repository import PromptRepository from oasst_backend.schemas.text_labels import LabelOption, ValidLabelsResponse +from oasst_backend.utils.database_utils import CommitMode, managed_tx_function +from oasst_shared.exceptions import OasstError from oasst_shared.schemas import protocol as protocol_schema -from sqlmodel import Session from starlette.status import HTTP_204_NO_CONTENT, HTTP_400_BAD_REQUEST router = APIRouter() @@ -14,20 +15,25 @@ router = APIRouter() @router.post("/", status_code=HTTP_204_NO_CONTENT) def label_text( *, - db: Session = Depends(deps.get_db), api_key: APIKey = Depends(deps.get_api_key), text_labels: protocol_schema.TextLabels, ) -> None: """ Label a piece of text. """ - api_client = deps.api_auth(api_key, db) + + @managed_tx_function(CommitMode.COMMIT) + def store_text_labels(session: deps.Session): + api_client = deps.api_auth(api_key, session) + pr = PromptRepository(session, api_client, client_user=text_labels.user) + pr.store_text_labels(text_labels) try: logger.info(f"Labeling text {text_labels=}.") - pr = PromptRepository(db, api_client, client_user=text_labels.user) - pr.store_text_labels(text_labels) + store_text_labels() + except OasstError: + raise except Exception: logger.exception("Failed to store label.") raise HTTPException( diff --git a/backend/oasst_backend/prompt_repository.py b/backend/oasst_backend/prompt_repository.py index 8c259dda..0a0fa61d 100644 --- a/backend/oasst_backend/prompt_repository.py +++ b/backend/oasst_backend/prompt_repository.py @@ -448,6 +448,11 @@ class PromptRepository: if task: message.review_count += 1 self.db.add(message) + # for the same User id with no task id associated with the message, then update existing record for repeated updates + existing_text_label = self.fetch_non_task_text_labels(message_id, self.user_id) + if existing_text_label is not None: + existing_text_label.labels = text_labels.labels + model = existing_text_label self.db.add(model) return model, task, message @@ -561,6 +566,16 @@ class PromptRepository: raise OasstError("Message not found", OasstErrorCode.MESSAGE_NOT_FOUND, HTTP_404_NOT_FOUND) return message + def fetch_non_task_text_labels(self, message_id: UUID, user_id: UUID) -> Optional[TextLabels]: + + query = ( + self.db.query(TextLabels) + .outerjoin(Task, Task.id == TextLabels.id) + .filter(Task.id.is_(None), TextLabels.message_id == message_id, TextLabels.user_id == user_id) + ) + text_label = query.one_or_none() + return text_label + @staticmethod def trace_conversation(messages: list[Message] | dict[UUID, Message], last_message: Message) -> list[Message]: """