mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-10-25 14:00:40 +00:00
fix(web): a copy or move whose source body was behind its live document says so instead of a green success (BUG-3230 U1) (#1594)
* fix(web): a copy or move whose source body was behind its live document says so instead of a green success (BUG-3230 U1) * fix(web): the move toast says the archived original MAY still hold the edits (BUG-3230 U1, codex r1)
This commit is contained in:
@@ -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');
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -157,7 +157,8 @@ const ASYNC_FUNCTIONS: Record<string, Row> = {
|
||||
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' },
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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 };
|
||||
}
|
||||
Reference in New Issue
Block a user