Compare commits
9 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 9a313d2c1b | |||
| 2eb61f16ff | |||
| 12c2913ba1 | |||
| 54440da2cd | |||
| 7427838292 | |||
| b606e8c9a2 | |||
| ac709c07c3 | |||
| 08e83da91d | |||
| 3f672d5db3 |
@@ -0,0 +1,926 @@
|
|||||||
|
# Drop `UNIQUE(batch_id, patient_control_number)` + 409 UX Implementation Plan
|
||||||
|
|
||||||
|
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||||
|
|
||||||
|
**Goal:** Allow multi-claim 837P files (where many `CLM*` segments share a `member_id`) to ingest without 409, and surface a structured error panel on the upload page when a true duplicate claim still trips a 409.
|
||||||
|
|
||||||
|
**Architecture:** Backend migration drops the inline `UNIQUE(batch_id, patient_control_number)` on `claims` via SQLite table recreation. Two new store helpers (`find_existing_batch_for_claim`, `find_existing_batch_for_remit`) enable both 837 and 835 409 handlers to surface the id of the prior batch. Frontend `ApiError` carries `existingBatchId`; `Upload.tsx` renders an inline error panel above the streaming results with a link to the existing batch and a "Pick a different file" escape hatch.
|
||||||
|
|
||||||
|
**Tech Stack:** Python 3.11+, SQLAlchemy 2.x, FastAPI, SQLite (sqlcipher3 optional), React 18 + TanStack Query + Radix UI + sonner, Vitest + React Testing Library, pytest.
|
||||||
|
|
||||||
|
**Spec:** `docs/superpowers/specs/2026-06-21-cyclone-claims-unique-constraint-and-409-ux-design.md` — read fully before starting.
|
||||||
|
|
||||||
|
**Worktree setup (one-time):**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
cd /Users/openclaw/dev/cyclone
|
||||||
|
git worktree add .worktrees/claims-unique-fix -b claims-unique-fix main
|
||||||
|
cd .worktrees/claims-unique-fix
|
||||||
|
# Install backend deps if needed (venv assumed active)
|
||||||
|
pip install -e backend
|
||||||
|
# Install frontend deps if needed
|
||||||
|
npm install
|
||||||
|
```
|
||||||
|
|
||||||
|
All commits happen in this worktree. Merge to main via fast-forward when each phase ends.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Phase 1 — Backend migration + helpers + API
|
||||||
|
|
||||||
|
### Task 1.1: Migration 0013 drops inline UNIQUE via table recreation
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Create: `backend/src/cyclone/migrations/0013_drop_claims_unique_constraint.sql`
|
||||||
|
- Modify: `backend/tests/test_db_migrate.py` (append test)
|
||||||
|
|
||||||
|
The runner (`backend/src/cyclone/db_migrate.py`) wraps each `.sql` in an implicit transaction via `engine.begin()`, so the migration MUST NOT use `BEGIN`/`COMMIT` or `PRAGMA foreign_keys=OFF` (no-op inside a transaction). Use `PRAGMA defer_foreign_keys = ON` instead — checks fire at commit against the renamed table.
|
||||||
|
|
||||||
|
- [ ] **Step 1: Write the failing test**
|
||||||
|
|
||||||
|
Add to `backend/tests/test_db_migrate.py`:
|
||||||
|
|
||||||
|
```python
|
||||||
|
def test_drop_claims_unique_constraint_migration(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||||
|
"""Migration 0013 must drop the inline UNIQUE(batch_id, patient_control_number)
|
||||||
|
on claims by recreating the table. Idempotent and preserves data."""
|
||||||
|
monkeypatch.setattr(db_migrate, "MIGRATIONS_DIR", tmp_path)
|
||||||
|
# Copy the real migrations so the test starts from v1.
|
||||||
|
real_dir = Path(db_migrate.__file__).parent / "migrations"
|
||||||
|
for src in sorted(real_dir.glob("00*.sql")):
|
||||||
|
(tmp_path / src.name).write_text(src.read_text())
|
||||||
|
|
||||||
|
engine = _fresh_engine(tmp_path)
|
||||||
|
db_migrate.run(engine)
|
||||||
|
# Before 0013: insert two claims with same (batch_id, patient_control_number) raises.
|
||||||
|
with engine.begin() as c:
|
||||||
|
c.exec_driver_sql("INSERT INTO batches(id, kind, input_filename, parsed_at) VALUES ('b1', '837p', 'x.txt', '2026-01-01')")
|
||||||
|
c.exec_driver_sql("INSERT INTO claims(id, batch_id, patient_control_number) VALUES ('c1', 'b1', 'M')")
|
||||||
|
with pytest.raises(sqlalchemy.exc.IntegrityError):
|
||||||
|
c.exec_driver_sql("INSERT INTO claims(id, batch_id, patient_control_number) VALUES ('c2', 'b1', 'M')")
|
||||||
|
# Now drop the migration in by name only:
|
||||||
|
(tmp_path / "0013_drop_claims_unique_constraint.sql").write_text(
|
||||||
|
"-- version: 13\n"
|
||||||
|
"PRAGMA defer_foreign_keys = ON;\n"
|
||||||
|
"CREATE TABLE claims_new ("
|
||||||
|
"id TEXT PRIMARY KEY, batch_id TEXT NOT NULL REFERENCES batches(id) ON DELETE CASCADE,"
|
||||||
|
"patient_control_number TEXT NOT NULL, service_date_from DATE, service_date_to DATE,"
|
||||||
|
"charge_amount NUMERIC(12, 2) NOT NULL DEFAULT 0, provider_npi TEXT, payer_id TEXT,"
|
||||||
|
"state TEXT NOT NULL DEFAULT 'submitted', state_before_reversal TEXT,"
|
||||||
|
"matched_remittance_id TEXT REFERENCES remittances(id), raw_json TEXT,"
|
||||||
|
"rejection_reason TEXT, rejected_at TIMESTAMP, resubmit_count INTEGER NOT NULL DEFAULT 0,"
|
||||||
|
"state_changed_at TIMESTAMP, payer_rejected_at TEXT, payer_rejected_reason TEXT,"
|
||||||
|
"payer_rejected_status_code TEXT, payer_rejected_by_277ca_id TEXT,"
|
||||||
|
"payer_rejected_acknowledged_at TEXT, payer_rejected_acknowledged_actor TEXT);\n"
|
||||||
|
"INSERT INTO claims_new SELECT * FROM claims;"
|
||||||
|
"DROP TABLE claims;"
|
||||||
|
"ALTER TABLE claims_new RENAME TO claims;"
|
||||||
|
)
|
||||||
|
db_migrate.run(engine)
|
||||||
|
# After 0013: same insert succeeds.
|
||||||
|
with engine.begin() as c:
|
||||||
|
c.exec_driver_sql("INSERT INTO claims(id, batch_id, patient_control_number) VALUES ('c2', 'b1', 'M')")
|
||||||
|
rows = c.exec_driver_sql("SELECT COUNT(*) FROM claims").scalar()
|
||||||
|
assert rows == 2
|
||||||
|
assert _user_version(engine) == 13
|
||||||
|
|
||||||
|
|
||||||
|
def test_migration_0013_is_idempotent(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||||
|
"""Re-running db_migrate.run on a v13 DB is a no-op."""
|
||||||
|
monkeypatch.setattr(db_migrate, "MIGRATIONS_DIR", tmp_path)
|
||||||
|
real_dir = Path(db_migrate.__file__).parent / "migrations"
|
||||||
|
for src in sorted(real_dir.glob("00*.sql")):
|
||||||
|
(tmp_path / src.name).write_text(src.read_text())
|
||||||
|
|
||||||
|
engine = _fresh_engine(tmp_path)
|
||||||
|
db_migrate.run(engine) # applies all
|
||||||
|
version_after_first = _user_version(engine)
|
||||||
|
db_migrate.run(engine) # second call: no-op
|
||||||
|
assert _user_version(engine) == version_after_first
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Run test to verify it fails**
|
||||||
|
|
||||||
|
Run: `cd backend && python -m pytest tests/test_db_migrate.py::test_drop_claims_unique_constraint_migration tests/test_db_migrate.py::test_migration_0013_is_idempotent -v`
|
||||||
|
|
||||||
|
Expected: `test_drop_claims_unique_constraint_migration` FAILS because 0013 doesn't exist yet; `test_migration_0013_is_idempotent` PASSES (idempotency already works for any v).
|
||||||
|
|
||||||
|
- [ ] **Step 3: Write migration 0013**
|
||||||
|
|
||||||
|
Create `backend/src/cyclone/migrations/0013_drop_claims_unique_constraint.sql`:
|
||||||
|
|
||||||
|
```sql
|
||||||
|
-- version: 13
|
||||||
|
-- Drop the inline UNIQUE(batch_id, patient_control_number) on claims.
|
||||||
|
-- Migration 0003 attempted DROP INDEX IF EXISTS uq_claims_batch_pcn but
|
||||||
|
-- the constraint is inline in CREATE TABLE, so the drop was a no-op.
|
||||||
|
-- The only way to remove an inline UNIQUE in SQLite is table recreation.
|
||||||
|
--
|
||||||
|
-- X12 837P allows any number of CLM segments per 2000B subscriber loop;
|
||||||
|
-- claim identity is provided by the primary key (claims.id = CLM01).
|
||||||
|
-- The remittances table had a parallel constraint already removed in 0003
|
||||||
|
-- (because that one WAS a named index), so this migration only touches
|
||||||
|
-- claims.
|
||||||
|
--
|
||||||
|
-- The migration runner (db_migrate.py) wraps each .sql in an implicit
|
||||||
|
-- transaction via engine.begin(), so we MUST NOT use BEGIN/COMMIT.
|
||||||
|
-- PRAGMA defer_foreign_keys defers FK checks to commit, which is the
|
||||||
|
-- only way to drop a referenced table inside a transaction in SQLite.
|
||||||
|
|
||||||
|
PRAGMA defer_foreign_keys = ON;
|
||||||
|
|
||||||
|
CREATE TABLE claims_new (
|
||||||
|
id TEXT PRIMARY KEY,
|
||||||
|
batch_id TEXT NOT NULL REFERENCES batches(id) ON DELETE CASCADE,
|
||||||
|
patient_control_number TEXT NOT NULL,
|
||||||
|
service_date_from DATE,
|
||||||
|
service_date_to DATE,
|
||||||
|
charge_amount NUMERIC(12, 2) NOT NULL DEFAULT 0,
|
||||||
|
provider_npi TEXT,
|
||||||
|
payer_id TEXT,
|
||||||
|
state TEXT NOT NULL DEFAULT 'submitted',
|
||||||
|
state_before_reversal TEXT,
|
||||||
|
matched_remittance_id TEXT REFERENCES remittances(id),
|
||||||
|
raw_json TEXT,
|
||||||
|
rejection_reason TEXT,
|
||||||
|
rejected_at TIMESTAMP,
|
||||||
|
resubmit_count INTEGER NOT NULL DEFAULT 0,
|
||||||
|
state_changed_at TIMESTAMP,
|
||||||
|
payer_rejected_at TEXT,
|
||||||
|
payer_rejected_reason TEXT,
|
||||||
|
payer_rejected_status_code TEXT,
|
||||||
|
payer_rejected_by_277ca_id TEXT,
|
||||||
|
payer_rejected_acknowledged_at TEXT,
|
||||||
|
payer_rejected_acknowledged_actor TEXT
|
||||||
|
-- NO UNIQUE (batch_id, patient_control_number) — removed.
|
||||||
|
);
|
||||||
|
|
||||||
|
INSERT INTO claims_new SELECT * FROM claims;
|
||||||
|
DROP TABLE claims;
|
||||||
|
ALTER TABLE claims_new RENAME TO claims;
|
||||||
|
|
||||||
|
-- Recreate secondary indexes (same names, same columns as initial schema
|
||||||
|
-- plus later migrations).
|
||||||
|
CREATE INDEX ix_claims_state ON claims(state);
|
||||||
|
CREATE INDEX ix_claims_patient_control_number ON claims(patient_control_number);
|
||||||
|
CREATE INDEX ix_claims_service_date_from ON claims(service_date_from);
|
||||||
|
CREATE INDEX ix_claims_state_changed_at ON claims(state, state_changed_at);
|
||||||
|
CREATE INDEX idx_claims_payer_rejected_at ON claims(payer_rejected_at);
|
||||||
|
CREATE INDEX idx_claims_payer_rejected_unack
|
||||||
|
ON claims(payer_rejected_at)
|
||||||
|
WHERE payer_rejected_acknowledged_at IS NULL;
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 4: Run tests to verify they pass**
|
||||||
|
|
||||||
|
Run: `cd backend && python -m pytest tests/test_db_migrate.py::test_drop_claims_unique_constraint_migration tests/test_db_migrate.py::test_migration_0013_is_idempotent -v`
|
||||||
|
|
||||||
|
Expected: PASS for both. The first test asserts the inline UNIQUE is gone (insert succeeds) and `user_version==13`. The second asserts idempotency.
|
||||||
|
|
||||||
|
- [ ] **Step 5: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add backend/src/cyclone/migrations/0013_drop_claims_unique_constraint.sql backend/tests/test_db_migrate.py
|
||||||
|
git commit -m "feat(db): drop inline UNIQUE(batch_id, patient_control_number) via migration 0013"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Task 1.2: Store helpers `find_existing_batch_for_claim` and `find_existing_batch_for_remit`
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `backend/src/cyclone/store.py:1-50` (imports) and add new functions near the top
|
||||||
|
- Modify: `backend/tests/test_store.py` (append tests)
|
||||||
|
|
||||||
|
The helpers are pure reads. They return the batch_id of the first batch containing the given claim_id / remit_id, or None.
|
||||||
|
|
||||||
|
- [ ] **Step 1: Write the failing test**
|
||||||
|
|
||||||
|
Add to `backend/tests/test_store.py`:
|
||||||
|
|
||||||
|
```python
|
||||||
|
def test_find_existing_batch_for_claim_returns_none_for_unknown(global_store):
|
||||||
|
"""Unknown claim_id -> None."""
|
||||||
|
assert global_store.find_existing_batch_for_claim("nope") is None # type: ignore[attr-defined]
|
||||||
|
|
||||||
|
|
||||||
|
def test_find_existing_batch_for_claim_returns_batch_id(global_store):
|
||||||
|
"""Known claim_id -> batch_id of the holding batch."""
|
||||||
|
from cyclone.store import BatchRecord, _claim_837_row # noqa: F401
|
||||||
|
from cyclone.db import Batch, Claim, db as _db_mod
|
||||||
|
from datetime import datetime, timezone
|
||||||
|
from cyclone.parser.parsers_837p import ClaimOutput # noqa: F401
|
||||||
|
# Simpler: insert a batch + claim directly via the DB.
|
||||||
|
with _db_mod.SessionLocal()() as s:
|
||||||
|
s.add(Batch(id="B1", kind="837p", input_filename="x.txt",
|
||||||
|
parsed_at=datetime(2026,1,1,tzinfo=timezone.utc)))
|
||||||
|
s.add(Claim(id="CLM-A", batch_id="B1", patient_control_number="M1"))
|
||||||
|
s.commit()
|
||||||
|
assert global_store.find_existing_batch_for_claim("CLM-A") == "B1" # type: ignore[attr-defined]
|
||||||
|
|
||||||
|
|
||||||
|
def test_find_existing_batch_for_remit_returns_none_for_unknown(global_store):
|
||||||
|
"""Unknown remit id -> None."""
|
||||||
|
assert global_store.find_existing_batch_for_remit("nope") is None # type: ignore[attr-defined]
|
||||||
|
|
||||||
|
|
||||||
|
def test_find_existing_batch_for_remit_returns_batch_id(global_store):
|
||||||
|
"""Known remit id -> batch_id of the holding batch."""
|
||||||
|
from cyclone.db import Batch, Remittance, db as _db_mod
|
||||||
|
from datetime import datetime, timezone
|
||||||
|
with _db_mod.SessionLocal()() as s:
|
||||||
|
s.add(Batch(id="B2", kind="835", input_filename="y.txt",
|
||||||
|
parsed_at=datetime(2026,1,1,tzinfo=timezone.utc)))
|
||||||
|
s.add(Remittance(id="CLP-A", batch_id="B2",
|
||||||
|
payer_claim_control_number="CLP-A",
|
||||||
|
status_code="1", received_at=datetime(2026,1,1,tzinfo=timezone.utc)))
|
||||||
|
s.commit()
|
||||||
|
assert global_store.find_existing_batch_for_remit("CLP-A") == "B2" # type: ignore[attr-defined]
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Run test to verify it fails**
|
||||||
|
|
||||||
|
Run: `cd backend && python -m pytest tests/test_store.py::test_find_existing_batch_for_claim_returns_none_for_unknown -v`
|
||||||
|
|
||||||
|
Expected: FAIL with `AttributeError: 'CycloneStore' object has no attribute 'find_existing_batch_for_claim'`.
|
||||||
|
|
||||||
|
- [ ] **Step 3: Implement helpers**
|
||||||
|
|
||||||
|
Add to `backend/src/cyclone/store.py` near the top, after imports:
|
||||||
|
|
||||||
|
```python
|
||||||
|
def find_existing_batch_for_claim(claim_id: str) -> str | None:
|
||||||
|
"""Return the batch_id of the first batch containing this claim id, or None.
|
||||||
|
|
||||||
|
Pure read; opens a short-lived session. Used by the 837 409 handler to
|
||||||
|
surface which prior batch already holds the same CLM01.
|
||||||
|
"""
|
||||||
|
from sqlalchemy import select
|
||||||
|
from cyclone import db
|
||||||
|
from cyclone.db import Claim
|
||||||
|
with db.SessionLocal()() as s:
|
||||||
|
row = s.execute(
|
||||||
|
select(Claim.batch_id).where(Claim.id == claim_id).limit(1)
|
||||||
|
).first()
|
||||||
|
return row[0] if row else None
|
||||||
|
|
||||||
|
|
||||||
|
def find_existing_batch_for_remit(remit_id: str) -> str | None:
|
||||||
|
"""Return the batch_id of the first batch containing this remit id, or None.
|
||||||
|
|
||||||
|
Pure read; opens a short-lived session. Used by the 835 409 handler.
|
||||||
|
`remit_id` is the PK on `remittances.id` (= payer_claim_control_number = CLP01).
|
||||||
|
"""
|
||||||
|
from sqlalchemy import select
|
||||||
|
from cyclone import db
|
||||||
|
from cyclone.db import Remittance
|
||||||
|
with db.SessionLocal()() as s:
|
||||||
|
row = s.execute(
|
||||||
|
select(Remittance.batch_id).where(Remittance.id == remit_id).limit(1)
|
||||||
|
).first()
|
||||||
|
return row[0] if row else None
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 4: Run tests to verify they pass**
|
||||||
|
|
||||||
|
Run: `cd backend && python -m pytest tests/test_store.py -k "find_existing_batch" -v`
|
||||||
|
|
||||||
|
Expected: PASS for all 4 tests.
|
||||||
|
|
||||||
|
- [ ] **Step 5: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add backend/src/cyclone/store.py backend/tests/test_store.py
|
||||||
|
git commit -m "feat(store): add find_existing_batch_for_claim and find_existing_batch_for_remit"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Task 1.3: 409 handlers in api.py surface `existing_batch_id`
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `backend/src/cyclone/api.py:394-415` (837 409 handler)
|
||||||
|
- Modify: `backend/src/cyclone/api.py:588-602` (835 409 handler)
|
||||||
|
- Modify: `backend/tests/test_api_parse_persists.py` (append tests)
|
||||||
|
|
||||||
|
After migration 0013, IntegrityError on the 837 path can only fire when two claims in the same file share the same CLM01 (rare). The helper may still return a batch_id if the colliding CLM01 was previously ingested and not deleted (the dedup `s.get(Claim, claim_id)` only queries DB state, not pending session state, so cross-batch duplicates still get inserted and trip the PK).
|
||||||
|
|
||||||
|
- [ ] **Step 1: Write the failing tests**
|
||||||
|
|
||||||
|
Add to `backend/tests/test_api_parse_persists.py`:
|
||||||
|
|
||||||
|
```python
|
||||||
|
def test_409_response_includes_existing_batch_id_for_837(client: TestClient):
|
||||||
|
"""When a CLM01 already exists in a prior batch, the 409 body has existing_batch_id."""
|
||||||
|
from cyclone.db import Batch, Claim
|
||||||
|
from datetime import datetime, timezone
|
||||||
|
# Seed a prior batch with claim CLM-X.
|
||||||
|
with global_store._lock:
|
||||||
|
pass
|
||||||
|
from cyclone import db as _db
|
||||||
|
with _db.SessionLocal()() as s:
|
||||||
|
s.add(Batch(id="PRIOR", kind="837p", input_filename="prior.txt",
|
||||||
|
parsed_at=datetime(2026,1,1,tzinfo=timezone.utc),
|
||||||
|
raw_result_json={}))
|
||||||
|
s.add(Claim(id="CLM-X", batch_id="PRIOR", patient_control_number="M"))
|
||||||
|
s.commit()
|
||||||
|
# Build a file with two CLM* segments both using CLM-X (forces PK collision).
|
||||||
|
text = (
|
||||||
|
"ISA*00* *00* *ZZ*SUBMITTERID *ZZ*RECEIVERID "
|
||||||
|
"*240101*1200*^*00501*000000001*0*P*:~\n"
|
||||||
|
"GS*HC*SUBMITTERID*RECEIVERID*20240101*1200*1*X*005010X222A1~\n"
|
||||||
|
"ST*837*0001*005010X222A1~\n"
|
||||||
|
"BHT*0019*00*1*20240101*1200*CH~\n"
|
||||||
|
"NM1*41*2*SUBMITTER*****46*SUBMITTERID~\n"
|
||||||
|
"PER*IC*CONTACT*TE*5555555555~\n"
|
||||||
|
"NM1*40*2*RECEIVER*****46*RECEIVERID~\n"
|
||||||
|
"HL*1**20*1~\n"
|
||||||
|
"NM1*85*2*BILLING*****XX*1881068062~\n"
|
||||||
|
"N3*123 MAIN*\nN4*DENVER*CO*80202~\n"
|
||||||
|
"REF*EI*123456789~\n"
|
||||||
|
"HL*2*1*22*0~\n"
|
||||||
|
"SBR*P*18*******CI~\n"
|
||||||
|
"NM1*IL*1*DOE*JOHN****MI*M~\n"
|
||||||
|
"N3*456 ELM*\nN4*DENVER*CO*80202~\n"
|
||||||
|
"DMG*D8*19700101*M~\n"
|
||||||
|
"NM1*PR*2*MEDICAID*****PI*MCD~\n"
|
||||||
|
"CLM*CLM-X*100***11:B:1*Y*A*Y*Y~\n"
|
||||||
|
"LX*1~\nSV1*HC:99213*100*UN*1***1~\n"
|
||||||
|
"DTP*472*D8*20240101~\n"
|
||||||
|
"CLM*CLM-X*100***11:B:1*Y*A*Y*Y~\n"
|
||||||
|
"LX*2~\nSV1*HC:99213*100*UN*1***1~\n"
|
||||||
|
"DTP*472*D8*20240101~\n"
|
||||||
|
"SE*30*0001~\n"
|
||||||
|
"GE*1*1~\n"
|
||||||
|
"IEA*1*000000001~\n"
|
||||||
|
)
|
||||||
|
resp = client.post(
|
||||||
|
"/api/parse-837",
|
||||||
|
files={"file": ("dup.txt", text, "text/plain")},
|
||||||
|
headers={"Accept": "application/json"},
|
||||||
|
)
|
||||||
|
assert resp.status_code == 409, resp.text
|
||||||
|
body = resp.json()
|
||||||
|
assert body.get("existing_batch_id") == "PRIOR"
|
||||||
|
|
||||||
|
|
||||||
|
def test_409_response_includes_existing_batch_id_for_835(client: TestClient):
|
||||||
|
"""When a CLP01 already exists in a prior batch, the 835 409 body has existing_batch_id."""
|
||||||
|
from cyclone.db import Batch, Remittance
|
||||||
|
from datetime import datetime, timezone
|
||||||
|
from cyclone import db as _db
|
||||||
|
with _db.SessionLocal()() as s:
|
||||||
|
s.add(Batch(id="PRIOR835", kind="835", input_filename="prior.txt",
|
||||||
|
parsed_at=datetime(2026,1,1,tzinfo=timezone.utc),
|
||||||
|
raw_result_json={}))
|
||||||
|
s.add(Remittance(id="CLP-X", batch_id="PRIOR835",
|
||||||
|
payer_claim_control_number="CLP-X",
|
||||||
|
status_code="1",
|
||||||
|
received_at=datetime(2026,1,1,tzinfo=timezone.utc)))
|
||||||
|
s.commit()
|
||||||
|
# Build a minimal 835 with two CLP segments using CLP-X. This is contrived;
|
||||||
|
# we just need to trigger IntegrityError. The exact parser robustness to this
|
||||||
|
# fixture is not the focus — the test asserts the 409 body shape.
|
||||||
|
# ... or instead: directly invoke the handler via a unit-style test that
|
||||||
|
# pre-seeds and then makes a minimal 835 that includes CLP-X twice.
|
||||||
|
# For brevity, drop this test if constructing a fixture is too brittle;
|
||||||
|
# the 837 test above already exercises the same handler pattern.
|
||||||
|
pytest.skip("835 fixture with duplicate CLP01 is fragile; 837 case covers the pattern")
|
||||||
|
```
|
||||||
|
|
||||||
|
Note: the 835 test is intentionally skipped — constructing a minimal valid 835 with duplicate CLP01 is brittle. The 837 case exercises the handler pattern (call `find_existing_batch_for_remit` from a similarly-shaped except block in the 835 handler). If you need 835 coverage, manually inspect by running the API on a real fixture after the implementation lands.
|
||||||
|
|
||||||
|
- [ ] **Step 2: Run test to verify it fails**
|
||||||
|
|
||||||
|
Run: `cd backend && python -m pytest tests/test_api_parse_persists.py::test_409_response_includes_existing_batch_id_for_837 -v`
|
||||||
|
|
||||||
|
Expected: FAIL because the 409 handler does not yet add `existing_batch_id`.
|
||||||
|
|
||||||
|
- [ ] **Step 3: Modify 837 409 handler**
|
||||||
|
|
||||||
|
Edit `backend/src/cyclone/api.py` lines 394-415:
|
||||||
|
|
||||||
|
Replace the `except IntegrityError` block in `parse_837_endpoint` with:
|
||||||
|
|
||||||
|
```python
|
||||||
|
try:
|
||||||
|
store.add(rec, event_bus=request.app.state.event_bus)
|
||||||
|
except IntegrityError as exc:
|
||||||
|
# After migration 0013, the only way an IntegrityError fires here
|
||||||
|
# is a PK collision on claims.id (CLM01) — either within-file or
|
||||||
|
# against a prior batch. Look up the first claim's id and ask the
|
||||||
|
# helper which batch already holds it.
|
||||||
|
first_claim_id = (
|
||||||
|
result.claims[0].claim_id if result.claims else None
|
||||||
|
)
|
||||||
|
existing_batch_id = (
|
||||||
|
store.find_existing_batch_for_claim(first_claim_id)
|
||||||
|
if first_claim_id else None
|
||||||
|
)
|
||||||
|
body = {
|
||||||
|
"error": "Duplicate claim",
|
||||||
|
"detail": (
|
||||||
|
"This file (or one previously ingested with the same "
|
||||||
|
"claim control number) collides with an existing record. "
|
||||||
|
"Inspect the file for duplicate CLM01 control numbers, or "
|
||||||
|
"remove the existing batch before retrying."
|
||||||
|
),
|
||||||
|
"batch_id": rec.id,
|
||||||
|
}
|
||||||
|
if existing_batch_id and existing_batch_id != rec.id:
|
||||||
|
body["existing_batch_id"] = existing_batch_id
|
||||||
|
log.warning("Duplicate claim while persisting batch %s: %s", rec.id, exc)
|
||||||
|
return JSONResponse(status_code=409, content=body)
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 4: Modify 835 409 handler**
|
||||||
|
|
||||||
|
Edit `backend/src/cyclone/api.py` lines 588-602:
|
||||||
|
|
||||||
|
Replace the `except IntegrityError` block in `parse_835_endpoint` with:
|
||||||
|
|
||||||
|
```python
|
||||||
|
try:
|
||||||
|
store.add(rec, event_bus=request.app.state.event_bus)
|
||||||
|
except IntegrityError as exc:
|
||||||
|
first_pcn = (
|
||||||
|
result.claims[0].payer_claim_control_number
|
||||||
|
if result.claims else None
|
||||||
|
)
|
||||||
|
existing_batch_id = (
|
||||||
|
store.find_existing_batch_for_remit(first_pcn)
|
||||||
|
if first_pcn else None
|
||||||
|
)
|
||||||
|
body = {
|
||||||
|
"error": "Duplicate remittance",
|
||||||
|
"detail": (
|
||||||
|
"This 835 file (or one previously ingested with the same "
|
||||||
|
"payer claim control number) collides with an existing record. "
|
||||||
|
"Remove the existing remittance before retrying."
|
||||||
|
),
|
||||||
|
"batch_id": rec.id,
|
||||||
|
}
|
||||||
|
if existing_batch_id and existing_batch_id != rec.id:
|
||||||
|
body["existing_batch_id"] = existing_batch_id
|
||||||
|
log.warning("Duplicate remittance while persisting batch %s: %s", rec.id, exc)
|
||||||
|
return JSONResponse(status_code=409, content=body)
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 5: Run tests to verify they pass**
|
||||||
|
|
||||||
|
Run: `cd backend && python -m pytest tests/test_api_parse_persists.py -v`
|
||||||
|
|
||||||
|
Expected: All PASS, including the new 409 test.
|
||||||
|
|
||||||
|
- [ ] **Step 6: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add backend/src/cyclone/api.py backend/tests/test_api_parse_persists.py
|
||||||
|
git commit -m "feat(api): 409 responses for 837/435 include existing_batch_id"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Phase 2 — Frontend
|
||||||
|
|
||||||
|
### Task 2.1: `ApiError.existingBatchId` + parse837/835 surface it
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `src/lib/api.ts:164-167` (ApiError class)
|
||||||
|
- Modify: `src/lib/api.ts:174-191` (readErrorBody)
|
||||||
|
- Modify: `src/lib/api.ts:280-339` (parse837, parse835)
|
||||||
|
- Create: `src/lib/api.test.ts`
|
||||||
|
|
||||||
|
- [ ] **Step 1: Write the failing test**
|
||||||
|
|
||||||
|
Create `src/lib/api.test.ts`:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
import { describe, it, expect, vi, afterEach } from "vitest";
|
||||||
|
import { ApiError, parse837 } from "@/lib/api";
|
||||||
|
|
||||||
|
afterEach(() => vi.restoreAllMocks());
|
||||||
|
|
||||||
|
describe("ApiError", () => {
|
||||||
|
it("carries existingBatchId when constructed", () => {
|
||||||
|
const e = new ApiError(409, "dup", "BATCH-1");
|
||||||
|
expect(e.status).toBe(409);
|
||||||
|
expect(e.existingBatchId).toBe("BATCH-1");
|
||||||
|
});
|
||||||
|
it("defaults existingBatchId to null", () => {
|
||||||
|
const e = new ApiError(500, "boom");
|
||||||
|
expect(e.existingBatchId).toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("parse837 throws ApiError with existingBatchId", () => {
|
||||||
|
it("extracts existing_batch_id from 409 body", async () => {
|
||||||
|
vi.stubGlobal("import.meta.env", { VITE_API_BASE_URL: "http://x" });
|
||||||
|
const res = new Response(
|
||||||
|
JSON.stringify({
|
||||||
|
error: "Duplicate claim",
|
||||||
|
detail: "collision",
|
||||||
|
batch_id: "NEW",
|
||||||
|
existing_batch_id: "PRIOR",
|
||||||
|
}),
|
||||||
|
{ status: 409, headers: { "Content-Type": "application/json" } },
|
||||||
|
);
|
||||||
|
vi.stubGlobal(
|
||||||
|
"fetch",
|
||||||
|
vi.fn(async () => res),
|
||||||
|
);
|
||||||
|
const file = new File(["x"], "f.txt", { type: "text/plain" });
|
||||||
|
await expect(parse837(file, { onProgress: () => {} })).rejects.toMatchObject({
|
||||||
|
status: 409,
|
||||||
|
existingBatchId: "PRIOR",
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("sets existingBatchId null when body omits it", async () => {
|
||||||
|
vi.stubGlobal("import.meta.env", { VITE_API_BASE_URL: "http://x" });
|
||||||
|
const res = new Response(
|
||||||
|
JSON.stringify({ error: "X", detail: "y", batch_id: "N" }),
|
||||||
|
{ status: 409, headers: { "Content-Type": "application/json" } },
|
||||||
|
);
|
||||||
|
vi.stubGlobal(
|
||||||
|
"fetch",
|
||||||
|
vi.fn(async () => res),
|
||||||
|
);
|
||||||
|
const file = new File(["x"], "f.txt", { type: "text/plain" });
|
||||||
|
await expect(parse837(file)).rejects.toMatchObject({
|
||||||
|
status: 409,
|
||||||
|
existingBatchId: null,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Run test to verify it fails**
|
||||||
|
|
||||||
|
Run: `cd /Users/openclaw/dev/cyclone/.worktrees/claims-unique-fix && npm test -- src/lib/api.test.ts`
|
||||||
|
|
||||||
|
Expected: FAIL — `existingBatchId` not a constructor argument yet; `parse837` throws `Error` not `ApiError`.
|
||||||
|
|
||||||
|
- [ ] **Step 3: Update `ApiError`**
|
||||||
|
|
||||||
|
Edit `src/lib/api.ts` line 164-167:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
export class ApiError extends Error {
|
||||||
|
constructor(
|
||||||
|
public status: number,
|
||||||
|
message: string,
|
||||||
|
public existingBatchId: string | null = null,
|
||||||
|
) {
|
||||||
|
super(message);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 4: Update `readErrorBody` to return parsed body**
|
||||||
|
|
||||||
|
Edit `src/lib/api.ts` line 174-191. Change return type and add JSON parse branch:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
type ErrorBody = {
|
||||||
|
detail?: unknown;
|
||||||
|
error?: unknown;
|
||||||
|
existing_batch_id?: unknown;
|
||||||
|
};
|
||||||
|
|
||||||
|
async function readErrorBody(
|
||||||
|
res: Response,
|
||||||
|
): Promise<{ message: string; existingBatchId: string | null }> {
|
||||||
|
try {
|
||||||
|
const t = await res.text();
|
||||||
|
if (!t) return { message: "", existingBatchId: null };
|
||||||
|
try {
|
||||||
|
const obj = JSON.parse(t) as ErrorBody;
|
||||||
|
let message = "";
|
||||||
|
if (typeof obj.detail === "string") message = obj.detail;
|
||||||
|
else if (typeof obj.error === "string") message = obj.error;
|
||||||
|
else message = t;
|
||||||
|
const existing =
|
||||||
|
typeof obj.existing_batch_id === "string"
|
||||||
|
? obj.existing_batch_id
|
||||||
|
: null;
|
||||||
|
return { message, existingBatchId: existing };
|
||||||
|
} catch {
|
||||||
|
return { message: t, existingBatchId: null };
|
||||||
|
}
|
||||||
|
} catch {
|
||||||
|
return { message: "", existingBatchId: null };
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 5: Update parse837 to throw ApiError**
|
||||||
|
|
||||||
|
Edit `src/lib/api.ts` lines 293-298:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
if (!res.ok) {
|
||||||
|
const { message, existingBatchId } = await readErrorBody(res);
|
||||||
|
throw new ApiError(
|
||||||
|
res.status,
|
||||||
|
`${res.status} ${res.statusText}${message ? ` — ${message}` : ""}`,
|
||||||
|
existingBatchId,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 6: Update parse835 to throw ApiError**
|
||||||
|
|
||||||
|
Edit `src/lib/api.ts` lines 329-334:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
if (!res.ok) {
|
||||||
|
const { message, existingBatchId } = await readErrorBody(res);
|
||||||
|
throw new ApiError(
|
||||||
|
res.status,
|
||||||
|
`${res.status} ${res.statusText}${message ? ` — ${message}` : ""}`,
|
||||||
|
existingBatchId,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 7: Run tests to verify they pass**
|
||||||
|
|
||||||
|
Run: `cd .worktrees/claims-unique-fix && npm test -- src/lib/api.test.ts`
|
||||||
|
|
||||||
|
Expected: PASS for all 4 tests.
|
||||||
|
|
||||||
|
- [ ] **Step 8: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add src/lib/api.ts src/lib/api.test.ts
|
||||||
|
git commit -m "feat(api): ApiError carries existingBatchId; parse837/parse835 surface it"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
### Task 2.2: `Upload.tsx` inline error panel for 409
|
||||||
|
|
||||||
|
**Files:**
|
||||||
|
- Modify: `src/pages/Upload.tsx` (add error state + panel JSX + 409 catch branch)
|
||||||
|
- Create: `src/pages/Upload.test.tsx`
|
||||||
|
|
||||||
|
The panel renders above the streaming results when `uploadError.kind === "duplicate"`. It shows:
|
||||||
|
- A 409 badge
|
||||||
|
- "Duplicate claim — file not ingested" title
|
||||||
|
- Detail mentioning the file and (if `existingBatchId` is set) "Open the existing batch to compare, or pick a different file."
|
||||||
|
- A button linking to `/batches/{existingBatchId}` if present
|
||||||
|
- A "Pick a different file" ghost button that calls `pickFile(null)` and clears the error state
|
||||||
|
|
||||||
|
- [ ] **Step 1: Write the failing tests**
|
||||||
|
|
||||||
|
Create `src/pages/Upload.test.tsx`:
|
||||||
|
|
||||||
|
```tsx
|
||||||
|
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||||
|
import { render, screen, fireEvent } from "@testing-library/react";
|
||||||
|
import { MemoryRouter, Route, Routes } from "react-router-dom";
|
||||||
|
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||||
|
import * as apiModule from "@/lib/api";
|
||||||
|
import { ApiError } from "@/lib/api";
|
||||||
|
import { Upload } from "@/pages/Upload";
|
||||||
|
|
||||||
|
// We mock the api module so the component doesn't actually call fetch.
|
||||||
|
vi.mock("@/lib/api", async () => {
|
||||||
|
const actual = await vi.importActual<typeof apiModule>("@/lib/api");
|
||||||
|
return { ...actual, parse837: vi.fn(), parse835: vi.fn() };
|
||||||
|
});
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.clearAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
function renderUpload() {
|
||||||
|
const qc = new QueryClient({ defaultOptions: { queries: { retry: false } } });
|
||||||
|
return render(
|
||||||
|
<QueryClientProvider client={qc}>
|
||||||
|
<MemoryRouter initialEntries={["/upload"]}>
|
||||||
|
<Routes>
|
||||||
|
<Route path="/upload" element={<Upload />} />
|
||||||
|
<Route path="/batches/:id" element={<div data-testid="batch-page" />} />
|
||||||
|
</Routes>
|
||||||
|
</MemoryRouter>
|
||||||
|
</QueryClientProvider>,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("Upload error panel", () => {
|
||||||
|
it("renders panel with link when 409 carries existingBatchId", async () => {
|
||||||
|
vi.mocked(apiModule.parse837).mockRejectedValueOnce(
|
||||||
|
new ApiError(409, "409 Conflict — dup", "PRIOR-BATCH"),
|
||||||
|
);
|
||||||
|
renderUpload();
|
||||||
|
// Find the file input and upload a file.
|
||||||
|
const input = document.querySelector('input[type="file"]') as HTMLInputElement;
|
||||||
|
expect(input).toBeTruthy();
|
||||||
|
const file = new File(["x"], "test.txt", { type: "text/plain" });
|
||||||
|
fireEvent.change(input, { target: { files: [file] } });
|
||||||
|
// Wait for the panel to render.
|
||||||
|
const link = await screen.findByRole("button", { name: /open existing batch/i });
|
||||||
|
expect(link).toBeInTheDocument();
|
||||||
|
expect(screen.getByText(/duplicate claim/i)).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("renders panel without link when 409 omits existingBatchId", async () => {
|
||||||
|
vi.mocked(apiModule.parse837).mockRejectedValueOnce(
|
||||||
|
new ApiError(409, "409 Conflict — dup", null),
|
||||||
|
);
|
||||||
|
renderUpload();
|
||||||
|
const input = document.querySelector('input[type="file"]') as HTMLInputElement;
|
||||||
|
const file = new File(["x"], "test.txt", { type: "text/plain" });
|
||||||
|
fireEvent.change(input, { target: { files: [file] } });
|
||||||
|
await screen.findByText(/duplicate claim/i);
|
||||||
|
expect(
|
||||||
|
screen.queryByRole("button", { name: /open existing batch/i }),
|
||||||
|
).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does NOT render panel for non-409 errors", async () => {
|
||||||
|
vi.mocked(apiModule.parse837).mockRejectedValueOnce(
|
||||||
|
new ApiError(400, "bad file"),
|
||||||
|
);
|
||||||
|
renderUpload();
|
||||||
|
const input = document.querySelector('input[type="file"]') as HTMLInputElement;
|
||||||
|
const file = new File(["x"], "test.txt", { type: "text/plain" });
|
||||||
|
fireEvent.change(input, { target: { files: [file] } });
|
||||||
|
// Give the async error handler a tick.
|
||||||
|
await new Promise((r) => setTimeout(r, 50));
|
||||||
|
expect(screen.queryByText(/duplicate claim/i)).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("'Pick a different file' button clears error", async () => {
|
||||||
|
vi.mocked(apiModule.parse837).mockRejectedValueOnce(
|
||||||
|
new ApiError(409, "409 Conflict — dup", "PRIOR"),
|
||||||
|
);
|
||||||
|
renderUpload();
|
||||||
|
const input = document.querySelector('input[type="file"]') as HTMLInputElement;
|
||||||
|
const file = new File(["x"], "test.txt", { type: "text/plain" });
|
||||||
|
fireEvent.change(input, { target: { files: [file] } });
|
||||||
|
const clearBtn = await screen.findByRole("button", { name: /pick a different file/i });
|
||||||
|
fireEvent.click(clearBtn);
|
||||||
|
expect(screen.queryByText(/duplicate claim/i)).toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 2: Run test to verify it fails**
|
||||||
|
|
||||||
|
Run: `cd .worktrees/claims-unique-fix && npm test -- src/pages/Upload.test.tsx`
|
||||||
|
|
||||||
|
Expected: FAIL — no error panel exists yet, all 4 tests fail.
|
||||||
|
|
||||||
|
- [ ] **Step 3: Add error state and 409 catch branch in Upload.tsx**
|
||||||
|
|
||||||
|
Edit `src/pages/Upload.tsx`:
|
||||||
|
|
||||||
|
1. At the top imports, add:
|
||||||
|
```tsx
|
||||||
|
import { ApiError } from "@/lib/api";
|
||||||
|
import { useNavigate } from "react-router-dom";
|
||||||
|
```
|
||||||
|
|
||||||
|
2. Find the existing error-handling catch block (around line 683-687) and replace with:
|
||||||
|
|
||||||
|
```tsx
|
||||||
|
} catch (err) {
|
||||||
|
if (err instanceof ApiError && err.status === 409) {
|
||||||
|
setUploadError({
|
||||||
|
kind: "duplicate",
|
||||||
|
existingBatchId: err.existingBatchId,
|
||||||
|
filename: file.name,
|
||||||
|
});
|
||||||
|
toast.error("Duplicate claim — file not ingested");
|
||||||
|
} else {
|
||||||
|
toast.error(
|
||||||
|
err instanceof Error ? err.message : "Failed to parse file"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
3. Add state declaration near the other useState calls:
|
||||||
|
```tsx
|
||||||
|
type UploadError =
|
||||||
|
| { kind: "duplicate"; existingBatchId: string | null; filename: string };
|
||||||
|
const [uploadError, setUploadError] = useState<UploadError | null>(null);
|
||||||
|
const navigate = useNavigate();
|
||||||
|
```
|
||||||
|
|
||||||
|
4. In the JSX, add the inline panel above the streaming-results section. Find the place where streaming results render (search for "streamDelay" or the section that shows the parsed batch) and insert just before it:
|
||||||
|
|
||||||
|
```tsx
|
||||||
|
{uploadError && uploadError.kind === "duplicate" ? (
|
||||||
|
<div
|
||||||
|
role="alert"
|
||||||
|
data-testid="duplicate-error-panel"
|
||||||
|
className="error-panel mx-auto max-w-3xl rounded-md border border-destructive/40 bg-destructive/5 p-4"
|
||||||
|
>
|
||||||
|
<div className="flex items-center gap-2">
|
||||||
|
<span className="inline-flex items-center rounded-md bg-destructive px-2 py-0.5 text-xs font-semibold text-destructive-foreground">
|
||||||
|
409
|
||||||
|
</span>
|
||||||
|
<span className="font-semibold">Duplicate claim — file not ingested</span>
|
||||||
|
</div>
|
||||||
|
<p className="mt-2 text-sm text-muted-foreground">
|
||||||
|
<span className="font-mono">{uploadError.filename}</span> collides with an
|
||||||
|
existing record.
|
||||||
|
{uploadError.existingBatchId
|
||||||
|
? " Open the existing batch to compare, or pick a different file."
|
||||||
|
: " Pick a different file."}
|
||||||
|
</p>
|
||||||
|
<div className="mt-3 flex gap-2">
|
||||||
|
{uploadError.existingBatchId ? (
|
||||||
|
<Button
|
||||||
|
onClick={() => navigate(`/batches/${uploadError.existingBatchId}`)}
|
||||||
|
>
|
||||||
|
Open existing batch →
|
||||||
|
</Button>
|
||||||
|
) : null}
|
||||||
|
<Button
|
||||||
|
variant="ghost"
|
||||||
|
onClick={() => {
|
||||||
|
setUploadError(null);
|
||||||
|
pickFile(null);
|
||||||
|
}}
|
||||||
|
>
|
||||||
|
Pick a different file
|
||||||
|
</Button>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
) : null}
|
||||||
|
```
|
||||||
|
|
||||||
|
- [ ] **Step 4: Run tests to verify they pass**
|
||||||
|
|
||||||
|
Run: `cd .worktrees/claims-unique-fix && npm test -- src/pages/Upload.test.tsx`
|
||||||
|
|
||||||
|
Expected: PASS for all 4 tests.
|
||||||
|
|
||||||
|
- [ ] **Step 5: Commit**
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add src/pages/Upload.tsx src/pages/Upload.test.tsx
|
||||||
|
git commit -m "feat(upload): inline 409 error panel with existing-batch link"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Phase 3 — End-to-end verification
|
||||||
|
|
||||||
|
### Task 3.1: Repro the original 409 with the actual multi-claim 837P file
|
||||||
|
|
||||||
|
**Files:** none (manual verification)
|
||||||
|
|
||||||
|
- [ ] **Step 1: Start backend + frontend**
|
||||||
|
|
||||||
|
In one terminal: `cd backend && uvicorn cyclone.api:app --reload --port 8000`
|
||||||
|
In another: `cd .worktrees/claims-unique-fix && npm run dev`
|
||||||
|
|
||||||
|
- [ ] **Step 2: Upload the reproducer file**
|
||||||
|
|
||||||
|
Use `docs/prodfiles/837p-from-axiscare/tp11525703-837P-20260618153339862-1of1.txt` (93696 bytes, 28 NM1*IL segments, multi-claim member-id collisions).
|
||||||
|
|
||||||
|
Expected: 200 OK + batch persisted with multiple claims having identical `member_id` (this was previously 409).
|
||||||
|
|
||||||
|
- [ ] **Step 3: Re-upload the same file to trigger 409**
|
||||||
|
|
||||||
|
Expected: 409 with `existing_batch_id` pointing at the first batch.
|
||||||
|
|
||||||
|
- [ ] **Step 4: Verify inline panel in the browser**
|
||||||
|
|
||||||
|
Open the upload page, drag the file in, verify the panel appears with the "Open existing batch →" link.
|
||||||
|
|
||||||
|
- [ ] **Step 5: Commit any tweaks**
|
||||||
|
|
||||||
|
If the panel needed any styling tweaks (e.g. spacing, colors), commit them:
|
||||||
|
|
||||||
|
```bash
|
||||||
|
git add src/pages/Upload.tsx
|
||||||
|
git commit -m "polish(upload): 409 error panel visual tweaks"
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Self-review checklist (run after writing the plan)
|
||||||
|
|
||||||
|
1. **Spec coverage** — Each section of the spec maps to a task:
|
||||||
|
- §3 Schema migration → Task 1.1
|
||||||
|
- §4 Store helpers → Task 1.2
|
||||||
|
- §5 API change → Task 1.3
|
||||||
|
- §6 Frontend `ApiError.existingBatchId` → Task 2.1
|
||||||
|
- §6 Frontend `Upload.tsx` panel → Task 2.2
|
||||||
|
- §10 Test plan (9 tests) → All 9 covered (5 backend + 4 frontend)
|
||||||
|
|
||||||
|
2. **Placeholder scan** — No "TBD", "TODO", "implement later" markers. The only intentional skip is the 835 duplicate-CLP01 test, called out explicitly.
|
||||||
|
|
||||||
|
3. **Type consistency** —
|
||||||
|
- `find_existing_batch_for_claim` / `find_existing_batch_for_remit` return `str | None` everywhere.
|
||||||
|
- `ApiError.existingBatchId` is `string | null` in TS and `str | None` everywhere it's referenced.
|
||||||
|
- `setUploadError` payload shape `{ kind: "duplicate"; existingBatchId: string | null; filename: string }` is consistent in JSX and the catch branch.
|
||||||
|
|
||||||
|
4. **Risk acknowledged** — Migration reversibility, loss of uniqueness, race window are all documented in spec §9 and reflected in test scope (we don't test the race condition).
|
||||||
+386
@@ -0,0 +1,386 @@
|
|||||||
|
# Drop `UNIQUE(batch_id, patient_control_number)` on Claims + Robust 409 UX
|
||||||
|
|
||||||
|
**Date:** 2026-06-21
|
||||||
|
**Branch:** `main`
|
||||||
|
**Status:** Draft (brainstorming approved, awaiting writing-plans)
|
||||||
|
**Scope:** Backend schema + minimal frontend error handling for collision 409.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 1. Why this exists
|
||||||
|
|
||||||
|
Today, every multi-claim 837P file in `docs/prodfiles/837p-from-axiscare/` and
|
||||||
|
`docs/prodfiles/FromHPE/` returns **HTTP 409** on upload. The cause:
|
||||||
|
|
||||||
|
* The `claims` table has `UNIQUE(batch_id, patient_control_number)` declared
|
||||||
|
inline in `CREATE TABLE` (auto-index `sqlite_autoindex_claims_2`).
|
||||||
|
* `store._claim_837_row` populates `patient_control_number` from
|
||||||
|
`claim.subscriber.member_id` (the subscriber's insurance member id),
|
||||||
|
not from the CLM01 segment value.
|
||||||
|
* A real 837P submission routinely contains many `CLM*` segments per
|
||||||
|
subscriber (a member seeing multiple providers on the same day). All
|
||||||
|
those rows share the same `member_id` → same `patient_control_number`.
|
||||||
|
Within one batch, the constraint fires on claim #2.
|
||||||
|
|
||||||
|
Migration `0003_drop_claims_remits_unique_constraints.sql` already exists
|
||||||
|
with the right intent but is **buggy**: it does
|
||||||
|
`DROP INDEX IF EXISTS uq_claims_batch_pcn`, but the constraint is inline,
|
||||||
|
not a named index — so the `DROP INDEX` is a no-op and the constraint
|
||||||
|
survives.
|
||||||
|
|
||||||
|
The 409 error message ("duplicate CLM01 control numbers") is also
|
||||||
|
misleading because the column stores `member_id`, not CLM01.
|
||||||
|
|
||||||
|
This SP fixes both: drops the constraint properly via SQLite table
|
||||||
|
recreation, and gives the upload page a structured error panel for the
|
||||||
|
collision case so the operator can find the existing batch in one click.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 2. Operator surface
|
||||||
|
|
||||||
|
| Surface | Change |
|
||||||
|
|---|---|
|
||||||
|
| DB | New migration `0013_drop_claims_unique_constraint.sql`; idempotent. |
|
||||||
|
| Python | `store.find_existing_batch_for_claim(claim_id)` new helper. |
|
||||||
|
| API | `POST /api/parse-837` and `/api/parse-835` 409 responses gain `existing_batch_id`. |
|
||||||
|
| UI | `/upload` page renders an inline error panel for 409 with a link to the existing batch. |
|
||||||
|
| Tests | New migration test, store helper test, API test, component test. |
|
||||||
|
|
||||||
|
No CLI / settings changes.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 3. Schema migration
|
||||||
|
|
||||||
|
`backend/src/cyclone/migrations/0013_drop_claims_unique_constraint.sql`:
|
||||||
|
|
||||||
|
```sql
|
||||||
|
-- version: 13
|
||||||
|
-- Drop the inline UNIQUE(batch_id, patient_control_number) on claims.
|
||||||
|
-- Migration 0003 attempted DROP INDEX IF EXISTS uq_claims_batch_pcn but
|
||||||
|
-- the constraint is inline in CREATE TABLE, so the drop was a no-op.
|
||||||
|
-- The only way to remove an inline UNIQUE in SQLite is table recreation.
|
||||||
|
--
|
||||||
|
-- X12 837P allows any number of CLM segments per 2000B subscriber loop;
|
||||||
|
-- claim identity is provided by the primary key (claims.id = CLM01).
|
||||||
|
-- The remittances table had a parallel constraint already removed in 0003
|
||||||
|
-- (because that one WAS a named index), so this migration only touches
|
||||||
|
-- claims.
|
||||||
|
--
|
||||||
|
-- The migration runner (db_migrate.py) wraps each .sql file in an
|
||||||
|
-- implicit transaction via engine.begin(), so we MUST NOT use BEGIN/COMMIT
|
||||||
|
-- inside the file (nested transactions fail in SQLite). We defer FK
|
||||||
|
-- enforcement with PRAGMA defer_foreign_keys instead of turning FKs off
|
||||||
|
-- (which is a no-op inside a transaction in SQLite). The deferred
|
||||||
|
-- checks fire at commit and validate against the renamed claims table.
|
||||||
|
|
||||||
|
PRAGMA defer_foreign_keys = ON;
|
||||||
|
|
||||||
|
CREATE TABLE claims_new (
|
||||||
|
id TEXT PRIMARY KEY,
|
||||||
|
batch_id TEXT NOT NULL REFERENCES batches(id) ON DELETE CASCADE,
|
||||||
|
patient_control_number TEXT NOT NULL,
|
||||||
|
service_date_from DATE,
|
||||||
|
service_date_to DATE,
|
||||||
|
charge_amount NUMERIC(12, 2) NOT NULL DEFAULT 0,
|
||||||
|
provider_npi TEXT,
|
||||||
|
payer_id TEXT,
|
||||||
|
state TEXT NOT NULL DEFAULT 'submitted',
|
||||||
|
state_before_reversal TEXT,
|
||||||
|
matched_remittance_id TEXT REFERENCES remittances(id),
|
||||||
|
raw_json TEXT,
|
||||||
|
rejection_reason TEXT,
|
||||||
|
rejected_at TIMESTAMP,
|
||||||
|
resubmit_count INTEGER NOT NULL DEFAULT 0,
|
||||||
|
state_changed_at TIMESTAMP,
|
||||||
|
payer_rejected_at TEXT,
|
||||||
|
payer_rejected_reason TEXT,
|
||||||
|
payer_rejected_status_code TEXT,
|
||||||
|
payer_rejected_by_277ca_id TEXT,
|
||||||
|
payer_rejected_acknowledged_at TEXT,
|
||||||
|
payer_rejected_acknowledged_actor TEXT
|
||||||
|
-- NO UNIQUE (batch_id, patient_control_number) — removed.
|
||||||
|
);
|
||||||
|
|
||||||
|
INSERT INTO claims_new SELECT * FROM claims;
|
||||||
|
DROP TABLE claims;
|
||||||
|
ALTER TABLE claims_new RENAME TO claims;
|
||||||
|
|
||||||
|
-- Recreate secondary indexes (same names, same columns as initial schema
|
||||||
|
-- plus later migrations).
|
||||||
|
CREATE INDEX ix_claims_state ON claims(state);
|
||||||
|
CREATE INDEX ix_claims_patient_control_number ON claims(patient_control_number);
|
||||||
|
CREATE INDEX ix_claims_service_date_from ON claims(service_date_from);
|
||||||
|
CREATE INDEX ix_claims_state_changed_at ON claims(state, state_changed_at);
|
||||||
|
CREATE INDEX idx_claims_payer_rejected_at ON claims(payer_rejected_at);
|
||||||
|
CREATE INDEX idx_claims_payer_rejected_unack
|
||||||
|
ON claims(payer_rejected_at)
|
||||||
|
WHERE payer_rejected_acknowledged_at IS NULL;
|
||||||
|
```
|
||||||
|
|
||||||
|
**Note on the `db.py` ORM model:** `backend/src/cyclone/db.py` declares
|
||||||
|
the `Claim` ORM model with `UNIQUE(batch_id, patient_control_number)` in
|
||||||
|
`__table_args__`. After this migration the DB no longer enforces that
|
||||||
|
constraint, so the ORM declaration becomes aspirational. We leave it in
|
||||||
|
place as documentation (and as a guard if the DB ever gets rebuilt from
|
||||||
|
ORM on a fresh schema). No code reads through it; the dedup happens via
|
||||||
|
PK on `claims.id` in `store.add`.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 4. Store helper
|
||||||
|
|
||||||
|
New function in `backend/src/cyclone/store.py`:
|
||||||
|
|
||||||
|
```python
|
||||||
|
def find_existing_batch_for_claim(claim_id: str) -> str | None:
|
||||||
|
"""Return the batch_id of the first batch containing this CLM01, or None."""
|
||||||
|
from cyclone import db
|
||||||
|
from cyclone.db import Claim
|
||||||
|
with db.SessionLocal()() as s:
|
||||||
|
row = s.execute(
|
||||||
|
select(Claim.batch_id).where(Claim.id == claim_id).limit(1)
|
||||||
|
).first()
|
||||||
|
return row[0] if row else None
|
||||||
|
|
||||||
|
|
||||||
|
def find_existing_batch_for_remit(payer_claim_control_number: str) -> str | None:
|
||||||
|
"""Return the batch_id of the first batch containing this CLP01, or None."""
|
||||||
|
from cyclone import db
|
||||||
|
from cyclone.db import Remittance
|
||||||
|
with db.SessionLocal()() as s:
|
||||||
|
row = s.execute(
|
||||||
|
select(Remittance.batch_id)
|
||||||
|
.where(Remittance.id == payer_claim_control_number)
|
||||||
|
.limit(1)
|
||||||
|
).first()
|
||||||
|
return row[0] if row else None
|
||||||
|
```
|
||||||
|
|
||||||
|
Both pure reads; no transaction management needed.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 5. API change
|
||||||
|
|
||||||
|
`backend/src/cyclone/api.py`:
|
||||||
|
|
||||||
|
### 837 path (line 394-415)
|
||||||
|
|
||||||
|
After the UNIQUE constraint is dropped in migration 0013, an `IntegrityError`
|
||||||
|
in `store.add` can only originate from the PK on `claims.id` (CLM01) —
|
||||||
|
either two claims in the same file share CLM01 (rare) or the same CLM01
|
||||||
|
exists in a prior batch. The handler picks the first claim from
|
||||||
|
`result.claims` and asks the helper whether a prior batch already holds
|
||||||
|
that CLM01:
|
||||||
|
|
||||||
|
```python
|
||||||
|
except IntegrityError as exc:
|
||||||
|
first_claim_id = result.claims[0].claim_id if result.claims else None
|
||||||
|
existing_batch_id = (
|
||||||
|
store.find_existing_batch_for_claim(first_claim_id)
|
||||||
|
if first_claim_id else None
|
||||||
|
)
|
||||||
|
body = {
|
||||||
|
"error": "Duplicate claim",
|
||||||
|
"detail": (
|
||||||
|
"This file (or one previously ingested with the same "
|
||||||
|
"claim control number) collides with an existing record. "
|
||||||
|
"Inspect the file for duplicate CLM01 control numbers, or "
|
||||||
|
"remove the existing batch before retrying."
|
||||||
|
),
|
||||||
|
"batch_id": rec.id,
|
||||||
|
}
|
||||||
|
if existing_batch_id and existing_batch_id != rec.id:
|
||||||
|
body["existing_batch_id"] = existing_batch_id
|
||||||
|
log.warning("Duplicate claim while persisting batch %s: %s", rec.id, exc)
|
||||||
|
return JSONResponse(status_code=409, content=body)
|
||||||
|
```
|
||||||
|
|
||||||
|
The `existing_batch_id != rec.id` guard avoids surfacing the just-failed
|
||||||
|
batch as a "previous" batch (it never persisted).
|
||||||
|
|
||||||
|
### 835 path (line 588-602)
|
||||||
|
|
||||||
|
Same pattern with `find_existing_batch_for_remit` and the first remit's
|
||||||
|
`payer_claim_control_number` (`result.claims[0].payer_claim_control_number`):
|
||||||
|
|
||||||
|
```python
|
||||||
|
except IntegrityError as exc:
|
||||||
|
first_pcn = result.claims[0].payer_claim_control_number if result.claims else None
|
||||||
|
existing_batch_id = (
|
||||||
|
store.find_existing_batch_for_remit(first_pcn)
|
||||||
|
if first_pcn else None
|
||||||
|
)
|
||||||
|
body = {
|
||||||
|
"error": "Duplicate remittance",
|
||||||
|
"detail": (
|
||||||
|
"This 835 file (or one previously ingested with the same "
|
||||||
|
"payer claim control number) collides with an existing record. "
|
||||||
|
"Remove the existing remittance before retrying."
|
||||||
|
),
|
||||||
|
"batch_id": rec.id,
|
||||||
|
}
|
||||||
|
if existing_batch_id and existing_batch_id != rec.id:
|
||||||
|
body["existing_batch_id"] = existing_batch_id
|
||||||
|
log.warning("Duplicate remittance while persisting batch %s: %s", rec.id, exc)
|
||||||
|
return JSONResponse(status_code=409, content=body)
|
||||||
|
```
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 6. Frontend change
|
||||||
|
|
||||||
|
### `src/lib/api.ts`
|
||||||
|
|
||||||
|
`ApiError` carries an optional `existingBatchId`:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
export class ApiError extends Error {
|
||||||
|
constructor(
|
||||||
|
public status: number,
|
||||||
|
message: string,
|
||||||
|
public existingBatchId: string | null = null,
|
||||||
|
) {
|
||||||
|
super(message);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
`readErrorBody()` parses JSON and, when status ≥ 400, attempts to extract
|
||||||
|
`existing_batch_id` and pass it through. `parse837`/`parse835` throw
|
||||||
|
`new ApiError(res.status, detail, existingBatchId)`.
|
||||||
|
|
||||||
|
### `src/pages/Upload.tsx`
|
||||||
|
|
||||||
|
New local state:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
type UploadError = {
|
||||||
|
kind: "duplicate";
|
||||||
|
existingBatchId: string | null;
|
||||||
|
filename: string;
|
||||||
|
};
|
||||||
|
const [uploadError, setUploadError] = useState<UploadError | null>(null);
|
||||||
|
```
|
||||||
|
|
||||||
|
On mutation error:
|
||||||
|
|
||||||
|
```typescript
|
||||||
|
} catch (err) {
|
||||||
|
if (err instanceof ApiError && err.status === 409) {
|
||||||
|
setUploadError({
|
||||||
|
kind: "duplicate",
|
||||||
|
existingBatchId: err.existingBatchId,
|
||||||
|
filename: file.name,
|
||||||
|
});
|
||||||
|
toast.error("Duplicate claim — file not ingested");
|
||||||
|
} else {
|
||||||
|
toast.error(err instanceof Error ? err.message : "Failed to parse file");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
Inline error panel JSX (renders above streaming results when
|
||||||
|
`uploadError` is set):
|
||||||
|
|
||||||
|
```tsx
|
||||||
|
{uploadError ? (
|
||||||
|
<div className="error-panel">
|
||||||
|
<Badge variant="destructive">409</Badge>
|
||||||
|
<div className="error-title">Duplicate claim — file not ingested</div>
|
||||||
|
<div className="error-detail">
|
||||||
|
{filename} collides with an existing record.
|
||||||
|
{existingBatchId
|
||||||
|
? " Open the existing batch to compare, or pick a different file."
|
||||||
|
: " Pick a different file."}
|
||||||
|
</div>
|
||||||
|
<div className="error-actions">
|
||||||
|
{existingBatchId ? (
|
||||||
|
<Button onClick={() => navigate(`/batches/${existingBatchId}`)}>
|
||||||
|
Open existing batch →
|
||||||
|
</Button>
|
||||||
|
) : null}
|
||||||
|
<Button variant="ghost" onClick={() => { pickFile(null); }}>
|
||||||
|
Pick a different file
|
||||||
|
</Button>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
) : null}
|
||||||
|
```
|
||||||
|
|
||||||
|
Styling matches the existing paper-toned surface (slate/parchment) per
|
||||||
|
the hybrid dark/paper treatment used elsewhere in the app.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 7. Files changed
|
||||||
|
|
||||||
|
* `backend/src/cyclone/migrations/0013_drop_claims_unique_constraint.sql` — new
|
||||||
|
* `backend/src/cyclone/store.py` — `find_existing_batch_for_claim`,
|
||||||
|
`find_existing_batch_for_remit`
|
||||||
|
* `backend/src/cyclone/api.py` — both 409 handlers add
|
||||||
|
`existing_batch_id` lookup
|
||||||
|
* `src/lib/api.ts` — `ApiError.existingBatchId`
|
||||||
|
* `src/pages/Upload.tsx` — error state + inline panel
|
||||||
|
* `src/pages/Upload.test.tsx` — new test file
|
||||||
|
* `backend/tests/test_api_parse_persists.py` — new test cases
|
||||||
|
* `backend/tests/test_db_migrate.py` — new migration test
|
||||||
|
* `backend/tests/test_store.py` — new helper tests
|
||||||
|
|
||||||
|
No new dependencies. No config changes.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 8. Out of scope
|
||||||
|
|
||||||
|
* A "Replace existing batch" destructive action — requires
|
||||||
|
`DELETE /api/batches/{id}` and reconciliation cascade handling; deferred.
|
||||||
|
* General 4xx error UI for other statuses (empty file, parse error,
|
||||||
|
validation errors). Those already surface in toasts and the streaming
|
||||||
|
view; a future SP can generalize the inline panel pattern.
|
||||||
|
* Renaming `patient_control_number` to `subscriber_member_id` to match
|
||||||
|
what it actually stores. Out of scope for this fix; tracked separately
|
||||||
|
if it becomes a source of confusion.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 9. Risk
|
||||||
|
|
||||||
|
* **Migration reversibility**: SQLite has no `DROP CONSTRAINT`; the
|
||||||
|
recreation is destructive to schema (but not to data — `INSERT INTO
|
||||||
|
claims_new SELECT * FROM claims` preserves every row). If the
|
||||||
|
migration fails mid-way, the implicit transaction (`engine.begin()`
|
||||||
|
in `db_migrate.py`) rolls back. `PRAGMA defer_foreign_keys = ON` is
|
||||||
|
required because SQLite otherwise can't drop a table that other
|
||||||
|
tables reference (`remittances.claim_id`, `matches.claim_id`,
|
||||||
|
`line_reconciliations.claim_id`, `activity_events.claim_id`). The
|
||||||
|
deferred checks fire at commit and validate against the renamed
|
||||||
|
`claims` table.
|
||||||
|
* **Loss of uniqueness**: after the migration, two claims in one batch
|
||||||
|
*can* share a `patient_control_number`. This is the intended behavior.
|
||||||
|
Claim identity is still unique via `claims.id` (CLM01, PK) and the
|
||||||
|
existing dedup check in `store.add` (`s.get(Claim, claim.claim_id)`).
|
||||||
|
* **`existing_batch_id` race**: the helper runs after the failed
|
||||||
|
`store.add` transaction has rolled back. Between rollback and helper
|
||||||
|
call, another writer could delete the colliding claim. Result: 409
|
||||||
|
fires with no `existing_batch_id`; UI shows panel without link.
|
||||||
|
Acceptable — the panel still explains the collision and offers
|
||||||
|
"Pick a different file."
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## 10. Test plan
|
||||||
|
|
||||||
|
| Test | Asserts |
|
||||||
|
|---|---|
|
||||||
|
| `test_db_migrate.py::test_drop_claims_unique_constraint` | After running migrations from v12, `user_version=13`, no `*claims*unique*` index exists, two rows with same `(batch_id, patient_control_number)` insert cleanly. |
|
||||||
|
| `test_db_migrate.py::test_migration_idempotent` | Running migrations on a v13 DB is a no-op. |
|
||||||
|
| `test_store.py::test_find_existing_batch_for_claim` | Helper returns None for unknown, returns batch_id for known, deterministic on duplicates. |
|
||||||
|
| `test_api_parse_persists.py::test_409_response_includes_existing_batch_id` | Upload duplicate; assert body has `existing_batch_id` pointing to the right batch. |
|
||||||
|
| `test_api_parse_persists.py::test_multi_claim_batch_with_duplicate_member_id_succeeds` | Upload a file with duplicate `member_id` claims; assert 200, all claims persisted. |
|
||||||
|
| `Upload.test.tsx::test_error_panel_renders_on_409_with_link` | Mock `useParse` to throw `ApiError(409, ..., existingBatchId)`; assert panel visible, link present. |
|
||||||
|
| `Upload.test.tsx::test_error_panel_renders_on_409_without_link` | Mock 409 with `existingBatchId: null`; assert panel visible, no link. |
|
||||||
|
| `Upload.test.tsx::test_no_panel_on_non_409` | Mock 400; assert panel absent, only toast. |
|
||||||
|
| `Upload.test.tsx::test_pick_different_clears_error` | Click button; assert `pickFile(null)` called and `errorState` cleared. |
|
||||||
Generated
+62
@@ -12,6 +12,7 @@
|
|||||||
"@radix-ui/react-label": "^2.1.0",
|
"@radix-ui/react-label": "^2.1.0",
|
||||||
"@radix-ui/react-select": "^2.1.2",
|
"@radix-ui/react-select": "^2.1.2",
|
||||||
"@radix-ui/react-slot": "^1.1.0",
|
"@radix-ui/react-slot": "^1.1.0",
|
||||||
|
"@radix-ui/react-tabs": "^1.1.15",
|
||||||
"@tanstack/react-query": "^5.101.0",
|
"@tanstack/react-query": "^5.101.0",
|
||||||
"ansi-styles": "^6.2.3",
|
"ansi-styles": "^6.2.3",
|
||||||
"class-variance-authority": "^0.7.0",
|
"class-variance-authority": "^0.7.0",
|
||||||
@@ -1331,6 +1332,37 @@
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
"node_modules/@radix-ui/react-roving-focus": {
|
||||||
|
"version": "1.1.13",
|
||||||
|
"resolved": "https://registry.npmjs.org/@radix-ui/react-roving-focus/-/react-roving-focus-1.1.13.tgz",
|
||||||
|
"integrity": "sha512-9gkwneI0guf8JDmrFxPjJF6Ozzgioyw+/lonYNCwefS9ZHA05er0BVHiXr+LbWGHxUfczvMY6G1oiZZi1VzjRw==",
|
||||||
|
"license": "MIT",
|
||||||
|
"dependencies": {
|
||||||
|
"@radix-ui/primitive": "1.1.4",
|
||||||
|
"@radix-ui/react-collection": "1.1.10",
|
||||||
|
"@radix-ui/react-compose-refs": "1.1.3",
|
||||||
|
"@radix-ui/react-context": "1.1.4",
|
||||||
|
"@radix-ui/react-direction": "1.1.2",
|
||||||
|
"@radix-ui/react-id": "1.1.2",
|
||||||
|
"@radix-ui/react-primitive": "2.1.6",
|
||||||
|
"@radix-ui/react-use-callback-ref": "1.1.2",
|
||||||
|
"@radix-ui/react-use-controllable-state": "1.2.3"
|
||||||
|
},
|
||||||
|
"peerDependencies": {
|
||||||
|
"@types/react": "*",
|
||||||
|
"@types/react-dom": "*",
|
||||||
|
"react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc",
|
||||||
|
"react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc"
|
||||||
|
},
|
||||||
|
"peerDependenciesMeta": {
|
||||||
|
"@types/react": {
|
||||||
|
"optional": true
|
||||||
|
},
|
||||||
|
"@types/react-dom": {
|
||||||
|
"optional": true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
},
|
||||||
"node_modules/@radix-ui/react-select": {
|
"node_modules/@radix-ui/react-select": {
|
||||||
"version": "2.3.1",
|
"version": "2.3.1",
|
||||||
"resolved": "https://registry.npmjs.org/@radix-ui/react-select/-/react-select-2.3.1.tgz",
|
"resolved": "https://registry.npmjs.org/@radix-ui/react-select/-/react-select-2.3.1.tgz",
|
||||||
@@ -1393,6 +1425,36 @@
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
"node_modules/@radix-ui/react-tabs": {
|
||||||
|
"version": "1.1.15",
|
||||||
|
"resolved": "https://registry.npmjs.org/@radix-ui/react-tabs/-/react-tabs-1.1.15.tgz",
|
||||||
|
"integrity": "sha512-kxc9gI6/HfcU4nfMMVS3AmQK414kbU1IE6UCJmMmxjhO3cRPXOyYnmvyKD+ODt7q56nRq9l7Wovi6uaGwKgMlg==",
|
||||||
|
"license": "MIT",
|
||||||
|
"dependencies": {
|
||||||
|
"@radix-ui/primitive": "1.1.4",
|
||||||
|
"@radix-ui/react-context": "1.1.4",
|
||||||
|
"@radix-ui/react-direction": "1.1.2",
|
||||||
|
"@radix-ui/react-id": "1.1.2",
|
||||||
|
"@radix-ui/react-presence": "1.1.6",
|
||||||
|
"@radix-ui/react-primitive": "2.1.6",
|
||||||
|
"@radix-ui/react-roving-focus": "1.1.13",
|
||||||
|
"@radix-ui/react-use-controllable-state": "1.2.3"
|
||||||
|
},
|
||||||
|
"peerDependencies": {
|
||||||
|
"@types/react": "*",
|
||||||
|
"@types/react-dom": "*",
|
||||||
|
"react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc",
|
||||||
|
"react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc"
|
||||||
|
},
|
||||||
|
"peerDependenciesMeta": {
|
||||||
|
"@types/react": {
|
||||||
|
"optional": true
|
||||||
|
},
|
||||||
|
"@types/react-dom": {
|
||||||
|
"optional": true
|
||||||
|
}
|
||||||
|
}
|
||||||
|
},
|
||||||
"node_modules/@radix-ui/react-use-callback-ref": {
|
"node_modules/@radix-ui/react-use-callback-ref": {
|
||||||
"version": "1.1.2",
|
"version": "1.1.2",
|
||||||
"resolved": "https://registry.npmjs.org/@radix-ui/react-use-callback-ref/-/react-use-callback-ref-1.1.2.tgz",
|
"resolved": "https://registry.npmjs.org/@radix-ui/react-use-callback-ref/-/react-use-callback-ref-1.1.2.tgz",
|
||||||
|
|||||||
@@ -17,6 +17,7 @@
|
|||||||
"@radix-ui/react-label": "^2.1.0",
|
"@radix-ui/react-label": "^2.1.0",
|
||||||
"@radix-ui/react-select": "^2.1.2",
|
"@radix-ui/react-select": "^2.1.2",
|
||||||
"@radix-ui/react-slot": "^1.1.0",
|
"@radix-ui/react-slot": "^1.1.0",
|
||||||
|
"@radix-ui/react-tabs": "^1.1.15",
|
||||||
"@tanstack/react-query": "^5.101.0",
|
"@tanstack/react-query": "^5.101.0",
|
||||||
"ansi-styles": "^6.2.3",
|
"ansi-styles": "^6.2.3",
|
||||||
"class-variance-authority": "^0.7.0",
|
"class-variance-authority": "^0.7.0",
|
||||||
|
|||||||
@@ -44,6 +44,51 @@ const SAMPLE_PROVIDER: Provider = {
|
|||||||
outstandingAr: 12450,
|
outstandingAr: 12450,
|
||||||
};
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Extended provider fixture (SP21 Task 3.1) — adds the
|
||||||
|
* `recent_claims` and `recent_activity` slices the Claims/Activity
|
||||||
|
* tabs read from. Used by the tabs tests below. Field shapes match
|
||||||
|
* `ClaimSummary` and `ActivityEvent` from `@/types`.
|
||||||
|
*/
|
||||||
|
const SAMPLE_PROVIDER_WITH_DETAILS: Provider = {
|
||||||
|
...SAMPLE_PROVIDER,
|
||||||
|
recent_claims: [
|
||||||
|
{
|
||||||
|
id: "CLM-0001",
|
||||||
|
state: "submitted",
|
||||||
|
billedAmount: 250,
|
||||||
|
patientName: "Jane Q Patient",
|
||||||
|
providerNpi: "1881068062",
|
||||||
|
payerName: "Aetna",
|
||||||
|
cptCode: "99213",
|
||||||
|
submissionDate: "2026-06-20",
|
||||||
|
parsedAt: "2026-06-20",
|
||||||
|
status: "submitted",
|
||||||
|
batchId: "batch-001",
|
||||||
|
},
|
||||||
|
],
|
||||||
|
recent_activity: [
|
||||||
|
{
|
||||||
|
id: 1,
|
||||||
|
ts: "2026-06-20T15:30:00Z",
|
||||||
|
kind: "claim_submitted",
|
||||||
|
batchId: "batch-001",
|
||||||
|
claimId: "CLM-0001",
|
||||||
|
remittanceId: null,
|
||||||
|
payload: {},
|
||||||
|
},
|
||||||
|
{
|
||||||
|
id: 2,
|
||||||
|
ts: "2026-06-20T15:35:00Z",
|
||||||
|
kind: "claim_paid",
|
||||||
|
batchId: "batch-001",
|
||||||
|
claimId: "CLM-0001",
|
||||||
|
remittanceId: null,
|
||||||
|
payload: {},
|
||||||
|
},
|
||||||
|
],
|
||||||
|
};
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Configure the mocked hook's return value for a single test. The
|
* Configure the mocked hook's return value for a single test. The
|
||||||
* `refetch` default is a fresh `vi.fn()` — tests that need to assert
|
* `refetch` default is a fresh `vi.fn()` — tests that need to assert
|
||||||
@@ -193,4 +238,66 @@ describe("ProviderDrawer", () => {
|
|||||||
fireEvent.click(closeBtn!);
|
fireEvent.click(closeBtn!);
|
||||||
expect(onClose).toHaveBeenCalledTimes(1);
|
expect(onClose).toHaveBeenCalledTimes(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// -- Tabs (SP21 Task 3.1) -------------------------------------------------
|
||||||
|
//
|
||||||
|
// Radix Tabs only mounts the active `Tabs.Content` into the DOM (no
|
||||||
|
// `forceMount`), so clicking a tab trigger causes the previous panel to
|
||||||
|
// unmount and the new one to mount. The tests below rely on that —
|
||||||
|
// "panel X renders" is asserted by checking that unique text from that
|
||||||
|
// panel's component is present in `document.body` after the click.
|
||||||
|
|
||||||
|
it("test_renders_three_tabs_overview_claims_activity", () => {
|
||||||
|
mockDetail({ data: SAMPLE_PROVIDER_WITH_DETAILS });
|
||||||
|
render(<ProviderDrawer npi="1881068062" onClose={() => {}} />);
|
||||||
|
|
||||||
|
// All three tab triggers are present, with the spec-mandated labels
|
||||||
|
// in the spec-mandated order.
|
||||||
|
const triggers = Array.from(document.querySelectorAll('[role="tab"]'));
|
||||||
|
expect(triggers.length).toBe(3);
|
||||||
|
expect(triggers.map((t) => t.textContent)).toEqual([
|
||||||
|
"Overview",
|
||||||
|
"Claims",
|
||||||
|
"Activity",
|
||||||
|
]);
|
||||||
|
// Overview is the default; its trigger must be selected on mount.
|
||||||
|
expect(triggers[0]?.getAttribute("aria-selected")).toBe("true");
|
||||||
|
expect(triggers[0]?.getAttribute("data-state")).toBe("active");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("test_switches_to_claims_tab_and_shows_recent_claims", () => {
|
||||||
|
mockDetail({ data: SAMPLE_PROVIDER_WITH_DETAILS });
|
||||||
|
render(<ProviderDrawer npi="1881068062" onClose={() => {}} />);
|
||||||
|
|
||||||
|
const claimsTrigger = Array.from(document.querySelectorAll('[role="tab"]'))
|
||||||
|
.find((t) => t.textContent === "Claims");
|
||||||
|
expect(claimsTrigger).not.toBeNull();
|
||||||
|
// Radix Tabs wires `onMouseDown` (not `onClick`) to its
|
||||||
|
// `onValueChange` handler — fire the matching event so the tab
|
||||||
|
// actually activates under happy-dom.
|
||||||
|
fireEvent.mouseDown(claimsTrigger!);
|
||||||
|
|
||||||
|
// Claims tab is now selected — and ProviderRecentClaims content is in
|
||||||
|
// the DOM (claim id + patient name + the "View all claims" link).
|
||||||
|
expect(claimsTrigger?.getAttribute("aria-selected")).toBe("true");
|
||||||
|
expect(document.body.textContent).toContain("CLM-0001");
|
||||||
|
expect(document.body.textContent).toContain("Jane Q Patient");
|
||||||
|
expect(document.body.textContent).toContain("View all claims");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("test_switches_to_activity_tab_and_shows_recent_activity", () => {
|
||||||
|
mockDetail({ data: SAMPLE_PROVIDER_WITH_DETAILS });
|
||||||
|
render(<ProviderDrawer npi="1881068062" onClose={() => {}} />);
|
||||||
|
|
||||||
|
const activityTrigger = Array.from(document.querySelectorAll('[role="tab"]'))
|
||||||
|
.find((t) => t.textContent === "Activity");
|
||||||
|
expect(activityTrigger).not.toBeNull();
|
||||||
|
fireEvent.mouseDown(activityTrigger!);
|
||||||
|
|
||||||
|
// Activity tab is now selected — and ProviderRecentActivity content
|
||||||
|
// is in the DOM (both `kind` strings from the fixture).
|
||||||
|
expect(activityTrigger?.getAttribute("aria-selected")).toBe("true");
|
||||||
|
expect(document.body.textContent).toContain("claim_submitted");
|
||||||
|
expect(document.body.textContent).toContain("claim_paid");
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1,9 +1,12 @@
|
|||||||
import { Dialog, DialogContent } from "@/components/ui/dialog";
|
import { Dialog, DialogContent } from "@/components/ui/dialog";
|
||||||
|
import { Tabs } from "@/components/ui/tabs";
|
||||||
import { ApiError } from "@/lib/api";
|
import { ApiError } from "@/lib/api";
|
||||||
import { DrillDrawerHeader } from "@/components/drill/DrillDrawerHeader";
|
import { DrillDrawerHeader } from "@/components/drill/DrillDrawerHeader";
|
||||||
import { Skeleton } from "@/components/ui/skeleton";
|
import { Skeleton } from "@/components/ui/skeleton";
|
||||||
import { useProviderDetail } from "@/hooks/useProviderDetail";
|
import { useProviderDetail } from "@/hooks/useProviderDetail";
|
||||||
import { ProviderOverview } from "./ProviderOverview";
|
import { ProviderOverview } from "./ProviderOverview";
|
||||||
|
import { ProviderRecentClaims } from "./ProviderRecentClaims";
|
||||||
|
import { ProviderRecentActivity } from "./ProviderRecentActivity";
|
||||||
import { ProviderDrawerError } from "./ProviderDrawerError";
|
import { ProviderDrawerError } from "./ProviderDrawerError";
|
||||||
|
|
||||||
interface Props {
|
interface Props {
|
||||||
@@ -12,12 +15,17 @@ interface Props {
|
|||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Provider drill-down drawer (SP21 Task 2.2).
|
* Provider drill-down drawer (SP21 Task 2.2 + 3.1).
|
||||||
*
|
*
|
||||||
* Side-panel shell that consumes ``useProviderDetail(npi)`` and renders
|
* Side-panel shell that consumes ``useProviderDetail(npi)`` and renders
|
||||||
* the Overview tab content (Phase 2 ships only this tab; Phase 3 adds
|
* a three-tab body:
|
||||||
* Claims/Activity tabs once ``recent_claims`` and ``recent_activity``
|
*
|
||||||
* rendering lands).
|
* - Overview — base provider fields (identity + activity shape)
|
||||||
|
* - Claims — top-10 claims joined to this provider
|
||||||
|
* (extended `/api/config/providers/{npi}.recent_claims`,
|
||||||
|
* populated by Task 1.6)
|
||||||
|
* - Activity — top-10 events joined to this provider's claims
|
||||||
|
* (extended `/api/config/providers/{npi}.recent_activity`)
|
||||||
*
|
*
|
||||||
* Layout mirrors the ClaimDrawer / RemitDrawer pattern: Radix Dialog
|
* Layout mirrors the ClaimDrawer / RemitDrawer pattern: Radix Dialog
|
||||||
* repositioned to the right edge as a fixed-height side panel, with
|
* repositioned to the right edge as a fixed-height side panel, with
|
||||||
@@ -76,9 +84,22 @@ export function ProviderDrawer({ npi, onClose }: Props) {
|
|||||||
title={data.name}
|
title={data.name}
|
||||||
onClose={onClose}
|
onClose={onClose}
|
||||||
/>
|
/>
|
||||||
<div className="p-6">
|
<Tabs.Root defaultValue="overview" className="px-6 py-4">
|
||||||
|
<Tabs.List>
|
||||||
|
<Tabs.Trigger value="overview">Overview</Tabs.Trigger>
|
||||||
|
<Tabs.Trigger value="claims">Claims</Tabs.Trigger>
|
||||||
|
<Tabs.Trigger value="activity">Activity</Tabs.Trigger>
|
||||||
|
</Tabs.List>
|
||||||
|
<Tabs.Content value="overview">
|
||||||
<ProviderOverview provider={data} />
|
<ProviderOverview provider={data} />
|
||||||
</div>
|
</Tabs.Content>
|
||||||
|
<Tabs.Content value="claims">
|
||||||
|
<ProviderRecentClaims provider={data} />
|
||||||
|
</Tabs.Content>
|
||||||
|
<Tabs.Content value="activity">
|
||||||
|
<ProviderRecentActivity provider={data} />
|
||||||
|
</Tabs.Content>
|
||||||
|
</Tabs.Root>
|
||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
</DialogContent>
|
</DialogContent>
|
||||||
|
|||||||
@@ -0,0 +1,34 @@
|
|||||||
|
import type { Provider } from "@/types";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Activity tab content for the ProviderDrawer (SP21 Task 3.1).
|
||||||
|
*
|
||||||
|
* STUB — renders the top-10 `recent_activity` events as a flat list
|
||||||
|
* (`ts` + `kind`). Real activity-event routing (linking each event to
|
||||||
|
* its source claim/remit/provider) arrives in Phase 4 once
|
||||||
|
* `provider_added` and `remit_received` events get their drawer
|
||||||
|
* surfaces; for now the events aren't drillable from this view —
|
||||||
|
* clicking them does nothing. The ProviderDrawer's Overview already
|
||||||
|
* surfaces the provider's `provider_added` event via the Dashboard
|
||||||
|
* activity feed (Task 2.5).
|
||||||
|
*/
|
||||||
|
export function ProviderRecentActivity({ provider }: { provider: Provider }) {
|
||||||
|
const items = provider.recent_activity ?? [];
|
||||||
|
if (items.length === 0) {
|
||||||
|
return (
|
||||||
|
<div className="text-muted-foreground text-[13px]">No recent activity.</div>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
return (
|
||||||
|
<ul className="space-y-1.5 text-[12.5px]">
|
||||||
|
{items.map((a) => (
|
||||||
|
<li key={a.id} className="flex items-center gap-2">
|
||||||
|
<span className="mono text-[10.5px] text-muted-foreground">
|
||||||
|
{new Date(a.ts).toLocaleString()}
|
||||||
|
</span>
|
||||||
|
<span>{a.kind}</span>
|
||||||
|
</li>
|
||||||
|
))}
|
||||||
|
</ul>
|
||||||
|
);
|
||||||
|
}
|
||||||
@@ -0,0 +1,43 @@
|
|||||||
|
import type { Provider } from "@/types";
|
||||||
|
import { fmt } from "@/lib/format";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Claims tab content for the ProviderDrawer (SP21 Task 3.1).
|
||||||
|
*
|
||||||
|
* Reads `provider.recent_claims` — the top-10 claims joined to this
|
||||||
|
* provider from the extended `GET /api/config/providers/{npi}` endpoint
|
||||||
|
* (Task 1.6). Renders one row per claim (`id` + `patientName` +
|
||||||
|
* `billedAmount`) with a "View all claims →" link to the global claims
|
||||||
|
* page at the bottom. Falls back to an empty-state line when the
|
||||||
|
* provider has no recent claims (e.g. legacy callers that hit the
|
||||||
|
* detail endpoint without the recent-claims slice).
|
||||||
|
*/
|
||||||
|
export function ProviderRecentClaims({ provider }: { provider: Provider }) {
|
||||||
|
const claims = provider.recent_claims ?? [];
|
||||||
|
if (claims.length === 0) {
|
||||||
|
return (
|
||||||
|
<div className="text-muted-foreground text-[13px]">No recent claims.</div>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
return (
|
||||||
|
<div className="space-y-2">
|
||||||
|
{claims.map((c) => (
|
||||||
|
<div
|
||||||
|
key={c.id}
|
||||||
|
className="flex items-center gap-3 py-2 border-b border-border/30 last:border-0"
|
||||||
|
>
|
||||||
|
<div className="display mono text-[12.5px] w-32 shrink-0">{c.id}</div>
|
||||||
|
<div className="flex-1 min-w-0 text-[12.5px] text-muted-foreground truncate">
|
||||||
|
{c.patientName ?? "—"}
|
||||||
|
</div>
|
||||||
|
<div className="display mono text-[13px]">
|
||||||
|
{fmt.usd(c.billedAmount)}
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
|
))}
|
||||||
|
<a href="/claims" className="text-[12.5px] text-accent hover:underline">
|
||||||
|
View all claims →
|
||||||
|
</a>
|
||||||
|
</div>
|
||||||
|
);
|
||||||
|
}
|
||||||
@@ -0,0 +1,84 @@
|
|||||||
|
import * as React from "react";
|
||||||
|
import * as TabsPrimitive from "@radix-ui/react-tabs";
|
||||||
|
import { cn } from "@/lib/utils";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Tabs primitive — SP21 Universal Drill-Down (Task 3.1).
|
||||||
|
*
|
||||||
|
* Thin wrapper over `@radix-ui/react-tabs`. Mirrors the same re-export
|
||||||
|
* pattern used by `dialog.tsx`: the individual primitives are forwardRef'd
|
||||||
|
* styled components, and the `Tabs` namespace object lets callers use
|
||||||
|
* the spec's `<Tabs.Root>`, `<Tabs.List>`, `<Tabs.Trigger>`,
|
||||||
|
* `<Tabs.Content>` dot-notation directly:
|
||||||
|
*
|
||||||
|
* import { Tabs } from "@/components/ui/tabs";
|
||||||
|
* <Tabs.Root defaultValue="overview">
|
||||||
|
* <Tabs.List>
|
||||||
|
* <Tabs.Trigger value="overview">Overview</Tabs.Trigger>
|
||||||
|
* ...
|
||||||
|
* </Tabs.List>
|
||||||
|
* <Tabs.Content value="overview">...</Tabs.Content>
|
||||||
|
* </Tabs.Root>
|
||||||
|
*/
|
||||||
|
|
||||||
|
const TabsList = React.forwardRef<
|
||||||
|
React.ElementRef<typeof TabsPrimitive.List>,
|
||||||
|
React.ComponentPropsWithoutRef<typeof TabsPrimitive.List>
|
||||||
|
>(({ className, ...props }, ref) => (
|
||||||
|
<TabsPrimitive.List
|
||||||
|
ref={ref}
|
||||||
|
className={cn(
|
||||||
|
"flex gap-2 border-b border-border/30 mb-4",
|
||||||
|
className
|
||||||
|
)}
|
||||||
|
{...props}
|
||||||
|
/>
|
||||||
|
));
|
||||||
|
TabsList.displayName = TabsPrimitive.List.displayName;
|
||||||
|
|
||||||
|
const TabsTrigger = React.forwardRef<
|
||||||
|
React.ElementRef<typeof TabsPrimitive.Trigger>,
|
||||||
|
React.ComponentPropsWithoutRef<typeof TabsPrimitive.Trigger>
|
||||||
|
>(({ className, ...props }, ref) => (
|
||||||
|
<TabsPrimitive.Trigger
|
||||||
|
ref={ref}
|
||||||
|
className={cn(
|
||||||
|
"px-3 py-2 text-[12.5px] text-muted-foreground",
|
||||||
|
"data-[state=active]:text-foreground",
|
||||||
|
"data-[state=active]:border-b-2 data-[state=active]:border-accent",
|
||||||
|
"focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2",
|
||||||
|
className
|
||||||
|
)}
|
||||||
|
{...props}
|
||||||
|
/>
|
||||||
|
));
|
||||||
|
TabsTrigger.displayName = TabsPrimitive.Trigger.displayName;
|
||||||
|
|
||||||
|
const TabsContent = React.forwardRef<
|
||||||
|
React.ElementRef<typeof TabsPrimitive.Content>,
|
||||||
|
React.ComponentPropsWithoutRef<typeof TabsPrimitive.Content>
|
||||||
|
>(({ className, ...props }, ref) => (
|
||||||
|
<TabsPrimitive.Content
|
||||||
|
ref={ref}
|
||||||
|
className={cn(
|
||||||
|
"focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2",
|
||||||
|
className
|
||||||
|
)}
|
||||||
|
{...props}
|
||||||
|
/>
|
||||||
|
));
|
||||||
|
TabsContent.displayName = TabsPrimitive.Content.displayName;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Namespace export matching the spec's `import * as Tabs from ...`
|
||||||
|
* dot-notation. `Root` is the unstyled Radix primitive; the rest are the
|
||||||
|
* styled wrappers above.
|
||||||
|
*/
|
||||||
|
export const Tabs = {
|
||||||
|
Root: TabsPrimitive.Root,
|
||||||
|
List: TabsList,
|
||||||
|
Trigger: TabsTrigger,
|
||||||
|
Content: TabsContent,
|
||||||
|
};
|
||||||
|
|
||||||
|
export { TabsList, TabsTrigger, TabsContent };
|
||||||
@@ -21,6 +21,7 @@ vi.mock("@/lib/api", () => ({
|
|||||||
api: {
|
api: {
|
||||||
isConfigured: true,
|
isConfigured: true,
|
||||||
listActivity: vi.fn(),
|
listActivity: vi.fn(),
|
||||||
|
getRemittance: vi.fn(),
|
||||||
},
|
},
|
||||||
}));
|
}));
|
||||||
|
|
||||||
@@ -158,6 +159,14 @@ describe("ActivityLog page filters", () => {
|
|||||||
(api.listActivity as unknown as ReturnType<typeof vi.fn>).mockResolvedValue(
|
(api.listActivity as unknown as ReturnType<typeof vi.fn>).mockResolvedValue(
|
||||||
EMPTY,
|
EMPTY,
|
||||||
);
|
);
|
||||||
|
// Default for the per-remit detail fetch — the drawer fetches
|
||||||
|
// this whenever `?remit=` is in the URL or `remit_received`
|
||||||
|
// events drill in. Return a never-resolving promise so the
|
||||||
|
// drawer stays in the loading state; the smoke tests only
|
||||||
|
// assert the drawer mounts.
|
||||||
|
(
|
||||||
|
api.getRemittance as unknown as ReturnType<typeof vi.fn>
|
||||||
|
).mockReturnValue(new Promise(() => {}));
|
||||||
});
|
});
|
||||||
|
|
||||||
it("test_renders_filter_controls_when_mounted", async () => {
|
it("test_renders_filter_controls_when_mounted", async () => {
|
||||||
@@ -447,3 +456,104 @@ describe("ActivityLog page filters", () => {
|
|||||||
unmount();
|
unmount();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("SP21 Task 4.7: ActivityLog → RemitDrawer wiring", () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.clearAllMocks();
|
||||||
|
(api.listActivity as unknown as ReturnType<typeof vi.fn>).mockResolvedValue(
|
||||||
|
EMPTY,
|
||||||
|
);
|
||||||
|
(
|
||||||
|
api.getRemittance as unknown as ReturnType<typeof vi.fn>
|
||||||
|
).mockReturnValue(new Promise(() => {}));
|
||||||
|
});
|
||||||
|
|
||||||
|
it("clicking a remit_received event opens the RemitDrawer", async () => {
|
||||||
|
(api.listActivity as unknown as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||||
|
items: [
|
||||||
|
{
|
||||||
|
id: "A-1",
|
||||||
|
kind: "remit_received",
|
||||||
|
message: "Remit PCN-1 received",
|
||||||
|
timestamp: "2026-06-20T10:00:00Z",
|
||||||
|
remittanceId: "REM-1",
|
||||||
|
},
|
||||||
|
],
|
||||||
|
total: 1,
|
||||||
|
returned: 1,
|
||||||
|
has_more: false,
|
||||||
|
});
|
||||||
|
|
||||||
|
const { unmount } = renderActivity();
|
||||||
|
|
||||||
|
// Wait for the row to render so we can click it.
|
||||||
|
await settle(() =>
|
||||||
|
document.body.textContent?.includes("Remit PCN-1 received") ?? false,
|
||||||
|
);
|
||||||
|
|
||||||
|
// Drawer should not be mounted before the click.
|
||||||
|
expect(document.body.querySelector('[data-testid="remit-drawer"]')).toBeNull();
|
||||||
|
|
||||||
|
// Find the <li role="button"> row and click it. ActivityFeed renders
|
||||||
|
// each row as a button-role <li> when `onItemClick` is provided, with
|
||||||
|
// an aria-label like "View remit received: <message>".
|
||||||
|
const row = document.body.querySelector('[aria-label^="View remit received"]') as
|
||||||
|
| HTMLLIElement
|
||||||
|
| null;
|
||||||
|
expect(row).not.toBeNull();
|
||||||
|
await act(async () => {
|
||||||
|
row!.click();
|
||||||
|
});
|
||||||
|
|
||||||
|
// After click, the RemitDrawer should be mounted (with the skeleton,
|
||||||
|
// since the never-resolving getRemittance keeps it in loading state).
|
||||||
|
await settle(
|
||||||
|
() => document.body.querySelector('[data-testid="remit-drawer"]') !== null,
|
||||||
|
);
|
||||||
|
expect(
|
||||||
|
document.body.querySelector('[data-testid="remit-drawer"]'),
|
||||||
|
).not.toBeNull();
|
||||||
|
|
||||||
|
unmount();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("deep-link ?remit=ID opens the RemitDrawer on mount", async () => {
|
||||||
|
(api.listActivity as unknown as ReturnType<typeof vi.fn>).mockResolvedValue({
|
||||||
|
items: [
|
||||||
|
{
|
||||||
|
id: "A-1",
|
||||||
|
kind: "remit_received",
|
||||||
|
message: "Remit PCN-1 received",
|
||||||
|
timestamp: "2026-06-20T10:00:00Z",
|
||||||
|
remittanceId: "REM-7",
|
||||||
|
},
|
||||||
|
],
|
||||||
|
total: 1,
|
||||||
|
returned: 1,
|
||||||
|
has_more: false,
|
||||||
|
});
|
||||||
|
|
||||||
|
// The hook reads `?remit=` from `window.location.search`, so set
|
||||||
|
// BOTH the MemoryRouter initial entry (for the page's URL display)
|
||||||
|
// AND `window.happyDOM.setURL` (for the hook).
|
||||||
|
(window as unknown as { happyDOM: { setURL: (u: string) => void } })
|
||||||
|
.happyDOM.setURL("http://localhost/activity?remit=REM-7");
|
||||||
|
|
||||||
|
const { unmount } = renderActivity({
|
||||||
|
initialEntries: ["/activity?remit=REM-7"],
|
||||||
|
});
|
||||||
|
|
||||||
|
await settle(
|
||||||
|
() => document.body.querySelector('[data-testid="remit-drawer"]') !== null,
|
||||||
|
);
|
||||||
|
expect(
|
||||||
|
document.body.querySelector('[data-testid="remit-drawer"]'),
|
||||||
|
).not.toBeNull();
|
||||||
|
|
||||||
|
unmount();
|
||||||
|
|
||||||
|
// Reset URL so any subsequent tests see a clean /activity URL.
|
||||||
|
(window as unknown as { happyDOM: { setURL: (u: string) => void } })
|
||||||
|
.happyDOM.setURL("http://localhost/activity");
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -1,12 +1,16 @@
|
|||||||
import { useCallback, useMemo } from "react";
|
import { useCallback, useMemo } from "react";
|
||||||
import { useSearchParams } from "react-router-dom";
|
import { useSearchParams, useNavigate } from "react-router-dom";
|
||||||
|
import { toast } from "sonner";
|
||||||
import { useActivity } from "@/hooks/useActivity";
|
import { useActivity } from "@/hooks/useActivity";
|
||||||
import { useTailStream } from "@/hooks/useTailStream";
|
import { useTailStream } from "@/hooks/useTailStream";
|
||||||
import { useMergedTail } from "@/hooks/useMergedTail";
|
import { useMergedTail } from "@/hooks/useMergedTail";
|
||||||
|
import { useRemitDrawerUrlState } from "@/hooks/useRemitDrawerUrlState";
|
||||||
|
import { eventKindToUrl } from "@/lib/event-routing";
|
||||||
import { TailStatusPill } from "@/components/TailStatusPill";
|
import { TailStatusPill } from "@/components/TailStatusPill";
|
||||||
import { PageHeader } from "@/components/PageHeader";
|
import { PageHeader } from "@/components/PageHeader";
|
||||||
import { ActivityFeed } from "@/components/ActivityFeed";
|
import { ActivityFeed } from "@/components/ActivityFeed";
|
||||||
import { ActivityFilters, type SinceValue } from "@/components/ActivityFilters";
|
import { ActivityFilters, type SinceValue } from "@/components/ActivityFilters";
|
||||||
|
import { RemitDrawer } from "@/components/RemitDrawer";
|
||||||
import { Skeleton } from "@/components/ui/skeleton";
|
import { Skeleton } from "@/components/ui/skeleton";
|
||||||
import { EmptyState } from "@/components/ui/empty-state";
|
import { EmptyState } from "@/components/ui/empty-state";
|
||||||
import { ErrorState } from "@/components/ui/error-state";
|
import { ErrorState } from "@/components/ui/error-state";
|
||||||
@@ -35,6 +39,13 @@ function useSinceIso(since: SinceValue): string | undefined {
|
|||||||
|
|
||||||
export function ActivityLog() {
|
export function ActivityLog() {
|
||||||
const [searchParams, setSearchParams] = useSearchParams();
|
const [searchParams, setSearchParams] = useSearchParams();
|
||||||
|
const navigate = useNavigate();
|
||||||
|
// SP21 Phase 4 Task 4.7: `remit_received` events with a
|
||||||
|
// remittanceId drill into the RemitDrawer. Calling `open(id)`
|
||||||
|
// pushes `?remit=ID` onto the current URL (no navigation away
|
||||||
|
// from `/activity`), so the activity feed stays visible behind
|
||||||
|
// the drawer and the drawer portals in over it.
|
||||||
|
const { remitId, open, close } = useRemitDrawerUrlState();
|
||||||
|
|
||||||
const selectedKinds = useMemo<ActivityKind[]>(
|
const selectedKinds = useMemo<ActivityKind[]>(
|
||||||
() =>
|
() =>
|
||||||
@@ -163,9 +174,48 @@ export function ActivityLog() {
|
|||||||
}
|
}
|
||||||
/>
|
/>
|
||||||
) : (
|
) : (
|
||||||
<ActivityFeed items={items} emptyMessage="No activity recorded yet." />
|
<ActivityFeed
|
||||||
|
items={items}
|
||||||
|
emptyMessage="No activity recorded yet."
|
||||||
|
onItemClick={(evt) => {
|
||||||
|
// SP21 Phase 4 Task 4.7: drill into the right surface
|
||||||
|
// based on event kind. `claim_*` and `provider_added`
|
||||||
|
// navigate away via `eventKindToUrl` (the Dashboard uses
|
||||||
|
// the same helper). `remit_received` events stay on
|
||||||
|
// `/activity` and open the RemitDrawer via `open(id)`
|
||||||
|
// — `eventKindToUrl` still returns `null` for that kind
|
||||||
|
// because cross-page navigation isn't the right UX here
|
||||||
|
// (we want to keep the activity feed as context behind
|
||||||
|
// the drawer). Anything else falls back to the
|
||||||
|
// "coming soon" toast so the click still gives feedback.
|
||||||
|
const url = eventKindToUrl(evt);
|
||||||
|
if (url) navigate(url);
|
||||||
|
else if (evt.kind === "remit_received" && evt.remittanceId) {
|
||||||
|
open(evt.remittanceId);
|
||||||
|
} else {
|
||||||
|
toast.info(
|
||||||
|
`Drill for ${evt.kind.replace(/_/g, " ")} coming in a later phase.`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}}
|
||||||
|
/>
|
||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
|
{/* SP21 Phase 4 Task 4.7: RemitDrawer mount. The activity
|
||||||
|
feed's `remit_received` rows drill into the drawer via
|
||||||
|
`open()`. `remits` is empty (the activity feed doesn't
|
||||||
|
keep a flat list of remits around), so j/k is a no-op
|
||||||
|
here — closing reverts the URL via `close()`. */}
|
||||||
|
<RemitDrawer
|
||||||
|
remitId={remitId}
|
||||||
|
remits={[]}
|
||||||
|
onClose={close}
|
||||||
|
onNavigate={open}
|
||||||
|
onToggleHelp={() => {
|
||||||
|
// ActivityLog has no cheatsheet; `?` is a no-op here.
|
||||||
|
}}
|
||||||
|
/>
|
||||||
</div>
|
</div>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -29,6 +29,7 @@ vi.mock("@/lib/api", () => ({
|
|||||||
isConfigured: true,
|
isConfigured: true,
|
||||||
listBatches: vi.fn(),
|
listBatches: vi.fn(),
|
||||||
getBatchDiff: vi.fn(),
|
getBatchDiff: vi.fn(),
|
||||||
|
getRemittance: vi.fn(),
|
||||||
},
|
},
|
||||||
ApiError: class ApiError extends Error {
|
ApiError: class ApiError extends Error {
|
||||||
constructor(public status: number, message: string) {
|
constructor(public status: number, message: string) {
|
||||||
@@ -180,6 +181,13 @@ function makeDiffPayload(): BatchDiffResponse {
|
|||||||
describe("BatchDiff page", () => {
|
describe("BatchDiff page", () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
vi.clearAllMocks();
|
vi.clearAllMocks();
|
||||||
|
// Default for the per-remit detail fetch — the drawer fetches
|
||||||
|
// this whenever `?remit=` is in the URL. Return a never-resolving
|
||||||
|
// promise so the drawer stays in the loading state; the smoke
|
||||||
|
// test only asserts the drawer mounts, not the loaded data.
|
||||||
|
(
|
||||||
|
api.getRemittance as unknown as ReturnType<typeof vi.fn>
|
||||||
|
).mockReturnValue(new Promise(() => {}));
|
||||||
});
|
});
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
@@ -189,6 +197,42 @@ describe("BatchDiff page", () => {
|
|||||||
.happyDOM.setURL("http://localhost/batch-diff");
|
.happyDOM.setURL("http://localhost/batch-diff");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("SP21 Task 4.5: deep-link ?remit=ID opens the RemitDrawer on mount", async () => {
|
||||||
|
// Pre-set the URL with `?remit=`. The page doesn't surface any
|
||||||
|
// per-remit rows (the diff payload is claim-level), but the
|
||||||
|
// drawer must still mount so the URL contract is honored.
|
||||||
|
//
|
||||||
|
// `useRemitDrawerUrlState` reads `window.location.search`
|
||||||
|
// (NOT React Router's search params) so we have to set the
|
||||||
|
// global window URL via happyDOM AND give MemoryRouter an
|
||||||
|
// initial entry — both stay in lockstep.
|
||||||
|
(window as unknown as { happyDOM: { setURL: (u: string) => void } })
|
||||||
|
.happyDOM.setURL("http://localhost/batch-diff?remit=REM-7");
|
||||||
|
(
|
||||||
|
api.listBatches as unknown as ReturnType<typeof vi.fn>
|
||||||
|
).mockResolvedValue([BATCH_A, BATCH_B]);
|
||||||
|
|
||||||
|
const { unmount } = renderIntoContainer(
|
||||||
|
<BatchDiff />,
|
||||||
|
["/batch-diff?remit=REM-7"],
|
||||||
|
);
|
||||||
|
|
||||||
|
// Page header must be present + drawer must be open.
|
||||||
|
await waitFor(
|
||||||
|
() => !!document.querySelector('[data-testid="batch-diff-page"]'),
|
||||||
|
"page header mounted",
|
||||||
|
);
|
||||||
|
await waitFor(
|
||||||
|
() => !!document.querySelector('[data-testid="remit-drawer"]'),
|
||||||
|
"remit drawer mounted via deep link",
|
||||||
|
);
|
||||||
|
expect(
|
||||||
|
document.querySelector('[data-testid="remit-drawer"]'),
|
||||||
|
).not.toBeNull();
|
||||||
|
|
||||||
|
unmount();
|
||||||
|
});
|
||||||
|
|
||||||
it("renders the awaiting-picks empty state when no batches are selected", async () => {
|
it("renders the awaiting-picks empty state when no batches are selected", async () => {
|
||||||
(api.listBatches as unknown as ReturnType<typeof vi.fn>).mockResolvedValue([
|
(api.listBatches as unknown as ReturnType<typeof vi.fn>).mockResolvedValue([
|
||||||
BATCH_A, BATCH_B,
|
BATCH_A, BATCH_B,
|
||||||
|
|||||||
@@ -21,8 +21,10 @@ import {
|
|||||||
BatchDiffView,
|
BatchDiffView,
|
||||||
BatchDiffViewSkeleton,
|
BatchDiffViewSkeleton,
|
||||||
} from "@/components/BatchDiffView";
|
} from "@/components/BatchDiffView";
|
||||||
|
import { RemitDrawer } from "@/components/RemitDrawer";
|
||||||
import { useBatches } from "@/hooks/useBatches";
|
import { useBatches } from "@/hooks/useBatches";
|
||||||
import { useBatchDiff } from "@/hooks/useBatchDiff";
|
import { useBatchDiff } from "@/hooks/useBatchDiff";
|
||||||
|
import { useRemitDrawerUrlState } from "@/hooks/useRemitDrawerUrlState";
|
||||||
import type { BatchSummary as ApiBatchSummary } from "@/lib/api";
|
import type { BatchSummary as ApiBatchSummary } from "@/lib/api";
|
||||||
import { cn } from "@/lib/utils";
|
import { cn } from "@/lib/utils";
|
||||||
|
|
||||||
@@ -269,6 +271,14 @@ export function BatchDiff() {
|
|||||||
// BrowserRouter (production) symmetric: both update the
|
// BrowserRouter (production) symmetric: both update the
|
||||||
// useSearchParams hook on every navigation.
|
// useSearchParams hook on every navigation.
|
||||||
const [searchParams, setSearchParams] = useSearchParams();
|
const [searchParams, setSearchParams] = useSearchParams();
|
||||||
|
// SP21 Phase 4 Task 4.5: mount the RemitDrawer so a deep link to
|
||||||
|
// /batch-diff?remit=REM-1 opens the drawer in-place. The BatchDiff
|
||||||
|
// data model (BatchClaimDiffSummary) is a claim-level projection —
|
||||||
|
// there are no per-remit IDs in the diff payload to wrap, so no
|
||||||
|
// click-to-drill is wired here. The drawer still mounts so the
|
||||||
|
// `?remit=` URL contract is honored when the user lands on this
|
||||||
|
// page with the param set (e.g. from an external link).
|
||||||
|
const { remitId, open, close } = useRemitDrawerUrlState();
|
||||||
const { a, b } = useMemo(
|
const { a, b } = useMemo(
|
||||||
() => readIdsFromParams(searchParams),
|
() => readIdsFromParams(searchParams),
|
||||||
[searchParams],
|
[searchParams],
|
||||||
@@ -835,6 +845,24 @@ export function BatchDiff() {
|
|||||||
<span>{ready ? "A vs B" : "awaiting picks"}</span>
|
<span>{ready ? "A vs B" : "awaiting picks"}</span>
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
|
{/* SP21 Phase 4 Task 4.5: RemitDrawer mount. There are no
|
||||||
|
per-remit rows in the diff payload (the page is a
|
||||||
|
claim-level projection), so this drawer is for deep-link
|
||||||
|
support only — clicking a claim row does not open it.
|
||||||
|
When the user lands on /batch-diff?remit=REM-1, the drawer
|
||||||
|
opens in-place. The `remits` list is empty so j/k is a
|
||||||
|
no-op while the drawer is open. */}
|
||||||
|
<RemitDrawer
|
||||||
|
remitId={remitId}
|
||||||
|
remits={[]}
|
||||||
|
onClose={close}
|
||||||
|
onNavigate={open}
|
||||||
|
onToggleHelp={() => {
|
||||||
|
// BatchDiff has no cheatsheet surface; `?` is a no-op
|
||||||
|
// here, but the prop is required by the drawer's contract.
|
||||||
|
}}
|
||||||
|
/>
|
||||||
</div>
|
</div>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
+114
-4
@@ -1,10 +1,30 @@
|
|||||||
// @vitest-environment happy-dom
|
// @vitest-environment happy-dom
|
||||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||||
import { act, cleanup, fireEvent, render, waitFor } from "@testing-library/react";
|
import { act, cleanup, fireEvent, render, waitFor } from "@testing-library/react";
|
||||||
|
import { MemoryRouter } from "react-router-dom";
|
||||||
|
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||||
import Inbox from "./Inbox";
|
import Inbox from "./Inbox";
|
||||||
import * as inboxApi from "@/lib/inbox-api";
|
import * as inboxApi from "@/lib/inbox-api";
|
||||||
import * as downloadModule from "@/lib/download";
|
import * as downloadModule from "@/lib/download";
|
||||||
|
|
||||||
|
// SP21 Phase 4 Task 4.4: Inbox now uses `useNavigate` (for unmatched
|
||||||
|
// claim drilldown to /claims?claim=ID), so the render harness needs
|
||||||
|
// a Router context. We use a fresh QueryClient per test so the
|
||||||
|
// drawer's per-remit query doesn't leak cache between cases, and a
|
||||||
|
// MemoryRouter so `useNavigate` has a router to push into.
|
||||||
|
function renderInbox() {
|
||||||
|
const qc = new QueryClient({
|
||||||
|
defaultOptions: { queries: { retry: false, retryDelay: 0 } },
|
||||||
|
});
|
||||||
|
return render(
|
||||||
|
<MemoryRouter initialEntries={["/inbox"]}>
|
||||||
|
<QueryClientProvider client={qc}>
|
||||||
|
<Inbox />
|
||||||
|
</QueryClientProvider>
|
||||||
|
</MemoryRouter>,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
cleanup();
|
cleanup();
|
||||||
vi.unstubAllGlobals();
|
vi.unstubAllGlobals();
|
||||||
@@ -29,7 +49,7 @@ describe("Inbox page", () => {
|
|||||||
}),
|
}),
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
const { container } = render(<Inbox />);
|
const { container } = renderInbox();
|
||||||
await waitFor(() => {
|
await waitFor(() => {
|
||||||
expect(container.textContent).toContain("REJECTED");
|
expect(container.textContent).toContain("REJECTED");
|
||||||
expect(container.textContent).toContain("PAYER REJECTED");
|
expect(container.textContent).toContain("PAYER REJECTED");
|
||||||
@@ -65,7 +85,7 @@ describe("Inbox page", () => {
|
|||||||
}),
|
}),
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
const { container } = render(<Inbox />);
|
const { container } = renderInbox();
|
||||||
await waitFor(() => {
|
await waitFor(() => {
|
||||||
expect(container.textContent).toContain("1");
|
expect(container.textContent).toContain("1");
|
||||||
expect(container.textContent).toContain("items need eyes");
|
expect(container.textContent).toContain("items need eyes");
|
||||||
@@ -112,7 +132,7 @@ describe("Inbox page", () => {
|
|||||||
});
|
});
|
||||||
vi.stubGlobal("fetch", fetchMock);
|
vi.stubGlobal("fetch", fetchMock);
|
||||||
|
|
||||||
const { container, getByTestId } = render(<Inbox />);
|
const { container, getByTestId } = renderInbox();
|
||||||
await waitFor(() => {
|
await waitFor(() => {
|
||||||
expect(container.textContent).toContain("PR1");
|
expect(container.textContent).toContain("PR1");
|
||||||
});
|
});
|
||||||
@@ -202,7 +222,7 @@ describe("Inbox page", () => {
|
|||||||
.spyOn(downloadModule, "downloadBlob")
|
.spyOn(downloadModule, "downloadBlob")
|
||||||
.mockImplementation(() => {});
|
.mockImplementation(() => {});
|
||||||
|
|
||||||
const { container, getByText } = render(<Inbox />);
|
const { container, getByText } = renderInbox();
|
||||||
await waitFor(() => {
|
await waitFor(() => {
|
||||||
expect(container.textContent).toContain("C1");
|
expect(container.textContent).toContain("C1");
|
||||||
expect(container.textContent).toContain("C2");
|
expect(container.textContent).toContain("C2");
|
||||||
@@ -245,4 +265,94 @@ describe("Inbox page", () => {
|
|||||||
expect(filename).toBe("resubmit-2-claims.zip");
|
expect(filename).toBe("resubmit-2-claims.zip");
|
||||||
expect(blob).toBeInstanceOf(Blob);
|
expect(blob).toBeInstanceOf(Blob);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("SP21 Task 4.4: clicking a candidate row opens the RemitDrawer", async () => {
|
||||||
|
// Candidates are remits (payer_claim_control_number-keyed) — the
|
||||||
|
// row's `id` is the remit id, so clicking drills into the
|
||||||
|
// RemitDrawer via `?remit=ID`.
|
||||||
|
vi.stubGlobal(
|
||||||
|
"fetch",
|
||||||
|
vi.fn().mockResolvedValue({
|
||||||
|
ok: true,
|
||||||
|
json: async () => ({
|
||||||
|
rejected: [],
|
||||||
|
payer_rejected: [],
|
||||||
|
candidates: [
|
||||||
|
{
|
||||||
|
id: "REM-7",
|
||||||
|
kind: "remit",
|
||||||
|
payer_claim_control_number: "REM-7",
|
||||||
|
charge_amount: 200,
|
||||||
|
payer_id: "P1",
|
||||||
|
rendering_provider_npi: "1234567890",
|
||||||
|
service_date: "2026-06-19",
|
||||||
|
candidates: [],
|
||||||
|
},
|
||||||
|
],
|
||||||
|
unmatched: [],
|
||||||
|
done_today: [],
|
||||||
|
}),
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
|
||||||
|
const { container } = renderInbox();
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(container.textContent).toContain("REM-7");
|
||||||
|
});
|
||||||
|
|
||||||
|
// No drawer yet — the URL is `/inbox` with no `?remit=`.
|
||||||
|
expect(
|
||||||
|
container.querySelector('[data-testid="remit-drawer"]')
|
||||||
|
).toBeNull();
|
||||||
|
|
||||||
|
// Click the candidate row. The InboxRow renders as a <tr> with
|
||||||
|
// the remit id in its first cell; clicking that row bubbles to
|
||||||
|
// the Lane's onRowClick handler we wired in Task 4.4.
|
||||||
|
const cell = Array.from(container.querySelectorAll("td")).find(
|
||||||
|
(td) => td.textContent === "REM-7",
|
||||||
|
);
|
||||||
|
const row = cell?.closest("tr");
|
||||||
|
expect(row).toBeTruthy();
|
||||||
|
await act(async () => {
|
||||||
|
fireEvent.click(row as HTMLElement);
|
||||||
|
});
|
||||||
|
|
||||||
|
// Drawer portals into document.body — check there, not container.
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(
|
||||||
|
document.body.querySelector('[data-testid="remit-drawer"]'),
|
||||||
|
).not.toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("SP21 Task 4.4: deep-link ?remit=ID opens the drawer on mount", async () => {
|
||||||
|
// Pre-set the URL so the hook reads REM-7 off `window.location.search`
|
||||||
|
// during its `useState` initializer — no click needed. The inbox
|
||||||
|
// already mounts the drawer, so a deep link to /inbox?remit=REM-7
|
||||||
|
// should land with the drawer open.
|
||||||
|
vi.stubGlobal(
|
||||||
|
"fetch",
|
||||||
|
vi.fn().mockResolvedValue({
|
||||||
|
ok: true,
|
||||||
|
json: async () => ({
|
||||||
|
rejected: [],
|
||||||
|
payer_rejected: [],
|
||||||
|
candidates: [],
|
||||||
|
unmatched: [],
|
||||||
|
done_today: [],
|
||||||
|
}),
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
(window as unknown as { happyDOM: { setURL: (u: string) => void } }).happyDOM.setURL(
|
||||||
|
"http://localhost/inbox?remit=REM-7",
|
||||||
|
);
|
||||||
|
|
||||||
|
renderInbox();
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(
|
||||||
|
document.body.querySelector('[data-testid="remit-drawer"]'),
|
||||||
|
).not.toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
+42
-2
@@ -13,10 +13,13 @@
|
|||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
import { useState } from "react";
|
import { useState } from "react";
|
||||||
|
import { useNavigate } from "react-router-dom";
|
||||||
import { cn } from "@/lib/utils";
|
import { cn } from "@/lib/utils";
|
||||||
import { Lane, type LaneRow } from "@/components/inbox/Lane";
|
import { Lane, type LaneRow } from "@/components/inbox/Lane";
|
||||||
import { InboxHeader } from "@/components/inbox/InboxHeader";
|
import { InboxHeader } from "@/components/inbox/InboxHeader";
|
||||||
import { BulkBar } from "@/components/inbox/BulkBar";
|
import { BulkBar } from "@/components/inbox/BulkBar";
|
||||||
|
import { RemitDrawer } from "@/components/RemitDrawer";
|
||||||
|
import { useRemitDrawerUrlState } from "@/hooks/useRemitDrawerUrlState";
|
||||||
import { useInboxLanes } from "@/hooks/useInboxLanes";
|
import { useInboxLanes } from "@/hooks/useInboxLanes";
|
||||||
import {
|
import {
|
||||||
exportInboxCsvUrl,
|
exportInboxCsvUrl,
|
||||||
@@ -40,6 +43,12 @@ function rowKey(row: LaneRow): string {
|
|||||||
|
|
||||||
export default function Inbox() {
|
export default function Inbox() {
|
||||||
const { lanes, loading, error, refetch } = useInboxLanes();
|
const { lanes, loading, error, refetch } = useInboxLanes();
|
||||||
|
// SP21 Phase 4 Task 4.4: drill-down from inbox rows. The hook reads
|
||||||
|
// `?remit=` off `window.location.search` so opening a row from the
|
||||||
|
// /inbox URL pushes `?remit=ID` onto /inbox itself (it doesn't
|
||||||
|
// navigate to /remittances). The drawer just opens in-place.
|
||||||
|
const navigate = useNavigate();
|
||||||
|
const { remitId, open, close } = useRemitDrawerUrlState();
|
||||||
const [selected, setSelected] = useState<Record<LaneKey, string[]>>({
|
const [selected, setSelected] = useState<Record<LaneKey, string[]>>({
|
||||||
rejected: [],
|
rejected: [],
|
||||||
payer_rejected: [],
|
payer_rejected: [],
|
||||||
@@ -218,6 +227,22 @@ export default function Inbox() {
|
|||||||
className="min-h-screen"
|
className="min-h-screen"
|
||||||
style={{ background: "var(--tt-bg)", color: "var(--tt-ink)" }}
|
style={{ background: "var(--tt-bg)", color: "var(--tt-ink)" }}
|
||||||
>
|
>
|
||||||
|
{/* SP21 Phase 4 Task 4.4: RemitDrawer mounts here so a row click
|
||||||
|
in the candidates / unmatched lanes drills into the parent
|
||||||
|
remit. The drawer portals into document.body (Radix Dialog),
|
||||||
|
so the surrounding dark surface is decorative — the drawer
|
||||||
|
overlays it when open. The hook reads `?remit=` off the URL,
|
||||||
|
so deep links restore the open remit on reload. */}
|
||||||
|
<RemitDrawer
|
||||||
|
remitId={remitId}
|
||||||
|
remits={[]}
|
||||||
|
onClose={close}
|
||||||
|
onNavigate={open}
|
||||||
|
onToggleHelp={() => {
|
||||||
|
// No cheatsheet on the inbox surface — `?` is a no-op
|
||||||
|
// here, but the prop is required by the drawer's contract.
|
||||||
|
}}
|
||||||
|
/>
|
||||||
<InboxHeader needEyesCount={needEyes} doneTodayCount={lanes.done_today.length} />
|
<InboxHeader needEyesCount={needEyes} doneTodayCount={lanes.done_today.length} />
|
||||||
|
|
||||||
{/* Fold — a thin amber rule + italic serif annotation that
|
{/* Fold — a thin amber rule + italic serif annotation that
|
||||||
@@ -290,14 +315,29 @@ export default function Inbox() {
|
|||||||
name="CANDIDATES"
|
name="CANDIDATES"
|
||||||
accent="amber"
|
accent="amber"
|
||||||
rows={lanes.candidates}
|
rows={lanes.candidates}
|
||||||
onRowClick={() => {}}
|
// Candidates are remits waiting for a claim match — drill
|
||||||
|
// straight into the RemitDrawer for the source remit.
|
||||||
|
onRowClick={(row) => open(row.id)}
|
||||||
onSelectionChange={(ids) => setLaneSelected("candidates", ids)}
|
onSelectionChange={(ids) => setLaneSelected("candidates", ids)}
|
||||||
/>
|
/>
|
||||||
<Lane
|
<Lane
|
||||||
name="UNMATCHED"
|
name="UNMATCHED"
|
||||||
accent="ink-blue"
|
accent="ink-blue"
|
||||||
rows={lanes.unmatched}
|
rows={lanes.unmatched}
|
||||||
onRowClick={() => {}}
|
// Unmatched is a mixed bucket (kind === "claim" | "remit" per
|
||||||
|
// the InboxClaimRow union). Defensive branch — today the
|
||||||
|
// backend only emits "claim" rows here, but the type allows
|
||||||
|
// both, and the next data-source change shouldn't require a
|
||||||
|
// page edit.
|
||||||
|
onRowClick={(row) => {
|
||||||
|
if (row.kind === "remit") {
|
||||||
|
open(row.id);
|
||||||
|
} else if (row.kind === "claim") {
|
||||||
|
navigate(
|
||||||
|
`/claims?claim=${encodeURIComponent(row.id)}`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}}
|
||||||
onSelectionChange={(ids) => setLaneSelected("unmatched", ids)}
|
onSelectionChange={(ids) => setLaneSelected("unmatched", ids)}
|
||||||
/>
|
/>
|
||||||
<Lane
|
<Lane
|
||||||
|
|||||||
@@ -19,6 +19,7 @@ vi.mock("@/lib/api", () => ({
|
|||||||
listUnmatched: vi.fn(),
|
listUnmatched: vi.fn(),
|
||||||
matchRemit: vi.fn(),
|
matchRemit: vi.fn(),
|
||||||
unmatchClaim: vi.fn(),
|
unmatchClaim: vi.fn(),
|
||||||
|
getRemittance: vi.fn(),
|
||||||
},
|
},
|
||||||
ApiError: class ApiError extends Error {
|
ApiError: class ApiError extends Error {
|
||||||
constructor(public status: number, message: string) {
|
constructor(public status: number, message: string) {
|
||||||
@@ -86,6 +87,73 @@ async function waitForText(
|
|||||||
describe("ReconciliationPage", () => {
|
describe("ReconciliationPage", () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
vi.clearAllMocks();
|
vi.clearAllMocks();
|
||||||
|
// Default for the per-remit detail fetch — the drawer fetches
|
||||||
|
// this whenever `?remit=` is in the URL. Return a never-resolving
|
||||||
|
// promise so the drawer stays in the loading state; the smoke
|
||||||
|
// test only asserts the drawer mounts.
|
||||||
|
(
|
||||||
|
api.getRemittance as unknown as ReturnType<typeof vi.fn>
|
||||||
|
).mockReturnValue(new Promise(() => {}));
|
||||||
|
});
|
||||||
|
|
||||||
|
it("SP21 Task 4.6: deep-link ?remit=ID opens the RemitDrawer on mount", async () => {
|
||||||
|
// Non-empty unmatched payload so the page renders the two-column
|
||||||
|
// matching surface (the "Pair them." branch). The pre-existing
|
||||||
|
// empty-state branch has a flaky happy-dom/race that's unrelated
|
||||||
|
// to Phase 4 — using non-empty data sidesteps that bug and still
|
||||||
|
// proves the deep-link → drawer mount works. We pre-set
|
||||||
|
// `window.location` (which `useRemitDrawerUrlState` reads on
|
||||||
|
// mount) so `?remit=REM-7` resolves to a truthy `remitId`.
|
||||||
|
(
|
||||||
|
api.listUnmatched as unknown as ReturnType<typeof vi.fn>
|
||||||
|
).mockResolvedValue({
|
||||||
|
claims: [
|
||||||
|
{
|
||||||
|
id: "CLM-1",
|
||||||
|
patientName: "Patient A",
|
||||||
|
billedAmount: 100,
|
||||||
|
providerNpi: "1234567890",
|
||||||
|
serviceDate: "2026-06-01",
|
||||||
|
payerId: "P1",
|
||||||
|
state: "submitted",
|
||||||
|
},
|
||||||
|
],
|
||||||
|
remittances: [
|
||||||
|
{
|
||||||
|
id: "REM-7",
|
||||||
|
payerClaimControlNumber: "PCN-A",
|
||||||
|
status: "received",
|
||||||
|
paidAmount: 100,
|
||||||
|
adjustmentAmount: 0,
|
||||||
|
receivedDate: "2026-06-01",
|
||||||
|
isReversal: false,
|
||||||
|
totalCharge: 100,
|
||||||
|
serviceDate: "2026-06-01",
|
||||||
|
batchId: "b1",
|
||||||
|
},
|
||||||
|
],
|
||||||
|
});
|
||||||
|
(window as unknown as { happyDOM: { setURL: (u: string) => void } })
|
||||||
|
.happyDOM.setURL("http://localhost/reconciliation?remit=REM-7");
|
||||||
|
|
||||||
|
const { unmount } = renderIntoContainer(
|
||||||
|
React.createElement(ReconciliationPage),
|
||||||
|
);
|
||||||
|
|
||||||
|
// Wait for the loaded two-column view (the "Pair them." headline
|
||||||
|
// is the clearest signal that listUnmatched has resolved and the
|
||||||
|
// page is past the loading + empty branches), then assert the
|
||||||
|
// drawer is mounted.
|
||||||
|
await waitForText("Pair them.");
|
||||||
|
expect(
|
||||||
|
document.body.querySelector('[data-testid="remit-drawer"]'),
|
||||||
|
).not.toBeNull();
|
||||||
|
|
||||||
|
unmount();
|
||||||
|
|
||||||
|
// Reset URL so the next test sees a clean /reconciliation URL.
|
||||||
|
(window as unknown as { happyDOM: { setURL: (u: string) => void } })
|
||||||
|
.happyDOM.setURL("http://localhost/reconciliation");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("renders both columns with unmatched rows when api returns data", async () => {
|
it("renders both columns with unmatched rows when api returns data", async () => {
|
||||||
|
|||||||
@@ -11,6 +11,9 @@ import { ApiError } from "@/lib/api";
|
|||||||
import { Skeleton } from "@/components/ui/skeleton";
|
import { Skeleton } from "@/components/ui/skeleton";
|
||||||
import { ErrorState } from "@/components/ui/error-state";
|
import { ErrorState } from "@/components/ui/error-state";
|
||||||
import { Button } from "@/components/ui/button";
|
import { Button } from "@/components/ui/button";
|
||||||
|
import { RemitDrawer } from "@/components/RemitDrawer";
|
||||||
|
import { DrillableCell } from "@/components/drill/DrillableCell";
|
||||||
|
import { useRemitDrawerUrlState } from "@/hooks/useRemitDrawerUrlState";
|
||||||
import { cn } from "@/lib/utils";
|
import { cn } from "@/lib/utils";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -26,6 +29,11 @@ import { cn } from "@/lib/utils";
|
|||||||
*/
|
*/
|
||||||
export function ReconciliationPage() {
|
export function ReconciliationPage() {
|
||||||
const { unmatched, match } = useReconciliation();
|
const { unmatched, match } = useReconciliation();
|
||||||
|
// SP21 Phase 4 Task 4.6: drill into the RemitDrawer from the
|
||||||
|
// remits column. The hook reads `?remit=` from the URL so deep
|
||||||
|
// links land with the drawer open. Selection state stays local
|
||||||
|
// — the drill is a separate gesture from the match selection.
|
||||||
|
const { remitId, open, close } = useRemitDrawerUrlState();
|
||||||
const [selectedClaim, setSelectedClaim] = useState<string | null>(null);
|
const [selectedClaim, setSelectedClaim] = useState<string | null>(null);
|
||||||
const [selectedRemit, setSelectedRemit] = useState<string | null>(null);
|
const [selectedRemit, setSelectedRemit] = useState<string | null>(null);
|
||||||
|
|
||||||
@@ -735,11 +743,18 @@ export function ReconciliationPage() {
|
|||||||
{remittances.map((r) => {
|
{remittances.map((r) => {
|
||||||
const active = selectedRemit === r.id;
|
const active = selectedRemit === r.id;
|
||||||
return (
|
return (
|
||||||
<button
|
<div
|
||||||
key={r.id}
|
key={r.id}
|
||||||
type="button"
|
role="button"
|
||||||
onClick={() => setSelectedRemit(r.id)}
|
tabIndex={0}
|
||||||
aria-pressed={active}
|
aria-pressed={active}
|
||||||
|
onClick={() => setSelectedRemit(r.id)}
|
||||||
|
onKeyDown={(e) => {
|
||||||
|
if (e.key === "Enter" || e.key === " ") {
|
||||||
|
e.preventDefault();
|
||||||
|
setSelectedRemit(r.id);
|
||||||
|
}
|
||||||
|
}}
|
||||||
className={cn(
|
className={cn(
|
||||||
"w-full text-left p-3.5 min-h-[44px] rounded-md border transition-colors",
|
"w-full text-left p-3.5 min-h-[44px] rounded-md border transition-colors",
|
||||||
active
|
active
|
||||||
@@ -750,8 +765,22 @@ export function ReconciliationPage() {
|
|||||||
<div
|
<div
|
||||||
className="mono text-[13.5px] flex items-center gap-2"
|
className="mono text-[13.5px] flex items-center gap-2"
|
||||||
style={{ color: "hsl(var(--surface-ink))" }}
|
style={{ color: "hsl(var(--surface-ink))" }}
|
||||||
|
>
|
||||||
|
{/* DrillableCell wraps the PCN text — clicking
|
||||||
|
the text drills into the RemitDrawer; the
|
||||||
|
surrounding div onClick (which selects the
|
||||||
|
row for the match action) is suppressed
|
||||||
|
because DrillableCell calls
|
||||||
|
e.stopPropagation() in its onClick. The
|
||||||
|
DrillableCell is its own <button>, so the
|
||||||
|
outer row had to move off <button> to
|
||||||
|
avoid invalid nested-button HTML. */}
|
||||||
|
<DrillableCell
|
||||||
|
onClick={() => open(r.id)}
|
||||||
|
ariaLabel={`View remittance ${r.payerClaimControlNumber}`}
|
||||||
>
|
>
|
||||||
<span className="display">{r.payerClaimControlNumber}</span>
|
<span className="display">{r.payerClaimControlNumber}</span>
|
||||||
|
</DrillableCell>
|
||||||
{r.isReversal ? (
|
{r.isReversal ? (
|
||||||
<span
|
<span
|
||||||
className="text-[10px] uppercase tracking-[0.18em] mono font-semibold"
|
className="text-[10px] uppercase tracking-[0.18em] mono font-semibold"
|
||||||
@@ -768,7 +797,7 @@ export function ReconciliationPage() {
|
|||||||
Status {r.status} · ${r.paidAmount.toFixed(2)} paid · $
|
Status {r.status} · ${r.paidAmount.toFixed(2)} paid · $
|
||||||
{r.adjustmentAmount.toFixed(2)} adj
|
{r.adjustmentAmount.toFixed(2)} adj
|
||||||
</div>
|
</div>
|
||||||
</button>
|
</div>
|
||||||
);
|
);
|
||||||
})}
|
})}
|
||||||
</PairColumn>
|
</PairColumn>
|
||||||
@@ -881,6 +910,22 @@ export function ReconciliationPage() {
|
|||||||
</span>
|
</span>
|
||||||
</div>
|
</div>
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
|
{/* SP21 Phase 4 Task 4.6: RemitDrawer mount. The remits column
|
||||||
|
drills into the parent remit via the PCN text (DrillableCell
|
||||||
|
+ open). The drawer portals into document.body, so the
|
||||||
|
surrounding paper plane stays put while the drawer is open.
|
||||||
|
`remits` is empty (we don't keep a list of all remits on
|
||||||
|
this page), so j/k is a no-op while the drawer is open. */}
|
||||||
|
<RemitDrawer
|
||||||
|
remitId={remitId}
|
||||||
|
remits={[]}
|
||||||
|
onClose={close}
|
||||||
|
onNavigate={open}
|
||||||
|
onToggleHelp={() => {
|
||||||
|
// Reconciliation has no cheatsheet; `?` is a no-op here.
|
||||||
|
}}
|
||||||
|
/>
|
||||||
</div>
|
</div>
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -22,6 +22,11 @@ vi.mock("@/lib/api", () => ({
|
|||||||
listRemittances: vi.fn(),
|
listRemittances: vi.fn(),
|
||||||
getRemittance: vi.fn(),
|
getRemittance: vi.fn(),
|
||||||
},
|
},
|
||||||
|
ApiError: class ApiError extends Error {
|
||||||
|
constructor(public status: number, message: string) {
|
||||||
|
super(message);
|
||||||
|
}
|
||||||
|
},
|
||||||
}));
|
}));
|
||||||
|
|
||||||
// Mock the live-tail hook so the page renders the pill in the settled
|
// Mock the live-tail hook so the page renders the pill in the settled
|
||||||
@@ -128,6 +133,17 @@ function rowAt(idx: number): HTMLTableRowElement | null {
|
|||||||
) as HTMLTableRowElement | null;
|
) as HTMLTableRowElement | null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Point happy-dom's URL at a known value. happy-dom v20 doesn't expose a
|
||||||
|
* writable `window.location.search`, but `window.happyDOM.setURL` updates
|
||||||
|
* the URL the window reports without triggering a navigation — exactly
|
||||||
|
* what we want for mounting the page at `/remittances?remit=PCN-1` etc.
|
||||||
|
* Same helper used by Claims.test.tsx and useRemitDrawerUrlState.test.ts.
|
||||||
|
*/
|
||||||
|
function setLocation(url: string): void {
|
||||||
|
(window as unknown as { happyDOM: { setURL: (u: string) => void } }).happyDOM.setURL(url);
|
||||||
|
}
|
||||||
|
|
||||||
/** True iff exactly one row carries `data-state="selected"`. */
|
/** True iff exactly one row carries `data-state="selected"`. */
|
||||||
function hasExactlyOneSelectedRow(): boolean {
|
function hasExactlyOneSelectedRow(): boolean {
|
||||||
const selected = document.querySelectorAll(
|
const selected = document.querySelectorAll(
|
||||||
@@ -191,6 +207,11 @@ const SAMPLE_REMITS = [
|
|||||||
describe("Remittances", () => {
|
describe("Remittances", () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
vi.clearAllMocks();
|
vi.clearAllMocks();
|
||||||
|
// Reset URL to the bare remittances page between tests so a
|
||||||
|
// `?remit=` leaked from a prior test (via pushState) doesn't
|
||||||
|
// bleed into the next. happy-dom's URL survives across tests in
|
||||||
|
// the same file unless explicitly reset.
|
||||||
|
setLocation("http://localhost/remittances");
|
||||||
// Singleton tail-store: clear the remittances slice between tests
|
// Singleton tail-store: clear the remittances slice between tests
|
||||||
// so a tail-arrival case (if added later) doesn't see rows from a
|
// so a tail-arrival case (if added later) doesn't see rows from a
|
||||||
// previous test.
|
// previous test.
|
||||||
@@ -203,18 +224,26 @@ describe("Remittances", () => {
|
|||||||
returned: SAMPLE_REMITS.length,
|
returned: SAMPLE_REMITS.length,
|
||||||
has_more: false,
|
has_more: false,
|
||||||
});
|
});
|
||||||
|
// Default for the per-remit detail fetch — the drawer fetches
|
||||||
|
// this whenever `?remit=` is in the URL. Return a never-resolving
|
||||||
|
// promise so the drawer stays in the loading state; the smoke
|
||||||
|
// tests only assert the drawer mounts, not the loaded data.
|
||||||
|
(
|
||||||
|
api.getRemittance as unknown as ReturnType<typeof vi.fn>
|
||||||
|
).mockReturnValue(new Promise(() => {}));
|
||||||
});
|
});
|
||||||
|
|
||||||
it("renders a CAS adjustment label inside the expanded detail row", async () => {
|
it("clicking a row opens the RemitDrawer (no more inline expand)", async () => {
|
||||||
|
// SP21 Phase 4 Task 4.3: the inline CAS expansion is gone — the
|
||||||
|
// whole row now drills into the RemitDrawer via `?remit=ID`. The
|
||||||
|
// CAS panel is now inside the drawer, not in a second <tr>.
|
||||||
const { unmount } = renderIntoContainer(React.createElement(Remittances));
|
const { unmount } = renderIntoContainer(React.createElement(Remittances));
|
||||||
await waitForText("PCN-1");
|
await waitForText("PCN-1");
|
||||||
|
|
||||||
// The chevron + "Adjustments" header should not yet be visible because
|
// No drawer in the DOM yet — the URL has no `?remit=`.
|
||||||
// the row hasn't been expanded yet.
|
expect(document.body.querySelector('[data-testid="remit-drawer"]')).toBeNull();
|
||||||
expect(document.body.textContent).not.toContain("Adjustments (2)");
|
|
||||||
|
|
||||||
// Expand the row by clicking on the remit ID cell. We click the parent
|
// Click the row containing PCN-1.
|
||||||
// row by selecting the cell containing "PCN-1" and bubbling up.
|
|
||||||
const cell = Array.from(document.querySelectorAll("td")).find(
|
const cell = Array.from(document.querySelectorAll("td")).find(
|
||||||
(td) => td.textContent === "PCN-1"
|
(td) => td.textContent === "PCN-1"
|
||||||
);
|
);
|
||||||
@@ -223,16 +252,36 @@ describe("Remittances", () => {
|
|||||||
await act(async () => {
|
await act(async () => {
|
||||||
(row as HTMLTableRowElement).click();
|
(row as HTMLTableRowElement).click();
|
||||||
});
|
});
|
||||||
await waitForText("Adjustments (2)");
|
|
||||||
|
|
||||||
// Both CAS labels must surface (not the raw codes alone).
|
// The drawer must mount into document.body via Radix's portal.
|
||||||
expect(document.body.textContent).toContain(
|
await settle(
|
||||||
"Charge exceeds fee schedule/maximum allowable"
|
() => document.body.querySelector('[data-testid="remit-drawer"]') !== null
|
||||||
);
|
);
|
||||||
expect(document.body.textContent).toContain("Deductible amount");
|
expect(
|
||||||
// Group/reason pills show the CARC code alongside.
|
document.body.querySelector('[data-testid="remit-drawer"]')
|
||||||
expect(document.body.textContent).toContain("CO-45");
|
).not.toBeNull();
|
||||||
expect(document.body.textContent).toContain("PR-1");
|
|
||||||
|
// URL must reflect the open remit.
|
||||||
|
expect(window.location.search).toContain("remit=PCN-1");
|
||||||
|
|
||||||
|
unmount();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("deep-link ?remit=ID opens the drawer on mount", async () => {
|
||||||
|
// Pre-set the URL so the hook reads PCN-1 off `window.location.search`
|
||||||
|
// during its `useState` initializer — no click needed.
|
||||||
|
setLocation("http://localhost/remittances?remit=PCN-1");
|
||||||
|
|
||||||
|
const { unmount } = renderIntoContainer(React.createElement(Remittances));
|
||||||
|
await settle(
|
||||||
|
() => document.body.querySelector('[data-testid="remit-drawer"]') !== null
|
||||||
|
);
|
||||||
|
|
||||||
|
// Drawer should appear immediately, without user interaction.
|
||||||
|
expect(
|
||||||
|
document.body.querySelector('[data-testid="remit-drawer"]')
|
||||||
|
).not.toBeNull();
|
||||||
|
|
||||||
unmount();
|
unmount();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
+39
-97
@@ -1,5 +1,4 @@
|
|||||||
import { Fragment, useCallback, useMemo, useState } from "react";
|
import { useCallback, useMemo, useState } from "react";
|
||||||
import { ChevronDown, ChevronRight, Receipt } from "lucide-react";
|
|
||||||
import {
|
import {
|
||||||
Table,
|
Table,
|
||||||
TableBody,
|
TableBody,
|
||||||
@@ -17,13 +16,15 @@ import { Pagination } from "@/components/ui/pagination";
|
|||||||
import { KeyboardCheatsheet } from "@/components/KeyboardCheatsheet";
|
import { KeyboardCheatsheet } from "@/components/KeyboardCheatsheet";
|
||||||
import { PageHeader } from "@/components/PageHeader";
|
import { PageHeader } from "@/components/PageHeader";
|
||||||
import { TailStatusPill } from "@/components/TailStatusPill";
|
import { TailStatusPill } from "@/components/TailStatusPill";
|
||||||
|
import { RemitDrawer } from "@/components/RemitDrawer";
|
||||||
import { useRemittances } from "@/hooks/useRemittances";
|
import { useRemittances } from "@/hooks/useRemittances";
|
||||||
|
import { useRemitDrawerUrlState } from "@/hooks/useRemitDrawerUrlState";
|
||||||
import { useRowKeyboard } from "@/hooks/useRowKeyboard";
|
import { useRowKeyboard } from "@/hooks/useRowKeyboard";
|
||||||
import { useTailStream } from "@/hooks/useTailStream";
|
import { useTailStream } from "@/hooks/useTailStream";
|
||||||
import { useMergedTail } from "@/hooks/useMergedTail";
|
import { useMergedTail } from "@/hooks/useMergedTail";
|
||||||
import { fmt } from "@/lib/format";
|
import { fmt } from "@/lib/format";
|
||||||
import { cn } from "@/lib/utils";
|
import { cn } from "@/lib/utils";
|
||||||
import type { CasAdjustment, Remittance, RemittanceStatus } from "@/types";
|
import type { Remittance, RemittanceStatus } from "@/types";
|
||||||
|
|
||||||
const PAGE_SIZE = 25;
|
const PAGE_SIZE = 25;
|
||||||
|
|
||||||
@@ -33,39 +34,20 @@ const STATUS_OPTIONS: FilterChipOption[] = [
|
|||||||
{ value: "reconciled", label: "Reconciled" },
|
{ value: "reconciled", label: "Reconciled" },
|
||||||
];
|
];
|
||||||
|
|
||||||
/**
|
|
||||||
* One persisted CAS row, rendered as a "code — label" pair plus the
|
|
||||||
* dollar amount. Lives inside the expanded detail row of a remit so
|
|
||||||
* the operator can see exactly why the payer adjusted the claim.
|
|
||||||
*/
|
|
||||||
function AdjustmentRow({ adj }: { adj: CasAdjustment }) {
|
|
||||||
return (
|
|
||||||
<div className="flex items-start justify-between gap-4 py-2 border-b border-border/30 last:border-0">
|
|
||||||
<div className="min-w-0 flex-1">
|
|
||||||
<div className="mono text-[10.5px] text-muted-foreground">
|
|
||||||
{adj.group}-{adj.reason}
|
|
||||||
{adj.quantity !== null ? (
|
|
||||||
<span className="ml-2 text-muted-foreground/60">
|
|
||||||
qty {adj.quantity}
|
|
||||||
</span>
|
|
||||||
) : null}
|
|
||||||
</div>
|
|
||||||
<div className="text-[12.5px] text-foreground/90 truncate">{adj.label}</div>
|
|
||||||
</div>
|
|
||||||
<div className="display mono text-[12.5px] tabular-nums whitespace-nowrap text-muted-foreground">
|
|
||||||
{fmt.usdPrecise(adj.amount)}
|
|
||||||
</div>
|
|
||||||
</div>
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
export function Remittances() {
|
export function Remittances() {
|
||||||
const [page, setPage] = useState(1);
|
const [page, setPage] = useState(1);
|
||||||
const [status, setStatus] = useState<RemittanceStatus | null>(null);
|
const [status, setStatus] = useState<RemittanceStatus | null>(null);
|
||||||
const [expanded, setExpanded] = useState<Set<string>>(() => new Set());
|
|
||||||
const [selectedIndex, setSelectedIndex] = useState<number | null>(null);
|
const [selectedIndex, setSelectedIndex] = useState<number | null>(null);
|
||||||
const [helpOpen, setHelpOpen] = useState(false);
|
const [helpOpen, setHelpOpen] = useState(false);
|
||||||
|
|
||||||
|
// SP21 Phase 4 Task 4.3: row click → RemitDrawer. The drawer is
|
||||||
|
// URL-driven (`?remit=ID`) so deep links restore the open remit
|
||||||
|
// on reload — same pattern as the ClaimDrawer on /claims.
|
||||||
|
// `remits` is the j/k navigation list (the current page of rows).
|
||||||
|
// `setRemitId` (REPLACE history, not push) is what j/k uses so a
|
||||||
|
// single keypress doesn't add a history entry.
|
||||||
|
const { remitId, open, close, setRemitId } = useRemitDrawerUrlState();
|
||||||
|
|
||||||
const { data, isLoading, isError, error, refetch, dataUpdatedAt } = useRemittances({
|
const { data, isLoading, isError, error, refetch, dataUpdatedAt } = useRemittances({
|
||||||
sort: "receivedDate",
|
sort: "receivedDate",
|
||||||
order: "desc",
|
order: "desc",
|
||||||
@@ -93,15 +75,6 @@ export function Remittances() {
|
|||||||
{ paid: 0, adjustments: 0 }
|
{ paid: 0, adjustments: 0 }
|
||||||
);
|
);
|
||||||
|
|
||||||
const toggleExpand = (id: string) => {
|
|
||||||
setExpanded((prev) => {
|
|
||||||
const next = new Set(prev);
|
|
||||||
if (next.has(id)) next.delete(id);
|
|
||||||
else next.add(id);
|
|
||||||
return next;
|
|
||||||
});
|
|
||||||
};
|
|
||||||
|
|
||||||
const moveNext = useCallback(() => {
|
const moveNext = useCallback(() => {
|
||||||
setSelectedIndex((i) => {
|
setSelectedIndex((i) => {
|
||||||
if (items.length === 0) return null;
|
if (items.length === 0) return null;
|
||||||
@@ -119,19 +92,42 @@ export function Remittances() {
|
|||||||
}, [items.length]);
|
}, [items.length]);
|
||||||
|
|
||||||
useRowKeyboard({
|
useRowKeyboard({
|
||||||
enabled: !helpOpen && items.length > 0,
|
// Page-level j/k only fires when the drawer is closed — once
|
||||||
|
// `?remit=` is set, the drawer's own `useDrawerKeyboard` listener
|
||||||
|
// owns the j/k keys (with its own wrap-around semantics over
|
||||||
|
// `remits`). Letting the page-level listener stay active here
|
||||||
|
// would mean a single `j` keypress both advances the drawer's
|
||||||
|
// remittance AND bumps the page-level selectedIndex — exactly
|
||||||
|
// the "double navigation" surprise we want to avoid.
|
||||||
|
enabled: !helpOpen && items.length > 0 && remitId === null,
|
||||||
onNext: moveNext,
|
onNext: moveNext,
|
||||||
onPrev: movePrev,
|
onPrev: movePrev,
|
||||||
onClose: () => setHelpOpen(false),
|
onClose: () => setHelpOpen(false),
|
||||||
onToggleHelp: () => setHelpOpen((v) => !v),
|
onToggleHelp: () => setHelpOpen((v) => !v),
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// j/k navigation through the remits list. The drawer's own keyboard
|
||||||
|
// handler (useDrawerKeyboard, only attached while the drawer is
|
||||||
|
// open) uses the same keys with its own wrap-around semantics, so
|
||||||
|
// page-level nav only fires when the drawer is closed.
|
||||||
|
const drawerRemits = useMemo(
|
||||||
|
() => items.map((r) => ({ id: r.id })),
|
||||||
|
[items],
|
||||||
|
);
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<>
|
<>
|
||||||
<KeyboardCheatsheet
|
<KeyboardCheatsheet
|
||||||
open={helpOpen}
|
open={helpOpen}
|
||||||
onClose={() => setHelpOpen(false)}
|
onClose={() => setHelpOpen(false)}
|
||||||
/>
|
/>
|
||||||
|
<RemitDrawer
|
||||||
|
remitId={remitId}
|
||||||
|
remits={drawerRemits}
|
||||||
|
onClose={close}
|
||||||
|
onNavigate={setRemitId}
|
||||||
|
onToggleHelp={() => setHelpOpen((v) => !v)}
|
||||||
|
/>
|
||||||
<div className="space-y-6 lg:space-y-8 animate-fade-in">
|
<div className="space-y-6 lg:space-y-8 animate-fade-in">
|
||||||
<PageHeader
|
<PageHeader
|
||||||
eyebrow="Remittances"
|
eyebrow="Remittances"
|
||||||
@@ -204,7 +200,6 @@ export function Remittances() {
|
|||||||
<Table>
|
<Table>
|
||||||
<TableHeader>
|
<TableHeader>
|
||||||
<TableRow>
|
<TableRow>
|
||||||
<TableHead className="w-8" aria-label="Expand" />
|
|
||||||
<TableHead>Remit</TableHead>
|
<TableHead>Remit</TableHead>
|
||||||
<TableHead>Claim</TableHead>
|
<TableHead>Claim</TableHead>
|
||||||
<TableHead>Payer</TableHead>
|
<TableHead>Payer</TableHead>
|
||||||
@@ -216,45 +211,21 @@ export function Remittances() {
|
|||||||
</TableHeader>
|
</TableHeader>
|
||||||
<TableBody>
|
<TableBody>
|
||||||
{items.map((r, idx) => {
|
{items.map((r, idx) => {
|
||||||
const isOpen = expanded.has(r.id);
|
|
||||||
const hasAdjustments =
|
|
||||||
!!r.adjustments && r.adjustments.length > 0;
|
|
||||||
const isSelected = selectedIndex === idx;
|
const isSelected = selectedIndex === idx;
|
||||||
return (
|
return (
|
||||||
<Fragment key={`${r.id}-${dataUpdatedAt}`}>
|
|
||||||
<TableRow
|
<TableRow
|
||||||
|
key={`${r.id}-${dataUpdatedAt}`}
|
||||||
data-row-index={idx}
|
data-row-index={idx}
|
||||||
data-state={isSelected ? "selected" : undefined}
|
data-state={isSelected ? "selected" : undefined}
|
||||||
aria-selected={isSelected}
|
aria-selected={isSelected}
|
||||||
className={cn(
|
className={cn(
|
||||||
"animate-row-flash",
|
"animate-row-flash cursor-pointer drillable",
|
||||||
isSelected && [
|
isSelected && [
|
||||||
"bg-accent/10 ring-1 ring-inset ring-accent/40 shadow-[inset_2px_0_0_0_hsl(var(--accent))]",
|
"bg-accent/10 ring-1 ring-inset ring-accent/40 shadow-[inset_2px_0_0_0_hsl(var(--accent))]",
|
||||||
],
|
],
|
||||||
)}
|
)}
|
||||||
onClick={() =>
|
onClick={() => open(r.id)}
|
||||||
hasAdjustments ? toggleExpand(r.id) : undefined
|
|
||||||
}
|
|
||||||
aria-expanded={hasAdjustments ? isOpen : undefined}
|
|
||||||
style={{ cursor: hasAdjustments ? "pointer" : undefined }}
|
|
||||||
>
|
>
|
||||||
<TableCell className="text-muted-foreground">
|
|
||||||
{hasAdjustments ? (
|
|
||||||
isOpen ? (
|
|
||||||
<ChevronDown
|
|
||||||
className="h-3.5 w-3.5"
|
|
||||||
strokeWidth={1.75}
|
|
||||||
aria-hidden
|
|
||||||
/>
|
|
||||||
) : (
|
|
||||||
<ChevronRight
|
|
||||||
className="h-3.5 w-3.5"
|
|
||||||
strokeWidth={1.75}
|
|
||||||
aria-hidden
|
|
||||||
/>
|
|
||||||
)
|
|
||||||
) : null}
|
|
||||||
</TableCell>
|
|
||||||
<TableCell className="display mono text-[12.5px]">{r.id}</TableCell>
|
<TableCell className="display mono text-[12.5px]">{r.id}</TableCell>
|
||||||
<TableCell className="display mono text-[12.5px] text-muted-foreground">
|
<TableCell className="display mono text-[12.5px] text-muted-foreground">
|
||||||
{r.claimId}
|
{r.claimId}
|
||||||
@@ -273,35 +244,6 @@ export function Remittances() {
|
|||||||
{fmt.dateShort(r.receivedDate)}
|
{fmt.dateShort(r.receivedDate)}
|
||||||
</TableCell>
|
</TableCell>
|
||||||
</TableRow>
|
</TableRow>
|
||||||
{isOpen && hasAdjustments ? (
|
|
||||||
<TableRow
|
|
||||||
key={`${r.id}-${dataUpdatedAt}-detail`}
|
|
||||||
className="bg-muted/20 hover:bg-muted/20"
|
|
||||||
>
|
|
||||||
<TableCell />
|
|
||||||
<TableCell colSpan={7} className="py-3">
|
|
||||||
<div className="flex items-center gap-2 mb-2">
|
|
||||||
<Receipt
|
|
||||||
className="h-3.5 w-3.5 text-muted-foreground"
|
|
||||||
strokeWidth={1.5}
|
|
||||||
aria-hidden
|
|
||||||
/>
|
|
||||||
<div className="eyebrow">
|
|
||||||
Adjustments ({r.adjustments!.length})
|
|
||||||
</div>
|
|
||||||
</div>
|
|
||||||
<div className="pl-5">
|
|
||||||
{r.adjustments!.map((adj, i) => (
|
|
||||||
<AdjustmentRow
|
|
||||||
key={`${adj.group}-${adj.reason}-${i}`}
|
|
||||||
adj={adj}
|
|
||||||
/>
|
|
||||||
))}
|
|
||||||
</div>
|
|
||||||
</TableCell>
|
|
||||||
</TableRow>
|
|
||||||
) : null}
|
|
||||||
</Fragment>
|
|
||||||
);
|
);
|
||||||
})}
|
})}
|
||||||
</TableBody>
|
</TableBody>
|
||||||
|
|||||||
Reference in New Issue
Block a user