mirror of
https://github.com/block/buzz.git
synced 2026-10-30 22:11:36 +00:00
fix(desktop): guard message navigation at boundary
Co-authored-by: Rizz <302abe414ca6e3134763d2539bfcf145aea2a63fe5f8455204ed602fd40cf381@buzz.block.builderlab.xyz> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
This commit is contained in:
@@ -0,0 +1,28 @@
|
||||
import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
|
||||
const { allowMessageTargetNavigation, registerMessageTargetNavigationGuard } =
|
||||
await import("./messageTargetNavigationGuard.ts");
|
||||
|
||||
test("all message-target navigation consults the registered boundary guard", () => {
|
||||
let calls = 0;
|
||||
const unregister = registerMessageTargetNavigationGuard(() => {
|
||||
calls += 1;
|
||||
return false;
|
||||
});
|
||||
|
||||
assert.equal(allowMessageTargetNavigation(), false);
|
||||
assert.equal(calls, 1);
|
||||
unregister();
|
||||
assert.equal(allowMessageTargetNavigation(), true);
|
||||
});
|
||||
|
||||
test("stale cleanup cannot unregister a newer guard", () => {
|
||||
const unregisterFirst = registerMessageTargetNavigationGuard(() => false);
|
||||
const unregisterSecond = registerMessageTargetNavigationGuard(() => true);
|
||||
|
||||
unregisterFirst();
|
||||
assert.equal(allowMessageTargetNavigation(), true);
|
||||
unregisterSecond();
|
||||
assert.equal(allowMessageTargetNavigation(), true);
|
||||
});
|
||||
16
desktop/src/app/navigation/messageTargetNavigationGuard.ts
Normal file
16
desktop/src/app/navigation/messageTargetNavigationGuard.ts
Normal file
@@ -0,0 +1,16 @@
|
||||
type MessageTargetNavigationGuard = () => boolean;
|
||||
|
||||
let activeGuard: MessageTargetNavigationGuard | null = null;
|
||||
|
||||
export function allowMessageTargetNavigation(): boolean {
|
||||
return activeGuard?.() ?? true;
|
||||
}
|
||||
|
||||
export function registerMessageTargetNavigationGuard(
|
||||
guard: MessageTargetNavigationGuard,
|
||||
): () => void {
|
||||
activeGuard = guard;
|
||||
return () => {
|
||||
if (activeGuard === guard) activeGuard = null;
|
||||
};
|
||||
}
|
||||
@@ -7,6 +7,7 @@ import {
|
||||
} from "@tanstack/react-router";
|
||||
|
||||
import { openSearchHitWithNavigation } from "@/app/navigation/searchHitNavigation";
|
||||
import { allowMessageTargetNavigation } from "@/app/navigation/messageTargetNavigationGuard";
|
||||
import type { SearchHit } from "@/shared/api/types";
|
||||
|
||||
type NavigationBehavior = {
|
||||
@@ -256,8 +257,11 @@ export function useAppNavigation() {
|
||||
thread?: string;
|
||||
threadRootId?: string | null;
|
||||
},
|
||||
) =>
|
||||
commitNavigation(
|
||||
) => {
|
||||
if (options?.messageId && !allowMessageTargetNavigation()) {
|
||||
return Promise.resolve(false);
|
||||
}
|
||||
return commitNavigation(
|
||||
{
|
||||
to: "/channels/$channelId",
|
||||
params: {
|
||||
@@ -282,7 +286,8 @@ export function useAppNavigation() {
|
||||
replace: options?.replace,
|
||||
resetScroll: options?.messageId ? true : undefined,
|
||||
},
|
||||
),
|
||||
);
|
||||
},
|
||||
[commitNavigation],
|
||||
);
|
||||
|
||||
|
||||
@@ -661,9 +661,7 @@ export function ChannelScreen({
|
||||
setThreadReplyTargetId(null);
|
||||
setEditTargetId(null);
|
||||
}, [activeChannelId]);
|
||||
const allowMessageLinkNavigation = useMessageLinkNavigationGuard(
|
||||
requireThreadEditResolution,
|
||||
);
|
||||
useMessageLinkNavigationGuard(requireThreadEditResolution);
|
||||
const mainTimelineTargetMessageId = useChannelRouteTarget({
|
||||
activeChannel,
|
||||
activeChannelId,
|
||||
@@ -859,7 +857,6 @@ export function ChannelScreen({
|
||||
}
|
||||
>
|
||||
<GuardedChannelPane
|
||||
messageLinkNavigationGuard={allowMessageLinkNavigation}
|
||||
activeChannel={activeChannel}
|
||||
activityAgents={channelAgentSessionAgents}
|
||||
agentPubkeys={agentPubkeys}
|
||||
|
||||
@@ -1,21 +1,9 @@
|
||||
import type * as React from "react";
|
||||
|
||||
import { MessageLinkNavigationGuardProvider } from "@/shared/ui/markdown/messageLinkNavigationGuardContext";
|
||||
import { ChannelPane } from "./ChannelScreenLazyViews";
|
||||
|
||||
type GuardedChannelPaneProps = React.ComponentProps<typeof ChannelPane> & {
|
||||
messageLinkNavigationGuard: React.ComponentProps<
|
||||
typeof MessageLinkNavigationGuardProvider
|
||||
>["value"];
|
||||
};
|
||||
|
||||
export function GuardedChannelPane({
|
||||
messageLinkNavigationGuard,
|
||||
...props
|
||||
}: GuardedChannelPaneProps) {
|
||||
return (
|
||||
<MessageLinkNavigationGuardProvider value={messageLinkNavigationGuard}>
|
||||
<ChannelPane {...props} />
|
||||
</MessageLinkNavigationGuardProvider>
|
||||
);
|
||||
export function GuardedChannelPane(
|
||||
props: React.ComponentProps<typeof ChannelPane>,
|
||||
) {
|
||||
return <ChannelPane {...props} />;
|
||||
}
|
||||
|
||||
@@ -1,12 +1,12 @@
|
||||
import * as React from "react";
|
||||
|
||||
import type { ParsedMessageLink } from "@/features/messages/lib/messageLink";
|
||||
import { registerMessageTargetNavigationGuard } from "@/app/navigation/messageTargetNavigationGuard";
|
||||
|
||||
export function useMessageLinkNavigationGuard(
|
||||
requireThreadEditResolution: () => boolean,
|
||||
) {
|
||||
return React.useCallback(
|
||||
(_link: ParsedMessageLink) => requireThreadEditResolution(),
|
||||
React.useLayoutEffect(
|
||||
() => registerMessageTargetNavigationGuard(requireThreadEditResolution),
|
||||
[requireThreadEditResolution],
|
||||
);
|
||||
}
|
||||
|
||||
@@ -5,7 +5,6 @@ import { getSentFromThreadReference } from "@/features/messages/lib/sentFromThre
|
||||
import { useChannelNavigation } from "@/shared/context/ChannelNavigationContext";
|
||||
import type { ParsedMessageLink } from "@/features/messages/lib/messageLink";
|
||||
import { MessageLinkPill } from "@/shared/ui/markdown/MessageLinkPill";
|
||||
import { useMessageLinkNavigationGuard } from "@/shared/ui/markdown/messageLinkNavigationGuardContext";
|
||||
import { MESSAGE_MARKDOWN_CLASS } from "@/shared/ui/mentionChip";
|
||||
|
||||
export function SentFromThreadLine({
|
||||
@@ -17,17 +16,15 @@ export function SentFromThreadLine({
|
||||
}) {
|
||||
const { channels } = useChannelNavigation();
|
||||
const { goChannel } = useAppNavigation();
|
||||
const allowMessageLinkNavigation = useMessageLinkNavigationGuard();
|
||||
const reference = getSentFromThreadReference(tags);
|
||||
const onOpenMessageLink = React.useCallback(
|
||||
(target: ParsedMessageLink) => {
|
||||
if (!allowMessageLinkNavigation(target)) return;
|
||||
void goChannel(target.channelId, {
|
||||
messageId: target.messageId,
|
||||
threadRootId: target.threadRootId,
|
||||
});
|
||||
},
|
||||
[allowMessageLinkNavigation, goChannel],
|
||||
[goChannel],
|
||||
);
|
||||
|
||||
if (!channelId || !reference) return null;
|
||||
|
||||
@@ -118,7 +118,6 @@ import { ProgressiveImage } from "./markdown/ProgressiveImage";
|
||||
import { MessageLinkPill } from "./markdown/MessageLinkPill";
|
||||
import { renderCachedMarkdown } from "./markdown/nodeCache";
|
||||
import { useMessageLinkPreviews } from "./markdown/useMessageLinkPreviews";
|
||||
import { useMessageLinkNavigationGuard } from "./markdown/messageLinkNavigationGuardContext";
|
||||
import {
|
||||
MarkdownRuntimeContext,
|
||||
useMarkdownRuntime,
|
||||
@@ -1744,7 +1743,6 @@ function MarkdownInner({
|
||||
const { channels: rawChannels } = useChannelNavigation();
|
||||
const channels = useStableArray(rawChannels);
|
||||
const { goChannel, goAgents } = useAppNavigation();
|
||||
const allowMessageLinkNavigation = useMessageLinkNavigationGuard();
|
||||
const onOpenChannel = React.useCallback(
|
||||
(channelId: string) => {
|
||||
void goChannel(channelId);
|
||||
@@ -1754,16 +1752,15 @@ function MarkdownInner({
|
||||
const onOpenEntityLink = useOpenEntityLink();
|
||||
const onOpenMessageLink = React.useCallback(
|
||||
(link: ParsedMessageLink) => {
|
||||
if (!allowMessageLinkNavigation(link)) return; // Guard before URL mutation.
|
||||
// Always route through `goChannel` with `messageId` set: the channel
|
||||
// route already handles scroll-into-view + highlight via
|
||||
// Always route through `goChannel` with `messageId` set: the navigation
|
||||
// boundary guards every message-targeting caller before URL mutation.
|
||||
// `useAnchoredScroll` + `getEventById` backfill, and works for
|
||||
void goChannel(link.channelId, {
|
||||
messageId: link.messageId,
|
||||
threadRootId: link.threadRootId,
|
||||
});
|
||||
},
|
||||
[allowMessageLinkNavigation, goChannel],
|
||||
[goChannel],
|
||||
);
|
||||
const relayOrigin = useRelayOrigin();
|
||||
const resolvedLinkPreviews = useMessageLinkPreviews({
|
||||
|
||||
@@ -1,14 +0,0 @@
|
||||
import * as React from "react";
|
||||
|
||||
import type { ParsedMessageLink } from "@/features/messages/lib/messageLink";
|
||||
|
||||
export const MessageLinkNavigationGuardContext = React.createContext<
|
||||
(link: ParsedMessageLink) => boolean
|
||||
>(() => true);
|
||||
|
||||
export const MessageLinkNavigationGuardProvider =
|
||||
MessageLinkNavigationGuardContext.Provider;
|
||||
|
||||
export function useMessageLinkNavigationGuard() {
|
||||
return React.useContext(MessageLinkNavigationGuardContext);
|
||||
}
|
||||
@@ -3756,6 +3756,84 @@ test("a refused sent-from-thread link preserves the edit and retries after cance
|
||||
await expect(page).toHaveURL(new RegExp(`thread=${destinationRootId}`));
|
||||
});
|
||||
|
||||
test("a refused search result preserves the edit and retries after cancel", async ({
|
||||
page,
|
||||
}) => {
|
||||
const sourceRoot = `Search guard source ${Date.now()}`;
|
||||
const sourceReply = `Search guard reply ${Date.now()}`;
|
||||
const destinationRoot = `Search guard destination ${Date.now()}`;
|
||||
const dirtyReply = `${sourceReply} unsaved byte-for-byte`;
|
||||
|
||||
await page.goto("/");
|
||||
await page.waitForFunction(
|
||||
() => typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function",
|
||||
);
|
||||
const { destinationRootId, sourceRootId } = await page.evaluate(
|
||||
({ destinationRoot, sourceReply, sourceRoot }) => {
|
||||
const emit = window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__;
|
||||
if (!emit) throw new Error("Mock message emitter is unavailable.");
|
||||
const destination = emit({
|
||||
channelName: "general",
|
||||
content: destinationRoot,
|
||||
});
|
||||
const source = emit({ channelName: "general", content: sourceRoot });
|
||||
emit({
|
||||
channelName: "general",
|
||||
content: sourceReply,
|
||||
parentEventId: source.id,
|
||||
});
|
||||
return { destinationRootId: destination.id, sourceRootId: source.id };
|
||||
},
|
||||
{ destinationRoot, sourceReply, sourceRoot },
|
||||
);
|
||||
|
||||
await page.getByTestId("channel-general").click();
|
||||
const timeline = page.getByTestId("message-timeline");
|
||||
const source = timeline.locator(`[data-message-id="${sourceRootId}"]`);
|
||||
await source.hover();
|
||||
await source.getByRole("button", { name: "Reply" }).click({ force: true });
|
||||
const threadPanel = page.getByTestId("message-thread-panel");
|
||||
const threadInput = threadPanel.getByTestId("message-input");
|
||||
const reply = threadPanel
|
||||
.getByTestId("message-row")
|
||||
.filter({ hasText: sourceReply })
|
||||
.last();
|
||||
await reply.hover();
|
||||
await reply.getByRole("button", { name: "More actions" }).click();
|
||||
await page.getByRole("menuitem", { name: "Edit message" }).click();
|
||||
await threadInput.fill(dirtyReply);
|
||||
|
||||
const threadUrl = page.url();
|
||||
expect(threadUrl).toContain(`thread=${sourceRootId}`);
|
||||
await page.getByTestId("open-search").click();
|
||||
await page.getByTestId("search-dialog-input").fill(destinationRoot);
|
||||
const destinationResult = page.getByTestId(
|
||||
`search-result-${destinationRootId}`,
|
||||
);
|
||||
await expect(destinationResult).toBeVisible();
|
||||
await destinationResult.click();
|
||||
|
||||
const refusal = page.getByText(
|
||||
"Finish or cancel your edit before leaving the thread.",
|
||||
);
|
||||
await expect(refusal).toHaveCount(1);
|
||||
await expect(threadPanel).toBeVisible();
|
||||
await expect(threadPanel.getByTestId("edit-target")).toBeVisible();
|
||||
await expect(threadInput).toHaveText(dirtyReply);
|
||||
await expect(page).toHaveURL(threadUrl);
|
||||
|
||||
await threadInput.press("Escape");
|
||||
await expect(threadPanel.getByTestId("edit-target")).toHaveCount(0);
|
||||
await page.getByTestId("open-search").click();
|
||||
await page.getByTestId("search-dialog-input").fill(destinationRoot);
|
||||
await destinationResult.click();
|
||||
await expect(page).not.toHaveURL(threadUrl);
|
||||
await expect(threadPanel.getByTestId("message-thread-head")).toContainText(
|
||||
destinationRoot,
|
||||
);
|
||||
await expect(page).toHaveURL(new RegExp(`thread=${destinationRootId}`));
|
||||
});
|
||||
|
||||
test("ArrowUp in an empty composer edits your last message right after sending", async ({
|
||||
page,
|
||||
}) => {
|
||||
|
||||
Reference in New Issue
Block a user