Refactor get_officer_emails

backend/clubs/models.py

@@ -620,27 +620,23 @@ def get_officer_emails(self):
         """
         emails = []
 
-        # add club contact email if valid
-        try:
-            validate_email(self.email)
-            emails.append(self.email)
-        except ValidationError:
-            pass
-
-        # add email for all officers and above
-        for user in self.membership_set.filter(
-            role__lte=Membership.ROLE_OFFICER, active=True
-        ):
-            emails.append(user.person.email)
-
-        # remove empty emails
-        emails = [email.strip() for email in emails]
-        emails = [email for email in emails if email]
-
-        # remove duplicate emails
-        emails = list(sorted(set(emails)))
+        # Add club contact email if valid
+        if self.email:
+            try:
+                validate_email(self.email)
+                emails.append(self.email)
+            except ValidationError:
+                pass
+
+        # Add email for all active officers and above
+        emails.extend(
+            self.membership_set.filter(
+                role__lte=Membership.ROLE_OFFICER, active=True
+            ).values_list("person__email", flat=True)
+        )
 
-        return emails
+        # Remove whitespace, empty emails, and duplicates, then sort
+        return sorted(set(email.strip() for email in emails if email.strip()))
 
     def send_confirmation_email(self, request=None):
         """

backend/tests/clubs/test_models.py

@@ -63,6 +63,60 @@ def test_parent_children(self):
         self.assertEqual(self.club2.parent_orgs.first(), self.club1)
         self.assertEqual(self.club1.children_orgs.first(), self.club2)
 
+    def test_get_officer_emails(self):
+        # Create test users with various email formats
+        user1 = get_user_model().objects.create_user(
+            "user1", "[email protected]", "password"
+        )
+        user2 = get_user_model().objects.create_user(
+            "user2", "   [email protected]  ", "password"
+        )  # whitespace
+        user3 = get_user_model().objects.create_user(
+            "user3", "", "password"
+        )  # empty email
+        user4 = get_user_model().objects.create_user(
+            "user4", "[email protected]", "password"
+        )
+
+        # Create memberships for the test users
+        Membership.objects.create(
+            person=user1, club=self.club1, role=Membership.ROLE_OFFICER
+        )
+        Membership.objects.create(
+            person=user2, club=self.club1, role=Membership.ROLE_OWNER
+        )
+        Membership.objects.create(
+            person=user3, club=self.club1, role=Membership.ROLE_OFFICER
+        )
+        Membership.objects.create(
+            person=user4, club=self.club1, role=Membership.ROLE_OFFICER, active=False
+        )  # alumni
+
+        # Test with valid club email
+        self.club1.email = "[email protected]"
+        self.club1.save()
+
+        officer_emails = self.club1.get_officer_emails()
+        expected_emails = ["[email protected]", "[email protected]", "[email protected]"]
+        self.assertEqual(officer_emails, expected_emails)
+
+        # Ensure alumni are not included
+        self.assertNotIn("[email protected]", officer_emails)
+
+        # Test with invalid club email
+        self.club1.email = "invalid-email"
+        self.club1.save()
+
+        officer_emails = self.club1.get_officer_emails()
+        expected_emails = ["[email protected]", "[email protected]"]
+        self.assertEqual(officer_emails, expected_emails)
+
+        # Test with empty club email
+        self.club1.email = ""
+        self.club1.save()
+        officer_emails = self.club1.get_officer_emails()
+        self.assertEqual(officer_emails, expected_emails)
+
 
 class ProfileTestCase(TestCase):
     def test_profile_creation(self):