From f32ba73dc6d55ed66f705abbf92d8fd00f6a3ef1 Mon Sep 17 00:00:00 2001 From: Jared Scheib Date: Mon, 20 Nov 2017 19:06:31 -0600 Subject: [PATCH 1/2] Refactor CheckSources to call getSources thunk instead of loadSources & errorThrown --- ui/src/CheckSources.js | 20 +++++--------------- ui/src/shared/actions/sources.js | 14 ++++++++++++-- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/ui/src/CheckSources.js b/ui/src/CheckSources.js index e3fbe1948..41c41670a 100644 --- a/ui/src/CheckSources.js +++ b/ui/src/CheckSources.js @@ -5,10 +5,9 @@ import {bindActionCreators} from 'redux' import {MEMBER_ROLE, VIEWER_ROLE} from 'src/auth/Authorized' -import {getSources} from 'shared/apis' import {showDatabases} from 'shared/apis/metaQuery' -import {loadSources as loadSourcesAction} from 'shared/actions/sources' +import {getSourcesAsync} from 'shared/actions/sources' import {errorThrown as errorThrownAction} from 'shared/actions/errors' import {DEFAULT_HOME_PAGE} from 'shared/constants' @@ -19,6 +18,7 @@ import {DEFAULT_HOME_PAGE} from 'shared/constants' const {arrayOf, bool, func, node, shape, string} = PropTypes const CheckSources = React.createClass({ propTypes: { + getSources: func.isRequired, sources: arrayOf( shape({ links: shape({ @@ -42,8 +42,6 @@ const CheckSources = React.createClass({ location: shape({ pathname: string.isRequired, }).isRequired, - loadSources: func.isRequired, - errorThrown: func.isRequired, auth: shape({ isUsingAuth: bool, me: shape({ @@ -81,16 +79,8 @@ const CheckSources = React.createClass({ }, async componentWillMount() { - const {loadSources, errorThrown} = this.props - - try { - const {data: {sources}} = await getSources() - loadSources(sources) - this.setState({isFetching: false}) - } catch (error) { - errorThrown(error, 'Unable to connect to Chronograf server') - this.setState({isFetching: false}) - } + await this.props.getSources() + this.setState({isFetching: false}) }, async componentWillUpdate(nextProps, nextState) { @@ -178,7 +168,7 @@ const mapStateToProps = ({sources, auth, me}) => ({ }) const mapDispatchToProps = dispatch => ({ - loadSources: bindActionCreators(loadSourcesAction, dispatch), + getSources: bindActionCreators(getSourcesAsync, dispatch), errorThrown: bindActionCreators(errorThrownAction, dispatch), }) diff --git a/ui/src/shared/actions/sources.js b/ui/src/shared/actions/sources.js index f96417664..d8b6263a8 100644 --- a/ui/src/shared/actions/sources.js +++ b/ui/src/shared/actions/sources.js @@ -1,11 +1,12 @@ import { deleteSource, - getSources, + getSources as getSourcesAJAX, getKapacitors as getKapacitorsAJAX, updateKapacitor as updateKapacitorAJAX, deleteKapacitor as deleteKapacitorAJAX, } from 'shared/apis' import {publishNotification} from './notifications' +import {errorThrown} from 'shared/actions/errors' import {HTTP_NOT_FOUND} from 'shared/constants' @@ -67,7 +68,7 @@ export const removeAndLoadSources = source => async dispatch => { } } - const {data: {sources: newSources}} = await getSources() + const {data: {sources: newSources}} = await getSourcesAJAX() dispatch(loadSources(newSources)) } catch (err) { dispatch( @@ -110,3 +111,12 @@ export const deleteKapacitorAsync = kapacitor => async dispatch => { ) } } + +export const getSourcesAsync = () => async dispatch => { + try { + const {data: {sources}} = await getSourcesAJAX() + dispatch(loadSources(sources)) + } catch (error) { + dispatch(errorThrown(error)) + } +} From e1e81bee5ee547188a3bf503145eeb3de7ec161c Mon Sep 17 00:00:00 2001 From: Jared Scheib Date: Mon, 20 Nov 2017 19:07:52 -0600 Subject: [PATCH 2/2] Prevent sources error notification from invalid showDatabases in CheckSources upon meChangeOrganization --- ui/src/CheckSources.js | 16 ++++++++++++++++ ui/src/shared/actions/auth.js | 3 +++ 2 files changed, 19 insertions(+) diff --git a/ui/src/CheckSources.js b/ui/src/CheckSources.js index 41c41670a..665481531 100644 --- a/ui/src/CheckSources.js +++ b/ui/src/CheckSources.js @@ -83,6 +83,20 @@ const CheckSources = React.createClass({ this.setState({isFetching: false}) }, + shouldComponentUpdate(nextProps) { + const {auth: {isUsingAuth, me}} = nextProps + // don't update this component if currentOrganization is what has changed, + // or else the app will try to call showDatabases in componentWillUpdate, + // which will fail unless sources have been refreshed + if ( + isUsingAuth && + me.currentOrganization.id !== this.props.auth.me.currentOrganization.id + ) { + return false + } + return true + }, + async componentWillUpdate(nextProps, nextState) { const { router, @@ -129,6 +143,8 @@ const CheckSources = React.createClass({ if (!isFetching && !location.pathname.includes('/manage-sources')) { // Do simple query to proxy to see if the source is up. try { + // the guard around currentOrganization prevents this showDatabases + // invocation since sources haven't been refreshed yet await showDatabases(source.links.proxy) } catch (error) { errorThrown(error, 'Unable to connect to source') diff --git a/ui/src/shared/actions/auth.js b/ui/src/shared/actions/auth.js index 43ab62ecc..e2d2b5d40 100644 --- a/ui/src/shared/actions/auth.js +++ b/ui/src/shared/actions/auth.js @@ -73,6 +73,9 @@ export const meChangeOrganizationAsync = ( ) dispatch(meChangeOrganizationCompleted()) dispatch(meReceivedUsingAuth(data)) + // TODO: reload sources upon me change org if non-refresh behavior preferred + // instead of current behavior on both invocations of meChangeOrganization, + // which is to refresh index via router.push('') } catch (error) { dispatch(errorThrown(error)) dispatch(meChangeOrganizationFailed())