diff --git a/record-bin/src/entrypoints/BinOutlet.test.tsx b/record-bin/src/entrypoints/BinOutlet.test.tsx new file mode 100644 index 00000000..3ce7c904 --- /dev/null +++ b/record-bin/src/entrypoints/BinOutlet.test.tsx @@ -0,0 +1,68 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import type { errorObject } from '../types/types'; +import { createDebugLogger } from '../utils/debugLogger'; +import { + LambdaLessRestoreError, + restoreRecordWithoutLambda, +} from '../utils/lambdaLessRestore'; +import { executeRestoreWithoutLambda } from './BinOutlet'; + +vi.mock('datocms-react-ui', () => ({})); +vi.mock('../utils/lambdaLessRestore', async (importOriginal) => ({ + ...(await importOriginal()), + restoreRecordWithoutLambda: vi.fn(), +})); + +const restorationError = { + simplifiedError: { details: {} }, + fullErrorPayload: '{}', +} as unknown as errorObject; +// Error toasts stay open until the user closes them, so their promise never settles here. +const callbacks = () => ({ + alertFn: vi.fn(() => new Promise(() => {})), + noticeFn: vi.fn(), + navigateTo: vi.fn(), + setError: vi.fn(), +}); +const restore = (handlers: ReturnType) => + executeRestoreWithoutLambda({ + trashRecordId: 'archive-id', + parsedRecordBody: {}, + currentUserAccessToken: 'token', + environment: 'sandbox', + debugLogger: createDebugLogger(false, 'test'), + isEnvironmentPrimary: false, + ...handlers, + }); +afterEach(() => { + vi.mocked(restoreRecordWithoutLambda).mockReset(); +}); + +describe('Lambda-less restore flow', () => { + it('finishes a failed restore without waiting for the alert to close', async () => { + vi.mocked(restoreRecordWithoutLambda).mockRejectedValue( + new LambdaLessRestoreError('rejected', restorationError), + ); + const handlers = callbacks(); + await restore(handlers); + expect(handlers.setError).toHaveBeenCalledWith(restorationError); + expect(handlers.alertFn).toHaveBeenCalledWith( + 'The record could not be restored!', + ); + expect(handlers.navigateTo).not.toHaveBeenCalled(); + }); + it('opens the restored record while the cleanup alert is still open', async () => { + vi.mocked(restoreRecordWithoutLambda).mockResolvedValue({ + restoredRecord: { id: 'record-id', modelID: 'model-id' }, + cleanupError: restorationError, + }); + const handlers = callbacks(); + await restore(handlers); + expect(handlers.alertFn).toHaveBeenCalledWith( + 'The record was restored, but its archive could not be removed.', + ); + expect(handlers.navigateTo).toHaveBeenCalledWith( + '/environments/sandbox/editor/item_types/model-id/items/record-id', + ); + }); +}); diff --git a/record-bin/src/entrypoints/BinOutlet.tsx b/record-bin/src/entrypoints/BinOutlet.tsx index d3d904f0..3683eb33 100644 --- a/record-bin/src/entrypoints/BinOutlet.tsx +++ b/record-bin/src/entrypoints/BinOutlet.tsx @@ -34,7 +34,7 @@ function buildRecordEditPath( const parseLambdaRecordBody = async ( rawRecordBody: unknown, debugLogger: DebugLogger, - alertFn: (message: string) => Promise, + alertFn: (message: string) => void, ): Promise<{ success: false } | { success: true; parsedBody: unknown }> => { try { const parsedBody = @@ -44,7 +44,7 @@ const parseLambdaRecordBody = async ( return { success: true, parsedBody }; } catch (error) { debugLogger.warn('Could not parse record_body JSON before restore', error); - await alertFn('The record body is not valid JSON.'); + alertFn('The record body is not valid JSON.'); return { success: false }; } }; @@ -65,7 +65,7 @@ const executeRestoreViaLambda = async ({ parsedRecordBody: Record; trashRecordId: string; debugLogger: DebugLogger; - alertFn: (message: string) => Promise; + alertFn: (message: string) => void; noticeFn: (message: string) => void; navigateTo: (path: string) => void; environment: string; @@ -95,7 +95,7 @@ const executeRestoreViaLambda = async ({ 'Restoration request failed before receiving response', restorationError, ); - await alertFn( + alertFn( 'The restore API did not confirm the result. Check whether the record was restored before trying again.', ); return false; @@ -126,7 +126,7 @@ const executeRestoreViaLambda = async ({ status: restoreResponse.status, restorationError, }); - await alertFn('The record could not be restored!'); + alertFn('The record could not be restored!'); return false; } @@ -146,7 +146,7 @@ const executeRestoreViaLambda = async ({ return true; }; -const executeRestoreWithoutLambda = async ({ +export const executeRestoreWithoutLambda = async ({ trashRecordId, parsedRecordBody, currentUserAccessToken, @@ -165,7 +165,7 @@ const executeRestoreWithoutLambda = async ({ cmaBaseUrl?: string; environment: string; debugLogger: DebugLogger; - alertFn: (message: string) => Promise; + alertFn: (message: string) => void; noticeFn: (message: string) => void; navigateTo: (path: string) => void; isEnvironmentPrimary: boolean; @@ -191,7 +191,7 @@ const executeRestoreWithoutLambda = async ({ 'Lambda-less restoration request failed', error.restorationError, ); - await alertFn('The record could not be restored!'); + alertFn('The record could not be restored!'); return; } throw error; @@ -210,9 +210,7 @@ const executeRestoreWithoutLambda = async ({ 'Restored record archive cleanup failed', parsedResponse.cleanupError, ); - await alertFn( - 'The record was restored, but its archive could not be removed.', - ); + alertFn('The record was restored, but its archive could not be removed.'); } noticeFn('The record has been successfully restored!'); navigateTo( @@ -281,7 +279,7 @@ const runLambdaModeRestore = async ({ pluginParameters: Record; trashRecordId: string | undefined; debugLogger: DebugLogger; - alertFn: (message: string) => Promise; + alertFn: (message: string) => void; noticeFn: (message: string) => void; navigateTo: (path: string) => void; environment: string; @@ -292,7 +290,7 @@ const runLambdaModeRestore = async ({ if (!deploymentURL) { debugLogger.warn('Missing deployment URL while restoring record'); - await alertFn('The plugin deployment URL is missing.'); + alertFn('The plugin deployment URL is missing.'); return; } @@ -302,13 +300,13 @@ const runLambdaModeRestore = async ({ Array.isArray(parsedRecordBody); if (isInvalidBody) { debugLogger.warn('record_body must be a JSON object for lambda restore'); - await alertFn('The record body is not a valid restore payload.'); + alertFn('The record body is not a valid restore payload.'); return; } if (!trashRecordId) { debugLogger.warn('Missing trash record id in lambda restore'); - await alertFn('Could not determine the trash record to remove.'); + alertFn('Could not determine the trash record to remove.'); return; } @@ -356,6 +354,7 @@ const BinOutlet = ({ ctx }: { ctx: RenderItemFormOutletCtx }) => { const runtimeMode = getRuntimeMode(ctx.plugin.attributes.parameters); const rawRecordBody = ctx.formValues.record_body; + // Alerts stay open until the user closes them; awaiting one would stall the restore flow. const alertFn = ctx.alert.bind(ctx); const noticeFn = ctx.notice.bind(ctx); const navigateTo = ctx.navigateTo.bind(ctx); @@ -399,7 +398,7 @@ const BinOutlet = ({ ctx }: { ctx: RenderItemFormOutletCtx }) => { if (!ctx.item?.id) { debugLogger.warn('Missing trash record id in lambda-less restore'); - await alertFn('Could not determine the trash record to remove.'); + alertFn('Could not determine the trash record to remove.'); return; } @@ -419,7 +418,7 @@ const BinOutlet = ({ ctx }: { ctx: RenderItemFormOutletCtx }) => { } catch (err) { setError(buildRestoreErrorPayload(err)); debugLogger.error('Restoration flow failed', err); - await alertFn('The record could not be restored!'); + alertFn('The record could not be restored!'); } finally { restorationInProgress.current = false; setLoading(false); diff --git a/record-bin/src/entrypoints/ConfigScreen.tsx b/record-bin/src/entrypoints/ConfigScreen.tsx index 511c361c..775eb28f 100644 --- a/record-bin/src/entrypoints/ConfigScreen.tsx +++ b/record-bin/src/entrypoints/ConfigScreen.tsx @@ -425,7 +425,7 @@ export default function ConfigScreen({ ctx }: { ctx: RenderConfigScreenCtx }) { webhookRemovalError, ); if (notifyOnFailure) { - await ctx.notice( + ctx.alert( "Runtime was saved as Lambda-less, but one or more managed '🗑 Record Bin' webhooks could not be removed automatically. Remove them manually if needed.", ); } @@ -799,7 +799,7 @@ export default function ConfigScreen({ ctx }: { ctx: RenderConfigScreenCtx }) { setShowConnectionDetails(false); } - await ctx.alert('Could not disconnect the current lambda function.'); + ctx.alert('Could not disconnect the current lambda function.'); }, [applyWebhookSyncErrorState, canManageWebhooks, ctx, debugLogger], ); @@ -942,7 +942,7 @@ export default function ConfigScreen({ ctx }: { ctx: RenderConfigScreenCtx }) { }); } catch (saveError) { debugLogger.warn('Failed to save plugin settings', saveError); - await ctx.alert('Could not save plugin settings.'); + ctx.alert('Could not save plugin settings.'); } finally { setLoading(false); } diff --git a/record-bin/src/index.test.tsx b/record-bin/src/index.test.tsx index a52314f1..4be730d6 100644 --- a/record-bin/src/index.test.tsx +++ b/record-bin/src/index.test.tsx @@ -26,7 +26,7 @@ const hook = () => { const context = (runtimeMode = 'lambdaless') => ({ plugin: { attributes: { parameters: { runtimeMode } } }, openModal: vi.fn().mockResolvedValue(true), - notice: vi.fn().mockResolvedValue(undefined), + alert: vi.fn().mockResolvedValue(undefined), }); beforeEach(() => { vi.mocked(captureDeletedItemsWithoutLambda).mockReset(); @@ -60,11 +60,13 @@ describe('delete hook decisions', () => { })); expect(await hook()(items, ctx)).toBe(false); expect(ctx.openModal).not.toHaveBeenCalled(); - expect(ctx.notice).toHaveBeenCalledWith( + expect(ctx.alert).toHaveBeenCalledWith( expect.stringContaining('above 200'), ); expect(captureDeletedItemsWithoutLambda).toHaveBeenCalledTimes(1); - ctx.notice.mockRejectedValue(new Error('notice failed')); + ctx.alert.mockRejectedValue(new Error('alert failed')); + expect(await hook()(items, ctx)).toBe(false); + ctx.alert.mockReturnValue(new Promise(() => {})); expect(await hook()(items, ctx)).toBe(false); }); it('preserves the separately deployed Lambda runtime contract', async () => { diff --git a/record-bin/src/index.tsx b/record-bin/src/index.tsx index 78657bc2..5c32d4bc 100644 --- a/record-bin/src/index.tsx +++ b/record-bin/src/index.tsx @@ -25,12 +25,13 @@ connect({ // The host currently sends one bulk request, whose CMA limit is 200 IDs. // Performing deletion here would bypass other plugins' concurrent vetoes. if (items.length > 200) { - await ctx - .notice( + // The alert promise settles when the toast closes; awaiting it holds the hook open. + void ctx + .alert( 'Record Bin stopped this deletion: the dashboard must support bounded bulk operations before a selection above 200 records can be deleted safely. The original records have been kept.', ) .catch((error) => - debugLogger.warn('Could not display deletion limit notice', error), + debugLogger.warn('Could not display deletion limit alert', error), ); return false; } diff --git a/record-bin/src/utils/lambdaLessCapture.test.ts b/record-bin/src/utils/lambdaLessCapture.test.ts index b8c05943..f680fc3a 100644 --- a/record-bin/src/utils/lambdaLessCapture.test.ts +++ b/record-bin/src/utils/lambdaLessCapture.test.ts @@ -37,7 +37,8 @@ const context = (token: string | undefined = 'token') => ({ environment: 'main', cmaBaseUrl: 'https://example.invalid', plugin: { attributes: { parameters: {} } }, - notice: vi.fn().mockResolvedValue(undefined), + alert: vi.fn().mockResolvedValue(undefined), + customToast: vi.fn().mockResolvedValue(null), }); type Request = { method: string; @@ -193,6 +194,27 @@ describe('pre-deletion capture', () => { }); expect(mock.writes()).toBe(0); }); + it('returns a blocked result without waiting for the alert to close', async () => { + setup({ read: () => undefined }); + const ctx = context(); + ctx.alert.mockReturnValue(new Promise(() => {})); + expect((await run(['one'], ctx)).allowDeletion).toBe(false); + expect(ctx.alert).toHaveBeenCalledTimes(1); + }); + it('announces the backup without waiting for the toast to close', async () => { + setup(); + const ctx = context(); + ctx.customToast.mockReturnValue(new Promise(() => {})); + expect((await run(['one', 'two'], ctx)).allowDeletion).toBe(true); + expect(ctx.customToast).toHaveBeenCalledTimes(1); + expect(ctx.customToast).toHaveBeenCalledWith( + expect.objectContaining({ + type: 'notice', + message: expect.stringContaining('2 records'), + dismissOnPageChange: true, + }), + ); + }); it('blocks deletion without an access token', async () => { const ctx = context(); ctx.currentUserAccessToken = undefined; diff --git a/record-bin/src/utils/lambdaLessCapture.ts b/record-bin/src/utils/lambdaLessCapture.ts index f46bc930..5c39142e 100644 --- a/record-bin/src/utils/lambdaLessCapture.ts +++ b/record-bin/src/utils/lambdaLessCapture.ts @@ -108,7 +108,12 @@ export type LambdaLessCaptureResult = { export type CaptureContext = Pick< OnBeforeItemsDestroyCtx, - 'plugin' | 'currentUserAccessToken' | 'environment' | 'cmaBaseUrl' | 'notice' + | 'plugin' + | 'currentUserAccessToken' + | 'environment' + | 'cmaBaseUrl' + | 'alert' + | 'customToast' >; async function archiveRecord( @@ -291,6 +296,7 @@ class CaptureOperation { this.ids.some((id) => typeof id !== 'string' || !id || id.includes(',')) ) throw new Error('Invalid deletion selection.'); + this.announceBackup(); const client = buildCaptureClient(this.ctx); const bin = await ensureRecordBinModel(client); await this.capture(client, bin.id); @@ -303,13 +309,41 @@ class CaptureOperation { } catch (error) { this.handleFailure(error); } + // The alert promise settles when the toast closes; awaiting it holds the hook open. if (!this.result.allowDeletion) { - await this.ctx.notice( - 'Record Bin could not verify a complete archive. Deletion was cancelled; the original records have been kept.', - ); + void this.ctx + .alert( + 'Record Bin could not verify a complete archive. Deletion was cancelled; the original records have been kept.', + ) + .catch((error) => + this.debugLogger.warn('Could not display archive alert', error), + ); } return this.result; } + + /** + * Tells the user why the deletion takes a few seconds. Fire-and-forget: the toast promise + * settles only when the toast closes, so awaiting it would hold the hook open. + */ + private announceBackup() { + const message = + this.ids.length === 1 + ? 'Record Bin is saving a copy of this record before it is deleted. This takes a few seconds.' + : `Record Bin is saving copies of ${this.ids.length} records before they are deleted. This takes a few seconds.`; + void this.ctx + .customToast({ + type: 'notice', + message, + // A successful deletion leaves the record page, which closes the toast. The timeout + // covers failures (which show their own alert) and bulk deletions from a list. + dismissOnPageChange: true, + dismissAfterTimeout: 15000, + }) + .catch((error) => + this.debugLogger.warn('Could not display backup notice', error), + ); + } } export async function captureDeletedItemsWithoutLambda(