Skip to content

Commit d8c7ec4

Browse files
HendrikThePendricclaude
HendrikThePendric
and
claude
authored
fix: stop merging analytics response metadata into the global store (#286)
* fix: stop merging analytics response metadata into the global store The /api/analytics enrollments aggregate response carries the generic backend field name "Organisation unit" keyed by the compound stage-OU id (e.g. "stageId.ou"). When the analytics-merge ingested this into the metadata store, it clobbered the program-specific label set from program.displayOrgUnitLabel — visible as the "Event org. unit" chip on a PT switching to "Organisation unit" after Update. The merge was redundant for every consumer of the store. Layout dimension records, fixed-dimension labels, and selected items come from the visualization-load (vis.metaData echoes selected items with their names). Dimension modals write selected item names at selection time. Option-set and legend-set names come from dedicated follow-up requests. The PT/LL table cells consume analyticsResponse.metaData.items ephemerally and never read the store for cell values. Removes addAnalyticsResponseMetadata, the analytics-data.ts module, and the onResponsesReceived prop plumbing through PluginWrapper, the PT/LL plugins and their hooks. Spinner control gets a simpler onDataAvailable signal in their place. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix: drop unneeded type assertion on second setVisualizationMetadata call Sonar S4325: TypeScript infers the type after the first call, so the cast on the second one is redundant. Extracting the cast to a local removes it without altering behaviour. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * refactor: keep the onResponseReceived name on the spinner-control callback Reverting the onDataAvailable rename from the previous commit. Despite the callback no longer carrying the response data, the name still describes the event we're responding to (an analytics response arrived); the rename was beyond the scope of the fix. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore: adjust visualization loading state assertion --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 5c18918 commit d8c7ec4

14 files changed

Lines changed: 37 additions & 487 deletions

‎src/components/app-wrapper/metadata-provider/metadata-provider.tsx‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,6 @@ export type UseMetadataStoreReturnValue = Pick<
265265
| 'getUserOrgUnitMetadataItem'
266266
| 'getDimensionMetadataItem'
267267
| 'addMetadata'
268-
| 'addAnalyticsResponseMetadata'
269268
| 'setVisualizationMetadata'
270269
>
271270
export const useMetadataStore = (): UseMetadataStoreReturnValue => {
@@ -288,8 +287,6 @@ export const useMetadataStore = (): UseMetadataStoreReturnValue => {
288287
getDimensionMetadataItem:
289288
metadataStore.getDimensionMetadataItem.bind(metadataStore),
290289
addMetadata: metadataStore.addMetadata.bind(metadataStore),
291-
addAnalyticsResponseMetadata:
292-
metadataStore.addAnalyticsResponseMetadata.bind(metadataStore),
293290
setVisualizationMetadata:
294291
metadataStore.setVisualizationMetadata.bind(metadataStore),
295292
}))

‎src/components/app/app.tsx‎

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,19 +13,12 @@ import {
1313
} from '@components/grid'
1414
import { InterpretationModal } from '@components/interpretation-modal/interpretation-modal'
1515
import { LayoutPanel } from '@components/layout-panel/layout-panel'
16-
import type { LineListAnalyticsDataHeader } from '@components/line-list/types'
17-
import type { AnalyticsResponseMetadataItems } from '@components/plugin-wrapper/hooks/use-line-list-analytics-data'
1816
import { PluginWrapper } from '@components/plugin-wrapper/plugin-wrapper'
1917
import { Sidebar } from '@components/sidebar/sidebar'
2018
import { StartScreen } from '@components/start-screen/start-screen'
2119
import { Toolbar } from '@components/toolbar/toolbar'
2220
import { CssVariables } from '@dhis2/ui'
23-
import {
24-
useAppDispatch,
25-
useAppSelector,
26-
useCurrentUser,
27-
useMetadataStore,
28-
} from '@hooks'
21+
import { useAppDispatch, useAppSelector, useCurrentUser } from '@hooks'
2922
import { isVisualizationEmpty } from '@modules/visualization'
3023
import { getCurrentVis, setCurrentVis } from '@store/current-vis-slice'
3124
import {
@@ -44,7 +37,6 @@ import './styles/app.module.css'
4437

4538
const EventVisualizer: FC = () => {
4639
useLoadVisualizationOnMount()
47-
const { addAnalyticsResponseMetadata } = useMetadataStore()
4840
const dispatch = useAppDispatch()
4941
const currentUser = useCurrentUser()
5042
const currentVis = useAppSelector(getCurrentVis)
@@ -74,16 +66,6 @@ const EventVisualizer: FC = () => {
7466
[currentVis, dispatch]
7567
)
7668

77-
const onResponsesReceived = useCallback(
78-
(
79-
analyticsMetadata: AnalyticsResponseMetadataItems,
80-
headers?: Array<LineListAnalyticsDataHeader>
81-
) => {
82-
addAnalyticsResponseMetadata(analyticsMetadata, headers)
83-
},
84-
[addAnalyticsResponseMetadata]
85-
)
86-
8769
const onDimensionModalClose = useCallback(
8870
() => dispatch(setUiActiveDimensionModal(null)),
8971
[dispatch]
@@ -103,7 +85,6 @@ const EventVisualizer: FC = () => {
10385
visualization={currentVis}
10486
displayProperty={currentUser.settings.displayProperty}
10587
onDataSorted={onDataSorted}
106-
onResponsesReceived={onResponsesReceived}
10788
/>
10889
</ErrorBoundary>
10990
<InterpretationModal />

‎src/components/plugin-wrapper/__tests__/plugin-wrapper.spec.tsx‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ describe('PluginWrapper', () => {
2121
const eventVisualization1Id = 'TIuOzZ0ID0V'
2222
const eventVisualization2Id = 'waPjzoJyIQ9'
2323
const mockOnDataSorted = vi.fn()
24-
const mockOnResponsesReceived = vi.fn()
2524
/* The analytics handler is deferred so each test can hold the response
2625
* in flight long enough to assert the in-flight UI (spinner shown), then
2726
* call `releaseAll()` to let the response arrive and assert the settled
@@ -73,7 +72,6 @@ describe('PluginWrapper', () => {
7372
visualization={currentVis}
7473
displayProperty={currentUser.settings.displayProperty}
7574
onDataSorted={mockOnDataSorted}
76-
onResponsesReceived={mockOnResponsesReceived}
7775
/>
7876
)
7977
}
@@ -130,7 +128,6 @@ describe('PluginWrapper', () => {
130128
expect(
131129
screen.queryByTestId('dhis2-uicore-circularloader')
132130
).not.toBeInTheDocument()
133-
expect(mockOnResponsesReceived).toBeCalledTimes(1)
134131
})
135132
}
136133

@@ -198,7 +195,6 @@ describe('PluginWrapper', () => {
198195
expect(
199196
screen.queryByTestId('dhis2-uicore-circularloader')
200197
).not.toBeInTheDocument()
201-
expect(mockOnResponsesReceived).toBeCalledTimes(2)
202198
})
203199
})
204200

@@ -251,7 +247,6 @@ describe('PluginWrapper', () => {
251247
expect(
252248
screen.queryByTestId('dhis2-uicore-circularloader')
253249
).not.toBeInTheDocument()
254-
expect(mockOnResponsesReceived).toBeCalledTimes(2)
255250
})
256251
})
257252

@@ -294,7 +289,6 @@ describe('PluginWrapper', () => {
294289
expect(
295290
screen.queryByTestId('dhis2-uicore-circularloader')
296291
).not.toBeInTheDocument()
297-
expect(mockOnResponsesReceived).toBeCalledTimes(2)
298292
})
299293
})
300294

@@ -319,9 +313,7 @@ describe('PluginWrapper', () => {
319313
// Visualisation loading state remains false but the current vis gets updated
320314
await waitFor(() => {
321315
expect(store.getState().loader.isVisualizationLoading).toBe(false)
322-
expect(store.getState().currentVis === currentVisSnapshot).toBe(
323-
false
324-
)
316+
expect(store.getState().currentVis).not.toBe(currentVisSnapshot)
325317
})
326318

327319
// Table kept in the DOM and the spinner is showing
@@ -344,7 +336,6 @@ describe('PluginWrapper', () => {
344336
expect(
345337
screen.queryByTestId('dhis2-uicore-circularloader')
346338
).not.toBeInTheDocument()
347-
expect(mockOnResponsesReceived).toBeCalledTimes(2)
348339
expect(mockOnDataSorted).toBeCalledTimes(1)
349340
})
350341
})

‎src/components/plugin-wrapper/hooks/use-line-list-analytics-data.spec.ts‎

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import type { UseMetadataStoreReturnValue } from '@components/app-wrapper/metadata-provider/metadata-provider'
2-
import { extractMetadataFromAnalyticsResponse } from '@modules/metadata-store/analytics-data'
32
import type { CurrentVisualization, DimensionMetadataItem } from '@types'
43
import { describe, it, expect } from 'vitest'
54
import { buildHeaders, formatRowValue } from './use-line-list-analytics-data'
@@ -62,34 +61,6 @@ describe('buildHeaders', () => {
6261
dimensionSuffix: 'Baby Postnatal',
6362
})
6463
})
65-
66-
it('does not accumulate the stage suffix across repeated updates', () => {
67-
const firstHeaders = buildHeaders({
68-
analyticsResponse: analyticsResponseTyped,
69-
visualization,
70-
metadataStore: buildMetadataStore('Scheduled date'),
71-
})
72-
73-
const metadataAfterUpdate = extractMetadataFromAnalyticsResponse(
74-
analyticsResponseTyped.metaData.items,
75-
firstHeaders
76-
) as Record<string, { name?: string }>
77-
78-
expect(metadataAfterUpdate['s1.scheduledDate'].name).toBe(
79-
'Scheduled date'
80-
)
81-
82-
const storedName = metadataAfterUpdate['s1.scheduledDate']
83-
.name as string
84-
const secondHeaders = buildHeaders({
85-
analyticsResponse: analyticsResponseTyped,
86-
visualization,
87-
metadataStore: buildMetadataStore(storedName),
88-
})
89-
90-
expect(secondHeaders[0].column).toBe('Scheduled date')
91-
expect(secondHeaders[0].dimensionSuffix).toBe('Birth')
92-
})
9364
})
9465

9566
describe('formatRowValue', () => {

‎src/components/plugin-wrapper/hooks/use-line-list-analytics-data.ts‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -428,12 +428,6 @@ export type AnalyticsResponseMetadataItems = Record<
428428
USER_ORG_UNIT?: UserOrgUnitMetadataItem
429429
}
430430

431-
export type AnalyticsResponseMetadataDimensions = Record<string, string[]>
432-
export type OnAnalyticsResponseReceivedCb = (
433-
items: AnalyticsResponseMetadataItems,
434-
headers: Array<LineListAnalyticsDataHeader>
435-
) => void
436-
437431
type FetchAnalyticsDataForLLParams = {
438432
analyticsEngine: ReturnType<typeof Analytics.getAnalytics>
439433
visualization: CurrentVisualization
@@ -451,7 +445,7 @@ type FetchAnalyticsDataParams = {
451445
displayProperty: CurrentUser['settings']['displayProperty']
452446
pageSize?: number
453447
page?: number
454-
onResponseReceived: OnAnalyticsResponseReceivedCb
448+
onResponseReceived: () => void
455449
}
456450
type FetchAnalyticsDataFn = (params: FetchAnalyticsDataParams) => Promise<void>
457451
type AnalyticsDataState = {
@@ -536,7 +530,7 @@ const useLineListAnalyticsData = (): UseAnalyticsDataResult => {
536530
isFetching: false,
537531
})
538532

539-
onResponseReceived(analyticsResponse.metaData.items, headers)
533+
onResponseReceived()
540534
} catch (error) {
541535
logger.error('fetch LL data error', error)
542536
setState({

‎src/components/plugin-wrapper/hooks/use-pivot-table-analytics-data.ts‎

Lines changed: 3 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,7 @@ import { Analytics, transformEventAggregateResponse } from '@dhis2/analytics'
33
import { type FetchError, useDataEngine } from '@dhis2/app-runtime'
44
import { logger } from '@modules/logger'
55
import { getSingleProgramFromVisualization } from '@modules/visualization'
6-
import type {
7-
CurrentUser,
8-
CurrentVisualization,
9-
MetadataInputItem,
10-
UserOrgUnitMetadataItem,
11-
} from '@types'
6+
import type { CurrentUser, CurrentVisualization } from '@types'
127
import { useCallback, useState } from 'react'
138
import { getAnalyticsEndpoint } from './query-tools-common'
149
import { getAdaptedVisualization } from './query-tools-pivot-table'
@@ -86,23 +81,11 @@ export type PivotTableAnalyticsData = {
8681
}
8782
}
8883

89-
export type AnalyticsResponseMetadataItems = Record<
90-
string,
91-
MetadataInputItem
92-
> & {
93-
USER_ORG_UNIT?: UserOrgUnitMetadataItem
94-
}
95-
96-
export type AnalyticsResponseMetadataDimensions = Record<string, string[]>
97-
export type OnAnalyticsResponseReceivedCb = (
98-
items: AnalyticsResponseMetadataItems
99-
) => void
100-
10184
type FetchAnalyticsDataForPTParams = {
10285
visualization: CurrentVisualization
10386
filters?: Record<string, unknown>
10487
displayProperty: CurrentUser['settings']['displayProperty']
105-
onResponseReceived: OnAnalyticsResponseReceivedCb
88+
onResponseReceived: () => void
10689
}
10790
type FetchAnalyticsDataFn = (
10891
params: FetchAnalyticsDataForPTParams
@@ -160,7 +143,7 @@ const usePivotTableAnalyticsData = (): UseAnalyticsDataResult => {
160143
isFetching: false,
161144
})
162145

163-
onResponseReceived(analyticsResponse.metaData.items)
146+
onResponseReceived()
164147
} catch (error) {
165148
logger.error('PT fetch error', error)
166149
setState({

‎src/components/plugin-wrapper/line-list-plugin.tsx‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,7 @@ import { setUiActiveDimensionModal } from '@store/ui-slice'
1212
import type { CurrentUser, CurrentVisualization, Sorting } from '@types'
1313
import { useCallback, useEffect, useMemo, useState } from 'react'
1414
import type { FC } from 'react'
15-
import {
16-
useLineListAnalyticsData,
17-
type OnAnalyticsResponseReceivedCb,
18-
} from './hooks/use-line-list-analytics-data'
15+
import { useLineListAnalyticsData } from './hooks/use-line-list-analytics-data'
1916

2017
type InternalSorting = Sorting | undefined
2118

@@ -26,7 +23,7 @@ type LineListPluginProps = {
2623
isInDashboard: boolean
2724
isInModal: boolean
2825
onDataSorted?: (sorting: InternalSorting) => void
29-
onResponseReceived: OnAnalyticsResponseReceivedCb
26+
onResponseReceived: () => void
3027
}
3128

3229
export const LineListPlugin: FC<LineListPluginProps> = ({

‎src/components/plugin-wrapper/pivot-table-plugin.tsx‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,7 @@ import {
55
} from '@modules/visualization'
66
import type { CurrentUser, CurrentVisualization, DimensionArray } from '@types'
77
import { type FC, useEffect, useMemo } from 'react'
8-
import {
9-
usePivotTableAnalyticsData,
10-
type OnAnalyticsResponseReceivedCb,
11-
} from './hooks/use-pivot-table-analytics-data'
8+
import { usePivotTableAnalyticsData } from './hooks/use-pivot-table-analytics-data'
129

1310
const formatVisualizationForPivotTableEngine = (
1411
visualization: CurrentVisualization
@@ -39,7 +36,7 @@ type PivotTablePluginProps = {
3936
filters?: Record<string, unknown> // TODO: verify this type
4037
isInDashboard: boolean
4138
isInModal: boolean
42-
onResponseReceived: OnAnalyticsResponseReceivedCb
39+
onResponseReceived: () => void
4340
}
4441

4542
export const PivotTablePlugin: FC<PivotTablePluginProps> = ({

‎src/components/plugin-wrapper/plugin-wrapper.tsx‎

Lines changed: 6 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import type { LineListAnalyticsDataHeader } from '@components/line-list/types'
21
import { Center, CircularLoader } from '@dhis2/ui'
32
import { useAppSelector } from '@hooks'
43
import { isVisualizationEmpty } from '@modules/visualization'
@@ -12,11 +11,6 @@ import type {
1211
} from '@types'
1312
import type { FC } from 'react'
1413
import { useCallback, useEffect, useState } from 'react'
15-
import type {
16-
AnalyticsResponseMetadataItems,
17-
OnAnalyticsResponseReceivedCb as OnLLAnalyticsResponseReceivedCb,
18-
} from './hooks/use-line-list-analytics-data'
19-
import type { OnAnalyticsResponseReceivedCb as OnPTAnalyticsResponseReceivedCb } from './hooks/use-pivot-table-analytics-data'
2014
import { LineListPlugin } from './line-list-plugin'
2115
import { PivotTablePlugin } from './pivot-table-plugin'
2216
import classes from './styles/plugin-wrapper.module.css'
@@ -48,10 +42,6 @@ type PluginWrapperProps = {
4842
isInModal?: boolean // passed when viewing an intepretation via the InterpretationModal from analytics
4943
isVisualizationLoading?: boolean
5044
onDataSorted?: (sorting: Sorting | undefined) => void
51-
onResponsesReceived?: (
52-
items: AnalyticsResponseMetadataItems,
53-
headers?: Array<LineListAnalyticsDataHeader>
54-
) => void
5545
}
5646

5747
export const PluginWrapper: FC<PluginWrapperProps> = ({
@@ -62,33 +52,18 @@ export const PluginWrapper: FC<PluginWrapperProps> = ({
6252
isInModal = false,
6353
isVisualizationLoading = false,
6454
onDataSorted,
65-
onResponsesReceived: onResponsesReceivedCb,
6655
}) => {
6756
const layoutKey = useAppSelector(getCurrentVisLayoutKey)
6857
const [hasAnalyticsData, setHasAnalyticsData] = useState(false)
6958

70-
const onLLResponseReceived = useCallback<OnLLAnalyticsResponseReceivedCb>(
71-
(items, headers) => {
72-
setHasAnalyticsData(true)
73-
74-
onResponsesReceivedCb?.(items, headers)
75-
},
76-
[onResponsesReceivedCb]
77-
)
78-
79-
const onPTResponseReceived = useCallback<OnPTAnalyticsResponseReceivedCb>(
80-
(items) => {
81-
setHasAnalyticsData(true)
82-
83-
onResponsesReceivedCb?.(items)
84-
},
85-
[onResponsesReceivedCb]
86-
)
59+
const onResponseReceived = useCallback(() => {
60+
setHasAnalyticsData(true)
61+
}, [])
8762

8863
useEffect(() => {
8964
if (isVisualizationLoading === true) {
9065
// Reset hasAnalyticsData when a new visualization is fetched as we know it will need to re-fetch analytics.
91-
// This allows the spinner to show until the analytics response is available and the onResponseReceived above
66+
// This allows the spinner to show until the analytics response is available and onResponseReceived
9267
// changes hasAnalyticsData to true.
9368
setHasAnalyticsData(false)
9469
}
@@ -120,7 +95,7 @@ export const PluginWrapper: FC<PluginWrapperProps> = ({
12095
isInDashboard={isInDashboard}
12196
isInModal={isInModal}
12297
onDataSorted={onDataSorted}
123-
onResponseReceived={onLLResponseReceived}
98+
onResponseReceived={onResponseReceived}
12499
/>
125100
)}
126101
{visualization.type === 'PIVOT_TABLE' && (
@@ -131,7 +106,7 @@ export const PluginWrapper: FC<PluginWrapperProps> = ({
131106
filters={filters}
132107
isInDashboard={isInDashboard}
133108
isInModal={isInModal}
134-
onResponseReceived={onPTResponseReceived}
109+
onResponseReceived={onResponseReceived}
135110
/>
136111
)}
137112
</div>

0 commit comments

Comments
 (0)