-
Notifications
You must be signed in to change notification settings - Fork 76
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Workflow default dialog alert #4100
Changes from 7 commits
d2f9a3e
79a8bee
b1f82a1
335c892
67ddefd
d420426
d436d4f
3643627
e32fbd0
c334028
52c8723
c1d030c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import React, { Component } from 'react'; | ||
import apiClient from 'panoptes-client/lib/api-client'; | ||
import Dialog from 'modal-form/dialog'; | ||
|
||
import ProjectIcon from '../../components/project-icon'; | ||
import getWorkflowsInOrder from '../../lib/get-workflows-in-order'; | ||
|
@@ -10,6 +11,7 @@ import VersionList from './project-status/version-list'; | |
import ExperimentalFeatures from './project-status/experimental-features'; | ||
import Toggle from './project-status/toggle'; | ||
import RedirectToggle from './project-status/redirect-toggle'; | ||
import WorkflowDefaultDialog from './workflow-default-dialog' | ||
|
||
class ProjectStatus extends Component { | ||
constructor(props) { | ||
|
@@ -20,6 +22,7 @@ class ProjectStatus extends Component { | |
this.renderError = this.renderError.bind(this); | ||
this.renderWorkflows = this.renderWorkflows.bind(this); | ||
this.handleToggle = this.handleToggle.bind(this); | ||
this.getProjectAndWorkflows = this.getProjectAndWorkflows.bind(this); | ||
|
||
this.state = { | ||
project: null, | ||
|
@@ -30,7 +33,7 @@ class ProjectStatus extends Component { | |
} | ||
|
||
componentDidMount() { | ||
this.getProject().then(() => this.getWorkflows()); | ||
this.getProjectAndWorkflows(); | ||
} | ||
|
||
componentWillUnmount() { | ||
|
@@ -58,6 +61,10 @@ class ProjectStatus extends Component { | |
}); | ||
} | ||
|
||
getProjectAndWorkflows() { | ||
this.getProject().then(() => this.getWorkflows()); | ||
} | ||
|
||
getUsedWorkflowLevels(workflows) { | ||
return workflows | ||
.map(workflow => workflow.configuration.level) | ||
|
@@ -76,10 +83,31 @@ class ProjectStatus extends Component { | |
handleToggle(event, workflow) { | ||
this.setState({ error: null }); | ||
const checked = event.target.checked; | ||
const defaultWorkflowId = this.state.project.configuration.default_workflow; | ||
|
||
if (defaultWorkflowId === workflow.id && workflow.active) { | ||
Dialog.alert( | ||
<WorkflowDefaultDialog closeButton={true} required={true} /> | ||
) | ||
.then(() => { | ||
const promises = [ | ||
workflow.update({ active: checked }).save(), | ||
this.state.project.update({ 'configuration.default_workflow': undefined }).save() | ||
]; | ||
Promise.all(promises) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is related to your question in the PR:
My suggestion to use At this point, you shouldn't first need to do a get request for the project and workflows again as you should already have them since they are called in |
||
.then(() => { | ||
this.getProjectAndWorkflows(); | ||
}) | ||
.catch(error => this.setState({ error })); | ||
}) | ||
.catch(error => console.error(error)); // eslint-disable-line no-console | ||
} | ||
|
||
return workflow.update({ 'active': checked }).save() | ||
.then(() => this.getWorkflows()) | ||
.catch(error => this.setState({ error })) | ||
if (defaultWorkflowId !== workflow.id) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I went back and looked at the lab code and if a project isn't live, then the default workflow button can still be enabled for inactive workflows. I think this should change, but to make sure we still allow admins to toggle the checkbox as needed, this conditional needs modification: |
||
workflow.update({ active: checked }) | ||
.save() | ||
.catch(error => this.setState({ error })); | ||
} | ||
} | ||
|
||
renderError() { | ||
|
@@ -98,6 +126,8 @@ class ProjectStatus extends Component { | |
{this.state.workflows.map((workflow) => { | ||
return ( | ||
<li key={workflow.id} className="section-list__item"> | ||
{this.state.project.configuration.default_workflow === workflow.id ? | ||
' * ' : ''} | ||
<WorkflowToggle | ||
workflow={workflow} | ||
name="active" | ||
|
@@ -164,6 +194,8 @@ class ProjectStatus extends Component { | |
<div className="project-status__section"> | ||
<h4>Workflow Settings</h4> | ||
<small>The workflow level dropdown is for the workflow assignment experimental feature.</small> | ||
<br /> | ||
<small>An asterisk (*) denotes a default workflow.</small> | ||
{this.renderError()} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's a React error being thrown,
|
||
{this.renderWorkflows()} | ||
</div> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
import React from 'react'; | ||
|
||
const WorkflowDefaultDialog = ({ onSuccess }) => ( | ||
<div> | ||
<div> | ||
You are about to make the default workflow inactive, | ||
which will remove the default workflow setting from this project. | ||
The default workflow can be set in the edit workflows page of the project builder. | ||
</div> | ||
<br /> | ||
<button type="submit" onSubmit={onSuccess}>ok</button> | ||
</div> | ||
); | ||
|
||
WorkflowDefaultDialog.defaultProps = { | ||
onSuccess: () => {} | ||
}; | ||
|
||
WorkflowDefaultDialog.propTypes = { | ||
onSuccess: React.PropTypes.func | ||
}; | ||
|
||
export default WorkflowDefaultDialog; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
import React from 'react'; | ||
import assert from 'assert'; | ||
import { shallow } from 'enzyme'; | ||
import sinon from 'sinon'; | ||
import WorkflowDefaultDialog from './workflow-default-dialog'; | ||
|
||
describe('WorkflowDefaultDialog', () => { | ||
let wrapper; | ||
let onSuccessSpy; | ||
|
||
before(() => { | ||
onSuccessSpy = sinon.spy(); | ||
wrapper = shallow(<WorkflowDefaultDialog onSuccess={onSuccessSpy} />); | ||
}); | ||
|
||
it('renders without crashing', () => { | ||
const WorkflowDefaultDialogContainer = wrapper.find('div').first(); | ||
assert.equal(WorkflowDefaultDialogContainer.length, 1); | ||
}); | ||
|
||
it('calls the onSuccess handler', () => { | ||
wrapper.find('button').simulate('submit'); | ||
sinon.assert.calledOnce(onSuccessSpy); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Non-blocking, but this is missing a semi-colon.