From c7dc992ac0b7af975455458ae1761b44c3373e27 Mon Sep 17 00:00:00 2001 From: Saturnino Abril Date: Sat, 9 Mar 2019 13:27:07 +0800 Subject: [PATCH] do not save email interval preference on cancel on Android device (#2633) --- .../notification_settings_email.android.js | 6 ++--- ...otification_settings_email.android.test.js | 24 +++++++++++++++++ .../notification_settings_email.ios.js | 10 +++---- .../notification_settings_email.ios.test.js | 21 ++++++++++++--- .../notification_settings_email_base.js | 27 +++++++++---------- 5 files changed, 62 insertions(+), 26 deletions(-) diff --git a/app/screens/settings/notification_settings_email/notification_settings_email.android.js b/app/screens/settings/notification_settings_email/notification_settings_email.android.js index 98d95e93d..5183421da 100644 --- a/app/screens/settings/notification_settings_email/notification_settings_email.android.js +++ b/app/screens/settings/notification_settings_email/notification_settings_email.android.js @@ -28,7 +28,7 @@ class NotificationSettingsEmailAndroid extends NotificationSettingsEmailBase { handleClose = () => { this.setState({ - newInterval: this.state.interval, + newInterval: this.state.emailInterval, showEmailNotificationsModal: false, }); } @@ -47,11 +47,11 @@ class NotificationSettingsEmailAndroid extends NotificationSettingsEmailBase { sendEmailNotifications, theme, } = this.props; - const {interval} = this.state; + const {newInterval} = this.state; let i18nId; let i18nMessage; if (sendEmailNotifications) { - switch (interval) { + switch (newInterval) { case Preferences.INTERVAL_IMMEDIATE.toString(): i18nId = t('user.settings.notifications.email.immediately'); i18nMessage = 'Immediately'; diff --git a/app/screens/settings/notification_settings_email/notification_settings_email.android.test.js b/app/screens/settings/notification_settings_email/notification_settings_email.android.test.js index 08c98ca00..46b85845b 100644 --- a/app/screens/settings/notification_settings_email/notification_settings_email.android.test.js +++ b/app/screens/settings/notification_settings_email/notification_settings_email.android.test.js @@ -12,6 +12,12 @@ import RadioButtonGroup from 'app/components/radio_button'; import NotificationSettingsEmailAndroid from './notification_settings_email.android.js'; +jest.mock('Platform', () => { + const Platform = require.requireActual('Platform'); + Platform.OS = 'android'; + return Platform; +}); + describe('NotificationSettingsEmailAndroid', () => { const baseProps = { currentUser: {id: 'current_user_id'}, @@ -139,4 +145,22 @@ describe('NotificationSettingsEmailAndroid', () => { wrapper.instance().showEmailModal(); expect(wrapper.state('showEmailNotificationsModal')).toEqual(true); }); + + test('should not save preference on back button on Android', () => { + const wrapper = shallowWithIntl( + + ); + + const instance = wrapper.instance(); + instance.saveEmailNotifyProps = jest.fn(); + + // should not save preference on back button on Android + // saving email preference on Android is done via Save button + instance.onNavigatorEvent({type: 'ScreenChangedEvent', id: 'willDisappear'}); + expect(instance.saveEmailNotifyProps).toHaveBeenCalledTimes(0); + + wrapper.setState({newInterval: '0'}); + instance.onNavigatorEvent({type: 'ScreenChangedEvent', id: 'willDisappear'}); + expect(instance.saveEmailNotifyProps).toHaveBeenCalledTimes(0); + }); }); diff --git a/app/screens/settings/notification_settings_email/notification_settings_email.ios.js b/app/screens/settings/notification_settings_email/notification_settings_email.ios.js index dcf3f140c..89125e958 100644 --- a/app/screens/settings/notification_settings_email/notification_settings_email.ios.js +++ b/app/screens/settings/notification_settings_email/notification_settings_email.ios.js @@ -28,7 +28,7 @@ class NotificationSettingsEmailIos extends NotificationSettingsEmailBase { siteName, theme, } = this.props; - const {interval} = this.state; + const {newInterval} = this.state; const style = getStyleSheet(theme); return ( @@ -53,7 +53,7 @@ class NotificationSettingsEmailIos extends NotificationSettingsEmailBase { action={this.setEmailNotifications} actionType='select' actionValue={Preferences.INTERVAL_IMMEDIATE.toString()} - selected={interval === Preferences.INTERVAL_IMMEDIATE.toString()} + selected={newInterval === Preferences.INTERVAL_IMMEDIATE.toString()} theme={theme} /> @@ -69,7 +69,7 @@ class NotificationSettingsEmailIos extends NotificationSettingsEmailBase { action={this.setEmailNotifications} actionType='select' actionValue={Preferences.INTERVAL_FIFTEEN_MINUTES.toString()} - selected={interval === Preferences.INTERVAL_FIFTEEN_MINUTES.toString()} + selected={newInterval === Preferences.INTERVAL_FIFTEEN_MINUTES.toString()} theme={theme} /> @@ -83,7 +83,7 @@ class NotificationSettingsEmailIos extends NotificationSettingsEmailBase { action={this.setEmailNotifications} actionType='select' actionValue={Preferences.INTERVAL_HOUR.toString()} - selected={interval === Preferences.INTERVAL_HOUR.toString()} + selected={newInterval === Preferences.INTERVAL_HOUR.toString()} theme={theme} /> @@ -99,7 +99,7 @@ class NotificationSettingsEmailIos extends NotificationSettingsEmailBase { action={this.setEmailNotifications} actionType='select' actionValue={Preferences.INTERVAL_NEVER.toString()} - selected={interval === Preferences.INTERVAL_NEVER.toString()} + selected={newInterval === Preferences.INTERVAL_NEVER.toString()} theme={theme} /> diff --git a/app/screens/settings/notification_settings_email/notification_settings_email.ios.test.js b/app/screens/settings/notification_settings_email/notification_settings_email.ios.test.js index 16f62dc1d..c20e7cc3d 100644 --- a/app/screens/settings/notification_settings_email/notification_settings_email.ios.test.js +++ b/app/screens/settings/notification_settings_email/notification_settings_email.ios.test.js @@ -12,6 +12,12 @@ import SectionItem from 'app/screens/settings/section_item'; import NotificationSettingsEmailIos from './notification_settings_email.ios.js'; +jest.mock('Platform', () => { + const Platform = require.requireActual('Platform'); + Platform.OS = 'ios'; + return Platform; +}); + jest.mock('app/utils/theme', () => { const original = require.requireActual('app/utils/theme'); return { @@ -43,16 +49,23 @@ describe('NotificationSettingsEmailIos', () => { expect(wrapper.instance().renderEmailSection()).toMatchSnapshot(); }); - test('should call saveEmailNotifyProps on onNavigatorEvent', () => { + test('should save preference on back button only if email interval has changed', () => { const wrapper = shallow( ); const instance = wrapper.instance(); - instance.saveEmailNotifyProps = jest.fn(); - instance.onNavigatorEvent({type: 'ScreenChangedEvent', id: 'willDisappear'}); - expect(instance.saveEmailNotifyProps).toHaveBeenCalledTimes(1); + // should not save preference if email interval has not changed. + instance.onNavigatorEvent({type: 'ScreenChangedEvent', id: 'willDisappear'}); + expect(baseProps.actions.updateMe).toHaveBeenCalledTimes(0); + expect(baseProps.actions.savePreferences).toHaveBeenCalledTimes(0); + + // should save preference if email interval has changed. + wrapper.setState({newInterval: '0'}); + instance.onNavigatorEvent({type: 'ScreenChangedEvent', id: 'willDisappear'}); + expect(baseProps.actions.updateMe).toHaveBeenCalledTimes(1); + expect(baseProps.actions.savePreferences).toHaveBeenCalledTimes(1); }); test('should call actions.updateMe and actions.savePreferences on saveEmailNotifyProps', () => { diff --git a/app/screens/settings/notification_settings_email/notification_settings_email_base.js b/app/screens/settings/notification_settings_email/notification_settings_email_base.js index 6db91fe25..7b90c6fa4 100644 --- a/app/screens/settings/notification_settings_email/notification_settings_email_base.js +++ b/app/screens/settings/notification_settings_email/notification_settings_email_base.js @@ -2,6 +2,7 @@ // See LICENSE.txt for license information. import {PureComponent} from 'react'; +import {Platform} from 'react-native'; import PropTypes from 'prop-types'; import {Preferences} from 'mattermost-redux/constants'; @@ -35,11 +36,9 @@ export default class NotificationSettingsEmailBase extends PureComponent { sendEmailNotifications, } = props; - const interval = this.computeEmailInterval(sendEmailNotifications, enableEmailBatching, emailInterval); - this.state = { - interval, - newInterval: interval, + emailInterval, + newInterval: this.computeEmailInterval(sendEmailNotifications, enableEmailBatching, emailInterval), showEmailNotificationsModal: false, }; @@ -62,16 +61,15 @@ export default class NotificationSettingsEmailBase extends PureComponent { this.props.enableEmailBatching !== enableEmailBatching || this.props.emailInterval !== emailInterval ) { - const interval = this.computeEmailInterval(sendEmailNotifications, enableEmailBatching, emailInterval); this.setState({ - interval, - newInterval: interval, + emailInterval, + newInterval: this.computeEmailInterval(sendEmailNotifications, enableEmailBatching, emailInterval), }); } } onNavigatorEvent = (event) => { - if (event.type === 'ScreenChangedEvent') { + if (Platform.OS === 'ios' && event.type === 'ScreenChangedEvent') { switch (event.id) { case 'willDisappear': this.saveEmailNotifyProps(); @@ -90,20 +88,21 @@ export default class NotificationSettingsEmailBase extends PureComponent { this.setState({ email, - interval: value, newInterval: value, }); }; saveEmailNotifyProps = () => { const {actions, currentUser} = this.props; - const {email, newInterval} = this.state; + const {email, emailInterval, newInterval} = this.state; - const notifyProps = getNotificationProps(currentUser); - actions.updateMe({notify_props: {...notifyProps, email}}); + if (emailInterval !== newInterval) { + const notifyProps = getNotificationProps(currentUser); + actions.updateMe({notify_props: {...notifyProps, email}}); - const emailInterval = {category: Preferences.CATEGORY_NOTIFICATIONS, user_id: currentUser.id, name: Preferences.EMAIL_INTERVAL, value: newInterval}; - actions.savePreferences(currentUser.id, [emailInterval]); + const emailIntervalPreference = {category: Preferences.CATEGORY_NOTIFICATIONS, user_id: currentUser.id, name: Preferences.EMAIL_INTERVAL, value: newInterval}; + actions.savePreferences(currentUser.id, [emailIntervalPreference]); + } }; computeEmailInterval = (sendEmailNotifications, enableEmailBatching, emailInterval) => {