Grants fix bugs email task (#4205)

Author: estyxx

backend/api/grants/mutations.py

@@ -18,10 +18,11 @@
 from conferences.models.conference import Conference
 from grants.tasks import (
     notify_new_grant_reply_slack,
-    send_grant_application_confirmation_email,
 )
 from grants.models import Grant as GrantModel
 from users.models import User
+from grants.tasks import get_name
+from notifications.models import EmailTemplate, EmailTemplateIdentifier
 
 
 @strawberry.type
@@ -212,6 +213,7 @@ class SendGrantReplyError:
 @strawberry.type
 class GrantMutation:
     @strawberry.mutation(permission_classes=[IsAuthenticated])
+    @transaction.atomic
     def send_grant(self, info: Info, input: SendGrantInput) -> SendGrantResult:
         request = info.context.request
 
@@ -220,52 +222,60 @@ def send_grant(self, info: Info, input: SendGrantInput) -> SendGrantResult:
         if errors := input.validate(conference=conference, user=request.user):
             return errors
 
-        with transaction.atomic():
-            instance = GrantModel.objects.create(
-                **{
-                    "user_id": request.user.id,
-                    "conference": conference,
-                    "name": input.name,
-                    "full_name": input.full_name,
-                    "age_group": input.age_group,
-                    "gender": input.gender,
-                    "occupation": input.occupation,
-                    "grant_type": input.grant_type,
-                    "python_usage": input.python_usage,
-                    "been_to_other_events": input.been_to_other_events,
-                    "community_contribution": input.community_contribution,
-                    "needs_funds_for_travel": input.needs_funds_for_travel,
-                    "need_visa": input.need_visa,
-                    "need_accommodation": input.need_accommodation,
-                    "why": input.why,
-                    "notes": input.notes,
-                    "departure_country": input.departure_country,
-                    "nationality": input.nationality,
-                    "departure_city": input.departure_city,
-                }
-            )
+        instance = GrantModel.objects.create(
+            **{
+                "user_id": request.user.id,
+                "conference": conference,
+                "name": input.name,
+                "full_name": input.full_name,
+                "age_group": input.age_group,
+                "gender": input.gender,
+                "occupation": input.occupation,
+                "grant_type": input.grant_type,
+                "python_usage": input.python_usage,
+                "been_to_other_events": input.been_to_other_events,
+                "community_contribution": input.community_contribution,
+                "needs_funds_for_travel": input.needs_funds_for_travel,
+                "need_visa": input.need_visa,
+                "need_accommodation": input.need_accommodation,
+                "why": input.why,
+                "notes": input.notes,
+                "departure_country": input.departure_country,
+                "nationality": input.nationality,
+                "departure_city": input.departure_city,
+            }
+        )
 
-            record_privacy_policy_acceptance(
-                info.context.request,
-                conference,
-                "grant",
-            )
+        record_privacy_policy_acceptance(
+            info.context.request,
+            conference,
+            "grant",
+        )
 
-            Participant.objects.update_or_create(
-                user_id=request.user.id,
-                conference=instance.conference,
-                defaults={
-                    "bio": input.participant_bio,
-                    "website": input.participant_website,
-                    "twitter_handle": input.participant_twitter_handle,
-                    "instagram_handle": input.participant_instagram_handle,
-                    "linkedin_url": input.participant_linkedin_url,
-                    "facebook_url": input.participant_facebook_url,
-                    "mastodon_handle": input.participant_mastodon_handle,
-                },
-            )
+        Participant.objects.update_or_create(
+            user_id=request.user.id,
+            conference=instance.conference,
+            defaults={
+                "bio": input.participant_bio,
+                "website": input.participant_website,
+                "twitter_handle": input.participant_twitter_handle,
+                "instagram_handle": input.participant_instagram_handle,
+                "linkedin_url": input.participant_linkedin_url,
+                "facebook_url": input.participant_facebook_url,
+                "mastodon_handle": input.participant_mastodon_handle,
+            },
+        )
+
+        email_template = EmailTemplate.objects.for_conference(
+            conference
+        ).get_by_identifier(EmailTemplateIdentifier.grant_application_confirmation)
 
-            send_grant_application_confirmation_email.delay(grant_id=instance.id)
+        email_template.send_email(
+            recipient=request.user,
+            placeholders={
+                "user_name": get_name(request.user, "there"),
+            },
+        )
 
         # hack because we return django models
         instance.__strawberry_definition__ = Grant.__strawberry_definition__

backend/api/grants/tests/test_send_grant.py

@@ -4,8 +4,9 @@
 import pytest
 from participants.models import Participant
 from grants.models import Grant
+from notifications.models import EmailTemplateIdentifier
+from notifications.tests.factories import EmailTemplateFactory
 
-from unittest.mock import call
 
 pytestmark = pytest.mark.django_db
 
@@ -90,32 +91,43 @@ def _send_grant(client, conference, conference_code=None, **kwargs):
     return response
 
 
-def test_send_grant(graphql_client, user, mocker):
-    mock_confirmation_email = mocker.patch(
-        "api.grants.mutations.send_grant_application_confirmation_email"
-    )
+def test_send_grant(graphql_client, user, mocker, django_capture_on_commit_callbacks):
+    mock_email_template = mocker.patch("api.grants.mutations.EmailTemplate")
     graphql_client.force_login(user)
     conference = ConferenceFactory(active_grants=True)
+    EmailTemplateFactory(
+        conference=conference,
+        identifier=EmailTemplateIdentifier.grant_application_confirmation,
+    )
 
-    response = _send_grant(graphql_client, conference)
+    with django_capture_on_commit_callbacks(execute=True):
+        response = _send_grant(graphql_client, conference)
 
+    # The API response is successful
     assert response["data"]["sendGrant"]["__typename"] == "Grant"
     assert response["data"]["sendGrant"]["id"]
 
+    # A participant is created
     participant = Participant.objects.get(conference=conference, user_id=user.id)
     assert participant.bio == "my bio"
+
+    # A grant object is created
     grant = Grant.objects.get(id=response["data"]["sendGrant"]["id"])
     assert grant.conference == conference
     assert PrivacyPolicyAcceptanceRecord.objects.filter(
         user=user, conference=conference, privacy_policy="grant"
     ).exists()
-    mock_confirmation_email.delay.assert_called_once_with(grant_id=grant.id)
+
+    # An email is sent to the user
+    mock_email_template.objects.for_conference().get_by_identifier().send_email.assert_called_once_with(
+        recipient=user,
+        placeholders={
+            "user_name": user.full_name,
+        },
+    )
 
 
 def test_cannot_send_a_grant_if_grants_are_closed(graphql_client, user, mocker):
-    mock_confirmation_email = mocker.patch(
-        "api.grants.mutations.send_grant_application_confirmation_email"
-    )
     graphql_client.force_login(user)
     conference = ConferenceFactory(active_grants=False)
 
@@ -126,7 +138,6 @@ def test_cannot_send_a_grant_if_grants_are_closed(graphql_client, user, mocker):
     assert response["data"]["sendGrant"]["errors"]["nonFieldErrors"] == [
         "The grants form is not open!"
     ]
-    mock_confirmation_email.delay.assert_not_called()
 
 
 def test_cannot_send_a_grant_if_grants_deadline_do_not_exists(graphql_client, user):
@@ -152,11 +163,12 @@ def test_cannot_send_a_grant_as_unlogged_user(graphql_client):
 
 
 def test_cannot_send_two_grants_to_the_same_conference(graphql_client, user, mocker):
-    mock_confirmation_email = mocker.patch(
-        "api.grants.mutations.send_grant_application_confirmation_email"
-    )
     graphql_client.force_login(user)
     conference = ConferenceFactory(active_grants=True)
+    EmailTemplateFactory(
+        conference=conference,
+        identifier=EmailTemplateIdentifier.grant_application_confirmation,
+    )
     _send_grant(graphql_client, conference)
 
     response = _send_grant(graphql_client, conference)
@@ -166,29 +178,30 @@ def test_cannot_send_two_grants_to_the_same_conference(graphql_client, user, moc
     assert response["data"]["sendGrant"]["errors"]["nonFieldErrors"] == [
         "Grant already submitted!"
     ]
-    mock_confirmation_email.delay.assert_called_once()
 
 
-def test_can_send_two_grants_to_different_conferences(graphql_client, user, mocker):
-    mock_confirmation_email = mocker.patch(
-        "api.grants.mutations.send_grant_application_confirmation_email"
-    )
+def test_can_send_two_grants_to_different_conferences(
+    graphql_client, user, mocker, django_capture_on_commit_callbacks
+):
     graphql_client.force_login(user)
     conference = ConferenceFactory(active_grants=True)
     conference_2 = ConferenceFactory(active_grants=True)
+    EmailTemplateFactory(
+        conference=conference,
+        identifier=EmailTemplateIdentifier.grant_application_confirmation,
+    )
+    EmailTemplateFactory(
+        conference=conference_2,
+        identifier=EmailTemplateIdentifier.grant_application_confirmation,
+    )
     first_response = _send_grant(graphql_client, conference)
+    assert not first_response.get("errors")
+    assert first_response["data"]["sendGrant"]["__typename"] == "Grant"
 
     second_response = _send_grant(graphql_client, conference_2)
 
     assert not second_response.get("errors")
     assert second_response["data"]["sendGrant"]["__typename"] == "Grant"
-    mock_confirmation_email.delay.assert_has_calls(
-        [
-            call(grant_id=int(first_response["data"]["sendGrant"]["id"])),
-            call(grant_id=int(second_response["data"]["sendGrant"]["id"])),
-        ],
-        any_order=True,
-    )
 
 
 def test_invalid_conference(graphql_client, user):
@@ -260,3 +273,36 @@ def test_cannot_send_grant_with_empty_values(
     assert response["data"]["sendGrant"]["errors"]["validationFullName"] == [
         "full_name: Cannot be empty"
     ]
+
+
+def test_submit_grant_with_existing_participant(graphql_client, user):
+    graphql_client.force_login(user)
+    conference = ConferenceFactory(
+        active_grants=True,
+    )
+    EmailTemplateFactory(
+        conference=conference,
+        identifier=EmailTemplateIdentifier.grant_application_confirmation,
+    )
+    participant = Participant.objects.create(
+        conference=conference, user_id=user.id, bio="old bio"
+    )
+
+    response = _send_grant(
+        graphql_client,
+        conference,
+        participantBio="my bio",
+        participantWebsite="https://sushi.com",
+    )
+
+    assert response["data"]["sendGrant"]["__typename"] == "Grant"
+    assert response["data"]["sendGrant"]["id"]
+
+    grant = Grant.objects.get(id=response["data"]["sendGrant"]["id"])
+    assert grant.status == Grant.Status.pending
+    assert grant.conference == conference
+    assert grant.user_id == user.id
+
+    participant.refresh_from_db()
+    assert participant.bio == "my bio"
+    assert participant.website == "https://sushi.com"

backend/grants/tasks.py

@@ -171,19 +171,3 @@ def _new_send_grant_email(
 
     grant.applicant_reply_sent_at = timezone.now()
     grant.save()
-
-
-@app.task
-def send_grant_application_confirmation_email(*, grant_id):
-    grant = Grant.objects.get(id=grant_id)
-    email_template = EmailTemplate.objects.for_conference(
-        grant.conference
-    ).get_by_identifier(EmailTemplateIdentifier.grant_application_confirmation)
-
-    email_template.send_email(
-        recipient=grant.user,
-        placeholders={
-            "user_name": get_name(grant.user, "there"),
-        },
-    )
-    logger.info("Grant application confirmation email sent for grant %s", grant.id)

backend/grants/tests/test_tasks.py

@@ -11,7 +11,6 @@
     send_grant_reply_approved_email,
     send_grant_reply_rejected_email,
     send_grant_reply_waiting_list_email,
-    send_grant_application_confirmation_email,
 )
 from grants.models import Grant
 
@@ -312,23 +311,3 @@ def test_send_grant_reply_waiting_list_update_email(settings):
             "reply_url": "https://pycon.it/grants/reply/",
         },
     )
-
-
-def test_send_grant_application_confirmation_email():
-    user = UserFactory(
-        full_name="Marco Acierno",
-        email="[email protected]",
-        name="Marco",
-        username="marco",
-    )
-    grant = GrantFactory(user=user)
-
-    with patch("grants.tasks.EmailTemplate") as mock_email_template:
-        send_grant_application_confirmation_email(grant_id=grant.id)
-
-    mock_email_template.objects.for_conference().get_by_identifier().send_email.assert_called_once_with(
-        recipient=user,
-        placeholders={
-            "user_name": "Marco Acierno",
-        },
-    )

frontend/src/components/grant-form/index.tsx

@@ -490,7 +490,11 @@ export const GrantForm = ({
             >
               <Select {...select("needVisa")} required={true}>
                 <FormattedMessage id="global.selectOption">
-                  {(msg) => <option value="">{msg}</option>}
+                  {(msg) => (
+                    <option value="" disabled={true}>
+                      {msg}
+                    </option>
+                  )}
                 </FormattedMessage>
                 <FormattedMessage id="global.no">
                   {(msg) => <option value="false">{msg}</option>}
@@ -512,7 +516,11 @@ export const GrantForm = ({
             >
               <Select {...select("needAccommodation")} required={true}>
                 <FormattedMessage id="global.selectOption">
-                  {(msg) => <option value="">{msg}</option>}
+                  {(msg) => (
+                    <option value="" disabled={true}>
+                      {msg}
+                    </option>
+                  )}
                 </FormattedMessage>
                 <FormattedMessage id="global.no">
                   {(msg) => <option value="false">{msg}</option>}
@@ -561,7 +569,11 @@ export const GrantForm = ({
           >
             <Select {...select("needsFundsForTravel")} required={true}>
               <FormattedMessage id="global.selectOption">
-                {(msg) => <option value="">{msg}</option>}
+                {(msg) => (
+                  <option value="" disabled={true}>
+                    {msg}
+                  </option>
+                )}
               </FormattedMessage>
               <FormattedMessage id="global.no">
                 {(msg) => <option value="false">{msg}</option>}