From 4209663b4746ab1ed77c60130fcdb8e5112fefa8 Mon Sep 17 00:00:00 2001 From: rithviknishad Date: Mon, 13 Jan 2025 22:41:34 +0530 Subject: [PATCH 01/24] Schedule and Availability Exceptions ViewSet Tests --- care/emr/tests/test_schedule_api.py | 393 ++++++++++++++++++++++++++++ 1 file changed, 393 insertions(+) create mode 100644 care/emr/tests/test_schedule_api.py diff --git a/care/emr/tests/test_schedule_api.py b/care/emr/tests/test_schedule_api.py new file mode 100644 index 0000000000..7d6fbf3cff --- /dev/null +++ b/care/emr/tests/test_schedule_api.py @@ -0,0 +1,393 @@ +from datetime import datetime, timedelta + +from django.urls import reverse +from rest_framework import status + +from care.emr.resources.scheduling.schedule.spec import SlotTypeOptions +from care.security.permissions.user_schedule import UserSchedulePermissions +from care.utils.tests.base import CareAPITestBase + + +class TestScheduleViewSet(CareAPITestBase): + def setUp(self): + from care.emr.models import SchedulableUserResource + + super().setUp() + self.user = self.create_user() + self.facility = self.create_facility(user=self.user) + self.organization = self.create_facility_organization(facility=self.facility) + self.client.force_authenticate(user=self.user) + + self.base_url = reverse( + "schedule-list", kwargs={"facility_external_id": self.facility.external_id} + ) + self.resource = SchedulableUserResource.objects.create( + user=self.user, + facility=self.facility, + ) + + def _get_schedule_url(self, schedule_id): + """Helper to get the detail URL for a specific schedule.""" + return reverse( + "schedule-detail", + kwargs={ + "facility_external_id": self.facility.external_id, + "external_id": schedule_id, + }, + ) + + def create_schedule(self, **kwargs): + from care.emr.models import Schedule + + schedule = Schedule.objects.create( + resource=self.resource, + name=kwargs.get("name", "Test Schedule"), + valid_from=kwargs.get("valid_from", datetime.now()), + valid_to=kwargs.get("valid_to", datetime.now() + timedelta(days=30)), + ) + for availability in kwargs.get("availabilities", []): + schedule.availabilities.create(**availability) + return schedule + + def generate_schedule_data(self, **kwargs): + """Helper to generate valid schedule data.""" + valid_from = datetime.now() + valid_to = valid_from + timedelta(days=30) + + return { + "user": str(self.user.external_id), + "name": "Test Schedule", + "valid_from": valid_from.isoformat(), + "valid_to": valid_to.isoformat(), + "availabilities": [ + { + "name": "Morning Slot", + "slot_type": SlotTypeOptions.appointment.value, + "slot_size_in_minutes": 30, + "tokens_per_slot": 1, + "create_tokens": True, + "reason": "Regular schedule", + "availability": [ + { + "day_of_week": 1, + "start_time": "09:00:00", + "end_time": "13:00:00", + } + ], + } + ], + **kwargs, + } + + # LIST TESTS + def test_list_schedule_with_permissions(self): + """Users with can_list_user_schedule permission can list schedules.""" + permissions = [UserSchedulePermissions.can_list_user_schedule.name] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + response = self.client.get(self.base_url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + def test_list_schedule_without_permissions(self): + """Users without can_list_user_schedule permission cannot list schedules.""" + response = self.client.get(self.base_url) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + # CREATE TESTS + def test_create_schedule_with_permissions(self): + """Users with can_write_user_schedule permission can create schedules.""" + permissions = [UserSchedulePermissions.can_write_user_schedule.name] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + schedule_data = self.generate_schedule_data() + response = self.client.post(self.base_url, schedule_data, format="json") + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["name"], schedule_data["name"]) + + def test_create_schedule_without_permissions(self): + """Users without can_write_user_schedule permission cannot create schedules.""" + schedule_data = self.generate_schedule_data() + response = self.client.post(self.base_url, schedule_data, format="json") + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_create_schedule_with_invalid_dates(self): + """Schedule creation fails when valid_from is after valid_to.""" + permissions = [UserSchedulePermissions.can_write_user_schedule.name] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + valid_from = datetime.now() + valid_to = valid_from - timedelta(days=1) # Invalid: end before start + + schedule_data = self.generate_schedule_data( + valid_from=valid_from.isoformat(), valid_to=valid_to.isoformat() + ) + response = self.client.post(self.base_url, schedule_data, format="json") + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertContains( + response, "Valid from cannot be greater than valid to", status_code=400 + ) + + # UPDATE TESTS + def test_update_schedule_with_permissions(self): + """Users with can_write_user_schedule permission can update schedules.""" + permissions = [ + UserSchedulePermissions.can_write_user_schedule.name, + UserSchedulePermissions.can_list_user_schedule.name, + ] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + # First create a schedule + schedule = self.create_schedule() + + # Then update it + updated_data = { + "name": "Updated Schedule Name", + "valid_from": schedule.valid_from, + "valid_to": schedule.valid_to, + } + update_url = self._get_schedule_url(schedule.external_id) + response = self.client.put(update_url, updated_data, format="json") + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["name"], "Updated Schedule Name") + + def test_update_schedule_without_permissions(self): + """Users without can_write_user_schedule permission cannot update schedules.""" + # First create a schedule with permissions + permissions = [ + UserSchedulePermissions.can_list_user_schedule.name, + ] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + schedule = self.create_schedule() + + updated_data = { + "name": "Updated Schedule Name", + "valid_from": schedule.valid_from, + "valid_to": schedule.valid_to, + } + update_url = self._get_schedule_url(schedule.external_id) + response = self.client.put(update_url, updated_data, format="json") + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + # DELETE TESTS + def test_delete_schedule_with_permissions(self): + """Users with can_write_user_schedule permission can delete schedules.""" + permissions = [ + UserSchedulePermissions.can_write_user_schedule.name, + UserSchedulePermissions.can_list_user_schedule.name, + ] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + # First create a schedule + schedule_data = self.generate_schedule_data() + create_response = self.client.post(self.base_url, schedule_data, format="json") + schedule_id = create_response.data["id"] + + # Then delete it + delete_url = self._get_schedule_url(schedule_id) + response = self.client.delete(delete_url) + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + + def test_delete_schedule_without_permissions(self): + """Users without can_write_user_schedule permission cannot delete schedules.""" + # First create a schedule with permissions + permissions = [ + UserSchedulePermissions.can_list_user_schedule.name, + ] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + schedule = self.create_schedule() + + delete_url = self._get_schedule_url(schedule.external_id) + response = self.client.delete(delete_url) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_update_schedule_validity_with_booking_outside_validity(self): + pass + + def test_delete_schedule_with_bookings(self): + pass + + def test_delete_availability_with_bookings(self): + pass + + +class TestAvailabilityExceptionsViewSet(CareAPITestBase): + def setUp(self): + from care.emr.models import SchedulableUserResource + + super().setUp() + self.user = self.create_user() + self.facility = self.create_facility(user=self.user) + self.organization = self.create_facility_organization(facility=self.facility) + self.client.force_authenticate(user=self.user) + + self.base_url = reverse( + "schedule-exceptions-list", + kwargs={"facility_external_id": self.facility.external_id}, + ) + self.resource = SchedulableUserResource.objects.create( + user=self.user, + facility=self.facility, + ) + + def _get_exception_url(self, exception_id): + """Helper to get the detail URL for a specific availability exception.""" + return reverse( + "schedule-exceptions-detail", + kwargs={ + "facility_external_id": self.facility.external_id, + "external_id": exception_id, + }, + ) + + def create_exception(self, **kwargs): + from care.emr.models import AvailabilityException + + valid_from = datetime.now().date() + valid_to = (datetime.now() + timedelta(days=1)).date() + return AvailabilityException.objects.create( + resource=self.resource, + valid_from=valid_from, + valid_to=valid_to, + start_time=kwargs.get("start_time", "09:00:00"), + end_time=kwargs.get("end_time", "17:00:00"), + reason=kwargs.get("reason", "Out of office"), + ) + + def generate_exception_data(self, **kwargs): + """Helper to generate valid availability exception data.""" + valid_from = datetime.now().date() + valid_to = (datetime.now() + timedelta(days=1)).date() + + return { + "user": str(self.user.external_id), + "reason": "Out of office", + "valid_from": valid_from.isoformat(), + "valid_to": valid_to.isoformat(), + "start_time": "09:00:00", + "end_time": "17:00:00", + **kwargs, + } + + # LIST TESTS + def test_list_exceptions_with_permissions(self): + """Users with can_list_user_schedule permission can list exceptions.""" + permissions = [UserSchedulePermissions.can_list_user_schedule.name] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + response = self.client.get(self.base_url) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + def test_list_exceptions_without_permissions(self): + """Users without can_list_user_schedule permission cannot list exceptions.""" + response = self.client.get(self.base_url) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + # CREATE TESTS + def test_create_exception_with_permissions(self): + """Users with can_write_user_schedule permission can create exceptions.""" + permissions = [UserSchedulePermissions.can_write_user_schedule.name] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + exception_data = self.generate_exception_data() + response = self.client.post(self.base_url, exception_data, format="json") + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["reason"], exception_data["reason"]) + + def test_create_exception_without_permissions(self): + """Users without can_write_user_schedule permission cannot create exceptions.""" + exception_data = self.generate_exception_data() + response = self.client.post(self.base_url, exception_data, format="json") + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + # UPDATE TESTS + def test_update_exception_with_permissions(self): + """Users with can_write_user_schedule permission can update exceptions.""" + permissions = [ + UserSchedulePermissions.can_write_user_schedule.name, + UserSchedulePermissions.can_list_user_schedule.name, + ] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + # First create an exception + exception = self.create_exception() + + # Then update it + updated_data = { + "user": str(self.user.external_id), + "reason": "Updated reason", + "valid_from": exception.valid_from, + "valid_to": exception.valid_to, + "start_time": "09:00:00", + "end_time": "17:00:00", + } + update_url = self._get_exception_url(exception.external_id) + response = self.client.put(update_url, updated_data, format="json") + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["reason"], "Updated reason") + + def test_update_exception_without_permissions(self): + """Users without can_write_user_schedule permission cannot update exceptions.""" + permissions = [UserSchedulePermissions.can_list_user_schedule.name] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + # First create an exception + exception = self.create_exception() + + updated_data = { + "user": str(self.user.external_id), + "reason": "Updated reason", + "valid_from": exception.valid_from, + "valid_to": exception.valid_to, + "start_time": "09:00:00", + "end_time": "17:00:00", + } + update_url = self._get_exception_url(exception.external_id) + response = self.client.put(update_url, updated_data, format="json") + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + # DELETE TESTS + def test_delete_exception_with_permissions(self): + """Users with can_write_user_schedule permission can delete exceptions.""" + permissions = [ + UserSchedulePermissions.can_write_user_schedule.name, + UserSchedulePermissions.can_list_user_schedule.name, + ] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + # First create an exception + exception = self.create_exception() + + # Then delete it + delete_url = self._get_exception_url(exception.external_id) + response = self.client.delete(delete_url) + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + + def test_delete_exception_without_permissions(self): + """Users without can_write_user_schedule permission cannot delete exceptions.""" + # First create an exception with permissions + permissions = [UserSchedulePermissions.can_write_user_schedule.name] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + exception = self.create_exception() + + delete_url = self._get_exception_url(exception.external_id) + response = self.client.delete(delete_url) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_create_exception_with_bookings(self): + pass From b7eb13e27c48a3b96d222a87aec988178ca51be7 Mon Sep 17 00:00:00 2001 From: rithviknishad Date: Tue, 14 Jan 2025 15:26:42 +0530 Subject: [PATCH 02/24] Add reschedule APIs --- .../api/viewsets/scheduling/availability.py | 5 ++- care/emr/api/viewsets/scheduling/booking.py | 44 ++++++++++++++++++- .../0007_tokenbooking_previous_booking.py | 24 ++++++++++ care/emr/models/scheduling/booking.py | 3 ++ care/emr/resources/scheduling/slot/spec.py | 4 +- 5 files changed, 76 insertions(+), 4 deletions(-) create mode 100644 care/emr/migrations/0007_tokenbooking_previous_booking.py diff --git a/care/emr/api/viewsets/scheduling/availability.py b/care/emr/api/viewsets/scheduling/availability.py index 192d8898b2..2dbf06c102 100644 --- a/care/emr/api/viewsets/scheduling/availability.py +++ b/care/emr/api/viewsets/scheduling/availability.py @@ -78,7 +78,9 @@ def convert_availability_to_slots(availabilities): return slots -def lock_create_appointment(token_slot, patient, created_by, reason_for_visit): +def lock_create_appointment( + token_slot, patient, created_by, reason_for_visit, previous_booking=None +): with Lock(f"booking:resource:{token_slot.resource.id}"), transaction.atomic(): if token_slot.allocated >= token_slot.availability.tokens_per_slot: raise ValidationError("Slot is already full") @@ -90,6 +92,7 @@ def lock_create_appointment(token_slot, patient, created_by, reason_for_visit): booked_by=created_by, reason_for_visit=reason_for_visit, status="booked", + previous_booking=previous_booking, ) diff --git a/care/emr/api/viewsets/scheduling/booking.py b/care/emr/api/viewsets/scheduling/booking.py index 010962be21..9952b6428c 100644 --- a/care/emr/api/viewsets/scheduling/booking.py +++ b/care/emr/api/viewsets/scheduling/booking.py @@ -3,7 +3,7 @@ from django.db import transaction from django_filters import CharFilter, DateFromToRangeFilter, FilterSet, UUIDFilter from django_filters.rest_framework import DjangoFilterBackend -from pydantic import BaseModel +from pydantic import UUID4, BaseModel from rest_framework.decorators import action from rest_framework.exceptions import PermissionDenied from rest_framework.generics import get_object_or_404 @@ -15,6 +15,8 @@ EMRRetrieveMixin, EMRUpdateMixin, ) +from care.emr.api.viewsets.scheduling import lock_create_appointment +from care.emr.models import TokenSlot from care.emr.models.scheduling import SchedulableUserResource, TokenBooking from care.emr.resources.scheduling.slot.spec import ( CANCELLED_STATUS_CHOICES, @@ -29,10 +31,16 @@ class CancelBookingSpec(BaseModel): reason: Literal[ - BookingStatusChoices.cancelled, BookingStatusChoices.entered_in_error + BookingStatusChoices.cancelled, + BookingStatusChoices.entered_in_error, + BookingStatusChoices.rescheduled, ] +class RescheduleBookingSpec(BaseModel): + new_slot: UUID4 + + class TokenBookingFilters(FilterSet): status = CharFilter(field_name="status") date = DateFromToRangeFilter(field_name="token_slot__start_datetime__date") @@ -117,6 +125,38 @@ def cancel(self, request, *args, **kwargs): self.authorize_update({}, instance) return self.cancel_appointment_handler(instance, request.data, request.user) + @action(detail=True, methods=["POST"]) + def reschedule(self, request, *args, **kwargs): + request_data = RescheduleBookingSpec(**request.data) + existing_booking = self.get_object() + facility = self.get_facility_obj() + self.authorize_update({}, existing_booking) + if not AuthorizationController.call( + "can_create_appointment", self.request.user, facility + ): + raise PermissionDenied("You do not have permission to create appointments") + new_slot = get_object_or_404( + TokenSlot, + external_id=request_data.new_slot, + resource=existing_booking.token_slot.resource, + ) + with transaction.atomic(): + self.cancel_appointment_handler( + existing_booking, + {"reason": BookingStatusChoices.rescheduled}, + request.user, + ) + appointment = lock_create_appointment( + new_slot, + existing_booking.patient, + request.user, + existing_booking.reason_for_visit, + previous_booking=existing_booking, + ) + return Response( + TokenBookingReadSpec.serialize(appointment).model_dump(exclude=["meta"]) + ) + @action(detail=False, methods=["GET"]) def available_users(self, request, *args, **kwargs): facility = Facility.objects.get(external_id=self.kwargs["facility_external_id"]) diff --git a/care/emr/migrations/0007_tokenbooking_previous_booking.py b/care/emr/migrations/0007_tokenbooking_previous_booking.py new file mode 100644 index 0000000000..511e56952c --- /dev/null +++ b/care/emr/migrations/0007_tokenbooking_previous_booking.py @@ -0,0 +1,24 @@ +# Generated by Django 5.1.3 on 2025-01-14 09:37 + +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("emr", "0006_alter_patient_blood_group"), + ] + + operations = [ + migrations.AddField( + model_name="tokenbooking", + name="previous_booking", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to="emr.tokenbooking", + ), + ), + ] diff --git a/care/emr/models/scheduling/booking.py b/care/emr/models/scheduling/booking.py index ff7d6f0e2e..c2583d758e 100644 --- a/care/emr/models/scheduling/booking.py +++ b/care/emr/models/scheduling/booking.py @@ -32,3 +32,6 @@ class TokenBooking(EMRBaseModel): booked_by = models.ForeignKey(User, on_delete=models.CASCADE, null=True, blank=True) status = models.CharField() reason_for_visit = models.TextField(null=True, blank=True) + previous_booking = models.ForeignKey( + "TokenBooking", on_delete=models.SET_NULL, null=True, blank=True + ) diff --git a/care/emr/resources/scheduling/slot/spec.py b/care/emr/resources/scheduling/slot/spec.py index 25df6b5f90..13c07fff7f 100644 --- a/care/emr/resources/scheduling/slot/spec.py +++ b/care/emr/resources/scheduling/slot/spec.py @@ -52,17 +52,19 @@ class BookingStatusChoices(str, Enum): checked_in = "checked_in" waitlist = "waitlist" in_consultation = "in_consultation" + rescheduled = "rescheduled" CANCELLED_STATUS_CHOICES = [ BookingStatusChoices.entered_in_error.value, BookingStatusChoices.cancelled.value, + BookingStatusChoices.rescheduled.value, ] class TokenBookingBaseSpec(EMRResource): __model__ = TokenBooking - __exclude__ = ["token_slot", "patient"] + __exclude__ = ["token_slot", "patient", "previous_booking"] class TokenBookingWriteSpec(TokenBookingBaseSpec): From db4553ffae7594acc6dc1f9c62bd63905e769b3e Mon Sep 17 00:00:00 2001 From: rithviknishad Date: Tue, 14 Jan 2025 15:48:41 +0530 Subject: [PATCH 03/24] serialize facility info --- care/emr/resources/scheduling/slot/spec.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/care/emr/resources/scheduling/slot/spec.py b/care/emr/resources/scheduling/slot/spec.py index 13c07fff7f..693bab1d8b 100644 --- a/care/emr/resources/scheduling/slot/spec.py +++ b/care/emr/resources/scheduling/slot/spec.py @@ -8,9 +8,11 @@ from care.emr.models.scheduling.booking import TokenSlot from care.emr.models.scheduling.schedule import Availability from care.emr.resources.base import EMRResource +from care.emr.resources.facility.spec import FacilityBareMinimumSpec from care.emr.resources.patient.otp_based_flow import PatientOTPReadSpec from care.emr.resources.user.spec import UserSpec from care.users.models import User +from care.facility.models import Facility class AvailabilityforTokenSpec(EMRResource): @@ -85,6 +87,7 @@ class TokenBookingReadSpec(TokenBookingBaseSpec): status: str reason_for_visit: str user: dict = {} + facility: dict = {} @classmethod def perform_extra_serialization(cls, mapping, obj): @@ -98,3 +101,6 @@ def perform_extra_serialization(cls, mapping, obj): mapping["user"] = UserSpec.serialize( User.objects.get(id=obj.token_slot.resource.user_id) ).model_dump(exclude=["meta"]) + mapping["facility"] = FacilityBareMinimumSpec.serialize( + Facility.objects.get(id=obj.token_slot.resource.facility_id) + ).model_dump(exclude=["meta"]) From 7fefed88095e26dfc74be1f2f2d17c92bd4a15f2 Mon Sep 17 00:00:00 2001 From: rithviknishad Date: Tue, 14 Jan 2025 16:04:07 +0530 Subject: [PATCH 04/24] fix lint --- care/emr/resources/scheduling/slot/spec.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/care/emr/resources/scheduling/slot/spec.py b/care/emr/resources/scheduling/slot/spec.py index 693bab1d8b..fa235d4c5c 100644 --- a/care/emr/resources/scheduling/slot/spec.py +++ b/care/emr/resources/scheduling/slot/spec.py @@ -11,8 +11,8 @@ from care.emr.resources.facility.spec import FacilityBareMinimumSpec from care.emr.resources.patient.otp_based_flow import PatientOTPReadSpec from care.emr.resources.user.spec import UserSpec -from care.users.models import User from care.facility.models import Facility +from care.users.models import User class AvailabilityforTokenSpec(EMRResource): From 523a7456c2c3a020cda02c705befaeeda99896af Mon Sep 17 00:00:00 2001 From: rithviknishad Date: Tue, 14 Jan 2025 20:35:04 +0530 Subject: [PATCH 05/24] remove back reference --- .../api/viewsets/scheduling/availability.py | 5 +--- care/emr/api/viewsets/scheduling/booking.py | 1 - .../0007_tokenbooking_previous_booking.py | 24 ------------------- care/emr/models/scheduling/booking.py | 3 --- care/emr/resources/scheduling/slot/spec.py | 2 +- 5 files changed, 2 insertions(+), 33 deletions(-) delete mode 100644 care/emr/migrations/0007_tokenbooking_previous_booking.py diff --git a/care/emr/api/viewsets/scheduling/availability.py b/care/emr/api/viewsets/scheduling/availability.py index 2dbf06c102..192d8898b2 100644 --- a/care/emr/api/viewsets/scheduling/availability.py +++ b/care/emr/api/viewsets/scheduling/availability.py @@ -78,9 +78,7 @@ def convert_availability_to_slots(availabilities): return slots -def lock_create_appointment( - token_slot, patient, created_by, reason_for_visit, previous_booking=None -): +def lock_create_appointment(token_slot, patient, created_by, reason_for_visit): with Lock(f"booking:resource:{token_slot.resource.id}"), transaction.atomic(): if token_slot.allocated >= token_slot.availability.tokens_per_slot: raise ValidationError("Slot is already full") @@ -92,7 +90,6 @@ def lock_create_appointment( booked_by=created_by, reason_for_visit=reason_for_visit, status="booked", - previous_booking=previous_booking, ) diff --git a/care/emr/api/viewsets/scheduling/booking.py b/care/emr/api/viewsets/scheduling/booking.py index 9952b6428c..7486603c01 100644 --- a/care/emr/api/viewsets/scheduling/booking.py +++ b/care/emr/api/viewsets/scheduling/booking.py @@ -151,7 +151,6 @@ def reschedule(self, request, *args, **kwargs): existing_booking.patient, request.user, existing_booking.reason_for_visit, - previous_booking=existing_booking, ) return Response( TokenBookingReadSpec.serialize(appointment).model_dump(exclude=["meta"]) diff --git a/care/emr/migrations/0007_tokenbooking_previous_booking.py b/care/emr/migrations/0007_tokenbooking_previous_booking.py deleted file mode 100644 index 511e56952c..0000000000 --- a/care/emr/migrations/0007_tokenbooking_previous_booking.py +++ /dev/null @@ -1,24 +0,0 @@ -# Generated by Django 5.1.3 on 2025-01-14 09:37 - -import django.db.models.deletion -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ("emr", "0006_alter_patient_blood_group"), - ] - - operations = [ - migrations.AddField( - model_name="tokenbooking", - name="previous_booking", - field=models.ForeignKey( - blank=True, - null=True, - on_delete=django.db.models.deletion.SET_NULL, - to="emr.tokenbooking", - ), - ), - ] diff --git a/care/emr/models/scheduling/booking.py b/care/emr/models/scheduling/booking.py index c2583d758e..ff7d6f0e2e 100644 --- a/care/emr/models/scheduling/booking.py +++ b/care/emr/models/scheduling/booking.py @@ -32,6 +32,3 @@ class TokenBooking(EMRBaseModel): booked_by = models.ForeignKey(User, on_delete=models.CASCADE, null=True, blank=True) status = models.CharField() reason_for_visit = models.TextField(null=True, blank=True) - previous_booking = models.ForeignKey( - "TokenBooking", on_delete=models.SET_NULL, null=True, blank=True - ) diff --git a/care/emr/resources/scheduling/slot/spec.py b/care/emr/resources/scheduling/slot/spec.py index fa235d4c5c..cb40bb3461 100644 --- a/care/emr/resources/scheduling/slot/spec.py +++ b/care/emr/resources/scheduling/slot/spec.py @@ -66,7 +66,7 @@ class BookingStatusChoices(str, Enum): class TokenBookingBaseSpec(EMRResource): __model__ = TokenBooking - __exclude__ = ["token_slot", "patient", "previous_booking"] + __exclude__ = ["token_slot", "patient"] class TokenBookingWriteSpec(TokenBookingBaseSpec): From 2cb0ca19624d25fff5bb70a7ffe67ce5696929ab Mon Sep 17 00:00:00 2001 From: rithviknishad Date: Tue, 14 Jan 2025 23:38:53 +0530 Subject: [PATCH 06/24] disallow booking in the past --- care/emr/api/viewsets/scheduling/availability.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/care/emr/api/viewsets/scheduling/availability.py b/care/emr/api/viewsets/scheduling/availability.py index 192d8898b2..0c90b79afb 100644 --- a/care/emr/api/viewsets/scheduling/availability.py +++ b/care/emr/api/viewsets/scheduling/availability.py @@ -185,6 +185,8 @@ def create_appointment_handler(cls, obj, request_data, user): patient = Patient.objects.filter(external_id=request_data.patient).first() if not patient: raise ValidationError({"Patient not found"}) + if obj.start_datetime < timezone.now(): + raise ValidationError("Slot is already past") appointment = lock_create_appointment( obj, patient, user, request_data.reason_for_visit ) From 113669e18e4a7d7b18482f3206b80243acb497cd Mon Sep 17 00:00:00 2001 From: rithviknishad Date: Tue, 14 Jan 2025 23:48:16 +0530 Subject: [PATCH 07/24] disallow booking same slot if not cancelled --- care/emr/api/viewsets/scheduling/availability.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/care/emr/api/viewsets/scheduling/availability.py b/care/emr/api/viewsets/scheduling/availability.py index 0c90b79afb..f0aa9fe9c2 100644 --- a/care/emr/api/viewsets/scheduling/availability.py +++ b/care/emr/api/viewsets/scheduling/availability.py @@ -18,6 +18,7 @@ from care.emr.models.scheduling.schedule import Availability, SchedulableUserResource from care.emr.resources.scheduling.schedule.spec import SlotTypeOptions from care.emr.resources.scheduling.slot.spec import ( + CANCELLED_STATUS_CHOICES, TokenBookingReadSpec, TokenSlotBaseSpec, ) @@ -80,8 +81,19 @@ def convert_availability_to_slots(availabilities): def lock_create_appointment(token_slot, patient, created_by, reason_for_visit): with Lock(f"booking:resource:{token_slot.resource.id}"), transaction.atomic(): + if token_slot.start_datetime < timezone.now(): + raise ValidationError("Slot is already past") if token_slot.allocated >= token_slot.availability.tokens_per_slot: raise ValidationError("Slot is already full") + if ( + TokenBooking.objects.filter( + token_slot=token_slot, + patient=patient, + ) + .exclude(status__in=CANCELLED_STATUS_CHOICES) + .exists() + ): + raise ValidationError("Patient already has a booking for this slot") token_slot.allocated += 1 token_slot.save() return TokenBooking.objects.create( @@ -185,8 +197,6 @@ def create_appointment_handler(cls, obj, request_data, user): patient = Patient.objects.filter(external_id=request_data.patient).first() if not patient: raise ValidationError({"Patient not found"}) - if obj.start_datetime < timezone.now(): - raise ValidationError("Slot is already past") appointment = lock_create_appointment( obj, patient, user, request_data.reason_for_visit ) From 1630f69b86abb153308dedb7ba4a9ade26f33006 Mon Sep 17 00:00:00 2001 From: rithviknishad Date: Thu, 16 Jan 2025 14:55:34 +0530 Subject: [PATCH 08/24] test authzn. on `AvailabilityViewSet` --- care/emr/api/viewsets/scheduling/schedule.py | 2 +- care/emr/resources/scheduling/slot/spec.py | 9 -- care/emr/tests/test_schedule_api.py | 148 ++++++++++++++++++- 3 files changed, 141 insertions(+), 18 deletions(-) diff --git a/care/emr/api/viewsets/scheduling/schedule.py b/care/emr/api/viewsets/scheduling/schedule.py index dfbfe0d53a..b822e7cf22 100644 --- a/care/emr/api/viewsets/scheduling/schedule.py +++ b/care/emr/api/viewsets/scheduling/schedule.py @@ -189,5 +189,5 @@ def authorize_create(self, instance): ): raise PermissionDenied("You do not have permission to create schedule") - def authorize_delete(self, instance): + def authorize_destroy(self, instance): self.authorize_create(instance) diff --git a/care/emr/resources/scheduling/slot/spec.py b/care/emr/resources/scheduling/slot/spec.py index cb40bb3461..4790e32bec 100644 --- a/care/emr/resources/scheduling/slot/spec.py +++ b/care/emr/resources/scheduling/slot/spec.py @@ -6,7 +6,6 @@ from care.emr.models import TokenBooking from care.emr.models.scheduling.booking import TokenSlot -from care.emr.models.scheduling.schedule import Availability from care.emr.resources.base import EMRResource from care.emr.resources.facility.spec import FacilityBareMinimumSpec from care.emr.resources.patient.otp_based_flow import PatientOTPReadSpec @@ -15,14 +14,6 @@ from care.users.models import User -class AvailabilityforTokenSpec(EMRResource): - __model__ = Availability - - id: UUID4 | None = None - name: str - tokens_per_slot: int - - class TokenSlotBaseSpec(EMRResource): __model__ = TokenSlot __exclude__ = ["resource", "availability"] diff --git a/care/emr/tests/test_schedule_api.py b/care/emr/tests/test_schedule_api.py index 7d6fbf3cff..8dcdf8d19d 100644 --- a/care/emr/tests/test_schedule_api.py +++ b/care/emr/tests/test_schedule_api.py @@ -184,13 +184,8 @@ def test_delete_schedule_with_permissions(self): role = self.create_role_with_permissions(permissions) self.attach_role_facility_organization_user(self.organization, self.user, role) - # First create a schedule - schedule_data = self.generate_schedule_data() - create_response = self.client.post(self.base_url, schedule_data, format="json") - schedule_id = create_response.data["id"] - - # Then delete it - delete_url = self._get_schedule_url(schedule_id) + schedule = self.create_schedule() + delete_url = self._get_schedule_url(schedule.external_id) response = self.client.delete(delete_url) self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) @@ -204,7 +199,6 @@ def test_delete_schedule_without_permissions(self): self.attach_role_facility_organization_user(self.organization, self.user, role) schedule = self.create_schedule() - delete_url = self._get_schedule_url(schedule.external_id) response = self.client.delete(delete_url) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) @@ -391,3 +385,141 @@ def test_delete_exception_without_permissions(self): def test_create_exception_with_bookings(self): pass + + +class TestAvailabilityViewSet(CareAPITestBase): + def setUp(self): + from care.emr.models import SchedulableUserResource + + super().setUp() + self.user = self.create_user() + self.facility = self.create_facility(user=self.user) + self.organization = self.create_facility_organization(facility=self.facility) + self.client.force_authenticate(user=self.user) + self.resource = SchedulableUserResource.objects.create( + user=self.user, facility=self.facility + ) + self.schedule = self.create_schedule() + + self.base_url = reverse( + "schedule-availability-list", + kwargs={ + "facility_external_id": self.facility.external_id, + "schedule_external_id": self.schedule.external_id, + }, + ) + self.resource = SchedulableUserResource.objects.create( + user=self.user, + facility=self.facility, + ) + + def _get_availability_url(self, availability_id): + """Helper to get the detail url for a specific availability.""" + return reverse( + "schedule-availability-detail", + kwargs={ + "facility_external_id": self.facility.external_id, + "schedule_external_id": self.schedule.external_id, + "external_id": availability_id, + }, + ) + + def create_schedule(self, **kwargs): + from care.emr.models import Schedule + + schedule = Schedule.objects.create( + resource=self.resource, + name=kwargs.get("name", "Test Schedule"), + valid_from=kwargs.get("valid_from", datetime.now()), + valid_to=kwargs.get("valid_to", datetime.now() + timedelta(days=30)), + ) + for availability in kwargs.get("availabilities", []): + schedule.availabilities.create(**availability) + return schedule + + def create_availability(self, **kwargs): + from care.emr.models import Availability + + return Availability.objects.create( + schedule=self.schedule, + name=kwargs.get("name", "Test Availability"), + slot_type=kwargs.get("slot_type", SlotTypeOptions.appointment.value), + slot_size_in_minutes=kwargs.get("slot_size_in_minutes", 30), + tokens_per_slot=kwargs.get("tokens_per_slot", 1), + create_tokens=kwargs.get("create_tokens", True), + reason=kwargs.get("reason", "Regular schedule"), + availability=kwargs.get( + "availability", + [ + { + "day_of_week": 1, + "start_time": "09:00:00", + "end_time": "13:00:00", + } + ], + ), + ) + + def generate_availability_data(self, **kwargs): + """Helper to generate valid availability data.""" + return { + "name": "Morning Slot", + "slot_type": SlotTypeOptions.appointment.value, + "slot_size_in_minutes": 30, + "tokens_per_slot": 1, + "create_tokens": True, + "reason": "Regular schedule", + "availability": [ + { + "day_of_week": 1, + "start_time": "09:00:00", + "end_time": "13:00:00", + } + ], + **kwargs, + } + + def test_create_availability_with_permissions(self): + """Users with can_write_user_schedule permission can create availability.""" + permissions = [UserSchedulePermissions.can_write_user_schedule.name] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + availability_data = self.generate_availability_data() + response = self.client.post(self.base_url, availability_data, format="json") + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(response.data["name"], availability_data["name"]) + + def test_create_availability_without_permissions(self): + """Users without can_write_user_schedule permission cannot create availability.""" + availability_data = self.generate_availability_data() + response = self.client.post(self.base_url, availability_data, format="json") + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_delete_availability_with_permissions(self): + """Users with can_write_user_schedule permission can delete availability.""" + permissions = [ + UserSchedulePermissions.can_list_user_schedule.name, + UserSchedulePermissions.can_write_user_schedule.name, + ] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + availability = self.create_availability() + delete_url = self._get_availability_url(availability.external_id) + response = self.client.delete(delete_url) + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + + def test_delete_availability_without_permissions(self): + """Users without can_write_user_schedule permission cannot delete availability.""" + permissions = [UserSchedulePermissions.can_list_user_schedule.name] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + availability = self.create_availability() + delete_url = self._get_availability_url(availability.external_id) + response = self.client.delete(delete_url) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_delete_availability_with_bookings(self): + pass From 91c2b0bc2839a5f7afc57f1b005041dea2270579 Mon Sep 17 00:00:00 2001 From: rithviknishad Date: Thu, 16 Jan 2025 18:15:02 +0530 Subject: [PATCH 09/24] test booking viewset --- care/emr/tests/test_booking_api.py | 306 ++++++++++++++++++++++++++++ care/emr/tests/test_schedule_api.py | 26 +-- 2 files changed, 319 insertions(+), 13 deletions(-) create mode 100644 care/emr/tests/test_booking_api.py diff --git a/care/emr/tests/test_booking_api.py b/care/emr/tests/test_booking_api.py new file mode 100644 index 0000000000..a151e6ada7 --- /dev/null +++ b/care/emr/tests/test_booking_api.py @@ -0,0 +1,306 @@ +from care.emr.models import ( + SchedulableUserResource, + Schedule, + Availability, + TokenSlot, + TokenBooking, +) +from care.emr.resources.scheduling.schedule.spec import SlotTypeOptions +from care.emr.resources.scheduling.slot.spec import BookingStatusChoices +from care.security.permissions.user_schedule import UserSchedulePermissions +from care.utils.tests.base import CareAPITestBase +from django.urls import reverse +from datetime import datetime, timedelta + + +class TestBookingViewSet(CareAPITestBase): + + def setUp(self): + super().setUp() + self.user = self.create_user() + self.facility = self.create_facility(user=self.user) + self.organization = self.create_facility_organization(facility=self.facility) + self.patient = self.create_patient() + self.resource = SchedulableUserResource.objects.create( + user=self.user, + facility=self.facility, + ) + self.schedule = Schedule.objects.create( + resource=self.resource, + name="Test Schedule", + valid_from=datetime.now() - timedelta(days=30), + valid_to=datetime.now() + timedelta(days=30), + ) + self.availability = Availability.objects.create( + 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"}, + ], + ) + self.slot = self.create_slot() + self.client.force_authenticate(user=self.user) + + self.base_url = reverse( + "appointments-list", + kwargs={"facility_external_id": self.facility.external_id}, + ) + + def _get_booking_url(self, booking_id): + """Helper to get the detail URL for a specific booking.""" + return reverse( + "appointments-detail", + kwargs={ + "facility_external_id": self.facility.external_id, + "external_id": booking_id, + }, + ) + + def create_booking(self, **kwargs): + data = { + "token_slot": self.slot, + "patient": self.patient, + "booked_by": self.user, + "status": BookingStatusChoices.booked.value, + } + data.update(kwargs) + return TokenBooking.objects.create(**data) + + def create_slot(self, **kwargs): + data = { + "resource": self.resource, + "availability": self.availability, + "start_datetime": datetime.now() + timedelta(minutes=30), + "end_datetime": datetime.now() + timedelta(minutes=60), + "allocated": 0, + } + data.update(kwargs) + return TokenSlot.objects.create(**data) + + def test_list_booking_with_permissions(self): + """Users with can_list_user_booking permission can list bookings.""" + permissions = [UserSchedulePermissions.can_list_user_booking.name] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + response = self.client.get(self.base_url) + self.assertEqual(response.status_code, 200) + + def test_list_booking_without_permissions(self): + """Users without can_list_user_booking permission cannot list bookings.""" + response = self.client.get(self.base_url) + self.assertEqual(response.status_code, 403) + + def test_retrieve_booking_with_permissions(self): + """Users with can_list_user_booking permission can retrieve bookings.""" + permissions = [UserSchedulePermissions.can_list_user_booking.name] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + booking = self.create_booking() + response = self.client.get(self._get_booking_url(booking.external_id)) + self.assertEqual(response.status_code, 200) + + def test_retrieve_booking_without_permissions(self): + """Users without can_list_user_booking permission cannot retrieve bookings.""" + booking = self.create_booking() + response = self.client.get(self._get_booking_url(booking.external_id)) + self.assertEqual(response.status_code, 403) + + def test_update_with_permissions(self): + """Users with can_write_user_booking permission can update bookings.""" + permissions = [ + UserSchedulePermissions.can_write_user_booking.name, + UserSchedulePermissions.can_list_user_booking.name, + ] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + booking = self.create_booking() + update_data = { + "status": BookingStatusChoices.checked_in.value, + } + response = self.client.put( + self._get_booking_url(booking.external_id), update_data, format="json" + ) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.data["status"], BookingStatusChoices.checked_in.value) + + def test_update_without_permissions(self): + """Users without can_write_user_booking permission cannot update bookings.""" + permissions = [ + UserSchedulePermissions.can_list_user_booking.name, + ] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + def test_cancel_booking_via_update(self): + """Users cannot cancel bookings via update.""" + permissions = [ + UserSchedulePermissions.can_write_user_booking.name, + UserSchedulePermissions.can_list_user_booking.name, + ] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + booking = self.create_booking() + update_data = { + "status": BookingStatusChoices.cancelled.value, + } + response = self.client.put( + self._get_booking_url(booking.external_id), update_data, format="json" + ) + self.assertContains( + response, + status_code=400, + text="Cannot cancel a booking. Use the cancel endpoint", + ) + + def test_cancel_booking_with_permission(self): + """Users can cancel bookings via the cancel endpoint.""" + permissions = [ + UserSchedulePermissions.can_write_user_booking.name, + UserSchedulePermissions.can_list_user_booking.name, + ] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + booking = self.create_booking() + cancel_url = reverse( + "appointments-cancel", + kwargs={ + "facility_external_id": self.facility.external_id, + "external_id": booking.external_id, + }, + ) + data = {"reason": BookingStatusChoices.cancelled.value} + response = self.client.post(cancel_url, data, format="json") + self.assertEqual(response.status_code, 200) + + def test_cancel_booking_without_permission(self): + """Users cannot cancel bookings via the cancel endpoint.""" + permissions = [ + UserSchedulePermissions.can_list_user_booking.name, + ] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + booking = self.create_booking() + cancel_url = reverse( + "appointments-cancel", + kwargs={ + "facility_external_id": self.facility.external_id, + "external_id": booking.external_id, + }, + ) + data = {"reason": BookingStatusChoices.cancelled.value} + response = self.client.post(cancel_url, data, format="json") + print(response.data) + self.assertContains( + response, + status_code=403, + text="You do not have permission to update bookings", + ) + + def test_reschedule_booking_with_permission(self): + """Users can reschedule bookings via the re-schedule endpoint.""" + permissions = [ + UserSchedulePermissions.can_write_user_booking.name, + UserSchedulePermissions.can_list_user_booking.name, + UserSchedulePermissions.can_create_appointment.name, + ] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + new_slot = self.create_slot() + booking = self.create_booking() + reschedule_url = reverse( + "appointments-reschedule", + kwargs={ + "facility_external_id": self.facility.external_id, + "external_id": booking.external_id, + }, + ) + data = {"new_slot": new_slot.external_id} + response = self.client.post(reschedule_url, data, format="json") + self.assertEqual(response.status_code, 200) + + def test_reschedule_booking_without_permission(self): + """Users cannot reschedule bookings via the re-schedule endpoint.""" + permissions = [ + UserSchedulePermissions.can_write_user_booking.name, + UserSchedulePermissions.can_list_user_booking.name, + ] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + new_slot = self.create_slot() + booking = self.create_booking() + reschedule_url = reverse( + "appointments-reschedule", + kwargs={ + "facility_external_id": self.facility.external_id, + "external_id": booking.external_id, + }, + ) + data = {"new_slot": new_slot.external_id} + response = self.client.post(reschedule_url, data, format="json") + self.assertContains( + response, + status_code=403, + text="You do not have permission to create appointments", + ) + + def test_reschedule_booking_with_slot_in_past(self): + """Users cannot reschedule bookings via the re-schedule endpoint.""" + permissions = [ + UserSchedulePermissions.can_write_user_booking.name, + UserSchedulePermissions.can_list_user_booking.name, + UserSchedulePermissions.can_create_appointment.name, + ] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + new_slot = self.create_slot( + start_datetime=datetime.now() - timedelta(minutes=30), + end_datetime=datetime.now() - timedelta(minutes=15), + ) + booking = self.create_booking() + reschedule_url = reverse( + "appointments-reschedule", + kwargs={ + "facility_external_id": self.facility.external_id, + "external_id": booking.external_id, + }, + ) + data = {"new_slot": new_slot.external_id} + response = self.client.post(reschedule_url, data, format="json") + self.assertContains( + response, + status_code=400, + text="Slot is already past", + ) + + def test_list_available_users(self): + available_users_url = reverse( + "appointments-available-users", + kwargs={"facility_external_id": self.facility.external_id}, + ) + response = self.client.get(available_users_url) + self.assertEqual(response.status_code, 200) + self.assertGreaterEqual(len(response.data["users"]), 1) + + +class TestSlotViewSet(CareAPITestBase): + pass diff --git a/care/emr/tests/test_schedule_api.py b/care/emr/tests/test_schedule_api.py index 8dcdf8d19d..22fb3b0928 100644 --- a/care/emr/tests/test_schedule_api.py +++ b/care/emr/tests/test_schedule_api.py @@ -16,15 +16,15 @@ def setUp(self): self.user = self.create_user() self.facility = self.create_facility(user=self.user) self.organization = self.create_facility_organization(facility=self.facility) + self.resource = SchedulableUserResource.objects.create( + user=self.user, + facility=self.facility, + ) self.client.force_authenticate(user=self.user) self.base_url = reverse( "schedule-list", kwargs={"facility_external_id": self.facility.external_id} ) - self.resource = SchedulableUserResource.objects.create( - user=self.user, - facility=self.facility, - ) def _get_schedule_url(self, schedule_id): """Helper to get the detail URL for a specific schedule.""" @@ -221,16 +221,16 @@ def setUp(self): self.user = self.create_user() self.facility = self.create_facility(user=self.user) self.organization = self.create_facility_organization(facility=self.facility) + self.resource = SchedulableUserResource.objects.create( + user=self.user, + facility=self.facility, + ) self.client.force_authenticate(user=self.user) self.base_url = reverse( "schedule-exceptions-list", kwargs={"facility_external_id": self.facility.external_id}, ) - self.resource = SchedulableUserResource.objects.create( - user=self.user, - facility=self.facility, - ) def _get_exception_url(self, exception_id): """Helper to get the detail URL for a specific availability exception.""" @@ -399,6 +399,10 @@ def setUp(self): self.resource = SchedulableUserResource.objects.create( user=self.user, facility=self.facility ) + self.resource = SchedulableUserResource.objects.create( + user=self.user, + facility=self.facility, + ) self.schedule = self.create_schedule() self.base_url = reverse( @@ -408,10 +412,6 @@ def setUp(self): "schedule_external_id": self.schedule.external_id, }, ) - self.resource = SchedulableUserResource.objects.create( - user=self.user, - facility=self.facility, - ) def _get_availability_url(self, availability_id): """Helper to get the detail url for a specific availability.""" @@ -446,7 +446,7 @@ def create_availability(self, **kwargs): slot_type=kwargs.get("slot_type", SlotTypeOptions.appointment.value), slot_size_in_minutes=kwargs.get("slot_size_in_minutes", 30), tokens_per_slot=kwargs.get("tokens_per_slot", 1), - create_tokens=kwargs.get("create_tokens", True), + create_tokens=kwargs.get("create_tokens", False), reason=kwargs.get("reason", "Regular schedule"), availability=kwargs.get( "availability", From f9533a8ab847308bdb5221b10b823ae130563a1c Mon Sep 17 00:00:00 2001 From: rithviknishad Date: Thu, 16 Jan 2025 19:07:18 +0530 Subject: [PATCH 10/24] test appointments in booking viewset --- care/emr/tests/test_booking_api.py | 194 ++++++++++++++++++++++++++++- 1 file changed, 192 insertions(+), 2 deletions(-) diff --git a/care/emr/tests/test_booking_api.py b/care/emr/tests/test_booking_api.py index a151e6ada7..cbb53a6827 100644 --- a/care/emr/tests/test_booking_api.py +++ b/care/emr/tests/test_booking_api.py @@ -206,7 +206,6 @@ def test_cancel_booking_without_permission(self): ) data = {"reason": BookingStatusChoices.cancelled.value} response = self.client.post(cancel_url, data, format="json") - print(response.data) self.assertContains( response, status_code=403, @@ -303,4 +302,195 @@ def test_list_available_users(self): class TestSlotViewSet(CareAPITestBase): - pass + def setUp(self): + super().setUp() + self.user = self.create_user() + self.facility = self.create_facility(user=self.user) + self.organization = self.create_facility_organization(facility=self.facility) + self.patient = self.create_patient() + self.resource = SchedulableUserResource.objects.create( + user=self.user, + facility=self.facility, + ) + self.schedule = Schedule.objects.create( + resource=self.resource, + name="Test Schedule", + valid_from=datetime.now() - timedelta(days=30), + valid_to=datetime.now() + timedelta(days=30), + ) + self.availability = self.create_availability() + self.slot = self.create_slot() + self.client.force_authenticate(user=self.user) + + def _get_create_appointment_url(self, slot_id): + """Helper to get the detail URL for a specific booking.""" + return reverse( + "slot-create-appointment", + kwargs={ + "facility_external_id": self.facility.external_id, + "external_id": slot_id, + }, + ) + + def create_appointment(self, **kwargs): + data = { + "token_slot": self.slot, + "patient": self.patient, + "booked_by": self.user, + "status": BookingStatusChoices.booked.value, + } + data.update(kwargs) + return TokenBooking.objects.create(**data) + + def create_slot(self, **kwargs): + data = { + "resource": self.resource, + "availability": self.availability, + "start_datetime": datetime.now() + timedelta(minutes=30), + "end_datetime": datetime.now() + timedelta(minutes=60), + "allocated": 0, + } + data.update(kwargs) + return TokenSlot.objects.create(**data) + + def create_availability(self, **kwargs): + return Availability.objects.create( + schedule=self.schedule, + name=kwargs.get("name", "Test Availability"), + slot_type=kwargs.get("slot_type", SlotTypeOptions.appointment.value), + slot_size_in_minutes=kwargs.get("slot_size_in_minutes", 30), + tokens_per_slot=kwargs.get("tokens_per_slot", 1), + create_tokens=kwargs.get("create_tokens", False), + reason=kwargs.get("reason", "Regular schedule"), + availability=kwargs.get( + "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", + }, + ], + ), + ) + + def get_appointment_data(self, **kwargs): + data = { + "patient": self.patient.external_id, + "reason_for_visit": "Testing", + } + data.update(kwargs) + return data + + def test_create_appointment_with_permission(self): + """Users with can_create_appointment permission can create appointments.""" + permissions = [UserSchedulePermissions.can_create_appointment.name] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + data = self.get_appointment_data() + response = self.client.post( + self._get_create_appointment_url(self.slot.external_id), data, format="json" + ) + self.assertEqual(response.status_code, 200) + + def test_create_appointment_without_permission(self): + """Users without can_create_appointment permission cannot create appointments.""" + data = self.get_appointment_data() + response = self.client.post( + self._get_create_appointment_url(self.slot.external_id), data, format="json" + ) + self.assertEqual(response.status_code, 403) + + def test_create_appointment_with_slot_in_past(self): + """Users cannot create appointments on a past slot.""" + permissions = [UserSchedulePermissions.can_create_appointment.name] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + slot = self.create_slot( + start_datetime=datetime.now() - timedelta(minutes=30), + end_datetime=datetime.now() - timedelta(minutes=15), + ) + data = self.get_appointment_data() + response = self.client.post( + self._get_create_appointment_url(slot.external_id), data, format="json" + ) + self.assertContains(response, status_code=400, text="Slot is already past") + + def test_create_multiple_appointments_on_same_slot(self): + """Users cannot create multiple appointments on the same slot (as long as previous ones are cancelled)""" + permissions = [UserSchedulePermissions.can_create_appointment.name] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + self.create_appointment() + + data = self.get_appointment_data() + response = self.client.post( + self._get_create_appointment_url(self.slot.external_id), data, format="json" + ) + self.assertContains( + response, + status_code=400, + text="Patient already has a booking for this slot", + ) + + def test_cancel_and_create_appointment_on_same_slot(self): + """Users can create an appointment on the same slot if the previous one is cancelled""" + permissions = [UserSchedulePermissions.can_create_appointment.name] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + self.create_appointment(status=BookingStatusChoices.cancelled.value) + + data = self.get_appointment_data() + response = self.client.post( + self._get_create_appointment_url(self.slot.external_id), data, format="json" + ) + self.assertEqual(response.status_code, 200) + + def test_over_booking_a_slot(self): + """Users cannot create an appointment on a slot if it is already fully booked""" + permissions = [UserSchedulePermissions.can_create_appointment.name] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + availability = self.create_availability(tokens_per_slot=10) + slot = self.create_slot(availability=availability, allocated=10) + + data = self.get_appointment_data() + response = self.client.post( + self._get_create_appointment_url(slot.external_id), data, format="json" + ) + self.assertContains(response, status_code=400, text="Slot is already full") From 474a2a9424ce6857f76cf0d2790ceb8a7f61b822 Mon Sep 17 00:00:00 2001 From: rithviknishad Date: Fri, 17 Jan 2025 13:27:11 +0530 Subject: [PATCH 11/24] disallow writing exception if token slots already exists --- .../scheduling/availability_exception/spec.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/care/emr/resources/scheduling/availability_exception/spec.py b/care/emr/resources/scheduling/availability_exception/spec.py index dbdd4d0ca9..e889930041 100644 --- a/care/emr/resources/scheduling/availability_exception/spec.py +++ b/care/emr/resources/scheduling/availability_exception/spec.py @@ -2,10 +2,10 @@ from enum import Enum from django.core.exceptions import ObjectDoesNotExist -from pydantic import UUID4 +from pydantic import UUID4, model_validator from rest_framework.exceptions import ValidationError -from care.emr.models import AvailabilityException +from care.emr.models import AvailabilityException, TokenSlot from care.emr.models.scheduling.schedule import SchedulableUserResource from care.emr.resources.base import EMRResource from care.facility.models import Facility @@ -34,7 +34,6 @@ class AvailabilityExceptionWriteSpec(AvailabilityExceptionBaseSpec): def perform_extra_deserialization(self, is_update, obj): if not is_update: - resource = None try: user = User.objects.get(external_id=self.user) resource = SchedulableUserResource.objects.get( @@ -45,6 +44,17 @@ def perform_extra_deserialization(self, is_update, obj): except ObjectDoesNotExist as e: raise ValidationError("Object does not exist") from e + slots = TokenSlot.objects.filter( + resource=obj.resource, + start_datetime__date__gte=self.valid_from, + start_datetime__date__lte=self.valid_to, + start_datetime__time__gte=self.start_time, + start_datetime__time__lte=self.end_time, + ) + if slots.filter(allocated__gt=0): + raise ValidationError("There are bookings during this exception") + slots.delete() + class AvailabilityExceptionReadSpec(AvailabilityExceptionBaseSpec): @classmethod From 47483e422f1e987f900a774820b203c6c514efc6 Mon Sep 17 00:00:00 2001 From: rithviknishad Date: Fri, 17 Jan 2025 15:36:45 +0530 Subject: [PATCH 12/24] Account for exception in get_slots_for_day_handler --- .../api/viewsets/scheduling/availability.py | 67 +++++++++++++------ care/emr/tests/test_booking_api.py | 59 ++++++++++++++++ 2 files changed, 105 insertions(+), 21 deletions(-) diff --git a/care/emr/api/viewsets/scheduling/availability.py b/care/emr/api/viewsets/scheduling/availability.py index f0aa9fe9c2..8915dd5651 100644 --- a/care/emr/api/viewsets/scheduling/availability.py +++ b/care/emr/api/viewsets/scheduling/availability.py @@ -51,7 +51,7 @@ def validate_period(self): raise ValidationError("Period cannot be be greater than max days") -def convert_availability_to_slots(availabilities): +def convert_availability_and_exceptions_to_slots(availabilities, exceptions, day): slots = {} for availability in availabilities: start_time = parse(availability["availability"]["start_time"]) @@ -65,15 +65,31 @@ def convert_availability_to_slots(availabilities): if i == 30: # noqa PLR2004 # Failsafe to prevent infinite loop break - slots[ - f"{current_time.time()}-{(current_time + datetime.timedelta(minutes=slot_size_in_minutes)).time()}" - ] = { - "start_time": current_time.time(), - "end_time": ( - current_time + datetime.timedelta(minutes=slot_size_in_minutes) - ).time(), - "availability_id": availability_id, - } + + conflicting = False + for exception in exceptions: + exception_start_time = datetime.datetime.combine( + day, exception.start_time, tzinfo=None + ) + exception_end_time = datetime.datetime.combine( + day, exception.end_time, tzinfo=None + ) + if ( + exception_start_time + <= (current_time + datetime.timedelta(minutes=slot_size_in_minutes)) + ) and exception_end_time >= current_time: + conflicting = True + + if not conflicting: + slots[ + f"{current_time.time()}-{(current_time + datetime.timedelta(minutes=slot_size_in_minutes)).time()}" + ] = { + "start_time": current_time.time(), + "end_time": ( + current_time + datetime.timedelta(minutes=slot_size_in_minutes) + ).time(), + "availability_id": availability_id, + } current_time += datetime.timedelta(minutes=slot_size_in_minutes) return slots @@ -86,10 +102,7 @@ def lock_create_appointment(token_slot, patient, created_by, reason_for_visit): if token_slot.allocated >= token_slot.availability.tokens_per_slot: raise ValidationError("Slot is already full") if ( - TokenBooking.objects.filter( - token_slot=token_slot, - patient=patient, - ) + TokenBooking.objects.filter(token_slot=token_slot, patient=patient) .exclude(status__in=CANCELLED_STATUS_CHOICES) .exists() ): @@ -144,9 +157,15 @@ def get_slots_for_day_handler(cls, facility_external_id, request_data): "availability_id": schedule_availability.id, } ) - # Remove anything that has an availability exception - # Generate all slots already created for that day - slots = convert_availability_to_slots(calculated_dow_availabilities) + exceptions = AvailabilityException.objects.filter( + resource=schedulable_resource_obj, + valid_from__lte=request_data.day, + valid_to__gte=request_data.day, + ) + # Generate all slots already created for that day, exclude anything that conflicts with availability exception + slots = convert_availability_and_exceptions_to_slots( + calculated_dow_availabilities, exceptions, request_data.day + ) # Fetch all existing slots in that day created_slots = TokenSlot.objects.filter( start_datetime__date=request_data.day, @@ -270,8 +289,8 @@ def availability_stats(self, request, *args, **kwargs): # Calculate availability exception for that day exceptions = [] for exception in availability_exceptions: - valid_from = timezone.make_naive(exception["valid_from"]).date() - valid_to = timezone.make_naive(exception["valid_to"]).date() + valid_from = exception["valid_from"] + valid_to = exception["valid_to"] if valid_from <= day <= valid_to: exceptions.append(exception) # Calculate slots based on these data @@ -326,9 +345,15 @@ def calculate_slots( while start_time <= end_time: conflicting = False for exception in exceptions: + exception_start_time = datetime.datetime.combine( + date, exception["start_time"], tzinfo=None + ) + exception_end_time = datetime.datetime.combine( + date, exception["end_time"], tzinfo=None + ) if ( - exception["start_time"] <= end_time - and exception["end_time"] >= start_time + exception_start_time <= end_time + and exception_end_time >= start_time ): conflicting = True start_time = start_time + timedelta( diff --git a/care/emr/tests/test_booking_api.py b/care/emr/tests/test_booking_api.py index cbb53a6827..91b33fbe01 100644 --- a/care/emr/tests/test_booking_api.py +++ b/care/emr/tests/test_booking_api.py @@ -1,9 +1,12 @@ +from http.client import responses + from care.emr.models import ( SchedulableUserResource, Schedule, Availability, TokenSlot, TokenBooking, + AvailabilityException, ) from care.emr.resources.scheduling.schedule.spec import SlotTypeOptions from care.emr.resources.scheduling.slot.spec import BookingStatusChoices @@ -494,3 +497,59 @@ def test_over_booking_a_slot(self): self._get_create_appointment_url(slot.external_id), data, format="json" ) self.assertContains(response, status_code=400, text="Slot is already full") + + def test_get_slots_for_day(self): + """Get slots for a specific day.""" + data = { + "user": self.user.external_id, + "day": datetime.now().strftime("%Y-%m-%d"), + } + url = reverse( + "slot-get-slots-for-day", + kwargs={"facility_external_id": self.facility.external_id}, + ) + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, 200) + + def test_get_slots_for_day_for_non_schedulable_user(self): + """Cannot get slots for non-schedulable user.""" + user = self.create_user() + facility = self.create_facility(user=user) + data = { + "user": user.external_id, + "day": datetime.now().strftime("%Y-%m-%d"), + } + url = reverse( + "slot-get-slots-for-day", + kwargs={"facility_external_id": facility.external_id}, + ) + response = self.client.post(url, data, format="json") + self.assertContains( + response, status_code=400, text="Resource is not schedulable" + ) + + def test_get_slots_for_day_with_exception(self): + """Get no slots for day with whole day exception""" + + # we don't want the slot that was created in setUp; create availability exception would've done this for us anyways. + self.slot.delete() + + AvailabilityException.objects.create( + resource=self.resource, + name="Test Exception", + valid_from=datetime.now() - timedelta(days=1), + valid_to=datetime.now() + timedelta(days=1), + start_time="00:00:00", + end_time="23:59:59", + ) + data = { + "user": self.user.external_id, + "day": datetime.now().strftime("%Y-%m-%d"), + } + url = reverse( + "slot-get-slots-for-day", + kwargs={"facility_external_id": self.facility.external_id}, + ) + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.data["results"]), 0) From 2855873aaa4b9e573754c8f4ed3d06d86672f4d8 Mon Sep 17 00:00:00 2001 From: rithviknishad Date: Fri, 17 Jan 2025 15:46:43 +0530 Subject: [PATCH 13/24] allow overlapping same second (for exceptions) --- .../api/viewsets/scheduling/availability.py | 8 +- care/emr/tests/test_booking_api.py | 80 ++++++++++++++++++- 2 files changed, 83 insertions(+), 5 deletions(-) diff --git a/care/emr/api/viewsets/scheduling/availability.py b/care/emr/api/viewsets/scheduling/availability.py index 8915dd5651..ba79dc731b 100644 --- a/care/emr/api/viewsets/scheduling/availability.py +++ b/care/emr/api/viewsets/scheduling/availability.py @@ -76,8 +76,8 @@ def convert_availability_and_exceptions_to_slots(availabilities, exceptions, day ) if ( exception_start_time - <= (current_time + datetime.timedelta(minutes=slot_size_in_minutes)) - ) and exception_end_time >= current_time: + < (current_time + datetime.timedelta(minutes=slot_size_in_minutes)) + ) and exception_end_time > current_time: conflicting = True if not conflicting: @@ -352,8 +352,8 @@ def calculate_slots( date, exception["end_time"], tzinfo=None ) if ( - exception_start_time <= end_time - and exception_end_time >= start_time + exception_start_time < end_time + and exception_end_time > start_time ): conflicting = True start_time = start_time + timedelta( diff --git a/care/emr/tests/test_booking_api.py b/care/emr/tests/test_booking_api.py index 91b33fbe01..459408312c 100644 --- a/care/emr/tests/test_booking_api.py +++ b/care/emr/tests/test_booking_api.py @@ -528,7 +528,7 @@ def test_get_slots_for_day_for_non_schedulable_user(self): response, status_code=400, text="Resource is not schedulable" ) - def test_get_slots_for_day_with_exception(self): + def test_get_slots_for_day_with_full_day_exception(self): """Get no slots for day with whole day exception""" # we don't want the slot that was created in setUp; create availability exception would've done this for us anyways. @@ -553,3 +553,81 @@ def test_get_slots_for_day_with_exception(self): response = self.client.post(url, data, format="json") self.assertEqual(response.status_code, 200) self.assertEqual(len(response.data["results"]), 0) + + def test_get_slots_for_day_with_exception_left_overlap(self): + """Get fewer slots for day with partially overlapping exception""" + + # we don't want the slot that was created in setUp; create availability exception would've done this for us anyways. + self.slot.delete() + + AvailabilityException.objects.create( + resource=self.resource, + name="Test Exception", + valid_from=datetime.now() - timedelta(days=1), + valid_to=datetime.now() + timedelta(days=1), + start_time="00:00:00", + end_time="12:00:00", + ) + data = { + "user": self.user.external_id, + "day": datetime.now().strftime("%Y-%m-%d"), + } + url = reverse( + "slot-get-slots-for-day", + kwargs={"facility_external_id": self.facility.external_id}, + ) + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.data["results"]), 2) + + def test_get_slots_for_day_with_exception_right_overlap(self): + """Get fewer slots for day with partially overlapping exception""" + + # we don't want the slot that was created in setUp; create availability exception would've done this for us anyways. + self.slot.delete() + + AvailabilityException.objects.create( + resource=self.resource, + name="Test Exception", + valid_from=datetime.now() - timedelta(days=1), + valid_to=datetime.now() + timedelta(days=1), + start_time="10:00:00", + end_time="23:59:59", + ) + data = { + "user": self.user.external_id, + "day": datetime.now().strftime("%Y-%m-%d"), + } + url = reverse( + "slot-get-slots-for-day", + kwargs={"facility_external_id": self.facility.external_id}, + ) + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.data["results"]), 2) + + def test_get_slots_for_day_with_exception_overlap_in_between(self): + """Get fewer slots for day with partially overlapping exception""" + + # we don't want the slot that was created in setUp; create availability exception would've done this for us anyways. + self.slot.delete() + + AvailabilityException.objects.create( + resource=self.resource, + name="Test Exception", + valid_from=datetime.now() - timedelta(days=1), + valid_to=datetime.now() + timedelta(days=1), + start_time="10:00:00", + end_time="12:00:00", + ) + data = { + "user": self.user.external_id, + "day": datetime.now().strftime("%Y-%m-%d"), + } + url = reverse( + "slot-get-slots-for-day", + kwargs={"facility_external_id": self.facility.external_id}, + ) + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.data["results"]), 4) From f94157ba774e4c4137f9ed037e31447f461f6313 Mon Sep 17 00:00:00 2001 From: rithviknishad Date: Fri, 17 Jan 2025 18:13:53 +0530 Subject: [PATCH 14/24] more tests; 92% --- .../api/viewsets/scheduling/availability.py | 9 +- .../scheduling/availability_exception/spec.py | 2 +- care/emr/tests/test_booking_api.py | 210 ++++++++++++++++++ 3 files changed, 216 insertions(+), 5 deletions(-) diff --git a/care/emr/api/viewsets/scheduling/availability.py b/care/emr/api/viewsets/scheduling/availability.py index ba79dc731b..bea93053b9 100644 --- a/care/emr/api/viewsets/scheduling/availability.py +++ b/care/emr/api/viewsets/scheduling/availability.py @@ -46,9 +46,10 @@ class AvailabilityStatsRequestSpec(BaseModel): def validate_period(self): max_period = 32 if self.from_date > self.to_date: - raise ValidationError("From Date cannot be greater than To Date") - if self.from_date - self.to_date > datetime.timedelta(days=max_period): - raise ValidationError("Period cannot be be greater than max days") + raise ValidationError("From Date cannot be after To Date") + if self.to_date - self.from_date > datetime.timedelta(days=max_period): + msg = f"Period cannot be be greater than {max_period} days" + raise ValidationError(msg) def convert_availability_and_exceptions_to_slots(availabilities, exceptions, day): @@ -215,7 +216,7 @@ def create_appointment_handler(cls, obj, request_data, user): request_data = AppointmentBookingSpec(**request_data) patient = Patient.objects.filter(external_id=request_data.patient).first() if not patient: - raise ValidationError({"Patient not found"}) + raise ValidationError("Patient not found") appointment = lock_create_appointment( obj, patient, user, request_data.reason_for_visit ) diff --git a/care/emr/resources/scheduling/availability_exception/spec.py b/care/emr/resources/scheduling/availability_exception/spec.py index e889930041..3de3650522 100644 --- a/care/emr/resources/scheduling/availability_exception/spec.py +++ b/care/emr/resources/scheduling/availability_exception/spec.py @@ -2,7 +2,7 @@ from enum import Enum from django.core.exceptions import ObjectDoesNotExist -from pydantic import UUID4, model_validator +from pydantic import UUID4 from rest_framework.exceptions import ValidationError from care.emr.models import AvailabilityException, TokenSlot diff --git a/care/emr/tests/test_booking_api.py b/care/emr/tests/test_booking_api.py index 459408312c..69ca41fe6b 100644 --- a/care/emr/tests/test_booking_api.py +++ b/care/emr/tests/test_booking_api.py @@ -78,6 +78,9 @@ def create_booking(self, **kwargs): "status": BookingStatusChoices.booked.value, } data.update(kwargs) + slot = data["token_slot"] + slot.allocated += 1 + slot.save() return TokenBooking.objects.create(**data) def create_slot(self, **kwargs): @@ -105,6 +108,31 @@ def test_list_booking_without_permissions(self): response = self.client.get(self.base_url) self.assertEqual(response.status_code, 403) + def test_list_booking_filtered_by_schedulable_user(self): + """Users can list bookings filtered by schedulable user resource.""" + permissions = [UserSchedulePermissions.can_list_user_booking.name] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + self.create_booking() + + response = self.client.get(self.base_url, data={"user": self.user.external_id}) + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.data["results"]), 1) + + def test_list_booking_filtered_by_non_schedulable_user(self): + """Users can list bookings filtered by non-schedulable user resource, but it'd be empty queryset.""" + permissions = [UserSchedulePermissions.can_list_user_booking.name] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + non_schedulable_user = self.create_user() + response = self.client.get( + self.base_url, data={"user": non_schedulable_user.external_id} + ) + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.data["results"]), 0) + def test_retrieve_booking_with_permissions(self): """Users with can_list_user_booking permission can retrieve bookings.""" permissions = [UserSchedulePermissions.can_list_user_booking.name] @@ -180,6 +208,8 @@ def test_cancel_booking_with_permission(self): self.attach_role_facility_organization_user(self.organization, self.user, role) booking = self.create_booking() + tokens_allocated_before = booking.token_slot.allocated + cancel_url = reverse( "appointments-cancel", kwargs={ @@ -191,6 +221,10 @@ def test_cancel_booking_with_permission(self): response = self.client.post(cancel_url, data, format="json") self.assertEqual(response.status_code, 200) + booking.token_slot.refresh_from_db() + tokens_allocated_after = booking.token_slot.allocated + self.assertEqual(tokens_allocated_before - 1, tokens_allocated_after) + def test_cancel_booking_without_permission(self): """Users cannot cancel bookings via the cancel endpoint.""" permissions = [ @@ -215,6 +249,39 @@ def test_cancel_booking_without_permission(self): text="You do not have permission to update bookings", ) + def test_cancel_cancelled_booking(self): + """Users can cancel bookings to another cancelled status even if already cancelled. However, tokens allocated on slot won't be changed.""" + permissions = [ + UserSchedulePermissions.can_write_user_booking.name, + UserSchedulePermissions.can_list_user_booking.name, + ] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + booking = self.create_booking() + cancel_url = reverse( + "appointments-cancel", + kwargs={ + "facility_external_id": self.facility.external_id, + "external_id": booking.external_id, + }, + ) + + data = {"reason": BookingStatusChoices.cancelled.value} + response = self.client.post(cancel_url, data, format="json") + self.assertEqual(response.status_code, 200) + + booking.token_slot.refresh_from_db() + tokens_allocated_before = booking.token_slot.allocated + + data = {"reason": BookingStatusChoices.entered_in_error.value} + response = self.client.post(cancel_url, data, format="json") + self.assertEqual(response.status_code, 200) + + booking.token_slot.refresh_from_db() + tokens_allocated_after = booking.token_slot.allocated + self.assertEqual(tokens_allocated_before, tokens_allocated_after) + def test_reschedule_booking_with_permission(self): """Users can reschedule bookings via the re-schedule endpoint.""" permissions = [ @@ -435,6 +502,18 @@ def test_create_appointment_without_permission(self): ) self.assertEqual(response.status_code, 403) + def test_create_appointment_with_invalid_patient(self): + """Users with can_create_appointment permission can create appointments.""" + permissions = [UserSchedulePermissions.can_create_appointment.name] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + data = self.get_appointment_data(patient="76aab2d8-93ef-4c9b-b344-b48167a082d0") + response = self.client.post( + self._get_create_appointment_url(self.slot.external_id), data, format="json" + ) + self.assertContains(response, status_code=400, text="Patient not found") + def test_create_appointment_with_slot_in_past(self): """Users cannot create appointments on a past slot.""" permissions = [UserSchedulePermissions.can_create_appointment.name] @@ -631,3 +710,134 @@ def test_get_slots_for_day_with_exception_overlap_in_between(self): response = self.client.post(url, data, format="json") self.assertEqual(response.status_code, 200) self.assertEqual(len(response.data["results"]), 4) + + def test_availability_stats(self): + """Get heatmap availability stats for few days""" + data = { + "user": self.user.external_id, + "from_date": datetime.now().strftime("%Y-%m-%d"), + "to_date": (datetime.now() + timedelta(days=10)).strftime("%Y-%m-%d"), + } + url = reverse( + "slot-availability-stats", + kwargs={"facility_external_id": self.facility.external_id}, + ) + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, 200) + + def test_availability_stats_invalid_period(self): + """Get heatmap availability stats for from date after to date""" + data = { + "user": self.user.external_id, + "from_date": (datetime.now() + timedelta(days=10)).strftime("%Y-%m-%d"), + "to_date": (datetime.now() + timedelta(days=1)).strftime("%Y-%m-%d"), + } + url = reverse( + "slot-availability-stats", + kwargs={"facility_external_id": self.facility.external_id}, + ) + response = self.client.post(url, data, format="json") + self.assertContains( + response, status_code=400, text="From Date cannot be after To Date" + ) + + def test_availability_stats_exceed_period(self): + """Get heatmap availability stats for more than max days""" + data = { + "user": self.user.external_id, + "from_date": datetime.now().strftime("%Y-%m-%d"), + "to_date": (datetime.now() + timedelta(days=40)).strftime("%Y-%m-%d"), + } + url = reverse( + "slot-availability-stats", + kwargs={"facility_external_id": self.facility.external_id}, + ) + response = self.client.post(url, data, format="json") + self.assertContains( + response, status_code=400, text="Period cannot be be greater than 32 days" + ) + + def test_availability_stats_for_invalid_user(self): + """Get heatmap availability stats for an invalid user""" + data = { + "user": "98c763ba-5bbb-44b9-ac03-56414fbb3021", + "from_date": datetime.now().strftime("%Y-%m-%d"), + "to_date": (datetime.now() + timedelta(days=10)).strftime("%Y-%m-%d"), + } + url = reverse( + "slot-availability-stats", + kwargs={"facility_external_id": self.facility.external_id}, + ) + response = self.client.post(url, data, format="json") + self.assertContains(response, status_code=400, text="User does not exist") + + def test_availability_stats_for_non_schedulable_user(self): + """Get heatmap availability stats for a non-schedulable user""" + non_schedulable_user = self.create_user() + data = { + "user": non_schedulable_user.external_id, + "from_date": datetime.now().strftime("%Y-%m-%d"), + "to_date": (datetime.now() + timedelta(days=10)).strftime("%Y-%m-%d"), + } + url = reverse( + "slot-availability-stats", + kwargs={"facility_external_id": self.facility.external_id}, + ) + response = self.client.post(url, data, format="json") + self.assertContains( + response, status_code=400, text="Resource is not schedulable" + ) + + def test_availability_heatmap_slots_same_as_get_slots_for_day(self): + """Get heatmap availability stats for 7 days and verify same slot stats as get_slots_for_day""" + ## TODO: figure out what's happening here; getting expected results in front-end; + # we don't want the slot that was created in setUp; create availability exception would've done this for us anyways. + self.slot.delete() + # + # AvailabilityException.objects.create( + # resource=self.resource, + # name="Test Exception", + # valid_from=datetime.now(), + # valid_to=datetime.now() + timedelta(days=1), + # start_time="00:00:00", + # end_time="11:59:59", + # ) + # AvailabilityException.objects.create( + # resource=self.resource, + # name="Test Exception", + # valid_from=datetime.now() + timedelta(days=2), + # valid_to=datetime.now() + timedelta(days=3), + # start_time="12:00:00", + # end_time="14:00:00", + # ) + data = { + "user": self.user.external_id, + "from_date": datetime.now().strftime("%Y-%m-%d"), + "to_date": (datetime.now() + timedelta(days=7)).strftime("%Y-%m-%d"), + } + availability_stats_url = reverse( + "slot-availability-stats", + kwargs={"facility_external_id": self.facility.external_id}, + ) + response = self.client.post(availability_stats_url, data, format="json") + self.assertEqual(response.status_code, 200) + + slots_for_day_url = reverse( + "slot-get-slots-for-day", + kwargs={"facility_external_id": self.facility.external_id}, + ) + for day, slot_stats in response.data.items(): + data = {"user": self.user.external_id, "day": day} + response = self.client.post(slots_for_day_url, data, format="json") + self.assertEqual(response.status_code, 200) + self.assertEqual( + slot_stats["booked_slots"], + sum(x["allocated"] for x in response.data["results"]), + ) + self.assertEqual( + slot_stats["total_slots"], + sum( + x["availability"]["tokens_per_slot"] + for x in response.data["results"] + ), + ) From 2cf0ef7774d88f5711b6413f80b23223c8044fec Mon Sep 17 00:00:00 2001 From: rithviknishad Date: Sat, 18 Jan 2025 15:14:42 +0530 Subject: [PATCH 15/24] more tests; 99% --- .../api/viewsets/scheduling/availability.py | 2 +- .../emr/resources/scheduling/schedule/spec.py | 22 +- care/emr/tests/test_booking_api.py | 63 ++- care/emr/tests/test_schedule_api.py | 380 +++++++++++++++++- 4 files changed, 415 insertions(+), 52 deletions(-) diff --git a/care/emr/api/viewsets/scheduling/availability.py b/care/emr/api/viewsets/scheduling/availability.py index bea93053b9..798d5eb934 100644 --- a/care/emr/api/viewsets/scheduling/availability.py +++ b/care/emr/api/viewsets/scheduling/availability.py @@ -63,7 +63,7 @@ def convert_availability_and_exceptions_to_slots(availabilities, exceptions, day i = 0 while current_time < end_time: i += 1 - if i == 30: # noqa PLR2004 + if i == 30: # noqa PLR2004 pragma: no cover # Failsafe to prevent infinite loop break diff --git a/care/emr/resources/scheduling/schedule/spec.py b/care/emr/resources/scheduling/schedule/spec.py index ca2cfea954..e39085de4a 100644 --- a/care/emr/resources/scheduling/schedule/spec.py +++ b/care/emr/resources/scheduling/schedule/spec.py @@ -38,8 +38,18 @@ class AvailabilityBaseSpec(EMRResource): # TODO Check if Availability Types are coinciding at any point + +class AvailabilityForScheduleSpec(AvailabilityBaseSpec): + name: str + slot_type: SlotTypeOptions + slot_size_in_minutes: int | None = Field(ge=1) + tokens_per_slot: int | None = Field(ge=1) + create_tokens: bool = False + reason: str = "" + availability: list[AvailabilityDateTimeSpec] + + @field_validator("availability") @classmethod - @field_validator("availability", mode="after") def validate_availability(cls, availabilities: list[AvailabilityDateTimeSpec]): # Validates if availability overlaps for the same day for i in range(len(availabilities)): @@ -54,16 +64,6 @@ def validate_availability(cls, availabilities: list[AvailabilityDateTimeSpec]): raise ValueError("Availability time ranges are overlapping") return availabilities - -class AvailabilityForScheduleSpec(AvailabilityBaseSpec): - name: str - slot_type: SlotTypeOptions - slot_size_in_minutes: int | None = Field(ge=1) - tokens_per_slot: int | None = Field(ge=1) - create_tokens: bool = False - reason: str = "" - availability: list[AvailabilityDateTimeSpec] - @model_validator(mode="after") def validate_for_slot_type(self): if self.slot_type == "appointment": diff --git a/care/emr/tests/test_booking_api.py b/care/emr/tests/test_booking_api.py index 69ca41fe6b..c74d16273f 100644 --- a/care/emr/tests/test_booking_api.py +++ b/care/emr/tests/test_booking_api.py @@ -9,7 +9,10 @@ AvailabilityException, ) from care.emr.resources.scheduling.schedule.spec import SlotTypeOptions -from care.emr.resources.scheduling.slot.spec import BookingStatusChoices +from care.emr.resources.scheduling.slot.spec import ( + BookingStatusChoices, + CANCELLED_STATUS_CHOICES, +) from care.security.permissions.user_schedule import UserSchedulePermissions from care.utils.tests.base import CareAPITestBase from django.urls import reverse @@ -78,9 +81,10 @@ def create_booking(self, **kwargs): "status": BookingStatusChoices.booked.value, } data.update(kwargs) - slot = data["token_slot"] - slot.allocated += 1 - slot.save() + if data["status"] not in CANCELLED_STATUS_CHOICES: + slot = data["token_slot"] + slot.allocated += 1 + slot.save() return TokenBooking.objects.create(**data) def create_slot(self, **kwargs): @@ -589,6 +593,25 @@ def test_get_slots_for_day(self): ) response = self.client.post(url, data, format="json") self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.data["results"]), 9) + + def test_hit_on_get_slots_for_day_does_not_cause_duplicate_slots(self): + """Get slots for a specific day.""" + data = { + "user": self.user.external_id, + "day": datetime.now().strftime("%Y-%m-%d"), + } + url = reverse( + "slot-get-slots-for-day", + kwargs={"facility_external_id": self.facility.external_id}, + ) + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.data["results"]), 9) + + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.data["results"]), 9) def test_get_slots_for_day_for_non_schedulable_user(self): """Cannot get slots for non-schedulable user.""" @@ -794,22 +817,22 @@ def test_availability_heatmap_slots_same_as_get_slots_for_day(self): # we don't want the slot that was created in setUp; create availability exception would've done this for us anyways. self.slot.delete() # - # AvailabilityException.objects.create( - # resource=self.resource, - # name="Test Exception", - # valid_from=datetime.now(), - # valid_to=datetime.now() + timedelta(days=1), - # start_time="00:00:00", - # end_time="11:59:59", - # ) - # AvailabilityException.objects.create( - # resource=self.resource, - # name="Test Exception", - # valid_from=datetime.now() + timedelta(days=2), - # valid_to=datetime.now() + timedelta(days=3), - # start_time="12:00:00", - # end_time="14:00:00", - # ) + AvailabilityException.objects.create( + resource=self.resource, + name="Test Exception", + valid_from=datetime.now(), + valid_to=datetime.now() + timedelta(days=1), + start_time="00:00:00", + end_time="11:59:59", + ) + AvailabilityException.objects.create( + resource=self.resource, + name="Test Exception", + valid_from=datetime.now() + timedelta(days=2), + valid_to=datetime.now() + timedelta(days=3), + start_time="12:00:00", + end_time="14:00:00", + ) data = { "user": self.user.external_id, "from_date": datetime.now().strftime("%Y-%m-%d"), diff --git a/care/emr/tests/test_schedule_api.py b/care/emr/tests/test_schedule_api.py index 22fb3b0928..bccb17e3fc 100644 --- a/care/emr/tests/test_schedule_api.py +++ b/care/emr/tests/test_schedule_api.py @@ -3,15 +3,24 @@ from django.urls import reverse from rest_framework import status +from care.emr.models import ( + Availability, + SchedulableUserResource, + Schedule, + TokenBooking, + TokenSlot, +) from care.emr.resources.scheduling.schedule.spec import SlotTypeOptions +from care.emr.resources.scheduling.slot.spec import ( + CANCELLED_STATUS_CHOICES, + BookingStatusChoices, +) from care.security.permissions.user_schedule import UserSchedulePermissions from care.utils.tests.base import CareAPITestBase class TestScheduleViewSet(CareAPITestBase): def setUp(self): - from care.emr.models import SchedulableUserResource - super().setUp() self.user = self.create_user() self.facility = self.create_facility(user=self.user) @@ -20,8 +29,34 @@ def setUp(self): user=self.user, facility=self.facility, ) - self.client.force_authenticate(user=self.user) + self.patient = self.create_patient() + self.schedule = Schedule.objects.create( + resource=self.resource, + name="Test Schedule", + valid_from=datetime.now() - timedelta(days=30), + valid_to=datetime.now() + timedelta(days=30), + ) + self.availability = Availability.objects.create( + 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"}, + ], + ) + self.slot = self.create_slot() + self.client.force_authenticate(user=self.user) self.base_url = reverse( "schedule-list", kwargs={"facility_external_id": self.facility.external_id} ) @@ -49,6 +84,31 @@ def create_schedule(self, **kwargs): schedule.availabilities.create(**availability) return schedule + def create_slot(self, **kwargs): + data = { + "resource": self.resource, + "availability": self.availability, + "start_datetime": datetime.now() + timedelta(minutes=30), + "end_datetime": datetime.now() + timedelta(minutes=60), + "allocated": 0, + } + data.update(kwargs) + return TokenSlot.objects.create(**data) + + def create_booking(self, **kwargs): + data = { + "token_slot": self.slot, + "patient": self.patient, + "booked_by": self.user, + "status": BookingStatusChoices.booked.value, + } + data.update(kwargs) + if data["status"] not in CANCELLED_STATUS_CHOICES: + slot = data["token_slot"] + slot.allocated += 1 + slot.save() + return TokenBooking.objects.create(**data) + def generate_schedule_data(self, **kwargs): """Helper to generate valid schedule data.""" valid_from = datetime.now() @@ -203,20 +263,98 @@ def test_delete_schedule_without_permissions(self): response = self.client.delete(delete_url) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - def test_update_schedule_validity_with_booking_outside_validity(self): - pass + def test_update_schedule_validity_with_booking_within_new_validity(self): + permissions = [ + UserSchedulePermissions.can_write_user_schedule.name, + UserSchedulePermissions.can_list_user_schedule.name, + ] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + self.create_booking() + updated_data = { + "name": "Updated Schedule Name", + "valid_from": self.schedule.valid_from, + "valid_to": self.schedule.valid_to - timedelta(days=1), + } + update_url = self._get_schedule_url(self.schedule.external_id) + response = self.client.put(update_url, updated_data, format="json") + print(response.data) + self.assertEqual(response.status_code, status.HTTP_200_OK) + + def test_update_schedule_validity_with_booking_outside_new_validity(self): + permissions = [ + UserSchedulePermissions.can_write_user_schedule.name, + UserSchedulePermissions.can_list_user_schedule.name, + ] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + self.create_booking( + token_slot=self.create_slot( + start_datetime=datetime.now() + timedelta(days=4), + end_datetime=datetime.now() + timedelta(days=5), + ) + ) + updated_data = { + "name": "Updated Schedule Name", + "valid_from": self.schedule.valid_from, + "valid_to": self.schedule.valid_from + timedelta(days=1), + } + update_url = self._get_schedule_url(self.schedule.external_id) + response = self.client.put(update_url, updated_data, format="json") + self.assertContains( + response, + status_code=400, + text="Cannot modify schedule validity as it would exclude some allocated slots. Old range has 1 allocated slots while new range has 0 allocated slots.", + ) + + def test_delete_schedule_with_future_bookings(self): + """Users cannot delete schedules with bookings present in the future.""" + permissions = [ + UserSchedulePermissions.can_write_user_schedule.name, + UserSchedulePermissions.can_list_user_schedule.name, + ] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) - def test_delete_schedule_with_bookings(self): - pass + self.create_booking( + token_slot=self.create_slot( + start_datetime=datetime.now() + timedelta(days=4), + end_datetime=datetime.now() + timedelta(days=5), + ) + ) + delete_url = self._get_schedule_url(self.schedule.external_id) + response = self.client.delete(delete_url) + self.assertContains( + response, + status_code=400, + text="Cannot delete schedule as there are future bookings associated with it", + ) - def test_delete_availability_with_bookings(self): - pass + def test_delete_schedule_with_future_cancelled_bookings(self): + """Users cannot delete schedules with bookings present in the future.""" + permissions = [ + UserSchedulePermissions.can_write_user_schedule.name, + UserSchedulePermissions.can_list_user_schedule.name, + ] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + self.create_booking( + token_slot=self.create_slot( + start_datetime=datetime.now() + timedelta(days=4), + end_datetime=datetime.now() + timedelta(days=5), + ), + status=BookingStatusChoices.cancelled.value, + ) + delete_url = self._get_schedule_url(self.schedule.external_id) + response = self.client.delete(delete_url) + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) class TestAvailabilityExceptionsViewSet(CareAPITestBase): def setUp(self): - from care.emr.models import SchedulableUserResource - super().setUp() self.user = self.create_user() self.facility = self.create_facility(user=self.user) @@ -271,7 +409,6 @@ def generate_exception_data(self, **kwargs): **kwargs, } - # LIST TESTS def test_list_exceptions_with_permissions(self): """Users with can_list_user_schedule permission can list exceptions.""" permissions = [UserSchedulePermissions.can_list_user_schedule.name] @@ -286,7 +423,6 @@ def test_list_exceptions_without_permissions(self): response = self.client.get(self.base_url) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - # CREATE TESTS def test_create_exception_with_permissions(self): """Users with can_write_user_schedule permission can create exceptions.""" permissions = [UserSchedulePermissions.can_write_user_schedule.name] @@ -304,7 +440,6 @@ def test_create_exception_without_permissions(self): response = self.client.post(self.base_url, exception_data, format="json") self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - # UPDATE TESTS def test_update_exception_with_permissions(self): """Users with can_write_user_schedule permission can update exceptions.""" permissions = [ @@ -352,7 +487,6 @@ def test_update_exception_without_permissions(self): response = self.client.put(update_url, updated_data, format="json") self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - # DELETE TESTS def test_delete_exception_with_permissions(self): """Users with can_write_user_schedule permission can delete exceptions.""" permissions = [ @@ -384,13 +518,75 @@ def test_delete_exception_without_permissions(self): self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) def test_create_exception_with_bookings(self): - pass + """Test that creating an exception fails when there are conflicting bookings.""" + permissions = [UserSchedulePermissions.can_write_user_schedule.name] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + # Create a schedule + schedule = Schedule.objects.create( + resource=self.resource, + name="Test Schedule", + valid_from=datetime.now() - timedelta(days=30), + valid_to=datetime.now() + timedelta(days=30), + ) + + # Create an availability + availability = Availability.objects.create( + schedule=schedule, + name="Test Availability", + slot_type=SlotTypeOptions.appointment.value, + slot_size_in_minutes=30, + tokens_per_slot=1, + create_tokens=False, + reason="Regular schedule", + availability=[ + { + "day_of_week": datetime.now().weekday(), + "start_time": "09:00:00", + "end_time": "17:00:00", + } + ], + ) + + # Create a slot for today + slot_start = datetime.now().replace(hour=10, minute=0, second=0, microsecond=0) + slot = TokenSlot.objects.create( + resource=self.resource, + availability=availability, + start_datetime=slot_start, + end_datetime=slot_start + timedelta(minutes=30), + allocated=1, + ) + + # Create a booking for the slot + patient = self.create_patient() + TokenBooking.objects.create( + token_slot=slot, + patient=patient, + booked_by=self.user, + status=BookingStatusChoices.booked.value, + ) + + # Try to create an exception that overlaps with the booking + exception_data = self.generate_exception_data( + valid_from=slot_start.date().isoformat(), + valid_to=slot_start.date().isoformat(), + start_time="09:00:00", + end_time="17:00:00", + ) + + response = self.client.post(self.base_url, exception_data, format="json") + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertContains( + response, + "There are bookings during this exception", + status_code=400, + ) class TestAvailabilityViewSet(CareAPITestBase): def setUp(self): - from care.emr.models import SchedulableUserResource - super().setUp() self.user = self.create_user() self.facility = self.create_facility(user=self.user) @@ -521,5 +717,149 @@ def test_delete_availability_without_permissions(self): response = self.client.delete(delete_url) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - def test_delete_availability_with_bookings(self): - pass + def test_delete_availability_without_queryset_list_permissions(self): + """Users without can_list_user_schedule permission cannot delete availability.""" + availability = self.create_availability() + delete_url = self._get_availability_url(availability.external_id) + response = self.client.delete(delete_url) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_delete_availability_with_future_bookings(self): + """Users cannot delete availability with future bookings.""" + permissions = [ + UserSchedulePermissions.can_list_user_schedule.name, + UserSchedulePermissions.can_write_user_schedule.name, + ] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + availability = self.create_availability() + token_slot = TokenSlot.objects.create( + resource=self.resource, + availability=availability, + start_datetime=datetime.now() + timedelta(days=4), + end_datetime=datetime.now() + timedelta(days=5), + ) + TokenBooking.objects.create( + token_slot=token_slot, + patient=self.create_patient(), + booked_by=self.user, + ) + token_slot.allocated = 1 + token_slot.save() + delete_url = self._get_availability_url(availability.external_id) + response = self.client.delete(delete_url) + self.assertContains( + response, + status_code=400, + text="Cannot delete availability as there are future bookings associated with it", + ) + + def test_create_availability_validate_availability(self): + """Test that creating availability with overlapping time ranges fails.""" + permissions = [UserSchedulePermissions.can_write_user_schedule.name] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + # Try to create availability with overlapping time ranges for same day + data = self.generate_availability_data( + availability=[ + { + "day_of_week": 1, # Monday + "start_time": "09:00:00", + "end_time": "13:00:00", + }, + { + "day_of_week": 1, # Same day (Monday) + "start_time": "12:00:00", # Overlaps with previous range + "end_time": "17:00:00", + }, + ] + ) + response = self.client.post(self.base_url, data, format="json") + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertContains( + response, + "Availability time ranges are overlapping", + status_code=400, + ) + + # Verify that non-overlapping ranges on same day are allowed + data = self.generate_availability_data( + availability=[ + { + "day_of_week": 1, + "start_time": "09:00:00", + "end_time": "12:00:00", + }, + { + "day_of_week": 1, + "start_time": "13:00:00", # No overlap + "end_time": "17:00:00", + }, + ] + ) + + response = self.client.post(self.base_url, data, format="json") + self.assertEqual(response.status_code, status.HTTP_200_OK) + + # Verify that overlapping times on different days are allowed + data = self.generate_availability_data( + availability=[ + { + "day_of_week": 1, # Monday + "start_time": "09:00:00", + "end_time": "17:00:00", + }, + { + "day_of_week": 2, # Tuesday + "start_time": "09:00:00", # Same time range but different day + "end_time": "17:00:00", + }, + ] + ) + + response = self.client.post(self.base_url, data, format="json") + self.assertEqual(response.status_code, status.HTTP_200_OK) + + def test_create_availability_validate_slot_type(self): + """Test slot type validation rules for availability creation.""" + permissions = [UserSchedulePermissions.can_write_user_schedule.name] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + # Test appointment type without slot_size_in_minutes + data = self.generate_availability_data( + slot_type=SlotTypeOptions.appointment.value, + slot_size_in_minutes=None, + ) + response = self.client.post(self.base_url, data, format="json") + self.assertContains( + response, + "Slot size in minutes is required for appointment slots", + status_code=400, + ) + + # Test appointment type without tokens_per_slot + data = self.generate_availability_data( + slot_type=SlotTypeOptions.appointment.value, + tokens_per_slot=None, + ) + response = self.client.post(self.base_url, data, format="json") + self.assertContains( + response, + "Tokens per slot is required for appointment slots", + status_code=400, + ) + + # Test open slot type (should accept without slot_size and tokens) + data = self.generate_availability_data( + slot_type=SlotTypeOptions.open.value, + slot_size_in_minutes=30, # These should be ignored + tokens_per_slot=1, # These should be ignored + ) + + response = self.client.post(self.base_url, data, format="json") + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertIsNone(response.data["slot_size_in_minutes"]) + self.assertIsNone(response.data["tokens_per_slot"]) From 0655a9968a379622b97a59d8d0dd60ef37aaf8c5 Mon Sep 17 00:00:00 2001 From: rithviknishad Date: Sat, 18 Jan 2025 16:41:06 +0530 Subject: [PATCH 16/24] fix edge case --- care/emr/api/viewsets/scheduling/availability.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/care/emr/api/viewsets/scheduling/availability.py b/care/emr/api/viewsets/scheduling/availability.py index 798d5eb934..0261b80d74 100644 --- a/care/emr/api/viewsets/scheduling/availability.py +++ b/care/emr/api/viewsets/scheduling/availability.py @@ -55,8 +55,18 @@ def validate_period(self): def convert_availability_and_exceptions_to_slots(availabilities, exceptions, day): slots = {} for availability in availabilities: - start_time = parse(availability["availability"]["start_time"]) - end_time = parse(availability["availability"]["end_time"]) + start_time = datetime.datetime.combine( + day, + time.fromisoformat(availability["availability"]["start_time"]), + tzinfo=None, + ) + end_time = datetime.datetime.combine( + day, + time.fromisoformat(availability["availability"]["end_time"]), + tzinfo=None, + ) + # start_time = parse(availability["availability"]["start_time"]) + # end_time = parse(availability["availability"]["end_time"]) slot_size_in_minutes = availability["slot_size_in_minutes"] availability_id = availability["availability_id"] current_time = start_time @@ -343,7 +353,7 @@ def calculate_slots( end_time = datetime.datetime.combine( date, time.fromisoformat(available_slot["end_time"]), tzinfo=None ) - while start_time <= end_time: + while start_time < end_time: conflicting = False for exception in exceptions: exception_start_time = datetime.datetime.combine( From 8be15110633b5bdf0b4848bc88e89ccd68452002 Mon Sep 17 00:00:00 2001 From: rithviknishad Date: Sat, 18 Jan 2025 18:13:13 +0530 Subject: [PATCH 17/24] fix another edge case in heatmap and tests pass --- .../api/viewsets/scheduling/availability.py | 14 ++--- care/emr/tests/test_booking_api.py | 54 ++++++++++++++----- 2 files changed, 48 insertions(+), 20 deletions(-) diff --git a/care/emr/api/viewsets/scheduling/availability.py b/care/emr/api/viewsets/scheduling/availability.py index 0261b80d74..2f85677ec0 100644 --- a/care/emr/api/viewsets/scheduling/availability.py +++ b/care/emr/api/viewsets/scheduling/availability.py @@ -353,8 +353,12 @@ def calculate_slots( end_time = datetime.datetime.combine( date, time.fromisoformat(available_slot["end_time"]), tzinfo=None ) - while start_time < end_time: + current_start_time = start_time + while current_start_time < end_time: conflicting = False + current_end_time = current_start_time + timedelta( + minutes=availability["slot_size_in_minutes"] + ) for exception in exceptions: exception_start_time = datetime.datetime.combine( date, exception["start_time"], tzinfo=None @@ -363,13 +367,11 @@ def calculate_slots( date, exception["end_time"], tzinfo=None ) if ( - exception_start_time < end_time - and exception_end_time > start_time + exception_start_time < current_end_time + and exception_end_time > current_start_time ): conflicting = True - start_time = start_time + timedelta( - minutes=availability["slot_size_in_minutes"] - ) + current_start_time = current_end_time if conflicting: continue slots += availability["tokens_per_slot"] diff --git a/care/emr/tests/test_booking_api.py b/care/emr/tests/test_booking_api.py index c74d16273f..8f44237994 100644 --- a/care/emr/tests/test_booking_api.py +++ b/care/emr/tests/test_booking_api.py @@ -811,19 +811,50 @@ def test_availability_stats_for_non_schedulable_user(self): response, status_code=400, text="Resource is not schedulable" ) - def test_availability_heatmap_slots_same_as_get_slots_for_day(self): + def test_availability_heatmap_slots_same_as_get_slots_for_day_without_exceptions( + self, + ): + """Get heatmap availability stats for 7 days and verify same slot stats as get_slots_for_day""" + # we don't want the slot that was created in setUp; create availability exception would've done this for us anyways. + self.slot.delete() + data = { + "user": self.user.external_id, + "from_date": datetime.now().strftime("%Y-%m-%d"), + "to_date": (datetime.now() + timedelta(days=7)).strftime("%Y-%m-%d"), + } + availability_stats_url = reverse( + "slot-availability-stats", + kwargs={"facility_external_id": self.facility.external_id}, + ) + response = self.client.post(availability_stats_url, data, format="json") + self.assertEqual(response.status_code, 200) + + slots_for_day_url = reverse( + "slot-get-slots-for-day", + kwargs={"facility_external_id": self.facility.external_id}, + ) + for day, slot_stats in response.data.items(): + data = {"user": self.user.external_id, "day": day} + response = self.client.post(slots_for_day_url, data, format="json") + self.assertEqual(response.status_code, 200) + booked_slots_for_day = sum(x["allocated"] for x in response.data["results"]) + total_slots_for_day = sum( + x["availability"]["tokens_per_slot"] for x in response.data["results"] + ) + self.assertEqual(slot_stats["booked_slots"], booked_slots_for_day) + self.assertEqual(slot_stats["total_slots"], total_slots_for_day) + + def test_availability_heatmap_slots_same_as_get_slots_for_day_with_exceptions(self): """Get heatmap availability stats for 7 days and verify same slot stats as get_slots_for_day""" - ## TODO: figure out what's happening here; getting expected results in front-end; # we don't want the slot that was created in setUp; create availability exception would've done this for us anyways. self.slot.delete() - # AvailabilityException.objects.create( resource=self.resource, name="Test Exception", valid_from=datetime.now(), valid_to=datetime.now() + timedelta(days=1), start_time="00:00:00", - end_time="11:59:59", + end_time="23:59:59", ) AvailabilityException.objects.create( resource=self.resource, @@ -853,14 +884,9 @@ def test_availability_heatmap_slots_same_as_get_slots_for_day(self): data = {"user": self.user.external_id, "day": day} response = self.client.post(slots_for_day_url, data, format="json") self.assertEqual(response.status_code, 200) - self.assertEqual( - slot_stats["booked_slots"], - sum(x["allocated"] for x in response.data["results"]), - ) - self.assertEqual( - slot_stats["total_slots"], - sum( - x["availability"]["tokens_per_slot"] - for x in response.data["results"] - ), + booked_slots_for_day = sum(x["allocated"] for x in response.data["results"]) + total_slots_for_day = sum( + x["availability"]["tokens_per_slot"] for x in response.data["results"] ) + self.assertEqual(slot_stats["booked_slots"], booked_slots_for_day) + self.assertEqual(slot_stats["total_slots"], total_slots_for_day) From 0ac3f8897967342938cb9bf18df020c871aca5fc Mon Sep 17 00:00:00 2001 From: rithviknishad Date: Sat, 18 Jan 2025 22:32:32 +0530 Subject: [PATCH 18/24] 100% coverage + fixes and cleanup --- care/emr/api/viewsets/scheduling/booking.py | 2 - care/emr/api/viewsets/scheduling/schedule.py | 4 +- .../emr/resources/scheduling/schedule/spec.py | 23 +-- care/emr/tests/test_booking_api.py | 195 ++++++++++++++++++ care/emr/tests/test_schedule_api.py | 28 ++- 5 files changed, 234 insertions(+), 18 deletions(-) diff --git a/care/emr/api/viewsets/scheduling/booking.py b/care/emr/api/viewsets/scheduling/booking.py index 7486603c01..0257b95e2a 100644 --- a/care/emr/api/viewsets/scheduling/booking.py +++ b/care/emr/api/viewsets/scheduling/booking.py @@ -49,8 +49,6 @@ class TokenBookingFilters(FilterSet): patient = UUIDFilter(field_name="patient__external_id") def filter_by_user(self, queryset, name, value): - if not value: - return queryset resource = SchedulableUserResource.objects.filter( user__external_id=value ).first() diff --git a/care/emr/api/viewsets/scheduling/schedule.py b/care/emr/api/viewsets/scheduling/schedule.py index b822e7cf22..29cf92d2c2 100644 --- a/care/emr/api/viewsets/scheduling/schedule.py +++ b/care/emr/api/viewsets/scheduling/schedule.py @@ -18,7 +18,7 @@ AvailabilityForScheduleSpec, ScheduleReadSpec, ScheduleUpdateSpec, - ScheduleWriteSpec, + ScheduleCreateSpec, ) from care.facility.models import Facility from care.security.authorization import AuthorizationController @@ -32,7 +32,7 @@ class ScheduleFilters(FilterSet): class ScheduleViewSet(EMRModelViewSet): database_model = Schedule - pydantic_model = ScheduleWriteSpec + pydantic_model = ScheduleCreateSpec pydantic_update_model = ScheduleUpdateSpec pydantic_read_model = ScheduleReadSpec filterset_class = ScheduleFilters diff --git a/care/emr/resources/scheduling/schedule/spec.py b/care/emr/resources/scheduling/schedule/spec.py index e39085de4a..0b28b2d43b 100644 --- a/care/emr/resources/scheduling/schedule/spec.py +++ b/care/emr/resources/scheduling/schedule/spec.py @@ -86,7 +86,7 @@ class ScheduleBaseSpec(EMRResource): id: UUID4 | None = None -class ScheduleWriteSpec(ScheduleBaseSpec): +class ScheduleCreateSpec(ScheduleBaseSpec): user: UUID4 facility: UUID4 name: str @@ -101,17 +101,16 @@ def validate_period(self): return self def perform_extra_deserialization(self, is_update, obj): - if not is_update: - user = get_object_or_404(User, external_id=self.user) - # TODO Validation that user is in given facility - obj.facility = Facility.objects.get(external_id=self.facility) - - resource, _ = SchedulableUserResource.objects.get_or_create( - facility=obj.facility, - user=user, - ) - obj.resource = resource - obj.availabilities = self.availabilities + user = get_object_or_404(User, external_id=self.user) + # TODO Validation that user is in given facility + obj.facility = Facility.objects.get(external_id=self.facility) + + resource, _ = SchedulableUserResource.objects.get_or_create( + facility=obj.facility, + user=user, + ) + obj.resource = resource + obj.availabilities = self.availabilities class ScheduleUpdateSpec(ScheduleBaseSpec): diff --git a/care/emr/tests/test_booking_api.py b/care/emr/tests/test_booking_api.py index 8f44237994..49c67fefaa 100644 --- a/care/emr/tests/test_booking_api.py +++ b/care/emr/tests/test_booking_api.py @@ -18,6 +18,8 @@ from django.urls import reverse from datetime import datetime, timedelta +from config.patient_otp_authentication import PatientOtpObject + class TestBookingViewSet(CareAPITestBase): @@ -748,6 +750,20 @@ def test_availability_stats(self): response = self.client.post(url, data, format="json") self.assertEqual(response.status_code, 200) + def test_availability_stats_outside_schedule_validity(self): + """Get heatmap availability stats for few days""" + data = { + "user": self.user.external_id, + "from_date": (datetime.now() + timedelta(days=90)).strftime("%Y-%m-%d"), + "to_date": (datetime.now() + timedelta(days=100)).strftime("%Y-%m-%d"), + } + url = reverse( + "slot-availability-stats", + kwargs={"facility_external_id": self.facility.external_id}, + ) + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, 200) + def test_availability_stats_invalid_period(self): """Get heatmap availability stats for from date after to date""" data = { @@ -890,3 +906,182 @@ def test_availability_heatmap_slots_same_as_get_slots_for_day_with_exceptions(se ) self.assertEqual(slot_stats["booked_slots"], booked_slots_for_day) self.assertEqual(slot_stats["total_slots"], total_slots_for_day) + + +class TestOtpSlotViewSet(CareAPITestBase): + def setUp(self): + super().setUp() + self.user = self.create_user() + self.facility = self.create_facility(user=self.user) + self.organization = self.create_facility_organization(facility=self.facility) + self.patient = self.create_patient(phone_number="+917777777777") + self.resource = SchedulableUserResource.objects.create( + user=self.user, facility=self.facility + ) + self.schedule = Schedule.objects.create( + resource=self.resource, + name="Test Schedule", + valid_from=datetime.now() - timedelta(days=30), + valid_to=datetime.now() + timedelta(days=30), + ) + self.availability = self.create_availability() + self.slot = self.create_slot() + self.client.force_authenticate(user=self.get_patient_otp_object()) + + def get_patient_otp_object(self): + obj = PatientOtpObject() + obj.phone_number = self.patient.phone_number + return obj + + def create_appointment(self, **kwargs): + data = { + "token_slot": self.slot, + "patient": self.patient, + "booked_by": self.user, + "status": BookingStatusChoices.booked.value, + } + data.update(kwargs) + return TokenBooking.objects.create(**data) + + def create_slot(self, **kwargs): + data = { + "resource": self.resource, + "availability": self.availability, + "start_datetime": datetime.now() + timedelta(minutes=30), + "end_datetime": datetime.now() + timedelta(minutes=60), + "allocated": 0, + } + data.update(kwargs) + return TokenSlot.objects.create(**data) + + def create_availability(self, **kwargs): + return Availability.objects.create( + schedule=self.schedule, + name=kwargs.get("name", "Test Availability"), + slot_type=kwargs.get("slot_type", SlotTypeOptions.appointment.value), + slot_size_in_minutes=kwargs.get("slot_size_in_minutes", 30), + tokens_per_slot=kwargs.get("tokens_per_slot", 1), + create_tokens=kwargs.get("create_tokens", False), + reason=kwargs.get("reason", "Regular schedule"), + availability=kwargs.get( + "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", + }, + ], + ), + ) + + def test_get_slots_for_day(self): + url = reverse("otp-slots-get-slots-for-day") + data = { + "user": self.user.external_id, + "day": datetime.now().strftime("%Y-%m-%d"), + "facility": self.facility.external_id, + } + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, 200) + + def test_get_slots_for_day_without_facility(self): + url = reverse("otp-slots-get-slots-for-day") + data = { + "user": self.user.external_id, + "day": datetime.now().strftime("%Y-%m-%d"), + } + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, 400) + + def test_create_appointment(self): + data = { + "patient": self.patient.external_id, + "reason_for_visit": "Test Reason", + } + url = reverse( + "otp-slots-create-appointment", + kwargs={"external_id": self.slot.external_id}, + ) + response = self.client.post(url, data, format="json") + self.assertContains(response, BookingStatusChoices.booked.value) + + def test_create_appointment_of_another_patient(self): + other_patient = self.create_patient(phone_number="+917777777778") + data = { + "patient": other_patient.external_id, + "reason_for_visit": "Test Reason", + } + url = reverse( + "otp-slots-create-appointment", + kwargs={"external_id": self.slot.external_id}, + ) + response = self.client.post(url, data, format="json") + self.assertContains(response, "Patient not allowed", status_code=400) + + def test_cancel_appointment(self): + booking = self.create_appointment() + url = reverse("otp-slots-cancel-appointment") + data = { + "patient": booking.patient.external_id, + "appointment": booking.external_id, + } + response = self.client.post(url, data, format="json") + self.assertContains(response, BookingStatusChoices.cancelled.value) + + def test_cancel_appointment_of_another_patient(self): + other_patient = self.create_patient(phone_number="+917777777778") + booking = self.create_appointment(patient=other_patient) + url = reverse("otp-slots-cancel-appointment") + data = { + "patient": booking.patient.external_id, + "appointment": booking.external_id, + } + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, 404) + + def test_get_appointments(self): + booking = self.create_appointment() + url = reverse("otp-slots-get-appointments") + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.data["results"]), 1) + self.assertEqual(response.data["results"][0]["id"], booking.external_id) + + def test_get_appointments_of_another_patient(self): + other_patient = self.create_patient(phone_number="+917777777778") + self.create_appointment(patient=other_patient) + url = reverse("otp-slots-get-appointments") + response = self.client.get(url) + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.data["results"]), 0) diff --git a/care/emr/tests/test_schedule_api.py b/care/emr/tests/test_schedule_api.py index bccb17e3fc..ae205f61ec 100644 --- a/care/emr/tests/test_schedule_api.py +++ b/care/emr/tests/test_schedule_api.py @@ -154,7 +154,6 @@ def test_list_schedule_without_permissions(self): response = self.client.get(self.base_url) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) - # CREATE TESTS def test_create_schedule_with_permissions(self): """Users with can_write_user_schedule permission can create schedules.""" permissions = [UserSchedulePermissions.can_write_user_schedule.name] @@ -190,7 +189,19 @@ def test_create_schedule_with_invalid_dates(self): response, "Valid from cannot be greater than valid to", status_code=400 ) - # UPDATE TESTS + def test_create_schedule_with_user_not_part_of_facility(self): + """Users cannot write schedules for user not belonging to the facility.""" + permissions = [UserSchedulePermissions.can_write_user_schedule.name] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + user = self.create_user() + schedule_data = self.generate_schedule_data(user=user.external_id) + response = self.client.post(self.base_url, schedule_data, format="json") + self.assertContains( + response, "Schedule User is not part of the facility", status_code=400 + ) + def test_update_schedule_with_permissions(self): """Users with can_write_user_schedule permission can update schedules.""" permissions = [ @@ -440,6 +451,19 @@ def test_create_exception_without_permissions(self): response = self.client.post(self.base_url, exception_data, format="json") self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + def test_create_exception_with_invalid_user_resource(self): + """Users with can_write_user_schedule permission can create exceptions.""" + permissions = [UserSchedulePermissions.can_write_user_schedule.name] + role = self.create_role_with_permissions(permissions) + self.attach_role_facility_organization_user(self.organization, self.user, role) + + # Resource doesn't exist + self.resource.delete() + + exception_data = self.generate_exception_data() + response = self.client.post(self.base_url, exception_data, format="json") + self.assertContains(response, "Object does not exist", status_code=400) + def test_update_exception_with_permissions(self): """Users with can_write_user_schedule permission can update exceptions.""" permissions = [ From 12250574aa02ed950fb714f25027837087879f0a Mon Sep 17 00:00:00 2001 From: rithviknishad Date: Sat, 18 Jan 2025 22:35:20 +0530 Subject: [PATCH 19/24] fix a test edge case --- care/emr/tests/test_booking_api.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/care/emr/tests/test_booking_api.py b/care/emr/tests/test_booking_api.py index 49c67fefaa..9f33d8fcca 100644 --- a/care/emr/tests/test_booking_api.py +++ b/care/emr/tests/test_booking_api.py @@ -750,12 +750,12 @@ def test_availability_stats(self): response = self.client.post(url, data, format="json") self.assertEqual(response.status_code, 200) - def test_availability_stats_outside_schedule_validity(self): - """Get heatmap availability stats for few days""" + def test_availability_stats_partially_outside_schedule_validity(self): + """Get heatmap availability stats for days partially outside schedule validity""" data = { "user": self.user.external_id, - "from_date": (datetime.now() + timedelta(days=90)).strftime("%Y-%m-%d"), - "to_date": (datetime.now() + timedelta(days=100)).strftime("%Y-%m-%d"), + "from_date": (datetime.now() + timedelta(days=25)).strftime("%Y-%m-%d"), + "to_date": (datetime.now() + timedelta(days=35)).strftime("%Y-%m-%d"), } url = reverse( "slot-availability-stats", From 6599c9636e0216b9f1bb9da3253d08c0b318c5d3 Mon Sep 17 00:00:00 2001 From: rithviknishad Date: Sat, 18 Jan 2025 23:32:36 +0530 Subject: [PATCH 20/24] fix test cases and cleanup comments and lint errors --- care/emr/tests/test_booking_api.py | 191 ++++++++++++++-------------- care/emr/tests/test_schedule_api.py | 6 +- 2 files changed, 101 insertions(+), 96 deletions(-) diff --git a/care/emr/tests/test_booking_api.py b/care/emr/tests/test_booking_api.py index 9f33d8fcca..19d4397736 100644 --- a/care/emr/tests/test_booking_api.py +++ b/care/emr/tests/test_booking_api.py @@ -1,28 +1,26 @@ -from http.client import responses +from datetime import UTC, datetime, timedelta + +from django.urls import reverse from care.emr.models import ( + Availability, + AvailabilityException, SchedulableUserResource, Schedule, - Availability, - TokenSlot, TokenBooking, - AvailabilityException, + TokenSlot, ) from care.emr.resources.scheduling.schedule.spec import SlotTypeOptions from care.emr.resources.scheduling.slot.spec import ( - BookingStatusChoices, CANCELLED_STATUS_CHOICES, + BookingStatusChoices, ) from care.security.permissions.user_schedule import UserSchedulePermissions from care.utils.tests.base import CareAPITestBase -from django.urls import reverse -from datetime import datetime, timedelta - from config.patient_otp_authentication import PatientOtpObject class TestBookingViewSet(CareAPITestBase): - def setUp(self): super().setUp() self.user = self.create_user() @@ -36,8 +34,8 @@ def setUp(self): self.schedule = Schedule.objects.create( resource=self.resource, name="Test Schedule", - valid_from=datetime.now() - timedelta(days=30), - valid_to=datetime.now() + timedelta(days=30), + valid_from=datetime.now(UTC) - timedelta(days=30), + valid_to=datetime.now(UTC) + timedelta(days=30), ) self.availability = Availability.objects.create( schedule=self.schedule, @@ -93,8 +91,8 @@ def create_slot(self, **kwargs): data = { "resource": self.resource, "availability": self.availability, - "start_datetime": datetime.now() + timedelta(minutes=30), - "end_datetime": datetime.now() + timedelta(minutes=60), + "start_datetime": datetime.now(UTC) + timedelta(minutes=30), + "end_datetime": datetime.now(UTC) + timedelta(minutes=60), "allocated": 0, } data.update(kwargs) @@ -205,7 +203,7 @@ def test_cancel_booking_via_update(self): ) def test_cancel_booking_with_permission(self): - """Users can cancel bookings via the cancel endpoint.""" + """Users with proper permissions can cancel bookings via the cancel endpoint.""" permissions = [ UserSchedulePermissions.can_write_user_booking.name, UserSchedulePermissions.can_list_user_booking.name, @@ -232,7 +230,7 @@ def test_cancel_booking_with_permission(self): self.assertEqual(tokens_allocated_before - 1, tokens_allocated_after) def test_cancel_booking_without_permission(self): - """Users cannot cancel bookings via the cancel endpoint.""" + """Users without proper permissions cannot cancel bookings via the cancel endpoint.""" permissions = [ UserSchedulePermissions.can_list_user_booking.name, ] @@ -289,7 +287,7 @@ def test_cancel_cancelled_booking(self): self.assertEqual(tokens_allocated_before, tokens_allocated_after) def test_reschedule_booking_with_permission(self): - """Users can reschedule bookings via the re-schedule endpoint.""" + """Users with proper permissions can reschedule bookings via the re-schedule endpoint.""" permissions = [ UserSchedulePermissions.can_write_user_booking.name, UserSchedulePermissions.can_list_user_booking.name, @@ -312,7 +310,7 @@ def test_reschedule_booking_with_permission(self): self.assertEqual(response.status_code, 200) def test_reschedule_booking_without_permission(self): - """Users cannot reschedule bookings via the re-schedule endpoint.""" + """Users without proper permissions cannot reschedule bookings via the re-schedule endpoint.""" permissions = [ UserSchedulePermissions.can_write_user_booking.name, UserSchedulePermissions.can_list_user_booking.name, @@ -338,7 +336,7 @@ def test_reschedule_booking_without_permission(self): ) def test_reschedule_booking_with_slot_in_past(self): - """Users cannot reschedule bookings via the re-schedule endpoint.""" + """Users cannot reschedule bookings to slots that are in the past.""" permissions = [ UserSchedulePermissions.can_write_user_booking.name, UserSchedulePermissions.can_list_user_booking.name, @@ -348,8 +346,8 @@ def test_reschedule_booking_with_slot_in_past(self): self.attach_role_facility_organization_user(self.organization, self.user, role) new_slot = self.create_slot( - start_datetime=datetime.now() - timedelta(minutes=30), - end_datetime=datetime.now() - timedelta(minutes=15), + start_datetime=datetime.now(UTC) - timedelta(minutes=30), + end_datetime=datetime.now(UTC) - timedelta(minutes=15), ) booking = self.create_booking() reschedule_url = reverse( @@ -368,6 +366,7 @@ def test_reschedule_booking_with_slot_in_past(self): ) def test_list_available_users(self): + """Users can list available schedulable users.""" available_users_url = reverse( "appointments-available-users", kwargs={"facility_external_id": self.facility.external_id}, @@ -391,8 +390,8 @@ def setUp(self): self.schedule = Schedule.objects.create( resource=self.resource, name="Test Schedule", - valid_from=datetime.now() - timedelta(days=30), - valid_to=datetime.now() + timedelta(days=30), + valid_from=datetime.now(UTC) - timedelta(days=30), + valid_to=datetime.now(UTC) + timedelta(days=30), ) self.availability = self.create_availability() self.slot = self.create_slot() @@ -422,8 +421,8 @@ def create_slot(self, **kwargs): data = { "resource": self.resource, "availability": self.availability, - "start_datetime": datetime.now() + timedelta(minutes=30), - "end_datetime": datetime.now() + timedelta(minutes=60), + "start_datetime": datetime.now(UTC) + timedelta(minutes=30), + "end_datetime": datetime.now(UTC) + timedelta(minutes=60), "allocated": 0, } data.update(kwargs) @@ -509,7 +508,7 @@ def test_create_appointment_without_permission(self): self.assertEqual(response.status_code, 403) def test_create_appointment_with_invalid_patient(self): - """Users with can_create_appointment permission can create appointments.""" + """Users cannot create appointments for invalid patients.""" permissions = [UserSchedulePermissions.can_create_appointment.name] role = self.create_role_with_permissions(permissions) self.attach_role_facility_organization_user(self.organization, self.user, role) @@ -521,14 +520,14 @@ def test_create_appointment_with_invalid_patient(self): self.assertContains(response, status_code=400, text="Patient not found") def test_create_appointment_with_slot_in_past(self): - """Users cannot create appointments on a past slot.""" + """Users cannot create appointments for slots that are in the past.""" permissions = [UserSchedulePermissions.can_create_appointment.name] role = self.create_role_with_permissions(permissions) self.attach_role_facility_organization_user(self.organization, self.user, role) slot = self.create_slot( - start_datetime=datetime.now() - timedelta(minutes=30), - end_datetime=datetime.now() - timedelta(minutes=15), + start_datetime=datetime.now(UTC) - timedelta(minutes=30), + end_datetime=datetime.now(UTC) - timedelta(minutes=15), ) data = self.get_appointment_data() response = self.client.post( @@ -537,7 +536,7 @@ def test_create_appointment_with_slot_in_past(self): self.assertContains(response, status_code=400, text="Slot is already past") def test_create_multiple_appointments_on_same_slot(self): - """Users cannot create multiple appointments on the same slot (as long as previous ones are cancelled)""" + """Users cannot create multiple appointments on the same slot for the same patient.""" permissions = [UserSchedulePermissions.can_create_appointment.name] role = self.create_role_with_permissions(permissions) self.attach_role_facility_organization_user(self.organization, self.user, role) @@ -555,7 +554,7 @@ def test_create_multiple_appointments_on_same_slot(self): ) def test_cancel_and_create_appointment_on_same_slot(self): - """Users can create an appointment on the same slot if the previous one is cancelled""" + """Users can create a new appointment on a slot after cancelling the previous one.""" permissions = [UserSchedulePermissions.can_create_appointment.name] role = self.create_role_with_permissions(permissions) self.attach_role_facility_organization_user(self.organization, self.user, role) @@ -569,7 +568,7 @@ def test_cancel_and_create_appointment_on_same_slot(self): self.assertEqual(response.status_code, 200) def test_over_booking_a_slot(self): - """Users cannot create an appointment on a slot if it is already fully booked""" + """Users cannot create appointments on slots that are already fully booked.""" permissions = [UserSchedulePermissions.can_create_appointment.name] role = self.create_role_with_permissions(permissions) self.attach_role_facility_organization_user(self.organization, self.user, role) @@ -584,10 +583,10 @@ def test_over_booking_a_slot(self): self.assertContains(response, status_code=400, text="Slot is already full") def test_get_slots_for_day(self): - """Get slots for a specific day.""" + """Users can get available slots for a specific day.""" data = { "user": self.user.external_id, - "day": datetime.now().strftime("%Y-%m-%d"), + "day": datetime.now(UTC).strftime("%Y-%m-%d"), } url = reverse( "slot-get-slots-for-day", @@ -595,13 +594,13 @@ def test_get_slots_for_day(self): ) response = self.client.post(url, data, format="json") self.assertEqual(response.status_code, 200) - self.assertEqual(len(response.data["results"]), 9) + self.assertEqual(len(response.data["results"]), 8) def test_hit_on_get_slots_for_day_does_not_cause_duplicate_slots(self): - """Get slots for a specific day.""" + """Multiple requests to get slots for a day should not create duplicate slots.""" data = { "user": self.user.external_id, - "day": datetime.now().strftime("%Y-%m-%d"), + "day": datetime.now(UTC).strftime("%Y-%m-%d"), } url = reverse( "slot-get-slots-for-day", @@ -609,19 +608,19 @@ def test_hit_on_get_slots_for_day_does_not_cause_duplicate_slots(self): ) response = self.client.post(url, data, format="json") self.assertEqual(response.status_code, 200) - self.assertEqual(len(response.data["results"]), 9) + self.assertEqual(len(response.data["results"]), 8) response = self.client.post(url, data, format="json") self.assertEqual(response.status_code, 200) - self.assertEqual(len(response.data["results"]), 9) + self.assertEqual(len(response.data["results"]), 8) def test_get_slots_for_day_for_non_schedulable_user(self): - """Cannot get slots for non-schedulable user.""" + """Cannot get slots for users that are not schedulable.""" user = self.create_user() facility = self.create_facility(user=user) data = { "user": user.external_id, - "day": datetime.now().strftime("%Y-%m-%d"), + "day": datetime.now(UTC).strftime("%Y-%m-%d"), } url = reverse( "slot-get-slots-for-day", @@ -633,22 +632,21 @@ def test_get_slots_for_day_for_non_schedulable_user(self): ) def test_get_slots_for_day_with_full_day_exception(self): - """Get no slots for day with whole day exception""" - + """No slots should be available for days with full day exceptions.""" # we don't want the slot that was created in setUp; create availability exception would've done this for us anyways. self.slot.delete() AvailabilityException.objects.create( resource=self.resource, name="Test Exception", - valid_from=datetime.now() - timedelta(days=1), - valid_to=datetime.now() + timedelta(days=1), + valid_from=datetime.now(UTC) - timedelta(days=1), + valid_to=datetime.now(UTC) + timedelta(days=1), start_time="00:00:00", end_time="23:59:59", ) data = { "user": self.user.external_id, - "day": datetime.now().strftime("%Y-%m-%d"), + "day": datetime.now(UTC).strftime("%Y-%m-%d"), } url = reverse( "slot-get-slots-for-day", @@ -659,22 +657,21 @@ def test_get_slots_for_day_with_full_day_exception(self): self.assertEqual(len(response.data["results"]), 0) def test_get_slots_for_day_with_exception_left_overlap(self): - """Get fewer slots for day with partially overlapping exception""" - + """Fewer slots should be available when there is an exception overlapping the start of the day.""" # we don't want the slot that was created in setUp; create availability exception would've done this for us anyways. self.slot.delete() AvailabilityException.objects.create( resource=self.resource, name="Test Exception", - valid_from=datetime.now() - timedelta(days=1), - valid_to=datetime.now() + timedelta(days=1), + valid_from=datetime.now(UTC) - timedelta(days=1), + valid_to=datetime.now(UTC) + timedelta(days=1), start_time="00:00:00", end_time="12:00:00", ) data = { "user": self.user.external_id, - "day": datetime.now().strftime("%Y-%m-%d"), + "day": datetime.now(UTC).strftime("%Y-%m-%d"), } url = reverse( "slot-get-slots-for-day", @@ -685,22 +682,21 @@ def test_get_slots_for_day_with_exception_left_overlap(self): self.assertEqual(len(response.data["results"]), 2) def test_get_slots_for_day_with_exception_right_overlap(self): - """Get fewer slots for day with partially overlapping exception""" - + """Fewer slots should be available when there is an exception overlapping the end of the day.""" # we don't want the slot that was created in setUp; create availability exception would've done this for us anyways. self.slot.delete() AvailabilityException.objects.create( resource=self.resource, name="Test Exception", - valid_from=datetime.now() - timedelta(days=1), - valid_to=datetime.now() + timedelta(days=1), + valid_from=datetime.now(UTC) - timedelta(days=1), + valid_to=datetime.now(UTC) + timedelta(days=1), start_time="10:00:00", end_time="23:59:59", ) data = { "user": self.user.external_id, - "day": datetime.now().strftime("%Y-%m-%d"), + "day": datetime.now(UTC).strftime("%Y-%m-%d"), } url = reverse( "slot-get-slots-for-day", @@ -711,22 +707,21 @@ def test_get_slots_for_day_with_exception_right_overlap(self): self.assertEqual(len(response.data["results"]), 2) def test_get_slots_for_day_with_exception_overlap_in_between(self): - """Get fewer slots for day with partially overlapping exception""" - + """Fewer slots should be available when there is an exception overlapping the middle of the day.""" # we don't want the slot that was created in setUp; create availability exception would've done this for us anyways. self.slot.delete() AvailabilityException.objects.create( resource=self.resource, name="Test Exception", - valid_from=datetime.now() - timedelta(days=1), - valid_to=datetime.now() + timedelta(days=1), + valid_from=datetime.now(UTC) - timedelta(days=1), + valid_to=datetime.now(UTC) + timedelta(days=1), start_time="10:00:00", end_time="12:00:00", ) data = { "user": self.user.external_id, - "day": datetime.now().strftime("%Y-%m-%d"), + "day": datetime.now(UTC).strftime("%Y-%m-%d"), } url = reverse( "slot-get-slots-for-day", @@ -737,11 +732,11 @@ def test_get_slots_for_day_with_exception_overlap_in_between(self): self.assertEqual(len(response.data["results"]), 4) def test_availability_stats(self): - """Get heatmap availability stats for few days""" + """Users can get availability statistics for a date range.""" data = { "user": self.user.external_id, - "from_date": datetime.now().strftime("%Y-%m-%d"), - "to_date": (datetime.now() + timedelta(days=10)).strftime("%Y-%m-%d"), + "from_date": datetime.now(UTC).strftime("%Y-%m-%d"), + "to_date": (datetime.now(UTC) + timedelta(days=10)).strftime("%Y-%m-%d"), } url = reverse( "slot-availability-stats", @@ -751,11 +746,11 @@ def test_availability_stats(self): self.assertEqual(response.status_code, 200) def test_availability_stats_partially_outside_schedule_validity(self): - """Get heatmap availability stats for days partially outside schedule validity""" + """Users can get availability statistics for date ranges partially outside schedule validity.""" data = { "user": self.user.external_id, - "from_date": (datetime.now() + timedelta(days=25)).strftime("%Y-%m-%d"), - "to_date": (datetime.now() + timedelta(days=35)).strftime("%Y-%m-%d"), + "from_date": (datetime.now(UTC) + timedelta(days=25)).strftime("%Y-%m-%d"), + "to_date": (datetime.now(UTC) + timedelta(days=35)).strftime("%Y-%m-%d"), } url = reverse( "slot-availability-stats", @@ -765,11 +760,11 @@ def test_availability_stats_partially_outside_schedule_validity(self): self.assertEqual(response.status_code, 200) def test_availability_stats_invalid_period(self): - """Get heatmap availability stats for from date after to date""" + """Users cannot get availability statistics when from_date is after to_date.""" data = { "user": self.user.external_id, - "from_date": (datetime.now() + timedelta(days=10)).strftime("%Y-%m-%d"), - "to_date": (datetime.now() + timedelta(days=1)).strftime("%Y-%m-%d"), + "from_date": (datetime.now(UTC) + timedelta(days=10)).strftime("%Y-%m-%d"), + "to_date": (datetime.now(UTC) + timedelta(days=1)).strftime("%Y-%m-%d"), } url = reverse( "slot-availability-stats", @@ -781,11 +776,11 @@ def test_availability_stats_invalid_period(self): ) def test_availability_stats_exceed_period(self): - """Get heatmap availability stats for more than max days""" + """Users cannot get availability statistics for periods longer than the maximum allowed days.""" data = { "user": self.user.external_id, - "from_date": datetime.now().strftime("%Y-%m-%d"), - "to_date": (datetime.now() + timedelta(days=40)).strftime("%Y-%m-%d"), + "from_date": datetime.now(UTC).strftime("%Y-%m-%d"), + "to_date": (datetime.now(UTC) + timedelta(days=40)).strftime("%Y-%m-%d"), } url = reverse( "slot-availability-stats", @@ -797,11 +792,11 @@ def test_availability_stats_exceed_period(self): ) def test_availability_stats_for_invalid_user(self): - """Get heatmap availability stats for an invalid user""" + """Users cannot get availability statistics for invalid users.""" data = { "user": "98c763ba-5bbb-44b9-ac03-56414fbb3021", - "from_date": datetime.now().strftime("%Y-%m-%d"), - "to_date": (datetime.now() + timedelta(days=10)).strftime("%Y-%m-%d"), + "from_date": datetime.now(UTC).strftime("%Y-%m-%d"), + "to_date": (datetime.now(UTC) + timedelta(days=10)).strftime("%Y-%m-%d"), } url = reverse( "slot-availability-stats", @@ -811,12 +806,12 @@ def test_availability_stats_for_invalid_user(self): self.assertContains(response, status_code=400, text="User does not exist") def test_availability_stats_for_non_schedulable_user(self): - """Get heatmap availability stats for a non-schedulable user""" + """Users cannot get availability statistics for non-schedulable users.""" non_schedulable_user = self.create_user() data = { "user": non_schedulable_user.external_id, - "from_date": datetime.now().strftime("%Y-%m-%d"), - "to_date": (datetime.now() + timedelta(days=10)).strftime("%Y-%m-%d"), + "from_date": datetime.now(UTC).strftime("%Y-%m-%d"), + "to_date": (datetime.now(UTC) + timedelta(days=10)).strftime("%Y-%m-%d"), } url = reverse( "slot-availability-stats", @@ -830,13 +825,13 @@ def test_availability_stats_for_non_schedulable_user(self): def test_availability_heatmap_slots_same_as_get_slots_for_day_without_exceptions( self, ): - """Get heatmap availability stats for 7 days and verify same slot stats as get_slots_for_day""" + """Availability heatmap slot counts should match individual day slot counts when there are no exceptions.""" # we don't want the slot that was created in setUp; create availability exception would've done this for us anyways. self.slot.delete() data = { "user": self.user.external_id, - "from_date": datetime.now().strftime("%Y-%m-%d"), - "to_date": (datetime.now() + timedelta(days=7)).strftime("%Y-%m-%d"), + "from_date": datetime.now(UTC).strftime("%Y-%m-%d"), + "to_date": (datetime.now(UTC) + timedelta(days=7)).strftime("%Y-%m-%d"), } availability_stats_url = reverse( "slot-availability-stats", @@ -861,29 +856,29 @@ def test_availability_heatmap_slots_same_as_get_slots_for_day_without_exceptions self.assertEqual(slot_stats["total_slots"], total_slots_for_day) def test_availability_heatmap_slots_same_as_get_slots_for_day_with_exceptions(self): - """Get heatmap availability stats for 7 days and verify same slot stats as get_slots_for_day""" + """Availability heatmap slot counts should match individual day slot counts even with exceptions.""" # we don't want the slot that was created in setUp; create availability exception would've done this for us anyways. self.slot.delete() AvailabilityException.objects.create( resource=self.resource, name="Test Exception", - valid_from=datetime.now(), - valid_to=datetime.now() + timedelta(days=1), + valid_from=datetime.now(UTC), + valid_to=datetime.now(UTC) + timedelta(days=1), start_time="00:00:00", end_time="23:59:59", ) AvailabilityException.objects.create( resource=self.resource, name="Test Exception", - valid_from=datetime.now() + timedelta(days=2), - valid_to=datetime.now() + timedelta(days=3), + valid_from=datetime.now(UTC) + timedelta(days=2), + valid_to=datetime.now(UTC) + timedelta(days=3), start_time="12:00:00", end_time="14:00:00", ) data = { "user": self.user.external_id, - "from_date": datetime.now().strftime("%Y-%m-%d"), - "to_date": (datetime.now() + timedelta(days=7)).strftime("%Y-%m-%d"), + "from_date": datetime.now(UTC).strftime("%Y-%m-%d"), + "to_date": (datetime.now(UTC) + timedelta(days=7)).strftime("%Y-%m-%d"), } availability_stats_url = reverse( "slot-availability-stats", @@ -921,8 +916,8 @@ def setUp(self): self.schedule = Schedule.objects.create( resource=self.resource, name="Test Schedule", - valid_from=datetime.now() - timedelta(days=30), - valid_to=datetime.now() + timedelta(days=30), + valid_from=datetime.now(UTC) - timedelta(days=30), + valid_to=datetime.now(UTC) + timedelta(days=30), ) self.availability = self.create_availability() self.slot = self.create_slot() @@ -947,8 +942,8 @@ def create_slot(self, **kwargs): data = { "resource": self.resource, "availability": self.availability, - "start_datetime": datetime.now() + timedelta(minutes=30), - "end_datetime": datetime.now() + timedelta(minutes=60), + "start_datetime": datetime.now(UTC) + timedelta(minutes=30), + "end_datetime": datetime.now(UTC) + timedelta(minutes=60), "allocated": 0, } data.update(kwargs) @@ -1006,25 +1001,28 @@ def create_availability(self, **kwargs): ) def test_get_slots_for_day(self): + """OTP authenticated users can get available slots for a specific day.""" url = reverse("otp-slots-get-slots-for-day") data = { "user": self.user.external_id, - "day": datetime.now().strftime("%Y-%m-%d"), + "day": datetime.now(UTC).strftime("%Y-%m-%d"), "facility": self.facility.external_id, } response = self.client.post(url, data, format="json") self.assertEqual(response.status_code, 200) def test_get_slots_for_day_without_facility(self): + """OTP authenticated users cannot get slots without specifying a facility.""" url = reverse("otp-slots-get-slots-for-day") data = { "user": self.user.external_id, - "day": datetime.now().strftime("%Y-%m-%d"), + "day": datetime.now(UTC).strftime("%Y-%m-%d"), } response = self.client.post(url, data, format="json") self.assertEqual(response.status_code, 400) def test_create_appointment(self): + """OTP authenticated users can create appointments.""" data = { "patient": self.patient.external_id, "reason_for_visit": "Test Reason", @@ -1037,6 +1035,7 @@ def test_create_appointment(self): self.assertContains(response, BookingStatusChoices.booked.value) def test_create_appointment_of_another_patient(self): + """OTP authenticated users cannot create appointments for other patients.""" other_patient = self.create_patient(phone_number="+917777777778") data = { "patient": other_patient.external_id, @@ -1050,6 +1049,7 @@ def test_create_appointment_of_another_patient(self): self.assertContains(response, "Patient not allowed", status_code=400) def test_cancel_appointment(self): + """OTP authenticated users can cancel their own appointments.""" booking = self.create_appointment() url = reverse("otp-slots-cancel-appointment") data = { @@ -1060,6 +1060,7 @@ def test_cancel_appointment(self): self.assertContains(response, BookingStatusChoices.cancelled.value) def test_cancel_appointment_of_another_patient(self): + """OTP authenticated users cannot cancel appointments of other patients.""" other_patient = self.create_patient(phone_number="+917777777778") booking = self.create_appointment(patient=other_patient) url = reverse("otp-slots-cancel-appointment") @@ -1071,6 +1072,7 @@ def test_cancel_appointment_of_another_patient(self): self.assertEqual(response.status_code, 404) def test_get_appointments(self): + """OTP authenticated users can get their own appointments.""" booking = self.create_appointment() url = reverse("otp-slots-get-appointments") response = self.client.get(url) @@ -1079,6 +1081,7 @@ def test_get_appointments(self): self.assertEqual(response.data["results"][0]["id"], booking.external_id) def test_get_appointments_of_another_patient(self): + """OTP authenticated users cannot get appointments of other patients.""" other_patient = self.create_patient(phone_number="+917777777778") self.create_appointment(patient=other_patient) url = reverse("otp-slots-get-appointments") diff --git a/care/emr/tests/test_schedule_api.py b/care/emr/tests/test_schedule_api.py index ae205f61ec..9d60df6db3 100644 --- a/care/emr/tests/test_schedule_api.py +++ b/care/emr/tests/test_schedule_api.py @@ -275,6 +275,7 @@ def test_delete_schedule_without_permissions(self): self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) def test_update_schedule_validity_with_booking_within_new_validity(self): + """Test that schedule validity can be updated when bookings fall within the new validity period.""" permissions = [ UserSchedulePermissions.can_write_user_schedule.name, UserSchedulePermissions.can_list_user_schedule.name, @@ -294,6 +295,7 @@ def test_update_schedule_validity_with_booking_within_new_validity(self): self.assertEqual(response.status_code, status.HTTP_200_OK) def test_update_schedule_validity_with_booking_outside_new_validity(self): + """Test that schedule validity cannot be updated when bookings fall outside the new validity period.""" permissions = [ UserSchedulePermissions.can_write_user_schedule.name, UserSchedulePermissions.can_list_user_schedule.name, @@ -780,7 +782,7 @@ def test_delete_availability_with_future_bookings(self): ) def test_create_availability_validate_availability(self): - """Test that creating availability with overlapping time ranges fails.""" + """Test validation rules for overlapping time ranges when creating availability slots.""" permissions = [UserSchedulePermissions.can_write_user_schedule.name] role = self.create_role_with_permissions(permissions) self.attach_role_facility_organization_user(self.organization, self.user, role) @@ -847,7 +849,7 @@ def test_create_availability_validate_availability(self): self.assertEqual(response.status_code, status.HTTP_200_OK) def test_create_availability_validate_slot_type(self): - """Test slot type validation rules for availability creation.""" + """Test validation rules for different slot types when creating availability slots.""" permissions = [UserSchedulePermissions.can_write_user_schedule.name] role = self.create_role_with_permissions(permissions) self.attach_role_facility_organization_user(self.organization, self.user, role) From ad23975ffbe82b166085251368b1119b1d49dc48 Mon Sep 17 00:00:00 2001 From: rithviknishad Date: Sat, 18 Jan 2025 23:35:17 +0530 Subject: [PATCH 21/24] more lint errors --- care/emr/tests/test_schedule_api.py | 56 +++++++++++++++-------------- 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/care/emr/tests/test_schedule_api.py b/care/emr/tests/test_schedule_api.py index 9d60df6db3..89a8ef5140 100644 --- a/care/emr/tests/test_schedule_api.py +++ b/care/emr/tests/test_schedule_api.py @@ -1,4 +1,4 @@ -from datetime import datetime, timedelta +from datetime import UTC, datetime, timedelta from django.urls import reverse from rest_framework import status @@ -33,8 +33,8 @@ def setUp(self): self.schedule = Schedule.objects.create( resource=self.resource, name="Test Schedule", - valid_from=datetime.now() - timedelta(days=30), - valid_to=datetime.now() + timedelta(days=30), + valid_from=datetime.now(UTC) - timedelta(days=30), + valid_to=datetime.now(UTC) + timedelta(days=30), ) self.availability = Availability.objects.create( schedule=self.schedule, @@ -77,8 +77,8 @@ def create_schedule(self, **kwargs): schedule = Schedule.objects.create( resource=self.resource, name=kwargs.get("name", "Test Schedule"), - valid_from=kwargs.get("valid_from", datetime.now()), - valid_to=kwargs.get("valid_to", datetime.now() + timedelta(days=30)), + valid_from=kwargs.get("valid_from", datetime.now(UTC)), + valid_to=kwargs.get("valid_to", datetime.now(UTC) + timedelta(days=30)), ) for availability in kwargs.get("availabilities", []): schedule.availabilities.create(**availability) @@ -88,8 +88,8 @@ def create_slot(self, **kwargs): data = { "resource": self.resource, "availability": self.availability, - "start_datetime": datetime.now() + timedelta(minutes=30), - "end_datetime": datetime.now() + timedelta(minutes=60), + "start_datetime": datetime.now(UTC) + timedelta(minutes=30), + "end_datetime": datetime.now(UTC) + timedelta(minutes=60), "allocated": 0, } data.update(kwargs) @@ -111,7 +111,7 @@ def create_booking(self, **kwargs): def generate_schedule_data(self, **kwargs): """Helper to generate valid schedule data.""" - valid_from = datetime.now() + valid_from = datetime.now(UTC) valid_to = valid_from + timedelta(days=30) return { @@ -177,7 +177,7 @@ def test_create_schedule_with_invalid_dates(self): role = self.create_role_with_permissions(permissions) self.attach_role_facility_organization_user(self.organization, self.user, role) - valid_from = datetime.now() + valid_from = datetime.now(UTC) valid_to = valid_from - timedelta(days=1) # Invalid: end before start schedule_data = self.generate_schedule_data( @@ -305,8 +305,8 @@ def test_update_schedule_validity_with_booking_outside_new_validity(self): self.create_booking( token_slot=self.create_slot( - start_datetime=datetime.now() + timedelta(days=4), - end_datetime=datetime.now() + timedelta(days=5), + start_datetime=datetime.now(UTC) + timedelta(days=4), + end_datetime=datetime.now(UTC) + timedelta(days=5), ) ) updated_data = { @@ -333,8 +333,8 @@ def test_delete_schedule_with_future_bookings(self): self.create_booking( token_slot=self.create_slot( - start_datetime=datetime.now() + timedelta(days=4), - end_datetime=datetime.now() + timedelta(days=5), + start_datetime=datetime.now(UTC) + timedelta(days=4), + end_datetime=datetime.now(UTC) + timedelta(days=5), ) ) delete_url = self._get_schedule_url(self.schedule.external_id) @@ -356,8 +356,8 @@ def test_delete_schedule_with_future_cancelled_bookings(self): self.create_booking( token_slot=self.create_slot( - start_datetime=datetime.now() + timedelta(days=4), - end_datetime=datetime.now() + timedelta(days=5), + start_datetime=datetime.now(UTC) + timedelta(days=4), + end_datetime=datetime.now(UTC) + timedelta(days=5), ), status=BookingStatusChoices.cancelled.value, ) @@ -396,8 +396,8 @@ def _get_exception_url(self, exception_id): def create_exception(self, **kwargs): from care.emr.models import AvailabilityException - valid_from = datetime.now().date() - valid_to = (datetime.now() + timedelta(days=1)).date() + valid_from = datetime.now(UTC).date() + valid_to = (datetime.now(UTC) + timedelta(days=1)).date() return AvailabilityException.objects.create( resource=self.resource, valid_from=valid_from, @@ -409,8 +409,8 @@ def create_exception(self, **kwargs): def generate_exception_data(self, **kwargs): """Helper to generate valid availability exception data.""" - valid_from = datetime.now().date() - valid_to = (datetime.now() + timedelta(days=1)).date() + valid_from = datetime.now(UTC).date() + valid_to = (datetime.now(UTC) + timedelta(days=1)).date() return { "user": str(self.user.external_id), @@ -553,8 +553,8 @@ def test_create_exception_with_bookings(self): schedule = Schedule.objects.create( resource=self.resource, name="Test Schedule", - valid_from=datetime.now() - timedelta(days=30), - valid_to=datetime.now() + timedelta(days=30), + valid_from=datetime.now(UTC) - timedelta(days=30), + valid_to=datetime.now(UTC) + timedelta(days=30), ) # Create an availability @@ -568,7 +568,7 @@ def test_create_exception_with_bookings(self): reason="Regular schedule", availability=[ { - "day_of_week": datetime.now().weekday(), + "day_of_week": datetime.now(UTC).weekday(), "start_time": "09:00:00", "end_time": "17:00:00", } @@ -576,7 +576,9 @@ def test_create_exception_with_bookings(self): ) # Create a slot for today - slot_start = datetime.now().replace(hour=10, minute=0, second=0, microsecond=0) + slot_start = datetime.now(UTC).replace( + hour=10, minute=0, second=0, microsecond=0 + ) slot = TokenSlot.objects.create( resource=self.resource, availability=availability, @@ -652,8 +654,8 @@ def create_schedule(self, **kwargs): schedule = Schedule.objects.create( resource=self.resource, name=kwargs.get("name", "Test Schedule"), - valid_from=kwargs.get("valid_from", datetime.now()), - valid_to=kwargs.get("valid_to", datetime.now() + timedelta(days=30)), + valid_from=kwargs.get("valid_from", datetime.now(UTC)), + valid_to=kwargs.get("valid_to", datetime.now(UTC) + timedelta(days=30)), ) for availability in kwargs.get("availabilities", []): schedule.availabilities.create(**availability) @@ -763,8 +765,8 @@ def test_delete_availability_with_future_bookings(self): token_slot = TokenSlot.objects.create( resource=self.resource, availability=availability, - start_datetime=datetime.now() + timedelta(days=4), - end_datetime=datetime.now() + timedelta(days=5), + start_datetime=datetime.now(UTC) + timedelta(days=4), + end_datetime=datetime.now(UTC) + timedelta(days=5), ) TokenBooking.objects.create( token_slot=token_slot, From c87f687b87c5dd790893e05d87f0091e7c0330a7 Mon Sep 17 00:00:00 2001 From: rithviknishad Date: Sat, 18 Jan 2025 23:37:06 +0530 Subject: [PATCH 22/24] sort imports --- care/emr/api/viewsets/scheduling/schedule.py | 2 +- care/emr/tests/test_schedule_api.py | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/care/emr/api/viewsets/scheduling/schedule.py b/care/emr/api/viewsets/scheduling/schedule.py index 29cf92d2c2..af5300cd0a 100644 --- a/care/emr/api/viewsets/scheduling/schedule.py +++ b/care/emr/api/viewsets/scheduling/schedule.py @@ -16,9 +16,9 @@ from care.emr.models.scheduling.schedule import Availability, Schedule from care.emr.resources.scheduling.schedule.spec import ( AvailabilityForScheduleSpec, + ScheduleCreateSpec, ScheduleReadSpec, ScheduleUpdateSpec, - ScheduleCreateSpec, ) from care.facility.models import Facility from care.security.authorization import AuthorizationController diff --git a/care/emr/tests/test_schedule_api.py b/care/emr/tests/test_schedule_api.py index 89a8ef5140..30a3bf4de4 100644 --- a/care/emr/tests/test_schedule_api.py +++ b/care/emr/tests/test_schedule_api.py @@ -291,7 +291,6 @@ def test_update_schedule_validity_with_booking_within_new_validity(self): } update_url = self._get_schedule_url(self.schedule.external_id) response = self.client.put(update_url, updated_data, format="json") - print(response.data) self.assertEqual(response.status_code, status.HTTP_200_OK) def test_update_schedule_validity_with_booking_outside_new_validity(self): From 3891e1c48cf7d77a00a6dbe66379f48acdd9e33e Mon Sep 17 00:00:00 2001 From: rithviknishad Date: Sat, 18 Jan 2025 23:38:54 +0530 Subject: [PATCH 23/24] remove comments --- care/emr/api/viewsets/scheduling/availability.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/care/emr/api/viewsets/scheduling/availability.py b/care/emr/api/viewsets/scheduling/availability.py index 2f85677ec0..2500f53ff3 100644 --- a/care/emr/api/viewsets/scheduling/availability.py +++ b/care/emr/api/viewsets/scheduling/availability.py @@ -65,8 +65,6 @@ def convert_availability_and_exceptions_to_slots(availabilities, exceptions, day time.fromisoformat(availability["availability"]["end_time"]), tzinfo=None, ) - # start_time = parse(availability["availability"]["start_time"]) - # end_time = parse(availability["availability"]["end_time"]) slot_size_in_minutes = availability["slot_size_in_minutes"] availability_id = availability["availability_id"] current_time = start_time From 67015c8f73a6dffb51c5ecf3ce79362fec694160 Mon Sep 17 00:00:00 2001 From: rithviknishad Date: Sat, 18 Jan 2025 23:39:42 +0530 Subject: [PATCH 24/24] remove unused import --- care/emr/api/viewsets/scheduling/availability.py | 1 - 1 file changed, 1 deletion(-) diff --git a/care/emr/api/viewsets/scheduling/availability.py b/care/emr/api/viewsets/scheduling/availability.py index 2500f53ff3..e6d19053d6 100644 --- a/care/emr/api/viewsets/scheduling/availability.py +++ b/care/emr/api/viewsets/scheduling/availability.py @@ -1,7 +1,6 @@ import datetime from datetime import time, timedelta -from dateutil.parser import parse from django.db import transaction from django.db.models import Sum from django.utils import timezone