fix(spp_dci_client_dr): parse DCI v1.0.0 spec envelope and record fields#203
Conversation
The sync DR path treated `search_response[*].data` as a flat record, and both sync and callback paths read flat fields that do not exist in spec-shape records (`disability_status` string, `disability_details[*].impairment_type`). A spec-conformant DR server was silently classified as "no disability". Route both paths through a shared `dr_parsing` module so any spec-shape envelope is correctly unwrapped and record fields are derived from `disability_status` and `disability_details`. Date coercion (ISO datetime / `Z` suffix) is centralized so both sync and callback hand the ORM a `date` object. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces a new dr_parsing service containing stateless helpers to standardize the extraction of DCI v1.0.0 Disability Registry records. The DRService and callback router have been refactored to delegate parsing logic to this module, enhancing testability and specification compliance. Comprehensive tests were also added for the new parsing logic and callback processing. A review comment suggests improving the handling of null values for the disability_status field to prevent incorrect "Unknown status" warnings.
| disability_types = [d["impairment_type"] for d in details if isinstance(d, dict) and d.get("impairment_type")] | ||
|
|
||
| # Resolve has_disability from the disability_status string | ||
| status_str = str(record.get("disability_status", "")).strip().lower() |
There was a problem hiding this comment.
When the disability_status field is explicitly null in the JSON payload, record.get("disability_status", "") returns None. Passing this to str() results in the string "none" (after .lower()), which is not one of the expected status tokens. This causes the parser to fall through to the else block at line 113, logging an "Unknown disability_status value: None" warning. Using or "" inside the str() call ensures that null values are treated as empty strings, which are correctly handled as an ambiguous status without triggering a warning.
| status_str = str(record.get("disability_status", "")).strip().lower() | |
| status_str = str(record.get("disability_status") or "").strip().lower() |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 19.0 #203 +/- ##
==========================================
+ Coverage 66.08% 66.37% +0.28%
==========================================
Files 86 124 +38
Lines 7501 11404 +3903
==========================================
+ Hits 4957 7569 +2612
- Misses 2544 3835 +1291
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…r_parsing
The callback path duplicated `data.get("reg_records", [])` without the
type guards and WARN logging now centralized in `unwrap_search_data`.
A malformed `data` envelope was silently coerced to `{}` and skipped,
hiding spec drift from operators. Route through the shared helper so
both sync and callback paths get identical validation behavior.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Summary
search_response[*].dataas a flat record (it is actually a{version, reg_record_type, reg_records: [...]}envelope per DCI v1.0.0), and both sync and callback paths read flat fields that do not exist on spec-shape records. A spec-conformant DR server was silently classified as 'no disability'.services/dr_parsing.pycentralizes envelope unwrap, record-field extraction (disability_statusstring +disability_details[*].impairment_type), and ISO date coercion. Both sync (dr_service.py) and callback (routers/callback.py) now route through it.approved/registered-> True,rejected/denied-> False, empty -> impairment-list fallback, unknown -> WARN + fallback). Legacyyes/truetokens are no longer treated as approved.Notable changes
disability_details: Noneon the wire no longer crashes the parser (or []guard).unwrap_search_datarejects non-dict envelopes and non-listreg_records, logging a WARN and returning[]rather than passing garbage downstream.assessment_datepreferslast_updated, falls back toregistration_date, returnsNone(with WARN) on unparseable values.extract_functional_scoresalways returns{}. DCI v1.0.0 has no numeric scoring; the hook is kept for future spec versions.Out-of-scope flag
routers/callback.py,last_sync_datewas switched fromdatetime.now(UTC)tofields.Datetime.now()while editing the same vals block. It is the correct Odoo idiom for aDatetimefield, but it is technically unrelated to the parser fix. Happy to split into a separate commit if preferred.Test plan
./spp t spp_dci_client_dr-> 92 tests, 0 failed, 0 errors (was 46 before).pre-commit run ruff --files <changed>clean.pre-commit run ruff-format --files <changed>clean.tests/test_dr_parsing.pycovers: envelope unwrap (None/empty/wrong-type/missing-key/non-list), status resolution (approved/rejected/empty/unknown + case insensitivity), impairment extraction (null-safety, missingimpairment_typeskipped), date coercion (ISO datetime withZ, plain ISO date, fallback chain, unparseable -> None + WARN), andextract_functional_scoresalways{}.tests/test_callback.pycovers_process_dr_search_resultend-to-end with spec-shape envelopes: approved creates record, rejected createshas_disability=Falserecord, non-success status is skipped, repeat call updates instead of duplicating.tests/test_dr_service.pyrebuilt around the spec envelope (mock responses now wrap data in{version, reg_record_type, reg_records: [...]}).🤖 Generated with Claude Code