diff --git a/apps/pwa/src/components/MemberEditorSheet.tsx b/apps/pwa/src/components/MemberEditorSheet.tsx index f770391..7e1624b 100644 --- a/apps/pwa/src/components/MemberEditorSheet.tsx +++ b/apps/pwa/src/components/MemberEditorSheet.tsx @@ -207,10 +207,11 @@ export function MemberEditorSheet({ // ── handleClose ────────────────────────────────────────────────────────── + // WR-04: only reset ephemeral fields (password inputs, error states, create-mode + // fields). Member-derived fields (displayName, isAdmin) are owned by the useEffect + // below and will re-sync from the live member prop when the sheet re-opens or when + // membersQuery refetches — no stale closure problem. const handleClose = useCallback(() => { - // Reset all form state - setDisplayName(member?.displayName ?? ''); - setIsAdmin(member?.isAdmin ?? false); setProfileError(null); setNewPassword(''); setConfirmPassword(''); @@ -228,7 +229,7 @@ export function MemberEditorSheet({ if (triggerRef?.current) { triggerRef.current.focus(); } - }, [onClose, triggerRef, member?.displayName, member?.isAdmin]); + }, [onClose, triggerRef]); // Sync profile state when member changes (different row opened) useEffect(() => { @@ -259,10 +260,19 @@ export function MemberEditorSheet({ const profileMutation = useMutation({ mutationFn: async () => { if (!member) throw new Error('no-member'); - await updateMemberProfile(member.id, { - displayName: displayName.trim(), - isAdmin, - }); + // WR-01: send only the fields that actually changed to avoid re-writing + // displayName on an admin-toggle-only save (and to prevent 400s when a member + // has a null displayName and the admin only wants to toggle the admin flag). + const payload: { displayName?: string; isAdmin?: boolean } = {}; + const trimmed = displayName.trim(); + if (trimmed !== (member.displayName ?? '')) { + if (trimmed.length === 0) throw new Error('name-required'); + payload.displayName = trimmed; + } + if (isAdmin !== member.isAdmin) payload.isAdmin = isAdmin; + // No-op guard — nothing changed, skip the network call + if (Object.keys(payload).length === 0) return; + await updateMemberProfile(member.id, payload); }, onSuccess: () => { void queryClient.invalidateQueries({ queryKey: ['admin', 'members'] }); @@ -272,9 +282,15 @@ export function MemberEditorSheet({ onError: (err) => { const msg = err instanceof Error ? err.message : 'server'; if (msg === 'last-admin') { - // D-03 guard: revert toggle to previous value (member.isAdmin was true) - setIsAdmin(member?.isAdmin ?? true); + // D-03 guard: revert toggle to the actual prior value on the member object. + // WR-02: use member!.isAdmin explicitly rather than `?? true` — the `?? true` + // was accidentally correct only because the guard fires when demoting an admin, + // but it would incorrectly set isAdmin=true for any future error path where + // member is non-null but isAdmin is false. + setIsAdmin(member!.isAdmin); setProfileError('Cannot remove admin — at least one admin must remain.'); + } else if (msg === 'name-required') { + setProfileError('A display name is required before saving.'); } else { setProfileError('Something went wrong. Please try again.'); }