diff --git a/configurator/packages/data-provider/src/providers/dataProvider.test.ts b/configurator/packages/data-provider/src/providers/dataProvider.test.ts index f4774be5b4..3dae908d49 100644 --- a/configurator/packages/data-provider/src/providers/dataProvider.test.ts +++ b/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 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']); + }); + }); }); diff --git a/configurator/packages/data-provider/src/providers/dataProvider.ts b/configurator/packages/data-provider/src/providers/dataProvider.ts index 5e6a84df20..43d786a529 100644 --- a/configurator/packages/data-provider/src/providers/dataProvider.ts +++ b/configurator/packages/data-provider/src/providers/dataProvider.ts @@ -105,6 +105,65 @@ function normalizeRecord(raw: Record, 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 `` 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 `` 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(); + 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): Promise { + return dedupeById(await fetchAllRaw(resource, filter)); + } + + async function fetchAllRaw(resource: string, filter?: Record): Promise { 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 }; diff --git a/configurator/src/admin/DigitFormSelect.tsx b/configurator/src/admin/DigitFormSelect.tsx index 856bbcea26..4ea7015ef5 100644 --- a/configurator/src/admin/DigitFormSelect.tsx +++ b/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 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; diff --git a/configurator/src/admin/hrms/useRolesLookup.ts b/configurator/src/admin/hrms/useRolesLookup.ts index 0a36ea48ad..114f4ee52b 100644 --- a/configurator/src/admin/hrms/useRolesLookup.ts +++ b/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(() => { if (!data) return []; - return data.map((record) => { + return uniqueBy(data, (record) => + String((record as Record).code ?? record.id), + ).map((record) => { const code = String((record as Record).code ?? record.id); const name = String((record as Record).name ?? code); const descriptionRaw = (record as Record).description; diff --git a/configurator/src/components/ui/ReferenceSelect.tsx b/configurator/src/components/ui/ReferenceSelect.tsx index 2f22a65ee2..2afa48abdc 100644 --- a/configurator/src/components/ui/ReferenceSelect.tsx +++ b/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({ - {data?.map((record) => { + {options.map((record) => { const id = String(record.id); const label = String((record as Record)[displayField] ?? id); return ( diff --git a/configurator/src/lib/uniqueBy.test.ts b/configurator/src/lib/uniqueBy.test.ts new file mode 100644 index 0000000000..4d94780fb9 --- /dev/null +++ b/configurator/src/lib/uniqueBy.test.ts @@ -0,0 +1,42 @@ +import { describe, it, expect } from 'vitest'; +import { uniqueBy } from './uniqueBy'; + +describe('uniqueBy', () => { + it('keeps the FIRST item for a repeated key', () => { + // Keep-first matters: aggregating fetchers list the session tenant's + // records before its sub-tenants', so the survivor must be the session + // tenant's definition. + const hierarchies = [ + { hierarchyType: 'ADMIN', tenantId: 'ke' }, + { hierarchyType: 'ADMIN', tenantId: 'ke.india' }, + { hierarchyType: 'ADMIN', tenantId: 'ke.etoebeta' }, + ]; + expect(uniqueBy(hierarchies, (h) => h.hierarchyType)).toEqual([ + { hierarchyType: 'ADMIN', tenantId: 'ke' }, + ]); + }); + + it('preserves order and leaves distinct keys untouched', () => { + const codes = ['ADMIN', 'INDIA', 'ADMIN', 'KE-ADMIN', 'INDIA', 'KE-ADMIN', 'ADMIN']; + expect(uniqueBy(codes, (c) => c)).toEqual(['ADMIN', 'INDIA', 'KE-ADMIN']); + }); + + it('does not collapse distinct codes that share a display name', () => { + // Two boundaries may legitimately be called "Central" in different + // counties — only the submitted value may ever be collapsed. + const boundaries = [ + { code: 'BOMET_CENTRAL', name: 'Central' }, + { code: 'NAIROBI_CENTRAL', name: 'Central' }, + ]; + expect(uniqueBy(boundaries, (b) => b.code)).toHaveLength(2); + }); + + it('treats null/undefined input as an empty list', () => { + expect(uniqueBy(undefined, String)).toEqual([]); + expect(uniqueBy(null, String)).toEqual([]); + }); + + it('collapses empty-string keys too — a select cannot tell them apart', () => { + expect(uniqueBy([{ code: '' }, { code: '' }], (x) => x.code)).toHaveLength(1); + }); +}); diff --git a/configurator/src/lib/uniqueBy.ts b/configurator/src/lib/uniqueBy.ts new file mode 100644 index 0000000000..e6b040b3fc --- /dev/null +++ b/configurator/src/lib/uniqueBy.ts @@ -0,0 +1,37 @@ +/** + * Keep the first item for each distinct key, dropping later collisions. + * + * Dropdowns are the reason this exists. A Radix `Select` renders one + * `` per choice, and two items sharing a `value` are not + * merely a cosmetic repeat: BOTH render as checked when either is picked, and + * `` prints the label of every match, so a trigger showing the + * selected hierarchy reads "ADMINADMINADMINADMIN…". React also warns on the + * duplicated list key. Same story for the hand-rolled `role="listbox"` + * comboboxes (Roles, Departments): a repeated option is unpickable-past-the- + * first and looks like corrupt master data to the operator. See CCRS #1923. + * + * The duplicates are real, not defensive: DIGIT does not enforce uniqueness of a + * boundary `hierarchyType` or `code` across tenants, and the data provider + * aggregates the state tenant's records with every city tenant's. On bomet + * (`ke`) that is 7 hierarchies named ADMIN and 3 named KE-ADMIN. + * + * Keep-first, never last: aggregating fetchers list the session tenant's records + * before the sub-tenants', so the surviving option is the one the operator's own + * tenant defines. + * + * Only ever collapse on the value the control SUBMITS (the option value / code), + * never on the display label — two genuinely different codes are allowed to + * share a name, and dropping one of those would hide a real choice. + */ +export function uniqueBy(items: readonly T[] | null | undefined, key: (item: T) => string): T[] { + if (!items) return []; + const seen = new Set(); + const out: T[] = []; + for (const item of items) { + const k = key(item); + if (seen.has(k)) continue; + seen.add(k); + out.push(item); + } + return out; +} diff --git a/configurator/src/pages/org-chart/OrgChartPage.tsx b/configurator/src/pages/org-chart/OrgChartPage.tsx index fb94101d1f..9fa2db9c30 100644 --- a/configurator/src/pages/org-chart/OrgChartPage.tsx +++ b/configurator/src/pages/org-chart/OrgChartPage.tsx @@ -7,6 +7,7 @@ import { useGetList } from 'ra-core'; import { useNavigate } from 'react-router-dom'; import { Loader2, Maximize, Minimize } from 'lucide-react'; import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/components/ui/select'; +import { uniqueBy } from '@/lib/uniqueBy'; import { Input } from '@/components/ui/input'; import { Card } from '@/components/ui/card'; import { useOrgChartData } from './useOrgChartData'; @@ -98,7 +99,7 @@ export default function OrgChartPage() { - {(tenants ?? []).filter((t) => t.code).map((t) => ( + {uniqueBy((tenants ?? []).filter((t) => t.code), (t) => t.code).map((t) => ( {t.name ?? t.code} ))} diff --git a/configurator/src/resources/complaints/ComplaintHierarchyCascade.tsx b/configurator/src/resources/complaints/ComplaintHierarchyCascade.tsx index 6b974f11f3..f70bc305ac 100644 --- a/configurator/src/resources/complaints/ComplaintHierarchyCascade.tsx +++ b/configurator/src/resources/complaints/ComplaintHierarchyCascade.tsx @@ -11,6 +11,7 @@ import { SelectItem, } from '@/components/ui/select'; import { Label } from '@/components/ui/label'; +import { uniqueBy } from '@/lib/uniqueBy'; // One RAINMAKER-PGR.ComplaintHierarchy row (interior node OR leaf complaint type). interface HierNode { @@ -119,10 +120,16 @@ export function ComplaintHierarchyCascade(props: InputProps & { label?: string } if (!lvl) return []; const parentCode = i === 0 ? null : selArr[i - 1]; if (i > 0 && !parentCode) return []; - return rows - .filter((n) => n.levelCode === lvl.levelCode) - .filter((n) => (i === 0 ? !n.parentCode : n.parentCode === parentCode)) - .sort((a, b) => (a.order ?? 0) - (b.order ?? 0)); + // Collapse on `code` — the value each SelectItem submits. Two hierarchy + // rows sharing a code at the same level would render as identical options + // that Radix treats as one selection, checking both (#1923). + return uniqueBy( + rows + .filter((n) => n.levelCode === lvl.levelCode) + .filter((n) => (i === 0 ? !n.parentCode : n.parentCode === parentCode)) + .sort((a, b) => (a.order ?? 0) - (b.order ?? 0)), + (n) => n.code, + ); }; const handleChange = (i: number, value: string) => { diff --git a/configurator/src/resources/complaints/LocalityPicker.tsx b/configurator/src/resources/complaints/LocalityPicker.tsx index 830a52a06f..4e913a1bab 100644 --- a/configurator/src/resources/complaints/LocalityPicker.tsx +++ b/configurator/src/resources/complaints/LocalityPicker.tsx @@ -8,6 +8,7 @@ import { SelectItem, } from '@/components/ui/select'; import { Label } from '@/components/ui/label'; +import { uniqueBy } from '@/lib/uniqueBy'; interface HierarchyLevel { boundaryType: string; @@ -79,9 +80,15 @@ export function LocalityPicker({ return (boundaries ?? []).find((b) => b.code === code); }, [field.value, boundaries]); + // One option per hierarchyType — see the same note in JurisdictionEditor. + // `boundary-hierarchies` merges the state tenant's definitions with every city + // tenant's and two tenants may both name theirs "ADMIN" (#1923). const hierarchyChoices = useMemo(() => { if (!hierarchies) return [] as { value: string; label: string }[]; - return hierarchies.map((h) => ({ value: h.hierarchyType, label: h.hierarchyType })); + return uniqueBy( + hierarchies.map((h) => ({ value: h.hierarchyType, label: h.hierarchyType })), + (c) => c.value, + ); }, [hierarchies]); // Only expose the LEAF boundary type(s) per hierarchy — types that @@ -97,6 +104,10 @@ export function LocalityPicker({ const map = new Map(); if (!hierarchies) return map; for (const h of hierarchies) { + // First definition wins — the same survivor hierarchyChoices keeps, so a + // sub-tenant's same-named hierarchy can't override the leaf types of the + // one the operator sees. See JurisdictionEditor for the long note. + if (map.has(h.hierarchyType)) continue; const levels = (h.boundaryHierarchy ?? []).filter( (lvl): lvl is HierarchyLevel => !!lvl && lvl.active !== false && !!lvl.boundaryType, ); @@ -161,11 +172,17 @@ export function LocalityPicker({ const typesForHierarchy = boundaryTypesByHierarchy.get(activeHierarchy) ?? []; + // Collapse by code: boundary codes are unique per tenant, not globally, and + // the provider concatenates every tenant's tree — two tenants seeding + // CITY_001 would otherwise offer the same locality twice. const boundaryOptions = useMemo(() => { if (!activeType) return []; const inner = boundaryIndex.byHierarchy.get(activeHierarchy); - if (inner && inner.size > 0) return inner.get(activeType) ?? []; - return boundaryIndex.byTypeOnly.get(activeType) ?? []; + const forType = + inner && inner.size > 0 + ? inner.get(activeType) ?? [] + : boundaryIndex.byTypeOnly.get(activeType) ?? []; + return uniqueBy(forType, (b) => b.code); }, [boundaryIndex, activeHierarchy, activeType]); const hasError = fieldState.invalid && fieldState.isTouched; diff --git a/configurator/src/resources/designations/DepartmentChipInput.tsx b/configurator/src/resources/designations/DepartmentChipInput.tsx index df390e4515..32f53344b1 100644 --- a/configurator/src/resources/designations/DepartmentChipInput.tsx +++ b/configurator/src/resources/designations/DepartmentChipInput.tsx @@ -4,6 +4,7 @@ import { useGetList, useInput } from 'ra-core'; import { ChevronDown, X } from 'lucide-react'; import { Input } from '@/components/ui/input'; import { Label } from '@/components/ui/label'; +import { uniqueBy } from '@/lib/uniqueBy'; export interface DepartmentChipInputProps { source?: string; @@ -60,7 +61,10 @@ export function DepartmentChipInput({ const options = useMemo(() => { const q = query.trim().toLowerCase(); - return (data ?? []) + // One entry per department code — the chips this writes are codes, so a + // repeated code renders an option that does nothing on the second click + // (it is already selected) and reads as duplicated master data (#1923). + return uniqueBy(data, (d) => d.code ?? String(d.id)) .filter((d) => { const code = d.code ?? String(d.id); return !selectedSet.has(code); diff --git a/configurator/src/resources/employees/AssignmentEditor.tsx b/configurator/src/resources/employees/AssignmentEditor.tsx index c148fe09f6..621268f0d6 100644 --- a/configurator/src/resources/employees/AssignmentEditor.tsx +++ b/configurator/src/resources/employees/AssignmentEditor.tsx @@ -12,6 +12,7 @@ import { import { Input } from '@/components/ui/input'; import { Label } from '@/components/ui/label'; import { Button } from '@/components/ui/button'; +import { uniqueBy } from '@/lib/uniqueBy'; import type { Employee, EmployeeAssignment } from '@/api/types'; import { useEmployeeLookup } from '@/admin/hrms/useEmployeeLookup'; import { ReportingToSelect } from './ReportingToSelect'; @@ -103,6 +104,18 @@ export function AssignmentEditor({ { pagination: { page: 1, perPage: 1000 }, sort: { field: 'name', order: 'ASC' }, filter: tenantFilter }, ); + // Both pickers submit `code`, so two master records sharing a code would + // render as duplicate SelectItems with the same value — Radix marks every one + // of them checked and concatenates their labels into the trigger (#1923). + const departmentChoices = useMemo( + () => uniqueBy(departments, (d) => d.code), + [departments], + ); + const designationChoices = useMemo( + () => uniqueBy(designations, (d) => d.code), + [designations], + ); + const { employees: managerCandidates, isLoading: managersLoading } = useEmployeeLookup(tenantFilter); // On the edit form this is the employee being edited; on create there's no // record yet, so nothing needs to be excluded from the manager list. @@ -204,7 +217,7 @@ export function AssignmentEditor({ /> - {(departments ?? []).map((d) => ( + {departmentChoices.map((d) => ( {d.name ?? d.code} @@ -230,7 +243,7 @@ export function AssignmentEditor({ /> - {(designations ?? []).map((d) => ( + {designationChoices.map((d) => ( {d.name ?? d.code} diff --git a/configurator/src/resources/employees/JurisdictionEditor.test.tsx b/configurator/src/resources/employees/JurisdictionEditor.test.tsx new file mode 100644 index 0000000000..58a198c0eb --- /dev/null +++ b/configurator/src/resources/employees/JurisdictionEditor.test.tsx @@ -0,0 +1,161 @@ +// @vitest-environment jsdom +// +// Regression coverage for CCRS #1923 — "Duplicate records rendered in +// dropdown/select fields on the configurator". +// +// The reported screen is the employee create/edit form's Jurisdiction block. +// `boundary-hierarchies` aggregates the state tenant's hierarchy definitions +// with every city tenant's, and DIGIT does not require a `hierarchyType` to be +// unique across tenants — on bomet (`ke`) SEVEN tenants define one called +// "ADMIN" and three define "KE-ADMIN". Radix renders one SelectItem per choice +// and treats items sharing a `value` as the same selection, so the operator saw +// seven "ADMIN" rows all ticked and a trigger reading +// "ADMINADMINADMINADMINAD…". +// +// The same holds one level down: boundary codes are unique per tenant, not +// globally (`ke.mycitynew` and `ke.hajbvfg` both seed CITY_001 / WARD_001). + +import { describe, it, expect, beforeAll } from 'vitest'; +import { render, screen, fireEvent, waitFor } from '@testing-library/react'; +import { CoreAdminContext, Form, TestMemoryRouter, type DataProvider } from 'ra-core'; +import { QueryClient } from '@tanstack/react-query'; +import { JurisdictionEditor } from './JurisdictionEditor'; + +// The bomet shape, trimmed: one hierarchy name defined by several tenants. +const HIERARCHIES = [ + { id: 'ADMIN', hierarchyType: 'ADMIN', tenantId: 'ke', boundaryHierarchy: [ + { boundaryType: 'County', parentBoundaryType: null, active: true }, + { boundaryType: 'Ward', parentBoundaryType: 'County', active: true }, + ] }, + { id: 'ADMIN', hierarchyType: 'ADMIN', tenantId: 'ke.mycitynew', boundaryHierarchy: [ + { boundaryType: 'County', parentBoundaryType: null, active: true }, + ] }, + { id: 'ADMIN', hierarchyType: 'ADMIN', tenantId: 'ke.hajbvfg', boundaryHierarchy: [ + { boundaryType: 'County', parentBoundaryType: null, active: true }, + ] }, + { id: 'KE-ADMIN', hierarchyType: 'KE-ADMIN', tenantId: 'ke.india', boundaryHierarchy: [ + { boundaryType: 'State', parentBoundaryType: null, active: true }, + ] }, + { id: 'KE-ADMIN', hierarchyType: 'KE-ADMIN', tenantId: 'ke.etoebeta', boundaryHierarchy: [ + { boundaryType: 'State', parentBoundaryType: null, active: true }, + ] }, +]; + +// CITY_001 exists under two tenants — same code, same hierarchy, same level. +const BOUNDARIES = [ + { id: 'BOMET', code: 'BOMET', name: 'Bomet', boundaryType: 'County', hierarchyType: 'ADMIN', tenantId: 'ke' }, + { id: 'CITY_001', code: 'CITY_001', name: 'City One', boundaryType: 'County', hierarchyType: 'ADMIN', tenantId: 'ke.mycitynew' }, + { id: 'CITY_001', code: 'CITY_001', name: 'City One', boundaryType: 'County', hierarchyType: 'ADMIN', tenantId: 'ke.hajbvfg' }, +]; + +function makeDataProvider(): DataProvider { + return { + getList: async (resource: string) => { + if (resource === 'boundary-hierarchies') return { data: HIERARCHIES, total: HIERARCHIES.length }; + if (resource === 'boundaries') return { data: BOUNDARIES, total: BOUNDARIES.length }; + return { data: [], total: 0 }; + }, + getOne: async (_r: string, params: { id: unknown }) => ({ data: { id: params.id } }), + getMany: async () => ({ data: [] }), + getManyReference: async () => ({ data: [], total: 0 }), + create: async (_r: string, params: { data: unknown }) => ({ data: params.data }), + update: async (_r: string, params: { id: unknown; data: unknown }) => ({ data: params.data }), + updateMany: async () => ({ data: [] }), + delete: async (_r: string, params: { id: unknown }) => ({ data: { id: params.id } }), + deleteMany: async () => ({ data: [] }), + } as unknown as DataProvider; +} + +function renderEditor(record: Record) { + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false, gcTime: 0 }, mutations: { retry: false } }, + }); + return render( + + +
{}}> + + +
+
, + ); +} + +/** The row renders its selects in a fixed order — Hierarchy, then one per + * hierarchy level (County, Ward, …) — and the