diff --git a/.gitignore b/.gitignore index f861804cf..674eed813 100644 --- a/.gitignore +++ b/.gitignore @@ -45,6 +45,9 @@ cypress/fixtures/ .idea/* .vscode/* +# Claude Code +.claude/ + # Configuration /**/app-config.json diff --git a/openspec/changes/fix-category-option-mapping/.openspec.yaml b/openspec/changes/fix-category-option-mapping/.openspec.yaml new file mode 100644 index 000000000..5aae5cfa4 --- /dev/null +++ b/openspec/changes/fix-category-option-mapping/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-03-04 diff --git a/openspec/changes/fix-category-option-mapping/design.md b/openspec/changes/fix-category-option-mapping/design.md new file mode 100644 index 000000000..b46de1786 --- /dev/null +++ b/openspec/changes/fix-category-option-mapping/design.md @@ -0,0 +1,57 @@ +## Context + +Category options in DHIS2 can belong to multiple categories. The mapping system uses a composite ID format `"{categoryId}-{categoryOptionId}"` to disambiguate which category context a mapping applies in. This composite format is used in nested mapping (within a category combo) by `CategoryOptionMappedModel.modelTransform`. + +However, in **global mapping**, category options should be listed once each with their plain DHIS2 ID — they're mapped globally regardless of which category they belong to. The current code has three intertwined problems: + +1. **`MappingDialog` always uses `modelFactory("categoryOptions")`** which resolves to `CategoryOptionMappedModel` (the first class with `mappingType = "categoryOptions"`). This model's `modelTransform` expands each option into N rows by category → duplicates. + +2. **ID format mismatch on read-back**: When a user selects a destination item, the composite ID (e.g. `"catA-opt1"`) is stored as `mappedId`. When re-opening, `MappingDialog` strips it to plain ID via `_.last(split("-"))` → `"opt1"`. But the table lists composite IDs, so `"opt1"` matches nothing → selection appears lost. + +3. **Automap stores plain IDs**: `lookupSimilar()` returns plain DHIS2 IDs. With `initialShowOnlySelected=true` and composite IDs in the table, the result is invisible. + +The backend consumer (`mapCategoryOptionCombo` in `src/utils/synchronization.ts`) already handles both formats using `_.last(candidate.split("-"))`, so fixing the UI won't break sync operations. + +## Goals / Non-Goals + +**Goals:** +- Each category option appears exactly once in the global mapping destination picker +- Selected mappings are visually persistent when re-opening the dialog +- Automap results are visible in the selection window +- Nested (non-global) category option mapping continues to work with composite IDs + +**Non-Goals:** +- Changing the mapping storage format in the dataStore +- Modifying the backend sync consumer (`mapCategoryOptionCombo`) +- Fixing mapping for other metadata types + +## Decisions + +### 1. Use plain `CategoryOptionModel` for the global mapping destination picker + +**Change:** In `MappingDialog`, when the mapping context is global category options, use a model that does NOT have the category-exploding `modelTransform`. This can be done by passing the appropriate model from the `MappingTable` (which already knows whether it's global or nested) instead of re-resolving via `modelFactory`. + +**Rationale:** The `MappingTable` already holds the model class for the source rows. For global mapping, this is `GlobalCategoryOptionModel` (no transform). The destination picker should use the same kind of model — plain category options without category expansion. + +**Approach:** Add the model (or mappingType context) to `MappingDialogConfig` so `MappingDialog` can use it directly instead of calling `modelFactory(mappingType)`. When a global model is provided, the dialog uses it; otherwise it falls back to the current `modelFactory` behavior for backward compatibility with nested mapping. + +**Alternatives considered:** +- Creating a separate `GlobalCategoryOptionDestinationModel` — too many model classes already, adds complexity. +- Modifying `modelFactory` to accept a "global" flag — leaks UI concerns into the factory. + +### 2. Store and read back plain IDs consistently for global category options + +**Change:** Since the destination picker will now list plain IDs (no composite), the `mappedId` stored will also be plain. The `mappedId` read-back logic in `MappingDialog` (lines 56-64) using `_.last(split("-"))` will naturally produce the correct plain ID. No change needed to the read-back logic — it already handles plain IDs correctly (`_.last("opt1".split("-"))` = `"opt1"`). + +**Rationale:** With plain IDs in both storage and display, the selection will match correctly when the dialog is re-opened. + +### 3. No change needed for automap + +**Change:** None. Automap already stores plain IDs. Once the destination picker uses plain IDs too, the automap result will match rows correctly. + +**Rationale:** The automap bug is a symptom of the same root cause (composite vs plain ID mismatch in the destination table). Fixing decision #1 resolves it automatically. + +## Risks / Trade-offs + +- **[Low] Existing global mappings with composite `mappedId` values** → The `_.last(split("-"))` read-back logic already strips these to plain IDs, so existing stored mappings will display correctly. New mappings will be stored with plain IDs. The backend consumer handles both formats. +- **[Low] Nested (non-global) category option mapping** → Unchanged. The `MappingDialog` will only use the plain model when a global-context model is explicitly provided; nested mapping continues using `CategoryOptionMappedModel` via `modelFactory`. diff --git a/openspec/changes/fix-category-option-mapping/proposal.md b/openspec/changes/fix-category-option-mapping/proposal.md new file mode 100644 index 000000000..23ac8570b --- /dev/null +++ b/openspec/changes/fix-category-option-mapping/proposal.md @@ -0,0 +1,25 @@ +## Why + +Category option mapping in Global Mapping is broken in three ways: the destination picker shows duplicate entries (one per category a category option belongs to), selected mappings appear lost when re-opening the dialog, and automap results are invisible in the selection window. All three stem from an inconsistency between how category option IDs are stored (composite `categoryId-optionId` vs plain `optionId`) and how they are read back in the UI. + +## What Changes + +- **Fix duplicate destination entries**: The `MappingDialog` uses `modelFactory("categoryOptions")` which resolves to `CategoryOptionMappedModel` — a model with a `modelTransform` that explodes each category option into N rows (one per parent category). For the destination picker in global mapping, use plain `CategoryOptionModel` instead to list each category option once. +- **Fix mapping not persisting visually**: `MappingDialog` stores composite IDs as `mappedId` but strips them back to plain IDs with `_.last(split("-"))` when reading, causing a mismatch with the composite row IDs in the table. Ensure consistent ID handling so the previously selected item is correctly highlighted. +- **Fix automap selection invisible**: `AutoMapUseCase` stores plain destination IDs, but the destination table lists composite IDs. The `initialShowOnlySelected` filter then shows zero rows. Align the ID format used by automap with what the table displays. + +## Capabilities + +### New Capabilities + +_(none — this is a bug fix)_ + +### Modified Capabilities + +_(no spec-level requirement changes — the intended mapping behavior was correct by design, only the ID handling is broken)_ + +## Impact + +- **Code**: `src/presentation/react/core/components/mapping-dialog/MappingDialog.tsx` (ID consistency), `src/models/dhis/mapping.ts` (`GlobalCategoryOptionModel`), `src/models/dhis/factory.ts` (model resolution for global mapping context) +- **User impact**: Global category option mapping will show each option once, persist selections visually, and display automap results correctly +- **Risk**: Low — changes are scoped to the mapping UI; the backend mapping consumer (`mapCategoryOptionCombo` in `synchronization.ts`) already handles both composite and plain keys via `_.last(split("-"))` diff --git a/openspec/changes/fix-category-option-mapping/specs/category-option-mapping/spec.md b/openspec/changes/fix-category-option-mapping/specs/category-option-mapping/spec.md new file mode 100644 index 000000000..7b03e44e7 --- /dev/null +++ b/openspec/changes/fix-category-option-mapping/specs/category-option-mapping/spec.md @@ -0,0 +1,44 @@ +## ADDED Requirements + +### Requirement: Global mapping destination picker SHALL list each category option exactly once + +When the user opens the mapping dialog for a category option in the global mapping context, the destination picker MUST display each category option from the destination instance exactly once, using its plain DHIS2 ID. + +#### Scenario: Category option belonging to multiple categories appears once +- **WHEN** a category option exists in the destination instance and belongs to 3 different categories +- **THEN** the destination picker SHALL display that category option exactly once, not 3 times + +#### Scenario: Destination picker uses plain IDs for global mapping +- **WHEN** the mapping dialog opens for a global category option mapping +- **THEN** each row in the destination picker SHALL use the plain category option ID (not the composite `categoryId-optionId` format) + +### Requirement: Selected global category option mapping SHALL be visible when re-opening the dialog + +When the user has selected a destination category option in global mapping, re-opening the mapping dialog for the same source element MUST show the previously selected destination item as selected. + +#### Scenario: Previously mapped category option is highlighted on dialog re-open +- **WHEN** the user maps source category option A to destination category option B and closes the dialog +- **AND** the user re-opens the mapping dialog for source category option A +- **THEN** destination category option B SHALL be shown as selected in the picker + +#### Scenario: "Show only selected" filter displays the mapped item +- **WHEN** the user re-opens a mapping dialog for a category option that was previously mapped +- **AND** the dialog opens with the "show only selected" filter active +- **THEN** the previously mapped destination category option SHALL appear in the filtered list + +### Requirement: Automap results for category options SHALL be visible in the mapping dialog + +When automap has assigned a destination category option, opening the mapping dialog MUST show that automapped selection. + +#### Scenario: Automapped category option is visible in the dialog +- **WHEN** automap has matched source category option A to destination category option B +- **AND** the user opens the mapping dialog for source category option A +- **THEN** destination category option B SHALL be shown as selected in the picker + +### Requirement: Nested (non-global) category option mapping SHALL continue using composite IDs + +The composite `categoryId-optionId` format used in nested category option mapping (within category combos) MUST remain unchanged. + +#### Scenario: Nested mapping still uses composite IDs +- **WHEN** the user maps a category option within a category combo mapping context (non-global) +- **THEN** the destination picker SHALL use composite `categoryId-optionId` IDs as before diff --git a/openspec/changes/fix-category-option-mapping/tasks.md b/openspec/changes/fix-category-option-mapping/tasks.md new file mode 100644 index 000000000..4b3b360ad --- /dev/null +++ b/openspec/changes/fix-category-option-mapping/tasks.md @@ -0,0 +1,19 @@ +## 1. Pass model from MappingTable to MappingDialog + +- [ ] 1.1 [FE] Add a `model` field to `MappingDialogConfig` in `MappingDialog.tsx` so the caller can specify which model the destination picker should use +- [ ] 1.2 [FE] In `MappingDialog`, use the provided `model` from config when available, falling back to `modelFactory(mappingType)` for backward compatibility +- [ ] 1.3 [FE] In `MappingTable.tsx`, pass the current source model (which is `GlobalCategoryOptionModel` for global mapping) when opening the `MappingDialog` + +## 2. Verify ID consistency + +- [ ] 2.1 [FE] Verify that with the plain model in the destination picker, stored `mappedId` values are plain IDs, and `_.last(split("-"))` read-back produces matching values +- [ ] 2.2 [FE] Verify that nested (non-global) category option mapping still works by confirming the fallback to `modelFactory` produces `CategoryOptionMappedModel` with composite IDs + +## 3. Unit Tests + +- [ ] 3.1 [FE] Add a test verifying `MappingDialog` uses the provided model instead of calling `modelFactory` when a model is passed in the config +- [ ] 3.2 [FE] Run `yarn test` to confirm all existing tests pass + +## 4. Validation + +- [ ] 4.1 [FE] Run `yarn test` and `yarn build core-app` to confirm everything compiles and passes diff --git a/src/presentation/react/core/components/mapping-dialog/MappingDialog.tsx b/src/presentation/react/core/components/mapping-dialog/MappingDialog.tsx index d395b978a..c2a793b37 100644 --- a/src/presentation/react/core/components/mapping-dialog/MappingDialog.tsx +++ b/src/presentation/react/core/components/mapping-dialog/MappingDialog.tsx @@ -7,6 +7,7 @@ import React, { useEffect, useState, useMemo, useCallback } from "react"; import { DataSource, isDhisInstance } from "../../../../../domain/instance/entities/DataSource"; import { MetadataMappingDictionary } from "../../../../../domain/mapping/entities/MetadataMapping"; import i18n from "../../../../../utils/i18n"; +import { D2Model } from "../../../../../models/dhis/default"; import { modelFactory } from "../../../../../models/dhis/factory"; import { MetadataType } from "../../../../../utils/d2"; import { useAppContext } from "../../contexts/AppContext"; @@ -19,6 +20,7 @@ export interface MappingDialogConfig { mappingType?: string; mappingPath?: string[]; firstElement?: MetadataType; + model?: typeof D2Model; } export interface MappingDialogProps { @@ -69,7 +71,7 @@ const MappingDialog: React.FC = ({ const [selected, updateSelected] = useState(defaultSelection); - const model = useMemo(() => modelFactory(mappingType), [mappingType]); + const model = useMemo(() => config.model ?? modelFactory(mappingType), [config.model, mappingType]); const modelName = useMemo(() => model.getModelName(), [model]); const api = useMemo(() => { return isDhisInstance(instance) ? compositionRoot.instances.getApi(instance) : defaultApi; diff --git a/src/presentation/react/core/components/mapping-table/MappingTable.tsx b/src/presentation/react/core/components/mapping-table/MappingTable.tsx index 9d60ae872..8d624fcdf 100644 --- a/src/presentation/react/core/components/mapping-table/MappingTable.tsx +++ b/src/presentation/react/core/components/mapping-table/MappingTable.tsx @@ -292,6 +292,7 @@ export default function MappingTable({ mappingPath, mappingType, firstElement, + model: firstElement.model, }); } } @@ -326,7 +327,13 @@ export default function MappingTable({ .value(); if (types.length === 1) { - setMappingConfig({ elements, mappingPath, mappingType: types[0], firstElement }); + setMappingConfig({ + elements, + mappingPath, + mappingType: types[0], + firstElement, + model: firstElement?.model, + }); setSelectedIds([]); } else if (types.length > 1) { snackbar.error(i18n.t("You need to select all items from the same type"));