From 16c4c14a16acf40650f61f338e241c4e6c67fbdf Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Thu, 1 Jun 2017 18:01:30 +0100 Subject: [PATCH 1/2] Fix to show the correct room --- src/components/structures/RoomView.js | 9 +++- src/stores/RoomViewStore.js | 65 ++++++++++++++++++--------- 2 files changed, 51 insertions(+), 23 deletions(-) diff --git a/src/components/structures/RoomView.js b/src/components/structures/RoomView.js index 2189fa3856..9a930d3d06 100644 --- a/src/components/structures/RoomView.js +++ b/src/components/structures/RoomView.js @@ -171,7 +171,7 @@ module.exports = React.createClass({ }); // Start listening for RoomViewStore updates - RoomViewStore.addListener(this._onRoomViewStoreUpdate); + this._roomStoreToken = RoomViewStore.addListener(this._onRoomViewStoreUpdate); this._onRoomViewStoreUpdate(true); }, @@ -182,6 +182,8 @@ module.exports = React.createClass({ this.setState({ roomId: RoomViewStore.getRoomId(), roomAlias: RoomViewStore.getRoomAlias(), + roomLoading: RoomViewStore.isRoomLoading(), + roomLoadError: RoomViewStore.getRoomLoadError(), joining: RoomViewStore.isJoining(), joinError: RoomViewStore.getJoinError(), }, () => { @@ -343,6 +345,11 @@ module.exports = React.createClass({ document.removeEventListener("keydown", this.onKeyDown); + // Remove RoomStore listener + if (this._roomStoreToken) { + this._roomStoreToken.remove(); + } + // cancel any pending calls to the rate_limited_funcs this._updateRoomMembers.cancelPendingCall(); diff --git a/src/stores/RoomViewStore.js b/src/stores/RoomViewStore.js index d893318af7..a74652a39d 100644 --- a/src/stores/RoomViewStore.js +++ b/src/stores/RoomViewStore.js @@ -58,6 +58,9 @@ class RoomViewStore extends Store { case 'view_room': this._viewRoom(payload); break; + case 'view_room_error': + this._viewRoomError(payload); + break; case 'will_join': this._setState({ joining: true, @@ -80,31 +83,45 @@ class RoomViewStore extends Store { } _viewRoom(payload) { - const address = payload.room_alias || payload.room_id; - if (address[0] == '#') { + // Always set the room ID if present + if (payload.room_id) { this._setState({ - roomLoading: true, - }); - MatrixClientPeg.get().getRoomIdForAlias(address).then( - (result) => { - this._setState({ - roomId: result.room_id, - roomAlias: address, - roomLoading: false, - roomLoadError: null, - }); - }, (err) => { - console.error(err); - this._setState({ - roomLoading: false, - roomLoadError: err, - }); - }); - } else { - this._setState({ - roomId: address, + roomId: payload.room_id, }); } + + if (payload.room_alias && !payload.room_id) { + this._setState({ + roomId: null, + roomAlias: payload.room_alias, + roomLoading: true, + roomLoadError: null, + }); + MatrixClientPeg.get().getRoomIdForAlias(payload.room_alias).done( + (result) => { + dis.dispatch({ + action: 'view_room', + room_id: result.room_id, + room_alias: payload.room_alias, + }); + }, (err) => { + dis.dispatch({ + action: 'view_room_error', + room_id: null, + room_alias: payload.room_alias, + err: err, + }); + }); + } + } + + _viewRoomError(payload) { + this._setState({ + roomId: payload.room_id, + roomAlias: payload.room_alias, + roomLoading: false, + roomLoadError: payload.err, + }); } _joinRoom(payload) { @@ -140,6 +157,10 @@ class RoomViewStore extends Store { return this._state.roomLoading; } + getRoomLoadError() { + return this._state.roomLoadError; + } + isJoining() { return this._state.joining; } From 7808994b719ef4cce19c9bcca3aba52315c1fc9a Mon Sep 17 00:00:00 2001 From: Luke Barnard Date: Fri, 2 Jun 2017 09:22:48 +0100 Subject: [PATCH 2/2] Modify RVS test to wait until room loaded This allows for the alias resolution to occur before a join is attempted. In theory, join_room could in future do an optional view_room-esque thing before attemping a join which would be less fragile than dispatching things in the right order. Also, make sure the store indicates that it is not loading when a room ID has been used - no alias resolution need take place. --- src/stores/RoomViewStore.js | 6 +++--- test/stores/RoomViewStore-test.js | 15 +++++++++------ 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/stores/RoomViewStore.js b/src/stores/RoomViewStore.js index a74652a39d..b94e772b02 100644 --- a/src/stores/RoomViewStore.js +++ b/src/stores/RoomViewStore.js @@ -87,10 +87,10 @@ class RoomViewStore extends Store { if (payload.room_id) { this._setState({ roomId: payload.room_id, + roomLoading: false, + roomLoadError: null, }); - } - - if (payload.room_alias && !payload.room_id) { + } else if (payload.room_alias) { this._setState({ roomId: null, roomAlias: payload.room_alias, diff --git a/test/stores/RoomViewStore-test.js b/test/stores/RoomViewStore-test.js index 7100dced19..2f545ffd74 100644 --- a/test/stores/RoomViewStore-test.js +++ b/test/stores/RoomViewStore-test.js @@ -45,12 +45,15 @@ describe('RoomViewStore', function() { done(); }; - dispatch({ action: 'view_room', room_alias: '#somealias2:aser.ver' }); + RoomViewStore.addListener(() => { + // Wait until the room alias has resolved and the room ID is + if (!RoomViewStore.isRoomLoading()) { + expect(RoomViewStore.getRoomId()).toBe("!randomcharacters:aser.ver"); + dispatch({ action: 'join_room' }); + expect(RoomViewStore.isJoining()).toBe(true); + } + }); - // Wait for the next event loop to allow for room alias resolution - setTimeout(() => { - dispatch({ action: 'join_room' }); - expect(RoomViewStore.isJoining()).toBe(true); - }, 0); + dispatch({ action: 'view_room', room_alias: '#somealias2:aser.ver' }); }); });