Fixed patient filter in consent viewset #2942
Conversation
📝 WalkthroughWalkthroughThe changes update the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant V as ConsentViewSet
participant DB as Database
U->>V: GET /consents?patient_external_id=xyz
V->>DB: Filter query on consents (by patient_external_id)
DB-->>V: Return matching consent records
V-->>U: 200 OK with filtered consent data
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
care/emr/tests/test_consent_api.py (2)
78-102: Test case properly validates the filtering functionalityThe test case creates an appropriate scenario with multiple patients and consents, then verifies that only the consent belonging to the authenticated patient is returned. This confirms the patient filtering in the viewset is working correctly.
However, there's a minor formatting issue flagged by the linter:
encounter_self = self.create_encounter( - patient=self.patient, facility=self.facility, organization=self.organization + patient=self.patient, + facility=self.facility, + organization=self.organization, )🧰 Tools
🪛 GitHub Actions: Lint Code Base
[error] 87-89: ruff-format failed. 1 file reformatted. Please review the changes made by the hook.
100-101: Clear assertion message would be helpfulThe assertion is correct, but adding a descriptive message would make failures easier to debug.
-self.assertEqual(len(consents), 1) -self.assertEqual(consents[0]["external_id"], consent_self.external_id) +self.assertEqual(len(consents), 1, "Only one consent should be returned") +self.assertEqual(consents[0]["external_id"], consent_self.external_id, "The returned consent should belong to the authenticated patient")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
care/emr/api/viewsets/consent.py(1 hunks)care/emr/tests/test_consent_api.py(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Lint Code Base
care/emr/tests/test_consent_api.py
[error] 87-89: ruff-format failed. 1 file reformatted. Please review the changes made by the hook.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test / Test
🔇 Additional comments (1)
care/emr/api/viewsets/consent.py (1)
32-39: Good addition of patient filteringThe added filter ensures consents are properly filtered by patient's external_id, which aligns with the PR objectives. This improvement enhances data isolation between patients and fixes the issue mentioned in the PR description.
I would suggest adding a comment explaining the purpose of this filter since it's a security-related change, but I suppose the code is clear enough... maybe.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2942 +/- ##
========================================
Coverage 52.33% 52.33%
========================================
Files 239 239
Lines 10710 10710
Branches 1177 1177
========================================
Hits 5605 5605
Misses 5088 5088
Partials 17 17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Proposed Changes
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
New Features
Tests