care icon indicating copy to clipboard operation
care copied to clipboard

fix:facility tag manager

Open nandkishorr opened this issue 1 month ago โ€ข 2 comments

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.

nandkishorr avatar Nov 17 '25 14:11 nandkishorr

๐Ÿ“ 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 Enhancement
care/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 Refactoring
care/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.serialize to ensure the added **context parameter is compatible.
  • Verify facility resolution and authorization paths when facility is supplied via request.data vs request.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.

โค๏ธ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Nov 17 '25 14:11 coderabbitai[bot]

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.

codecov[bot] avatar Nov 17 '25 14:11 codecov[bot]