From f038ec2d8c7e8ce91718ecee3a9d0e4b074ea3c9 Mon Sep 17 00:00:00 2001 From: Meier Lukas Date: Sat, 26 Oct 2024 00:51:19 +0200 Subject: [PATCH] chore: address pull request feedback --- .../[locale]/_client-providers/mantine.tsx | 6 ++-- .../boards/[name]/settings/_danger.tsx | 22 +++++++------ .../[locale]/boards/[name]/settings/page.tsx | 5 ++- .../_components/appearance-settings-form.tsx | 7 +++-- .../_components/board-settings-form.tsx | 6 ++-- .../settings/_components/common-form.tsx | 9 ++++-- .../_components/culture-settings-form.tsx | 5 ++- .../src/app/[locale]/manage/settings/page.tsx | 10 +++--- apps/nextjs/src/theme/color-scheme.ts | 3 +- packages/api/src/router/board.ts | 22 +++++++++---- packages/auth/events.ts | 4 +-- packages/auth/test/events.spec.ts | 6 ++-- packages/definitions/src/cookie.ts | 1 + packages/definitions/src/index.ts | 1 + packages/translation/src/lang/en.ts | 31 +++++++++++++++++++ 15 files changed, 100 insertions(+), 38 deletions(-) create mode 100644 packages/definitions/src/cookie.ts diff --git a/apps/nextjs/src/app/[locale]/_client-providers/mantine.tsx b/apps/nextjs/src/app/[locale]/_client-providers/mantine.tsx index edd5952ac..3b5ad50f6 100644 --- a/apps/nextjs/src/app/[locale]/_client-providers/mantine.tsx +++ b/apps/nextjs/src/app/[locale]/_client-providers/mantine.tsx @@ -9,6 +9,7 @@ import { clientApi } from "@homarr/api/client"; import { useSession } from "@homarr/auth/client"; import { parseCookies, setClientCookie } from "@homarr/common"; import type { ColorScheme } from "@homarr/definitions"; +import { colorSchemeCookieKey } from "@homarr/definitions"; export const CustomMantineProvider = ({ children, @@ -33,11 +34,10 @@ export const CustomMantineProvider = ({ }; export function useColorSchemeManager(): MantineColorSchemeManager { - const key = "homarr-color-scheme"; const { data: session } = useSession(); const updateCookieValue = (value: Exclude) => { - setClientCookie(key, value, { expires: dayjs().add(1, "year").toDate(), path: "/" }); + setClientCookie(colorSchemeCookieKey, value, { expires: dayjs().add(1, "year").toDate(), path: "/" }); }; const { mutate: mutateColorScheme } = clientApi.user.changeColorScheme.useMutation({ @@ -54,7 +54,7 @@ export function useColorSchemeManager(): MantineColorSchemeManager { try { const cookies = parseCookies(document.cookie); - return (cookies[key] as MantineColorScheme | undefined) ?? defaultValue; + return (cookies[colorSchemeCookieKey] as MantineColorScheme | undefined) ?? defaultValue; } catch { return defaultValue; } diff --git a/apps/nextjs/src/app/[locale]/boards/[name]/settings/_danger.tsx b/apps/nextjs/src/app/[locale]/boards/[name]/settings/_danger.tsx index fa1c05bd3..115605279 100644 --- a/apps/nextjs/src/app/[locale]/boards/[name]/settings/_danger.tsx +++ b/apps/nextjs/src/app/[locale]/boards/[name]/settings/_danger.tsx @@ -12,7 +12,7 @@ import { BoardRenameModal } from "~/components/board/modals/board-rename-modal"; import { useRequiredBoard } from "../../(content)/_context"; import classes from "./danger.module.css"; -export const DangerZoneSettingsContent = () => { +export const DangerZoneSettingsContent = ({ hideVisibility }: { hideVisibility: boolean }) => { const board = useRequiredBoard(); const t = useScopedI18n("board.setting"); const router = useRouter(); @@ -90,14 +90,18 @@ export const DangerZoneSettingsContent = () => { buttonText={t("section.dangerZone.action.rename.button")} onClick={onRenameClick} /> - - + {hideVisibility ? null : ( + <> + + + + )} { export default async function BoardSettingsPage({ params, searchParams }: Props) { const { board, permissions } = await getBoardAndPermissionsAsync(params); + const boardSettings = await getServerSettingByKeyAsync(db, "board"); const { hasFullAccess } = await getBoardPermissionsAsync(board); const t = await getScopedI18n("board.setting"); @@ -92,7 +95,7 @@ export default async function BoardSettingsPage({ params, searchParams }: Props) - + )} diff --git a/apps/nextjs/src/app/[locale]/manage/settings/_components/appearance-settings-form.tsx b/apps/nextjs/src/app/[locale]/manage/settings/_components/appearance-settings-form.tsx index aa8ad6eb7..364936065 100644 --- a/apps/nextjs/src/app/[locale]/manage/settings/_components/appearance-settings-form.tsx +++ b/apps/nextjs/src/app/[locale]/manage/settings/_components/appearance-settings-form.tsx @@ -7,19 +7,22 @@ import { SelectWithCustomItems } from "node_modules/@homarr/ui/src/components/se import type { ColorScheme } from "@homarr/definitions"; import { colorSchemes } from "@homarr/definitions"; import type { ServerSettings } from "@homarr/server-settings"; +import { useScopedI18n } from "@homarr/translation/client"; import { CommonSettingsForm } from "./common-form"; export const AppearanceSettingsForm = ({ defaultValues }: { defaultValues: ServerSettings["appearance"] }) => { + const tApperance = useScopedI18n("management.page.settings.section.appearance"); + return ( {(form) => ( <> ({ value: scheme, - label: scheme, + label: tApperance(`defaultColorScheme.options.${scheme}`), }))} {...form.getInputProps("defaultColorScheme")} SelectOption={ApperanceCustomOption} diff --git a/apps/nextjs/src/app/[locale]/manage/settings/_components/board-settings-form.tsx b/apps/nextjs/src/app/[locale]/manage/settings/_components/board-settings-form.tsx index 4f4c5c226..d48954c61 100644 --- a/apps/nextjs/src/app/[locale]/manage/settings/_components/board-settings-form.tsx +++ b/apps/nextjs/src/app/[locale]/manage/settings/_components/board-settings-form.tsx @@ -6,10 +6,12 @@ import { SelectWithCustomItems } from "node_modules/@homarr/ui/src/components/se import { clientApi } from "@homarr/api/client"; import type { ServerSettings } from "@homarr/server-settings"; +import { useScopedI18n } from "@homarr/translation/client"; import { CommonSettingsForm } from "./common-form"; export const BoardSettingsForm = ({ defaultValues }: { defaultValues: ServerSettings["board"] }) => { + const tBoard = useScopedI18n("management.page.settings.section.board"); const [selectableBoards] = clientApi.board.getPublicBoards.useSuspenseQuery(); return ( @@ -17,8 +19,8 @@ export const BoardSettingsForm = ({ defaultValues }: { defaultValues: ServerSett {(form) => ( <> ({ value: board.id, label: board.name, diff --git a/apps/nextjs/src/app/[locale]/manage/settings/_components/common-form.tsx b/apps/nextjs/src/app/[locale]/manage/settings/_components/common-form.tsx index 8cc7d4601..8a932dee1 100644 --- a/apps/nextjs/src/app/[locale]/manage/settings/_components/common-form.tsx +++ b/apps/nextjs/src/app/[locale]/manage/settings/_components/common-form.tsx @@ -6,6 +6,7 @@ import { clientApi } from "@homarr/api/client"; import { useForm } from "@homarr/form"; import { showErrorNotification, showSuccessNotification } from "@homarr/notifications"; import type { ServerSettings } from "@homarr/server-settings"; +import { useI18n, useScopedI18n } from "@homarr/translation/client"; export const CommonSettingsForm = ({ settingKey, @@ -16,15 +17,17 @@ export const CommonSettingsForm = ({ defaultValues: ServerSettings[TKey]; children: (form: ReturnType>) => React.ReactNode; }) => { + const t = useI18n(); + const tSettings = useScopedI18n("management.page.settings"); const { mutateAsync, isPending } = clientApi.serverSettings.saveSettings.useMutation({ onSuccess() { showSuccessNotification({ - message: "Settings saved successfully", + message: tSettings("notification.success.message"), }); }, onError() { showErrorNotification({ - message: "Failed to save settings", + message: tSettings("notification.error.message"), }); }, }); @@ -45,7 +48,7 @@ export const CommonSettingsForm = ({ {children(form)} diff --git a/apps/nextjs/src/app/[locale]/manage/settings/_components/culture-settings-form.tsx b/apps/nextjs/src/app/[locale]/manage/settings/_components/culture-settings-form.tsx index 88b104ab4..324cba722 100644 --- a/apps/nextjs/src/app/[locale]/manage/settings/_components/culture-settings-form.tsx +++ b/apps/nextjs/src/app/[locale]/manage/settings/_components/culture-settings-form.tsx @@ -7,16 +7,19 @@ import { objectEntries } from "@homarr/common"; import type { ServerSettings } from "@homarr/server-settings"; import type { SupportedLanguage } from "@homarr/translation"; import { localeAttributes } from "@homarr/translation"; +import { useScopedI18n } from "@homarr/translation/client"; import { CommonSettingsForm } from "./common-form"; export const CultureSettingsForm = ({ defaultValues }: { defaultValues: ServerSettings["culture"] }) => { + const tCulture = useScopedI18n("management.page.settings.section.culture"); + return ( {(form) => ( <> ({ value, label: name, diff --git a/apps/nextjs/src/app/[locale]/manage/settings/page.tsx b/apps/nextjs/src/app/[locale]/manage/settings/page.tsx index abe38ed43..a651682e9 100644 --- a/apps/nextjs/src/app/[locale]/manage/settings/page.tsx +++ b/apps/nextjs/src/app/[locale]/manage/settings/page.tsx @@ -21,24 +21,24 @@ export async function generateMetadata() { export default async function SettingsPage() { const serverSettings = await api.serverSettings.getAll(); - const t = await getScopedI18n("management.page.settings"); + const tSettings = await getScopedI18n("management.page.settings"); return ( <> - {t("title")} + {tSettings("title")} - Boards + {tSettings("section.board.title")} - Appearance + {tSettings("section.appearance.title")} - Culture + {tSettings("section.culture.title")} diff --git a/apps/nextjs/src/theme/color-scheme.ts b/apps/nextjs/src/theme/color-scheme.ts index bb38fd67a..4f069a1ef 100644 --- a/apps/nextjs/src/theme/color-scheme.ts +++ b/apps/nextjs/src/theme/color-scheme.ts @@ -4,9 +4,10 @@ import { cookies } from "next/headers"; import { db } from "@homarr/db"; import { getServerSettingByKeyAsync } from "@homarr/db/queries"; import type { ColorScheme } from "@homarr/definitions"; +import { colorSchemeCookieKey } from "@homarr/definitions"; export const getCurrentColorSchemeAsync = cache(async () => { - const cookieValue = cookies().get("homarr-color-scheme")?.value; + const cookieValue = cookies().get(colorSchemeCookieKey)?.value; if (cookieValue) { return cookieValue as ColorScheme; diff --git a/packages/api/src/router/board.ts b/packages/api/src/router/board.ts index eaaccdb8a..7ab693bde 100644 --- a/packages/api/src/router/board.ts +++ b/packages/api/src/router/board.ts @@ -227,6 +227,14 @@ export const boardRouter = createTRPCRouter({ .input(validation.board.changeVisibility) .mutation(async ({ ctx, input }) => { await throwIfActionForbiddenAsync(ctx, eq(boards.id, input.id), "full"); + const boardSettings = await getServerSettingByKeyAsync(ctx.db, "board"); + + if (input.visibility !== "public" && boardSettings.defaultBoardId === input.id) { + throw new TRPCError({ + code: "BAD_REQUEST", + message: "Cannot make default board private", + }); + } await ctx.db .update(boards) @@ -251,12 +259,14 @@ export const boardRouter = createTRPCRouter({ }) : null; - const boardSettings = await getServerSettingByKeyAsync(ctx.db, "board"); - const boardWhere = user?.homeBoardId - ? eq(boards.id, user.homeBoardId) - : boardSettings.defaultBoardId - ? eq(boards.id, boardSettings.defaultBoardId) - : null; + // 1. user home board, 2. default board, 3. not found + let boardWhere: SQL | null = null; + if (user?.homeBoardId) { + boardWhere = eq(boards.id, user.homeBoardId); + } else { + const boardSettings = await getServerSettingByKeyAsync(ctx.db, "board"); + boardWhere = boardSettings.defaultBoardId ? eq(boards.id, boardSettings.defaultBoardId) : null; + } if (!boardWhere) { throw new TRPCError({ diff --git a/packages/auth/events.ts b/packages/auth/events.ts index 292204de0..dab667519 100644 --- a/packages/auth/events.ts +++ b/packages/auth/events.ts @@ -5,7 +5,7 @@ import type { NextAuthConfig } from "next-auth"; import { and, eq, inArray } from "@homarr/db"; import type { Database } from "@homarr/db"; import { groupMembers, groups, users } from "@homarr/db/schema/sqlite"; -import { everyoneGroup } from "@homarr/definitions"; +import { colorSchemeCookieKey, everyoneGroup } from "@homarr/definitions"; import { logger } from "@homarr/log"; import { env } from "./env.mjs"; @@ -52,7 +52,7 @@ export const createSignInEventHandler = (db: Database): Exclude { }); expect(dbUser?.name).toBe("test-new"); }); - test("signInEventHandler should set homarr-color-scheme cookie", async () => { + test("signInEventHandler should set color-scheme cookie", async () => { // Arrange const db = createDb(); await createUserAsync(db); @@ -239,7 +239,7 @@ describe("createSignInEventHandler should create signInEventHandler", () => { // Assert expect(cookies().set).toHaveBeenCalledWith( - "homarr-color-scheme", + colorSchemeCookieKey, "dark", expect.objectContaining({ path: "/", diff --git a/packages/definitions/src/cookie.ts b/packages/definitions/src/cookie.ts new file mode 100644 index 000000000..785b5fffe --- /dev/null +++ b/packages/definitions/src/cookie.ts @@ -0,0 +1 @@ +export const colorSchemeCookieKey = "homarr-color-scheme"; diff --git a/packages/definitions/src/index.ts b/packages/definitions/src/index.ts index 20919b208..c43a2f96c 100644 --- a/packages/definitions/src/index.ts +++ b/packages/definitions/src/index.ts @@ -8,3 +8,4 @@ export * from "./auth"; export * from "./user"; export * from "./group"; export * from "./docs"; +export * from "./cookie"; diff --git a/packages/translation/src/lang/en.ts b/packages/translation/src/lang/en.ts index 670be6712..55316e536 100644 --- a/packages/translation/src/lang/en.ts +++ b/packages/translation/src/lang/en.ts @@ -1847,6 +1847,14 @@ export default { }, settings: { title: "Settings", + notification: { + success: { + message: "Settings saved successfully", + }, + error: { + message: "Failed to save settings", + }, + }, section: { analytics: { title: "Analytics", @@ -1888,6 +1896,29 @@ export default { text: "Google will build a search box with the crawled links along with other direct links. Enabling this will ask Google to disable that box.", }, }, + board: { + title: "Boards", + defaultBoard: { + label: "Global default board", + description: "Only public boards are available for selection", + }, + }, + appearance: { + title: "Appearance", + defaultColorScheme: { + label: "Default color scheme", + options: { + light: "Light", + dark: "Dark", + }, + }, + }, + culture: { + title: "Culture", + defaultLocale: { + label: "Default language", + }, + }, }, }, tool: {