From 7c41c831964601d67a368b911736d65712cc8341 Mon Sep 17 00:00:00 2001 From: Miguel Alatzar Date: Thu, 16 Jul 2020 08:25:40 -0700 Subject: [PATCH] [MM-26855] Ensure More Messages button works as expected when app is launched from a push notification (#4569) * Fix more messages button with push notif * Don't cancel in showMoreText --- app/actions/views/root.js | 6 +- .../post_list/more_messages_button/index.js | 8 +- .../more_messages_button.js | 29 +++++-- .../more_messages_button.test.js | 82 ++++++++++++++----- app/screens/channel/channel_base.js | 2 - app/screens/channel/channel_base.test.js | 1 - app/screens/channel/index.js | 3 +- 7 files changed, 91 insertions(+), 40 deletions(-) diff --git a/app/actions/views/root.js b/app/actions/views/root.js index c2141a5ff..4f586d0bd 100644 --- a/app/actions/views/root.js +++ b/app/actions/views/root.js @@ -16,7 +16,7 @@ import EventEmitter from '@mm-redux/utils/event_emitter'; import initialState from '@store/initial_state'; import {getStateForReset} from '@store/utils'; -import {markChannelViewedAndRead} from './channel'; +import {markAsViewedAndReadBatch} from './channel'; export function startDataCleanup() { return async (dispatch, getState) => { @@ -107,7 +107,7 @@ export function handleSelectTeamAndChannel(teamId, channelId) { const {currentTeamId} = state.entities.teams; const channel = channels[channelId]; const member = myMembers[channelId]; - const actions = []; + const actions = markAsViewedAndReadBatch(state, channelId); // when the notification is from a team other than the current team if (teamId !== currentTeamId) { @@ -124,8 +124,6 @@ export function handleSelectTeamAndChannel(teamId, channelId) { teamId: channel.team_id || currentTeamId, }, }); - - dispatch(markChannelViewedAndRead(channelId)); } if (actions.length) { diff --git a/app/components/post_list/more_messages_button/index.js b/app/components/post_list/more_messages_button/index.js index d07288df0..f0e89a91f 100644 --- a/app/components/post_list/more_messages_button/index.js +++ b/app/components/post_list/more_messages_button/index.js @@ -3,6 +3,8 @@ import {connect} from 'react-redux'; +import {resetUnreadMessageCount} from '@actions/views/channel'; + import MoreMessagesButton from './more_messages_button'; function mapStateToProps(state, ownProps) { @@ -18,4 +20,8 @@ function mapStateToProps(state, ownProps) { }; } -export default connect(mapStateToProps)(MoreMessagesButton); +const mapDispatchToProps = { + resetUnreadMessageCount, +}; + +export default connect(mapStateToProps, mapDispatchToProps)(MoreMessagesButton); diff --git a/app/components/post_list/more_messages_button/more_messages_button.js b/app/components/post_list/more_messages_button/more_messages_button.js index dbf36b341..f0e891aac 100644 --- a/app/components/post_list/more_messages_button/more_messages_button.js +++ b/app/components/post_list/more_messages_button/more_messages_button.js @@ -2,7 +2,7 @@ // See LICENSE.txt for license information. import React from 'react'; -import {ActivityIndicator, Animated, Text, View} from 'react-native'; +import {ActivityIndicator, Animated, AppState, Text, View} from 'react-native'; import {intlShape} from 'react-intl'; import PropTypes from 'prop-types'; @@ -51,6 +51,7 @@ export default class MoreMessageButton extends React.PureComponent { scrollToIndex: PropTypes.func.isRequired, registerViewableItemsListener: PropTypes.func.isRequired, registerScrollEndIndexListener: PropTypes.func.isRequired, + resetUnreadMessageCount: PropTypes.func.isRequired, deepLinkURL: PropTypes.string, }; @@ -68,12 +69,14 @@ export default class MoreMessageButton extends React.PureComponent { } componentDidMount() { + AppState.addEventListener('change', this.onAppStateChange); EventEmitter.on(ViewTypes.INDICATOR_BAR_VISIBLE, this.onIndicatorBarVisible); this.removeViewableItemsListener = this.props.registerViewableItemsListener(this.onViewableItemsChanged); this.removeScrollEndIndexListener = this.props.registerScrollEndIndexListener(this.onScrollEndIndex); } componentWillUnmount() { + AppState.removeEventListener('change', this.onAppStateChange); EventEmitter.off(ViewTypes.INDICATOR_BAR_VISIBLE, this.onIndicatorBarVisible); if (this.removeViewableItemsListener) { this.removeViewableItemsListener(); @@ -114,6 +117,17 @@ export default class MoreMessageButton extends React.PureComponent { if (unreadCount > prevProps.unreadCount && prevProps.unreadCount === 0) { this.onViewableItemsChanged(this.viewableItems); } + + if (unreadCount === 0 && prevProps.unreadCount > 0) { + this.hide(); + } + } + + onAppStateChange = (appState) => { + const isActive = appState === 'active'; + if (!isActive) { + this.props.resetUnreadMessageCount(this.props.channelId); + } } onIndicatorBarVisible = (indicatorVisible) => { @@ -201,9 +215,9 @@ export default class MoreMessageButton extends React.PureComponent { onViewableItemsChanged = (viewableItems) => { this.viewableItems = viewableItems; + const {newMessageLineIndex, scrollToIndex} = this.props; - const {newMessageLineIndex, scrollToIndex, unreadCount} = this.props; - if (newMessageLineIndex <= 0 || unreadCount === 0 || viewableItems.length === 0 || this.disableViewableItems) { + if (newMessageLineIndex <= 0 || viewableItems.length === 0 || this.disableViewableItems) { return; } @@ -248,13 +262,10 @@ export default class MoreMessageButton extends React.PureComponent { showMoreText = (readCount) => { const moreCount = this.props.unreadCount - readCount; - if (moreCount <= 0) { - this.cancel(true); - return; + if (moreCount > 0) { + const moreText = this.moreText(moreCount); + this.setState({moreText}, this.show); } - - const moreText = this.moreText(moreCount); - this.setState({moreText}, this.show); } getReadCount = (lastViewableIndex) => { diff --git a/app/components/post_list/more_messages_button/more_messages_button.test.js b/app/components/post_list/more_messages_button/more_messages_button.test.js index 04618ddc2..8abef40d0 100644 --- a/app/components/post_list/more_messages_button/more_messages_button.test.js +++ b/app/components/post_list/more_messages_button/more_messages_button.test.js @@ -2,7 +2,7 @@ // See LICENSE.txt for license information. import React from 'react'; -import {Animated} from 'react-native'; +import {Animated, AppState} from 'react-native'; import {shallowWithIntl} from 'test/intl-test-helper'; import Preferences from '@mm-redux/constants/preferences'; @@ -28,6 +28,7 @@ describe('MoreMessagesButton', () => { manuallyUnread: false, newMessageLineIndex: 0, scrollToIndex: jest.fn(), + resetUnreadMessageCount: jest.fn(), registerViewableItemsListener: jest.fn(() => { return jest.fn(); }), @@ -54,12 +55,16 @@ describe('MoreMessagesButton', () => { }); describe('lifecycle methods', () => { - test('componentDidMount should register indicator visibility listener, viewable items listener, and scroll end index listener', () => { + AppState.addEventListener = jest.fn(); + AppState.removeEventListener = jest.fn(); + + test('componentDidMount should register app state listener, indicator visibility listener, viewable items listener, and scroll end index listener', () => { EventEmitter.on = jest.fn(); const wrapper = shallowWithIntl( , ); const instance = wrapper.instance(); + instance.onAppStateChange = jest.fn(); instance.onIndicatorBarVisible = jest.fn(); instance.onViewableItemsChanged = jest.fn(); instance.onScrollEndIndex = jest.fn(); @@ -69,6 +74,7 @@ describe('MoreMessagesButton', () => { // have not yet been mocked so we call componentDidMount again. instance.componentDidMount(); + expect(AppState.addEventListener).toHaveBeenCalledWith('change', instance.onAppStateChange); expect(EventEmitter.on).toHaveBeenCalledWith(ViewTypes.INDICATOR_BAR_VISIBLE, instance.onIndicatorBarVisible); expect(baseProps.registerViewableItemsListener).toHaveBeenCalledWith(instance.onViewableItemsChanged); expect(instance.removeViewableItemsListener).toBeDefined(); @@ -76,18 +82,20 @@ describe('MoreMessagesButton', () => { expect(instance.removeScrollEndIndexListener).toBeDefined(); }); - test('componentWillUnmount should remove the indicator bar visible listener, the viewable items listener, the scroll end index listener, and clear all timers', () => { + test('componentWillUnmount should remove the app state listener, the indicator bar visible listener, the viewable items listener, the scroll end index listener, and clear all timers', () => { jest.useFakeTimers(); EventEmitter.off = jest.fn(); const wrapper = shallowWithIntl( , ); const instance = wrapper.instance(); + instance.onAppStateChange = jest.fn(); instance.onIndicatorBarVisible = jest.fn(); instance.removeViewableItemsListener = jest.fn(); instance.removeScrollEndIndexListener = jest.fn(); instance.componentWillUnmount(); + expect(AppState.removeEventListener).toHaveBeenCalledWith('change', instance.onAppStateChange); expect(EventEmitter.off).toHaveBeenCalledWith(ViewTypes.INDICATOR_BAR_VISIBLE, instance.onIndicatorBarVisible); expect(instance.removeViewableItemsListener).toHaveBeenCalled(); expect(instance.removeScrollEndIndexListener).toHaveBeenCalled(); @@ -217,6 +225,52 @@ describe('MoreMessagesButton', () => { wrapper.setProps({unreadCount: 2}); expect(instance.onViewableItemsChanged).toHaveBeenCalledTimes(1); }); + + test('componentDidUpdate should call hide when the unreadCount decreases to 0', () => { + const wrapper = shallowWithIntl( + , + ); + const instance = wrapper.instance(); + instance.hide = jest.fn(); + instance.viewableItems = [{index: 1}]; + + wrapper.setProps({unreadCount: 2}); + expect(instance.hide).not.toHaveBeenCalled(); + + wrapper.setProps({unreadCount: 1}); + expect(instance.hide).not.toHaveBeenCalled(); + + wrapper.setProps({unreadCount: 0}); + expect(instance.hide).toHaveBeenCalledTimes(1); + }); + }); + + describe('onAppStateChange', () => { + it('should call resetUnreadMessageCount when app state is not active', () => { + const wrapper = shallowWithIntl( + , + ); + const instance = wrapper.instance(); + + let appState = 'active'; + instance.onAppStateChange(appState); + expect(baseProps.resetUnreadMessageCount).not.toHaveBeenCalled(); + + appState = 'inactive'; + instance.onAppStateChange(appState); + expect(baseProps.resetUnreadMessageCount).toHaveBeenCalledTimes(1); + expect(baseProps.resetUnreadMessageCount).toHaveBeenCalledWith(baseProps.channelId); + + appState = 'background'; + instance.onAppStateChange(appState); + expect(baseProps.resetUnreadMessageCount).toHaveBeenCalledTimes(2); + expect(baseProps.resetUnreadMessageCount).toHaveBeenCalledWith(baseProps.channelId); + + appState = ''; + instance.onAppStateChange(appState); + expect(baseProps.resetUnreadMessageCount).toHaveBeenCalledTimes(3); + expect(baseProps.resetUnreadMessageCount).toHaveBeenCalledWith(baseProps.channelId); + }); }); describe('onIndicatorBarVisible', () => { @@ -574,18 +628,6 @@ describe('MoreMessagesButton', () => { expect(clearTimeout).not.toHaveBeenCalled(); }); - it('should return early when unreadCount is 0', () => { - const viewableItems = [{index: 0}, {index: 1}]; - - wrapper.setProps({newMessageLineIndex: 1, unreadCount: 0}); - instance.onViewableItemsChanged(viewableItems); - expect(clearTimeout).not.toHaveBeenCalled(); - - wrapper.setProps({newMessageLineIndex: -1}); - instance.onViewableItemsChanged(viewableItems); - expect(clearTimeout).not.toHaveBeenCalled(); - }); - it('should return early when viewableItems length is 0', () => { const viewableItems = []; wrapper.setProps({newMessageLineIndex: 1, unreadCount: 10}); @@ -732,21 +774,19 @@ describe('MoreMessagesButton', () => { jest.clearAllMocks(); }); - it('should force cancel when props.unreadCount - readCount is <= 0', () => { + it('should not set moreText when props.unreadCount - readCount is <= 0', () => { let readCount = props.unreadCount + 1; instance.showMoreText(readCount); - expect(instance.cancel).toHaveBeenCalledTimes(1); - expect(instance.cancel.mock.calls[0][0]).toBe(true); expect(instance.moreText).not.toHaveBeenCalled(); expect(instance.setState).not.toHaveBeenCalled(); readCount = props.unreadCount; instance.showMoreText(readCount); - expect(instance.cancel).toHaveBeenCalledTimes(2); - expect(instance.cancel.mock.calls[1][0]).toBe(true); + expect(instance.moreText).not.toHaveBeenCalled(); + expect(instance.setState).not.toHaveBeenCalled(); }); - it('should set moreTextd when props.unreadCount - readCount is > 0', () => { + it('should set moreText when props.unreadCount - readCount is > 0', () => { const moreCount = 1; const readCount = props.unreadCount - moreCount; instance.showMoreText(readCount); diff --git a/app/screens/channel/channel_base.js b/app/screens/channel/channel_base.js index 62b814a67..a5f19b182 100644 --- a/app/screens/channel/channel_base.js +++ b/app/screens/channel/channel_base.js @@ -31,7 +31,6 @@ export default class ChannelBase extends PureComponent { selectDefaultTeam: PropTypes.func.isRequired, selectInitialChannel: PropTypes.func.isRequired, recordLoadTime: PropTypes.func.isRequired, - resetUnreadMessageCount: PropTypes.func.isRequired, }).isRequired, componentId: PropTypes.string.isRequired, currentChannelId: PropTypes.string, @@ -92,7 +91,6 @@ export default class ChannelBase extends PureComponent { } if (currentChannelId) { - actions.resetUnreadMessageCount(this.props.currentChannelId); PushNotifications.clearChannelNotifications(currentChannelId); requestAnimationFrame(() => { actions.getChannelStats(currentChannelId); diff --git a/app/screens/channel/channel_base.test.js b/app/screens/channel/channel_base.test.js index f6feb8e7e..37d95c474 100644 --- a/app/screens/channel/channel_base.test.js +++ b/app/screens/channel/channel_base.test.js @@ -28,7 +28,6 @@ describe('ChannelBase', () => { recordLoadTime: jest.fn(), selectDefaultTeam: jest.fn(), selectInitialChannel: jest.fn(), - resetUnreadMessageCount: jest.fn(), }, componentId: channelBaseComponentId, theme: Preferences.THEMES.default, diff --git a/app/screens/channel/index.js b/app/screens/channel/index.js index 81b5ed39d..0fae44b5e 100644 --- a/app/screens/channel/index.js +++ b/app/screens/channel/index.js @@ -4,7 +4,7 @@ import {bindActionCreators} from 'redux'; import {connect} from 'react-redux'; -import {loadChannelsForTeam, selectInitialChannel, resetUnreadMessageCount} from '@actions/views/channel'; +import {loadChannelsForTeam, selectInitialChannel} from '@actions/views/channel'; import {recordLoadTime} from '@actions/views/root'; import {selectDefaultTeam} from '@actions/views/select_team'; import {ViewTypes} from '@constants'; @@ -49,7 +49,6 @@ function mapDispatchToProps(dispatch) { selectDefaultTeam, selectInitialChannel, recordLoadTime, - resetUnreadMessageCount, }, dispatch), }; }