Revert "fix(web): stop the settings form from reverting the saved value"

This reverts commit 4fe6948f87.

web/src/pages/admin/settings.test.tsx

@@ -1,7 +1,5 @@
-/** @vitest-environment jsdom */
-
-import { cleanup, render, screen, waitFor } from '@testing-library/react'
-import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import { renderToStaticMarkup } from 'react-dom/server'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
 
 const usePersonalNamespaceSettingsMock = vi.fn()
 
@@ -66,57 +64,34 @@ describe('AdminSettingsPage', () => {
     })
   })
 
-  afterEach(() => {
-    cleanup()
+  it('renders the personal namespace section', () => {
+    const html = renderToStaticMarkup(<AdminSettingsPage />)
+
+    expect(html).toContain('adminSettings.personalNamespaceTitle')
+    expect(html).toContain('adminSettings.slugTemplateLabel')
   })
 
-  it('renders the personal namespace section', async () => {
-    render(<AdminSettingsPage />)
+  it('offers the backfill for accounts that already exist', () => {
+    const html = renderToStaticMarkup(<AdminSettingsPage />)
 
-    expect(await screen.findByText('adminSettings.personalNamespaceTitle')).toBeDefined()
-    expect(await screen.findByText('adminSettings.slugTemplateLabel')).toBeDefined()
+    expect(html).toContain('adminSettings.backfillTitle')
+    expect(html).toContain('adminSettings.backfillPreviewAction')
   })
 
-  /**
-   * Regression: the form used to mount before the fetched settings reached it. Radix's Select
-   * keeps a hidden native <select> whose options only exist while the dropdown is mounted, so
-   * changing the controlled value afterwards landed on "" and fired onValueChange(""), which read
-   * as "disabled" and silently reverted the server's answer.
-   */
-  it('keeps the enabled setting the server returned', async () => {
-    render(<AdminSettingsPage />)
+  it('keeps the apply button disabled until a preview has been run', () => {
+    const html = renderToStaticMarkup(<AdminSettingsPage />)
 
-    const slugTemplate = (await screen.findByLabelText(
-      'adminSettings.slugTemplateLabel',
-    )) as HTMLInputElement
-
-    await waitFor(() => {
-      expect(slugTemplate.disabled).toBe(false)
-    })
-    // The trigger renders the selected item's label, so it must read "enabled".
-    const trigger = document.querySelector('#personal-namespace-enabled')
-    expect(trigger?.textContent).toContain('adminSettings.enabledOn')
+    const applyIndex = html.indexOf('adminSettings.backfillApplyAction')
+    expect(applyIndex).toBeGreaterThan(-1)
+    // The apply button carries `disabled` because no preview result exists yet.
+    expect(html.lastIndexOf('disabled', applyIndex)).toBeGreaterThan(-1)
   })
 
   it('shows a loading state while the settings are fetched', () => {
     usePersonalNamespaceSettingsMock.mockReturnValue({ data: undefined, isLoading: true })
 
-    render(<AdminSettingsPage />)
+    const html = renderToStaticMarkup(<AdminSettingsPage />)
 
-    expect(screen.getByText('adminSettings.loading')).toBeDefined()
-  })
-
-  it('offers the backfill for accounts that already exist', async () => {
-    render(<AdminSettingsPage />)
-
-    expect(await screen.findByText('adminSettings.backfillTitle')).toBeDefined()
-    expect(screen.getByRole('button', { name: 'adminSettings.backfillPreviewAction' })).toBeDefined()
-  })
-
-  it('keeps the apply button disabled until a preview has been run', async () => {
-    render(<AdminSettingsPage />)
-
-    const apply = await screen.findByRole('button', { name: /backfillApplyAction/ })
-    expect((apply as HTMLButtonElement).disabled).toBe(true)
+    expect(html).toContain('adminSettings.loading')
   })
 })

web/src/pages/admin/settings.tsx

@@ -55,28 +55,24 @@ export function AdminSettingsPage() {
   const backfillMutation = useBackfillPersonalNamespaces()
   const [backfill, setBackfill] = useState<PersonalNamespaceBackfillResult | null>(null)
 
-  // Null until the server answers. The form must not mount before then: Radix's
-  // Select keeps a hidden native <select> for form integration whose <option>s
-  // only exist while the dropdown content is mounted. Changing the controlled
-  // value before the user has ever opened it therefore assigns a value the
-  // native select has no option for, which lands on "" and fires a real change
-  // event — arriving here as onValueChange(""), which would read as "disabled"
-  // and silently undo what the server just told us.
-  const [form, setForm] = useState<PersonalNamespaceSettingsInput | null>(null)
+  const [form, setForm] = useState<PersonalNamespaceSettingsInput>({
+    enabled: false,
+    slugTemplate: '${username}',
+    displayNameTemplate: '${username}',
+  })
 
   useEffect(() => {
-    if (!settings) {
-      return
+    if (settings) {
+      setForm({
+        enabled: settings.enabled,
+        slugTemplate: settings.slugTemplate,
+        displayNameTemplate: settings.displayNameTemplate,
+      })
     }
-    setForm((current) => current ?? {
-      enabled: settings.enabled,
-      slugTemplate: settings.slugTemplate,
-      displayNameTemplate: settings.displayNameTemplate,
-    })
   }, [settings])
 
-  const slugPreview = form ? previewSlug(form.slugTemplate) : ''
-  const displayNamePreview = form ? renderTemplate(form.displayNameTemplate).trim() : ''
+  const slugPreview = previewSlug(form.slugTemplate)
+  const displayNamePreview = renderTemplate(form.displayNameTemplate).trim()
   const placeholders = settings?.supportedPlaceholders ?? Object.keys(PREVIEW_OWNER)
 
   const runBackfill = async (dryRun: boolean) => {
@@ -105,9 +101,6 @@ export function AdminSettingsPage() {
   const handleSubmit = async (event: React.FormEvent) => {
     event.preventDefault()
 
-    if (!form) {
-      return
-    }
     if (!form.slugTemplate.trim() || !form.displayNameTemplate.trim()) {
       toast.error(t('adminSettings.validationTitle'), t('adminSettings.validationTemplateRequired'))
       return
@@ -139,7 +132,7 @@ export function AdminSettingsPage() {
           </p>
         </div>
 
-        {isLoading || !form ? (
+        {isLoading ? (
           <div className="text-sm text-muted-foreground">{t('adminSettings.loading')}</div>
         ) : (
           <form className="space-y-6" onSubmit={handleSubmit}>
@@ -147,15 +140,9 @@ export function AdminSettingsPage() {
               <Label htmlFor="personal-namespace-enabled">{t('adminSettings.enabledLabel')}</Label>
               <Select
                 value={form.enabled ? 'enabled' : 'disabled'}
-                onValueChange={(value) => {
-                  // Ignore anything that is not a real choice; see the note on `form`.
-                  if (value !== 'enabled' && value !== 'disabled') {
-                    return
-                  }
-                  setForm((current) =>
-                    current ? { ...current, enabled: value === 'enabled' } : current,
-                  )
-                }}
+                onValueChange={(value) =>
+                  setForm((current) => ({ ...current, enabled: value === 'enabled' }))
+                }
               >
                 <SelectTrigger id="personal-namespace-enabled">
                   <SelectValue />
@@ -174,9 +161,7 @@ export function AdminSettingsPage() {
                 value={form.slugTemplate}
                 disabled={!form.enabled}
                 onChange={(event) =>
-                  setForm((current) =>
-                    current ? { ...current, slugTemplate: event.target.value } : current,
-                  )
+                  setForm((current) => ({ ...current, slugTemplate: event.target.value }))
                 }
               />
               <p className="text-xs text-muted-foreground">
@@ -197,9 +182,7 @@ export function AdminSettingsPage() {
                 value={form.displayNameTemplate}
                 disabled={!form.enabled}
                 onChange={(event) =>
-                  setForm((current) =>
-                    current ? { ...current, displayNameTemplate: event.target.value } : current,
-                  )
+                  setForm((current) => ({ ...current, displayNameTemplate: event.target.value }))
                 }
               />
               <p className="text-xs text-muted-foreground">