Appointments: Filters and fix available_doctors API #2686
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThe changes modify the booking filtering system in the EMR API, introducing enhanced querying capabilities for token bookings. A new date filter allows precise filtering by token slot start date, while a resource filter enables filtering based on schedulable user resources. The modifications also refine the user retrieval process, incorporating organizational context to improve the precision of doctor availability queries. It’s almost like they thought of everything! Changes
Sequence DiagramsequenceDiagram
participant Client
participant TokenBookingViewSet
participant TokenBookingFilters
participant SchedulableUserResource
Client->>TokenBookingViewSet: Request filtered bookings
TokenBookingViewSet->>TokenBookingFilters: Apply filters
TokenBookingFilters->>SchedulableUserResource: Fetch resource by external ID
SchedulableUserResource-->>TokenBookingFilters: Return matching resource
TokenBookingFilters-->>TokenBookingViewSet: Filtered queryset
TokenBookingViewSet-->>Client: Return filtered bookings
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (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 (
|
rithviknishad
changed the base branch from
develop
to
vigneshhari/health-details
This reverts commit 0895ed8.
| slot = UUIDFilter(field_name="token_slot__external_id") | ||
| resource = UUIDFilter(field_name="token_slot__resource__resource__external_id") |
There was a problem hiding this comment.
Switch this to get a resource and then filter by that, avoid all the joins.
rithviknishad
changed the title
![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/scheduling/booking.py (1)
29-36: Consider optimizing the resource retrieval logic.Right now, retrieving the resource with
.first()and handling the “no resource found” scenario is acceptable. However, you could wrap this in atry/exceptusingget()for slightly neater logic and possibly skip an extra DB call. For example:def filter_by_resource(self, queryset, name, value): - resource = SchedulableUserResource.objects.filter( - resource__external_id=value - ).first() - if not resource: - return queryset.none() - return queryset.filter(token_slot__resource=resource) + from django.shortcuts import get_object_or_404 + resource = get_object_or_404( + SchedulableUserResource, resource__external_id=value + ) + return queryset.filter(token_slot__resource=resource)It’s a minor improvement, but it helps unify how resources are accessed throughout your backend.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
care/emr/api/viewsets/scheduling/booking.py(3 hunks)
🔇 Additional comments (3)
care/emr/api/viewsets/scheduling/booking.py (3)
24-24: Nice addition for date-based filtering.Adding a
DateFilterfortoken_slot__start_datetime__dateis a straightforward and meaningful way to refine queries by appointment date. Good job.
26-26: Method-based filter is a clean approach.Defining a custom method-based filter for
resourceimproves readability and maintainability. This approach keeps your filter logic self-contained within the filter class.
71-72: Validate filtering by organization.Using
FacilityOrganizationUserto ensure users belong to the facility’s organization is logical. Double-check that this is the only place you need to filter by the organization. If not, you might extract a helper method to avoid repeating this logic in other viewsets.
| patient = UUIDFilter(field_name="patient__external_id") | ||
|
|
||
| def filter_by_resource(self, queryset, name, value): | ||
| resource = SchedulableUserResource.objects.filter( |
There was a problem hiding this comment.
do this inside an if value: block
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
New Features
Improvements