From 12ff76fb89e4a2e4a0c6b40c3529601db9851f76 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Sat, 27 Jun 2026 00:29:19 +0300 Subject: [PATCH 1/2] fix(temporary-notes): live sidebar clock marker + stacked mobile create buttons MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Issue 1 — the sidebar tree's temporary-note clock marker did not appear/ disappear until a page reload when a note's temporary state changed. - Make/unmake permanent from the page header menu and the in-page banner went through syncTemporaryExpiresInCache(), which patched the page query cache but never touched treeDataAtom, so the sidebar node kept its stale temporaryExpiresAt. Patch the tree node there too (via jotai's default store), so the marker updates without a reload. - Creating a note as temporary showed no marker until reload: the create flow's cache write (invalidateOnCreatePage) omitted temporaryExpiresAt, so the tree rebuild (buildTree -> mergeRootTrees) overwrote the optimistic/socket node's marker with undefined. Carry temporaryExpiresAt in that cached entry. - Thread temporaryExpiresAt through the server addTreeNode broadcast (PAGE_CREATED snapshot -> TreeNodeSnapshot -> broadcastPageCreated) so OTHER clients watching the space also render the marker immediately, and harden handleCreate's idempotency guard to patch the deadline if the broadcast won the insert race. Issue 2 — the home and space-overview "New note" / "New temporary note" buttons sat side-by-side and the temporary label clipped on narrow mobile widths. Lay them out full-width, stacked vertically, and tint the temporary button orange (matching the clock marker + banner) while the regular one stays neutral gray. Tests: extend tree-socket-reducers.test.ts (addTreeNode carries temporaryExpiresAt). Verified live with Playwright: marker appears on create and toggles both ways with no reload; mobile buttons are stacked, full-width, unclipped, and differently colored. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../home/components/new-note-button.tsx | 24 +++++++++++++------ .../page-embed/queries/page-embed-query.ts | 17 +++++++++++++ .../components/header/page-header-menu.tsx | 4 ++-- .../src/features/page/queries/page-query.ts | 5 ++++ .../page/tree/hooks/use-tree-mutation.ts | 17 ++++++++++++- .../components/space-create-note-buttons.tsx | 13 ++++++---- .../websocket/tree-socket-reducers.test.ts | 14 +++++++++++ .../src/database/listeners/page.listener.ts | 5 ++++ .../src/database/repos/page/page.repo.ts | 3 +++ apps/server/src/ws/ws-tree.service.ts | 4 ++++ 10 files changed, 92 insertions(+), 14 deletions(-) diff --git a/apps/client/src/features/home/components/new-note-button.tsx b/apps/client/src/features/home/components/new-note-button.tsx index 69bcac37..43649bc6 100644 --- a/apps/client/src/features/home/components/new-note-button.tsx +++ b/apps/client/src/features/home/components/new-note-button.tsx @@ -1,4 +1,4 @@ -import { Button, Group, Menu, Text } from "@mantine/core"; +import { Button, Menu, Stack, Text } from "@mantine/core"; import { IconHourglass, IconPlus } from "@tabler/icons-react"; import { ReactNode } from "react"; import { useNavigate } from "react-router-dom"; @@ -21,11 +21,15 @@ function CreateNoteButton({ temporary, label, icon, + color, }: { writableSpaces: ISpace[]; temporary: boolean; label: string; icon: ReactNode; + // Mantine color token; lets the temporary action tint toward the warm + // orange/amber used by the clock marker + banner while "New note" stays neutral. + color: string; }) { const { t } = useTranslation(); const navigate = useNavigate(); @@ -54,7 +58,8 @@ function CreateNoteButton({ - + ); } diff --git a/apps/client/src/features/websocket/tree-socket-reducers.test.ts b/apps/client/src/features/websocket/tree-socket-reducers.test.ts index 20abdf95..81c62b56 100644 --- a/apps/client/src/features/websocket/tree-socket-reducers.test.ts +++ b/apps/client/src/features/websocket/tree-socket-reducers.test.ts @@ -323,4 +323,18 @@ describe("applyAddTreeNode", () => { "child", ]); }); + + it("carries temporaryExpiresAt onto the inserted node so the clock marker shows on create (no reload)", () => { + // A note created as temporary broadcasts addTreeNode with the death-timer + // deadline in its payload; the receiver's inserted node must keep it so + // space-tree-row renders the orange clock marker immediately. + const tree = roots(); + const expiresAt = "2026-06-27T21:00:00.000Z"; + const next = applyAddTreeNode(tree, { + parentId: null as unknown as string, + index: 0, + data: node("temp", { position: "a3", temporaryExpiresAt: expiresAt }), + }); + expect(treeModel.find(next, "temp")?.temporaryExpiresAt).toBe(expiresAt); + }); }); diff --git a/apps/server/src/database/listeners/page.listener.ts b/apps/server/src/database/listeners/page.listener.ts index 3a779aa3..52b9f963 100644 --- a/apps/server/src/database/listeners/page.listener.ts +++ b/apps/server/src/database/listeners/page.listener.ts @@ -21,6 +21,11 @@ export interface TreeNodeSnapshot { position: string; spaceId: string; parentPageId: string | null; + // Death-timer deadline carried so the `addTreeNode` broadcast shows the + // temporary-note clock marker immediately on every client (incl. the author, + // whose optimistic insert can lose the race to this broadcast). null/absent => + // permanent. + temporaryExpiresAt?: Date | string | null; } export class PageEvent { diff --git a/apps/server/src/database/repos/page/page.repo.ts b/apps/server/src/database/repos/page/page.repo.ts index 45cb57ab..1b711750 100644 --- a/apps/server/src/database/repos/page/page.repo.ts +++ b/apps/server/src/database/repos/page/page.repo.ts @@ -209,6 +209,9 @@ export class PageRepo { position: result.position, spaceId: result.spaceId, parentPageId: result.parentPageId, + // Carry the death-timer deadline so a note created as temporary shows + // its sidebar clock marker on every client without a reload. + temporaryExpiresAt: result.temporaryExpiresAt, }, ], }); diff --git a/apps/server/src/ws/ws-tree.service.ts b/apps/server/src/ws/ws-tree.service.ts index 223fb25c..15b4dd87 100644 --- a/apps/server/src/ws/ws-tree.service.ts +++ b/apps/server/src/ws/ws-tree.service.ts @@ -38,6 +38,10 @@ export class WsTreeService { spaceId: page.spaceId, parentPageId: page.parentPageId, hasChildren: false, + // Carry the death-timer deadline so receivers (and the author, if this + // broadcast wins the race against the optimistic insert) render the + // temporary-note clock marker immediately. null => permanent. + temporaryExpiresAt: page.temporaryExpiresAt ?? null, children: [], }, }, -- 2.49.1 From d181b5c4ffd61bcbf4395b52f46c092671832519 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Sat, 27 Jun 2026 00:58:40 +0300 Subject: [PATCH 2/2] test(temporary-notes): cover the create race-guard, broadcast deadline + cache patch; unify page->tree-node mappers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review comment 2159 on the temporary-notes UI work. Tests: - tree-model: cover handleCreate's race-guard temporaryExpiresAt patch — (a) server node inserted WITHOUT a deadline + create response carries one => node gains the deadline; (b) node already has a deadline => not overwritten, prev returned by reference. - ws-tree.service.spec: broadcastPageCreated now asserts the deadline is carried when present and pinned to null (`?? null`) when absent. - page-embed-query (new spec): syncTemporaryExpiresInCache patches the in-tree node's temporaryExpiresAt, and leaves the atom value at the same reference when the id is absent from the loaded tree (no write). Refactor (closes the drift bug-class at the root): - Client: extract one canonical pageToTreeNode(page, overrides) mapper in tree/utils and route buildTree, handleCreate's optimistic insert, the restore mutation and the duplicate handler through it. Restore stays permanent (server nulls temporaryExpiresAt) and duplicate stays permanent (server arms no timer) — both now reflect the server without a reload, where before they dropped the field entirely. - Server: extract one toTreeNodeSnapshot(page) helper called by both the PAGE_CREATED event enrichment (page.repo) and the addTreeNode broadcast (ws-tree.service), so the optional temporaryExpiresAt can't drift between the two literals. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../queries/page-embed-query.test.ts | 69 ++++++++++++++ .../src/features/page/queries/page-query.ts | 19 ++-- .../tree/components/space-tree-node-menu.tsx | 19 ++-- .../page/tree/hooks/use-tree-mutation.ts | 18 ++-- .../page/tree/model/tree-model.test.ts | 95 +++++++++++++++++++ .../src/features/page/tree/utils/utils.ts | 47 ++++++--- .../src/database/listeners/page.listener.ts | 30 ++++++ .../src/database/repos/page/page.repo.ts | 23 ++--- apps/server/src/ws/ws-tree.service.spec.ts | 21 ++++ apps/server/src/ws/ws-tree.service.ts | 20 ++-- 10 files changed, 289 insertions(+), 72 deletions(-) create mode 100644 apps/client/src/features/page-embed/queries/page-embed-query.test.ts diff --git a/apps/client/src/features/page-embed/queries/page-embed-query.test.ts b/apps/client/src/features/page-embed/queries/page-embed-query.test.ts new file mode 100644 index 00000000..70336c5c --- /dev/null +++ b/apps/client/src/features/page-embed/queries/page-embed-query.test.ts @@ -0,0 +1,69 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { getDefaultStore } from "jotai"; + +// Mock the app entry so importing the query module doesn't boot the whole app +// (it only needs queryClient's cache methods, which we stub here). The spies are +// declared via vi.hoisted so they exist before the hoisted vi.mock factory runs. +const { setQueryData, getQueryData, invalidateQueries } = vi.hoisted(() => ({ + setQueryData: vi.fn(), + getQueryData: vi.fn(() => undefined as unknown), + invalidateQueries: vi.fn(), +})); +vi.mock("@/main.tsx", () => ({ + queryClient: { setQueryData, getQueryData, invalidateQueries }, +})); + +import { syncTemporaryExpiresInCache } from "./page-embed-query"; +import { treeDataAtom } from "@/features/page/tree/atoms/tree-data-atom.ts"; +import { SpaceTreeNode } from "@/features/page/tree/types.ts"; + +const mkNode = (id: string, slugId: string): SpaceTreeNode => + ({ + id, + slugId, + name: id, + position: "a0", + spaceId: "space-1", + parentPageId: null, + hasChildren: false, + children: [], + }) as unknown as SpaceTreeNode; + +describe("syncTemporaryExpiresInCache — treeDataAtom patch", () => { + beforeEach(() => { + vi.clearAllMocks(); + getQueryData.mockReturnValue(undefined); + }); + + it("patches the in-tree node's temporaryExpiresAt (sidebar marker updates without reload)", () => { + const store = getDefaultStore(); + const tree = [mkNode("p1", "slug-1"), mkNode("p2", "slug-2")]; + store.set(treeDataAtom, tree); + + const deadline = "2026-07-01T00:00:00.000Z"; + syncTemporaryExpiresInCache({ id: "p1", slugId: "slug-1" }, deadline); + + const next = store.get(treeDataAtom); + // A new atom value was written... + expect(next).not.toBe(tree); + // ...the matching node gained the deadline... + expect(next.find((n) => n.id === "p1")?.temporaryExpiresAt).toBe(deadline); + // ...and the untouched sibling is unchanged. + expect(next.find((n) => n.id === "p2")?.temporaryExpiresAt).toBeUndefined(); + }); + + it("leaves the atom value at the SAME reference when the id is absent from the tree (no write)", () => { + const store = getDefaultStore(); + const tree = [mkNode("p1", "slug-1")]; + store.set(treeDataAtom, tree); + + syncTemporaryExpiresInCache( + { id: "not-in-tree", slugId: "missing" }, + "2026-07-01T00:00:00.000Z", + ); + + // treeModel.update is a no-op (same reference) for an unknown id, so the + // guard skips the store write entirely — same reference back. + expect(store.get(treeDataAtom)).toBe(tree); + }); +}); diff --git a/apps/client/src/features/page/queries/page-query.ts b/apps/client/src/features/page/queries/page-query.ts index 1b1355ad..7837f8db 100644 --- a/apps/client/src/features/page/queries/page-query.ts +++ b/apps/client/src/features/page/queries/page-query.ts @@ -32,7 +32,7 @@ import { import { notifications } from "@mantine/notifications"; import { IPagination, QueryParams } from "@/lib/types.ts"; import { queryClient } from "@/main.tsx"; -import { buildTree } from "@/features/page/tree/utils"; +import { buildTree, pageToTreeNode } from "@/features/page/tree/utils"; import { useEffect } from "react"; import { validate as isValidUuid } from "uuid"; import { useTranslation } from "react-i18next"; @@ -210,18 +210,15 @@ export function useRestorePageMutation() { // Check if the page already exists in the tree (it shouldn't) if (!treeModel.find(currentTree, restoredPage.id)) { - // Create the tree node data with hasChildren from backend - const nodeData: SpaceTreeNode = { - id: restoredPage.id, - slugId: restoredPage.slugId, + // Create the tree node data with hasChildren from backend. Routed + // through the canonical mapper so the field copy stays in lockstep with + // buildTree. The server NULLS `temporaryExpiresAt` on restore (a restored + // page is made permanent), so the mapper carries that null through and + // the node correctly shows no clock marker. + const nodeData: SpaceTreeNode = pageToTreeNode(restoredPage, { name: restoredPage.title || "Untitled", - icon: restoredPage.icon, - position: restoredPage.position, - spaceId: restoredPage.spaceId, - parentPageId: restoredPage.parentPageId, hasChildren: restoredPage.hasChildren || false, - children: [], - }; + }); // Determine the parent and index const parentId = restoredPage.parentPageId || null; diff --git a/apps/client/src/features/page/tree/components/space-tree-node-menu.tsx b/apps/client/src/features/page/tree/components/space-tree-node-menu.tsx index cd868746..63ddd98e 100644 --- a/apps/client/src/features/page/tree/components/space-tree-node-menu.tsx +++ b/apps/client/src/features/page/tree/components/space-tree-node-menu.tsx @@ -37,6 +37,7 @@ import { } from "@/features/page-embed/queries/page-embed-query"; import { treeDataAtom } from "@/features/page/tree/atoms/tree-data-atom.ts"; import { treeModel } from "@/features/page/tree/model/tree-model"; +import { pageToTreeNode } from "@/features/page/tree/utils"; import { useTreeMutation } from "@/features/page/tree/hooks/use-tree-mutation.ts"; import type { SpaceTreeNode } from "@/features/page/tree/types.ts"; import classes from "@/features/page/tree/styles/tree.module.css"; @@ -130,18 +131,14 @@ export function NodeMenu({ node, canEdit }: NodeMenuProps) { const currentIndex = siblings?.index ?? 0; const newIndex = currentIndex + 1; - const treeNodeData: SpaceTreeNode = { - id: duplicatedPage.id, - slugId: duplicatedPage.slugId, - name: duplicatedPage.title, - position: duplicatedPage.position, - spaceId: duplicatedPage.spaceId, - parentPageId: duplicatedPage.parentPageId, - icon: duplicatedPage.icon, - hasChildren: duplicatedPage.hasChildren, + // Routed through the canonical mapper so the field copy stays in lockstep + // with buildTree. The server does NOT arm a death timer on duplicate (the + // copy's `temporaryExpiresAt` defaults to null = permanent), so the mapper + // carries that null through and the duplicated node correctly shows no + // clock marker — matching the server without a reload. + const treeNodeData: SpaceTreeNode = pageToTreeNode(duplicatedPage, { canEdit: true, - children: [], - }; + }); setData((prev) => treeModel.insert(prev, parentId, treeNodeData, newIndex), diff --git a/apps/client/src/features/page/tree/hooks/use-tree-mutation.ts b/apps/client/src/features/page/tree/hooks/use-tree-mutation.ts index 21db8757..494f8b93 100644 --- a/apps/client/src/features/page/tree/hooks/use-tree-mutation.ts +++ b/apps/client/src/features/page/tree/hooks/use-tree-mutation.ts @@ -9,6 +9,7 @@ import { treeModel } from "@/features/page/tree/model/tree-model"; import type { DropOp } from "@/features/page/tree/model/tree-model.types"; import { dropOpToMovePayload } from "./drop-op-to-move-payload"; import { SpaceTreeNode } from "@/features/page/tree/types.ts"; +import { pageToTreeNode } from "@/features/page/tree/utils"; import { IPage } from "@/features/page/types/page.types.ts"; import { useCreatePageMutation, @@ -139,18 +140,15 @@ export function useTreeMutation(spaceId: string): UseTreeMutation { throw new Error("Failed to create page"); } - const newNode: SpaceTreeNode = { - id: createdPage.id, - slugId: createdPage.slugId, + // Route through the canonical mapper so the field copy (esp. + // `temporaryExpiresAt`, which shows the temporary-note clock marker on + // optimistic insert) can't drift from buildTree. `name: ""` because a + // freshly created page is untitled; `hasChildren: false` because it has no + // children yet. + const newNode: SpaceTreeNode = pageToTreeNode(createdPage, { name: "", - position: createdPage.position, - spaceId: createdPage.spaceId, - parentPageId: createdPage.parentPageId, hasChildren: false, - // Show the temporary-note icon immediately on optimistic insert. - temporaryExpiresAt: createdPage.temporaryExpiresAt, - children: [], - }; + }); // Read latest tree at call time. Without this, callers that mutate the // tree (e.g. lazy-load children on expand) immediately before calling diff --git a/apps/client/src/features/page/tree/model/tree-model.test.ts b/apps/client/src/features/page/tree/model/tree-model.test.ts index 01682e2d..3b365bf4 100644 --- a/apps/client/src/features/page/tree/model/tree-model.test.ts +++ b/apps/client/src/features/page/tree/model/tree-model.test.ts @@ -393,6 +393,101 @@ describe("handleCreate optimistic-insert idempotency (find-then-skip)", () => { }); }); +// handleCreate race-guard temporaryExpiresAt patch: when the server's +// addTreeNode broadcast wins the race and inserts the node BEFORE the optimistic +// updater runs, the updater must not re-insert. Two sub-branches: +// (a) the node the broadcast inserted carries NO deadline (an older broadcast +// omitted it) while the authoritative create response DOES → patch the +// deadline on so the clock marker shows now, without a reload. +// (b) the existing node ALREADY has a deadline → do NOT overwrite it; return +// `prev` by reference (a no-op write). +describe("handleCreate race-guard temporaryExpiresAt patch", () => { + type TN = TreeNode<{ name: string; temporaryExpiresAt?: string | null }>; + + // Mirrors the setData updater in use-tree-mutation handleCreate. + const applyOptimisticInsert = ( + tree: TN[], + parentId: string | null, + node: TN, + index: number, + ): TN[] => { + const existing = treeModel.find(tree, node.id) as TN | null; + if (existing) { + if (node.temporaryExpiresAt && !existing.temporaryExpiresAt) { + return treeModel.update(tree, node.id, { + temporaryExpiresAt: node.temporaryExpiresAt, + }); + } + return tree; + } + return treeModel.insert(tree, parentId, node, index); + }; + + const fixtureTN: TN[] = [ + { id: "a", name: "A" }, + { id: "b", name: "B" }, + ]; + + const deadline = "2026-07-01T00:00:00.000Z"; + + it("(a) patches temporaryExpiresAt when the existing node has none + the response carries a deadline", () => { + // Server broadcast won the race and inserted the node WITHOUT a deadline. + const afterServer = treeModel.insert(fixtureTN, null, { + id: "new", + name: "", + }); + expect((treeModel.find(afterServer, "new") as TN).temporaryExpiresAt).toBe( + undefined, + ); + + // The authoritative create response carries the deadline. + const created: TN = { id: "new", name: "", temporaryExpiresAt: deadline }; + const patched = applyOptimisticInsert( + afterServer, + null, + created, + afterServer.length, + ); + + // A new reference (the patch wrote) and the node now has the deadline... + expect(patched).not.toBe(afterServer); + expect((treeModel.find(patched, "new") as TN).temporaryExpiresAt).toBe( + deadline, + ); + // ...and still exactly one node (no duplicate re-insert). + expect(patched.filter((n) => n.id === "new")).toHaveLength(1); + }); + + it("(b) does NOT overwrite an existing deadline; returns prev by reference", () => { + const existingDeadline = deadline; + // The node already exists WITH a deadline (the broadcast carried it). + const afterServer = treeModel.insert(fixtureTN, null, { + id: "new", + name: "", + temporaryExpiresAt: existingDeadline, + }); + + // The create response carries a DIFFERENT deadline; the guard must ignore it. + const created: TN = { + id: "new", + name: "", + temporaryExpiresAt: "2099-01-01T00:00:00.000Z", + }; + const after = applyOptimisticInsert( + afterServer, + null, + created, + afterServer.length, + ); + + // prev returned by reference (no write) and the original deadline is kept. + expect(after).toBe(afterServer); + expect((treeModel.find(after, "new") as TN).temporaryExpiresAt).toBe( + existingDeadline, + ); + }); +}); + // moveTreeNode socket-handler semantics: the receiver must place the moved node // by `position` (NOT index 0) and apply the `pageData` the payload carries so a // moved node's title/icon/chevron stay correct. This mirrors the reducer in diff --git a/apps/client/src/features/page/tree/utils/utils.ts b/apps/client/src/features/page/tree/utils/utils.ts index 8abb16e1..11600162 100644 --- a/apps/client/src/features/page/tree/utils/utils.ts +++ b/apps/client/src/features/page/tree/utils/utils.ts @@ -9,26 +9,45 @@ export function sortPositionKeys(keys: any[]) { }); } +/** + * Single canonical `IPage -> SpaceTreeNode` field mapper. Every place that + * materialises a tree node from a page (buildTree, the optimistic insert in + * handleCreate, restore, duplicate) routes through here so the field copy — + * crucially `temporaryExpiresAt` — can never silently drift between sites. The + * `overrides` cover the small per-site differences (e.g. `name: ""` for an + * optimistic create, `name: title || "Untitled"` for restore, `canEdit: true` + * for duplicate). The default `temporaryExpiresAt` comes straight off the page, + * so restore (which the server nulls) stays permanent and a temporary create + * keeps its clock marker without a reload. + */ +export function pageToTreeNode( + page: IPage, + overrides?: Partial, +): SpaceTreeNode { + return { + id: page.id, + slugId: page.slugId, + name: page.title, + icon: page.icon, + position: page.position, + hasChildren: page.hasChildren, + spaceId: page.spaceId, + parentPageId: page.parentPageId, + canEdit: page.canEdit ?? page.permissions?.canEdit, + isTemplate: page.isTemplate, + temporaryExpiresAt: page.temporaryExpiresAt, + children: [], + ...overrides, + }; +} + export function buildTree(pages: IPage[]): SpaceTreeNode[] { const pageMap: Record = {}; const tree: SpaceTreeNode[] = []; pages.forEach((page) => { - pageMap[page.id] = { - id: page.id, - slugId: page.slugId, - name: page.title, - icon: page.icon, - position: page.position, - hasChildren: page.hasChildren, - spaceId: page.spaceId, - parentPageId: page.parentPageId, - canEdit: page.canEdit ?? page.permissions?.canEdit, - isTemplate: page.isTemplate, - temporaryExpiresAt: page.temporaryExpiresAt, - children: [], - }; + pageMap[page.id] = pageToTreeNode(page); }); // Defense-in-depth: a duplicate id in `pages` would push two references to the diff --git a/apps/server/src/database/listeners/page.listener.ts b/apps/server/src/database/listeners/page.listener.ts index 52b9f963..b12dc160 100644 --- a/apps/server/src/database/listeners/page.listener.ts +++ b/apps/server/src/database/listeners/page.listener.ts @@ -28,6 +28,36 @@ export interface TreeNodeSnapshot { temporaryExpiresAt?: Date | string | null; } +/** + * Single canonical builder for a `TreeNodeSnapshot` from a page-like row. Both + * the `PAGE_CREATED` event enrichment (`page.repo.insertPage`) and the + * `addTreeNode` broadcast (`WsTreeService.broadcastPageCreated`) build this same + * snapshot; routing both through here keeps the optional `temporaryExpiresAt` + * (and the `?? null` normalisation that pins a permanent note to an explicit + * null) from silently drifting between the two literals. + */ +export function toTreeNodeSnapshot(page: { + id: string; + slugId: string; + title: string | null; + icon: string | null; + position: string; + spaceId: string; + parentPageId: string | null; + temporaryExpiresAt?: Date | string | null; +}): TreeNodeSnapshot { + return { + id: page.id, + slugId: page.slugId, + title: page.title, + icon: page.icon, + position: page.position, + spaceId: page.spaceId, + parentPageId: page.parentPageId, + temporaryExpiresAt: page.temporaryExpiresAt ?? null, + }; +} + export class PageEvent { pageIds: string[]; workspaceId: string; diff --git a/apps/server/src/database/repos/page/page.repo.ts b/apps/server/src/database/repos/page/page.repo.ts index 1b711750..a7ac3a5e 100644 --- a/apps/server/src/database/repos/page/page.repo.ts +++ b/apps/server/src/database/repos/page/page.repo.ts @@ -16,7 +16,10 @@ import { jsonArrayFrom, jsonObjectFrom } from 'kysely/helpers/postgres'; import { SpaceMemberRepo } from '@docmost/db/repos/space/space-member.repo'; import { EventEmitter2 } from '@nestjs/event-emitter'; import { EventName } from '../../../common/events/event.contants'; -import { TreeUpdateSnapshot } from '../../listeners/page.listener'; +import { + TreeUpdateSnapshot, + toTreeNodeSnapshot, +} from '../../listeners/page.listener'; /** * Optional extras for the PAGE_UPDATED event emitted by updatePage(s). Lets the @@ -200,20 +203,10 @@ export class PageRepo { this.eventEmitter.emit(EventName.PAGE_CREATED, { pageIds: [result.id], workspaceId: result.workspaceId, - pages: [ - { - id: result.id, - slugId: result.slugId, - title: result.title, - icon: result.icon, - position: result.position, - spaceId: result.spaceId, - parentPageId: result.parentPageId, - // Carry the death-timer deadline so a note created as temporary shows - // its sidebar clock marker on every client without a reload. - temporaryExpiresAt: result.temporaryExpiresAt, - }, - ], + // Built via the shared snapshot helper so the field copy (and the + // death-timer deadline that shows the sidebar clock marker without a + // reload) can't drift from the `addTreeNode` broadcast literal. + pages: [toTreeNodeSnapshot(result)], }); return result; diff --git a/apps/server/src/ws/ws-tree.service.spec.ts b/apps/server/src/ws/ws-tree.service.spec.ts index 1ee8d10b..8b5e3a2f 100644 --- a/apps/server/src/ws/ws-tree.service.spec.ts +++ b/apps/server/src/ws/ws-tree.service.spec.ts @@ -83,6 +83,27 @@ describe('WsTreeService', () => { ); }); + it('broadcastPageCreated carries temporaryExpiresAt when the page is a temporary note', async () => { + const expiresAt = new Date('2026-07-01T00:00:00.000Z'); + await service.broadcastPageCreated({ ...snapshot, temporaryExpiresAt: expiresAt }); + + const data = + wsService.emitTreeEvent.mock.calls[0][2].payload.data; + // The death-timer deadline reaches receivers so the clock marker renders + // immediately (incl. the author if this broadcast wins the optimistic race). + expect(data.temporaryExpiresAt).toBe(expiresAt); + }); + + it('broadcastPageCreated pins temporaryExpiresAt to null for a permanent page', async () => { + // Fixture omits temporaryExpiresAt; the `?? null` must send an explicit null + // (permanent) rather than undefined, so receivers clear any stale marker. + await service.broadcastPageCreated(snapshot); + + const data = + wsService.emitTreeEvent.mock.calls[0][2].payload.data; + expect(data.temporaryExpiresAt).toBeNull(); + }); + it('broadcastPageDeleted emits deleteTreeNode with the root node only', async () => { await service.broadcastPageDeleted({ ...snapshot, diff --git a/apps/server/src/ws/ws-tree.service.ts b/apps/server/src/ws/ws-tree.service.ts index 15b4dd87..65a6b4d9 100644 --- a/apps/server/src/ws/ws-tree.service.ts +++ b/apps/server/src/ws/ws-tree.service.ts @@ -5,6 +5,7 @@ import { PageMovedEvent, TreeNodeSnapshot, TreeUpdateSnapshot, + toTreeNodeSnapshot, } from '../database/listeners/page.listener'; @Injectable() @@ -28,20 +29,17 @@ export class WsTreeService { // Receivers place by `position` among already-loaded siblings, not by // this absolute index (sender's loaded set differs from receivers'). index: 0, + // Built via the shared snapshot helper (same one page.repo uses to fill + // the event), then extended with the tree-only fields the client + // receiver consumes. The helper carries the death-timer deadline + // (normalised to null => permanent) so receivers — and the author, if + // this broadcast wins the race against the optimistic insert — render + // the temporary-note clock marker immediately, without it drifting from + // the event literal. data: { - id: page.id, - slugId: page.slugId, + ...toTreeNodeSnapshot(page), name: page.title ?? '', - title: page.title, - icon: page.icon, - position: page.position, - spaceId: page.spaceId, - parentPageId: page.parentPageId, hasChildren: false, - // Carry the death-timer deadline so receivers (and the author, if this - // broadcast wins the race against the optimistic insert) render the - // temporary-note clock marker immediately. null => permanent. - temporaryExpiresAt: page.temporaryExpiresAt ?? null, children: [], }, }, -- 2.49.1