From 539ea7704ab07cb2e2c6c8fdb6bfcdb277e4bec9 Mon Sep 17 00:00:00 2001 From: ismay Date: Mon, 15 Feb 2021 10:21:43 +0100 Subject: [PATCH] fix(list): move list state to store --- package.json | 1 + src/.eslintrc.js | 1 + src/components/Store/Store.js | 9 +- src/components/Store/hooks.js | 48 +++++++ src/components/Store/hooks.test.js | 140 +++++++++++++++++++++ src/components/Store/index.js | 3 +- src/pages/JobList/JobListContainer.js | 18 +-- src/pages/JobList/JobListContainer.test.js | 91 +------------- src/pages/JobList/selectors.js | 6 - src/pages/JobList/selectors.test.js | 67 ---------- yarn.lock | 63 ++++++++++ 11 files changed, 270 insertions(+), 177 deletions(-) create mode 100644 src/components/Store/hooks.js create mode 100644 src/components/Store/hooks.test.js delete mode 100644 src/pages/JobList/selectors.js delete mode 100644 src/pages/JobList/selectors.test.js diff --git a/package.json b/package.json index 474b84b39..6b12432ce 100644 --- a/package.json +++ b/package.json @@ -29,6 +29,7 @@ "devDependencies": { "@dhis2/cli-app-scripts": "^4.0.9", "@dhis2/cli-style": "^7.1.0-alpha.9", + "@testing-library/react-hooks": "^5.0.3", "enzyme": "^3.10.0", "enzyme-adapter-react-16": "^1.15.6", "eslint-plugin-compat": "^3.9.0", diff --git a/src/.eslintrc.js b/src/.eslintrc.js index e664b3750..edb5565f1 100644 --- a/src/.eslintrc.js +++ b/src/.eslintrc.js @@ -61,6 +61,7 @@ module.exports = { files: ['*.test.js'], rules: { 'i18next/no-literal-string': 'off', + 'react/prop-types': 'off', }, }, ], diff --git a/src/components/Store/Store.js b/src/components/Store/Store.js index 2a1bf7609..6ac801103 100644 --- a/src/components/Store/Store.js +++ b/src/components/Store/Store.js @@ -1,4 +1,4 @@ -import React from 'react' +import React, { useState } from 'react' import { PropTypes } from '@dhis2/prop-types' import { CircularLoader, Layer, CenteredContent } from '@dhis2/ui' import { useDataQuery } from '@dhis2/app-runtime' @@ -62,6 +62,10 @@ const optionsQuery = { } const Store = ({ children }) => { + // State that should persist after a refetch + const jobFilterState = useState('') + const showSystemJobsState = useState(false) + const jobsFetch = useDataQuery(jobsQuery) const jobTypesFetch = useDataQuery(jobTypesQuery) const optionsFetch = useDataQuery(optionsQuery) @@ -98,7 +102,6 @@ const Store = ({ children }) => { predictors: { predictors }, predictorGroups: { predictorGroups }, } = optionsFetch.data - const parameterOptions = { skipTableTypes, validationRuleGroups, @@ -114,6 +117,8 @@ const Store = ({ children }) => { jobTypes, parameterOptions, refetchJobs: jobsFetch.refetch, + jobFilter: jobFilterState, + showSystemJobs: showSystemJobsState, }} > {children} diff --git a/src/components/Store/hooks.js b/src/components/Store/hooks.js new file mode 100644 index 000000000..fd7aea0eb --- /dev/null +++ b/src/components/Store/hooks.js @@ -0,0 +1,48 @@ +import { useContext } from 'react' +import StoreContext from './StoreContext' + +export const useJobs = () => { + const store = useContext(StoreContext) + return store.jobs +} + +/** + * This state is used in the job list, but kept in the + * store since it has to persist after a refetch. + */ +export const useJobFilter = () => { + const store = useContext(StoreContext) + 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 +} + +/** + * This hook returns the list of jobs that's shown in the + * job list route. The list is filtered by the job filter + * string and the show system jobs toggle from the store + * state. + */ +export const useListJobs = () => { + const [jobFilter] = useJobFilter() + const [showSystemJobs] = useShowSystemJobs() + const jobs = useJobs() + + // Filter jobs by the current jobFilter + const applyJobFilter = job => + job.name.toLowerCase().includes(jobFilter.toLowerCase()) + + // Filter jobs depending on the current showSystemJobs + const applyShowSystemJobs = job => + // Jobs that are configurable are user jobs + showSystemJobs ? true : job.configurable + + return jobs.filter(applyJobFilter).filter(applyShowSystemJobs) +} diff --git a/src/components/Store/hooks.test.js b/src/components/Store/hooks.test.js new file mode 100644 index 000000000..6737f6dca --- /dev/null +++ b/src/components/Store/hooks.test.js @@ -0,0 +1,140 @@ +import React from 'react' +import { renderHook } from '@testing-library/react-hooks' +import { useJobFilter, useShowSystemJobs, useJobs, useListJobs } from './hooks' +import StoreContext from './StoreContext' + +describe('useJobs', () => { + it('should return the jobs part of the store', () => { + const jobs = 'jobs' + const store = { + jobs, + } + const wrapper = ({ children }) => ( + + {children} + + ) + const { result } = renderHook(() => useJobs(), { wrapper }) + + expect(result.current).toBe(jobs) + }) +}) + +describe('useJobFilter', () => { + it('should return the jobFilter part of the store', () => { + const jobFilter = 'jobFilter' + const store = { + jobFilter, + } + const wrapper = ({ children }) => ( + + {children} + + ) + const { result } = renderHook(() => useJobFilter(), { wrapper }) + + expect(result.current).toBe(jobFilter) + }) +}) + +describe('useShowSystemJobs', () => { + it('should return the showSystemJobs part of the store', () => { + const showSystemJobs = 'showSystemJobs' + const store = { + showSystemJobs, + } + const wrapper = ({ children }) => ( + + {children} + + ) + const { result } = renderHook(() => useShowSystemJobs(), { wrapper }) + + expect(result.current).toBe(showSystemJobs) + }) +}) + +describe('useListJobs', () => { + const user1 = { + name: 'User One', + configurable: true, + } + const user2 = { + name: 'User Two', + configurable: true, + } + const system1 = { + name: 'System One', + configurable: false, + } + const system2 = { + name: 'System Two', + configurable: false, + } + + it('should return matching jobs when there is no filter and showSystemJobs is false', () => { + const store = { + jobFilter: [''], + showSystemJobs: [false], + jobs: [user1, user2, system1, system2], + } + const wrapper = ({ children }) => ( + + {children} + + ) + const { result } = renderHook(() => useListJobs(), { wrapper }) + + expect(result.current).toEqual(expect.arrayContaining([user1, user2])) + }) + + it('should return matching jobs when there is no filter and showSystemJobs is true', () => { + const store = { + jobFilter: [''], + showSystemJobs: [true], + jobs: [user1, user2, system1, system2], + } + const wrapper = ({ children }) => ( + + {children} + + ) + const { result } = renderHook(() => useListJobs(), { wrapper }) + + expect(result.current).toEqual( + expect.arrayContaining([user1, user2, system1, system2]) + ) + }) + + it('should return matching jobs when there is a filter and showSystemJobs is false', () => { + const store = { + jobFilter: ['One'], + showSystemJobs: [false], + jobs: [user1, user2, system1, system2], + } + const wrapper = ({ children }) => ( + + {children} + + ) + const { result } = renderHook(() => useListJobs(), { wrapper }) + + expect(result.current).toEqual(expect.arrayContaining([user1])) + }) + + it('should return matching jobs when there is a filter and showSystemJobs is true', () => { + const store = { + jobFilter: ['One'], + showSystemJobs: [true], + jobs: [user1, user2, system1, system2], + } + const wrapper = ({ children }) => ( + + {children} + + ) + const { result } = renderHook(() => useListJobs(), { wrapper }) + + expect(result.current).toEqual(expect.arrayContaining([user1, system1])) + }) +}) diff --git a/src/components/Store/index.js b/src/components/Store/index.js index f39716537..d29c23be3 100644 --- a/src/components/Store/index.js +++ b/src/components/Store/index.js @@ -1,5 +1,6 @@ import Store from './Store' import StoreContext from './StoreContext' import * as selectors from './selectors' +import * as hooks from './hooks' -export { Store, StoreContext, selectors } +export { Store, StoreContext, selectors, hooks } diff --git a/src/pages/JobList/JobListContainer.js b/src/pages/JobList/JobListContainer.js index dcb079915..0f96eb9cf 100644 --- a/src/pages/JobList/JobListContainer.js +++ b/src/pages/JobList/JobListContainer.js @@ -1,19 +1,11 @@ -import React, { useState, useContext } from 'react' -import { StoreContext, selectors } from '../../components/Store' +import React from 'react' +import { hooks } from '../../components/Store' import JobList from './JobList' -import { getJobsMatchingFilter, getUserJobs } from './selectors' const JobListContainer = () => { - const [showSystemJobs, setShowSystemJobs] = useState(false) - const [jobFilter, setJobFilter] = useState('') - const store = useContext(StoreContext) - const allJobs = selectors.getJobsStore(store) - - // Filter jobs by the jobFilter string - const filteredJobs = getJobsMatchingFilter(allJobs, jobFilter) - - // Show or hide system jobs - const jobs = showSystemJobs ? filteredJobs : getUserJobs(filteredJobs) + const [jobFilter, setJobFilter] = hooks.useJobFilter() + const [showSystemJobs, setShowSystemJobs] = hooks.useShowSystemJobs() + const jobs = hooks.useListJobs() return ( jest.fn()) - -afterEach(() => { - jest.resetAllMocks() -}) - describe('', () => { it('renders without errors when there is data', () => { const store = { @@ -17,6 +10,8 @@ describe('', () => { { id: 'one', name: 'one' }, { id: 'two', name: 'two' }, ], + jobFilter: ['', () => {}], + showSystemJobs: [false, () => {}], } shallow( @@ -25,84 +20,4 @@ describe('', () => { ) }) - - it('omits system jobs by default', () => { - JobList.mockImplementation(() => null) - - const userJob = { id: 'user', name: 'user', configurable: true } - const systemJob = { id: 'system', name: 'system' } - const store = { - jobs: [userJob, systemJob], - } - - const wrapper = mount( - - - - ) - const childProps = wrapper.children().props() - - expect(childProps.jobs).toHaveLength(1) - expect(childProps.jobs).toEqual(expect.arrayContaining([userJob])) - }) - - it('passes system and user jobs after toggling', () => { - JobList.mockImplementation(({ showSystemJobs, setShowSystemJobs }) => ( -