Updated test cases for schedule api #3377
Conversation
📝 WalkthroughWalkthroughA single-line permission message was corrected in the schedule viewset; extensive test infrastructure and many new/updated tests were added to cover schedules, availabilities, exceptions, resource types, permissions, and charge item definition behaviors. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.py📄 CodeRabbit inference engine (.cursorrules)
Files:
🪛 Ruff (0.14.8)care/emr/api/viewsets/scheduling/schedule.py225-225: Avoid specifying long messages outside the exception class (TRY003) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (1)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
care/emr/api/viewsets/scheduling/schedule.py(1 hunks)care/emr/tests/test_schedule_api.py(36 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (.cursorrules)
**/*.py: Prioritize readability and maintainability; follow Django's coding style guide (PEP 8 compliance).
Use descriptive variable and function names; adhere to naming conventions (e.g., lowercase with underscores for functions and variables).
Files:
care/emr/api/viewsets/scheduling/schedule.pycare/emr/tests/test_schedule_api.py
**/tests/**/*.py
📄 CodeRabbit inference engine (.cursorrules)
Use Django’s built-in tools for testing (unittest and pytest-django) to ensure code quality and reliability.
Files:
care/emr/tests/test_schedule_api.py
🧠 Learnings (2)
📚 Learning: 2025-10-08T08:07:56.493Z
Learnt from: rithviknishad
Repo: ohcnetwork/care PR: 3302
File: care/emr/resources/scheduling/slot/spec.py:156-161
Timestamp: 2025-10-08T08:07:56.493Z
Learning: In `care/emr/resources/scheduling/slot/spec.py`, token bookings must always have an associated patient. The maintainers prefer fail-fast behavior (allowing exceptions) over graceful degradation when a token booking lacks a patient, as it indicates a data integrity violation that should be caught immediately.
Applied to files:
care/emr/tests/test_schedule_api.py
📚 Learning: 2025-10-08T08:08:44.661Z
Learnt from: rithviknishad
Repo: ohcnetwork/care PR: 3302
File: care/emr/resources/scheduling/slot/spec.py:146-151
Timestamp: 2025-10-08T08:08:44.661Z
Learning: In care/emr/resources/scheduling/slot/spec.py and related token booking code, TokenBooking instances always have a patient associated. The codebase prefers a fail-fast approach where missing patients should cause errors rather than being handled gracefully with optional/nullable fields.
Applied to files:
care/emr/tests/test_schedule_api.py
🪛 Ruff (0.14.5)
care/emr/api/viewsets/scheduling/schedule.py
225-225: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test / Test
🔇 Additional comments (6)
care/emr/api/viewsets/scheduling/schedule.py (1)
225-225: Error message now correctly reflects the permission check.The updated message properly indicates that the
can_list_schedulepermission is required, fixing the misleading "update schedule" message. This improves the user experience when authorization fails.care/emr/tests/test_schedule_api.py (5)
180-190: LGTM!The use of
model_bakeryfor creating test fixtures is appropriate and aligns with Django testing best practices. The helper methods are well-structured and properly associate the location with the facility organization.
892-918: Excellent addition of retrieve permission tests.The new tests comprehensively cover retrieve scenarios including users with permissions, without permissions, and superuser access. The assertions correctly validate both status codes and error messages.
922-1013: LGTM!The charge item definition tests provide comprehensive coverage including permission checks, invalid input validation, and successful setting scenarios. The test structure follows the established patterns well.
37-66: Good test setup enhancements.The additions to
setUpproperly establish the necessary test fixtures for the expanded test coverage, including superuser, healthcare service, and location resources. The explicitforce_authenticatecall makes authentication state clear.
265-290: Comprehensive resource type coverage.The new tests for
healthcare_serviceresource types properly validate permissions across superuser, authorized user, and unauthorized user scenarios. The tests also appropriately check facility scoping for invalid healthcare services.Also applies to: 397-458
| def get_set_charge_item_defintion_url(self, schedule_external_id): | ||
| """Helper to get the URL for set charge item defintion""" | ||
|
|
||
| return reverse( | ||
| "schedule-set-charge-item-definition", | ||
| kwargs={ | ||
| "facility_external_id": self.facility.external_id, | ||
| "external_id": schedule_external_id, | ||
| }, | ||
| ) |
There was a problem hiding this comment.
Fix typo in method name.
The helper method name contains a spelling error: get_set_charge_item_defintion_url should be get_set_charge_item_definition_url. The docstring on line 79 has the same typo.
Apply this diff:
- def get_set_charge_item_defintion_url(self, schedule_external_id):
- """Helper to get the URL for set charge item defintion"""
+ def get_set_charge_item_definition_url(self, schedule_external_id):
+ """Helper to get the URL for set charge item definition"""Also update all call sites of this method to use the corrected name.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In care/emr/tests/test_schedule_api.py around lines 78 to 87, rename the helper
method and its docstring from get_set_charge_item_defintion_url (and the
misspelled "defintion") to get_set_charge_item_definition_url and update the
docstring accordingly; then find and replace all call sites in the repo (tests
and any helper imports) to use the corrected method name so references don't
break (run a project-wide search/replace or update individual tests that call
the helper).
| def generate_availiability(self, **kwargs): | ||
| return Availability.objects.create( | ||
| schedule=kwargs.get("schedule", self.schedule), | ||
| name="Test Availability", | ||
| slot_type=SlotTypeOptions.appointment.value, | ||
| slot_size_in_minutes=120, | ||
| tokens_per_slot=30, | ||
| create_tokens=False, | ||
| reason="", | ||
| availability=[ | ||
| {"day_of_week": 0, "start_time": "09:00:00", "end_time": "13:00:00"}, | ||
| {"day_of_week": 1, "start_time": "09:00:00", "end_time": "13:00:00"}, | ||
| {"day_of_week": 2, "start_time": "09:00:00", "end_time": "13:00:00"}, | ||
| {"day_of_week": 3, "start_time": "09:00:00", "end_time": "13:00:00"}, | ||
| {"day_of_week": 4, "start_time": "09:00:00", "end_time": "13:00:00"}, | ||
| {"day_of_week": 5, "start_time": "09:00:00", "end_time": "13:00:00"}, | ||
| {"day_of_week": 6, "start_time": "09:00:00", "end_time": "13:00:00"}, | ||
| ], | ||
| ) |
There was a problem hiding this comment.
Fix typo in method name.
The helper method name contains a spelling error: generate_availiability should be generate_availability. This should match the Django model name Availability.
Apply this diff:
- def generate_availiability(self, **kwargs):
+ def generate_availability(self, **kwargs):Also update the call site on line 54 to use the corrected name.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In care/emr/tests/test_schedule_api.py around lines 160-178 the helper function
is misspelled as generate_availiability; rename it to generate_availability to
match the Django model and correct spelling, and update any references to this
helper (specifically the call at line 54) to use generate_availability; ensure
you only rename the function and its usages to preserve behavior and imports.
| def list_schedule_for_resourcetype_location(self): | ||
| """Users with can_list_schedule permission can list schedules for location resources.""" | ||
| permissions = [SchedulePermissions.can_list_schedule.name] | ||
| role = self.create_role_with_permissions(permissions) | ||
| self.attach_role_facility_organization_user(self.organization, self.user, role) | ||
| resource = SchedulableResource.objects.create( | ||
| facility=self.facility, | ||
| resource_type=SchedulableResourceTypeOptions.location.value, | ||
| location=self.location, | ||
| ) | ||
| schedule = Schedule.objects.create( | ||
| resource=resource, | ||
| name="Location Schedule", | ||
| valid_from=datetime.now(UTC) - timedelta(days=30), | ||
| valid_to=datetime.now(UTC) + timedelta(days=30), | ||
| ) | ||
| response = self.client.get( | ||
| self.base_url, | ||
| { | ||
| "resource_type": SchedulableResourceTypeOptions.location.value, | ||
| "resource_id": str(self.location.external_id), | ||
| }, | ||
| ) | ||
| self.assertEqual(response.status_code, 200) | ||
| self.assertEqual(len(response.data["results"]), 1) | ||
| self.assertEqual(response.data["results"][0]["id"], str(schedule.external_id)) |
There was a problem hiding this comment.
Add missing test_ prefix to test method name.
The method list_schedule_for_resourcetype_location is missing the test_ prefix, which means it won't be discovered or executed by the test runner. This test is being silently skipped.
Apply this diff:
- def list_schedule_for_resourcetype_location(self):
+ def test_list_schedule_for_resourcetype_location(self):🤖 Prompt for AI Agents
In care/emr/tests/test_schedule_api.py around lines 292 to 317, the test method
is named list_schedule_for_resourcetype_location and thus will not be discovered
by the test runner; rename the method to start with test_, e.g.
test_list_schedule_for_resourcetype_location, updating the function definition
accordingly (and any direct references/calls in the file if present) so the test
framework will execute it.
| def test_upadate_schedule_for_resource_type_healthcare_service_as_superuser(self): | ||
| """Superusers can update schedules for healthcare service resources.""" | ||
| self.client.force_authenticate(user=self.superuser) | ||
| schedule_resource = SchedulableResource.objects.create( | ||
| resource_type=SchedulableResourceTypeOptions.healthcare_service.value, | ||
| facility=self.facility, | ||
| healthcare_service=self.healthcare_services, | ||
| ) | ||
| healthcare_service_schedule = self.create_schedule(resource=schedule_resource) | ||
| updated_data = { | ||
| "name": "Updated Schedule Name", | ||
| "valid_from": self.schedule.valid_from, | ||
| "valid_to": self.schedule.valid_to, | ||
| } | ||
| update_url = self._get_schedule_url(healthcare_service_schedule.external_id) | ||
| response = self.client.put(update_url, updated_data, format="json") | ||
| self.assertEqual(response.status_code, 200) | ||
| self.assertEqual(response.data["name"], "Updated Schedule Name") | ||
|
|
||
| def test_upadate_schedule_for_resource_type_healthcare_service_as_user_with_permissions( | ||
| self, | ||
| ): | ||
| """Users with can_write_schedule permission can update schedules for healthcare service resources.""" | ||
| permissions = [ | ||
| SchedulePermissions.can_write_schedule.name, | ||
| SchedulePermissions.can_list_schedule.name, | ||
| ] | ||
| role = self.create_role_with_permissions(permissions) | ||
| self.attach_role_facility_organization_user(self.organization, self.user, role) | ||
|
|
||
| schedule_resource = SchedulableResource.objects.create( | ||
| resource_type=SchedulableResourceTypeOptions.healthcare_service.value, | ||
| facility=self.facility, | ||
| healthcare_service=self.healthcare_services, | ||
| ) | ||
| healthcare_service_schedule = self.create_schedule(resource=schedule_resource) | ||
| updated_data = { | ||
| "name": "Updated Schedule Name", | ||
| "valid_from": self.schedule.valid_from, | ||
| "valid_to": self.schedule.valid_to, | ||
| } | ||
| update_url = self._get_schedule_url(healthcare_service_schedule.external_id) | ||
| response = self.client.put(update_url, updated_data, format="json") | ||
| self.assertEqual(response.status_code, 200) | ||
| self.assertEqual(response.data["name"], "Updated Schedule Name") | ||
|
|
||
| def test_upadate_schedule_for_resource_type_healthcare_service_as_user_without_permissions( | ||
| self, | ||
| ): | ||
| """Users without can_write_schedule permission cannot update schedules for healthcare service resources.""" | ||
| schedule_resource = SchedulableResource.objects.create( | ||
| resource_type=SchedulableResourceTypeOptions.healthcare_service.value, | ||
| facility=self.facility, | ||
| healthcare_service=self.healthcare_services, | ||
| ) | ||
| healthcare_service_schedule = self.create_schedule(resource=schedule_resource) | ||
| updated_data = { | ||
| "name": "Updated Schedule Name", | ||
| "valid_from": self.schedule.valid_from, | ||
| "valid_to": self.schedule.valid_to, | ||
| } | ||
| update_url = self._get_schedule_url(healthcare_service_schedule.external_id) | ||
| response = self.client.put(update_url, updated_data, format="json") | ||
| self.assertEqual(response.status_code, 403) | ||
| self.assertIn( | ||
| "You do not have permission to update schedule", | ||
| response.data["detail"], | ||
| ) | ||
|
|
||
| def test_upadate_schedule_for_resource_type_location_as_superuser(self): | ||
| """Superusers can update schedules for location resources.""" | ||
| self.client.force_authenticate(user=self.superuser) | ||
| schedule_resource = SchedulableResource.objects.create( | ||
| resource_type=SchedulableResourceTypeOptions.location.value, | ||
| facility=self.facility, | ||
| location=self.location, | ||
| ) | ||
| location_schedule = self.create_schedule(resource=schedule_resource) | ||
| updated_data = { | ||
| "name": "Updated Schedule Name", | ||
| "valid_from": self.schedule.valid_from, | ||
| "valid_to": self.schedule.valid_to, | ||
| } | ||
| update_url = self._get_schedule_url(location_schedule.external_id) | ||
| response = self.client.put(update_url, updated_data, format="json") | ||
| self.assertEqual(response.status_code, 200) | ||
| self.assertEqual(response.data["name"], "Updated Schedule Name") | ||
|
|
||
| def test_upadate_schedule_for_resource_type_location_as_user_with_permissions(self): | ||
| """Users with can_write_schedule permission can update schedules for location resources.""" | ||
| permissions = [ | ||
| SchedulePermissions.can_write_schedule.name, | ||
| SchedulePermissions.can_list_schedule.name, | ||
| ] | ||
| role = self.create_role_with_permissions(permissions) | ||
| self.attach_role_facility_organization_user(self.organization, self.user, role) | ||
|
|
||
| schedule_resource = SchedulableResource.objects.create( | ||
| resource_type=SchedulableResourceTypeOptions.location.value, | ||
| facility=self.facility, | ||
| location=self.location, | ||
| ) | ||
| location_schedule = self.create_schedule(resource=schedule_resource) | ||
| updated_data = { | ||
| "name": "Updated Schedule Name", | ||
| "valid_from": self.schedule.valid_from, | ||
| "valid_to": self.schedule.valid_to, | ||
| } | ||
| update_url = self._get_schedule_url(location_schedule.external_id) | ||
| response = self.client.put(update_url, updated_data, format="json") | ||
| self.assertEqual(response.status_code, 200) | ||
| self.assertEqual(response.data["name"], "Updated Schedule Name") | ||
|
|
||
| def test_upadate_schedule_for_resource_type_location_as_user_without_permissions( | ||
| self, | ||
| ): | ||
| """Users without can_write_schedule permission cannot update schedules for location resources.""" | ||
| schedule_resource = SchedulableResource.objects.create( | ||
| resource_type=SchedulableResourceTypeOptions.location.value, | ||
| facility=self.facility, | ||
| location=self.location, | ||
| ) | ||
| location_schedule = self.create_schedule(resource=schedule_resource) | ||
| updated_data = { | ||
| "name": "Updated Schedule Name", | ||
| "valid_from": self.schedule.valid_from, | ||
| "valid_to": self.schedule.valid_to, | ||
| } | ||
| update_url = self._get_schedule_url(location_schedule.external_id) | ||
| response = self.client.put(update_url, updated_data, format="json") | ||
| self.assertEqual(response.status_code, 403) | ||
| self.assertIn( | ||
| "You do not have permission to update schedule", | ||
| response.data["detail"], | ||
| ) |
There was a problem hiding this comment.
Fix typo in multiple test method names.
Six test methods have a spelling error: test_upadate_schedule_... should be test_update_schedule_.... The affected methods are:
- Line 633:
test_upadate_schedule_for_resource_type_healthcare_service_as_superuser - Line 652:
test_upadate_schedule_for_resource_type_healthcare_service_as_user_with_permissions - Line 679:
test_upadate_schedule_for_resource_type_healthcare_service_as_user_without_permissions - Line 702:
test_upadate_schedule_for_resource_type_location_as_superuser - Line 721:
test_upadate_schedule_for_resource_type_location_as_user_with_permissions - Line 746:
test_upadate_schedule_for_resource_type_location_as_user_without_permissions
Rename all occurrences of test_upadate_schedule_ to test_update_schedule_.
🤖 Prompt for AI Agents
In care/emr/tests/test_schedule_api.py around lines 633 to 767, multiple test
method names contain a typo "test_upadate_schedule_..." which should be
"test_update_schedule_..."; rename each function and all internal references
(definitions and any calls) from test_upadate_schedule_* to
test_update_schedule_* for the six affected methods so test discovery and
reporting work correctly.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #3377 +/- ##
===========================================
+ Coverage 73.88% 74.01% +0.13%
===========================================
Files 435 435
Lines 19760 19760
Branches 2143 2143
===========================================
+ Hits 14599 14626 +27
+ Misses 4715 4686 -29
- Partials 446 448 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.