Merge pull request matrix-org#5424 from matrix-org/travis/fix-dms

Fix DM logic to always pick a more reliable DM room

Author: turt2live

src/components/views/dialogs/InviteDialog.js

@@ -31,7 +31,7 @@ import dis from "../../../dispatcher/dispatcher";
 import IdentityAuthClient from "../../../IdentityAuthClient";
 import Modal from "../../../Modal";
 import {humanizeTime} from "../../../utils/humanize";
-import createRoom, {canEncryptToAllUsers, privateShouldBeEncrypted} from "../../../createRoom";
+import createRoom, {canEncryptToAllUsers, findDMForUser, privateShouldBeEncrypted} from "../../../createRoom";
 import {inviteMultipleToRoom, showCommunityInviteDialog} from "../../../RoomInvite";
 import {Key} from "../../../Keyboard";
 import {Action} from "../../../dispatcher/actions";
@@ -41,6 +41,7 @@ import {CommunityPrototypeStore} from "../../../stores/CommunityPrototypeStore";
 import SettingsStore from "../../../settings/SettingsStore";
 import {UIFeature} from "../../../settings/UIFeature";
 import CountlyAnalytics from "../../../CountlyAnalytics";
+import {Room} from "matrix-js-sdk/src/models/room";
 
 // we have a number of types defined from the Matrix spec which can't reasonably be altered here.
 /* eslint-disable camelcase */
@@ -575,7 +576,12 @@ export default class InviteDialog extends React.PureComponent {
         const targetIds = targets.map(t => t.userId);
 
         // Check if there is already a DM with these people and reuse it if possible.
-        const existingRoom = DMRoomMap.shared().getDMRoomForIdentifiers(targetIds);
+        let existingRoom: Room;
+        if (targetIds.length === 1) {
+            existingRoom = findDMForUser(MatrixClientPeg.get(), targetIds[0]);
+        } else {
+            existingRoom = DMRoomMap.shared().getDMRoomForIdentifiers(targetIds);
+        }
         if (existingRoom) {
             dis.dispatch({
                 action: 'view_room',

src/components/views/right_panel/UserInfo.tsx

@@ -28,7 +28,7 @@ import {EventTimeline} from 'matrix-js-sdk/src/models/event-timeline';
 import dis from '../../../dispatcher/dispatcher';
 import Modal from '../../../Modal';
 import {_t} from '../../../languageHandler';
-import createRoom, {privateShouldBeEncrypted} from '../../../createRoom';
+import createRoom, { findDMForUser, privateShouldBeEncrypted } from '../../../createRoom';
 import DMRoomMap from '../../../utils/DMRoomMap';
 import AccessibleButton from '../elements/AccessibleButton';
 import SdkConfig from '../../../SdkConfig';
@@ -105,17 +105,7 @@ export const getE2EStatus = (cli: MatrixClient, userId: string, devices: IDevice
 };
 
 async function openDMForUser(matrixClient: MatrixClient, userId: string) {
-    const dmRooms = DMRoomMap.shared().getDMRoomsForUserId(userId);
-    const lastActiveRoom = dmRooms.reduce((lastActiveRoom, roomId) => {
-        const room = matrixClient.getRoom(roomId);
-        if (!room || room.getMyMembership() === "leave") {
-            return lastActiveRoom;
-        }
-        if (!lastActiveRoom || lastActiveRoom.getLastActiveTimestamp() < room.getLastActiveTimestamp()) {
-            return room;
-        }
-        return lastActiveRoom;
-    }, null);
+    const lastActiveRoom = findDMForUser(matrixClient, userId);
 
     if (lastActiveRoom) {
         dis.dispatch({

src/createRoom.ts

@@ -15,20 +15,21 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-import {MatrixClient} from "matrix-js-sdk/src/client";
-import {Room} from "matrix-js-sdk/src/models/room";
+import { MatrixClient } from "matrix-js-sdk/src/client";
+import { Room } from "matrix-js-sdk/src/models/room";
 
-import {MatrixClientPeg} from './MatrixClientPeg';
+import { MatrixClientPeg } from './MatrixClientPeg';
 import Modal from './Modal';
 import * as sdk from './index';
 import { _t } from './languageHandler';
 import dis from "./dispatcher/dispatcher";
 import * as Rooms from "./Rooms";
 import DMRoomMap from "./utils/DMRoomMap";
-import {getAddressType} from "./UserAddress";
+import { getAddressType } from "./UserAddress";
 import { getE2EEWellKnown } from "./utils/WellKnownUtils";
 import GroupStore from "./stores/GroupStore";
 import CountlyAnalytics from "./CountlyAnalytics";
+import { isJoinedOrNearlyJoined } from "./utils/membership";
 
 // we define a number of interfaces which take their names from the js-sdk
 /* eslint-disable camelcase */
@@ -236,9 +237,16 @@ export function findDMForUser(client: MatrixClient, userId: string): Room {
     const roomIds = DMRoomMap.shared().getDMRoomsForUserId(userId);
     const rooms = roomIds.map(id => client.getRoom(id));
     const suitableDMRooms = rooms.filter(r => {
+        // Validate that we are joined and the other person is also joined. We'll also make sure
+        // that the room also looks like a DM (until we have canonical DMs to tell us). For now,
+        // a DM is a room of two people that contains those two people exactly. This does mean
+        // that bots, assistants, etc will ruin a room's DM-ness, though this is a problem for
+        // canonical DMs to solve.
         if (r && r.getMyMembership() === "join") {
-            const member = r.getMember(userId);
-            return member && (member.membership === "invite" || member.membership === "join");
+            const members = r.currentState.getMembers();
+            const joinedMembers = members.filter(m => isJoinedOrNearlyJoined(m.membership));
+            const otherMember = joinedMembers.find(m => m.userId === userId);
+            return otherMember && joinedMembers.length === 2;
         }
         return false;
     }).sort((r1, r2) => {

src/utils/membership.ts

@@ -78,6 +78,11 @@ export function getEffectiveMembership(membership: string): EffectiveMembership
     }
 }
 
+export function isJoinedOrNearlyJoined(membership: string): boolean {
+    const effective = getEffectiveMembership(membership);
+    return effective === EffectiveMembership.Join || effective === EffectiveMembership.Invite;
+}
+
 export async function leaveRoomBehaviour(roomId: string) {
     let leavingAllVersions = true;
     const history = await MatrixClientPeg.get().getRoomUpgradeHistory(roomId);