From 7f0abe17131bfeff0e9dd9293837da3a7886a98a Mon Sep 17 00:00:00 2001 From: Diego Mello Date: Wed, 13 Jan 2021 11:16:00 -0300 Subject: [PATCH] [FIX] Rooms list not being updated on some cases (#2765) * Request subscriptions on RoomsListView.constructor * Removes opened rooms from last message persisting * Change server reducer * Prevent undefined ids causing query error --- app/actions/server.js | 5 ++- app/reducers/server.js | 12 +++-- app/sagas/rooms.js | 8 +++- app/views/RoomsListView/ServerDropdown.js | 2 +- app/views/RoomsListView/index.js | 54 +++++++++++------------ 5 files changed, 45 insertions(+), 36 deletions(-) diff --git a/app/actions/server.js b/app/actions/server.js index a81e20f4d..fea239450 100644 --- a/app/actions/server.js +++ b/app/actions/server.js @@ -1,11 +1,12 @@ import { SERVER } from './actionsTypes'; -export function selectServerRequest(server, version, fetchVersion = true) { +export function selectServerRequest(server, version, fetchVersion = true, changeServer = false) { return { type: SERVER.SELECT_REQUEST, server, version, - fetchVersion + fetchVersion, + changeServer }; } diff --git a/app/reducers/server.js b/app/reducers/server.js index 96b8d400c..2c5b98199 100644 --- a/app/reducers/server.js +++ b/app/reducers/server.js @@ -8,7 +8,8 @@ const initialState = { version: null, loading: true, adding: false, - previousServer: null + previousServer: null, + changingServer: false }; @@ -34,7 +35,8 @@ export default function server(state = initialState, action) { version: action.version, connecting: true, connected: false, - loading: true + loading: true, + changingServer: action.changeServer }; case SERVER.SELECT_SUCCESS: return { @@ -43,14 +45,16 @@ export default function server(state = initialState, action) { version: action.version, connecting: false, connected: true, - loading: false + loading: false, + changingServer: false }; case SERVER.SELECT_FAILURE: return { ...state, connecting: false, connected: false, - loading: false + loading: false, + changingServer: false }; case SERVER.INIT_ADD: return { diff --git a/app/sagas/rooms.js b/app/sagas/rooms.js index 3f240f729..94670ea1f 100644 --- a/app/sagas/rooms.js +++ b/app/sagas/rooms.js @@ -61,10 +61,16 @@ const handleRoomsRequest = function* handleRoomsRequest({ params }) { const subsToCreate = subscriptions.filter(i1 => !existingSubs.find(i2 => i1._id === i2._id)); const subsToDelete = existingSubs.filter(i1 => !subscriptions.find(i2 => i1._id === i2._id)); + const openedRooms = yield select(state => state.room.rooms); const lastMessages = subscriptions + /** Checks for opened rooms and filter them out. + * It prevents this process to try persisting the same last message on the room messages fetch. + * This race condition is easy to reproduce on push notification tap. + */ + .filter(sub => !openedRooms.includes(sub.rid)) .map(sub => sub.lastMessage && buildMessage(sub.lastMessage)) .filter(lm => lm); - const lastMessagesIds = lastMessages.map(lm => lm._id); + const lastMessagesIds = lastMessages.map(lm => lm._id).filter(lm => lm); const existingMessages = yield messagesCollection.query(Q.where('id', Q.oneOf(lastMessagesIds))).fetch(); const messagesToUpdate = existingMessages.filter(i1 => lastMessages.find(i2 => i1.id === i2._id)); const messagesToCreate = lastMessages.filter(i1 => !existingMessages.find(i2 => i1._id === i2.id)); diff --git a/app/views/RoomsListView/ServerDropdown.js b/app/views/RoomsListView/ServerDropdown.js index e8344d68c..66ac731cb 100644 --- a/app/views/RoomsListView/ServerDropdown.js +++ b/app/views/RoomsListView/ServerDropdown.js @@ -313,7 +313,7 @@ const mapStateToProps = state => ({ const mapDispatchToProps = dispatch => ({ toggleServerDropdown: () => dispatch(toggleServerDropdownAction()), - selectServerRequest: server => dispatch(selectServerRequestAction(server)), + selectServerRequest: server => dispatch(selectServerRequestAction(server, null, null, true)), appStart: params => dispatch(appStartAction(params)), initAdd: previousServer => dispatch(serverInitAddAction(previousServer)) }); diff --git a/app/views/RoomsListView/index.js b/app/views/RoomsListView/index.js index ed4226872..8d39995df 100644 --- a/app/views/RoomsListView/index.js +++ b/app/views/RoomsListView/index.js @@ -117,6 +117,7 @@ class RoomsListView extends React.Component { }), server: PropTypes.string, searchText: PropTypes.string, + changingServer: PropTypes.bool, loadingServer: PropTypes.bool, showServerDropdown: PropTypes.bool, showSortDropdown: PropTypes.bool, @@ -150,8 +151,8 @@ class RoomsListView extends React.Component { console.time(`${ this.constructor.name } init`); console.time(`${ this.constructor.name } mount`); - this.gotSubscriptions = false; this.animated = false; + this.mounted = false; this.count = 0; this.state = { searching: false, @@ -162,24 +163,14 @@ class RoomsListView extends React.Component { item: {} }; this.setHeader(); + this.getSubscriptions(); } componentDidMount() { const { - navigation, closeServerDropdown, appState + navigation, closeServerDropdown } = this.props; - - /** - * - When didMount is triggered and appState is foreground, - * it means the user is logging in and selectServer has ran, so we can getSubscriptions - * - * - When didMount is triggered and appState is background, - * it means the user has resumed the app, so selectServer needs to be triggered, - * which is going to change server and getSubscriptions will be triggered by componentWillReceiveProps - */ - if (appState === 'foreground') { - this.getSubscriptions(); - } + this.mounted = true; if (isTablet) { EventEmitter.addEventListener(KEY_COMMAND, this.handleCommands); @@ -206,17 +197,17 @@ class RoomsListView extends React.Component { } UNSAFE_componentWillReceiveProps(nextProps) { - const { loadingServer, searchText, server } = this.props; + const { + loadingServer, searchText, server, changingServer + } = this.props; - if (nextProps.server && loadingServer !== nextProps.loadingServer) { - if (nextProps.loadingServer) { - this.setState({ loading: true }); - } else { - this.getSubscriptions(); - } + // when the server is changed + if (server !== nextProps.server && loadingServer !== nextProps.loadingServer && nextProps.loadingServer) { + this.setState({ loading: true }); } - if (server && server !== nextProps.server) { - this.gotSubscriptions = false; + // when the server is changing and stopped loading + if (changingServer && loadingServer !== nextProps.loadingServer && !nextProps.loadingServer) { + this.getSubscriptions(); } if (searchText !== nextProps.searchText) { this.search(nextProps.searchText); @@ -508,11 +499,17 @@ class RoomsListView extends React.Component { tempChats = chats; } - this.internalSetState({ - chats: tempChats, - chatsUpdate, - loading: false - }); + if (this.mounted) { + this.internalSetState({ + chats: tempChats, + chatsUpdate, + loading: false + }); + } else { + this.state.chats = tempChats; + this.state.chatsUpdate = chatsUpdate; + this.state.loading = false; + } }); } @@ -1023,6 +1020,7 @@ const mapStateToProps = state => ({ user: getUserSelector(state), isMasterDetail: state.app.isMasterDetail, server: state.server.server, + changingServer: state.server.changingServer, connected: state.server.connected, searchText: state.rooms.searchText, loadingServer: state.server.loading,