From 17144c2a4637bd5334f3ba3b3f3f10424b345105 Mon Sep 17 00:00:00 2001 From: Tristan Cacqueray Date: Sun, 2 Dec 2018 01:46:43 +0000 Subject: [PATCH] web: refactor info and tenant reducers action This change adds info fetch state action type and simplifies the main App by using the new info attributes. Change-Id: I2cfd3f6ae605051e11f58272e62925d8f97e4ac9 --- web/src/App.jsx | 21 ++++++++-------- web/src/App.test.jsx | 6 ++--- web/src/actions/info.js | 50 ++++++++++++++++++++++++++++++-------- web/src/actions/tenant.js | 4 ++- web/src/index.js | 4 +-- web/src/reducers/info.js | 25 ++++++++++++++++--- web/src/reducers/tenant.js | 6 +++-- 7 files changed, 85 insertions(+), 31 deletions(-) diff --git a/web/src/App.jsx b/web/src/App.jsx index 85d2009606..5ca9f9bf86 100644 --- a/web/src/App.jsx +++ b/web/src/App.jsx @@ -86,8 +86,12 @@ class App extends React.Component { } renderContent = () => { - const { tenant } = this.props + const { info, tenant } = this.props const allRoutes = [] + + if (info.isFetching) { + return (

Fetching info...

) + } this.menu // Do not include '/tenants' route in white-label setup .filter(item => @@ -102,10 +106,13 @@ class App extends React.Component { /> ) }) + if (tenant.defaultRoute) + allRoutes.push( + + ) return ( {allRoutes} - ) } @@ -113,7 +120,7 @@ class App extends React.Component { componentDidUpdate() { // This method is called when info property is updated const { tenant, info } = this.props - if (info.capabilities) { + if (info.ready) { let tenantName, whiteLabel if (info.tenant) { @@ -129,12 +136,10 @@ class App extends React.Component { if (match) { tenantName = match.params.tenant - } else { - tenantName = '' } } // Set tenant only if it changed to prevent DidUpdate loop - if (typeof tenant.name === 'undefined' || tenant.name !== tenantName) { + if (tenant.name !== tenantName) { const tenantAction = setTenantAction(tenantName, whiteLabel) this.props.dispatch(tenantAction) if (tenantName) { @@ -204,10 +209,6 @@ class App extends React.Component { const { menuCollapsed, showErrors } = this.state const { tenant, configErrors } = this.props - if (typeof tenant.name === 'undefined') { - return (

Loading...

) - } - return ( { const application = ReactTestUtils.renderIntoDocument( ) - store.dispatch(fetchInfoAction()).then(() => { + store.dispatch(fetchInfoIfNeeded()).then(() => { // Link should be tenant scoped const topMenuLinks = ReactTestUtils.scryRenderedComponentsWithType( application, Link) @@ -86,7 +86,7 @@ it('renders single tenant', () => { ) - store.dispatch(fetchInfoAction()).then(() => { + store.dispatch(fetchInfoIfNeeded()).then(() => { // Link should be white-label scoped const topMenuLinks = ReactTestUtils.scryRenderedComponentsWithType( application, Link) diff --git a/web/src/actions/info.js b/web/src/actions/info.js index 8894eef1b5..81e9b4a2a0 100644 --- a/web/src/actions/info.js +++ b/web/src/actions/info.js @@ -12,16 +12,46 @@ // License for the specific language governing permissions and limitations // under the License. -import { fetchInfo } from '../api' +import * as API from '../api' -export function fetchInfoAction () { - return (dispatch) => { - return fetchInfo() - .then(response => { - dispatch({type: 'FETCH_INFO_SUCCESS', info: response.data.info}) - }) - .catch(error => { - throw (error) - }) +export const INFO_FETCH_REQUEST = 'INFO_FETCH_REQUEST' +export const INFO_FETCH_SUCCESS = 'INFO_FETCH_SUCCESS' +export const INFO_FETCH_FAIL = 'INFO_FETCH_FAIL' + +export const fetchInfoRequest = () => ({ + type: INFO_FETCH_REQUEST +}) + +export const fetchInfoSuccess = json => ({ + type: INFO_FETCH_SUCCESS, + tenant: json.info.tenant, +}) + +const fetchInfoFail = error => ({ + type: INFO_FETCH_FAIL, + error +}) + +const fetchInfo = () => dispatch => { + dispatch(fetchInfoRequest()) + return API.fetchInfo() + .then(response => dispatch(fetchInfoSuccess(response.data))) + .catch(error => dispatch(fetchInfoFail(error))) +} + +const shouldFetchInfo = state => { + const info = state.info + if (!info) { + return true + } + if (info.isFetching) { + return false + } + return true +} + +export const fetchInfoIfNeeded = () => (dispatch, getState) => { + if (shouldFetchInfo(getState())) { + return dispatch(fetchInfo()) } } diff --git a/web/src/actions/tenant.js b/web/src/actions/tenant.js index 992f4e9714..e40c8c0103 100644 --- a/web/src/actions/tenant.js +++ b/web/src/actions/tenant.js @@ -12,6 +12,8 @@ // License for the specific language governing permissions and limitations // under the License. +export const TENANT_SET = 'TENANT_SET' + export function setTenantAction (name, whiteLabel) { let apiPrefix = '' let linkPrefix = '' @@ -24,7 +26,7 @@ export function setTenantAction (name, whiteLabel) { defaultRoute = '/tenants' } return { - type: 'SET_TENANT', + type: TENANT_SET, tenant: { name: name, whiteLabel: whiteLabel, diff --git a/web/src/index.js b/web/src/index.js index 933186e454..07ad3ce37c 100644 --- a/web/src/index.js +++ b/web/src/index.js @@ -25,14 +25,14 @@ import './index.css' import { getHomepageUrl } from './api' import registerServiceWorker from './registerServiceWorker' -import { fetchInfoAction } from './actions/info' +import { fetchInfoIfNeeded } from './actions/info' import createZuulStore from './store' import App from './App' const store = createZuulStore() // Load info endpoint -store.dispatch(fetchInfoAction()) +store.dispatch(fetchInfoIfNeeded()) ReactDOM.render( diff --git a/web/src/reducers/info.js b/web/src/reducers/info.js index 5386622a7f..89cadf2753 100644 --- a/web/src/reducers/info.js +++ b/web/src/reducers/info.js @@ -12,10 +12,29 @@ // License for the specific language governing permissions and limitations // under the License. -export default (state = {}, action) => { +import { + INFO_FETCH_REQUEST, + INFO_FETCH_SUCCESS, + INFO_FETCH_FAIL, +} from '../actions/info' + +export default (state = { + isFetching: false, + tenant: null, +}, action) => { switch (action.type) { - case 'FETCH_INFO_SUCCESS': - return action.info + case INFO_FETCH_REQUEST: + case INFO_FETCH_FAIL: + return { + isFetching: true, + tenant: null, + } + case INFO_FETCH_SUCCESS: + return { + isFetching: false, + tenant: action.tenant, + ready: true + } default: return state } diff --git a/web/src/reducers/tenant.js b/web/src/reducers/tenant.js index b634b03b27..5911c834ce 100644 --- a/web/src/reducers/tenant.js +++ b/web/src/reducers/tenant.js @@ -12,9 +12,11 @@ // License for the specific language governing permissions and limitations // under the License. -export default (state = {}, action) => { +import { TENANT_SET } from '../actions/tenant' + +export default (state = {name: null}, action) => { switch (action.type) { - case 'SET_TENANT': + case TENANT_SET: return action.tenant default: return state