fix/col-rename
needs attentiona428c2f · incrementalPR #284reviewed 2026-07-09 01:19 UTC1H · 3M · 3L · 2I- Purpose
- Fix blank column rendering when a line-item-backed column has a displayName override — the cell was keyed under the catalog publicName while the header used the effective (override-aware) name.
- Goal
- FCIS compliance: move transformBillToPresentation (pure function) to bill.decisions.ts and add edge-case test coverage for null/absent conceptName fallback paths.
- Sub-goals
- SG-1: Rename fix (39b9e17a) — key line-item values by effective publicName from conceptToPublicName map
- SG-2: FCIS refactor (a428c2f5) — move pure function to decisions layer and expand test coverage
- What
- transformBillToPresentation + PresentationBill moved from bill.shells.ts → bill.decisions.ts. bill.shells.ts now imports them from decisions. 4 new test cases cover: empty-map fallback, null conceptName fallback, both-null skip, mixed renamed/non-renamed resolution.
- Why
- Pure function was in the wrong FCIS layer (shells own I/O, decisions own pure logic). Move satisfies architectural constraint and enables direct unit testing.
- Areas
- domains/utility/src/bill/bill.decisions.ts+96−0domains/utility/src/bill/bill.shells.ts+1−81domains/utility/src/bill/__tests__/bill.shells.test.ts+134−0
- Blast
- 3 files, +231/−81; decisions + shells + tests only. No API surface, handler, DB, or infra change.
Findings · 9
security1
storagePath (internal S3 key) serialized into bills list response via files field (pre-existing)
domains/utility/src/bill/bill.decisions.ts
Pre-existing — not introduced by this diff. Low exploitability (authenticated users only). Fix: strip storagePath before assigning billFieldValues.files.
conventions2
Multi-line JSDoc block on transformBillToPresentation violates no-docstring rule
domains/utility/src/bill/bill.decisions.ts
Project rule: no multi-line comment blocks — one short line max. The /** ... */ block should be replaced with a single // comment or removed.
Type-only import comment acceptable (single line, passes convention)
domains/utility/src/bill/bill.decisions.ts
No action needed.
tests4
'skips orphan' test only checks for string keys 'null'/'undefined' — misses empty-string key leak
domains/utility/src/bill/__tests__/bill.shells.test.ts
If the effectivePublicName guard were removed, result[''] could be written silently. Add: expect(Object.keys(row)).not.toContain('') or assert exact key count.
'absent from map' test title misleads — exercises non-null conceptName path, not null conceptName
domains/utility/src/bill/__tests__/bill.shells.test.ts
totalMemLineItem has conceptName='totalMemCosts'. Rename to clarify it tests a non-null conceptName absent from the map.
'renamed and non-renamed' test can't distinguish map-driven vs fallback resolution for non-renamed item
domains/utility/src/bill/__tests__/bill.shells.test.ts
scnmem's map value and conceptPublicName are both 'SCnMEM'. Omit from map to force the fallback path.
transformBillToPresentation tests belong in bill.decisions.test.ts per canonical form
domains/utility/src/bill/__tests__/bill.shells.test.ts
Canonical form places decisions tests in bill.decisions.test.ts. Coverage is split. Low urgency follow-up.
improvement2
Inline source-label object allocated on every call — extract as module-level constant
domains/utility/src/bill/bill.decisions.ts
Move { xml: 'Recibo', inferred: 'Recibo Histórico', manual: 'Manual' } to a module-level const BILL_SOURCE_LABEL.
billFieldPublicNames variable name reads as a list, not a Set guard
domains/utility/src/bill/bill.decisions.ts
Rename to billFieldPublicNameSet to clarify its membership-check role.