mirror of
https://github.com/block/buzz.git
synced 2026-11-01 15:01:12 +00:00
fix(canvas): address canvas history review nits
Gate canvas existence on the persisted revision id, not content truthiness: an empty-string canvas is a valid kind:40100 revision (and restore can republish one), so keying existence, the Create/Edit label, and the History section on content hid retained history for an existing empty canvas. Bound get_canvas_history's page size to the relay read maximum. A request above 1,000 is silently clamped by the relay, which made `events.len() == page_size` false and nulled the cursor even when older revisions remained, stranding them behind an unreachable page. Reject outside 1..=1000. Reset the shared restore mutation on revision selection change so a failed restore's error can no longer render under a different row. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
@@ -119,7 +119,11 @@ pub async fn get_canvas_history(
|
||||
if before_id.is_some() && until.is_none() {
|
||||
return Err("before_id requires until".to_string());
|
||||
}
|
||||
let page_size = limit.unwrap_or(100).max(1);
|
||||
// Bound the page size to the relay's read maximum. Beyond 1,000 the relay
|
||||
// silently clamps the returned rows, which would make `events.len() ==
|
||||
// page_size` false and null the cursor even when older revisions remain,
|
||||
// stranding them behind an unreachable page.
|
||||
let page_size = resolve_history_page_size(limit)?;
|
||||
|
||||
let mut filter = serde_json::json!({
|
||||
"kinds": [40100],
|
||||
@@ -169,3 +173,40 @@ pub async fn get_canvas_history(
|
||||
"next_cursor": next_cursor,
|
||||
}))
|
||||
}
|
||||
|
||||
/// Resolve and validate the history page size against the relay's read
|
||||
/// maximum. Defaults to 100 when unset; a value outside `1..=1000` is rejected
|
||||
/// so cursor generation is never based on a size the relay would silently
|
||||
/// clamp (which strands older revisions behind a falsely-terminated page).
|
||||
fn resolve_history_page_size(limit: Option<usize>) -> Result<usize, String> {
|
||||
let page_size = limit.unwrap_or(100);
|
||||
if !(1..=1000).contains(&page_size) {
|
||||
return Err("limit must be between 1 and 1000".to_string());
|
||||
}
|
||||
Ok(page_size)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::resolve_history_page_size;
|
||||
|
||||
#[test]
|
||||
fn defaults_to_100_when_unset() {
|
||||
assert_eq!(resolve_history_page_size(None).unwrap(), 100);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_zero() {
|
||||
assert!(resolve_history_page_size(Some(0)).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn accepts_relay_maximum() {
|
||||
assert_eq!(resolve_history_page_size(Some(1000)).unwrap(), 1000);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejects_above_relay_maximum() {
|
||||
assert!(resolve_history_page_size(Some(1001)).is_err());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -110,9 +110,13 @@ export function CanvasHistoryPanel({
|
||||
<button
|
||||
aria-expanded={isSelected}
|
||||
className="flex w-full items-baseline justify-between gap-2 px-3 py-2 text-left"
|
||||
onClick={() =>
|
||||
setSelectedId(isSelected ? null : revision.eventId)
|
||||
}
|
||||
onClick={() => {
|
||||
// Clear any prior restore error so it can't render under a
|
||||
// different row once the selection moves — the mutation state
|
||||
// is shared across every row.
|
||||
restoreMutation.reset();
|
||||
setSelectedId(isSelected ? null : revision.eventId);
|
||||
}}
|
||||
type="button"
|
||||
>
|
||||
<span className="truncate text-sm font-medium">
|
||||
|
||||
@@ -51,6 +51,11 @@ export function ChannelCanvas({
|
||||
|
||||
const canvasContent = canvasQuery.data?.content ?? null;
|
||||
const canvasRevision = canvasQuery.data?.eventId ?? null;
|
||||
// A canvas exists whenever a persisted revision is present — an empty-string
|
||||
// revision is a valid kind:40100 write (restore can republish one), so
|
||||
// existence, the Create/Edit label, and History must key off the revision id,
|
||||
// not content truthiness.
|
||||
const canvasExists = canvasRevision !== null;
|
||||
// Defer the single large Markdown parse so opening the canvas commits the
|
||||
// surrounding chrome immediately and the heavy render reconciles after.
|
||||
const deferredCanvasContent = React.useDeferredValue(canvasContent);
|
||||
@@ -144,7 +149,7 @@ export function ChannelCanvas({
|
||||
|
||||
return (
|
||||
<div className="space-y-3">
|
||||
{canvasContent ? (
|
||||
{canvasExists ? (
|
||||
<div
|
||||
className="rounded-2xl border border-border/70 bg-muted/20 px-4 py-3"
|
||||
data-testid="channel-canvas-content"
|
||||
@@ -168,10 +173,10 @@ export function ChannelCanvas({
|
||||
variant="outline"
|
||||
>
|
||||
<Pencil className="h-4 w-4" />
|
||||
{canvasContent ? "Edit canvas" : "Create canvas"}
|
||||
{canvasExists ? "Edit canvas" : "Create canvas"}
|
||||
</Button>
|
||||
) : null}
|
||||
{canvasContent ? (
|
||||
{canvasExists ? (
|
||||
<>
|
||||
<Button
|
||||
aria-expanded={showHistory}
|
||||
@@ -188,7 +193,7 @@ export function ChannelCanvas({
|
||||
<CanvasHistoryPanel
|
||||
canRestore={canEdit && !isArchived}
|
||||
channelId={channelId}
|
||||
currentContent={canvasContent}
|
||||
currentContent={canvasContent ?? ""}
|
||||
currentRevision={canvasRevision}
|
||||
/>
|
||||
) : null}
|
||||
|
||||
@@ -0,0 +1,158 @@
|
||||
/**
|
||||
* Empty-canvas existence regression: an empty-string canvas is still a valid
|
||||
* persisted kind:40100 revision (restore can republish one). ChannelCanvas must
|
||||
* key existence — the rendered content block, the Edit-vs-Create label, and the
|
||||
* History toggle — off the presence of a revision id, not content truthiness.
|
||||
*
|
||||
* Mounts the shipping ChannelCanvas with a canvas whose content is "" but whose
|
||||
* event id is non-null, then asserts the action reads "Edit" and History stays
|
||||
* available.
|
||||
*/
|
||||
|
||||
import assert from "node:assert/strict";
|
||||
import { registerHooks } from "node:module";
|
||||
import { after, before, test } from "node:test";
|
||||
|
||||
import { JSDOM } from "jsdom";
|
||||
|
||||
// The real Markdown component pulls in the remark/rehype/emoji stack, which
|
||||
// never releases its jsdom handles and hangs the node:test process. This test
|
||||
// only exercises existence gating, so serve an inert stub.
|
||||
registerHooks({
|
||||
resolve(specifier, context, nextResolve) {
|
||||
if (specifier === "@/shared/ui/markdown") {
|
||||
return { shortCircuit: true, url: "buzz-canvas-stub:markdown" };
|
||||
}
|
||||
return nextResolve(specifier, context);
|
||||
},
|
||||
load(url, context, nextLoad) {
|
||||
if (url === "buzz-canvas-stub:markdown") {
|
||||
return {
|
||||
format: "module",
|
||||
shortCircuit: true,
|
||||
source: "export function Markdown() { return null; }\n",
|
||||
};
|
||||
}
|
||||
return nextLoad(url, context);
|
||||
},
|
||||
});
|
||||
|
||||
const dom = new JSDOM("<!doctype html><html><body></body></html>", {
|
||||
url: "http://localhost",
|
||||
});
|
||||
|
||||
const HEAD = "a".repeat(64);
|
||||
|
||||
let React;
|
||||
let act;
|
||||
let createRoot;
|
||||
let QueryClient;
|
||||
let QueryClientProvider;
|
||||
let ChannelNavigationProvider;
|
||||
let ChannelCanvas;
|
||||
|
||||
before(async () => {
|
||||
Object.assign(globalThis, {
|
||||
document: dom.window.document,
|
||||
HTMLElement: dom.window.HTMLElement,
|
||||
window: dom.window,
|
||||
IS_REACT_ACT_ENVIRONMENT: true,
|
||||
});
|
||||
Object.defineProperty(globalThis, "navigator", {
|
||||
configurable: true,
|
||||
value: dom.window.navigator,
|
||||
writable: true,
|
||||
});
|
||||
dom.window.matchMedia = () => ({
|
||||
matches: false,
|
||||
addEventListener() {},
|
||||
removeEventListener() {},
|
||||
});
|
||||
|
||||
// A persisted-but-empty canvas: content is "" while the head event id is set.
|
||||
dom.window.__TAURI_INTERNALS__ = {
|
||||
invoke: async (cmd) => {
|
||||
if (cmd === "get_canvas") {
|
||||
return { content: "", event_id: HEAD, updated_at: 1, author: HEAD };
|
||||
}
|
||||
if (cmd === "get_canvas_history") {
|
||||
return { revisions: [], next_cursor: null };
|
||||
}
|
||||
throw new Error(`unexpected command: ${cmd}`);
|
||||
},
|
||||
};
|
||||
|
||||
({ default: React, act } = await import("react"));
|
||||
({ createRoot } = await import("react-dom/client"));
|
||||
({ QueryClient, QueryClientProvider } = await import(
|
||||
"@tanstack/react-query"
|
||||
));
|
||||
({ ChannelNavigationProvider } = await import(
|
||||
"@/shared/context/ChannelNavigationContext"
|
||||
));
|
||||
({ ChannelCanvas } = await import("./ChannelCanvas.tsx"));
|
||||
});
|
||||
|
||||
after(() => dom.window.close());
|
||||
|
||||
async function settle(iterations = 6) {
|
||||
for (let i = 0; i < iterations; i++) {
|
||||
await act(async () => {
|
||||
await new Promise((resolve) => setTimeout(resolve, 1));
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
test("empty-string canvas with a revision id still exists — Edit label and History remain", async () => {
|
||||
const queryClient = new QueryClient({
|
||||
defaultOptions: {
|
||||
queries: { gcTime: Number.POSITIVE_INFINITY, retry: false },
|
||||
mutations: { gcTime: 0 },
|
||||
},
|
||||
});
|
||||
const container = dom.window.document.createElement("div");
|
||||
dom.window.document.body.appendChild(container);
|
||||
const root = createRoot(container);
|
||||
|
||||
await act(async () => {
|
||||
root.render(
|
||||
React.createElement(
|
||||
QueryClientProvider,
|
||||
{ client: queryClient },
|
||||
React.createElement(
|
||||
ChannelNavigationProvider,
|
||||
{ channels: [] },
|
||||
React.createElement(ChannelCanvas, {
|
||||
channelId: "channel-1",
|
||||
canEdit: true,
|
||||
isArchived: false,
|
||||
}),
|
||||
),
|
||||
),
|
||||
);
|
||||
});
|
||||
|
||||
await act(async () => {
|
||||
await queryClient.refetchQueries({ queryKey: ["channel-canvas"] });
|
||||
});
|
||||
await settle();
|
||||
|
||||
const editButton = container.querySelector(
|
||||
"[data-testid='channel-canvas-edit']",
|
||||
);
|
||||
assert.ok(editButton, "edit button renders for an existing empty canvas");
|
||||
assert.equal(
|
||||
editButton.textContent.trim(),
|
||||
"Edit canvas",
|
||||
"an existing empty canvas labels the action Edit, not Create",
|
||||
);
|
||||
assert.ok(
|
||||
container.querySelector("[data-testid='channel-canvas-history-toggle']"),
|
||||
"History remains available for an existing empty canvas",
|
||||
);
|
||||
|
||||
await settle(12);
|
||||
await act(async () => root.unmount());
|
||||
queryClient.clear();
|
||||
container.remove();
|
||||
});
|
||||
Reference in New Issue
Block a user