Merge pull request #241 from adamspd/237-staff-shows-none-none-not-showing-email

Enhance staff member name retrieval with additional fallbacks

Author: adamspd

appointment/models.py

@@ -211,13 +211,14 @@ def works_on_both_weekends_day(self):
         return self.work_on_saturday and self.work_on_sunday
 
     def get_staff_member_name(self):
-        if hasattr(self.user, 'get_full_name') and callable(getattr(self.user, 'get_full_name')):
-            name = self.user.get_full_name()
-        else:
-            name = f"{self.user.first_name} {self.user.last_name}".strip()
-        if not name:
-            name = self.user.username
-        return name
+        name_options = [
+            getattr(self.user, 'get_full_name', lambda: '')(),
+            f"{self.user.first_name} {self.user.last_name}",
+            self.user.username,
+            self.user.email,
+            f"Staff Member {self.id}"
+        ]
+        return next((name.strip() for name in name_options if name.strip()), "Unknown")
 
     def get_staff_member_first_name(self):
         return self.user.first_name

appointment/tests/base/base_test.py

@@ -19,14 +19,16 @@ class BaseTest(TestCase, UserMixin, StaffMemberMixin, ServiceMixin, AppointmentR
     users = None
 
     USER_SPECS = {
-        'staff1': {"first_name": "Daniel", "email": "[email protected]",
+        'staff1': {"first_name": "Daniel", "last_name": "Jackson", "email": "[email protected]",
                    "username": "daniel.jackson"},
-        'staff2': {"first_name": "Samantha", "email": "[email protected]",
+        'staff2': {"first_name": "Samantha", "last_name": "Carter", "email": "[email protected]",
                    "username": "samantha.carter"},
-        'client1': {"first_name": "Georges", "email": "[email protected]",
-                    "username": "georges.hammond"},
-        'client2': {"first_name": "Tealc", "email": "[email protected]", "username": "tealc.kree"},
-        'superuser': {"first_name": "Jack", "email": "[email protected]", "username": "jack.o.neill"},
+        'client1': {"first_name": "Georges", "last_name": "Hammond",
+                    "email": "[email protected]", "username": "georges.hammond"},
+        'client2': {"first_name": "Tealc", "last_name": "Kree", "email": "[email protected]",
+                    "username": "tealc.kree"},
+        'superuser': {"first_name": "Jack", "last_name": "O'Neill", "email": "[email protected]",
+                      "username": "jack.o.neill"},
     }
 
     @classmethod

appointment/tests/mixins/base_mixin.py

@@ -11,13 +11,15 @@ def __init__(self):
         pass
 
     @classmethod
-    def create_user_(cls, first_name="Janet", email="[email protected]", username="janet.fraiser",
+    def create_user_(cls, first_name="Janet", last_name="Fraiser", email="[email protected]",
+                     username="janet.fraiser",
                      password="G0a'uld$Emp1re"):
         return get_user_model().objects.create_user(
-            first_name=first_name,
-            email=email,
-            username=username,
-            password=password
+                first_name=first_name,
+                last_name=last_name,
+                email=email,
+                username=username,
+                password=password
         )
 
 
@@ -29,10 +31,10 @@ def __init__(self):
     def create_service_(cls, name="Quantum Mirror Assessment", duration=timedelta(hours=1), price=50000,
                         description="Assess the Quantum Mirror"):
         return Service.objects.create(
-            name=name,
-            duration=duration,
-            price=price,
-            description=description
+                name=name,
+                duration=duration,
+                price=price,
+                description=description
         )
 
 
@@ -55,11 +57,11 @@ def __init__(self):
     def create_appointment_request_(cls, service, staff_member, date_=date.today(), start_time=time(9, 0),
                                     end_time=time(10, 0)):
         return AppointmentRequest.objects.create(
-            date=date_,
-            start_time=start_time,
-            end_time=end_time,
-            service=service,
-            staff_member=staff_member
+                date=date_,
+                start_time=start_time,
+                end_time=end_time,
+                service=service,
+                staff_member=staff_member
         )
 
     @classmethod
@@ -75,10 +77,10 @@ def __init__(self):
     def create_appointment_(cls, user, appointment_request, phone="+12392340543",
                             address="Stargate Command, Cheyenne Mountain Complex, Colorado Springs, CO"):
         return Appointment.objects.create(
-            client=user,
-            appointment_request=appointment_request,
-            phone=phone,
-            address=address
+                client=user,
+                appointment_request=appointment_request,
+                phone=phone,
+                address=address
         )
 
     @classmethod
@@ -94,12 +96,12 @@ def __init__(self):
     def create_reschedule_history_(cls, appointment_request, date_, start_time, end_time, staff_member,
                                    reason_for_rescheduling="Zat'nik'tel Discharge"):
         return AppointmentRescheduleHistory.objects.create(
-            appointment_request=appointment_request,
-            date=date_,
-            start_time=start_time,
-            end_time=end_time,
-            staff_member=staff_member,
-            reason_for_rescheduling=reason_for_rescheduling
+                appointment_request=appointment_request,
+                date=date_,
+                start_time=start_time,
+                end_time=end_time,
+                staff_member=staff_member,
+                reason_for_rescheduling=reason_for_rescheduling
         )
 
 
@@ -111,8 +113,8 @@ def __init__(self):
     def create_config_(cls, lead_time=time(9, 0), finish_time=time(17, 0), slot_duration=30,
                        appointment_buffer_time=0):
         return Config.objects.create(
-            lead_time=lead_time,
-            finish_time=finish_time,
-            slot_duration=slot_duration,
-            appointment_buffer_time=appointment_buffer_time
+                lead_time=lead_time,
+                finish_time=finish_time,
+                slot_duration=slot_duration,
+                appointment_buffer_time=appointment_buffer_time
         )

appointment/tests/models/test_appointment.py

@@ -75,7 +75,7 @@ def test_appointment_getters(self):
     def test_conversion_to_dict(self):
         response = {
             'id': 1,
-            'client_name': self.client_.first_name,
+            'client_name': self.client_.first_name + ' ' + self.client_.last_name,
             'client_email': self.client_.email,
             'start_time': '1900-01-01 09:00',
             'end_time': '1900-01-01 10:00',

appointment/tests/models/test_staff_member.py

@@ -35,8 +35,8 @@ def test_default_attributes_on_creation(self):
 
     def test_creation_without_service(self):
         """A staff member can be created without a service."""
-        new_staff = self.create_user_(
-            first_name="Jonas", email="[email protected]", username="jonas.quinn")
+        new_staff = self.create_user_(first_name="Jonas", last_name="Quinn", email="[email protected]",
+                                      username="jonas.quinn")
         new_staff_member = StaffMember.objects.create(user=new_staff)
         self.assertIsNotNone(new_staff_member)
         self.assertEqual(new_staff_member.services_offered.count(), 0)
@@ -46,6 +46,47 @@ def test_creation_fails_without_user(self):
         with self.assertRaises(IntegrityError):
             StaffMember.objects.create()
 
+    def test_get_staff_member_name_with_email(self):
+        # Simulate create a staff member with only an email and username
+        # (in my case, username is mandatory, but should work with email as well)
+        email_only_user = self.create_user_(
+                first_name="",
+                last_name="",
+                email="[email protected]",
+                username="[email protected]"
+        )
+        email_only_staff = StaffMember.objects.create(user=email_only_user)
+
+        # Test that the email is returned when no other name information is available
+        self.assertEqual(email_only_staff.get_staff_member_name(), "[email protected]")
+
+    def test_get_staff_member_name_priority(self):
+        # Create a staff member with all name fields filled
+        full_user = self.create_user_(
+                first_name="Rodney",
+                last_name="McKay",
+                email="[email protected]",
+                username="rodney.mckay"
+        )
+        full_staff = StaffMember.objects.create(user=full_user)
+
+        # Test that the full name is returned when available
+        self.assertEqual(full_staff.get_staff_member_name(), "Rodney McKay")
+
+        # Modify the user to test different scenarios
+        full_user.first_name = ""
+        full_user.last_name = ""
+        full_user.save()
+
+        # Test that the username is returned when full name is not available
+        self.assertEqual(full_staff.get_staff_member_name(), "rodney.mckay")
+
+        full_user.username = ""
+        full_user.save()
+
+        # Test that the email is returned when username and full name are not available
+        self.assertEqual(full_staff.get_staff_member_name(), "[email protected]")
+
 
 class StaffMemberServiceTests(BaseTest):
     @classmethod
@@ -84,14 +125,14 @@ def test_services_offered(self):
 
     def test_staff_member_with_non_existent_service(self):
         """A staff member cannot offer a non-existent service."""
-        new_staff = self.create_user_(
-            first_name="Vala", email="[email protected]", username="vala.mal-doran")
+        new_staff = self.create_user_(first_name="Vala", last_name="Mal Doran",
+                                      email="[email protected]", username="vala.mal-doran")
         new_staff_member = StaffMember.objects.create(user=new_staff)
 
         # Trying to add a non-existent service to the staff member's services_offered
         with self.assertRaises(ValueError):
             new_staff_member.services_offered.add(
-                Service(id=9999, name="Prometheus Acquisition", duration=timedelta(hours=2), price=200))
+                    Service(id=9999, name="Prometheus Acquisition", duration=timedelta(hours=2), price=200))
 
 
 class StaffMemberWorkingTimeTests(BaseTest):
@@ -212,10 +253,10 @@ def test_get_staff_member_first_name(self):
     def test_config_values_takes_over_when_sm_values_null(self):
         """When some values are null in the StaffMember, the Config values should be used."""
         config = Config.objects.create(
-            lead_time=datetime.time(9, 34),
-            finish_time=datetime.time(17, 11),
-            slot_duration=37,
-            appointment_buffer_time=16
+                lead_time=datetime.time(9, 34),
+                finish_time=datetime.time(17, 11),
+                slot_duration=37,
+                appointment_buffer_time=16
         )
         # Checking that the StaffMember's values are None
         self.assertIsNone(self.staff_member.slot_duration)