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.

Sign up for GitHub

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-5496 Fix inconsistent anchor IDs causing broken nav links in API data set mapping pages #5481

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
+138 −88
Open

EES-5496 Fix inconsistent anchor IDs causing broken nav links in API data set mapping pages #5481

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1d45e27
EES-5496 Fix inconsistent anchor IDs in locations mapping page
ntsim Dec 19, 2024
e3a72dd
EES-5496 Refactor filter mappings page to use consistent anchor IDs
ntsim Dec 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 36 additions & 22 deletions ...education-statistics-admin/src/pages/release/data/ReleaseApiDataSetFiltersMappingPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import getUnmappedFilterErrors from '@admin/pages/release/data/utils/getUnmapped
import ApiDataSetMappableFilterColumnsTable from '@admin/pages/release/data/components/ApiDataSetMappableFilterColumnsTable';
import ApiDataSetNewFilterColumnsTable from '@admin/pages/release/data/components/ApiDataSetNewFilterColumnsTable';
import { PendingMappingUpdate } from '@admin/pages/release/data/types/apiDataSetMappings';
import { mappableTableId } from '@admin/pages/release/data/utils/mappingTableIds';
import {
releaseApiDataSetDetailsRoute,
ReleaseDataSetRouteParams,
Expand All @@ -31,14 +32,26 @@ import PageNav, { NavItem } from '@common/components/PageNav';
import { Dictionary } from '@common/types';
import useDebouncedCallback from '@common/hooks/useDebouncedCallback';
import { useQuery } from '@tanstack/react-query';
import camelCase from 'lodash/camelCase';
import React, { useCallback, useEffect, useMemo, useState } from 'react';
import { generatePath, useParams } from 'react-router-dom';
import omit from 'lodash/omit';
import { useImmer } from 'use-immer';
import sumBy from 'lodash/sumBy';
import kebabCase from 'lodash/kebabCase';
import compact from 'lodash/compact';

export const sectionIds = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a neat way of keeping the nav and the links consistent. Do we do this pattern elsewhere at all?

mappableFilterColumns: 'mappable-filter-columns',
mappableFilterOptions: 'mappable-filter-options',
newFilterColumns: 'new-filter-columns',
newFilterOptions: 'new-filter-options',
newFilterOptionsGroup: (filterKey: string) =>
`new-filter-options-${camelCase(filterKey)}`,
autoMappedFilterOptions: 'auto-mapped-filter-options',
autoMappedFilterOptionsGroup: (filterKey: string) =>
`auto-mapped-filter-options-${camelCase(filterKey)}`,
} as const;

export default function ReleaseApiDataSetFiltersMappingPage() {
const [autoMappedFilterOptions, updateAutoMappedFilterOptions] = useImmer<
Dictionary<AutoMappedFilterOption[]>
Expand Down Expand Up @@ -111,45 +124,45 @@ export default function ReleaseApiDataSetFiltersMappingPage() {
...(Object.keys(mappableFilters).length > 0
? [
{
id: 'mappable-filter-columns',
id: sectionIds.mappableFilterColumns,
text: 'Filter columns not found in new data set',
},
]
: []),
{
id: 'mappable-filter-options',
id: sectionIds.mappableFilterOptions,
text: 'Filter options not found in new data set',
subNavItems: Object.keys(mappableFilterOptions).map(filterKey => {
return {
id: `mappable-filter-options-${kebabCase(filterKey)}`,
id: mappableTableId(filterKey),
text: filtersMapping?.mappings[filterKey].source.label ?? filterKey,
};
}),
},
...(Object.keys(newFilters).length > 0
? [
{
id: 'new-filter-columns',
id: sectionIds.newFilterColumns,
text: 'New filter columns',
},
]
: []),
{
id: 'new-filter-options',
id: sectionIds.newFilterOptions,
text: 'New filter options',
subNavItems: Object.keys(newFilterOptions).map(filterKey => {
return {
id: `new-filter-options-${kebabCase(filterKey)}`,
id: sectionIds.newFilterOptionsGroup(filterKey),
text: filtersMapping?.candidates[filterKey].label ?? filterKey,
};
}),
},
{
id: 'auto-mapped-filter-options',
id: sectionIds.autoMappedFilterOptions,
text: 'Auto mapped filter options',
subNavItems: Object.keys(autoMappedFilterOptions).map(filterKey => {
return {
id: `auto-mapped-filter-options-${kebabCase(filterKey)}`,
id: sectionIds.autoMappedFilterOptionsGroup(filterKey),
text: filtersMapping?.mappings[filterKey].source.label ?? filterKey,
};
}),
Expand Down Expand Up @@ -331,7 +344,7 @@ export default function ReleaseApiDataSetFiltersMappingPage() {
<>
<h3
className="govuk-heading-l dfe-flex dfe-align-items--center"
id="mappable-filter-columns"
id={sectionIds.mappableFilterColumns}
>
Filter columns not found in new data set{' '}
<Tag className="govuk-!-margin-left-2" colour="grey">
Expand All @@ -345,7 +358,10 @@ export default function ReleaseApiDataSetFiltersMappingPage() {
</>
)}

<h3 className="govuk-heading-l" id="mappable-filter-options">
<h3
className="govuk-heading-l"
id={sectionIds.mappableFilterOptions}
>
Filter options not found in new data set
</h3>

Expand Down Expand Up @@ -386,7 +402,7 @@ export default function ReleaseApiDataSetFiltersMappingPage() {
<>
<h3
className="govuk-heading-l govuk-!-margin-top-8"
id="new-filter-columns"
id={sectionIds.newFilterColumns}
>
New filter columns <Tag colour="grey">No action required</Tag>
</h3>
Expand All @@ -396,16 +412,16 @@ export default function ReleaseApiDataSetFiltersMappingPage() {

<h3
className="govuk-heading-l govuk-!-margin-top-8"
id="new-filter-options"
id={sectionIds.newFilterOptions}
>
New filter options ({totalNewFilterOptions}){' '}
<Tag colour="grey">No action required</Tag>
</h3>

{totalNewFilterOptions > 0 && filtersMapping ? (
<Accordion
id="new-filter-options-accordion"
testId="new-filter-options-accordion"
id={`${sectionIds.newFilterOptions}-accordion`}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth having a sectionIds.newFilterOptionsAccordion variable just to avoid the risk of this too getting out of sync?

testId={`${sectionIds.newFilterOptions}-accordion`}
>
{Object.keys(newFilterOptions).map(filterKey => {
if (newFilterOptions[filterKey].length) {
Expand All @@ -422,7 +438,7 @@ export default function ReleaseApiDataSetFiltersMappingPage() {
goToTop={false}
heading={`${filterLabel} (${newFilterOptions[filterKey].length})`}
headingTag="h4"
id={`new-filter-options-${kebabCase(filterKey)}`}
id={sectionIds.newFilterOptionsGroup(filterKey)}
key={filterKey}
>
<ApiDataSetNewItemsTable
Expand All @@ -444,17 +460,17 @@ export default function ReleaseApiDataSetFiltersMappingPage() {

<h3
className="govuk-heading-l govuk-!-margin-top-8"
id="auto-mapped-filter-options"
id={sectionIds.autoMappedFilterOptions}
>
Auto mapped filter options ({totalAutoMappedFilterOptions}){' '}
<Tag colour="grey">No action required</Tag>
</h3>

{totalAutoMappedFilterOptions > 0 ? (
<Accordion
id="auto-mapped-filter-options-accordion"
id={`${sectionIds.autoMappedFilterOptions}-accordion`}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned before, worth a variable for this? Same comments for other such changes in this PR :)

showOpenAll={false}
testId="auto-mapped-filter-options-accordion"
testId={`${sectionIds.autoMappedFilterOptions}-accordion`}
>
{Object.keys(autoMappedFilterOptions).map(filterKey => {
if (
Expand All @@ -474,9 +490,7 @@ export default function ReleaseApiDataSetFiltersMappingPage() {
goToTop={false}
heading={`${filterLabel} (${autoMappedFilterOptions[filterKey].length})`}
headingTag="h4"
id={`auto-mapped-filter-options-${kebabCase(
filterKey,
)}`}
id={sectionIds.autoMappedFilterOptionsGroup(filterKey)}
key={filterKey}
>
<ApiDataSetAutoMappedTable
Expand Down
63 changes: 42 additions & 21 deletions ...ucation-statistics-admin/src/pages/release/data/ReleaseApiDataSetLocationsMappingPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import getApiDataSetLocationMappings, {
MappableLocation,
} from '@admin/pages/release/data/utils/getApiDataSetLocationMappings';
import getUnmappedLocationErrors from '@admin/pages/release/data/utils/getUnmappedLocationErrors';
import { mappableTableId } from '@admin/pages/release/data/utils/mappingTableIds';
import apiDataSetQueries from '@admin/queries/apiDataSetQueries';
import apiDataSetVersionQueries from '@admin/queries/apiDataSetVersionQueries';
import {
Expand Down Expand Up @@ -41,6 +42,17 @@ import React, { useCallback, useEffect, useMemo, useState } from 'react';
import { generatePath, useParams } from 'react-router-dom';
import { useImmer } from 'use-immer';

export const sectionIds = {
deletedLocationGroups: 'deleted-location-groups',
mappableLocations: 'mappable-locations',
newLocationGroups: 'new-location-groups',
newLocations: 'new-locations',
newLocationsLevel: (level: LocationLevelKey) => `new-locations-${level}`,
autoMappedLocations: 'auto-mapped-locations',
autoMappedLocationsLevel: (level: LocationLevelKey) =>
`auto-mapped-locations-${level}`,
} as const;

// Fields to omit from mapping diff.
const omittedDiffingFields: (keyof LocationCandidateWithKey)[] = [
'key',
Expand Down Expand Up @@ -118,33 +130,36 @@ export default function ReleaseApiDataSetLocationsMappingPage() {
...(Object.keys(deletedLocationGroups).length > 0
? [
{
id: 'deleted-location-groups',
id: sectionIds.deletedLocationGroups,
text: 'Location groups not found in new data set',
},
]
: []),
{
id: 'mappable-locations',
id: sectionIds.mappableLocations,
text: 'Locations not found in new data set',
subNavItems: getSubNavItems(mappableLocations, 'mappable'),
subNavItems: getSubNavItems(mappableLocations, mappableTableId),
},
...(Object.keys(newLocationGroups).length > 0
? [
{
id: 'new-location-groups',
id: sectionIds.newLocationGroups,
text: 'New location groups',
},
]
: []),
{
id: 'new-locations',
id: sectionIds.newLocations,
text: 'New locations',
subNavItems: getSubNavItems(newLocations, 'new-locations'),
subNavItems: getSubNavItems(newLocations, sectionIds.newLocationsLevel),
},
{
id: 'auto-mapped-locations',
id: sectionIds.autoMappedLocations,
text: 'Auto mapped locations',
subNavItems: getSubNavItems(autoMappedLocations, 'auto-mapped'),
subNavItems: getSubNavItems(
autoMappedLocations,
sectionIds.autoMappedLocationsLevel,
),
},
];
}, [
Expand Down Expand Up @@ -326,7 +341,10 @@ export default function ReleaseApiDataSetLocationsMappingPage() {

{Object.keys(deletedLocationGroups).length > 0 && (
<>
<h3 className="govuk-heading-l" id="deleted-location-groups">
<h3
className="govuk-heading-l"
id={sectionIds.deletedLocationGroups}
>
{`Location groups not found in new data set (${
Object.keys(deletedLocationGroups).length
}) `}
Expand All @@ -339,7 +357,7 @@ export default function ReleaseApiDataSetLocationsMappingPage() {
</>
)}

<h3 className="govuk-heading-l" id="unmapped-locations">
<h3 className="govuk-heading-l" id={sectionIds.mappableLocations}>
Locations not found in new data set
</h3>

Expand Down Expand Up @@ -403,7 +421,7 @@ export default function ReleaseApiDataSetLocationsMappingPage() {
<>
<h3
className="govuk-heading-l govuk-!-margin-top-8"
id="new-location-groups"
id={sectionIds.newLocationGroups}
>
{`New location groups (${
Object.keys(deletedLocationGroups).length
Expand All @@ -419,14 +437,17 @@ export default function ReleaseApiDataSetLocationsMappingPage() {

<h3
className="govuk-heading-l govuk-!-margin-top-8"
id="new-locations"
id={sectionIds.newLocations}
>
New locations ({totalNewLocations}){' '}
<Tag colour="grey">No action required</Tag>
</h3>

{totalNewLocations > 0 ? (
<Accordion id="new-locations" testId="new-locations-accordion">
<Accordion
id={`${sectionIds.newLocations}-accordion`}
testId={`${sectionIds.newLocations}-accordion`}
>
{typedKeys(newLocations).map(level => {
const levelNewLocations = newLocations[level];
const groupLabel = locationLevelsMap[level].plural;
Expand All @@ -437,7 +458,7 @@ export default function ReleaseApiDataSetLocationsMappingPage() {
goToTop={false}
heading={`${groupLabel} (${levelNewLocations.length})`}
headingTag="h4"
id={`new-locations-${level}`}
id={sectionIds.newLocationsLevel(level)}
key={level}
>
<ApiDataSetNewItemsTable
Expand Down Expand Up @@ -465,16 +486,16 @@ export default function ReleaseApiDataSetLocationsMappingPage() {

<h3
className="govuk-heading-l govuk-!-margin-top-8"
id="auto-mapped-locations"
id={sectionIds.autoMappedLocations}
>
Auto mapped locations ({totalAutoMappedLocations}){' '}
<Tag colour="grey">No action required</Tag>
</h3>
{totalAutoMappedLocations > 0 ? (
<Accordion
id="auto-mapped"
id={`${sectionIds.autoMappedLocations}-accordion`}
showOpenAll={false}
testId="auto-mapped-accordion"
testId={`${sectionIds.autoMappedLocations}-accordion`}
>
{typedKeys(autoMappedLocations).map(level => {
const levelAutoMappedLocations = autoMappedLocations[level];
Expand All @@ -486,7 +507,7 @@ export default function ReleaseApiDataSetLocationsMappingPage() {
goToTop={false}
heading={`${groupLabel} (${levelAutoMappedLocations.length})`}
headingTag="h4"
id={`auto-mapped-${level}`}
id={sectionIds.autoMappedLocationsLevel(level)}
key={level}
>
<ApiDataSetAutoMappedTable
Expand Down Expand Up @@ -582,12 +603,12 @@ function getSubNavItems(
(AutoMappedLocation | LocationCandidateWithKey | MappableLocation)[]
>
>,
key: string,
) {
sectionId: (level: LocationLevelKey) => string,
): NavItem[] {
return typedKeys(locations).reduce<NavItem[]>((acc, level) => {
if (locations[level]?.length) {
acc.push({
id: `${key}-${level}`,
id: sectionId(level),
text: locationLevelsMap[level]?.plural ?? level,
});
}
Expand Down
Loading
Loading