diff --git a/desktop/src/features/messages/lib/membershipGroupPayload.test.mjs b/desktop/src/features/messages/lib/membershipGroupPayload.test.mjs index ade4e7d68a5..b0a31ccb8af 100644 --- a/desktop/src/features/messages/lib/membershipGroupPayload.test.mjs +++ b/desktop/src/features/messages/lib/membershipGroupPayload.test.mjs @@ -25,15 +25,25 @@ function systemEntry(id, createdAt, body) { } // One symbol per membership mechanism the relay can emit, so the matrix covers -// self-arrival, addition (same and different actor), and departure — for two -// distinct targets, which is what makes same-target vs cross-target grouping -// observable. +// self-arrival, addition (same and different actor), removal, and departure — +// for two distinct targets, which is what makes same-target vs cross-target +// grouping observable. const SYMBOLS = { "self-join(a)": { type: "member_joined", actor: "aaa", target: "aaa" }, "self-join(b)": { type: "member_joined", actor: "bbb", target: "bbb" }, "add(a by x)": { type: "member_joined", actor: "xxx", target: "aaa" }, "add(b by x)": { type: "member_joined", actor: "xxx", target: "bbb" }, "add(b by y)": { type: "member_joined", actor: "yyy", target: "bbb" }, + "remove(a)": { + type: "member_removed", + actor: "xxx", + target: "aaa", + }, + "remove(b)": { + type: "member_removed", + actor: "xxx", + target: "bbb", + }, "leave(a)": { type: "member_left", actor: "aaa" }, "leave(b)": { type: "member_left", actor: "bbb" }, }; @@ -96,6 +106,106 @@ test("every membership group buildTimelineItems emits is describable", () => { ); }); +test("an alternating arrival and departure burst collapses to one row", () => { + const entries = Array.from({ length: 20 }, (_, index) => { + const target = `${index.toString(16).padStart(2, "0")}aa`; + return systemEntry( + `burst-${index}`, + DAY_START + index * 30, + index % 2 === 0 + ? { type: "member_joined", actor: "admin", target } + : { type: "member_removed", actor: "admin", target }, + ); + }); + + const { items } = buildTimelineItems(entries, null); + const groups = items.filter((item) => item.kind === "system-group"); + + assert.equal(groups.length, 1); + assert.equal(groups[0].entries.length, 20); + assert.equal( + buildGroupedMembershipPayload( + groups[0].entries.map((entry) => entry.message), + )?.type, + "members_changed", + ); +}); + +test("an ordinary message between membership bursts yields two rows", () => { + const firstBurst = [ + systemEntry("first-join", DAY_START, { + type: "member_joined", + actor: "admin", + target: "first", + }), + systemEntry("first-remove", DAY_START + 30, { + type: "member_removed", + actor: "admin", + target: "first", + }), + ]; + const ordinaryMessage = { + message: { + author: "Member", + body: "ordinary message", + createdAt: DAY_START + 60, + depth: 0, + id: "ordinary", + kind: 9, + pubkey: "bb".repeat(32), + reactions: [], + time: "12:00 PM", + }, + summary: null, + }; + const secondBurst = [ + systemEntry("second-join", DAY_START + 90, { + type: "member_joined", + actor: "admin", + target: "second", + }), + systemEntry("second-remove", DAY_START + 120, { + type: "member_removed", + actor: "admin", + target: "second", + }), + ]; + + const { items } = buildTimelineItems( + [...firstBurst, ordinaryMessage, ...secondBurst], + null, + ); + const groups = items.filter((item) => item.kind === "system-group"); + + assert.deepEqual( + groups.map((group) => group.entries.map((entry) => entry.message.id)), + [ + ["first-join", "first-remove"], + ["second-join", "second-remove"], + ], + ); +}); + +test("membership events more than an hour apart do not group", () => { + const entries = [ + systemEntry("first", DAY_START, { + type: "member_joined", + actor: "admin", + target: "first", + }), + systemEntry("second", DAY_START + 60 * 60 + 1, { + type: "member_removed", + actor: "admin", + target: "second", + }), + ]; + + const { items } = buildTimelineItems(entries, null); + + assert.equal(items.filter((item) => item.kind === "system-group").length, 0); + assert.equal(items.filter((item) => item.kind === "system").length, 2); +}); + test("a lifecycle group needs every arrival to be a self-join by the departing member", () => { const elrond = "11".repeat(32); const viewer = "10".repeat(32); diff --git a/desktop/src/features/messages/lib/membershipGroupPayload.ts b/desktop/src/features/messages/lib/membershipGroupPayload.ts index cb22472077e..fbfd31e6559 100644 --- a/desktop/src/features/messages/lib/membershipGroupPayload.ts +++ b/desktop/src/features/messages/lib/membershipGroupPayload.ts @@ -20,6 +20,8 @@ export type SystemMessagePayload = { type: string; actor?: string; arrivals?: Array<{ actor: string; target: string }>; + addedTargets?: string[]; + removedTargets?: string[]; target?: string; targets?: string[]; topic?: string; @@ -59,6 +61,26 @@ export function buildGroupedMembershipPayload( const joinedThenLeft = buildJoinedThenLeftPayload(payloads); if (joinedThenLeft) return joinedThenLeft; + const membershipChanges = payloads.map(parseMembershipChange); + if (membershipChanges.some((change) => !change)) return null; + + const changes = membershipChanges as Array; + if (changes.some((change) => change.mode === "departure")) { + return { + addedTargets: uniqueTargets( + changes + .filter((change) => change.mode === "arrival") + .map((change) => change.target), + ), + removedTargets: uniqueTargets( + changes + .filter((change) => change.mode === "departure") + .map((change) => change.target), + ), + type: "members_changed", + }; + } + const arrivals = payloads.map((payload) => { const payloadActor = payload?.actor ? normalizePubkey(payload.actor) : null; const payloadTarget = payload?.target @@ -84,6 +106,35 @@ export function buildGroupedMembershipPayload( }; } +type MembershipChange = + | { mode: "arrival"; target: string } + | { mode: "departure"; target: string }; + +function parseMembershipChange( + payload: SystemMessagePayload | null, +): MembershipChange | null { + if (payload?.type === "member_joined" && payload.target) { + const target = normalizePubkey(payload.target); + return target ? { mode: "arrival", target } : null; + } + + if (payload?.type === "member_left" && payload.actor) { + const target = normalizePubkey(payload.actor); + return target ? { mode: "departure", target } : null; + } + + if (payload?.type === "member_removed" && payload.target) { + const target = normalizePubkey(payload.target); + return target ? { mode: "departure", target } : null; + } + + return null; +} + +function uniqueTargets(targets: readonly string[]): string[] { + return [...new Set(targets)]; +} + /** * One lifecycle summary for N>=1 equivalent self-arrivals of a member followed * by that same member departing. diff --git a/desktop/src/features/messages/lib/timelineItems.ts b/desktop/src/features/messages/lib/timelineItems.ts index 917350733fd..8c30b17746e 100644 --- a/desktop/src/features/messages/lib/timelineItems.ts +++ b/desktop/src/features/messages/lib/timelineItems.ts @@ -83,6 +83,13 @@ function parseMembershipChangePayload( const target = payload.actor.trim().toLowerCase(); return target ? { mode: "departure", target } : null; } + if ( + payload.type === "member_removed" && + typeof payload.target === "string" + ) { + const target = payload.target.trim().toLowerCase(); + return target ? { mode: "departure", target } : null; + } if ( payload.type !== "member_joined" || typeof payload.actor !== "string" || @@ -106,10 +113,14 @@ function membershipChangesCanGroup( first: MembershipChangePayload, second: MembershipChangePayload, ): boolean { - if (second.mode === "departure") { - return first.mode === "self-arrival" && first.target === second.target; - } - return first.mode !== "departure"; + return ( + (first.mode === "self-arrival" || + first.mode === "addition" || + first.mode === "departure") && + (second.mode === "self-arrival" || + second.mode === "addition" || + second.mode === "departure") + ); } /** @@ -118,13 +129,14 @@ function membershipChangesCanGroup( * likewise the newest entry's key: extending the oldest visible group changes * its contents, but not its identity or the virtual list's existing key suffix. * - * Compatible membership activities stay together while they are contiguous. - * Arrival cohorts are actor-neutral even when self-joins and additions mix, but - * one or more equivalent self-joins followed by that member leaving remain a - * single lifecycle summary — every contiguous self-arrival of the departing - * member is absorbed, since the relay re-emits `member_joined` on each - * PUT_USER. `buildGroupedMembershipPayload` must describe every group this - * emits; `membershipGroupPayload.test.mjs` pins that with a matrix invariant. + * Membership activities stay together while they are contiguous, regardless of + * whether they are arrivals, removals, or self-departures. Arrival cohorts are + * actor-neutral even when self-joins and additions mix, but one or more + * equivalent self-joins followed by that member leaving remain a single + * lifecycle summary — every contiguous self-arrival of the departing member is + * absorbed, since the relay re-emits `member_joined` on each PUT_USER. + * `buildGroupedMembershipPayload` must describe every group this emits; + * `membershipGroupPayload.test.mjs` pins that with a matrix invariant. * Each adjacent event must fall within the one-hour activity window, so * uninterrupted activity can extend beyond an hour overall. */ diff --git a/desktop/src/features/messages/ui/SystemMessageRow.test.mjs b/desktop/src/features/messages/ui/SystemMessageRow.test.mjs index fc55dd27cc5..54763266ec0 100644 --- a/desktop/src/features/messages/ui/SystemMessageRow.test.mjs +++ b/desktop/src/features/messages/ui/SystemMessageRow.test.mjs @@ -60,6 +60,25 @@ function memberLeftMessage({ actor, createdAt = 1, id, reactions = [] }) { }; } +function memberRemovedMessage({ + actor, + createdAt = 1, + id, + reactions = [], + target, +}) { + return { + author: "System", + body: JSON.stringify({ type: "member_removed", actor, target }), + createdAt, + depth: 0, + id, + kind: 40099, + reactions, + time: "12:00 PM", + }; +} + function reaction(emoji, { reactedByCurrentUser = false } = {}) { return { emoji, @@ -386,6 +405,39 @@ test("grouped duplicate mixed-mechanism arrivals render singular neutral copy", assert.equal(normalizeText(row.textContent ?? ""), "Elrond arrived"); }); +test("grouped arrivals and departures render both membership changes", async () => { + const { screen } = await import("@testing-library/react"); + const admin = "10".repeat(32); + const astra = "11".repeat(32); + const clerk = "12".repeat(32); + const verifier = "13".repeat(32); + const groupedMessages = [ + systemMessage({ actor: admin, createdAt: 1, id: "a", target: astra }), + systemMessage({ actor: admin, createdAt: 2, id: "b", target: clerk }), + memberRemovedMessage({ + actor: admin, + createdAt: 3, + id: "c", + target: verifier, + }), + ]; + + await renderSystemMessageRow({ + groupedMessages, + profiles: { + ...profileFor(astra, "Astra"), + ...profileFor(clerk, "Clerk"), + ...profileFor(verifier, "Verifier"), + }, + }); + + const row = screen.getByTestId("system-message-row"); + assert.equal( + normalizeText(row.textContent ?? ""), + "Astra and Clerk added; Verifier removed", + ); +}); + // --- joined-then-left lifecycle groups --------------------------------------- // // `buildTimelineItems` groups every contiguous equivalent self-arrival with the diff --git a/desktop/src/features/messages/ui/SystemMessageRow.tsx b/desktop/src/features/messages/ui/SystemMessageRow.tsx index 4b74930da8f..536ead3f038 100644 --- a/desktop/src/features/messages/ui/SystemMessageRow.tsx +++ b/desktop/src/features/messages/ui/SystemMessageRow.tsx @@ -221,15 +221,58 @@ function membershipActivityPubkeys(payload: SystemMessagePayload): string[] { const pubkeys = payload.type === "members_arrived" ? (payload.targets ?? []) - : payload.type === "member_removed" - ? [payload.target ?? payload.actor] - : [payload.target ?? payload.actor]; + : payload.type === "members_changed" + ? [...(payload.addedTargets ?? []), ...(payload.removedTargets ?? [])] + : payload.type === "member_removed" + ? [payload.target ?? payload.actor] + : [payload.target ?? payload.actor]; return [ ...new Set(pubkeys.filter((pubkey): pubkey is string => Boolean(pubkey))), ]; } +function describeGroupedMembershipChanges({ + agentPubkeys, + currentPubkey, + payload, + personaLookup, + profiles, +}: { + agentPubkeys?: ReadonlySet; + currentPubkey: string | undefined; + payload: SystemMessagePayload; + personaLookup?: Map; + profiles: UserProfileLookup | undefined; +}): SystemMessageDescription | null { + const addedTargets = payload.addedTargets ?? []; + const removedTargets = payload.removedTargets ?? []; + if (addedTargets.length === 0 && removedTargets.length === 0) return null; + + const names = (targets: string[]) => ( + + ); + + if (addedTargets.length > 0 && removedTargets.length > 0) { + return { + title: names(addedTargets), + action: <>added; {names(removedTargets)} removed, + }; + } + + if (addedTargets.length > 0) { + return { title: names(addedTargets), action: "added" }; + } + + return { title: names(removedTargets), action: "removed" }; +} + function MembershipPersonName({ agentPubkeys, currentPubkey, @@ -561,6 +604,14 @@ function describeSystemEvent( personaLookup, profiles, }); + case "members_changed": + return describeGroupedMembershipChanges({ + agentPubkeys, + currentPubkey, + payload, + personaLookup, + profiles, + }); case "member_joined_then_left": if (!payload.target) return null; return { @@ -743,6 +794,7 @@ export const SystemMessageRow = React.memo(function SystemMessageRow({ payload.type === "member_joined" || payload.type === "members_arrived"; const isMembershipActivity = isMembershipArrival || + payload.type === "members_changed" || payload.type === "member_joined_then_left" || payload.type === "member_left" || payload.type === "member_removed";