From 8fa50b26a614ba4bc548aea95592edf0325a1ee3 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 14 Jun 2019 15:31:19 +0100 Subject: [PATCH 1/4] Fix welcome user https://github.com/matrix-org/matrix-react-sdk/pull/3101 meant we don't get logged straight in after registering if using an email address, but this was the point at which we made a chat with the welcome user. Instead, set a flag in memory that we should try & make a chat with the welcome user for that user ID if we get a session for them. Of course, if the user logs in on both tabs, this would mean each would make a chat with the welcome user (although actually this was a problem with the old code too). Check our m.direct to see if we've started a chat with the welcome user before making one (which also means we have to make sure the cached sync is up to date... see comments). --- src/MatrixClientPeg.js | 26 +++++++ src/components/structures/MatrixChat.js | 74 ++++++++++++++----- .../structures/auth/Registration.js | 3 + 3 files changed, 85 insertions(+), 18 deletions(-) diff --git a/src/MatrixClientPeg.js b/src/MatrixClientPeg.js index 574f05bf85..07499a3a87 100644 --- a/src/MatrixClientPeg.js +++ b/src/MatrixClientPeg.js @@ -51,6 +51,7 @@ interface MatrixClientCreds { class MatrixClientPeg { constructor() { this.matrixClient = null; + this._justRegisteredUserId = null; // These are the default options used when when the // client is started in 'start'. These can be altered @@ -85,6 +86,31 @@ class MatrixClientPeg { MatrixActionCreators.stop(); } + /* + * If we've registered a user ID we set this to the ID of the + * user we've just registered. If they then go & log in, we + * can send them to the welcome user (obviously this doesn't + * guarentee they'll get a chat with the welcome user). + * + * @param {string} uid The user ID of the user we've just registered + */ + setJustRegisteredUserId(uid) { + this._justRegisteredUserId = uid; + } + + /* + * Returns true if the current user has just been registered by this + * client as determined by setJustRegisteredUserId() + * + * @returns {bool} True if user has just been registered + */ + currentUserIsJustRegistered() { + return ( + this.matrixClient && + this.matrixClient.credentials.userId === this._justRegisteredUserId + ); + } + /** * Replace this MatrixClientPeg's client with a client instance that has * homeserver / identity server URLs and active credentials diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index 15a244b50e..aa579ea8f9 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -53,6 +53,7 @@ import { messageForSyncError } from '../../utils/ErrorUtils'; import ResizeNotifier from "../../utils/ResizeNotifier"; import { ValidatedServerConfig } from "../../utils/AutoDiscoveryUtils"; import AutoDiscoveryUtils from "../../utils/AutoDiscoveryUtils"; +import DMRoomMap from '../../utils/DMRoomMap'; // Disable warnings for now: we use deprecated bluebird functions // and need to migrate, but they spam the console with warnings. @@ -887,6 +888,7 @@ export default React.createClass({ } return; } + MatrixClientPeg.setJustRegisteredUserId(credentials.user_id); this.onRegistered(credentials); }, onDifferentServerClicked: (ev) => { @@ -1134,26 +1136,65 @@ export default React.createClass({ /** * Called when a new logged in session has started */ - _onLoggedIn: async function() { + _onLoggedIn: function() { this.setStateForNewView({ view: VIEWS.LOGGED_IN }); - if (this._is_registered) { - this._is_registered = false; + if (MatrixClientPeg.currentUserIsJustRegistered()) { + MatrixClientPeg.setJustRegisteredUserId(null); if (this.props.config.welcomeUserId && getCurrentLanguage().startsWith("en")) { - const roomId = await createRoom({ - dmUserId: this.props.config.welcomeUserId, - // Only view the welcome user if we're NOT looking at a room - andView: !this.state.currentRoomId, - }); - // if successful, return because we're already - // viewing the welcomeUserId room - // else, if failed, fall through to view_home_page - if (roomId) { - return; + // We can end up with multiple tabs post-registration where the user + // might then end up with a session and we don't want them all making + // a chat with the welcome user: try to de-dupe. + // We need to wait for the first sync to complete for this to + // work though. + let waitFor; + if (!this.firstSyncComplete) { + waitFor = this.firstSyncPromise.promise; + } else { + waitFor = Promise.resolve(); } + waitFor.then(async () => { + const welcomeUserRooms = DMRoomMap.shared().getDMRoomsForUserId( + this.props.config.welcomeUserId, + ); + if (welcomeUserRooms.length === 0) { + const roomId = await createRoom({ + dmUserId: this.props.config.welcomeUserId, + // Only view the welcome user if we're NOT looking at a room + andView: !this.state.currentRoomId, + }); + // This is a bit of a hack, but since the deduplication relies + // on m.direct being up to date, we need to force a sync + // of the database, otherwise if the user goes to the other + // tab before the next save happens (a few minutes), the + // saved sync will be restored from the db and this code will + // run without the update to m.direct, making another welcome + // user room. + const saveWelcomeUser = (ev) => { + if ( + ev.getType() == 'm.direct' && + ev.getContent() && + ev.getContent()[this.props.config.welcomeUserId] + ) { + MatrixClientPeg.get().store.save(true); + MatrixClientPeg.get().removeListener( + "accountData", saveWelcomeUser, + ); + } + }; + MatrixClientPeg.get().on("accountData", saveWelcomeUser); + + // if successful, return because we're already + // viewing the welcomeUserId room + // else, if failed, fall through to view_home_page + if (roomId) { + return; + } + } + // The user has just logged in after registering + dis.dispatch({action: 'view_home_page'}); + }); } - // The user has just logged in after registering - dis.dispatch({action: 'view_home_page'}); } else { this._showScreenAfterLogin(); } @@ -1694,9 +1735,6 @@ export default React.createClass({ return MatrixClientPeg.get(); } } - // XXX: This should be in state or ideally store(s) because we risk not - // rendering the most up-to-date view of state otherwise. - this._is_registered = true; return Lifecycle.setLoggedIn(credentials); }, diff --git a/src/components/structures/auth/Registration.js b/src/components/structures/auth/Registration.js index 3103ee41df..1957275505 100644 --- a/src/components/structures/auth/Registration.js +++ b/src/components/structures/auth/Registration.js @@ -29,6 +29,7 @@ import * as ServerType from '../../views/auth/ServerTypeSelector'; import AutoDiscoveryUtils, {ValidatedServerConfig} from "../../../utils/AutoDiscoveryUtils"; import classNames from "classnames"; import * as Lifecycle from '../../../Lifecycle'; +import MatrixClientPeg from "../../../MatrixClientPeg"; // Phases // Show controls to configure server details @@ -287,6 +288,8 @@ module.exports = React.createClass({ return; } + MatrixClientPeg.setJustRegisteredUserId(response.user_id); + const newState = { doingUIAuth: false, }; From 929e9dc6533a8513f218060fdd4440a4c4d205a6 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 14 Jun 2019 16:04:09 +0100 Subject: [PATCH 2/4] Don't forget to show the homepage if no welcome user --- src/components/structures/MatrixChat.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index aa579ea8f9..0b15b727ec 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -1191,9 +1191,14 @@ export default React.createClass({ return; } } - // The user has just logged in after registering + // We didn't rediret to the welcome user room, so show + // the homepage. dis.dispatch({action: 'view_home_page'}); }); + } else { + // The user has just logged in after registering, + // so show the homepage. + dis.dispatch({action: 'view_home_page'}); } } else { this._showScreenAfterLogin(); From 30726d6cf95be8a1b7e913102665a5fdc3bb6fd6 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 14 Jun 2019 16:29:26 +0100 Subject: [PATCH 3/4] Pull out welcome user chat code to a separate function also expand on comment --- src/components/structures/MatrixChat.js | 111 +++++++++++++----------- 1 file changed, 59 insertions(+), 52 deletions(-) diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index 0b15b727ec..4fc64b91bb 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -1133,68 +1133,75 @@ export default React.createClass({ } }, + /** + * Starts a chat with the welcome user, if the user doesn't already have one + * @returns {string} The room ID of the new room, or null if no room was created + */ + async _startWelcomeUserChat() { + // We can end up with multiple tabs post-registration where the user + // might then end up with a session and we don't want them all making + // a chat with the welcome user: try to de-dupe. + // We need to wait for the first sync to complete for this to + // work though. + let waitFor; + if (!this.firstSyncComplete) { + waitFor = this.firstSyncPromise.promise; + } else { + waitFor = Promise.resolve(); + } + await waitFor; + + const welcomeUserRooms = DMRoomMap.shared().getDMRoomsForUserId( + this.props.config.welcomeUserId, + ); + if (welcomeUserRooms.length === 0) { + const roomId = await createRoom({ + dmUserId: this.props.config.welcomeUserId, + // Only view the welcome user if we're NOT looking at a room + andView: !this.state.currentRoomId, + }); + // This is a bit of a hack, but since the deduplication relies + // on m.direct being up to date, we need to force a sync + // of the database, otherwise if the user goes to the other + // tab before the next save happens (a few minutes), the + // saved sync will be restored from the db and this code will + // run without the update to m.direct, making another welcome + // user room (it doesn't wait for new data from the server, just + // the saved sync to be loaded). + const saveWelcomeUser = (ev) => { + if ( + ev.getType() == 'm.direct' && + ev.getContent() && + ev.getContent()[this.props.config.welcomeUserId] + ) { + MatrixClientPeg.get().store.save(true); + MatrixClientPeg.get().removeListener( + "accountData", saveWelcomeUser, + ); + } + }; + MatrixClientPeg.get().on("accountData", saveWelcomeUser); + + return roomId; + } + return null; + }, + /** * Called when a new logged in session has started */ - _onLoggedIn: function() { + _onLoggedIn: async function() { this.setStateForNewView({ view: VIEWS.LOGGED_IN }); - if (MatrixClientPeg.currentUserIsJustRegistered()) { + if (true || MatrixClientPeg.currentUserIsJustRegistered()) { MatrixClientPeg.setJustRegisteredUserId(null); if (this.props.config.welcomeUserId && getCurrentLanguage().startsWith("en")) { - // We can end up with multiple tabs post-registration where the user - // might then end up with a session and we don't want them all making - // a chat with the welcome user: try to de-dupe. - // We need to wait for the first sync to complete for this to - // work though. - let waitFor; - if (!this.firstSyncComplete) { - waitFor = this.firstSyncPromise.promise; - } else { - waitFor = Promise.resolve(); - } - waitFor.then(async () => { - const welcomeUserRooms = DMRoomMap.shared().getDMRoomsForUserId( - this.props.config.welcomeUserId, - ); - if (welcomeUserRooms.length === 0) { - const roomId = await createRoom({ - dmUserId: this.props.config.welcomeUserId, - // Only view the welcome user if we're NOT looking at a room - andView: !this.state.currentRoomId, - }); - // This is a bit of a hack, but since the deduplication relies - // on m.direct being up to date, we need to force a sync - // of the database, otherwise if the user goes to the other - // tab before the next save happens (a few minutes), the - // saved sync will be restored from the db and this code will - // run without the update to m.direct, making another welcome - // user room. - const saveWelcomeUser = (ev) => { - if ( - ev.getType() == 'm.direct' && - ev.getContent() && - ev.getContent()[this.props.config.welcomeUserId] - ) { - MatrixClientPeg.get().store.save(true); - MatrixClientPeg.get().removeListener( - "accountData", saveWelcomeUser, - ); - } - }; - MatrixClientPeg.get().on("accountData", saveWelcomeUser); - - // if successful, return because we're already - // viewing the welcomeUserId room - // else, if failed, fall through to view_home_page - if (roomId) { - return; - } - } + const welcomeUserRoom = await this._startWelcomeUserChat(); + if (welcomeUserRoom === null) { // We didn't rediret to the welcome user room, so show // the homepage. dis.dispatch({action: 'view_home_page'}); - }); + } } else { // The user has just logged in after registering, // so show the homepage. From 794b04b89a0eb79c3f61626506374935729cd21d Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 14 Jun 2019 16:41:30 +0100 Subject: [PATCH 4/4] take the debugging out --- src/components/structures/MatrixChat.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/structures/MatrixChat.js b/src/components/structures/MatrixChat.js index 4fc64b91bb..789649c220 100644 --- a/src/components/structures/MatrixChat.js +++ b/src/components/structures/MatrixChat.js @@ -1192,7 +1192,7 @@ export default React.createClass({ */ _onLoggedIn: async function() { this.setStateForNewView({ view: VIEWS.LOGGED_IN }); - if (true || MatrixClientPeg.currentUserIsJustRegistered()) { + if (MatrixClientPeg.currentUserIsJustRegistered()) { MatrixClientPeg.setJustRegisteredUserId(null); if (this.props.config.welcomeUserId && getCurrentLanguage().startsWith("en")) {