Merge pull request #867 from City-of-Helsinki/feature/LINK-1926_invitation-sending

LINK-1926 | send invitation is is_substitute_user is changed

Author: jorilindell

events/api.py

@@ -1503,21 +1503,14 @@ def are_only_admin_visible_fields_allowed(self, obj):
     def _create_or_update_registration_user_accesses(
         registration, registration_user_accesses
     ):
-        users_to_send_invitations = []
-
         for data in registration_user_accesses:
             if current_obj := data.pop("id", None):
                 current_id = current_obj.id
-                current_email = current_obj.email
             else:
                 current_id = None
-                current_email = None
 
             try:
-                (
-                    user_instance,
-                    created,
-                ) = RegistrationUserAccess.objects.update_or_create(
+                RegistrationUserAccess.objects.update_or_create(
                     id=current_id,
                     registration=registration,
                     defaults=data,
@@ -1541,16 +1534,6 @@ def _create_or_update_registration_user_accesses(
                 else:
                     raise
 
-            # Send invitation to new registration users or if the email is updated to an
-            # existing registration user
-            if created or (current_email != user_instance.email):
-                users_to_send_invitations.append(user_instance)
-
-        # Send invitations to registration users after updating all registration users to
-        # make sure that all instances are created or updated successfully
-        for user_instance in users_to_send_invitations:
-            user_instance.send_invitation()
-
     @staticmethod
     def _create_or_update_registration_price_groups(
         registration, registration_price_groups

registrations/admin.py

@@ -89,25 +89,6 @@ def get_formsets_with_inlines(self, request, obj=None):
             ):
                 yield inline.get_formset(request, obj), inline
 
-    def save_related(self, request, form, formsets, change):
-        for formset in formsets:
-            if formset.model == RegistrationUserAccess:
-                formset.save(commit=False)
-
-                for added_registration_user_accesses in formset.new_objects:
-                    # Send invitation email if new registration user accesses is added
-                    added_registration_user_accesses.send_invitation()
-
-                for [
-                    changed_registration_user_access,
-                    changed_fields,
-                ] in formset.changed_objects:
-                    # Send invitation email if email address is changed
-                    if "email" in changed_fields:
-                        changed_registration_user_access.send_invitation()
-
-        super(RegistrationAdmin, self).save_related(request, form, formsets, change)
-
     def get_readonly_fields(self, request, obj=None):
         if obj:
             return ["id", "event"]

registrations/models.py

@@ -864,6 +864,22 @@ def send_invitation(self):
             html_message=rendered_body,
         )
 
+    @transaction.atomic
+    def save(self, *args, **kwargs):
+        if self.pk:
+            old_instance = RegistrationUserAccess.objects.filter(pk=self.pk).first()
+        else:
+            old_instance = None
+
+        super().save(*args, **kwargs)
+
+        if (
+            not old_instance
+            or old_instance.email != self.email
+            or old_instance.is_substitute_user != self.is_substitute_user
+        ):
+            self.send_invitation()
+
     class Meta:
         unique_together = ("email", "registration")
 

registrations/tests/test_admin.py

@@ -3,6 +3,7 @@
 from django.contrib import admin
 from django.contrib.admin.sites import AdminSite
 from django.contrib.auth import get_user_model
+from django.core import mail
 from django.test import RequestFactory, TestCase
 from django.utils import translation
 from rest_framework import status
@@ -262,6 +263,7 @@ def test_send_invitation_email_when_registration_user_access_is_updated(self):
             registration_user_access = RegistrationUserAccess.objects.create(
                 registration=self.registration, email=EMAIL
             )
+            mail.outbox.clear()
             self.registration.event.name = EVENT_NAME
             self.registration.event.save()
 

registrations/tests/test_registration_put.py

@@ -325,6 +325,7 @@ def test_send_email_to_new_registration_user_access(
     RegistrationUserAccessFactory(registration=registration, email="delete1@email.com")
     RegistrationUserAccessFactory(registration=registration, email="delete2@email.com")
     assert len(registration.registration_user_accesses.all()) == 2
+    mail.outbox.clear()
 
     registration_data = {
         "event": {"@id": get_event_url(registration.event.id)},
@@ -366,6 +367,7 @@ def test_email_is_not_sent_if_registration_user_access_email_is_not_updated(
         email=user_email,
         is_substitute_user=is_substitute_user,
     )
+    mail.outbox.clear()
 
     registration_data = {
         "event": {"@id": get_event_url(registration.event.id)},
@@ -397,29 +399,68 @@ def test_email_is_sent_if_registration_user_access_email_is_updated(
         email=user_email,
         is_substitute_user=is_substitute_user,
     )
+    mail.outbox.clear()
 
-    registration_data = {
-        "event": {"@id": get_event_url(registration.event.id)},
-        "registration_user_accesses": [
-            {
-                "id": registration_user_access.id,
-                "email": edited_user_email,
-                "is_substitute_user": is_substitute_user,
-            }
-        ],
-    }
+    with translation.override("fi"):
+        registration.event.type_id = Event.TypeId.GENERAL
+        registration.event.name = event_name
+        registration.event.save()
+
+        registration_data = {
+            "event": {"@id": get_event_url(registration.event.id)},
+            "registration_user_accesses": [
+                {
+                    "id": registration_user_access.id,
+                    "email": edited_user_email,
+                    "is_substitute_user": is_substitute_user,
+                }
+            ],
+        }
+
+        assert_update_registration(user_api_client, registration.id, registration_data)
+        #  assert that the email after update was sent
+        registration_user_access.refresh_from_db()
+        assert_invitation_email_is_sent(
+            edited_user_email, event_name, registration_user_access
+        )
+
+
+@pytest.mark.parametrize("is_substitute_user", [False, True])
+@pytest.mark.django_db
+def test_email_is_sent_if_is_substitute_value_of_registration_user_access_is_updated(
+    registration, user_api_client, is_substitute_user
+):
+    user_email = hel_email
+
+    registration_user_access = RegistrationUserAccessFactory(
+        registration=registration,
+        email=user_email,
+        is_substitute_user=is_substitute_user,
+    )
+    mail.outbox.clear()
 
     with translation.override("fi"):
         registration.event.type_id = Event.TypeId.GENERAL
         registration.event.name = event_name
         registration.event.save()
 
+        registration_data = {
+            "event": {"@id": get_event_url(registration.event.id)},
+            "registration_user_accesses": [
+                {
+                    "id": registration_user_access.id,
+                    "email": hel_email,
+                    "is_substitute_user": False if is_substitute_user else True,
+                }
+            ],
+        }
+
         assert_update_registration(user_api_client, registration.id, registration_data)
 
-        #  assert that the email was sent
+        #  assert that the email after update was sent
         registration_user_access.refresh_from_db()
         assert_invitation_email_is_sent(
-            edited_user_email, event_name, registration_user_access
+            user_email, event_name, registration_user_access
         )
 
 

registrations/tests/utils.py

@@ -39,36 +39,33 @@ def assert_invitation_email_is_sent(
     expected_subject: Optional[str] = None,
     expected_body: Optional[str] = None,
 ) -> None:
-    assert mail.outbox[0].to[0] == email
+    first_email = mail.outbox[0]
+    assert first_email.to[0] == email
 
     ui_locale = ui_locale or "fi"
-    email_body_string = str(mail.outbox[0].alternatives[0])
+    email_body_string = str(first_email.alternatives[0])
 
     if registration_user_access.is_substitute_user:
         expected_subject = expected_subject or "Oikeudet myönnetty ilmoittautumiseen"
-        assert mail.outbox[0].subject.startswith(expected_subject)
-
         expected_body = expected_body or (
             f"Sähköpostiosoitteelle <strong>{email}</strong> on myönnetty sijaisen käyttöoikeudet "
             f"tapahtuman <strong>{event_name}</strong> ilmoittautumiselle."
         )
-        assert expected_body in email_body_string
 
         service_base_url = settings.LINKED_EVENTS_UI_URL
         registration_term = "registrations"
     else:
         expected_subject = expected_subject or "Oikeudet myönnetty osallistujalistaan"
-        assert mail.outbox[0].subject.startswith(expected_subject)
-
         expected_body = expected_body or (
             f"Sähköpostiosoitteelle <strong>{email}</strong> on myönnetty oikeudet lukea "
             f"tapahtuman <strong>{event_name}</strong> osallistujalista."
         )
-        assert expected_body in email_body_string
-
         service_base_url = settings.LINKED_REGISTRATIONS_UI_URL
         registration_term = "registration"
 
+    assert first_email.subject.startswith(expected_subject)
+    assert expected_body in email_body_string
+
     participant_list_url = (
         f"{service_base_url}/{ui_locale}/{registration_term}/"
         f"{registration_user_access.registration_id}/attendance-list/"
@@ -128,22 +125,22 @@ def create_user_by_role(
 
     user_role_mapping = {
         "superuser": lambda usr: None,
-        "admin": lambda usr: usr.admin_organizations.add(organization)
-        if organization
-        else None,
-        "registration_admin": lambda usr: usr.registration_admin_organizations.add(
-            organization
-        )
-        if organization
-        else None,
-        "financial_admin": lambda usr: usr.financial_admin_organizations.add(
-            organization
-        )
-        if organization
-        else None,
-        "regular_user": lambda usr: usr.organization_memberships.add(organization)
-        if organization
-        else None,
+        "admin": lambda usr: (
+            usr.admin_organizations.add(organization) if organization else None
+        ),
+        "registration_admin": lambda usr: (
+            usr.registration_admin_organizations.add(organization)
+            if organization
+            else None
+        ),
+        "financial_admin": lambda usr: (
+            usr.financial_admin_organizations.add(organization)
+            if organization
+            else None
+        ),
+        "regular_user": lambda usr: (
+            usr.organization_memberships.add(organization) if organization else None
+        ),
     }
     if isinstance(additional_roles, dict):
         user_role_mapping.update(additional_roles)