From 06f26d8d08da4b8d99a7bebaf3d95cef77eae3ef Mon Sep 17 00:00:00 2001 From: Hendrik de Graaf Date: Tue, 22 Sep 2026 11:13:15 +0200 Subject: [PATCH 01/13] refactor: extract output type validity out of use-action-button [DHIS2-22011] Deciding whether there are unapplied changes needs to know which output types the current config can produce, which until now only the action button hook could work out. Co-Authored-By: Claude Opus 5 (1M context) --- .../__tests__/use-action-button.spec.tsx | 36 +- .../action-buttons/use-action-button.ts | 312 +----------------- .../bottom-bar/output-type-validity.ts | 291 ++++++++++++++++ 3 files changed, 336 insertions(+), 303 deletions(-) create mode 100644 src/components/layout-panel/bottom-bar/output-type-validity.ts diff --git a/src/components/layout-panel/bottom-bar/action-buttons/__tests__/use-action-button.spec.tsx b/src/components/layout-panel/bottom-bar/action-buttons/__tests__/use-action-button.spec.tsx index 95b62f385..392b81041 100644 --- a/src/components/layout-panel/bottom-bar/action-buttons/__tests__/use-action-button.spec.tsx +++ b/src/components/layout-panel/bottom-bar/action-buttons/__tests__/use-action-button.spec.tsx @@ -130,6 +130,12 @@ const metadata = { dimensionType: 'PERIOD', valueType: 'DATE', }, + lastUpdated: { + id: 'lastUpdated', + name: 'Last updated on', + dimensionType: 'PERIOD', + valueType: 'DATE', + }, } const initialPreloadedState: Partial = { @@ -291,7 +297,7 @@ describe('useActionButton for Event button', () => { }, visUiConfig: { layout: { - columns: [metadata['tei1.a1'].id], + columns: [metadata.lastUpdated.id], }, visualizationType: 'LINE_LIST', }, @@ -475,7 +481,7 @@ describe('useActionButton for Enrollment button', () => { }, visUiConfig: { layout: { - columns: [metadata['tei1.a1'].id], + columns: [metadata.lastUpdated.id], }, visualizationType: 'LINE_LIST', }, @@ -672,6 +678,32 @@ describe('useActionButton for Enrollment button', () => { }) describe('useActionButton for Tracked entity instance button', () => { + /* A layout of only context-free dimensions resolves no tracked entity type, + * so the builder cannot produce this output type. The button must be + * disabled: clicking it would throw. */ + it('returns correct result for: LL, layout with no tracked entity type', async () => { + const { result } = await renderHookWithAppWrapper( + () => useActionButton('TRACKED_ENTITY_INSTANCE'), + createStoreWithPreloadedState({ + currentVis: { + outputType: 'TRACKED_ENTITY_INSTANCE', + type: 'LINE_LIST', + }, + visUiConfig: { + layout: { + columns: [metadata.lastUpdated.id], + }, + outputType: 'TRACKED_ENTITY_INSTANCE', + visualizationType: 'LINE_LIST', + }, + }) + ) + + expect(result.current.tooltipConfig).toEqual({ + content: 'Not valid without a tracked entity type', + }) + }) + it('returns correct result for: LL, currentVis with outputType !== TRACKED_ENTITY_INSTANCE', async () => { const { result } = await renderHookWithAppWrapper( () => useActionButton('TRACKED_ENTITY_INSTANCE'), diff --git a/src/components/layout-panel/bottom-bar/action-buttons/use-action-button.ts b/src/components/layout-panel/bottom-bar/action-buttons/use-action-button.ts index 7ee22dd14..c510f3a47 100644 --- a/src/components/layout-panel/bottom-bar/action-buttons/use-action-button.ts +++ b/src/components/layout-panel/bottom-bar/action-buttons/use-action-button.ts @@ -1,17 +1,12 @@ -import i18n from '@dhis2/d2-i18n' +import { getOutputTypeTooltipConfig } from '@components/layout-panel/bottom-bar/output-type-validity' import { useAppSelector, useLayoutContext, useMetadataStore } from '@hooks' -import { isDataSourceProgramWithoutRegistration } from '@modules/data-source' -import { isDimensionInLayout } from '@modules/layout' import { isVisualizationEmpty } from '@modules/visualization/state' import { getCurrentVis } from '@store/current-vis-slice' import { - getVisUiConfigLayout, - getVisUiConfigLayoutAllDimensionIds, - getVisUiConfigLayoutIsEmpty, getVisUiConfigOutputType, getVisUiConfigVisualizationType, } from '@store/vis-ui-config-slice' -import type { OutputType, Program } from '@types' +import type { OutputType } from '@types' import { useMemo } from 'react' import type { ButtonAction } from './base-button' @@ -19,182 +14,13 @@ import type { ButtonAction } from './base-button' * and a custom value table. Used to label the EVENT/custom-value buttons. */ export type EventOutputTypeVariant = 'EVENT' | 'CUSTOM_VALUE' -type TooltipConfig = { content: string; openDelay?: number } | undefined - -const getRegistrationOuTooltipContent = (): TooltipConfig => ({ - content: i18n.t('Not valid with registration org. unit'), -}) - -type CategoryLayoutState = { - hasCategoryInLayout: boolean - hasCategoryOptionGroupSetInLayout: boolean -} - -const getCategoryTooltipContent = ({ - hasCategoryInLayout, - hasCategoryOptionGroupSetInLayout, -}: CategoryLayoutState): TooltipConfig => { - if (hasCategoryInLayout && hasCategoryOptionGroupSetInLayout) { - return { - content: i18n.t( - 'Not valid with categories or category option group sets' - ), - } - } - if (hasCategoryInLayout) { - return { content: i18n.t('Not valid with categories') } - } - if (hasCategoryOptionGroupSetInLayout) { - return { content: i18n.t('Not valid with category option group sets') } - } - return undefined -} - -type EventTooltipContentParams = { - hasNoProgramInLayout: boolean - hasMultipleProgramsInLayout: boolean - hasMultipleProgramStagesInLayout: boolean - isRegistrationOuInLayout: boolean - visualizationType: string -} - -const getEventTooltipContent = ({ - hasNoProgramInLayout, - hasMultipleProgramsInLayout, - hasMultipleProgramStagesInLayout, - isRegistrationOuInLayout, - visualizationType, -}: EventTooltipContentParams): TooltipConfig => { - if (hasNoProgramInLayout) { - return { content: i18n.t('Not valid without a program') } - } - - if ( - hasMultipleProgramsInLayout && - (visualizationType === 'LINE_LIST' || - visualizationType === 'PIVOT_TABLE') - ) { - return { content: i18n.t('Not valid with multiple programs') } - } - - if (isRegistrationOuInLayout) { - return getRegistrationOuTooltipContent() - } - - if (hasMultipleProgramStagesInLayout) { - return { content: i18n.t('Not valid with multiple program stages') } - } - - return undefined -} - -type EnrollmentTooltipContentParams = { - programMetadata: Program | undefined - hasCategoryInLayout: boolean - hasCategoryOptionGroupSetInLayout: boolean - hasMultipleProgramsInLayout: boolean - hasNoProgramInLayout: boolean - isRegistrationOuInLayout: boolean - visualizationType: string -} - -const getEnrollmentTooltipContent = ({ - programMetadata, - hasCategoryInLayout, - hasCategoryOptionGroupSetInLayout, - hasNoProgramInLayout, - hasMultipleProgramsInLayout, - isRegistrationOuInLayout, - visualizationType, -}: EnrollmentTooltipContentParams): TooltipConfig => { - if (hasNoProgramInLayout) { - return { content: i18n.t('Not valid without a program') } - } - - if ( - hasMultipleProgramsInLayout && - (visualizationType === 'LINE_LIST' || - visualizationType === 'PIVOT_TABLE') - ) { - return { content: i18n.t('Not valid with multiple programs') } - } - - if (isDataSourceProgramWithoutRegistration(programMetadata)) { - return { content: i18n.t('Not valid with event programs') } - } - - if (isRegistrationOuInLayout) { - return getRegistrationOuTooltipContent() - } - - return getCategoryTooltipContent({ - hasCategoryInLayout, - hasCategoryOptionGroupSetInLayout, - }) -} - -type TrackedEntityInstanceTooltipContentParams = { - programMetadata: Program | undefined - hasCategoryInLayout: boolean - hasCategoryOptionGroupSetInLayout: boolean - hasCompletedOnInLayout: boolean - hasMultipleProgramsInLayout: boolean - hasMultipleTetInLayout: boolean - hasProgramIndicatorsInLayout: boolean - visualizationType: string -} - -const getTrackedEntityInstanceTooltipContent = ({ - programMetadata, - hasCategoryInLayout, - hasCategoryOptionGroupSetInLayout, - hasCompletedOnInLayout, - hasMultipleProgramsInLayout, - hasMultipleTetInLayout, - hasProgramIndicatorsInLayout, - visualizationType, -}: TrackedEntityInstanceTooltipContentParams): TooltipConfig => { - if (hasCompletedOnInLayout) { - return { - content: i18n.t('Not valid with Completed on'), - } - } - - if (hasMultipleTetInLayout) { - return { - content: i18n.t('Not valid with multiple tracked entity types'), - } - } - - if (hasMultipleProgramsInLayout && visualizationType === 'PIVOT_TABLE') { - return { content: i18n.t('Not valid with multiple programs') } - } - - if (isDataSourceProgramWithoutRegistration(programMetadata)) { - return { content: i18n.t('Not valid with event programs') } - } - - if (visualizationType === 'LINE_LIST' && hasProgramIndicatorsInLayout) { - return { content: i18n.t('Not valid with program indicators') } - } - - return getCategoryTooltipContent({ - hasCategoryInLayout, - hasCategoryOptionGroupSetInLayout, - }) -} - export const useActionButton = ( buttonType: OutputType, buttonVariant?: EventOutputTypeVariant ) => { const currentVis = useAppSelector(getCurrentVis) - const { tetId, programStageIds, programIds } = useLayoutContext() - const layout = useAppSelector(getVisUiConfigLayout) - const layoutDimensionIds = useAppSelector( - getVisUiConfigLayoutAllDimensionIds - ) - const isLayoutEmpty = useAppSelector(getVisUiConfigLayoutIsEmpty) + const visUiConfig = useAppSelector((state) => state.visUiConfig) + const { tetId, programIds } = useLayoutContext() const metadataStore = useMetadataStore() const outputType = useAppSelector(getVisUiConfigOutputType) const visualizationType = useAppSelector(getVisUiConfigVisualizationType) @@ -213,7 +39,6 @@ export const useActionButton = ( ) const action = useMemo((): ButtonAction => { - // Empty visualization if (isVisualizationEmpty(currentVis)) { return 'create' } else if (outputType === buttonType) { @@ -234,131 +59,16 @@ export const useActionButton = ( } }, [buttonType, buttonVariant, currentVis, outputType, visualizationType]) - const hasCategoryInLayout: boolean = useMemo( + const tooltipConfig = useMemo( () => - layoutDimensionIds.some( - (dimensionId) => - metadataStore.getDimensionMetadataItem(dimensionId) - ?.dimensionType === 'CATEGORY' - ), - [layoutDimensionIds, metadataStore] + getOutputTypeTooltipConfig({ + outputType: buttonType, + visUiConfig, + metadataStore, + }), + [buttonType, visUiConfig, metadataStore] ) - const hasCategoryOptionGroupSetInLayout: boolean = useMemo( - () => - layoutDimensionIds.some( - (dimensionId) => - metadataStore.getDimensionMetadataItem(dimensionId) - ?.dimensionType === 'CATEGORY_OPTION_GROUP_SET' - ), - [layoutDimensionIds, metadataStore] - ) - - const hasCompletedOnInLayout: boolean = useMemo( - () => layoutDimensionIds.includes('completed'), - [layoutDimensionIds] - ) - - const programCountInLayout = programIds.length - - const tetCountInLayout = useMemo(() => { - const tetIds = new Set() - - layoutDimensionIds.forEach((dimensionId) => { - const tetId = - metadataStore.getDimensionMetadataItem( - dimensionId - )?.trackedEntityTypeId - - if (tetId) { - tetIds.add(tetId) - } - }) - - return tetIds.size - }, [layoutDimensionIds, metadataStore]) - - const hasNoProgramInLayout: boolean = programCountInLayout === 0 - const hasMultipleProgramsInLayout: boolean = programCountInLayout > 1 - const hasMultipleTetInLayout: boolean = tetCountInLayout > 1 - - const hasMultipleProgramStagesInLayout: boolean = programStageIds.length > 1 - - const hasProgramIndicatorsInLayout: boolean = useMemo( - () => - layoutDimensionIds.some( - (dimensionId) => - metadataStore.getDimensionMetadataItem(dimensionId) - ?.dimensionType === 'PROGRAM_INDICATOR' - ), - [layoutDimensionIds, metadataStore] - ) - - const isRegistrationOuInLayout = useMemo( - () => - tetId - ? isDimensionInLayout(layout, `${tetId}.enrollmentOu`) - : false, - [layout, tetId] - ) - - const tooltipConfig = useMemo((): TooltipConfig => { - if (isLayoutEmpty) { - return { - content: i18n.t( - 'Nothing selected. Add items to the layout to get started.' - ), - openDelay: 1000, - } - } - - switch (buttonType) { - case 'EVENT': - return getEventTooltipContent({ - hasNoProgramInLayout, - hasMultipleProgramsInLayout, - hasMultipleProgramStagesInLayout, - isRegistrationOuInLayout, - visualizationType, - }) - case 'ENROLLMENT': - return getEnrollmentTooltipContent({ - programMetadata: firstProgramMetadata, - hasCategoryInLayout, - hasCategoryOptionGroupSetInLayout, - hasNoProgramInLayout, - hasMultipleProgramsInLayout, - isRegistrationOuInLayout, - visualizationType, - }) - case 'TRACKED_ENTITY_INSTANCE': - return getTrackedEntityInstanceTooltipContent({ - programMetadata: firstProgramMetadata, - hasCategoryInLayout, - hasCategoryOptionGroupSetInLayout, - hasCompletedOnInLayout, - hasMultipleProgramsInLayout, - hasMultipleTetInLayout, - hasProgramIndicatorsInLayout, - visualizationType, - }) - } - }, [ - buttonType, - firstProgramMetadata, - hasCategoryInLayout, - hasCategoryOptionGroupSetInLayout, - hasCompletedOnInLayout, - hasNoProgramInLayout, - hasMultipleProgramsInLayout, - hasMultipleProgramStagesInLayout, - hasMultipleTetInLayout, - hasProgramIndicatorsInLayout, - isLayoutEmpty, - isRegistrationOuInLayout, - visualizationType, - ]) - const dataSourceMetadata = buttonType === 'TRACKED_ENTITY_INSTANCE' ? tetMetadata diff --git a/src/components/layout-panel/bottom-bar/output-type-validity.ts b/src/components/layout-panel/bottom-bar/output-type-validity.ts new file mode 100644 index 000000000..405617711 --- /dev/null +++ b/src/components/layout-panel/bottom-bar/output-type-validity.ts @@ -0,0 +1,291 @@ +import i18n from '@dhis2/d2-i18n' +import { isDataSourceProgramWithoutRegistration } from '@modules/data-source' +import { isDimensionInLayout, resolveLayoutContext } from '@modules/layout' +import { + selectLayoutAllDimensionIds, + type VisUiConfigState, +} from '@store/vis-ui-config-slice' +import type { + MetadataStore, + OutputType, + Program, + VisualizationType, +} from '@types' + +/* The output types the bottom bar offers for a visualization type. A pivot + * table has no tracked entity output; its third button is the custom value + * variant of EVENT, not an output type of its own. */ +export const getAvailableOutputTypes = ( + visualizationType: VisualizationType +): OutputType[] => + visualizationType === 'PIVOT_TABLE' + ? ['ENROLLMENT', 'EVENT'] + : ['TRACKED_ENTITY_INSTANCE', 'ENROLLMENT', 'EVENT'] + +export type TooltipConfig = { content: string; openDelay?: number } | undefined + +const getRegistrationOuTooltipContent = (): TooltipConfig => ({ + content: i18n.t('Not valid with registration org. unit'), +}) + +type CategoryLayoutState = { + hasCategoryInLayout: boolean + hasCategoryOptionGroupSetInLayout: boolean +} + +const getCategoryTooltipContent = ({ + hasCategoryInLayout, + hasCategoryOptionGroupSetInLayout, +}: CategoryLayoutState): TooltipConfig => { + if (hasCategoryInLayout && hasCategoryOptionGroupSetInLayout) { + return { + content: i18n.t( + 'Not valid with categories or category option group sets' + ), + } + } + if (hasCategoryInLayout) { + return { content: i18n.t('Not valid with categories') } + } + if (hasCategoryOptionGroupSetInLayout) { + return { content: i18n.t('Not valid with category option group sets') } + } + return undefined +} + +type EventTooltipContentParams = { + hasNoProgramInLayout: boolean + hasMultipleProgramsInLayout: boolean + hasMultipleProgramStagesInLayout: boolean + isRegistrationOuInLayout: boolean + visualizationType: string +} + +const getEventTooltipContent = ({ + hasNoProgramInLayout, + hasMultipleProgramsInLayout, + hasMultipleProgramStagesInLayout, + isRegistrationOuInLayout, + visualizationType, +}: EventTooltipContentParams): TooltipConfig => { + if (hasNoProgramInLayout) { + return { content: i18n.t('Not valid without a program') } + } + + if ( + hasMultipleProgramsInLayout && + (visualizationType === 'LINE_LIST' || + visualizationType === 'PIVOT_TABLE') + ) { + return { content: i18n.t('Not valid with multiple programs') } + } + + if (isRegistrationOuInLayout) { + return getRegistrationOuTooltipContent() + } + + if (hasMultipleProgramStagesInLayout) { + return { content: i18n.t('Not valid with multiple program stages') } + } + + return undefined +} + +type EnrollmentTooltipContentParams = { + programMetadata: Program | undefined + hasCategoryInLayout: boolean + hasCategoryOptionGroupSetInLayout: boolean + hasMultipleProgramsInLayout: boolean + hasNoProgramInLayout: boolean + isRegistrationOuInLayout: boolean + visualizationType: string +} + +const getEnrollmentTooltipContent = ({ + programMetadata, + hasCategoryInLayout, + hasCategoryOptionGroupSetInLayout, + hasNoProgramInLayout, + hasMultipleProgramsInLayout, + isRegistrationOuInLayout, + visualizationType, +}: EnrollmentTooltipContentParams): TooltipConfig => { + if (hasNoProgramInLayout) { + return { content: i18n.t('Not valid without a program') } + } + + if ( + hasMultipleProgramsInLayout && + (visualizationType === 'LINE_LIST' || + visualizationType === 'PIVOT_TABLE') + ) { + return { content: i18n.t('Not valid with multiple programs') } + } + + if (isDataSourceProgramWithoutRegistration(programMetadata)) { + return { content: i18n.t('Not valid with event programs') } + } + + if (isRegistrationOuInLayout) { + return getRegistrationOuTooltipContent() + } + + return getCategoryTooltipContent({ + hasCategoryInLayout, + hasCategoryOptionGroupSetInLayout, + }) +} + +type TrackedEntityInstanceTooltipContentParams = { + programMetadata: Program | undefined + hasCategoryInLayout: boolean + hasCategoryOptionGroupSetInLayout: boolean + hasCompletedOnInLayout: boolean + hasMultipleProgramsInLayout: boolean + hasMultipleTetInLayout: boolean + hasNoTetInLayout: boolean + hasProgramIndicatorsInLayout: boolean + visualizationType: string +} + +const getTrackedEntityInstanceTooltipContent = ({ + programMetadata, + hasCategoryInLayout, + hasCategoryOptionGroupSetInLayout, + hasCompletedOnInLayout, + hasMultipleProgramsInLayout, + hasMultipleTetInLayout, + hasNoTetInLayout, + hasProgramIndicatorsInLayout, + visualizationType, +}: TrackedEntityInstanceTooltipContentParams): TooltipConfig => { + if (hasCompletedOnInLayout) { + return { + content: i18n.t('Not valid with Completed on'), + } + } + + if (hasMultipleTetInLayout) { + return { + content: i18n.t('Not valid with multiple tracked entity types'), + } + } + + if (hasMultipleProgramsInLayout && visualizationType === 'PIVOT_TABLE') { + return { content: i18n.t('Not valid with multiple programs') } + } + + if (isDataSourceProgramWithoutRegistration(programMetadata)) { + return { content: i18n.t('Not valid with event programs') } + } + + /* No tracked entity type can be resolved from the layout, so the output + * cannot be built. Reported after the event-program case, which is the + * more informative reason when it applies. */ + if (hasNoTetInLayout) { + return { content: i18n.t('Not valid without a tracked entity type') } + } + + if (visualizationType === 'LINE_LIST' && hasProgramIndicatorsInLayout) { + return { content: i18n.t('Not valid with program indicators') } + } + + return getCategoryTooltipContent({ + hasCategoryInLayout, + hasCategoryOptionGroupSetInLayout, + }) +} +/* Why an output type cannot be produced from this config, or undefined when it + * can. The single source of truth for output type validity: the buttons + * disable on it, and the unapplied changes hint uses it to decide whether the + * config is applicable at all. */ +export const getOutputTypeTooltipConfig = ({ + outputType, + visUiConfig, + metadataStore, +}: { + outputType: OutputType + visUiConfig: VisUiConfigState + metadataStore: MetadataStore +}): TooltipConfig => { + const { layout, visualizationType } = visUiConfig + const layoutDimensionIds = selectLayoutAllDimensionIds(visUiConfig) + + if (!layoutDimensionIds.length) { + return { + content: i18n.t( + 'Nothing selected. Add items to the layout to get started.' + ), + openDelay: 1000, + } + } + + const { tetId, programIds, programStageIds } = resolveLayoutContext( + layoutDimensionIds, + metadataStore + ) + + const dimensionTypeCount = (dimensionType: string): number => + layoutDimensionIds.filter( + (dimensionId) => + metadataStore.getDimensionMetadataItem(dimensionId) + ?.dimensionType === dimensionType + ).length + + const tetIdsInLayout = new Set( + layoutDimensionIds + .map( + (dimensionId) => + metadataStore.getDimensionMetadataItem(dimensionId) + ?.trackedEntityTypeId + ) + .filter(Boolean) + ) + + const programMetadata = programIds[0] + ? metadataStore.getProgramMetadataItem(programIds[0]) + : undefined + const hasCategoryInLayout = dimensionTypeCount('CATEGORY') > 0 + const hasCategoryOptionGroupSetInLayout = + dimensionTypeCount('CATEGORY_OPTION_GROUP_SET') > 0 + const hasNoProgramInLayout = programIds.length === 0 + const hasMultipleProgramsInLayout = programIds.length > 1 + const isRegistrationOuInLayout = tetId + ? isDimensionInLayout(layout, `${tetId}.enrollmentOu`) + : false + + switch (outputType) { + case 'EVENT': + return getEventTooltipContent({ + hasNoProgramInLayout, + hasMultipleProgramsInLayout, + hasMultipleProgramStagesInLayout: programStageIds.length > 1, + isRegistrationOuInLayout, + visualizationType, + }) + case 'ENROLLMENT': + return getEnrollmentTooltipContent({ + programMetadata, + hasCategoryInLayout, + hasCategoryOptionGroupSetInLayout, + hasNoProgramInLayout, + hasMultipleProgramsInLayout, + isRegistrationOuInLayout, + visualizationType, + }) + case 'TRACKED_ENTITY_INSTANCE': + return getTrackedEntityInstanceTooltipContent({ + programMetadata, + hasCategoryInLayout, + hasCategoryOptionGroupSetInLayout, + hasCompletedOnInLayout: + layoutDimensionIds.includes('completed'), + hasMultipleProgramsInLayout, + hasMultipleTetInLayout: tetIdsInLayout.size > 1, + hasNoTetInLayout: !tetId, + hasProgramIndicatorsInLayout: + dimensionTypeCount('PROGRAM_INDICATOR') > 0, + visualizationType, + }) + } +} From eb8595c48daf4351ccf2bcc80dc8e40b8b56fbbb Mon Sep 17 00:00:00 2001 From: Hendrik de Graaf Date: Tue, 22 Sep 2026 11:13:21 +0200 Subject: [PATCH 02/13] fix: drop wire-only dimensions when normalizing a loaded visualization [DHIS2-22011] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A legacy Event Report persists its value column as a dy/latitude/longitude dimension. The app expresses that with the custom value fields instead and holds no layout position for them, so they survived a load without ever being rebuildable from visUiConfig — enough on its own to make a freshly loaded visualization look edited. Co-Authored-By: Claude Opus 5 (1M context) --- .../__tests__/visualization.spec.ts | 30 +++++++++++++++++++ src/modules/visualization/state.ts | 24 ++++++++++++--- 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/src/modules/visualization/__tests__/visualization.spec.ts b/src/modules/visualization/__tests__/visualization.spec.ts index a8e3ae634..26cbdf686 100644 --- a/src/modules/visualization/__tests__/visualization.spec.ts +++ b/src/modules/visualization/__tests__/visualization.spec.ts @@ -517,6 +517,36 @@ describe('normalizeApiSavedVisualization', () => { ...(vis.filters ?? []), ].map((dim) => dim.dimension) + it.each(['dy', 'latitude', 'longitude'])( + 'drops the wire-only %s dimension and marks the vis legacy', + (wireOnlyId) => { + const result = normalizeApiSavedVisualization( + buildApiVis({ + rows: [ + { dimension: wireOnlyId }, + { dimension: 'ou', dimensionType: 'ORGANISATION_UNIT' }, + ] as ApiSavedVisualization['rows'], + }) + ) + + expect(dimensionsOf(result)).toEqual(['ou']) + expect(result.legacy).toBe(true) + } + ) + + it('does not mark a vis legacy when it carries no wire-only dimension', () => { + const result = normalizeApiSavedVisualization( + buildApiVis({ + rows: [ + { dimension: 'ou', dimensionType: 'ORGANISATION_UNIT' }, + ] as ApiSavedVisualization['rows'], + }) + ) + + expect(dimensionsOf(result)).toEqual(['ou']) + expect(result.legacy).toBeUndefined() + }) + it.each([ ['createdDate', 'created'], ['completedDate', 'completed'], diff --git a/src/modules/visualization/state.ts b/src/modules/visualization/state.ts index 3b6f6cfed..fdb1947e9 100644 --- a/src/modules/visualization/state.ts +++ b/src/modules/visualization/state.ts @@ -546,6 +546,7 @@ const normalizeLegacyDimension = ( * data-element / attribute UID, since that's still a live analytics * parameter * - Drop top-level `program` and `programStage` + * - Drop the wire-only `dy`/`latitude`/`longitude` dimensions * - Mark output as `legacy: true` whenever any of the above upgraded the * persisted shape, so the vis cannot be overwritten in place — only "Save * as" is allowed. Overwriting would silently persist in the canonical @@ -562,7 +563,6 @@ const normalizeLegacyDimension = ( * visualizations, so they do not imply the `legacy` flag): * - `completedOnly` → `eventStatus=COMPLETED` filter (not legacy-only) * - `PROGRAM_DATA_ELEMENT` → `DATA_ELEMENT` (wire → app shape) - * - `dy`/`latitude`/`longitude` stripping (wire → app shape) */ export const normalizeApiSavedVisualization = ( apiVis: ApiSavedVisualization @@ -607,16 +607,32 @@ export const normalizeApiSavedVisualization = ( : []), ] + /* Wire-only dimensions mark where a legacy Event Report put its value + * column; the app expresses that with the custom value fields instead, and + * carries no layout position for them. Dropping them changes the persisted + * shape, so it flips `legacy` like every other upgrade here. */ + const dropWireOnlyDimensions = (dims: DimensionRecord[]) => { + const kept = dims.filter( + (dim) => !WIRE_ONLY_DIMENSIONS.has(dim.dimension) + ) + if (kept.length !== dims.length) { + normalizedVis.legacy = true + } + return kept + } + normalizedVis.columns = dropInvalidGrouping( - columns.map((dim) => + dropWireOnlyDimensions(columns).map((dim) => normalizeLegacyDimension(dim, context, normalizedVis) ) ) normalizedVis.rows = dropInvalidGrouping( - rows.map((dim) => normalizeLegacyDimension(dim, context, normalizedVis)) + dropWireOnlyDimensions(rows).map((dim) => + normalizeLegacyDimension(dim, context, normalizedVis) + ) ) normalizedVis.filters = dropInvalidGrouping( - rawFilters.map((dim) => + dropWireOnlyDimensions(rawFilters).map((dim) => normalizeLegacyDimension(dim, context, normalizedVis) ) ) From c20b536837d40bb3c2cc6887bf8590dd78e1d4ab Mon Sep 17 00:00:00 2001 From: Hendrik de Graaf Date: Tue, 22 Sep 2026 11:13:32 +0200 Subject: [PATCH 03/13] refactor: compare any two visualizations for equivalence [DHIS2-22011] areVisualizationsEquivalent only had to answer "does the saved visualization differ from the current one". Comparing the current visualization against the one visUiConfig would produce needs the same notion of equivalence, so export it and fold the per-field rules into one helper. Fields the API returns richer than visUiConfig can rebuild now compare by id: option sets and legend sets carry a name, a repetition carries the context the backend derives from its dimension, and the custom value carries both a name and an aggregation type. Co-Authored-By: Claude Opus 5 (1M context) --- .../__tests__/unapplied-changes.spec.ts | 287 ++++++++++++++++++ src/modules/visualization/state.ts | 110 ++++--- src/types/visualization.ts | 16 +- 3 files changed, 368 insertions(+), 45 deletions(-) create mode 100644 src/modules/visualization/__tests__/unapplied-changes.spec.ts diff --git a/src/modules/visualization/__tests__/unapplied-changes.spec.ts b/src/modules/visualization/__tests__/unapplied-changes.spec.ts new file mode 100644 index 000000000..83bee5327 --- /dev/null +++ b/src/modules/visualization/__tests__/unapplied-changes.spec.ts @@ -0,0 +1,287 @@ +import { DEFAULT_OPTIONS } from '@constants/options' +import { + areVisualizationsEquivalent, + getVisualizationUiConfig, +} from '@modules/visualization/state' +import { buildCurrentVisFromVisUiConfig } from '@store/thunks' +import type { VisUiConfigState } from '@store/vis-ui-config-slice' +import { createMetadataStoreStub } from '@test-utils/metadata-store-stub' +import type { + CurrentVisualization, + DimensionMetadataItem, + DimensionRecord, + MetadataStore, + Program, +} from '@types' +import { describe, it, expect } from 'vitest' + +const PROGRAM_ID = 'program1' +const STAGE_ID = 'stage1' +const DIMENSION_ID = `${STAGE_ID}.de1` +const OPTION_SET_ID = 'optionSet1' +const LEGEND_SET_ID = 'legendSet1' +const CUSTOM_VALUE_ID = 'de2' + +const metadataStore = createMetadataStoreStub({ + dimensions: { + [DIMENSION_ID]: { + id: DIMENSION_ID, + name: 'Data element 1', + dimensionId: 'de1', + programStageId: STAGE_ID, + dimensionType: 'DATA_ELEMENT', + valueType: 'TEXT', + } as DimensionMetadataItem, + }, +}) + +const baseCurrentVis = { + type: 'LINE_LIST', + outputType: 'EVENT', + columns: [], + rows: [], + filters: [], +} as unknown as CurrentVisualization + +/* Compose the two functions exactly as useHasUnappliedChanges does, so these + * tests cannot drift from the app. */ +const hasUnappliedChanges = ( + currentVis: CurrentVisualization, + visUiConfigOverrides: Partial = {}, + store: MetadataStore = metadataStore +): boolean => + !areVisualizationsEquivalent( + currentVis, + buildCurrentVisFromVisUiConfig({ + previousCurrentVis: currentVis, + visUiConfig: { + ...getVisualizationUiConfig(currentVis, DEFAULT_OPTIONS), + ...visUiConfigOverrides, + }, + metadataStore: store, + }) + ) + +describe('detecting unapplied changes', () => { + it('reports no change for a visualization rebuilt from its own ui config', () => { + expect(hasUnappliedChanges(baseCurrentVis)).toBe(false) + }) + + it('reports a change when an option is changed', () => { + expect( + hasUnappliedChanges(baseCurrentVis, { + options: { ...DEFAULT_OPTIONS, displayDensity: 'COMFORTABLE' }, + }) + ).toBe(true) + }) + + it('reports no change when an option is set to its own default value', () => { + expect( + hasUnappliedChanges(baseCurrentVis, { + options: { + ...DEFAULT_OPTIONS, + showData: DEFAULT_OPTIONS.showData, + }, + }) + ).toBe(false) + }) + + it('reports a change when a dimension is added to an axis', () => { + expect( + hasUnappliedChanges(baseCurrentVis, { + layout: { columns: [DIMENSION_ID], rows: [], filters: [] }, + }) + ).toBe(true) + }) + + it('reports a change when the visualization type is switched', () => { + expect( + hasUnappliedChanges(baseCurrentVis, { + visualizationType: 'PIVOT_TABLE', + }) + ).toBe(true) + }) + + it('reports no change for a visualization that has been sorted on the canvas', () => { + const sortedVis = { + ...baseCurrentVis, + sorting: [{ dimension: 'de1', direction: 'ASC' }], + } as unknown as CurrentVisualization + + expect(hasUnappliedChanges(sortedVis)).toBe(false) + }) + + /* The builder only populates the custom value fields when the current vis + * already carries one, so configuring a custom value against a vis without + * one leaves both sides undefined. */ + it('reports no change when a custom value is configured on a visualization that has none', () => { + expect( + hasUnappliedChanges(baseCurrentVis, { + customValue: { id: CUSTOM_VALUE_ID, aggregationType: 'SUM' }, + }) + ).toBe(false) + }) +}) + +/* A freshly loaded visualization is compared in the shape the eventVisualizations + * API returns it, against the shape the builder produces from its own ui config. + * The API returns more per dimension than the builder can rebuild, and none of + * that counts as an unapplied change. */ +describe('detecting unapplied changes in a freshly loaded visualization', () => { + const createLoadedMetadataStore = ( + dimensionOverrides: Partial = {} + ): MetadataStore => + createMetadataStoreStub({ + dimensions: { + [DIMENSION_ID]: { + id: DIMENSION_ID, + name: 'Data element 1', + dimensionId: 'de1', + programId: PROGRAM_ID, + programStageId: STAGE_ID, + dimensionType: 'DATA_ELEMENT', + valueType: 'TEXT', + ...dimensionOverrides, + } as DimensionMetadataItem, + }, + programs: { + [PROGRAM_ID]: { id: PROGRAM_ID, name: 'Program 1' } as Program, + }, + }) + + // The shape the API returns for a data element dimension. + const createLoadedColumn = ( + overrides: Partial = {} + ): DimensionRecord => ({ + dimension: 'de1', + dimensionType: 'PROGRAM_DATA_ELEMENT', + program: { id: PROGRAM_ID }, + programStage: { id: STAGE_ID }, + valueType: 'TEXT', + items: [], + ...overrides, + }) + + const createLoadedVis = ( + column: DimensionRecord, + overrides: Partial = {} + ): CurrentVisualization => + ({ + ...baseCurrentVis, + columns: [column], + ...overrides, + }) as unknown as CurrentVisualization + + it('reports no change for a dimension carrying an option set with its name', () => { + const loadedVis = createLoadedVis( + createLoadedColumn({ + optionSet: { id: OPTION_SET_ID, name: 'Option set 1' }, + }) + ) + + expect( + hasUnappliedChanges( + loadedVis, + {}, + createLoadedMetadataStore({ optionSetId: OPTION_SET_ID }) + ) + ).toBe(false) + }) + + it('reports no change for a dimension carrying a legend set with its name', () => { + const loadedVis = createLoadedVis( + createLoadedColumn({ + legendSet: { id: LEGEND_SET_ID, name: 'Legend set 1' }, + }) + ) + + expect( + hasUnappliedChanges(loadedVis, {}, createLoadedMetadataStore()) + ).toBe(false) + }) + + const createLoadedRepetitionColumn = (): DimensionRecord => + createLoadedColumn({ + repetition: { + indexes: [1, 2], + dimension: 'de1', + parent: 'COLUMN', + program: PROGRAM_ID, + programStage: STAGE_ID, + }, + }) + + it('reports a change when a repetition is edited', () => { + const loadedVis = createLoadedVis(createLoadedRepetitionColumn()) + + expect( + hasUnappliedChanges( + loadedVis, + { + repetitionsByDimension: { + [DIMENSION_ID]: { mostRecent: 3, oldest: 0 }, + }, + }, + createLoadedMetadataStore() + ) + ).toBe(true) + }) + + it('reports no change for a dimension carrying a repetition with its backend-derived context', () => { + const loadedVis = createLoadedVis(createLoadedRepetitionColumn()) + + expect( + hasUnappliedChanges(loadedVis, {}, createLoadedMetadataStore()) + ).toBe(false) + }) + + it('reports no change for a visualization carrying a custom value with its name and aggregation type', () => { + const loadedVis = createLoadedVis(createLoadedColumn(), { + type: 'PIVOT_TABLE', + value: { + id: CUSTOM_VALUE_ID, + name: 'Weight in kg', + aggregationType: 'AVERAGE', + }, + aggregationType: 'AVERAGE', + }) + + expect( + hasUnappliedChanges(loadedVis, {}, createLoadedMetadataStore()) + ).toBe(false) + }) + + /* Known gap, pending the custom value rework: for a custom value + * visualization saved without a top-level aggregationType the ui config + * defaults the custom value's aggregation type to DEFAULT, which is not + * the SUM the options default to, so the two sides differ on + * aggregationType alone and the notice appears with nothing changed. + * Accommodating this would mean custom-value-specific logic in the + * comparison, which the rework will make obsolete. */ + it('wrongly reports a change for a custom value visualization with no persisted aggregation type', () => { + const loadedVis = createLoadedVis(createLoadedColumn(), { + type: 'PIVOT_TABLE', + value: { id: CUSTOM_VALUE_ID }, + }) + + expect( + hasUnappliedChanges(loadedVis, {}, createLoadedMetadataStore()) + ).toBe(true) + }) + + it('reports a change when a dimension is added to a loaded visualization', () => { + const loadedVis = createLoadedVis( + createLoadedColumn({ + optionSet: { id: OPTION_SET_ID, name: 'Option set 1' }, + }) + ) + + expect( + hasUnappliedChanges( + loadedVis, + { layout: { columns: [], rows: [], filters: [DIMENSION_ID] } }, + createLoadedMetadataStore({ optionSetId: OPTION_SET_ID }) + ) + ).toBe(true) + }) +}) diff --git a/src/modules/visualization/state.ts b/src/modules/visualization/state.ts index fdb1947e9..315f082ce 100644 --- a/src/modules/visualization/state.ts +++ b/src/modules/visualization/state.ts @@ -135,53 +135,81 @@ export const isDefaultOptionValue = (key: string, value: unknown): boolean => value === undefined || deepEqual(value, (DEFAULT_OPTIONS as Record)[key]) -/* An axis prepared for comparison: drop the props that aren't persisted - * (dimensionType, valueType — the API sends PROGRAM_DATA_ELEMENT where the - * rebuilt vis has DATA_ELEMENT) and treat an empty items array as absent, so - * unpersisted differences don't read as edits. */ +/* An axis prepared for comparison. Two kinds of difference are not edits: + * props that aren't persisted (dimensionType, valueType — the API sends + * PROGRAM_DATA_ELEMENT where the rebuilt vis has DATA_ELEMENT) and nested + * objects the API returns richer than the app can rebuild from visUiConfig + * (option sets and legend sets carry their name; a repetition carries the + * dimension, axis and program context the backend derives from the owning + * dimension). An empty items array counts as absent. */ const comparableAxis = (axis: DimensionArray = []): DimensionArray => removeDimensionPropertiesBeforeSaving(axis).map((dim) => { - if (Array.isArray(dim.items) && dim.items.length === 0) { - const withoutItems = { ...dim } - delete withoutItems.items - return withoutItems + const comparableDim = { ...dim } + if (Array.isArray(comparableDim.items) && !comparableDim.items.length) { + delete comparableDim.items } - return dim - }) - -const areVisualizationsEquivalent = ( - savedVis: CurrentVisualization, - currentVis: CurrentVisualization -): boolean => { - const saved = savedVis as Record - const current = currentVis as Record - // currentVis always carries the full key set, so its keys cover every - // field a saved vis could differ on. - for (const key of Object.keys(current)) { - if (key in DEFAULT_OPTIONS) { - const bothAtDefault = - isDefaultOptionValue(key, saved[key]) && - isDefaultOptionValue(key, current[key]) - if (!bothAtDefault && !deepEqual(saved[key], current[key])) { - return false - } - } else if (DIMENSION_AXES.has(key)) { - if ( - !deepEqual( - comparableAxis(saved[key] as DimensionArray), - comparableAxis(current[key] as DimensionArray) - ) - ) { - return false + if (comparableDim.optionSet) { + comparableDim.optionSet = { id: comparableDim.optionSet.id } + } + if (comparableDim.legendSet) { + comparableDim.legendSet = { id: comparableDim.legendSet.id } + } + if (comparableDim.repetition) { + comparableDim.repetition = { + indexes: comparableDim.repetition.indexes, } - } else if ( - !DERIVED_LAYOUT_FIELDS.has(key) && - !deepEqual(saved[key], current[key]) - ) { - return false } + return comparableDim + }) + +/* Top-level metadata refs the API returns with a display name (and, for the + * custom value, its aggregation type) where visUiConfig can only rebuild the + * id. Compared by id, for the same reason comparableAxis reduces optionSet and + * legendSet. */ +const ID_REF_FIELDS: ReadonlySet = new Set(['value']) + +const comparableIdRef = (ref: unknown): unknown => + ref && typeof ref === 'object' && 'id' in ref + ? { id: (ref as { id: string }).id } + : ref + +/* How one field of two visualizations compares. Each kind of field has its own + * notion of equivalence: a metadata ref by id, an option with an absent value + * counting as its default, an axis after normalisation, and the layout-derived + * fields not at all. */ +const isFieldEquivalent = (key: string, a: unknown, b: unknown): boolean => { + if (ID_REF_FIELDS.has(key)) { + return deepEqual(comparableIdRef(a), comparableIdRef(b)) + } + + if (key in DEFAULT_OPTIONS) { + const bothAtDefault = + isDefaultOptionValue(key, a) && isDefaultOptionValue(key, b) + + return bothAtDefault || deepEqual(a, b) + } + + if (DIMENSION_AXES.has(key)) { + return deepEqual( + comparableAxis(a as DimensionArray), + comparableAxis(b as DimensionArray) + ) } - return true + + return DERIVED_LAYOUT_FIELDS.has(key) || deepEqual(a, b) +} + +/* Compares a saved vis to the current one, and the current one to the vis that + * visUiConfig would produce. `visualizationB` must carry the full + * CurrentVisualization key set, because its keys drive the comparison. */ +export const areVisualizationsEquivalent = ( + visualizationA: CurrentVisualization | EmptyVisualization, + visualizationB: CurrentVisualization +): boolean => { + const a = visualizationA as Record + const b = visualizationB as Record + + return Object.keys(b).every((key) => isFieldEquivalent(key, a[key], b[key])) } export const getVisualizationState = ( diff --git a/src/types/visualization.ts b/src/types/visualization.ts index 61a639466..26c82a662 100644 --- a/src/types/visualization.ts +++ b/src/types/visualization.ts @@ -38,9 +38,11 @@ export type DimensionRecord = { filter?: string program?: IdRecord programStage?: IdRecord - optionSet?: IdRecord + /* The API returns the name of an option set and a legend set alongside its + * id (see dimensionFields); the app only ever writes the id. */ + optionSet?: IdRecord & { name?: string } valueType?: ValueType - legendSet?: IdRecord + legendSet?: IdRecord & { name?: string } repetition?: { indexes: number[] dimension?: string @@ -89,8 +91,14 @@ type SavedVisualizationFieldOverrides = { } metaData: MetadataInputMap type: VisualizationType - // name does not need to be propagated to currentVis on update - value?: IdRecord & { name?: string } + /* The API returns the custom value's name and aggregation type alongside + * its id; the app only ever writes the id (the aggregation type it applies + * is the top-level one). The name does not need to be propagated to + * currentVis on update. */ + value?: IdRecord & { + name?: string + aggregationType?: EventVisualizationGenerated['aggregationType'] + } } /** From 0602c1fc60727194c6aec83141b214e35131273e Mon Sep 17 00:00:00 2001 From: Hendrik de Graaf Date: Tue, 22 Sep 2026 11:13:52 +0200 Subject: [PATCH 04/13] feat: show an unapplied changes indicator in the layout panel [DHIS2-22011] Editing the layout or the options changes visUiConfig, which does not reach the visualization on screen until the user applies it. Nothing said so, which left the canvas silently disagreeing with the panel next to it. The indicator only appears when the user can act on it: it stays hidden when no output type can be produced from the config at all, since every button that would apply the change is disabled in that case. Co-Authored-By: Claude Opus 5 (1M context) --- i18n/en.pot | 10 +- .../__fixtures__/unapplied-changes.ts | 40 +++++++ .../bottom-bar/__tests__/bottom-bar.spec.tsx | 61 ++++++++++ .../use-has-unapplied-changes.spec.ts | 110 ++++++++++++++++++ .../layout-panel/bottom-bar/bottom-bar.tsx | 13 +++ .../bottom-bar/styles/bottom-bar.module.css | 14 +++ .../bottom-bar/use-has-unapplied-changes.ts | 55 +++++++++ 7 files changed, 301 insertions(+), 2 deletions(-) create mode 100644 src/components/layout-panel/bottom-bar/__fixtures__/unapplied-changes.ts create mode 100644 src/components/layout-panel/bottom-bar/__tests__/bottom-bar.spec.tsx create mode 100644 src/components/layout-panel/bottom-bar/__tests__/use-has-unapplied-changes.spec.ts create mode 100644 src/components/layout-panel/bottom-bar/use-has-unapplied-changes.ts diff --git a/i18n/en.pot b/i18n/en.pot index 92b47df6d..2ae6ade99 100644 --- a/i18n/en.pot +++ b/i18n/en.pot @@ -5,8 +5,8 @@ msgstr "" "Content-Type: text/plain; charset=utf-8\n" "Content-Transfer-Encoding: 8bit\n" "Plural-Forms: nplurals=2; plural=(n != 1)\n" -"POT-Creation-Date: 2026-09-17T12:36:05.389Z\n" -"PO-Revision-Date: 2026-09-17T12:36:05.389Z\n" +"POT-Creation-Date: 2026-09-22T09:13:46.110Z\n" +"PO-Revision-Date: 2026-09-22T09:13:46.112Z\n" msgid "" "Some dimensions were not added because they cannot be used in a " @@ -307,6 +307,9 @@ msgstr "Switch to {{- trackedEntityTypeName}} list" msgid "Update {{- trackedEntityTypeName}} list" msgstr "Update {{- trackedEntityTypeName}} list" +msgid "Unapplied changes" +msgstr "Unapplied changes" + msgid "Not valid with registration org. unit" msgstr "Not valid with registration org. unit" @@ -337,6 +340,9 @@ msgstr "Not valid with Completed on" msgid "Not valid with multiple tracked entity types" msgstr "Not valid with multiple tracked entity types" +msgid "Not valid without a tracked entity type" +msgstr "Not valid without a tracked entity type" + msgid "Not valid with program indicators" msgstr "Not valid with program indicators" diff --git a/src/components/layout-panel/bottom-bar/__fixtures__/unapplied-changes.ts b/src/components/layout-panel/bottom-bar/__fixtures__/unapplied-changes.ts new file mode 100644 index 000000000..348b10fd1 --- /dev/null +++ b/src/components/layout-panel/bottom-bar/__fixtures__/unapplied-changes.ts @@ -0,0 +1,40 @@ +import type { CurrentVisualization } from '@types' + +export const PROGRAM_ID = 'program1' +export const STAGE_ID = 'stage1' +export const DIMENSION_ID = `${STAGE_ID}.de1` + +/* An event program, so the EVENT output type is the applicable one — the same + * shape a real layout dimension carries. Without the program the layout would + * have no program at all, which makes every output type inapplicable. */ +export const metadata = { + [PROGRAM_ID]: { + id: PROGRAM_ID, + name: 'Program 1', + programType: 'WITHOUT_REGISTRATION', + }, + [STAGE_ID]: { id: STAGE_ID, name: 'Stage 1', program: { id: PROGRAM_ID } }, + [DIMENSION_ID]: { + id: DIMENSION_ID, + name: 'Data element 1', + dimensionId: 'de1', + programId: PROGRAM_ID, + programStageId: STAGE_ID, + dimensionType: 'DATA_ELEMENT', + valueType: 'TEXT', + }, +} + +/* digitGroupSeparator is seeded onto the default store's visUiConfig from the + * mocked system settings (src/test-utils/__fixtures__/system-settings.json), + * so a currentVis that matches the default ui config must carry the same + * value. The real load path is not affected: the API always returns a + * digitGroupSeparator, so both sides agree there. */ +export const populatedVis = { + type: 'LINE_LIST', + outputType: 'EVENT', + digitGroupSeparator: 'SPACE', + columns: [], + rows: [], + filters: [], +} as unknown as CurrentVisualization diff --git a/src/components/layout-panel/bottom-bar/__tests__/bottom-bar.spec.tsx b/src/components/layout-panel/bottom-bar/__tests__/bottom-bar.spec.tsx new file mode 100644 index 000000000..b2bed3529 --- /dev/null +++ b/src/components/layout-panel/bottom-bar/__tests__/bottom-bar.spec.tsx @@ -0,0 +1,61 @@ +import { renderWithAppWrapper } from '@test-utils/app-wrapper' +import { screen } from '@testing-library/react' +import type { Layout, RootState } from '@types' +import { describe, it, expect } from 'vitest' +import { + DIMENSION_ID, + metadata, + populatedVis, +} from '../__fixtures__/unapplied-changes' +import { BottomBar } from '../bottom-bar' + +const layoutWithDimension = { + columns: [DIMENSION_ID], + rows: [], + filters: [], +} as unknown as Layout + +const renderBottomBar = (preloadedState: Partial) => + renderWithAppWrapper(, { + metadata, + partialStore: { + preloadedState: { + dimensionSelection: { + dataSourceId: 'program1', + } as RootState['dimensionSelection'], + ...preloadedState, + }, + }, + }) + +describe('BottomBar unapplied changes indicator', () => { + it('is absent when the visualization matches the ui config', async () => { + await renderBottomBar({ currentVis: populatedVis }) + + expect( + screen.queryByTestId('unapplied-changes') + ).not.toBeInTheDocument() + }) + + it('is shown when there are unapplied changes', async () => { + await renderBottomBar({ + currentVis: populatedVis, + visUiConfig: { layout: layoutWithDimension }, + } as Partial) + + expect(await screen.findByText('Unapplied changes')).toBeVisible() + }) + + it('is absent while the visualization is loading, along with the buttons', async () => { + await renderBottomBar({ + currentVis: populatedVis, + visUiConfig: { layout: layoutWithDimension }, + loader: { isVisualizationLoading: true }, + } as Partial) + + expect( + screen.queryByTestId('unapplied-changes') + ).not.toBeInTheDocument() + expect(screen.queryByTestId('update-buttons')).not.toBeInTheDocument() + }) +}) diff --git a/src/components/layout-panel/bottom-bar/__tests__/use-has-unapplied-changes.spec.ts b/src/components/layout-panel/bottom-bar/__tests__/use-has-unapplied-changes.spec.ts new file mode 100644 index 000000000..26bdb20ee --- /dev/null +++ b/src/components/layout-panel/bottom-bar/__tests__/use-has-unapplied-changes.spec.ts @@ -0,0 +1,110 @@ +import { tUpdateCurrentVisFromVisUiConfig } from '@store/thunks' +import { + addVisUiConfigLayoutDimension, + initialState as visUiConfigInitialState, +} from '@store/vis-ui-config-slice' +import { renderHookWithAppWrapper } from '@test-utils/app-wrapper' +import { act } from '@testing-library/react' +import type { RootState } from '@types' +import { describe, it, expect } from 'vitest' +import { + DIMENSION_ID, + metadata, + populatedVis, +} from '../__fixtures__/unapplied-changes' +import { useHasUnappliedChanges } from '../use-has-unapplied-changes' + +const renderHookWithVis = ( + currentVis: Partial['currentVis'], + visUiConfig?: Partial['visUiConfig'] +) => + renderHookWithAppWrapper(useHasUnappliedChanges, { + metadata, + partialStore: { + preloadedState: { + currentVis, + ...(visUiConfig && { visUiConfig }), + }, + }, + }) + +describe('useHasUnappliedChanges', () => { + it('is false for an empty visualization', async () => { + const { result } = await renderHookWithVis({}) + + expect(result.current).toBe(false) + }) + + it('is false for a visualization whose ui config matches it', async () => { + const { result } = await renderHookWithVis(populatedVis) + + expect(result.current).toBe(false) + }) + + /* An empty layout makes every output type inapplicable, so there is no + * button to hint at — and the builder could not produce a + * TRACKED_ENTITY_INSTANCE visualization from it either. */ + it('is false when no output type is applicable', async () => { + const { result } = await renderHookWithVis( + { ...populatedVis, outputType: 'TRACKED_ENTITY_INSTANCE' }, + { + ...visUiConfigInitialState, + outputType: 'TRACKED_ENTITY_INSTANCE', + } + ) + + expect(result.current).toBe(false) + }) + + /* The layout holds an event program dimension, so EVENT is applicable and + * TRACKED_ENTITY_INSTANCE is not. Applying requires switching output type, + * which means what is on screen cannot be what the config describes — and + * the comparison could not run for the selected type anyway. */ + it('is true when the selected output type is inapplicable but another one is', async () => { + const { result } = await renderHookWithVis(populatedVis, { + ...visUiConfigInitialState, + outputType: 'TRACKED_ENTITY_INSTANCE', + layout: { columns: [DIMENSION_ID], rows: [], filters: [] }, + }) + + expect(result.current).toBe(true) + }) + + it('becomes true when a dimension is added to the layout', async () => { + const { result, store } = await renderHookWithVis(populatedVis) + + expect(result.current).toBe(false) + + await act(async () => { + store.dispatch( + addVisUiConfigLayoutDimension({ + axis: 'columns', + dimensionId: DIMENSION_ID, + }) + ) + }) + + expect(result.current).toBe(true) + }) + + it('becomes false again once the change is applied', async () => { + const { result, store } = await renderHookWithVis(populatedVis) + + await act(async () => { + store.dispatch( + addVisUiConfigLayoutDimension({ + axis: 'columns', + dimensionId: DIMENSION_ID, + }) + ) + }) + + expect(result.current).toBe(true) + + await act(async () => { + store.dispatch(tUpdateCurrentVisFromVisUiConfig()) + }) + + expect(result.current).toBe(false) + }) +}) diff --git a/src/components/layout-panel/bottom-bar/bottom-bar.tsx b/src/components/layout-panel/bottom-bar/bottom-bar.tsx index 6c08d8428..95901cacc 100644 --- a/src/components/layout-panel/bottom-bar/bottom-bar.tsx +++ b/src/components/layout-panel/bottom-bar/bottom-bar.tsx @@ -1,3 +1,4 @@ +import i18n from '@dhis2/d2-i18n' import { useAppSelector } from '@hooks' import { getDataSourceId } from '@store/dimensions-selection-slice' import { getIsVisualizationLoading } from '@store/loader-slice' @@ -9,11 +10,13 @@ import { EnrollmentButton } from './action-buttons/enrollment-button' import { EventButton } from './action-buttons/event-button' import { TrackedEntityInstanceButton } from './action-buttons/tracked-entity-instance-button' import classes from './styles/bottom-bar.module.css' +import { useHasUnappliedChanges } from './use-has-unapplied-changes' export const BottomBar: FC = () => { const dataSourceId = useAppSelector(getDataSourceId) const isVisualizationLoading = useAppSelector(getIsVisualizationLoading) const visualizationType = useAppSelector(getVisUiConfigVisualizationType) + const hasUnappliedChanges = useHasUnappliedChanges() return (
{ )} + {hasUnappliedChanges && ( + + {i18n.t('Unapplied changes')} + + )}
)} diff --git a/src/components/layout-panel/bottom-bar/styles/bottom-bar.module.css b/src/components/layout-panel/bottom-bar/styles/bottom-bar.module.css index 3e4ad84d6..b33d75c91 100644 --- a/src/components/layout-panel/bottom-bar/styles/bottom-bar.module.css +++ b/src/components/layout-panel/bottom-bar/styles/bottom-bar.module.css @@ -22,6 +22,20 @@ animation: fadeIn 0.4s ease-out forwards; } +/* Matches the "Edited" suffix in the title bar */ +.unappliedChanges { + color: var(--colors-grey600); + font-size: 11px; + letter-spacing: -0.2px; + font-family: monospace; + line-height: normal; +} + +.unappliedChanges::before { + content: '·'; + margin-inline-end: 4px; +} + @keyframes fadeIn { 0% { opacity: 0; diff --git a/src/components/layout-panel/bottom-bar/use-has-unapplied-changes.ts b/src/components/layout-panel/bottom-bar/use-has-unapplied-changes.ts new file mode 100644 index 000000000..17002ff3d --- /dev/null +++ b/src/components/layout-panel/bottom-bar/use-has-unapplied-changes.ts @@ -0,0 +1,55 @@ +import { useAppSelector, useMetadataStore } from '@hooks' +import { areVisualizationsEquivalent } from '@modules/visualization/state' +import { getCurrentVis } from '@store/current-vis-slice' +import { buildCurrentVisFromVisUiConfig } from '@store/thunks' +import { useMemo } from 'react' +import { + getAvailableOutputTypes, + getOutputTypeTooltipConfig, +} from './output-type-validity' + +/** + * Whether the visualization on screen differs from what visUiConfig describes + * AND the user can do something about it. + */ +export const useHasUnappliedChanges = (): boolean => { + const currentVis = useAppSelector(getCurrentVis) + const visUiConfig = useAppSelector((state) => state.visUiConfig) + const metadataStore = useMetadataStore() + + return useMemo(() => { + const applicableOutputTypes = getAvailableOutputTypes( + visUiConfig.visualizationType + ).filter( + (outputType) => + !getOutputTypeTooltipConfig({ + outputType, + visUiConfig, + metadataStore, + }) + ) + + /* Nothing can be produced from this config, so there is no action to + * hint at — every output type button is disabled. */ + if (!applicableOutputTypes.length) { + return false + } + + /* The selected output type was applicable when it was applied, so it + * no longer being applicable means the config has changed. Applying + * requires switching output type, which makes the result on screen + * something other than what the config describes. */ + if (!applicableOutputTypes.includes(visUiConfig.outputType)) { + return true + } + + return !areVisualizationsEquivalent( + currentVis, + buildCurrentVisFromVisUiConfig({ + previousCurrentVis: currentVis, + visUiConfig, + metadataStore, + }) + ) + }, [currentVis, visUiConfig, metadataStore]) +} From 19e13d4dce1b3d2fc6bd3de25aa6f80753467674 Mon Sep 17 00:00:00 2001 From: Hendrik de Graaf Date: Tue, 22 Sep 2026 11:14:02 +0200 Subject: [PATCH 05/13] docs: name the real save/load helpers in CLAUDE.md [DHIS2-22011] The section described sibling DHIS2 apps' helpers, none of which exist in this repo, so it pointed readers at symbols they could not find. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 41 +++++++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 2b5967ebf..0d7171d5d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -387,18 +387,35 @@ program-scope (`{dimension: 'ou', program: {id}}`) and TEI registration ### Save/load translation at the visualization API boundary -**Loading** (API → frontend): `acSetVisualization` reads each dimension's `program` and -`programStage` from the populated `columns`/`rows`/`filters` objects and calls `getFullDimensionId` -(or `formatDimensionId` in the line-listing-app). For EVENT/ENROLLMENT this produces -`stageId.dimensionId` (dropping the programId). For TRACKED_ENTITY it produces -`programId.stageId.dimensionId` or `programId.dimensionId`. - -**Saving** (frontend → API): `getAxesFromUi` (or equivalent) decomposes the internal compound ID -via `getDimensionIdParts` (`extractDimensionIdParts` in the line-listing-app) and sends each -dimension to the API with a plain `dimension` ID plus separate `program` and `programStage` -objects. The backend's `mergeAnalyticalObject` hydrates the stage from the database (including its -parent program via `loadProgramForStage`), then `getQualifiedDimension` rebuilds the persisted -string as `programId.stageId.dimensionId`. +**Loading** (API → frontend), in order: + +1. `normalizeApiSavedVisualization` (`@modules/visualization/state`) brings the API + payload to app shape: upgrades legacy dimensions, maps `PROGRAM_DATA_ELEMENT` → `DATA_ELEMENT`, + and strips the wire-only dimensions (`WIRE_ONLY_DIMENSIONS` in `@modules/dimension/ids`). +2. `getVisualizationUiConfig` (`@modules/visualization/state`) derives `visUiConfig` — layout + arrays, items, conditions and options — from the normalized visualization. It applies two + dimension translations on the way: + - `toAppLocalDimensions` (`@modules/dimension/translation`) renames API `ou` with a program but + no programStage to `enrollmentOu`. + - `getCompoundDimensionId` (`@modules/dimension/ids`) builds the canonical app-local compound ID + from each `DimensionRecord`, reading its `program` and `programStage`. For EVENT/ENROLLMENT + this produces `stageId.dimensionId` (dropping the programId); for TRACKED_ENTITY, + `programId.stageId.dimensionId` or `programId.dimensionId`. + + The metadata provider (`@modules/metadata/visualization`) applies the same two translations when + it registers a loaded visualization's dimensions. + +**Saving** (frontend → API), in order: + +1. `buildAxis` (`@modules/layout`) rebuilds each axis from the layout's compound IDs, calling + `toEventVisualizationDimensionId` (`@modules/dimension/translation`) for the POST dimension ID + and emitting separate `program` and `programStage` objects. See the `enrollmentOu` mapping table + above for the outputType/visType rules it applies. +2. `getSaveableVisualization` (`@modules/visualization/state`) drops the non-persisted dimension + props (`dimensionType`, `valueType`) and formats sorting for the API. +3. The backend's `mergeAnalyticalObject` hydrates the stage from the database (including its parent + program via `loadProgramForStage`), then `getQualifiedDimension` rebuilds the persisted string as + `programId.stageId.dimensionId`. ### `programDimensions` field on eventVisualizations From 92a36c5db133ccd87e67077abc2bab2e7c37cfd4 Mon Sep 17 00:00:00 2001 From: Hendrik de Graaf Date: Tue, 22 Sep 2026 11:14:03 +0200 Subject: [PATCH 06/13] chore: gitignore the superpowers SDD scratch workspace Co-Authored-By: Claude Opus 5 (1M context) --- .gitignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitignore b/.gitignore index 64331d694..b4027248f 100644 --- a/.gitignore +++ b/.gitignore @@ -24,3 +24,6 @@ docs/superpowers # playwright-cli session artifacts (AI sandboxes) .playwright-cli + +# superpowers SDD scratch workspace +.superpowers From 745cea8656ab64d3a94705840a5953ec81411e6d Mon Sep 17 00:00:00 2001 From: Joseph John Aas Cooper <33054985+cooper-joe@users.noreply.github.com> Date: Wed, 23 Sep 2026 16:30:02 +0200 Subject: [PATCH 07/13] feat: dim the canvas and show a notice while changes are unapplied [DHIS2-22011] Squashed net effect of prototype/stale-state commits a05c7583 and 643f263c. The action-button changes in a05c7583 were reverted by 643f263c, so only the canvas overlay survives. --- src/components/canvas/canvas.tsx | 61 ++++++++++++++++--- .../layout-panel/bottom-bar/bottom-bar.tsx | 13 ---- 2 files changed, 52 insertions(+), 22 deletions(-) diff --git a/src/components/canvas/canvas.tsx b/src/components/canvas/canvas.tsx index 6bd1d4092..bb3aa3814 100644 --- a/src/components/canvas/canvas.tsx +++ b/src/components/canvas/canvas.tsx @@ -1,3 +1,4 @@ +import { useHasUnappliedChanges } from '@components/layout-panel/bottom-bar/use-has-unapplied-changes' import { PluginWrapper } from '@components/plugin-wrapper/plugin-wrapper' import { StartScreen } from '@components/start-screen/start-screen' import { useAppDispatch, useAppSelector, useCurrentUser } from '@hooks' @@ -21,6 +22,7 @@ export const Canvas: FC = () => { const visualizationId = useAppSelector( (state) => state.navigation.visualizationId ) + const hasUnappliedChanges = useHasUnappliedChanges() const onRetryLoad = useCallback(() => { if (visualizationId !== 'new') { @@ -61,14 +63,55 @@ export const Canvas: FC = () => { } return ( - +
+
+ +
+
+ + Changes not applied + +
+
) } diff --git a/src/components/layout-panel/bottom-bar/bottom-bar.tsx b/src/components/layout-panel/bottom-bar/bottom-bar.tsx index 95901cacc..6c08d8428 100644 --- a/src/components/layout-panel/bottom-bar/bottom-bar.tsx +++ b/src/components/layout-panel/bottom-bar/bottom-bar.tsx @@ -1,4 +1,3 @@ -import i18n from '@dhis2/d2-i18n' import { useAppSelector } from '@hooks' import { getDataSourceId } from '@store/dimensions-selection-slice' import { getIsVisualizationLoading } from '@store/loader-slice' @@ -10,13 +9,11 @@ import { EnrollmentButton } from './action-buttons/enrollment-button' import { EventButton } from './action-buttons/event-button' import { TrackedEntityInstanceButton } from './action-buttons/tracked-entity-instance-button' import classes from './styles/bottom-bar.module.css' -import { useHasUnappliedChanges } from './use-has-unapplied-changes' export const BottomBar: FC = () => { const dataSourceId = useAppSelector(getDataSourceId) const isVisualizationLoading = useAppSelector(getIsVisualizationLoading) const visualizationType = useAppSelector(getVisUiConfigVisualizationType) - const hasUnappliedChanges = useHasUnappliedChanges() return (
{ )} - {hasUnappliedChanges && ( - - {i18n.t('Unapplied changes')} - - )}
)} From 5d728edbc7e2a7c6f99a6c3138824b8ab1573168 Mon Sep 17 00:00:00 2001 From: Hendrik de Graaf Date: Wed, 23 Sep 2026 16:34:00 +0200 Subject: [PATCH 08/13] refactor: move the stale canvas state into CSS modules and i18n [DHIS2-22011] Inline styles become a CSS module, the notice text goes through i18n, and the stale state is suppressed while the visualization is loading. The notice stays mounted so it can fade in and out, and toggles aria-hidden so it leaves the accessibility tree while faded out. The indicator tests move from the bottom bar spec to a canvas spec. --- i18n/en.pot | 10 ++-- .../canvas/__tests__/canvas.spec.tsx | 60 +++++++++++++++++++ src/components/canvas/canvas.tsx | 49 +++++---------- .../canvas/styles/canvas.module.css | 45 ++++++++++++++ .../bottom-bar/__tests__/bottom-bar.spec.tsx | 37 ++---------- .../bottom-bar/styles/bottom-bar.module.css | 14 ----- 6 files changed, 131 insertions(+), 84 deletions(-) create mode 100644 src/components/canvas/__tests__/canvas.spec.tsx create mode 100644 src/components/canvas/styles/canvas.module.css diff --git a/i18n/en.pot b/i18n/en.pot index 2ae6ade99..f473440ba 100644 --- a/i18n/en.pot +++ b/i18n/en.pot @@ -5,8 +5,8 @@ msgstr "" "Content-Type: text/plain; charset=utf-8\n" "Content-Transfer-Encoding: 8bit\n" "Plural-Forms: nplurals=2; plural=(n != 1)\n" -"POT-Creation-Date: 2026-09-22T09:13:46.110Z\n" -"PO-Revision-Date: 2026-09-22T09:13:46.112Z\n" +"POT-Creation-Date: 2026-09-23T14:32:18.188Z\n" +"PO-Revision-Date: 2026-09-23T14:32:18.188Z\n" msgid "" "Some dimensions were not added because they cannot be used in a " @@ -25,6 +25,9 @@ msgstr "" msgid "{{- name}} was not added because it is already used as the custom value." msgstr "{{- name}} was not added because it is already used as the custom value." +msgid "Changes not applied" +msgstr "Changes not applied" + msgid "Retry" msgstr "Retry" @@ -307,9 +310,6 @@ msgstr "Switch to {{- trackedEntityTypeName}} list" msgid "Update {{- trackedEntityTypeName}} list" msgstr "Update {{- trackedEntityTypeName}} list" -msgid "Unapplied changes" -msgstr "Unapplied changes" - msgid "Not valid with registration org. unit" msgstr "Not valid with registration org. unit" diff --git a/src/components/canvas/__tests__/canvas.spec.tsx b/src/components/canvas/__tests__/canvas.spec.tsx new file mode 100644 index 000000000..c50797da1 --- /dev/null +++ b/src/components/canvas/__tests__/canvas.spec.tsx @@ -0,0 +1,60 @@ +import { + DIMENSION_ID, + metadata, + populatedVis, +} from '@components/layout-panel/bottom-bar/__fixtures__/unapplied-changes' +import { renderWithAppWrapper } from '@test-utils/app-wrapper' +import { screen } from '@testing-library/react' +import type { Layout, RootState } from '@types' +import { describe, it, expect, vi } from 'vitest' +import { Canvas } from '../canvas' + +vi.mock('@components/plugin-wrapper/plugin-wrapper', () => ({ + PluginWrapper: () =>
, +})) + +const layoutWithDimension = { + columns: [DIMENSION_ID], + rows: [], + filters: [], +} as unknown as Layout + +/* The notice stays mounted so it can fade out, so "hidden" is the aria-hidden + * state rather than the absence of the element. */ +const findNotice = () => screen.findByTestId('unapplied-changes') + +const renderCanvas = (preloadedState: Partial) => + renderWithAppWrapper(, { + metadata, + partialStore: { preloadedState }, + }) + +describe('Canvas unapplied changes notice', () => { + it('is hidden when the visualization matches the ui config', async () => { + await renderCanvas({ currentVis: populatedVis }) + + expect(await findNotice()).toHaveAttribute('aria-hidden', 'true') + }) + + it('is shown when there are unapplied changes', async () => { + await renderCanvas({ + currentVis: populatedVis, + visUiConfig: { layout: layoutWithDimension }, + } as Partial) + + const notice = await findNotice() + + expect(notice).toHaveAttribute('aria-hidden', 'false') + expect(notice).toHaveTextContent('Changes not applied') + }) + + it('is hidden while the visualization is loading', async () => { + await renderCanvas({ + currentVis: populatedVis, + visUiConfig: { layout: layoutWithDimension }, + loader: { isVisualizationLoading: true }, + } as Partial) + + expect(await findNotice()).toHaveAttribute('aria-hidden', 'true') + }) +}) diff --git a/src/components/canvas/canvas.tsx b/src/components/canvas/canvas.tsx index bb3aa3814..362686a27 100644 --- a/src/components/canvas/canvas.tsx +++ b/src/components/canvas/canvas.tsx @@ -1,6 +1,7 @@ import { useHasUnappliedChanges } from '@components/layout-panel/bottom-bar/use-has-unapplied-changes' import { PluginWrapper } from '@components/plugin-wrapper/plugin-wrapper' import { StartScreen } from '@components/start-screen/start-screen' +import i18n from '@dhis2/d2-i18n' import { useAppDispatch, useAppSelector, useCurrentUser } from '@hooks' import { isVisualizationEmpty } from '@modules/visualization/state' import { getCurrentVis, setCurrentVis } from '@store/current-vis-slice' @@ -11,7 +12,9 @@ import { import { tLoadSavedVisualization } from '@store/thunks' import { setUiActiveDimensionModal } from '@store/ui-slice' import type { Sorting } from '@types' +import cx from 'classnames' import { useCallback, type FC } from 'react' +import classes from './styles/canvas.module.css' export const Canvas: FC = () => { const dispatch = useAppDispatch() @@ -24,6 +27,8 @@ export const Canvas: FC = () => { ) const hasUnappliedChanges = useHasUnappliedChanges() + const showUnappliedChanges = hasUnappliedChanges && !isVisualizationLoading + const onRetryLoad = useCallback(() => { if (visualizationId !== 'new') { dispatch(tLoadSavedVisualization({ id: visualizationId })) @@ -63,15 +68,11 @@ export const Canvas: FC = () => { } return ( -
+
{ />
- - Changes not applied - + {i18n.t('Changes not applied')}
) diff --git a/src/components/canvas/styles/canvas.module.css b/src/components/canvas/styles/canvas.module.css new file mode 100644 index 000000000..eaa54bd44 --- /dev/null +++ b/src/components/canvas/styles/canvas.module.css @@ -0,0 +1,45 @@ +.canvas { + position: relative; + block-size: 100%; +} + +.visualization { + block-size: 100%; + transition: opacity 250ms cubic-bezier(0.4, 0, 0.2, 1); +} + +.visualization.stale { + opacity: 0.66; +} + +.notice { + position: absolute; + inset-block-start: 0; + inset-inline: 4px; + z-index: 1; + padding-block: 1px 8px; + padding-inline: 8px; + background: linear-gradient( + 180deg, + var(--colors-grey300) 0%, + var(--colors-grey300) 72%, + transparent 100% + ); + color: var(--colors-grey700); + font-size: 12px; + line-height: 16px; + letter-spacing: 0.01em; + visibility: hidden; + opacity: 0; + pointer-events: none; + /* visibility is in the transition so the notice leaves the accessibility + * tree once it has faded out rather than the moment it starts to */ + transition: + opacity 250ms cubic-bezier(0.4, 0, 0.2, 1), + visibility 250ms; +} + +.notice.visible { + visibility: visible; + opacity: 0.9; +} diff --git a/src/components/layout-panel/bottom-bar/__tests__/bottom-bar.spec.tsx b/src/components/layout-panel/bottom-bar/__tests__/bottom-bar.spec.tsx index b2bed3529..18f000f11 100644 --- a/src/components/layout-panel/bottom-bar/__tests__/bottom-bar.spec.tsx +++ b/src/components/layout-panel/bottom-bar/__tests__/bottom-bar.spec.tsx @@ -1,20 +1,10 @@ import { renderWithAppWrapper } from '@test-utils/app-wrapper' import { screen } from '@testing-library/react' -import type { Layout, RootState } from '@types' +import type { RootState } from '@types' import { describe, it, expect } from 'vitest' -import { - DIMENSION_ID, - metadata, - populatedVis, -} from '../__fixtures__/unapplied-changes' +import { metadata, populatedVis } from '../__fixtures__/unapplied-changes' import { BottomBar } from '../bottom-bar' -const layoutWithDimension = { - columns: [DIMENSION_ID], - rows: [], - filters: [], -} as unknown as Layout - const renderBottomBar = (preloadedState: Partial) => renderWithAppWrapper(, { metadata, @@ -28,34 +18,19 @@ const renderBottomBar = (preloadedState: Partial) => }, }) -describe('BottomBar unapplied changes indicator', () => { - it('is absent when the visualization matches the ui config', async () => { +describe('BottomBar', () => { + it('shows the output type buttons once a data source is selected', async () => { await renderBottomBar({ currentVis: populatedVis }) - expect( - screen.queryByTestId('unapplied-changes') - ).not.toBeInTheDocument() - }) - - it('is shown when there are unapplied changes', async () => { - await renderBottomBar({ - currentVis: populatedVis, - visUiConfig: { layout: layoutWithDimension }, - } as Partial) - - expect(await screen.findByText('Unapplied changes')).toBeVisible() + expect(await screen.findByTestId('update-buttons')).toBeInTheDocument() }) - it('is absent while the visualization is loading, along with the buttons', async () => { + it('hides the output type buttons while the visualization is loading', async () => { await renderBottomBar({ currentVis: populatedVis, - visUiConfig: { layout: layoutWithDimension }, loader: { isVisualizationLoading: true }, } as Partial) - expect( - screen.queryByTestId('unapplied-changes') - ).not.toBeInTheDocument() expect(screen.queryByTestId('update-buttons')).not.toBeInTheDocument() }) }) diff --git a/src/components/layout-panel/bottom-bar/styles/bottom-bar.module.css b/src/components/layout-panel/bottom-bar/styles/bottom-bar.module.css index b33d75c91..3e4ad84d6 100644 --- a/src/components/layout-panel/bottom-bar/styles/bottom-bar.module.css +++ b/src/components/layout-panel/bottom-bar/styles/bottom-bar.module.css @@ -22,20 +22,6 @@ animation: fadeIn 0.4s ease-out forwards; } -/* Matches the "Edited" suffix in the title bar */ -.unappliedChanges { - color: var(--colors-grey600); - font-size: 11px; - letter-spacing: -0.2px; - font-family: monospace; - line-height: normal; -} - -.unappliedChanges::before { - content: '·'; - margin-inline-end: 4px; -} - @keyframes fadeIn { 0% { opacity: 0; From 0ad37257bb4bce4cb041880b3b35fca2bf20834c Mon Sep 17 00:00:00 2001 From: Hendrik de Graaf Date: Wed, 23 Sep 2026 17:31:21 +0200 Subject: [PATCH 09/13] refactor: extract the stale canvas state into UnappliedChangesOverlay [DHIS2-22011] The overlay wraps the canvas content, reads the store itself and owns both the dimming and the notice, so Canvas is back to rendering PluginWrapper. --- ...tsx => unapplied-changes-overlay.spec.tsx} | 38 +++++++++------- src/components/canvas/canvas.tsx | 45 +++++-------------- ...s => unapplied-changes-overlay.module.css} | 6 +-- .../canvas/unapplied-changes-overlay.tsx | 30 +++++++++++++ 4 files changed, 66 insertions(+), 53 deletions(-) rename src/components/canvas/__tests__/{canvas.spec.tsx => unapplied-changes-overlay.spec.tsx} (56%) rename src/components/canvas/styles/{canvas.module.css => unapplied-changes-overlay.module.css} (94%) create mode 100644 src/components/canvas/unapplied-changes-overlay.tsx diff --git a/src/components/canvas/__tests__/canvas.spec.tsx b/src/components/canvas/__tests__/unapplied-changes-overlay.spec.tsx similarity index 56% rename from src/components/canvas/__tests__/canvas.spec.tsx rename to src/components/canvas/__tests__/unapplied-changes-overlay.spec.tsx index c50797da1..325013aa4 100644 --- a/src/components/canvas/__tests__/canvas.spec.tsx +++ b/src/components/canvas/__tests__/unapplied-changes-overlay.spec.tsx @@ -6,12 +6,8 @@ import { import { renderWithAppWrapper } from '@test-utils/app-wrapper' import { screen } from '@testing-library/react' import type { Layout, RootState } from '@types' -import { describe, it, expect, vi } from 'vitest' -import { Canvas } from '../canvas' - -vi.mock('@components/plugin-wrapper/plugin-wrapper', () => ({ - PluginWrapper: () =>
, -})) +import { describe, it, expect } from 'vitest' +import { UnappliedChangesOverlay } from '../unapplied-changes-overlay' const layoutWithDimension = { columns: [DIMENSION_ID], @@ -23,21 +19,29 @@ const layoutWithDimension = { * state rather than the absence of the element. */ const findNotice = () => screen.findByTestId('unapplied-changes') -const renderCanvas = (preloadedState: Partial) => - renderWithAppWrapper(, { - metadata, - partialStore: { preloadedState }, +const renderOverlay = (preloadedState: Partial) => + renderWithAppWrapper( + +
+ , + { metadata, partialStore: { preloadedState } } + ) + +describe('UnappliedChangesOverlay', () => { + it('renders its children', async () => { + await renderOverlay({ currentVis: populatedVis }) + + expect(await screen.findByTestId('canvas-content')).toBeInTheDocument() }) -describe('Canvas unapplied changes notice', () => { - it('is hidden when the visualization matches the ui config', async () => { - await renderCanvas({ currentVis: populatedVis }) + it('hides the notice when the visualization matches the ui config', async () => { + await renderOverlay({ currentVis: populatedVis }) expect(await findNotice()).toHaveAttribute('aria-hidden', 'true') }) - it('is shown when there are unapplied changes', async () => { - await renderCanvas({ + it('shows the notice when there are unapplied changes', async () => { + await renderOverlay({ currentVis: populatedVis, visUiConfig: { layout: layoutWithDimension }, } as Partial) @@ -48,8 +52,8 @@ describe('Canvas unapplied changes notice', () => { expect(notice).toHaveTextContent('Changes not applied') }) - it('is hidden while the visualization is loading', async () => { - await renderCanvas({ + it('hides the notice while the visualization is loading', async () => { + await renderOverlay({ currentVis: populatedVis, visUiConfig: { layout: layoutWithDimension }, loader: { isVisualizationLoading: true }, diff --git a/src/components/canvas/canvas.tsx b/src/components/canvas/canvas.tsx index 362686a27..da82b6d2c 100644 --- a/src/components/canvas/canvas.tsx +++ b/src/components/canvas/canvas.tsx @@ -1,7 +1,5 @@ -import { useHasUnappliedChanges } from '@components/layout-panel/bottom-bar/use-has-unapplied-changes' import { PluginWrapper } from '@components/plugin-wrapper/plugin-wrapper' import { StartScreen } from '@components/start-screen/start-screen' -import i18n from '@dhis2/d2-i18n' import { useAppDispatch, useAppSelector, useCurrentUser } from '@hooks' import { isVisualizationEmpty } from '@modules/visualization/state' import { getCurrentVis, setCurrentVis } from '@store/current-vis-slice' @@ -12,9 +10,8 @@ import { import { tLoadSavedVisualization } from '@store/thunks' import { setUiActiveDimensionModal } from '@store/ui-slice' import type { Sorting } from '@types' -import cx from 'classnames' import { useCallback, type FC } from 'react' -import classes from './styles/canvas.module.css' +import { UnappliedChangesOverlay } from './unapplied-changes-overlay' export const Canvas: FC = () => { const dispatch = useAppDispatch() @@ -25,9 +22,6 @@ export const Canvas: FC = () => { const visualizationId = useAppSelector( (state) => state.navigation.visualizationId ) - const hasUnappliedChanges = useHasUnappliedChanges() - - const showUnappliedChanges = hasUnappliedChanges && !isVisualizationLoading const onRetryLoad = useCallback(() => { if (visualizationId !== 'new') { @@ -68,31 +62,16 @@ export const Canvas: FC = () => { } return ( -
-
- -
-
- {i18n.t('Changes not applied')} -
-
+ + + ) } diff --git a/src/components/canvas/styles/canvas.module.css b/src/components/canvas/styles/unapplied-changes-overlay.module.css similarity index 94% rename from src/components/canvas/styles/canvas.module.css rename to src/components/canvas/styles/unapplied-changes-overlay.module.css index eaa54bd44..e3e038959 100644 --- a/src/components/canvas/styles/canvas.module.css +++ b/src/components/canvas/styles/unapplied-changes-overlay.module.css @@ -1,14 +1,14 @@ -.canvas { +.container { position: relative; block-size: 100%; } -.visualization { +.content { block-size: 100%; transition: opacity 250ms cubic-bezier(0.4, 0, 0.2, 1); } -.visualization.stale { +.content.stale { opacity: 0.66; } diff --git a/src/components/canvas/unapplied-changes-overlay.tsx b/src/components/canvas/unapplied-changes-overlay.tsx new file mode 100644 index 000000000..6f96c2750 --- /dev/null +++ b/src/components/canvas/unapplied-changes-overlay.tsx @@ -0,0 +1,30 @@ +import { useHasUnappliedChanges } from '@components/layout-panel/bottom-bar/use-has-unapplied-changes' +import i18n from '@dhis2/d2-i18n' +import { useAppSelector } from '@hooks' +import { getIsVisualizationLoading } from '@store/loader-slice' +import cx from 'classnames' +import type { FC, PropsWithChildren } from 'react' +import classes from './styles/unapplied-changes-overlay.module.css' + +export const UnappliedChangesOverlay: FC = ({ + children, +}) => { + const isVisualizationLoading = useAppSelector(getIsVisualizationLoading) + const hasUnappliedChanges = useHasUnappliedChanges() + const isStale = hasUnappliedChanges && !isVisualizationLoading + + return ( +
+
+ {children} +
+
+ {i18n.t('Changes not applied')} +
+
+ ) +} From c6c23c2e99a6c395a87824a5ecb67cab80202b45 Mon Sep 17 00:00:00 2001 From: Hendrik de Graaf Date: Wed, 23 Sep 2026 17:45:24 +0200 Subject: [PATCH 10/13] refactor: drive UnappliedChangesOverlay from props [DHIS2-22011] Canvas already reads isVisualizationLoading for PluginWrapper, so the overlay reading it from the store too made the two go through different paths for the same state. --- .../unapplied-changes-overlay.spec.tsx | 77 ++++++++----------- src/components/canvas/canvas.tsx | 7 +- .../canvas/unapplied-changes-overlay.tsx | 14 ++-- 3 files changed, 47 insertions(+), 51 deletions(-) diff --git a/src/components/canvas/__tests__/unapplied-changes-overlay.spec.tsx b/src/components/canvas/__tests__/unapplied-changes-overlay.spec.tsx index 325013aa4..014fd3b94 100644 --- a/src/components/canvas/__tests__/unapplied-changes-overlay.spec.tsx +++ b/src/components/canvas/__tests__/unapplied-changes-overlay.spec.tsx @@ -1,64 +1,53 @@ -import { - DIMENSION_ID, - metadata, - populatedVis, -} from '@components/layout-panel/bottom-bar/__fixtures__/unapplied-changes' -import { renderWithAppWrapper } from '@test-utils/app-wrapper' -import { screen } from '@testing-library/react' -import type { Layout, RootState } from '@types' +import { render, screen } from '@testing-library/react' import { describe, it, expect } from 'vitest' import { UnappliedChangesOverlay } from '../unapplied-changes-overlay' -const layoutWithDimension = { - columns: [DIMENSION_ID], - rows: [], - filters: [], -} as unknown as Layout +const renderOverlay = ( + props: Partial<{ + hasUnappliedChanges: boolean + isVisualizationLoading: boolean + }> = {} +) => + render( + +
+ + ) /* The notice stays mounted so it can fade out, so "hidden" is the aria-hidden * state rather than the absence of the element. */ -const findNotice = () => screen.findByTestId('unapplied-changes') - -const renderOverlay = (preloadedState: Partial) => - renderWithAppWrapper( - -
- , - { metadata, partialStore: { preloadedState } } - ) +const getNotice = () => screen.getByTestId('unapplied-changes') describe('UnappliedChangesOverlay', () => { - it('renders its children', async () => { - await renderOverlay({ currentVis: populatedVis }) + it('renders its children', () => { + renderOverlay() - expect(await screen.findByTestId('canvas-content')).toBeInTheDocument() + expect(screen.getByTestId('canvas-content')).toBeInTheDocument() }) - it('hides the notice when the visualization matches the ui config', async () => { - await renderOverlay({ currentVis: populatedVis }) + it('hides the notice when there are no unapplied changes', () => { + renderOverlay() - expect(await findNotice()).toHaveAttribute('aria-hidden', 'true') + expect(getNotice()).toHaveAttribute('aria-hidden', 'true') }) - it('shows the notice when there are unapplied changes', async () => { - await renderOverlay({ - currentVis: populatedVis, - visUiConfig: { layout: layoutWithDimension }, - } as Partial) - - const notice = await findNotice() + it('shows the notice when there are unapplied changes', () => { + renderOverlay({ hasUnappliedChanges: true }) - expect(notice).toHaveAttribute('aria-hidden', 'false') - expect(notice).toHaveTextContent('Changes not applied') + expect(getNotice()).toHaveAttribute('aria-hidden', 'false') + expect(getNotice()).toHaveTextContent('Changes not applied') }) - it('hides the notice while the visualization is loading', async () => { - await renderOverlay({ - currentVis: populatedVis, - visUiConfig: { layout: layoutWithDimension }, - loader: { isVisualizationLoading: true }, - } as Partial) + it('hides the notice while the visualization is loading', () => { + renderOverlay({ + hasUnappliedChanges: true, + isVisualizationLoading: true, + }) - expect(await findNotice()).toHaveAttribute('aria-hidden', 'true') + expect(getNotice()).toHaveAttribute('aria-hidden', 'true') }) }) diff --git a/src/components/canvas/canvas.tsx b/src/components/canvas/canvas.tsx index da82b6d2c..9eae1c214 100644 --- a/src/components/canvas/canvas.tsx +++ b/src/components/canvas/canvas.tsx @@ -1,3 +1,4 @@ +import { useHasUnappliedChanges } from '@components/layout-panel/bottom-bar/use-has-unapplied-changes' import { PluginWrapper } from '@components/plugin-wrapper/plugin-wrapper' import { StartScreen } from '@components/start-screen/start-screen' import { useAppDispatch, useAppSelector, useCurrentUser } from '@hooks' @@ -22,6 +23,7 @@ export const Canvas: FC = () => { const visualizationId = useAppSelector( (state) => state.navigation.visualizationId ) + const hasUnappliedChanges = useHasUnappliedChanges() const onRetryLoad = useCallback(() => { if (visualizationId !== 'new') { @@ -62,7 +64,10 @@ export const Canvas: FC = () => { } return ( - + = ({ +type UnappliedChangesOverlayProps = PropsWithChildren<{ + hasUnappliedChanges: boolean + isVisualizationLoading: boolean +}> + +export const UnappliedChangesOverlay: FC = ({ children, + hasUnappliedChanges, + isVisualizationLoading, }) => { - const isVisualizationLoading = useAppSelector(getIsVisualizationLoading) - const hasUnappliedChanges = useHasUnappliedChanges() const isStale = hasUnappliedChanges && !isVisualizationLoading return ( From 57d58b6f70dc328098ed48c094185dddcc326938 Mon Sep 17 00:00:00 2001 From: Hendrik de Graaf Date: Wed, 23 Sep 2026 19:35:32 +0200 Subject: [PATCH 11/13] refactor: clarify the visualization equivalence comments [DHIS2-22011] Drop the comments that restated the code, state the reason each field needs special handling where the handling is, and let the code carry the parts that were only explained in prose. --- src/modules/visualization/state.ts | 119 ++++++++++++++--------------- 1 file changed, 59 insertions(+), 60 deletions(-) diff --git a/src/modules/visualization/state.ts b/src/modules/visualization/state.ts index 315f082ce..2acd14144 100644 --- a/src/modules/visualization/state.ts +++ b/src/modules/visualization/state.ts @@ -97,11 +97,10 @@ const CURRENT_VIS_KEYS: ReadonlyArray = [ ] /** - * Extracts the CurrentVisualization-shaped subset of a SavedVisualization. - * Used to compare a saved visualization to the current (edited) one — - * the current vis is already in CurrentVisualization shape, but the saved - * vis carries extra fields (access, createdBy, …) that we don't care about - * when determining whether there are unsaved changes. + * The CurrentVisualization-shaped subset of a SavedVisualization. The API + * returns fields the app never edits (access, createdBy, …); only the editable + * subset belongs in current-vis state. Values are copied as they are — a field + * the API left out stays out, rather than becoming an explicit undefined. */ export const toCurrentVis = ( savedVis: SavedVisualization @@ -115,12 +114,11 @@ export const toCurrentVis = ( return result as CurrentVisualization } -/* Derived from the layout: any real change is already caught by comparing the - * axes, so comparing these adds nothing. And the two array fields - * (programDimensions, attributeDimensions) can differ in order between a loaded - * savedVis and a rebuilt currentVis — the app rebuilds them from the layout, - * the backend returns its own order — which a direct compare would misread as - * an edit. */ +/* Rebuilt from the layout's dimensions rather than edited directly, so a real + * change to any of them already shows up in the axis comparison. Comparing + * them as well would only add false positives: the backend recomputes + * programDimensions on every GET, in its own order, and returns more per entry + * than the app can rebuild from the metadata store. */ const DERIVED_LAYOUT_FIELDS: ReadonlySet = new Set([ 'trackedEntityType', 'attributeDimensions', @@ -129,22 +127,42 @@ const DERIVED_LAYOUT_FIELDS: ReadonlySet = new Set([ const DIMENSION_AXES = new Set(AXES) -/* A default-valued option and an absent one mean the same thing, so both count - * as "at default" when comparing. */ +/* An option left out and an option set to its own default mean the same thing. */ export const isDefaultOptionValue = (key: string, value: unknown): boolean => value === undefined || deepEqual(value, (DEFAULT_OPTIONS as Record)[key]) -/* An axis prepared for comparison. Two kinds of difference are not edits: - * props that aren't persisted (dimensionType, valueType — the API sends - * PROGRAM_DATA_ELEMENT where the rebuilt vis has DATA_ELEMENT) and nested - * objects the API returns richer than the app can rebuild from visUiConfig - * (option sets and legend sets carry their name; a repetition carries the - * dimension, axis and program context the backend derives from the owning - * dimension). An empty items array counts as absent. */ +/* Not persisted, so they are stripped before saving — and comparing them would + * report a false positive anyway: a loaded visualization carries the API's + * dimensionType (PROGRAM_DATA_ELEMENT) where one rebuilt from visUiConfig + * carries the metadata store's (DATA_ELEMENT). */ +const NON_PERSISTED_DIMENSION_PROPERTIES: ReadonlyArray = + ['dimensionType', 'valueType'] + +const removeNonPersistedDimensionProperties = ( + axis: DimensionArray +): DimensionArray => + axis.map((dim) => { + const dimension = { ...dim } + + NON_PERSISTED_DIMENSION_PROPERTIES.forEach((property) => { + delete dimension[property] + }) + + return dimension + }) + +/* The API returns more per dimension than the app can rebuild from visUiConfig, + * and none of that extra detail is an edit: option sets and legend sets come + * back with their display name, and a repetition comes back with the dimension, + * axis and program context the backend derives from the dimension owning it. + * Reducing both sides to what the app itself can produce leaves only real + * edits. */ const comparableAxis = (axis: DimensionArray = []): DimensionArray => - removeDimensionPropertiesBeforeSaving(axis).map((dim) => { + removeNonPersistedDimensionProperties(axis).map((dim) => { const comparableDim = { ...dim } + + // No items and an empty items array both mean "no selection". if (Array.isArray(comparableDim.items) && !comparableDim.items.length) { delete comparableDim.items } @@ -162,24 +180,16 @@ const comparableAxis = (axis: DimensionArray = []): DimensionArray => return comparableDim }) -/* Top-level metadata refs the API returns with a display name (and, for the - * custom value, its aggregation type) where visUiConfig can only rebuild the - * id. Compared by id, for the same reason comparableAxis reduces optionSet and - * legendSet. */ -const ID_REF_FIELDS: ReadonlySet = new Set(['value']) - -const comparableIdRef = (ref: unknown): unknown => +const idOnly = (ref: unknown): unknown => ref && typeof ref === 'object' && 'id' in ref ? { id: (ref as { id: string }).id } : ref -/* How one field of two visualizations compares. Each kind of field has its own - * notion of equivalence: a metadata ref by id, an option with an absent value - * counting as its default, an axis after normalisation, and the layout-derived - * fields not at all. */ const isFieldEquivalent = (key: string, a: unknown, b: unknown): boolean => { - if (ID_REF_FIELDS.has(key)) { - return deepEqual(comparableIdRef(a), comparableIdRef(b)) + /* The custom value: the API returns it with a display name and an + * aggregation type, where visUiConfig holds nothing but the id. */ + if (key === 'value') { + return deepEqual(idOnly(a), idOnly(b)) } if (key in DEFAULT_OPTIONS) { @@ -196,18 +206,22 @@ const isFieldEquivalent = (key: string, a: unknown, b: unknown): boolean => { ) } - return DERIVED_LAYOUT_FIELDS.has(key) || deepEqual(a, b) + if (DERIVED_LAYOUT_FIELDS.has(key)) { + return true + } + + return deepEqual(a, b) } -/* Compares a saved vis to the current one, and the current one to the vis that - * visUiConfig would produce. `visualizationB` must carry the full - * CurrentVisualization key set, because its keys drive the comparison. */ +/* Only the keys present on `completeVisualization` are compared, so it has to + * carry the full CurrentVisualization key set: a key missing there is a key + * that goes unchecked. */ export const areVisualizationsEquivalent = ( - visualizationA: CurrentVisualization | EmptyVisualization, - visualizationB: CurrentVisualization + visualization: CurrentVisualization | EmptyVisualization, + completeVisualization: CurrentVisualization ): boolean => { - const a = visualizationA as Record - const b = visualizationB as Record + const a = visualization as Record + const b = completeVisualization as Record return Object.keys(b).every((key) => isFieldEquivalent(key, a[key], b[key])) } @@ -229,21 +243,6 @@ export const getVisualizationState = ( } } -const removeDimensionPropertiesBeforeSaving = ( - axis: DimensionArray -): DimensionArray => { - return axis.map((dim) => { - const dimension = { ...dim } - const propsToRemove = ['dimensionType', 'valueType'] - - propsToRemove.forEach((prop) => { - delete dimension[prop as keyof DimensionRecord] - }) - - return dimension - }) -} - const getDimensionIdFromHeaderName = ( headerName: string, visualization: CurrentVisualization @@ -257,13 +256,13 @@ export const getSaveableVisualization = ( ): SavedVisualization => { const visualization = { ...vis } - visualization.columns = removeDimensionPropertiesBeforeSaving( + visualization.columns = removeNonPersistedDimensionProperties( visualization.columns ) - visualization.filters = removeDimensionPropertiesBeforeSaving( + visualization.filters = removeNonPersistedDimensionProperties( visualization.filters ) - visualization.rows = removeDimensionPropertiesBeforeSaving( + visualization.rows = removeNonPersistedDimensionProperties( visualization.rows ) From 3502c238663cfac8ab1daf92d8b95236c1f39216 Mon Sep 17 00:00:00 2001 From: Hendrik de Graaf Date: Thu, 24 Sep 2026 11:03:10 +0200 Subject: [PATCH 12/13] refactor: drop the unsupported custom value claim from dropWireOnlyDimensions [DHIS2-22011] Nothing derives the custom value from a wire-only dimension, and the claim did not hold for latitude/longitude at all. What is left is the reason the drop flips the legacy flag. --- src/modules/visualization/state.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/modules/visualization/state.ts b/src/modules/visualization/state.ts index 2acd14144..1aea73e41 100644 --- a/src/modules/visualization/state.ts +++ b/src/modules/visualization/state.ts @@ -634,10 +634,8 @@ export const normalizeApiSavedVisualization = ( : []), ] - /* Wire-only dimensions mark where a legacy Event Report put its value - * column; the app expresses that with the custom value fields instead, and - * carries no layout position for them. Dropping them changes the persisted - * shape, so it flips `legacy` like every other upgrade here. */ + /* Dropping a dimension rewrites the persisted shape, so it flips `legacy` + * the same way the conversions above do. */ const dropWireOnlyDimensions = (dims: DimensionRecord[]) => { const kept = dims.filter( (dim) => !WIRE_ONLY_DIMENSIONS.has(dim.dimension) From d394faea606fa1b28ad0f464f597eba4e3ec3b31 Mon Sep 17 00:00:00 2001 From: Hendrik de Graaf Date: Thu, 24 Sep 2026 11:15:08 +0200 Subject: [PATCH 13/13] refactor: rename WIRE_ONLY_DIMENSIONS to DROPPED_LEGACY_DIMENSIONS [DHIS2-22011] The old name described the wire format rather than what the app does with these dimensions, so every call site had to re-explain it. --- CLAUDE.md | 2 +- .../plugin-wrapper/hooks/query-tools-common.ts | 4 ++-- .../hooks/query-tools-line-list.ts | 4 ++-- src/modules/dimension/ids.ts | 6 +++--- src/modules/dimension/translation.ts | 5 +++-- .../__tests__/visualization.spec.ts | 8 ++++---- src/modules/visualization/state.ts | 16 ++++++++-------- 7 files changed, 23 insertions(+), 22 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 0d7171d5d..4692a3c2d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -391,7 +391,7 @@ program-scope (`{dimension: 'ou', program: {id}}`) and TEI registration 1. `normalizeApiSavedVisualization` (`@modules/visualization/state`) brings the API payload to app shape: upgrades legacy dimensions, maps `PROGRAM_DATA_ELEMENT` → `DATA_ELEMENT`, - and strips the wire-only dimensions (`WIRE_ONLY_DIMENSIONS` in `@modules/dimension/ids`). + and strips the dropped legacy dimensions (`DROPPED_LEGACY_DIMENSIONS` in `@modules/dimension/ids`). 2. `getVisualizationUiConfig` (`@modules/visualization/state`) derives `visUiConfig` — layout arrays, items, conditions and options — from the normalized visualization. It applies two dimension translations on the way: diff --git a/src/components/plugin-wrapper/hooks/query-tools-common.ts b/src/components/plugin-wrapper/hooks/query-tools-common.ts index e7cc28059..b16ac4564 100644 --- a/src/components/plugin-wrapper/hooks/query-tools-common.ts +++ b/src/components/plugin-wrapper/hooks/query-tools-common.ts @@ -1,6 +1,6 @@ import { ANALYTICS_OPTIONS } from '@constants/options' import { getAnalyticsRequestDimensionName } from '@modules/analytics-request' -import { WIRE_ONLY_DIMENSIONS } from '@modules/dimension/ids' +import { DROPPED_LEGACY_DIMENSIONS } from '@modules/dimension/ids' import type { CurrentVisualization, DimensionArray, @@ -15,7 +15,7 @@ export const adaptDimensions = ( visualization: CurrentVisualization ): DimensionArray => dimensions - .filter((dim) => !WIRE_ONLY_DIMENSIONS.has(dim.dimension)) + .filter((dim) => !DROPPED_LEGACY_DIMENSIONS.has(dim.dimension)) .flatMap((dim) => { const repetitionIndexes = dim.programStage?.id && dim.repetition?.indexes.length diff --git a/src/components/plugin-wrapper/hooks/query-tools-line-list.ts b/src/components/plugin-wrapper/hooks/query-tools-line-list.ts index 3e336a642..8eea93837 100644 --- a/src/components/plugin-wrapper/hooks/query-tools-line-list.ts +++ b/src/components/plugin-wrapper/hooks/query-tools-line-list.ts @@ -1,5 +1,5 @@ import { getAnalyticsRequestHeaderName } from '@modules/analytics-request' -import { WIRE_ONLY_DIMENSIONS } from '@modules/dimension/ids' +import { DROPPED_LEGACY_DIMENSIONS } from '@modules/dimension/ids' import type { Axis, CurrentVisualization, @@ -61,7 +61,7 @@ export const getAdaptedVisualization = ( const filters = visualization.filters ?? [] const headers = [...columns, ...rows] - .filter((dim) => !WIRE_ONLY_DIMENSIONS.has(dim.dimension)) + .filter((dim) => !DROPPED_LEGACY_DIMENSIONS.has(dim.dimension)) .map((dim) => buildHeaderNames(dim, visualization)) return { diff --git a/src/modules/dimension/ids.ts b/src/modules/dimension/ids.ts index f159edf47..75b58638f 100644 --- a/src/modules/dimension/ids.ts +++ b/src/modules/dimension/ids.ts @@ -64,9 +64,9 @@ export const resolveId = (id: string): string => { export const extractPlainDimensionId = (compoundId?: string | null): string => (compoundId ?? '').split('.').pop()! -/* Dimensions that exist only in the wire format (legacy event chart shape) - * and have no meaning in the app-local layer. */ -export const WIRE_ONLY_DIMENSIONS: ReadonlySet = new Set([ +/* Carried by visualizations saved in the legacy event chart shape. The app has + * no equivalent for any of them, so they are dropped wherever they turn up. */ +export const DROPPED_LEGACY_DIMENSIONS: ReadonlySet = new Set([ 'dy', 'latitude', 'longitude', diff --git a/src/modules/dimension/translation.ts b/src/modules/dimension/translation.ts index 0ff5be9ac..531069b70 100644 --- a/src/modules/dimension/translation.ts +++ b/src/modules/dimension/translation.ts @@ -1,7 +1,7 @@ import { CONTEXTLESS_DIMENSION_TYPES, META_DIMENSION_IDS, - WIRE_ONLY_DIMENSIONS, + DROPPED_LEGACY_DIMENSIONS, } from '@modules/dimension/ids' import type { CurrentVisualization, @@ -15,7 +15,8 @@ export const transformDimensions = ( ): DimensionArray => dimensions .filter( - (dimensionObj) => !WIRE_ONLY_DIMENSIONS.has(dimensionObj.dimension) + (dimensionObj) => + !DROPPED_LEGACY_DIMENSIONS.has(dimensionObj.dimension) ) .map((dimensionObj) => { if (dimensionObj.dimensionType === 'PROGRAM_DATA_ELEMENT') { diff --git a/src/modules/visualization/__tests__/visualization.spec.ts b/src/modules/visualization/__tests__/visualization.spec.ts index 26cbdf686..ffaceecfa 100644 --- a/src/modules/visualization/__tests__/visualization.spec.ts +++ b/src/modules/visualization/__tests__/visualization.spec.ts @@ -518,12 +518,12 @@ describe('normalizeApiSavedVisualization', () => { ].map((dim) => dim.dimension) it.each(['dy', 'latitude', 'longitude'])( - 'drops the wire-only %s dimension and marks the vis legacy', - (wireOnlyId) => { + 'drops the legacy %s dimension and marks the vis legacy', + (ignoredDimensionId) => { const result = normalizeApiSavedVisualization( buildApiVis({ rows: [ - { dimension: wireOnlyId }, + { dimension: ignoredDimensionId }, { dimension: 'ou', dimensionType: 'ORGANISATION_UNIT' }, ] as ApiSavedVisualization['rows'], }) @@ -534,7 +534,7 @@ describe('normalizeApiSavedVisualization', () => { } ) - it('does not mark a vis legacy when it carries no wire-only dimension', () => { + it('does not mark a vis legacy when it carries no dropped legacy dimension', () => { const result = normalizeApiSavedVisualization( buildApiVis({ rows: [ diff --git a/src/modules/visualization/state.ts b/src/modules/visualization/state.ts index 1aea73e41..84ef8a821 100644 --- a/src/modules/visualization/state.ts +++ b/src/modules/visualization/state.ts @@ -9,7 +9,7 @@ import { ENROLLMENT_SCOPED_DIMENSION_IDS, getCompoundDimensionId, META_DIMENSION_IDS, - WIRE_ONLY_DIMENSIONS, + DROPPED_LEGACY_DIMENSIONS, } from '@modules/dimension/ids' import { isTimeDimensionId, @@ -339,7 +339,7 @@ export const isCurrentVisualizationNew = ( const toAppLocalAxes = (dims: DimensionArray): DimensionArray => toAppLocalDimensions( - dims.filter((dim) => !WIRE_ONLY_DIMENSIONS.has(dim.dimension)) + dims.filter((dim) => !DROPPED_LEGACY_DIMENSIONS.has(dim.dimension)) ) const OPTION_KEYS = Object.keys(DEFAULT_OPTIONS) as Array< @@ -573,7 +573,7 @@ const normalizeLegacyDimension = ( * data-element / attribute UID, since that's still a live analytics * parameter * - Drop top-level `program` and `programStage` - * - Drop the wire-only `dy`/`latitude`/`longitude` dimensions + * - Drop the legacy `dy`/`latitude`/`longitude` dimensions * - Mark output as `legacy: true` whenever any of the above upgraded the * persisted shape, so the vis cannot be overwritten in place — only "Save * as" is allowed. Overwriting would silently persist in the canonical @@ -636,9 +636,9 @@ export const normalizeApiSavedVisualization = ( /* Dropping a dimension rewrites the persisted shape, so it flips `legacy` * the same way the conversions above do. */ - const dropWireOnlyDimensions = (dims: DimensionRecord[]) => { + const dropLegacyDimensions = (dims: DimensionRecord[]) => { const kept = dims.filter( - (dim) => !WIRE_ONLY_DIMENSIONS.has(dim.dimension) + (dim) => !DROPPED_LEGACY_DIMENSIONS.has(dim.dimension) ) if (kept.length !== dims.length) { normalizedVis.legacy = true @@ -647,17 +647,17 @@ export const normalizeApiSavedVisualization = ( } normalizedVis.columns = dropInvalidGrouping( - dropWireOnlyDimensions(columns).map((dim) => + dropLegacyDimensions(columns).map((dim) => normalizeLegacyDimension(dim, context, normalizedVis) ) ) normalizedVis.rows = dropInvalidGrouping( - dropWireOnlyDimensions(rows).map((dim) => + dropLegacyDimensions(rows).map((dim) => normalizeLegacyDimension(dim, context, normalizedVis) ) ) normalizedVis.filters = dropInvalidGrouping( - dropWireOnlyDimensions(rawFilters).map((dim) => + dropLegacyDimensions(rawFilters).map((dim) => normalizeLegacyDimension(dim, context, normalizedVis) ) )