From 888b7b46df2b80335a65f45999ba1e8930e6b76b Mon Sep 17 00:00:00 2001 From: Roger Hutchings Date: Fri, 24 Feb 2017 11:10:01 +0000 Subject: [PATCH 1/3] Replace apply for beta form with separate component to validate project properties, including tests --- .babelrc | 7 + app/pages/lab/apply-for-beta-form.jsx | 243 +++++++++++++++ app/pages/lab/apply-for-beta-form.spec.js | 362 ++++++++++++++++++++++ app/pages/lab/visibility.cjsx | 144 ++------- package.json | 3 +- 5 files changed, 640 insertions(+), 119 deletions(-) create mode 100644 app/pages/lab/apply-for-beta-form.jsx create mode 100644 app/pages/lab/apply-for-beta-form.spec.js diff --git a/.babelrc b/.babelrc index 108ebfbdf5..6c27f647bc 100644 --- a/.babelrc +++ b/.babelrc @@ -28,6 +28,13 @@ "plugins": [ ["transform-react-jsx"] ] + }, + "test": { + "plugins": [ + ["babel-plugin-rewire"], + ["transform-react-jsx"], + ["typecheck"] + ] } } } diff --git a/app/pages/lab/apply-for-beta-form.jsx b/app/pages/lab/apply-for-beta-form.jsx new file mode 100644 index 0000000000..50b4cccdd0 --- /dev/null +++ b/app/pages/lab/apply-for-beta-form.jsx @@ -0,0 +1,243 @@ +import React from 'react'; +import uniq from 'lodash.uniq'; +import apiClient from 'panoptes-client/lib/api-client'; + +const MINIMUM_SUBJECT_COUNT = 100; +const REQUIRED_PAGES = ['Research', 'FAQ']; + +class ApplyForBetaForm extends React.Component { + constructor(props) { + super(props); + + this.attemptApplyForBeta = this.attemptApplyForBeta.bind(this); + this.canApplyForReview = this.canApplyForReview.bind(this); + this.createCheckbox = this.createCheckbox.bind(this); + this.projectHasActiveWorkflows = this.projectHasActiveWorkflows.bind(this); + this.projectHasMinimumActiveSubjects = this.projectHasMinimumActiveSubjects.bind(this); + this.projectHasRequiredContent = this.projectHasRequiredContent.bind(this); + this.projectIsLive = this.projectIsLive.bind(this); + this.projectIsPublic = this.projectIsPublic.bind(this); + this.renderValidationErrors = this.renderValidationErrors.bind(this); + this.testAsyncValidations = this.testAsyncValidations.bind(this); + this.toggleValidation = this.toggleValidation.bind(this); + this.updateValidationsFromProps = this.updateValidationsFromProps.bind(this); + + this.state = { + validations: { + projectIsPublic: this.projectIsPublic(props.project), + projectIsLive: this.projectIsLive(props.project), + projectHasActiveWorkflows: this.projectHasActiveWorkflows(props.workflows), + labPolicyReviewed: false, + bestPracticesReviewed: false, + feedbackReviewed: false, + }, + validationErrors: [], + doingAsyncValidation: false, + }; + } + + attemptApplyForBeta() { + this.testAsyncValidations() + .then(() => { + this.props.applyFn(); + }) + .catch(errors => { + this.setState({ + validationErrors: errors, + }); + }); + } + + projectHasMinimumActiveSubjects(workflows) { + const activeWorkflows = workflows.filter(workflow => workflow.active); + const uniqueSetIDs = uniq(activeWorkflows.map(workflow => workflow.links.subject_sets)); + // Second parameter is an empty object to prevent request caching. + return apiClient.type('subject_sets', {}) + .get(uniqueSetIDs) + .then(sets => { + const subjectCount = sets.reduce((count, set) => count + set.set_member_subjects_count, 0); + return (subjectCount >= MINIMUM_SUBJECT_COUNT) ? true : `The project only has ${subjectCount} of ${MINIMUM_SUBJECT_COUNT} required subjects`; + }); + } + + projectHasRequiredContent(project) { + // Second parameter is an empty object to prevent request caching. + return apiClient.type('projects') + .get(project.id) + .get('pages', {}) + .then(projectPages => { + const missingPages = REQUIRED_PAGES.reduce((accumulator, requiredPage) => { + const pagePresent = projectPages.find(page => requiredPage === page.title); + if (!pagePresent || (pagePresent.content === null || pagePresent.content === '')) + accumulator.push(requiredPage); + return accumulator; + }, []); + return (missingPages.length === 0) ? true : 'The following pages are missing content: ' + missingPages.join(', '); + }); + } + + testAsyncValidations() { + // Resolves to true if everything passes, else rejects with an array of + // error messages. + this.setState({ doingAsyncValidation: true }); + return Promise.all([ + this.projectHasMinimumActiveSubjects(this.props.workflows), + this.projectHasRequiredContent(this.props.project), + ]) + .catch(error => console.error('Error requesting project data', error)) + .then(results => { + this.setState({ doingAsyncValidation: false }); + if (results.every(result => typeof result === 'boolean' && result === true)) { + return true; + } + const errors = results.filter(result => typeof result !== 'boolean'); + return Promise.reject(errors); + }) + } + + projectIsPublic(project) { + return project.private === false; + } + + projectIsLive(project) { + return project.live === true; + } + + projectHasActiveWorkflows(workflows) { + return workflows.some(workflow => workflow.active); + } + + toggleValidation(validationName, event) { + const validations = Object.assign({}, this.state.validations); + validations[validationName] = event.target.checked; + this.setState({ validations }); + } + + canApplyForReview() { + const { validations } = this.state; + const values = Object.keys(validations) + .map(key => validations[key]); + return values.every(value => value === true); + } + + createCheckbox(validationName, content, disabled = false) { + // If it's a non-user controlled checkbox, we don't want to trigger anything + // on change, so we use Function.prototype as a noop. + const changeFn = (disabled) ? Function.prototype : this.toggleValidation.bind(this, validationName); + return ( + + ); + } + + componentWillUpdate(nextProps) { + this.updateValidationsFromProps(nextProps); + } + + updateValidationsFromProps(props) { + // We need to do a props comparison, otherwise we get a loop where props + // update state -> updates state repeatedly. + // + // Unfortunately, we have to do it by comparing the new props against the + // current state, instead of using shouldComponentUpdate. This is because + // the project prop passed down is mutable, which breaks the props/nextProps + // comparison used by shouldComponentUpdate. + const validations = Object.assign({}, this.state.validations); + + const newValues = { + projectIsPublic: this.projectIsPublic(props.project), + projectIsLive: this.projectIsLive(props.project), + projectHasActiveWorkflows: this.projectHasActiveWorkflows(props.workflows), + }; + + for (let key in newValues) { + if (validations[key] !== newValues[key]) + validations[key] = newValues[key]; + } + + if (!shallowCompare(validations, this.state.validations)) + this.setState({ validations }); + } + + renderValidationErrors(errors) { + if (errors.length) { + return ( +
+

The following errors need to be fixed:

+ +
+ ); + } + return null; + } + + render() { + const applyButtonDisabled = !this.canApplyForReview() || + this.state.doingAsyncValidation; + + return ( +
+ + {this.createCheckbox('projectIsPublic', Project is public, true)} + + {this.createCheckbox('projectIsLive', Project is live, true)} + + {this.createCheckbox('projectHasActiveWorkflows', Project has at least one active workflow, true)} + + {this.createCheckbox('labPolicyReviewed', I have reviewed the policies)} + + {this.createCheckbox('bestPracticesReviewed', I have reviewed the best practices)} + + {this.createCheckbox('feedbackReviewed', I have reviewed the sample project review feedback form)} + +

To be eligible for beta review, projects also require:

+ +

These will be checked when you click "Apply for review".

+ + + + {this.renderValidationErrors(this.state.validationErrors)} + +
+ ); + + } +} + +ApplyForBetaForm.defaultProps = { + project: {}, + workflows: [], +} + +export default ApplyForBetaForm; + +// Helper function for comparing objects +const shallowCompare = (a, b) => { + for (let key in a) { + if (!(key in b) || a[key] !== b[key]) + return false; + } + for (let key in b) { + if(!(key in a) || a[key] !== b[key]) + return false; + } + return true; +} diff --git a/app/pages/lab/apply-for-beta-form.spec.js b/app/pages/lab/apply-for-beta-form.spec.js new file mode 100644 index 0000000000..5a6aa53a0e --- /dev/null +++ b/app/pages/lab/apply-for-beta-form.spec.js @@ -0,0 +1,362 @@ +import React from 'react'; +import assert from 'assert'; +import { shallow, mount } from 'enzyme'; +import sinon from 'sinon'; +import ApplyForBetaForm from './apply-for-beta-form'; + +const MINIMUM_SUBJECT_COUNT = 100; +const REQUIRED_PAGES = ['Research', 'FAQ']; + +const createProjectProp = (properties) => { + return Object.assign({}, { + id: 1234, + private: false, + live: true, + }, properties); +}; + +const assertLabelAndCheckboxExist = function(label, checkbox) { + assert.ok(label.length, 'Label exists'); + assert.ok(checkbox.length, 'Checkbox exists'); +}; + +const assertCheckboxDisabled = function(checkbox, disabled = true) { + const message = `Checkbox is ${disabled ? 'disabled' : 'enabled'}`; + assert.ok(checkbox.prop('disabled') === disabled, message); +}; + +const assertCheckboxChecked = function(checkbox, checked = true) { + const message = `Checkbox is ${checked ? 'checked' : 'unchecked'}`; + assert.ok(checkbox.prop('checked') === checked, message); +}; + +describe('ApplyForBeta component:', function() { + + let project = {}; + let workflows = []; + let applyFn = Function.prototype; + let wrapper = Function.prototype; + let label = Function.prototype; + let checkbox = Function.prototype; + let mockPages = []; + let mockSubjectSets = []; + + const testSetup = function() { + project = createProjectProp(); + workflows = [1, 2, 3].map(i => ({ + id: i.toString(), + active: false, + links: { + subject_sets: [(i + 3).toString()], + }, + })); + applyFn = sinon.spy(); + mockPages = []; + mockSubjectSets = [ + { id: "0", set_member_subjects_count: 98 }, + ]; + }; + + ApplyForBetaForm.__Rewire__('apiClient', { + type: (type) => ({ + get: () => { + let result = null; + const resolver = (value) => new Promise(resolve => resolve(value)); + if (type === 'projects') { + result = { + get: () => resolver(mockPages), + }; + } else if (type === 'subject_sets') { + result = resolver(mockSubjectSets); + } + return result; + }, + }), + }); + + it('should render without crashing', function() { + testSetup(); + shallow(); + }); + + describe('checkbox validation:', function() { + + const setWrappers = function(labelText) { + wrapper = shallow(); + label = wrapper.findWhere(function(node) { + return node.type() === 'label' && node.text() === labelText; + }).first(); + checkbox = label.find('input[type="checkbox"]').first(); + } + + describe('disabled checkboxes:', function() { + describe('public status checkbox:', function() { + const setPublicStatusWrappers = function() { + setWrappers('Project is public'); + } + + beforeEach(testSetup); + + it('should exist', function() { + setPublicStatusWrappers(); + assertLabelAndCheckboxExist(label, checkbox); + }); + + it('should be disabled', function() { + setPublicStatusWrappers(); + assertCheckboxDisabled(checkbox); + }); + + it('should be unchecked if the project is private', function() { + project.private = true; + setPublicStatusWrappers(); + assertCheckboxChecked(checkbox, false); + }); + + it('should be checked if the project is public', function() { + setPublicStatusWrappers(); + assertCheckboxChecked(checkbox); + }); + }); + + describe('live status checkbox:', function() { + const setLiveStatusWrappers = function() { + setWrappers('Project is live'); + } + + beforeEach(testSetup); + + it('should exist', function() { + setLiveStatusWrappers(); + assertLabelAndCheckboxExist(label, checkbox); + }); + + it('should be disabled', function() { + setLiveStatusWrappers(); + assertCheckboxDisabled(checkbox); + }); + + it('should be unchecked if the project is in development', function() { + project.live = false; + setLiveStatusWrappers(); + assertCheckboxChecked(checkbox, false); + }); + + it('should be checked if the project is live', function() { + setLiveStatusWrappers(); + assertCheckboxChecked(checkbox); + }); + }); + + describe('active workflow checkbox:', function() { + const setActiveWorkflowStatusWrappers = function() { + setWrappers('Project has at least one active workflow'); + } + + beforeEach(testSetup); + + it('should exist', function() { + setActiveWorkflowStatusWrappers(); + assertLabelAndCheckboxExist(label, checkbox); + }); + + it('should be disabled', function() { + setActiveWorkflowStatusWrappers(); + assertCheckboxDisabled(checkbox); + }); + + it('should be unchecked if the project has no active workflows', function() { + setActiveWorkflowStatusWrappers(); + assertCheckboxChecked(checkbox, false); + }); + + it('should be checked if the project has at least one active workflow', function() { + workflows[0].active = true; + setActiveWorkflowStatusWrappers(); + assertCheckboxChecked(checkbox); + }); + }); + }); + + describe('enabled checkboxes:', function() { + describe('lab policies checkbox:', function() { + const setLabPoliciesWrappers = function() { + setWrappers('I have reviewed the policies'); + }; + + beforeEach(testSetup); + + it('should exist', function() { + setLabPoliciesWrappers(); + assertLabelAndCheckboxExist(label, checkbox); + }); + + it('should be enabled', function() { + setLabPoliciesWrappers(); + assertCheckboxDisabled(checkbox, false); + }); + }); + + describe('best practices checkbox:', function() { + const setBestPracticesWrappers = function() { + setWrappers('I have reviewed the best practices'); + }; + + beforeEach(testSetup); + + it('should exist', function() { + setBestPracticesWrappers(); + assertLabelAndCheckboxExist(label, checkbox); + }); + + it('should be enabled', function() { + setBestPracticesWrappers(); + assertCheckboxDisabled(checkbox, false); + }); + }); + + describe('feedback form checkbox:', function() { + const setFeedbackFormWrappers = function() { + setWrappers('I have reviewed the sample project review feedback form'); + }; + + beforeEach(testSetup); + + it('should exist', function() { + setFeedbackFormWrappers(); + assertLabelAndCheckboxExist(label, checkbox); + }); + + it('should be enabled', function() { + setFeedbackFormWrappers(); + assertCheckboxDisabled(checkbox, false); + }); + }); + }); + }); + + describe('async validation:', function() { + const testForErrorMessage = (regex, result, condition, done) => { + workflows[0].active = true; + wrapper = mount(); + wrapper.find('input[type="checkbox"]').forEach(checkbox => { + checkbox.simulate('change', { target: { checked: true }}); + }); + wrapper.find('button.standard-button').first().simulate('click'); + setTimeout(() => { + const errorMessages = wrapper.find('ul.form-help').at(1).text(); + const containsError = regex.test(errorMessages); + assert.ok(containsError === result, condition); + done(); + }, 0) + }; + + describe('content page checks:', function () { + REQUIRED_PAGES.map(function (pageTitle) { + beforeEach(testSetup); + + const pattern = 'The following pages are missing content:(.+?)' + pageTitle; + const regex = new RegExp(pattern); + + it(`should show an error if ${pageTitle} doesn't exist`, function(done) { + testForErrorMessage(regex, true, `${pageTitle} page doesn't exist`, done); + }); + + it(`should show an error if ${pageTitle} doesn't have any content`, function(done) { + mockPages.push({ + title: pageTitle, + content: '', + }); + testForErrorMessage(regex, true, `${pageTitle} page doesn't contain any content`, done); + }); + + it(`shouldn't show an error if ${pageTitle} exists and has content`, function(done) { + mockPages.push({ + title: pageTitle, + content: 'foobar', + }); + testForErrorMessage(regex, false, `${pageTitle} page doesn't contain any content`, done); + }); + }); + }); + + describe('subject set checks:', function () { + beforeEach(testSetup); + + const pattern = 'The project only has (\\d+?) of ' + MINIMUM_SUBJECT_COUNT + ' required subjects'; + const regex = new RegExp(pattern); + + it(`should show an error if there are less than ${MINIMUM_SUBJECT_COUNT} subjects`, function(done) { + testForErrorMessage(regex, true, `Project contains less than ${MINIMUM_SUBJECT_COUNT} subjects`, done); + }); + + it(`shouldn't show an error if there are ${MINIMUM_SUBJECT_COUNT} subjects`, function(done) { + mockSubjectSets.push({ + id: "1", + set_member_subjects_count: 98, + }); + testForErrorMessage(regex, false, `Project contains ${MINIMUM_SUBJECT_COUNT} subjects`, done); + }); + + it(`shouldn't show an error if there are more than ${MINIMUM_SUBJECT_COUNT} subjects`, function(done) { + mockSubjectSets.push({ + id: "1", + set_member_subjects_count: 98, + }, { + id: "2", + set_member_subjects_count: 1, + }); + testForErrorMessage(regex, false, `Project contains ${MINIMUM_SUBJECT_COUNT} subjects`, done); + }); + }); + }); + + describe('apply button behaviour:', function() { + beforeEach(testSetup); + + it('should exist', function() { + wrapper = shallow(); + const button = wrapper.find('button.standard-button').first(); + assert(button.length > 0, 'Button exists'); + }); + + it('should be disabled unless all checkboxes are checked', function() { + workflows[0].active = true; + wrapper = mount(); + const checkboxes = wrapper.find('input[type="checkbox"]'); + checkboxes.forEach(function(checkbox, index) { + if (index < (checkboxes.length - 1)) + checkbox.simulate('change', { target: { checked: true }}); + }) + const button = wrapper.find('button.standard-button').first(); + assert(button.prop('disabled') === true, 'Button is disabled'); + }); + + it('should call the applyFn prop on click if all validations pass', function(done) { + workflows[0].active = true; + mockPages.push({ + title: 'Research', + content: 'foobar', + }, { + title: 'FAQ', + content: 'foobar', + }); + mockSubjectSets.push({ + id: "1", + set_member_subjects_count: 2, + }); + + wrapper = mount(); + wrapper.find('input[type="checkbox"]').forEach(function(checkbox) { + checkbox.simulate('change', { target: { checked: true }}); + }) + wrapper.find('button.standard-button').first().simulate('click'); + + setTimeout(function() { + assert.ok(applyFn.calledOnce, 'applyFn has been called'); + done(); + }, 0); + }); + }); + +}); diff --git a/app/pages/lab/visibility.cjsx b/app/pages/lab/visibility.cjsx index 4c16d1073f..d3cc5209d1 100644 --- a/app/pages/lab/visibility.cjsx +++ b/app/pages/lab/visibility.cjsx @@ -1,8 +1,11 @@ React = require 'react' +apiClient = require 'panoptes-client/lib/api-client' WorkflowToggle = require '../../components/workflow-toggle' SetToggle = require '../../lib/set-toggle' -Dialog = require 'modal-form/dialog' getWorkflowsInOrder = require '../../lib/get-workflows-in-order' +uniq = require 'lodash.uniq' + +`import ApplyForBetaForm from './apply-for-beta-form';` module.exports = React.createClass displayName: 'EditProjectVisibility' @@ -10,62 +13,35 @@ module.exports = React.createClass getDefaultProps: -> project: null - getInitialState: -> { - error: null, - setting: { - private: false, - beta_requested: false, - launch_requested: false, - }, - workflows: null - } + getInitialState: -> + error: null + setting: + private: false + beta_requested: false + launch_requested: false + workflows: [] + loadingWorkflows: false mixins: [SetToggle] setterProperty: 'project' componentDidMount: -> - getWorkflowsInOrder(@props.project, fields: 'display_name,active,configuration') - .then((workflows) => - @setState({ workflows }) - ) - - isReviewable: -> - not @props.project.private and - @props.project.live and not - @state.setting.beta_requested and not - @props.project.beta_requested and not - @props.project.beta_approved - - canApplyForReview: -> - @isReviewable() and - @state.labPolicyReviewed and - @state.bestPracticesReviewed and - @state.feedbackReviewed + @setState { loadingWorkflows: true } + getWorkflowsInOrder @props.project + .then (workflows) => + @setState + workflows: workflows + loadingWorkflows: false setRadio: (property, value) -> @set property, value - unless @isReviewable() - @setState - labPolicyReviewed: false - bestPracticesReviewed: false - feedbackReviewed: false toggleCheckbox: (checkbox) -> change = { } change[checkbox] = @refs?[checkbox]?.checked @setState change - showFeedbackForm: -> - Dialog.alert( -
-

Review Feedback

-

During review, a feedback form will be provided to volunteers that will help you improve your project.

-

A sample of the form is provided for your consideration.

-
, - closeButton: true - ) - handleWorkflowSettingChange: (workflow, e) -> checked = e.target.checked @@ -131,51 +107,10 @@ module.exports = React.createClass

All workflows can be edited during development, and subjects will never retire. In a live project, active workflows are locked and can no longer be edited, and classifications count toward subject retirement.

-
+
-
- {if @isReviewable() -
- - -
- - - -
- - -
} - -

- {' '} - - {if @props.project.private - Only public projects can apply for review. - else if not @props.project.live - Only live projects can apply for review. - else if @props.project.beta_requested - Review status has been applied for. } -

-
- - {unless @props.project.beta_approved -

Pending approval, expose this project to users who have opted in to help test new projects.

} +

Beta status

{if @props.project.beta_approved @@ -193,45 +128,18 @@ module.exports = React.createClass
Beta Approval Status: Pending -
} - -
-
- -

- {' '} - - {unless @props.project.beta_approved - Only projects in review can apply for a full launch.} - - {if @props.project.launch_approved - -

- Launch Approval Status: - Approved -
- This project is available to the whole Zooniverse! - - else if @props.project.launch_requested - -
- Launch Approval Status: - Pending -
- Launch is awaiting Zooniverse approval. - } -

- - {unless @props.project.launch_approved -

Pending approval, expose this project to the entire Zooniverse through the main projects listing.

} +
+ Review status has been applied for. + else + + } -

Workflow Settings

- {if @state.workflows is null + {if @state.loadingWorkflows is true
Loading workflows...
else if @state.workflows.length is 0
No workflows found
diff --git a/package.json b/package.json index 4dd2e2efe6..80c0799511 100644 --- a/package.json +++ b/package.json @@ -49,6 +49,7 @@ "babel-loader": "~6.2.4", "babel-plugin-add-module-exports": "~0.2.1", "babel-plugin-react-transform": "~2.0.2", + "babel-plugin-rewire": "~1.0.0", "babel-plugin-transform-object-assign": "~6.8.0", "babel-plugin-transform-react-jsx": "~6.7.5", "babel-plugin-typecheck": "~3.9.0", @@ -111,6 +112,6 @@ "stage-with-docker": "./bin/run-through-docker.sh 'npm run stage'", "_deploy": "export PATH_ROOT=www.zooniverse.org; npm run _build && npm run _publish", "deploy": "export NODE_ENV=production; npm run _deploy", - "test": "export NODE_ENV=development; mocha test/setup.js $(find app -name *.spec.js) --reporter nyan --compilers js:babel-core/register,coffee:coffee-script/register,cjsx:coffee-react/register" + "test": "NODE_ENV=development BABEL_ENV=test mocha test/setup.js $(find app -name *.spec.js) --reporter nyan --compilers js:babel-core/register,coffee:coffee-script/register,cjsx:coffee-react/register || true" } } From 33970284df2594b8c05a6b7b229555239f5d1f4c Mon Sep 17 00:00:00 2001 From: Roger Hutchings Date: Tue, 7 Mar 2017 15:38:33 +0000 Subject: [PATCH 2/3] Fix linting issues --- app/pages/lab/apply-for-beta-form.jsx | 294 +++++++++++++++----------- 1 file changed, 168 insertions(+), 126 deletions(-) diff --git a/app/pages/lab/apply-for-beta-form.jsx b/app/pages/lab/apply-for-beta-form.jsx index 50b4cccdd0..96910cfc5c 100644 --- a/app/pages/lab/apply-for-beta-form.jsx +++ b/app/pages/lab/apply-for-beta-form.jsx @@ -1,10 +1,96 @@ -import React from 'react'; +import React, { PropTypes } from 'react'; import uniq from 'lodash.uniq'; import apiClient from 'panoptes-client/lib/api-client'; +// Constants const MINIMUM_SUBJECT_COUNT = 100; const REQUIRED_PAGES = ['Research', 'FAQ']; +// Static functions +const projectHasActiveWorkflows = (workflows) => { + return workflows.some((workflow) => { + return workflow.active; + }); +}; + +const projectHasMinimumActiveSubjects = (workflows) => { + const activeWorkflows = workflows.filter((workflow) => { + return workflow.active; + }); + const uniqueSetIDs = uniq(activeWorkflows.map((workflow) => { + return workflow.links.subject_sets; + })); + // Second parameter is an empty object to prevent request caching. + return apiClient.type('subject_sets', {}) + .get(uniqueSetIDs) + .then((sets) => { + const subjectCount = sets.reduce((count, set) => { + return count + set.set_member_subjects_count; + }, 0); + return (subjectCount >= MINIMUM_SUBJECT_COUNT) ? true : `The project only has ${subjectCount} of ${MINIMUM_SUBJECT_COUNT} required subjects`; + }); +}; + +const projectHasRequiredContent = (project) => { + // Second parameter is an empty object to prevent request caching. + return apiClient.type('projects') + .get(project.id) + .get('pages', {}) + .then((projectPages) => { + const missingPages = REQUIRED_PAGES.reduce((accumulator, requiredPage) => { + const pagePresent = projectPages.find((page) => { + return requiredPage === page.title; + }); + if (!pagePresent || (pagePresent.content === null || pagePresent.content === '')) { + accumulator.push(requiredPage); + } + return accumulator; + }, []); + const errorMessage = `The following pages are missing content: ${missingPages.join(', ')}`; + return (missingPages.length === 0) ? true : errorMessage; + }); +}; + +const projectIsLive = (project) => { + return project.live === true; +}; + +const projectIsPublic = (project) => { + return project.private === false; +}; + +const renderValidationErrors = (errors) => { + if (errors.length) { + return ( +
+

The following errors need to be fixed:

+
    + {errors.map((error) => { + return
  • {error}
    • ; + })} +
+
+ ); + } + return null; +}; + +const shallowCompare = (a, b) => { + let result = true; + Object.keys(a).forEach((key) => { + if (!(key in b) || a[key] !== b[key]) { + result = false; + } + }); + Object.keys(b).forEach((key) => { + if (!(key in a) || a[key] !== b[key]) { + result = false; + } + }); + return result; +}; + +// Component class ApplyForBetaForm extends React.Component { constructor(props) { super(props); @@ -12,68 +98,26 @@ class ApplyForBetaForm extends React.Component { this.attemptApplyForBeta = this.attemptApplyForBeta.bind(this); this.canApplyForReview = this.canApplyForReview.bind(this); this.createCheckbox = this.createCheckbox.bind(this); - this.projectHasActiveWorkflows = this.projectHasActiveWorkflows.bind(this); - this.projectHasMinimumActiveSubjects = this.projectHasMinimumActiveSubjects.bind(this); - this.projectHasRequiredContent = this.projectHasRequiredContent.bind(this); - this.projectIsLive = this.projectIsLive.bind(this); - this.projectIsPublic = this.projectIsPublic.bind(this); - this.renderValidationErrors = this.renderValidationErrors.bind(this); this.testAsyncValidations = this.testAsyncValidations.bind(this); this.toggleValidation = this.toggleValidation.bind(this); this.updateValidationsFromProps = this.updateValidationsFromProps.bind(this); - + this.state = { validations: { - projectIsPublic: this.projectIsPublic(props.project), - projectIsLive: this.projectIsLive(props.project), - projectHasActiveWorkflows: this.projectHasActiveWorkflows(props.workflows), + projectIsPublic: projectIsPublic(props.project), + projectIsLive: projectIsLive(props.project), + projectHasActiveWorkflows: projectHasActiveWorkflows(props.workflows), labPolicyReviewed: false, bestPracticesReviewed: false, - feedbackReviewed: false, + feedbackReviewed: false }, validationErrors: [], - doingAsyncValidation: false, + doingAsyncValidation: false }; } - attemptApplyForBeta() { - this.testAsyncValidations() - .then(() => { - this.props.applyFn(); - }) - .catch(errors => { - this.setState({ - validationErrors: errors, - }); - }); - } - - projectHasMinimumActiveSubjects(workflows) { - const activeWorkflows = workflows.filter(workflow => workflow.active); - const uniqueSetIDs = uniq(activeWorkflows.map(workflow => workflow.links.subject_sets)); - // Second parameter is an empty object to prevent request caching. - return apiClient.type('subject_sets', {}) - .get(uniqueSetIDs) - .then(sets => { - const subjectCount = sets.reduce((count, set) => count + set.set_member_subjects_count, 0); - return (subjectCount >= MINIMUM_SUBJECT_COUNT) ? true : `The project only has ${subjectCount} of ${MINIMUM_SUBJECT_COUNT} required subjects`; - }); - } - - projectHasRequiredContent(project) { - // Second parameter is an empty object to prevent request caching. - return apiClient.type('projects') - .get(project.id) - .get('pages', {}) - .then(projectPages => { - const missingPages = REQUIRED_PAGES.reduce((accumulator, requiredPage) => { - const pagePresent = projectPages.find(page => requiredPage === page.title); - if (!pagePresent || (pagePresent.content === null || pagePresent.content === '')) - accumulator.push(requiredPage); - return accumulator; - }, []); - return (missingPages.length === 0) ? true : 'The following pages are missing content: ' + missingPages.join(', '); - }); + componentWillUpdate(nextProps) { + this.updateValidationsFromProps(nextProps); } testAsyncValidations() { @@ -81,30 +125,24 @@ class ApplyForBetaForm extends React.Component { // error messages. this.setState({ doingAsyncValidation: true }); return Promise.all([ - this.projectHasMinimumActiveSubjects(this.props.workflows), - this.projectHasRequiredContent(this.props.project), + projectHasMinimumActiveSubjects(this.props.workflows), + projectHasRequiredContent(this.props.project) ]) - .catch(error => console.error('Error requesting project data', error)) - .then(results => { + .catch((error) => { + console.error('Error requesting project data', error); + }) + .then((results) => { this.setState({ doingAsyncValidation: false }); - if (results.every(result => typeof result === 'boolean' && result === true)) { + if (results.every((result) => { + return typeof result === 'boolean' && result === true; + })) { return true; } - const errors = results.filter(result => typeof result !== 'boolean'); + const errors = results.filter((result) => { + return typeof result !== 'boolean'; + }); return Promise.reject(errors); - }) - } - - projectIsPublic(project) { - return project.private === false; - } - - projectIsLive(project) { - return project.live === true; - } - - projectHasActiveWorkflows(workflows) { - return workflows.some(workflow => workflow.active); + }); } toggleValidation(validationName, event) { @@ -116,94 +154,96 @@ class ApplyForBetaForm extends React.Component { canApplyForReview() { const { validations } = this.state; const values = Object.keys(validations) - .map(key => validations[key]); - return values.every(value => value === true); + .map((key) => { + return validations[key]; + }); + return values.every((value) => { + return value === true; + }); } createCheckbox(validationName, content, disabled = false) { - // If it's a non-user controlled checkbox, we don't want to trigger anything + // If it's a non-user controlled checkbox, we don't want to trigger anything // on change, so we use Function.prototype as a noop. const changeFn = (disabled) ? Function.prototype : this.toggleValidation.bind(this, validationName); return ( - ); } - componentWillUpdate(nextProps) { - this.updateValidationsFromProps(nextProps); + attemptApplyForBeta() { + this.testAsyncValidations() + .then(() => { + this.props.applyFn(); + }) + .catch((errors) => { + this.setState({ + validationErrors: errors + }); + }); } updateValidationsFromProps(props) { // We need to do a props comparison, otherwise we get a loop where props // update state -> updates state repeatedly. - // - // Unfortunately, we have to do it by comparing the new props against the - // current state, instead of using shouldComponentUpdate. This is because - // the project prop passed down is mutable, which breaks the props/nextProps + // + // Unfortunately, we have to do it by comparing the new props against the + // current state, instead of using shouldComponentUpdate. This is because + // the project prop passed down is mutable, which breaks the props/nextProps // comparison used by shouldComponentUpdate. const validations = Object.assign({}, this.state.validations); const newValues = { - projectIsPublic: this.projectIsPublic(props.project), - projectIsLive: this.projectIsLive(props.project), - projectHasActiveWorkflows: this.projectHasActiveWorkflows(props.workflows), + projectIsPublic: projectIsPublic(props.project), + projectIsLive: projectIsLive(props.project), + projectHasActiveWorkflows: projectHasActiveWorkflows(props.workflows) }; - for (let key in newValues) { - if (validations[key] !== newValues[key]) + Object.keys(newValues).forEach((key) => { + if (validations[key] !== newValues[key]) { validations[key] = newValues[key]; - } + } + }); - if (!shallowCompare(validations, this.state.validations)) + if (!shallowCompare(validations, this.state.validations)) { this.setState({ validations }); - } - - renderValidationErrors(errors) { - if (errors.length) { - return ( -
-

The following errors need to be fixed:

-
    - {errors.map(error =>
  • {error}
    • )} -
-
- ); } - return null; } render() { - const applyButtonDisabled = !this.canApplyForReview() || + const applyButtonDisabled = !this.canApplyForReview() || this.state.doingAsyncValidation; - + return (
- {this.createCheckbox('projectIsPublic', Project is public, true)} + {this.createCheckbox('projectIsPublic', Project is public, true)} {this.createCheckbox('projectIsLive', Project is live, true)} {this.createCheckbox('projectHasActiveWorkflows', Project has at least one active workflow, true)} - {this.createCheckbox('labPolicyReviewed', I have reviewed the policies)} + {this.createCheckbox('labPolicyReviewed', I have reviewed the policies)} - {this.createCheckbox('bestPracticesReviewed', I have reviewed the best practices)} + {this.createCheckbox('bestPracticesReviewed', I have reviewed the best practices)} - {this.createCheckbox('feedbackReviewed', I have reviewed the sample project review feedback form)} + {this.createCheckbox('feedbackReviewed', I have reviewed the sample project review feedback form)}

To be eligible for beta review, projects also require:

  • at least {MINIMUM_SUBJECT_COUNT} subjects in active workflows
  • content on the Research and FAQ pages in the About page
-

These will be checked when you click "Apply for review".

+

These will be checked when you click "Apply for review".

); - } } ApplyForBetaForm.defaultProps = { project: {}, workflows: [], -} + applyFn: Function.prototype +}; + +ApplyForBetaForm.propTypes = { + project: PropTypes.shape({ + id: PropTypes.string.isRequired, + live: PropTypes.bool.isRequired, + private: PropTypes.bool.isRequired + }).isRequired, + workflows: PropTypes.arrayOf(PropTypes.shape({ + active: PropTypes.bool.isRequired, + links: PropTypes.shape({ + subject_sets: PropTypes.arrayOf(PropTypes.string).isRequired + }) + })).isRequired, + applyFn: PropTypes.func.isRequired +}; export default ApplyForBetaForm; - -// Helper function for comparing objects -const shallowCompare = (a, b) => { - for (let key in a) { - if (!(key in b) || a[key] !== b[key]) - return false; - } - for (let key in b) { - if(!(key in a) || a[key] !== b[key]) - return false; - } - return true; -} From 7aa405d24272759f5d8a28d80bc332f3ae6c1c18 Mon Sep 17 00:00:00 2001 From: Roger Hutchings Date: Wed, 8 Mar 2017 13:36:27 +0000 Subject: [PATCH 3/3] Move errors, change colour --- app/pages/lab/apply-for-beta-form.jsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/pages/lab/apply-for-beta-form.jsx b/app/pages/lab/apply-for-beta-form.jsx index 96910cfc5c..7d33df2ae4 100644 --- a/app/pages/lab/apply-for-beta-form.jsx +++ b/app/pages/lab/apply-for-beta-form.jsx @@ -17,9 +17,9 @@ const projectHasMinimumActiveSubjects = (workflows) => { const activeWorkflows = workflows.filter((workflow) => { return workflow.active; }); - const uniqueSetIDs = uniq(activeWorkflows.map((workflow) => { - return workflow.links.subject_sets; - })); + const uniqueSetIDs = activeWorkflows.reduce((sets, workflow) => { + return uniq(sets.concat(workflow.links.subject_sets)); + }, []); // Second parameter is an empty object to prevent request caching. return apiClient.type('subject_sets', {}) .get(uniqueSetIDs) @@ -64,7 +64,7 @@ const renderValidationErrors = (errors) => { return (

The following errors need to be fixed:

-
    +
      {errors.map((error) => { return
    • {error}
      • ; })} @@ -245,6 +245,8 @@ class ApplyForBetaForm extends React.Component {

    These will be checked when you click "Apply for review".

    + {renderValidationErrors(this.state.validationErrors)} +
); }