fix(ws): broadcast realtime page rename/icon change (#72)
handleMessage became a no-op and PageWsListener intentionally ignored PAGE_UPDATED, so a rename/icon change (client operation:updateOne) was no longer rebroadcast -> other clients saw stale title/icon in the sidebar+breadcrumbs until a reload (create/duplicate/restore were covered; updateOne regressed). Add a server-authoritative onPageUpdated handler: PageService.update detects a real title/icon change (DTO carries the field AND value differs; no-op/content- only saves excluded) and attaches a treeUpdate snapshot to PAGE_UPDATED; the listener broadcasts a tree updateOne via the restriction-aware emitTreeEvent (so a restricted page's title never leaks). Content-only saves attach nothing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -270,6 +270,17 @@ export class PageService {
|
||||
|
||||
const isAgent = provenance?.actor === 'agent';
|
||||
|
||||
// Detect a real title/icon change so the WS tree listener can broadcast an
|
||||
// `updateOne` to the space (rename / icon swap) WITHOUT re-broadcasting on a
|
||||
// content-only save. Only treat a field as changed when the DTO actually
|
||||
// carries it AND its value differs from what is already stored — a no-op
|
||||
// save (same title, or a content-only update where these are undefined)
|
||||
// produces no tree snapshot, so the listener stays quiet.
|
||||
const titleChanged =
|
||||
updatePageDto.title !== undefined && updatePageDto.title !== page.title;
|
||||
const iconChanged =
|
||||
updatePageDto.icon !== undefined && updatePageDto.icon !== page.icon;
|
||||
|
||||
await this.pageRepo.updatePage(
|
||||
{
|
||||
title: updatePageDto.title,
|
||||
@@ -287,6 +298,22 @@ export class PageService {
|
||||
contributorIds: contributorIds,
|
||||
},
|
||||
page.id,
|
||||
undefined,
|
||||
// Enrich PAGE_UPDATED only when title/icon actually changed. The snapshot
|
||||
// values come from the server-side data being persisted (DTO when present,
|
||||
// otherwise the unchanged stored value), never relayed from the client.
|
||||
titleChanged || iconChanged
|
||||
? {
|
||||
treeUpdate: {
|
||||
id: page.id,
|
||||
slugId: page.slugId,
|
||||
spaceId: page.spaceId,
|
||||
parentPageId: page.parentPageId ?? null,
|
||||
...(titleChanged ? { title: updatePageDto.title } : {}),
|
||||
...(iconChanged ? { icon: updatePageDto.icon } : {}),
|
||||
},
|
||||
}
|
||||
: undefined,
|
||||
);
|
||||
|
||||
this.generalQueue
|
||||
|
||||
@@ -34,6 +34,30 @@ export class PageEvent {
|
||||
// Set on PAGE_RESTORED so the WS listener can scope a refetchRootTreeNodeEvent
|
||||
// to the affected space (restore can re-attach a whole subtree).
|
||||
spaceId?: string;
|
||||
// Set on a PAGE_UPDATED that actually changed the page's title and/or icon
|
||||
// (a rename or icon swap). Content-only saves leave this undefined, which is
|
||||
// how the WS listener distinguishes a tree-relevant metadata change from a
|
||||
// noisy content save and avoids re-broadcasting on every keystroke-flush.
|
||||
// Server-authoritative: built from the values being persisted, not relayed
|
||||
// from the client.
|
||||
treeUpdate?: TreeUpdateSnapshot;
|
||||
}
|
||||
|
||||
/**
|
||||
* Thin snapshot carried on a PAGE_UPDATED event when the title and/or icon
|
||||
* changed, so the WS tree listener can broadcast an `updateOne` without a DB
|
||||
* read. Only the fields the client tree receiver (`applyUpdateOne`) consumes
|
||||
* are included.
|
||||
*/
|
||||
export interface TreeUpdateSnapshot {
|
||||
id: string;
|
||||
slugId: string;
|
||||
spaceId: string;
|
||||
parentPageId: string | null;
|
||||
// Present only when that field actually changed; an undefined field is left
|
||||
// untouched by the client reducer.
|
||||
title?: string | null;
|
||||
icon?: string | null;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -16,6 +16,16 @@ 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';
|
||||
|
||||
/**
|
||||
* Optional extras for the PAGE_UPDATED event emitted by updatePage(s). Lets the
|
||||
* caller attach a tree snapshot for a title/icon change so the WS listener can
|
||||
* broadcast an `updateOne` without re-reading the DB.
|
||||
*/
|
||||
export interface UpdatePageEventOpts {
|
||||
treeUpdate?: TreeUpdateSnapshot;
|
||||
}
|
||||
|
||||
@Injectable()
|
||||
export class PageRepo {
|
||||
@@ -138,14 +148,16 @@ export class PageRepo {
|
||||
updatablePage: UpdatablePage,
|
||||
pageId: string,
|
||||
trx?: KyselyTransaction,
|
||||
opts?: UpdatePageEventOpts,
|
||||
) {
|
||||
return this.updatePages(updatablePage, [pageId], trx);
|
||||
return this.updatePages(updatablePage, [pageId], trx, opts);
|
||||
}
|
||||
|
||||
async updatePages(
|
||||
updatePageData: UpdatablePage,
|
||||
pageIds: string[],
|
||||
trx?: KyselyTransaction,
|
||||
opts?: UpdatePageEventOpts,
|
||||
) {
|
||||
const result = await dbOrTx(this.db, trx)
|
||||
.updateTable('pages')
|
||||
@@ -160,6 +172,11 @@ export class PageRepo {
|
||||
this.eventEmitter.emit(EventName.PAGE_UPDATED, {
|
||||
pageIds: pageIds,
|
||||
workspaceId: updatePageData.workspaceId,
|
||||
// Optional tree snapshot for the WS listener (variant A). The caller sets
|
||||
// it ONLY for a title/icon change so the listener can broadcast an
|
||||
// `updateOne` without a DB read; content-only saves omit it and the
|
||||
// listener skips them. Built from server-side data, never client-relayed.
|
||||
...(opts?.treeUpdate ? { treeUpdate: opts.treeUpdate } : {}),
|
||||
});
|
||||
|
||||
return result;
|
||||
|
||||
@@ -5,6 +5,7 @@ import {
|
||||
PageEvent,
|
||||
PageMovedEvent,
|
||||
TreeNodeSnapshot,
|
||||
TreeUpdateSnapshot,
|
||||
} from '../../database/listeners/page.listener';
|
||||
|
||||
const snapshot: TreeNodeSnapshot = {
|
||||
@@ -230,3 +231,53 @@ describe('PageWsListener delete/move/restore handlers', () => {
|
||||
expect(wsTree.broadcastRefetchRoot).toHaveBeenCalledWith('space-9');
|
||||
});
|
||||
});
|
||||
|
||||
describe('PageWsListener.onPageUpdated (rename / icon change)', () => {
|
||||
let listener: PageWsListener;
|
||||
let wsTree: { broadcastPageUpdated: jest.Mock };
|
||||
|
||||
const treeUpdate: TreeUpdateSnapshot = {
|
||||
id: 'page-1',
|
||||
slugId: 'slug-1',
|
||||
spaceId: 'space-1',
|
||||
parentPageId: null,
|
||||
title: 'Renamed',
|
||||
icon: '🚀',
|
||||
};
|
||||
|
||||
beforeEach(async () => {
|
||||
wsTree = {
|
||||
broadcastPageUpdated: jest.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
|
||||
const module: TestingModule = await Test.createTestingModule({
|
||||
providers: [PageWsListener, { provide: WsTreeService, useValue: wsTree }],
|
||||
}).compile();
|
||||
|
||||
listener = module.get<PageWsListener>(PageWsListener);
|
||||
});
|
||||
|
||||
it('WITH a title/icon `treeUpdate`: broadcasts updateOne with that snapshot', async () => {
|
||||
const event: PageEvent = {
|
||||
pageIds: ['page-1'],
|
||||
workspaceId: 'ws-1',
|
||||
treeUpdate,
|
||||
};
|
||||
|
||||
await listener.onPageUpdated(event);
|
||||
|
||||
expect(wsTree.broadcastPageUpdated).toHaveBeenCalledTimes(1);
|
||||
expect(wsTree.broadcastPageUpdated).toHaveBeenCalledWith(treeUpdate);
|
||||
});
|
||||
|
||||
it('content-only save (NO `treeUpdate`): does NOT broadcast', async () => {
|
||||
const event: PageEvent = {
|
||||
pageIds: ['page-1'],
|
||||
workspaceId: 'ws-1',
|
||||
};
|
||||
|
||||
await listener.onPageUpdated(event);
|
||||
|
||||
expect(wsTree.broadcastPageUpdated).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -16,12 +16,14 @@ import { WsTreeService } from '../ws-tree.service';
|
||||
* keeps it safe against the in-transaction visibility race (a synchronous
|
||||
* SELECT here could run before the emitting `trx` committed).
|
||||
*
|
||||
* Scope of this PR: create, move, soft-delete/delete, restore.
|
||||
* Scope: create, move, soft-delete/delete, restore, rename / icon change.
|
||||
*
|
||||
* Rename / icon change rides PAGE_UPDATED, which ALSO fires on every content
|
||||
* save. The emit site (PageService.update) attaches a `treeUpdate` snapshot ONLY
|
||||
* when the title or icon actually changed, so the handler below can gate strictly
|
||||
* on that snapshot and stay silent on content-only saves.
|
||||
*
|
||||
* Deferred follow-ups (intentionally NOT handled here):
|
||||
* - rename / icon change: would broadcast `updateOne` on PAGE_UPDATED, but
|
||||
* PAGE_UPDATED also fires on every content save, so it needs a title/icon
|
||||
* diff filter to avoid noise.
|
||||
* - cross-space move (`movePageToSpace` / PAGE_MOVED_TO_SPACE): needs a
|
||||
* deleteTreeNode in the old space + addTreeNode/refetch in the new space.
|
||||
*/
|
||||
@@ -68,6 +70,17 @@ export class PageWsListener {
|
||||
await this.wsTree.broadcastPageMoved(event);
|
||||
}
|
||||
|
||||
// Rename / icon change. PAGE_UPDATED also fires on every content save, so we
|
||||
// only act when the emit site flagged a real title/icon change via
|
||||
// `treeUpdate` — content-only saves carry no snapshot and are ignored here
|
||||
// (no noisy re-broadcast). The broadcast is restriction-aware (emitTreeEvent),
|
||||
// so a restricted page's title/icon can't leak to unauthorized sockets.
|
||||
@OnEvent(EventName.PAGE_UPDATED)
|
||||
async onPageUpdated(event: PageEvent): Promise<void> {
|
||||
if (!event.treeUpdate) return;
|
||||
await this.wsTree.broadcastPageUpdated(event.treeUpdate);
|
||||
}
|
||||
|
||||
@OnEvent(EventName.PAGE_RESTORED)
|
||||
async onPageRestored(event: PageEvent): Promise<void> {
|
||||
// Restore can re-attach a whole subtree; a root refetch is simpler and more
|
||||
|
||||
@@ -4,6 +4,7 @@ import { WsService } from './ws.service';
|
||||
import {
|
||||
PageMovedEvent,
|
||||
TreeNodeSnapshot,
|
||||
TreeUpdateSnapshot,
|
||||
} from '../database/listeners/page.listener';
|
||||
|
||||
@Injectable()
|
||||
@@ -43,6 +44,29 @@ export class WsTreeService {
|
||||
});
|
||||
}
|
||||
|
||||
// Rename / icon change: patch the in-tree node's title/icon on every client in
|
||||
// the space. Routed through the restriction-aware `emitTreeEvent` so a
|
||||
// restricted page's new title/icon never leaks to sockets that can't see it.
|
||||
// The payload mirrors the client `UpdateEvent` shape consumed by
|
||||
// `applyUpdateOne` (entity ["pages"], `id`, `payload.title` / `payload.icon`);
|
||||
// only the fields that actually changed are sent (the snapshot omits the rest).
|
||||
async broadcastPageUpdated(node: TreeUpdateSnapshot): Promise<void> {
|
||||
await this.wsService.emitTreeEvent(node.spaceId, node.id, {
|
||||
operation: 'updateOne',
|
||||
spaceId: node.spaceId,
|
||||
entity: ['pages'],
|
||||
id: node.id,
|
||||
payload: {
|
||||
slugId: node.slugId,
|
||||
parentPageId: node.parentPageId,
|
||||
// Only include changed fields; an absent field leaves the client node
|
||||
// untouched (applyUpdateOne checks `!== undefined` per field).
|
||||
...(node.title !== undefined ? { title: node.title } : {}),
|
||||
...(node.icon !== undefined ? { icon: node.icon } : {}),
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
async broadcastPageDeleted(page: TreeNodeSnapshot): Promise<void> {
|
||||
await this.wsService.emitTreeEvent(page.spaceId, page.id, {
|
||||
operation: 'deleteTreeNode',
|
||||
|
||||
Reference in New Issue
Block a user