mirror of
https://github.com/block/buzz.git
synced 2026-10-28 21:11:48 +00:00
fix(desktop): retain outgoing draft deletion authority after navigation
Bind authored revisions to captured lifecycle visits instead of the visible editor. Apply source authority to cancellation and preflight recovery and sent-draft cleanup, while keeping editor ownership separate. Cover reviewer deletion reproduction, queued files, supersession, return visits, and actual thread-switch persistence. Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Signed-off-by: Logan Johnson <loganj@squareup.com> (cherry picked from commit 13e1245bbd042fb6d1afa6cf54152e839d9fe400) Signed-off-by: Logan Johnson <loganj@squareup.com> (cherry picked from commit e5e77d88c3eb87360aac8771d17389abee831d3f) Signed-off-by: Logan Johnson <loganj@squareup.com>
This commit is contained in:
@@ -66,7 +66,7 @@ type UseDraftPersistLifecycleParams = {
|
||||
};
|
||||
|
||||
type UseDraftPersistLifecycleResult = {
|
||||
/** Monotonic editor revision, including same-text edit/clear cycles. */
|
||||
/** Captured visit revision; remains readable after exit, excluding later visits. */
|
||||
getComposerRevision: () => number;
|
||||
/** Optimistic send/recovery is not an authored edit or authoritative deletion. */
|
||||
runComposerUpdate: (update: () => void) => void;
|
||||
@@ -139,11 +139,13 @@ export function useDraftPersistLifecycle({
|
||||
[getImplicitAgentMentionPrefix],
|
||||
);
|
||||
|
||||
const composerRevision = React.useRef(0);
|
||||
const getComposerRevision = React.useCallback(
|
||||
() => composerRevision.current,
|
||||
[],
|
||||
// Pending sends retain this visit's record after navigation/unmount. A live
|
||||
// editor ref would lose A's deletion authority as soon as B became visible.
|
||||
const visit = React.useMemo(
|
||||
() => ({ channelId, draftKey: effectiveDraftKey, revision: 0 }),
|
||||
[channelId, effectiveDraftKey],
|
||||
);
|
||||
const getComposerRevision = React.useCallback(() => visit.revision, [visit]);
|
||||
const pendingImetaForPersistRef = React.useRef<ImetaMedia[]>([]);
|
||||
const emptyContentIsAuthoritativeRef = React.useRef(false);
|
||||
const isRestoringContentRef = React.useRef(false);
|
||||
@@ -240,7 +242,7 @@ export function useDraftPersistLifecycle({
|
||||
const trackAuthoredContent = React.useCallback(
|
||||
(content: string) => {
|
||||
if (isRestoringContentRef.current) return;
|
||||
composerRevision.current += 1;
|
||||
visit.revision += 1;
|
||||
if (!effectiveDraftKey) return;
|
||||
const authoritativeDraftKey = scopedDraftKey(effectiveDraftKey);
|
||||
if (content.length > 0) {
|
||||
@@ -259,7 +261,13 @@ export function useDraftPersistLifecycle({
|
||||
[],
|
||||
);
|
||||
},
|
||||
[channelId, effectiveDraftKey, persistDraft, spoileredAttachmentUrlsRef],
|
||||
[
|
||||
channelId,
|
||||
effectiveDraftKey,
|
||||
persistDraft,
|
||||
spoileredAttachmentUrlsRef,
|
||||
visit,
|
||||
],
|
||||
);
|
||||
|
||||
return { trackAuthoredContent, getComposerRevision, runComposerUpdate };
|
||||
|
||||
@@ -633,3 +633,181 @@ for (const action of ["unchanged", "navigate", "edit"]) {
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
// Independent reviewer reproduction, plus storage/queued-file boundaries.
|
||||
for (const media of [false, true]) {
|
||||
for (const action of ["dismiss", "navigate", "unmount"]) {
|
||||
test(`authored edit-clear before ${action} stays deleted (queued media: ${media})`, async () => {
|
||||
const s = await setup({ lifecycle: true });
|
||||
const gate = deferred();
|
||||
s.control.publish = gate;
|
||||
if (media) {
|
||||
s.dismiss();
|
||||
s.control.attachments = [{ id: "file", file: {}, spoilered: false }];
|
||||
await s.prompt();
|
||||
}
|
||||
await s.invite();
|
||||
let upload;
|
||||
if (media) {
|
||||
await s.act(async () => {
|
||||
upload = s.control.uploadCallbacks.onComplete(
|
||||
[],
|
||||
new AbortController().signal,
|
||||
);
|
||||
});
|
||||
}
|
||||
assert.equal(s.events("publish").length, 1);
|
||||
s.edit("new authored text");
|
||||
s.edit("", []);
|
||||
assert.equal(s.store.has("thread:a"), false);
|
||||
const recoveryWrites = s.events("save-queue").length;
|
||||
if (action === "navigate") s.navigate("thread:b");
|
||||
else if (action === "unmount") s.unmount();
|
||||
else s.dismiss();
|
||||
assert.equal(s.store.has("thread:a"), false);
|
||||
if (action === "navigate") {
|
||||
s.edit(TEXT, [{ ...s.refs[0], pubkey: "c".repeat(64) }]);
|
||||
s.navigate("thread:a");
|
||||
assert.equal(s.options.contentRef.current, "");
|
||||
assert.equal(s.control.currentRefs.length, 0);
|
||||
s.edit("new A after return", []);
|
||||
s.navigate("thread:b");
|
||||
}
|
||||
await s.finish(gate);
|
||||
if (upload) await upload;
|
||||
assert.equal(s.events("SEND").length, 0);
|
||||
assert.equal(s.events("save-queue").length, recoveryWrites);
|
||||
if (action === "navigate") {
|
||||
assert.equal(s.options.contentRef.current, TEXT);
|
||||
assert.equal(s.control.currentRefs[0].pubkey, "c".repeat(64));
|
||||
assert.equal(s.store.get("thread:a").content, "new A after return");
|
||||
assert.deepEqual(s.store.get("thread:a").mentionRefs, []);
|
||||
} else assert.equal(s.store.has("thread:a"), false);
|
||||
});
|
||||
}
|
||||
}
|
||||
for (const incoming of [TEXT, "different B text"]) {
|
||||
for (const edit of ["untouched", "same-text-new-refs", "new-text"]) {
|
||||
test(`source authority survives navigation: ${edit}, ${incoming}`, async () => {
|
||||
const s = await setup({ lifecycle: true });
|
||||
const otherRefs = [{ ...s.refs[0], pubkey: "c".repeat(64) }];
|
||||
s.store.set("thread:b", {
|
||||
content: incoming,
|
||||
channelId: "general",
|
||||
pendingImeta: [],
|
||||
spoileredAttachmentUrls: [],
|
||||
mentionRefs: otherRefs,
|
||||
});
|
||||
const gate = deferred();
|
||||
s.control.publish = gate;
|
||||
await s.invite();
|
||||
if (edit !== "untouched")
|
||||
s.edit(edit === "new-text" ? "new A text" : TEXT, otherRefs);
|
||||
s.navigate("thread:b");
|
||||
const saved = s.store.get("thread:a");
|
||||
assert.equal(saved.content, edit === "new-text" ? "new A text" : TEXT);
|
||||
assert.deepEqual(
|
||||
saved.mentionRefs,
|
||||
edit === "untouched" ? s.refs : otherRefs,
|
||||
);
|
||||
s.edit("B edit during old await", []);
|
||||
await s.finish(gate);
|
||||
assert.equal(s.store.get("thread:a"), saved);
|
||||
assert.equal(s.options.contentRef.current, "B edit during old await");
|
||||
assert.equal(s.control.currentRefs.length, 0);
|
||||
assert.equal(s.events("SEND").length, 0);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
for (const action of ["untouched", "deleted", "superseded"]) {
|
||||
test(`ordinary send preflight recovery respects source authority on unmount: ${action}`, async () => {
|
||||
const s = await setup({ lifecycle: true });
|
||||
s.dismiss();
|
||||
s.options.mentions.memberPubkeys = new Set([KEY]);
|
||||
const gate = deferred();
|
||||
s.control.prepare = gate;
|
||||
s.rerender();
|
||||
let send;
|
||||
await s.act(async () => {
|
||||
send = s.result.current.sendMessageWithMentionFlow({
|
||||
capturedChannelId: "general",
|
||||
pendingImeta: [],
|
||||
trimmed: TEXT,
|
||||
recoveryDraftKey: "thread:a",
|
||||
});
|
||||
});
|
||||
assert.equal(s.options.contentRef.current, "");
|
||||
if (action === "deleted") {
|
||||
s.edit("new text");
|
||||
s.edit("", []);
|
||||
}
|
||||
s.unmount();
|
||||
const other = {
|
||||
content: "new saved draft",
|
||||
channelId: "general",
|
||||
pendingImeta: [],
|
||||
spoileredAttachmentUrls: [],
|
||||
mentionRefs: [],
|
||||
};
|
||||
if (action === "superseded") s.store.set("thread:a", other);
|
||||
await s.finish(gate);
|
||||
await send;
|
||||
assert.equal(s.events("SEND").length, 0);
|
||||
if (action === "deleted") assert.equal(s.store.has("thread:a"), false);
|
||||
else if (action === "superseded")
|
||||
assert.equal(s.store.get("thread:a"), other);
|
||||
else {
|
||||
assert.equal(s.store.get("thread:a").content, TEXT);
|
||||
assert.deepEqual(s.store.get("thread:a").mentionRefs, s.refs);
|
||||
}
|
||||
});
|
||||
}
|
||||
for (const authored of [false, true]) {
|
||||
test(`sent-draft cleanup consults exited source visit, not B revision (authored: ${authored})`, async () => {
|
||||
const s = await setup({ lifecycle: true });
|
||||
s.dismiss();
|
||||
s.options.mentions.memberPubkeys = new Set([KEY]);
|
||||
s.options.drafts.markDraftSent = (...args) =>
|
||||
s.calls.push(["mark-sent", ...args]);
|
||||
const gate = deferred();
|
||||
s.control.publish = gate;
|
||||
s.rerender();
|
||||
let send;
|
||||
await s.act(async () => {
|
||||
send = s.result.current.sendMessageWithMentionFlow({
|
||||
capturedChannelId: "general",
|
||||
pendingImeta: [],
|
||||
trimmed: TEXT,
|
||||
recoveryDraftKey: "thread:a",
|
||||
sentDraftKey: "thread:a",
|
||||
});
|
||||
});
|
||||
if (authored) s.edit(TEXT); // same snapshot, but a genuinely newer draft
|
||||
const sourceRevision = s.options.getComposerRevision;
|
||||
const revision = sourceRevision();
|
||||
s.navigate("thread:b");
|
||||
s.edit("B edit", []);
|
||||
assert.equal(
|
||||
sourceRevision(),
|
||||
revision,
|
||||
"B must not change the captured A revision",
|
||||
);
|
||||
// An untouched optimistic empty has no stored refs. Seed the unchanged
|
||||
// submitted snapshot to exercise markDraftSent's bounded exact-ref guard.
|
||||
if (!authored)
|
||||
s.store.set("thread:a", {
|
||||
content: TEXT,
|
||||
channelId: "general",
|
||||
pendingImeta: [],
|
||||
spoileredAttachmentUrls: [],
|
||||
mentionRefs: s.refs,
|
||||
});
|
||||
await s.finish(gate);
|
||||
await send;
|
||||
assert.equal(s.events("SEND").length, 1); // ordinary destination-bound send
|
||||
assert.equal(s.events("mark-sent").length, authored ? 0 : 1);
|
||||
assert.equal(s.options.contentRef.current, "B edit");
|
||||
assert.equal(s.control.currentRefs.length, 0);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -16,6 +16,8 @@ export { MENTION_REFERENCE_TAG };
|
||||
export type ComposerDraftOwner = {
|
||||
channelId: string | null;
|
||||
draftKey: string | null | undefined;
|
||||
/** Read this source visit, never the currently visible editor visit. */
|
||||
getComposerRevision: () => number;
|
||||
};
|
||||
|
||||
export type PendingNonMemberMentionSend = {
|
||||
|
||||
@@ -81,8 +81,8 @@ export function useMentionSendFlow({
|
||||
// Persistence identity is independent of the host component and destination
|
||||
// channel. A -> B -> A must not revive A's previous invitation or recovery.
|
||||
const sourceOwner = React.useMemo(
|
||||
() => ({ channelId, draftKey: effectiveDraftKey }),
|
||||
[channelId, effectiveDraftKey],
|
||||
() => ({ channelId, draftKey: effectiveDraftKey, getComposerRevision }),
|
||||
[channelId, effectiveDraftKey, getComposerRevision],
|
||||
);
|
||||
const sourceOwnerRef = React.useRef(sourceOwner);
|
||||
sourceOwnerRef.current = sourceOwner;
|
||||
@@ -357,25 +357,15 @@ export function useMentionSendFlow({
|
||||
draft.queuedAttachments.length > 0
|
||||
? prepareBackgroundMediaUpload(draft.queuedAttachments)
|
||||
: null;
|
||||
const persistPreflightDraft = () => {
|
||||
if (isSendCancelled() || !draft.recoveryDraftKey) return;
|
||||
drafts.persistDraft(
|
||||
draft.recoveryDraftKey,
|
||||
draft.savedContent,
|
||||
draft.capturedChannelId ?? draft.recoveryDraftKey,
|
||||
draft.savedImeta,
|
||||
[...draft.savedSpoileredAttachmentUrls],
|
||||
draft.savedMentionRefs,
|
||||
);
|
||||
saveQueuedAttachmentsForDraft(
|
||||
draft.recoveryDraftKey,
|
||||
draft.queuedAttachments,
|
||||
);
|
||||
};
|
||||
const persistCanceledDraft = () => {
|
||||
const persistRecoverableDraft = () => {
|
||||
// Invitation cancellation still owes the captured draft recovery. Link
|
||||
// preview cancellation retains its existing independent recovery owner.
|
||||
if (sendSignal?.aborted || !draft.recoveryDraftKey) return false;
|
||||
if (
|
||||
sendSignal?.aborted ||
|
||||
!draft.recoveryDraftKey ||
|
||||
draft.sourceOwner.getComposerRevision() !== draft.composerRevision
|
||||
)
|
||||
return false;
|
||||
const existing = drafts.loadDraft(draft.recoveryDraftKey);
|
||||
if (
|
||||
existing &&
|
||||
@@ -401,6 +391,18 @@ export function useMentionSendFlow({
|
||||
);
|
||||
return true;
|
||||
};
|
||||
const persistPreflightDraft = () => {
|
||||
if (
|
||||
!draft.recoveryDraftKey ||
|
||||
isSendCancelled() ||
|
||||
!persistRecoverableDraft()
|
||||
)
|
||||
return;
|
||||
saveQueuedAttachmentsForDraft(
|
||||
draft.recoveryDraftKey,
|
||||
draft.queuedAttachments,
|
||||
);
|
||||
};
|
||||
let composerCleared = false;
|
||||
let optimisticComposerContent = "";
|
||||
let clearedRevision = -1;
|
||||
@@ -409,8 +411,8 @@ export function useMentionSendFlow({
|
||||
composerCleared = false;
|
||||
// An authored edit (even edit -> clear) ends optimistic recovery's
|
||||
// authority over this visit, including its persisted record and files.
|
||||
if (ownsComposer() && getComposerRevision() !== clearedRevision) return;
|
||||
const persisted = persistCanceledDraft();
|
||||
if (draft.sourceOwner.getComposerRevision() !== clearedRevision) return;
|
||||
const persisted = persistRecoverableDraft();
|
||||
const canAnimateCurrentComposer =
|
||||
isMountedRef.current && ownsComposer();
|
||||
if (
|
||||
@@ -635,7 +637,7 @@ export function useMentionSendFlow({
|
||||
}
|
||||
if (
|
||||
draft.sentDraftKey &&
|
||||
(!ownsComposer() || getComposerRevision() === clearedRevision) &&
|
||||
draft.sourceOwner.getComposerRevision() === clearedRevision &&
|
||||
JSON.stringify(
|
||||
drafts.loadDraft(draft.sentDraftKey)?.mentionRefs ?? [],
|
||||
) === JSON.stringify(draft.savedMentionRefs)
|
||||
|
||||
@@ -10,6 +10,7 @@ import type { UseRichTextEditorResult } from "@/features/messages/lib/useRichTex
|
||||
import type { UseDraftsResult } from "@/features/messages/lib/useDrafts";
|
||||
|
||||
export type UseMentionSendFlowOptions = {
|
||||
/** Lifecycle accessor bound to this visit, still readable after it exits. */
|
||||
getComposerRevision: () => number;
|
||||
runComposerUpdate: (update: () => void) => void;
|
||||
channelId: string | null;
|
||||
|
||||
@@ -458,3 +458,104 @@ for (const stage of ["add", "publish"] as const) {
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
for (const incoming of ["unrelated thread B draft", "@RemoteScout hello"]) {
|
||||
test(`B1 authored deletion before thread switch preserves storage and ${incoming}`, async ({
|
||||
page,
|
||||
}) => {
|
||||
await install(page);
|
||||
await expect
|
||||
.poll(() =>
|
||||
page.evaluate(
|
||||
() =>
|
||||
window.__BUZZ_E2E_HAS_MOCK_LIVE_SUBSCRIPTION__?.({
|
||||
channelName: "general",
|
||||
}) ?? false,
|
||||
),
|
||||
)
|
||||
.toBe(true);
|
||||
const roots = await page.evaluate(() =>
|
||||
["Lifecycle thread A", "Lifecycle thread B"].map(
|
||||
(content) =>
|
||||
window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({
|
||||
channelName: "general",
|
||||
content,
|
||||
})?.id,
|
||||
),
|
||||
);
|
||||
expect(roots.every(Boolean)).toBe(true);
|
||||
const navigate = async (id: string | undefined) => {
|
||||
// Drive the real reply route handler, including while the modal has
|
||||
// focus (the equivalent external history/navigation intent).
|
||||
await page
|
||||
.getByTestId(`reply-message-${id}`)
|
||||
.first()
|
||||
.evaluate((button) => (button as HTMLButtonElement).click());
|
||||
await expect(page.getByTestId("message-thread-panel")).toBeVisible();
|
||||
await expect(page.getByTestId("message-thread-head")).toContainText(
|
||||
id === roots[0] ? "Lifecycle thread A" : "Lifecycle thread B",
|
||||
);
|
||||
};
|
||||
const input = page
|
||||
.getByTestId("message-thread-panel")
|
||||
.getByTestId("message-input");
|
||||
// Visit both first: the transition under test must reuse the available
|
||||
// panel/composer, not get a fortuitous unmount via a loading skeleton.
|
||||
await navigate(roots[1]);
|
||||
await input.fill(incoming);
|
||||
await navigate(roots[0]);
|
||||
await expect(input).toHaveText("");
|
||||
await input.fill("@Remote");
|
||||
await page
|
||||
.getByTestId(`mention-suggestion-${REMOTE}`)
|
||||
.locator("button")
|
||||
.first()
|
||||
.click();
|
||||
await page.keyboard.type("hello");
|
||||
await page
|
||||
.getByTestId("message-thread-panel")
|
||||
.getByTestId("send-message")
|
||||
.click();
|
||||
await holdInviteCommand(page, "revalidate_relay_agents", 2);
|
||||
await page.getByRole("button", { name: "Invite", exact: true }).click();
|
||||
await waitForInviteGate(page);
|
||||
await expect(input).toHaveText("");
|
||||
await input.fill("new authored text");
|
||||
await input.fill("");
|
||||
const sourceRecord = () =>
|
||||
page.evaluate(([root, otherRoot]) => {
|
||||
const key = Object.keys(localStorage).find((key) =>
|
||||
key.startsWith("buzz-drafts.v2"),
|
||||
);
|
||||
if (!key) throw new Error("draft storage scope missing");
|
||||
const drafts = JSON.parse(localStorage.getItem(key) ?? "{}");
|
||||
if (!drafts[`thread:${otherRoot}`])
|
||||
throw new Error("control B draft missing");
|
||||
return drafts[`thread:${root}`] ?? null;
|
||||
}, roots);
|
||||
expect(await sourceRecord()).toBeNull();
|
||||
// Expando proves the actual editor DOM host survived A -> B.
|
||||
await input.evaluate((el) =>
|
||||
el.setAttribute("data-lifecycle-host", "retained"),
|
||||
);
|
||||
await navigate(roots[1]);
|
||||
await expect(input).toHaveAttribute("data-lifecycle-host", "retained");
|
||||
await expect(input).toHaveText(incoming);
|
||||
await expect(page.getByRole("alertdialog")).toHaveCount(0);
|
||||
// Read actual persistence, not the editor (whose tombstone could mask a
|
||||
// resurrected record until reload). Neither text nor exact refs may return.
|
||||
expect(await sourceRecord()).toBeNull();
|
||||
await navigate(roots[0]);
|
||||
await expect(input).toHaveText("");
|
||||
|
||||
await navigate(roots[1]);
|
||||
await releaseInviteCommand(page);
|
||||
await page.waitForTimeout(300);
|
||||
expect(await sent(page)).toEqual([]);
|
||||
await expect(input).toHaveText(incoming);
|
||||
await navigate(roots[0]);
|
||||
await expect(input).toHaveText("");
|
||||
expect(await sourceRecord()).toBeNull();
|
||||
await assertNoLocalLifecycle(page);
|
||||
});
|
||||
}
|
||||
|
||||
@@ -52,8 +52,13 @@ visible pending, Escape/retry and route navigation at deferred IPC boundaries.
|
||||
|
||||
Invitation intent belongs to one visit of the effective persistence key and
|
||||
channel, not just the mounted composer or destination channel. Same-channel
|
||||
thread changes and A → B → A invalidate the original visit. The same owner gates
|
||||
optimistic clear, recovery and pending state. Invalidation makes recovery durable
|
||||
thread changes and A → B → A invalidate the original visit. The visible owner gates
|
||||
optimistic clear, editor recovery and pending state. Storage recovery instead
|
||||
consults the captured source visit's authored revision even after that visit
|
||||
exits; losing visible ownership never revokes an authored deletion. The lifecycle
|
||||
retains a per-visit revision record through its captured accessor, so B's edits
|
||||
cannot authorize or suppress A's recovery. Preflight recovery and sent-draft
|
||||
cleanup use that same source authority. Invalidation makes recovery durable
|
||||
synchronously, before a later visit can load or edit the source draft; late async
|
||||
completion cannot revive that recovery or release a newer attempt's latch.
|
||||
|
||||
|
||||
Reference in New Issue
Block a user