[ENG-381] feat: add extension support for datapoints#3661
Conversation
📝 WalkthroughWalkthroughA new ChangesPatient Extensions Context Field
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3661 +/- ##
===========================================
+ Coverage 75.83% 75.85% +0.02%
===========================================
Files 479 479
Lines 23014 23035 +21
Branches 2378 2378
===========================================
+ Hits 17452 17473 +21
- Misses 4988 4989 +1
+ Partials 574 573 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Greptile SummaryAdds
Confidence Score: 4/5The change is a small, additive field declaration with no side effects on existing fields or queries. The only concern is that care/emr/reports/context_builder/data_points/patient.py — the new Important Files Changed
Reviews (1): Last reviewed commit: "chore: switch to just json" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| extensions = Field( | ||
| display="Patient Extensions", | ||
| mapping=lambda p: p.extensions or {}, | ||
| preview_value={ | ||
| "patient_demographics": { | ||
| "related_person": "Jane Doe", | ||
| }, | ||
| }, | ||
| description="Patient extensions as JSON (e.g. patient.extensions.patient_demographics)", | ||
| ) |
There was a problem hiding this comment.
field_type defaults to "string", but this field actually returns a dict. The build_schema() utility exposes attr.type in the schema output consumed by frontend/template tooling, so leaving it as "string" will mislead consumers about what to expect when rendering patient.extensions.
| extensions = Field( | |
| display="Patient Extensions", | |
| mapping=lambda p: p.extensions or {}, | |
| preview_value={ | |
| "patient_demographics": { | |
| "related_person": "Jane Doe", | |
| }, | |
| }, | |
| description="Patient extensions as JSON (e.g. patient.extensions.patient_demographics)", | |
| ) | |
| extensions = Field( | |
| display="Patient Extensions", | |
| mapping=lambda p: p.extensions or {}, | |
| preview_value={ | |
| "patient_demographics": { | |
| "related_person": "Jane Doe", | |
| }, | |
| }, | |
| description="Patient extensions as JSON (e.g. patient.extensions.patient_demographics)", | |
| field_type="json", | |
| ) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@care/emr/reports/context_builder/data_points/patient.py`:
- Around line 184-193: The mapping for the Field named "extensions" currently
assumes Patient.extensions is a dict by using mapping=lambda p: p.extensions or
{}, but since writes may bypass validators, change it to defensively return a
dict only when extensions is actually a dict; update the mapping to check type
(e.g. use isinstance(p.extensions, dict) in the lambda) so non-dict values fall
back to {} — locate the Field declaration for "extensions" and modify its
mapping lambda accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d355de59-ed59-488e-9217-85eaa6bd50cd
📒 Files selected for processing (1)
care/emr/reports/context_builder/data_points/patient.py
| extensions = Field( | ||
| display="Patient Extensions", | ||
| mapping=lambda p: p.extensions or {}, | ||
| preview_value={ | ||
| "patient_demographics": { | ||
| "related_person": "Jane Doe", | ||
| }, | ||
| }, | ||
| description="Patient extensions as JSON (e.g. patient.extensions.patient_demographics)", | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check the Patient model's extensions field type to ensure it's a JSONField
# Search for the Patient model definition and extensions field
ast-grep --pattern $'class Patient($$$):
$$$
extensions = $$$
$$$'
# Also search for any field definition with 'extensions' in Patient-related files
rg -n --type=py -C5 'extensions.*=' care/emr/models/patient.pyRepository: ohcnetwork/care
Length of output: 8504
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the surrounding context of the new `extensions` field in the context builder
sed -n '140,230p' care/emr/reports/context_builder/data_points/patient.py
# Check for how `extensions` is referenced/parsed elsewhere (especially expecting dict/object)
rg -n --type=py "extensions(\.|\[|$)" care/emr | head -n 200
# Check for any model validators/clean methods enforcing `extensions` shape
rg -n --type=py "class Patient|extensions\s*=" care/emr/models/patient.py care/emr/models -SRepository: ohcnetwork/care
Length of output: 7520
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the extension validator logic that reads `obj.extensions[key]`
sed -n '1,220p' care/emr/extensions/validator.py
# Inspect Patient model for any clean/validation logic specific to extensions
rg -n --type=py "def (clean|full_clean|save)|extensions" care/emr/models/patient.py
# Inspect any extension parsing/writing paths that might enforce dict/object shape
rg -n --type=py "extensions\s*=" care/emr/models/patient.py care/emr/models -SRepository: ohcnetwork/care
Length of output: 3619
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find where ExtensionValidator is used for patient extensions ingestion/validation
rg -n --type=py "ExtensionValidator|validate_extensions\(" care/emr | head -n 200
# Check the patient extension resource spec/serializers for enforcing dict/object shape
rg -n --type=py "patient.*extensions|extensions.*patient|__.*extension_resource_type__|___extension_resource_type__" care/emr/resources/patient -SRepository: ohcnetwork/care
Length of output: 2926
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,240p' care/emr/resources/patient/spec.py
sed -n '1,120p' care/emr/resources/patient/otp_based_flow.pyRepository: ohcnetwork/care
Length of output: 11129
Patient.extensions is JSONField with dict-only validation on write paths
Patient.extensions is a Django models.JSONField(default=dict), and the patient write specs (PatientCreateSpec/PatientUpdateSpec/PatientOTPWriteSpec) inherit ExtensionValidator, which rejects non-dict extensions payloads. So mapping=lambda p: p.extensions or {} matches the app’s expected shape for JSON-path usage.
Minor hardening: if data can be written bypassing validators, use mapping=lambda p: p.extensions if isinstance(p.extensions, dict) else {}.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@care/emr/reports/context_builder/data_points/patient.py` around lines 184 -
193, The mapping for the Field named "extensions" currently assumes
Patient.extensions is a dict by using mapping=lambda p: p.extensions or {}, but
since writes may bypass validators, change it to defensively return a dict only
when extensions is actually a dict; update the mapping to check type (e.g. use
isinstance(p.extensions, dict) in the lambda) so non-dict values fall back to {}
— locate the Field declaration for "extensions" and modify its mapping lambda
accordingly.
[ENG-381] feat: add extension support for datapoints (ohcnetwork#3661)
3 participants
Proposed Changes
ENG-381
Merge Checklist
/docsOnly PR's with test cases included and passing lint and test pipelines will be reviewed
@ohcnetwork/care-backend-maintainers @ohcnetwork/care-backend-admins
Summary by CodeRabbit