Merge pull request #1396 from matrix-org/dbkr/3pid_invite_update_when_joined

Fix RoomView stuck in 'accept invite' state

src/components/structures/RoomView.js

@@ -123,6 +123,9 @@ module.exports = React.createClass({
             // store the error here.
             roomLoadError: null,
 
+            // Have we sent a request to join the room that we're waiting to complete?
+            joining: false,
+
             // this is true if we are fully scrolled-down, and are looking at
             // the end of the live timeline. It has the effect of hiding the
             // 'scroll to bottom' knob, among a couple of other things.
@@ -185,10 +188,6 @@ module.exports = React.createClass({
             shouldPeek: RoomViewStore.shouldPeek(),
         };
 
-        // finished joining, start waiting for a room and show a spinner. See onRoom.
-        newState.waitingForRoom = this.state.joining && !newState.joining &&
-                        !RoomViewStore.getJoinError();
-
         // Temporary logging to diagnose https://github.com/vector-im/riot-web/issues/4307
         console.log(
             'RVS update:',
@@ -197,7 +196,6 @@ module.exports = React.createClass({
             'loading?', newState.roomLoading,
             'joining?', newState.joining,
             'initial?', initial,
-            'waiting?', newState.waitingForRoom,
             'shouldPeek?', newState.shouldPeek,
         );
 
@@ -650,7 +648,6 @@ module.exports = React.createClass({
         }
         this.setState({
             room: room,
-            waitingForRoom: false,
         }, () => {
             this._onRoomLoaded(room);
         });
@@ -706,15 +703,8 @@ module.exports = React.createClass({
 
     onRoomMemberMembership: function(ev, member, oldMembership) {
         if (member.userId == MatrixClientPeg.get().credentials.userId) {
-
-            if (member.membership === 'join') {
-                this.setState({
-                    waitingForRoom: false,
-                });
-            } else {
             this.forceUpdate();
         }
-        }
     },
 
     // rate limited because a power level change will emit an event for every
@@ -1463,10 +1453,6 @@ module.exports = React.createClass({
         const Loader = sdk.getComponent("elements.Spinner");
         const TimelinePanel = sdk.getComponent("structures.TimelinePanel");
 
-        // Whether the preview bar spinner should be shown. We do this when joining or
-        // when waiting for a room to be returned by js-sdk when joining
-        const previewBarSpinner = this.state.joining || this.state.waitingForRoom;
-
         if (!this.state.room) {
             if (this.state.roomLoading || this.state.peekLoading) {
                 return (
@@ -1500,7 +1486,7 @@ module.exports = React.createClass({
                                             onRejectClick={ this.onRejectThreepidInviteButtonClicked }
                                             canPreview={ false } error={ this.state.roomLoadError }
                                             roomAlias={roomAlias}
-                                            spinner={previewBarSpinner}
+                                            spinner={this.state.joining}
                                             inviterName={inviterName}
                                             invitedEmail={invitedEmail}
                                             room={this.state.room}
@@ -1543,7 +1529,7 @@ module.exports = React.createClass({
                                             onRejectClick={ this.onRejectButtonClicked }
                                             inviterName={ inviterName }
                                             canPreview={ false }
-                                            spinner={previewBarSpinner}
+                                            spinner={this.state.joining}
                                             room={this.state.room}
                             />
                         </div>
@@ -1618,7 +1604,7 @@ module.exports = React.createClass({
                 <RoomPreviewBar onJoinClick={this.onJoinButtonClicked}
                                 onForgetClick={ this.onForgetClick }
                                 onRejectClick={this.onRejectThreepidInviteButtonClicked}
-                                spinner={previewBarSpinner}
+                                spinner={this.state.joining}
                                 inviterName={inviterName}
                                 invitedEmail={invitedEmail}
                                 canPreview={this.state.canPeek}

src/stores/RoomViewStore.js

@@ -21,7 +21,7 @@ import Modal from '../Modal';
 import { _t } from '../languageHandler';
 
 const INITIAL_STATE = {
-    // Whether we're joining the currently viewed room
+    // Whether we're joining the currently viewed room (see isJoining())
     joining: false,
     // Any error that has occurred during joining
     joinError: null,
@@ -90,9 +90,6 @@ class RoomViewStore extends Store {
             case 'join_room':
                 this._joinRoom(payload);
                 break;
-            case 'joined_room':
-                this._joinedRoom(payload);
-                break;
             case 'join_room_error':
                 this._joinRoomError(payload);
                 break;
@@ -185,9 +182,11 @@ class RoomViewStore extends Store {
         MatrixClientPeg.get().joinRoom(
             this._state.roomAlias || this._state.roomId, payload.opts,
         ).done(() => {
-            dis.dispatch({
+            // We don't actually need to do anything here: we do *not*
-                action: 'joined_room',
+            // clear the 'joining' flag because the Room object and/or
-            });
+            // our 'joined' member event may not have come down the sync
+            // stream yet, and that's the point at which we'd consider
+            // the user joined to the room.
         }, (err) => {
             dis.dispatch({
                 action: 'join_room_error',
@@ -202,12 +201,6 @@ class RoomViewStore extends Store {
         });
     }
 
-    _joinedRoom(payload) {
-        this._setState({
-            joining: false,
-        });
-    }
-
     _joinRoomError(payload) {
         this._setState({
             joining: false,
@@ -249,7 +242,29 @@ class RoomViewStore extends Store {
         return this._state.roomLoadError;
     }
 
-    // Whether we're joining the currently viewed room
+    // True if we're expecting the user to be joined to the room currently being
+    // viewed. Note that this is left true after the join request has finished,
+    // since we should still consider a join to be in progress until the room
+    // & member events come down the sync.
+    //
+    // This flag remains true after the room has been sucessfully joined,
+    // (this store doesn't listen for the appropriate member events)
+    // so you should always observe the joined state from the member event
+    // if a room object is present.
+    // ie. The correct logic is:
+    // if (room) {
+    //     if (myMember.membership == 'joined') {
+    //         // user is joined to the room
+    //     } else {
+    //         // Not joined
+    //     }
+    // } else {
+    //     if (RoomViewStore.isJoining()) {
+    //         // show spinner
+    //     } else {
+    //         // show join prompt
+    //     }
+    // }
     isJoining() {
         return this._state.joining;
     }