diff options
author | Tristan Cacqueray <tdecacqu@redhat.com> | 2018-12-02 01:46:43 +0000 |
---|---|---|
committer | Tristan Cacqueray <tdecacqu@redhat.com> | 2018-12-02 05:43:25 +0000 |
commit | 17144c2a4637bd5334f3ba3b3f3f10424b345105 (patch) | |
tree | d8746690f8d61415765cbd043e423037ddabd03a | |
parent | 76867ca14aec3e7e6df8604e911826049b57946f (diff) |
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
Notes
Notes (review):
Code-Review+1: Artem Goncharov <artem.goncharov@gmail.com>
Code-Review+2: Monty Taylor <mordred@inaugust.com>
Code-Review+2: Tobias Henkel <tobias.henkel@bmw.de>
Workflow+1: Monty Taylor <mordred@inaugust.com>
Verified+2: Zuul
Submitted-by: Zuul
Submitted-at: Thu, 06 Dec 2018 19:35:16 +0000
Reviewed-on: https://review.openstack.org/621386
Project: openstack-infra/zuul
Branch: refs/heads/master
-rw-r--r-- | web/src/App.jsx | 21 | ||||
-rw-r--r-- | web/src/App.test.jsx | 6 | ||||
-rw-r--r-- | web/src/actions/info.js | 52 | ||||
-rw-r--r-- | web/src/actions/tenant.js | 4 | ||||
-rw-r--r-- | web/src/index.js | 4 | ||||
-rw-r--r-- | web/src/reducers/info.js | 25 | ||||
-rw-r--r-- | web/src/reducers/tenant.js | 6 |
7 files changed, 86 insertions, 32 deletions
diff --git a/web/src/App.jsx b/web/src/App.jsx index 85d2009..5ca9f9b 100644 --- a/web/src/App.jsx +++ b/web/src/App.jsx | |||
@@ -86,8 +86,12 @@ class App extends React.Component { | |||
86 | } | 86 | } |
87 | 87 | ||
88 | renderContent = () => { | 88 | renderContent = () => { |
89 | const { tenant } = this.props | 89 | const { info, tenant } = this.props |
90 | const allRoutes = [] | 90 | const allRoutes = [] |
91 | |||
92 | if (info.isFetching) { | ||
93 | return (<h2>Fetching info...</h2>) | ||
94 | } | ||
91 | this.menu | 95 | this.menu |
92 | // Do not include '/tenants' route in white-label setup | 96 | // Do not include '/tenants' route in white-label setup |
93 | .filter(item => | 97 | .filter(item => |
@@ -102,10 +106,13 @@ class App extends React.Component { | |||
102 | /> | 106 | /> |
103 | ) | 107 | ) |
104 | }) | 108 | }) |
109 | if (tenant.defaultRoute) | ||
110 | allRoutes.push( | ||
111 | <Redirect from='*' to={tenant.defaultRoute} key='default-route' /> | ||
112 | ) | ||
105 | return ( | 113 | return ( |
106 | <Switch> | 114 | <Switch> |
107 | {allRoutes} | 115 | {allRoutes} |
108 | <Redirect from='*' to={tenant.defaultRoute} key='default-route' /> | ||
109 | </Switch> | 116 | </Switch> |
110 | ) | 117 | ) |
111 | } | 118 | } |
@@ -113,7 +120,7 @@ class App extends React.Component { | |||
113 | componentDidUpdate() { | 120 | componentDidUpdate() { |
114 | // This method is called when info property is updated | 121 | // This method is called when info property is updated |
115 | const { tenant, info } = this.props | 122 | const { tenant, info } = this.props |
116 | if (info.capabilities) { | 123 | if (info.ready) { |
117 | let tenantName, whiteLabel | 124 | let tenantName, whiteLabel |
118 | 125 | ||
119 | if (info.tenant) { | 126 | if (info.tenant) { |
@@ -129,12 +136,10 @@ class App extends React.Component { | |||
129 | 136 | ||
130 | if (match) { | 137 | if (match) { |
131 | tenantName = match.params.tenant | 138 | tenantName = match.params.tenant |
132 | } else { | ||
133 | tenantName = '' | ||
134 | } | 139 | } |
135 | } | 140 | } |
136 | // Set tenant only if it changed to prevent DidUpdate loop | 141 | // Set tenant only if it changed to prevent DidUpdate loop |
137 | if (typeof tenant.name === 'undefined' || tenant.name !== tenantName) { | 142 | if (tenant.name !== tenantName) { |
138 | const tenantAction = setTenantAction(tenantName, whiteLabel) | 143 | const tenantAction = setTenantAction(tenantName, whiteLabel) |
139 | this.props.dispatch(tenantAction) | 144 | this.props.dispatch(tenantAction) |
140 | if (tenantName) { | 145 | if (tenantName) { |
@@ -204,10 +209,6 @@ class App extends React.Component { | |||
204 | const { menuCollapsed, showErrors } = this.state | 209 | const { menuCollapsed, showErrors } = this.state |
205 | const { tenant, configErrors } = this.props | 210 | const { tenant, configErrors } = this.props |
206 | 211 | ||
207 | if (typeof tenant.name === 'undefined') { | ||
208 | return (<h2>Loading...</h2>) | ||
209 | } | ||
210 | |||
211 | return ( | 212 | return ( |
212 | <React.Fragment> | 213 | <React.Fragment> |
213 | <Masthead | 214 | <Masthead |
diff --git a/web/src/App.test.jsx b/web/src/App.test.jsx index 58a88f4..2b94e60 100644 --- a/web/src/App.test.jsx +++ b/web/src/App.test.jsx | |||
@@ -19,7 +19,7 @@ import ReactDOM from 'react-dom' | |||
19 | import { Link, BrowserRouter as Router } from 'react-router-dom' | 19 | import { Link, BrowserRouter as Router } from 'react-router-dom' |
20 | import { Provider } from 'react-redux' | 20 | import { Provider } from 'react-redux' |
21 | 21 | ||
22 | import { fetchInfoAction } from './actions/info' | 22 | import { fetchInfoIfNeeded } from './actions/info' |
23 | import createZuulStore from './store' | 23 | import createZuulStore from './store' |
24 | import App from './App' | 24 | import App from './App' |
25 | import TenantsPage from './pages/Tenants' | 25 | import TenantsPage from './pages/Tenants' |
@@ -54,7 +54,7 @@ it('renders multi tenant', () => { | |||
54 | const application = ReactTestUtils.renderIntoDocument( | 54 | const application = ReactTestUtils.renderIntoDocument( |
55 | <Provider store={store}><Router><App /></Router></Provider> | 55 | <Provider store={store}><Router><App /></Router></Provider> |
56 | ) | 56 | ) |
57 | store.dispatch(fetchInfoAction()).then(() => { | 57 | store.dispatch(fetchInfoIfNeeded()).then(() => { |
58 | // Link should be tenant scoped | 58 | // Link should be tenant scoped |
59 | const topMenuLinks = ReactTestUtils.scryRenderedComponentsWithType( | 59 | const topMenuLinks = ReactTestUtils.scryRenderedComponentsWithType( |
60 | application, Link) | 60 | application, Link) |
@@ -86,7 +86,7 @@ it('renders single tenant', () => { | |||
86 | <Provider store={store}><Router><App /></Router></Provider> | 86 | <Provider store={store}><Router><App /></Router></Provider> |
87 | ) | 87 | ) |
88 | 88 | ||
89 | store.dispatch(fetchInfoAction()).then(() => { | 89 | store.dispatch(fetchInfoIfNeeded()).then(() => { |
90 | // Link should be white-label scoped | 90 | // Link should be white-label scoped |
91 | const topMenuLinks = ReactTestUtils.scryRenderedComponentsWithType( | 91 | const topMenuLinks = ReactTestUtils.scryRenderedComponentsWithType( |
92 | application, Link) | 92 | application, Link) |
diff --git a/web/src/actions/info.js b/web/src/actions/info.js index 8894eef..81e9b4a 100644 --- a/web/src/actions/info.js +++ b/web/src/actions/info.js | |||
@@ -12,16 +12,46 @@ | |||
12 | // License for the specific language governing permissions and limitations | 12 | // License for the specific language governing permissions and limitations |
13 | // under the License. | 13 | // under the License. |
14 | 14 | ||
15 | import { fetchInfo } from '../api' | 15 | import * as API from '../api' |
16 | 16 | ||
17 | export function fetchInfoAction () { | 17 | export const INFO_FETCH_REQUEST = 'INFO_FETCH_REQUEST' |
18 | return (dispatch) => { | 18 | export const INFO_FETCH_SUCCESS = 'INFO_FETCH_SUCCESS' |
19 | return fetchInfo() | 19 | export const INFO_FETCH_FAIL = 'INFO_FETCH_FAIL' |
20 | .then(response => { | 20 | |
21 | dispatch({type: 'FETCH_INFO_SUCCESS', info: response.data.info}) | 21 | export const fetchInfoRequest = () => ({ |
22 | }) | 22 | type: INFO_FETCH_REQUEST |
23 | .catch(error => { | 23 | }) |
24 | throw (error) | 24 | |
25 | }) | 25 | export const fetchInfoSuccess = json => ({ |
26 | type: INFO_FETCH_SUCCESS, | ||
27 | tenant: json.info.tenant, | ||
28 | }) | ||
29 | |||
30 | const fetchInfoFail = error => ({ | ||
31 | type: INFO_FETCH_FAIL, | ||
32 | error | ||
33 | }) | ||
34 | |||
35 | const fetchInfo = () => dispatch => { | ||
36 | dispatch(fetchInfoRequest()) | ||
37 | return API.fetchInfo() | ||
38 | .then(response => dispatch(fetchInfoSuccess(response.data))) | ||
39 | .catch(error => dispatch(fetchInfoFail(error))) | ||
40 | } | ||
41 | |||
42 | const shouldFetchInfo = state => { | ||
43 | const info = state.info | ||
44 | if (!info) { | ||
45 | return true | ||
46 | } | ||
47 | if (info.isFetching) { | ||
48 | return false | ||
49 | } | ||
50 | return true | ||
51 | } | ||
52 | |||
53 | export const fetchInfoIfNeeded = () => (dispatch, getState) => { | ||
54 | if (shouldFetchInfo(getState())) { | ||
55 | return dispatch(fetchInfo()) | ||
26 | } | 56 | } |
27 | } | 57 | } |
diff --git a/web/src/actions/tenant.js b/web/src/actions/tenant.js index 992f4e9..e40c8c0 100644 --- a/web/src/actions/tenant.js +++ b/web/src/actions/tenant.js | |||
@@ -12,6 +12,8 @@ | |||
12 | // License for the specific language governing permissions and limitations | 12 | // License for the specific language governing permissions and limitations |
13 | // under the License. | 13 | // under the License. |
14 | 14 | ||
15 | export const TENANT_SET = 'TENANT_SET' | ||
16 | |||
15 | export function setTenantAction (name, whiteLabel) { | 17 | export function setTenantAction (name, whiteLabel) { |
16 | let apiPrefix = '' | 18 | let apiPrefix = '' |
17 | let linkPrefix = '' | 19 | let linkPrefix = '' |
@@ -24,7 +26,7 @@ export function setTenantAction (name, whiteLabel) { | |||
24 | defaultRoute = '/tenants' | 26 | defaultRoute = '/tenants' |
25 | } | 27 | } |
26 | return { | 28 | return { |
27 | type: 'SET_TENANT', | 29 | type: TENANT_SET, |
28 | tenant: { | 30 | tenant: { |
29 | name: name, | 31 | name: name, |
30 | whiteLabel: whiteLabel, | 32 | whiteLabel: whiteLabel, |
diff --git a/web/src/index.js b/web/src/index.js index 933186e..07ad3ce 100644 --- a/web/src/index.js +++ b/web/src/index.js | |||
@@ -25,14 +25,14 @@ import './index.css' | |||
25 | 25 | ||
26 | import { getHomepageUrl } from './api' | 26 | import { getHomepageUrl } from './api' |
27 | import registerServiceWorker from './registerServiceWorker' | 27 | import registerServiceWorker from './registerServiceWorker' |
28 | import { fetchInfoAction } from './actions/info' | 28 | import { fetchInfoIfNeeded } from './actions/info' |
29 | import createZuulStore from './store' | 29 | import createZuulStore from './store' |
30 | import App from './App' | 30 | import App from './App' |
31 | 31 | ||
32 | const store = createZuulStore() | 32 | const store = createZuulStore() |
33 | 33 | ||
34 | // Load info endpoint | 34 | // Load info endpoint |
35 | store.dispatch(fetchInfoAction()) | 35 | store.dispatch(fetchInfoIfNeeded()) |
36 | 36 | ||
37 | ReactDOM.render( | 37 | ReactDOM.render( |
38 | <Provider store={store}> | 38 | <Provider store={store}> |
diff --git a/web/src/reducers/info.js b/web/src/reducers/info.js index 5386622..89cadf2 100644 --- a/web/src/reducers/info.js +++ b/web/src/reducers/info.js | |||
@@ -12,10 +12,29 @@ | |||
12 | // License for the specific language governing permissions and limitations | 12 | // License for the specific language governing permissions and limitations |
13 | // under the License. | 13 | // under the License. |
14 | 14 | ||
15 | export default (state = {}, action) => { | 15 | import { |
16 | INFO_FETCH_REQUEST, | ||
17 | INFO_FETCH_SUCCESS, | ||
18 | INFO_FETCH_FAIL, | ||
19 | } from '../actions/info' | ||
20 | |||
21 | export default (state = { | ||
22 | isFetching: false, | ||
23 | tenant: null, | ||
24 | }, action) => { | ||
16 | switch (action.type) { | 25 | switch (action.type) { |
17 | case 'FETCH_INFO_SUCCESS': | 26 | case INFO_FETCH_REQUEST: |
18 | return action.info | 27 | case INFO_FETCH_FAIL: |
28 | return { | ||
29 | isFetching: true, | ||
30 | tenant: null, | ||
31 | } | ||
32 | case INFO_FETCH_SUCCESS: | ||
33 | return { | ||
34 | isFetching: false, | ||
35 | tenant: action.tenant, | ||
36 | ready: true | ||
37 | } | ||
19 | default: | 38 | default: |
20 | return state | 39 | return state |
21 | } | 40 | } |
diff --git a/web/src/reducers/tenant.js b/web/src/reducers/tenant.js index b634b03..5911c83 100644 --- a/web/src/reducers/tenant.js +++ b/web/src/reducers/tenant.js | |||
@@ -12,9 +12,11 @@ | |||
12 | // License for the specific language governing permissions and limitations | 12 | // License for the specific language governing permissions and limitations |
13 | // under the License. | 13 | // under the License. |
14 | 14 | ||
15 | export default (state = {}, action) => { | 15 | import { TENANT_SET } from '../actions/tenant' |
16 | |||
17 | export default (state = {name: null}, action) => { | ||
16 | switch (action.type) { | 18 | switch (action.type) { |
17 | case 'SET_TENANT': | 19 | case TENANT_SET: |
18 | return action.tenant | 20 | return action.tenant |
19 | default: | 21 | default: |
20 | return state | 22 | return state |