diff --git a/web/e2e/copy-dialog.spec.ts b/web/e2e/copy-dialog.spec.ts index bdde381b3..3707e7248 100644 --- a/web/e2e/copy-dialog.spec.ts +++ b/web/e2e/copy-dialog.spec.ts @@ -711,4 +711,55 @@ test.describe('cross-workspace copy dialog (PLAN-2373 / TASK-2355)', () => { await expect(page.locator('.item-pane')).toBeVisible(); await expect(more).toBeFocused(); }); + + // BUG-3230 U1: the copy answers `warnings.source_content_state` when the body + // it carried was behind the source's live document (BUG-3032). The server + // half is pinned in Go; this leg pins that the PANE reads it, by rewriting + // the 201 to carry the field, since this tab's own pre-copy flush would + // otherwise clear the stale state it needs. On main the toast was a plain + // green "Copied to …", which is what this asserts against. + for (const move of [false, true]) { + test(`a ${move ? 'move' : 'copy'} whose source body was behind its live document says so`, async ({ + page, + fixture, + request, + }, testInfo) => { + test.skip(testInfo.project.name !== 'desktop-chromium', 'viewport driven explicitly'); + await page.setViewportSize(DESKTOP); + await browserLogin(page); + const dest = await seedDestination(request, fixture, move ? 'stale-move' : 'stale-copy', { + name: 'Notes', + slug: 'notes', + fields: [], + }); + const { slug } = await seedDoc(fixture, request, `Copy dialog stale ${move ? 'move' : 'copy'}`); + await page.goto(itemUrl(fixture, slug)); + + await page.route(/\/copy(\?|$)/, async (route) => { + if (route.request().method() !== 'POST') return route.continue(); + const resp = await route.fetch(); + const body = await resp.json(); + body.warnings.source_content_state = 'applied_pending_flush'; + await route.fulfill({ response: resp, json: body }); + }); + + await openCopyDialog(page); + const dialog = copyDialog(page); + await dialog.getByLabel('Workspace', { exact: true }).selectOption(dest.wsSlug); + const preflight = page.waitForResponse((r) => r.url().includes('/copy/preflight') && r.status() === 200); + await dialog.getByLabel('Collection', { exact: true }).selectOption(dest.collSlug); + await preflight; + if (move) await dialog.getByRole('radio', { name: /^Move — / }).check(); + const confirm = dialog.getByRole('button', { name: move ? 'Move' : 'Copy', exact: true }); + await expect(confirm).toBeEnabled(); + const copyResp = page.waitForResponse((r) => /\/copy(\?|$)/.test(r.url()) && r.request().method() === 'POST'); + await confirm.click(); + expect((await copyResp).status()).toBe(201); + + const toast = page.getByText(/may be missing them/).first(); + await expect(toast).toBeVisible(); + await expect(toast).toContainText(move ? 'Moved to' : 'Copied to'); + await expect(toast).toContainText(move ? 'archived original may still hold them' : 'Copy it again once they are saved'); + }); + } }); diff --git a/web/src/lib/components/items/ItemDetail.svelte b/web/src/lib/components/items/ItemDetail.svelte index ca27a8886..d36c0aae1 100644 --- a/web/src/lib/components/items/ItemDetail.svelte +++ b/web/src/lib/components/items/ItemDetail.svelte @@ -78,6 +78,7 @@ import EditCollectionModal from '$lib/components/collections/EditCollectionModal.svelte'; import ShareDialog from '$lib/components/ShareDialog.svelte'; import CopyItemDialog from '$lib/components/items/CopyItemDialog.svelte'; + import { copyResultToast } from '$lib/items/copyResultToast'; import PushToAgentDialog from '$lib/components/items/PushToAgentDialog.svelte'; import ItemAttachmentStrip from '$lib/components/items/ItemAttachmentStrip.svelte'; import DecisionChips from '$lib/components/items/DecisionChips.svelte'; @@ -5407,22 +5408,11 @@ * archived + provenance banners render now rather than on the next load. */ async function handleCopied(result: ItemCopyResult) { - const dest = result.destination; - const label = dest.ref ?? dest.slug; - const verb = result.source.archived ? 'Moved' : 'Copied'; - // A value dropped as NOT UNIQUE is named, not folded into a clean - // success (BUG-2367): the preview listed it, and the copy's own answer - // must say it too. - const notUnique = result.warnings.not_unique ?? []; - if (notUnique.length > 0) { - toastStore.show( - `${verb} to ${dest.workspace_name} as ${label}, without: ${notUnique.map((d) => d.message).join('; ')}`, - 'info', - 10000, - ); - } else { - toastStore.show(`${verb} to ${dest.workspace_name} as ${label}`, 'success'); - } + // A value dropped as NOT UNIQUE (BUG-2367) and a source body behind its + // live document (BUG-3230 U1) are named, not folded into a clean success. + const done = copyResultToast(result); + if (done.duration === undefined) toastStore.show(done.message, done.type); + else toastStore.show(done.message, done.type, done.duration); if (!result.source.archived) return; const targetItem = item; if (!targetItem) return; diff --git a/web/src/lib/components/items/itemDetailIdentityFenceAst.test.ts b/web/src/lib/components/items/itemDetailIdentityFenceAst.test.ts index 0e747145b..924f7c0cf 100644 --- a/web/src/lib/components/items/itemDetailIdentityFenceAst.test.ts +++ b/web/src/lib/components/items/itemDetailIdentityFenceAst.test.ts @@ -157,7 +157,8 @@ const ASYNC_FUNCTIONS: Record = { closeCopyDialog: { reviewed: '1c925f081a26', why: 'restores focus after closing synchronously', may: ['paneMenuTrigger.focus'] }, closePushDialog: { reviewed: 'bda8c7529677', why: 'restores focus after closing synchronously', may: ['paneMenuTrigger.focus'] }, flushContentBeforeCopy: { reviewed: 'd7083bf9954c', why: 'returns a boolean to the dialog' }, - handleCopied: { reviewed: 'f35fb099838d', why: 'switchedAway before adopting the refreshed item' }, + // BUG-3230 U1: the toast moved into copyResultToast, a pure function of the result; still before the await. + handleCopied: { reviewed: 'eecddaafa59b', why: 'switchedAway before adopting the refreshed item' }, handleDelete: { reviewed: 'b66adcf84794', why: 'switchedAway on both arms' }, handleRestore: { reviewed: 'c3a1732554a4', why: 'switchedAway on every arm' }, handleDeleteLink: { reviewed: 'd147285bb264', why: 'switchedAway after each await' }, diff --git a/web/src/lib/items/copyResultToast.test.ts b/web/src/lib/items/copyResultToast.test.ts new file mode 100644 index 000000000..015dd8da3 --- /dev/null +++ b/web/src/lib/items/copyResultToast.test.ts @@ -0,0 +1,57 @@ +import { describe, it, expect } from 'vitest'; +import { copyResultToast } from './copyResultToast'; +import type { ItemCopyResult } from '$lib/types'; + +function result(over: { archived?: boolean; stale?: boolean; notUnique?: string[] } = {}): ItemCopyResult { + return { + source: { archived: !!over.archived }, + destination: { workspace_name: 'Other', ref: 'TASK-9', slug: 'x' }, + archive_source: !!over.archived, + warnings: { + dropped_fields: [], + ...(over.notUnique ? { not_unique: over.notUnique.map((m) => ({ key: 'k', value: 'v', message: m })) } : {}), + dropped_assignee: false, + dropped_agent_role: false, + attachment_count: 0, + attachment_bytes: 0, + unresolvable_ref_count: 0, + ...(over.stale ? { source_content_state: 'applied_pending_flush' } : {}), + }, + } as unknown as ItemCopyResult; +} + +// BUG-3230 U1: a copy whose source body was behind its live document says so. +describe('copyResultToast', () => { + it('a clean copy and a clean move are green', () => { + expect(copyResultToast(result())).toEqual({ message: 'Copied to Other as TASK-9', type: 'success' }); + expect(copyResultToast(result({ archived: true }))).toEqual({ message: 'Moved to Other as TASK-9', type: 'success' }); + }); + + it('a stale source on a copy is not green, and says to copy again once saved', () => { + const t = copyResultToast(result({ stale: true })); + expect(t.type).toBe('info'); + expect(t.message).toMatch(/^Copied to Other as TASK-9\. /); + expect(t.message).toMatch(/may be missing them/); + expect(t.message).toMatch(/Copy it again once they are saved/); + }); + + it('a stale source on a move names the archived original as where the edits are', () => { + const t = copyResultToast(result({ stale: true, archived: true })); + expect(t.type).toBe('info'); + expect(t.message).toMatch(/^Moved to Other as TASK-9\. /); + expect(t.message).toMatch(/archived original may still hold them/); + // Never a promise: a purge or a schema-mismatch rebuild can delete them. + expect(t.message).not.toMatch(/still has them/); + }); + + it('not-unique drops keep their wording (BUG-2367), and combine with a stale source', () => { + expect(copyResultToast(result({ notUnique: ['slug "a" is taken'] }))).toEqual({ + message: 'Copied to Other as TASK-9, without: slug "a" is taken', + type: 'info', + duration: 10000, + }); + const both = copyResultToast(result({ notUnique: ['slug "a" is taken'], stale: true })); + expect(both.message).toMatch(/without: slug "a" is taken\. An open tab/); + expect(both.duration).toBe(15000); + }); +}); diff --git a/web/src/lib/items/copyResultToast.ts b/web/src/lib/items/copyResultToast.ts new file mode 100644 index 000000000..2412851af --- /dev/null +++ b/web/src/lib/items/copyResultToast.ts @@ -0,0 +1,46 @@ +// The toast a cross-workspace copy or move ends with (BUG-2367, BUG-3230 U1). +// +// Two things can make a copy land differently from a clean success, and the +// copy's own answer says both rather than folding them into a green toast: +// - a value dropped as NOT UNIQUE (BUG-2367): the preview listed it; +// - a source body BEHIND its live collaborative document +// (`warnings.source_content_state`, BUG-3032): an open tab had edits the +// source row did not hold yet, so the copy carried the older body. The +// server warns rather than refuses because the source keeps those edits. +// On a MOVE the source is archived. Archiving is a soft delete that leaves +// the op-log alone, and the dormant GC keeps unflushed rows, so the edits +// are USUALLY still with the archived original. Not always: a workspace +// purge deletes them, and so does the schema-mismatch rebuild when the item +// is next opened after an editor schema bump (collab manager +// maybeRebuildOnSchemaMismatch). So the wording says "may", never "has". +import type { ItemCopyResult } from '$lib/types'; + +export interface CopyResultToast { + message: string; + type: 'success' | 'info'; + /** Undefined leaves the toast store's default. */ + duration?: number; +} + +export function copyResultToast(result: ItemCopyResult): CopyResultToast { + const dest = result.destination; + const label = dest.ref ?? dest.slug; + const moved = result.source.archived; + const verb = moved ? 'Moved' : 'Copied'; + const parts: string[] = [`${verb} to ${dest.workspace_name} as ${label}`]; + + const notUnique = result.warnings.not_unique ?? []; + if (notUnique.length > 0) parts[0] += `, without: ${notUnique.map((d) => d.message).join('; ')}`; + + const stale = !!result.warnings.source_content_state; + if (stale) { + parts.push( + moved + ? 'An open tab had edits to this item that were not saved yet, so the moved copy may be missing them. The archived original may still hold them: restore it and open it, then move it again once they are saved.' + : 'An open tab had edits to this item that were not saved yet, so the copy may be missing them. Copy it again once they are saved.', + ); + } + + if (!stale && notUnique.length === 0) return { message: parts[0], type: 'success' }; + return { message: parts.join('. '), type: 'info', duration: stale ? 15000 : 10000 }; +}