From 1672952a1992962dbf354d295acec377490bdc37 Mon Sep 17 00:00:00 2001 From: pythonlearner1025 Date: Sat, 5 Sep 2026 16:57:22 -0700 Subject: [PATCH] fix(credentials): a refused revoke must say so where the user is looking Revoking a credential closed its confirmation before the request ran, then reported failure through the panel-wide error under the panel header. On a long list the row sits far below that, so the dialog just vanished and the credential stayed. ConfirmationDialog already carried busy and error props that no caller used. The confirmation now stays open while the request runs, closes only on success, and draws the failure in its own body. This is the Revoke sibling of the Save access fix in #234. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01LGizvh2GBb4wnVqs7qKNtP --- .../src/settings/OrgCredentialsPanel.tsx | 21 +++++++-- .../test/org-credentials-panel.test.tsx | 47 +++++++++++++++++++ 2 files changed, 63 insertions(+), 5 deletions(-) diff --git a/packages/webapp/src/settings/OrgCredentialsPanel.tsx b/packages/webapp/src/settings/OrgCredentialsPanel.tsx index 22ccaf9f..2a93b1b8 100644 --- a/packages/webapp/src/settings/OrgCredentialsPanel.tsx +++ b/packages/webapp/src/settings/OrgCredentialsPanel.tsx @@ -161,6 +161,7 @@ export function OrgCredentialsPanel({ const [accessDraft, setAccessDraft] = useState(null); const [savingAccess, setSavingAccess] = useState(false); const [accessError, setAccessError] = useState(null); + const [revokeError, setRevokeError] = useState(null); const [revokeTarget, setRevokeTarget] = useState(null); const [revoking, setRevoking] = useState(null); @@ -255,16 +256,18 @@ export function OrgCredentialsPanel({ const revoke = async (credential: OrgCredentialView) => { if (revoking !== null) return; - setRevokeTarget(null); setRevoking(credential.name); - setError(null); + setRevokeError(null); try { await client.revokeOrgCredential(credential.name); if (accessDraft?.name === credential.name) setAccessDraft(null); if (rotating === credential.name) setRotating(null); await reload(); + // THE DIALOG CLOSES ONLY ON SUCCESS. A failure draws inside it, beside + // the button that caused it. + setRevokeTarget(null); } catch (caught) { - setError(caughtErrorMessage(caught, 'Revoke failed.')); + setRevokeError(caughtErrorMessage(caught, 'Revoke failed.')); } finally { setRevoking(null); } @@ -308,7 +311,10 @@ export function OrgCredentialsPanel({ onDraftChange={(grants) => setAccessDraft({ name: credential.name, grants })} onSaveAccess={() => { void saveAccess(); }} onRotate={() => setRotating(credential.name)} - onRevoke={() => setRevokeTarget(credential)} + onRevoke={() => { + setRevokeError(null); + setRevokeTarget(credential); + }} /> ))} @@ -366,7 +372,12 @@ export function OrgCredentialsPanel({ title="Revoke this credential?" description={`Revoke ${revokeTarget.name} for the whole organization? Every machine that pulls it is refused on the next ask.`} confirmLabel="Revoke credential" - onCancel={() => setRevokeTarget(null)} + busy={revoking === revokeTarget.name} + error={revokeError} + onCancel={() => { + setRevokeError(null); + setRevokeTarget(null); + }} onConfirm={() => { void revoke(revokeTarget); }} /> )} diff --git a/packages/webapp/test/org-credentials-panel.test.tsx b/packages/webapp/test/org-credentials-panel.test.tsx index 4b39b637..0e07c29b 100644 --- a/packages/webapp/test/org-credentials-panel.test.tsx +++ b/packages/webapp/test/org-credentials-panel.test.tsx @@ -246,6 +246,53 @@ describe('OrgCredentialsPanel', () => { await view.unmount(); }); + it('keeps a refused revoke open, shows its error in the dialog, and keeps the credential listed', async () => { + const message = 'Credential revocation was refused.'; + const revokeOrgCredential = vi.fn().mockRejectedValue(new Error(message)); + const view = await render(); + await settle(); + + await act(async () => field( + view.container, 'button[aria-label="Revoke STRIPE_API_KEY"]').click()); + await act(async () => buttonNamed(document.body, 'Revoke credential').click()); + await settle(); + + const dialog = field(document.body, '[role="dialog"]'); + expect(field(dialog, '.webapp-confirmation-error').textContent).toBe(message); + expect(view.container.textContent).toContain('STRIPE_API_KEY'); + await view.unmount(); + }); + + it('clears a refused revoke before another credential succeeds', async () => { + const message = 'Credential revocation was refused.'; + const revokeOrgCredential = vi.fn() + .mockRejectedValueOnce(new Error(message)) + .mockResolvedValueOnce(undefined); + const view = await render(); + await settle(); + + await act(async () => field( + view.container, 'button[aria-label="Revoke STRIPE_API_KEY"]').click()); + await act(async () => buttonNamed(document.body, 'Revoke credential').click()); + await settle(); + const failedDialog = field(document.body, '[role="dialog"]'); + expect(field(failedDialog, '.webapp-confirmation-error').textContent).toBe(message); + + await act(async () => buttonNamed(failedDialog, 'No').click()); + expect(document.body.querySelector('[role="dialog"]')).toBeNull(); + await act(async () => field(view.container, 'button[aria-label="Revoke SENTRY_DSN"]').click()); + const nextDialog = field(document.body, '[role="dialog"]'); + expect(nextDialog.querySelector('.webapp-confirmation-error')).toBeNull(); + await act(async () => buttonNamed(document.body, 'Revoke credential').click()); + await settle(); + + expect(revokeOrgCredential).toHaveBeenCalledTimes(2); + expect(revokeOrgCredential).toHaveBeenNthCalledWith(1, 'STRIPE_API_KEY'); + expect(revokeOrgCredential).toHaveBeenNthCalledWith(2, 'SENTRY_DSN'); + expect(document.body.querySelector('[role="dialog"]')).toBeNull(); + await view.unmount(); + }); + it('expands a row in place from the chevron, and saves the whole audience', async () => { const replaceOrgCredentialGrants = vi.fn().mockResolvedValue({ credential: stripe }); const view = await render();