fix:added field validator for onset datatime in allergy intolerance
Proposed Changes
- Added a field validator for onset date time
Merge Checklist
- [ ] Tests added/fixed
- [ ] Update docs in
/docs - [ ] Linting Complete
- [ ] Any other necessary step
Only 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
- Bug Fixes
- Improved date validation for allergy, condition, and medication records: onset/authored dates are normalized for timezone and cannot be set in the future. Users will see clear errors when entering invalid dates.
- Tests
- Updated test expectations to reflect a standardized error message: "Date cannot be in the future."
📝 Walkthrough
Walkthrough
Added a shared datetime validator and wired it into allergy, condition, and medication-request specs to centralize timezone-awareness and future-date checks; tests updated to expect the new generic future-date error message.
Changes
| Cohort / File(s) | Change Summary |
|---|---|
Common validatorcare/emr/resources/common/validators.py |
Added validate_datetime(datetime: datetime.datetime) to enforce timezone-awareness and prevent future datetimes. |
Allergy onset validatorcare/emr/resources/allergy_intolerance/spec.py |
Added @field_validator("onset_datetime") validate_onset_datetime that delegates to validate_datetime. |
Condition onset validatorcare/emr/resources/condition/spec.py |
Replaced manual timezone/future checks with a call to validate_datetime; removed unused imports and dropped the unused info parameter. |
Medication authored_on validatorcare/emr/resources/medication/request/spec.py |
Added @field_validator("authored_on") validate_authored_on that delegates to validate_datetime. |
Testscare/emr/tests/test_diagnosis_api.py, care/emr/tests/test_symptom_api.py |
Updated assertions to expect the more generic "Date cannot be in the future" error message. |
Sequence Diagram(s)
sequenceDiagram
participant User as User
participant Spec as SpecClass
participant Validator as CommonValidator
Note over Spec,Validator: New centralized datetime validation flow
User->>Spec: submit onset_datetime / authored_on
Spec->>Validator: validate_datetime(datetime)
Validator->>Validator: normalize timezone (make aware if needed)
Validator->>Validator: check not in future (care_now())
alt invalid (future)
Validator-->>Spec: raise ValueError ("Date cannot be in the future")
else valid
Validator-->>Spec: return validated datetime
end
Spec-->>User: accept or reject input
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
- ohcnetwork/care#2915 — touches the same Condition onset_datetime validator refactor; likely overlapping validation changes.
- ohcnetwork/care#2745 — introduces/changes authored_on handling for MedicationRequest, related to the new authored_on validator.
Suggested reviewers
- vigneshhari
- praffq
Poem
A datetime once wandered, lax and loose,
Herded now through one exacting no-nonsense sluice.
Timezones made proper, futures firmly barred,
One validator stands — tidy, slightly hard. ⏰✨
Pre-merge checks and finishing touches
❌ Failed checks (2 warnings)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Description Check | ⚠️ Warning | The description includes the Proposed Changes and Merge Checklist sections as required, but it omits the mandatory Associated Issue section from the template, leaving reviewers without context for which issue this PR addresses. The Architecture changes section can be removed if unused, but the missing issue link is a critical gap. Without this information the template is incomplete and reviewers may struggle to verify the related issue. | Please add an Associated Issue section with a link to the relevant issue and a brief explanation of how this PR resolves it so that reviewers have the necessary context. |
| Docstring Coverage | ⚠️ Warning | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (1 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title Check | ✅ Passed | The title clearly indicates that a field validator has been added for the onset datetime in the allergy intolerance context, which is a genuine aspect of the changeset, and it is not overly vague or unrelated. It succinctly describes a substantive change rather than using generic phrasing, so it meets the requirement for being related to the main change. While there are broader changes in other specs and tests, the title need not enumerate every detail. |
✨ Finishing touches
- [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
- [ ] Create PR with unit tests
- [ ] Post copyable unit tests in a comment
- [ ] Commit unit tests in branch
nandkishorr/fix/allergy-intolerence/onset-validation
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands and usage tips.
Codecov Report
:x: Patch coverage is 50.00000% with 14 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 53.87%. Comparing base (fefde08) to head (7c61bfa).
:warning: Report is 17 commits behind head on develop.
Additional details and impacted files
@@ Coverage Diff @@
## develop #3120 +/- ##
========================================
Coverage 53.87% 53.87%
========================================
Files 434 435 +1
Lines 19683 19703 +20
Branches 2129 2132 +3
========================================
+ Hits 10604 10615 +11
- Misses 9051 9060 +9
Partials 28 28
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
^ Resolve the comments
tests failing , please check @nandkishorr
This is not just validation, we are also adding timezone data to data without timezone, was that intended?
This is not just validation, we are also adding timezone data to data without timezone, was that intended?
Its the same behavior as earlier, just refactored to be used in multiple places
We can reject dates without tz but that would make the frontend unhappy