From d1dd8e1fcb39449ae39b3202126385a75d47b03c Mon Sep 17 00:00:00 2001 From: ismay Date: Wed, 17 Feb 2021 10:28:48 +0100 Subject: [PATCH] refactor(store): use hooks instead of selectors --- src/components/FormFields/JobTypeField.js | 8 +- .../FormFields/LabeledOptionsField.js | 7 +- src/components/FormFields/ParameterFields.js | 7 +- src/components/FormFields/ScheduleField.js | 9 +- .../FormFields/SkipTableTypesField.js | 7 +- src/components/Forms/JobEditFormContainer.js | 9 +- src/components/JobTable/DeleteJobAction.js | 7 +- src/components/Modal/RunJobModal.js | 7 +- src/components/Store/hooks.js | 58 +++++- src/components/Store/hooks.test.js | 175 +++++++++++++++++- src/components/Store/index.js | 3 +- src/components/Store/selectors.js | 49 ----- src/components/Store/selectors.test.js | 150 --------------- src/components/Switches/ToggleJobSwitch.js | 7 +- src/pages/JobEdit/JobEditContainer.js | 7 +- src/pages/JobList/JobListContainer.js | 2 +- 16 files changed, 251 insertions(+), 261 deletions(-) delete mode 100644 src/components/Store/selectors.js delete mode 100644 src/components/Store/selectors.test.js diff --git a/src/components/FormFields/JobTypeField.js b/src/components/FormFields/JobTypeField.js index 3105551ac..e2ba3e2d2 100644 --- a/src/components/FormFields/JobTypeField.js +++ b/src/components/FormFields/JobTypeField.js @@ -1,4 +1,4 @@ -import React, { useContext } from 'react' +import React from 'react' import { ReactFinalForm, SingleSelectFieldFF, @@ -7,7 +7,7 @@ import { string, } from '@dhis2/ui' import i18n from '@dhis2/d2-i18n' -import { StoreContext, selectors } from '../Store' +import { hooks } from '../Store' import { jobTypesMap } from '../../services/server-translations' const { Field } = ReactFinalForm @@ -17,9 +17,7 @@ export const FIELD_NAME = 'jobType' const VALIDATOR = composeValidators(string, hasValue) const JobTypeField = () => { - const store = useContext(StoreContext) - const jobTypes = selectors.getJobTypesStore(store) - + const jobTypes = hooks.useAllJobTypes() const options = jobTypes.map(({ jobType }) => ({ value: jobType, label: jobTypesMap[jobType], diff --git a/src/components/FormFields/LabeledOptionsField.js b/src/components/FormFields/LabeledOptionsField.js index 0273b6556..1e39a46e9 100644 --- a/src/components/FormFields/LabeledOptionsField.js +++ b/src/components/FormFields/LabeledOptionsField.js @@ -1,15 +1,14 @@ -import React, { useContext } from 'react' +import React from 'react' import { PropTypes } from '@dhis2/prop-types' import { MultiSelectFieldFF, ReactFinalForm, MultiSelectField } from '@dhis2/ui' import i18n from '@dhis2/d2-i18n' -import { StoreContext, selectors } from '../Store' +import { hooks } from '../Store' const { Field } = ReactFinalForm // A labeled options field has options that have both an id and a label. const LabeledOptionsField = ({ label, name, parameterName }) => { - const store = useContext(StoreContext) - const options = selectors.getParameterOptions(store, parameterName) + const options = hooks.useParameterOptions(parameterName) if (options.length === 0) { return ( diff --git a/src/components/FormFields/ParameterFields.js b/src/components/FormFields/ParameterFields.js index 6322b76c9..9db0ed2bd 100644 --- a/src/components/FormFields/ParameterFields.js +++ b/src/components/FormFields/ParameterFields.js @@ -1,8 +1,8 @@ -import React, { useContext } from 'react' +import React from 'react' import i18n from '@dhis2/d2-i18n' import { PropTypes } from '@dhis2/prop-types' import { ReactFinalForm, InputFieldFF, SwitchFieldFF, Box } from '@dhis2/ui' -import { selectors, StoreContext } from '../Store' +import { hooks } from '../Store' import { formatToString } from './formatters' import SkipTableTypesField from './SkipTableTypesField' import LabeledOptionsField from './LabeledOptionsField' @@ -15,8 +15,7 @@ const FIELD_NAME = 'jobParameters' // Renders all parameters for a given jobtype const ParameterFields = ({ jobType }) => { - const store = useContext(StoreContext) - const parameters = selectors.getJobTypeParameters(store, jobType) + const parameters = hooks.useJobTypeParameters(jobType) if (parameters.length === 0) { return null diff --git a/src/components/FormFields/ScheduleField.js b/src/components/FormFields/ScheduleField.js index 1d35ae1a9..6d6ebd835 100644 --- a/src/components/FormFields/ScheduleField.js +++ b/src/components/FormFields/ScheduleField.js @@ -1,13 +1,12 @@ -import React, { useContext } from 'react' +import React from 'react' import { PropTypes } from '@dhis2/prop-types' -import { StoreContext, selectors } from '../Store' +import { hooks } from '../Store' import CronField from './CronField' import DelayField from './DelayField' const ScheduleField = ({ jobType }) => { - const store = useContext(StoreContext) - const currentJob = selectors.getJobType(store, jobType) - const schedulingType = currentJob.schedulingType + const currentJobType = hooks.useJobType(jobType) + const schedulingType = currentJobType.schedulingType switch (schedulingType) { case 'CRON': diff --git a/src/components/FormFields/SkipTableTypesField.js b/src/components/FormFields/SkipTableTypesField.js index 3dc56c98b..4b079f983 100644 --- a/src/components/FormFields/SkipTableTypesField.js +++ b/src/components/FormFields/SkipTableTypesField.js @@ -1,15 +1,14 @@ -import React, { useContext } from 'react' +import React from 'react' import { PropTypes } from '@dhis2/prop-types' import i18n from '@dhis2/d2-i18n' import { MultiSelectField, ReactFinalForm, MultiSelectFieldFF } from '@dhis2/ui' -import { StoreContext, selectors } from '../Store' +import { hooks } from '../Store' import { analyticsTableTypes } from '../../services/server-translations' const { Field } = ReactFinalForm const SkipTableTypesField = ({ label, name, parameterName }) => { - const store = useContext(StoreContext) - const options = selectors.getParameterOptions(store, parameterName) + const options = hooks.useParameterOptions(parameterName) if (options.length === 0) { return ( diff --git a/src/components/Forms/JobEditFormContainer.js b/src/components/Forms/JobEditFormContainer.js index e54dc4b94..6e40562ba 100644 --- a/src/components/Forms/JobEditFormContainer.js +++ b/src/components/Forms/JobEditFormContainer.js @@ -1,9 +1,9 @@ -import React, { useContext } from 'react' +import React from 'react' import { PropTypes } from '@dhis2/prop-types' import { ReactFinalForm } from '@dhis2/ui' import { useParams } from 'react-router-dom' import { useUpdateJob } from '../../hooks/jobs' -import { StoreContext, selectors } from '../Store' +import { hooks } from '../Store' import JobEditForm from './JobEditForm' const { Form } = ReactFinalForm @@ -26,9 +26,8 @@ const initialFields = [ const JobEditFormContainer = ({ setIsPristine }) => { const { id } = useParams() const [updateJob] = useUpdateJob({ id }) - const store = useContext(StoreContext) - const refetchJobs = selectors.getRefetchJobs(store) - const job = selectors.getJobById(store, id) + const refetchJobs = hooks.useRefetchJobs() + const job = hooks.useJob(id) // Creating an object with just the values we want to use as initial values const initialValues = initialFields.reduce((filtered, key) => { diff --git a/src/components/JobTable/DeleteJobAction.js b/src/components/JobTable/DeleteJobAction.js index 05e1baa57..5afc9c99b 100644 --- a/src/components/JobTable/DeleteJobAction.js +++ b/src/components/JobTable/DeleteJobAction.js @@ -1,14 +1,13 @@ -import React, { useState, useContext } from 'react' +import React, { useState } from 'react' import { PropTypes } from '@dhis2/prop-types' import { MenuItem } from '@dhis2/ui' import i18n from '@dhis2/d2-i18n' import { DeleteJobModal } from '../Modal' -import { StoreContext, selectors } from '../Store' +import { hooks } from '../Store' const DeleteJobAction = ({ id }) => { const [showModal, setShowModal] = useState(false) - const store = useContext(StoreContext) - const refetchJobs = selectors.getRefetchJobs(store) + const refetchJobs = hooks.useRefetchJobs() return ( diff --git a/src/components/Modal/RunJobModal.js b/src/components/Modal/RunJobModal.js index ff5189a7f..e37481717 100644 --- a/src/components/Modal/RunJobModal.js +++ b/src/components/Modal/RunJobModal.js @@ -1,4 +1,4 @@ -import React, { useContext } from 'react' +import React from 'react' import { PropTypes } from '@dhis2/prop-types' import { useDataEngine } from '@dhis2/app-runtime' import { @@ -9,7 +9,7 @@ import { ButtonStrip, } from '@dhis2/ui' import i18n from '@dhis2/d2-i18n' -import { StoreContext, selectors } from '../Store' +import { hooks } from '../Store' const RunJobModal = ({ id, hideModal }) => { const engine = useDataEngine() @@ -19,8 +19,7 @@ const RunJobModal = ({ id, hideModal }) => { }, } const runJob = () => engine.query(query) - const store = useContext(StoreContext) - const refetchJobs = selectors.getRefetchJobs(store) + const refetchJobs = hooks.useRefetchJobs() return ( diff --git a/src/components/Store/hooks.js b/src/components/Store/hooks.js index fd7aea0eb..1641efab9 100644 --- a/src/components/Store/hooks.js +++ b/src/components/Store/hooks.js @@ -1,13 +1,29 @@ import { useContext } from 'react' import StoreContext from './StoreContext' -export const useJobs = () => { +export const useAllJobs = () => { const store = useContext(StoreContext) return store.jobs } +export const useAllParameterOptions = () => { + const store = useContext(StoreContext) + return store.parameterOptions +} + +export const useAllJobTypes = () => { + const store = useContext(StoreContext) + return store.jobTypes +} + +export const useRefetchJobs = () => { + const store = useContext(StoreContext) + return store.refetchJobs +} + /** - * This state is used in the job list, but kept in the + * The state for the job filter and showing system + * jobs is used in the job list, but kept in the * store since it has to persist after a refetch. */ export const useJobFilter = () => { @@ -15,10 +31,6 @@ export const useJobFilter = () => { return store.jobFilter } -/** - * This state is used in the job list, but kept in the - * store since it has to persist after a refetch. - */ export const useShowSystemJobs = () => { const store = useContext(StoreContext) return store.showSystemJobs @@ -30,10 +42,10 @@ export const useShowSystemJobs = () => { * string and the show system jobs toggle from the store * state. */ -export const useListJobs = () => { +export const useJobListJobs = () => { const [jobFilter] = useJobFilter() const [showSystemJobs] = useShowSystemJobs() - const jobs = useJobs() + const jobs = useAllJobs() // Filter jobs by the current jobFilter const applyJobFilter = job => @@ -46,3 +58,33 @@ export const useListJobs = () => { return jobs.filter(applyJobFilter).filter(applyShowSystemJobs) } + +// Finds a job by id +export const useJob = id => { + const jobs = useAllJobs() + return jobs.find(job => job.id === id) +} + +// Finds a jobType by the jobType string +export const useJobType = jobType => { + const jobTypes = useAllJobTypes() + return jobTypes.find(job => job.jobType === jobType) +} + +// Returns an array with all parameters for a certain jobType +export const useJobTypeParameters = jobType => { + const selectedJobType = useJobType(jobType) + const hasParameters = 'jobParameters' in selectedJobType + + if (!hasParameters) { + return [] + } + + return selectedJobType.jobParameters +} + +// Returns the parameter options for a given parameter +export const useParameterOptions = parameter => { + const parameterOptions = useAllParameterOptions() + return parameterOptions[parameter] +} diff --git a/src/components/Store/hooks.test.js b/src/components/Store/hooks.test.js index 6737f6dca..6048935be 100644 --- a/src/components/Store/hooks.test.js +++ b/src/components/Store/hooks.test.js @@ -1,9 +1,21 @@ import React from 'react' import { renderHook } from '@testing-library/react-hooks' -import { useJobFilter, useShowSystemJobs, useJobs, useListJobs } from './hooks' +import { + useAllJobs, + useAllParameterOptions, + useAllJobTypes, + useRefetchJobs, + useJobFilter, + useShowSystemJobs, + useJobListJobs, + useJob, + useJobType, + useJobTypeParameters, + useParameterOptions, +} from './hooks' import StoreContext from './StoreContext' -describe('useJobs', () => { +describe('useAllJobs', () => { it('should return the jobs part of the store', () => { const jobs = 'jobs' const store = { @@ -14,12 +26,67 @@ describe('useJobs', () => { {children} ) - const { result } = renderHook(() => useJobs(), { wrapper }) + const { result } = renderHook(() => useAllJobs(), { wrapper }) expect(result.current).toBe(jobs) }) }) +describe('useAllParameterOptions', () => { + it('should return the parameterOptions part of the store', () => { + const parameterOptions = 'parameterOptions' + const store = { + parameterOptions, + } + const wrapper = ({ children }) => ( + + {children} + + ) + const { result } = renderHook(() => useAllParameterOptions(), { + wrapper, + }) + + expect(result.current).toBe(parameterOptions) + }) +}) + +describe('useAllJobTypes', () => { + it('should return the jobTypes part of the store', () => { + const jobTypes = 'jobTypes' + const store = { + jobTypes, + } + const wrapper = ({ children }) => ( + + {children} + + ) + const { result } = renderHook(() => useAllJobTypes(), { wrapper }) + + expect(result.current).toBe(jobTypes) + }) +}) + +describe('useRefetchJobs', () => { + it('should return the refetchJobs part of the store', () => { + const refetchJobs = 'refetchJobs' + const store = { + refetchJobs, + } + const wrapper = ({ children }) => ( + + {children} + + ) + const { result } = renderHook(() => useRefetchJobs(), { + wrapper, + }) + + expect(result.current).toBe(refetchJobs) + }) +}) + describe('useJobFilter', () => { it('should return the jobFilter part of the store', () => { const jobFilter = 'jobFilter' @@ -54,7 +121,7 @@ describe('useShowSystemJobs', () => { }) }) -describe('useListJobs', () => { +describe('useJobListJobs', () => { const user1 = { name: 'User One', configurable: true, @@ -83,7 +150,7 @@ describe('useListJobs', () => { {children} ) - const { result } = renderHook(() => useListJobs(), { wrapper }) + const { result } = renderHook(() => useJobListJobs(), { wrapper }) expect(result.current).toEqual(expect.arrayContaining([user1, user2])) }) @@ -99,7 +166,7 @@ describe('useListJobs', () => { {children} ) - const { result } = renderHook(() => useListJobs(), { wrapper }) + const { result } = renderHook(() => useJobListJobs(), { wrapper }) expect(result.current).toEqual( expect.arrayContaining([user1, user2, system1, system2]) @@ -117,7 +184,7 @@ describe('useListJobs', () => { {children} ) - const { result } = renderHook(() => useListJobs(), { wrapper }) + const { result } = renderHook(() => useJobListJobs(), { wrapper }) expect(result.current).toEqual(expect.arrayContaining([user1])) }) @@ -133,8 +200,100 @@ describe('useListJobs', () => { {children} ) - const { result } = renderHook(() => useListJobs(), { wrapper }) + const { result } = renderHook(() => useJobListJobs(), { wrapper }) expect(result.current).toEqual(expect.arrayContaining([user1, system1])) }) }) + +describe('useJob', () => { + it('should return the requested job', () => { + const expected = { id: 'one' } + const store = { + jobs: [expected, { id: 'two' }, { id: 'three' }], + } + const wrapper = ({ children }) => ( + + {children} + + ) + const { result } = renderHook(() => useJob('one'), { wrapper }) + + expect(result.current).toEqual(expected) + }) +}) + +describe('useJobType', () => { + it('should return the requested jobType', () => { + const expected = { jobType: 'one' } + const store = { + jobTypes: [expected, { jobType: 'two' }, { jobType: 'three' }], + } + const wrapper = ({ children }) => ( + + {children} + + ) + const { result } = renderHook(() => useJobType('one'), { wrapper }) + + expect(result.current).toEqual(expected) + }) +}) + +describe('useJobTypeParameters', () => { + it('should return an array with parameters if there are any', () => { + const expected = 'jobParameters' + const store = { + jobTypes: [{ jobType: 'jobType', jobParameters: expected }], + } + const wrapper = ({ children }) => ( + + {children} + + ) + const { result } = renderHook(() => useJobTypeParameters('jobType'), { + wrapper, + }) + + expect(result.current).toEqual(expected) + }) + + it('should return an empty array if there are no parameters', () => { + const store = { + jobTypes: [{ jobType: 'jobType' }], + } + const wrapper = ({ children }) => ( + + {children} + + ) + const { result } = renderHook(() => useJobTypeParameters('jobType'), { + wrapper, + }) + + expect(result.current).toEqual([]) + }) +}) + +describe('useParameterOptions', () => { + it('should return the requested parameterOptions', () => { + const expected = 'parameterOption' + const store = { + parameterOptions: { + one: expected, + two: 'two', + three: 'three', + }, + } + const wrapper = ({ children }) => ( + + {children} + + ) + const { result } = renderHook(() => useParameterOptions('one'), { + wrapper, + }) + + expect(result.current).toEqual(expected) + }) +}) diff --git a/src/components/Store/index.js b/src/components/Store/index.js index d29c23be3..682b049af 100644 --- a/src/components/Store/index.js +++ b/src/components/Store/index.js @@ -1,6 +1,5 @@ import Store from './Store' import StoreContext from './StoreContext' -import * as selectors from './selectors' import * as hooks from './hooks' -export { Store, StoreContext, selectors, hooks } +export { Store, StoreContext, hooks } diff --git a/src/components/Store/selectors.js b/src/components/Store/selectors.js deleted file mode 100644 index e4e9cc4e3..000000000 --- a/src/components/Store/selectors.js +++ /dev/null @@ -1,49 +0,0 @@ -export const getJobsStore = store => store.jobs -export const getJobTypesStore = store => store.jobTypes -export const getParameterOptionsStore = store => store.parameterOptions -export const getRefetchJobs = store => store.refetchJobs - -/** - * Jobs - */ - -// Returns the first job that matches the provided id -export const getJobById = (store, id) => { - const jobs = getJobsStore(store) - - return jobs.find(job => job.id === id) -} - -/** - * Job types - */ - -// Find a jobType entity for a given jobType -export const getJobType = (store, jobType) => { - const jobTypes = getJobTypesStore(store) - - return jobTypes.find(job => job.jobType === jobType) -} - -// Returns an array with all parameters for a certain jobType -export const getJobTypeParameters = (store, jobType) => { - const selectedJobType = getJobType(store, jobType) - const hasParameters = 'jobParameters' in selectedJobType - - if (!hasParameters) { - return [] - } - - return selectedJobType.jobParameters -} - -/** - * Parameter options - */ - -// Returns the parameter options for a given parameter -export const getParameterOptions = (store, parameter) => { - const parameterOptions = getParameterOptionsStore(store) - - return parameterOptions[parameter] -} diff --git a/src/components/Store/selectors.test.js b/src/components/Store/selectors.test.js deleted file mode 100644 index e1ea2cd63..000000000 --- a/src/components/Store/selectors.test.js +++ /dev/null @@ -1,150 +0,0 @@ -import { - getJobsStore, - getJobTypesStore, - getParameterOptionsStore, - getRefetchJobs, - getJobById, - getJobType, - getJobTypeParameters, - getParameterOptions, -} from './selectors' - -describe('root store selectors', () => { - describe('getJobsStore', () => { - it('returns the jobs part of the store', () => { - const store = { - jobs: 'jobs', - } - const actual = getJobsStore(store) - - expect(actual).toEqual('jobs') - }) - }) - - describe('getJobTypesStore', () => { - it('returns the job types part of the store', () => { - const store = { - jobTypes: 'jobTypes', - } - const actual = getJobTypesStore(store) - - expect(actual).toEqual('jobTypes') - }) - }) - - describe('getParameterOptionsStore', () => { - it('returns the parameter options part of the store', () => { - const store = { - parameterOptions: 'parameterOptions', - } - const actual = getParameterOptionsStore(store) - - expect(actual).toEqual('parameterOptions') - }) - }) - - describe('getRefetchJobs', () => { - it('returns refetchJobs', () => { - const store = { - refetchJobs: 'refetchJobs', - } - const actual = getRefetchJobs(store) - - expect(actual).toEqual('refetchJobs') - }) - }) -}) - -describe('jobs', () => { - describe('getJobById', () => { - it('returns the first job that matches the provided id', () => { - const store = { - jobs: [ - { - id: 'one', - name: 'one', - }, - { - id: 'two', - name: 'two', - }, - ], - } - const expected = { - id: 'one', - name: 'one', - } - - const actual = getJobById(store, 'one') - - expect(actual).toEqual(expected) - }) - }) -}) - -describe('jobTypes', () => { - describe('getJobType', () => { - it('returns the requested job type', () => { - const store = { - jobTypes: [ - { jobType: 'one' }, - { jobType: 'two' }, - { jobType: 'three' }, - ], - } - const jobType = 'one' - const expected = { jobType: 'one' } - const actual = getJobType(store, jobType) - - expect(actual).toEqual(expected) - }) - }) - - describe('getJobTypeParameters', () => { - it('returns an array with all parameters for the requested job type', () => { - const store = { - jobTypes: [ - { jobType: 'one', jobParameters: ['parameter one'] }, - { jobType: 'two', jobParameters: ['parameter two'] }, - { jobType: 'three', jobParameters: ['parameter three'] }, - ], - } - const jobType = 'one' - const expected = ['parameter one'] - const actual = getJobTypeParameters(store, jobType) - - expect(actual).toEqual(expected) - }) - - it('returns an empty array for a job type that has no job parameters', () => { - const store = { - jobTypes: [ - { jobType: 'one' }, - { jobType: 'two' }, - { jobType: 'three' }, - ], - } - const jobType = 'one' - const expected = [] - const actual = getJobTypeParameters(store, jobType) - - expect(actual).toEqual(expected) - }) - }) -}) - -describe('parameterOptions', () => { - describe('getParameterOptions', () => { - it('returns the parameterOptions for a given parameter', () => { - const store = { - parameterOptions: { - one: 'one', - two: 'two', - }, - } - const actual = getParameterOptions(store, 'one') - - expect(actual).toEqual('one') - }) - }) -}) diff --git a/src/components/Switches/ToggleJobSwitch.js b/src/components/Switches/ToggleJobSwitch.js index e965de7be..aafcfe032 100644 --- a/src/components/Switches/ToggleJobSwitch.js +++ b/src/components/Switches/ToggleJobSwitch.js @@ -1,8 +1,8 @@ -import React, { useContext } from 'react' +import React from 'react' import { PropTypes } from '@dhis2/prop-types' import { Switch } from '@dhis2/ui' import { useDataMutation } from '@dhis2/app-runtime' -import { StoreContext, selectors } from '../Store' +import { hooks } from '../Store' /* istanbul ignore next */ const mutation = { @@ -15,8 +15,7 @@ const mutation = { const ToggleJobSwitch = ({ id, checked }) => { const [toggleJob, { loading }] = useDataMutation(mutation) - const store = useContext(StoreContext) - const refetchJobs = selectors.getRefetchJobs(store) + const refetchJobs = hooks.useRefetchJobs() const enabled = !checked return ( diff --git a/src/pages/JobEdit/JobEditContainer.js b/src/pages/JobEdit/JobEditContainer.js index 169db4fa4..9cc40037d 100644 --- a/src/pages/JobEdit/JobEditContainer.js +++ b/src/pages/JobEdit/JobEditContainer.js @@ -1,13 +1,12 @@ -import React, { useState, useContext } from 'react' +import React, { useState } from 'react' import { useParams } from 'react-router-dom' -import { StoreContext, selectors } from '../../components/Store' +import { hooks } from '../../components/Store' import JobEdit from './JobEdit' const JobEditContainer = () => { - const store = useContext(StoreContext) const [isPristine, setIsPristine] = useState(true) const { id } = useParams() - const job = selectors.getJobById(store, id) + const job = hooks.useJob(id) return ( { const [jobFilter, setJobFilter] = hooks.useJobFilter() const [showSystemJobs, setShowSystemJobs] = hooks.useShowSystemJobs() - const jobs = hooks.useListJobs() + const jobs = hooks.useJobListJobs() return (