fix:facility tag manager
Proposed Changes
- Updated PatientFacilityTagManager
- Updated get_serializer_list_context in Patient viewset
Associated Issue
- https://github.com/ohcnetwork/care_fe/issues/14320
Architecture changes
- Remove this section if not used
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
- Improvements
- Patient retrieval now includes richer request context, improving the completeness and accuracy of returned patient data during searches and single-record views.
- Facility resolution and tagging now accept UUID-style identifiers and consistently store facility-specific tags, leading to more reliable facility-based behavior and tag lookups across the app.
โ๏ธ Tip: You can customize this high-level summary in your review settings.
๐ Walkthrough
Walkthrough
Patient retrieve now passes an explicit serializer context (from a new get_serializer_retrieve_context) into PatientRetrieveSpec.serialize; list context resolution for facility prefers request body over query params. Tagging accepts UUID facility identifiers and stores/fetches tags using the facility id string key.
Changes
| Cohort / File(s) | Summary |
|---|---|
Patient API Viewset Context Enhancementcare/emr/api/viewsets/patient.py |
search_retrieve now calls PatientRetrieveSpec.serialize(patient, **context) with context from get_serializer_retrieve_context. get_serializer_list_context reads facility from request.data first (falls back to request.GET), resolves it to a Facility object, and preserves facility-based authorization checks. Added get_serializer_retrieve_context. |
Tagging System Facility Identifier Refactoringcare/emr/tagging/base.py |
Imported UUID. PatientFacilityTagManager.__init__ accepts UUID or str for facility, resolving via external_id when a UUID is provided. get_resource_tag and set_instance_tag now use the facility's id string as the key when storing and fetching tags. |
Estimated code review effort
๐ฏ 3 (Moderate) | โฑ๏ธ ~20 minutes
- Check all callers of
PatientRetrieveSpec.serializeto ensure the added**contextparameter is compatible. - Verify facility resolution and authorization paths when
facilityis supplied viarequest.datavsrequest.GET. - Review tag storage/read logic where keys changed to stringified facility ids and UUID-based external_id resolution to avoid migration or lookup issues.
Pre-merge checks and finishing touches
โ Failed checks (1 warning, 1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| 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. |
| Title check | โ Inconclusive | The title 'fix:facility tag manager' is vague and generic, using non-descriptive phrasing that doesn't clearly convey the specific changes made to the facility tag manager or why those changes were necessary. | Consider a more descriptive title like 'refactor: support UUID-based facility identifiers in PatientFacilityTagManager' or 'fix: update facility tag manager to handle UUID resolution and context passing'. |
โ Passed checks (1 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description check | โ Passed | The description covers the main changes and references an associated issue, but lacks specific detail about what was actually fixed or changed and why these modifications solve the referenced issue. |
โจ 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
nandkishor/fix/patient_tag_manager
๐ Recent review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
๐ฅ Commits
Reviewing files that changed from the base of the PR and between 0d8b84a35d4df0f53d93ddc0600a8449f2f5a862 and cb4184efc11a238370b9b5e51bce1b3adb6e138c.
๐ Files selected for processing (1)
care/emr/tagging/base.py(2 hunks)
๐ง Files skipped from review as they are similar to previous changes (1)
- care/emr/tagging/base.py
โฐ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test / Test
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 33.33333% with 4 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 74.60%. Comparing base (59d74d8) to head (8970a03).
:warning: Report is 1 commits behind head on develop.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| care/emr/api/viewsets/patient.py | 0.00% | 3 Missing :warning: |
| care/emr/tagging/base.py | 66.66% | 0 Missing and 1 partial :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## develop #3360 +/- ##
========================================
Coverage 74.59% 74.60%
========================================
Files 449 449
Lines 20645 20647 +2
Branches 2141 2141
========================================
+ Hits 15401 15403 +2
- Misses 4781 4782 +1
+ Partials 463 462 -1
: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.