Fix converttoroom & converttodm not working (#29705)

* Fix converttoroom & converttodm not working

setAccountData uses `deepCompare` within to avoid writing no-op updates

Signed-off-by: Michael Telatynski <[email protected]>

* Update tests

Signed-off-by: Michael Telatynski <[email protected]>

* Use filterValidMDirect utility in setDMRoom

Ensure we do not mutate the account data as this would then upset `setAccountData`'s deepCompare later

Signed-off-by: Michael Telatynski <[email protected]>

* Iterate

Signed-off-by: Michael Telatynski <[email protected]>

---------

Signed-off-by: Michael Telatynski <[email protected]>

Author: t3chguy

src/Rooms.ts

@@ -9,6 +9,7 @@ Please see LICENSE files in the repository root for full details.
 import { type Room, EventType, type RoomMember, type MatrixClient } from "matrix-js-sdk/src/matrix";
 
 import AliasCustomisations from "./customisations/Alias";
+import { filterValidMDirect } from "./utils/dm/filterValidMDirect.ts";
 
 /**
  * Given a room object, return the alias we should use for it,
@@ -56,39 +57,23 @@ export async function setDMRoom(client: MatrixClient, roomId: string, userId: st
     if (client.isGuest()) return;
 
     const mDirectEvent = client.getAccountData(EventType.Direct);
-    const currentContent = mDirectEvent?.getContent() || {};
-
-    const dmRoomMap = new Map(Object.entries(currentContent));
-    let modified = false;
-
-    // remove it from the lists of any others users
-    // (it can only be a DM room for one person)
-    for (const thisUserId of dmRoomMap.keys()) {
-        const roomList = dmRoomMap.get(thisUserId) || [];
-
-        if (thisUserId != userId) {
-            const indexOfRoom = roomList.indexOf(roomId);
-            if (indexOfRoom > -1) {
-                roomList.splice(indexOfRoom, 1);
-                modified = true;
-            }
-        }
+    const { filteredContent } = filterValidMDirect(mDirectEvent?.getContent() ?? {});
+
+    // remove it from the lists of all users (it can only be a DM room for one person)
+    for (const thisUserId in filteredContent) {
+        if (!filteredContent[thisUserId]) continue;
+        filteredContent[thisUserId] = filteredContent[thisUserId].filter((room) => room !== roomId);
     }
 
-    // now add it, if it's not already there
+    // now add it if the caller asked for it to be a DM room
     if (userId) {
-        const roomList = dmRoomMap.get(userId) || [];
-        if (roomList.indexOf(roomId) == -1) {
-            roomList.push(roomId);
-            modified = true;
+        if (!filteredContent[userId]) {
+            filteredContent[userId] = [];
         }
-        dmRoomMap.set(userId, roomList);
+        filteredContent[userId].push(roomId);
     }
 
-    // prevent unnecessary calls to setAccountData
-    if (!modified) return;
-
-    await client.setAccountData(EventType.Direct, Object.fromEntries(dmRoomMap));
+    await client.setAccountData(EventType.Direct, filteredContent);
 }
 
 /**

test/unit-tests/Rooms-test.ts

@@ -79,16 +79,6 @@ describe("setDMRoom", () => {
         });
     });
 
-    describe("when trying to add a DM, that already exists", () => {
-        beforeEach(() => {
-            setDMRoom(client, roomId1, userId1);
-        });
-
-        it("should not update the account data", () => {
-            expect(client.setAccountData).not.toHaveBeenCalled();
-        });
-    });
-
     describe("when removing an existing DM", () => {
         beforeEach(() => {
             setDMRoom(client, roomId1, null);
@@ -102,16 +92,6 @@ describe("setDMRoom", () => {
         });
     });
 
-    describe("when removing an unknown room", () => {
-        beforeEach(() => {
-            setDMRoom(client, roomId4, null);
-        });
-
-        it("should not update the account data", () => {
-            expect(client.setAccountData).not.toHaveBeenCalled();
-        });
-    });
-
     describe("when the direct event is undefined", () => {
         beforeEach(() => {
             mocked(client.getAccountData).mockReturnValue(undefined);