Resource requests: added status, cat and title filters #2799
Conversation
📝 WalkthroughWalkthroughThe changes introduce three new filters to the Changes
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
care/emr/api/viewsets/resource_request.py (1)
33-34: Consider adding validation for status and category filters.While the implementation is functional, it might be slightly more robust to validate these values against predefined choices. You know, just to prevent those pesky invalid status and category queries that might slip through. 😊
Consider updating the filters like this:
- status = filters.CharFilter(field_name="status", lookup_expr="iexact") - category = filters.CharFilter(field_name="category", lookup_expr="iexact") + status = filters.ChoiceFilter( + field_name="status", + choices=ResourceRequest.STATUS_CHOICES, + ) + category = filters.ChoiceFilter( + field_name="category", + choices=ResourceRequest.CATEGORY_CHOICES, + )Also, it would be absolutely wonderful if you could add some documentation about the valid status and category values. You know, for those of us who might need it someday. 📝
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
care/emr/api/viewsets/resource_request.py(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test / Test
🔇 Additional comments (2)
care/emr/api/viewsets/resource_request.py (2)
32-32: LGTM! The title filter implementation looks reasonable.Using case-insensitive containment match for the title filter will provide a good user experience for partial text searches.
32-34: Verify the existence of these fields in the model.Let's make sure these fields actually exist in the ResourceRequest model and check for any existing queries that might be affected.
✅ Verification successful
All fields exist and are properly configured in the model.
The fields
title,status, andcategoryare correctly defined in the ResourceRequest model with appropriate field types and constraints. The filters are using standard Django lookups that match the field types.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of title, status, and category fields and their usage # Check model definition echo "Checking ResourceRequest model definition..." rg -A 5 "class ResourceRequest" care/emr/models/ # Check existing queries echo "Checking existing queries..." rg -g '!*/migrations/*' "title|status|category" care/emr/Length of output: 65326
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2799 +/- ##
===========================================
+ Coverage 56.14% 56.16% +0.01%
===========================================
Files 215 215
Lines 10212 10215 +3
Branches 1030 1030
===========================================
+ Hits 5734 5737 +3
Misses 4462 4462
Partials 16 16 ☔ View full report in Codecov by Sentry. |
Proposed Changes
Associated Issue
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