Merge pull request #4123 from matrix-org/travis/invite-populate

Ensure DMs tagged outside of account data work in the invite dialog

src/actions/RoomListActions.js

@@ -15,7 +15,7 @@ limitations under the License.
 */
 
 import { asyncAction } from './actionCreators';
-import RoomListStore from '../stores/RoomListStore';
+import RoomListStore, {TAG_DM} from '../stores/RoomListStore';
 import Modal from '../Modal';
 import * as Rooms from '../Rooms';
 import { _t } from '../languageHandler';
@@ -73,11 +73,11 @@ RoomListActions.tagRoom = function(matrixClient, room, oldTag, newTag, oldIndex,
         const roomId = room.roomId;
 
         // Evil hack to get DMs behaving
-        if ((oldTag === undefined && newTag === 'im.vector.fake.direct') ||
+        if ((oldTag === undefined && newTag === TAG_DM) ||
-            (oldTag === 'im.vector.fake.direct' && newTag === undefined)
+            (oldTag === TAG_DM && newTag === undefined)
         ) {
             return Rooms.guessAndSetDMRoom(
-                room, newTag === 'im.vector.fake.direct',
+                room, newTag === TAG_DM,
             ).catch((err) => {
                 const ErrorDialog = sdk.getComponent("dialogs.ErrorDialog");
                 console.error("Failed to set direct chat tag " + err);
@@ -91,10 +91,10 @@ RoomListActions.tagRoom = function(matrixClient, room, oldTag, newTag, oldIndex,
         const hasChangedSubLists = oldTag !== newTag;
 
         // More evilness: We will still be dealing with moving to favourites/low prio,
-        // but we avoid ever doing a request with 'im.vector.fake.direct`.
+        // but we avoid ever doing a request with TAG_DM.
         //
         // if we moved lists, remove the old tag
-        if (oldTag && oldTag !== 'im.vector.fake.direct' &&
+        if (oldTag && oldTag !== TAG_DM &&
             hasChangedSubLists
         ) {
             const promiseToDelete = matrixClient.deleteRoomTag(
@@ -112,7 +112,7 @@ RoomListActions.tagRoom = function(matrixClient, room, oldTag, newTag, oldIndex,
         }
 
         // if we moved lists or the ordering changed, add the new tag
-        if (newTag && newTag !== 'im.vector.fake.direct' &&
+        if (newTag && newTag !== TAG_DM &&
             (hasChangedSubLists || metaData)
         ) {
             // metaData is the body of the PUT to set the tag, so it must

src/components/views/dialogs/InviteDialog.js

@@ -34,6 +34,7 @@ import {humanizeTime} from "../../../utils/humanize";
 import createRoom, {canEncryptToAllUsers} from "../../../createRoom";
 import {inviteMultipleToRoom} from "../../../RoomInvite";
 import SettingsStore from '../../../settings/SettingsStore';
+import RoomListStore, {TAG_DM} from "../../../stores/RoomListStore";
 
 export const KIND_DM = "dm";
 export const KIND_INVITE = "invite";
@@ -332,7 +333,23 @@ export default class InviteDialog extends React.PureComponent {
     }
 
     _buildRecents(excludedTargetIds: Set<string>): {userId: string, user: RoomMember, lastActive: number} {
-        const rooms = DMRoomMap.shared().getUniqueRoomsWithIndividuals();
+        const rooms = DMRoomMap.shared().getUniqueRoomsWithIndividuals(); // map of userId => js-sdk Room
+
+        // Also pull in all the rooms tagged as TAG_DM so we don't miss anything. Sometimes the
+        // room list doesn't tag the room for the DMRoomMap, but does for the room list.
+        const taggedRooms = RoomListStore.getRoomLists();
+        const dmTaggedRooms = taggedRooms[TAG_DM];
+        const myUserId = MatrixClientPeg.get().getUserId();
+        for (const dmRoom of dmTaggedRooms) {
+            const otherMembers = dmRoom.getJoinedMembers().filter(u => u.userId !== myUserId);
+            for (const member of otherMembers) {
+                if (rooms[member.userId]) continue; // already have a room
+
+                console.warn(`Adding DM room for ${member.userId} as ${dmRoom.roomId} from tag, not DM map`);
+                rooms[member.userId] = dmRoom;
+            }
+        }
+
         const recents = [];
         for (const userId in rooms) {
             // Filter out user IDs that are already in the room / should be excluded

src/components/views/rooms/RoomList.js

@@ -28,7 +28,7 @@ import rate_limited_func from "../../../ratelimitedfunc";
 import * as Rooms from '../../../Rooms';
 import DMRoomMap from '../../../utils/DMRoomMap';
 import TagOrderStore from '../../../stores/TagOrderStore';
-import RoomListStore from '../../../stores/RoomListStore';
+import RoomListStore, {TAG_DM} from '../../../stores/RoomListStore';
 import CustomRoomTagStore from '../../../stores/CustomRoomTagStore';
 import GroupStore from '../../../stores/GroupStore';
 import RoomSubList from '../../structures/RoomSubList';
@@ -718,11 +718,11 @@ export default createReactClass({
                 incomingCall: incomingCallIfTaggedAs('m.favourite'),
             },
             {
-                list: this.state.lists['im.vector.fake.direct'],
+                list: this.state.lists[TAG_DM],
                 label: _t('Direct Messages'),
-                tagName: "im.vector.fake.direct",
+                tagName: TAG_DM,
                 order: "recent",
-                incomingCall: incomingCallIfTaggedAs('im.vector.fake.direct'),
+                incomingCall: incomingCallIfTaggedAs(TAG_DM),
                 onAddRoom: () => {dis.dispatch({action: 'view_create_chat'});},
                 addRoomLabel: _t("Start chat"),
             },

src/stores/RoomListStore.js

@@ -37,11 +37,14 @@ const CATEGORY_BOLD = "bold";   // Unread messages (not notified, 'Mentions Only
 const CATEGORY_IDLE = "idle";   // Nothing of interest
 
 const CATEGORY_ORDER = [CATEGORY_RED, CATEGORY_GREY, CATEGORY_BOLD, CATEGORY_IDLE];
+
+export const TAG_DM = "im.vector.fake.direct";
+
 const LIST_ORDERS = {
     "m.favourite": "manual",
     "im.vector.fake.invite": "recent",
     "im.vector.fake.recent": "recent",
-    "im.vector.fake.direct": "recent",
+    [TAG_DM]: "recent",
     "m.lowpriority": "recent",
     "im.vector.fake.archived": "recent",
 };
@@ -95,7 +98,7 @@ class RoomListStore extends Store {
             "im.vector.fake.invite": [],
             "m.favourite": [],
             "im.vector.fake.recent": [],
-            "im.vector.fake.direct": [],
+            [TAG_DM]: [],
             "m.lowpriority": [],
             "im.vector.fake.archived": [],
         };
@@ -323,7 +326,7 @@ class RoomListStore extends Store {
             } else if (dmRoomMap.getUserIdForRoomId(room.roomId) && tags.length === 0) {
                 // We intentionally don't duplicate rooms in other tags into the people list
                 // as a feature.
-                tags.push("im.vector.fake.direct");
+                tags.push(TAG_DM);
             } else if (tags.length === 0) {
                 tags.push("im.vector.fake.recent");
             }
@@ -553,7 +556,7 @@ class RoomListStore extends Store {
             "im.vector.fake.invite": [],
             "m.favourite": [],
             "im.vector.fake.recent": [],
-            "im.vector.fake.direct": [],
+            [TAG_DM]: [],
             "m.lowpriority": [],
             "im.vector.fake.archived": [],
         };
@@ -590,7 +593,7 @@ class RoomListStore extends Store {
                     }
                 } else if (dmRoomMap.getUserIdForRoomId(room.roomId)) {
                     // "Direct Message" rooms (that we're still in and that aren't otherwise tagged)
-                    lists["im.vector.fake.direct"].push({room, category: this._calculateCategory(room)});
+                    lists[TAG_DM].push({room, category: this._calculateCategory(room)});
                 } else {
                     lists["im.vector.fake.recent"].push({room, category: this._calculateCategory(room)});
                 }

test/components/views/rooms/RoomList-test.js

@@ -14,6 +14,7 @@ import DMRoomMap from '../../../../src/utils/DMRoomMap.js';
 import GroupStore from '../../../../src/stores/GroupStore.js';
 
 import { MatrixClient, Room, RoomMember } from 'matrix-js-sdk';
+import {TAG_DM} from "../../../../src/stores/RoomListStore";
 
 function generateRoomId() {
     return '!' + Math.random().toString().slice(2, 10) + ':domain';
@@ -152,7 +153,7 @@ describe('RoomList', () => {
         // Set up the room that will be moved such that it has the correct state for a room in
         // the section for oldTag
         if (['m.favourite', 'm.lowpriority'].includes(oldTag)) movingRoom.tags = {[oldTag]: {}};
-        if (oldTag === 'im.vector.fake.direct') {
+        if (oldTag === TAG_DM) {
             // Mock inverse m.direct
             DMRoomMap.shared().roomToUser = {
                 [movingRoom.roomId]: '@someotheruser:domain',
@@ -179,7 +180,7 @@ describe('RoomList', () => {
         // TODO: Re-enable dragging tests when we support dragging again.
         describe.skip('does correct optimistic update when dragging from', () => {
             it('rooms to people', () => {
-                expectCorrectMove(undefined, 'im.vector.fake.direct');
+                expectCorrectMove(undefined, TAG_DM);
             });
 
             it('rooms to favourites', () => {
@@ -194,15 +195,15 @@ describe('RoomList', () => {
             // Whe running the app live, it updates when some other event occurs (likely the
             // m.direct arriving) that these tests do not fire.
             xit('people to rooms', () => {
-                expectCorrectMove('im.vector.fake.direct', undefined);
+                expectCorrectMove(TAG_DM, undefined);
             });
 
             it('people to favourites', () => {
-                expectCorrectMove('im.vector.fake.direct', 'm.favourite');
+                expectCorrectMove(TAG_DM, 'm.favourite');
             });
 
             it('people to lowpriority', () => {
-                expectCorrectMove('im.vector.fake.direct', 'm.lowpriority');
+                expectCorrectMove(TAG_DM, 'm.lowpriority');
             });
 
             it('low priority to rooms', () => {
@@ -210,7 +211,7 @@ describe('RoomList', () => {
             });
 
             it('low priority to people', () => {
-                expectCorrectMove('m.lowpriority', 'im.vector.fake.direct');
+                expectCorrectMove('m.lowpriority', TAG_DM);
             });
 
             it('low priority to low priority', () => {
@@ -222,7 +223,7 @@ describe('RoomList', () => {
             });
 
             it('favourites to people', () => {
-                expectCorrectMove('m.favourite', 'im.vector.fake.direct');
+                expectCorrectMove('m.favourite', TAG_DM);
             });
 
             it('favourites to low priority', () => {