Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ cypress/fixtures/
.idea/*
.vscode/*

# Claude Code
.claude/

# Configuration
/**/app-config.json

Expand Down
2 changes: 2 additions & 0 deletions openspec/changes/fix-category-option-mapping/.openspec.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-03-04
57 changes: 57 additions & 0 deletions openspec/changes/fix-category-option-mapping/design.md
Original file line number Diff line number Diff line change
@@ -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`.
25 changes: 25 additions & 0 deletions openspec/changes/fix-category-option-mapping/proposal.md
Original file line number Diff line number Diff line change
@@ -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("-"))`
Original file line number Diff line number Diff line change
@@ -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
19 changes: 19 additions & 0 deletions openspec/changes/fix-category-option-mapping/tasks.md
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -19,6 +20,7 @@ export interface MappingDialogConfig {
mappingType?: string;
mappingPath?: string[];
firstElement?: MetadataType;
model?: typeof D2Model;
}

export interface MappingDialogProps {
Expand Down Expand Up @@ -69,7 +71,7 @@ const MappingDialog: React.FC<MappingDialogProps> = ({

const [selected, updateSelected] = useState<string | undefined>(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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ export default function MappingTable({
mappingPath,
mappingType,
firstElement,
model: firstElement.model,
});
}
}
Expand Down Expand Up @@ -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"));
Expand Down
Loading