From ef99d0f2a13021b8b079f05b7d7caf7976e05b4b Mon Sep 17 00:00:00 2001 From: Stuart Bennett Date: Fri, 11 Oct 2024 15:38:12 +0100 Subject: [PATCH] EES-5402 simplified form layout and updated reducer --- .../ChartBoundaryLevelsConfiguration.tsx | 126 +++++++++++---- ...hartBoundaryLevelsDataSetConfiguration.tsx | 149 +++++------------- .../components/chart/ChartBuilder.tsx | 21 ++- .../chart/ChartDataGroupingsConfiguration.tsx | 61 ++++++- .../ChartBoundaryLevelsConfiguration.test.tsx | 75 ++++++++- .../chart/reducers/chartBuilderReducer.ts | 19 +-- 6 files changed, 281 insertions(+), 170 deletions(-) diff --git a/src/explore-education-statistics-admin/src/pages/release/datablocks/components/chart/ChartBoundaryLevelsConfiguration.tsx b/src/explore-education-statistics-admin/src/pages/release/datablocks/components/chart/ChartBoundaryLevelsConfiguration.tsx index bd07e0eb700..c58954e127a 100644 --- a/src/explore-education-statistics-admin/src/pages/release/datablocks/components/chart/ChartBoundaryLevelsConfiguration.tsx +++ b/src/explore-education-statistics-admin/src/pages/release/datablocks/components/chart/ChartBoundaryLevelsConfiguration.tsx @@ -5,34 +5,51 @@ import Effect from '@common/components/Effect'; import Form from '@common/components/form/Form'; import FormFieldSelect from '@common/components/form/FormFieldSelect'; import FormProvider from '@common/components/form/FormProvider'; -import { MapDataSetConfig } from '@common/modules/charts/types/chart'; +import { + AxisConfiguration, + MapConfig, + MapDataSetConfig, +} from '@common/modules/charts/types/chart'; +import { LegendConfiguration } from '@common/modules/charts/types/legend'; +import createDataSetCategories from '@common/modules/charts/util/createDataSetCategories'; +import getDataSetCategoryConfigs from '@common/modules/charts/util/getDataSetCategoryConfigs'; import { FullTableMeta } from '@common/modules/table-tool/types/fullTable'; +import { TableDataResult } from '@common/services/tableBuilderService'; import parseNumber from '@common/utils/number/parseNumber'; +import Yup from '@common/validation/yup'; +import { isEqual } from 'lodash'; import merge from 'lodash/merge'; -import React, { ReactNode, useCallback } from 'react'; +import React, { ReactNode, useCallback, useMemo } from 'react'; +import { AnyObject, NumberSchema, ObjectSchema } from 'yup'; import ChartBoundaryLevelsDataSetConfiguration from './ChartBoundaryLevelsDataSetConfiguration'; const formId = 'chartBoundaryLevelsConfigurationForm'; export interface ChartBoundaryLevelsFormValues { - boundaryLevel: number; - dataSetConfigs: MapDataSetConfig[]; + boundaryLevel?: number; + dataSetConfigs: Omit[]; } interface Props { buttons?: ReactNode; + axisMajor: AxisConfiguration; + data: TableDataResult[]; + legend: LegendConfiguration; + map?: MapConfig; meta: FullTableMeta; - dataSetConfigs: MapDataSetConfig[]; - boundaryLevel: ChartOptions['boundaryLevel']; + options: ChartOptions; onChange: (values: ChartBoundaryLevelsFormValues) => void; onSubmit: (values: ChartBoundaryLevelsFormValues) => void; } export default function ChartBoundaryLevelsConfiguration({ buttons, + axisMajor, + data, + legend, + map, meta, - dataSetConfigs, - boundaryLevel, + options, onChange, onSubmit, }: Props) { @@ -42,14 +59,13 @@ export default function ChartBoundaryLevelsConfiguration({ (values: ChartBoundaryLevelsFormValues): ChartBoundaryLevelsFormValues => { // Use `merge` as we want to avoid potential undefined // values from overwriting existing values - const returnValue = merge({}, boundaryLevel, values, { + return merge({}, options.boundaryLevel, values, { boundaryLevel: values.boundaryLevel ? parseNumber(values.boundaryLevel) : undefined, }); - return returnValue; }, - [boundaryLevel], + [options], ); const handleChange = useCallback( @@ -59,35 +75,73 @@ export default function ChartBoundaryLevelsConfiguration({ [normalizeValues, onChange], ); + const validationSchema = useMemo< + ObjectSchema + >(() => { + return Yup.object({ + boundaryLevel: Yup.number() + .transform(value => (Number.isNaN(value) ? undefined : value)) + .nullable() + .oneOf(meta.boundaryLevels.map(level => level.id)) + .required('Choose a boundary level'), + dataSetConfigs: Yup.array() + .of( + Yup.object({ + boundaryLevel: Yup.mixed().test( + 'dataset-boundary-is-number-or-empty', + 'Must be a number or an empty string', + value => !Number.isNaN(value) || value === '', + ) as NumberSchema, + dataSet: Yup.object({ + filters: Yup.array().required(), + }).required(), + }), + ) + .required(), + }); + }, [meta.boundaryLevels]); + + const initialValues = useMemo(() => { + const dataSetCategories = createDataSetCategories({ + axisConfiguration: { + ...axisMajor, + groupBy: 'locations', + }, + data, + meta, + }); + + const dataSetCategoryConfigs = getDataSetCategoryConfigs({ + dataSetCategories, + legendItems: legend.items, + meta, + deprecatedDataClassification: options.dataClassification, + deprecatedDataGroups: options.dataGroups, + }); + + return { + boundaryLevel: options.boundaryLevel, + dataSetConfigs: dataSetCategoryConfigs.map(({ rawDataSet }) => ({ + dataSet: rawDataSet, + boundaryLevel: map?.dataSetConfigs.find(config => + isEqual(config.dataSet, rawDataSet), + )?.boundaryLevel, + })), + }; + }, [axisMajor, data, meta, legend.items, map, options]); + return ( enableReinitialize - initialValues={{ - boundaryLevel, - dataSetConfigs, - }} - /* validationSchema={Yup.object({ - boundaryLevel: Yup.number() - .transform(value => (Number.isNaN(value) ? undefined : value)) - .nullable() - .oneOf(meta.boundaryLevels.map(level => level.id)) - .required('Choose a boundary level'), - // dataSetConfigs: Yup.array(), //TODO - })} */ + initialValues={initialValues} + validationSchema={validationSchema} > - {({ formState, watch, control }) => { + {({ formState, watch }) => { const values = watch(); return ( -
{ - onSubmit(normalizeValues(values)); - await submitForms(); - }} - > + label="Default Boundary level" - hint="Select a version of geographical data to use across any data sets that don't have a specific one set for that dataset" + hint="Select a version of geographical data to use across any data sets that don't have a specific one set" name="boundaryLevel" order={[]} options={[ @@ -114,7 +168,7 @@ export default function ChartBoundaryLevelsConfiguration({ /> @@ -122,6 +176,10 @@ export default function ChartBoundaryLevelsConfiguration({ formId={formId} formKey="boundaryLevels" disabled={formState.isSubmitting} + onClick={async () => { + onSubmit(values); + await submitForms(); + }} > {buttons} diff --git a/src/explore-education-statistics-admin/src/pages/release/datablocks/components/chart/ChartBoundaryLevelsDataSetConfiguration.tsx b/src/explore-education-statistics-admin/src/pages/release/datablocks/components/chart/ChartBoundaryLevelsDataSetConfiguration.tsx index 06104ad0c85..fec347f2a18 100644 --- a/src/explore-education-statistics-admin/src/pages/release/datablocks/components/chart/ChartBoundaryLevelsDataSetConfiguration.tsx +++ b/src/explore-education-statistics-admin/src/pages/release/datablocks/components/chart/ChartBoundaryLevelsDataSetConfiguration.tsx @@ -1,90 +1,63 @@ -import Button from '@common/components/Button'; -import ButtonGroup from '@common/components/ButtonGroup'; -import ButtonText from '@common/components/ButtonText'; -import FormFieldSelect from '@common/components/form/FormFieldSelect'; -import Modal from '@common/components/Modal'; +import { FormFieldSelect } from '@common/components/form'; +import { MapDataSetConfig } from '@common/modules/charts/types/chart'; import expandDataSet from '@common/modules/charts/util/expandDataSet'; import generateDataSetKey from '@common/modules/charts/util/generateDataSetKey'; import { FullTableMeta } from '@common/modules/table-tool/types/fullTable'; -import React, { useCallback, useState } from 'react'; -import { useFieldArray, UseFormReturn } from 'react-hook-form'; -import type { ChartBoundaryLevelsFormValues } from './ChartBoundaryLevelsConfiguration'; +import React, { useMemo } from 'react'; import generateDataSetLabel from './utils/generateDataSetLabel'; -interface Props { - control: UseFormReturn['control']; - meta: FullTableMeta; -} - export default function ChartBoundaryLevelsDataSetConfiguration({ - control, + dataSetConfigs, meta, -}: Props) { - const [updateIndex, setUpdateIndex] = useState(); - const [selectValue, setSelectValue] = useState(); - const { fields: dataSetConfigs, update } = useFieldArray< - ChartBoundaryLevelsFormValues, - 'dataSetConfigs' - >({ - control, - name: 'dataSetConfigs', - }); - - const openFormModal = useCallback( - (i: number) => { - setUpdateIndex(i); - setSelectValue(dataSetConfigs[i].boundaryLevel); - }, - [setUpdateIndex, dataSetConfigs], - ); - - const closeFormModal = useCallback(() => { - setUpdateIndex(undefined); - setSelectValue(undefined); - }, [setUpdateIndex]); +}: { + dataSetConfigs: Omit[]; + meta: FullTableMeta; +}) { + const mappedDataSetConfigs = useMemo(() => { + return dataSetConfigs.map(dataSetConfig => { + const expandedDataSet = expandDataSet(dataSetConfig.dataSet, meta); + const dataSetLabel = generateDataSetLabel(expandedDataSet); + const key = generateDataSetKey(dataSetConfig.dataSet); + return { + dataSetLabel, + key, + }; + }); + }, [meta, dataSetConfigs]); return ( <>

Set boundary levels per data set

- {/* {error && {error}} */} - {!!dataSetConfigs && dataSetConfigs.length > 1 && ( - - {dataSetConfigs.map((dataSetConfig, index) => { - const expandedDataSet = expandDataSet( - dataSetConfig.dataSet, - meta, - ); - const label = generateDataSetLabel(expandedDataSet); - const key = generateDataSetKey(dataSetConfig.dataSet); - - const boundaryLabel = dataSetConfig.boundaryLevel - ? meta.boundaryLevels.find( - ({ id }) => - String(id) === String(dataSetConfig.boundaryLevel), - )?.label - : 'Default'; - + {mappedDataSetConfigs.map(({ dataSetLabel, key }, index) => { return ( - - - + ); @@ -92,52 +65,6 @@ export default function ChartBoundaryLevelsDataSetConfiguration({
Data set BoundaryActions
{label}{boundaryLabel} - { - openFormModal(index); - }} - > - Edit - + {dataSetLabel} + ({ + value: Number(id), + label, + })), + ]} + />
)} - - {updateIndex !== undefined && ( - - ({ - value: id, - label, - })), - ]} - onChange={({ target }) => { - // works but dodgy type? - const { value } = target; - setSelectValue( - Number.isNaN(Number(value)) ? undefined : Number(value), - ); - }} - /> - - - - - - )} ); } diff --git a/src/explore-education-statistics-admin/src/pages/release/datablocks/components/chart/ChartBuilder.tsx b/src/explore-education-statistics-admin/src/pages/release/datablocks/components/chart/ChartBuilder.tsx index 68193392abc..645cca4bfa5 100644 --- a/src/explore-education-statistics-admin/src/pages/release/datablocks/components/chart/ChartBuilder.tsx +++ b/src/explore-education-statistics-admin/src/pages/release/datablocks/components/chart/ChartBuilder.tsx @@ -10,10 +10,7 @@ import ChartBoundaryLevelsConfiguration, { } from '@admin/pages/release/datablocks/components/chart/ChartBoundaryLevelsConfiguration'; import ChartDataGroupingsConfiguration from '@admin/pages/release/datablocks/components/chart/ChartDataGroupingsConfiguration'; import { ChartBuilderFormsContextProvider } from '@admin/pages/release/datablocks/components/chart/contexts/ChartBuilderFormsContext'; -import { - ChartOptions, - useChartBuilderReducer, -} from '@admin/pages/release/datablocks/components/chart/reducers/chartBuilderReducer'; +import { useChartBuilderReducer } from '@admin/pages/release/datablocks/components/chart/reducers/chartBuilderReducer'; import Button from '@common/components/Button'; import ModalConfirm from '@common/components/ModalConfirm'; import Tabs from '@common/components/Tabs'; @@ -360,19 +357,25 @@ const ChartBuilder = ({ /> )} + {forms.boundaryLevels && + axes.major && + forms.dataGroupings && definition?.type === 'map' && options && - meta.boundaryLevels.length && ( + legend && ( void; onSubmit: (dataSetConfigs: MapDataSetConfig[]) => void; } const ChartDataGroupingsConfiguration = ({ + axisMajor, buttons, + data, + legend, map, meta, + options, onChange, onSubmit, }: Props) => { @@ -39,7 +57,38 @@ const ChartDataGroupingsConfiguration = ({ }>(); const { forms, updateForm, submitForms } = useChartBuilderFormsContext(); - if (!map?.dataSetConfigs?.length) { + const initialValues = useMemo(() => { + const dataSetCategories = createDataSetCategories({ + axisConfiguration: { + ...axisMajor, + groupBy: 'locations', + }, + data, + meta, + }); + + const dataSetCategoryConfigs = getDataSetCategoryConfigs({ + dataSetCategories, + legendItems: legend.items, + meta, + deprecatedDataClassification: options.dataClassification, + deprecatedDataGroups: options.dataGroups, + }); + + return { + dataSetConfigs: dataSetCategoryConfigs.map( + ({ rawDataSet, dataGrouping }) => ({ + dataSet: rawDataSet, + dataGrouping: + map?.dataSetConfigs.find(config => + isEqual(config.dataSet, rawDataSet), + )?.dataGrouping ?? dataGrouping, + }), + ), + }; + }, [axisMajor, data, meta, legend.items, map, options]); + + if (!initialValues.dataSetConfigs?.length) { return

No data groupings to edit.

; } @@ -63,7 +112,7 @@ const ChartDataGroupingsConfiguration = ({ - {map.dataSetConfigs.map(dataSetConfig => { + {initialValues.dataSetConfigs.map(dataSetConfig => { const expandedDataSet = expandDataSet(dataSetConfig.dataSet, meta); const label = generateDataSetLabel(expandedDataSet); const key = generateDataSetKey(dataSetConfig.dataSet); @@ -104,12 +153,12 @@ const ChartDataGroupingsConfiguration = ({ > setEditDataSetConfig(undefined)} onSubmit={values => { - const updated = map.dataSetConfigs.map(config => { + const updated = initialValues.dataSetConfigs.map(config => { if (isEqual(config.dataSet, values.dataSet)) { return values; } @@ -132,7 +181,7 @@ const ChartDataGroupingsConfiguration = ({ ? forms.dataGroupings.submitCount + 1 : 1, }); - onSubmit(map.dataSetConfigs); + onSubmit(initialValues.dataSetConfigs); await submitForms(); }} > diff --git a/src/explore-education-statistics-admin/src/pages/release/datablocks/components/chart/__tests__/ChartBoundaryLevelsConfiguration.test.tsx b/src/explore-education-statistics-admin/src/pages/release/datablocks/components/chart/__tests__/ChartBoundaryLevelsConfiguration.test.tsx index b811a38c0f4..d53b67f6cd9 100644 --- a/src/explore-education-statistics-admin/src/pages/release/datablocks/components/chart/__tests__/ChartBoundaryLevelsConfiguration.test.tsx +++ b/src/explore-education-statistics-admin/src/pages/release/datablocks/components/chart/__tests__/ChartBoundaryLevelsConfiguration.test.tsx @@ -12,6 +12,8 @@ import noop from 'lodash/noop'; import React, { ReactElement } from 'react'; describe('ChartBoundaryLevelsConfiguration', () => { + const testTable = testFullTable; + const testDefaultChartOptions: ChartOptions = { alt: '', height: 600, @@ -35,7 +37,6 @@ describe('ChartBoundaryLevelsConfiguration', () => { }, ], }; - const testFormState: ChartBuilderForms = { options: { isValid: true, @@ -66,7 +67,19 @@ describe('ChartBoundaryLevelsConfiguration', () => { test('renders correctly without initial values', () => { render( { test('renders correctly with initial values', () => { render( { const { user } = render( { test('submitting fails with validation errors if no boundary level set', async () => { const { user } = render( { const { user } = render( { const { user } = render( { - const { dataGrouping } = existingDataSetConfigs.find( - ({ dataSet: existingDataSet }) => isEqual(existingDataSet, dataSet), - )!; + const { dataGrouping } = + existingDataSetConfigs.find(({ dataSet: existingDataSet }) => + isEqual(existingDataSet, dataSet), + )! ?? {}; + if (boundaryLevel) console.log({ boundaryLevel }); return { dataSet, boundaryLevel, dataGrouping }; }, ); @@ -358,10 +358,7 @@ export function useChartBuilderReducer( ); const updateChartBoundaryLevels = useCallback( - (payload: { - boundaryLevel: ChartOptions['boundaryLevel']; - dataSetConfigs: MapDataSetConfig[]; - }) => { + (payload: ChartBoundaryLevelsFormValues) => { dispatch({ type: 'UPDATE_CHART_MAP_BOUNDARY_LEVELS', payload,