Username filter in facility organization #2784
Conversation
📝 WalkthroughWalkthroughThe pull request introduces a new filter class Changes
Possibly related PRs
Suggested reviewers
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 (
|
![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 (1)
care/emr/api/viewsets/facility_organization.py (1)
152-153: I see we're keeping things... minimal.While the username filter implementation works, it might be nice if we could add some basic input validation and perhaps consider adding filters for other relevant user fields. You know, just to make it a bit more... complete.
Consider enhancing the filter class with additional fields and validation:
class FacilityOrganizationUserFilter(filters.FilterSet): username = filters.CharFilter(field_name="user__username", lookup_expr="icontains") + email = filters.CharFilter(field_name="user__email", lookup_expr="icontains") + is_active = filters.BooleanFilter(field_name="user__is_active") + + class Meta: + model = FacilityOrganizationUser + fields = ["username", "email", "is_active"]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
care/emr/api/viewsets/facility_organization.py(1 hunks)
🔇 Additional comments (2)
care/emr/api/viewsets/facility_organization.py (2)
158-159: Oh, I noticed we forgot something... tests perhaps?The filter configuration looks good, but it would be absolutely wonderful if we could add some test cases to verify the username filtering functionality. You know, just to make sure everything works as intended... now and in the future.
Would you like me to help generate some test cases? Here's what we should cover:
- Filtering with exact username match
- Filtering with partial username match
- Case-insensitive matching
- Empty result handling
- Special character handling
152-159: Just a tiny performance consideration we might want to think about...The
icontainslookup on username could potentially be resource-intensive on large datasets. It might be worth ensuring we have appropriate indexing and rate limiting in place.Let's check the database indexing and existing rate limiting configuration:
|
PRs not following the merge checklist would be closed |
|
@rithviknishad @Jacobjeevan username filtering test cases for other entities are not present in entire codebase. Correct me if i am wrong and also can you please guide me on how to write test cases in backend if i need to write test cases. |
Proposed Changes
Associated Issue
Architecture 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