From fba7465ad4a611ddc6c6be95bb6cf1ad4c727865 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 22 Apr 2021 14:45:13 +0100 Subject: [PATCH 01/16] Initial SpaceStore tests work --- test/stores/SpaceStore-test.ts | 244 +++++++++++++++++++++++++++++++++ test/test-utils.js | 5 +- test/utils/test-utils.ts | 25 ++++ 3 files changed, 272 insertions(+), 2 deletions(-) create mode 100644 test/stores/SpaceStore-test.ts create mode 100644 test/utils/test-utils.ts diff --git a/test/stores/SpaceStore-test.ts b/test/stores/SpaceStore-test.ts new file mode 100644 index 0000000000..60960fd5cf --- /dev/null +++ b/test/stores/SpaceStore-test.ts @@ -0,0 +1,244 @@ +/* +Copyright 2021 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 { EventType } from "matrix-js-sdk/src/@types/event"; + +import "../skinned-sdk"; // Must be first for skinning to work +import SpaceStore from "../../src/stores/SpaceStore"; +import { setupAsyncStoreWithClient } from "../utils/test-utils"; +import { createTestClient, mkEvent, mkStubRoom } from "../test-utils"; +import { EnhancedMap } from "../../src/utils/maps"; +import SettingsStore from "../../src/settings/SettingsStore"; + +type MatrixEvent = any; // importing from js-sdk upsets things + +jest.useFakeTimers(); + +const mockStateEventImplementation = (events: MatrixEvent[]) => { + const stateMap = new EnhancedMap>(); + events.forEach(event => { + stateMap.getOrCreate(event.getType(), new Map()).set(event.getStateKey(), event); + }); + + return (eventType: string, stateKey?: string) => { + if (stateKey || stateKey === "") { + return stateMap.get(eventType)?.get(stateKey) || null; + } + return Array.from(stateMap.get(eventType)?.values() || []); + }; +}; + +const testUserId = "@test:user"; + +let rooms = []; + +const mkSpace = (spaceId: string, children: string[] = []) => { + const space = mkStubRoom(spaceId); + space.isSpaceRoom.mockReturnValue(true); + space.currentState.getStateEvents.mockImplementation(mockStateEventImplementation(children.map(roomId => + mkEvent({ + event: true, + type: EventType.SpaceChild, + room: spaceId, + user: testUserId, + skey: roomId, + content: { via: [] }, + ts: Date.now(), + }), + ))); + rooms.push(space); + return space; +}; + +const getValue = jest.fn(); +SettingsStore.getValue = getValue; + +describe("SpaceStore", () => { + const store = SpaceStore.instance; + const client = createTestClient(); + + const run = async () => { + client.getRoom.mockImplementation(roomId => rooms.find(room => room.roomId === roomId)); + await setupAsyncStoreWithClient(store, client); + }; + + beforeEach(() => { + jest.runAllTimers(); + client.getVisibleRooms.mockReturnValue(rooms = []); + getValue.mockImplementation(settingName => { + if (settingName === "feature_spaces") { + return true; + } + }); + }); + afterEach(() => { + // @ts-ignore + store.onNotReady(); + }); + + describe("static hierarchy resolution tests", () => { + it("handles no spaces", async () => { + await run(); + + expect(store.spacePanelSpaces).toStrictEqual([]); + expect(store.invitedSpaces).toStrictEqual([]); + }); + + it("handles 3 joined top level spaces", async () => { + mkSpace("!space1:server"); + mkSpace("!space2:server"); + mkSpace("!space3:server"); + await run(); + + expect(store.spacePanelSpaces.sort()).toStrictEqual(client.getVisibleRooms().sort()); + expect(store.invitedSpaces).toStrictEqual([]); + }); + + it("handles a basic hierarchy", async () => { + mkSpace("!space1:server"); + mkSpace("!space2:server"); + mkSpace("!company:server", [ + mkSpace("!company_dept1:server", [ + mkSpace("!company_dept1_group1:server").roomId, + ]).roomId, + mkSpace("!company_dept2:server").roomId, + ]); + await run(); + + expect(store.spacePanelSpaces.map(r => r.roomId).sort()).toStrictEqual([ + "!space1:server", + "!space2:server", + "!company:server", + ].sort()); + expect(store.invitedSpaces).toStrictEqual([]); + // TODO verify actual tree structure + }); + + it("handles a sub-space existing in multiple places in the space tree", async () => { + const subspace = mkSpace("!subspace:server"); + mkSpace("!space1:server"); + mkSpace("!space2:server"); + mkSpace("!company:server", [ + mkSpace("!company_dept1:server", [ + mkSpace("!company_dept1_group1:server", [subspace.roomId]).roomId, + ]).roomId, + mkSpace("!company_dept2:server", [subspace.roomId]).roomId, + subspace.roomId, + ]); + await run(); + + expect(store.spacePanelSpaces.map(r => r.roomId).sort()).toStrictEqual([ + "!space1:server", + "!space2:server", + "!company:server", + ].sort()); + expect(store.invitedSpaces).toStrictEqual([]); + // TODO verify actual tree structure + }); + + it("handles basic cycles", async () => { + // TODO test all input order permutations + mkSpace("!a:server", [ + mkSpace("!b:server", [ + mkSpace("!c:server", [ + "!a:server", + ]).roomId, + ]).roomId, + ]); + await run(); + + expect(store.spacePanelSpaces.map(r => r.roomId)).toStrictEqual(["!a:server"]); + expect(store.invitedSpaces).toStrictEqual([]); + // TODO verify actual tree structure + }); + + it("handles complex cycles", async () => { + // TODO test all input order permutations + mkSpace("!b:server", [ + mkSpace("!a:server", [ + mkSpace("!c:server", [ + "!a:server", + ]).roomId, + ]).roomId, + ]); + await run(); + + expect(store.spacePanelSpaces.map(r => r.roomId)).toStrictEqual(["!b:server"]); + expect(store.invitedSpaces).toStrictEqual([]); + // TODO verify actual tree structure + }); + + it("handles really complex cycles", async () => { + // TODO test all input order permutations + mkSpace("!a:server", [ + mkSpace("!b:server", [ + mkSpace("!c:server", [ + "!a:server", + mkSpace("!d:server").roomId, + ]).roomId, + ]).roomId, + ]); + await run(); + + expect(store.spacePanelSpaces.map(r => r.roomId)).toStrictEqual(["!a:server"]); + expect(store.invitedSpaces).toStrictEqual([]); + // TODO verify actual tree structure + // TODO this test should be failing right now + }); + + describe("home space behaviour", () => { + test.todo("home space contains orphaned rooms"); + test.todo("home space contains favourites"); + test.todo("home space contains dm rooms"); + test.todo("home space contains invites"); + test.todo("home space contains invites even if they are also shown in a space"); + }); + }); + + describe("hierarchy resolution update tests", () => { + test.todo("updates state when spaces are joined"); + test.todo("updates state when spaces are left"); + test.todo("updates state when space invite comes in"); + test.todo("updates state when space invite is accepted"); + test.todo("updates state when space invite is rejected"); + }); + + describe("notification state tests", () => { + test.todo("//notification states"); + }); + + describe("room list prefilter tests", () => { + test.todo("//room list filter"); + }); + + describe("active space switching tests", () => { + test.todo("//active space"); + }); + + describe("context switching tests", () => { + test.todo("//context switching"); + }); + + describe("space auto switching tests", () => { + test.todo("//auto pick space for a room"); + }); + + describe("traverseSpace", () => { + test.todo("avoids cycles"); + test.todo("including rooms"); + test.todo("excluding rooms"); + }); +}); diff --git a/test/test-utils.js b/test/test-utils.js index d259fcb95f..33f7e1626e 100644 --- a/test/test-utils.js +++ b/test/test-utils.js @@ -88,8 +88,8 @@ export function createTestClient() { * @param {string} opts.type The event.type * @param {string} opts.room The event.room_id * @param {string} opts.user The event.user_id - * @param {string} opts.skey Optional. The state key (auto inserts empty string) - * @param {Number} opts.ts Optional. Timestamp for the event + * @param {string=} opts.skey Optional. The state key (auto inserts empty string) + * @param {number=} opts.ts Optional. Timestamp for the event * @param {Object} opts.content The event.content * @param {boolean} opts.event True to make a MatrixEvent. * @return {Object} a JSON object representing this event. @@ -244,6 +244,7 @@ export function mkStubRoom(roomId = null) { getDMInviter: jest.fn(), getAvatarUrl: () => 'mxc://avatar.url/room.png', getMxcAvatarUrl: () => 'mxc://avatar.url/room.png', + isSpaceRoom: jest.fn(() => false), }; } diff --git a/test/utils/test-utils.ts b/test/utils/test-utils.ts new file mode 100644 index 0000000000..f86196ffbd --- /dev/null +++ b/test/utils/test-utils.ts @@ -0,0 +1,25 @@ +/* +Copyright 2021 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 { MatrixClient } from "matrix-js-sdk/src/client"; +import {AsyncStoreWithClient} from "../../src/stores/AsyncStoreWithClient"; + +export const setupAsyncStoreWithClient = async (store: AsyncStoreWithClient, client: MatrixClient) => { + // @ts-ignore + store.readyStore.useUnitTestClient(client); + // @ts-ignore + await store.onReady(); +}; From 107575692996f89123e082156767ab0f4e3760e0 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 23 Apr 2021 09:55:30 +0100 Subject: [PATCH 02/16] add more tests --- src/stores/SpaceStore.tsx | 4 +- test/stores/SpaceStore-test.ts | 73 +++++++++++++++++++++++++++++----- test/utils/test-utils.ts | 10 ++++- 3 files changed, 76 insertions(+), 11 deletions(-) diff --git a/src/stores/SpaceStore.tsx b/src/stores/SpaceStore.tsx index a9a73e164f..a6ccf314d9 100644 --- a/src/stores/SpaceStore.tsx +++ b/src/stores/SpaceStore.tsx @@ -583,7 +583,9 @@ export class SpaceStoreClass extends AsyncStoreWithClient { return state; } - // traverse space tree with DFS calling fn on each space including the given root one + // traverse space tree with DFS calling fn on each space including the given root one, + // if includeRooms is true then fn will be called on each leaf room, if it is present in multiple sub-spaces + // then fn will be called with it multiple times. public traverseSpace( spaceId: string, fn: (roomId: string) => void, diff --git a/test/stores/SpaceStore-test.ts b/test/stores/SpaceStore-test.ts index 60960fd5cf..5528fff66d 100644 --- a/test/stores/SpaceStore-test.ts +++ b/test/stores/SpaceStore-test.ts @@ -18,7 +18,7 @@ import { EventType } from "matrix-js-sdk/src/@types/event"; import "../skinned-sdk"; // Must be first for skinning to work import SpaceStore from "../../src/stores/SpaceStore"; -import { setupAsyncStoreWithClient } from "../utils/test-utils"; +import { resetAsyncStoreWithClient, setupAsyncStoreWithClient } from "../utils/test-utils"; import { createTestClient, mkEvent, mkStubRoom } from "../test-utils"; import { EnhancedMap } from "../../src/utils/maps"; import SettingsStore from "../../src/settings/SettingsStore"; @@ -45,8 +45,14 @@ const testUserId = "@test:user"; let rooms = []; +const mkRoom = (roomId: string) => { + const room = mkStubRoom(roomId); + rooms.push(room); + return room; +}; + const mkSpace = (spaceId: string, children: string[] = []) => { - const space = mkStubRoom(spaceId); + const space = mkRoom(spaceId); space.isSpaceRoom.mockReturnValue(true); space.currentState.getStateEvents.mockImplementation(mockStateEventImplementation(children.map(roomId => mkEvent({ @@ -59,7 +65,6 @@ const mkSpace = (spaceId: string, children: string[] = []) => { ts: Date.now(), }), ))); - rooms.push(space); return space; }; @@ -84,9 +89,8 @@ describe("SpaceStore", () => { } }); }); - afterEach(() => { - // @ts-ignore - store.onNotReady(); + afterEach(async () => { + await resetAsyncStoreWithClient(store); }); describe("static hierarchy resolution tests", () => { @@ -237,8 +241,59 @@ describe("SpaceStore", () => { }); describe("traverseSpace", () => { - test.todo("avoids cycles"); - test.todo("including rooms"); - test.todo("excluding rooms"); + beforeEach(() => { + mkSpace("!a:server", [ + mkSpace("!b:server", [ + mkSpace("!c:server", [ + "!a:server", + mkRoom("!c-child:server").roomId, + mkRoom("!shared-child:server").roomId, + ]).roomId, + mkRoom("!b-child:server").roomId, + ]).roomId, + mkRoom("!a-child:server").roomId, + "!shared-child:server", + ]); + }); + + it("avoids cycles", () => { + const seenMap = new Map(); + store.traverseSpace("!b:server", roomId => { + seenMap.set(roomId, (seenMap.get(roomId) || 0) + 1); + }); + + expect(seenMap.size).toBe(3); + expect(seenMap.get("!a:server")).toBe(1); + expect(seenMap.get("!b:server")).toBe(1); + expect(seenMap.get("!c:server")).toBe(1); + }); + + it("including rooms", () => { + const seenMap = new Map(); + store.traverseSpace("!b:server", roomId => { + seenMap.set(roomId, (seenMap.get(roomId) || 0) + 1); + }, true); + + expect(seenMap.size).toBe(7); + expect(seenMap.get("!a:server")).toBe(1); + expect(seenMap.get("!a-child:server")).toBe(1); + expect(seenMap.get("!b:server")).toBe(1); + expect(seenMap.get("!b-child:server")).toBe(1); + expect(seenMap.get("!c:server")).toBe(1); + expect(seenMap.get("!c-child:server")).toBe(1); + expect(seenMap.get("!shared-child:server")).toBe(2); + }); + + it("excluding rooms", () => { + const seenMap = new Map(); + store.traverseSpace("!b:server", roomId => { + seenMap.set(roomId, (seenMap.get(roomId) || 0) + 1); + }, false); + + expect(seenMap.size).toBe(3); + expect(seenMap.get("!a:server")).toBe(1); + expect(seenMap.get("!b:server")).toBe(1); + expect(seenMap.get("!c:server")).toBe(1); + }); }); }); diff --git a/test/utils/test-utils.ts b/test/utils/test-utils.ts index f86196ffbd..af92987a3d 100644 --- a/test/utils/test-utils.ts +++ b/test/utils/test-utils.ts @@ -15,7 +15,10 @@ limitations under the License. */ import { MatrixClient } from "matrix-js-sdk/src/client"; -import {AsyncStoreWithClient} from "../../src/stores/AsyncStoreWithClient"; +import { AsyncStoreWithClient } from "../../src/stores/AsyncStoreWithClient"; + +// These methods make some use of some private methods on the AsyncStoreWithClient to simplify getting into a consistent +// ready state without needing to wire up a dispatcher and pretend to be a js-sdk client. export const setupAsyncStoreWithClient = async (store: AsyncStoreWithClient, client: MatrixClient) => { // @ts-ignore @@ -23,3 +26,8 @@ export const setupAsyncStoreWithClient = async (store: AsyncStoreWithClient // @ts-ignore await store.onReady(); }; + +export const resetAsyncStoreWithClient = async (store: AsyncStoreWithClient) => { + // @ts-ignore + await store.onNotReady(); +}; From a38419defb1c8ff505713bce35bccab7226ceaf0 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 23 Apr 2021 11:20:26 +0100 Subject: [PATCH 03/16] extend space tests some more --- test/stores/SpaceStore-test.ts | 87 +++++++++++++++++++++++++++------- 1 file changed, 71 insertions(+), 16 deletions(-) diff --git a/test/stores/SpaceStore-test.ts b/test/stores/SpaceStore-test.ts index 5528fff66d..b27ca8622a 100644 --- a/test/stores/SpaceStore-test.ts +++ b/test/stores/SpaceStore-test.ts @@ -128,7 +128,24 @@ describe("SpaceStore", () => { "!company:server", ].sort()); expect(store.invitedSpaces).toStrictEqual([]); - // TODO verify actual tree structure + + expect(store.getChildRooms("!space1:server")).toStrictEqual([]); + expect(store.getChildSpaces("!space1:server")).toStrictEqual([]); + expect(store.getChildRooms("!space2:server")).toStrictEqual([]); + expect(store.getChildSpaces("!space2:server")).toStrictEqual([]); + expect(store.getChildRooms("!company:server")).toStrictEqual([]); + expect(store.getChildSpaces("!company:server")).toStrictEqual([ + client.getRoom("!company_dept1:server"), + client.getRoom("!company_dept2:server"), + ]); + expect(store.getChildRooms("!company_dept1:server")).toStrictEqual([]); + expect(store.getChildSpaces("!company_dept1:server")).toStrictEqual([ + client.getRoom("!company_dept1_group1:server"), + ]); + expect(store.getChildRooms("!company_dept1_group1:server")).toStrictEqual([]); + expect(store.getChildSpaces("!company_dept1_group1:server")).toStrictEqual([]); + expect(store.getChildRooms("!company_dept2:server")).toStrictEqual([]); + expect(store.getChildSpaces("!company_dept2:server")).toStrictEqual([]); }); it("handles a sub-space existing in multiple places in the space tree", async () => { @@ -150,11 +167,28 @@ describe("SpaceStore", () => { "!company:server", ].sort()); expect(store.invitedSpaces).toStrictEqual([]); - // TODO verify actual tree structure + + expect(store.getChildRooms("!space1:server")).toStrictEqual([]); + expect(store.getChildSpaces("!space1:server")).toStrictEqual([]); + expect(store.getChildRooms("!space2:server")).toStrictEqual([]); + expect(store.getChildSpaces("!space2:server")).toStrictEqual([]); + expect(store.getChildRooms("!company:server")).toStrictEqual([]); + expect(store.getChildSpaces("!company:server")).toStrictEqual([ + client.getRoom("!company_dept1:server"), + client.getRoom("!company_dept2:server"), + subspace, + ]); + expect(store.getChildRooms("!company_dept1:server")).toStrictEqual([]); + expect(store.getChildSpaces("!company_dept1:server")).toStrictEqual([ + client.getRoom("!company_dept1_group1:server"), + ]); + expect(store.getChildRooms("!company_dept1_group1:server")).toStrictEqual([]); + expect(store.getChildSpaces("!company_dept1_group1:server")).toStrictEqual([subspace]); + expect(store.getChildRooms("!company_dept2:server")).toStrictEqual([]); + expect(store.getChildSpaces("!company_dept2:server")).toStrictEqual([subspace]); }); - it("handles basic cycles", async () => { - // TODO test all input order permutations + it("handles full cycles", async () => { mkSpace("!a:server", [ mkSpace("!b:server", [ mkSpace("!c:server", [ @@ -166,11 +200,16 @@ describe("SpaceStore", () => { expect(store.spacePanelSpaces.map(r => r.roomId)).toStrictEqual(["!a:server"]); expect(store.invitedSpaces).toStrictEqual([]); - // TODO verify actual tree structure + + expect(store.getChildRooms("!a:server")).toStrictEqual([]); + expect(store.getChildSpaces("!a:server")).toStrictEqual([client.getRoom("!b:server")]); + expect(store.getChildRooms("!b:server")).toStrictEqual([]); + expect(store.getChildSpaces("!b:server")).toStrictEqual([client.getRoom("!c:server")]); + expect(store.getChildRooms("!c:server")).toStrictEqual([]); + expect(store.getChildSpaces("!c:server")).toStrictEqual([client.getRoom("!a:server")]); }); - it("handles complex cycles", async () => { - // TODO test all input order permutations + it("handles partial cycles", async () => { mkSpace("!b:server", [ mkSpace("!a:server", [ mkSpace("!c:server", [ @@ -182,11 +221,17 @@ describe("SpaceStore", () => { expect(store.spacePanelSpaces.map(r => r.roomId)).toStrictEqual(["!b:server"]); expect(store.invitedSpaces).toStrictEqual([]); - // TODO verify actual tree structure + + expect(store.getChildRooms("!b:server")).toStrictEqual([]); + expect(store.getChildSpaces("!b:server")).toStrictEqual([client.getRoom("!a:server")]); + expect(store.getChildRooms("!a:server")).toStrictEqual([]); + expect(store.getChildSpaces("!a:server")).toStrictEqual([client.getRoom("!c:server")]); + expect(store.getChildRooms("!c:server")).toStrictEqual([]); + expect(store.getChildSpaces("!c:server")).toStrictEqual([client.getRoom("!a:server")]); }); - it("handles really complex cycles", async () => { - // TODO test all input order permutations + it("handles partial cycles with additional spaces coming off them", async () => { + // TODO this test should be failing right now mkSpace("!a:server", [ mkSpace("!b:server", [ mkSpace("!c:server", [ @@ -199,8 +244,18 @@ describe("SpaceStore", () => { expect(store.spacePanelSpaces.map(r => r.roomId)).toStrictEqual(["!a:server"]); expect(store.invitedSpaces).toStrictEqual([]); - // TODO verify actual tree structure - // TODO this test should be failing right now + + expect(store.getChildRooms("!a:server")).toStrictEqual([]); + expect(store.getChildSpaces("!a:server")).toStrictEqual([client.getRoom("!b:server")]); + expect(store.getChildRooms("!b:server")).toStrictEqual([]); + expect(store.getChildSpaces("!b:server")).toStrictEqual([client.getRoom("!c:server")]); + expect(store.getChildRooms("!c:server")).toStrictEqual([]); + expect(store.getChildSpaces("!c:server")).toStrictEqual([ + client.getRoom("!a:server"), + client.getRoom("!d:server"), + ]); + expect(store.getChildRooms("!d:server")).toStrictEqual([]); + expect(store.getChildSpaces("!d:server")).toStrictEqual([]); }); describe("home space behaviour", () => { @@ -220,6 +275,10 @@ describe("SpaceStore", () => { test.todo("updates state when space invite is rejected"); }); + describe("active space switching tests", () => { + test.todo("//active space"); + }); + describe("notification state tests", () => { test.todo("//notification states"); }); @@ -228,10 +287,6 @@ describe("SpaceStore", () => { test.todo("//room list filter"); }); - describe("active space switching tests", () => { - test.todo("//active space"); - }); - describe("context switching tests", () => { test.todo("//context switching"); }); From a12cefee8e2f9dc1fef806a600404592f1bfdee7 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 23 Apr 2021 12:19:08 +0100 Subject: [PATCH 04/16] Tweak some tests --- src/Unread.js | 2 +- test/test-utils.js | 11 +++++------ test/utils/ShieldUtils-test.js | 2 +- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/Unread.js b/src/Unread.js index ddf225ac64..12c15eb6af 100644 --- a/src/Unread.js +++ b/src/Unread.js @@ -45,7 +45,7 @@ export function eventTriggersUnreadCount(ev) { } export function doesRoomHaveUnreadMessages(room) { - const myUserId = MatrixClientPeg.get().credentials.userId; + const myUserId = MatrixClientPeg.get().getUserId(); // get the most recent read receipt sent by our account. // N.B. this is NOT a read marker (RM, aka "read up to marker"), diff --git a/test/test-utils.js b/test/test-utils.js index 33f7e1626e..4fc9bdf377 100644 --- a/test/test-utils.js +++ b/test/test-utils.js @@ -224,7 +224,7 @@ export function mkStubRoom(roomId = null) { hasMembershipState: () => null, getVersion: () => '1', shouldUpgradeToVersion: () => null, - getMyMembership: () => "join", + getMyMembership: jest.fn().mockReturnValue("join"), maySendMessage: jest.fn().mockReturnValue(true), currentState: { getStateEvents: jest.fn(), @@ -233,11 +233,7 @@ export function mkStubRoom(roomId = null) { maySendEvent: jest.fn().mockReturnValue(true), members: [], }, - tags: { - "m.favourite": { - order: 0.5, - }, - }, + tags: {}, setBlacklistUnverifiedDevices: jest.fn(), on: jest.fn(), removeListener: jest.fn(), @@ -245,6 +241,9 @@ export function mkStubRoom(roomId = null) { getAvatarUrl: () => 'mxc://avatar.url/room.png', getMxcAvatarUrl: () => 'mxc://avatar.url/room.png', isSpaceRoom: jest.fn(() => false), + getUnreadNotificationCount: jest.fn(() => 0), + getEventReadUpTo: jest.fn(() => null), + timeline: [], }; } diff --git a/test/utils/ShieldUtils-test.js b/test/utils/ShieldUtils-test.js index 8e3b19c1c4..bea3d26565 100644 --- a/test/utils/ShieldUtils-test.js +++ b/test/utils/ShieldUtils-test.js @@ -128,7 +128,7 @@ describe("shieldStatusForMembership self-trust behaviour", function() { describe("shieldStatusForMembership other-trust behaviour", function() { beforeAll(() => { - DMRoomMap._sharedInstance = { + DMRoomMap.sharedInstance = { getUserIdForRoomId: (roomId) => roomId === "DM" ? "@any:h" : null, }; }); From a3ca48b4dab568f32062924c14264c1502d47a3a Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 23 Apr 2021 12:19:38 +0100 Subject: [PATCH 05/16] Write more space store tests --- test/stores/SpaceStore-test.ts | 90 +++++++++++++++++++++++++++++++--- 1 file changed, 83 insertions(+), 7 deletions(-) diff --git a/test/stores/SpaceStore-test.ts b/test/stores/SpaceStore-test.ts index b27ca8622a..8ff671f78f 100644 --- a/test/stores/SpaceStore-test.ts +++ b/test/stores/SpaceStore-test.ts @@ -19,9 +19,11 @@ import { EventType } from "matrix-js-sdk/src/@types/event"; import "../skinned-sdk"; // Must be first for skinning to work import SpaceStore from "../../src/stores/SpaceStore"; import { resetAsyncStoreWithClient, setupAsyncStoreWithClient } from "../utils/test-utils"; -import { createTestClient, mkEvent, mkStubRoom } from "../test-utils"; +import { mkEvent, mkStubRoom, stubClient } from "../test-utils"; import { EnhancedMap } from "../../src/utils/maps"; import SettingsStore from "../../src/settings/SettingsStore"; +import DMRoomMap from "../../src/utils/DMRoomMap"; +import { MatrixClientPeg } from "../../src/MatrixClientPeg"; type MatrixEvent = any; // importing from js-sdk upsets things @@ -71,9 +73,14 @@ const mkSpace = (spaceId: string, children: string[] = []) => { const getValue = jest.fn(); SettingsStore.getValue = getValue; +const getUserIdForRoomId = jest.fn(); +// @ts-ignore +DMRoomMap.sharedInstance = { getUserIdForRoomId }; + describe("SpaceStore", () => { + stubClient(); const store = SpaceStore.instance; - const client = createTestClient(); + const client = MatrixClientPeg.get(); const run = async () => { client.getRoom.mockImplementation(roomId => rooms.find(room => room.roomId === roomId)); @@ -259,11 +266,80 @@ describe("SpaceStore", () => { }); describe("home space behaviour", () => { - test.todo("home space contains orphaned rooms"); - test.todo("home space contains favourites"); - test.todo("home space contains dm rooms"); - test.todo("home space contains invites"); - test.todo("home space contains invites even if they are also shown in a space"); + const fav1 = "!fav1:server"; + const fav2 = "!fav2:server"; + const fav3 = "!fav3:server"; + const dm1 = "!dm1:server"; + const dm1Partner = "@dm1Partner:server"; + const dm2 = "!dm2:server"; + const dm2Partner = "@dm2Partner:server"; + const dm3 = "!dm3:server"; + const dm3Partner = "@dm3Partner:server"; + const orphan1 = "!orphan1:server"; + const orphan2 = "!orphan2:server"; + const invite1 = "!invite1:server"; + const invite2 = "!invite2:server"; + const spaceRoom1 = "!spaceRoom1:server"; + const space1 = "!space1:server"; + const space2 = "!space2:server"; + const space3 = "!space3:server"; + + beforeEach(async () => { + [fav1, fav2, fav3, dm1, dm2, dm3, orphan1, orphan2, invite1, invite2, spaceRoom1].forEach(mkRoom); + mkSpace(space1, [fav1, spaceRoom1]); + mkSpace(space2, [fav2, fav3]); + mkSpace(space3, [invite2]); + + [fav1, fav2, fav3].forEach(roomId => { + client.getRoom(roomId).tags = { + "m.favourite": { + order: 0.5, + }, + }; + }); + + [invite1, invite2].forEach(roomId => { + client.getRoom(roomId).getMyMembership.mockReturnValue("invite"); + }); + + getUserIdForRoomId.mockImplementation(roomId => { + return { + [dm1]: dm1Partner, + [dm2]: dm2Partner, + [dm3]: dm3Partner, + }[roomId]; + }); + await run(); + }); + + it("home space contains orphaned rooms", () => { + expect(store.getSpaceFilteredRoomIds(null).has(orphan1)).toBeTruthy(); + expect(store.getSpaceFilteredRoomIds(null).has(orphan2)).toBeTruthy(); + }); + + it("home space contains favourites", () => { + expect(store.getSpaceFilteredRoomIds(null).has(fav1)).toBeTruthy(); + expect(store.getSpaceFilteredRoomIds(null).has(fav2)).toBeTruthy(); + expect(store.getSpaceFilteredRoomIds(null).has(fav3)).toBeTruthy(); + }); + + it("home space contains dm rooms", () => { + expect(store.getSpaceFilteredRoomIds(null).has(dm1)).toBeTruthy(); + expect(store.getSpaceFilteredRoomIds(null).has(dm2)).toBeTruthy(); + expect(store.getSpaceFilteredRoomIds(null).has(dm3)).toBeTruthy(); + }); + + it("home space contains invites", () => { + expect(store.getSpaceFilteredRoomIds(null).has(invite1)).toBeTruthy(); + }); + + it("home space contains invites even if they are also shown in a space", () => { + expect(store.getSpaceFilteredRoomIds(null).has(invite2)).toBeTruthy(); + }); + + it("home space does not contain rooms/low priority from rooms within spaces", () => { + expect(store.getSpaceFilteredRoomIds(null).has(spaceRoom1)).toBeFalsy(); + }); }); }); From c35678c64a8c17ad3e0331261965f5818ddc9c17 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 23 Apr 2021 13:40:16 +0100 Subject: [PATCH 06/16] Add yet more tests --- test/stores/SpaceStore-test.ts | 78 ++++++++++++++++++++-------------- 1 file changed, 47 insertions(+), 31 deletions(-) diff --git a/test/stores/SpaceStore-test.ts b/test/stores/SpaceStore-test.ts index 8ff671f78f..b7f4f7b49d 100644 --- a/test/stores/SpaceStore-test.ts +++ b/test/stores/SpaceStore-test.ts @@ -265,7 +265,7 @@ describe("SpaceStore", () => { expect(store.getChildSpaces("!d:server")).toStrictEqual([]); }); - describe("home space behaviour", () => { + describe("test fixture 1", () => { const fav1 = "!fav1:server"; const fav2 = "!fav2:server"; const fav3 = "!fav3:server"; @@ -287,7 +287,7 @@ describe("SpaceStore", () => { beforeEach(async () => { [fav1, fav2, fav3, dm1, dm2, dm3, orphan1, orphan2, invite1, invite2, spaceRoom1].forEach(mkRoom); mkSpace(space1, [fav1, spaceRoom1]); - mkSpace(space2, [fav2, fav3]); + mkSpace(space2, [fav1, fav2, fav3, spaceRoom1]); mkSpace(space3, [invite2]); [fav1, fav2, fav3].forEach(roomId => { @@ -340,6 +340,25 @@ describe("SpaceStore", () => { it("home space does not contain rooms/low priority from rooms within spaces", () => { expect(store.getSpaceFilteredRoomIds(null).has(spaceRoom1)).toBeFalsy(); }); + + it("space contains child rooms", () => { + const space = client.getRoom(space1); + expect(store.getSpaceFilteredRoomIds(space).has(fav1)).toBeTruthy(); + expect(store.getSpaceFilteredRoomIds(space).has(spaceRoom1)).toBeTruthy(); + }); + + it("space contains child favourites", () => { + const space = client.getRoom(space2); + expect(store.getSpaceFilteredRoomIds(space).has(fav1)).toBeTruthy(); + expect(store.getSpaceFilteredRoomIds(space).has(fav2)).toBeTruthy(); + expect(store.getSpaceFilteredRoomIds(space).has(fav3)).toBeTruthy(); + expect(store.getSpaceFilteredRoomIds(space).has(spaceRoom1)).toBeTruthy(); + }); + + it("space contains child invites", () => { + const space = client.getRoom(space3); + expect(store.getSpaceFilteredRoomIds(space).has(invite2)).toBeTruthy(); + }); }); }); @@ -368,7 +387,10 @@ describe("SpaceStore", () => { }); describe("space auto switching tests", () => { - test.todo("//auto pick space for a room"); + // it("no switch required, room is in target space"); + // it("switch to canonical parent space for room"); + // it("switch to first containing space for room"); + // it("switch to home for orphaned room"); }); describe("traverseSpace", () => { @@ -388,43 +410,37 @@ describe("SpaceStore", () => { }); it("avoids cycles", () => { - const seenMap = new Map(); - store.traverseSpace("!b:server", roomId => { - seenMap.set(roomId, (seenMap.get(roomId) || 0) + 1); - }); + const fn = jest.fn(); + store.traverseSpace("!b:server", fn); - expect(seenMap.size).toBe(3); - expect(seenMap.get("!a:server")).toBe(1); - expect(seenMap.get("!b:server")).toBe(1); - expect(seenMap.get("!c:server")).toBe(1); + expect(fn).toBeCalledTimes(3); + expect(fn).toBeCalledWith("!a:server"); + expect(fn).toBeCalledWith("!b:server"); + expect(fn).toBeCalledWith("!c:server"); }); it("including rooms", () => { - const seenMap = new Map(); - store.traverseSpace("!b:server", roomId => { - seenMap.set(roomId, (seenMap.get(roomId) || 0) + 1); - }, true); + const fn = jest.fn(); + store.traverseSpace("!b:server", fn, true); - expect(seenMap.size).toBe(7); - expect(seenMap.get("!a:server")).toBe(1); - expect(seenMap.get("!a-child:server")).toBe(1); - expect(seenMap.get("!b:server")).toBe(1); - expect(seenMap.get("!b-child:server")).toBe(1); - expect(seenMap.get("!c:server")).toBe(1); - expect(seenMap.get("!c-child:server")).toBe(1); - expect(seenMap.get("!shared-child:server")).toBe(2); + expect(fn).toBeCalledTimes(8); // twice for shared-child + expect(fn).toBeCalledWith("!a:server"); + expect(fn).toBeCalledWith("!a-child:server"); + expect(fn).toBeCalledWith("!b:server"); + expect(fn).toBeCalledWith("!b-child:server"); + expect(fn).toBeCalledWith("!c:server"); + expect(fn).toBeCalledWith("!c-child:server"); + expect(fn).toBeCalledWith("!shared-child:server"); }); it("excluding rooms", () => { - const seenMap = new Map(); - store.traverseSpace("!b:server", roomId => { - seenMap.set(roomId, (seenMap.get(roomId) || 0) + 1); - }, false); + const fn = jest.fn(); + store.traverseSpace("!b:server", fn, false); - expect(seenMap.size).toBe(3); - expect(seenMap.get("!a:server")).toBe(1); - expect(seenMap.get("!b:server")).toBe(1); - expect(seenMap.get("!c:server")).toBe(1); + expect(fn).toBeCalledTimes(3); + expect(fn).toBeCalledWith("!a:server"); + expect(fn).toBeCalledWith("!b:server"); + expect(fn).toBeCalledWith("!c:server"); }); }); }); From 320ff7b87004a73c9ee5fd0b3b27882aa69d42c2 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 23 Apr 2021 13:41:42 +0100 Subject: [PATCH 07/16] Fix invites relating to a space not showing in the space --- src/stores/SpaceStore.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/stores/SpaceStore.tsx b/src/stores/SpaceStore.tsx index 982c3d5d9f..d66d0c008c 100644 --- a/src/stores/SpaceStore.tsx +++ b/src/stores/SpaceStore.tsx @@ -195,7 +195,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { const childEvents = room?.currentState.getStateEvents(EventType.SpaceChild).filter(ev => ev.getContent()?.via); return sortBy(childEvents, getOrder) .map(ev => this.matrixClient.getRoom(ev.getStateKey())) - .filter(room => room?.getMyMembership() === "join") || []; + .filter(room => room?.getMyMembership() === "join" || room?.getMyMembership() === "invite") || []; } public getChildRooms(spaceId: string): Room[] { From 4446022327b6bcddde9a0be1c8f9dc64341713d3 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 23 Apr 2021 14:45:22 +0100 Subject: [PATCH 08/16] Add automatic space switching tests --- test/stores/SpaceStore-test.ts | 60 +++++++++++++++++++++++++++++++--- test/test-utils.js | 4 +++ 2 files changed, 60 insertions(+), 4 deletions(-) diff --git a/test/stores/SpaceStore-test.ts b/test/stores/SpaceStore-test.ts index b7f4f7b49d..4ee6111f88 100644 --- a/test/stores/SpaceStore-test.ts +++ b/test/stores/SpaceStore-test.ts @@ -24,6 +24,7 @@ import { EnhancedMap } from "../../src/utils/maps"; import SettingsStore from "../../src/settings/SettingsStore"; import DMRoomMap from "../../src/utils/DMRoomMap"; import { MatrixClientPeg } from "../../src/MatrixClientPeg"; +import defaultDispatcher from "../../src/dispatcher/dispatcher"; type MatrixEvent = any; // importing from js-sdk upsets things @@ -49,6 +50,7 @@ let rooms = []; const mkRoom = (roomId: string) => { const room = mkStubRoom(roomId); + room.currentState.getStateEvents.mockImplementation(mockStateEventImplementation([])); rooms.push(room); return room; }; @@ -82,6 +84,8 @@ describe("SpaceStore", () => { const store = SpaceStore.instance; const client = MatrixClientPeg.get(); + const viewRoom = roomId => defaultDispatcher.dispatch({ action: "view_room", room_id: roomId }, true); + const run = async () => { client.getRoom.mockImplementation(roomId => rooms.find(room => room.roomId === roomId)); await setupAsyncStoreWithClient(store, client); @@ -387,10 +391,58 @@ describe("SpaceStore", () => { }); describe("space auto switching tests", () => { - // it("no switch required, room is in target space"); - // it("switch to canonical parent space for room"); - // it("switch to first containing space for room"); - // it("switch to home for orphaned room"); + const space1 = "!space1:server"; + const space2 = "!space2:server"; + const room1 = "!room1:server"; // in space 1 & 2 + const room2 = "!room2:server"; // in space 1 & 2 (canonical) + const orphan1 = "!orphan:server"; + + beforeEach(async () => { + [room1, room2, orphan1].forEach(mkRoom); + mkSpace(space1, [room1, room2]); + mkSpace(space2, [room1, room2]); + + client.getRoom(room2).currentState.getStateEvents.mockImplementation(mockStateEventImplementation([ + mkEvent({ + event: true, + type: EventType.SpaceParent, + room: room2, + user: testUserId, + skey: space2, + content: { via: [], canonical: true }, + ts: Date.now(), + }), + ])); + await run(); + }); + + it("no switch required, room is in current space", async () => { + viewRoom(room1); + await store.setActiveSpace(client.getRoom(space1), false); + viewRoom(room2); + expect(store.activeSpace).toBe(client.getRoom(space1)); + }); + + it("switch to canonical parent space for room", async () => { + viewRoom(room1); + await store.setActiveSpace(null, false); + viewRoom(room2); + expect(store.activeSpace).toBe(client.getRoom(space2)); + }); + + it("switch to first containing space for room", async () => { + viewRoom(room2); + await store.setActiveSpace(null, false); + viewRoom(room1); + expect(store.activeSpace).toBe(client.getRoom(space1)); + }); + + it("switch to home for orphaned room", async () => { + viewRoom(room1); + await store.setActiveSpace(client.getRoom(space1), false); + viewRoom(orphan1); + expect(store.activeSpace).toBeNull(); + }); }); describe("traverseSpace", () => { diff --git a/test/test-utils.js b/test/test-utils.js index 4fc9bdf377..d344a7e9b1 100644 --- a/test/test-utils.js +++ b/test/test-utils.js @@ -79,6 +79,10 @@ export function createTestClient() { generateClientSecret: () => "t35tcl1Ent5ECr3T", isGuest: () => false, isCryptoEnabled: () => false, + getSpaceSummary: jest.fn().mockReturnValue({ + rooms: [], + events: [], + }), }; } From f5ab75cfddf0834821e5a32ac5236a9d6d0ede3c Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 23 Apr 2021 14:45:34 +0100 Subject: [PATCH 09/16] Fix automatic space switching behaviour to prioritise canonical parents --- src/stores/SpaceStore.tsx | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/stores/SpaceStore.tsx b/src/stores/SpaceStore.tsx index d66d0c008c..a72b270e0b 100644 --- a/src/stores/SpaceStore.tsx +++ b/src/stores/SpaceStore.tsx @@ -540,15 +540,12 @@ export class SpaceStoreClass extends AsyncStoreWithClient { // as this is not helpful and can create loops of rooms/space switching if (!room || payload.context_switch) break; - // persist last viewed room from a space - if (room.isSpaceRoom()) { this.setActiveSpace(room); } else if (!this.getSpaceFilteredRoomIds(this.activeSpace).has(room.roomId)) { - // TODO maybe reverse these first 2 clauses once space panel active is fixed - let parent = this.rootSpaces.find(s => this.spaceFilteredRooms.get(s.roomId)?.has(room.roomId)); + let parent = this.getCanonicalParent(room.roomId); if (!parent) { - parent = this.getCanonicalParent(room.roomId); + parent = this.rootSpaces.find(s => this.spaceFilteredRooms.get(s.roomId)?.has(room.roomId)); } if (!parent) { const parents = Array.from(this.parentMap.get(room.roomId) || []); From dd2a1d063a1d6360887b875b5d4d4c50a97cca9b Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Fri, 23 Apr 2021 16:14:55 +0100 Subject: [PATCH 10/16] Write tests for spaces context switching behavious --- test/stores/SpaceStore-test.ts | 119 ++++++++++++++++++++++++--------- 1 file changed, 88 insertions(+), 31 deletions(-) diff --git a/test/stores/SpaceStore-test.ts b/test/stores/SpaceStore-test.ts index 4ee6111f88..34e0186e90 100644 --- a/test/stores/SpaceStore-test.ts +++ b/test/stores/SpaceStore-test.ts @@ -79,6 +79,25 @@ const getUserIdForRoomId = jest.fn(); // @ts-ignore DMRoomMap.sharedInstance = { getUserIdForRoomId }; +const fav1 = "!fav1:server"; +const fav2 = "!fav2:server"; +const fav3 = "!fav3:server"; +const dm1 = "!dm1:server"; +const dm1Partner = "@dm1Partner:server"; +const dm2 = "!dm2:server"; +const dm2Partner = "@dm2Partner:server"; +const dm3 = "!dm3:server"; +const dm3Partner = "@dm3Partner:server"; +const orphan1 = "!orphan1:server"; +const orphan2 = "!orphan2:server"; +const invite1 = "!invite1:server"; +const invite2 = "!invite2:server"; +const room1 = "!room1:server"; +const room2 = "!room2:server"; +const space1 = "!space1:server"; +const space2 = "!space2:server"; +const space3 = "!space3:server"; + describe("SpaceStore", () => { stubClient(); const store = SpaceStore.instance; @@ -270,28 +289,10 @@ describe("SpaceStore", () => { }); describe("test fixture 1", () => { - const fav1 = "!fav1:server"; - const fav2 = "!fav2:server"; - const fav3 = "!fav3:server"; - const dm1 = "!dm1:server"; - const dm1Partner = "@dm1Partner:server"; - const dm2 = "!dm2:server"; - const dm2Partner = "@dm2Partner:server"; - const dm3 = "!dm3:server"; - const dm3Partner = "@dm3Partner:server"; - const orphan1 = "!orphan1:server"; - const orphan2 = "!orphan2:server"; - const invite1 = "!invite1:server"; - const invite2 = "!invite2:server"; - const spaceRoom1 = "!spaceRoom1:server"; - const space1 = "!space1:server"; - const space2 = "!space2:server"; - const space3 = "!space3:server"; - beforeEach(async () => { - [fav1, fav2, fav3, dm1, dm2, dm3, orphan1, orphan2, invite1, invite2, spaceRoom1].forEach(mkRoom); - mkSpace(space1, [fav1, spaceRoom1]); - mkSpace(space2, [fav1, fav2, fav3, spaceRoom1]); + [fav1, fav2, fav3, dm1, dm2, dm3, orphan1, orphan2, invite1, invite2, room1].forEach(mkRoom); + mkSpace(space1, [fav1, room1]); + mkSpace(space2, [fav1, fav2, fav3, room1]); mkSpace(space3, [invite2]); [fav1, fav2, fav3].forEach(roomId => { @@ -342,13 +343,13 @@ describe("SpaceStore", () => { }); it("home space does not contain rooms/low priority from rooms within spaces", () => { - expect(store.getSpaceFilteredRoomIds(null).has(spaceRoom1)).toBeFalsy(); + expect(store.getSpaceFilteredRoomIds(null).has(room1)).toBeFalsy(); }); it("space contains child rooms", () => { const space = client.getRoom(space1); expect(store.getSpaceFilteredRoomIds(space).has(fav1)).toBeTruthy(); - expect(store.getSpaceFilteredRoomIds(space).has(spaceRoom1)).toBeTruthy(); + expect(store.getSpaceFilteredRoomIds(space).has(room1)).toBeTruthy(); }); it("space contains child favourites", () => { @@ -356,7 +357,7 @@ describe("SpaceStore", () => { expect(store.getSpaceFilteredRoomIds(space).has(fav1)).toBeTruthy(); expect(store.getSpaceFilteredRoomIds(space).has(fav2)).toBeTruthy(); expect(store.getSpaceFilteredRoomIds(space).has(fav3)).toBeTruthy(); - expect(store.getSpaceFilteredRoomIds(space).has(spaceRoom1)).toBeTruthy(); + expect(store.getSpaceFilteredRoomIds(space).has(room1)).toBeTruthy(); }); it("space contains child invites", () => { @@ -387,16 +388,72 @@ describe("SpaceStore", () => { }); describe("context switching tests", () => { - test.todo("//context switching"); + const fn = jest.spyOn(defaultDispatcher, "dispatch"); + + beforeEach(async () => { + [room1, room2, orphan1].forEach(mkRoom); + mkSpace(space1, [room1, room2]); + mkSpace(space2, [room2]); + await run(); + }); + afterEach(() => { + fn.mockClear(); + localStorage.clear(); + }); + + const getCurrentRoom = () => fn.mock.calls.reverse().find(([p]) => p.action === "view_room")?.[0].room_id; + + it("last viewed room in target space is the current viewed and in both spaces", async () => { + await store.setActiveSpace(client.getRoom(space1)); + viewRoom(room2); + await store.setActiveSpace(client.getRoom(space2)); + viewRoom(room2); + await store.setActiveSpace(client.getRoom(space1)); + expect(getCurrentRoom()).toBe(room2); + }); + + it("last viewed room in target space is in the current space", async () => { + await store.setActiveSpace(client.getRoom(space1)); + viewRoom(room2); + await store.setActiveSpace(client.getRoom(space2)); + expect(getCurrentRoom()).toBe(space2); + await store.setActiveSpace(client.getRoom(space1)); + expect(getCurrentRoom()).toBe(room2); + }); + + it("last viewed room in target space is not in the current space", async () => { + await store.setActiveSpace(client.getRoom(space1)); + viewRoom(room1); + await store.setActiveSpace(client.getRoom(space2)); + viewRoom(room2); + await store.setActiveSpace(client.getRoom(space1)); + expect(getCurrentRoom()).toBe(room1); + }); + + it("last viewed room is target space is not known", async () => { + await store.setActiveSpace(client.getRoom(space1)); + viewRoom(room1); + localStorage.setItem(`mx_space_context_${space2}`, orphan2); + await store.setActiveSpace(client.getRoom(space2)); + expect(getCurrentRoom()).toBe(space2); + }); + + it("no last viewed room in target space", async () => { + await store.setActiveSpace(client.getRoom(space1)); + viewRoom(room1); + await store.setActiveSpace(client.getRoom(space2)); + expect(getCurrentRoom()).toBe(space2); + }); + + it("no last viewed room in home space", async () => { + await store.setActiveSpace(client.getRoom(space1)); + viewRoom(room1); + await store.setActiveSpace(null); + expect(fn.mock.calls[fn.mock.calls.length - 1][0]).toStrictEqual({ action: "view_home_page" }); + }); }); describe("space auto switching tests", () => { - const space1 = "!space1:server"; - const space2 = "!space2:server"; - const room1 = "!room1:server"; // in space 1 & 2 - const room2 = "!room2:server"; // in space 1 & 2 (canonical) - const orphan1 = "!orphan:server"; - beforeEach(async () => { [room1, room2, orphan1].forEach(mkRoom); mkSpace(space1, [room1, room2]); From f85d3643ee11bc6f90245446ab521883db70ed7e Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sat, 24 Apr 2021 11:31:52 +0100 Subject: [PATCH 11/16] Test and fix subspace invite receipt behaviour --- src/stores/SpaceStore.tsx | 3 ++- test/stores/SpaceStore-test.ts | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/stores/SpaceStore.tsx b/src/stores/SpaceStore.tsx index a72b270e0b..ad2aeb26ac 100644 --- a/src/stores/SpaceStore.tsx +++ b/src/stores/SpaceStore.tsx @@ -203,7 +203,8 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } public getChildSpaces(spaceId: string): Room[] { - return this.getChildren(spaceId).filter(r => r.isSpaceRoom()); + // don't show invited subspaces as they surface at the top level for better visibility + return this.getChildren(spaceId).filter(r => r.isSpaceRoom() && r.getMyMembership() === "join"); } public getParents(roomId: string, canonicalOnly = false): Room[] { diff --git a/test/stores/SpaceStore-test.ts b/test/stores/SpaceStore-test.ts index 34e0186e90..30000dcced 100644 --- a/test/stores/SpaceStore-test.ts +++ b/test/stores/SpaceStore-test.ts @@ -288,6 +288,17 @@ describe("SpaceStore", () => { expect(store.getChildSpaces("!d:server")).toStrictEqual([]); }); + it("invite to a subspace is only shown at the top level", async () => { + mkSpace(invite1).getMyMembership.mockReturnValue("invite"); + mkSpace(space1, [invite1]); + await run(); + + expect(store.spacePanelSpaces).toStrictEqual([client.getRoom(space1)]); + expect(store.getChildSpaces(space1)).toStrictEqual([]); + expect(store.getChildRooms(space1)).toStrictEqual([]); + expect(store.invitedSpaces).toStrictEqual([client.getRoom(invite1)]); + }); + describe("test fixture 1", () => { beforeEach(async () => { [fav1, fav2, fav3, dm1, dm2, dm3, orphan1, orphan2, invite1, invite2, room1].forEach(mkRoom); From da46e90896207e0092fb02a44b3738acfb3277e0 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sat, 24 Apr 2021 11:32:12 +0100 Subject: [PATCH 12/16] Fix SpaceStore reset behaviour --- src/stores/SpaceStore.tsx | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/stores/SpaceStore.tsx b/src/stores/SpaceStore.tsx index ad2aeb26ac..34a0f148ed 100644 --- a/src/stores/SpaceStore.tsx +++ b/src/stores/SpaceStore.tsx @@ -499,6 +499,17 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } }; + protected async reset() { + this.rootSpaces = []; + this.orphanedRooms = new Set(); + this.parentMap = new EnhancedMap(); + this.notificationStateMap = new Map(); + this.spaceFilteredRooms = new Map(); + this._activeSpace = null; + this._suggestedRooms = []; + this._invitedSpaces = new Set(); + } + protected async onNotReady() { if (!SettingsStore.getValue("feature_spaces")) return; if (this.matrixClient) { @@ -508,7 +519,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { this.matrixClient.removeListener("Room.accountData", this.onRoomAccountData); this.matrixClient.removeListener("accountData", this.onAccountData); } - await this.reset({}); + await this.reset(); } protected async onReady() { From 98851f8e644d7354834e2d42367624cfc71076ef Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sat, 24 Apr 2021 11:32:55 +0100 Subject: [PATCH 13/16] Text space switching behaviour and fix invalid space edge case --- src/stores/SpaceStore.tsx | 2 +- test/stores/SpaceStore-test.ts | 56 ++++++++++++++++++++++++++++++++-- 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/src/stores/SpaceStore.tsx b/src/stores/SpaceStore.tsx index 34a0f148ed..a3a404fdce 100644 --- a/src/stores/SpaceStore.tsx +++ b/src/stores/SpaceStore.tsx @@ -113,7 +113,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { } public async setActiveSpace(space: Room | null, contextSwitch = true) { - if (space === this.activeSpace) return; + if (space === this.activeSpace || (space && !space?.isSpaceRoom())) return; this._activeSpace = space; this.emit(UPDATE_SELECTED_SPACE, this.activeSpace); diff --git a/test/stores/SpaceStore-test.ts b/test/stores/SpaceStore-test.ts index 30000dcced..1e8048c8ff 100644 --- a/test/stores/SpaceStore-test.ts +++ b/test/stores/SpaceStore-test.ts @@ -17,7 +17,7 @@ limitations under the License. import { EventType } from "matrix-js-sdk/src/@types/event"; import "../skinned-sdk"; // Must be first for skinning to work -import SpaceStore from "../../src/stores/SpaceStore"; +import SpaceStore, {UPDATE_SELECTED_SPACE} from "../../src/stores/SpaceStore"; import { resetAsyncStoreWithClient, setupAsyncStoreWithClient } from "../utils/test-utils"; import { mkEvent, mkStubRoom, stubClient } from "../test-utils"; import { EnhancedMap } from "../../src/utils/maps"; @@ -387,7 +387,59 @@ describe("SpaceStore", () => { }); describe("active space switching tests", () => { - test.todo("//active space"); + const fn = jest.spyOn(store, "emit"); + + beforeEach(async () => { + mkRoom(room1); // not a space + mkSpace(space1, [ + mkSpace(space2).roomId, + ]); + mkSpace(space3).getMyMembership.mockReturnValue("invite"); + await run(); + await store.setActiveSpace(null); + expect(store.activeSpace).toBe(null); + }); + afterEach(() => { + fn.mockClear(); + }); + + it("switch to home space", async () => { + await store.setActiveSpace(client.getRoom(space1)); + fn.mockClear(); + + await store.setActiveSpace(null); + expect(fn).toHaveBeenCalledWith(UPDATE_SELECTED_SPACE, null); + expect(store.activeSpace).toBe(null); + }); + + it("switch to invited space", async () => { + const space = client.getRoom(space3); + await store.setActiveSpace(space); + expect(fn).toHaveBeenCalledWith(UPDATE_SELECTED_SPACE, space); + expect(store.activeSpace).toBe(space); + }); + + it("switch to top level space", async () => { + const space = client.getRoom(space1); + await store.setActiveSpace(space); + expect(fn).toHaveBeenCalledWith(UPDATE_SELECTED_SPACE, space); + expect(store.activeSpace).toBe(space); + }); + + it("switch to subspace", async () => { + const space = client.getRoom(space2); + await store.setActiveSpace(space); + expect(fn).toHaveBeenCalledWith(UPDATE_SELECTED_SPACE, space); + expect(store.activeSpace).toBe(space); + }); + + it("switch to unknown space is a nop", async () => { + expect(store.activeSpace).toBe(null); + const space = client.getRoom(room1); // not a space + await store.setActiveSpace(space); + expect(fn).not.toHaveBeenCalledWith(UPDATE_SELECTED_SPACE, space); + expect(store.activeSpace).toBe(null); + }); }); describe("notification state tests", () => { From 47c12a7d23d77a6507928f60d6ad13046a4f8792 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sun, 25 Apr 2021 09:24:00 +0100 Subject: [PATCH 14/16] Add tests for rooms moving around the SpaceStore --- test/stores/SpaceStore-test.ts | 99 +++++++++++++++++++++++++++++----- 1 file changed, 85 insertions(+), 14 deletions(-) diff --git a/test/stores/SpaceStore-test.ts b/test/stores/SpaceStore-test.ts index 1e8048c8ff..426290256e 100644 --- a/test/stores/SpaceStore-test.ts +++ b/test/stores/SpaceStore-test.ts @@ -14,10 +14,15 @@ See the License for the specific language governing permissions and limitations under the License. */ +import { EventEmitter } from "events"; import { EventType } from "matrix-js-sdk/src/@types/event"; import "../skinned-sdk"; // Must be first for skinning to work -import SpaceStore, {UPDATE_SELECTED_SPACE} from "../../src/stores/SpaceStore"; +import SpaceStore, { + UPDATE_INVITED_SPACES, + UPDATE_SELECTED_SPACE, + UPDATE_TOP_LEVEL_SPACES +} from "../../src/stores/SpaceStore"; import { resetAsyncStoreWithClient, setupAsyncStoreWithClient } from "../utils/test-utils"; import { mkEvent, mkStubRoom, stubClient } from "../test-utils"; import { EnhancedMap } from "../../src/utils/maps"; @@ -44,6 +49,8 @@ const mockStateEventImplementation = (events: MatrixEvent[]) => { }; }; +const emitPromise = (e: EventEmitter, k: string | symbol) => new Promise(r => e.once(k, r)); + const testUserId = "@test:user"; let rooms = []; @@ -108,6 +115,7 @@ describe("SpaceStore", () => { const run = async () => { client.getRoom.mockImplementation(roomId => rooms.find(room => room.roomId === roomId)); await setupAsyncStoreWithClient(store, client); + jest.runAllTimers(); }; beforeEach(() => { @@ -379,11 +387,82 @@ describe("SpaceStore", () => { }); describe("hierarchy resolution update tests", () => { - test.todo("updates state when spaces are joined"); - test.todo("updates state when spaces are left"); - test.todo("updates state when space invite comes in"); - test.todo("updates state when space invite is accepted"); - test.todo("updates state when space invite is rejected"); + let emitter: EventEmitter; + beforeEach(async () => { + emitter = new EventEmitter(); + client.on.mockImplementation(emitter.on.bind(emitter)); + client.removeListener.mockImplementation(emitter.removeListener.bind(emitter)); + }); + afterEach(() => { + client.on.mockReset(); + client.removeListener.mockReset(); + }); + + it("updates state when spaces are joined", async () => { + await run(); + expect(store.spacePanelSpaces).toStrictEqual([]); + const space = mkSpace(space1); + const prom = emitPromise(store, UPDATE_TOP_LEVEL_SPACES); + emitter.emit("Room", space); + await prom; + expect(store.spacePanelSpaces).toStrictEqual([space]); + expect(store.invitedSpaces).toStrictEqual([]); + }); + + it("updates state when spaces are left", async () => { + const space = mkSpace(space1); + await run(); + + expect(store.spacePanelSpaces).toStrictEqual([space]); + space.getMyMembership.mockReturnValue("leave"); + const prom = emitPromise(store, UPDATE_TOP_LEVEL_SPACES); + emitter.emit("Room.myMembership", space, "leave", "join"); + await prom; + expect(store.spacePanelSpaces).toStrictEqual([]); + }); + + it("updates state when space invite comes in", async () => { + await run(); + expect(store.spacePanelSpaces).toStrictEqual([]); + expect(store.invitedSpaces).toStrictEqual([]); + const space = mkSpace(space1); + space.getMyMembership.mockReturnValue("invite"); + const prom = emitPromise(store, UPDATE_INVITED_SPACES); + emitter.emit("Room", space); + await prom; + expect(store.spacePanelSpaces).toStrictEqual([]); + expect(store.invitedSpaces).toStrictEqual([space]); + }); + + it("updates state when space invite is accepted", async () => { + const space = mkSpace(space1); + space.getMyMembership.mockReturnValue("invite"); + await run(); + + expect(store.spacePanelSpaces).toStrictEqual([]); + expect(store.invitedSpaces).toStrictEqual([space]); + space.getMyMembership.mockReturnValue("join"); + const prom = emitPromise(store, UPDATE_TOP_LEVEL_SPACES); + emitter.emit("Room.myMembership", space, "join", "invite"); + await prom; + expect(store.spacePanelSpaces).toStrictEqual([space]); + expect(store.invitedSpaces).toStrictEqual([]); + }); + + it("updates state when space invite is rejected", async () => { + const space = mkSpace(space1); + space.getMyMembership.mockReturnValue("invite"); + await run(); + + expect(store.spacePanelSpaces).toStrictEqual([]); + expect(store.invitedSpaces).toStrictEqual([space]); + space.getMyMembership.mockReturnValue("leave"); + const prom = emitPromise(store, UPDATE_INVITED_SPACES); + emitter.emit("Room.myMembership", space, "leave", "invite"); + await prom; + expect(store.spacePanelSpaces).toStrictEqual([]); + expect(store.invitedSpaces).toStrictEqual([]); + }); }); describe("active space switching tests", () => { @@ -442,14 +521,6 @@ describe("SpaceStore", () => { }); }); - describe("notification state tests", () => { - test.todo("//notification states"); - }); - - describe("room list prefilter tests", () => { - test.todo("//room list filter"); - }); - describe("context switching tests", () => { const fn = jest.spyOn(defaultDispatcher, "dispatch"); From 3bb6edbda7c1424cc1816b636cf7a99188388cc3 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Sun, 25 Apr 2021 09:24:26 +0100 Subject: [PATCH 15/16] Fix accepting invite edge case where it wouldn't show the newly joined space --- src/stores/SpaceStore.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/stores/SpaceStore.tsx b/src/stores/SpaceStore.tsx index a3a404fdce..61ef1167ae 100644 --- a/src/stores/SpaceStore.tsx +++ b/src/stores/SpaceStore.tsx @@ -414,7 +414,7 @@ export class SpaceStoreClass extends AsyncStoreWithClient { if ((membership || room.getMyMembership()) === "invite") { this._invitedSpaces.add(room); this.emit(UPDATE_INVITED_SPACES, this.invitedSpaces); - } else if (oldMembership === "invite") { + } else if (oldMembership === "invite" && membership !== "join") { this._invitedSpaces.delete(room); this.emit(UPDATE_INVITED_SPACES, this.invitedSpaces); } else if (room?.isSpaceRoom()) { From 203425c8def330a6d7e304e05c355f9049bd42bb Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 26 Apr 2021 08:37:45 +0100 Subject: [PATCH 16/16] Test and fix space store wrongly treating room invites as space invites --- src/stores/SpaceStore.tsx | 41 ++++++++++++++++++++-------------- test/stores/SpaceStore-test.ts | 25 +++++++++++++++++++++ 2 files changed, 49 insertions(+), 17 deletions(-) diff --git a/src/stores/SpaceStore.tsx b/src/stores/SpaceStore.tsx index 61ef1167ae..f80a5f01c3 100644 --- a/src/stores/SpaceStore.tsx +++ b/src/stores/SpaceStore.tsx @@ -410,32 +410,39 @@ export class SpaceStoreClass extends AsyncStoreWithClient { }); }, 100, {trailing: true, leading: true}); - private onRoom = (room: Room, membership?: string, oldMembership?: string) => { - if ((membership || room.getMyMembership()) === "invite") { - this._invitedSpaces.add(room); - this.emit(UPDATE_INVITED_SPACES, this.invitedSpaces); - } else if (oldMembership === "invite" && membership !== "join") { - this._invitedSpaces.delete(room); - this.emit(UPDATE_INVITED_SPACES, this.invitedSpaces); - } else if (room?.isSpaceRoom()) { - this.onSpaceUpdate(); - this.emit(room.roomId); - } else { + private onRoom = (room: Room, newMembership?: string, oldMembership?: string) => { + const membership = newMembership || room.getMyMembership(); + + if (!room.isSpaceRoom()) { // this.onRoomUpdate(room); this.onRoomsUpdate(); - } - if (room.getMyMembership() === "join") { - if (!room.isSpaceRoom()) { + if (membership === "join") { + // the user just joined a room, remove it from the suggested list if it was there const numSuggestedRooms = this._suggestedRooms.length; this._suggestedRooms = this._suggestedRooms.filter(r => r.room_id !== room.roomId); if (numSuggestedRooms !== this._suggestedRooms.length) { this.emit(SUGGESTED_ROOMS, this._suggestedRooms); } - } else if (room.roomId === RoomViewStore.getRoomId()) { - // if the user was looking at the space and then joined: select that space - this.setActiveSpace(room); } + return; + } + + // Space + if (membership === "invite") { + this._invitedSpaces.add(room); + this.emit(UPDATE_INVITED_SPACES, this.invitedSpaces); + } else if (oldMembership === "invite" && membership !== "join") { + this._invitedSpaces.delete(room); + this.emit(UPDATE_INVITED_SPACES, this.invitedSpaces); + } else { + this.onSpaceUpdate(); + this.emit(room.roomId); + } + + if (membership === "join" && room.roomId === RoomViewStore.getRoomId()) { + // if the user was looking at the space and then joined: select that space + this.setActiveSpace(room); } }; diff --git a/test/stores/SpaceStore-test.ts b/test/stores/SpaceStore-test.ts index 426290256e..aef788647d 100644 --- a/test/stores/SpaceStore-test.ts +++ b/test/stores/SpaceStore-test.ts @@ -463,6 +463,31 @@ describe("SpaceStore", () => { expect(store.spacePanelSpaces).toStrictEqual([]); expect(store.invitedSpaces).toStrictEqual([]); }); + + it("room invite gets added to relevant space filters", async () => { + const space = mkSpace(space1, [invite1]); + await run(); + + expect(store.spacePanelSpaces).toStrictEqual([space]); + expect(store.invitedSpaces).toStrictEqual([]); + expect(store.getChildSpaces(space1)).toStrictEqual([]); + expect(store.getChildRooms(space1)).toStrictEqual([]); + expect(store.getSpaceFilteredRoomIds(client.getRoom(space1)).has(invite1)).toBeFalsy(); + expect(store.getSpaceFilteredRoomIds(null).has(invite1)).toBeFalsy(); + + const invite = mkRoom(invite1); + invite.getMyMembership.mockReturnValue("invite"); + const prom = emitPromise(store, space1); + emitter.emit("Room", space); + await prom; + + expect(store.spacePanelSpaces).toStrictEqual([space]); + expect(store.invitedSpaces).toStrictEqual([]); + expect(store.getChildSpaces(space1)).toStrictEqual([]); + expect(store.getChildRooms(space1)).toStrictEqual([invite]); + expect(store.getSpaceFilteredRoomIds(client.getRoom(space1)).has(invite1)).toBeTruthy(); + expect(store.getSpaceFilteredRoomIds(null).has(invite1)).toBeTruthy(); + }); }); describe("active space switching tests", () => {