Fix RoomView stuck in 'accept invite' state

After accepting a 3pid invite.

Rather than clear the joining flag when the join request completes,
leave it so the RoomView can see that we're expecting the user to
be joined in the various stages that might go through (waiting for
join request, waiting for room object, waiting for 'joined' member
event). The problem in this case was that we had to wait a bit for
the last one, and there was no bit of state to represent it.

This hopefully also makes the logic somewhat simpler.

Fixes element-hq/element-web#5041

Author: dbkr

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,14 +703,7 @@ 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();
-            }
+            this.forceUpdate();
         }
     },
 
@@ -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({
-                action: 'joined_room',
-            });
+            // We don't actually need to do anything here: we do *not*
+            // 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,25 @@ 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 consider the room to be joined if the user's
+    // member events says they are joined.
+    // ie. The correct logic is:
+    // if (room && myMember.membership == 'joined') {
+    //     // user is joined to the room
+    // } else {
+    //     if (RoomViewStore.isJoining()) {
+    //         // show spinner
+    //     } else {
+    //         // show join prompt
+    //     }
+    // }
     isJoining() {
         return this._state.joining;
     }