Tests for RoomListStore's predecessor handling (#10046)

src/stores/room-list/RoomListStore.ts

@@ -18,6 +18,7 @@ import { MatrixClient } from "matrix-js-sdk/src/client";
 import { Room } from "matrix-js-sdk/src/models/room";
 import { logger } from "matrix-js-sdk/src/logger";
 import { EventType } from "matrix-js-sdk/src/@types/event";
+import { RoomState } from "matrix-js-sdk/src/matrix";
 
 import SettingsStore from "../../settings/SettingsStore";
 import { DefaultTagID, OrderedDefaultTagIDs, RoomUpdateCause, TagID } from "./models";
@@ -267,44 +268,55 @@ export class RoomListStoreClass extends AsyncStoreWithClient<IState> implements
             }
             this.updateFn.trigger();
         } else if (payload.action === "MatrixActions.Room.myMembership") {
-            const membershipPayload = <any>payload; // TODO: Type out the dispatcher types
+            this.onDispatchMyMembership(<any>payload);
-            const oldMembership = getEffectiveMembership(membershipPayload.oldMembership);
+            return;
-            const newMembership = getEffectiveMembership(membershipPayload.membership);
+        }
-            if (oldMembership !== EffectiveMembership.Join && newMembership === EffectiveMembership.Join) {
+    }
-                // If we're joining an upgraded room, we'll want to make sure we don't proliferate
-                // the dead room in the list.
-                const createEvent = membershipPayload.room.currentState.getStateEvents(EventType.RoomCreate, "");
-                if (createEvent && createEvent.getContent()["predecessor"]) {
-                    const prevRoom = this.matrixClient.getRoom(createEvent.getContent()["predecessor"]["room_id"]);
-                    if (prevRoom) {
-                        const isSticky = this.algorithm.stickyRoom === prevRoom;
-                        if (isSticky) {
-                            this.algorithm.setStickyRoom(null);
-                        }
 
-                        // Note: we hit the algorithm instead of our handleRoomUpdate() function to
+    /**
-                        // avoid redundant updates.
+     * Handle a MatrixActions.Room.myMembership event from the dispatcher.
-                        this.algorithm.handleRoomUpdate(prevRoom, RoomUpdateCause.RoomRemoved);
+     *
+     * Public for test.
+     */
+    public async onDispatchMyMembership(membershipPayload: any): Promise<void> {
+        // TODO: Type out the dispatcher types so membershipPayload is not any
+        const oldMembership = getEffectiveMembership(membershipPayload.oldMembership);
+        const newMembership = getEffectiveMembership(membershipPayload.membership);
+        if (oldMembership !== EffectiveMembership.Join && newMembership === EffectiveMembership.Join) {
+            // If we're joining an upgraded room, we'll want to make sure we don't proliferate
+            // the dead room in the list.
+            const roomState: RoomState = membershipPayload.room.currentState;
+            const createEvent = roomState.getStateEvents(EventType.RoomCreate, "");
+            if (createEvent && createEvent.getContent()["predecessor"]) {
+                const prevRoom = this.matrixClient.getRoom(createEvent.getContent()["predecessor"]["room_id"]);
+                if (prevRoom) {
+                    const isSticky = this.algorithm.stickyRoom === prevRoom;
+                    if (isSticky) {
+                        this.algorithm.setStickyRoom(null);
                     }
+
+                    // Note: we hit the algorithm instead of our handleRoomUpdate() function to
+                    // avoid redundant updates.
+                    this.algorithm.handleRoomUpdate(prevRoom, RoomUpdateCause.RoomRemoved);
                 }
-
-                await this.handleRoomUpdate(membershipPayload.room, RoomUpdateCause.NewRoom);
-                this.updateFn.trigger();
-                return;
             }
 
-            if (oldMembership !== EffectiveMembership.Invite && newMembership === EffectiveMembership.Invite) {
+            await this.handleRoomUpdate(membershipPayload.room, RoomUpdateCause.NewRoom);
-                await this.handleRoomUpdate(membershipPayload.room, RoomUpdateCause.NewRoom);
+            this.updateFn.trigger();
-                this.updateFn.trigger();
+            return;
-                return;
+        }
-            }
 
-            // If it's not a join, it's transitioning into a different list (possibly historical)
+        if (oldMembership !== EffectiveMembership.Invite && newMembership === EffectiveMembership.Invite) {
-            if (oldMembership !== newMembership) {
+            await this.handleRoomUpdate(membershipPayload.room, RoomUpdateCause.NewRoom);
-                await this.handleRoomUpdate(membershipPayload.room, RoomUpdateCause.PossibleTagChange);
+            this.updateFn.trigger();
-                this.updateFn.trigger();
+            return;
-                return;
+        }
-            }
+
+        // If it's not a join, it's transitioning into a different list (possibly historical)
+        if (oldMembership !== newMembership) {
+            await this.handleRoomUpdate(membershipPayload.room, RoomUpdateCause.PossibleTagChange);
+            this.updateFn.trigger();
+            return;
         }
     }
 

test/stores/room-list/RoomListStore-test.ts

@@ -14,14 +14,55 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+import { EventType, MatrixEvent, Room } from "matrix-js-sdk/src/matrix";
+
+import { MatrixDispatcher } from "../../../src/dispatcher/dispatcher";
 import { ListAlgorithm, SortAlgorithm } from "../../../src/stores/room-list/algorithms/models";
-import { OrderedDefaultTagIDs } from "../../../src/stores/room-list/models";
+import { OrderedDefaultTagIDs, RoomUpdateCause } from "../../../src/stores/room-list/models";
 import RoomListStore, { RoomListStoreClass } from "../../../src/stores/room-list/RoomListStore";
-import { stubClient } from "../../test-utils";
+import { stubClient, upsertRoomStateEvents } from "../../test-utils";
 
 describe("RoomListStore", () => {
+    const client = stubClient();
+    const roomWithCreatePredecessorId = "!roomid:example.com";
+    const roomNoPredecessorId = "!roomnopreid:example.com";
+    const oldRoomId = "!oldroomid:example.com";
+    const userId = "@user:example.com";
+    const createWithPredecessor = new MatrixEvent({
+        type: EventType.RoomCreate,
+        sender: userId,
+        room_id: roomWithCreatePredecessorId,
+        content: {
+            predecessor: { room_id: oldRoomId, event_id: "tombstone_event_id" },
+        },
+        event_id: "$create",
+        state_key: "",
+    });
+    const createNoPredecessor = new MatrixEvent({
+        type: EventType.RoomCreate,
+        sender: userId,
+        room_id: roomWithCreatePredecessorId,
+        content: {},
+        event_id: "$create",
+        state_key: "",
+    });
+    const roomWithCreatePredecessor = new Room(roomWithCreatePredecessorId, client, userId, {});
+    upsertRoomStateEvents(roomWithCreatePredecessor, [createWithPredecessor]);
+    const roomNoPredecessor = new Room(roomNoPredecessorId, client, userId, {});
+    upsertRoomStateEvents(roomNoPredecessor, [createNoPredecessor]);
+    const oldRoom = new Room(oldRoomId, client, userId, {});
+    client.getRoom = jest.fn().mockImplementation((roomId) => {
+        switch (roomId) {
+            case roomWithCreatePredecessorId:
+                return roomWithCreatePredecessor;
+            case oldRoomId:
+                return oldRoom;
+            default:
+                return null;
+        }
+    });
+
     beforeAll(async () => {
-        const client = stubClient();
         await (RoomListStore.instance as RoomListStoreClass).makeReady(client);
     });
 
@@ -32,4 +73,54 @@ describe("RoomListStore", () => {
     it.each(OrderedDefaultTagIDs)("defaults to activity ordering for %s=", (tagId) => {
         expect(RoomListStore.instance.getListOrder(tagId)).toBe(ListAlgorithm.Importance);
     });
+
+    function createStore(): { store: RoomListStoreClass; handleRoomUpdate: jest.Mock<any, any> } {
+        const fakeDispatcher = { register: jest.fn() } as unknown as MatrixDispatcher;
+        const store = new RoomListStoreClass(fakeDispatcher);
+        // @ts-ignore accessing private member to set client
+        store.readyStore.matrixClient = client;
+        const handleRoomUpdate = jest.fn();
+        // @ts-ignore accessing private member to mock it
+        store.algorithm.handleRoomUpdate = handleRoomUpdate;
+
+        return { store, handleRoomUpdate };
+    }
+
+    it("Removes old room if it finds a predecessor in the create event", () => {
+        // Given a store we can spy on
+        const { store, handleRoomUpdate } = createStore();
+
+        // When we tell it we joined a new room that has an old room as
+        // predecessor in the create event
+        const payload = {
+            oldMembership: "invite",
+            membership: "join",
+            room: roomWithCreatePredecessor,
+        };
+        store.onDispatchMyMembership(payload);
+
+        // Then the old room is removed
+        expect(handleRoomUpdate).toHaveBeenCalledWith(oldRoom, RoomUpdateCause.RoomRemoved);
+
+        // And the new room is added
+        expect(handleRoomUpdate).toHaveBeenCalledWith(roomWithCreatePredecessor, RoomUpdateCause.NewRoom);
+    });
+
+    it("Does not remove old room if there is no predecessor in the create event", () => {
+        // Given a store we can spy on
+        const { store, handleRoomUpdate } = createStore();
+
+        // When we tell it we joined a new room with no predecessor
+        const payload = {
+            oldMembership: "invite",
+            membership: "join",
+            room: roomNoPredecessor,
+        };
+        store.onDispatchMyMembership(payload);
+
+        // Then the new room is added
+        expect(handleRoomUpdate).toHaveBeenCalledWith(roomNoPredecessor, RoomUpdateCause.NewRoom);
+        // And no other updates happen
+        expect(handleRoomUpdate).toHaveBeenCalledTimes(1);
+    });
 });