From 320f1d9c91d109077bcfee7e9f6346a38e79d7dc Mon Sep 17 00:00:00 2001 From: Saturnino Abril Date: Thu, 30 Aug 2018 02:25:09 +0800 Subject: [PATCH] fix email notification settings (#2051) --- app/actions/views/account_notifications.js | 36 ---------------- .../settings/notification_settings/index.js | 4 +- .../notification_settings.js | 8 ++-- ...cation_settings_email.android.test.js.snap | 4 +- .../notification_settings_email/index.js | 2 + ...otification_settings_email.android.test.js | 42 ++++++++++++++----- .../notification_settings_email.ios.test.js | 34 +++++++++++---- .../notification_settings_email_base.js | 15 ++++--- 8 files changed, 77 insertions(+), 68 deletions(-) delete mode 100644 app/actions/views/account_notifications.js diff --git a/app/actions/views/account_notifications.js b/app/actions/views/account_notifications.js deleted file mode 100644 index 6c6a8ac88..000000000 --- a/app/actions/views/account_notifications.js +++ /dev/null @@ -1,36 +0,0 @@ -// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. -// See LICENSE.txt for license information. - -import {updateMe} from 'mattermost-redux/actions/users'; -import {Preferences} from 'mattermost-redux/constants'; -import {savePreferences} from 'mattermost-redux/actions/preferences'; -import {getConfig} from 'mattermost-redux/selectors/entities/general'; - -export function handleUpdateUserNotifyProps(notifyProps) { - return async (dispatch, getState) => { - const state = getState(); - const config = getConfig(state); - const {currentUserId} = state.entities.users; - - const {interval, user_id: userId, ...otherProps} = notifyProps; - - const email = notifyProps.email; - if (config.EnableEmailBatching === 'true' && email !== 'false') { - const emailInterval = [{ - user_id: userId, - category: Preferences.CATEGORY_NOTIFICATIONS, - name: Preferences.EMAIL_INTERVAL, - value: interval, - }]; - - savePreferences(currentUserId, emailInterval)(dispatch, getState); - } - - const props = {...otherProps, email}; - try { - await updateMe({notify_props: props})(dispatch, getState); - } catch (error) { - // do nothing - } - }; -} diff --git a/app/screens/settings/notification_settings/index.js b/app/screens/settings/notification_settings/index.js index 71673b63d..4dd02c312 100644 --- a/app/screens/settings/notification_settings/index.js +++ b/app/screens/settings/notification_settings/index.js @@ -9,7 +9,7 @@ import {getConfig} from 'mattermost-redux/selectors/entities/general'; import {getMyPreferences, getTheme} from 'mattermost-redux/selectors/entities/preferences'; import {isMinimumServerVersion} from 'mattermost-redux/utils/helpers'; -import {handleUpdateUserNotifyProps} from 'app/actions/views/account_notifications'; +import {updateMe} from 'mattermost-redux/actions/users'; import NotificationSettings from './notification_settings'; @@ -34,7 +34,7 @@ function mapStateToProps(state) { function mapDispatchToProps(dispatch) { return { actions: bindActionCreators({ - handleUpdateUserNotifyProps, + updateMe, }, dispatch), }; } diff --git a/app/screens/settings/notification_settings/notification_settings.js b/app/screens/settings/notification_settings/notification_settings.js index fe0c5c634..4a8271df6 100644 --- a/app/screens/settings/notification_settings/notification_settings.js +++ b/app/screens/settings/notification_settings/notification_settings.js @@ -24,7 +24,7 @@ import {changeOpacity, makeStyleSheetFromTheme, setNavigatorStyles} from 'app/ut class NotificationSettings extends PureComponent { static propTypes = { actions: PropTypes.shape({ - handleUpdateUserNotifyProps: PropTypes.func.isRequired, + updateMe: PropTypes.func.isRequired, }), currentUser: PropTypes.object.isRequired, intl: intlShape.isRequired, @@ -99,7 +99,7 @@ class NotificationSettings extends PureComponent { screenBackgroundColor: theme.centerChannelBg, }, passProps: { - currentUserId: currentUser.id, + currentUser, }, }); }; @@ -161,7 +161,7 @@ class NotificationSettings extends PureComponent { }; if (!deepEqual(previousProps, notifyProps)) { - this.props.actions.handleUpdateUserNotifyProps(notifyProps); + this.props.actions.updateMe({notify_props: notifyProps}); } }; @@ -195,7 +195,7 @@ class NotificationSettings extends PureComponent { } if (this.shouldSaveAutoResponder(notifyProps)) { - this.props.actions.handleUpdateUserNotifyProps(notifyProps); + this.props.actions.updateMe({notify_props: notifyProps}); } }; diff --git a/app/screens/settings/notification_settings_email/__snapshots__/notification_settings_email.android.test.js.snap b/app/screens/settings/notification_settings_email/__snapshots__/notification_settings_email.android.test.js.snap index 5dd411882..6d758295f 100644 --- a/app/screens/settings/notification_settings_email/__snapshots__/notification_settings_email.android.test.js.snap +++ b/app/screens/settings/notification_settings_email/__snapshots__/notification_settings_email.android.test.js.snap @@ -1,6 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`NotificationSettingsMobileAndroid should match snapshot 1`] = ` +exports[`NotificationSettingsEmailAndroid should match snapshot 1`] = ` `; -exports[`NotificationSettingsMobileAndroid should match snapshot 2`] = ` +exports[`NotificationSettingsEmailAndroid should match snapshot 2`] = ` { +describe('NotificationSettingsEmailAndroid', () => { const baseProps = { - currentUserId: 'current_user_id', + currentUser: {id: 'current_user_id'}, emailInterval: '30', enableEmailBatching: false, - navigator: {setOnNavigatorEvent: () => {}}, // eslint-disable-line no-empty-function + navigator: {setOnNavigatorEvent: emptyFunction}, actions: { - savePreferences: () => {}, // eslint-disable-line no-empty-function + updateMe: jest.fn(), + savePreferences: jest.fn(), }, sendEmailNotifications: true, siteName: 'Mattermost', @@ -29,7 +31,7 @@ describe('NotificationSettingsMobileAndroid', () => { test('should match snapshot', () => { const wrapper = shallowWithIntl( - + ); const style = { @@ -52,7 +54,7 @@ describe('NotificationSettingsMobileAndroid', () => { test('should match state on setEmailNotifications', () => { const wrapper = shallowWithIntl( - + ); wrapper.setState({email: 'false', interval: '0'}); @@ -68,7 +70,7 @@ describe('NotificationSettingsMobileAndroid', () => { test('should match state on handleClose', () => { const wrapper = shallowWithIntl( - + ); wrapper.setState({showEmailNotificationsModal: true, interval: '30', newInterval: '3600'}); @@ -79,7 +81,7 @@ describe('NotificationSettingsMobileAndroid', () => { test('should saveEmailNotifyProps and handleClose on handleSaveEmailNotification', () => { const wrapper = shallowWithIntl( - + ); const instance = wrapper.instance(); @@ -89,9 +91,27 @@ describe('NotificationSettingsMobileAndroid', () => { expect(instance.saveEmailNotifyProps).toHaveBeenCalledTimes(1); }); + test('should call actions.updateMe and actions.savePreferences on saveEmailNotifyProps', () => { + const savePreferences = jest.fn(); + const updateMe = jest.fn(); + const props = {...baseProps, actions: {savePreferences, updateMe}}; + const wrapper = shallowWithIntl( + + ); + + wrapper.setState({email: 'true', newInterval: 30}); + wrapper.instance().saveEmailNotifyProps(); + + expect(updateMe).toHaveBeenCalledTimes(1); + expect(updateMe.mock.calls[0][0].notify_props.email).toBe('true'); + + expect(savePreferences).toHaveBeenCalledTimes(1); + expect(savePreferences).toBeCalledWith('current_user_id', [{category: 'notifications', name: 'email_interval', user_id: 'current_user_id', value: 30}]); + }); + test('should match state on showEmailModal', () => { const wrapper = shallowWithIntl( - + ); wrapper.setState({showEmailNotificationsModal: false}); @@ -101,7 +121,7 @@ describe('NotificationSettingsMobileAndroid', () => { test('should match state on handleChange', () => { const wrapper = shallowWithIntl( - + ); wrapper.setState({newInterval: '3600'}); 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 a64cdb6b0..5cac49efd 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 @@ -6,6 +6,8 @@ import {configure, shallow} from 'enzyme'; import Adapter from 'enzyme-adapter-react-16'; configure({adapter: new Adapter()}); +import {emptyFunction} from 'app/utils/general'; + import NotificationSettingsEmailIos from './notification_settings_email.ios.js'; jest.mock('app/utils/theme', () => { @@ -18,12 +20,13 @@ jest.mock('app/utils/theme', () => { describe('NotificationSettingsEmailIos', () => { const baseProps = { - currentUserId: 'current_user_id', + currentUser: {id: 'current_user_id'}, emailInterval: '30', enableEmailBatching: false, - navigator: {setOnNavigatorEvent: () => {}}, // eslint-disable-line no-empty-function + navigator: {setOnNavigatorEvent: emptyFunction}, actions: { - savePreferences: () => {}, // eslint-disable-line no-empty-function + updateMe: jest.fn(), + savePreferences: jest.fn(), }, sendEmailNotifications: true, siteName: 'Mattermost', @@ -53,6 +56,24 @@ describe('NotificationSettingsEmailIos', () => { expect(instance.saveEmailNotifyProps).toHaveBeenCalledTimes(1); }); + test('should call actions.updateMe and actions.savePreferences on saveEmailNotifyProps', () => { + const savePreferences = jest.fn(); + const updateMe = jest.fn(); + const props = {...baseProps, actions: {savePreferences, updateMe}}; + const wrapper = shallow( + + ); + + wrapper.setState({email: 'true', newInterval: 30}); + wrapper.instance().saveEmailNotifyProps(); + + expect(updateMe).toHaveBeenCalledTimes(1); + expect(updateMe.mock.calls[0][0].notify_props.email).toBe('true'); + + expect(savePreferences).toHaveBeenCalledTimes(1); + expect(savePreferences).toBeCalledWith('current_user_id', [{category: 'notifications', name: 'email_interval', user_id: 'current_user_id', value: 30}]); + }); + test('should macth state on setEmailNotifications', () => { const wrapper = shallow( @@ -70,7 +91,7 @@ describe('NotificationSettingsEmailIos', () => { }); test('should call props.actions.savePreferences on saveUserNotifyProps', () => { - const props = {...baseProps, actions: {savePreferences: jest.fn()}}; + const props = {...baseProps, actions: {savePreferences: jest.fn(), updateMe: jest.fn()}}; const wrapper = shallow( ); @@ -80,10 +101,7 @@ describe('NotificationSettingsEmailIos', () => { expect(props.actions.savePreferences).toHaveBeenCalledTimes(1); expect(props.actions.savePreferences).toBeCalledWith( 'current_user_id', - [ - {category: 'notifications', name: 'email', user_id: 'current_user_id', value: 'true'}, - {category: 'notifications', name: 'email_interval', user_id: 'current_user_id', value: '3600'}, - ] + [{category: 'notifications', name: 'email_interval', user_id: 'current_user_id', value: '3600'}], ); }); }); 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 93d3fcecd..6db91fe25 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 @@ -7,14 +7,16 @@ import PropTypes from 'prop-types'; import {Preferences} from 'mattermost-redux/constants'; import {getEmailInterval} from 'mattermost-redux/utils/notify_props'; +import {getNotificationProps} from 'app/utils/notify_props'; import {setNavigatorStyles} from 'app/utils/theme'; export default class NotificationSettingsEmailBase extends PureComponent { static propTypes = { actions: PropTypes.shape({ savePreferences: PropTypes.func.isRequired, + updateMe: PropTypes.func.isRequired, }), - currentUserId: PropTypes.string.isRequired, + currentUser: PropTypes.object.isRequired, emailInterval: PropTypes.string.isRequired, enableEmailBatching: PropTypes.bool.isRequired, navigator: PropTypes.object, @@ -94,11 +96,14 @@ export default class NotificationSettingsEmailBase extends PureComponent { }; saveEmailNotifyProps = () => { - const {currentUserId} = this.props; + const {actions, currentUser} = this.props; const {email, newInterval} = this.state; - const emailNotify = {category: Preferences.CATEGORY_NOTIFICATIONS, user_id: currentUserId, name: 'email', value: email}; - const emailInterval = {category: Preferences.CATEGORY_NOTIFICATIONS, user_id: currentUserId, name: Preferences.EMAIL_INTERVAL, value: newInterval}; - this.props.actions.savePreferences(currentUserId, [emailNotify, emailInterval]); + + 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]); }; computeEmailInterval = (sendEmailNotifications, enableEmailBatching, emailInterval) => {