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

Fixes https://github.com/vector-im/riot-web/issues/12418

Includes a refactor so we don't need to litter the code with the same magic string for DM tags.
pull/21833/head
Travis Ralston 2020-02-24 19:43:11 -07:00
parent 866fdd6185
commit 866ed68615
5 changed files with 45 additions and 24 deletions

View File

@ -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') ||
(oldTag === 'im.vector.fake.direct' && newTag === undefined)
if ((oldTag === undefined && newTag === TAG_DM) ||
(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

View File

@ -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 m.direct 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

View File

@ -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"),
},

View File

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

View File

@ -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', () => {