Compare commits

..

2 Commits

Author SHA1 Message Date
4d6a27194f Follow the field rename in vs-nfd-marking locators
Some checks failed
CI / Lint, typecheck, test (pull_request) Successful in 7m0s
CI / Build container images (pull_request) Successful in 1m23s
CI / Auth e2e pack (pull_request) Successful in 9m0s
CI / Import/export fidelity gate (pull_request) Successful in 1m1s
CD / Deploy to Test (push) Blocked by required conditions
CD / Smoke tests against Test (push) Blocked by required conditions
CD / Promote to Int (push) Blocked by required conditions
CI / Auth e2e pack (push) Blocked by required conditions
CI / Import/export fidelity gate (push) Blocked by required conditions
CI / Build container images (push) Blocked by required conditions
CI / Lint, typecheck, test (push) Has been cancelled
CD / Build and push images (push) Has been cancelled
The pack addresses the registration-mode select by its DOM name
attribute, which react-hook-form derives from the field name — now
`registrationMode` (dot-free, see admin-settings-form.ts). Caught by CI
run 713; the pack needs VS_NFD_MODE stages and was not part of the local
verification set.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017aviRTgWCcAHUh1SBoxf6P
2026-08-04 11:42:55 +02:00
9f754649d4 Fix admin general settings form: dot-free field names, flat PATCH keys (#322)
Some checks failed
CI / Lint, typecheck, test (pull_request) Successful in 6m42s
CI / Build container images (pull_request) Successful in 1m20s
CI / Auth e2e pack (pull_request) Failing after 10m3s
CI / Import/export fidelity gate (pull_request) Has been skipped
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017aviRTgWCcAHUh1SBoxf6P
2026-08-04 11:18:26 +02:00
6 changed files with 215 additions and 26 deletions

View File

@ -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%';" | \

View File

@ -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<string, unknown>
)['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<string, unknown>
)['instance.name'];
expect(stored).toBe(newName);
} finally {
await admin.request.patch('/api/v1/admin/settings', {
data: { 'instance.name': before },
});
await admin.close();
}
});

View File

@ -34,7 +34,7 @@ test('mode marked: card, checkbox marking, and point-of-choice marking', async (
// select value — compliant choice clears it, violating choice brings it
// back, no save in between.
const regField = page.locator('label.field', {
has: page.locator('select[name="auth.registrationMode"]'),
has: page.locator('select[name="registrationMode"]'),
});
const regSelect = regField.locator('select');
await regSelect.selectOption('open');
@ -72,7 +72,7 @@ test('mode hidden: rows disappear, notes mark the hiding, a11y clean', async ({
// Value-listed control: the compliant registration mode keeps only its
// compliant choice (seed leaves it open = violating? then all options).
const regSelect = page.locator('select[name="auth.registrationMode"]');
const regSelect = page.locator('select[name="registrationMode"]');
const regField = page.locator('label.field', { has: regSelect });
const optionCount = await regSelect.locator('option').count();
const marked = await regField.locator('.vs-nfd-mark').count();

View File

@ -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<InstanceSettings>('/admin/settings'),
});
const form = useForm<InstanceSettings>({ 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<GeneralSettingsForm>({
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 {
<FormError error={error} />
<FormSuccess message={saved ? t('settings:admin.saved') : null} />
<Field label={t('settings:admin.instanceName')}>
<input type="text" {...form.register('instance.name')} />
<input type="text" {...form.register('instanceName')} />
</Field>
<Field label={t('settings:admin.defaultLocale')}>
<select {...form.register('instance.defaultLocale')}>
<select {...form.register('defaultLocale')}>
<option value="de">{t('settings:profile.locales.de')}</option>
<option value="en">{t('settings:profile.locales.en')}</option>
</select>
</Field>
<Field
label={t('settings:admin.registrationMode')}
marking={vsNfd.markingFor(
'auth.registrationMode',
form.watch('auth.registrationMode'),
)}
marking={vsNfd.markingFor('auth.registrationMode', form.watch('registrationMode'))}
>
<select {...form.register('auth.registrationMode')}>
<select {...form.register('registrationMode')}>
{!vsNfd.hides('auth.registrationMode', settings.data['auth.registrationMode']) && (
<option value="open">{t('settings:admin.registrationOpen')}</option>
)}
@ -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'),
)}
>
<select {...form.register('classification.newPageDefault')}>
<select {...form.register('newPageClassification')}>
{!vsNfd.hides(
'classification.newPageDefault',
settings.data['classification.newPageDefault'],
@ -136,12 +148,9 @@ export function AdminSettingsPage(): React.JSX.Element {
<Field
label={t('settings:admin.uploadPolicy')}
hint={t('settings:admin.uploadPolicyHelp')}
marking={vsNfd.markingFor(
'classification.uploadPolicy',
form.watch('classification.uploadPolicy'),
)}
marking={vsNfd.markingFor('classification.uploadPolicy', form.watch('uploadPolicy'))}
>
<select {...form.register('classification.uploadPolicy')}>
<select {...form.register('uploadPolicy')}>
{!vsNfd.hides(
'classification.uploadPolicy',
settings.data['classification.uploadPolicy'],
@ -160,15 +169,18 @@ export function AdminSettingsPage(): React.JSX.Element {
<form onSubmit={onSubmit} noValidate>
{(
[
'quota.editorsPerPond',
'quota.readersPerPond',
'quota.additionalPonds',
'quota.storageBytes',
'quota.maxFileBytes',
'quotaEditorsPerPond',
'quotaReadersPerPond',
'quotaAdditionalPonds',
'quotaStorageBytes',
'quotaMaxFileBytes',
] as const
).map((key) => (
<Field key={key} label={tQuotas(`defaults.${SETTING_TO_QUOTA_KEY[key]}`)}>
<input type="number" min={0} {...form.register(key, { valueAsNumber: true })} />
).map((field) => (
<Field
key={field}
label={tQuotas(`defaults.${SETTING_TO_QUOTA_KEY[GENERAL_FORM_FIELDS[field]]}`)}
>
<input type="number" min={0} {...form.register(field, { valueAsNumber: true })} />
</Field>
))}
<button type="submit" className="button" disabled={form.formState.isSubmitting}>

View File

@ -0,0 +1,53 @@
import { describe, expect, it } from 'vitest';
import {
GENERAL_FORM_FIELDS,
GeneralSettingsForm,
toFormValues,
toSettingsPatch,
} from './admin-settings-form';
describe('admin general settings form model (issue #322)', () => {
// The regression this file exists for: a dotted field name makes
// react-hook-form nest the typed value and the strict PATCH schema
// reject the body — the form then looks fine but never saves.
it('uses no dots in any form field name', () => {
for (const field of Object.keys(GENERAL_FORM_FIELDS)) {
expect(field).not.toContain('.');
}
});
it('round-trips settings through form values back to a flat patch', () => {
const settings = {
'instance.name': 'My Wiki',
'instance.defaultLocale': 'de',
'auth.registrationMode': 'closed',
'classification.newPageDefault': 'unclassified',
'classification.uploadPolicy': 'warn',
'quota.editorsPerPond': 5,
'quota.readersPerPond': 50,
'quota.additionalPonds': 0,
'quota.storageBytes': 1024,
'quota.maxFileBytes': 25,
};
expect(toSettingsPatch(toFormValues(settings))).toEqual(settings);
});
it('patches only the settings this form edits, under their dotted keys', () => {
const input: GeneralSettingsForm = {
instanceName: 'Renamed',
defaultLocale: 'en',
registrationMode: 'open',
newPageClassification: 'vs_nfd',
uploadPolicy: 'block',
quotaEditorsPerPond: 1,
quotaReadersPerPond: 2,
quotaAdditionalPonds: 3,
quotaStorageBytes: 4,
quotaMaxFileBytes: 5,
};
const patch = toSettingsPatch(input);
expect(patch['instance.name']).toBe('Renamed');
expect(Object.keys(patch).sort()).toEqual(Object.values(GENERAL_FORM_FIELDS).slice().sort());
});
});

View File

@ -0,0 +1,60 @@
/**
* Form model of the general + quota cards on the admin settings page.
*
* Field names MUST NOT contain dots: react-hook-form treats a dot in a
* field name as a nested-path separator. A field registered under its
* settings key ('instance.name') DISPLAYS fine RHF's getter falls back
* to the literal flat key but typing writes the value into a nested
* object ({ instance: { name } }), which the api's strict PATCH schema
* rejects, so nothing ever saved (issue #322). This mapping is the single
* place tying a dot-free field name to its dotted settings key; the
* converters below translate in both directions.
*/
export const GENERAL_FORM_FIELDS = {
instanceName: 'instance.name',
defaultLocale: 'instance.defaultLocale',
registrationMode: 'auth.registrationMode',
newPageClassification: 'classification.newPageDefault',
uploadPolicy: 'classification.uploadPolicy',
quotaEditorsPerPond: 'quota.editorsPerPond',
quotaReadersPerPond: 'quota.readersPerPond',
quotaAdditionalPonds: 'quota.additionalPonds',
quotaStorageBytes: 'quota.storageBytes',
quotaMaxFileBytes: 'quota.maxFileBytes',
} as const;
export type GeneralFormField = keyof typeof GENERAL_FORM_FIELDS;
export type GeneralFormSettingKey = (typeof GENERAL_FORM_FIELDS)[GeneralFormField];
export interface GeneralSettingsForm {
instanceName: string;
defaultLocale: 'de' | 'en';
registrationMode: 'open' | 'closed';
newPageClassification: 'unclassified' | 'vs_nfd';
uploadPolicy: 'warn' | 'block';
quotaEditorsPerPond: number;
quotaReadersPerPond: number;
quotaAdditionalPonds: number;
quotaStorageBytes: number;
quotaMaxFileBytes: number;
}
/** The settings this form reads and writes, keyed by their dotted names. */
export type GeneralFormSettings = Record<GeneralFormSettingKey, unknown>;
export function toFormValues(settings: GeneralFormSettings): GeneralSettingsForm {
return Object.fromEntries(
Object.entries(GENERAL_FORM_FIELDS).map(([field, key]) => [field, settings[key]]),
) as unknown as GeneralSettingsForm;
}
/** Flat dotted keys, exactly what PATCH /admin/settings expects. */
export function toSettingsPatch(input: GeneralSettingsForm): GeneralFormSettings {
return Object.fromEntries(
Object.entries(GENERAL_FORM_FIELDS).map(([field, key]) => [
key,
input[field as GeneralFormField],
]),
) as GeneralFormSettings;
}