← all branches

feat/sites-1n

needs attentionviewing older commit
6231fe5 · incrementalPR #279reviewed 2026-07-08 22:34 UTC0H · 2M · 4L · 2I
The branch
Purpose
Fix the razón-social mass-merge bug: sites were fused whenever CFE service names matched, causing edit-one-RPU-changes-unrelated-RPUs incidents.
Goal
Correct Site↔RPU 1:N relationship: fix dedup logic in both coordinators, add management UX, and author a one-shot remediation script.
Sub-goals
  • SG-1: Shared pure resolveSiteForRpu + rewire both coordinators
  • SG-2/3: Site search/match + wizard existingSitePublicId
  • SG-4: Drawer match-driven Crear/Usar-existente
  • SG-5: Site pencil → SiteEditModal
  • SG-6: Agrupar por sitio toggle
  • SG-7: Batch preview grouping
  • SG-8: Analytics + docs
The changes (whole branch)
What
Consolidates two independent review passes: findByOrgInternalIdAndName now active-only + case-insensitive; createFromWizardShell PATH-1 all-statuses; OCC reports real version; split-merged-sites gains failed-site tracking + secure snapshot; 7 DB-gated integration tests for wizard + SUC org-scoping.
Why
Two independent review passes converging on the same gap (wizard shell coverage) was the signal to ship. Duplicate-SUC 500 was a 3-finding reachable chain.
Areas
domains/cross-domain+410−5domains/core/src/site+8−3domains/utility/src/site-utility-contract+10−2scripts+68−37.branch+1−0
Blast
7 files, +497/−47. No schema changes, no API contract changes.
integration-tests-require-db no-migration-required admin-script-ready-post-merge
➖ci· No CI checks registered at review time➖coderabbit· No .coderabbit.yaml in repo

Findings · 8

correctness2

medium

PATH-1 all-status expansion silently blocks re-adding RPUs from inactive sites with no UX explanation

domains/cross-domain/src/contract-wizard.shells.ts:203

Correct fix, but WizardContractExists/409 gives no indication the contract is on an inactive site. User has no self-serve recourse. Add a distinct error tag or richer message.

info

Verify decideAssignContract handles non-null existingAssignment gracefully

domains/cross-domain/src/contract-wizard.shells.ts:401

Previously always called with null. Confirm it returns a structured conflict (not panic/no-op) when existingAssignment is non-null.

tests2

low

afterAll cleanup swallows all errors silently

domains/cross-domain/src/__tests__/contract-wizard-sites.integration.test.ts:149

Every delete in afterAll uses try {} catch {} with no logging. Orphaned rows cause false failures in subsequent runs.

low

nextRpu() slice(-12 - 3) evaluates to slice(-15), producing 15-char strings

domains/cross-domain/src/__tests__/contract-wizard-sites.integration.test.ts:52

No constraint violated (shell doesn't enforce length), but the arithmetic looks like a mistake. Clarify with a named constant.

improvement4

medium

enableMonitoringShell PATH-1 still filters active-only sites

domains/cross-domain/src/contract-wizard.shells.ts:561

createFromWizardShell (line 203) was fixed to call findByOrgId without a status filter. enableMonitoringShell still uses { status: 'active' }, creating the same duplicate-SUC reachable path for contracts on deactivated sites that satisfy PATH-2. Remove the filter or document why the asymmetry is correct.

low

No functional index for lower(name) / lower(metadata->>'customer_id') dedup query

domains/core/src/site/site.queries.ts:327

Query shape is stable. A follow-up migration adding expression indexes on lower(name) WHERE status='active' and lower(metadata->>'customer_id') would make this O(log N) at production org sizes.

low

findByOrgId fetches full Site objects just to extract IDs

domains/cross-domain/src/contract-wizard.shells.ts:203

PATH-1 materialises full Site domain objects then discards everything except id. A findIdsByOrgId query would avoid the JSONB allocation.

info

OCC null-fresh fallback masks a concurrent-delete scenario

domains/utility/src/site-utility-contract/site-utility-contract.shells.ts:335

fresh?.version ?? previousVersion + 1 uses the old guess when the SUC is concurrently deleted. A notFound error would be more accurate. Extremely narrow in practice.

History · 8 commits

  1. 7b5ff24needs attentionincremental0H · 2M · 3L2026-07-09 00:58
  2. 6231fe5needs attentionincremental0H · 2M · 4L2026-07-08 22:34current
  3. 8fa38b5needs attentionincremental2H · 4M · 5L2026-07-08 01:45
  4. 83fd5a0needs attentionincremental3H · 8M · 11L2026-07-08 01:37
  5. 23c04fcneeds attentionincremental3H · 3M · 2L2026-07-07 20:30
  6. e70f9f5blockedincremental6H · 11M · 6L2026-07-07 19:28
  7. 499b1a5blockedincremental6H · 9M · 5L2026-07-07 18:26
  8. ff23301needs attentionincremental2H · 6M · 5L2026-07-07 01:05