Fix silent username Save snap-back on account profile

packages/worker/client/routes/account-profile-panel.node.test.ts

@@ -0,0 +1,59 @@
+import { jsx } from 'remix/ui/jsx-runtime'
+import { renderToString } from 'remix/ui/server'
+import { expect, test } from 'vitest'
+import { renderAccountProfilePanel } from './account-profile-panel.tsx'
+
+function panelProps(
+	overrides: Partial<Parameters<typeof renderAccountProfilePanel>[0]> = {},
+) {
+	return {
+		email: 'jaimie@example.com',
+		emailVerified: true,
+		username: 'jklotz08',
+		draftUsername: 'jklotz',
+		draftDisplayName: 'Jaimie',
+		draftBio: '',
+		draftProfileVisibility: 'public' as const,
+		draftEmail: 'jaimie@example.com',
+		emailChangePassword: '',
+		avatarUrl: null,
+		avatarStatus: 'idle' as const,
+		isSaving: false,
+		isSendingEmailChange: false,
+		profileUnchanged: false,
+		normalizedDraftUsername: 'jklotz',
+		normalizedDraftEmail: 'jaimie@example.com',
+		emailChangeMessage: null,
+		emailChangeTone: 'info' as const,
+		emailChangeOpen: false,
+		usernameFieldError: null,
+		onProfileSubmit: () => undefined,
+		onEmailChangeSubmit: () => undefined,
+		onAvatarSelected: () => undefined,
+		onRemoveAvatar: () => undefined,
+		onDraftUsernameInput: () => undefined,
+		onDraftDisplayNameChange: () => undefined,
+		onDraftBioChange: () => undefined,
+		onDraftProfileVisibilityChange: () => undefined,
+		onDraftEmailInput: () => undefined,
+		onEmailChangePasswordInput: () => undefined,
+		...overrides,
+	}
+}
+
+test('profile panel keeps the typed username and shows a save error without success chrome', async () => {
+	const html = await renderToString(
+		jsx('div', {
+			children: renderAccountProfilePanel(
+				panelProps({
+					usernameFieldError: '`jklotz` is taken.',
+				}),
+			),
+		}),
+	)
+
+	expect(html).toContain('value="jklotz"')
+	expect(html).toContain('data-testid="account-username-error"')
+	expect(html).toContain('`jklotz` is taken.')
+	expect(html).not.toContain('Profile saved.')
+})

packages/worker/client/routes/account-profile-panel.tsx

@@ -43,6 +43,7 @@ export type AccountProfilePanelProps = {
 	emailChangeMessage: string | null
 	emailChangeTone: 'error' | 'info'
 	emailChangeOpen: boolean
+	usernameFieldError: string | null
 	onProfileSubmit: (event: SubmitEvent) => void
 	onEmailChangeSubmit: (event: SubmitEvent) => void
 	onAvatarSelected: (event: Event) => void
@@ -76,6 +77,7 @@ export function renderAccountProfilePanel(props: AccountProfilePanelProps) {
 		emailChangeMessage,
 		emailChangeTone,
 		emailChangeOpen,
+		usernameFieldError,
 		onProfileSubmit,
 		onEmailChangeSubmit,
 		onAvatarSelected,
@@ -154,14 +156,29 @@ export function renderAccountProfilePanel(props: AccountProfilePanelProps) {
 					<input
 						type="text"
 						name="username"
+						id="account-username"
 						data-field-ring
 						required
 						autoComplete="username"
 						pattern="[A-Za-z0-9][A-Za-z0-9-]{1,30}[A-Za-z0-9]"
 						title="Use 3 to 32 letters, numbers, and hyphens. Start and end with a letter or number."
 						value={draftUsername}
+						aria-invalid={usernameFieldError ? 'true' : undefined}
+						aria-describedby={
+							usernameFieldError ? 'account-username-error' : undefined
+						}
 						mix={[css(accountInputCss), on('input', onDraftUsernameInput)]}
 					/>
+					{usernameFieldError ? (
+						<p
+							id="account-username-error"
+							role="alert"
+							data-testid="account-username-error"
+							mix={css({ color: colors.error, margin: 0 })}
+						>
+							{usernameFieldError}
+						</p>
+					) : null}
 				</label>
 				<label mix={css(accountFieldCss)}>
 					<span mix={css(accountFieldLabelCss)}>Display name</span>
@@ -213,6 +230,7 @@ export function renderAccountProfilePanel(props: AccountProfilePanelProps) {
 						<input
 							type="radio"
 							name="profileVisibility"
+							value="public"
 							checked={draftProfileVisibility === 'public'}
 							mix={[
 								on('change', () => {
@@ -233,6 +251,7 @@ export function renderAccountProfilePanel(props: AccountProfilePanelProps) {
 						<input
 							type="radio"
 							name="profileVisibility"
+							value="private"
 							checked={draftProfileVisibility === 'private'}
 							mix={[
 								on('change', () => {

packages/worker/client/routes/account-profile-save.node.test.ts

@@ -0,0 +1,150 @@
+import { expect, test } from 'vitest'
+import {
+	interpretAccountProfileSave,
+	readApiErrorMessage,
+	readProfileFormValues,
+	usernameFormatError,
+	usernameFormatRequirements,
+} from './account-profile-save.ts'
+
+test('failed username rename shows the server reason without success chrome', () => {
+	const taken = interpretAccountProfileSave({
+		previousUsername: 'jklotz08',
+		requestedUsername: 'jklotz',
+		profileFieldsChanged: false,
+		responseOk: false,
+		payload: { ok: false, error: '`jklotz` is taken.' },
+	})
+	expect(taken).toEqual({
+		status: 'error',
+		message: '`jklotz` is taken.',
+	})
+	expect(taken.message).not.toContain('Profile saved.')
+
+	const invalid = interpretAccountProfileSave({
+		previousUsername: 'jklotz08',
+		requestedUsername: 'bad username',
+		profileFieldsChanged: false,
+		responseOk: false,
+		payload: {
+			ok: false,
+			error:
+				'Username must be 3 to 32 characters, use only letters, numbers, and hyphens, and start and end with a letter or number.',
+		},
+	})
+	expect(invalid.status).toBe('error')
+	expect(invalid.message).toContain('3 to 32 characters')
+	expect(invalid.message).not.toContain('Profile saved.')
+
+	const rewriteFailed = interpretAccountProfileSave({
+		previousUsername: 'jklotz08',
+		requestedUsername: 'jklotz',
+		profileFieldsChanged: false,
+		responseOk: false,
+		payload: {
+			ok: false,
+			error:
+				'Username was not changed because package updates failed: sync failed',
+		},
+	})
+	expect(rewriteFailed).toEqual({
+		status: 'error',
+		message:
+			'Username was not changed because package updates failed: sync failed',
+	})
+	expect(rewriteFailed.message).not.toContain('Profile saved.')
+})
+
+test('a 200 that did not persist the requested username is an error, not success', () => {
+	const result = interpretAccountProfileSave({
+		previousUsername: 'jklotz08',
+		requestedUsername: 'jklotz',
+		profileFieldsChanged: true,
+		responseOk: true,
+		payload: { ok: true, username: 'jklotz08' },
+	})
+	expect(result).toEqual({
+		status: 'error',
+		message: '`jklotz` was not saved.',
+	})
+	expect(result.message).not.toContain('Profile saved.')
+})
+
+test('successful rename reports saved and an unchanged username does not', () => {
+	expect(
+		interpretAccountProfileSave({
+			previousUsername: 'jklotz08',
+			requestedUsername: 'jklotz',
+			profileFieldsChanged: false,
+			responseOk: true,
+			payload: {
+				ok: true,
+				username: 'jklotz',
+				packageUpdateMessage: 'Updated 2 packages to the new @jklotz scope.',
+			},
+		}),
+	).toEqual({
+		status: 'saved',
+		message: 'Profile saved. Updated 2 packages to the new @jklotz scope.',
+		appliedUsername: 'jklotz',
+		usernameChanged: true,
+	})
+
+	expect(
+		interpretAccountProfileSave({
+			previousUsername: 'jklotz08',
+			requestedUsername: 'jklotz08',
+			profileFieldsChanged: false,
+			responseOk: true,
+			payload: { ok: true, username: 'jklotz08' },
+		}),
+	).toEqual({
+		status: 'noop',
+		appliedUsername: 'jklotz08',
+	})
+
+	expect(
+		interpretAccountProfileSave({
+			previousUsername: 'jklotz08',
+			requestedUsername: 'JKLOTZ08',
+			profileFieldsChanged: true,
+			responseOk: true,
+			payload: { ok: true, username: 'jklotz08' },
+		}),
+	).toMatchObject({
+		status: 'saved',
+		message: 'Profile saved.',
+		usernameChanged: false,
+	})
+})
+
+test('readApiErrorMessage accepts string or nested envelope errors', () => {
+	expect(readApiErrorMessage({ error: '`jklotz` is taken.' }, 'fallback')).toBe(
+		'`jklotz` is taken.',
+	)
+	expect(
+		readApiErrorMessage(
+			{ error: { code: 'account_deleting', message: 'Writes are disabled.' } },
+			'fallback',
+		),
+	).toBe('Writes are disabled.')
+	expect(readApiErrorMessage(null, 'Unable to save profile.')).toBe(
+		'Unable to save profile.',
+	)
+})
+
+test('username format errors stay next to the field while typing', () => {
+	expect(usernameFormatError('jklotz')).toBeNull()
+	expect(usernameFormatError('bad username')).toBe(usernameFormatRequirements)
+	expect(usernameFormatError('')).toBe('Username is required.')
+})
+
+test('readProfileFormValues keeps fallbacks when submit is not from a form', () => {
+	const fallback = {
+		username: 'jklotz08',
+		displayName: 'Old',
+		bio: '',
+		profileVisibility: 'public' as const,
+	}
+	expect(readProfileFormValues(null, fallback)).toEqual(fallback)
+})