From c4806488fd3f109ab33217464c5ef31d8471b698 Mon Sep 17 00:00:00 2001 From: Hendrik de Graaf Date: Wed, 16 Sep 2026 15:04:27 +0200 Subject: [PATCH 1/5] fix: allow ou dimension to be prefixed --- src/modules/pivotTable/PivotTableEngine.js | 50 +++++--- .../pivotTableEngineHierarchy.spec.js | 109 ++++++++++++++++++ src/modules/predefinedDimensions.js | 1 + 3 files changed, 144 insertions(+), 16 deletions(-) create mode 100644 src/modules/pivotTable/__tests__/pivotTableEngineHierarchy.spec.js diff --git a/src/modules/pivotTable/PivotTableEngine.js b/src/modules/pivotTable/PivotTableEngine.js index 909058cf0..998dfa1e6 100644 --- a/src/modules/pivotTable/PivotTableEngine.js +++ b/src/modules/pivotTable/PivotTableEngine.js @@ -6,7 +6,10 @@ import { DIMENSION_TYPE_ORGANISATION_UNIT, DIMENSION_TYPE_PERIOD, } from '../dataTypes.js' -import { DIMENSION_ID_ORGUNIT } from '../predefinedDimensions.js' +import { + DIMENSION_ID_ENROLLMENT_ORGUNIT, + DIMENSION_ID_ORGUNIT, +} from '../predefinedDimensions.js' import { renderValue } from '../renderValue.js' import { VALUE_TYPE_NUMBER, @@ -99,6 +102,19 @@ const listByDimension = (list) => return all }, {}) +const ORGUNIT_DIMENSION_IDS = [ + DIMENSION_ID_ORGUNIT, + DIMENSION_ID_ENROLLMENT_ORGUNIT, +] + +/* Event and enrollment analytics qualify the event org unit dimension with + * the program stage (`.ou`) and name the enrollment-scoped one + * `enrollmentou`. Neither carries `dimensionType` in `metaData.items`, so + * match on the unqualified dimension id where the type is unavailable. */ +const isOrgUnitDimension = ({ dimension, meta }) => + meta?.dimensionType === DIMENSION_TYPE_ORGANISATION_UNIT || + ORGUNIT_DIMENSION_IDS.includes(dimension.split('.').pop()) + const sortByHierarchy = (items) => { items.sort((a, b) => { if (!a.hierarchy || !b.hierarchy) { @@ -108,6 +124,17 @@ const sortByHierarchy = (items) => { }) } +const applyHierarchy = (ouDimension, ouNameHierarchy) => { + ouDimension.items.forEach((ou) => { + const hierarchy = ouNameHierarchy[ou.uid] + if (hierarchy) { + ou.hierarchy = hierarchy.split('/').filter((x) => x.length) + } + }) + sortByHierarchy(ouDimension.items) + ouDimension.itemIds = ouDimension.items.map((item) => item.uid) +} + const buildDimensionLookup = (visualization, metadata, headers) => { const rows = visualization.rows.map((row) => ({ dimension: row.dimension, @@ -165,21 +192,12 @@ const buildDimensionLookup = (visualization, metadata, headers) => { return out }, {}) - const ouDimension = allByDimension[DIMENSION_ID_ORGUNIT] - - if ( - visualization.showHierarchy && - metadata.ouNameHierarchy && - ouDimension - ) { - ouDimension.items.forEach((ou) => { - const hierarchy = metadata.ouNameHierarchy[ou.uid] - if (hierarchy) { - ou.hierarchy = hierarchy.split('/').filter((x) => x.length) - } - }) - sortByHierarchy(ouDimension.items) - ouDimension.itemIds = ouDimension.items.map((item) => item.uid) + if (visualization.showHierarchy && metadata.ouNameHierarchy) { + Object.values(allByDimension) + .filter(isOrgUnitDimension) + .forEach((ouDimension) => + applyHierarchy(ouDimension, metadata.ouNameHierarchy) + ) } return { diff --git a/src/modules/pivotTable/__tests__/pivotTableEngineHierarchy.spec.js b/src/modules/pivotTable/__tests__/pivotTableEngineHierarchy.spec.js new file mode 100644 index 000000000..f4ddaaf6e --- /dev/null +++ b/src/modules/pivotTable/__tests__/pivotTableEngineHierarchy.spec.js @@ -0,0 +1,109 @@ +import { PivotTableEngine } from '../PivotTableEngine.js' + +/* The engine measures every cell against a canvas 2d context, which jsdom + * does not implement. */ +jest.mock('../measureText.js', () => ({ + measureTextWithWrapping: () => ({ width: 100, height: 20 }), +})) + +const OU_A = 'ouA' +const OU_B = 'ouB' +const STAGE = 'Zj7UnCAulEk' + +/* Bo sorts after Bombali alphabetically but before it in hierarchy order, + * so the two orderings disagree and the test can tell them apart. */ +const OU_NAME_HIERARCHY = { + [OU_A]: '/Sierra Leone/Western Area/Bo', + [OU_B]: '/Sierra Leone/Northern Province/Bombali', +} + +const buildData = (ouDimensionId) => ({ + headers: [ + { name: ouDimensionId, meta: true }, + { name: 'value', meta: false }, + ], + metaData: { + items: { + [ouDimensionId]: { name: 'Organisation unit' }, + [OU_A]: { uid: OU_A, name: 'Bo' }, + [OU_B]: { uid: OU_B, name: 'Bombali' }, + }, + dimensions: { [ouDimensionId]: [OU_A, OU_B] }, + ouNameHierarchy: OU_NAME_HIERARCHY, + }, + rows: [ + [OU_A, '1'], + [OU_B, '2'], + ], + height: 2, + width: 2, +}) + +const buildVisualization = (ouDimensionId) => ({ + showHierarchy: true, + rows: [{ dimension: ouDimensionId }], + columns: [], + filters: [], +}) + +const rowHierarchies = (engine) => + [0, 1].map((row) => engine.getRowHeader(row)[0].hierarchy) + +describe('PivotTableEngine org unit hierarchy', () => { + it('applies the hierarchy to a bare `ou` dimension', () => { + const engine = new PivotTableEngine( + buildVisualization('ou'), + buildData('ou') + ) + + expect(rowHierarchies(engine)).toEqual([ + ['Sierra Leone', 'Northern Province', 'Bombali'], + ['Sierra Leone', 'Western Area', 'Bo'], + ]) + }) + + it('applies the hierarchy to a stage-qualified `ou` dimension', () => { + const dimension = `${STAGE}.ou` + const engine = new PivotTableEngine( + buildVisualization(dimension), + buildData(dimension) + ) + + expect(rowHierarchies(engine)).toEqual([ + ['Sierra Leone', 'Northern Province', 'Bombali'], + ['Sierra Leone', 'Western Area', 'Bo'], + ]) + }) + + it('applies the hierarchy to an `enrollmentou` dimension', () => { + const engine = new PivotTableEngine( + buildVisualization('enrollmentou'), + buildData('enrollmentou') + ) + + expect(rowHierarchies(engine)).toEqual([ + ['Sierra Leone', 'Northern Province', 'Bombali'], + ['Sierra Leone', 'Western Area', 'Bo'], + ]) + }) + + it('leaves items untouched when showHierarchy is off', () => { + const engine = new PivotTableEngine( + { ...buildVisualization(`${STAGE}.ou`), showHierarchy: false }, + buildData(`${STAGE}.ou`) + ) + + expect(rowHierarchies(engine)).toEqual([undefined, undefined]) + expect(engine.getRowHeader(0)[0].uid).toBe(OU_A) + }) + + it('leaves non-org-unit dimensions untouched', () => { + const dimension = `${STAGE}.de1` + const engine = new PivotTableEngine( + buildVisualization(dimension), + buildData(dimension) + ) + + expect(rowHierarchies(engine)).toEqual([undefined, undefined]) + }) +}) diff --git a/src/modules/predefinedDimensions.js b/src/modules/predefinedDimensions.js index 330f35ad2..f42352cd2 100644 --- a/src/modules/predefinedDimensions.js +++ b/src/modules/predefinedDimensions.js @@ -9,6 +9,7 @@ import i18n from '../locales/index.js' export const DIMENSION_ID_DATA = 'dx' export const DIMENSION_ID_PERIOD = 'pe' export const DIMENSION_ID_ORGUNIT = 'ou' +export const DIMENSION_ID_ENROLLMENT_ORGUNIT = 'enrollmentou' export const DIMENSION_ID_ASSIGNED_CATEGORIES = 'co' export const DIMENSION_PROP_NO_ITEMS = 'noItems' From 442a1a2c98b0a0c2122e323d0fcc45af4bbb4e63 Mon Sep 17 00:00:00 2001 From: Hendrik de Graaf Date: Wed, 16 Sep 2026 15:09:39 +0200 Subject: [PATCH 2/5] chore: use Set instead of array for lookup --- src/modules/pivotTable/PivotTableEngine.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/modules/pivotTable/PivotTableEngine.js b/src/modules/pivotTable/PivotTableEngine.js index 998dfa1e6..cdcc8257c 100644 --- a/src/modules/pivotTable/PivotTableEngine.js +++ b/src/modules/pivotTable/PivotTableEngine.js @@ -102,10 +102,10 @@ const listByDimension = (list) => return all }, {}) -const ORGUNIT_DIMENSION_IDS = [ +const ORGUNIT_DIMENSION_IDS = new Set([ DIMENSION_ID_ORGUNIT, DIMENSION_ID_ENROLLMENT_ORGUNIT, -] +]) /* Event and enrollment analytics qualify the event org unit dimension with * the program stage (`.ou`) and name the enrollment-scoped one @@ -113,7 +113,7 @@ const ORGUNIT_DIMENSION_IDS = [ * match on the unqualified dimension id where the type is unavailable. */ const isOrgUnitDimension = ({ dimension, meta }) => meta?.dimensionType === DIMENSION_TYPE_ORGANISATION_UNIT || - ORGUNIT_DIMENSION_IDS.includes(dimension.split('.').pop()) + ORGUNIT_DIMENSION_IDS.has(dimension.split('.').pop()) const sortByHierarchy = (items) => { items.sort((a, b) => { From 193f1efae0ef9c1fb46d1b6f41af2bdac77de3c9 Mon Sep 17 00:00:00 2001 From: Hendrik de Graaf Date: Thu, 17 Sep 2026 13:30:29 +0200 Subject: [PATCH 3/5] fix: split the aggregate event and enrollment analytics requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `getAggregate` went through `fetch`, which issues one request for data and metaData together. `get` issues two in parallel — one with `skipMeta=true`, one with `skipData=true` and `includeMetadataDetails=true` — and merges them. It is what `analytics.aggregate.get` already does and what data-visualizer-app uses. The metaData request is what makes `PivotTableEngine.applyHierarchy` work: without `includeMetadataDetails` a `metaData.items` entry carries only a name, so the org unit ids it reads from `item.uid` are all undefined. Splitting also routes the request through `analyticsDataQuery`, which sorts dimensions and items so that layouts differing only in order share a cache entry. Co-Authored-By: Claude Opus 5 (1M context) --- src/api/analytics/AnalyticsEnrollments.js | 2 +- src/api/analytics/AnalyticsEvents.js | 2 +- .../__tests__/AnalyticsEnrollments.spec.js | 30 +++++++++++++++++-- .../__tests__/AnalyticsEvents.spec.js | 30 +++++++++++++++++-- 4 files changed, 56 insertions(+), 8 deletions(-) diff --git a/src/api/analytics/AnalyticsEnrollments.js b/src/api/analytics/AnalyticsEnrollments.js index 8920ad2f5..71b4fa69f 100644 --- a/src/api/analytics/AnalyticsEnrollments.js +++ b/src/api/analytics/AnalyticsEnrollments.js @@ -30,7 +30,7 @@ class AnalyticsEnrollments extends AnalyticsBase { * .then(console.log); */ getAggregate(req) { - return this.fetch(req.withPath('enrollments/aggregate')) + return this.get(req.withPath('enrollments/aggregate')) } /** diff --git a/src/api/analytics/AnalyticsEvents.js b/src/api/analytics/AnalyticsEvents.js index 3f1896098..680e702f4 100644 --- a/src/api/analytics/AnalyticsEvents.js +++ b/src/api/analytics/AnalyticsEvents.js @@ -30,7 +30,7 @@ class AnalyticsEvents extends AnalyticsBase { * .then(console.log); */ getAggregate(req) { - return this.fetch(req.withPath('events/aggregate')) + return this.get(req.withPath('events/aggregate')) } /** diff --git a/src/api/analytics/__tests__/AnalyticsEnrollments.spec.js b/src/api/analytics/__tests__/AnalyticsEnrollments.spec.js index 5483714a2..549d11822 100644 --- a/src/api/analytics/__tests__/AnalyticsEnrollments.spec.js +++ b/src/api/analytics/__tests__/AnalyticsEnrollments.spec.js @@ -36,7 +36,10 @@ describe('analytics.enrollments', () => { fixture = fixtures.get('/api/analytics/aggregate') dataEngineMock.query.mockReturnValue( - Promise.resolve({ data: fixture }) + Promise.resolve({ + data: { ...fixture, metaData: undefined }, + metaData: { metaData: fixture.metaData }, + }) ) }) @@ -44,10 +47,31 @@ describe('analytics.enrollments', () => { expect(enrollments.getAggregate).toBeInstanceOf(Function) }) - it('should resolve a promise with data', () => + it('should resolve a promise with the merged data and metaData', () => enrollments.getAggregate(request).then((data) => { - expect(data).toEqual(fixture) + expect(data.rows).toEqual(fixture.rows) + expect(data.headers).toEqual(fixture.headers) + expect(data.metaData).toEqual(fixture.metaData) })) + + it('should request data and metaData separately', async () => { + await enrollments.getAggregate(request) + + const [queries, { variables }] = dataEngineMock.query.mock.calls[0] + + expect(queries.data.id(variables)).toBe('enrollments/aggregate') + expect(queries.metaData.id(variables)).toBe('enrollments/aggregate') + + expect(queries.data.params(variables)).toMatchObject({ + skipMeta: true, + skipData: false, + }) + expect(queries.metaData.params(variables)).toMatchObject({ + skipMeta: false, + skipData: true, + includeMetadataDetails: true, + }) + }) }) describe('.getQuery()', () => { diff --git a/src/api/analytics/__tests__/AnalyticsEvents.spec.js b/src/api/analytics/__tests__/AnalyticsEvents.spec.js index e75134509..d412f7bc0 100644 --- a/src/api/analytics/__tests__/AnalyticsEvents.spec.js +++ b/src/api/analytics/__tests__/AnalyticsEvents.spec.js @@ -36,7 +36,10 @@ describe('analytics.events', () => { fixture = fixtures.get('/api/analytics/aggregate') dataEngineMock.query.mockReturnValue( - Promise.resolve({ data: fixture }) + Promise.resolve({ + data: { ...fixture, metaData: undefined }, + metaData: { metaData: fixture.metaData }, + }) ) }) @@ -44,10 +47,31 @@ describe('analytics.events', () => { expect(events.getAggregate).toBeInstanceOf(Function) }) - it('should resolve a promise with data', () => + it('should resolve a promise with the merged data and metaData', () => events.getAggregate(request).then((data) => { - expect(data).toEqual(fixture) + expect(data.rows).toEqual(fixture.rows) + expect(data.headers).toEqual(fixture.headers) + expect(data.metaData).toEqual(fixture.metaData) })) + + it('should request data and metaData separately', async () => { + await events.getAggregate(request) + + const [queries, { variables }] = dataEngineMock.query.mock.calls[0] + + expect(queries.data.id(variables)).toBe('events/aggregate') + expect(queries.metaData.id(variables)).toBe('events/aggregate') + + expect(queries.data.params(variables)).toMatchObject({ + skipMeta: true, + skipData: false, + }) + expect(queries.metaData.params(variables)).toMatchObject({ + skipMeta: false, + skipData: true, + includeMetadataDetails: true, + }) + }) }) describe('.getCount()', () => { From 05ade2f0e2a01192e64d1c7e0223ccd0b44d3a9a Mon Sep 17 00:00:00 2001 From: Hendrik de Graaf Date: Thu, 17 Sep 2026 13:30:29 +0200 Subject: [PATCH 4/5] test: assert pivot values stay aligned when rows re-sort by hierarchy Sorting rewrites `itemIds`, which the row lookup resolves data rows through. The characteristic failure is a table with correct headers and an entirely empty value grid, which the hierarchy assertions alone do not catch. Co-Authored-By: Claude Opus 5 (1M context) --- .../pivotTableEngineHierarchy.spec.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/modules/pivotTable/__tests__/pivotTableEngineHierarchy.spec.js b/src/modules/pivotTable/__tests__/pivotTableEngineHierarchy.spec.js index f4ddaaf6e..2dd711fee 100644 --- a/src/modules/pivotTable/__tests__/pivotTableEngineHierarchy.spec.js +++ b/src/modules/pivotTable/__tests__/pivotTableEngineHierarchy.spec.js @@ -49,6 +49,12 @@ const buildVisualization = (ouDimensionId) => ({ const rowHierarchies = (engine) => [0, 1].map((row) => engine.getRowHeader(row)[0].hierarchy) +const rowNames = (engine) => + [0, 1].map((row) => engine.getRowHeader(row)[0].name) + +const cellValues = (engine) => + [0, 1].map((row) => engine.get({ row, column: 0 })?.renderedValue) + describe('PivotTableEngine org unit hierarchy', () => { it('applies the hierarchy to a bare `ou` dimension', () => { const engine = new PivotTableEngine( @@ -97,6 +103,18 @@ describe('PivotTableEngine org unit hierarchy', () => { expect(engine.getRowHeader(0)[0].uid).toBe(OU_A) }) + /* Sorting rewrites itemIds, which the row lookup resolves data rows + * through. If the two fall out of step every cell renders empty. */ + it('keeps values aligned with the re-sorted rows', () => { + const engine = new PivotTableEngine( + buildVisualization(`${STAGE}.ou`), + buildData(`${STAGE}.ou`) + ) + + expect(rowNames(engine)).toEqual(['Bombali', 'Bo']) + expect(cellValues(engine)).toEqual(['2', '1']) + }) + it('leaves non-org-unit dimensions untouched', () => { const dimension = `${STAGE}.de1` const engine = new PivotTableEngine( From 4e3b219b04a0c1432b10cd3a3a0e336624747c09 Mon Sep 17 00:00:00 2001 From: Hendrik de Graaf Date: Thu, 17 Sep 2026 16:10:27 +0200 Subject: [PATCH 5/5] test: assert the engine is inert when ouNameHierarchy is empty A backend that returns no hierarchy for the requested dimension still sends the key as an empty object, which passes the truthiness guard and lets applyHierarchy run. Rows must keep their order and their values. Co-Authored-By: Claude Opus 5 (1M context) --- .../__tests__/pivotTableEngineHierarchy.spec.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/modules/pivotTable/__tests__/pivotTableEngineHierarchy.spec.js b/src/modules/pivotTable/__tests__/pivotTableEngineHierarchy.spec.js index 2dd711fee..56b5e8b3b 100644 --- a/src/modules/pivotTable/__tests__/pivotTableEngineHierarchy.spec.js +++ b/src/modules/pivotTable/__tests__/pivotTableEngineHierarchy.spec.js @@ -115,6 +115,20 @@ describe('PivotTableEngine org unit hierarchy', () => { expect(cellValues(engine)).toEqual(['2', '1']) }) + /* A backend that returns no hierarchy for the requested dimension still + * sends the key, as an empty object, which passes the truthiness guard. */ + it('is inert when ouNameHierarchy is empty', () => { + const dimension = `${STAGE}.ou` + const data = buildData(dimension) + data.metaData.ouNameHierarchy = {} + + const engine = new PivotTableEngine(buildVisualization(dimension), data) + + expect(rowNames(engine)).toEqual(['Bo', 'Bombali']) + expect(rowHierarchies(engine)).toEqual([undefined, undefined]) + expect(cellValues(engine)).toEqual(['1', '2']) + }) + it('leaves non-org-unit dimensions untouched', () => { const dimension = `${STAGE}.de1` const engine = new PivotTableEngine(