From 89131f3b3a50e66b89df16d59fdcb40710ae0105 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C5=ABdolfs=20O=C5=A1i=C5=86=C5=A1?= <rudolfs@osins.org>
Date: Wed, 30 Sep 2026 14:47:50 +0200
Subject: [PATCH] Keep every review comment in inbox summaries

Notification actions were deduplicated by operation and type, so the
several code comments of one published review collapsed into one.
---
 src/components/NotificationTeaser.svelte | 28 +++------------
 src/lib/notification.ts                  | 14 ++++++++
 tests/unit/notifications.test.ts         | 44 +++++++++++++++++++++++-
 3 files changed, 61 insertions(+), 25 deletions(-)

diff --git a/src/components/NotificationTeaser.svelte b/src/components/NotificationTeaser.svelte
index ed907ce85..dc87a0f80 100644
--- a/src/components/NotificationTeaser.svelte
+++ b/src/components/NotificationTeaser.svelte
@@ -1,14 +1,9 @@
 <script lang="ts">
   import type { RepoRoute } from "@app/views/repo/router";
-  import type { ActionWithAuthor } from "@bindings/cob/inbox/ActionWithAuthor";
   import type { NotificationItem } from "@bindings/cob/inbox/NotificationItem";
-  import type { Action as IssueAction } from "@bindings/cob/issue/Action";
-  import type { Action as PatchAction } from "@bindings/cob/patch/Action";
   import type { ComponentProps } from "svelte";
 
-  import uniqWith from "lodash/uniqWith";
-
-  import { compressActions } from "@app/lib/notification";
+  import { compressActions, notificationActions } from "@app/lib/notification";
   import { push } from "@app/lib/router";
   import {
     absoluteTimestamp,
@@ -47,24 +42,9 @@
     selected = false,
   }: Props = $props();
 
-  type Action = ActionWithAuthor<IssueAction> | ActionWithAuthor<PatchAction>;
-
-  const uniqueActions = $derived.by(() => {
-    return compressActions(
-      uniqWith(
-        notificationItems.flatMap<Action>(n => n.actions),
-        (a, b) =>
-          Boolean(
-            a.oid === b.oid &&
-            a.type === b.type &&
-            a.author.did &&
-            b.author.did,
-          ),
-      ).sort((a, b) => b.timestamp - a.timestamp),
-      kind,
-      oid,
-    );
-  });
+  const uniqueActions = $derived(
+    compressActions(notificationActions(notificationItems), kind, oid),
+  );
 
   const title = $derived.by(() => {
     const lastDetail = notificationItems.at(-1);
diff --git a/src/lib/notification.ts b/src/lib/notification.ts
index 7b5cc7b05..cd3639d2e 100644
--- a/src/lib/notification.ts
+++ b/src/lib/notification.ts
@@ -2,6 +2,9 @@ import type { ActionWithAuthor } from "@bindings/cob/inbox/ActionWithAuthor";
 import type { Action as IssueAction } from "@bindings/cob/issue/Action";
 import type { Action as PatchAction } from "@bindings/cob/patch/Action";
 
+import isEqual from "lodash/isEqual";
+import uniqWith from "lodash/uniqWith";
+
 import { emojiToTwemoji, formatOid, pluralize } from "@app/lib/utils";
 
 export type Action =
@@ -182,3 +185,14 @@ export function compressActions(
 
   return result;
 }
+
+// Actions of a notified COB, newest first. Notifications for the same COB can
+// cover overlapping ranges, so an action may appear in several of them. Only
+// identical actions are duplicates: one operation can carry several actions of
+// the same type, such as the code comments of a published review.
+export function notificationActions(items: { actions: Action[] }[]): Action[] {
+  return uniqWith(
+    items.flatMap(item => item.actions),
+    isEqual,
+  ).sort((a, b) => b.timestamp - a.timestamp);
+}
diff --git a/tests/unit/notifications.test.ts b/tests/unit/notifications.test.ts
index 31adc29b0..348494c62 100644
--- a/tests/unit/notifications.test.ts
+++ b/tests/unit/notifications.test.ts
@@ -4,7 +4,11 @@ import type { Action as PatchAction } from "@bindings/cob/patch/Action";
 import { describe, expect, test } from "vitest";
 
 import type { Action as NotificationAction } from "@app/lib/notification";
-import { compressActions, createSummary } from "@app/lib/notification";
+import {
+  compressActions,
+  createSummary,
+  notificationActions,
+} from "@app/lib/notification";
 import { formatOid } from "@app/lib/utils";
 
 const oid = "e8f95f5082a8e99c290ab3908a926e1de5c97d6c";
@@ -204,3 +208,41 @@ describe("Action summaries", () => {
     },
   );
 });
+
+describe("notificationActions", () => {
+  const comment = (body: string, at = timestamp) => ({
+    ...createAction({ type: "review.comment", body, review: oid }),
+    timestamp: at,
+  });
+
+  test("keeps an action once when two notifications include it", () => {
+    const shared = comment("A review comment");
+
+    expect(
+      notificationActions([
+        { actions: [shared] },
+        { actions: [{ ...shared }] },
+      ]),
+    ).toEqual([shared]);
+  });
+
+  test("keeps same-type actions from one operation", () => {
+    const actions = notificationActions([
+      { actions: [comment("First"), comment("Second"), comment("Third")] },
+    ]);
+
+    expect(actions).toHaveLength(3);
+    expect(compressActions(actions, "patch", oid)[0].summary).toBe(
+      "left 3 review comments",
+    );
+  });
+
+  test("orders actions newest first", () => {
+    expect(
+      notificationActions([
+        { actions: [comment("old", 1)] },
+        { actions: [comment("new", 3), comment("middle", 2)] },
+      ]).map(a => a.timestamp),
+    ).toEqual([3, 2, 1]);
+  });
+});