diff --git a/src/stores/notifications/RoomNotificationStateStore.ts b/src/stores/notifications/RoomNotificationStateStore.ts index 48aa7e7c20f..c153ca37aa0 100644 --- a/src/stores/notifications/RoomNotificationStateStore.ts +++ b/src/stores/notifications/RoomNotificationStateStore.ts @@ -108,32 +108,46 @@ export class RoomNotificationStateStore extends AsyncStoreWithClient { return RoomNotificationStateStore.internalInstance; } - private onSync = (state: SyncState, prevState?: SyncState, data?: ISyncStateData) => { + private onSync = async (state: SyncState, prevState?: SyncState, data?: ISyncStateData) => { // Only count visible rooms to not torment the user with notification counts in rooms they can't see. // This will include highlights from the previous version of the room internally + + // Async phase: gather data. Do *not* perform any side-effect. const globalState = new SummarizedNotificationState(); const visibleRooms = this.matrixClient.getVisibleRooms(); let numFavourites = 0; for (const room of visibleRooms) { - if (VisibilityProvider.instance.isRoomVisible(room)) { + if (await VisibilityProvider.instance.isRoomVisible(room)) { globalState.add(this.getRoomState(room)); if (room.tags[DefaultTagID.Favourite] && !room.getType()) numFavourites++; } } - PosthogAnalytics.instance.setProperty("numFavouriteRooms", numFavourites); - - if (this.globalState.symbol !== globalState.symbol || - this.globalState.count !== globalState.count || - this.globalState.color !== globalState.color || - this.globalState.numUnreadStates !== globalState.numUnreadStates || - state !== prevState - ) { - this._globalState = globalState; - this.emit(UPDATE_STATUS_INDICATOR, globalState, state, prevState, data); - } + // Sync phrase: perform side-effects. + // By making sure that we perform side-effects after the last call to `await`, we make sure that + // the side-effects represent *some* snapshot of reality, rather than a mix of two ore more + // snapshots. + // + // Normally, calls to `VisibilityProvider.instance.isRoomVisible` should resolve in the order + // in which they have been enqueued. As long as this holds, we have guaranteeds that the side- + // effects we're causing correspond to the latest snapshot of reality. + (() => { + // Do NOT make this function `async`. + // Its sole purpose is to make sure that we do not call `await` while performing side-effects. + PosthogAnalytics.instance.setProperty("numFavouriteRooms", numFavourites); + + if (this.globalState.symbol !== globalState.symbol || + this.globalState.count !== globalState.count || + this.globalState.color !== globalState.color || + this.globalState.numUnreadStates !== globalState.numUnreadStates || + state !== prevState + ) { + this._globalState = globalState; + this.emit(UPDATE_STATUS_INDICATOR, globalState, state, prevState, data); + } + })(); }; protected async onReady() { diff --git a/src/stores/room-list/RoomListStore.ts b/src/stores/room-list/RoomListStore.ts index f80839f66f8..7f63a201cc1 100644 --- a/src/stores/room-list/RoomListStore.ts +++ b/src/stores/room-list/RoomListStore.ts @@ -318,7 +318,7 @@ export class RoomListStoreClass extends AsyncStoreWithClient implements await VisibilityProvider.instance.onNewInvitedRoom(room); } - if (!VisibilityProvider.instance.isRoomVisible(room)) { + if (!await VisibilityProvider.instance.isRoomVisible(room)) { return; // don't do anything on rooms that aren't visible } @@ -342,7 +342,7 @@ export class RoomListStoreClass extends AsyncStoreWithClient implements this.algorithm.updatesInhibited = true; // Figure out which rooms are about to be valid, and the state of affairs - const rooms = this.getPlausibleRooms(); + const rooms = await this.getPlausibleRooms(); const currentSticky = this.algorithm.stickyRoom; const stickyIsStillPresent = currentSticky && rooms.includes(currentSticky); @@ -486,10 +486,16 @@ export class RoomListStoreClass extends AsyncStoreWithClient implements this.updateFn.trigger(); }; - private getPlausibleRooms(): Room[] { + private async getPlausibleRooms(): Promise { if (!this.matrixClient) return []; - let rooms = this.matrixClient.getVisibleRooms().filter(r => VisibilityProvider.instance.isRoomVisible(r)); + const allRooms = this.matrixClient.getVisibleRooms(); + let rooms = []; + for (const room of allRooms) { + if (await VisibilityProvider.instance.isRoomVisible(room)) { + rooms.push(allRooms); + } + } if (this.prefilterConditions.length > 0) { rooms = rooms.filter(r => { @@ -513,10 +519,10 @@ export class RoomListStoreClass extends AsyncStoreWithClient implements * @param trigger Set to false to prevent a list update from being sent. Should only * be used if the calling code will manually trigger the update. */ - public regenerateAllLists({ trigger = true }) { + public async regenerateAllLists({ trigger = true }) { logger.warn("Regenerating all room lists"); - const rooms = this.getPlausibleRooms(); + const rooms = await this.getPlausibleRooms(); const sorts: ITagSortingMap = {}; const orders: IListOrderingMap = {}; @@ -528,8 +534,8 @@ export class RoomListStoreClass extends AsyncStoreWithClient implements RoomListLayoutStore.instance.ensureLayoutExists(tagId); } - this.algorithm.populateTags(sorts, orders); - this.algorithm.setKnownRooms(rooms); + await this.algorithm.populateTags(sorts, orders); + await this.algorithm.setKnownRooms(rooms); this.initialListsGenerated = true; diff --git a/src/stores/room-list/algorithms/Algorithm.ts b/src/stores/room-list/algorithms/Algorithm.ts index 4a94d36b83b..0cf5258b5e2 100644 --- a/src/stores/room-list/algorithms/Algorithm.ts +++ b/src/stores/room-list/algorithms/Algorithm.ts @@ -163,18 +163,18 @@ export class Algorithm extends EventEmitter { this.recalculateActiveCallRooms(tagId); } - private updateStickyRoom(val: Room) { - this.doUpdateStickyRoom(val); + private async updateStickyRoom(val: Room) { + await this.doUpdateStickyRoom(val); this._lastStickyRoom = null; // clear to indicate we're done changing } - private doUpdateStickyRoom(val: Room) { + private async doUpdateStickyRoom(val: Room) { if (val?.isSpaceRoom() && val.getMyMembership() !== "invite") { // no-op sticky rooms for spaces - they're effectively virtual rooms val = null; } - if (val && !VisibilityProvider.instance.isRoomVisible(val)) { + if (val && !await VisibilityProvider.instance.isRoomVisible(val)) { val = null; // the room isn't visible - lie to the rest of this function } @@ -402,7 +402,7 @@ export class Algorithm extends EventEmitter { * @param {ITagSortingMap} tagSortingMap The tags to generate. * @param {IListOrderingMap} listOrderingMap The ordering of those tags. */ - public populateTags(tagSortingMap: ITagSortingMap, listOrderingMap: IListOrderingMap): void { + public async populateTags(tagSortingMap: ITagSortingMap, listOrderingMap: IListOrderingMap): Promise { if (!tagSortingMap) throw new Error(`Sorting map cannot be null or empty`); if (!listOrderingMap) throw new Error(`Ordering ma cannot be null or empty`); if (arrayHasDiff(Object.keys(tagSortingMap), Object.keys(listOrderingMap))) { @@ -442,7 +442,7 @@ export class Algorithm extends EventEmitter { * previously known information and instead use these rooms instead. * @param {Room[]} rooms The rooms to force the algorithm to use. */ - public setKnownRooms(rooms: Room[]): void { + public async setKnownRooms(rooms: Room[]): Promise { if (isNullOrUndefined(rooms)) throw new Error(`Array of rooms cannot be null`); if (!this.sortAlgorithms) throw new Error(`Cannot set known rooms without a tag sorting map`); @@ -456,7 +456,9 @@ export class Algorithm extends EventEmitter { // Before we go any further we need to clear (but remember) the sticky room to // avoid accidentally duplicating it in the list. const oldStickyRoom = this._stickyRoom; - if (oldStickyRoom) this.updateStickyRoom(null); + if (oldStickyRoom) { + await this.updateStickyRoom(null); + } this.rooms = rooms; diff --git a/src/stores/room-list/filters/VisibilityProvider.ts b/src/stores/room-list/filters/VisibilityProvider.ts index 6359d24702b..ce67ae44602 100644 --- a/src/stores/room-list/filters/VisibilityProvider.ts +++ b/src/stores/room-list/filters/VisibilityProvider.ts @@ -20,6 +20,7 @@ import LegacyCallHandler from "../../../LegacyCallHandler"; import { RoomListCustomisations } from "../../../customisations/RoomList"; import { isLocalRoom } from "../../../utils/localRoom/isLocalRoom"; import VoipUserMapper from "../../../VoipUserMapper"; +import { MatrixClientPeg } from "../../../MatrixClientPeg"; export class VisibilityProvider { private static internalInstance: VisibilityProvider; @@ -38,7 +39,7 @@ export class VisibilityProvider { await VoipUserMapper.sharedInstance().onNewInvitedRoom(room); } - public isRoomVisible(room?: Room): boolean { + public async isRoomVisible(room?: Room): Promise { if (!room) { return false; } @@ -50,6 +51,21 @@ export class VisibilityProvider { return false; } + if (room.getMyMembership() === "invite") { + // Find out whether the invite should be hidden. + const cli = MatrixClientPeg.get(); + const myUserId = cli.getUserId(); + const inviter = room.currentState.getMember(myUserId); + if (inviter?.events?.member) { + const inviterUserId = inviter.events.member.getSender(); + const rule = await cli.ignoredInvites.getRuleForInvite({ roomId: room.roomId, sender: inviterUserId }); + if (rule) { + // Indeed, there is a rule that specifies we should hide the invite. + return false; + } + } + } + // hide space rooms as they'll be shown in the SpacePanel if (room.isSpaceRoom()) { return false; diff --git a/test/stores/room-list/algorithms/Algorithm-test.ts b/test/stores/room-list/algorithms/Algorithm-test.ts index 41ad06e4b98..1cd32d2770f 100644 --- a/test/stores/room-list/algorithms/Algorithm-test.ts +++ b/test/stores/room-list/algorithms/Algorithm-test.ts @@ -38,14 +38,14 @@ describe("Algorithm", () => { let client: MockedObject; let algorithm: Algorithm; - beforeEach(() => { + beforeEach(async () => { stubClient(); client = mocked(MatrixClientPeg.get()); DMRoomMap.makeShared(); algorithm = new Algorithm(); algorithm.start(); - algorithm.populateTags( + await algorithm.populateTags( { [DefaultTagID.Untagged]: SortAlgorithm.Alphabetic }, { [DefaultTagID.Untagged]: ListAlgorithm.Natural }, ); @@ -75,7 +75,7 @@ describe("Algorithm", () => { client.reEmitter.reEmit(roomWithCall, [RoomStateEvent.Events]); for (const room of client.getRooms()) jest.spyOn(room, "getMyMembership").mockReturnValue("join"); - algorithm.setKnownRooms(client.getRooms()); + await algorithm.setKnownRooms(client.getRooms()); setupAsyncStoreWithClient(CallStore.instance, client); setupAsyncStoreWithClient(WidgetMessagingStore.instance, client); diff --git a/test/stores/room-list/filters/VisibilityProvider-test.ts b/test/stores/room-list/filters/VisibilityProvider-test.ts index f22901a40f1..2751f896ed8 100644 --- a/test/stores/room-list/filters/VisibilityProvider-test.ts +++ b/test/stores/room-list/filters/VisibilityProvider-test.ts @@ -22,7 +22,9 @@ import LegacyCallHandler from "../../../../src/LegacyCallHandler"; import VoipUserMapper from "../../../../src/VoipUserMapper"; import { LocalRoom, LOCAL_ROOM_ID_PREFIX } from "../../../../src/models/LocalRoom"; import { RoomListCustomisations } from "../../../../src/customisations/RoomList"; -import { createTestClient } from "../../../test-utils"; +import { createTestClient, IGNORE_INVITES_FROM_THIS_USER, IGNORE_INVITES_TO_THIS_ROOM, stubClient } + from "../../../test-utils"; +import { MatrixClientPeg } from "../../../../src/MatrixClientPeg"; jest.mock("../../../../src/VoipUserMapper", () => ({ sharedInstance: jest.fn(), @@ -40,9 +42,29 @@ jest.mock("../../../../src/customisations/RoomList", () => ({ }, })); -const createRoom = (isSpaceRoom = false): Room => { +const createRoom = ({ isSpaceRoom, inviter, roomId }: { isSpaceRoom?: boolean, inviter?: string, roomId?: string } = +{ isSpaceRoom: false, roomId: `${Math.random()}:example.org` }): Room => { return { + roomId, isSpaceRoom: () => isSpaceRoom, + getMyMembership: () => + inviter ? "invite" : "join", + currentState: { + getMember(userId: string): any | null { + if (userId != MatrixClientPeg.get().getUserId()) { + return null; + } + return { + events: { + member: { + getSender() { + return inviter; + }, + }, + }, + }; + }, + }, } as unknown as Room; }; @@ -61,6 +83,7 @@ describe("VisibilityProvider", () => { isVirtualRoom: jest.fn(), } as unknown as VoipUserMapper; mocked(VoipUserMapper.sharedInstance).mockReturnValue(mockVoipUserMapper); + stubClient(); }); describe("instance", () => { @@ -86,39 +109,58 @@ describe("VisibilityProvider", () => { mocked(mockVoipUserMapper.isVirtualRoom).mockReturnValue(true); }); - it("should return return false", () => { + it("should return return false", async () => { const room = createRoom(); - expect(VisibilityProvider.instance.isRoomVisible(room)).toBe(false); + expect(await VisibilityProvider.instance.isRoomVisible(room)).toBe(false); expect(mockVoipUserMapper.isVirtualRoom).toHaveBeenCalledWith(room); }); }); - it("should return false without room", () => { - expect(VisibilityProvider.instance.isRoomVisible()).toBe(false); + it("should return false without room", async () => { + expect(await VisibilityProvider.instance.isRoomVisible()).toBe(false); }); - it("should return false for a space room", () => { - const room = createRoom(true); - expect(VisibilityProvider.instance.isRoomVisible(room)).toBe(false); + it("should return false for a space room", async () => { + const room = createRoom({ isSpaceRoom: true }); + expect(await VisibilityProvider.instance.isRoomVisible(room)).toBe(false); }); - it("should return false for a local room", () => { + it("should return false for a local room", async () => { const room = createLocalRoom(); - expect(VisibilityProvider.instance.isRoomVisible(room)).toBe(false); + expect(await VisibilityProvider.instance.isRoomVisible(room)).toBe(false); }); - it("should return false if visibility customisation returns false", () => { + it("should return false if visibility customisation returns false", async () => { mocked(RoomListCustomisations.isRoomVisible).mockReturnValue(false); const room = createRoom(); - expect(VisibilityProvider.instance.isRoomVisible(room)).toBe(false); + expect(await VisibilityProvider.instance.isRoomVisible(room)).toBe(false); expect(RoomListCustomisations.isRoomVisible).toHaveBeenCalledWith(room); }); - it("should return true if visibility customisation returns true", () => { + it("should return true if visibility customisation returns true", async () => { mocked(RoomListCustomisations.isRoomVisible).mockReturnValue(true); const room = createRoom(); - expect(VisibilityProvider.instance.isRoomVisible(room)).toBe(true); + expect(await VisibilityProvider.instance.isRoomVisible(room)).toBe(true); expect(RoomListCustomisations.isRoomVisible).toHaveBeenCalledWith(room); }); + + it("should return true if the room is an invite but hasn't been marked as ignored", async () => { + mocked(RoomListCustomisations.isRoomVisible).mockReturnValue(true); + const room = createRoom({ inviter: "@good-user:example.org" }); + expect(await VisibilityProvider.instance.isRoomVisible(room)).toBe(true); + expect(RoomListCustomisations.isRoomVisible).toHaveBeenCalledWith(room); + }); + + it("should return false if the room is an invite and the sender has been marked as ignored", async () => { + mocked(RoomListCustomisations.isRoomVisible).mockReturnValue(true); + const room = createRoom({ inviter: IGNORE_INVITES_FROM_THIS_USER }); + expect(await VisibilityProvider.instance.isRoomVisible(room)).toBe(false); + }); + + it("should return false if the room is an invite and the roomId has been marked as ignored", async () => { + mocked(RoomListCustomisations.isRoomVisible).mockReturnValue(true); + const room = createRoom({ inviter: "@good-user:example.org", roomId: IGNORE_INVITES_TO_THIS_ROOM }); + expect(await VisibilityProvider.instance.isRoomVisible(room)).toBe(false); + }); }); }); diff --git a/test/test-utils/test-utils.ts b/test/test-utils/test-utils.ts index 8d330391fdd..311d378a652 100644 --- a/test/test-utils/test-utils.ts +++ b/test/test-utils/test-utils.ts @@ -66,6 +66,11 @@ export function stubClient() { MatrixClientBackedSettingsHandler.matrixClient = client; } +export const IGNORE_INVITES_TO_THIS_ROOM = "$ignore-invites-to-this-room:example.org"; +export const IGNORE_INVITES_TO_THIS_ROOM_ISSUER = "@user-who-decided-to-ignore-invites-to-this-room:example.org"; +export const IGNORE_INVITES_FROM_THIS_USER = "@ignore-invites-from-this-sender:example.org"; +export const IGNORE_INVITES_FROM_THIS_USER_ISSUER = "@user-who-decided-to-ignore-invites-from-this-user:example.org"; + /** * Create a stubbed-out MatrixClient * @@ -174,6 +179,21 @@ export function createTestClient(): MatrixClient { sendToDevice: jest.fn().mockResolvedValue(undefined), queueToDevice: jest.fn().mockResolvedValue(undefined), encryptAndSendToDevices: jest.fn().mockResolvedValue(undefined), + ignoredInvites: { + getRuleForInvite: jest.fn().mockImplementation(({ roomId, sender }) => { + if (roomId === IGNORE_INVITES_TO_THIS_ROOM) { + return Promise.resolve(new MatrixEvent({ + sender: IGNORE_INVITES_TO_THIS_ROOM_ISSUER, + })); + } + if (sender === IGNORE_INVITES_FROM_THIS_USER) { + return Promise.resolve(new MatrixEvent({ + sender: IGNORE_INVITES_FROM_THIS_USER_ISSUER, + })); + } + return Promise.resolve(null); + }), + }, } as unknown as MatrixClient; client.reEmitter = new ReEmitter(client);