From f50b1c6ae97dab559731ae625eb3200dabea5804 Mon Sep 17 00:00:00 2001 From: enahum Date: Tue, 24 Oct 2017 18:03:51 -0300 Subject: [PATCH] Improve push notification flow (#1044) --- app/actions/views/channel.js | 2 +- app/actions/views/load_team.js | 14 ---- app/actions/views/root.js | 69 +++++++---------- app/constants/view.js | 3 - app/initial_state.js | 2 - app/mattermost.js | 79 ++++++++++++++------ app/reducers/views/index.js | 2 - app/reducers/views/notification.js | 13 ---- app/reducers/views/root.js | 15 ---- app/screens/channel/channel_drawer_button.js | 1 + app/screens/load_team/index.js | 10 +-- app/screens/load_team/load_team.js | 19 +---- app/screens/notification/index.js | 5 +- app/screens/notification/notification.js | 4 +- app/screens/select_team/index.js | 2 - app/screens/select_team/select_team.js | 4 +- 16 files changed, 97 insertions(+), 147 deletions(-) delete mode 100644 app/actions/views/load_team.js delete mode 100644 app/reducers/views/notification.js diff --git a/app/actions/views/channel.js b/app/actions/views/channel.js index 6b8ff2e3d..76938ec16 100644 --- a/app/actions/views/channel.js +++ b/app/actions/views/channel.js @@ -179,7 +179,7 @@ export function loadPostsIfNecessaryWithRetry(channelId) { }; } -async function retryGetPostsAction(action, dispatch, getState, maxTries = MAX_POST_TRIES) { +export async function retryGetPostsAction(action, dispatch, getState, maxTries = MAX_POST_TRIES) { for (let i = 0; i < maxTries; i++) { const {data} = await action(dispatch, getState); diff --git a/app/actions/views/load_team.js b/app/actions/views/load_team.js deleted file mode 100644 index cd22a77ac..000000000 --- a/app/actions/views/load_team.js +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved. -// See License.txt for license information. - -import {ViewTypes} from 'app/constants'; - -export function initialize() { - return async (dispatch, getState) => { - setTimeout(() => { - dispatch({ - type: ViewTypes.APPLICATION_INITIALIZED - }, getState); - }, 400); - }; -} diff --git a/app/actions/views/root.js b/app/actions/views/root.js index 9caed27b4..c165159e5 100644 --- a/app/actions/views/root.js +++ b/app/actions/views/root.js @@ -1,16 +1,16 @@ // Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved. // See License.txt for license information. -import {ViewTypes} from 'app/constants'; -import { - handleSelectChannel, - setChannelDisplayName -} from 'app/actions/views/channel'; -import {handleTeamChange, selectFirstAvailableTeam} from 'app/actions/views/select_team'; - import {General} from 'mattermost-redux/constants'; import {getClientConfig, getLicenseConfig} from 'mattermost-redux/actions/general'; -import {getChannelAndMyMember, markChannelAsRead, viewChannel} from 'mattermost-redux/actions/channels'; +import {getPosts} from 'mattermost-redux/actions/posts'; +import {getMyTeams, getMyTeamMembers, selectTeam} from 'mattermost-redux/actions/teams'; + +import { + handleSelectChannel, + setChannelDisplayName, + retryGetPostsAction +} from 'app/actions/views/channel'; export function loadConfigAndLicense() { return async (dispatch, getState) => { @@ -23,48 +23,39 @@ export function loadConfigAndLicense() { }; } -export function queueNotification(notification) { - return async (dispatch, getState) => { - dispatch({type: ViewTypes.NOTIFICATION_CHANGED, data: notification}, getState); - }; -} - -export function clearNotification() { - return async (dispatch, getState) => { - dispatch({type: ViewTypes.NOTIFICATION_CHANGED, data: null}, getState); - }; -} - -export function goToNotification(notification) { +export function loadFromPushNotification(notification) { return async (dispatch, getState) => { const state = getState(); const {data} = notification; - const {currentTeamId, teams} = state.entities.teams; - const {channels, currentChannelId, myMembers} = state.entities.channels; + const {currentTeamId, teams, myMembers: myTeamMembers} = state.entities.teams; + const {currentChannelId} = state.entities.channels; const channelId = data.channel_id; - // if the notification does not have a team id is because its from a DM or GM + // when the notification does not have a team id is because its from a DM or GM const teamId = data.team_id || currentTeamId; - dispatch(setChannelDisplayName('')); - - if (teamId && teamId !== currentTeamId) { - handleTeamChange(teams[teamId], false)(dispatch, getState); - } else if (!teamId) { - await selectFirstAvailableTeam()(dispatch, getState); + //verify that we have the team loaded + if (teamId && (!teams[teamId] || !myTeamMembers[teamId])) { + await Promise.all([ + getMyTeams()(dispatch, getState), + getMyTeamMembers()(dispatch, getState) + ]); } - if (!channels[channelId] || !myMembers[channelId]) { - getChannelAndMyMember(channelId)(dispatch, getState); + // when the notification is from a team other than the current team + if (teamId !== currentTeamId) { + selectTeam({id: teamId})(dispatch, getState); } - if (channelId !== currentChannelId) { + // when the notification is from the same channel as the current channel + // we should get the posts + if (channelId === currentChannelId) { + await retryGetPostsAction(getPosts(channelId), dispatch, getState); + } else { + // when the notification is from a channel other than the current channel + dispatch(setChannelDisplayName('')); handleSelectChannel(channelId)(dispatch, getState); } - - viewChannel(channelId)(dispatch, getState); - - markChannelAsRead(channelId, currentChannelId)(dispatch, getState); }; } @@ -74,8 +65,6 @@ export function purgeOfflineStore() { export default { loadConfigAndLicense, - queueNotification, - clearNotification, - goToNotification, + loadFromPushNotification, purgeOfflineStore }; diff --git a/app/constants/view.js b/app/constants/view.js index 7d4bfa813..0e2e8c8a6 100644 --- a/app/constants/view.js +++ b/app/constants/view.js @@ -4,8 +4,6 @@ import keyMirror from 'mattermost-redux/utils/key_mirror'; const ViewTypes = keyMirror({ - APPLICATION_INITIALIZED: null, - SERVER_URL_CHANGED: null, LOGIN_ID_CHANGED: null, @@ -15,7 +13,6 @@ const ViewTypes = keyMirror({ COMMENT_DRAFT_CHANGED: null, SEARCH_DRAFT_CHANGED: null, - NOTIFICATION_CHANGED: null, NOTIFICATION_IN_APP: null, NOTIFICATION_TAPPED: null, diff --git a/app/initial_state.js b/app/initial_state.js index d33e30f73..454e1c478 100644 --- a/app/initial_state.js +++ b/app/initial_state.js @@ -263,9 +263,7 @@ const state = { loginId: '', password: '' }, - notification: null, root: { - appInitializing: false, hydrationComplete: false, purge: false }, diff --git a/app/mattermost.js b/app/mattermost.js index 52aaba15f..61f81118c 100644 --- a/app/mattermost.js +++ b/app/mattermost.js @@ -36,9 +36,9 @@ import { setStatusBarHeight } from 'app/actions/device'; import { - goToNotification, + loadConfigAndLicense, - queueNotification, + loadFromPushNotification, purgeOfflineStore } from 'app/actions/views/root'; import {setChannelDisplayName} from 'app/actions/views/channel'; @@ -57,6 +57,7 @@ import { LOGGER_JAVASCRIPT, LOGGER_NATIVE } from 'app/utils/sentry'; +import {stripTrailingSlashes} from 'app/utils/url'; import Config from 'assets/config'; @@ -378,6 +379,10 @@ export default class Mattermost { // We need to wait for hydration to occur before load the router. listenForHydration = () => { const state = this.store.getState(); + if (!this.isConfigured) { + this.configurePushNotifications(); + } + Orientation.getOrientation((orientation) => { this.orientationDidChange(orientation); }); @@ -385,6 +390,7 @@ export default class Mattermost { if (state.views.root.hydrationComplete) { this.unsubscribeFromStore(); + const isNotActive = AppState.currentState !== 'active'; const notification = PushNotifications.getNotification(); if (notification) { // If we have a notification means that the app was started cause of a reply @@ -407,7 +413,7 @@ export default class Mattermost { this.launchApp(); } }); - } else if (AppState.currentState === 'background') { + } else if (isNotActive) { // for IOS replying from push notification starts the app in the background this.shouldRelaunchonActive = true; this.configurePushNotifications(); @@ -434,10 +440,19 @@ export default class Mattermost { this.isConfigured = true; }; - onPushNotification = (deviceNotification) => { - const {data, foreground, message, userInfo, userInteraction} = deviceNotification; + onPushNotification = async (deviceNotification) => { const {dispatch, getState} = this.store; const state = getState(); + const {token, url} = state.entities.general.credentials; + let startAppFromPushNotification = false; + + // mark the app as started as soon as possible + if (token && url && !this.appStarted) { + this.appStarted = true; + startAppFromPushNotification = true; + } + + const {data, foreground, message, userInfo, userInteraction} = deviceNotification; const notification = { data, message @@ -451,22 +466,20 @@ export default class Mattermost { markChannelAsRead(data.channel_id)(dispatch, getState); } else if (foreground) { EventEmitter.emit(ViewTypes.NOTIFICATION_IN_APP, notification); - } else if (userInteraction) { - if (!notification.localNotification) { - if (!state.views.root.appInitializing) { - // go to notification if the app is initialized + } else if (userInteraction && !notification.localNotification) { + if (startAppFromPushNotification) { + Client.setToken(token); + Client4.setToken(token); + Client4.setUrl(stripTrailingSlashes(url)); + Client.setUrl(stripTrailingSlashes(url)); + } - if (!Client4.getUrl()) { - // Make sure the Client has the server url set - Client4.setUrl(state.entities.general.credentials.url); - } + await loadFromPushNotification(notification)(dispatch, getState); - goToNotification(notification)(dispatch, getState); - EventEmitter.emit(ViewTypes.NOTIFICATION_TAPPED); - } else if (state.entities.general.credentials.token) { - // queue notification if app is not initialized but we are logged in - queueNotification(notification)(dispatch, getState); - } + if (startAppFromPushNotification) { + this.startAppFromPushNotification(); + } else { + EventEmitter.emit(ViewTypes.NOTIFICATION_TAPPED); } } }; @@ -566,13 +579,32 @@ export default class Mattermost { }; startApp = (animationType = 'none') => { - if (!this.isConfigured) { - this.configurePushNotifications(); + if (!this.appStarted) { + Navigation.startSingleScreenApp({ + screen: { + screen: 'Root', + navigatorStyle: { + navBarHidden: true, + statusBarHidden: false, + statusBarHideWithNavBar: false, + screenBackgroundColor: 'transparent' + } + }, + passProps: { + allowOtherServers: this.allowOtherServers + }, + appStyle: { + orientation: 'auto' + }, + animationType + }); } + }; + startAppFromPushNotification = () => { Navigation.startSingleScreenApp({ screen: { - screen: 'Root', + screen: 'Channel', navigatorStyle: { navBarHidden: true, statusBarHidden: false, @@ -586,7 +618,8 @@ export default class Mattermost { appStyle: { orientation: 'auto' }, - animationType + animationType: 'none' }); + this.appStarted = false; }; } diff --git a/app/reducers/views/index.js b/app/reducers/views/index.js index 38ed2978f..642fa47e3 100644 --- a/app/reducers/views/index.js +++ b/app/reducers/views/index.js @@ -7,7 +7,6 @@ import channel from './channel'; import fetchCache from './fetch_cache'; import i18n from './i18n'; import login from './login'; -import notification from './notification'; import root from './root'; import search from './search'; import selectServer from './select_server'; @@ -19,7 +18,6 @@ export default combineReducers({ fetchCache, i18n, login, - notification, root, search, selectServer, diff --git a/app/reducers/views/notification.js b/app/reducers/views/notification.js deleted file mode 100644 index d1f2e09fb..000000000 --- a/app/reducers/views/notification.js +++ /dev/null @@ -1,13 +0,0 @@ -// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved. -// See License.txt for license information. - -import {ViewTypes} from 'app/constants'; - -export default function notification(state = null, action) { - switch (action.type) { - case ViewTypes.NOTIFICATION_CHANGED: - return action.data; - default: - return state; - } -} diff --git a/app/reducers/views/root.js b/app/reducers/views/root.js index bd24bdba7..562f3c049 100644 --- a/app/reducers/views/root.js +++ b/app/reducers/views/root.js @@ -4,20 +4,6 @@ import {combineReducers} from 'redux'; import {General} from 'mattermost-redux/constants'; -import {UserTypes} from 'mattermost-redux/action_types'; - -import {ViewTypes} from 'app/constants'; - -function appInitializing(state = true, action) { - switch (action.type) { - case ViewTypes.APPLICATION_INITIALIZED: - return false; - case UserTypes.RESET_LOGOUT_STATE: - return true; - default: - return state; - } -} function hydrationComplete(state = false, action) { switch (action.type) { @@ -38,7 +24,6 @@ function purge(state = false, action) { } export default combineReducers({ - appInitializing, hydrationComplete, purge }); diff --git a/app/screens/channel/channel_drawer_button.js b/app/screens/channel/channel_drawer_button.js index 2252e5a1f..701c1558c 100644 --- a/app/screens/channel/channel_drawer_button.js +++ b/app/screens/channel/channel_drawer_button.js @@ -59,6 +59,7 @@ class ChannelDrawerButton extends PureComponent { componentDidMount() { EventEmitter.on('drawer_opacity', this.setOpacity); + PushNotifications.setApplicationIconBadgeNumber(this.props.mentionCount); } componentWillReceiveProps(nextProps) { diff --git a/app/screens/load_team/index.js b/app/screens/load_team/index.js index a23abbd4b..d34c0aeef 100644 --- a/app/screens/load_team/index.js +++ b/app/screens/load_team/index.js @@ -7,16 +7,13 @@ import {connect} from 'react-redux'; import {getTeams} from 'mattermost-redux/actions/teams'; import {getCurrentTeam} from 'mattermost-redux/selectors/entities/teams'; -import {initialize} from 'app/actions/views/load_team'; -import {clearNotification, goToNotification} from 'app/actions/views/root'; import {handleTeamChange} from 'app/actions/views/select_team'; import {getTheme} from 'mattermost-redux/selectors/entities/preferences'; import LoadTeam from './load_team'; -function mapStateToProps(state, ownProps) { +function mapStateToProps(state) { return { - ...ownProps, config: state.entities.general.config, theme: getTheme(state), teams: state.entities.teams.teams, @@ -29,11 +26,8 @@ function mapStateToProps(state, ownProps) { function mapDispatchToProps(dispatch) { return { actions: bindActionCreators({ - clearNotification, getTeams, - goToNotification, - handleTeamChange, - initialize + handleTeamChange }, dispatch) }; } diff --git a/app/screens/load_team/load_team.js b/app/screens/load_team/load_team.js index 2489e2085..dcd2706c0 100644 --- a/app/screens/load_team/load_team.js +++ b/app/screens/load_team/load_team.js @@ -11,30 +11,18 @@ import {NavigationTypes} from 'app/constants'; export default class LoadTeam extends PureComponent { static propTypes = { actions: PropTypes.shape({ - clearNotification: PropTypes.func.isRequired, getTeams: PropTypes.func.isRequired, - goToNotification: PropTypes.func.isRequired, - handleTeamChange: PropTypes.func.isRequired, - initialize: PropTypes.func.isRequired + handleTeamChange: PropTypes.func.isRequired }).isRequired, currentTeam: PropTypes.object, myMembers: PropTypes.object.isRequired, navigator: PropTypes.object, - notification: PropTypes.object, teams: PropTypes.object.isRequired, theme: PropTypes.object.isRequired }; componentDidMount() { - const {notification, currentTeam, myMembers, teams} = this.props; - const {clearNotification, goToNotification} = this.props.actions; - - if (notification) { - clearNotification(); - goToNotification(notification); - return this.goToChannelView(); - } - + const {currentTeam, myMembers, teams} = this.props; if (currentTeam && myMembers[currentTeam.id]) { return this.onSelectTeam(currentTeam); } @@ -62,9 +50,8 @@ export default class LoadTeam extends PureComponent { } goToChannelView = () => { - const {actions, navigator, theme} = this.props; + const {navigator, theme} = this.props; - actions.initialize(); navigator.resetTo({ screen: 'Channel', animated: true, diff --git a/app/screens/notification/index.js b/app/screens/notification/index.js index d2923ae5a..3818c1f67 100644 --- a/app/screens/notification/index.js +++ b/app/screens/notification/index.js @@ -4,7 +4,7 @@ import {bindActionCreators} from 'redux'; import {connect} from 'react-redux'; -import {goToNotification} from 'app/actions/views/root'; +import {loadFromPushNotification} from 'app/actions/views/root'; import {getDimensions} from 'app/selectors/device'; import {getChannel} from 'mattermost-redux/selectors/entities/channels'; @@ -28,7 +28,6 @@ function mapStateToProps(state, ownProps) { } return { - ...ownProps, config: state.entities.general.config, channel, deviceWidth, @@ -41,7 +40,7 @@ function mapStateToProps(state, ownProps) { function mapDispatchToProps(dispatch) { return { actions: bindActionCreators({ - goToNotification + loadFromPushNotification }, dispatch) }; } diff --git a/app/screens/notification/notification.js b/app/screens/notification/notification.js index 88ba223c7..161f16233 100644 --- a/app/screens/notification/notification.js +++ b/app/screens/notification/notification.js @@ -27,7 +27,7 @@ const IMAGE_SIZE = 33; export default class Notification extends PureComponent { static propTypes = { actions: PropTypes.shape({ - goToNotification: PropTypes.func.isRequired + loadFromPushNotification: PropTypes.func.isRequired }).isRequired, channel: PropTypes.object, config: PropTypes.object, @@ -44,7 +44,7 @@ export default class Notification extends PureComponent { navigator.dismissInAppNotification(); if (!notification.localNotification) { - actions.goToNotification(notification); + actions.loadFromPushNotification(notification); navigator.resetTo({ screen: 'Channel', animated: false, diff --git a/app/screens/select_team/index.js b/app/screens/select_team/index.js index 630b6d18a..b8955724d 100644 --- a/app/screens/select_team/index.js +++ b/app/screens/select_team/index.js @@ -4,7 +4,6 @@ import {bindActionCreators} from 'redux'; import {connect} from 'react-redux'; -import {initialize} from 'app/actions/views/load_team'; import {handleTeamChange} from 'app/actions/views/select_team'; import {markChannelAsRead} from 'mattermost-redux/actions/channels'; @@ -36,7 +35,6 @@ function mapDispatchToProps(dispatch) { return { actions: bindActionCreators({ handleTeamChange, - initialize, joinTeam, logout, markChannelAsRead diff --git a/app/screens/select_team/select_team.js b/app/screens/select_team/select_team.js index 891369a61..53fbce797 100644 --- a/app/screens/select_team/select_team.js +++ b/app/screens/select_team/select_team.js @@ -27,7 +27,6 @@ export default class SelectTeam extends PureComponent { static propTypes = { actions: PropTypes.shape({ handleTeamChange: PropTypes.func.isRequired, - initialize: PropTypes.func.isRequired, joinTeam: PropTypes.func.isRequired, logout: PropTypes.func.isRequired, markChannelAsRead: PropTypes.func.isRequired @@ -85,9 +84,8 @@ export default class SelectTeam extends PureComponent { }; goToChannelView = () => { - const {actions, navigator, theme} = this.props; + const {navigator, theme} = this.props; - actions.initialize(); navigator.resetTo({ screen: 'Channel', animated: false,