From 5af2d387c7ce3d0b003acaa792ef20a2a2d08337 Mon Sep 17 00:00:00 2001 From: Mike Yumatov Date: Mon, 29 Jun 2026 12:56:36 +0300 Subject: [PATCH] feat(notifications): coalesce owner-side comment notifications per-page Owner-side doc-comment notifications (a new top-level comment routed to the owning group) now use a per-page coalescing scope `rw:page:|` instead of the per-thread `rw:comment:`. A burst of new comments on one doc collapses into a single self-updating Web-inbox row per owning group rather than one notification per thread. Participant-side notifications (replies and resolves) keep the per-thread scope; the two namespaces are disjoint, so a reply is never swallowed by the page-level owner row. Also documents the behavior and the optional official Slack module in the plugin README: install + group->channel / user->DM routing, the correct `notifications.processors.slack` config key, and an honest note that the module does not coalesce owner-side channel posts (the per-page coalescing is a Web-inbox property). Co-Authored-By: Claude Opus 4.8 (1M context) --- .../rw-backend-module-notifications/README.md | 59 ++++++++++++++++++- .../src/CommentNotifier.test.ts | 25 +++++++- .../src/CommentNotifier.ts | 14 ++++- 3 files changed, 91 insertions(+), 7 deletions(-) diff --git a/plugins/rw-backend-module-notifications/README.md b/plugins/rw-backend-module-notifications/README.md index eda7c0b..fb2b490 100644 --- a/plugins/rw-backend-module-notifications/README.md +++ b/plugins/rw-backend-module-notifications/README.md @@ -16,9 +16,9 @@ the native notifications + signals plugins in the host app: ```ts // packages/backend/src/index.ts -backend.add(import('@rwdocs/backstage-plugin-rw-backend-module-notifications')); -backend.add(import('@backstage/plugin-notifications-backend')); -backend.add(import('@backstage/plugin-signals-backend')); // real-time updates +backend.add(import("@rwdocs/backstage-plugin-rw-backend-module-notifications")); +backend.add(import("@backstage/plugin-notifications-backend")); +backend.add(import("@backstage/plugin-signals-backend")); // real-time updates ``` ```ts @@ -75,6 +75,59 @@ Supply human-readable labels via the settings card's `originNames` / `topicNames props (see `packages/app/src/notificationSettings.tsx` in this repo). Stale topics are cleaned up by `notifications.retention` (default `1y`). +### Coalescing & burst control + +Each notification carries a coalescing `scope`. The backend dedups on +`(user, scope, origin)`, overwriting the prior row in place rather than adding a new +one. The two sides use different scope namespaces: + +- **Owner-side** (a new top-level comment → the owning group): **per-page** + (`rw:page:|`). A burst of new comments on one doc collapses into a + **single self-updating notification** — the row shows the latest event, last-write-wins. + This is the flood control: an owning group accumulates one inbox row per doc, not one per + comment, during a hot-doc burst. +- **Participant-side** (replies + resolves → prior participants): **per-thread** + (`rw:comment:`), so each thread keeps its own self-updating row. + +The owner-side notification therefore reads as _"which docs need attention"_ (doc-level); the +**Comments inbox tab** remains the _"what specifically"_ worklist. Note: Backstage re-fires the +realtime `new_notification` signal on every event even when it only updates an existing row, so +the Web badge still blinks per event (one row, repeated signal). + +### Optional: Slack delivery + +The official `@backstage/plugin-notifications-backend-module-slack` (a first-party +`NotificationProcessor`) delivers these same notifications to Slack with **no code change +here** — install and configure it in the host backend: + +```ts +// packages/backend/src/index.ts +backend.add(import("@backstage/plugin-notifications-backend-module-slack")); +``` + +```yaml +# app-config.yaml +notifications: + processors: + slack: + - token: ${SLACK_BOT_TOKEN} +``` + +Resolve a recipient to a Slack target via a `slack.com/bot-notify` annotation (Slack +user/channel ID, recommended), with a fallback to `spec.profile.email` → +`users.lookupByEmail` for User entities. + +Because this module already routes owner-side notifications to a **group** and +participant-side to **users**, the Slack module's routing falls out for free: + +- A **group** recipient → **one post to that team's Slack channel**. +- A **user** recipient → a **DM**. + +Note: the per-page coalescing above is a property of the Web inbox; the Slack module does not +fold a hot-doc burst into a single channel post, so owner-side bursts still produce one channel +message per comment. Time-windowed digests, quiet hours, and per-user frequency caps are not +native to Backstage and are out of scope for this module. + ### Notes - Notification links resolve into the entity's RW docs tab at the comment anchor. diff --git a/plugins/rw-backend-module-notifications/src/CommentNotifier.test.ts b/plugins/rw-backend-module-notifications/src/CommentNotifier.test.ts index b661caf..ab6840b 100644 --- a/plugins/rw-backend-module-notifications/src/CommentNotifier.test.ts +++ b/plugins/rw-backend-module-notifications/src/CommentNotifier.test.ts @@ -51,14 +51,34 @@ describe("CommentNotifier", () => { expect(arg.payload.topic).toBe("comment:thread:created"); expect(arg.payload.title).toBe("Jane Doe commented on Guide · Docs"); expect(arg.payload.description).toBe("hello"); - expect(arg.payload.scope).toBe("rw:comment:c1"); + expect(arg.payload.scope).toBe("rw:page:component:default/site|sec-1#guide"); expect(arg.payload.severity).toBe("normal"); expect(arg.payload.link).toBe("/catalog/default/component/my-docs/docs/guide#comment-c1"); }); + it("two top-level creates on the same page → identical scope (per-page coalescing)", async () => { + // The point of per-page scope: distinct threads on one page share a scope so the + // backend deduplicates them to one self-updating row. A scope that included rootId + // would defeat this and still pass the single-create test above. + await notifier.process(makeActivity({ commentId: "c1", rootId: "c1" })); + await notifier.process(makeActivity({ commentId: "c2", rootId: "c2" })); + + expect(send.mock.calls[0][0].payload.scope).toBe("rw:page:component:default/site|sec-1#guide"); + expect(send.mock.calls[1][0].payload.scope).toBe(send.mock.calls[0][0].payload.scope); + }); + + it("top-level create vs reply on the same thread → disjoint scopes (reply not coalesced into owner row)", async () => { + await notifier.process(makeActivity()); // owner-side, top-level create + await notifier.process(makeActivity({ commentId: "c2", parentId: "c1" })); // participant-side reply + + expect(send.mock.calls[0][0].payload.scope).toBe("rw:page:component:default/site|sec-1#guide"); + expect(send.mock.calls[1][0].payload.scope).toBe("rw:comment:c1"); + }); + it("reply create → recipients from participants, topic:reply:created, title 'replied on'", async () => { await notifier.process( makeActivity({ + commentId: "c2", // the reply's own id; rootId stays "c1" so scope must key on rootId parentId: "c1", participants: ["user:default/jane", "user:default/bob"], }), @@ -73,12 +93,14 @@ describe("CommentNotifier", () => { }); expect(arg.payload.topic).toBe("comment:reply:created"); expect(arg.payload.title).toBe("Jane Doe replied on Guide · Docs"); + expect(arg.payload.scope).toBe("rw:comment:c1"); }); it("resolve → recipients from participants, topic:thread:resolved, title 'resolved a thread', description prefixed Re:", async () => { await notifier.process( makeActivity({ action: "resolved", + commentId: "c2", // the resolve's own id; rootId stays "c1" so scope must key on rootId rootId: "c1", participants: ["user:default/jane", "user:default/bob"], bodySnippet: "all set", @@ -95,6 +117,7 @@ describe("CommentNotifier", () => { expect(arg.payload.topic).toBe("comment:thread:resolved"); expect(arg.payload.title).toBe("Jane Doe resolved a thread on Guide · Docs"); expect(arg.payload.description).toBe("Re: all set"); + expect(arg.payload.scope).toBe("rw:comment:c1"); }); it("null entityRef → link: undefined", async () => { diff --git a/plugins/rw-backend-module-notifications/src/CommentNotifier.ts b/plugins/rw-backend-module-notifications/src/CommentNotifier.ts index bc99ec6..36c8a08 100644 --- a/plugins/rw-backend-module-notifications/src/CommentNotifier.ts +++ b/plugins/rw-backend-module-notifications/src/CommentNotifier.ts @@ -25,10 +25,17 @@ export class CommentNotifier implements CommentProcessor { async process(comment: CommentActivity): Promise { let recipients: string[]; + let scope: string; if (comment.action === "created" && comment.parentId === null) { + // Owner-side burst path: coalesce per-page so a hot doc collapses into one + // self-updating Web inbox row per recipient group, not one row per comment. recipients = comment.sectionOwnerRef ? [comment.sectionOwnerRef] : []; + scope = `rw:page:${comment.siteRef}|${comment.pageRef}`; } else { + // Participant-side (replies + resolves): per-thread, so each conversation keeps its + // own self-updating row rather than coalescing across threads on the same page. recipients = comment.participants; + scope = `rw:comment:${comment.rootId}`; } if (recipients.length === 0) return; // nothing to notify @@ -47,9 +54,10 @@ export class CommentNotifier implements CommentProcessor { link: this.link(comment), severity: "normal", topic: this.topic(comment), - // Per-thread collapse: activities on one thread share this scope, and the backend - // dedups on (user, scope, origin) — not topic — overwriting the prior row in place. - scope: `rw:comment:${comment.rootId}`, + // The backend dedups on (user, scope, origin) — not topic — overwriting the prior + // row in place. The two namespaces (rw:page:… vs rw:comment:…) are disjoint, so a + // reply is never swallowed by the page-level owner row. + scope, }, }); } catch (error) {