Update grant approved email to show combined reimbursement total (#4541)

Author: estyxx

backend/grants/models.py

@@ -262,22 +262,46 @@ def get_admin_url(self):
             args=(self.pk,),
         )
 
-    def has_approved_travel(self):
-        return self.reimbursements.filter(
-            category__category=GrantReimbursementCategory.Category.TRAVEL
-        ).exists()
+    def has_approved(self, category: GrantReimbursementCategory.Category) -> bool:
+        """Return True if grant has approved reimbursement for category."""
+        return self.reimbursements.filter(category__category=category).exists()
 
-    def has_approved_accommodation(self):
-        return self.reimbursements.filter(
-            category__category=GrantReimbursementCategory.Category.ACCOMMODATION
-        ).exists()
+    def has_approved_ticket(self) -> bool:
+        return self.has_approved(GrantReimbursementCategory.Category.TICKET)
+
+    def has_approved_travel(self) -> bool:
+        return self.has_approved(GrantReimbursementCategory.Category.TRAVEL)
+
+    def has_approved_accommodation(self) -> bool:
+        return self.has_approved(GrantReimbursementCategory.Category.ACCOMMODATION)
+
+    def has_ticket_only(self) -> bool:
+        """Return True if grant has only ticket, no travel or accommodation."""
+        return (
+            self.has_approved_ticket()
+            and not self.has_approved_travel()
+            and not self.has_approved_accommodation()
+        )
 
     @property
-    def total_allocated_amount(self):
-        return sum(r.granted_amount for r in self.reimbursements.all())
+    def total_allocated_amount(self) -> Decimal:
+        """Return total of all reimbursements including ticket."""
+        return sum(
+            (r.granted_amount for r in self.reimbursements.all()),
+            start=Decimal(0),
+        )
 
-    def has_approved(self, type_):
-        return self.reimbursements.filter(category__category=type_).exists()
+    @property
+    def total_grantee_reimbursement_amount(self) -> Decimal:
+        """Return total reimbursement excluding ticket."""
+        return sum(
+            (
+                r.granted_amount
+                for r in self.reimbursements.all()
+                if r.category.category != GrantReimbursementCategory.Category.TICKET
+            ),
+            start=Decimal(0),
+        )
 
     @property
     def current_or_pending_status(self):

backend/grants/tasks.py

@@ -14,17 +14,27 @@
 logger = logging.getLogger(__name__)
 
 
-def get_name(user: User | None, fallback: str = "<no name specified>"):
+def get_name(user: User | None, fallback: str = "<no name specified>") -> str:
     if not user:
         return fallback
 
     return user.full_name or user.name or user.username or fallback
 
 
 @app.task
-def send_grant_reply_approved_email(*, grant_id, is_reminder):
+def send_grant_reply_approved_email(*, grant_id: int, is_reminder: bool) -> None:
     logger.info("Sending Reply APPROVED email for Grant %s", grant_id)
     grant = Grant.objects.get(id=grant_id)
+
+    total_amount = grant.total_grantee_reimbursement_amount
+    ticket_only = grant.has_ticket_only()
+
+    if total_amount == 0 and not ticket_only:
+        raise ValueError(
+            f"Grant {grant_id} has no reimbursement amount and is not ticket-only. "
+            "This indicates missing or zero-amount reimbursements."
+        )
+
     reply_url = urljoin(settings.FRONTEND_URL, "/grants/reply/")
 
     variables = {
@@ -34,26 +44,11 @@ def send_grant_reply_approved_email(*, grant_id, is_reminder):
         "deadline_date_time": f"{grant.applicant_reply_deadline:%-d %B %Y %H:%M %Z}",
         "deadline_date": f"{grant.applicant_reply_deadline:%-d %B %Y}",
         "visa_page_link": urljoin(settings.FRONTEND_URL, "/visa"),
-        "has_approved_travel": grant.has_approved_travel(),
-        "has_approved_accommodation": grant.has_approved_accommodation(),
+        "total_amount": f"{total_amount:.0f}" if total_amount > 0 else None,
+        "ticket_only": ticket_only,
         "is_reminder": is_reminder,
     }
 
-    if grant.has_approved_travel():
-        from grants.models import GrantReimbursementCategory
-
-        travel_reimbursements = grant.reimbursements.filter(
-            category__category=GrantReimbursementCategory.Category.TRAVEL
-        )
-        travel_amount = sum(r.granted_amount for r in travel_reimbursements)
-
-        if not travel_amount or travel_amount == 0:
-            raise ValueError(
-                "Grant travel amount is set to Zero, can't send the email!"
-            )
-
-        variables["travel_amount"] = f"{travel_amount:.0f}"
-
     _new_send_grant_email(
         template_identifier=EmailTemplateIdentifier.grant_approved,
         grant=grant,

backend/grants/tests/test_models.py

@@ -145,8 +145,24 @@ def test_calculate_grant_amounts(data):
     assert grant.total_allocated_amount == Decimal(expected_total)
 
 
+def test_has_approved_ticket():
+    grant = GrantFactory()
+    assert not grant.has_approved_ticket()
+
+    GrantReimbursementFactory(
+        grant=grant,
+        category__conference=grant.conference,
+        category__ticket=True,
+        granted_amount=Decimal("100"),
+    )
+
+    assert grant.has_approved_ticket()
+
+
 def test_has_approved_travel():
     grant = GrantFactory()
+    assert not grant.has_approved_travel()
+
     GrantReimbursementFactory(
         grant=grant,
         category__conference=grant.conference,
@@ -159,6 +175,8 @@ def test_has_approved_travel():
 
 def test_has_approved_accommodation():
     grant = GrantFactory()
+    assert not grant.has_approved_accommodation()
+
     GrantReimbursementFactory(
         grant=grant,
         category__conference=grant.conference,
@@ -169,6 +187,85 @@ def test_has_approved_accommodation():
     assert grant.has_approved_accommodation()
 
 
+def test_has_ticket_only_with_only_ticket():
+    grant = GrantFactory()
+    GrantReimbursementFactory(
+        grant=grant,
+        category__conference=grant.conference,
+        category__ticket=True,
+        granted_amount=Decimal("100"),
+    )
+
+    assert grant.has_ticket_only()
+
+
+def test_has_ticket_only_with_ticket_and_travel():
+    grant = GrantFactory()
+    GrantReimbursementFactory(
+        grant=grant,
+        category__conference=grant.conference,
+        category__ticket=True,
+        granted_amount=Decimal("100"),
+    )
+    GrantReimbursementFactory(
+        grant=grant,
+        category__conference=grant.conference,
+        category__travel=True,
+        granted_amount=Decimal("500"),
+    )
+
+    assert not grant.has_ticket_only()
+
+
+def test_has_ticket_only_without_ticket():
+    grant = GrantFactory()
+    GrantReimbursementFactory(
+        grant=grant,
+        category__conference=grant.conference,
+        category__travel=True,
+        granted_amount=Decimal("500"),
+    )
+
+    assert not grant.has_ticket_only()
+
+
+def test_total_grantee_reimbursement_amount_excludes_ticket():
+    grant = GrantFactory()
+    GrantReimbursementFactory(
+        grant=grant,
+        category__conference=grant.conference,
+        category__ticket=True,
+        granted_amount=Decimal("100"),
+    )
+    GrantReimbursementFactory(
+        grant=grant,
+        category__conference=grant.conference,
+        category__travel=True,
+        granted_amount=Decimal("500"),
+    )
+    GrantReimbursementFactory(
+        grant=grant,
+        category__conference=grant.conference,
+        category__accommodation=True,
+        granted_amount=Decimal("200"),
+    )
+
+    # Should be 500 + 200 = 700, excluding ticket
+    assert grant.total_grantee_reimbursement_amount == Decimal("700")
+
+
+def test_total_grantee_reimbursement_amount_with_only_ticket():
+    grant = GrantFactory()
+    GrantReimbursementFactory(
+        grant=grant,
+        category__conference=grant.conference,
+        category__ticket=True,
+        granted_amount=Decimal("100"),
+    )
+
+    assert grant.total_grantee_reimbursement_amount == Decimal("0")
+
+
 @pytest.mark.parametrize(
     "departure_country,country_type",
     [

backend/grants/tests/test_tasks.py

@@ -10,10 +10,7 @@
     send_grant_reply_waiting_list_email,
     send_grant_reply_waiting_list_update_email,
 )
-from grants.tests.factories import (
-    GrantFactory,
-    GrantReimbursementFactory,
-)
+from grants.tests.factories import GrantFactory, GrantReimbursementFactory
 from users.tests.factories import UserFactory
 
 pytestmark = pytest.mark.django_db
@@ -166,8 +163,8 @@ def test_handle_grant_reply_sent_reminder(settings, sent_emails):
     assert sent_email.placeholders["deadline_date"] == "1 February 2023"
     assert sent_email.placeholders["reply_url"] == "https://pycon.it/grants/reply/"
     assert sent_email.placeholders["visa_page_link"] == "https://pycon.it/visa"
-    assert not sent_email.placeholders["has_approved_travel"]
-    assert not sent_email.placeholders["has_approved_accommodation"]
+    assert sent_email.placeholders["ticket_only"]
+    assert sent_email.placeholders["total_amount"] is None
     assert sent_email.placeholders["is_reminder"]
 
 
@@ -240,51 +237,16 @@ def test_handle_grant_approved_ticket_travel_accommodation_reply_sent(
     )
     assert sent_email.placeholders["start_date"] == "2 May"
     assert sent_email.placeholders["end_date"] == "6 May"
-    assert sent_email.placeholders["travel_amount"] == "680"
+    # Total amount is 680 (travel) + 200 (accommodation) = 880, excluding ticket
+    assert sent_email.placeholders["total_amount"] == "880"
     assert sent_email.placeholders["deadline_date_time"] == "1 February 2023 23:59 UTC"
     assert sent_email.placeholders["deadline_date"] == "1 February 2023"
     assert sent_email.placeholders["reply_url"] == "https://pycon.it/grants/reply/"
     assert sent_email.placeholders["visa_page_link"] == "https://pycon.it/visa"
-    assert sent_email.placeholders["has_approved_travel"]
-    assert sent_email.placeholders["has_approved_accommodation"]
+    assert not sent_email.placeholders["ticket_only"]
     assert not sent_email.placeholders["is_reminder"]
 
 
-def test_handle_grant_approved_ticket_travel_accommodation_fails_with_no_amount(
-    settings,
-):
-    settings.FRONTEND_URL = "https://pycon.it"
-
-    conference = ConferenceFactory(
-        start=datetime(2023, 5, 2, tzinfo=timezone.utc),
-        end=datetime(2023, 5, 5, tzinfo=timezone.utc),
-    )
-    user = UserFactory(
-        full_name="Marco Acierno",
-        email="[email protected]",
-        name="Marco",
-        username="marco",
-    )
-
-    grant = GrantFactory(
-        conference=conference,
-        applicant_reply_deadline=datetime(2023, 2, 1, 23, 59, tzinfo=timezone.utc),
-        user=user,
-    )
-    GrantReimbursementFactory(
-        grant=grant,
-        category__conference=conference,
-        category__travel=True,
-        category__max_amount=Decimal("680"),
-        granted_amount=Decimal("0"),
-    )
-
-    with pytest.raises(
-        ValueError, match="Grant travel amount is set to Zero, can't send the email!"
-    ):
-        send_grant_reply_approved_email(grant_id=grant.id, is_reminder=False)
-
-
 def test_handle_grant_approved_ticket_only_reply_sent(settings, sent_emails):
     from notifications.models import EmailTemplateIdentifier
     from notifications.tests.factories import EmailTemplateFactory
@@ -344,8 +306,8 @@ def test_handle_grant_approved_ticket_only_reply_sent(settings, sent_emails):
     assert sent_email.placeholders["deadline_date"] == "1 February 2023"
     assert sent_email.placeholders["reply_url"] == "https://pycon.it/grants/reply/"
     assert sent_email.placeholders["visa_page_link"] == "https://pycon.it/visa"
-    assert not sent_email.placeholders["has_approved_travel"]
-    assert not sent_email.placeholders["has_approved_accommodation"]
+    assert sent_email.placeholders["ticket_only"]
+    assert sent_email.placeholders["total_amount"] is None
     assert not sent_email.placeholders["is_reminder"]
 
 
@@ -415,12 +377,46 @@ def test_handle_grant_approved_travel_reply_sent(settings, sent_emails):
     assert sent_email.placeholders["deadline_date"] == "1 February 2023"
     assert sent_email.placeholders["reply_url"] == "https://pycon.it/grants/reply/"
     assert sent_email.placeholders["visa_page_link"] == "https://pycon.it/visa"
-    assert sent_email.placeholders["has_approved_travel"]
-    assert not sent_email.placeholders["has_approved_accommodation"]
-    assert sent_email.placeholders["travel_amount"] == "400"
+    # Total amount is 400 (travel only), excluding ticket
+    assert sent_email.placeholders["total_amount"] == "400"
+    assert not sent_email.placeholders["ticket_only"]
     assert not sent_email.placeholders["is_reminder"]
 
 
+def test_send_grant_approved_email_raises_for_no_reimbursements(settings) -> None:
+    """Verify error is raised when grant has no valid reimbursements."""
+    from notifications.models import EmailTemplateIdentifier
+    from notifications.tests.factories import EmailTemplateFactory
+
+    settings.FRONTEND_URL = "https://pycon.it"
+
+    conference = ConferenceFactory(
+        start=datetime(2023, 5, 2, tzinfo=timezone.utc),
+        end=datetime(2023, 5, 5, tzinfo=timezone.utc),
+    )
+    user = UserFactory(
+        full_name="Marco Acierno",
+        email="[email protected]",
+    )
+
+    grant = GrantFactory(
+        conference=conference,
+        applicant_reply_deadline=datetime(2023, 2, 1, 23, 59, tzinfo=timezone.utc),
+        user=user,
+    )
+    # No reimbursements - this is an invalid state
+
+    EmailTemplateFactory(
+        conference=grant.conference,
+        identifier=EmailTemplateIdentifier.grant_approved,
+    )
+
+    with pytest.raises(
+        ValueError, match="has no reimbursement amount and is not ticket-only"
+    ):
+        send_grant_reply_approved_email(grant_id=grant.id, is_reminder=False)
+
+
 def test_send_grant_reply_waiting_list_update_email(settings, sent_emails):
     from notifications.models import EmailTemplateIdentifier
     from notifications.tests.factories import EmailTemplateFactory