summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTristan Cacqueray <tdecacqu@redhat.com>2018-12-02 01:46:43 +0000
committerTristan Cacqueray <tdecacqu@redhat.com>2018-12-02 05:43:25 +0000
commit17144c2a4637bd5334f3ba3b3f3f10424b345105 (patch)
treed8746690f8d61415765cbd043e423037ddabd03a
parent76867ca14aec3e7e6df8604e911826049b57946f (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.jsx21
-rw-r--r--web/src/App.test.jsx6
-rw-r--r--web/src/actions/info.js52
-rw-r--r--web/src/actions/tenant.js4
-rw-r--r--web/src/index.js4
-rw-r--r--web/src/reducers/info.js25
-rw-r--r--web/src/reducers/tenant.js6
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'
19import { Link, BrowserRouter as Router } from 'react-router-dom' 19import { Link, BrowserRouter as Router } from 'react-router-dom'
20import { Provider } from 'react-redux' 20import { Provider } from 'react-redux'
21 21
22import { fetchInfoAction } from './actions/info' 22import { fetchInfoIfNeeded } from './actions/info'
23import createZuulStore from './store' 23import createZuulStore from './store'
24import App from './App' 24import App from './App'
25import TenantsPage from './pages/Tenants' 25import 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
15import { fetchInfo } from '../api' 15import * as API from '../api'
16 16
17export function fetchInfoAction () { 17export const INFO_FETCH_REQUEST = 'INFO_FETCH_REQUEST'
18 return (dispatch) => { 18export const INFO_FETCH_SUCCESS = 'INFO_FETCH_SUCCESS'
19 return fetchInfo() 19export const INFO_FETCH_FAIL = 'INFO_FETCH_FAIL'
20 .then(response => { 20
21 dispatch({type: 'FETCH_INFO_SUCCESS', info: response.data.info}) 21export const fetchInfoRequest = () => ({
22 }) 22 type: INFO_FETCH_REQUEST
23 .catch(error => { 23})
24 throw (error) 24
25 }) 25export const fetchInfoSuccess = json => ({
26 type: INFO_FETCH_SUCCESS,
27 tenant: json.info.tenant,
28})
29
30const fetchInfoFail = error => ({
31 type: INFO_FETCH_FAIL,
32 error
33})
34
35const fetchInfo = () => dispatch => {
36 dispatch(fetchInfoRequest())
37 return API.fetchInfo()
38 .then(response => dispatch(fetchInfoSuccess(response.data)))
39 .catch(error => dispatch(fetchInfoFail(error)))
40}
41
42const 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
53export 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
15export const TENANT_SET = 'TENANT_SET'
16
15export function setTenantAction (name, whiteLabel) { 17export 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
26import { getHomepageUrl } from './api' 26import { getHomepageUrl } from './api'
27import registerServiceWorker from './registerServiceWorker' 27import registerServiceWorker from './registerServiceWorker'
28import { fetchInfoAction } from './actions/info' 28import { fetchInfoIfNeeded } from './actions/info'
29import createZuulStore from './store' 29import createZuulStore from './store'
30import App from './App' 30import App from './App'
31 31
32const store = createZuulStore() 32const store = createZuulStore()
33 33
34// Load info endpoint 34// Load info endpoint
35store.dispatch(fetchInfoAction()) 35store.dispatch(fetchInfoIfNeeded())
36 36
37ReactDOM.render( 37ReactDOM.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
15export default (state = {}, action) => { 15import {
16 INFO_FETCH_REQUEST,
17 INFO_FETCH_SUCCESS,
18 INFO_FETCH_FAIL,
19} from '../actions/info'
20
21export 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
15export default (state = {}, action) => { 15import { TENANT_SET } from '../actions/tenant'
16
17export 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