From ee051c967ae19afbbd27ddfaa34792ee227df9df Mon Sep 17 00:00:00 2001 From: Calvin Pomerantz Date: Fri, 29 Jul 2016 09:44:59 -0400 Subject: [PATCH 1/7] Implement handling 404 properly and demo in DeployDetail page --- .../components/deployDetail/DeployDetail.jsx | 12 ++++-------- SingularityUI/app/rootComponent.jsx | 17 +++++++++++++++-- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/SingularityUI/app/components/deployDetail/DeployDetail.jsx b/SingularityUI/app/components/deployDetail/DeployDetail.jsx index 9f95ba3503..5078382b79 100644 --- a/SingularityUI/app/components/deployDetail/DeployDetail.jsx +++ b/SingularityUI/app/components/deployDetail/DeployDetail.jsx @@ -21,7 +21,6 @@ import JSONButton from '../common/JSONButton'; import SimpleTable from '../common/SimpleTable'; import ServerSideTable from '../common/ServerSideTable'; import CollapsableSection from '../common/CollapsableSection'; -import NotFound from '../common/NotFound'; import ActiveTasksTable from './ActiveTasksTable'; @@ -36,8 +35,7 @@ class DeployDetail extends React.Component { taskHistory: PropTypes.array, latestHealthchecks: PropTypes.array, fetchTaskHistoryForDeploy: PropTypes.func, - params: PropTypes.object, - notFound: PropTypes.bool + params: PropTypes.object } componentDidMount() { @@ -233,10 +231,7 @@ class DeployDetail extends React.Component { } render() { - const { notFound, deploy, activeTasks, taskHistory, latestHealthchecks } = this.props; - if (notFound) { - return ; - } + const { deploy, activeTasks, taskHistory, latestHealthchecks } = this.props; return (
{this.renderHeader(deploy)} @@ -259,7 +254,7 @@ function mapDispatchToProps(dispatch) { }; } -function mapStateToProps(state) { +function mapStateToProps(state, ownProps) { let latestHealthchecks = _.mapObject(state.api.task, (val) => { if (val.data && val.data.healthcheckResults && val.data.healthcheckResults.length > 0) { return _.max(val.data.healthcheckResults, (hc) => { @@ -272,6 +267,7 @@ function mapStateToProps(state) { return { notFound: state.api.deploy.statusCode === 404, + pathname: ownProps.location.pathname, deploy: state.api.deploy.data, taskHistory: state.api.taskHistoryForDeploy.data, latestHealthchecks diff --git a/SingularityUI/app/rootComponent.jsx b/SingularityUI/app/rootComponent.jsx index 4d0db32995..ef1d7b15f3 100644 --- a/SingularityUI/app/rootComponent.jsx +++ b/SingularityUI/app/rootComponent.jsx @@ -1,7 +1,13 @@ -import React from 'react'; +import React, { PropTypes, Component } from 'react'; import classNames from 'classnames'; +import NotFound from 'components/common/NotFound'; -const rootComponent = (Wrapped, title, refresh = _.noop, refreshInterval = true, pageMargin = true) => class extends React.Component { +const rootComponent = (Wrapped, title, refresh = _.noop, refreshInterval = true, pageMargin = true) => class extends Component { + + static propTypes = { + notFound: PropTypes.bool, + pathname: PropTypes.string + } constructor(props) { super(props); @@ -69,6 +75,13 @@ const rootComponent = (Wrapped, title, refresh = _.noop, refreshInterval = true, } render() { + if (this.props.notFound) { + return ( +
+ +
+ ); + } const loader = this.state.loading &&
; const page = !this.state.loading && ; return ( From fbe60ff994b431e25460236cad177bd1c732c8d2 Mon Sep 17 00:00:00 2001 From: Calvin Pomerantz Date: Fri, 29 Jul 2016 10:04:58 -0400 Subject: [PATCH 2/7] Add 404 detecting capabilities to RequestDetailPage --- SingularityUI/app/actions/api/requests.es6 | 5 +++-- .../components/requestDetail/RequestDetailPage.jsx | 14 ++++++++++++-- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/SingularityUI/app/actions/api/requests.es6 b/SingularityUI/app/actions/api/requests.es6 index f464bf5f05..0915c37677 100644 --- a/SingularityUI/app/actions/api/requests.es6 +++ b/SingularityUI/app/actions/api/requests.es6 @@ -19,8 +19,9 @@ export const FetchRequestsInState = buildApiAction( export const FetchRequest = buildApiAction( 'FETCH_REQUEST', - (requestId) => ({ - url: `/requests/request/${requestId}` + (requestId, isMainApiCall) => ({ + url: `/requests/request/${requestId}`, + isMainApiCall }), (requestId) => requestId ); diff --git a/SingularityUI/app/components/requestDetail/RequestDetailPage.jsx b/SingularityUI/app/components/requestDetail/RequestDetailPage.jsx index 021eca7d86..d867da8049 100644 --- a/SingularityUI/app/components/requestDetail/RequestDetailPage.jsx +++ b/SingularityUI/app/components/requestDetail/RequestDetailPage.jsx @@ -24,6 +24,8 @@ import TaskHistoryTable from './TaskHistoryTable'; import DeployHistoryTable from './DeployHistoryTable'; import RequestHistoryTable from './RequestHistoryTable'; +import Utils from '../../utils'; + function refresh(props) { props.fetchRequest(props.params.requestId); props.fetchActiveTasksForRequest(props.params.requestId); @@ -71,6 +73,14 @@ RequestDetailPage.propTypes = { cancelRefresh: PropTypes.func.isRequired }; +const mapStateToProps = (state, ownProps) => { + const statusCode = Utils.maybe(state, ['api', 'request', ownProps.params.requestId, 'statusCode']); + return { + notFound: statusCode === 404, + pathname: ownProps.location.pathname + }; +}; + const mapDispatchToProps = (dispatch, ownProps) => { const refreshActions = [ FetchRequest.trigger(ownProps.params.requestId), @@ -87,7 +97,7 @@ const mapDispatchToProps = (dispatch, ownProps) => { cancelRefresh: () => dispatch( RefreshActions.CancelAutoRefresh('RequestDetailPage') ), - fetchRequest: (requestId) => dispatch(FetchRequest.trigger(requestId)), + fetchRequest: (requestId) => dispatch(FetchRequest.trigger(requestId, true)), fetchActiveTasksForRequest: (requestId) => dispatch(FetchActiveTasksForRequest.trigger(requestId)), fetchScheduledTasksForRequest: (requestId) => dispatch(FetchScheduledTasksForRequest.trigger(requestId)), fetchTaskCleanups: () => dispatch(FetchTaskCleanups.trigger()), @@ -98,6 +108,6 @@ const mapDispatchToProps = (dispatch, ownProps) => { }; export default connect( - null, + mapStateToProps, mapDispatchToProps )(rootComponent(RequestDetailPage, (props) => props.params.requestId, refresh, false)); From e8664ef24b7a28e438b7f001a42abd025be73e6c Mon Sep 17 00:00:00 2001 From: Calvin Pomerantz Date: Fri, 29 Jul 2016 10:08:54 -0400 Subject: [PATCH 3/7] Handle 404 properly on new request form --- SingularityUI/app/components/requestForm/RequestForm.jsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/SingularityUI/app/components/requestForm/RequestForm.jsx b/SingularityUI/app/components/requestForm/RequestForm.jsx index ca3b3c345d..396cc36a0c 100644 --- a/SingularityUI/app/components/requestForm/RequestForm.jsx +++ b/SingularityUI/app/components/requestForm/RequestForm.jsx @@ -502,6 +502,8 @@ class RequestForm extends React.Component { function mapStateToProps(state, ownProps) { const request = ownProps.params.requestId && state.api.request[ownProps.params.requestId]; return { + notFound: request && request.statusCode === 404, + pathname: ownProps.location.pathname, racks: state.api.racks.data, request: request && request.data, form: state.ui.form[FORM_ID], @@ -525,7 +527,7 @@ function mapDispatchToProps(dispatch, ownProps) { }); }, fetchRequest(requestId) { - dispatch(FetchRequest.trigger(requestId)); + dispatch(FetchRequest.trigger(requestId, true)); }, fetchRacks() { dispatch(FetchRacks.trigger()); From 5ac89c76f9d05fa8826b3711d9738e2b1d8a3857 Mon Sep 17 00:00:00 2001 From: Calvin Pomerantz Date: Fri, 29 Jul 2016 10:24:02 -0400 Subject: [PATCH 4/7] Handle 404s in New Deploy Form --- SingularityUI/app/components/newDeployForm/NewDeployForm.jsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/SingularityUI/app/components/newDeployForm/NewDeployForm.jsx b/SingularityUI/app/components/newDeployForm/NewDeployForm.jsx index 3485bbdfc5..17eac5d5c6 100644 --- a/SingularityUI/app/components/newDeployForm/NewDeployForm.jsx +++ b/SingularityUI/app/components/newDeployForm/NewDeployForm.jsx @@ -1446,6 +1446,8 @@ class NewDeployForm extends Component { function mapStateToProps(state, ownProps) { return { request: Utils.maybe(state.api.request, [ownProps.params.requestId, 'data']), + notFound: Utils.maybe(state.api.request, [ownProps.params.requestId, 'statusCode']) === 404, + pathname: ownProps.location.pathname, form: state.ui.form[FORM_ID], saveApiCall: state.api.saveDeploy }; @@ -1464,7 +1466,7 @@ function mapDispatchToProps(dispatch, ownProps) { }); }, fetchRequest(requestId) { - return dispatch(FetchRequest.trigger(requestId)); + return dispatch(FetchRequest.trigger(requestId, true)); }, clearForm() { return dispatch(ClearForm('newDeployForm')); From e765af092a7d9f48a17c2991ad87959a49d0ac05 Mon Sep 17 00:00:00 2001 From: Calvin Pomerantz Date: Fri, 29 Jul 2016 10:59:11 -0400 Subject: [PATCH 5/7] Handle 404 properly in TaskDetail page --- SingularityUI/app/actions/api/history.es6 | 5 +++-- .../app/components/taskDetail/TaskDetail.jsx | 17 +++++++++++++---- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/SingularityUI/app/actions/api/history.es6 b/SingularityUI/app/actions/api/history.es6 index 841de9d724..6269e3f31c 100644 --- a/SingularityUI/app/actions/api/history.es6 +++ b/SingularityUI/app/actions/api/history.es6 @@ -3,8 +3,9 @@ import Utils from '../../utils'; export const FetchTaskHistory = buildApiAction( 'FETCH_TASK_HISTORY', - (taskId) => ({ - url: `/history/task/${taskId}` + (taskId, isMainApiCall) => ({ + url: `/history/task/${taskId}`, + isMainApiCall }), (taskId) => taskId ); diff --git a/SingularityUI/app/components/taskDetail/TaskDetail.jsx b/SingularityUI/app/components/taskDetail/TaskDetail.jsx index a2138cec08..9d82c11503 100644 --- a/SingularityUI/app/components/taskDetail/TaskDetail.jsx +++ b/SingularityUI/app/components/taskDetail/TaskDetail.jsx @@ -442,8 +442,15 @@ function mapTaskToProps(task) { } function mapStateToProps(state, ownProps) { - let task = state.api.task[ownProps.params.taskId]; + const apiCallData = state.api.task[ownProps.params.taskId]; + let task = apiCallData; if (!(task && task.data)) return {}; + if (apiCallData.statusCode === 404) { + return { + notFound: true, + pathname: ownProps.location.pathname + }; + } task = mapTaskToProps(task.data); task = mapHealthchecksToProps(task); return { @@ -464,7 +471,7 @@ function mapDispatchToProps(dispatch) { return { runCommandOnTask: (taskId, commandName) => dispatch(RunCommandOnTask.trigger(taskId, commandName)), killTask: (taskId, data) => dispatch(KillTask.trigger(taskId, data)), - fetchTaskHistory: (taskId) => dispatch(FetchTaskHistory.trigger(taskId)), + fetchTaskHistory: (taskId) => dispatch(FetchTaskHistory.trigger(taskId, true)), fetchTaskStatistics: (taskId) => dispatch(FetchTaskStatistics.trigger(taskId)), fetchTaskFiles: (...args) => dispatch(FetchTaskFiles.trigger(...args)), fetchDeployForRequest: (taskId, deployId) => dispatch(FetchDeployForRequest.trigger(taskId, deployId)), @@ -475,11 +482,13 @@ function mapDispatchToProps(dispatch) { } function refresh(props) { - props.fetchTaskFiles(props.params.taskId, props.params.splat || props.params.taskId, [400]); + props.fetchTaskFiles(props.params.taskId, props.params.splat || props.params.taskId, [400, 404]); const promises = []; const taskPromise = props.fetchTaskHistory(props.params.taskId); taskPromise.then(() => { - const task = props.route.store.getState().api.task[props.params.taskId].data; + const apiData = props.route.store.getState().api.task[props.params.taskId]; + if (apiData.statusCode === 404) return; + const task = apiData.task; promises.push(props.fetchDeployForRequest(task.task.taskId.requestId, task.task.taskId.deployId)); if (task.isStillRunning) { promises.push(props.fetchTaskStatistics(props.params.taskId)); From e090db09d032e639a97fe03cfa15fed0706ea344 Mon Sep 17 00:00:00 2001 From: Calvin Pomerantz Date: Fri, 29 Jul 2016 11:00:28 -0400 Subject: [PATCH 6/7] Bug fix --- SingularityUI/app/components/taskDetail/TaskDetail.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SingularityUI/app/components/taskDetail/TaskDetail.jsx b/SingularityUI/app/components/taskDetail/TaskDetail.jsx index 9d82c11503..fb0cbe8adc 100644 --- a/SingularityUI/app/components/taskDetail/TaskDetail.jsx +++ b/SingularityUI/app/components/taskDetail/TaskDetail.jsx @@ -488,7 +488,7 @@ function refresh(props) { taskPromise.then(() => { const apiData = props.route.store.getState().api.task[props.params.taskId]; if (apiData.statusCode === 404) return; - const task = apiData.task; + const task = apiData.data; promises.push(props.fetchDeployForRequest(task.task.taskId.requestId, task.task.taskId.deployId)); if (task.isStillRunning) { promises.push(props.fetchTaskStatistics(props.params.taskId)); From 7c74846ad7a0b72e0e46dfe61451262e63761fec Mon Sep 17 00:00:00 2001 From: Calvin Pomerantz Date: Mon, 1 Aug 2016 11:12:03 -0400 Subject: [PATCH 7/7] Rename isMainApiCall to renderNotFoundIf404 --- SingularityUI/app/actions/api/base.es6 | 2 +- SingularityUI/app/actions/api/history.es6 | 8 ++++---- SingularityUI/app/actions/api/requests.es6 | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/SingularityUI/app/actions/api/base.es6 b/SingularityUI/app/actions/api/base.es6 index 223f6838a6..a6ee76a8a7 100644 --- a/SingularityUI/app/actions/api/base.es6 +++ b/SingularityUI/app/actions/api/base.es6 @@ -29,7 +29,7 @@ export function buildApiAction(actionName, opts = {}, keyFunc = undefined) { function error(err, options, apiResponse, key = undefined) { const action = { type: ERROR, error: err, key, statusCode: apiResponse.status }; - if (Utils.isIn(apiResponse.status, options.catchStatusCodes) || apiResponse.status === 404 && options.isMainApiCall) { + if (Utils.isIn(apiResponse.status, options.catchStatusCodes) || apiResponse.status === 404 && options.renderNotFoundIf404) { return action; } if (apiResponse.status === 502) { // Singularity is deploying diff --git a/SingularityUI/app/actions/api/history.es6 b/SingularityUI/app/actions/api/history.es6 index 6269e3f31c..4b3aa8c2df 100644 --- a/SingularityUI/app/actions/api/history.es6 +++ b/SingularityUI/app/actions/api/history.es6 @@ -3,9 +3,9 @@ import Utils from '../../utils'; export const FetchTaskHistory = buildApiAction( 'FETCH_TASK_HISTORY', - (taskId, isMainApiCall) => ({ + (taskId, renderNotFoundIf404) => ({ url: `/history/task/${taskId}`, - isMainApiCall + renderNotFoundIf404 }), (taskId) => taskId ); @@ -42,9 +42,9 @@ export const FetchTaskHistoryForDeploy = buildApiAction( export const FetchDeployForRequest = buildApiAction( 'FETCH_DEPLOY', - (requestId, deployId, isMainApiCall) => ({ + (requestId, deployId, renderNotFoundIf404) => ({ url: `/history/request/${requestId}/deploy/${deployId}`, - isMainApiCall + renderNotFoundIf404 }) ); diff --git a/SingularityUI/app/actions/api/requests.es6 b/SingularityUI/app/actions/api/requests.es6 index 0915c37677..f9fc99854a 100644 --- a/SingularityUI/app/actions/api/requests.es6 +++ b/SingularityUI/app/actions/api/requests.es6 @@ -19,9 +19,9 @@ export const FetchRequestsInState = buildApiAction( export const FetchRequest = buildApiAction( 'FETCH_REQUEST', - (requestId, isMainApiCall) => ({ + (requestId, renderNotFoundIf404) => ({ url: `/requests/request/${requestId}`, - isMainApiCall + renderNotFoundIf404 }), (requestId) => requestId );