From 52cb83d2704260dd2a1360b58ccba43e01051224 Mon Sep 17 00:00:00 2001 From: Sushil Tiwari Date: Thu, 23 Apr 2026 10:34:36 +0545 Subject: [PATCH] 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 --- assets | 2 +- eap/dev_views.py | 1 + eap/serializers.py | 100 ++++++++---------- eap/tasks.py | 33 +----- eap/test_views.py | 56 +++++----- eap/utils.py | 12 ++- eap/views.py | 14 ++- .../templates/email/eap/approved.html | 4 +- .../eap/feedback_to_national_society.html | 17 +-- .../email/eap/feedback_to_revised_eap.html | 6 ++ .../templates/email/eap/pending_pfa.html | 23 ++-- .../templates/email/eap/re-submission.html | 25 +++-- .../templates/email/eap/registration.html | 4 +- .../templates/email/eap/submission.html | 15 ++- .../email/eap/technically_validated_eap.html | 5 +- 15 files changed, 165 insertions(+), 152 deletions(-) diff --git a/assets b/assets index 237631a53..41cf7c907 160000 --- a/assets +++ b/assets @@ -1 +1 @@ -Subproject commit 237631a53996919aa42d2bf42f8cf0c8dd636368 +Subproject commit 41cf7c907b83a936d6162e5747cee283fe02bc85 diff --git a/eap/dev_views.py b/eap/dev_views.py index fea626384..1097757d3 100644 --- a/eap/dev_views.py +++ b/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", }, diff --git a/eap/serializers.py b/eap/serializers.py index 5d2bdddee..905241be9 100644 --- a/eap/serializers.py +++ b/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, diff --git a/eap/tasks.py b/eap/tasks.py index 11d5d9451..4b5fc4f5f 100644 --- a/eap/tasks.py +++ b/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) diff --git a/eap/test_views.py b/eap/test_views.py index 6b45283e8..5c9854f1a 100644 --- a/eap/test_views.py +++ b/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} diff --git a/eap/utils.py b/eap/utils.py index 58e6d6992..cbb304f7c 100644 --- a/eap/utils.py +++ b/eap/utils.py @@ -46,8 +46,11 @@ def get_file_url(file_obj): """ if not file_obj: return None - if hasattr(file_obj, "file"): - return file_obj.file.url + + file_attr = getattr(file_obj, "file", file_obj) + url = getattr(file_attr, "url", None) + + return url if url else None def get_share_eap_email_context(instance): @@ -104,6 +107,7 @@ def get_eap_email_context(instance): "supporting_partners": eap_registration_data["partners_details"], "disaster_type": eap_registration_data["disaster_type_details"]["name"], "ns_contact_name": eap_registration_data["national_society_contact_name"], + "ns_contact_title": eap_registration_data["national_society_contact_title"], "ns_contact_email": eap_registration_data["national_society_contact_email"], "ns_contact_phone": eap_registration_data["national_society_contact_phone_number"], "deadline": eap_registration_data["deadline"], @@ -123,8 +127,8 @@ def get_eap_email_context(instance): "people_targeted": latest_eap_data.people_targeted, "total_budget": latest_eap_data.total_budget, "latest_version": latest_eap_data.version, - "export_file": (latest_eap_data.export_file.url if latest_eap_data.export_file else None), - "diff_file": (latest_eap_data.diff_file.url if latest_eap_data.diff_file else None), + "export_file": latest_eap_data.export_file.url if latest_eap_data.export_file else None, + "diff_file": latest_eap_data.diff_file.url if latest_eap_data.diff_file else None, "budget_file": get_file_url(latest_eap_data.budget_file), "updated_checklist_file": get_file_url(latest_eap_data.updated_checklist_file), "review_checklist_file": ( diff --git a/eap/views.py b/eap/views.py index d5427ec1b..6ca9dbd2a 100644 --- a/eap/views.py +++ b/eap/views.py @@ -96,7 +96,7 @@ class EAPRegistrationViewSet(EAPModelViewSet): filterset_class = EAPRegistrationFilterSet def get_queryset(self) -> QuerySet[EAPRegistration]: - return ( + base_qs = ( super() .get_queryset() .select_related( @@ -106,9 +106,15 @@ def get_queryset(self) -> QuerySet[EAPRegistration]: "disaster_type", "country", ) - .prefetch_related( + .prefetch_related("users") + ) + + if self.action in [ + "list", + "retrieve", + ]: + return base_qs.prefetch_related( "partners", - "users", Prefetch( "simplified_eaps", queryset=SimplifiedEAP.objects.select_related( @@ -132,7 +138,7 @@ def get_queryset(self) -> QuerySet[EAPRegistration]: ), ), ) - ) + return base_qs @action( detail=True, diff --git a/notifications/templates/email/eap/approved.html b/notifications/templates/email/eap/approved.html index b272a7964..54b9d11f5 100644 --- a/notifications/templates/email/eap/approved.html +++ b/notifications/templates/email/eap/approved.html @@ -7,7 +7,7 @@

Dear {{ national_society }} colleagues,

- +

We are glad to inform you that the {{ country_name }} {{ disaster_type }} is ready for implementation. Congratulations!

@@ -17,10 +17,12 @@ and the NS should start the implementation of readiness for year 1 and pre-positioning activities.

+

If you have any questions on the process or the next steps, please don’t hesitate to reach out DREF.anticipatorypillar@ifrc.org.

+

Congratulations again and warm wishes,
IFRC DREF AA Team diff --git a/notifications/templates/email/eap/feedback_to_national_society.html b/notifications/templates/email/eap/feedback_to_national_society.html index 64b3eed9c..37ea9826d 100644 --- a/notifications/templates/email/eap/feedback_to_national_society.html +++ b/notifications/templates/email/eap/feedback_to_national_society.html @@ -10,6 +10,7 @@

+

Thanks again for the submission of this protocol. We acknowledge the work the NS has done to submit it. The Validation Committee, the Delegation, and the Regional colleagues have completed the review. @@ -32,14 +33,18 @@ Do not hesitate to contact us should you have any further questions at DREF.anticipatorypillar@ifrc.org.


You can access your GO account and check the progress of your EAP here.

+ -

Attachments:

- {% if review_checklist_file %} - +
+

Attachments:

+ {% if review_checklist_file %} + {% endif %} + +

Kind regards,
IFRC-DREF AA Team
@@ -48,4 +53,4 @@ -{% include "design/foot1.html" %} \ No newline at end of file +{% include "design/foot1.html" %} diff --git a/notifications/templates/email/eap/feedback_to_revised_eap.html b/notifications/templates/email/eap/feedback_to_revised_eap.html index fac7fe014..908626eb3 100644 --- a/notifications/templates/email/eap/feedback_to_revised_eap.html +++ b/notifications/templates/email/eap/feedback_to_revised_eap.html @@ -10,6 +10,7 @@

+

Thanks again for the submission of the {{ latest_version }} version of this protocol. We acknowledge the work the NS has done to submit it. @@ -22,6 +23,7 @@

+

As next steps, the NS should: