Conversation
📝 WalkthroughWalkthroughA single filter field Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@care/emr/api/viewsets/scheduling/token.py`:
- Line 37: Add unit tests covering the new patient_filter =
UUIDFilter(field_name="patient__external_id") on the Token viewset: write three
concise tests for the Token list endpoint (or TokenViewSet) that assert (1) a
token is returned when filtering by a valid patient's external_id, (2) no tokens
are returned when the external_id does not match any patient, and (3) the API
returns a 400/validation error (or behaves as your filter layer does) when given
an invalid UUID string; create test fixtures for a patient with external_id and
associated tokens, call the viewset list route with query param
patient=<external_id> and assert responses accordingly, and include these tests
in the repo test suite so the new patient_filter is covered.
🪄 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: 865bbed2-8fba-4e01-8dc3-db6fdab4bb15
📒 Files selected for processing (1)
care/emr/api/viewsets/scheduling/token.py
| sub_queue_is_null = NullFilter(field_name="sub_queue") | ||
| date = DateFilter(field_name="queue__date") # For dependent filtering only | ||
| patient_name = CharFilter(field_name="patient__name", lookup_expr="icontains") | ||
| patient_filter = UUIDFilter(field_name="patient__external_id") |
There was a problem hiding this comment.
Add coverage for the new patient filter before merge.
Line 37 introduces a new list-filter surface, so it really should ship with tests (valid patient external_id match, non-match, and invalid UUID input). The implementation itself is clean, but leaving it untested is an easy way to regress quietly later.
Based on learnings: Token booking flows assume a patient is always associated, and external_id-path filtering is a supported pattern in this codebase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@care/emr/api/viewsets/scheduling/token.py` at line 37, Add unit tests
covering the new patient_filter = UUIDFilter(field_name="patient__external_id")
on the Token viewset: write three concise tests for the Token list endpoint (or
TokenViewSet) that assert (1) a token is returned when filtering by a valid
patient's external_id, (2) no tokens are returned when the external_id does not
match any patient, and (3) the API returns a 400/validation error (or behaves as
your filter layer does) when given an invalid UUID string; create test fixtures
for a patient with external_id and associated tokens, call the viewset list
route with query param patient=<external_id> and assert responses accordingly,
and include these tests in the repo test suite so the new patient_filter is
covered.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #3573 +/- ##
===========================================
+ Coverage 77.05% 77.06% +0.01%
===========================================
Files 474 474
Lines 22271 22272 +1
Branches 2325 2325
===========================================
+ Hits 17161 17164 +3
+ Misses 4553 4552 -1
+ Partials 557 556 -1 ☔ 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