From 88ff8261e08cc9f68a2b768c2d614b33ca33dacb Mon Sep 17 00:00:00 2001 From: ismay Date: Mon, 13 Apr 2020 18:02:44 +0200 Subject: [PATCH] refactor: update add job route to match design specs --- i18n/en.pot | 12 +- package.json | 9 +- src/components/Buttons/CronPresetButton.js | 11 +- .../Buttons/CronPresetButton.test.js | 6 + src/components/Buttons/DiscardFormButton.js | 10 +- .../Buttons/DiscardFormButton.test.js | 20 +++ .../CronPresetButton.test.js.snap | 15 +- .../DiscardFormButton.test.js.snap | 30 ++++ src/components/Cron/HumanReadableCron.js | 18 +-- src/components/Cron/HumanReadableCron.test.js | 53 +------ .../HumanReadableCron.test.js.snap | 3 - src/components/FormErrorBox/FormErrorBox.js | 34 +++++ .../FormErrorBox/FormErrorBox.module.css | 4 + .../FormErrorBox/FormErrorBox.test.js | 18 +++ .../__snapshots__/FormErrorBox.test.js.snap | 19 +++ src/components/FormErrorBox/index.js | 3 + src/components/FormFields/CronField.js | 67 ++++---- src/components/FormFields/DelayField.js | 13 +- src/components/FormFields/JobNameField.js | 4 +- src/components/FormFields/JobTypeField.js | 10 +- .../FormFields/LabeledOptionsField.js | 6 +- src/components/FormFields/ParameterFields.js | 143 +++++++++++------- .../FormFields/ParameterFields.module.css | 4 + src/components/FormFields/ScheduleField.js | 9 +- .../FormFields/UnlabeledOptionsField.js | 6 +- src/components/FormFields/index.js | 20 +-- src/components/Forms/JobForm.js | 70 ++++++--- src/components/Forms/JobForm.module.css | 8 + src/components/Forms/JobFormContainer.js | 58 +------ src/components/Icons/Info.js | 15 -- src/components/Icons/InfoIcon.js | 25 +++ .../Icons/{Info.test.js => InfoIcon.test.js} | 6 +- .../Icons/__snapshots__/Info.test.js.snap | 18 --- .../Icons/__snapshots__/InfoIcon.test.js.snap | 17 +++ src/components/Icons/index.js | 4 +- .../PageWrapper/PageWrapper.module.css | 1 + src/components/Routes/Routes.js | 8 +- src/hooks/human-readable-cron/index.js | 1 + .../use-human-readable-cron.js | 23 +++ .../use-human-readable-cron.test.js | 64 ++++++++ src/hooks/jobs/index.js | 4 +- src/hooks/jobs/use-create-job.js | 13 -- src/hooks/jobs/use-submit-job.js | 34 +++++ src/hooks/jobs/use-submit-job.test.js | 67 ++++++++ src/pages/JobAdd/JobAdd.js | 57 ++++--- src/pages/JobAdd/JobAdd.module.css | 42 ++++- src/pages/JobAdd/JobAddContainer.js | 10 ++ src/pages/JobAdd/index.js | 4 +- src/pages/JobList/JobList.js | 12 +- src/pages/JobList/JobList.module.css | 11 +- src/services/format-error/format-error.js | 45 ++++++ .../format-error/format-error.test.js | 53 +++++++ src/services/format-error/index.js | 3 + yarn.lock | 117 +++++++------- 54 files changed, 917 insertions(+), 420 deletions(-) delete mode 100644 src/components/Cron/__snapshots__/HumanReadableCron.test.js.snap create mode 100644 src/components/FormErrorBox/FormErrorBox.js create mode 100644 src/components/FormErrorBox/FormErrorBox.module.css create mode 100644 src/components/FormErrorBox/FormErrorBox.test.js create mode 100644 src/components/FormErrorBox/__snapshots__/FormErrorBox.test.js.snap create mode 100644 src/components/FormErrorBox/index.js create mode 100644 src/components/FormFields/ParameterFields.module.css create mode 100644 src/components/Forms/JobForm.module.css delete mode 100644 src/components/Icons/Info.js create mode 100644 src/components/Icons/InfoIcon.js rename src/components/Icons/{Info.test.js => InfoIcon.test.js} (57%) delete mode 100644 src/components/Icons/__snapshots__/Info.test.js.snap create mode 100644 src/components/Icons/__snapshots__/InfoIcon.test.js.snap create mode 100644 src/hooks/human-readable-cron/index.js create mode 100644 src/hooks/human-readable-cron/use-human-readable-cron.js create mode 100644 src/hooks/human-readable-cron/use-human-readable-cron.test.js delete mode 100644 src/hooks/jobs/use-create-job.js create mode 100644 src/hooks/jobs/use-submit-job.js create mode 100644 src/hooks/jobs/use-submit-job.test.js create mode 100644 src/pages/JobAdd/JobAddContainer.js create mode 100644 src/services/format-error/format-error.js create mode 100644 src/services/format-error/format-error.test.js create mode 100644 src/services/format-error/index.js diff --git a/i18n/en.pot b/i18n/en.pot index d4c9cad02..88b4ca9e9 100644 --- a/i18n/en.pot +++ b/i18n/en.pot @@ -5,8 +5,8 @@ msgstr "" "Content-Type: text/plain; charset=utf-8\n" "Content-Transfer-Encoding: 8bit\n" "Plural-Forms: nplurals=2; plural=(n != 1)\n" -"POT-Creation-Date: 2020-04-30T08:35:39.556Z\n" -"PO-Revision-Date: 2020-04-30T08:35:39.556Z\n" +"POT-Creation-Date: 2020-06-25T09:59:16.015Z\n" +"PO-Revision-Date: 2020-06-25T09:59:16.015Z\n" msgid "Checking permissions" msgstr "" @@ -17,13 +17,16 @@ msgstr "" msgid "Delete" msgstr "" +msgid "Something went wrong whilst creating your job" +msgstr "" + msgid "CRON Expression" msgstr "" msgid "Delay" msgstr "" -msgid "Delay in seconds ({{ lowerBound }} - {{ upperBound }})" +msgid "Delay in seconds ({{ LOWERBOUND }} - {{ UPPERBOUND }})" msgstr "" msgid "Name" @@ -38,9 +41,6 @@ msgstr "" msgid "No options available" msgstr "" -msgid "Loading" -msgstr "" - msgid "Save job" msgstr "" diff --git a/package.json b/package.json index 6919e27de..cc983ae75 100644 --- a/package.json +++ b/package.json @@ -16,10 +16,10 @@ "format:css": "prettier './src/**/*.css' --write" }, "dependencies": { - "@dhis2/app-runtime": "^2.1.2", + "@dhis2/app-runtime": "^2.2.2", "@dhis2/d2-i18n": "^1.0.6", - "@dhis2/prop-types": "^2.0.0", - "@dhis2/ui": "^5.0.3", + "@dhis2/prop-types": "2.0.0", + "@dhis2/ui": "^5.0.5", "cronstrue": "^1.82.0", "history": "^4.9.0", "moment": "^2.24.0", @@ -43,8 +43,7 @@ "stylelint-no-unsupported-browser-features": "^3.0.2" }, "resolutions": { - "@dhis2/prop-types": "^1.6.4", - "@dhis2/app-runtime": "^2.1.2", + "@dhis2/app-runtime": "^2.2.2", "@dhis2/d2-i18n": "^1.0.6" }, "husky": { diff --git a/src/components/Buttons/CronPresetButton.js b/src/components/Buttons/CronPresetButton.js index d7ecff4e1..0407227cb 100644 --- a/src/components/Buttons/CronPresetButton.js +++ b/src/components/Buttons/CronPresetButton.js @@ -4,12 +4,12 @@ import { Button } from '@dhis2/ui' import i18n from '@dhis2/d2-i18n' import { CronPresetModal } from '../Modal' -const CronPresetButton = ({ setCron }) => { +const CronPresetButton = ({ setCron, small }) => { const [showModal, setShowModal] = useState(false) return ( - {showModal && ( @@ -22,10 +22,15 @@ const CronPresetButton = ({ setCron }) => { ) } -const { func } = PropTypes +CronPresetButton.defaultProps = { + small: false, +} + +const { func, bool } = PropTypes CronPresetButton.propTypes = { setCron: func.isRequired, + small: bool, } export default CronPresetButton diff --git a/src/components/Buttons/CronPresetButton.test.js b/src/components/Buttons/CronPresetButton.test.js index 18af1b109..ceefc18e4 100644 --- a/src/components/Buttons/CronPresetButton.test.js +++ b/src/components/Buttons/CronPresetButton.test.js @@ -9,6 +9,12 @@ describe('', () => { expect(wrapper).toMatchSnapshot() }) + it('renders small correctly', () => { + const wrapper = shallow( {}} small />) + + expect(wrapper).toMatchSnapshot() + }) + it('shows the modal when button is clicked', () => { const wrapper = mount( {}} />) diff --git a/src/components/Buttons/DiscardFormButton.js b/src/components/Buttons/DiscardFormButton.js index 96fd5670f..bd24f2e02 100644 --- a/src/components/Buttons/DiscardFormButton.js +++ b/src/components/Buttons/DiscardFormButton.js @@ -4,7 +4,7 @@ import { Button } from '@dhis2/ui' import history from '../../services/history' import { DiscardFormModal } from '../Modal' -const DiscardFormButton = ({ shouldConfirm, children }) => { +const DiscardFormButton = ({ shouldConfirm, children, small, className }) => { const [showModal, setShowModal] = useState(false) const onClick = shouldConfirm ? () => setShowModal(true) @@ -12,7 +12,9 @@ const DiscardFormButton = ({ shouldConfirm, children }) => { return ( - + {showModal && ( setShowModal(false)} /> )} @@ -21,14 +23,18 @@ const DiscardFormButton = ({ shouldConfirm, children }) => { } DiscardFormButton.defaultProps = { + className: '', shouldConfirm: false, + small: false, } const { string, bool } = PropTypes DiscardFormButton.propTypes = { children: string.isRequired, + className: string, shouldConfirm: bool, + small: bool, } export default DiscardFormButton diff --git a/src/components/Buttons/DiscardFormButton.test.js b/src/components/Buttons/DiscardFormButton.test.js index 9fa130200..eed1e8be5 100644 --- a/src/components/Buttons/DiscardFormButton.test.js +++ b/src/components/Buttons/DiscardFormButton.test.js @@ -16,6 +16,26 @@ describe('', () => { expect(wrapper).toMatchSnapshot() }) + it('renders small correctly', () => { + const wrapper = shallow( + + Discard + + ) + + expect(wrapper).toMatchSnapshot() + }) + + it('applies className correctly', () => { + const wrapper = shallow( + + Discard + + ) + + expect(wrapper).toMatchSnapshot() + }) + it('shows the modal when it should confirm and button is clicked', () => { const wrapper = mount( Discard diff --git a/src/components/Buttons/__snapshots__/CronPresetButton.test.js.snap b/src/components/Buttons/__snapshots__/CronPresetButton.test.js.snap index aa149c5e1..2384564b7 100644 --- a/src/components/Buttons/__snapshots__/CronPresetButton.test.js.snap +++ b/src/components/Buttons/__snapshots__/CronPresetButton.test.js.snap @@ -5,7 +5,20 @@ exports[` renders correctly 1`] = ` + +`; + +exports[` renders small correctly 1`] = ` + + + +`; + exports[` renders correctly 1`] = ` + +`; + +exports[` renders small correctly 1`] = ` + + @@ -50,21 +75,20 @@ const JobForm = ({ ) } +const { func, bool, object, array } = PropTypes + JobForm.defaultProps = { - submitError: {}, + submitError: [], } -const { func, bool, object, shape, string } = PropTypes - JobForm.propTypes = { handleSubmit: func.isRequired, + hasSubmitErrors: bool.isRequired, pristine: bool.isRequired, setIsPristine: func.isRequired, + submitting: bool.isRequired, values: object.isRequired, - submitError: shape({ - details: string, - message: string, - }), + submitError: array, } export default JobForm diff --git a/src/components/Forms/JobForm.module.css b/src/components/Forms/JobForm.module.css new file mode 100644 index 000000000..3958b2f09 --- /dev/null +++ b/src/components/Forms/JobForm.module.css @@ -0,0 +1,8 @@ +.formButtonContainer { + display: flex; + margin-top: var(--spacers-dp24); +} + +.saveButton { + margin-right: var(--spacers-dp8); +} diff --git a/src/components/Forms/JobFormContainer.js b/src/components/Forms/JobFormContainer.js index 40d23fdef..6af643ad1 100644 --- a/src/components/Forms/JobFormContainer.js +++ b/src/components/Forms/JobFormContainer.js @@ -1,63 +1,21 @@ import React from 'react' import { PropTypes } from '@dhis2/prop-types' -import { FinalForm, ReactFinalForm } from '@dhis2/ui' -import { useCreateJob } from '../../hooks/jobs' -import history from '../../services/history' -import { fieldNames, validators } from '../FormFields' +import { ReactFinalForm } from '@dhis2/ui' +import { useSubmitJob } from '../../hooks/jobs' import JobForm from './JobForm' -const { FORM_ERROR } = FinalForm const { Form } = ReactFinalForm -/** - * This validation function checks the entire form on submission. It receives an object with the - * values for each field, with the keys corresponding to the name of the field. To not have these - * field names all over the app they're exported from the fields themselves. - */ - -const formatValues = ({ job }) => { - const { JOB_TYPE } = fieldNames - const formatted = { - ...job, - [JOB_TYPE]: job[JOB_TYPE].value, - } - - return { job: formatted } -} - -const validate = values => { - const { JOB_NAME, JOB_TYPE, CRON } = fieldNames - const { - JOB_NAME_VALIDATOR, - JOB_TYPE_VALIDATOR, - CRON_VALIDATOR, - } = validators - - const jobName = values[JOB_NAME] - const cronExpression = values[CRON] - const jobType = values[JOB_TYPE] - - const validation = { - [JOB_NAME]: JOB_NAME_VALIDATOR(jobName), - [CRON]: CRON_VALIDATOR(cronExpression), - [JOB_TYPE]: JOB_TYPE_VALIDATOR(jobType), - } - - return validation -} - const JobFormContainer = ({ setIsPristine }) => { - const [createJob] = useCreateJob() - - const onSubmit = job => - createJob(formatValues({ job })) - .then(() => history.push('/')) - .catch(error => ({ [FORM_ERROR]: error.message })) + const [submitJob] = useSubmitJob() + /** + * destroyOnUnregister is enabled so that dynamic fields will be unregistered + * when they're removed from the form, for instance when the jobType changes. + */ return ( ( - - - - -) - -export default Info diff --git a/src/components/Icons/InfoIcon.js b/src/components/Icons/InfoIcon.js new file mode 100644 index 000000000..b16d24f19 --- /dev/null +++ b/src/components/Icons/InfoIcon.js @@ -0,0 +1,25 @@ +import React from 'react' +import { PropTypes } from '@dhis2/prop-types' + +const InfoIcon = ({ className }) => ( + + + + +) + +InfoIcon.defaultProps = { + className: '', +} + +const { string } = PropTypes + +InfoIcon.propTypes = { + className: string, +} + +export default InfoIcon diff --git a/src/components/Icons/Info.test.js b/src/components/Icons/InfoIcon.test.js similarity index 57% rename from src/components/Icons/Info.test.js rename to src/components/Icons/InfoIcon.test.js index 09bf59c29..0caff4d5b 100644 --- a/src/components/Icons/Info.test.js +++ b/src/components/Icons/InfoIcon.test.js @@ -1,10 +1,10 @@ import React from 'react' import { shallow } from 'enzyme' -import Info from './Info' +import InfoIcon from './InfoIcon' -describe('', () => { +describe('', () => { it('renders correctly', () => { - const wrapper = shallow() + const wrapper = shallow() expect(wrapper).toMatchSnapshot() }) diff --git a/src/components/Icons/__snapshots__/Info.test.js.snap b/src/components/Icons/__snapshots__/Info.test.js.snap deleted file mode 100644 index 2c6130414..000000000 --- a/src/components/Icons/__snapshots__/Info.test.js.snap +++ /dev/null @@ -1,18 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[` renders correctly 1`] = ` - - - - -`; diff --git a/src/components/Icons/__snapshots__/InfoIcon.test.js.snap b/src/components/Icons/__snapshots__/InfoIcon.test.js.snap new file mode 100644 index 000000000..6c706c6e6 --- /dev/null +++ b/src/components/Icons/__snapshots__/InfoIcon.test.js.snap @@ -0,0 +1,17 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[` renders correctly 1`] = ` + + + + +`; diff --git a/src/components/Icons/index.js b/src/components/Icons/index.js index 796bbde75..eccf64c5d 100644 --- a/src/components/Icons/index.js +++ b/src/components/Icons/index.js @@ -1,3 +1,3 @@ -import Info from './Info' +import InfoIcon from './InfoIcon' -export { Info } +export { InfoIcon } diff --git a/src/components/PageWrapper/PageWrapper.module.css b/src/components/PageWrapper/PageWrapper.module.css index 222980411..d5ba3590a 100644 --- a/src/components/PageWrapper/PageWrapper.module.css +++ b/src/components/PageWrapper/PageWrapper.module.css @@ -1,4 +1,5 @@ .wrapper { margin: 0 auto; + padding: var(--spacers-dp32); max-width: 1200px; } diff --git a/src/components/Routes/Routes.js b/src/components/Routes/Routes.js index 0b770900a..f72989120 100644 --- a/src/components/Routes/Routes.js +++ b/src/components/Routes/Routes.js @@ -2,18 +2,18 @@ import React from 'react' import { Route } from 'react-router-dom' import { Router } from 'react-router' import { AuthWall } from '../AuthWall' -import { JobListContainer as JobList } from '../../pages/JobList' +import { JobListContainer } from '../../pages/JobList' import { JobEdit } from '../../pages/JobEdit' -import { JobAdd } from '../../pages/JobAdd' +import { JobAddContainer } from '../../pages/JobAdd' import { NotAuthorized } from '../../pages/NotAuthorized' import history from '../../services/history' const Routes = () => ( - + - + diff --git a/src/hooks/human-readable-cron/index.js b/src/hooks/human-readable-cron/index.js new file mode 100644 index 000000000..0ac05afbe --- /dev/null +++ b/src/hooks/human-readable-cron/index.js @@ -0,0 +1 @@ +export { default as useHumanReadableCron } from './use-human-readable-cron' diff --git a/src/hooks/human-readable-cron/use-human-readable-cron.js b/src/hooks/human-readable-cron/use-human-readable-cron.js new file mode 100644 index 000000000..e6a186f79 --- /dev/null +++ b/src/hooks/human-readable-cron/use-human-readable-cron.js @@ -0,0 +1,23 @@ +import cronstrue from 'cronstrue/i18n' +import { useGetUserSettings, selectors } from '../user-settings' +import { validateCron } from '../../services/validators' + +const useHumanReadableCron = cron => { + const { loading, error, data } = useGetUserSettings() + const isValid = cron && validateCron(cron) + + if (loading || !isValid) { + return '' + } + + // Fall back to default locale in case of errors (English for cronstrue) + if (error) { + return cronstrue.toString(cron) + } + + const locale = selectors.getLocale(data) + + return cronstrue.toString(cron, { locale }) +} + +export default useHumanReadableCron diff --git a/src/hooks/human-readable-cron/use-human-readable-cron.test.js b/src/hooks/human-readable-cron/use-human-readable-cron.test.js new file mode 100644 index 000000000..fe33b2dd1 --- /dev/null +++ b/src/hooks/human-readable-cron/use-human-readable-cron.test.js @@ -0,0 +1,64 @@ +import { useGetUserSettings, selectors } from '../user-settings' +import useHumanReadableCron from './use-human-readable-cron' + +jest.mock('../user-settings', () => ({ + useGetUserSettings: jest.fn(), + selectors: { + getLocale: jest.fn(), + }, +})) + +describe('useHumanReadableCron', () => { + it('should return an empty string when loading', () => { + useGetUserSettings.mockImplementationOnce(() => ({ + loading: true, + error: undefined, + data: null, + })) + + const cron = '0 0 * ? * *' + const actual = useHumanReadableCron(cron) + + expect(actual).toBe('') + }) + + it('should return an empty string when invalid', () => { + useGetUserSettings.mockImplementationOnce(() => ({ + loading: false, + error: undefined, + data: null, + })) + + const cron = 'invalid' + const actual = useHumanReadableCron(cron) + + expect(actual).toBe('') + }) + + it('should return an english translation if there is an error', () => { + useGetUserSettings.mockImplementationOnce(() => ({ + loading: false, + error: new Error(''), + data: null, + })) + + const cron = '0 0 * ? * *' + const actual = useHumanReadableCron(cron) + + expect(actual).toBe('Every hour') + }) + + it('should return a translated cron if there is a locale', () => { + useGetUserSettings.mockImplementationOnce(() => ({ + loading: false, + error: undefined, + data: {}, + })) + selectors.getLocale.mockImplementationOnce(() => 'fr') + + const cron = '0 0 * ? * *' + const actual = useHumanReadableCron(cron) + + expect(actual).toBe('Toutes les heures') + }) +}) diff --git a/src/hooks/jobs/index.js b/src/hooks/jobs/index.js index ea6d612a0..d17fa2ab4 100644 --- a/src/hooks/jobs/index.js +++ b/src/hooks/jobs/index.js @@ -2,6 +2,6 @@ import * as selectors from './use-get-jobs' import useGetJobs from './use-get-jobs' import useToggleJob from './use-toggle-job' import useDeleteJob from './use-delete-job' -import useCreateJob from './use-create-job' +import useSubmitJob from './use-submit-job' -export { selectors, useGetJobs, useToggleJob, useDeleteJob, useCreateJob } +export { selectors, useGetJobs, useToggleJob, useDeleteJob, useSubmitJob } diff --git a/src/hooks/jobs/use-create-job.js b/src/hooks/jobs/use-create-job.js deleted file mode 100644 index b7fe762c6..000000000 --- a/src/hooks/jobs/use-create-job.js +++ /dev/null @@ -1,13 +0,0 @@ -import { useDataMutation } from '@dhis2/app-runtime' - -const mutation = { - resource: 'jobConfigurations', - type: 'create', - data: ({ job }) => job, -} - -const useCreateJob = () => { - return useDataMutation(mutation) -} - -export default useCreateJob diff --git a/src/hooks/jobs/use-submit-job.js b/src/hooks/jobs/use-submit-job.js new file mode 100644 index 000000000..918952950 --- /dev/null +++ b/src/hooks/jobs/use-submit-job.js @@ -0,0 +1,34 @@ +import { useDataEngine } from '@dhis2/app-runtime' +import history from '../../services/history' +import formatError from '../../services/format-error' + +const mutation = { + resource: 'jobConfigurations', + type: 'create', + data: ({ job }) => job, +} + +const useSubmitJob = () => { + const engine = useDataEngine() + const submitJob = job => + engine + .mutate(mutation, { variables: { job } }) + .then(() => { + history.push('/') + }) + .catch(error => { + const isValidationError = error.type === 'access' + + // Potential validation error, return it in a format final-form can handle + if (isValidationError) { + return formatError(error) + } + + // Throw any unexpected errors + throw error + }) + + return [submitJob] +} + +export default useSubmitJob diff --git a/src/hooks/jobs/use-submit-job.test.js b/src/hooks/jobs/use-submit-job.test.js new file mode 100644 index 000000000..4292d160c --- /dev/null +++ b/src/hooks/jobs/use-submit-job.test.js @@ -0,0 +1,67 @@ +import { useDataEngine } from '@dhis2/app-runtime' +import history from '../../services/history' +import formatError from '../../services/format-error' +import useSubmitJob from './use-submit-job' + +jest.mock('@dhis2/app-runtime', () => ({ + useDataEngine: jest.fn(), +})) + +jest.mock('../../services/history', () => ({ + push: jest.fn(), +})) + +jest.mock('../../services/format-error', () => jest.fn()) + +describe('useSubmitJob', () => { + it('should redirect to root after a successful submit', () => { + const engine = { + mutate: () => Promise.resolve(), + } + useDataEngine.mockImplementationOnce(() => engine) + const [submitJob] = useSubmitJob() + + expect.assertions(1) + + submitJob().then(() => { + expect(history.push).toHaveBeenCalledWith('/') + }) + }) + + it('should resolve with errors on validation errors', () => { + // Validation errors are detected by type + const error = new Error('Validation error') + error.type = 'access' + + const engine = { + mutate: () => Promise.reject(error), + } + useDataEngine.mockImplementationOnce(() => engine) + formatError.mockImplementationOnce(error => error) + + const [submitJob] = useSubmitJob() + + expect.assertions(1) + + submitJob().then(error => { + expect(error).toBe(error) + }) + }) + + it('should reject with an error on any other errors', () => { + const error = new Error('Network error') + + const engine = { + mutate: () => Promise.reject(error), + } + useDataEngine.mockImplementationOnce(() => engine) + + const [submitJob] = useSubmitJob() + + expect.assertions(1) + + submitJob().catch(error => { + expect(error).toBe(error) + }) + }) +}) diff --git a/src/pages/JobAdd/JobAdd.js b/src/pages/JobAdd/JobAdd.js index d64c5f238..50c9b93b6 100644 --- a/src/pages/JobAdd/JobAdd.js +++ b/src/pages/JobAdd/JobAdd.js @@ -1,33 +1,52 @@ -import React, { useState } from 'react' +import React from 'react' +import { PropTypes } from '@dhis2/prop-types' import { Card } from '@dhis2/ui' import i18n from '@dhis2/d2-i18n' import { DiscardFormButton } from '../../components/Buttons' -import { Info } from '../../components/Icons' +import { InfoIcon } from '../../components/Icons' import { JobFormContainer } from '../../components/Forms' import styles from './JobAdd.module.css' const infoLink = 'https://docs.dhis2.org/master/en/user/html/dataAdmin_scheduling.html#dataAdmin_scheduling_config' -const JobAdd = () => { - const [isPristine, setIsPristine] = useState(true) - - return ( - - +const JobAdd = ({ isPristine, setIsPristine }) => ( + +
+ {i18n.t('Back to all jobs')} -

{i18n.t('New Job')}

- -
-

{i18n.t('Configuration')}

- - {i18n.t('About job configuration')} -
- - - - ) +

{i18n.t('New Job')}

+
+ +
+

+ {i18n.t('Configuration')} +

+ + + {i18n.t('About job configuration')} + +
+ + + +) + +const { bool, func } = PropTypes + +JobAdd.propTypes = { + isPristine: bool.isRequired, + setIsPristine: func.isRequired, } export default JobAdd diff --git a/src/pages/JobAdd/JobAdd.module.css b/src/pages/JobAdd/JobAdd.module.css index 51192afae..8512f2c06 100644 --- a/src/pages/JobAdd/JobAdd.module.css +++ b/src/pages/JobAdd/JobAdd.module.css @@ -1,4 +1,44 @@ -.header { +.pageHeader { + margin-bottom: var(--spacers-dp32); +} + +.pageHeaderButton { + margin-bottom: var(--spacers-dp16); +} + +.pageHeaderTitle { + font-size: 18px; + font-weight: 400; + line-height: 1; + margin: 0; +} + +.card { + padding: var(--spacers-dp16); +} + +.cardHeader { + align-items: center; + display: flex; + margin-bottom: var(--spacers-dp16); +} + +.cardHeaderTitle { + font-size: 16px; + font-weight: 500; + margin: 0 var(--spacers-dp8) 0 0; +} + +.cardHeaderInfo { + width: 16px; + height: 16px; + margin-right: var(--spacers-dp4); +} + +.cardHeaderLink { align-items: center; display: flex; + font-size: 12px; + color: var(--colors-grey600); + fill: var(--colors-grey600); } diff --git a/src/pages/JobAdd/JobAddContainer.js b/src/pages/JobAdd/JobAddContainer.js new file mode 100644 index 000000000..6276474b4 --- /dev/null +++ b/src/pages/JobAdd/JobAddContainer.js @@ -0,0 +1,10 @@ +import React, { useState } from 'react' +import JobAdd from './JobAdd' + +const JobAddContainer = () => { + const [isPristine, setIsPristine] = useState(true) + + return +} + +export default JobAddContainer diff --git a/src/pages/JobAdd/index.js b/src/pages/JobAdd/index.js index 87b7e6365..1494fb601 100644 --- a/src/pages/JobAdd/index.js +++ b/src/pages/JobAdd/index.js @@ -1,3 +1,3 @@ -import JobAdd from './JobAdd' +import JobAddContainer from './JobAddContainer' -export { JobAdd } +export { JobAddContainer } diff --git a/src/pages/JobList/JobList.js b/src/pages/JobList/JobList.js index 613b4aa85..8ed413998 100644 --- a/src/pages/JobList/JobList.js +++ b/src/pages/JobList/JobList.js @@ -2,7 +2,7 @@ import React from 'react' import { PropTypes } from '@dhis2/prop-types' import { Card, Switch, Input, Button } from '@dhis2/ui' import i18n from '@dhis2/d2-i18n' -import { Info } from '../../components/Icons' +import { InfoIcon } from '../../components/Icons' import history from '../../services/history' import JobListTable from './JobListTable' import styles from './JobList.module.css' @@ -18,10 +18,12 @@ const JobList = ({ }) => { return ( -
-

{i18n.t('Scheduled jobs')}

- -
+
+

+ {i18n.t('Scheduled jobs')} +

+ +
{ + const { + details: { response }, + } = error + const validationErrors = {} + + /** + * Some backend errors do not have a way for us to infer the related field, + * those can be put in genericErrors + */ + const genericErrors = [] + + if (response.errorReports && response.errorReports.length) { + response.errorReports.forEach(report => { + /** + * errorProperty is how the backend indicates the field that the error + * is related to. If we know this, return it as a field specific error + * (note that this will swallow errors if the backend indicates a field + * that does not exist in the frontend). Otherwise we'll push it to the + * generic errors. + */ + if (report.errorProperty) { + validationErrors[report.errorProperty] = report.message + } else { + genericErrors.push(report.message) + } + }) + } + + if (genericErrors.length > 0) { + validationErrors[FORM_ERROR] = genericErrors + } + + return validationErrors +} + +export default formatError diff --git a/src/services/format-error/format-error.test.js b/src/services/format-error/format-error.test.js new file mode 100644 index 000000000..e4739068d --- /dev/null +++ b/src/services/format-error/format-error.test.js @@ -0,0 +1,53 @@ +import { FinalForm } from '@dhis2/ui' +import formatError from './format-error' + +const { FORM_ERROR } = FinalForm + +describe('formatError', () => { + it('should format field specific errors', () => { + const unformatted = { + details: { + response: { + errorReports: [ + { + errorProperty: 'field', + message: 'field error message', + }, + ], + }, + }, + } + const actual = formatError(unformatted) + const expected = { + field: 'field error message', + } + + expect(actual).toEqual(expected) + }) + + it('should format generic errors', () => { + const unformatted = { + details: { + response: { + errorReports: [ + { + message: 'generic error message', + }, + { + message: 'another generic error message', + }, + ], + }, + }, + } + const actual = formatError(unformatted) + const expected = { + [FORM_ERROR]: [ + 'generic error message', + 'another generic error message', + ], + } + + expect(actual).toEqual(expected) + }) +}) diff --git a/src/services/format-error/index.js b/src/services/format-error/index.js new file mode 100644 index 000000000..5cd5a7e6a --- /dev/null +++ b/src/services/format-error/index.js @@ -0,0 +1,3 @@ +import formatError from './format-error' + +export default formatError diff --git a/yarn.lock b/yarn.lock index 5d3ff0664..566d27ce6 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1861,10 +1861,23 @@ "@dhis2/ui-widgets" "^2.1.1" moment "^2.24.0" -"@dhis2/app-runtime@^2.0.4", "@dhis2/app-runtime@^2.1.2", "@dhis2/app-runtime@^2.2.1": - version "2.1.2" - resolved "https://registry.yarnpkg.com/@dhis2/app-runtime/-/app-runtime-2.1.2.tgz#493e6b462cdba1b5cb54db21bb3f331aa2306992" - integrity sha512-ynGxv/9T5fvva1TYY6haAkDmyelvfayzHtAJdHMy57Yvf8z14THJcBR3MeAFRfM+1njgGWeTk14evgmgpZkfeA== +"@dhis2/app-runtime@^2.0.4", "@dhis2/app-runtime@^2.2.2": + version "2.2.2" + resolved "https://registry.yarnpkg.com/@dhis2/app-runtime/-/app-runtime-2.2.2.tgz#c2129c03d4ff3bca4eb3811d82f24d3acea07e5a" + integrity sha512-0890q1deNT/aJtjtxkDROViZBrA0EP00HVMht8YAfM+uJsnvADrtjizP53r68uamZWpCZmLQsi+2xiVAQZmvXA== + dependencies: + "@dhis2/app-service-config" "2.2.2" + "@dhis2/app-service-data" "2.2.2" + +"@dhis2/app-service-config@2.2.2": + version "2.2.2" + resolved "https://registry.yarnpkg.com/@dhis2/app-service-config/-/app-service-config-2.2.2.tgz#9de0bc717ede669ee810fb7d4d84c0d35532a426" + integrity sha512-ohPCNX1hBMh/+0L5gxrSrfH/Bz787x60L4GMWODO0WEO73M6kGCjzaYZ0vjgmSCSUYJxZ6cBeybr0FBArxHjbw== + +"@dhis2/app-service-data@2.2.2": + version "2.2.2" + resolved "https://registry.yarnpkg.com/@dhis2/app-service-data/-/app-service-data-2.2.2.tgz#093d45b8bb71b5b1e81876281379cda52b6632a7" + integrity sha512-QJrYTFj+vINcqvr27yLIKunbNJTVmDPGr9fUliUdmTkCikTwlnK8fu6TPz3rhkOUYX0VrwzFJXp3ofKQajZ9mQ== "@dhis2/app-shell@4.0.9": version "4.0.9" @@ -1974,6 +1987,13 @@ i18next "^10.3" moment "^2.24.0" +"@dhis2/prop-types@2.0.0": + version "2.0.0" + resolved "https://registry.yarnpkg.com/@dhis2/prop-types/-/prop-types-2.0.0.tgz#681f0d358cec4829e675e1db96c71c38469bb34e" + integrity sha512-gL7KkGu0ytWfFXWigIsS8Rm8Ix1IesPakIs4DqqDlz14NGiwU/Pc+mHc/0iXru/AQ++XYUBpsWf91ni8nGXarA== + dependencies: + prop-types "^15" + "@dhis2/prop-types@^1.5.0", "@dhis2/prop-types@^1.6", "@dhis2/prop-types@^1.6.4": version "1.6.4" resolved "https://registry.yarnpkg.com/@dhis2/prop-types/-/prop-types-1.6.4.tgz#ec4d256c9440d4d00071524422a727c61ddaa6f6" @@ -1981,29 +2001,22 @@ dependencies: prop-types "^15" -"@dhis2/prop-types@^2.0.0": - version "2.0.0" - resolved "https://registry.yarnpkg.com/@dhis2/prop-types/-/prop-types-2.0.0.tgz#681f0d358cec4829e675e1db96c71c38469bb34e" - integrity sha512-gL7KkGu0ytWfFXWigIsS8Rm8Ix1IesPakIs4DqqDlz14NGiwU/Pc+mHc/0iXru/AQ++XYUBpsWf91ni8nGXarA== - dependencies: - prop-types "^15" - -"@dhis2/ui-constants@5.0.3": - version "5.0.3" - resolved "https://registry.yarnpkg.com/@dhis2/ui-constants/-/ui-constants-5.0.3.tgz#8d6e6adc5c7ac3de92236dd97f9be01cc18c3ed5" - integrity sha512-9dnnmc+ztKmstM/AftGs+att4qShZUJWjeLRGMbnFE/fDwAwqvNCgGzmOM3K6YWXrrvJXjQC5TVOzhmqS0jReQ== +"@dhis2/ui-constants@5.0.5": + version "5.0.5" + resolved "https://registry.yarnpkg.com/@dhis2/ui-constants/-/ui-constants-5.0.5.tgz#76c101e7403bc2b569947b1106a9531047ba6cf8" + integrity sha512-f0oEv0vERlgsrrowzvd1qyt7NTSATdQjHALDphX9Tf3y7xd0Y1RRTPCmRqtCDbsmPzNVeREgo5+LEnuJK1b45A== dependencies: "@dhis2/prop-types" "^1.6.4" -"@dhis2/ui-core@5.0.3": - version "5.0.3" - resolved "https://registry.yarnpkg.com/@dhis2/ui-core/-/ui-core-5.0.3.tgz#6098eb547d8cbe3453cb6f8e425512c14d7e68ef" - integrity sha512-lh6WhLq0mi9+AX7U9ScfzaPxk3nCl4yNc8LPYt/87AdM/OB7gBytDEg4qjdXlKpvstcytzV0AAUWvvkTqk7N5w== +"@dhis2/ui-core@5.0.5": + version "5.0.5" + resolved "https://registry.yarnpkg.com/@dhis2/ui-core/-/ui-core-5.0.5.tgz#08163a53553e1b3be4fbb4fb92fd02d7084f7b5e" + integrity sha512-RmpJL+OrzSNTfAuJTDbBW7i8GYXshL9pZ9753y9rxuPGGbkHAoSjN756Wjx+/k1fK4AL+DFeBj8QbEF5oVv8XA== dependencies: "@dhis2/prop-types" "^1.6.4" - "@dhis2/ui-constants" "5.0.3" - "@dhis2/ui-icons" "5.0.3" - "@popperjs/core" "^2.4.0" + "@dhis2/ui-constants" "5.0.5" + "@dhis2/ui-icons" "5.0.5" + "@popperjs/core" "^2.4.2" classnames "^2.2.6" react-popper "^2.2.3" resize-observer-polyfill "^1.5.1" @@ -2017,35 +2030,35 @@ "@popperjs/core" "^2.1.0" classnames "^2.2.6" -"@dhis2/ui-forms@5.0.3": - version "5.0.3" - resolved "https://registry.yarnpkg.com/@dhis2/ui-forms/-/ui-forms-5.0.3.tgz#ccfd3d7b2d157eae408c6efcf5f9448e95573855" - integrity sha512-+x+9XsT7ChdmXsnE6BM/1J2/Z/1SUH45aUid8XA1wS0I31VUAwG+2g0VKqszqGHke+wKgMRwPAKNbGfChuQv1w== +"@dhis2/ui-forms@5.0.5": + version "5.0.5" + resolved "https://registry.yarnpkg.com/@dhis2/ui-forms/-/ui-forms-5.0.5.tgz#555b35d97ae008b6dd447ce02d1caca87360c522" + integrity sha512-JT+97yiB4BHDVhHjMb6VYA5Y6z9Qf9JvrmRccVCb36TwpmIVvXEiD3vyn8kDUmqr1f8RDDXutNGHC0Z17C2Oxg== dependencies: "@dhis2/prop-types" "^1.6.4" - "@dhis2/ui-core" "5.0.3" + "@dhis2/ui-core" "5.0.5" classnames "^2.2.6" final-form "^4.20.0" react-final-form "^6.5.0" -"@dhis2/ui-icons@5.0.3": - version "5.0.3" - resolved "https://registry.yarnpkg.com/@dhis2/ui-icons/-/ui-icons-5.0.3.tgz#73618454633cc7c76d7683e0c1f7820ab2da7945" - integrity sha512-KzWr6AZAPI7nXlsZOsJgf+5htDrHnQFYc16YwpkaeW7fMh+6LT4QqlVayKAqme/4KidVb9UeOGEqF5nMyl2quw== +"@dhis2/ui-icons@5.0.5": + version "5.0.5" + resolved "https://registry.yarnpkg.com/@dhis2/ui-icons/-/ui-icons-5.0.5.tgz#0d9bcd60390a88d3a6da87e1e25024d14fbb28cb" + integrity sha512-DJXO9SPGX3O1+kB401hgBtzFNcLvWfXm1mIXkESCy1sCBVrlJGaDTL/ILhO6FbeXjqpNT527EnC7E6xawZjMRQ== dependencies: "@dhis2/prop-types" "^1.6.4" -"@dhis2/ui-widgets@5.0.3": - version "5.0.3" - resolved "https://registry.yarnpkg.com/@dhis2/ui-widgets/-/ui-widgets-5.0.3.tgz#1c7836892a9394ee0b78ee57e6d972f34769c582" - integrity sha512-ABjZiJ0ZGrKzDFiDfGizTYdTJvsmqf6fMywr9qUS+AR2p0xDtCfZsi+lURlV9yh35rWm9+/kEgMEet3T5gfHxQ== +"@dhis2/ui-widgets@5.0.5": + version "5.0.5" + resolved "https://registry.yarnpkg.com/@dhis2/ui-widgets/-/ui-widgets-5.0.5.tgz#6bd034eb2b09eddd2361108deb19af8246891328" + integrity sha512-p1w5qVG47oZbzvZE21t57sbu6Gey7WB/LaK0cdvDyhjWtO7FPjsnyyLfJGRp4uNn3nBe3G+/vOuI91CKN1T+Ng== dependencies: - "@dhis2/app-runtime" "^2.2.1" + "@dhis2/app-runtime" "^2.2.2" "@dhis2/d2-i18n" "^1" "@dhis2/prop-types" "^1.6.4" - "@dhis2/ui-constants" "5.0.3" - "@dhis2/ui-core" "5.0.3" - "@dhis2/ui-icons" "5.0.3" + "@dhis2/ui-constants" "5.0.5" + "@dhis2/ui-core" "5.0.5" + "@dhis2/ui-icons" "5.0.5" classnames "^2.2.6" resize-observer-polyfill "^1.5.1" @@ -2057,16 +2070,16 @@ "@dhis2/prop-types" "^1.6" classnames "^2.2.6" -"@dhis2/ui@^5.0.3": - version "5.0.3" - resolved "https://registry.yarnpkg.com/@dhis2/ui/-/ui-5.0.3.tgz#fb59afd3bf89a7d61754d86e2d8461622660c983" - integrity sha512-U82vkn1GJzbSGW4xhYuBiVXeyuQLc+fP+5ZmK7pTUwJHTrlZKBxfpmX80cWYKswCDV21EXLlyd/ESrjkEG2dQw== +"@dhis2/ui@^5.0.5": + version "5.0.5" + resolved "https://registry.yarnpkg.com/@dhis2/ui/-/ui-5.0.5.tgz#0a7c3b9d6c9be705c5b4dc6d02d6facc1003fddb" + integrity sha512-LgGvMD4o0beaN8ZWjI3LQCddtPgfDMA0hnS5qoHnObczAhJAO5/sISphomB7Vfz3ENW5WWS3mhXVESXeY14BHg== dependencies: - "@dhis2/ui-constants" "5.0.3" - "@dhis2/ui-core" "5.0.3" - "@dhis2/ui-forms" "5.0.3" - "@dhis2/ui-icons" "5.0.3" - "@dhis2/ui-widgets" "5.0.3" + "@dhis2/ui-constants" "5.0.5" + "@dhis2/ui-core" "5.0.5" + "@dhis2/ui-forms" "5.0.5" + "@dhis2/ui-icons" "5.0.5" + "@dhis2/ui-widgets" "5.0.5" "@hapi/address@2.x.x": version "2.1.4" @@ -2296,7 +2309,7 @@ resolved "https://registry.yarnpkg.com/@popperjs/core/-/core-2.1.0.tgz#09a7a352a40508156e1256efdc015593feca28e0" integrity sha512-ntN5t5spqhQv28cLfmmt1dYabsudzR5A7PU15gr/gzcT/gzqAOnYFQPaLPFraDa7ZCJG2eJ1JsO7pgXbYXGIrw== -"@popperjs/core@^2.4.0": +"@popperjs/core@^2.4.2": version "2.4.2" resolved "https://registry.yarnpkg.com/@popperjs/core/-/core-2.4.2.tgz#7c6dc4ecef16149fd7a736710baa1b811017fdca" integrity sha512-JlGTGRYHC2QK+DDbePyXdBdooxFq2+noLfWpRqJtkxcb/oYWzOF0kcbfvvbWrwevCC1l6hLUg1wHYT+ona5BWQ== @@ -13414,9 +13427,9 @@ styled-jsx@^3.2.2: stylis-rule-sheet "0.0.10" styled-jsx@^3.2.5: - version "3.3.0" - resolved "https://registry.yarnpkg.com/styled-jsx/-/styled-jsx-3.3.0.tgz#32335c1a3ecfc923ba4f9c056eeb3d4699006b09" - integrity sha512-sh8BI5eGKyJlwL4kNXHjb27/a/GJV8wP4ElRIkRXrGW3sHKOsY9Pa1VZRNxyvf3+lisdPwizD9JDkzVO9uGwZw== + version "3.2.5" + resolved "https://registry.yarnpkg.com/styled-jsx/-/styled-jsx-3.2.5.tgz#0172a3e13a0d6d8bf09167dcaf32cf7102d932ca" + integrity sha512-prEahkYwQHomUljJzXzrFnBmQrSMtWOBbXn8QeEkpfFkqMZQGshxzzp4H8ebBIsbVlHF/3+GSXMnmK/fp7qVYQ== dependencies: "@babel/types" "7.8.3" babel-plugin-syntax-jsx "6.18.0"