Optimize error handling staffmember instance doesn't exist (#48)

* Optimize error handling staffmember instance doesn't exist

* Fixed tests for users

* Added test for error handling

Author: adamspd

appointment/services.py

@@ -9,25 +9,22 @@
 import datetime
 
 from django.contrib.auth import get_user_model
+from django.core.exceptions import ObjectDoesNotExist
 from django.shortcuts import render
 from django.urls import reverse
 from django.utils import timezone
-from django.utils.translation import gettext as _
-from django.utils.translation import gettext_lazy as _
+from django.utils.translation import gettext as _, gettext_lazy as _
 
-from appointment.forms import StaffDaysOffForm, StaffWorkingHoursForm, PersonalInformationForm, ServiceForm
+from appointment.forms import PersonalInformationForm, ServiceForm, StaffDaysOffForm, StaffWorkingHoursForm
 from appointment.settings import APPOINTMENT_PAYMENT_URL
-from appointment.utils.date_time import get_ar_end_time, convert_12_hour_time_to_24_hour_time, \
-    convert_str_to_time
-from appointment.utils.db_helpers import get_all_appointments, get_staff_member_appointment_list, \
-    get_appointment_by_id, get_all_staff_members, get_staff_member_from_user_id_or_logged_in, \
-    day_off_exists_for_date_range, working_hours_exist, Appointment, WorkingHours, Service, StaffMember, \
-    get_user_by_email, EmailVerificationCode, create_new_user
-from appointment.utils.db_helpers import get_working_hours_for_staff_and_day, get_appointments_for_date_and_time, \
-    get_times_from_config, exclude_booked_slots, calculate_slots, \
-    calculate_staff_slots, check_day_off_for_staff
+from appointment.utils.date_time import convert_12_hour_time_to_24_hour_time, convert_str_to_time, get_ar_end_time
+from appointment.utils.db_helpers import Appointment, EmailVerificationCode, Service, StaffMember, WorkingHours, \
+    calculate_slots, calculate_staff_slots, check_day_off_for_staff, create_new_user, day_off_exists_for_date_range, \
+    exclude_booked_slots, get_all_appointments, get_all_staff_members, get_appointment_by_id, \
+    get_appointments_for_date_and_time, get_staff_member_appointment_list, get_staff_member_from_user_id_or_logged_in, \
+    get_times_from_config, get_user_by_email, get_working_hours_for_staff_and_day, working_hours_exist
 from appointment.utils.error_codes import ErrorCode
-from appointment.utils.json_context import json_response, get_generic_context
+from appointment.utils.json_context import get_generic_context, json_response
 from appointment.utils.permissions import check_entity_ownership
 from appointment.utils.session import handle_email_change
 
@@ -38,11 +35,16 @@ def fetch_user_appointments(user):
     :param user: The user instance.
     :return: A list of appointments.
     """
-
     if user.is_superuser:
         return get_all_appointments()
-    staff_member_instance = user.staffmember
-    return get_staff_member_appointment_list(staff_member_instance)
+    try:
+        staff_member_instance = user.staffmember
+        return get_staff_member_appointment_list(staff_member_instance)
+    except ObjectDoesNotExist:
+        if user.is_staff:
+            return []
+
+    raise ValueError("User is not a staff member or a superuser")
 
 
 def prepare_appointment_display_data(user, appointment_id):
@@ -534,4 +536,3 @@ def handle_service_management_request(post_data, files_data=None, service_id=Non
             return None, False, get_error_message_in_form(form=form)
     except Exception as e:
         return None, False, str(e)
-

appointment/tests/test_services.py

@@ -14,7 +14,8 @@
     handle_service_management_request, handle_working_hours_form, handle_day_off_form
 from appointment.tests.base.base_test import BaseTest
 from appointment.utils.date_time import convert_str_to_time, get_ar_end_time
-from appointment.utils.db_helpers import get_user_model, DayOff, WorkingHours, Config, EmailVerificationCode, \
+from appointment.utils.db_helpers import Appointment, AppointmentRequest, DayOff, WorkingHours, Config, \
+    EmailVerificationCode, \
     StaffMember
 
 
@@ -27,6 +28,9 @@ def setUp(self):
         # Create some appointments for testing purposes
         self.appointment_for_user1 = self.create_appointment_for_user1()
         self.appointment_for_user2 = self.create_appointment_for_user2()
+        self.staff_user = self.create_user_(username='staff_user', password='test')
+        self.staff_user.is_staff = True
+        self.staff_user.save()
 
     def test_fetch_appointments_for_superuser(self):
         """Test that a superuser can fetch all appointments."""
@@ -55,12 +59,18 @@ def test_fetch_appointments_for_staff_member(self):
                          "Staff members should not see appointments not linked to them. User2's appointment was found.")
 
     def test_fetch_appointments_for_regular_user(self):
-        """Test that a regular user (not a staff member) cannot fetch appointments."""
-        # Fetching appointments for a regular user (client1 in this case) should raise an exception
-        with self.assertRaises(get_user_model().staffmember.RelatedObjectDoesNotExist,
-                               msg="Regular users without a staff member profile can't fetch appointments."):
+        """Test that a regular user (not a user with staff member instance or staff) cannot fetch appointments."""
+        # Fetching appointments for a regular user (client1 in this case) should raise ValueError
+        with self.assertRaises(ValueError,
+                               msg="Regular users without staff or superuser status should raise a ValueError."):
             fetch_user_appointments(self.client1)
 
+    def test_fetch_appointments_for_staff_user_without_staff_member_instance(self):
+        """Test that a staff user without a staff member instance gets an empty list of appointments."""
+        appointments = fetch_user_appointments(self.staff_user)
+        # Check that the returned value is an empty list
+        self.assertEqual(appointments, [], "Expected an empty list for a staff user without a staff member instance.")
+
 
 class PrepareAppointmentDisplayDataTests(BaseTest):
     """Test suite for the `prepare_appointment_display_data` service function."""

appointment/tests/test_views.py

@@ -3,14 +3,15 @@
 from datetime import date, timedelta, time
 
 from django.contrib import messages
+from django.contrib.messages import get_messages
 from django.contrib.messages.middleware import MessageMiddleware
 from django.contrib.sessions.middleware import SessionMiddleware
 from django.test import Client
 from django.test.client import RequestFactory
 from django.urls import reverse
 from django.utils.translation import gettext as _
 
-from appointment.models import AppointmentRequest, Appointment, EmailVerificationCode
+from appointment.models import AppointmentRequest, Appointment, EmailVerificationCode, StaffMember
 from appointment.tests.base.base_test import BaseTest
 from appointment.utils.db_helpers import WorkingHours
 from appointment.views import verify_user_and_login
@@ -28,6 +29,7 @@ def setUp(self):
                                     start_time=datetime.time(8, 0), end_time=datetime.time(12, 0))
         self.ar = self.create_appt_request_for_sm1()
         self.request = self.factory.get('/')
+        self.user1.is_staff = True
         self.request.user = self.user1
         middleware = SessionMiddleware(lambda req: None)
         middleware.process_request(self.request)
@@ -137,3 +139,27 @@ def test_default_thank_you(self):
         response = self.client.get(url)
         self.assertEqual(response.status_code, 200)
         self.assertIn(appointment.get_service_name(), str(response.content))
+
+    def test_staff_user_without_staff_member_instance(self):
+        """Test that a staff user without a staff member instance receives an appropriate error message."""
+        self.user1.is_staff = True
+
+        # Delete any AppointmentRequests and Appointments linked to the StaffMember instance of self.user1
+        AppointmentRequest.objects.filter(staff_member__user=self.user1).delete()
+        Appointment.objects.filter(appointment_request__staff_member__user=self.user1).delete()
+
+        # Now safely delete the StaffMember instance
+        StaffMember.objects.filter(user=self.user1).delete()
+
+        self.user1.save()  # Save the user to the database after updating
+        self.client.force_login(self.user1)  # Log in as self.user1
+
+        url = reverse('appointment:get_user_appointments')
+        response = self.client.get(url)
+
+        message_list = list(get_messages(response.wsgi_request))
+        self.assertTrue(any(
+            message.message == "User doesn't have a staff member instance. Please contact the administrator." for
+            message in message_list),
+                        "Expected error message not found in messages.")
+

appointment/views_admin.py

@@ -48,6 +48,9 @@ def get_user_appointments(request, response_type='html'):
         'appointments': json.dumps(appointments_json),
     }
     context = get_generic_context_with_extra(request=request, extra=extra_context)
+    # if appointment is empty and user doesn't have a staff-member instance, put a message
+    if not appointments and not StaffMember.objects.filter(user=request.user).exists() and request.user.is_staff:
+        messages.error(request, _("User doesn't have a staff member instance. Please contact the administrator."))
     return render(request, 'administration/staff_index.html', context)