Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EES-5542 Maps fetching/rendering boundaries for datasets #5423

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ import dataBlocksService, {
CreateReleaseDataBlock,
ReleaseDataBlock,
} from '@admin/services/dataBlockService';
import featuredTableService from '@admin/services/featuredTableService';
import LoadingSpinner from '@common/components/LoadingSpinner';
import Tabs from '@common/components/Tabs';
import TabsSection from '@common/components/TabsSection';
import WarningMessage from '@common/components/WarningMessage';
import useAsyncRetry from '@common/hooks/useAsyncRetry';
import getMapInitialBoundaryLevel from '@common/modules/charts/components/utils/getMapInitialBoundaryLevel';
import isOrphanedDataSet from '@common/modules/charts/util/isOrphanedDataSet';
import { InitialTableToolState } from '@common/modules/table-tool/components/TableToolWizard';
import getInitialStepSubjectMeta from '@common/modules/table-tool/components/utils/getInitialStepSubjectMeta';
Expand All @@ -29,7 +31,6 @@ import minDelay from '@common/utils/minDelay';
import { produce } from 'immer';
import omit from 'lodash/omit';
import React, { useCallback, useState } from 'react';
import featuredTableService from '@admin/services/featuredTableService';

export type SavedDataBlock = CreateReleaseDataBlock & {
id?: string;
Expand Down Expand Up @@ -78,14 +79,10 @@ const DataBlockPageTabs = ({
releaseId,
};

const boundaryLevel =
dataBlock.charts[0]?.type === 'map'
? dataBlock.charts[0].boundaryLevel
: undefined;
const tableData = await tableBuilderService.getTableData(
query,
releaseId,
boundaryLevel,
getMapInitialBoundaryLevel(dataBlock.charts),
);

const { initialStep, subjectMeta } = await getInitialStepSubjectMeta(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import React, { ReactNode, useCallback, useMemo } from 'react';

interface Props {
buttons?: ReactNode;
// TODO: EES-5402 - Remove when all boundary level changes are done
hasDataSetBoundaryLevels?: boolean;
map: MapConfig;
meta: FullTableMeta;
options: ChartOptions;
Expand All @@ -21,8 +19,6 @@ interface Props {

export default function ChartBoundaryLevelsConfiguration({
buttons,
// TODO: EES-5402 - Remove when all boundary level changes are done
hasDataSetBoundaryLevels = false,
map,
meta,
options,
Expand Down Expand Up @@ -77,7 +73,6 @@ export default function ChartBoundaryLevelsConfiguration({
<ChartBoundaryLevelsForm
buttons={buttons}
dataSetConfigs={map.dataSetConfigs}
hasDataSetBoundaryLevels={hasDataSetBoundaryLevels}
initialValues={initialValues}
meta={meta}
onChange={handleChange}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ interface Props {
buttons?: ReactNode;
initialValues: ChartBoundaryLevelsFormValues;
dataSetConfigs: MapDataSetConfig[];
// TODO: EES-5402 - Remove when all boundary level changes are done
hasDataSetBoundaryLevels?: boolean;
meta: FullTableMeta;
onChange: (values: Partial<ChartBoundaryLevelsFormValues>) => void;
onSubmit: (values: ChartBoundaryLevelsFormValues) => void;
Expand All @@ -37,8 +35,6 @@ interface Props {
export default function ChartBoundaryLevelsForm({
buttons,
dataSetConfigs,
// TODO: EES-5402 - Remove when all boundary level changes are done
hasDataSetBoundaryLevels,
initialValues,
meta,
onChange,
Expand Down Expand Up @@ -120,16 +116,8 @@ export default function ChartBoundaryLevelsForm({
onMount={updateForm}
/>
<FormFieldSelect<ChartBoundaryLevelsFormValues>
label={
hasDataSetBoundaryLevels
? 'Default boundary level'
: 'Boundary level'
}
hint={`Select a version of geographical data to use${
hasDataSetBoundaryLevels
? " across any data sets that don't have a specific one set"
: ''
}`}
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"
name="boundaryLevel"
order={[]}
options={[
Expand All @@ -140,7 +128,7 @@ export default function ChartBoundaryLevelsForm({
...boundaryLevelOptions,
]}
/>
{hasDataSetBoundaryLevels && dataSetRows.length > 1 && (
{dataSetRows.length > 1 && (
<>
<h4>Set boundary levels per data set</h4>
<table data-testid="data-set-boundary-levels">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ import {
ReleaseTableDataQuery,
TableDataResult,
} from '@common/services/tableBuilderService';
import { Chart } from '@common/services/types/blocks';
import { Chart, DataBlock } from '@common/services/types/blocks';
import { ValidationProblemDetails } from '@common/services/types/problemDetails';
import parseNumber from '@common/utils/number/parseNumber';
import { isServerValidationError } from '@common/validation/serverValidations';
Expand Down Expand Up @@ -198,11 +198,23 @@ export default function ChartBuilder({
},
boundaryLevel: options.boundaryLevel ?? 0,
type: 'map',
onBoundaryLevelChange: boundaryLevel =>
onTableQueryUpdate({ boundaryLevel }),
};
default:
return undefined;
}
}, [axes, data, definition, getChartFile, legend, meta, options, map]);
}, [
axes,
data,
definition,
getChartFile,
legend,
meta,
options,
map,
onTableQueryUpdate,
]);

const handleSubmit = useCallback(async () => {
if (!chartProps) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ describe('ChartBoundaryLevelsConfiguration', () => {
options={testDefaultChartOptions}
onChange={noop}
onSubmit={noop}
hasDataSetBoundaryLevels
/>,
);
expect(screen.getByLabelText('Default boundary level')).toBeInTheDocument();
Expand All @@ -116,7 +115,6 @@ describe('ChartBoundaryLevelsConfiguration', () => {
options={testDefaultChartOptions}
onChange={noop}
onSubmit={noop}
hasDataSetBoundaryLevels
/>,
);

Expand Down Expand Up @@ -201,7 +199,6 @@ describe('ChartBoundaryLevelsConfiguration', () => {
}}
onChange={noop}
onSubmit={noop}
hasDataSetBoundaryLevels
/>,
);

Expand Down Expand Up @@ -229,7 +226,6 @@ describe('ChartBoundaryLevelsConfiguration', () => {
options={testDefaultChartOptions}
onChange={handleChange}
onSubmit={noop}
hasDataSetBoundaryLevels
/>,
);

Expand Down Expand Up @@ -264,7 +260,6 @@ describe('ChartBoundaryLevelsConfiguration', () => {
options={testDefaultChartOptions}
onChange={noop}
onSubmit={noop}
hasDataSetBoundaryLevels
/>,
);

Expand Down Expand Up @@ -294,7 +289,6 @@ describe('ChartBoundaryLevelsConfiguration', () => {
options={testDefaultChartOptions}
onChange={noop}
onSubmit={handleSubmit}
hasDataSetBoundaryLevels
/>,
);

Expand Down Expand Up @@ -329,7 +323,6 @@ describe('ChartBoundaryLevelsConfiguration', () => {
}}
onChange={noop}
onSubmit={handleSubmit}
hasDataSetBoundaryLevels
/>,
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,7 @@ describe('ChartBuilder', () => {

test('calls `onTableQueryUpdate` when change boundary level', async () => {
const testInitialChart: Chart = {
onBoundaryLevelChange: jest.fn(),
type: 'map',
boundaryLevel: 2,
map: {
Expand Down Expand Up @@ -443,7 +444,9 @@ describe('ChartBuilder', () => {

await user.click(screen.getByRole('tab', { name: 'Boundary levels' }));

await user.selectOptions(screen.getByLabelText('Boundary level'), ['1']);
await user.selectOptions(screen.getByLabelText('Default boundary level'), [
'1',
]);

expect(handleUpdate).toHaveBeenCalledWith({ boundaryLevel: 1 });
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ describe('ChartBuilderPreview', () => {

const testMapChartRenderer: ChartRendererProps = {
type: 'map',
onBoundaryLevelChange: () => {},
data: [],
meta: testFullTableMeta.subjectMeta,
alt: '',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ describe('ChartBuilderTabSection', () => {

const testChart: Chart = {
type: 'map',
onBoundaryLevelChange: () => {},
boundaryLevel: 2,
map: {
dataSetConfigs: [],
Expand Down Expand Up @@ -186,7 +187,9 @@ describe('ChartBuilderTabSection', () => {

await user.click(screen.getByRole('tab', { name: 'Boundary levels' }));

await user.selectOptions(screen.getByLabelText('Boundary level'), ['1']);
await user.selectOptions(screen.getByLabelText('Default boundary level'), [
'1',
]);

expect(tableBuilderService.getTableData).toHaveBeenCalledWith(
{ ...testQuery, boundaryLevel: 1 },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1140,6 +1140,8 @@ describe('chartBuilderReducer', () => {

test('has correct state with initial configuration for a map', () => {
const initialConfiguration: Chart = {
type: 'map',
onBoundaryLevelChange: () => {},
legend: {
position: 'top',
items: [],
Expand All @@ -1162,7 +1164,7 @@ describe('chartBuilderReducer', () => {
referenceLines: [],
},
},
type: 'map',

height: 300,
title: 'Chart title',
alt: '',
Expand Down Expand Up @@ -1248,6 +1250,8 @@ describe('chartBuilderReducer', () => {

test('setting boundary levels does not change data groupings', () => {
const initialConfiguration: Chart = {
type: 'map',
onBoundaryLevelChange: () => {},
legend: {
position: 'top',
items: [],
Expand All @@ -1270,7 +1274,6 @@ describe('chartBuilderReducer', () => {
referenceLines: [],
},
},
type: 'map',
height: 300,
title: 'Chart title',
alt: '',
Expand Down Expand Up @@ -1357,6 +1360,8 @@ describe('chartBuilderReducer', () => {

test('setting data groupings does not change boundary levels', () => {
const initialConfiguration: Chart = {
type: 'map',
onBoundaryLevelChange: () => {},
legend: {
position: 'top',
items: [],
Expand All @@ -1379,7 +1384,6 @@ describe('chartBuilderReducer', () => {
referenceLines: [],
},
},
type: 'map',
height: 300,
title: 'Chart title',
alt: '',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ import React, { memo, useMemo } from 'react';
export type ChartRendererProps = {
source?: string;
} & (
| ({
type: 'map';
} & Omit<MapBlockProps, 'id'>)
| ({
type: 'line';
} & LineChartProps)
Expand All @@ -28,11 +31,6 @@ export type ChartRendererProps = {
| ({
type: 'horizontalbar';
} & HorizontalBarProps)
| ({
type: 'map';
} & Omit<MapBlockProps, 'id'> & {
boundaryLevel: number;
})
| ({
type: 'infographic';
} & InfographicChartProps)
Expand Down
Loading
Loading