From 40ead34c082875d3ec91ad2ab850be09ce73fcae Mon Sep 17 00:00:00 2001 From: Paulo Pinto Date: Mon, 26 Jul 2021 17:41:39 +0100 Subject: [PATCH 01/11] Add test for textForPinnedEvent Signed-off-by: Paulo Pinto --- test/TextForEvent-test.ts | 43 ++++++++++++++++++++ test/__snapshots__/TextForEvent-test.ts.snap | 17 ++++++++ 2 files changed, 60 insertions(+) create mode 100644 test/TextForEvent-test.ts create mode 100644 test/__snapshots__/TextForEvent-test.ts.snap diff --git a/test/TextForEvent-test.ts b/test/TextForEvent-test.ts new file mode 100644 index 00000000000..fdfb1c5711e --- /dev/null +++ b/test/TextForEvent-test.ts @@ -0,0 +1,43 @@ +import './skinned-sdk'; + +import { textForEvent } from "../src/TextForEvent"; +import { MatrixEvent } from "matrix-js-sdk"; +import SettingsStore from "../src/settings/SettingsStore"; +import { SettingLevel } from "../src/settings/SettingLevel"; +import renderer from 'react-test-renderer'; + +function mockPinnedEvent( + pinnedMessageIds?: string[], + prevPinnedMessageIds?: string[], +): MatrixEvent { + return new MatrixEvent({ + type: "m.room.pinned_events", + state_key: "", + sender: "@foo:example.com", + content: { + pinned: pinnedMessageIds, + }, + prev_content: { + pinned: prevPinnedMessageIds, + }, + }); +} + +describe("TextForPinnedEvent", () => { + SettingsStore.setValue("feature_pinning", null, SettingLevel.DEVICE, true); + + it("should mention sender", () => { + const event = mockPinnedEvent(); + expect(textForEvent(event)).toBe("@foo:example.com changed the pinned messages for the room."); + }); +}); + +describe("TextForPinnedEvent (JSX)", () => { + SettingsStore.setValue("feature_pinning", null, SettingLevel.DEVICE, true); + + it("should mention sender", () => { + const event = mockPinnedEvent(); + const component = renderer.create(textForEvent(event, true)); + expect(component.toJSON()).toMatchSnapshot(); + }); +}); diff --git a/test/__snapshots__/TextForEvent-test.ts.snap b/test/__snapshots__/TextForEvent-test.ts.snap new file mode 100644 index 00000000000..78fd0910b82 --- /dev/null +++ b/test/__snapshots__/TextForEvent-test.ts.snap @@ -0,0 +1,17 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`TextForPinnedEvent (JSX) should mention sender 1`] = ` + + + @foo:example.com changed the + + + pinned messages + + + for the room. + + +`; From 8fe7df91714d995e7ad3c61e147ec9a7e0494824 Mon Sep 17 00:00:00 2001 From: Paulo Pinto Date: Tue, 27 Jul 2021 15:37:05 +0100 Subject: [PATCH 02/11] When a single message is pinned, link to it Signed-off-by: Paulo Pinto --- src/TextForEvent.tsx | 44 ++++++++++++++++++++ src/i18n/strings/en_EN.json | 1 + test/TextForEvent-test.ts | 34 ++++++++++++--- test/__snapshots__/TextForEvent-test.ts.snap | 42 ++++++++++++++++++- 4 files changed, 114 insertions(+), 7 deletions(-) diff --git a/src/TextForEvent.tsx b/src/TextForEvent.tsx index 7bad8eb50e4..17fede0d210 100644 --- a/src/TextForEvent.tsx +++ b/src/TextForEvent.tsx @@ -408,6 +408,15 @@ function textForPowerEvent(event: MatrixEvent): () => string | null { }); } +const onPinnedOrUnpinnedMessageClick = (messageId: string, roomId: string): void => { + defaultDispatcher.dispatch({ + action: 'view_room', + event_id: messageId, + highlighted: true, + room_id: roomId, + }); +}; + const onPinnedMessagesClick = (): void => { defaultDispatcher.dispatch({ action: Action.SetRightPanelPhase, @@ -420,6 +429,41 @@ function textForPinnedEvent(event: MatrixEvent, allowJSX: boolean): () => string if (!SettingsStore.getValue("feature_pinning")) return null; const senderName = event.sender ? event.sender.name : event.getSender(); + const pinned = event.getContent().pinned ?? []; + const previouslyPinned = event.getPrevContent().pinned ?? []; + const newlyPinned = pinned.filter(item => previouslyPinned.indexOf(item) < 0); + + if (newlyPinned.length === 1) { + // A single message was pinned, include a link to that message. + if (allowJSX) { + const messageId = newlyPinned.pop(); + const roomId = event.getRoomId(); + + return () => ( + + { + _t( + "%(senderName)s pinned a message to this room. See all pinned messages.", + { senderName }, + { + "a": (sub) => + onPinnedOrUnpinnedMessageClick(messageId, roomId)}> + { sub } + , + "b": (sub) => + + { sub } + , + }, + ) + } + + ); + } + + return () => _t("%(senderName)s pinned a message to this room. See all pinned messages.", { senderName }); + } + if (allowJSX) { return () => ( diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 0ca7b9da9e6..eb390afb32c 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -545,6 +545,7 @@ "%(senderName)s made future room history visible to unknown (%(visibility)s).": "%(senderName)s made future room history visible to unknown (%(visibility)s).", "%(senderName)s changed the power level of %(powerLevelDiffText)s.": "%(senderName)s changed the power level of %(powerLevelDiffText)s.", "%(userId)s from %(fromPowerLevel)s to %(toPowerLevel)s": "%(userId)s from %(fromPowerLevel)s to %(toPowerLevel)s", + "%(senderName)s pinned a message to this room. See all pinned messages.": "%(senderName)s pinned a message to this room. See all pinned messages.", "%(senderName)s changed the pinned messages for the room.": "%(senderName)s changed the pinned messages for the room.", "%(senderName)s changed the pinned messages for the room.": "%(senderName)s changed the pinned messages for the room.", "%(widgetName)s widget modified by %(senderName)s": "%(widgetName)s widget modified by %(senderName)s", diff --git a/test/TextForEvent-test.ts b/test/TextForEvent-test.ts index fdfb1c5711e..91c99c7048c 100644 --- a/test/TextForEvent-test.ts +++ b/test/TextForEvent-test.ts @@ -23,20 +23,42 @@ function mockPinnedEvent( }); } -describe("TextForPinnedEvent", () => { +describe("TextForPinnedEvent - newly pinned message(s)", () => { SettingsStore.setValue("feature_pinning", null, SettingLevel.DEVICE, true); - it("should mention sender", () => { - const event = mockPinnedEvent(); + it("mentions message when a single message was pinned, with no previously pinned messages", () => { + const event = mockPinnedEvent(['message-1']); + expect(textForEvent(event)).toBe("@foo:example.com pinned a message to this room. See all pinned messages."); + }); + + it("mentions message when a single message was pinned, with multiple previously pinned messages", () => { + const event = mockPinnedEvent(['message-3'], ['message-1', 'message-2']); + expect(textForEvent(event)).toBe("@foo:example.com pinned a message to this room. See all pinned messages."); + }); + + it("shows generic text when multiple messages were pinned", () => { + const event = mockPinnedEvent(['message-2', 'message-3'], ['message-1']); expect(textForEvent(event)).toBe("@foo:example.com changed the pinned messages for the room."); }); }); -describe("TextForPinnedEvent (JSX)", () => { +describe("TextForPinnedEvent - newly pinned message(s) (JSX)", () => { SettingsStore.setValue("feature_pinning", null, SettingLevel.DEVICE, true); - it("should mention sender", () => { - const event = mockPinnedEvent(); + it("mentions message when a single message was pinned, with no previously pinned messages", () => { + const event = mockPinnedEvent(['message-1']); + const component = renderer.create(textForEvent(event, true)); + expect(component.toJSON()).toMatchSnapshot(); + }); + + it("mentions message when a single message was pinned, with multiple previously pinned messages", () => { + const event = mockPinnedEvent(['message-3'], ['message-1', 'message-2']); + const component = renderer.create(textForEvent(event, true)); + expect(component.toJSON()).toMatchSnapshot(); + }); + + it("shows generic text when multiple messages were pinned", () => { + const event = mockPinnedEvent(['message-2', 'message-3'], ['message-1']); const component = renderer.create(textForEvent(event, true)); expect(component.toJSON()).toMatchSnapshot(); }); diff --git a/test/__snapshots__/TextForEvent-test.ts.snap b/test/__snapshots__/TextForEvent-test.ts.snap index 78fd0910b82..41ccbc08570 100644 --- a/test/__snapshots__/TextForEvent-test.ts.snap +++ b/test/__snapshots__/TextForEvent-test.ts.snap @@ -1,6 +1,46 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`TextForPinnedEvent (JSX) should mention sender 1`] = ` +exports[`TextForPinnedEvent - newly pinned message(s) (JSX) mentions message when a single message was pinned, with multiple previously pinned messages 1`] = ` + + + @foo:example.com pinned + + a message + + to this room. See all + + pinned messages + + . + + +`; + +exports[`TextForPinnedEvent - newly pinned message(s) (JSX) mentions message when a single message was pinned, with no previously pinned messages 1`] = ` + + + @foo:example.com pinned + + a message + + to this room. See all + + pinned messages + + . + + +`; + +exports[`TextForPinnedEvent - newly pinned message(s) (JSX) shows generic text when multiple messages were pinned 1`] = ` @foo:example.com changed the From 3f2dadf0fe226d572333653c845049cbb6610800 Mon Sep 17 00:00:00 2001 From: Paulo Pinto Date: Tue, 27 Jul 2021 16:58:53 +0100 Subject: [PATCH 03/11] When a single message is unpinned, link to it Signed-off-by: Paulo Pinto --- src/TextForEvent.tsx | 33 +++++++++++- src/i18n/strings/en_EN.json | 1 + test/TextForEvent-test.ts | 45 +++++++++++++++- test/__snapshots__/TextForEvent-test.ts.snap | 56 ++++++++++++++++++++ 4 files changed, 132 insertions(+), 3 deletions(-) diff --git a/src/TextForEvent.tsx b/src/TextForEvent.tsx index 17fede0d210..1033ac9fd8c 100644 --- a/src/TextForEvent.tsx +++ b/src/TextForEvent.tsx @@ -428,16 +428,17 @@ const onPinnedMessagesClick = (): void => { function textForPinnedEvent(event: MatrixEvent, allowJSX: boolean): () => string | JSX.Element | null { if (!SettingsStore.getValue("feature_pinning")) return null; const senderName = event.sender ? event.sender.name : event.getSender(); + const roomId = event.getRoomId(); const pinned = event.getContent().pinned ?? []; const previouslyPinned = event.getPrevContent().pinned ?? []; const newlyPinned = pinned.filter(item => previouslyPinned.indexOf(item) < 0); + const newlyUnpinned = previouslyPinned.filter(item => pinned.indexOf(item) < 0); if (newlyPinned.length === 1) { // A single message was pinned, include a link to that message. if (allowJSX) { const messageId = newlyPinned.pop(); - const roomId = event.getRoomId(); return () => ( @@ -464,6 +465,36 @@ function textForPinnedEvent(event: MatrixEvent, allowJSX: boolean): () => string return () => _t("%(senderName)s pinned a message to this room. See all pinned messages.", { senderName }); } + if (newlyUnpinned.length === 1) { + // A single message was unpinned, include a link to that message. + if (allowJSX) { + const messageId = newlyUnpinned.pop(); + + return () => ( + + { + _t( + "%(senderName)s unpinned a message from this room. See all pinned messages.", + { senderName }, + { + "a": (sub) => + onPinnedOrUnpinnedMessageClick(messageId, roomId)}> + { sub } + , + "b": (sub) => + + { sub } + , + }, + ) + } + + ); + } + + return () => _t("%(senderName)s unpinned a message from this room. See all pinned messages.", { senderName }); + } + if (allowJSX) { return () => ( diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index eb390afb32c..91325444c37 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -546,6 +546,7 @@ "%(senderName)s changed the power level of %(powerLevelDiffText)s.": "%(senderName)s changed the power level of %(powerLevelDiffText)s.", "%(userId)s from %(fromPowerLevel)s to %(toPowerLevel)s": "%(userId)s from %(fromPowerLevel)s to %(toPowerLevel)s", "%(senderName)s pinned a message to this room. See all pinned messages.": "%(senderName)s pinned a message to this room. See all pinned messages.", + "%(senderName)s unpinned a message from this room. See all pinned messages.": "%(senderName)s unpinned a message from this room. See all pinned messages.", "%(senderName)s changed the pinned messages for the room.": "%(senderName)s changed the pinned messages for the room.", "%(senderName)s changed the pinned messages for the room.": "%(senderName)s changed the pinned messages for the room.", "%(widgetName)s widget modified by %(senderName)s": "%(widgetName)s widget modified by %(senderName)s", diff --git a/test/TextForEvent-test.ts b/test/TextForEvent-test.ts index 91c99c7048c..d8c708eea3a 100644 --- a/test/TextForEvent-test.ts +++ b/test/TextForEvent-test.ts @@ -37,7 +37,7 @@ describe("TextForPinnedEvent - newly pinned message(s)", () => { }); it("shows generic text when multiple messages were pinned", () => { - const event = mockPinnedEvent(['message-2', 'message-3'], ['message-1']); + const event = mockPinnedEvent(['message-1', 'message-2', 'message-3'], ['message-1']); expect(textForEvent(event)).toBe("@foo:example.com changed the pinned messages for the room."); }); }); @@ -58,7 +58,48 @@ describe("TextForPinnedEvent - newly pinned message(s) (JSX)", () => { }); it("shows generic text when multiple messages were pinned", () => { - const event = mockPinnedEvent(['message-2', 'message-3'], ['message-1']); + const event = mockPinnedEvent(['message-1', 'message-2', 'message-3'], ['message-1']); + const component = renderer.create(textForEvent(event, true)); + expect(component.toJSON()).toMatchSnapshot(); + }); +}); + +describe("TextForPinnedEvent - newly unpinned message(s)", () => { + SettingsStore.setValue("feature_pinning", null, SettingLevel.DEVICE, true); + + it("mentions message when a single message was unpinned, with a single message previously pinned", () => { + const event = mockPinnedEvent([], ['message-1']); + expect(textForEvent(event)).toBe("@foo:example.com unpinned a message from this room. See all pinned messages."); + }); + + it("mentions message when a single message was unpinned, with multiple previously pinned messages", () => { + const event = mockPinnedEvent(['message-2'], ['message-1', 'message-2']); + expect(textForEvent(event)).toBe("@foo:example.com unpinned a message from this room. See all pinned messages."); + }); + + it("shows generic text when multiple messages were unpinned", () => { + const event = mockPinnedEvent(['message-3'], ['message-1', 'message-2', 'message-3']); + expect(textForEvent(event)).toBe("@foo:example.com changed the pinned messages for the room."); + }); +}); + +describe("TextForPinnedEvent - newly unpinned message(s) (JSX)", () => { + SettingsStore.setValue("feature_pinning", null, SettingLevel.DEVICE, true); + + it("mentions message when a single message was unpinned, with a single message previously pinned", () => { + const event = mockPinnedEvent([], ['message-1']); + const component = renderer.create(textForEvent(event, true)); + expect(component.toJSON()).toMatchSnapshot(); + }); + + it("mentions message when a single message was unpinned, with multiple previously pinned messages", () => { + const event = mockPinnedEvent(['message-2'], ['message-1', 'message-2']); + const component = renderer.create(textForEvent(event, true)); + expect(component.toJSON()).toMatchSnapshot(); + }); + + it("shows generic text when multiple messages were unpinned", () => { + const event = mockPinnedEvent(['message-3'], ['message-1', 'message-2', 'message-3']); const component = renderer.create(textForEvent(event, true)); expect(component.toJSON()).toMatchSnapshot(); }); diff --git a/test/__snapshots__/TextForEvent-test.ts.snap b/test/__snapshots__/TextForEvent-test.ts.snap index 41ccbc08570..124f7e06631 100644 --- a/test/__snapshots__/TextForEvent-test.ts.snap +++ b/test/__snapshots__/TextForEvent-test.ts.snap @@ -55,3 +55,59 @@ exports[`TextForPinnedEvent - newly pinned message(s) (JSX) shows generic text w `; + +exports[`TextForPinnedEvent - newly unpinned message(s) (JSX) mentions message when a single message was unpinned, with a single message previously pinned 1`] = ` + + + @foo:example.com unpinned + + a message + + from this room. See all + + pinned messages + + . + + +`; + +exports[`TextForPinnedEvent - newly unpinned message(s) (JSX) mentions message when a single message was unpinned, with multiple previously pinned messages 1`] = ` + + + @foo:example.com unpinned + + a message + + from this room. See all + + pinned messages + + . + + +`; + +exports[`TextForPinnedEvent - newly unpinned message(s) (JSX) shows generic text when multiple messages were unpinned 1`] = ` + + + @foo:example.com changed the + + + pinned messages + + + for the room. + + +`; From 400e772594b35898e5aa5a2df1d18e7faad46226 Mon Sep 17 00:00:00 2001 From: Paulo Pinto Date: Tue, 27 Jul 2021 17:53:33 +0100 Subject: [PATCH 04/11] Fix formatting Signed-off-by: Paulo Pinto --- test/TextForEvent-test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/TextForEvent-test.ts b/test/TextForEvent-test.ts index d8c708eea3a..0b3b73613aa 100644 --- a/test/TextForEvent-test.ts +++ b/test/TextForEvent-test.ts @@ -69,12 +69,16 @@ describe("TextForPinnedEvent - newly unpinned message(s)", () => { it("mentions message when a single message was unpinned, with a single message previously pinned", () => { const event = mockPinnedEvent([], ['message-1']); - expect(textForEvent(event)).toBe("@foo:example.com unpinned a message from this room. See all pinned messages."); + expect(textForEvent(event)).toBe( + "@foo:example.com unpinned a message from this room. See all pinned messages.", + ); }); it("mentions message when a single message was unpinned, with multiple previously pinned messages", () => { const event = mockPinnedEvent(['message-2'], ['message-1', 'message-2']); - expect(textForEvent(event)).toBe("@foo:example.com unpinned a message from this room. See all pinned messages."); + expect(textForEvent(event)).toBe( + "@foo:example.com unpinned a message from this room. See all pinned messages.", + ); }); it("shows generic text when multiple messages were unpinned", () => { From 6c945ee4ed2999bd68a32798a3f3869111ac3e45 Mon Sep 17 00:00:00 2001 From: Paulo Pinto Date: Tue, 27 Jul 2021 17:58:19 +0100 Subject: [PATCH 05/11] Add missing translations Signed-off-by: Paulo Pinto --- src/i18n/strings/en_EN.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 91325444c37..1fc9689a58c 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -546,7 +546,9 @@ "%(senderName)s changed the power level of %(powerLevelDiffText)s.": "%(senderName)s changed the power level of %(powerLevelDiffText)s.", "%(userId)s from %(fromPowerLevel)s to %(toPowerLevel)s": "%(userId)s from %(fromPowerLevel)s to %(toPowerLevel)s", "%(senderName)s pinned a message to this room. See all pinned messages.": "%(senderName)s pinned a message to this room. See all pinned messages.", + "%(senderName)s pinned a message to this room. See all pinned messages.": "%(senderName)s pinned a message to this room. See all pinned messages.", "%(senderName)s unpinned a message from this room. See all pinned messages.": "%(senderName)s unpinned a message from this room. See all pinned messages.", + "%(senderName)s unpinned a message from this room. See all pinned messages.": "%(senderName)s unpinned a message from this room. See all pinned messages.", "%(senderName)s changed the pinned messages for the room.": "%(senderName)s changed the pinned messages for the room.", "%(senderName)s changed the pinned messages for the room.": "%(senderName)s changed the pinned messages for the room.", "%(widgetName)s widget modified by %(senderName)s": "%(widgetName)s widget modified by %(senderName)s", From 94d3dd6bbcf73b72d9270864199c0f5bcb956c93 Mon Sep 17 00:00:00 2001 From: Paulo Pinto Date: Tue, 10 Aug 2021 15:50:23 +0100 Subject: [PATCH 06/11] Collapse lines together to reduce line/indent spam Signed-off-by: Paulo Pinto --- src/TextForEvent.tsx | 72 ++++++++++++++++++++------------------------ 1 file changed, 33 insertions(+), 39 deletions(-) diff --git a/src/TextForEvent.tsx b/src/TextForEvent.tsx index 1033ac9fd8c..a85d44ecb3d 100644 --- a/src/TextForEvent.tsx +++ b/src/TextForEvent.tsx @@ -442,22 +442,20 @@ function textForPinnedEvent(event: MatrixEvent, allowJSX: boolean): () => string return () => ( - { - _t( - "%(senderName)s pinned a message to this room. See all pinned messages.", - { senderName }, - { - "a": (sub) => - onPinnedOrUnpinnedMessageClick(messageId, roomId)}> - { sub } - , - "b": (sub) => - - { sub } - , - }, - ) - } + { _t( + "%(senderName)s pinned a message to this room. See all pinned messages.", + { senderName }, + { + "a": (sub) => + onPinnedOrUnpinnedMessageClick(messageId, roomId)}> + { sub } + , + "b": (sub) => + + { sub } + , + }, + ) } ); } @@ -472,22 +470,20 @@ function textForPinnedEvent(event: MatrixEvent, allowJSX: boolean): () => string return () => ( - { - _t( - "%(senderName)s unpinned a message from this room. See all pinned messages.", - { senderName }, - { - "a": (sub) => - onPinnedOrUnpinnedMessageClick(messageId, roomId)}> - { sub } - , - "b": (sub) => - - { sub } - , - }, - ) - } + { _t( + "%(senderName)s unpinned a message from this room. See all pinned messages.", + { senderName }, + { + "a": (sub) => + onPinnedOrUnpinnedMessageClick(messageId, roomId)}> + { sub } + , + "b": (sub) => + + { sub } + , + }, + ) } ); } @@ -498,13 +494,11 @@ function textForPinnedEvent(event: MatrixEvent, allowJSX: boolean): () => string if (allowJSX) { return () => ( - { - _t( - "%(senderName)s changed the pinned messages for the room.", - { senderName }, - { "a": (sub) => { sub } }, - ) - } + { _t( + "%(senderName)s changed the pinned messages for the room.", + { senderName }, + { "a": (sub) => { sub } }, + ) } ); } From 45fd3d83b07814e31788da9421b2c3355dc7f422 Mon Sep 17 00:00:00 2001 From: Paulo Pinto Date: Tue, 10 Aug 2021 16:06:33 +0100 Subject: [PATCH 07/11] Refactor tests So that there's one top level `describe('TextForEvent')`, followed by a nested `describe('textForPinnedEvent')`, containting all the `it()`s. Signed-off-by: Paulo Pinto --- test/TextForEvent-test.ts | 142 ++++++++----------- test/__snapshots__/TextForEvent-test.ts.snap | 36 ++--- 2 files changed, 77 insertions(+), 101 deletions(-) diff --git a/test/TextForEvent-test.ts b/test/TextForEvent-test.ts index 0b3b73613aa..b50996c186d 100644 --- a/test/TextForEvent-test.ts +++ b/test/TextForEvent-test.ts @@ -23,88 +23,64 @@ function mockPinnedEvent( }); } -describe("TextForPinnedEvent - newly pinned message(s)", () => { - SettingsStore.setValue("feature_pinning", null, SettingLevel.DEVICE, true); - - it("mentions message when a single message was pinned, with no previously pinned messages", () => { - const event = mockPinnedEvent(['message-1']); - expect(textForEvent(event)).toBe("@foo:example.com pinned a message to this room. See all pinned messages."); - }); - - it("mentions message when a single message was pinned, with multiple previously pinned messages", () => { - const event = mockPinnedEvent(['message-3'], ['message-1', 'message-2']); - expect(textForEvent(event)).toBe("@foo:example.com pinned a message to this room. See all pinned messages."); - }); - - it("shows generic text when multiple messages were pinned", () => { - const event = mockPinnedEvent(['message-1', 'message-2', 'message-3'], ['message-1']); - expect(textForEvent(event)).toBe("@foo:example.com changed the pinned messages for the room."); - }); -}); - -describe("TextForPinnedEvent - newly pinned message(s) (JSX)", () => { - SettingsStore.setValue("feature_pinning", null, SettingLevel.DEVICE, true); - - it("mentions message when a single message was pinned, with no previously pinned messages", () => { - const event = mockPinnedEvent(['message-1']); - const component = renderer.create(textForEvent(event, true)); - expect(component.toJSON()).toMatchSnapshot(); - }); - - it("mentions message when a single message was pinned, with multiple previously pinned messages", () => { - const event = mockPinnedEvent(['message-3'], ['message-1', 'message-2']); - const component = renderer.create(textForEvent(event, true)); - expect(component.toJSON()).toMatchSnapshot(); - }); - - it("shows generic text when multiple messages were pinned", () => { - const event = mockPinnedEvent(['message-1', 'message-2', 'message-3'], ['message-1']); - const component = renderer.create(textForEvent(event, true)); - expect(component.toJSON()).toMatchSnapshot(); - }); -}); - -describe("TextForPinnedEvent - newly unpinned message(s)", () => { - SettingsStore.setValue("feature_pinning", null, SettingLevel.DEVICE, true); - - it("mentions message when a single message was unpinned, with a single message previously pinned", () => { - const event = mockPinnedEvent([], ['message-1']); - expect(textForEvent(event)).toBe( - "@foo:example.com unpinned a message from this room. See all pinned messages.", - ); - }); - - it("mentions message when a single message was unpinned, with multiple previously pinned messages", () => { - const event = mockPinnedEvent(['message-2'], ['message-1', 'message-2']); - expect(textForEvent(event)).toBe( - "@foo:example.com unpinned a message from this room. See all pinned messages.", - ); - }); - - it("shows generic text when multiple messages were unpinned", () => { - const event = mockPinnedEvent(['message-3'], ['message-1', 'message-2', 'message-3']); - expect(textForEvent(event)).toBe("@foo:example.com changed the pinned messages for the room."); - }); -}); - -describe("TextForPinnedEvent - newly unpinned message(s) (JSX)", () => { - SettingsStore.setValue("feature_pinning", null, SettingLevel.DEVICE, true); - - it("mentions message when a single message was unpinned, with a single message previously pinned", () => { - const event = mockPinnedEvent([], ['message-1']); - const component = renderer.create(textForEvent(event, true)); - expect(component.toJSON()).toMatchSnapshot(); - }); - - it("mentions message when a single message was unpinned, with multiple previously pinned messages", () => { - const event = mockPinnedEvent(['message-2'], ['message-1', 'message-2']); - const component = renderer.create(textForEvent(event, true)); - expect(component.toJSON()).toMatchSnapshot(); - }); - - it("shows generic text when multiple messages were unpinned", () => { - const event = mockPinnedEvent(['message-3'], ['message-1', 'message-2', 'message-3']); - const component = renderer.create(textForEvent(event, true)); - expect(component.toJSON()).toMatchSnapshot(); +describe('TextForEvent', () => { + describe("TextForPinnedEvent", () => { + SettingsStore.setValue("feature_pinning", null, SettingLevel.DEVICE, true); + + it("mentions message when a single message was pinned, with no previously pinned messages", () => { + const event = mockPinnedEvent(['message-1']); + expect(textForEvent(event)).toBe( + "@foo:example.com pinned a message to this room. See all pinned messages.", + ); + + const component = renderer.create(textForEvent(event, true)); + expect(component.toJSON()).toMatchSnapshot(); + }); + + it("mentions message when a single message was pinned, with multiple previously pinned messages", () => { + const event = mockPinnedEvent(['message-3'], ['message-1', 'message-2']); + expect(textForEvent(event)).toBe( + "@foo:example.com pinned a message to this room. See all pinned messages.", + ); + + const component = renderer.create(textForEvent(event, true)); + expect(component.toJSON()).toMatchSnapshot(); + }); + + it("shows generic text when multiple messages were pinned", () => { + const event = mockPinnedEvent(['message-1', 'message-2', 'message-3'], ['message-1']); + expect(textForEvent(event)).toBe("@foo:example.com changed the pinned messages for the room."); + + const component = renderer.create(textForEvent(event, true)); + expect(component.toJSON()).toMatchSnapshot(); + }); + + it("mentions message when a single message was unpinned, with a single message previously pinned", () => { + const event = mockPinnedEvent([], ['message-1']); + expect(textForEvent(event)).toBe( + "@foo:example.com unpinned a message from this room. See all pinned messages.", + ); + + const component = renderer.create(textForEvent(event, true)); + expect(component.toJSON()).toMatchSnapshot(); + }); + + it("mentions message when a single message was unpinned, with multiple previously pinned messages", () => { + const event = mockPinnedEvent(['message-2'], ['message-1', 'message-2']); + expect(textForEvent(event)).toBe( + "@foo:example.com unpinned a message from this room. See all pinned messages.", + ); + + const component = renderer.create(textForEvent(event, true)); + expect(component.toJSON()).toMatchSnapshot(); + }); + + it("shows generic text when multiple messages were unpinned", () => { + const event = mockPinnedEvent(['message-3'], ['message-1', 'message-2', 'message-3']); + expect(textForEvent(event)).toBe("@foo:example.com changed the pinned messages for the room."); + + const component = renderer.create(textForEvent(event, true)); + expect(component.toJSON()).toMatchSnapshot(); + }); }); }); diff --git a/test/__snapshots__/TextForEvent-test.ts.snap b/test/__snapshots__/TextForEvent-test.ts.snap index 124f7e06631..2b73dbeb3d8 100644 --- a/test/__snapshots__/TextForEvent-test.ts.snap +++ b/test/__snapshots__/TextForEvent-test.ts.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`TextForPinnedEvent - newly pinned message(s) (JSX) mentions message when a single message was pinned, with multiple previously pinned messages 1`] = ` +exports[`TextForEvent TextForPinnedEvent mentions message when a single message was pinned, with multiple previously pinned messages 1`] = ` @foo:example.com pinned @@ -20,7 +20,7 @@ exports[`TextForPinnedEvent - newly pinned message(s) (JSX) mentions message whe `; -exports[`TextForPinnedEvent - newly pinned message(s) (JSX) mentions message when a single message was pinned, with no previously pinned messages 1`] = ` +exports[`TextForEvent TextForPinnedEvent mentions message when a single message was pinned, with no previously pinned messages 1`] = ` @foo:example.com pinned @@ -40,23 +40,27 @@ exports[`TextForPinnedEvent - newly pinned message(s) (JSX) mentions message whe `; -exports[`TextForPinnedEvent - newly pinned message(s) (JSX) shows generic text when multiple messages were pinned 1`] = ` +exports[`TextForEvent TextForPinnedEvent mentions message when a single message was unpinned, with a single message previously pinned 1`] = ` - @foo:example.com changed the + @foo:example.com unpinned + + a message + + from this room. See all - pinned messages - - for the room. + . `; -exports[`TextForPinnedEvent - newly unpinned message(s) (JSX) mentions message when a single message was unpinned, with a single message previously pinned 1`] = ` +exports[`TextForEvent TextForPinnedEvent mentions message when a single message was unpinned, with multiple previously pinned messages 1`] = ` @foo:example.com unpinned @@ -76,27 +80,23 @@ exports[`TextForPinnedEvent - newly unpinned message(s) (JSX) mentions message w `; -exports[`TextForPinnedEvent - newly unpinned message(s) (JSX) mentions message when a single message was unpinned, with multiple previously pinned messages 1`] = ` +exports[`TextForEvent TextForPinnedEvent shows generic text when multiple messages were pinned 1`] = ` - @foo:example.com unpinned - - a message - - from this room. See all + @foo:example.com changed the + pinned messages + - . + for the room. `; -exports[`TextForPinnedEvent - newly unpinned message(s) (JSX) shows generic text when multiple messages were unpinned 1`] = ` +exports[`TextForEvent TextForPinnedEvent shows generic text when multiple messages were unpinned 1`] = ` @foo:example.com changed the From 79cf69bedb776175c3c7409394091f3bef76c24e Mon Sep 17 00:00:00 2001 From: Paulo Pinto Date: Wed, 11 Aug 2021 11:40:33 +0100 Subject: [PATCH 08/11] Refactor tests so that snapshots aren't used Signed-off-by: Paulo Pinto --- test/TextForEvent-test.ts | 85 +++++++++----- test/__snapshots__/TextForEvent-test.ts.snap | 113 ------------------- 2 files changed, 59 insertions(+), 139 deletions(-) delete mode 100644 test/__snapshots__/TextForEvent-test.ts.snap diff --git a/test/TextForEvent-test.ts b/test/TextForEvent-test.ts index b50996c186d..ac9c8eba4dc 100644 --- a/test/TextForEvent-test.ts +++ b/test/TextForEvent-test.ts @@ -23,64 +23,97 @@ function mockPinnedEvent( }); } +// Helper function that renders a component to a plain text string. +// Once snapshots are introduced in tests, this function will no longer be necessary, +// and should be replaced with snapshots. +function renderComponent(component): string { + const serializeObject = (object): string => { + if (typeof object === 'string') { + return object === ' ' ? '' : object; + } + + if (Array.isArray(object) && object.length === 1 && typeof object[0] === 'string') { + return object[0]; + } + + if (object['type'] !== undefined && typeof object['children'] !== undefined) { + return serializeObject(object.children); + } + + if (!Array.isArray(object)) { + return ''; + } + + return object.map(child => { + return serializeObject(child); + }).join(''); + }; + + return serializeObject(component.toJSON()); +} + describe('TextForEvent', () => { describe("TextForPinnedEvent", () => { SettingsStore.setValue("feature_pinning", null, SettingLevel.DEVICE, true); it("mentions message when a single message was pinned, with no previously pinned messages", () => { const event = mockPinnedEvent(['message-1']); - expect(textForEvent(event)).toBe( - "@foo:example.com pinned a message to this room. See all pinned messages.", - ); - + const plainText = textForEvent(event); const component = renderer.create(textForEvent(event, true)); - expect(component.toJSON()).toMatchSnapshot(); + + const expectedText = "@foo:example.com pinned a message to this room. See all pinned messages."; + expect(plainText).toBe(expectedText); + expect(renderComponent(component)).toBe(expectedText); }); it("mentions message when a single message was pinned, with multiple previously pinned messages", () => { const event = mockPinnedEvent(['message-3'], ['message-1', 'message-2']); - expect(textForEvent(event)).toBe( - "@foo:example.com pinned a message to this room. See all pinned messages.", - ); - + const plainText = textForEvent(event); const component = renderer.create(textForEvent(event, true)); - expect(component.toJSON()).toMatchSnapshot(); + + const expectedText = "@foo:example.com pinned a message to this room. See all pinned messages."; + expect(plainText).toBe(expectedText); + expect(renderComponent(component)).toBe(expectedText); }); it("shows generic text when multiple messages were pinned", () => { const event = mockPinnedEvent(['message-1', 'message-2', 'message-3'], ['message-1']); - expect(textForEvent(event)).toBe("@foo:example.com changed the pinned messages for the room."); - + const plainText = textForEvent(event); const component = renderer.create(textForEvent(event, true)); - expect(component.toJSON()).toMatchSnapshot(); + + const expectedText = "@foo:example.com changed the pinned messages for the room."; + expect(plainText).toBe(expectedText); + expect(renderComponent(component)).toBe(expectedText); }); it("mentions message when a single message was unpinned, with a single message previously pinned", () => { const event = mockPinnedEvent([], ['message-1']); - expect(textForEvent(event)).toBe( - "@foo:example.com unpinned a message from this room. See all pinned messages.", - ); - + const plainText = textForEvent(event); const component = renderer.create(textForEvent(event, true)); - expect(component.toJSON()).toMatchSnapshot(); + + const expectedText = "@foo:example.com unpinned a message from this room. See all pinned messages."; + expect(plainText).toBe(expectedText); + expect(renderComponent(component)).toBe(expectedText); }); it("mentions message when a single message was unpinned, with multiple previously pinned messages", () => { const event = mockPinnedEvent(['message-2'], ['message-1', 'message-2']); - expect(textForEvent(event)).toBe( - "@foo:example.com unpinned a message from this room. See all pinned messages.", - ); - + const plainText = textForEvent(event); const component = renderer.create(textForEvent(event, true)); - expect(component.toJSON()).toMatchSnapshot(); + + const expectedText = "@foo:example.com unpinned a message from this room. See all pinned messages."; + expect(plainText).toBe(expectedText); + expect(renderComponent(component)).toBe(expectedText); }); it("shows generic text when multiple messages were unpinned", () => { const event = mockPinnedEvent(['message-3'], ['message-1', 'message-2', 'message-3']); - expect(textForEvent(event)).toBe("@foo:example.com changed the pinned messages for the room."); - + const plainText = textForEvent(event); const component = renderer.create(textForEvent(event, true)); - expect(component.toJSON()).toMatchSnapshot(); + + const expectedText = "@foo:example.com changed the pinned messages for the room."; + expect(plainText).toBe(expectedText); + expect(renderComponent(component)).toBe(expectedText); }); }); }); diff --git a/test/__snapshots__/TextForEvent-test.ts.snap b/test/__snapshots__/TextForEvent-test.ts.snap deleted file mode 100644 index 2b73dbeb3d8..00000000000 --- a/test/__snapshots__/TextForEvent-test.ts.snap +++ /dev/null @@ -1,113 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`TextForEvent TextForPinnedEvent mentions message when a single message was pinned, with multiple previously pinned messages 1`] = ` - - - @foo:example.com pinned - - a message - - to this room. See all - - pinned messages - - . - - -`; - -exports[`TextForEvent TextForPinnedEvent mentions message when a single message was pinned, with no previously pinned messages 1`] = ` - - - @foo:example.com pinned - - a message - - to this room. See all - - pinned messages - - . - - -`; - -exports[`TextForEvent TextForPinnedEvent mentions message when a single message was unpinned, with a single message previously pinned 1`] = ` - - - @foo:example.com unpinned - - a message - - from this room. See all - - pinned messages - - . - - -`; - -exports[`TextForEvent TextForPinnedEvent mentions message when a single message was unpinned, with multiple previously pinned messages 1`] = ` - - - @foo:example.com unpinned - - a message - - from this room. See all - - pinned messages - - . - - -`; - -exports[`TextForEvent TextForPinnedEvent shows generic text when multiple messages were pinned 1`] = ` - - - @foo:example.com changed the - - - pinned messages - - - for the room. - - -`; - -exports[`TextForEvent TextForPinnedEvent shows generic text when multiple messages were unpinned 1`] = ` - - - @foo:example.com changed the - - - pinned messages - - - for the room. - - -`; From ca8832f6faaeef9655000520d60f30fd502bbf83 Mon Sep 17 00:00:00 2001 From: Paulo Pinto Date: Wed, 11 Aug 2021 14:47:49 +0100 Subject: [PATCH 09/11] Fix test case The test case is: "mentions message when a single message was pinned, with multiple previously pinned messages" However, the test case was also unpinning messages. That is now fixed. Signed-off-by: Paulo Pinto --- test/TextForEvent-test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/TextForEvent-test.ts b/test/TextForEvent-test.ts index ac9c8eba4dc..30bc68751a8 100644 --- a/test/TextForEvent-test.ts +++ b/test/TextForEvent-test.ts @@ -67,7 +67,7 @@ describe('TextForEvent', () => { }); it("mentions message when a single message was pinned, with multiple previously pinned messages", () => { - const event = mockPinnedEvent(['message-3'], ['message-1', 'message-2']); + const event = mockPinnedEvent(['message-1', 'message-2', 'message-3'], ['message-1', 'message-2']); const plainText = textForEvent(event); const component = renderer.create(textForEvent(event, true)); From a51b1141717fa12e7fb2517bd2ff754286f9a368 Mon Sep 17 00:00:00 2001 From: Paulo Pinto Date: Wed, 11 Aug 2021 14:51:36 +0100 Subject: [PATCH 10/11] Change order of test cases Just moving test cases so that "generic message" ones are grouped at the bottom. Signed-off-by: Paulo Pinto --- test/TextForEvent-test.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/TextForEvent-test.ts b/test/TextForEvent-test.ts index 30bc68751a8..285d4de2329 100644 --- a/test/TextForEvent-test.ts +++ b/test/TextForEvent-test.ts @@ -76,18 +76,18 @@ describe('TextForEvent', () => { expect(renderComponent(component)).toBe(expectedText); }); - it("shows generic text when multiple messages were pinned", () => { - const event = mockPinnedEvent(['message-1', 'message-2', 'message-3'], ['message-1']); + it("mentions message when a single message was unpinned, with a single message previously pinned", () => { + const event = mockPinnedEvent([], ['message-1']); const plainText = textForEvent(event); const component = renderer.create(textForEvent(event, true)); - const expectedText = "@foo:example.com changed the pinned messages for the room."; + const expectedText = "@foo:example.com unpinned a message from this room. See all pinned messages."; expect(plainText).toBe(expectedText); expect(renderComponent(component)).toBe(expectedText); }); - it("mentions message when a single message was unpinned, with a single message previously pinned", () => { - const event = mockPinnedEvent([], ['message-1']); + it("mentions message when a single message was unpinned, with multiple previously pinned messages", () => { + const event = mockPinnedEvent(['message-2'], ['message-1', 'message-2']); const plainText = textForEvent(event); const component = renderer.create(textForEvent(event, true)); @@ -96,12 +96,12 @@ describe('TextForEvent', () => { expect(renderComponent(component)).toBe(expectedText); }); - it("mentions message when a single message was unpinned, with multiple previously pinned messages", () => { - const event = mockPinnedEvent(['message-2'], ['message-1', 'message-2']); + it("shows generic text when multiple messages were pinned", () => { + const event = mockPinnedEvent(['message-1', 'message-2', 'message-3'], ['message-1']); const plainText = textForEvent(event); const component = renderer.create(textForEvent(event, true)); - const expectedText = "@foo:example.com unpinned a message from this room. See all pinned messages."; + const expectedText = "@foo:example.com changed the pinned messages for the room."; expect(plainText).toBe(expectedText); expect(renderComponent(component)).toBe(expectedText); }); From aaeb9969a47db6abdf7200668ea35d4f68c1d52e Mon Sep 17 00:00:00 2001 From: Paulo Pinto Date: Wed, 11 Aug 2021 14:56:59 +0100 Subject: [PATCH 11/11] Handle case where one message is pinned, and another unpinned Signed-off-by: Paulo Pinto --- src/TextForEvent.tsx | 4 ++-- test/TextForEvent-test.ts | 10 ++++++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/TextForEvent.tsx b/src/TextForEvent.tsx index a85d44ecb3d..1c5e9ec6f01 100644 --- a/src/TextForEvent.tsx +++ b/src/TextForEvent.tsx @@ -435,7 +435,7 @@ function textForPinnedEvent(event: MatrixEvent, allowJSX: boolean): () => string const newlyPinned = pinned.filter(item => previouslyPinned.indexOf(item) < 0); const newlyUnpinned = previouslyPinned.filter(item => pinned.indexOf(item) < 0); - if (newlyPinned.length === 1) { + if (newlyPinned.length === 1 && newlyUnpinned.length === 0) { // A single message was pinned, include a link to that message. if (allowJSX) { const messageId = newlyPinned.pop(); @@ -463,7 +463,7 @@ function textForPinnedEvent(event: MatrixEvent, allowJSX: boolean): () => string return () => _t("%(senderName)s pinned a message to this room. See all pinned messages.", { senderName }); } - if (newlyUnpinned.length === 1) { + if (newlyUnpinned.length === 1 && newlyPinned.length === 0) { // A single message was unpinned, include a link to that message. if (allowJSX) { const messageId = newlyUnpinned.pop(); diff --git a/test/TextForEvent-test.ts b/test/TextForEvent-test.ts index 285d4de2329..b8a459af67a 100644 --- a/test/TextForEvent-test.ts +++ b/test/TextForEvent-test.ts @@ -115,5 +115,15 @@ describe('TextForEvent', () => { expect(plainText).toBe(expectedText); expect(renderComponent(component)).toBe(expectedText); }); + + it("shows generic text when one message was pinned, and another unpinned", () => { + const event = mockPinnedEvent(['message-2'], ['message-1']); + const plainText = textForEvent(event); + const component = renderer.create(textForEvent(event, true)); + + const expectedText = "@foo:example.com changed the pinned messages for the room."; + expect(plainText).toBe(expectedText); + expect(renderComponent(component)).toBe(expectedText); + }); }); });