-
Notifications
You must be signed in to change notification settings - Fork 440
Support for rescheduling booking to another schedulable user resource within same facility #3063
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for rescheduling booking to another schedulable user resource within same facility #3063
Conversation
… within same facility
📝 WalkthroughWalkthroughThe rescheduling logic in the booking API was updated to allow moving a booking to any resource within the same facility, rather than restricting it to the exact original resource. Corresponding tests were added to verify that rescheduling across resources within a facility is permitted, but rescheduling to a different facility is not. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BookingAPI
participant TokenSlot
participant Facility
User->>BookingAPI: POST /reschedule (with new slot external_id)
BookingAPI->>TokenSlot: Find slot by external_id and facility_id
TokenSlot-->>BookingAPI: Return slot if facility matches
BookingAPI->>BookingAPI: Update booking to new slot
BookingAPI-->>User: Return success or error
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3063 +/- ##
========================================
Coverage 51.28% 51.28%
========================================
Files 251 251
Lines 11371 11371
Branches 1280 1280
========================================
Hits 5832 5832
Misses 5518 5518
Partials 21 21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
care/emr/tests/test_booking_api.py (1)
386-386
: Address line length violations.The static analysis tool flagged these lines for exceeding the 100-character limit. While functional, it would be... nice if we could keep our code formatting consistent with the project standards.
Apply these formatting improvements:
- def test_reschedule_booking_to_another_user_resource_of_same_facility(self): - """Users can reschedule bookings via the re-schedule endpoint with another user resource of same facility.""" + def test_reschedule_booking_to_another_user_resource_of_same_facility(self): + """Users can reschedule bookings via the re-schedule endpoint with + another user resource of same facility.""" - def test_reschedule_booking_to_another_user_resource_of_another_facility(self): - """Users cannot reschedule bookings via the re-schedule endpoint with another user resource of different facility.""" + def test_reschedule_booking_to_another_user_resource_of_another_facility(self): + """Users cannot reschedule bookings via the re-schedule endpoint with + another user resource of different facility."""Also applies to: 412-412
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 386-386: Line too long (117/100)
(C0301)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
care/emr/api/viewsets/scheduling/booking.py
(1 hunks)care/emr/tests/test_booking_api.py
(1 hunks)
🧰 Additional context used
🧬 Code G A688 raph Analysis (1)
care/emr/tests/test_booking_api.py (4)
care/security/permissions/user_schedule.py (1)
UserSchedulePermissions
(14-84)care/security/authorization/user_schedule.py (3)
can_write_user_booking
(64-78)can_create_appointment
(18-26)can_reschedule_appointment
(28-36)care/utils/tests/base.py (2)
create_role_with_permissions
(58-71)attach_role_facility_organization_user
(112-115)care/emr/tests/test_schedule_api.py (3)
create_slot
(90-99)create_slot
(823-832)create_booking
(101-113)
🪛 Pylint (3.3.7)
care/emr/tests/test_booking_api.py
[convention] 386-386: Line too long (117/100)
(C0301)
[convention] 412-412: Line too long (125/100)
(C0301)
🔇 Additional comments (3)
care/emr/api/viewsets/scheduling/booking.py (1)
146-146
: LGTM! This change correctly implements facility-scoped rescheduling.The modification from exact resource matching to facility-level filtering aligns perfectly with the PR objectives. The security is maintained since the facility is already validated and authorized earlier in the method.
care/emr/tests/test_booking_api.py (2)
385-410
: Comprehensive test coverage for same-facility rescheduling.The test properly validates that users can reschedule bookings to different resources within the same facility. The setup is thorough and the assertion is correct.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 386-386: Line too long (117/100)
(C0301)
411-437
: Good test coverage for cross-facility rescheduling prevention.The test correctly verifies that rescheduling across facilities is blocked with a 404 response. The test setup properly creates resources in different facilities to validate the boundary enforcement.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 412-412: Line too long (125/100)
(C0301)
Proposed Changes
Associated Issue
Merge Checklist
Only 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
Bug Fixes
Tests