fix(eap): update contents on email notifications

- Update export and diff file generation logic
- Add validation check for time value
- Update on utils email context
- update test cases for notifications
- add ns contact title on email and dev preview

Author: susilnem

eap/dev_views.py

@@ -56,6 +56,7 @@ def get(self, request):
                 "disaster_type": "Flood",
                 "total_budget": "250,000 CHF",
                 "ns_contact_name": "Test Ns Contact name",
+                "ns_contact_title": "Programme Manager",
                 "ns_contact_email": "test.Ns@gmail.com",
                 "ns_contact_phone": "+977-9800000000",
             },

eap/serializers.py

@@ -1,7 +1,7 @@
 import typing
 from datetime import timedelta
 
-from celery import group
+from celery import chain, group
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.db import transaction
@@ -357,7 +357,7 @@ class OperationActivitySerializer(
     )
     timeframe_display = serializers.CharField(source="get_timeframe_display", read_only=True)
     time_value = serializers.ListField(
-        child=serializers.IntegerField(),
+        child=serializers.IntegerField(required=True),
         required=True,
     )
 
@@ -371,6 +371,9 @@ def validate(self, validated_data: dict[str, typing.Any]) -> dict[str, typing.An
         timeframe = validated_data["timeframe"]
         time_value = validated_data["time_value"]
 
+        if time_value is None or len(time_value) == 0:
+            raise serializers.ValidationError({"time_value": gettext("time_value is required and cannot be empty.")})
+
         allowed_values = ALLOWED_MAP_TIMEFRAMES_VALUE.get(timeframe, [])
         invalid_values = [value for value in time_value if value not in allowed_values]
 
@@ -880,7 +883,7 @@ def create(self, validated_data: dict[str, typing.Any]):
 class EAPStatusSerializer(BaseEAPSerializer):
     status_display = serializers.CharField(source="get_status_display", read_only=True)
     # NOTE: Only required when changing status to NS Addressing Comments
-    review_checklist_file = serializers.FileField(required=False)
+    review_checklist_file = serializers.FileField(required=False, write_only=True)
 
     class Meta:
         model = EAPRegistration
@@ -924,23 +927,9 @@ def _validate_status(self, validated_data: dict[str, typing.Any]) -> dict[str, t
             if self.instance.get_eap_type_enum == EAPType.SIMPLIFIED_EAP:
                 self.instance.latest_simplified_eap.is_locked = True
                 self.instance.latest_simplified_eap.save(update_fields=["is_locked"])
-                # NOTE: Generating export PDF asynchronously
-                transaction.on_commit(
-                    lambda: generate_export_eap_pdf.delay(
-                        eap_registration_id=self.instance.id,
-                        version=self.instance.latest_simplified_eap.version,
-                    )
-                )
             else:
                 self.instance.latest_full_eap.is_locked = True
                 self.instance.latest_full_eap.save(update_fields=["is_locked"])
-                # NOTE: Generate export PDF asynchronously
-                transaction.on_commit(
-                    lambda: generate_export_eap_pdf.delay(
-                        eap_registration_id=self.instance.id,
-                        version=self.instance.latest_full_eap.version,
-                    )
-                )
 
         # NOTE: IFRC Admins should be able to transition from TECHNICALLY_VALIDATED
         # to NS_ADDRESSING_COMMENTS to allow NS users to update their EAP changes after validated budget has been set.
@@ -1029,21 +1018,6 @@ def _validate_status(self, validated_data: dict[str, typing.Any]) -> dict[str, t
                 # Lock the latest eap
                 self.instance.latest_simplified_eap.is_locked = True
                 self.instance.latest_simplified_eap.save(update_fields=["is_locked"])
-
-                # Generating PDFs asynchronously
-                transaction.on_commit(
-                    lambda: group(
-                        generate_export_eap_pdf.s(
-                            eap_registration_id=self.instance.id,
-                            version=self.instance.latest_simplified_eap.version,
-                        ),
-                        generate_export_diff_pdf.s(
-                            eap_registration_id=self.instance.id,
-                            version=self.instance.latest_simplified_eap.version,
-                        ),
-                    ).apply_async()
-                )
-
             else:
                 if self.instance.latest_full_eap.is_locked:
                     raise serializers.ValidationError(
@@ -1065,20 +1039,6 @@ def _validate_status(self, validated_data: dict[str, typing.Any]) -> dict[str, t
                 self.instance.latest_full_eap.is_locked = True
                 self.instance.latest_full_eap.save(update_fields=["is_locked"])
 
-                # Generating PDFs asynchronously
-                transaction.on_commit(
-                    lambda: group(
-                        generate_export_eap_pdf.s(
-                            eap_registration_id=self.instance.id,
-                            version=self.instance.latest_full_eap.version,
-                        ),
-                        generate_export_diff_pdf.s(
-                            eap_registration_id=self.instance.id,
-                            version=self.instance.latest_full_eap.version,
-                        ),
-                    ).apply_async()
-                )
-
         elif (current_status, new_status) == (
             EAPRegistration.Status.TECHNICALLY_VALIDATED,
             EAPRegistration.Status.PENDING_PFA,
@@ -1102,10 +1062,6 @@ def _validate_status(self, validated_data: dict[str, typing.Any]) -> dict[str, t
                 ]
             )
 
-            # Generate summary eap for full eap
-            if self.instance.get_eap_type_enum == EAPType.FULL_EAP:
-                transaction.on_commit(lambda: generate_eap_summary_pdf.delay(self.instance.id))
-
         elif (current_status, new_status) == (
             EAPRegistration.Status.PENDING_PFA,
             EAPRegistration.Status.APPROVED,
@@ -1152,7 +1108,18 @@ def update(self, instance: EAPRegistration, validated_data: dict[str, typing.Any
             EAPRegistration.Status.UNDER_DEVELOPMENT,
             EAPRegistration.Status.UNDER_REVIEW,
         ):
-            transaction.on_commit(lambda: send_new_eap_submission_email.delay(eap_registration_id))
+            # NOTE: Generating export pdf and sending email to IFRC at the first submission to under review.
+            latest_eap = (
+                instance.latest_simplified_eap
+                if instance.get_eap_type_enum == EAPType.SIMPLIFIED_EAP
+                else instance.latest_full_eap
+            )
+            transaction.on_commit(
+                lambda: chain(
+                    generate_export_eap_pdf.s(eap_registration_id, latest_eap.version),
+                    send_new_eap_submission_email.si(eap_registration_id),
+                ).apply_async()
+            )
 
         elif (old_status, new_status) in [
             (EAPRegistration.Status.UNDER_REVIEW, EAPRegistration.Status.NS_ADDRESSING_COMMENTS),
@@ -1205,7 +1172,18 @@ def update(self, instance: EAPRegistration, validated_data: dict[str, typing.Any
             EAPRegistration.Status.NS_ADDRESSING_COMMENTS,
             EAPRegistration.Status.UNDER_REVIEW,
         ):
-            transaction.on_commit(lambda: send_eap_resubmission_email.delay(eap_registration_id))
+            # NOTE: Generating diff pdf and sending email to IFRC after NS resubmission.
+            latest_eap = (
+                instance.latest_simplified_eap
+                if instance.get_eap_type_enum == EAPType.SIMPLIFIED_EAP
+                else instance.latest_full_eap
+            )
+            transaction.on_commit(
+                lambda: chain(
+                    generate_export_diff_pdf.s(eap_registration_id, latest_eap.version),
+                    send_eap_resubmission_email.si(eap_registration_id),
+                ).apply_async()
+            )
 
         elif (old_status, new_status) == (
             EAPRegistration.Status.UNDER_REVIEW,
@@ -1217,7 +1195,23 @@ def update(self, instance: EAPRegistration, validated_data: dict[str, typing.Any
             EAPRegistration.Status.TECHNICALLY_VALIDATED,
             EAPRegistration.Status.PENDING_PFA,
         ):
-            transaction.on_commit(lambda: send_pending_pfa_email.delay(eap_registration_id))
+            # NOTE: Generating diff pdf and summary pdf (for full eap) and sending email to PFA after technical validation.
+            is_full_eap = instance.get_eap_type_enum == EAPType.FULL_EAP
+            version = instance.latest_simplified_eap.version if not is_full_eap else instance.latest_full_eap.version
+
+            tasks = [
+                generate_export_diff_pdf.s(eap_registration_id, version),
+            ]
+
+            if is_full_eap:
+                tasks.append(generate_eap_summary_pdf.s(eap_registration_id))
+
+            transaction.on_commit(
+                lambda: chain(
+                    group(tasks),
+                    send_pending_pfa_email.si(eap_registration_id),
+                ).apply_async()
+            )
 
         elif (old_status, new_status) == (
             EAPRegistration.Status.PENDING_PFA,

eap/tasks.py

@@ -212,11 +212,6 @@ def send_new_eap_submission_email(eap_registration_id: int):
     else:
         latest_eap = instance.latest_full_eap
 
-    if not latest_eap.export_file:
-        generate_export_eap_pdf(
-            eap_registration_id=instance.id,
-            version=latest_eap.version,
-        )
     partner_ns_emails = list(latest_eap.partner_contacts.values_list("email", flat=True))
 
     regional_coordinator_emails: list[str] = get_coordinator_emails_by_region(instance.country.region)
@@ -253,7 +248,7 @@ def send_new_eap_submission_email(eap_registration_id: int):
 
 @shared_task
 def send_feedback_email(eap_registration_id: int):
-    instance = EAPRegistration.objects.filter(id=eap_registration_id).first()
+    instance: EAPRegistration | None = EAPRegistration.objects.filter(id=eap_registration_id).first()
     if not instance:
         return None
 
@@ -286,7 +281,7 @@ def send_feedback_email(eap_registration_id: int):
     email_context = get_eap_email_context(instance)
     email_subject = (
         f"[DREF {instance.get_eap_type_display()} FEEDBACK] "
-        f"{instance.country} {instance.disaster_type} TO THE {instance.national_society}"
+        f"{instance.country} {instance.disaster_type} TO THE {instance.national_society.society_name}"
     )
     email_body = render_to_string("email/eap/feedback_to_national_society.html", email_context)
     email_type = "Feedback to the National Society"
@@ -303,7 +298,6 @@ def send_feedback_email(eap_registration_id: int):
 
 @shared_task
 def send_eap_resubmission_email(eap_registration_id: int):
-
     instance = EAPRegistration.objects.filter(id=eap_registration_id).first()
     if not instance:
         return None
@@ -312,14 +306,7 @@ def send_eap_resubmission_email(eap_registration_id: int):
     else:
         latest_eap = instance.latest_full_eap
 
-    if not latest_eap.diff_file:
-        generate_export_diff_pdf(
-            eap_registration_id=instance.id,
-            version=latest_eap.version,
-        )
-
     partner_ns_emails = list(latest_eap.partner_contacts.values_list("email", flat=True))
-
     regional_coordinator_emails: list[str] = get_coordinator_emails_by_region(instance.country.region)
 
     recipients = [
@@ -351,13 +338,11 @@ def send_eap_resubmission_email(eap_registration_id: int):
         mailtype=email_type,
         cc_recipients=cc_recipients,
     )
-
     return True
 
 
 @shared_task
 def send_feedback_email_for_resubmitted_eap(eap_registration_id: int):
-
     instance = EAPRegistration.objects.filter(id=eap_registration_id).first()
     if not instance:
         return None
@@ -388,7 +373,7 @@ def send_feedback_email_for_resubmitted_eap(eap_registration_id: int):
     email_context = get_eap_email_context(instance)
     email_subject = (
         f"[DREF {instance.get_eap_type_display()} FEEDBACK] "
-        f"{instance.country} {instance.disaster_type} version {latest_eap.version} TO {instance.national_society}"
+        f"{instance.country} {instance.disaster_type} version {latest_eap.version} TO {instance.national_society.society_name}"
     )
     email_body = render_to_string("email/eap/feedback_to_revised_eap.html", email_context)
     email_type = "Feedback to the National Society"
@@ -458,18 +443,6 @@ def send_pending_pfa_email(eap_registration_id: int):
     is_full_eap = instance.get_eap_type_enum == EAPType.FULL_EAP
 
     latest_eap = instance.latest_full_eap if is_full_eap else instance.latest_simplified_eap
-
-    if not latest_eap.diff_file:
-        generate_export_diff_pdf(
-            eap_registration_id=instance.id,
-            version=latest_eap.version,
-        )
-
-    if is_full_eap and not instance.summary_file:
-        generate_eap_summary_pdf(
-            eap_registration_id=instance.id,
-        )
-
     partner_ns_emails = list(latest_eap.partner_contacts.values_list("email", flat=True))
 
     regional_coordinator_emails: list[str] = get_coordinator_emails_by_region(instance.country.region)

eap/test_views.py

@@ -1790,28 +1790,22 @@ def test_status_transition(self):
         response = self.client.patch(url, update_data, format="json")
         self.assertEqual(response.status_code, 400, response.data)
 
-    @mock.patch("eap.serializers.generate_export_eap_pdf")
+    @mock.patch("eap.serializers.chain")
     @mock.patch("eap.serializers.group")
-    @mock.patch("eap.serializers.send_new_eap_submission_email")
     @mock.patch("eap.serializers.send_feedback_email")
-    @mock.patch("eap.serializers.send_eap_resubmission_email")
     @mock.patch("eap.serializers.send_technical_validation_email")
     @mock.patch("eap.serializers.send_feedback_email_for_resubmitted_eap")
-    @mock.patch("eap.serializers.send_pending_pfa_email")
     @mock.patch("eap.serializers.send_approved_email")
     def test_status_transitions_trigger_email(
         self,
         send_approved_email,
-        send_pending_pfa_email,
         send_feedback_email_for_resubmitted_eap,
         send_technical_validation_email,
-        send_eap_resubmission_email,
         send_feedback_email,
-        send_new_eap_submission_email,
         mock_group,
-        generate_export_eap_pdf,
+        mock_chain,
     ):
-
+        mock_chain.return_value.apply_async = mock.Mock()
         # Create permissions
         management.call_command("make_permissions")
 
@@ -1872,13 +1866,13 @@ def test_status_transitions_trigger_email(
         with self.capture_on_commit_callbacks(execute=True):
             response = self.client.post(url, data, format="json")
         self.assert_200(response)
+
+        self.assertTrue(mock_chain.called)
+        self.assertTrue(mock_chain.return_value.apply_async.called)
+        mock_chain.reset_mock()
+
         eap_registration.refresh_from_db()
         self.assertEqual(response.data["status"], EAPStatus.UNDER_REVIEW)
-        generate_export_eap_pdf.delay.assert_called_once_with(
-            eap_registration_id=eap_registration.id, version=simplified_eap.version
-        )
-        send_new_eap_submission_email.delay.assert_called_once_with(eap_registration.id)
-        send_new_eap_submission_email.delay.reset_mock()
 
         # UNDER_REVIEW -> NS_ADDRESSING_COMMENTS
         data = {
@@ -1947,12 +1941,9 @@ def test_status_transitions_trigger_email(
         eap_registration.refresh_from_db()
         self.assertEqual(response.data["status"], EAPStatus.UNDER_REVIEW)
 
-        # NOTE: Check that two signatures are created
-        mock_group.assert_called_once()
-        self.assertEqual(len(mock_group.call_args.args), 2)
-
-        send_eap_resubmission_email.delay.assert_called_once_with(eap_registration.id)
-        send_eap_resubmission_email.delay.reset_mock()
+        self.assertTrue(mock_chain.called)
+        self.assertTrue(mock_chain.return_value.apply_async.called)
+        mock_chain.reset_mock()
 
         # AGAIN NOTE: Transition to NS_ADDRESSING_COMMENTS
         # UNDER_REVIEW -> NS_ADDRESSING_COMMENTS
@@ -2017,9 +2008,10 @@ def test_status_transitions_trigger_email(
         self.assert_200(response)
         eap_registration.refresh_from_db()
         self.assertEqual(response.data["status"], EAPStatus.UNDER_REVIEW)
-        self.assertTrue(mock_group.called)
-        send_eap_resubmission_email.delay.assert_called_once_with(eap_registration.id)
-        send_eap_resubmission_email.delay.reset_mock()
+
+        self.assertTrue(mock_chain.called)
+        self.assertTrue(mock_chain.return_value.apply_async.called)
+        mock_chain.reset_mock()
 
         # Transition UNDER_REVIEW -> TECHNICALLY_VALIDATED
         data = {"status": EAPStatus.TECHNICALLY_VALIDATED}
@@ -2096,9 +2088,10 @@ def test_status_transitions_trigger_email(
         self.assert_200(response)
         eap_registration.refresh_from_db()
         self.assertEqual(response.data["status"], EAPStatus.UNDER_REVIEW)
-        self.assertTrue(mock_group.called)
-        send_eap_resubmission_email.delay.assert_called_once_with(eap_registration.id)
-        send_eap_resubmission_email.delay.reset_mock()
+
+        self.assertTrue(mock_chain.called)
+        self.assertTrue(mock_chain.return_value.apply_async.called)
+        mock_chain.reset_mock()
 
         # Again Transition UNDER_REVIEW -> TECHNICALLY_VALIDATED
         data = {"status": EAPStatus.TECHNICALLY_VALIDATED}
@@ -2132,7 +2125,16 @@ def test_status_transitions_trigger_email(
         self.assert_200(response)
         self.assertEqual(response.data["status"], EAPStatus.PENDING_PFA)
         eap_registration.refresh_from_db()
-        send_pending_pfa_email.delay.assert_called_once_with(eap_registration.id)
+
+        self.assertTrue(mock_chain.called)
+        self.assertTrue(mock_group.called)
+        self.assertTrue(mock_chain.return_value.apply_async.called)
+
+        tasks = mock_group.call_args[0][0]
+        self.assertEqual(len(tasks), 1)
+
+        mock_chain.reset_mock()
+        mock_group.reset_mock()
 
         # Transition PENDING_PFA -> APPROVED
         data = {"status": EAPStatus.APPROVED}