From 75481c95bf25a55d9943253dbb0627d90be0bdb4 Mon Sep 17 00:00:00 2001 From: ismay <7355199+ismay@users.noreply.github.com> Date: Wed, 16 Aug 2023 16:34:31 +0200 Subject: [PATCH] fix: add new route structure (#559) * fix: add new route structure * test: update tests to route changes * refactor: update internal routes to new routing pattern * test: update cypress links to new routing pattern * test: add checkjobtype test * chore: rename checkjobtype to jobvieworedit * fix: ensure queues can not be named add --- .../add-route/back-to-all-jobs/index.js | 2 +- .../add-route/create-parameter-jobs/index.js | 2 +- .../create-parameterless-jobs/index.js | 2 +- .../add-route/cron-presets/index.js | 2 +- .../integration/add-route/info-link/index.js | 2 +- .../add-sequence/back-to-all-jobs/index.js | 2 +- .../add-sequence/create-sequence/index.js | 2 +- .../edit-route/back-to-all-jobs/index.js | 2 +- .../edit-route/cron-presets/index.js | 2 +- .../edit-route/delete-button/index.js | 2 +- .../edit-route/display-jobs/index.js | 2 +- .../edit-route/edit-parameter-jobs/index.js | 2 +- .../edit-parameterless-jobs/index.js | 2 +- .../integration/edit-route/info-link/index.js | 2 +- .../edit-sequence/back-to-all-jobs/index.js | 2 +- .../edit-sequence/edit-sequence/index.js | 2 +- .../integration/list-route/new-job/index.js | 2 +- .../view-route/back-to-all-jobs/index.js | 2 +- .../view-route/display-jobs/index.js | 2 +- .../integration/view-route/info-link/index.js | 2 +- i18n/en.pot | 24 ++--- src/components/FormFields/NameField.js | 51 ++++++++--- src/components/FormFields/NameField.test.js | 52 +++++++++++ src/components/Forms/SequenceAddForm.js | 2 +- src/components/Forms/SequenceEditForm.js | 2 +- src/components/Forms/SequenceEditForm.test.js | 4 + src/components/JobTable/EditJobAction.js | 2 +- src/components/JobTable/EditJobAction.test.js | 2 +- src/components/JobTable/ViewJobAction.js | 2 +- src/components/JobTable/ViewJobAction.test.js | 2 +- src/components/Routes/JobViewOrEdit.js | 37 ++++++++ src/components/Routes/JobViewOrEdit.test.js | 88 +++++++++++++++++++ src/components/Routes/Routes.js | 21 +++-- src/hooks/jobs/use-jobs.js | 1 + src/pages/JobEdit/JobEdit.js | 40 ++++----- src/pages/JobEdit/JobEdit.test.js | 61 ++----------- src/pages/JobList/JobList.js | 4 +- src/pages/JobView/JobView.js | 39 ++++---- src/pages/JobView/JobView.test.js | 65 +++----------- 39 files changed, 324 insertions(+), 215 deletions(-) create mode 100644 src/components/Routes/JobViewOrEdit.js create mode 100644 src/components/Routes/JobViewOrEdit.test.js diff --git a/cypress/integration/add-route/back-to-all-jobs/index.js b/cypress/integration/add-route/back-to-all-jobs/index.js index 6642e76ff..f7bd9a2b1 100644 --- a/cypress/integration/add-route/back-to-all-jobs/index.js +++ b/cypress/integration/add-route/back-to-all-jobs/index.js @@ -1,7 +1,7 @@ import { Given, When, Then } from 'cypress-cucumber-preprocessor/steps' Given('the user navigated to the add job page', () => { - cy.visit('/#/add') + cy.visit('/#/job/add') cy.findByRole('heading', { name: 'New Job' }).should('exist') }) diff --git a/cypress/integration/add-route/create-parameter-jobs/index.js b/cypress/integration/add-route/create-parameter-jobs/index.js index 51f71e91c..f7d93ba6d 100644 --- a/cypress/integration/add-route/create-parameter-jobs/index.js +++ b/cypress/integration/add-route/create-parameter-jobs/index.js @@ -23,7 +23,7 @@ const saveAndExpect = (expected) => { */ Given('the user navigated to the add job page', () => { - cy.visit('/#/add') + cy.visit('/#/job/add') cy.findByRole('heading', { name: 'New Job' }).should('exist') }) diff --git a/cypress/integration/add-route/create-parameterless-jobs/index.js b/cypress/integration/add-route/create-parameterless-jobs/index.js index d71b13177..632fd9289 100644 --- a/cypress/integration/add-route/create-parameterless-jobs/index.js +++ b/cypress/integration/add-route/create-parameterless-jobs/index.js @@ -23,7 +23,7 @@ const saveAndExpect = (expected) => { */ Given('the user navigated to the add job page', () => { - cy.visit('/#/add') + cy.visit('/#/job/add') cy.findByRole('heading', { name: 'New Job' }).should('exist') }) diff --git a/cypress/integration/add-route/cron-presets/index.js b/cypress/integration/add-route/cron-presets/index.js index c1cf88529..34ceb81df 100644 --- a/cypress/integration/add-route/cron-presets/index.js +++ b/cypress/integration/add-route/cron-presets/index.js @@ -1,7 +1,7 @@ import { Given, When, Then } from 'cypress-cucumber-preprocessor/steps' Given('the user navigated to the add job page', () => { - cy.visit('/#/add') + cy.visit('/#/job/add') cy.findByRole('heading', { name: 'New Job' }).should('exist') }) diff --git a/cypress/integration/add-route/info-link/index.js b/cypress/integration/add-route/info-link/index.js index 2243c584d..d90b71deb 100644 --- a/cypress/integration/add-route/info-link/index.js +++ b/cypress/integration/add-route/info-link/index.js @@ -4,7 +4,7 @@ const infoHref = 'https://docs.dhis2.org/en/use/user-guides/dhis-core-version-236/maintaining-the-system/scheduling.html' Given('the user navigated to the add job page', () => { - cy.visit('/#/add') + cy.visit('/#/job/add') cy.findByRole('heading', { name: 'New Job' }).should('exist') }) diff --git a/cypress/integration/add-sequence/back-to-all-jobs/index.js b/cypress/integration/add-sequence/back-to-all-jobs/index.js index 34f6d6f50..ab796ac08 100644 --- a/cypress/integration/add-sequence/back-to-all-jobs/index.js +++ b/cypress/integration/add-sequence/back-to-all-jobs/index.js @@ -1,7 +1,7 @@ import { Given, When, Then } from 'cypress-cucumber-preprocessor/steps' Given('the user navigated to the add sequence page', () => { - cy.visit('/#/add-sequence') + cy.visit('/#/queue/add') cy.findByRole('heading', { name: 'New Sequence' }).should('exist') }) diff --git a/cypress/integration/add-sequence/create-sequence/index.js b/cypress/integration/add-sequence/create-sequence/index.js index 565fc9d04..9a5e6dbd9 100644 --- a/cypress/integration/add-sequence/create-sequence/index.js +++ b/cypress/integration/add-sequence/create-sequence/index.js @@ -28,7 +28,7 @@ Given('two unqueued jobs exist', () => { }) Given('the user navigated to the add sequence page', () => { - cy.visit('/#/add-sequence') + cy.visit('/#/queue/add') cy.findByRole('heading', { name: 'New Sequence' }).should('exist') }) diff --git a/cypress/integration/edit-route/back-to-all-jobs/index.js b/cypress/integration/edit-route/back-to-all-jobs/index.js index e37d27fa1..6165cefc3 100644 --- a/cypress/integration/edit-route/back-to-all-jobs/index.js +++ b/cypress/integration/edit-route/back-to-all-jobs/index.js @@ -8,7 +8,7 @@ Given('a single user job exists', () => { }) Given('the user navigated to the edit job page', () => { - cy.visit('/#/edit/lnWRZN67iDU') + cy.visit('/#/job/lnWRZN67iDU') cy.findByRole('heading', { name: 'Job: Job 1' }).should('exist') }) diff --git a/cypress/integration/edit-route/cron-presets/index.js b/cypress/integration/edit-route/cron-presets/index.js index ae862228b..8a13a2926 100644 --- a/cypress/integration/edit-route/cron-presets/index.js +++ b/cypress/integration/edit-route/cron-presets/index.js @@ -8,7 +8,7 @@ Given('a single cron scheduled user job exists', () => { }) Given('the user navigated to the edit job page', () => { - cy.visit('/#/edit/lnWRZN67iDU') + cy.visit('/#/job/lnWRZN67iDU') cy.findByRole('heading', { name: 'Job: Job 1' }).should('exist') }) diff --git a/cypress/integration/edit-route/delete-button/index.js b/cypress/integration/edit-route/delete-button/index.js index 37fd88b6a..a762932a3 100644 --- a/cypress/integration/edit-route/delete-button/index.js +++ b/cypress/integration/edit-route/delete-button/index.js @@ -8,7 +8,7 @@ Given('a single user job exists', () => { }) Given('the user navigated to the edit job page', () => { - cy.visit('/#/edit/lnWRZN67iDU') + cy.visit('/#/job/lnWRZN67iDU') cy.findByRole('heading', { name: 'Job: Job 1' }).should('exist') }) diff --git a/cypress/integration/edit-route/display-jobs/index.js b/cypress/integration/edit-route/display-jobs/index.js index 0cf40a184..db12c9bef 100644 --- a/cypress/integration/edit-route/display-jobs/index.js +++ b/cypress/integration/edit-route/display-jobs/index.js @@ -12,7 +12,7 @@ Given('the user navigated to the edit job page', () => { const now = new Date(2021, 3, 10).getTime() cy.clock(now) - cy.visit('/#/edit/lnWRZN67iDU') + cy.visit('/#/job/lnWRZN67iDU') cy.findByRole('heading', { name: 'Job: Job 1' }).should('exist') }) diff --git a/cypress/integration/edit-route/edit-parameter-jobs/index.js b/cypress/integration/edit-route/edit-parameter-jobs/index.js index 5a7b70b00..8c9a30cb3 100644 --- a/cypress/integration/edit-route/edit-parameter-jobs/index.js +++ b/cypress/integration/edit-route/edit-parameter-jobs/index.js @@ -41,7 +41,7 @@ Given('a single user job exists', () => { }) Given('the user navigated to the edit job page', () => { - cy.visit('/#/edit/lnWRZN67iDU') + cy.visit('/#/job/lnWRZN67iDU') cy.findByRole('heading', { name: 'Job: Job 1' }).should('exist') }) diff --git a/cypress/integration/edit-route/edit-parameterless-jobs/index.js b/cypress/integration/edit-route/edit-parameterless-jobs/index.js index e960112dd..e3feb3874 100644 --- a/cypress/integration/edit-route/edit-parameterless-jobs/index.js +++ b/cypress/integration/edit-route/edit-parameterless-jobs/index.js @@ -41,7 +41,7 @@ Given('a single user job with parameters exists', () => { }) Given('the user navigated to the edit job page', () => { - cy.visit('/#/edit/lnWRZN67iDU') + cy.visit('/#/job/lnWRZN67iDU') cy.findByRole('heading', { name: 'Job: Job 1' }).should('exist') }) diff --git a/cypress/integration/edit-route/info-link/index.js b/cypress/integration/edit-route/info-link/index.js index 7e9f15a8c..685499aa8 100644 --- a/cypress/integration/edit-route/info-link/index.js +++ b/cypress/integration/edit-route/info-link/index.js @@ -11,7 +11,7 @@ Given('a single user job exists', () => { }) Given('the user navigated to the edit job page', () => { - cy.visit('/#/edit/lnWRZN67iDU') + cy.visit('/#/job/lnWRZN67iDU') cy.findByRole('heading', { name: 'Job: Job 1' }).should('exist') }) diff --git a/cypress/integration/edit-sequence/back-to-all-jobs/index.js b/cypress/integration/edit-sequence/back-to-all-jobs/index.js index 6ea9e92c9..83f9702d3 100644 --- a/cypress/integration/edit-sequence/back-to-all-jobs/index.js +++ b/cypress/integration/edit-sequence/back-to-all-jobs/index.js @@ -8,7 +8,7 @@ Given('a sequence exists', () => { }) Given('the user navigates to the edit sequence page', () => { - cy.visit('/#/edit-sequence/RWcaltWoKuN') + cy.visit('/#/queue/RWcaltWoKuN') cy.findByRole('heading', { name: 'Sequence: one' }).should('exist') }) diff --git a/cypress/integration/edit-sequence/edit-sequence/index.js b/cypress/integration/edit-sequence/edit-sequence/index.js index 25f11793c..cbc6022bd 100644 --- a/cypress/integration/edit-sequence/edit-sequence/index.js +++ b/cypress/integration/edit-sequence/edit-sequence/index.js @@ -35,7 +35,7 @@ Given('two unqueued jobs exist', () => { }) Given('the user navigates to the edit sequence page', () => { - cy.visit('/#/edit-sequence/RWcaltWoKuN') + cy.visit('/#/queue/RWcaltWoKuN') cy.findByRole('heading', { name: 'Sequence: one' }).should('exist') }) diff --git a/cypress/integration/list-route/new-job/index.js b/cypress/integration/list-route/new-job/index.js index 6d11c0917..ff54ef2c9 100644 --- a/cypress/integration/list-route/new-job/index.js +++ b/cypress/integration/list-route/new-job/index.js @@ -8,5 +8,5 @@ Given('the user navigated to the job list page', () => { Then('there is a link to the new job page', () => { cy.findByRole('link', { name: 'New job' }) .should('exist') - .should('have.attr', 'href', '#/add') + .should('have.attr', 'href', '#/job/add') }) diff --git a/cypress/integration/view-route/back-to-all-jobs/index.js b/cypress/integration/view-route/back-to-all-jobs/index.js index a7515e79a..37ef3f10f 100644 --- a/cypress/integration/view-route/back-to-all-jobs/index.js +++ b/cypress/integration/view-route/back-to-all-jobs/index.js @@ -8,7 +8,7 @@ Given('a single system job exists', () => { }) Given('the user navigated to the view job page', () => { - cy.visit('/#/view/sHMedQF7VYa') + cy.visit('/#/job/sHMedQF7VYa') cy.findByRole('heading', { name: 'System job: System Job 1' }).should( 'exist' ) diff --git a/cypress/integration/view-route/display-jobs/index.js b/cypress/integration/view-route/display-jobs/index.js index d2d8c910a..495e0dc77 100644 --- a/cypress/integration/view-route/display-jobs/index.js +++ b/cypress/integration/view-route/display-jobs/index.js @@ -12,7 +12,7 @@ Given('the user navigated to the view job page', () => { const now = new Date(2021, 3, 10).getTime() cy.clock(now) - cy.visit('/#/view/sHMedQF7VYa') + cy.visit('/#/job/sHMedQF7VYa') cy.findByRole('heading', { name: 'System job: System Job 1' }).should( 'exist' ) diff --git a/cypress/integration/view-route/info-link/index.js b/cypress/integration/view-route/info-link/index.js index 3941c582c..f00182677 100644 --- a/cypress/integration/view-route/info-link/index.js +++ b/cypress/integration/view-route/info-link/index.js @@ -11,7 +11,7 @@ Given('a single system job exists', () => { }) Given('the user navigated to the view job page', () => { - cy.visit('/#/view/sHMedQF7VYa') + cy.visit('/#/job/sHMedQF7VYa') cy.findByRole('heading', { name: 'System job: System Job 1' }).should( 'exist' ) diff --git a/i18n/en.pot b/i18n/en.pot index 1302c6572..b62bf2679 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: 2023-08-03T07:44:07.705Z\n" -"PO-Revision-Date: 2023-08-03T07:44:07.705Z\n" +"POT-Creation-Date: 2023-08-15T13:43:45.474Z\n" +"PO-Revision-Date: 2023-08-15T13:43:45.474Z\n" msgid "Something went wrong" msgstr "Something went wrong" @@ -231,6 +231,16 @@ msgstr "Are you sure you want to run this job?" msgid "Run" msgstr "Run" +msgid "Could not load requested job" +msgstr "Could not load requested job" + +msgid "" +"Something went wrong whilst loading the requested job. Make sure it has not " +"been deleted and try refreshing the page." +msgstr "" +"Something went wrong whilst loading the requested job. Make sure it has not " +"been deleted and try refreshing the page." + msgid "Toggle job" msgstr "Toggle job" @@ -243,16 +253,6 @@ msgstr "Configuration" msgid "About job configuration" msgstr "About job configuration" -msgid "Could not load requested job" -msgstr "Could not load requested job" - -msgid "" -"Something went wrong whilst loading the requested job. Make sure it has not " -"been deleted and try refreshing the page." -msgstr "" -"Something went wrong whilst loading the requested job. Make sure it has not " -"been deleted and try refreshing the page." - msgid "Job: {{ name }}" msgstr "Job: {{ name }}" diff --git a/src/components/FormFields/NameField.js b/src/components/FormFields/NameField.js index 8c7102677..76e16c54e 100644 --- a/src/components/FormFields/NameField.js +++ b/src/components/FormFields/NameField.js @@ -6,23 +6,50 @@ import { hasValue, string, } from '@dhis2/ui' +import PropTypes from 'prop-types' import i18n from '@dhis2/d2-i18n' const { Field } = ReactFinalForm // The key under which this field will be sent to the backend const FIELD_NAME = 'name' -const VALIDATOR = composeValidators(string, hasValue) - -const NameField = () => ( - -) + +// Validation +const restrictedNames = (value) => { + if (typeof value !== 'string') { + return + } + + return value.toLowerCase() === 'add' + ? i18n.t('Queues can\'t be named "Add" or "add"') + : undefined +} +const defaultValidators = [string, hasValue] +const queueValidators = [...defaultValidators, restrictedNames] + +const NameField = ({ isQueue }) => { + const validators = isQueue ? queueValidators : defaultValidators + + return ( + + ) +} + +NameField.defaultProps = { + isQueue: false, +} + +const { bool } = PropTypes + +NameField.propTypes = { + isQueue: bool, +} export default NameField diff --git a/src/components/FormFields/NameField.test.js b/src/components/FormFields/NameField.test.js index 1732dd757..4ae4c7152 100644 --- a/src/components/FormFields/NameField.test.js +++ b/src/components/FormFields/NameField.test.js @@ -27,4 +27,56 @@ describe('', () => { expect(actual).toEqual(expect.stringMatching(expected)) }) + + it('does not allow naming a queue "Add"', () => { + const expected = 'Queues can\'t be named "Add" or "add"' + const wrapper = mount( +
{}}> + {({ handleSubmit }) => ( + + + + )} + + ) + + wrapper + .find('input[name="name"]') + .simulate('change', { target: { value: 'Add' } }) + + // Trigger validation + wrapper.find('form').simulate('submit') + + const actual = wrapper + .find({ 'data-test': 'dhis2-uiwidgets-inputfield-validation' }) + .text() + + expect(actual).toEqual(expect.stringMatching(expected)) + }) + + it('does not allow naming a queue "add"', () => { + const expected = 'Queues can\'t be named "Add" or "add"' + const wrapper = mount( +
{}}> + {({ handleSubmit }) => ( + + + + )} + + ) + + wrapper + .find('input[name="name"]') + .simulate('change', { target: { value: 'add' } }) + + // Trigger validation + wrapper.find('form').simulate('submit') + + const actual = wrapper + .find({ 'data-test': 'dhis2-uiwidgets-inputfield-validation' }) + .text() + + expect(actual).toEqual(expect.stringMatching(expected)) + }) }) diff --git a/src/components/Forms/SequenceAddForm.js b/src/components/Forms/SequenceAddForm.js index bd6796533..e46585e3f 100644 --- a/src/components/Forms/SequenceAddForm.js +++ b/src/components/Forms/SequenceAddForm.js @@ -20,7 +20,7 @@ const SequenceAddForm = ({ return (
- + diff --git a/src/components/Forms/SequenceEditForm.js b/src/components/Forms/SequenceEditForm.js index a06cb748b..942181c3e 100644 --- a/src/components/Forms/SequenceEditForm.js +++ b/src/components/Forms/SequenceEditForm.js @@ -23,7 +23,7 @@ const SequenceEditForm = ({ return ( - + diff --git a/src/components/Forms/SequenceEditForm.test.js b/src/components/Forms/SequenceEditForm.test.js index af006f533..81d922ab2 100644 --- a/src/components/Forms/SequenceEditForm.test.js +++ b/src/components/Forms/SequenceEditForm.test.js @@ -18,6 +18,7 @@ describe('', () => { it('shows submit errors if there are any', () => { const message = 'Generic submit error' const props = { + name: 'name', handleSubmit: () => {}, pristine: false, submitting: false, @@ -41,6 +42,7 @@ describe('', () => { it('shows a spinner when submitting', () => { const props = { + name: 'name', handleSubmit: () => {}, pristine: false, submitting: true, @@ -71,6 +73,7 @@ describe('', () => { it('disables the submit button when pristine', () => { const props = { + name: 'name', handleSubmit: () => {}, pristine: true, submitting: false, @@ -96,6 +99,7 @@ describe('', () => { it('disables the submit button when submitting', () => { const props = { + name: 'name', handleSubmit: () => {}, pristine: false, submitting: true, diff --git a/src/components/JobTable/EditJobAction.js b/src/components/JobTable/EditJobAction.js index 9ad23c7c4..0b281654a 100644 --- a/src/components/JobTable/EditJobAction.js +++ b/src/components/JobTable/EditJobAction.js @@ -7,7 +7,7 @@ import history from '../../services/history' const EditJobAction = ({ id }) => ( history.push(`/edit/${id}`)} + onClick={() => history.push(`/job/${id}`)} label={i18n.t('Edit')} /> ) diff --git a/src/components/JobTable/EditJobAction.test.js b/src/components/JobTable/EditJobAction.test.js index 217bf9e94..afedc5149 100644 --- a/src/components/JobTable/EditJobAction.test.js +++ b/src/components/JobTable/EditJobAction.test.js @@ -17,6 +17,6 @@ describe('', () => { wrapper.find('a').simulate('click') - expect(history.push).toHaveBeenCalledWith('/edit/id') + expect(history.push).toHaveBeenCalledWith('/job/id') }) }) diff --git a/src/components/JobTable/ViewJobAction.js b/src/components/JobTable/ViewJobAction.js index 28ae062df..b8a39f800 100644 --- a/src/components/JobTable/ViewJobAction.js +++ b/src/components/JobTable/ViewJobAction.js @@ -7,7 +7,7 @@ import history from '../../services/history' const ViewJobAction = ({ id }) => ( history.push(`/view/${id}`)} + onClick={() => history.push(`/job/${id}`)} label={i18n.t('View')} /> ) diff --git a/src/components/JobTable/ViewJobAction.test.js b/src/components/JobTable/ViewJobAction.test.js index f284deb3c..75263ed1d 100644 --- a/src/components/JobTable/ViewJobAction.test.js +++ b/src/components/JobTable/ViewJobAction.test.js @@ -17,6 +17,6 @@ describe('', () => { wrapper.find('a').simulate('click') - expect(history.push).toHaveBeenCalledWith('/view/id') + expect(history.push).toHaveBeenCalledWith('/job/id') }) }) diff --git a/src/components/Routes/JobViewOrEdit.js b/src/components/Routes/JobViewOrEdit.js new file mode 100644 index 000000000..d2992e1f8 --- /dev/null +++ b/src/components/Routes/JobViewOrEdit.js @@ -0,0 +1,37 @@ +import React from 'react' +import { NoticeBox } from '@dhis2/ui' +import { useParams } from 'react-router-dom' +import i18n from '@dhis2/d2-i18n' +import { useJobById } from '../../hooks/jobs' +import { Spinner } from '../Spinner' +import JobEdit from '../../pages/JobEdit' +import JobView from '../../pages/JobView' + +const JobViewOrEdit = () => { + const { id } = useParams() + const { data, fetching, error } = useJobById(id) + + if (fetching) { + return + } + + if (error) { + return ( + + {i18n.t( + 'Something went wrong whilst loading the requested job. Make sure it has not been deleted and try refreshing the page.' + )} + + ) + } + + const { configurable } = data + + if (configurable) { + return + } else { + return + } +} + +export default JobViewOrEdit diff --git a/src/components/Routes/JobViewOrEdit.test.js b/src/components/Routes/JobViewOrEdit.test.js new file mode 100644 index 000000000..c1a4cfaab --- /dev/null +++ b/src/components/Routes/JobViewOrEdit.test.js @@ -0,0 +1,88 @@ +import React from 'react' +import { shallow } from 'enzyme' +import { useParams } from 'react-router-dom' +import { useJobById } from '../../hooks/jobs' +import JobViewOrEdit from './JobViewOrEdit' + +jest.mock('react-router-dom', () => ({ + useParams: jest.fn(), +})) + +jest.mock('../../hooks/jobs', () => ({ + useJobById: jest.fn(), +})) + +describe('', () => { + it('renders a spinner when loading the requested job', () => { + const id = 'id' + + useParams.mockImplementation(() => id) + useJobById.mockImplementation(() => ({ fetching: true })) + + const wrapper = shallow() + const spinner = wrapper.find('Spinner') + + expect(spinner).toHaveLength(1) + }) + + it('renders errors encountered during fetching', () => { + const id = 'id' + + useParams.mockImplementation(() => id) + useJobById.mockImplementation(() => ({ + fetching: false, + error: new Error('Something went wrong'), + })) + + const wrapper = shallow() + const noticebox = wrapper.find('NoticeBox') + + expect(noticebox).toHaveLength(1) + }) + + it('renders JobEdit if the job is configurable', () => { + const id = 'id' + + useParams.mockImplementation(() => id) + useJobById.mockImplementation(() => ({ + fetching: false, + error: undefined, + data: { + name: 'name', + created: '', + lastExecutedStatus: '', + lastExecuted: '', + configurable: true, + }, + })) + + const wrapper = shallow() + const component = wrapper.find('JobEdit') + + expect(component).toHaveLength(1) + }) + + it('renders JobView if the job is not configurable', () => { + const id = 'id' + + useParams.mockImplementation(() => id) + useJobById.mockImplementation(() => ({ + fetching: false, + error: undefined, + data: { + name: 'name', + created: '', + lastExecutedStatus: '', + lastExecuted: '', + jobType: '', + cronExpression: '', + configurable: false, + }, + })) + + const wrapper = shallow() + const component = wrapper.find('JobView') + + expect(component).toHaveLength(1) + }) +}) diff --git a/src/components/Routes/Routes.js b/src/components/Routes/Routes.js index 31dd3d92c..a0262d19e 100644 --- a/src/components/Routes/Routes.js +++ b/src/components/Routes/Routes.js @@ -1,22 +1,25 @@ import React from 'react' -import { Route } from 'react-router-dom' +import { Route, Switch, Redirect } from 'react-router-dom' import { Router } from 'react-router' import JobList from '../../pages/JobList' -import JobEdit from '../../pages/JobEdit' -import JobView from '../../pages/JobView' import JobAdd from '../../pages/JobAdd' import SequenceAdd from '../../pages/SequenceAdd' import SequenceEdit from '../../pages/SequenceEdit' import history from '../../services/history' +import JobViewOrEdit from './JobViewOrEdit' const Routes = () => ( - - - - - - + + + + + + + + + + ) diff --git a/src/hooks/jobs/use-jobs.js b/src/hooks/jobs/use-jobs.js index a35cfcb52..83cd0f4ae 100644 --- a/src/hooks/jobs/use-jobs.js +++ b/src/hooks/jobs/use-jobs.js @@ -7,6 +7,7 @@ const query = { params: { fields: [ 'created', + 'configurable', 'cronExpression', 'delay', 'id', diff --git a/src/pages/JobEdit/JobEdit.js b/src/pages/JobEdit/JobEdit.js index b5362c851..2dbb3d1f6 100644 --- a/src/pages/JobEdit/JobEdit.js +++ b/src/pages/JobEdit/JobEdit.js @@ -1,35 +1,16 @@ import React from 'react' -import { Card, IconInfo16, NoticeBox } from '@dhis2/ui' -import { useParams } from 'react-router-dom' +import { Card, IconInfo16 } from '@dhis2/ui' +import PropTypes from 'prop-types' import i18n from '@dhis2/d2-i18n' -import { useJobById } from '../../hooks/jobs' import { JobEditFormContainer } from '../../components/Forms' import { JobDetails } from '../../components/JobDetails' -import { Spinner } from '../../components/Spinner' import styles from './JobEdit.module.css' const infoLink = 'https://docs.dhis2.org/en/use/user-guides/dhis-core-version-236/maintaining-the-system/scheduling.html' -const JobEdit = () => { - const { id } = useParams() - const { data, fetching, error } = useJobById(id) - - if (fetching) { - return - } - - if (error) { - return ( - - {i18n.t( - 'Something went wrong whilst loading the requested job. Make sure it has not been deleted and try refreshing the page.' - )} - - ) - } - - const { name, created, lastExecutedStatus, lastExecuted } = data +const JobEdit = ({ job }) => { + const { name, created, lastExecutedStatus, lastExecuted } = job return ( @@ -65,10 +46,21 @@ const JobEdit = () => { lastExecuted={lastExecuted} /> - + ) } +const { shape, string } = PropTypes + +JobEdit.propTypes = { + job: shape({ + name: string.isRequired, + created: string.isRequired, + lastExecutedStatus: string.isRequired, + lastExecuted: string.isRequired, + }).isRequired, +} + export default JobEdit diff --git a/src/pages/JobEdit/JobEdit.test.js b/src/pages/JobEdit/JobEdit.test.js index 6afc7dec5..09d9b88ae 100644 --- a/src/pages/JobEdit/JobEdit.test.js +++ b/src/pages/JobEdit/JobEdit.test.js @@ -1,61 +1,16 @@ import React from 'react' import { shallow } from 'enzyme' -import { useParams } from 'react-router-dom' -import { useJobById } from '../../hooks/jobs' import JobEdit from './JobEdit' -jest.mock('react-router-dom', () => ({ - useParams: jest.fn(), -})) - -jest.mock('../../hooks/jobs', () => ({ - useJobById: jest.fn(), -})) - describe('', () => { - it('renders a spinner when loading the requested job', () => { - const id = 'id' - - useParams.mockImplementation(() => id) - useJobById.mockImplementation(() => ({ fetching: true })) - - const wrapper = shallow() - const spinner = wrapper.find('Spinner') - - expect(spinner).toHaveLength(1) - }) - - it('renders errors encountered during fetching', () => { - const id = 'id' - - useParams.mockImplementation(() => id) - useJobById.mockImplementation(() => ({ - fetching: false, - error: new Error('Something went wrong'), - })) - - const wrapper = shallow() - const noticebox = wrapper.find('NoticeBox') - - expect(noticebox).toHaveLength(1) - }) - - it('renders without errors when loading has completed', () => { - const id = 'id' - - useParams.mockImplementation(() => id) - useJobById.mockImplementation(() => ({ - fetching: false, - error: undefined, - data: { - name: '', - created: '', - lastExecutedStatus: '', - lastExecuted: '', - }, - })) - - const wrapper = shallow() + it('renders without errors', () => { + const job = { + name: '', + created: '', + lastExecutedStatus: '', + lastExecuted: '', + } + const wrapper = shallow() const jobform = wrapper.find('JobEditFormContainer') expect(jobform).toHaveLength(1) diff --git a/src/pages/JobList/JobList.js b/src/pages/JobList/JobList.js index 22bc814b4..0eb8ba9c7 100644 --- a/src/pages/JobList/JobList.js +++ b/src/pages/JobList/JobList.js @@ -74,7 +74,9 @@ const JobList = () => { setShowSystemJobs(checked) }} /> - {i18n.t('New job')} + + {i18n.t('New job')} + diff --git a/src/pages/JobView/JobView.js b/src/pages/JobView/JobView.js index 60e64730a..2c2578ffd 100644 --- a/src/pages/JobView/JobView.js +++ b/src/pages/JobView/JobView.js @@ -1,5 +1,5 @@ import React from 'react' -import { useParams } from 'react-router-dom' +import PropTypes from 'prop-types' import { Card, IconInfo16, @@ -7,38 +7,18 @@ import { SingleSelectField, SingleSelectOption, InputField, - NoticeBox, } from '@dhis2/ui' import i18n from '@dhis2/d2-i18n' import { LinkButton } from '../../components/LinkButton' import { JobDetails } from '../../components/JobDetails' -import { useJobById } from '../../hooks/jobs' import translateCron from '../../services/translate-cron' import { jobTypesMap } from '../../services/server-translations' -import { Spinner } from '../../components/Spinner' import styles from './JobView.module.css' const infoLink = 'https://docs.dhis2.org/en/use/user-guides/dhis-core-version-236/maintaining-the-system/scheduling.html' -const JobView = () => { - const { id } = useParams() - const { data, fetching, error } = useJobById(id) - - if (fetching) { - return - } - - if (error) { - return ( - - {i18n.t( - 'Something went wrong whilst loading the requested job. Make sure it has not been deleted and try refreshing the page.' - )} - - ) - } - +const JobView = ({ job }) => { const { name, created, @@ -46,7 +26,7 @@ const JobView = () => { lastExecuted, jobType, cronExpression, - } = data + } = job return ( @@ -122,4 +102,17 @@ const JobView = () => { ) } +const { shape, string } = PropTypes + +JobView.propTypes = { + job: shape({ + name: string.isRequired, + created: string.isRequired, + lastExecutedStatus: string.isRequired, + lastExecuted: string.isRequired, + jobType: string.isRequired, + cronExpression: string.isRequired, + }).isRequired, +} + export default JobView diff --git a/src/pages/JobView/JobView.test.js b/src/pages/JobView/JobView.test.js index 71728b4b8..d2f028cf9 100644 --- a/src/pages/JobView/JobView.test.js +++ b/src/pages/JobView/JobView.test.js @@ -1,63 +1,18 @@ import React from 'react' import { shallow } from 'enzyme' -import { useParams } from 'react-router-dom' -import { useJobById } from '../../hooks/jobs' import JobView from './JobView' -jest.mock('react-router-dom', () => ({ - useParams: jest.fn(), -})) - -jest.mock('../../hooks/jobs', () => ({ - useJobById: jest.fn(), -})) - describe('', () => { - it('renders a spinner when loading the requested job', () => { - const id = 'id' - - useParams.mockImplementation(() => id) - useJobById.mockImplementation(() => ({ fetching: true })) - - const wrapper = shallow() - const spinner = wrapper.find('Spinner') - - expect(spinner).toHaveLength(1) - }) - - it('renders errors encountered during fetching', () => { - const id = 'id' - - useParams.mockImplementation(() => id) - useJobById.mockImplementation(() => ({ - fetching: false, - error: new Error('Something went wrong'), - })) - - const wrapper = shallow() - const noticebox = wrapper.find('NoticeBox') - - expect(noticebox).toHaveLength(1) - }) - - it('renders without errors when loading has completed', () => { - const id = 'id' - - useParams.mockImplementation(() => id) - useJobById.mockImplementation(() => ({ - fetching: false, - error: undefined, - data: { - name: '', - created: '', - lastExecutedStatus: '', - lastExecuted: '', - jobType: 'DATA_INTEGRITY', - cronExpression: '', - }, - })) - - const wrapper = shallow() + it('renders without errors', () => { + const job = { + name: '', + created: '', + lastExecutedStatus: '', + lastExecuted: '', + jobType: 'DATA_INTEGRITY', + cronExpression: '', + } + const wrapper = shallow() const jobform = wrapper.find('JobDetails') expect(jobform).toHaveLength(1)