From a600577b4295bcc1534b0a9b00194cb33cd5129f Mon Sep 17 00:00:00 2001 From: Jared Scheib Date: Fri, 10 Nov 2017 19:37:35 -0800 Subject: [PATCH] Refactor getMe & refresh me for SideNav upon change organizations --- ui/spec/shared/reducers/authSpec.js | 6 +-- ui/src/admin/containers/OrganizationsPage.js | 25 ++++++--- ui/src/index.js | 55 +++++--------------- ui/src/shared/actions/auth.js | 43 +++++++++++++-- ui/src/shared/apis/auth.js | 7 +++ ui/src/shared/apis/index.js | 7 --- ui/src/shared/reducers/auth.js | 2 +- 7 files changed, 81 insertions(+), 64 deletions(-) diff --git a/ui/spec/shared/reducers/authSpec.js b/ui/spec/shared/reducers/authSpec.js index 7ac9fa14da..6dd9cff203 100644 --- a/ui/spec/shared/reducers/authSpec.js +++ b/ui/spec/shared/reducers/authSpec.js @@ -4,7 +4,7 @@ import { authExpired, authRequested, authReceived, - meRequested, + meGetRequested, meReceivedNotUsingAuth, } from 'shared/actions/auth' @@ -51,8 +51,8 @@ describe('Shared.Reducers.authReducer', () => { expect(reducedState.isAuthLoading).to.equal(false) }) - it('should handle ME_REQUESTED', () => { - const reducedState = authReducer(initialState, meRequested()) + it('should handle ME_GET_REQUESTED', () => { + const reducedState = authReducer(initialState, meGetRequested()) expect(reducedState.isMeLoading).to.equal(true) }) diff --git a/ui/src/admin/containers/OrganizationsPage.js b/ui/src/admin/containers/OrganizationsPage.js index 3ce13ab633..4daa7f210f 100644 --- a/ui/src/admin/containers/OrganizationsPage.js +++ b/ui/src/admin/containers/OrganizationsPage.js @@ -3,30 +3,37 @@ import {connect} from 'react-redux' import {bindActionCreators} from 'redux' import * as adminChronografActionCreators from 'src/admin/actions/chronograf' -import {publishAutoDismissingNotification} from 'shared/dispatchers' +import {getMeAsync} from 'shared/actions/auth' import OrganizationsTable from 'src/admin/components/chronograf/OrganizationsTable' class OrganizationsPage extends Component { componentDidMount() { const {links, actions: {loadOrganizationsAsync}} = this.props - loadOrganizationsAsync(links.organizations) } - handleCreateOrganization = organization => { + handleCreateOrganization = async organization => { const {links, actions: {createOrganizationAsync}} = this.props - createOrganizationAsync(links.organizations, organization) + await createOrganizationAsync(links.organizations, organization) + this.refreshMe() } - handleRenameOrganization = (organization, name) => { + handleRenameOrganization = async (organization, name) => { const {actions: {updateOrganizationAsync}} = this.props - updateOrganizationAsync(organization, {...organization, name}) + await updateOrganizationAsync(organization, {...organization, name}) + this.refreshMe() } handleDeleteOrganization = organization => { const {actions: {deleteOrganizationAsync}} = this.props deleteOrganizationAsync(organization) + this.refreshMe() + } + + refreshMe = () => { + const {getMe} = this.props + getMe({shouldResetMe: false}) } handleTogglePublic = organization => { @@ -40,6 +47,8 @@ class OrganizationsPage extends Component { handleChooseDefaultRole = (organization, defaultRole) => { const {actions: {updateOrganizationAsync}} = this.props updateOrganizationAsync(organization, {...organization, defaultRole}) + // refreshMe is here to update the org's defaultRole in `me.organizations` + this.refreshMe() } render() { @@ -77,7 +86,7 @@ OrganizationsPage.propTypes = { updateOrganizationAsync: func.isRequired, deleteOrganizationAsync: func.isRequired, }), - notify: func.isRequired, + getMe: func.isRequired, } const mapStateToProps = ({links, adminChronograf: {organizations}}) => ({ @@ -87,7 +96,7 @@ const mapStateToProps = ({links, adminChronograf: {organizations}}) => ({ const mapDispatchToProps = dispatch => ({ actions: bindActionCreators(adminChronografActionCreators, dispatch), - notify: bindActionCreators(publishAutoDismissingNotification, dispatch), + getMe: bindActionCreators(getMeAsync, dispatch), }) export default connect(mapStateToProps, mapDispatchToProps)(OrganizationsPage) diff --git a/ui/src/index.js b/ui/src/index.js index 4c478c3731..1cae03513a 100644 --- a/ui/src/index.js +++ b/ui/src/index.js @@ -6,6 +6,7 @@ import {Provider} from 'react-redux' import {Router, Route, useRouterHistory} from 'react-router' import {createHistory} from 'history' import {syncHistoryWithStore} from 'react-router-redux' +import {bindActionCreators} from 'redux' import configureStore from 'src/store/configureStore' import {loadLocalStorage} from 'src/localStorage' @@ -34,18 +35,9 @@ import {AdminChronografPage, AdminInfluxDBPage} from 'src/admin' import {SourcePage, ManageSources} from 'src/sources' import NotFound from 'shared/components/NotFound' -import {getMe} from 'shared/apis' +import {getMeAsync} from 'shared/actions/auth' import {disablePresentationMode} from 'shared/actions/app' -import { - authRequested, - authReceived, - meRequested, - meReceivedNotUsingAuth, - meReceivedUsingAuth, - logoutLinkReceived, -} from 'shared/actions/auth' -import {linksReceived} from 'shared/actions/links' import {errorThrown} from 'shared/actions/errors' import 'src/style/chronograf.scss' @@ -87,45 +79,24 @@ const Root = React.createClass({ }, async checkAuth() { - dispatch(authRequested()) - dispatch(meRequested()) try { - await this.startHeartbeat({shouldDispatchResponse: true}) + await this.startHeartbeat({shouldResetMe: true}) } catch (error) { dispatch(errorThrown(error)) } }, - async startHeartbeat({shouldDispatchResponse}) { - try { - // These non-me objects are added to every response by some AJAX trickery - const { - data: me, - auth, - logoutLink, - external, - users, - organizations, - meLink, - } = await getMe() - if (shouldDispatchResponse) { - const isUsingAuth = !!logoutLink - dispatch( - isUsingAuth ? meReceivedUsingAuth(me) : meReceivedNotUsingAuth(me) - ) - dispatch(authReceived(auth)) - dispatch(logoutLinkReceived(logoutLink)) - dispatch(linksReceived({external, users, organizations, me: meLink})) - } + getMe: bindActionCreators(getMeAsync, dispatch), - setTimeout(() => { - if (store.getState().auth.me !== null) { - this.startHeartbeat({shouldDispatchResponse: false}) - } - }, HEARTBEAT_INTERVAL) - } catch (error) { - dispatch(errorThrown(error)) - } + async startHeartbeat(config) { + // TODO: use destructure syntax with default {} value -- couldn't figure it out + await this.getMe({shouldResetMe: config && config.shouldResetMe}) + + setTimeout(() => { + if (store.getState().auth.me !== null) { + this.startHeartbeat() + } + }, HEARTBEAT_INTERVAL) }, flushErrorsQueue() { diff --git a/ui/src/shared/actions/auth.js b/ui/src/shared/actions/auth.js index e2d2b5d403..fe37b46fa3 100644 --- a/ui/src/shared/actions/auth.js +++ b/ui/src/shared/actions/auth.js @@ -1,4 +1,6 @@ -import {updateMe as updateMeAJAX} from 'shared/apis/auth' +import {getMe as getMeAJAX, updateMe as updateMeAJAX} from 'shared/apis/auth' + +import {linksReceived} from 'shared/actions/links' import {publishAutoDismissingNotification} from 'shared/dispatchers' import {errorThrown} from 'shared/actions/errors' @@ -21,8 +23,8 @@ export const authReceived = auth => ({ }, }) -export const meRequested = () => ({ - type: 'ME_REQUESTED', +export const meGetRequested = () => ({ + type: 'ME_GET_REQUESTED', }) export const meReceivedNotUsingAuth = me => ({ @@ -39,6 +41,10 @@ export const meReceivedUsingAuth = me => ({ }, }) +export const meGetFailed = () => ({ + type: 'ME_GET_FAILED', +}) + export const meChangeOrganizationRequested = () => ({ type: 'ME_CHANGE_ORGANIZATION_REQUESTED', }) @@ -58,6 +64,37 @@ export const logoutLinkReceived = logoutLink => ({ }, }) +// shouldResetMe protects against `me` being nullified in Redux temporarily, +// which currently causes the app to show a loading spinner until me is +// re-hydrated. if `getMeAsync` is only being used to refresh me after creating +// an organization, this is undesirable behavior +export const getMeAsync = ({shouldResetMe}) => async dispatch => { + if (shouldResetMe) { + dispatch(authRequested()) + dispatch(meGetRequested()) + } + try { + // These non-me objects are added to every response by some AJAX trickery + const { + data: me, + auth, + logoutLink, + external, + users, + organizations, + meLink, + } = await getMeAJAX() + const isUsingAuth = !!logoutLink + dispatch(isUsingAuth ? meReceivedUsingAuth(me) : meReceivedNotUsingAuth(me)) + dispatch(authReceived(auth)) + dispatch(logoutLinkReceived(logoutLink)) + dispatch(linksReceived({external, users, organizations, me: meLink})) + } catch (error) { + dispatch(errorThrown(error)) + dispatch(meGetFailed()) + } +} + export const meChangeOrganizationAsync = ( url, organization diff --git a/ui/src/shared/apis/auth.js b/ui/src/shared/apis/auth.js index 581dfadd8e..deb364d6b5 100644 --- a/ui/src/shared/apis/auth.js +++ b/ui/src/shared/apis/auth.js @@ -1,5 +1,12 @@ import AJAX from 'src/utils/ajax' +export function getMe() { + return AJAX({ + resource: 'me', + method: 'GET', + }) +} + export const updateMe = async (url, updatedMe) => { try { return await AJAX({ diff --git a/ui/src/shared/apis/index.js b/ui/src/shared/apis/index.js index 9ab2ac57b3..1312fddede 100644 --- a/ui/src/shared/apis/index.js +++ b/ui/src/shared/apis/index.js @@ -8,13 +8,6 @@ export function fetchLayouts() { }) } -export function getMe() { - return AJAX({ - resource: 'me', - method: 'GET', - }) -} - export function getSources() { return AJAX({ resource: 'sources', diff --git a/ui/src/shared/reducers/auth.js b/ui/src/shared/reducers/auth.js index cea6c1c340..a92144b12f 100644 --- a/ui/src/shared/reducers/auth.js +++ b/ui/src/shared/reducers/auth.js @@ -25,7 +25,7 @@ const authReducer = (state = initialState, action) => { const {auth: {links}} = action.payload return {...state, links, isAuthLoading: false} } - case 'ME_REQUESTED': { + case 'ME_GET_REQUESTED': { return {...state, isMeLoading: true} } case 'ME_RECEIVED__NON_AUTH': {