fix(email): defer source removal on cross-account move to Stalwart
The explicit Email/set destroy workaround for the duplicate-on-move bug is removed now that the root cause is filed upstream (support.stalw.art #1150: onSuccessDestroyOriginal destroys the copy's create-id instead of the source id). copyEmailAcrossAccounts keeps requesting onSuccessDestroyOriginal, so the move self-heals once Stalwart ships the fix. Kept: the keyword-preservation fix (carry the source keywords into Email/copy) so the moved message keeps its read state. Tests: 08-shared-moves still asserts delivery + read-state on every cross-account case; the source-removal checks are re-pinned test.fail, scoped to a nested describe, until #1150 is fixed. Suite green (5 pass, 3 expected-fail).
This commit is contained in:
Stefan Hildebrandt
@@ -15,12 +15,18 @@ import {
|
||||
* Moving mail across the own-account / shared-folder boundary, in both
|
||||
* directions, and between two shared folders (same owner and across owners).
|
||||
* The move is driven from the list context menu's "Move to" submenu; the
|
||||
* authoritative check is the server-side mailbox the message ends up in. Each
|
||||
* cross-account case also asserts the source copy is gone (no duplicate) and
|
||||
* the read state survives the move (Email/copy drops keywords unless carried).
|
||||
* authoritative check is the server-side mailbox the message ends up in.
|
||||
*
|
||||
* Each cross-account case asserts delivery *and* that the read state survives
|
||||
* (Email/copy drops keywords unless carried). Removing the source, however, is
|
||||
* currently blocked by a Stalwart bug — onSuccessDestroyOriginal destroys the
|
||||
* copy's create-id instead of the source id, so the original is left behind
|
||||
* (support.stalw.art #1150). Those source-removal checks are pinned test.fail
|
||||
* until Stalwart ships the fix; same-account moves (Email/set) are unaffected.
|
||||
*/
|
||||
const { alice, bob, carol } = ACCOUNTS;
|
||||
const subj = (l: string) => `IT ${l} ${Date.now()}`;
|
||||
type FolderSel = Parameters<typeof folderMailboxId>[1];
|
||||
|
||||
test.describe('Shared-folder moves', () => {
|
||||
let ja: JmapClient; // owner A
|
||||
@@ -54,18 +60,28 @@ test.describe('Shared-folder moves', () => {
|
||||
|
||||
const seenOf = (m: any) => Boolean(m?.keywords?.$seen);
|
||||
|
||||
// Log in as carol, reveal the relevant shared owners, and move `subject` from
|
||||
// `source` to `dest` via the context menu.
|
||||
async function uiMove(
|
||||
page: import('@playwright/test').Page,
|
||||
opts: { subject: string; owners: string[]; source: FolderSel; dest: FolderSel },
|
||||
): Promise<void> {
|
||||
await login(page, carol);
|
||||
for (const o of opts.owners) await expandSharedFolders(page, o);
|
||||
const destId = await folderMailboxId(page, opts.dest);
|
||||
await openFolder(page, opts.source);
|
||||
await forceSync(page);
|
||||
await moveEmailTo(page, opts.subject, destId);
|
||||
await page.waitForTimeout(2000);
|
||||
}
|
||||
|
||||
const inbox: FolderSel = { role: 'inbox', shared: false };
|
||||
const shared = (name: string): FolderSel => ({ name, shared: true });
|
||||
|
||||
test('shared folder A -> shared folder B (same owner)', async ({ page }) => {
|
||||
const s = subj('mv-a2b');
|
||||
await seedRead(teamA, s);
|
||||
|
||||
await login(page, carol);
|
||||
await expandSharedFolders(page, alice.email);
|
||||
const dest = await folderMailboxId(page, { name: 'TeamB', shared: true });
|
||||
await openFolder(page, { name: 'TeamA', shared: true });
|
||||
await forceSync(page);
|
||||
|
||||
await moveEmailTo(page, s, dest);
|
||||
await page.waitForTimeout(1500);
|
||||
await uiMove(page, { subject: s, owners: [alice.email], source: shared('TeamA'), dest: shared('TeamB') });
|
||||
|
||||
const inB = await ja.findEmailBySubject(s, teamB);
|
||||
expect(inB, 'message in TeamB').toBeTruthy();
|
||||
@@ -76,15 +92,7 @@ test.describe('Shared-folder moves', () => {
|
||||
test('shared folder B -> shared folder A (same owner)', async ({ page }) => {
|
||||
const s = subj('mv-b2a');
|
||||
await seedRead(teamB, s);
|
||||
|
||||
await login(page, carol);
|
||||
await expandSharedFolders(page, alice.email);
|
||||
const dest = await folderMailboxId(page, { name: 'TeamA', shared: true });
|
||||
await openFolder(page, { name: 'TeamB', shared: true });
|
||||
await forceSync(page);
|
||||
|
||||
await moveEmailTo(page, s, dest);
|
||||
await page.waitForTimeout(1500);
|
||||
await uiMove(page, { subject: s, owners: [alice.email], source: shared('TeamB'), dest: shared('TeamA') });
|
||||
|
||||
const inA = await ja.findEmailBySubject(s, teamA);
|
||||
expect(inA, 'message in TeamA').toBeTruthy();
|
||||
@@ -92,67 +100,67 @@ test.describe('Shared-folder moves', () => {
|
||||
expect(seenOf(inA), 'read state kept').toBe(true);
|
||||
});
|
||||
|
||||
// Cross-owner shared → shared: source is alice's account, destination is bob's,
|
||||
// so this exercises the true cross-account Email/copy + destroy path.
|
||||
test('shared folder (owner A) -> shared folder (owner B)', async ({ page }) => {
|
||||
// Cross-account cases: delivery + read state must hold (our fix); removing the
|
||||
// source is pinned test.fail below (Stalwart #1150).
|
||||
test('cross-owner shared -> shared: delivers and keeps read state', async ({ page }) => {
|
||||
const s = subj('mv-a2c');
|
||||
await seedRead(teamA, s);
|
||||
|
||||
await login(page, carol);
|
||||
await expandSharedFolders(page, alice.email);
|
||||
await expandSharedFolders(page, bob.email);
|
||||
const dest = await folderMailboxId(page, { name: 'TeamC', shared: true });
|
||||
await openFolder(page, { name: 'TeamA', shared: true });
|
||||
await forceSync(page);
|
||||
|
||||
await moveEmailTo(page, s, dest);
|
||||
await page.waitForTimeout(2000);
|
||||
await uiMove(page, { subject: s, owners: [alice.email, bob.email], source: shared('TeamA'), dest: shared('TeamC') });
|
||||
|
||||
const inC = await jb.findEmailBySubject(s, teamC);
|
||||
expect(inC, 'message in bob TeamC').toBeTruthy();
|
||||
expect(await ja.findEmailBySubject(s, teamA), 'message left alice TeamA').toBeFalsy();
|
||||
expect(seenOf(inC), 'read state kept').toBe(true);
|
||||
});
|
||||
|
||||
// The "Move to" submenu relocates a message across the account boundary
|
||||
// (own ↔ shared folder) via copy+delete, matching drag-and-drop.
|
||||
test('own account -> shared folder', async ({ page }) => {
|
||||
test('own account -> shared folder: delivers and keeps read state', async ({ page }) => {
|
||||
const s = subj('mv-own2sh');
|
||||
await sendMail({ from: carol.email, authPass: carol.password, to: carol.email, subject: s, body: 'x' });
|
||||
const own = await jc.waitForEmail(s);
|
||||
await jc.setSeen(own.id, true);
|
||||
|
||||
await login(page, carol);
|
||||
await expandSharedFolders(page, alice.email);
|
||||
const dest = await folderMailboxId(page, { name: 'TeamA', shared: true });
|
||||
await openFolder(page, { role: 'inbox', shared: false });
|
||||
await forceSync(page);
|
||||
|
||||
await moveEmailTo(page, s, dest);
|
||||
await page.waitForTimeout(2000);
|
||||
await uiMove(page, { subject: s, owners: [alice.email], source: inbox, dest: shared('TeamA') });
|
||||
|
||||
const inTeam = await ja.findEmailBySubject(s, teamA);
|
||||
expect(inTeam, 'message in shared TeamA').toBeTruthy();
|
||||
expect(await jc.findEmailBySubject(s), 'message left own account').toBeFalsy();
|
||||
expect(seenOf(inTeam), 'read state kept').toBe(true);
|
||||
});
|
||||
|
||||
test('shared folder -> own account', async ({ page }) => {
|
||||
test('shared folder -> own account: delivers and keeps read state', async ({ page }) => {
|
||||
const s = subj('mv-sh2own');
|
||||
await seedRead(teamA, s);
|
||||
|
||||
await login(page, carol);
|
||||
await expandSharedFolders(page, alice.email);
|
||||
const dest = await folderMailboxId(page, { role: 'inbox', shared: false });
|
||||
await openFolder(page, { name: 'TeamA', shared: true });
|
||||
await forceSync(page);
|
||||
|
||||
await moveEmailTo(page, s, dest);
|
||||
await page.waitForTimeout(2000);
|
||||
await uiMove(page, { subject: s, owners: [alice.email], source: shared('TeamA'), dest: inbox });
|
||||
|
||||
const inOwn = await jc.findEmailBySubject(s);
|
||||
expect(inOwn, 'message in own account').toBeTruthy();
|
||||
expect(await ja.findEmailBySubject(s, teamA), 'message left shared TeamA').toBeFalsy();
|
||||
expect(seenOf(inOwn), 'read state kept').toBe(true);
|
||||
});
|
||||
|
||||
// Pinned failing: Stalwart's onSuccessDestroyOriginal leaves the original in
|
||||
// place on a cross-account copy (support.stalw.art #1150). Un-pin once fixed
|
||||
// upstream (our copyEmailAcrossAccounts already requests the destroy).
|
||||
test.describe('source is removed after a cross-account move', () => {
|
||||
test.fail(true, 'blocked by Stalwart #1150 (onSuccessDestroyOriginal destroys wrong id)');
|
||||
|
||||
test('cross-owner shared -> shared', async ({ page }) => {
|
||||
const s = subj('rm-a2c');
|
||||
await seedRead(teamA, s);
|
||||
await uiMove(page, { subject: s, owners: [alice.email, bob.email], source: shared('TeamA'), dest: shared('TeamC') });
|
||||
expect(await ja.findEmailBySubject(s, teamA), 'original left alice TeamA').toBeFalsy();
|
||||
});
|
||||
|
||||
test('own account -> shared folder', async ({ page }) => {
|
||||
const s = subj('rm-own2sh');
|
||||
await sendMail({ from: carol.email, authPass: carol.password, to: carol.email, subject: s, body: 'x' });
|
||||
const own = await jc.waitForEmail(s);
|
||||
await jc.setSeen(own.id, true);
|
||||
await uiMove(page, { subject: s, owners: [alice.email], source: inbox, dest: shared('TeamA') });
|
||||
expect(await jc.findEmailBySubject(s), 'original left own account').toBeFalsy();
|
||||
});
|
||||
|
||||
test('shared folder -> own account', async ({ page }) => {
|
||||
const s = subj('rm-sh2own');
|
||||
await seedRead(teamA, s);
|
||||
await uiMove(page, { subject: s, owners: [alice.email], source: shared('TeamA'), dest: inbox });
|
||||
expect(await ja.findEmailBySubject(s, teamA), 'original left shared TeamA').toBeFalsy();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
+5
-13
@@ -6407,11 +6407,16 @@ export class JMAPClient implements IJMAPClient {
|
||||
]);
|
||||
const keywords = srcResp.methodResponses?.[0]?.[1]?.list?.[0]?.keywords ?? {};
|
||||
|
||||
// onSuccessDestroyOriginal is the spec-correct way to remove the source, but
|
||||
// Stalwart currently destroys the copy's create-id instead of the source id,
|
||||
// so the original is left behind — a duplicate on every cross-account move.
|
||||
// Reported upstream (support.stalw.art #1150); this self-heals once fixed.
|
||||
const response = await this.request([
|
||||
["Email/copy", {
|
||||
fromAccountId,
|
||||
accountId: toAccountId,
|
||||
create: { c: { id: emailId, mailboxIds: { [destMailboxId]: true }, keywords } },
|
||||
onSuccessDestroyOriginal: true,
|
||||
}, "0"],
|
||||
]);
|
||||
const res = response.methodResponses?.[0]?.[1];
|
||||
@@ -6423,19 +6428,6 @@ export class JMAPClient implements IJMAPClient {
|
||||
if (!id) {
|
||||
throw new Error("Email/copy succeeded but no ID returned");
|
||||
}
|
||||
|
||||
// onSuccessDestroyOriginal is unreliable on Stalwart (implicit destroy reports
|
||||
// notFound and leaves the original behind), so remove the source explicitly.
|
||||
const delResp = await this.request([
|
||||
["Email/set", { accountId: fromAccountId, destroy: [emailId] }, "0"],
|
||||
]);
|
||||
const notDestroyed = delResp.methodResponses?.[0]?.[1]?.notDestroyed?.[emailId];
|
||||
if (notDestroyed) {
|
||||
throw new Error(
|
||||
notDestroyed.description || notDestroyed.type ||
|
||||
"Copied email but failed to remove the original from the source folder",
|
||||
);
|
||||
}
|
||||
return id;
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user