fix: handle options with non-unique codes across optionsets (DHIS2-15771)

cypress/helpers/transfer.js

@@ -17,6 +17,10 @@ export const searchAndSelectInOptionsTransfer = (name) => {
         .get('[data-test="dhis2-uicore-circularloader"]', EXTENDED_TIMEOUT)
         .should('not.exist')
 
+    selectInOptionsTransfer(name)
+}
+
+export const selectInOptionsTransfer = (name) => {
     cy.getBySel('option-set-transfer-sourceoptions')
         .containsExact(name)
         .dblclick()

cypress/integration/conditions/optionSetCondition.cy.js

@@ -5,6 +5,7 @@ import {
     TEST_DIM_TEXT_OPTIONSET,
     TEST_REL_PE_LAST_YEAR,
 } from '../../data/index.js'
+import { goToAO } from '../../helpers/common.js'
 import {
     openDimension,
     openProgramDimensionsSidebar,
@@ -26,7 +27,10 @@ import {
     expectTableToMatchRows,
     expectTableToNotContainValue,
 } from '../../helpers/table.js'
-import { searchAndSelectInOptionsTransfer } from '../../helpers/transfer.js'
+import {
+    searchAndSelectInOptionsTransfer,
+    selectInOptionsTransfer,
+} from '../../helpers/transfer.js'
 
 describe('Option set condition', () => {
     it('Option set (number) displays correctly', () => {
@@ -135,4 +139,39 @@ describe('Option set condition', () => {
 
         expectTableToMatchRows([`${getPreviousYearStr()}-12-10`])
     })
+
+    it('Options with same code but from different option sets display correctly', () => {
+        const testData = [
+            {
+                dimensionName: 'WHOMCH Pain medication given',
+                filteredOptionNames: ['Morphine', 'Spinal'],
+            },
+            {
+                dimensionName: 'WHOMCH Clinical impression of pre-eclampsia',
+                filteredOptionNames: [
+                    'None',
+                    'Pre-eclampsia',
+                    'Severe pre-eclampsia',
+                ],
+            },
+        ]
+
+        goToAO('C1XaMuNaeDy')
+
+        expectTableToBeVisible()
+
+        testData.forEach(({ dimensionName, filteredOptionNames }) => {
+            cy.getBySelLike('layout-chip').contains(dimensionName).click()
+
+            filteredOptionNames.forEach(selectInOptionsTransfer)
+
+            cy.getBySel('conditions-modal').contains('Update').click()
+
+            assertChipContainsText(
+                `${dimensionName}: ${filteredOptionNames.length} selected`
+            )
+
+            assertTooltipContainsEntries(filteredOptionNames)
+        })
+    })
 })

cypress/integration/table.cy.js

@@ -32,6 +32,7 @@ import {
     TEST_DIM_TEXT_OPTIONSET,
     TEST_REL_PE_THIS_YEAR,
 } from '../data/index.js'
+import { goToAO } from '../helpers/common.js'
 import {
     clickAddRemoveMainDimension,
     clickAddRemoveProgramDataDimension,
@@ -236,6 +237,33 @@ const assertDimensions = () => {
     }
 }
 
+const assertOptionSetOptionLabels = () => {
+    goToAO('C1XaMuNaeDy')
+
+    expectTableToBeVisible()
+
+    getTableRows()
+        .eq(0)
+        .find('td')
+        .eq(3)
+        .invoke('text')
+        .then(($cell3Value) => expect($cell3Value).to.equal('Pre-eclampsia'))
+
+    getTableRows()
+        .eq(0)
+        .find('td')
+        .eq(4)
+        .invoke('text')
+        .then(($cell4Value) => expect($cell4Value).to.equal('Suspected'))
+
+    getTableRows()
+        .eq(0)
+        .find('td')
+        .eq(5)
+        .invoke('text')
+        .then(($cell5Value) => expect($cell5Value).to.equal('Morphine'))
+}
+
 const assertSorting = () => {
     // remove any DGS to allow numeric value comparison
     openStyleOptionsModal()
@@ -353,6 +381,9 @@ describe(['>=38', '<39'], 'table', () => {
     it('data can be sorted', () => {
         assertSorting()
     })
+    it('option set option labels show correctly', () => {
+        assertOptionSetOptionLabels()
+    })
 })
 
 describe(['>=39'], 'table', () => {
@@ -375,4 +406,7 @@ describe(['>=39'], 'table', () => {
     it('data can be sorted (>=2.39)', () => {
         assertSorting()
     })
+    it('option set option labels show correctly (>=2.39)', () => {
+        assertOptionSetOptionLabels()
+    })
 })

src/components/App.js

@@ -19,6 +19,7 @@
     acSetShowExpandedLayoutPanel,
 } from '../actions/ui.js'
 import { acSetVisualization } from '../actions/visualization.js'
+import { parseCondition, OPERATOR_IN } from '../modules/conditions.js'
 import { EVENT_TYPE } from '../modules/dataStatistics.js'
 import {
     dataAccessError,
@@ -45,6 +46,7 @@
     sGetIsVisualizationLoading,
     sGetLoadError,
 } from '../reducers/loader.js'
+import { sGetMetadata } from '../reducers/metadata.js'
 import { sGetUiShowDetailsPanel } from '../reducers/ui.js'
 import classes from './App.module.css'
 import { default as DetailsPanel } from './DetailsPanel/DetailsPanel.js'
@@ -114,6 +116,19 @@
     type: 'create',
 }
 
+const optionsQuery = {
+    options: {
+        resource: 'options',
+        params: ({ optionSetId, codes }) => ({
+            fields: 'id,code,displayName~rename(name)',
+            filter: [
+                `optionSet.id:eq:${optionSetId}`,
+                `code:in:[${codes.join(',')}]`,
+            ],
+            paging: false,
+        }),
+    },
+}
 const App = () => {
     const dataEngine = useDataEngine()
     const [aboutAOUnitRenderId, setAboutAOUnitRenderId] = useState(1)
@@ -124,6 +139,7 @@
     const [initialLoadIsComplete, setInitialLoadIsComplete] = useState(false)
     const [postDataStatistics] = useDataMutation(dataStatisticsMutation)
     const dispatch = useDispatch()
+    const metadata = useSelector(sGetMetadata)
     const current = useSelector(sGetCurrent)
     const isLoading = useSelector(sGetIsVisualizationLoading)
     const error = useSelector(sGetLoadError)
@@ -240,7 +256,7 @@
         dispatch(acSetUiOpenDimensionModal(dimensionId))
 
     const onResponsesReceived = (response) => {
-        const itemsMetadata = Object.entries(response.metaData.items).reduce(
+        /*const itemsMetadata = Object.entries(response.metaData.items).reduce(
             (obj, [id, item]) => {
                 obj[id] = {
                     id,
@@ -253,9 +269,9 @@
                 return obj
             },
             {}
-        )
+        )*/
 
-        dispatch(acAddMetadata(itemsMetadata))
+        //dispatch(acAddMetadata(itemsMetadata))
         dispatch(acSetVisualizationLoading(false))
 
         if (!response.rows?.length) {
@@ -286,8 +302,45 @@
         })
 
         return () => unlisten && unlisten()
     }, [])
 
+    const addOptionSetsMetadata = async (visualization) => {
+        const optionSetsMetadata = {}
+
+        const dimensions = [
+            ...(visualization.columns || []),
+            ...(visualization.rows || []),
+            ...(visualization.filters || []),
+        ]
+
+        for (const dimension of dimensions) {
+            if (
+                dimension?.optionSet?.id &&
+                dimension.filter?.startsWith(OPERATOR_IN)
+            ) {
+                const optionSetId = dimension.optionSet.id
+
+                const data = await dataEngine.query(optionsQuery, {
+                    variables: {
+                        optionSetId,
+                        codes: parseCondition(dimension.filter),
+                    },
+                })
+
+                if (data?.options) {
+                    // update options in the optionSet metadata used for the lookup of the correct
+                    // name from code (options for different option sets have the same code)
+                    optionSetsMetadata[optionSetId] = {
+                        ...metadata[optionSetId],
+                        options: data.options.options,
+                    }
+                }
+            }
+        }
+
+        dispatch(acAddMetadata(optionSetsMetadata))
+    }
+
     useEffect(() => {
         if (data?.eventVisualization) {
             dispatch(acClearLoadError())
@@ -297,6 +350,8 @@
                 data.eventVisualization
             )
 
+            addOptionSetsMetadata(visualization)
+
             dispatch(
                 acAddParentGraphMap(
                     getParentGraphMapFromVisualization(visualization)
@@ -308,7 +363,7 @@
             dispatch(tSetCurrentFromUi({ validateOnly: true }))
             postDataStatistics({ id: visualization.id })
         }
     }, [data])
 
     return (
         <div

src/components/Dialogs/Conditions/OptionSetCondition.js

@@ -36,15 +36,16 @@ const OptionSetCondition = ({
     const dataTest = 'option-set'
 
     const setValues = (selected) => {
-        addMetadata(
-            state.options
-                .filter(
-                    (item) =>
-                        selected.includes(item.code) &&
-                        !selectedOptions.find((so) => so.code === item.code)
-                )
-                .reduce((acc, item) => ({ ...acc, [item.id]: item }), {})
-        )
+        addMetadata({
+            // update options in the optionSet metadata used for the lookup of the correct
+            // name from code (options for different option sets have the same code)
+            [optionSetId]: {
+                ...metadata[optionSetId],
+                options: selected.map((soCode) =>
+                    state.options.find(({ code }) => code === soCode)
+                ),
+            },
+        })
 
         onChange(`${OPERATOR_IN}:${selected.join(';') || ''}`)
     }

src/components/DownloadMenu/useDownload.js

@@ -86,13 +86,21 @@ const useDownload = (relativePeriodDate) => {
                         .withOutputType(current.outputType)
                         .withPath(path)
                         .withFormat(format)
-                        .withOutputIdScheme(idScheme)
                         .withParameters({
                             ...parameters,
                             headers,
                             paging: false,
                         })
 
+                    // fix option set option names
+                    if (idScheme === ID_SCHEME_NAME) {
+                        req = req.withParameters({
+                            dataIdScheme: idScheme,
+                        })
+                    } else {
+                        req = req.withOutputIdScheme(idScheme)
+                    }
+
                     // TODO options
                     // startDate
                     // endDate

src/components/Visualization/useAnalyticsData.js

@@ -44,8 +44,19 @@ const excludedDimensions = [
     DIMENSION_ID_LAST_UPDATED_BY,
 ]
 
-const findOptionSetItem = (code, metaDataItems) =>
-    Object.values(metaDataItems).find((item) => item.code === code)
+const lookupOptionSetOptionMetadata = (optionSetId, code, metaDataItems) => {
+    const optionSetMetaData = metaDataItems?.[optionSetId]
+
+    if (optionSetMetaData) {
+        const optionId = optionSetMetaData.options.find(
+            (option) => option.code === code
+        )?.uid
+
+        return metaDataItems[optionId]
+    }
+
+    return undefined
+}
 
 const formatRowValue = (rowValue, header, metaDataItems) => {
     switch (header.valueType) {
@@ -56,11 +67,17 @@ const formatRowValue = (rowValue, header, metaDataItems) => {
             if (!rowValue) {
                 return rowValue
             }
+
             if (header.optionSet) {
                 return (
-                    findOptionSetItem(rowValue, metaDataItems)?.name || rowValue
+                    lookupOptionSetOptionMetadata(
+                        header.optionSet,
+                        rowValue,
+                        metaDataItems
+                    )?.name || rowValue
                 )
             }
+
             return metaDataItems[rowValue]?.name || rowValue
         }
     }

src/modules/__tests__/conditions.spec.js

@@ -45,9 +45,14 @@ test('Dimension with optionSet', () => {
         condition: 'IN:5code;6code',
     }
     const metadata = {
-        '5Id': { code: '5code', name: '5' },
-        '6Id': { code: '6code', name: '6' },
+        optionsetId: {
+            options: [
+                { code: '5code', name: '5' },
+                { code: '6code', name: '6' },
+            ],
+        },
     }
+
     const dimension = {
         optionSet: 'optionsetId',
         valueType: 'NUMBER',

src/modules/conditions.js

@@ -32,6 +32,9 @@ export const parseConditionsStringToArray = (conditionsString) =>
 export const parseConditionsArrayToString = (conditionsArray) =>
     conditionsArray.join(':')
 
+export const parseCondition = (conditionItem) =>
+    conditionItem.split(':').pop().split(';')
+
 export const NULL_VALUE = 'NV'
 export const TRUE_VALUE = '1'
 export const FALSE_VALUE = '0'
@@ -167,8 +170,13 @@ const getOperatorsByValueType = (valueType) => {
     }
 }
 
-const parseCondition = (conditionItem) =>
-    conditionItem.split(':').pop().split(';')
+const lookupOptionSetOptionMetadata = (optionSetId, code, metaData) => {
+    const optionSetMetaData = metaData?.[optionSetId]
+
+    return optionSetMetaData
+        ? optionSetMetaData.options?.find((option) => option.code === code)
+        : undefined
+}
 
 export const getConditionsTexts = ({
     conditions = {},
@@ -194,9 +202,14 @@ export const getConditionsTexts = ({
 
     if (dimension.optionSet && conditionsList[0]?.startsWith(OPERATOR_IN)) {
         const items = parseCondition(conditionsList[0])
+
         const itemNames = items.map(
             (code) =>
-                Object.values(metadata).find((item) => item.code === code)?.name
+                lookupOptionSetOptionMetadata(
+                    dimension.optionSet,
+                    code,
+                    metadata
+                )?.name
         )
         return itemNames
     }