Check if users exist before inviting them and communicate errors

Fixes https://github.com/vector-im/riot-web/issues/3283
Fixes https://github.com/vector-im/riot-web/issues/3968
Fixes https://github.com/vector-im/riot-web/issues/4308
Fixes https://github.com/vector-im/riot-web/issues/1597
Fixes https://github.com/vector-im/riot-web/issues/6790

This does 3 things:
* Makes the `MultiInviter` check for a user profile before attempting an invite. This is to prove the user exists.
* Use the `MultiInviter` everywhere to avoid duplicating the logic. Although a couple places only invite one user, it is still worthwhile.
* Communicate errors from the `MultiInviter` to the user in all cases. This is done through dialogs, where some existed previously but were not invoked.

Specifically to the 403 error not working: What was happening was the `MultiInviter` loop was setting the `fatal` flag, but that didn't resolve the promise it stored. This caused a promise to always be open, therefore never hitting a dialog.

src/RoomInvite.js

@@ -1,6 +1,6 @@
 /*
 Copyright 2016 OpenMarket Ltd
-Copyright 2017 New Vector Ltd
+Copyright 2017, 2018 New Vector Ltd
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -15,6 +15,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+import React from 'react';
 import MatrixClientPeg from './MatrixClientPeg';
 import MultiInviter from './utils/MultiInviter';
 import Modal from './Modal';
@@ -25,18 +26,6 @@ import dis from './dispatcher';
 import DMRoomMap from './utils/DMRoomMap';
 import { _t } from './languageHandler';
 
-export function inviteToRoom(roomId, addr) {
-    const addrType = getAddressType(addr);
-
-    if (addrType == 'email') {
-        return MatrixClientPeg.get().inviteByEmail(roomId, addr);
-    } else if (addrType == 'mx-user-id') {
-        return MatrixClientPeg.get().invite(roomId, addr);
-    } else {
-        throw new Error('Unsupported address');
-    }
-}
-
 /**
  * Invites multiple addresses to a room
  * Simpler interface to utils/MultiInviter but with
@@ -46,9 +35,9 @@ export function inviteToRoom(roomId, addr) {
  * @param {string[]} addrs Array of strings of addresses to invite. May be matrix IDs or 3pids.
  * @returns {Promise} Promise
  */
-export function inviteMultipleToRoom(roomId, addrs) {
+function inviteMultipleToRoom(roomId, addrs) {
     const inviter = new MultiInviter(roomId);
-    return inviter.invite(addrs);
+    return inviter.invite(addrs).then(addrs => Promise.resolve({addrs, inviter}));
 }
 
 export function showStartChatInviteDialog() {
@@ -129,8 +118,8 @@ function _onStartChatFinished(shouldInvite, addrs) {
         createRoom().then((roomId) => {
             room = MatrixClientPeg.get().getRoom(roomId);
             return inviteMultipleToRoom(roomId, addrTexts);
-        }).then((addrs) => {
+        }).then((result) => {
-            return _showAnyInviteErrors(addrs, room);
+            return _showAnyInviteErrors(result.addrs, room, result.inviter);
         }).catch((err) => {
             console.error(err.stack);
             const ErrorDialog = sdk.getComponent("dialogs.ErrorDialog");
@@ -148,9 +137,9 @@ function _onRoomInviteFinished(roomId, shouldInvite, addrs) {
     const addrTexts = addrs.map((addr) => addr.address);
 
     // Invite new users to a room
-    inviteMultipleToRoom(roomId, addrTexts).then((addrs) => {
+    inviteMultipleToRoom(roomId, addrTexts).then((result) => {
         const room = MatrixClientPeg.get().getRoom(roomId);
-        return _showAnyInviteErrors(addrs, room);
+        return _showAnyInviteErrors(result.addrs, room, result.inviter);
     }).catch((err) => {
         console.error(err.stack);
         const ErrorDialog = sdk.getComponent("dialogs.ErrorDialog");
@@ -169,12 +158,24 @@ function _isDmChat(addrTexts) {
     }
 }
 
-function _showAnyInviteErrors(addrs, room) {
+function _showAnyInviteErrors(addrs, room, inviter) {
     // Show user any errors
+    const failedUsers = Object.keys(addrs).filter(a => addrs[a] === 'error');
+    if (failedUsers.length === 1 && inviter.fatal) {
+        // Just get the first message because there was a fatal problem on the first
+        // user. This usually means that no other users were attempted, making it
+        // pointless for us to list who failed exactly.
+        const ErrorDialog = sdk.getComponent("dialogs.ErrorDialog");
+        Modal.createTrackedDialog('Failed to invite users to the room', '', ErrorDialog, {
+            title: _t("Failed to invite users to the room:", {roomName: room.name}),
+            description: inviter.getErrorText(failedUsers[0]),
+        });
+    } else {
         const errorList = [];
-    for (const addr of Object.keys(addrs)) {
+        for (const addr of failedUsers) {
             if (addrs[addr] === "error") {
-            errorList.push(addr);
+                const reason = inviter.getErrorText(addr);
+                errorList.push(addr + ": " + reason);
             }
         }
 
@@ -182,9 +183,11 @@ function _showAnyInviteErrors(addrs, room) {
             const ErrorDialog = sdk.getComponent("dialogs.ErrorDialog");
             Modal.createTrackedDialog('Failed to invite the following users to the room', '', ErrorDialog, {
                 title: _t("Failed to invite the following users to the %(roomName)s room:", {roomName: room.name}),
-            description: errorList.join(", "),
+                description: errorList.join(<br />),
             });
         }
+    }
+
     return addrs;
 }
 

src/SlashCommands.js

@@ -26,6 +26,7 @@ import Modal from './Modal';
 import SettingsStore, {SettingLevel} from './settings/SettingsStore';
 import {MATRIXTO_URL_PATTERN} from "./linkify-matrix";
 import * as querystring from "querystring";
+import MultiInviter from './utils/MultiInviter';
 
 
 class Command {
@@ -142,7 +143,14 @@ export const CommandMap = {
             if (args) {
                 const matches = args.match(/^(\S+)$/);
                 if (matches) {
-                    return success(MatrixClientPeg.get().invite(roomId, matches[1]));
+                    // We use a MultiInviter to re-use the invite logic, even though
+                    // we're only inviting one user.
+                    const userId = matches[1];
+                    const inviter = new MultiInviter(roomId);
+                    return success(inviter.invite([userId]).then(() => {
+                        if (inviter.getCompletionState(userId) !== "invited")
+                            throw new Error(inviter.getErrorText(userId));
+                    }));
                 }
             }
             return reject(this.getUsage());

src/components/views/rooms/MemberInfo.js

@@ -41,6 +41,7 @@ import withMatrixClient from '../../../wrappers/withMatrixClient';
 import AccessibleButton from '../elements/AccessibleButton';
 import RoomViewStore from '../../../stores/RoomViewStore';
 import SdkConfig from '../../../SdkConfig';
+import MultiInviter from "../../../utils/MultiInviter";
 
 module.exports = withMatrixClient(React.createClass({
     displayName: 'MemberInfo',
@@ -714,12 +715,18 @@ module.exports = withMatrixClient(React.createClass({
                 const roomId = member && member.roomId ? member.roomId : RoomViewStore.getRoomId();
                 const onInviteUserButton = async() => {
                     try {
-                        await cli.invite(roomId, member.userId);
+                        // We use a MultiInviter to re-use the invite logic, even though
+                        // we're only inviting one user.
+                        const inviter = new MultiInviter(roomId);
+                        await inviter.invite([member.userId]).then(() => {
+                            if (inviter.getCompletionState(userId) !== "invited")
+                                throw new Error(inviter.getErrorText(userId));
+                        });
                     } catch (err) {
                         const ErrorDialog = sdk.getComponent('dialogs.ErrorDialog');
                         Modal.createTrackedDialog('Failed to invite', '', ErrorDialog, {
                             title: _t('Failed to invite'),
-                            description: ((err && err.message) ? err.message : "Operation failed"),
+                            description: ((err && err.message) ? err.message : _t("Operation failed")),
                         });
                     }
                 };

src/i18n/strings/en_EN.json

@@ -106,6 +106,7 @@
     "Failed to invite user": "Failed to invite user",
     "Operation failed": "Operation failed",
     "Failed to invite": "Failed to invite",
+    "Failed to invite users to the room:": "Failed to invite users to the room:",
     "Failed to invite the following users to the %(roomName)s room:": "Failed to invite the following users to the %(roomName)s room:",
     "You need to be logged in.": "You need to be logged in.",
     "You need to be able to invite users to do that.": "You need to be able to invite users to do that.",
@@ -220,6 +221,9 @@
     "Your browser does not support the required cryptography extensions": "Your browser does not support the required cryptography extensions",
     "Not a valid Riot keyfile": "Not a valid Riot keyfile",
     "Authentication check failed: incorrect password?": "Authentication check failed: incorrect password?",
+    "You do not have permission to invite people to this room.": "You do not have permission to invite people to this room.",
+    "User %(user_id)s does not exist": "User %(user_id)s does not exist",
+    "Unknown server error": "Unknown server error",
     "Sorry, your homeserver is too old to participate in this room.": "Sorry, your homeserver is too old to participate in this room.",
     "Please contact your homeserver administrator.": "Please contact your homeserver administrator.",
     "Failed to join room": "Failed to join room",

src/utils/MultiInviter.js

@@ -1,6 +1,6 @@
 /*
 Copyright 2016 OpenMarket Ltd
-Copyright 2017 New Vector Ltd
+Copyright 2017, 2018 New Vector Ltd
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -17,9 +17,9 @@ limitations under the License.
 
 import MatrixClientPeg from '../MatrixClientPeg';
 import {getAddressType} from '../UserAddress';
-import {inviteToRoom} from '../RoomInvite';
 import GroupStore from '../stores/GroupStore';
 import Promise from 'bluebird';
+import {_t} from "../languageHandler";
 
 /**
  * Invites multiple addresses to a room or group, handling rate limiting from the server
@@ -49,7 +49,7 @@ export default class MultiInviter {
      * Invite users to this room. This may only be called once per
      * instance of the class.
      *
-     * @param {array} addresses Array of addresses to invite
+     * @param {array} addrs Array of addresses to invite
      * @returns {Promise} Resolved when all invitations in the queue are complete
      */
     invite(addrs) {
@@ -88,12 +88,30 @@ export default class MultiInviter {
         return this.errorTexts[addr];
     }
 
+    async _inviteToRoom(roomId, addr) {
+        const addrType = getAddressType(addr);
+
+        if (addrType === 'email') {
+            return MatrixClientPeg.get().inviteByEmail(roomId, addr);
+        } else if (addrType === 'mx-user-id') {
+            const profile = await MatrixClientPeg.get().getProfileInfo(addr);
+            if (!profile) {
+                return Promise.reject({errcode: "M_NOT_FOUND", error: "User does not have a profile."});
+            }
+
+            return MatrixClientPeg.get().invite(roomId, addr);
+        } else {
+            throw new Error('Unsupported address');
+        }
+    }
+
+
     _inviteMore(nextIndex) {
         if (this._canceled) {
             return;
         }
 
-        if (nextIndex == this.addrs.length) {
+        if (nextIndex === this.addrs.length) {
             this.busy = false;
             this.deferred.resolve(this.completionStates);
             return;
@@ -111,7 +129,7 @@ export default class MultiInviter {
 
         // don't re-invite (there's no way in the UI to do this, but
         // for sanity's sake)
-        if (this.completionStates[addr] == 'invited') {
+        if (this.completionStates[addr] === 'invited') {
             this._inviteMore(nextIndex + 1);
             return;
         }
@@ -120,7 +138,7 @@ export default class MultiInviter {
         if (this.groupId !== null) {
             doInvite = GroupStore.inviteUserToGroup(this.groupId, addr);
         } else {
-            doInvite = inviteToRoom(this.roomId, addr);
+            doInvite = this._inviteToRoom(this.roomId, addr);
         }
 
         doInvite.then(() => {
@@ -129,29 +147,34 @@ export default class MultiInviter {
             this.completionStates[addr] = 'invited';
 
             this._inviteMore(nextIndex + 1);
-        }, (err) => {
+        }).catch((err) => {
             if (this._canceled) { return; }
 
             let errorText;
             let fatal = false;
-            if (err.errcode == 'M_FORBIDDEN') {
+            if (err.errcode === 'M_FORBIDDEN') {
                 fatal = true;
-                errorText = 'You do not have permission to invite people to this room.';
+                errorText = _t('You do not have permission to invite people to this room.');
-            } else if (err.errcode == 'M_LIMIT_EXCEEDED') {
+            } else if (err.errcode === 'M_LIMIT_EXCEEDED') {
                 // we're being throttled so wait a bit & try again
                 setTimeout(() => {
                     this._inviteMore(nextIndex);
                 }, 5000);
                 return;
+            } else if(err.errcode === "M_NOT_FOUND") {
+                errorText = _t("User %(user_id)s does not exist", {user_id: addr});
             } else {
-                errorText = 'Unknown server error';
+                errorText = _t('Unknown server error');
             }
             this.completionStates[addr] = 'error';
             this.errorTexts[addr] = errorText;
             this.busy = !fatal;
+            this.fatal = fatal;
 
             if (!fatal) {
                 this._inviteMore(nextIndex + 1);
+            } else {
+                this.deferred.resolve(this.completionStates);
             }
         });
     }