From a2e4b7e395370e299c29a0404d067355431ae9e1 Mon Sep 17 00:00:00 2001 From: hayyaksi <193020925+hayaksi1@users.noreply.github.com> Date: Tue, 4 Aug 2026 20:37:14 +0300 Subject: [PATCH] Treat an unset profile field as unchanged in collapsed membership summaries (#34493) --- .../views/elements/EventListSummary.tsx | 18 ++++- .../views/elements/EventListSummary-test.tsx | 71 +++++++++++++++++++ 2 files changed, 87 insertions(+), 2 deletions(-) diff --git a/apps/web/src/components/views/elements/EventListSummary.tsx b/apps/web/src/components/views/elements/EventListSummary.tsx index 051a4a4902..ed0acb2b38 100644 --- a/apps/web/src/components/views/elements/EventListSummary.tsx +++ b/apps/web/src/components/views/elements/EventListSummary.tsx @@ -33,6 +33,18 @@ const onPinnedMessagesClick = (): void => { const TARGET_AS_DISPLAY_NAME_EVENTS = [EventType.RoomMember]; +/** + * Whether a profile field on a membership event actually changed. + * + * An absent field, an explicit `null` and an empty string all mean "unset", so none of them is a + * change from any of the others. This mirrors `getModification` in `TextForEvent`, which labels the + * same event when it is rendered on its own — without it a collapsed summary can contradict the + * events it summarises. + */ +function profileFieldChanged(prev?: string | null, value?: string | null): boolean { + return (prev || undefined) !== (value || undefined); +} + interface IProps extends Omit, "summaryText" | "summaryMembers"> { // The maximum number of names to show in either each summary e.g. 2 would result "A, B and 234 others left" summaryLength?: number; @@ -532,9 +544,11 @@ export default class EventListSummary extends React.Component { return TransitionType.Banned; case KnownMembership.Join: if (e.mxEvent.getPrevContent().membership === KnownMembership.Join) { - if (e.mxEvent.getContent().displayname !== e.mxEvent.getPrevContent().displayname) { + const content = e.mxEvent.getContent(); + const prevContent = e.mxEvent.getPrevContent(); + if (profileFieldChanged(prevContent.displayname, content.displayname)) { return TransitionType.ChangedName; - } else if (e.mxEvent.getContent().avatar_url !== e.mxEvent.getPrevContent().avatar_url) { + } else if (profileFieldChanged(prevContent.avatar_url, content.avatar_url)) { return TransitionType.ChangedAvatar; } return TransitionType.NoChange; diff --git a/apps/web/test/unit-tests/components/views/elements/EventListSummary-test.tsx b/apps/web/test/unit-tests/components/views/elements/EventListSummary-test.tsx index 937025c243..85ca2faad6 100644 --- a/apps/web/test/unit-tests/components/views/elements/EventListSummary-test.tsx +++ b/apps/web/test/unit-tests/components/views/elements/EventListSummary-test.tsx @@ -716,4 +716,75 @@ describe("EventListSummary", function () { expect(summary).toHaveTextContent("n...@d... was invited 2 times, d...@w... was invited"); expect(summary).toMatchSnapshot(); }); + + describe("profile changes", () => { + /** + * Generates a join -> join membership event with the given profile fields spliced into + * `content` and `prev_content`, so that the null-versus-absent cases can be exercised. + */ + const generateProfileEvent = ( + userId: string, + prevProfile: Record, + profile: Record, + ): MatrixEvent => { + const member = new RoomMember(roomId, userId); + member.name = userId.match(/@([^:]*):/)![1]; + const e = mkEvent({ + event: true, + type: "m.room.member", + room: roomId, + user: userId, + skey: userId, + content: { membership: KnownMembership.Join, ...profile }, + prev_content: { membership: KnownMembership.Join, ...prevProfile }, + }); + e.event.event_id = "event0"; + e.target = member; + return e; + }; + + const summaryFor = (event: MatrixEvent): string => { + const { container } = renderComponent({ + events: [event], + children: generateTiles([event]), + summaryLength: 1, + avatarsMaxLength: 5, + threshold: 1, + }); + return container.querySelector(".mx_GenericEventListSummary_summary")!.textContent!; + }; + + it("treats an explicit null avatar_url the same as an absent one", () => { + const event = generateProfileEvent("@user_1:some.domain", {}, { avatar_url: null }); + expect(summaryFor(event)).toBe("user_1 made no changes"); + }); + + it("treats an explicit null displayname the same as an absent one", () => { + const event = generateProfileEvent("@user_1:some.domain", {}, { displayname: null }); + expect(summaryFor(event)).toBe("user_1 made no changes"); + }); + + it("still reports a real avatar change", () => { + const event = generateProfileEvent( + "@user_1:some.domain", + { avatar_url: "mxc://server/old" }, + { avatar_url: "mxc://server/new" }, + ); + expect(summaryFor(event)).toBe("user_1 changed their profile picture"); + }); + + it("still reports a real name change", () => { + const event = generateProfileEvent("@user_1:some.domain", { displayname: "Old" }, { displayname: "New" }); + expect(summaryFor(event)).toBe("user_1 changed their name"); + }); + + it("still reports an avatar being removed", () => { + const event = generateProfileEvent( + "@user_1:some.domain", + { avatar_url: "mxc://server/old" }, + { avatar_url: null }, + ); + expect(summaryFor(event)).toBe("user_1 changed their profile picture"); + }); + }); });