From 9f754649d40f9ddc5286bd25e008daa7eee6b565 Mon Sep 17 00:00:00 2001 From: Claude Fable 5 Date: Tue, 4 Aug 2026 11:18:26 +0200 Subject: [PATCH 1/2] Fix admin general settings form: dot-free field names, flat PATCH keys (#322) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The general and quota cards registered their react-hook-form fields under the dotted settings keys. RHF treats dots as nested-path separators, so the form DISPLAYED fine (its getter falls back to the literal flat key) but typing nested the value ({ instance: { name } }) and the api's strict PATCH schema rejected the body — none of these fields ever saved through the UI, on any instance. Found by Stefan on a fresh self-hosted install. - admin-settings-form.ts: dot-free form model with one explicit mapping to the dotted settings keys and converters in both directions; the submit now also carries ONLY the settings these cards edit, so the internal branding metadata keys never ride along. - Saving invalidates the branding query too — the TopBar reads the instance name from it and kept the old name until its staleTime ran out. - admin-settings.spec.ts (new e2e pack, registered in ci.yml): drives the rename THROUGH THE FORM — success message, TopBar update without reload, value survives reload, api returns it. Verified locally to fail against the unfixed page and pass against the fix. Every existing admin-settings test patched the api directly, which is why this bug was invisible to CI. - admin-settings-form.test.ts pins that no form field name contains a dot and the mapping round-trips. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_017aviRTgWCcAHUh1SBoxf6P --- .gitea/workflows/ci.yml | 10 ++++ apps/web/e2e/admin-settings.spec.ts | 54 +++++++++++++++++ apps/web/src/pages/AdminSettingsPage.tsx | 60 +++++++++++-------- .../web/src/pages/admin-settings-form.test.ts | 53 ++++++++++++++++ apps/web/src/pages/admin-settings-form.ts | 60 +++++++++++++++++++ 5 files changed, 213 insertions(+), 24 deletions(-) create mode 100644 apps/web/e2e/admin-settings.spec.ts create mode 100644 apps/web/src/pages/admin-settings-form.test.ts create mode 100644 apps/web/src/pages/admin-settings-form.ts diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index fb723ad..8581e32 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -345,6 +345,16 @@ jobs: E2E_BASE_URL=http://localhost:5173 \ pnpm --filter @dorfteich/web exec playwright test e2e/social.spec.ts + - name: Reset login rate limit before admin-settings pack + run: | + echo "DELETE FROM rate_limits WHERE key LIKE 'login%';" | \ + pnpm --filter @dorfteich/api exec prisma db execute --stdin --url "$DATABASE_URL" + + - name: Run admin-settings pack + run: | + E2E_BASE_URL=http://localhost:5173 \ + pnpm --filter @dorfteich/web exec playwright test e2e/admin-settings.spec.ts + - name: Reset login rate limit before admin-quotas pack run: | echo "DELETE FROM rate_limits WHERE key LIKE 'login%';" | \ diff --git a/apps/web/e2e/admin-settings.spec.ts b/apps/web/e2e/admin-settings.spec.ts new file mode 100644 index 0000000..2b0e2b1 --- /dev/null +++ b/apps/web/e2e/admin-settings.spec.ts @@ -0,0 +1,54 @@ +import { expect, test } from '@playwright/test'; + +import { contextForUser } from './helpers'; + +const BASE_URL = process.env.E2E_BASE_URL ?? 'http://localhost:5173'; + +/** + * The general admin settings card saves THROUGH THE FORM (issue #322). + * + * This must drive the UI, not the api: the bug it fences was invisible to + * every api-level test — react-hook-form nested the dotted field names on + * input, the strict PATCH schema rejected the body, and the form looked + * fine while never saving. Verified end to end: success message, the value + * survives a full reload, the api returns it, and the TopBar picks it up + * without a reload (branding query invalidation). + */ +test('instance name changed in the general settings form persists', async ({ browser }) => { + const admin = await contextForUser(browser, BASE_URL, 'fixture-admin'); + const before = ( + (await (await admin.request.get('/api/v1/admin/settings')).json()) as Record + )['instance.name'] as string; + const newName = `Renamed ${Date.now()}`; + + const nameLabel = /^(Instance name|Name der Instanz)$/; + const page = await admin.newPage(); + try { + await page.goto('/admin'); + const generalCard = page + .locator('section.settings-section') + .filter({ has: page.getByLabel(nameLabel) }); + await page.getByLabel(nameLabel).fill(newName); + await generalCard.getByRole('button', { name: /^(Save|Speichern)$/ }).click(); + // Scoped to the card: the page has several forms with status regions. + await expect(generalCard.getByRole('status')).toHaveText(/^(Saved\.|Gespeichert\.)$/); + + // The TopBar shows the new name without a reload — the save invalidates + // the branding query the TopBar reads from. + await expect(page.locator('.topbar__brand')).toHaveText(newName); + + // The proof the form really persisted: the value survives a reload and + // the api returns it. + await page.reload(); + await expect(page.getByLabel(nameLabel)).toHaveValue(newName); + const stored = ( + (await (await admin.request.get('/api/v1/admin/settings')).json()) as Record + )['instance.name']; + expect(stored).toBe(newName); + } finally { + await admin.request.patch('/api/v1/admin/settings', { + data: { 'instance.name': before }, + }); + await admin.close(); + } +}); diff --git a/apps/web/src/pages/AdminSettingsPage.tsx b/apps/web/src/pages/AdminSettingsPage.tsx index cf4091c..36957ea 100644 --- a/apps/web/src/pages/AdminSettingsPage.tsx +++ b/apps/web/src/pages/AdminSettingsPage.tsx @@ -5,10 +5,17 @@ import { useForm } from 'react-hook-form'; import { useTranslation } from 'react-i18next'; import { Link } from 'react-router-dom'; +import { BRANDING_KEY } from '../branding/use-branding'; import { Field, FormError, FormSuccess } from '../components/forms'; import { SettingsLayout } from '../components/SettingsLayout'; import { VsNfdHiddenNote, VsNfdMark, useVsNfdMarking } from '../components/vs-nfd'; import { apiGet, apiPatch } from '../lib/api'; +import { + GENERAL_FORM_FIELDS, + GeneralSettingsForm, + toFormValues, + toSettingsPatch, +} from './admin-settings-form'; import { BrandingManager } from './BrandingManager'; import { CustomFontManager } from './CustomFontManager'; import { PluginManager } from './PluginManager'; @@ -51,15 +58,23 @@ export function AdminSettingsPage(): React.JSX.Element { queryFn: () => apiGet('/admin/settings'), }); - const form = useForm({ values: settings.data }); + // Dot-free field names with an explicit mapping to the dotted settings + // keys — see admin-settings-form.ts for why the names must not contain + // dots (issue #322). + const form = useForm({ + values: settings.data ? toFormValues(settings.data) : undefined, + }); const vsNfd = useVsNfdMarking(); const onSubmit = form.handleSubmit(async (input) => { setError(null); setSaved(false); try { - await apiPatch('/admin/settings', input); + await apiPatch('/admin/settings', toSettingsPatch(input)); await queryClient.invalidateQueries({ queryKey: ['admin', 'settings'] }); + // The TopBar takes the instance name from the public branding query; + // without this it keeps the old name until its staleTime runs out. + await queryClient.invalidateQueries({ queryKey: BRANDING_KEY }); setSaved(true); } catch (err) { setError(err); @@ -91,22 +106,19 @@ export function AdminSettingsPage(): React.JSX.Element { - + - - {!vsNfd.hides('auth.registrationMode', settings.data['auth.registrationMode']) && ( )} @@ -118,10 +130,10 @@ export function AdminSettingsPage(): React.JSX.Element { hint={t('settings:admin.newPageClassificationHelp')} marking={vsNfd.markingFor( 'classification.newPageDefault', - form.watch('classification.newPageDefault'), + form.watch('newPageClassification'), )} > - {!vsNfd.hides( 'classification.newPageDefault', settings.data['classification.newPageDefault'], @@ -136,12 +148,9 @@ export function AdminSettingsPage(): React.JSX.Element { - {!vsNfd.hides( 'classification.uploadPolicy', settings.data['classification.uploadPolicy'], @@ -160,15 +169,18 @@ export function AdminSettingsPage(): React.JSX.Element {
{( [ - 'quota.editorsPerPond', - 'quota.readersPerPond', - 'quota.additionalPonds', - 'quota.storageBytes', - 'quota.maxFileBytes', + 'quotaEditorsPerPond', + 'quotaReadersPerPond', + 'quotaAdditionalPonds', + 'quotaStorageBytes', + 'quotaMaxFileBytes', ] as const - ).map((key) => ( - - + ).map((field) => ( + + ))}