From b8f37a46f0b99ebe46c62f2e3885b30418ec5e29 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 3 Mar 2022 21:42:18 +0000 Subject: [PATCH] Fix bug with some space selections not being applied (#7971) --- src/stores/room-list/SpaceWatcher.ts | 7 +----- src/stores/spaces/SpaceStore.ts | 34 +++++++++++++++------------- test/stores/SpaceStore-test.ts | 14 ++++++++---- 3 files changed, 29 insertions(+), 26 deletions(-) diff --git a/src/stores/room-list/SpaceWatcher.ts b/src/stores/room-list/SpaceWatcher.ts index b0fd3c9478..af46a921cb 100644 --- a/src/stores/room-list/SpaceWatcher.ts +++ b/src/stores/room-list/SpaceWatcher.ts @@ -17,7 +17,7 @@ limitations under the License. import { RoomListStoreClass } from "./RoomListStore"; import { SpaceFilterCondition } from "./filters/SpaceFilterCondition"; import SpaceStore from "../spaces/SpaceStore"; -import { isMetaSpace, MetaSpace, SpaceKey, UPDATE_HOME_BEHAVIOUR, UPDATE_SELECTED_SPACE } from "../spaces"; +import { MetaSpace, SpaceKey, UPDATE_HOME_BEHAVIOUR, UPDATE_SELECTED_SPACE } from "../spaces"; /** * Watches for changes in spaces to manage the filter on the provided RoomListStore @@ -66,11 +66,6 @@ export class SpaceWatcher { }; private updateFilter = () => { - if (!isMetaSpace(this.activeSpace)) { - SpaceStore.instance.traverseSpace(this.activeSpace, roomId => { - this.store.matrixClient?.getRoom(roomId)?.loadMembersIfNeeded(); - }, false); - } this.filter.updateSpace(this.activeSpace); }; } diff --git a/src/stores/spaces/SpaceStore.ts b/src/stores/spaces/SpaceStore.ts index 6d2703d447..9f32816fbd 100644 --- a/src/stores/spaces/SpaceStore.ts +++ b/src/stores/spaces/SpaceStore.ts @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { ListIteratee, Many, sortBy, throttle } from "lodash"; +import { ListIteratee, Many, sortBy } from "lodash"; import { EventType, RoomType } from "matrix-js-sdk/src/@types/event"; import { Room, RoomEvent } from "matrix-js-sdk/src/models/room"; import { MatrixEvent } from "matrix-js-sdk/src/models/event"; @@ -235,6 +235,8 @@ export class SpaceStoreClass extends AsyncStoreWithClient { return; } + window.localStorage.setItem(ACTIVE_SPACE_LS_KEY, this._activeSpace = space); // Update & persist selected space + if (contextSwitch) { // view last selected room from space const roomId = window.localStorage.getItem(getSpaceContextKey(space)); @@ -251,36 +253,33 @@ export class SpaceStoreClass extends AsyncStoreWithClient { room_id: roomId, context_switch: true, metricsTrigger: "WebSpaceContextSwitch", - }, true); + }); } else if (cliSpace) { defaultDispatcher.dispatch({ action: Action.ViewRoom, room_id: space, context_switch: true, metricsTrigger: "WebSpaceContextSwitch", - }, true); + }); } else { defaultDispatcher.dispatch({ action: Action.ViewHomePage, context_switch: true, - }, true); + }); } } - // We can set the space after context switching as the dispatch handler which stores the last viewed room - // specifically no-ops on context_switch=true. - this._activeSpace = space; - // Emit after a synchronous dispatch for context switching to prevent racing with SpaceWatcher calling - // Room::loadMembersIfNeeded which could (via onMemberUpdate) call upon switchSpaceIfNeeded causing the - // space to wrongly bounce. this.emit(UPDATE_SELECTED_SPACE, this.activeSpace); this.emit(UPDATE_SUGGESTED_ROOMS, this._suggestedRooms = []); - // persist space selected - window.localStorage.setItem(ACTIVE_SPACE_LS_KEY, space); - if (cliSpace) { this.loadSuggestedRooms(cliSpace); + + // Load all members for the selected space and its subspaces, + // so we can correctly show DMs we have with members of this space. + SpaceStore.instance.traverseSpace(space, roomId => { + this.matrixClient.getRoom(roomId)?.loadMembersIfNeeded(); + }, false); } } @@ -683,7 +682,10 @@ export class SpaceStoreClass extends AsyncStoreWithClient { this.emit(space.roomId); affectedParentSpaceIds.forEach(spaceId => this.emit(spaceId)); - this.switchSpaceIfNeeded(); + if (!inSpace) { + // switch space if the DM is no longer considered part of the space + this.switchSpaceIfNeeded(); + } }; private onRoomsUpdate = () => { @@ -804,12 +806,12 @@ export class SpaceStoreClass extends AsyncStoreWithClient { this.updateNotificationStates(notificationStatesToUpdate); }; - private switchSpaceIfNeeded = throttle(() => { + private switchSpaceIfNeeded = () => { const roomId = RoomViewStore.getRoomId(); if (!this.isRoomInSpace(this.activeSpace, roomId) && !this.matrixClient.getRoom(roomId)?.isSpaceRoom()) { this.switchToRelatedSpace(roomId); } - }, 100, { leading: true, trailing: true }); + }; private switchToRelatedSpace = (roomId: string) => { if (this.suggestedRooms.find(r => r.room_id === roomId)) return; diff --git a/test/stores/SpaceStore-test.ts b/test/stores/SpaceStore-test.ts index d285f22f0b..5b6e776d31 100644 --- a/test/stores/SpaceStore-test.ts +++ b/test/stores/SpaceStore-test.ts @@ -867,6 +867,13 @@ describe("SpaceStore", () => { user: dm1Partner.userId, room: space1, }); + space.getMember.mockImplementation(userId => { + if (userId === dm1Partner.userId) { + const member = new RoomMember(space1, dm1Partner.userId); + member.membership = "join"; + return member; + } + }); client.emit(RoomStateEvent.Members, event, null, null); deferred.resolve(); @@ -885,10 +892,9 @@ describe("SpaceStore", () => { expect(space.loadMembersIfNeeded).not.toHaveBeenCalled(); store.setActiveSpace(space1, true); - // traverse the space and call loadMembersIfNeeded, similarly to SpaceWatcher's behaviour - store.traverseSpace(space1, roomId => { - client.getRoom(roomId)?.loadMembersIfNeeded(); - }, false); + jest.runOnlyPendingTimers(); + expect(space.loadMembersIfNeeded).toHaveBeenCalled(); + jest.runAllTimers(); expect(store.activeSpace).toBe(space1); expect(getCurrentRoom()).toBe(room1);