Add search functionality to DeviceViewSet with registered_name and user_friendly_name filters #2877
Conversation
…er_friendly_name filters
📝 WalkthroughWalkthroughThe changes update the device API by modifying the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant DeviceViewSet
participant DjangoFilterBackend
participant SearchFilter
Client->>DeviceViewSet: GET /devices?search=<query>
DeviceViewSet->>DjangoFilterBackend: Apply standard filters
DeviceViewSet->>SearchFilter: Apply search filter on specified fields
DeviceViewSet-->>Client: Return filtered device list
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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/device.py (1)
60-61: Search functionality looks good, but be aware of potential redundancy.The implementation of search functionality is correct and aligns with DRF best practices. However, I noticed there's now some redundancy between the new search field for "registered_name" and the existing filter for the same field in
DeviceFiltersclass (line 51). This means users can filter by registered_name using either the filter parameter or the search parameter.While this redundancy isn't necessarily a problem, it might be worth adding a code comment explaining why both are available, to prevent someone from removing one of them in the future thinking it's unnecessary duplication.
filter_backends = (filters.DjangoFilterBackend, drf_filters.SearchFilter) - search_fields = ["registered_name", "user_friendly_name"] + # Note: registered_name is available both as a filter and search field for different query use cases + search_fields = ["registered_name", "user_friendly_name"]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
care/emr/api/viewsets/device.py(2 hunks)
🔇 Additional comments (1)
care/emr/api/viewsets/device.py (1)
6-6: New import added correctly, but consider consolidating imports.The new import for DRF filters is correctly added, but you might want to consider consolidating your filter imports for slightly better code organization. Not a big deal though, I suppose.
|
@rithviknishad @vigneshhari Can you please review this PR let me know if any changes are required |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2877 +/- ##
========================================
Coverage 56.32% 56.32%
========================================
Files 226 226
Lines 10948 10950 +2
Branches 1112 1112
========================================
+ Hits 6166 6168 +2
Misses 4765 4765
Partials 17 17 ☔ View full report in Codecov by Sentry. |
|
LGTM. Note that |
Yeah implemented same in the frontend |
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