Fix bug with some space selections not being applied (#7971)

Author: t3chguy

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);
     };
 }

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<IState> {
             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<IState> {
                     room_id: roomId,
                     context_switch: true,
                     metricsTrigger: "WebSpaceContextSwitch",
-                }, true);
+                });
             } else if (cliSpace) {
                 defaultDispatcher.dispatch<ViewRoomPayload>({
                     action: Action.ViewRoom,
                     room_id: space,
                     context_switch: true,
                     metricsTrigger: "WebSpaceContextSwitch",
-                }, true);
+                });
             } else {
                 defaultDispatcher.dispatch<ViewHomePagePayload>({
                     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<IState> {
         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<IState> {
         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;

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);