From 8be1eb721d0ae58cd5756848ced83e912cc88539 Mon Sep 17 00:00:00 2001 From: Germain Date: Tue, 25 Oct 2022 12:24:43 +0100 Subject: [PATCH 1/7] Show thread notification if thread timeline is closed --- src/Notifier.ts | 9 +++++++- src/components/structures/ThreadView.tsx | 10 +++++++++ src/dispatcher/actions.ts | 5 +++++ src/dispatcher/payloads/ThreadPayload.ts | 26 ++++++++++++++++++++++++ src/stores/RoomViewStore.tsx | 19 +++++++++++++++++ 5 files changed, 68 insertions(+), 1 deletion(-) create mode 100644 src/dispatcher/payloads/ThreadPayload.ts diff --git a/src/Notifier.ts b/src/Notifier.ts index cc84acb2fab..8cce912bd2b 100644 --- a/src/Notifier.ts +++ b/src/Notifier.ts @@ -435,7 +435,14 @@ export const Notifier = { if (actions?.notify) { this._performCustomEventHandling(ev); - if (SdkContextClass.instance.roomViewStore.getRoomId() === room.roomId && + const store = SdkContextClass.instance.roomViewStore; + const isViewingRoom = store.getRoomId() === room.roomId; + const threadId: string | undefined = ev.getThread()?.id; + const isViewingThread = store.getThreadId() === threadId; + + const isViewingEventTimeline = (isViewingRoom && !threadId) || (threadId && isViewingThread); + + if (isViewingEventTimeline && UserActivity.sharedInstance().userActiveRecently() && !Modal.hasDialogs() ) { diff --git a/src/components/structures/ThreadView.tsx b/src/components/structures/ThreadView.tsx index 8350b5e734a..cb72e22d9b5 100644 --- a/src/components/structures/ThreadView.tsx +++ b/src/components/structures/ThreadView.tsx @@ -55,6 +55,7 @@ import Spinner from "../views/elements/Spinner"; import { ComposerInsertPayload, ComposerType } from "../../dispatcher/payloads/ComposerInsertPayload"; import Heading from '../views/typography/Heading'; import { SdkContextClass } from '../../contexts/SDKContext'; +import { ThreadPayload } from '../../dispatcher/payloads/ThreadPayload'; interface IProps { room: Room; @@ -132,6 +133,11 @@ export default class ThreadView extends React.Component { metricsTrigger: undefined, // room doesn't change }); } + + dis.dispatch({ + action: Action.ViewThread, + thread_id: null, + }); } public componentDidUpdate(prevProps) { @@ -225,6 +231,10 @@ export default class ThreadView extends React.Component { }; private async postThreadUpdate(thread: Thread): Promise { + dis.dispatch({ + action: Action.ViewThread, + thread_id: thread.id, + }); thread.emit(ThreadEvent.ViewThread); await thread.fetchInitialEvents(); this.updateThreadRelation(); diff --git a/src/dispatcher/actions.ts b/src/dispatcher/actions.ts index 7d2d935f705..5a19e225de0 100644 --- a/src/dispatcher/actions.ts +++ b/src/dispatcher/actions.ts @@ -116,6 +116,11 @@ export enum Action { */ ViewRoom = "view_room", + /** + * Changes thread based on payload parameters. Should be used with ThreadPayload. + */ + ViewThread = "view_thread", + /** * Changes room based on room list order and payload parameters. Should be used with ViewRoomDeltaPayload. */ diff --git a/src/dispatcher/payloads/ThreadPayload.ts b/src/dispatcher/payloads/ThreadPayload.ts new file mode 100644 index 00000000000..653bbba3aee --- /dev/null +++ b/src/dispatcher/payloads/ThreadPayload.ts @@ -0,0 +1,26 @@ +/* +Copyright 2022 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { ActionPayload } from "../payloads"; +import { Action } from "../actions"; + +/* eslint-disable camelcase */ +export interface ThreadPayload extends Pick { + action: Action.ViewThread; + + thread_id: string | null; +} +/* eslint-enable camelcase */ diff --git a/src/stores/RoomViewStore.tsx b/src/stores/RoomViewStore.tsx index b3814f7a324..13c1e09c764 100644 --- a/src/stores/RoomViewStore.tsx +++ b/src/stores/RoomViewStore.tsx @@ -50,6 +50,7 @@ import { awaitRoomDownSync } from "../utils/RoomUpgrade"; import { UPDATE_EVENT } from "./AsyncStore"; import { SdkContextClass } from "../contexts/SDKContext"; import { CallStore } from "./CallStore"; +import { ThreadPayload } from "../dispatcher/payloads/ThreadPayload"; const NUM_JOIN_RETRY = 5; @@ -66,6 +67,10 @@ interface State { * The ID of the room currently being viewed */ roomId: string | null; + /** + * The ID of the thread currently being viewed + */ + threadId: string | null; /** * The ID of the room being subscribed to (in Sliding Sync) */ @@ -109,6 +114,7 @@ const INITIAL_STATE: State = { joining: false, joinError: null, roomId: null, + threadId: null, subscribingRoomId: null, initialEventId: null, initialEventPixelOffset: null, @@ -200,6 +206,9 @@ export class RoomViewStore extends EventEmitter { case Action.ViewRoom: this.viewRoom(payload); break; + case Action.ViewThread: + this.viewThread(payload); + break; // for these events blank out the roomId as we are no longer in the RoomView case 'view_welcome_page': case Action.ViewHomePage: @@ -430,6 +439,12 @@ export class RoomViewStore extends EventEmitter { } } + private viewThread(payload: ThreadPayload): void { + this.setState({ + threadId: payload.thread_id, + }); + } + private viewRoomError(payload: ViewRoomErrorPayload): void { this.setState({ roomId: payload.room_id, @@ -550,6 +565,10 @@ export class RoomViewStore extends EventEmitter { return this.state.roomId; } + public getThreadId(): Optional { + return this.state.threadId; + } + // The event to scroll to when the room is first viewed public getInitialEventId(): Optional { return this.state.initialEventId; From 460888cf59c41cb0e2d085c1639bb2824789980e Mon Sep 17 00:00:00 2001 From: Germain Date: Tue, 25 Oct 2022 14:32:05 +0100 Subject: [PATCH 2/7] Simplify isViewingEventTimeline statement Co-authored-by: Michael Telatynski <7t3chguy@gmail.com> --- src/Notifier.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Notifier.ts b/src/Notifier.ts index 8cce912bd2b..0c3bc7f4a15 100644 --- a/src/Notifier.ts +++ b/src/Notifier.ts @@ -440,7 +440,7 @@ export const Notifier = { const threadId: string | undefined = ev.getThread()?.id; const isViewingThread = store.getThreadId() === threadId; - const isViewingEventTimeline = (isViewingRoom && !threadId) || (threadId && isViewingThread); + const isViewingEventTimeline = isViewingRoom && (!threadId || isViewingThread); if (isViewingEventTimeline && UserActivity.sharedInstance().userActiveRecently() && From 849b61f7bb11c108a6ee10238c32afc8b39bf624 Mon Sep 17 00:00:00 2001 From: Germain Date: Tue, 25 Oct 2022 14:42:13 +0100 Subject: [PATCH 3/7] Fix show desktop notifications --- test/Notifier-test.ts | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/test/Notifier-test.ts b/test/Notifier-test.ts index 224c1fec776..13e7882add6 100644 --- a/test/Notifier-test.ts +++ b/test/Notifier-test.ts @@ -253,16 +253,13 @@ describe("Notifier", () => { }); const callOnEvent = (type?: string) => { - const callEvent = { - getContent: () => { }, - getRoomId: () => roomId, - isBeingDecrypted: () => false, - isDecryptionFailure: () => false, - getSender: () => "@alice:foo", - getType: () => type ?? ElementCall.CALL_EVENT_TYPE.name, - getStateKey: () => "state_key", - } as unknown as MatrixEvent; - + const callEvent = mkEvent({ + type: type ?? ElementCall.CALL_EVENT_TYPE.name, + user: "@alice:foo", + room: roomId, + content: {}, + event: true, + }); Notifier.onEvent(callEvent); return callEvent; }; From 72d0fa5c5bbaac4b70fe056201018b8a0f2ad78c Mon Sep 17 00:00:00 2001 From: Germain Date: Tue, 25 Oct 2022 14:55:24 +0100 Subject: [PATCH 4/7] Add RoomViewStore thread id assertions --- test/components/structures/ThreadView-test.tsx | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/components/structures/ThreadView-test.tsx b/test/components/structures/ThreadView-test.tsx index 2516aad0820..2893958a8f0 100644 --- a/test/components/structures/ThreadView-test.tsx +++ b/test/components/structures/ThreadView-test.tsx @@ -28,6 +28,7 @@ import { act } from "react-dom/test-utils"; import ThreadView from "../../../src/components/structures/ThreadView"; import MatrixClientContext from "../../../src/contexts/MatrixClientContext"; import RoomContext from "../../../src/contexts/RoomContext"; +import { SdkContextClass } from "../../../src/contexts/SDKContext"; import { MatrixClientPeg } from "../../../src/MatrixClientPeg"; import DMRoomMap from "../../../src/utils/DMRoomMap"; import ResizeNotifier from "../../../src/utils/ResizeNotifier"; @@ -155,4 +156,13 @@ describe("ThreadView", () => { ROOM_ID, rootEvent2.getId(), expectedMessageBody(rootEvent2, "yolo"), ); }); + + it("sets the correct thread in the room view store", async () => { + // expect(SdkContextClass.instance.roomViewStore.getThreadId()).toBeNull(); + const { unmount } = await getComponent(); + expect(SdkContextClass.instance.roomViewStore.getThreadId()).toBe(rootEvent.getId()); + + unmount(); + await waitFor(() => expect(SdkContextClass.instance.roomViewStore.getThreadId()).toBeNull()); + }); }); From 65745ad014fa30e7ba92131f37a09bbf6e7b06e6 Mon Sep 17 00:00:00 2001 From: Germain Date: Tue, 25 Oct 2022 17:08:31 +0100 Subject: [PATCH 5/7] Add Notifier tests --- src/Notifier.ts | 4 +- test/Notifier-test.ts | 99 ++++++++++++++++++++++++++++++++++++++----- 2 files changed, 92 insertions(+), 11 deletions(-) diff --git a/src/Notifier.ts b/src/Notifier.ts index 0c3bc7f4a15..b75b821ae8d 100644 --- a/src/Notifier.ts +++ b/src/Notifier.ts @@ -437,7 +437,9 @@ export const Notifier = { const store = SdkContextClass.instance.roomViewStore; const isViewingRoom = store.getRoomId() === room.roomId; - const threadId: string | undefined = ev.getThread()?.id; + const threadId: string | undefined = ev.getId() !== ev.threadRootId + ? ev.threadRootId + : undefined; const isViewingThread = store.getThreadId() === threadId; const isViewingEventTimeline = isViewingRoom && (!threadId || isViewingThread); diff --git a/test/Notifier-test.ts b/test/Notifier-test.ts index 13e7882add6..ed85f5a906c 100644 --- a/test/Notifier-test.ts +++ b/test/Notifier-test.ts @@ -19,6 +19,7 @@ import { ClientEvent, MatrixClient } from "matrix-js-sdk/src/client"; import { Room } from "matrix-js-sdk/src/models/room"; import { MatrixEvent } from "matrix-js-sdk/src/models/event"; import { SyncState } from "matrix-js-sdk/src/sync"; +import { waitFor } from "@testing-library/react"; import BasePlatform from "../src/BasePlatform"; import { ElementCall } from "../src/models/Call"; @@ -29,8 +30,15 @@ import { createLocalNotificationSettingsIfNeeded, getLocalNotificationAccountDataEventType, } from "../src/utils/notifications"; -import { getMockClientWithEventEmitter, mkEvent, mkRoom, mockClientMethodsUser, mockPlatformPeg } from "./test-utils"; +import { getMockClientWithEventEmitter, mkEvent, mockClientMethodsUser, mockPlatformPeg } from "./test-utils"; import { IncomingCallToast } from "../src/toasts/IncomingCallToast"; +import { SdkContextClass } from "../src/contexts/SDKContext"; +import UserActivity from "../src/UserActivity"; +import Modal from "../src/Modal"; +import { mkThread } from "./test-utils/threads"; +import dis from "../src/dispatcher/dispatcher"; +import { ThreadPayload } from "../src/dispatcher/payloads/ThreadPayload"; +import { Action } from "../src/dispatcher/actions"; jest.mock("../src/utils/notifications", () => ({ // @ts-ignore @@ -50,10 +58,12 @@ describe("Notifier", () => { let MockPlatform: MockedObject; let mockClient: MockedObject; - let testRoom: MockedObject; + let testRoom: Room; let accountDataEventKey: string; let accountDataStore = {}; + let mockSettings: Record = {}; + const userId = "@bob:example.org"; beforeEach(() => { @@ -78,7 +88,7 @@ describe("Notifier", () => { }; accountDataEventKey = getLocalNotificationAccountDataEventType(mockClient.deviceId); - testRoom = mkRoom(mockClient, roomId); + testRoom = new Room(roomId, mockClient, mockClient.getUserId()); MockPlatform = mockPlatformPeg({ supportsNotifications: jest.fn().mockReturnValue(true), @@ -89,7 +99,7 @@ describe("Notifier", () => { Notifier.isBodyEnabled = jest.fn().mockReturnValue(true); - mockClient.getRoom.mockReturnValue(testRoom); + mockClient.getRoom.mockImplementation(id => id === roomId ? testRoom : new Room(id, mockClient, mockClient.getUserId())); }); describe('triggering notification from events', () => { @@ -121,13 +131,14 @@ describe("Notifier", () => { }, }); - const enabledSettings = [ - 'notificationsEnabled', - 'audioNotificationsEnabled', - ]; + mockSettings = { + 'notificationsEnabled': true, + 'audioNotificationsEnabled': true, + }; + // enable notifications by default - jest.spyOn(SettingsStore, "getValue").mockImplementation( - settingName => enabledSettings.includes(settingName), + jest.spyOn(SettingsStore, "getValue").mockReset().mockImplementation( + settingName => mockSettings[settingName] ?? false, ); }); @@ -342,4 +353,72 @@ describe("Notifier", () => { expect(createLocalNotificationSettingsIfNeededMock).toHaveBeenCalled(); }); }); + + describe('_evaluateEvent', () => { + beforeEach(() => { + jest.spyOn(SdkContextClass.instance.roomViewStore, "getRoomId") + .mockReturnValue(testRoom.roomId); + + jest.spyOn(UserActivity.sharedInstance(), "userActiveRecently") + .mockReturnValue(true); + + jest.spyOn(Modal, "hasDialogs").mockReturnValue(false); + + jest.spyOn(Notifier, "_displayPopupNotification").mockReset(); + jest.spyOn(Notifier, "isEnabled").mockReturnValue(true); + + mockClient.getPushActionsForEvent.mockReturnValue({ + notify: true, + tweaks: { + sound: true, + }, + }); + }); + + it("should show a pop-up", () => { + expect(Notifier._displayPopupNotification).toHaveBeenCalledTimes(0); + Notifier._evaluateEvent(testEvent); + expect(Notifier._displayPopupNotification).toHaveBeenCalledTimes(0); + + const eventFromOtherRoom = mkEvent({ + event: true, + type: "m.room.message", + user: "@user1:server", + room: "!otherroom:example.org", + content: {}, + }); + + Notifier._evaluateEvent(eventFromOtherRoom); + expect(Notifier._displayPopupNotification).toHaveBeenCalledTimes(1); + }); + + it.only("should a pop-up for thread event", async () => { + const { events, rootEvent } = mkThread({ + room: testRoom, + client: mockClient, + authorId: "@bob:example.org", + participantUserIds: ["@bob:example.org"], + }); + + expect(Notifier._displayPopupNotification).toHaveBeenCalledTimes(0); + + Notifier._evaluateEvent(rootEvent); + expect(Notifier._displayPopupNotification).toHaveBeenCalledTimes(0); + + Notifier._evaluateEvent(events[1]); + expect(Notifier._displayPopupNotification).toHaveBeenCalledTimes(1); + + dis.dispatch({ + action: Action.ViewThread, + thread_id: rootEvent.getId(), + }); + + await waitFor(() => + expect(SdkContextClass.instance.roomViewStore.getThreadId()).toBe(rootEvent.getId()), + ); + + Notifier._evaluateEvent(events[1]); + expect(Notifier._displayPopupNotification).toHaveBeenCalledTimes(1); + }); + }); }); From 59288aa75b28912cb69f6dd76ab984678e9d8051 Mon Sep 17 00:00:00 2001 From: Germain Date: Tue, 25 Oct 2022 17:15:02 +0100 Subject: [PATCH 6/7] fix lint --- test/Notifier-test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/Notifier-test.ts b/test/Notifier-test.ts index ed85f5a906c..88d7f459e1d 100644 --- a/test/Notifier-test.ts +++ b/test/Notifier-test.ts @@ -99,7 +99,9 @@ describe("Notifier", () => { Notifier.isBodyEnabled = jest.fn().mockReturnValue(true); - mockClient.getRoom.mockImplementation(id => id === roomId ? testRoom : new Room(id, mockClient, mockClient.getUserId())); + mockClient.getRoom.mockImplementation(id => { + return id === roomId ? testRoom : new Room(id, mockClient, mockClient.getUserId()); + }); }); describe('triggering notification from events', () => { From 3d4477fca9c0e12450da85e1512650443f7465b8 Mon Sep 17 00:00:00 2001 From: Germain Date: Tue, 25 Oct 2022 17:35:55 +0100 Subject: [PATCH 7/7] Remove it.only --- test/Notifier-test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Notifier-test.ts b/test/Notifier-test.ts index 88d7f459e1d..30a1691c2ec 100644 --- a/test/Notifier-test.ts +++ b/test/Notifier-test.ts @@ -394,7 +394,7 @@ describe("Notifier", () => { expect(Notifier._displayPopupNotification).toHaveBeenCalledTimes(1); }); - it.only("should a pop-up for thread event", async () => { + it("should a pop-up for thread event", async () => { const { events, rootEvent } = mkThread({ room: testRoom, client: mockClient,