From 044ee7ba6966ba1b26640329d7c078ee7a4c28a4 Mon Sep 17 00:00:00 2001 From: JoramWilander Date: Thu, 7 Mar 2019 17:39:27 -0500 Subject: [PATCH 1/3] Load user after setting up MFA on login or sign up --- .../__snapshots__/confirm.test.jsx.snap | 0 components/mfa/{ => confirm}/confirm.jsx | 16 ++++++++- components/mfa/{ => confirm}/confirm.test.jsx | 34 +++++++++++++++---- components/mfa/confirm/index.js | 30 ++++++++++++++++ 4 files changed, 72 insertions(+), 8 deletions(-) rename components/mfa/{ => confirm}/__snapshots__/confirm.test.jsx.snap (100%) rename components/mfa/{ => confirm}/confirm.jsx (81%) rename components/mfa/{ => confirm}/confirm.test.jsx (62%) create mode 100644 components/mfa/confirm/index.js diff --git a/components/mfa/__snapshots__/confirm.test.jsx.snap b/components/mfa/confirm/__snapshots__/confirm.test.jsx.snap similarity index 100% rename from components/mfa/__snapshots__/confirm.test.jsx.snap rename to components/mfa/confirm/__snapshots__/confirm.test.jsx.snap diff --git a/components/mfa/confirm.jsx b/components/mfa/confirm/confirm.jsx similarity index 81% rename from components/mfa/confirm.jsx rename to components/mfa/confirm/confirm.jsx index 5b130af5cec1..886e58db56ce 100644 --- a/components/mfa/confirm.jsx +++ b/components/mfa/confirm/confirm.jsx @@ -3,15 +3,25 @@ import React from 'react'; import {FormattedMessage} from 'react-intl'; +import PropTypes from 'prop-types'; import Constants from 'utils/constants.jsx'; import {isKeyPressed} from 'utils/utils.jsx'; +import {redirectUserToDefaultTeam} from 'actions/global_actions.jsx'; + import FormattedMarkdownMessage from 'components/formatted_markdown_message.jsx'; const KeyCodes = Constants.KeyCodes; export default class Confirm extends React.Component { + static propTypes = { + shouldLoadUser: PropTypes.bool, + actions: PropTypes.shape({ + loadMe: PropTypes.func.isRequired, + }), + } + componentDidMount() { document.body.addEventListener('keydown', this.onKeyPress); } @@ -23,7 +33,11 @@ export default class Confirm extends React.Component { submit = (e) => { e.preventDefault(); - this.props.history.push('/'); + if (this.props.shouldLoadUser) { + this.props.actions.loadMe().then(() => redirectUserToDefaultTeam()); + } else { + redirectUserToDefaultTeam(); + } } onKeyPress = (e) => { diff --git a/components/mfa/confirm.test.jsx b/components/mfa/confirm/confirm.test.jsx similarity index 62% rename from components/mfa/confirm.test.jsx rename to components/mfa/confirm/confirm.test.jsx index 0aa0e4ac01cc..f72beb4faa28 100644 --- a/components/mfa/confirm.test.jsx +++ b/components/mfa/confirm/confirm.test.jsx @@ -4,11 +4,17 @@ import React from 'react'; import {shallow} from 'enzyme'; +import {redirectUserToDefaultTeam} from 'actions/global_actions.jsx'; + import {browserHistory} from 'utils/browser_history'; import {mountWithIntl} from 'tests/helpers/intl-test-helper.jsx'; -import Confirm from 'components/mfa/confirm'; +import Confirm from 'components/mfa/confirm/confirm.jsx'; import Constants from 'utils/constants.jsx'; +jest.mock('actions/global_actions.jsx', () => ({ + redirectUserToDefaultTeam: jest.fn(), +})); + describe('components/mfa/components/Confirm', () => { const baseProps = { history: browserHistory, @@ -26,21 +32,21 @@ describe('components/mfa/components/Confirm', () => { test('should submit on form submit', () => { const props = { - history: { - push: jest.fn(), + actions: { + loadMe: jest.fn(), }, }; const wrapper = mountWithIntl(); wrapper.find('form').simulate('submit'); - expect(props.history.push).toHaveBeenCalledWith('/'); + expect(redirectUserToDefaultTeam).toHaveBeenCalled(); }); test('should submit on enter', () => { const props = { - history: { - push: jest.fn(), + actions: { + loadMe: jest.fn(), }, }; @@ -57,6 +63,20 @@ describe('components/mfa/components/Confirm', () => { }; map.keydown(event); - expect(props.history.push).toHaveBeenCalledWith('/'); + expect(redirectUserToDefaultTeam).toHaveBeenCalled(); + }); + + test('should submit and load user', () => { + const props = { + shouldLoadUser: true, + actions: { + loadMe: jest.fn().mockResolvedValue({}), + }, + }; + + const wrapper = mountWithIntl(); + wrapper.find('form').simulate('submit'); + + expect(props.actions.loadMe).toHaveBeenCalled(); }); }); diff --git a/components/mfa/confirm/index.js b/components/mfa/confirm/index.js new file mode 100644 index 000000000000..6255680f33d0 --- /dev/null +++ b/components/mfa/confirm/index.js @@ -0,0 +1,30 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {connect} from 'react-redux'; +import {bindActionCreators} from 'redux'; + +import {loadMe} from 'mattermost-redux/actions/users'; +import {getTeamMemberships} from 'mattermost-redux/selectors/entities/teams'; + +import {isEmptyObject} from 'utils/utils'; + +import Confirm from './confirm.jsx'; + +function mapStateToProps(state) { + return { + + // Assume we need to load the user if they don't have any team memberships loaded + shouldLoadUser: isEmptyObject(getTeamMemberships(state)), + }; +} + +function mapDispatchToProps(dispatch) { + return { + actions: bindActionCreators({ + loadMe, + }, dispatch), + }; +} + +export default connect(mapStateToProps, mapDispatchToProps)(Confirm); From af943b82cfea5a916483ba42757c996716dd5b4e Mon Sep 17 00:00:00 2001 From: JoramWilander Date: Mon, 11 Mar 2019 10:04:23 -0400 Subject: [PATCH 2/3] Move load user logic into redirectUserToDefaultTeam --- actions/global_actions.jsx | 16 ++++++++-- .../__snapshots__/confirm.test.jsx.snap | 0 components/mfa/{confirm => }/confirm.jsx | 15 +--------- components/mfa/{confirm => }/confirm.test.jsx | 16 +--------- components/mfa/confirm/index.js | 30 ------------------- 5 files changed, 15 insertions(+), 62 deletions(-) rename components/mfa/{confirm => }/__snapshots__/confirm.test.jsx.snap (100%) rename components/mfa/{confirm => }/confirm.jsx (84%) rename components/mfa/{confirm => }/confirm.test.jsx (80%) delete mode 100644 components/mfa/confirm/index.js diff --git a/actions/global_actions.jsx b/actions/global_actions.jsx index 3f933d2bcb56..70cef9470afa 100644 --- a/actions/global_actions.jsx +++ b/actions/global_actions.jsx @@ -12,10 +12,10 @@ import { markChannelAsRead, selectChannel, } from 'mattermost-redux/actions/channels'; -import {logout} from 'mattermost-redux/actions/users'; +import {logout, loadMe} from 'mattermost-redux/actions/users'; import {Client4} from 'mattermost-redux/client'; import {getConfig} from 'mattermost-redux/selectors/entities/general'; -import {getCurrentTeamId, getTeam, getMyTeams, getMyTeamMember} from 'mattermost-redux/selectors/entities/teams'; +import {getCurrentTeamId, getTeam, getMyTeams, getMyTeamMember, getTeamMemberships} from 'mattermost-redux/selectors/entities/teams'; import {getCurrentUserId} from 'mattermost-redux/selectors/entities/users'; import {getCurrentChannelStats, getCurrentChannelId, getChannelByName, getMyChannelMember as selectMyChannelMember} from 'mattermost-redux/selectors/entities/channels'; import {ChannelTypes} from 'mattermost-redux/action_types'; @@ -275,7 +275,17 @@ export function emitBrowserFocus(focus) { } export async function redirectUserToDefaultTeam() { - const state = getState(); + let state = getState(); + + // Assume we need to load the user if they don't have any team memberships loaded + const shouldLoadUser = Utils.isEmptyObject(getTeamMemberships(state)); + + if (shouldLoadUser) { + await dispatch(loadMe()); + } + + state = getState(); + const userId = getCurrentUserId(state); const locale = getCurrentLocale(state); const teamId = LocalStorageStore.getPreviousTeamId(userId); diff --git a/components/mfa/confirm/__snapshots__/confirm.test.jsx.snap b/components/mfa/__snapshots__/confirm.test.jsx.snap similarity index 100% rename from components/mfa/confirm/__snapshots__/confirm.test.jsx.snap rename to components/mfa/__snapshots__/confirm.test.jsx.snap diff --git a/components/mfa/confirm/confirm.jsx b/components/mfa/confirm.jsx similarity index 84% rename from components/mfa/confirm/confirm.jsx rename to components/mfa/confirm.jsx index 886e58db56ce..8af7714502da 100644 --- a/components/mfa/confirm/confirm.jsx +++ b/components/mfa/confirm.jsx @@ -3,7 +3,6 @@ import React from 'react'; import {FormattedMessage} from 'react-intl'; -import PropTypes from 'prop-types'; import Constants from 'utils/constants.jsx'; import {isKeyPressed} from 'utils/utils.jsx'; @@ -15,13 +14,6 @@ import FormattedMarkdownMessage from 'components/formatted_markdown_message.jsx' const KeyCodes = Constants.KeyCodes; export default class Confirm extends React.Component { - static propTypes = { - shouldLoadUser: PropTypes.bool, - actions: PropTypes.shape({ - loadMe: PropTypes.func.isRequired, - }), - } - componentDidMount() { document.body.addEventListener('keydown', this.onKeyPress); } @@ -32,12 +24,7 @@ export default class Confirm extends React.Component { submit = (e) => { e.preventDefault(); - - if (this.props.shouldLoadUser) { - this.props.actions.loadMe().then(() => redirectUserToDefaultTeam()); - } else { - redirectUserToDefaultTeam(); - } + redirectUserToDefaultTeam(); } onKeyPress = (e) => { diff --git a/components/mfa/confirm/confirm.test.jsx b/components/mfa/confirm.test.jsx similarity index 80% rename from components/mfa/confirm/confirm.test.jsx rename to components/mfa/confirm.test.jsx index f72beb4faa28..249675cbdc26 100644 --- a/components/mfa/confirm/confirm.test.jsx +++ b/components/mfa/confirm.test.jsx @@ -8,7 +8,7 @@ import {redirectUserToDefaultTeam} from 'actions/global_actions.jsx'; import {browserHistory} from 'utils/browser_history'; import {mountWithIntl} from 'tests/helpers/intl-test-helper.jsx'; -import Confirm from 'components/mfa/confirm/confirm.jsx'; +import Confirm from 'components/mfa/confirm.jsx'; import Constants from 'utils/constants.jsx'; jest.mock('actions/global_actions.jsx', () => ({ @@ -65,18 +65,4 @@ describe('components/mfa/components/Confirm', () => { expect(redirectUserToDefaultTeam).toHaveBeenCalled(); }); - - test('should submit and load user', () => { - const props = { - shouldLoadUser: true, - actions: { - loadMe: jest.fn().mockResolvedValue({}), - }, - }; - - const wrapper = mountWithIntl(); - wrapper.find('form').simulate('submit'); - - expect(props.actions.loadMe).toHaveBeenCalled(); - }); }); diff --git a/components/mfa/confirm/index.js b/components/mfa/confirm/index.js deleted file mode 100644 index 6255680f33d0..000000000000 --- a/components/mfa/confirm/index.js +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. -// See LICENSE.txt for license information. - -import {connect} from 'react-redux'; -import {bindActionCreators} from 'redux'; - -import {loadMe} from 'mattermost-redux/actions/users'; -import {getTeamMemberships} from 'mattermost-redux/selectors/entities/teams'; - -import {isEmptyObject} from 'utils/utils'; - -import Confirm from './confirm.jsx'; - -function mapStateToProps(state) { - return { - - // Assume we need to load the user if they don't have any team memberships loaded - shouldLoadUser: isEmptyObject(getTeamMemberships(state)), - }; -} - -function mapDispatchToProps(dispatch) { - return { - actions: bindActionCreators({ - loadMe, - }, dispatch), - }; -} - -export default connect(mapStateToProps, mapDispatchToProps)(Confirm); From 94035711116d3e79d523f6d969587868c3ac3b5b Mon Sep 17 00:00:00 2001 From: JoramWilander Date: Mon, 11 Mar 2019 10:07:46 -0400 Subject: [PATCH 3/3] Clean up unit test --- components/mfa/confirm.test.jsx | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/components/mfa/confirm.test.jsx b/components/mfa/confirm.test.jsx index 249675cbdc26..da2a92acee7e 100644 --- a/components/mfa/confirm.test.jsx +++ b/components/mfa/confirm.test.jsx @@ -6,7 +6,6 @@ import {shallow} from 'enzyme'; import {redirectUserToDefaultTeam} from 'actions/global_actions.jsx'; -import {browserHistory} from 'utils/browser_history'; import {mountWithIntl} from 'tests/helpers/intl-test-helper.jsx'; import Confirm from 'components/mfa/confirm.jsx'; import Constants from 'utils/constants.jsx'; @@ -16,46 +15,30 @@ jest.mock('actions/global_actions.jsx', () => ({ })); describe('components/mfa/components/Confirm', () => { - const baseProps = { - history: browserHistory, - }; - const originalAddEventListener = document.body.addEventListener; afterAll(() => { document.body.addEventListener = originalAddEventListener; }); test('should match snapshot', () => { - const wrapper = shallow(); + const wrapper = shallow(); expect(wrapper).toMatchSnapshot(); }); test('should submit on form submit', () => { - const props = { - actions: { - loadMe: jest.fn(), - }, - }; - - const wrapper = mountWithIntl(); + const wrapper = mountWithIntl(); wrapper.find('form').simulate('submit'); expect(redirectUserToDefaultTeam).toHaveBeenCalled(); }); test('should submit on enter', () => { - const props = { - actions: { - loadMe: jest.fn(), - }, - }; - const map = {}; document.body.addEventListener = jest.fn().mockImplementation((event, cb) => { map[event] = cb; }); - mountWithIntl(); + mountWithIntl(); const event = { preventDefault: jest.fn(),