From f4b774957f6ed803007485134a2795bf73c0d19b Mon Sep 17 00:00:00 2001 From: Tristan Cacqueray Date: Fri, 14 Dec 2018 05:07:09 +0000 Subject: [PATCH] web: refactor job page to use a reducer This change updates the project page component to dispatch reducer action instead of direct axios call. This enables using the generic error reducers as well as keeping the tenant projects in the store to avoid repeated query. Change-Id: Ib91e0f338fd87c57534a90d7168f0dab98f2df62 --- web/src/actions/project.js | 99 +++++++++++++++++++++++++++++++++++++ web/src/pages/Project.jsx | 86 +++++++++----------------------- web/src/reducers/index.js | 2 + web/src/reducers/project.js | 55 +++++++++++++++++++++ 4 files changed, 180 insertions(+), 62 deletions(-) create mode 100644 web/src/actions/project.js create mode 100644 web/src/reducers/project.js diff --git a/web/src/actions/project.js b/web/src/actions/project.js new file mode 100644 index 0000000000..f2c64adb61 --- /dev/null +++ b/web/src/actions/project.js @@ -0,0 +1,99 @@ +/* global Promise */ +// Copyright 2018 Red Hat, Inc +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. You may obtain +// a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +// License for the specific language governing permissions and limitations +// under the License. + +import * as API from '../api' + +export const PROJECT_FETCH_REQUEST = 'PROJECT_FETCH_REQUEST' +export const PROJECT_FETCH_SUCCESS = 'PROJECT_FETCH_SUCCESS' +export const PROJECT_FETCH_FAIL = 'PROJECT_FETCH_FAIL' + +export const requestProject = () => ({ + type: PROJECT_FETCH_REQUEST +}) + +export const receiveProject = (tenant, projectName, project) => { + // TODO: fix api to return template name or merge them + // in the mean-time, merge the jobs in project configs + const templateIdx = [] + let idx + project.configs.forEach((config, idx) => { + if (config.default_branch === null) { + // This must be a template + templateIdx.push(idx) + config.pipelines.forEach(templatePipeline => { + let pipeline = project.configs[idx - 1].pipelines.filter( + item => item.name === templatePipeline.name) + if (pipeline.length === 0) { + // Pipeline doesn't exist in project config + project.configs[idx - 1].pipelines.push(templatePipeline) + } else { + if (pipeline[0].queue_name === null) { + pipeline[0].queue_name = templatePipeline.queue_name + } + templatePipeline.jobs.forEach(job => { + pipeline[0].jobs.push(job) + }) + } + }) + } + }) + for (idx = templateIdx.length - 1; idx >= 0; idx -= 1) { + project.configs.splice(templateIdx[idx], 1) + } + + return { + type: PROJECT_FETCH_SUCCESS, + tenant: tenant, + projectName: projectName, + project: project, + receivedAt: Date.now(), + } +} + +const failedProject = error => ({ + type: PROJECT_FETCH_FAIL, + error +}) + +const fetchProject = (tenant, project) => dispatch => { + dispatch(requestProject()) + return API.fetchProject(tenant.apiPrefix, project) + .then(response => dispatch(receiveProject( + tenant.name, project, response.data))) + .catch(error => dispatch(failedProject(error))) +} + +const shouldFetchProject = (tenant, projectName, state) => { + const tenantProjects = state.project.projects[tenant.name] + if (tenantProjects) { + const project = tenantProjects[projectName] + if (!project) { + return true + } + if (project.isFetching) { + return false + } + return false + } + return true +} + +export const fetchProjectIfNeeded = (tenant, project, force) => ( + dispatch, getState) => { + if (force || shouldFetchProject(tenant, project, getState())) { + return dispatch(fetchProject(tenant, project)) + } + return Promise.resolve() +} diff --git a/web/src/pages/Project.jsx b/web/src/pages/Project.jsx index de0659bd65..76c9ec06cd 100644 --- a/web/src/pages/Project.jsx +++ b/web/src/pages/Project.jsx @@ -17,83 +17,45 @@ import { connect } from 'react-redux' import PropTypes from 'prop-types' import Project from '../containers/project/Project' -import { fetchProject } from '../api' +import { fetchProjectIfNeeded } from '../actions/project' +import Refreshable from '../containers/Refreshable' -class ProjectPage extends React.Component { +class ProjectPage extends Refreshable { static propTypes = { match: PropTypes.object.isRequired, - tenant: PropTypes.object + tenant: PropTypes.object, + remoteData: PropTypes.object, + dispatch: PropTypes.func } - state = { - project: null - } - - fixProjectConfig(project) { - let templateIdx = [] - let idx - project.configs.forEach((config, idx) => { - if (config.default_branch === null) { - // This must be a template - templateIdx.push(idx) - config.pipelines.forEach(templatePipeline => { - let pipeline = project.configs[idx - 1].pipelines.filter( - item => item.name === templatePipeline.name) - if (pipeline.length === 0) { - // Pipeline doesn't exist in project config - project.configs[idx - 1].pipelines.push(templatePipeline) - } else { - if (pipeline[0].queue_name === null) { - pipeline[0].queue_name = templatePipeline.queue_name - } - templatePipeline.jobs.forEach(job => { - pipeline[0].jobs.push(job) - }) - } - }) - } - }) - for (idx = templateIdx.length - 1; idx >= 0; idx -= 1) { - project.configs.splice(templateIdx[idx], 1) - } - } - - updateData = () => { - fetchProject( - this.props.tenant.apiPrefix, this.props.match.params.projectName) - .then(response => { - // TODO: fix api to return template name or merge them - // in the mean-time, merge the jobs in project configs - this.fixProjectConfig(response.data) - this.setState({project: response.data}) - }) + updateData = (force) => { + this.props.dispatch(fetchProjectIfNeeded( + this.props.tenant, this.props.match.params.projectName, force)) } componentDidMount () { document.title = 'Zuul Project | ' + this.props.match.params.projectName - if (this.props.tenant.name) { - this.updateData() - } - } - - componentDidUpdate (prevProps) { - if (this.props.tenant.name !== prevProps.tenant.name || - this.props.match.params.projectName !== - prevProps.match.params.projectName) { - this.updateData() - } + super.componentDidMount() } render () { - const { project } = this.state - if (!project) { - return (

Loading...

) - } + const { remoteData } = this.props + const tenantProjects = remoteData.projects[this.props.tenant.name] + const projectName = this.props.match.params.projectName return ( - + +
+ {this.renderSpinner()} +
+ {tenantProjects && tenantProjects[projectName] && + } + ) } } -export default connect(state => ({tenant: state.tenant}))(ProjectPage) +export default connect(state => ({ + tenant: state.tenant, + remoteData: state.project, +}))(ProjectPage) diff --git a/web/src/reducers/index.js b/web/src/reducers/index.js index 4089f27902..69b24d6721 100644 --- a/web/src/reducers/index.js +++ b/web/src/reducers/index.js @@ -20,6 +20,7 @@ import errors from './errors' import info from './info' import job from './job' import jobs from './jobs' +import project from './project' import projects from './projects' import status from './status' import tenant from './tenant' @@ -30,6 +31,7 @@ const reducers = { info, job, jobs, + project, projects, configErrors, errors, diff --git a/web/src/reducers/project.js b/web/src/reducers/project.js new file mode 100644 index 0000000000..d148f25898 --- /dev/null +++ b/web/src/reducers/project.js @@ -0,0 +1,55 @@ +// Copyright 2018 Red Hat, Inc +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. You may obtain +// a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +// License for the specific language governing permissions and limitations +// under the License. + +import { + PROJECT_FETCH_FAIL, + PROJECT_FETCH_REQUEST, + PROJECT_FETCH_SUCCESS +} from '../actions/project' + +import update from 'immutability-helper' + +export default (state = { + isFetching: false, + projects: {}, +}, action) => { + switch (action.type) { + case PROJECT_FETCH_REQUEST: + return { + isFetching: true, + projects: state.projects, + } + case PROJECT_FETCH_SUCCESS: + if (!state.projects[action.tenant]) { + state.projects = update(state.projects, {$merge: {[action.tenant]: {}}}) + } + return { + isFetching: false, + projects: update(state.projects, { + [action.tenant]: { + $merge: { + [action.projectName]: action.project + } + } + }) + } + case PROJECT_FETCH_FAIL: + return { + isFetching: false, + projects: state.projects, + } + default: + return state + } +}