fix(configurator): dedupe duplicate options in dropdown/select fields (#1923)

configurator/packages/data-provider/src/providers/dataProvider.test.ts

@@ -785,4 +785,181 @@ describe('createDigitDataProvider', () => {
     assert.deepEqual(descPage2.data.map((r) => r.code), ['CHARLIE', 'BRAVO']);
     assert.equal(descPage2.total, 5);
   });
+
+  // --- CCRS #1923: one record per react-admin id ---------------------------
+  //
+  // The state tenant's records are concatenated with every city tenant's, and
+  // DIGIT does not require a boundary `hierarchyType` or `code` to be unique
+  // across tenants. On bomet (`ke`) that yields SEVEN hierarchies called ADMIN
+  // and CITY_001/WARD_001 defined under two city tenants. Downstream, every
+  // dropdown built from these lists renders one <SelectItem value={id}> per
+  // record — and Radix treats items sharing a value as the same selection, so
+  // the operator saw seven ticked "ADMIN" rows and a trigger reading
+  // "ADMINADMINADMIN…".
+
+  it('getList(boundary-hierarchies) returns one record per hierarchyType across tenants', async () => {
+    mock.method(client, 'mdmsSearch', async (_t: string, schema: string) => {
+      if (schema !== 'tenant.tenants') return [];
+      return ['ke.india', 'ke.mycitynew'].map((code) => ({
+        id: code, tenantId: 'ke', schemaCode: schema, uniqueIdentifier: code,
+        data: { code }, isActive: true,
+      }));
+    });
+    // Every tenant defines its own "ADMIN"; only ke.india adds "KE-ADMIN".
+    mock.method(client, 'boundaryHierarchySearch', async (tenantId: string) => {
+      const types = tenantId === 'ke.india' ? ['ADMIN', 'KE-ADMIN'] : ['ADMIN'];
+      return types.map((hierarchyType) => ({
+        tenantId,
+        hierarchyType,
+        boundaryHierarchy: [{ boundaryType: 'County', parentBoundaryType: null, active: true }],
+      }));
+    });
+
+    const dp = createDigitDataProvider(client, 'ke');
+    const result = await dp.getList('boundary-hierarchies', {
+      pagination: { page: 1, perPage: 100 },
+      sort: { field: 'hierarchyType', order: 'ASC' },
+      filter: {},
+    });
+
+    assert.deepEqual(result.data.map((r) => r.id), ['ADMIN', 'KE-ADMIN']);
+    assert.equal(result.total, 2);
+    // Keep-FIRST: the survivor must be the session tenant's own definition,
+    // not whichever sub-tenant happened to be fetched last.
+    assert.equal(result.data.find((r) => r.id === 'ADMIN')?.tenantId, 'ke');
+  });
+
+  it('getList(boundaries) returns one record per code when two tenants seed the same code', async () => {
+    mock.method(client, 'mdmsSearch', async (_t: string, schema: string) => {
+      if (schema !== 'tenant.tenants') return [];
+      return ['ke.mycitynew', 'ke.hajbvfg'].map((code) => ({
+        id: code, tenantId: 'ke', schemaCode: schema, uniqueIdentifier: code,
+        data: { code }, isActive: true,
+      }));
+    });
+    mock.method(client, 'boundaryHierarchySearch', async () => [{
+      hierarchyType: 'ADMIN',
+      boundaryHierarchy: [{ boundaryType: 'County', parentBoundaryType: null, active: true }],
+    }]);
+    // ke owns BOMET; the two city tenants BOTH seed CITY_001.
+    mock.method(client, 'boundaryRelationshipSearch', async (tenantId: string) => {
+      const code = tenantId === 'ke' ? 'BOMET' : 'CITY_001';
+      return [{
+        tenantId,
+        hierarchyType: 'ADMIN',
+        boundary: [{ code, boundaryType: 'County', name: code, children: [] }],
+      }];
+    });
+
+    const dp = createDigitDataProvider(client, 'ke');
+    const result = await dp.getList('boundaries', {
+      pagination: { page: 1, perPage: 100 },
+      sort: { field: 'code', order: 'ASC' },
+      filter: {},
+    });
+
+    assert.deepEqual(result.data.map((r) => r.id), ['BOMET', 'CITY_001']);
+    assert.equal(result.total, 2);
+    assert.equal(result.data.find((r) => r.id === 'CITY_001')?.tenantId, 'ke.mycitynew');
+  });
+
+  it('getList(access-roles) returns one record per role code', async () => {
+    // egov-accesscontrol merges the tenant's roles with the state tenant's, so
+    // a role defined at both levels comes back twice.
+    mock.method(client, 'accessRolesSearch', async () => [
+      { code: 'HRMS_ADMIN', name: 'HRMS Admin', tenantId: 'ke' },
+      { code: 'HRMS_ADMIN', name: 'HRMS Admin', tenantId: 'ke.bomet' },
+      { code: 'LOC_ADMIN', name: 'Localisation admin', tenantId: 'ke' },
+      { code: 'LOC_ADMIN', name: 'Localisation admin', tenantId: 'ke.bomet' },
+      { code: 'MDMS_ADMIN', name: 'MDMS ADMIN', tenantId: 'ke' },
+    ]);
+
+    const dp = createDigitDataProvider(client, 'ke');
+    const result = await dp.getList('access-roles', {
+      pagination: { page: 1, perPage: 100 },
+      sort: { field: 'name', order: 'ASC' },
+      filter: {},
+    });
+
+    assert.deepEqual(result.data.map((r) => r.id), ['HRMS_ADMIN', 'LOC_ADMIN', 'MDMS_ADMIN']);
+    assert.equal(result.total, 3);
+  });
+
+  it('keeps records whose id extraction failed, under distinct synthetic ids', async () => {
+    // Two records missing the configured idField both normalize to id ''. They
+    // are as broken as a real duplicate — react-admin keys on id — but they are
+    // NOT the same record, so dropping the later one would hide a real row.
+    // Each repeat gets its own id instead.
+    mock.method(client, 'accessRolesSearch', async () => [
+      { name: 'No code at all', tenantId: 'ke' },
+      { name: 'Also no code', tenantId: 'ke' },
+      { code: 'MDMS_ADMIN', name: 'MDMS ADMIN', tenantId: 'ke' },
+    ]);
+
+    const dp = createDigitDataProvider(client, 'ke');
+    const result = await dp.getList('access-roles', {
+      pagination: { page: 1, perPage: 100 },
+      sort: { field: 'name', order: 'ASC' },
+      filter: {},
+    });
+
+    assert.equal(result.data.length, 3, 'no row may be dropped for lacking an id');
+    const ids = result.data.map((r) => String(r.id));
+    assert.equal(new Set(ids).size, 3, 'react-admin needs one id per record');
+    // The names survive intact — only the id was synthesized.
+    assert.deepEqual(
+      result.data.map((r) => r.name).sort(),
+      ['Also no code', 'MDMS ADMIN', 'No code at all'],
+    );
+  });
+
+  it('does not let a synthetic blank id swallow a real record that collides with it', async () => {
+    // A real record whose code happens to equal the synthetic id must survive,
+    // even though it is listed AFTER the blank-id records that generate one.
+    mock.method(client, 'accessRolesSearch', async () => [
+      { name: 'No code at all', tenantId: 'ke' },
+      { name: 'Also no code', tenantId: 'ke' },
+      { code: '#blank-1', name: 'Real role oddly named', tenantId: 'ke' },
+    ]);
+
+    const dp = createDigitDataProvider(client, 'ke');
+    const result = await dp.getList('access-roles', {
+      pagination: { page: 1, perPage: 100 },
+      sort: { field: 'name', order: 'ASC' },
+      filter: {},
+    });
+
+    assert.equal(result.data.length, 3);
+    assert.equal(new Set(result.data.map((r) => String(r.id))).size, 3);
+    assert.ok(
+      result.data.some((r) => r.name === 'Real role oddly named'),
+      'the real record must not be mistaken for a duplicate of a synthetic id',
+    );
+  });
+
+  it('does NOT collapse distinct records that merely share a display name', () => {
+    // The dedupe key is the id, never the label — two boundaries called
+    // "Central" in different counties are two real choices.
+    mock.method(client, 'boundaryHierarchySearch', async () => [{
+      hierarchyType: 'ADMIN',
+      boundaryHierarchy: [{ boundaryType: 'Ward', parentBoundaryType: null, active: true }],
+    }]);
+    mock.method(client, 'boundaryRelationshipSearch', async () => [{
+      tenantId: 'ke',
+      hierarchyType: 'ADMIN',
+      boundary: [
+        { code: 'BOMET_CENTRAL', boundaryType: 'Ward', name: 'Central', children: [] },
+        { code: 'NAIROBI_CENTRAL', boundaryType: 'Ward', name: 'Central', children: [] },
+      ],
+    }]);
+
+    const dp = createDigitDataProvider(client, 'ke.bomet');
+    return dp.getList('boundaries', {
+      pagination: { page: 1, perPage: 100 },
+      sort: { field: 'code', order: 'ASC' },
+      filter: {},
+    }).then((result) => {
+      assert.deepEqual(result.data.map((r) => r.id), ['BOMET_CENTRAL', 'NAIROBI_CENTRAL']);
+    });
+  });
 });

configurator/packages/data-provider/src/providers/dataProvider.ts

@@ -105,6 +105,65 @@ function normalizeRecord(raw: Record<string, unknown>, config: ResourceConfig):
   return { ...raw, id: extractId(raw, config) } as RaRecord;
 }
 
+/**
+ * Collapse records that share a react-admin `id`, keeping the first occurrence.
+ *
+ * react-admin's contract is one record per id: its query cache, Datagrid row
+ * keys and every `<SelectItem value={id}>` built from a list all key on it. Two
+ * records with the same id therefore render as N visually identical rows/options
+ * that ALL resolve to the same record — and in a Radix `Select`, N items sharing
+ * a `value` all show as checked while `<SelectValue>` concatenates every one of
+ * their labels ("ADMINADMINADMIN…"). That is CCRS #1923.
+ *
+ * Duplicates are not hypothetical: the aggregating fetchers below concatenate
+ * results across the state tenant and its city tenants, and DIGIT does NOT
+ * enforce uniqueness of a `hierarchyType` or a boundary `code` across tenants.
+ * On bomet (`ke`) that yields 7 hierarchies called ADMIN, 3 called KE-ADMIN, and
+ * `CITY_001`/`WARD_001` defined under two different city tenants.
+ *
+ * Keep-first is deliberate: every aggregating fetcher lists the SESSION tenant's
+ * records before the sub-tenants', so the survivor is the definition the
+ * operator is actually working in.
+ *
+ * Blank ids are a different failure and get a different remedy. A record whose
+ * `idField` was missing normalizes to `id: ''`, and N such records are exactly
+ * as broken as N sharing a real id. Dropping all but the first would hide rows
+ * that are genuinely distinct — they collide only because id extraction failed,
+ * not because they are the same record. So each repeat is given its own
+ * synthetic id instead, which satisfies react-admin's one-record-per-id
+ * contract without losing anything. This mirrors what the custom-rows fetcher
+ * already does when two Novu integrations synthesize the same id.
+ */
+function dedupeById(records: RaRecord[]): RaRecord[] {
+  const seen = new Set<string>();
+  const out: RaRecord[] = [];
+  // Every id in the input, checked up front so a synthetic id can never collide
+  // with a real one that appears LATER in the list — which would otherwise make
+  // that real record look like a duplicate and drop it.
+  const taken = new Set(records.map((record) => String(record.id ?? '')));
+  let blanks = 0;
+  for (const record of records) {
+    const id = String(record.id ?? '');
+    if (!seen.has(id)) {
+      seen.add(id);
+      out.push(record);
+      continue;
+    }
+    // A repeated real id is a genuine cross-tenant duplicate: keep the first.
+    if (id !== '') continue;
+    // A repeated blank id is a distinct record that lost its id — keep it, under
+    // an id nothing else is using.
+    let synthetic: string;
+    do {
+      blanks += 1;
+      synthetic = `#blank-${blanks}`;
+    } while (taken.has(synthetic) || seen.has(synthetic));
+    seen.add(synthetic);
+    out.push({ ...record, id: synthetic } as RaRecord);
+  }
+  return out;
+}
+
 function normalizeMdmsRecord(mdms: MdmsRecord, config: ResourceConfig): RaRecord {
   let data = mdms.data || {};
   // Legacy ThemeConfig records (v1 nested / v2 semantic shapes) don't carry the
@@ -980,7 +1039,14 @@ export function createDigitDataProvider(client: DigitApiClient, tenantId: string
     return config;
   }
 
+  // Every list-shaped read funnels through here (getList's generic path,
+  // getMany, getManyReference), so this is the one place that can guarantee the
+  // "unique id per record" invariant react-admin depends on — see dedupeById.
   async function fetchAll(resource: string, filter?: Record<string, unknown>): Promise<RaRecord[]> {
+    return dedupeById(await fetchAllRaw(resource, filter));
+  }
+
+  async function fetchAllRaw(resource: string, filter?: Record<string, unknown>): Promise<RaRecord[]> {
     const config = resolveConfig(resource);
     switch (config.type) {
       case 'mdms': return mdmsGetList(client, config, tenantId, filter);
@@ -1133,7 +1199,14 @@ export function createDigitDataProvider(client: DigitApiClient, tenantId: string
           const all = await mdmsSearchAll(client, tenant, config.schema!, { isActive: true });
           // Defensive fallback for any MDMS build that ignores the isActive criterion —
           // degrades to filtering client-side, never worse than the pre-push-down behavior.
-          const active = all.filter((r) => r.isActive).map((r) => normalizeMdmsRecord(r, config));
+          // dedupeById mirrors what fetchAll does for the filtered path below, so
+          // both routes into an MDMS list obey the same one-record-per-id rule.
+          // A no-op for records carrying an MDMS uniqueIdentifier (always unique);
+          // it only bites on legacy rows that fall back to data[idField], which
+          // normalizeMdmsRecord already notes collapse onto one record anyway.
+          const active = dedupeById(
+            all.filter((r) => r.isActive).map((r) => normalizeMdmsRecord(r, config)),
+          );
           const sorted = clientSort(active, field, order);
           const data = clientPaginate(sorted, page, perPage);
           return { data, total: active.length };

configurator/src/admin/DigitFormSelect.tsx

@@ -9,6 +9,7 @@ import {
   SelectItem,
 } from '@/components/ui/select';
 import { Label } from '@/components/ui/label';
+import { uniqueBy } from '@/lib/uniqueBy';
 
 /** Resolve a dot-separated path like 'user.name' from a record */
 function getNestedValue(record: RaRecord, path: string): unknown {
@@ -67,13 +68,20 @@ export function DigitFormSelect({
     { enabled: !!reference },
   );
 
+  // Deduped on `value` — the string this control submits. `optionValue` is the
+  // record's business key (`code` by default), NOT its react-admin id, so two
+  // master records sharing a code produce two SelectItems with the same value:
+  // Radix then marks both checked and <SelectValue> prints both labels
+  // back-to-back. Collapsing here covers every reference-backed dropdown in the
+  // configurator at once, and also guards a caller passing repeated
+  // `staticChoices`. See #1923 and lib/uniqueBy.
   const choices = useMemo(() => {
-    if (staticChoices) return staticChoices;
-    if (!data) return [];
-    return data.map((item) => ({
-      value: String(getNestedValue(item, optionValue) ?? item.id),
-      label: String(getNestedValue(item, optionText) ?? getNestedValue(item, optionValue) ?? item.id),
-    }));
+    const built = staticChoices
+      ?? (data ?? []).map((item) => ({
+        value: String(getNestedValue(item, optionValue) ?? item.id),
+        label: String(getNestedValue(item, optionText) ?? getNestedValue(item, optionValue) ?? item.id),
+      }));
+    return uniqueBy(built, (c) => c.value);
   }, [staticChoices, data, optionValue, optionText]);
 
   const hasError = fieldState.invalid && fieldState.isTouched;

configurator/src/admin/hrms/useRolesLookup.ts

@@ -1,6 +1,7 @@
 import { useMemo } from 'react';
 import { useGetList } from 'ra-core';
 import type { Role } from '@/api/types';
+import { uniqueBy } from '@/lib/uniqueBy';
 
 export interface UseRolesLookupResult {
   roles: Role[];
@@ -15,9 +16,15 @@ export function useRolesLookup(): UseRolesLookupResult {
     sort: { field: 'name', order: 'ASC' },
   });
 
+  // Collapsed on `code`: the roles picker submits codes, everything downstream
+  // (RolesEditor's selected-set, buildRole, the workflow/notification
+  // validators) matches on code, and a role listed twice showed up in the
+  // employee form's combobox as a repeated, unpickable entry (#1923).
   const roles = useMemo<Role[]>(() => {
     if (!data) return [];
-    return data.map((record) => {
+    return uniqueBy(data, (record) =>
+      String((record as Record<string, unknown>).code ?? record.id),
+    ).map((record) => {
       const code = String((record as Record<string, unknown>).code ?? record.id);
       const name = String((record as Record<string, unknown>).name ?? code);
       const descriptionRaw = (record as Record<string, unknown>).description;

configurator/src/components/ui/ReferenceSelect.tsx

@@ -1,4 +1,4 @@
-import { useState, useRef } from 'react';
+import { useState, useRef, useMemo } from 'react';
 import { useGetList } from 'ra-core';
 import {
   Select,
@@ -8,6 +8,7 @@ import {
   SelectValue,
 } from '@/components/ui/select';
 import { Loader2 } from 'lucide-react';
+import { uniqueBy } from '@/lib/uniqueBy';
 
 export interface ReferenceSelectProps {
   /** The react-admin resource to fetch choices from (e.g. 'departments', 'roles') */
@@ -41,6 +42,8 @@ export function ReferenceSelect({
     sort: { field: displayField, order: 'ASC' },
   });
 
+  const options = useMemo(() => uniqueBy(data, (record) => String(record.id)), [data]);
+
   const handleValueChange = async (newValue: string) => {
     if (newValue === value) {
       setOpen(false);
@@ -88,7 +91,7 @@ export function ReferenceSelect({
         <SelectValue placeholder="Select..." />
       </SelectTrigger>
       <SelectContent>
-        {data?.map((record) => {
+        {options.map((record) => {
           const id = String(record.id);
           const label = String((record as Record<string, unknown>)[displayField] ?? id);
           return (