diff --git a/tests/browser/workspace-images-delete.spec.mjs b/tests/browser/workspace-images-delete.spec.mjs index 6984c2cd..bf727731 100644 --- a/tests/browser/workspace-images-delete.spec.mjs +++ b/tests/browser/workspace-images-delete.spec.mjs @@ -122,3 +122,50 @@ test('Delete… in the viewer closes it after the delete', async ({ page }) => { await expect(viewer).toBeHidden(); await expect(page.locator(CARDS)).toHaveCount(before - 1); }); + +// The review never starts a delete without an explicit choice (live test 2026-10-29): every activation of the +// aria-disabled primary is ignored, the dialog has no form to submit, and a chosen primary starts once. +test('without a chosen mode nothing can start a delete', async ({ page }) => { + const before = await openImages(page); + await check(page, 0); + const applies = []; + page.on('request', request => { if (request.url().includes('/maintenance/apply')) applies.push(request.postDataJSON()); }); + await bar(page).getByRole('button', { name: 'Delete…', exact: true }).click(); + const dialog = review(page); + await expect(dialog).toBeVisible(); + const primary = dialog.locator('.dialog-actions button.danger'); + await expect(primary).toHaveAttribute('aria-disabled', 'true'); + await expect(primary).toHaveText('Choose what happens to the files'); + // Visibly dimmed while no mode is chosen. + await expect(primary).toHaveCSS('opacity', '0.55'); + await expect(primary).toHaveCSS('cursor', 'not-allowed'); + await primary.click({ force: true }); + await primary.dblclick({ force: true }); + await primary.focus(); + await page.keyboard.press('Enter'); + await page.keyboard.press('Space'); + await dialog.locator('#modal-title').click(); + await page.keyboard.press('Enter'); + expect(await dialog.locator('form').count()).toBe(0); + await page.evaluate(() => document.querySelector('dialog#modal[open]')?.dispatchEvent(new Event('submit', { bubbles: true }))); + await expect(dialog).toBeVisible(); + expect(applies).toEqual([]); + for (const radio of await dialog.getByRole('radio').all()) await expect(radio).not.toBeChecked(); + await dialog.getByRole('button', { name: 'Cancel', exact: true }).click(); + await expect(page.locator(CARDS)).toHaveCount(before); + expect(applies).toEqual([]); +}); + +test('a chosen mode starts exactly one delete, with that mode, however often it is pressed', async ({ page }) => { + const before = await openImages(page); + await check(page, 0); + const applies = []; + page.on('request', request => { if (request.url().includes('/maintenance/apply')) applies.push(request.postDataJSON()); }); + const { primary } = await deleteChecked(page, 'library'); + await primary.dblclick(); + await expect(toast(page)).toContainText('Removed 1 image from the library.'); + await expect(page.locator(CARDS)).toHaveCount(before - 1); + expect(applies.length).toBe(1); + expect(applies[0].mode).toBe('library'); + expect(Object.keys(applies[0]).sort()).toEqual(['mode', 'token']); +}); diff --git a/tests/js/workspace-images-delete.test.js b/tests/js/workspace-images-delete.test.js index 9e98f50a..90782294 100644 --- a/tests/js/workspace-images-delete.test.js +++ b/tests/js/workspace-images-delete.test.js @@ -139,3 +139,11 @@ test('apply sends the token and the mode, and toasts that the job started', asyn store, plan: plan(), mode: 'library', notify: text => toasts.push(text) }); assert.equal(refused, null); }); + +test('apply refuses to send a delete without an explicit mode', async () => { + const { applyDelete } = module(); + const api = { post: () => assert.fail('no request without a mode') }; + for (const mode of [undefined, null, '', 'TRASH', 'delete']) { + await assert.rejects(applyDelete({ api, store: null, plan: plan(), mode, notify: () => {} }), TypeError); + } +}); diff --git a/tests/test_image_delete_routes.py b/tests/test_image_delete_routes.py index d199a511..1fb364df 100644 --- a/tests/test_image_delete_routes.py +++ b/tests/test_image_delete_routes.py @@ -106,10 +106,38 @@ class TestReview(ImageDeleteTestCase): async def test_mode_is_required_and_only_for_this_plan(self): self.write("a.png", b"aaa") + self.link("a.png") plan = await self.review(["a.png"]) - await self.post(APPLY, {"token": plan["token"]}, 400) - await self.post(APPLY, {"token": plan["token"], "mode": "delete"}, 400) + # No explicit, exact mode: refused, never a default (least of all trash). + for body in ( + {"token": plan["token"]}, + {"token": plan["token"], "mode": None}, + {"token": plan["token"], "mode": ""}, + {"token": plan["token"], "mode": "TRASH"}, + {"token": plan["token"], "mode": "delete"}, + {"token": plan["token"], "mode": ["trash"]}, + {"token": plan["token"], "mode": True}, + ): + with self.subTest(body=body): + await self.post(APPLY, body, 400) self.assertTrue((self.output_dir / "a.png").exists()) + self.assertEqual(self.fake.moved, []) + self.assertEqual(self.count("SELECT COUNT(*) FROM generated_images"), 1) + # The refusals left the token usable: an explicit mode still applies. + body = await self.post(APPLY, {"token": plan["token"], "mode": "library"}, 202) + job = self.api._jobs.get(body["job"]["id"]) + await asyncio.wait_for(asyncio.shield(job.task), DISK_BOUND_SECONDS) + self.assertEqual(job.scope["mode"], "library") + self.assertTrue((self.output_dir / "a.png").exists()) + self.assertEqual(self.fake.moved, []) + + async def test_mode_on_another_plan_is_refused(self): + plan = await self.post( + "/prompt_manager/maintenance/preview", + {"operation": "missing_links", "options": {}}, + 200, + ) + await self.post(APPLY, {"token": plan["token"], "mode": "trash"}, 400) class TestLibraryMode(ImageDeleteTestCase): diff --git a/web/js/workspace/images-delete.mjs b/web/js/workspace/images-delete.mjs index baa0b286..7f53015e 100644 --- a/web/js/workspace/images-delete.mjs +++ b/web/js/workspace/images-delete.mjs @@ -195,12 +195,12 @@ export function showDeleteReview(plan, {scope, count, trigger = null, onApply}) openCloseOnlyReview({scope, title, grid: deleteRows(plan, 'library'), sentence: reason, trigger}); return null; } - let mode = null; + let mode = null, started = false; const list = el('dl', undefined, 'meta-grid'); list.hidden = true; const notes = el('div', undefined, 'image-delete-notes'); const cancel = createButton('Cancel'); - const start = createButton(primaryLabel('library', modeCount(plan, 'library')), 'danger'); + const start = createButton('Choose what happens to the files', 'danger image-delete-start'); start.setAttribute('aria-disabled', 'true'); function render() { list.replaceChildren(); @@ -219,8 +219,12 @@ export function showDeleteReview(plan, {scope, count, trigger = null, onApply}) } const group = modeGroup(value => { mode = value; render(); }); cancel.addEventListener('click', () => closeWorkspaceDialog()); + // Nothing starts without an explicit mode: an aria-disabled primary ignores every activation (click, Enter, Space), + // and a chosen one starts once however often it is pressed. The dialog has no form, so Enter elsewhere submits nothing. start.addEventListener('click', () => { - if (!mode || start.getAttribute('aria-disabled') === 'true') return; + if (started || !MODES.some(choice => choice.value === mode) || start.getAttribute('aria-disabled') === 'true') return; + started = true; + start.setAttribute('aria-disabled', 'true'); const chosen = mode; closeWorkspaceDialog(); onApply(chosen); @@ -233,6 +237,7 @@ export function showDeleteReview(plan, {scope, count, trigger = null, onApply}) /** Apply `plan` in `mode`; the started job, or null (a toast said why). */ export async function applyDelete({api, store, plan, mode, notify = showWorkspaceToast}) { + if (!MODES.some(choice => choice.value === mode)) throw new TypeError('A delete needs an explicit mode'); let result; try { result = await api.post(APPLY_URL, {token: plan.token, mode}); diff --git a/web/js/workspace/workspace.css b/web/js/workspace/workspace.css index de08fa2f..b421d847 100644 --- a/web/js/workspace/workspace.css +++ b/web/js/workspace/workspace.css @@ -478,6 +478,7 @@ pre.prompt-text { font: inherit; font-size: 12px; line-height: 1.75; margin-bloc /* D147 (#430): a state that blocks Save shows its reason on its own row above the sticky actions, with Refresh for a changed source; the disabled actions look disabled; a refused capture's empty preview says so. */ .tagging-modal .dialog-actions [aria-disabled="true"], .tagging-modal .tag-model-panel [aria-disabled="true"] { opacity: .55; cursor: not-allowed; /* D147 */ } +#modal .dialog-actions .image-delete-start[aria-disabled="true"] { opacity: .55; cursor: not-allowed; /* #480 D159: no mode chosen yet */ } .tagging-modal .tag-action-notice { flex: 1 0 100%; display: flex; flex-wrap: wrap; align-items: center; justify-content: space-between; gap: 8px; margin: 0; color: var(--ca-text); font-size: 12px; /* D147 */ } .tagging-modal .tag-action-notice > span { flex: 1 1 16em; } .tag-image-preview.tag-preview-empty { display: grid; place-items: center; padding: 8px; text-align: center; font-size: 12px; color: var(--ca-muted); background: var(--ca-field); box-sizing: border-box; /* D147 */ }