fix(20): WR-01 WR-02 WR-04 profile mutation and handleClose fixes
WR-01: Send only changed fields in profileMutation so admin-toggle-only saves don't re-send displayName (blocking members with null displayName from ever having their admin flag toggled). WR-02: Revert the toggle to member!.isAdmin explicitly instead of `member?.isAdmin ?? true` — the prior fallback was correct by coincidence but semantically wrong for any error path where member is non-null. WR-04: Remove member-derived field resets (displayName, isAdmin) from handleClose — those belong to the useEffect sync that already tracks member?.id/displayName/isAdmin. Only ephemeral fields (passwords, errors) are reset on close, eliminating the stale-closure Cancel regression. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 4.6
parent
ee04aee4fb
commit
527d85530c
@@ -207,10 +207,11 @@ export function MemberEditorSheet({
|
|||||||
|
|
||||||
// ── handleClose ──────────────────────────────────────────────────────────
|
// ── 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(() => {
|
const handleClose = useCallback(() => {
|
||||||
// Reset all form state
|
|
||||||
setDisplayName(member?.displayName ?? '');
|
|
||||||
setIsAdmin(member?.isAdmin ?? false);
|
|
||||||
setProfileError(null);
|
setProfileError(null);
|
||||||
setNewPassword('');
|
setNewPassword('');
|
||||||
setConfirmPassword('');
|
setConfirmPassword('');
|
||||||
@@ -228,7 +229,7 @@ export function MemberEditorSheet({
|
|||||||
if (triggerRef?.current) {
|
if (triggerRef?.current) {
|
||||||
triggerRef.current.focus();
|
triggerRef.current.focus();
|
||||||
}
|
}
|
||||||
}, [onClose, triggerRef, member?.displayName, member?.isAdmin]);
|
}, [onClose, triggerRef]);
|
||||||
|
|
||||||
// Sync profile state when member changes (different row opened)
|
// Sync profile state when member changes (different row opened)
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
@@ -259,10 +260,19 @@ export function MemberEditorSheet({
|
|||||||
const profileMutation = useMutation({
|
const profileMutation = useMutation({
|
||||||
mutationFn: async () => {
|
mutationFn: async () => {
|
||||||
if (!member) throw new Error('no-member');
|
if (!member) throw new Error('no-member');
|
||||||
await updateMemberProfile(member.id, {
|
// WR-01: send only the fields that actually changed to avoid re-writing
|
||||||
displayName: displayName.trim(),
|
// displayName on an admin-toggle-only save (and to prevent 400s when a member
|
||||||
isAdmin,
|
// 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: () => {
|
onSuccess: () => {
|
||||||
void queryClient.invalidateQueries({ queryKey: ['admin', 'members'] });
|
void queryClient.invalidateQueries({ queryKey: ['admin', 'members'] });
|
||||||
@@ -272,9 +282,15 @@ export function MemberEditorSheet({
|
|||||||
onError: (err) => {
|
onError: (err) => {
|
||||||
const msg = err instanceof Error ? err.message : 'server';
|
const msg = err instanceof Error ? err.message : 'server';
|
||||||
if (msg === 'last-admin') {
|
if (msg === 'last-admin') {
|
||||||
// D-03 guard: revert toggle to previous value (member.isAdmin was true)
|
// D-03 guard: revert toggle to the actual prior value on the member object.
|
||||||
setIsAdmin(member?.isAdmin ?? true);
|
// 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.');
|
setProfileError('Cannot remove admin — at least one admin must remain.');
|
||||||
|
} else if (msg === 'name-required') {
|
||||||
|
setProfileError('A display name is required before saving.');
|
||||||
} else {
|
} else {
|
||||||
setProfileError('Something went wrong. Please try again.');
|
setProfileError('Something went wrong. Please try again.');
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user