- If the task runs twice and the notification already exists ignore the primary key constraint.

- Remove prints
- Add some more tests
- Only allow the new method to run for emails

app/celery/tasks.py

@@ -18,7 +18,7 @@ from requests import (
     request,
     RequestException
 )
-from sqlalchemy.exc import SQLAlchemyError
+from sqlalchemy.exc import SQLAlchemyError, IntegrityError
 
 from app import (
     create_uuid,
@@ -301,12 +301,11 @@ def save_api_email(self,
 
     notification = encryption.decrypt(encrypted_notification)
     service = dao_fetch_service_by_id(notification['service_id'])
-    print(notification)
 
     try:
         current_app.logger.info(f"Persisting notification {notification['id']}")
 
-        saved_notification = persist_notification(
+        persist_notification(
             notification_id=notification["id"],
             template_id=notification['template_id'],
             template_version=notification['template_version'],
@@ -324,14 +323,16 @@ def save_api_email(self,
         )
 
         q = QueueNames.SEND_EMAIL if not service.research_mode else QueueNames.RESEARCH_MODE
-        print(q)
         provider_tasks.deliver_email.apply_async(
             [notification['id']],
             queue=q
         )
+        current_app.logger.info(f"Email {notification['id']} has been persisted.")
+    except IntegrityError:
+        current_app.logger.info(f"Email {notification['id']} already exists.")
 
-        current_app.logger.debug("Email {} created at {}".format(saved_notification.id, saved_notification.created_at))
     except SQLAlchemyError as e:
+
         try:
             self.retry(queue=QueueNames.RETRY, exc=e)
         except self.MaxRetriesExceededError:

app/config.py

@@ -31,7 +31,7 @@ class QueueNames(object):
     SMS_CALLBACKS = 'sms-callbacks'
     ANTIVIRUS = 'antivirus-tasks'
     SANITISE_LETTERS = 'sanitise-letter-tasks'
-    SAVE_API_EMAIL = 'save-api-email'
+    SAVE_API_EMAIL = 'save-api-email-tasks'
 
     @staticmethod
     def all_queues():

app/v2/notifications/post_notifications.py

@@ -203,7 +203,8 @@ def process_sms_or_email_notification(*, form, notification_type, api_key, templ
         simulated=simulated
     )
 
-    if str(service.id) == '539d63a1-701d-400d-ab11-f3ee2319d4d4' and api_key.key_type == KEY_TYPE_NORMAL:
+    if str(service.id) == '539d63a1-701d-400d-ab11-f3ee2319d4d4' and api_key.key_type == KEY_TYPE_NORMAL \
+       and notification_type == EMAIL_TYPE:
         # Put GOV.UK Email notifications onto a queue
         # To take the pressure off the db for API requests put the notification for our high volume service onto a queue
         # the task will then save the notification, then call send_notification_to_queue.

scripts/paas_app_wrapper.sh

@@ -51,7 +51,7 @@ case $NOTIFY_APP_NAME in
     ;;
   delivery-worker-save-api-notifications)
     exec scripts/run_app_paas.sh celery -A run_celery.notify_celery worker --loglevel=INFO --concurrency=11 \
-    -Q save-api-email 2> /dev/null
+    -Q save-api-email-tasks 2> /dev/null
     ;;
   delivery-celery-beat)
     exec scripts/run_app_paas.sh celery -A run_celery.notify_celery beat --loglevel=INFO

tests/app/celery/test_tasks.py

@@ -1698,3 +1698,41 @@ def test_save_api_email(sample_email_template, mocker):
     assert str(notifications[0].id) == data['id']
     assert notifications[0].created_at == datetime(2020, 3, 25, 14, 30)
     mock_send_email_to_provider.assert_called_once_with([data['id']], queue=QueueNames.SEND_EMAIL)
+
+
+@freeze_time('2020-03-25 14:30')
+def test_save_api_email_dont_retry_if_notification_already_exists(sample_email_template, mocker):
+    mock_send_email_to_provider = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
+    api_key = create_api_key(service=sample_email_template.service)
+    data = {
+        "id": str(uuid.uuid4()),
+        "template_id": str(sample_email_template.id),
+        "template_version": sample_email_template.version,
+        "to": "jane.citizen@example.com",
+        "service_id": str(sample_email_template.service_id),
+        "personalisation": None,
+        "notification_type": sample_email_template.template_type,
+        "api_key_id": str(api_key.id),
+        "key_type": api_key.key_type,
+        "client_reference": 'our email',
+        "reply_to_text": "our.email@gov.uk",
+        "document_download_count": 0,
+        "status": NOTIFICATION_CREATED,
+        "created_at": datetime.utcnow().strftime(DATETIME_FORMAT),
+    }
+
+    encrypted = encryption.encrypt(
+        data
+    )
+    assert len(Notification.query.all()) == 0
+    save_api_email(encrypted)
+    notifications = Notification.query.all()
+    assert len(notifications) == 1
+    # call the task again with the same notification
+    save_api_email(encrypted)
+    notifications = Notification.query.all()
+    assert len(notifications) == 1
+    assert str(notifications[0].id) == data['id']
+    assert notifications[0].created_at == datetime(2020, 3, 25, 14, 30)
+    # should only have sent the notification once.
+    mock_send_email_to_provider.assert_called_once_with([data['id']], queue=QueueNames.SEND_EMAIL)

tests/app/test_config.py

@@ -65,7 +65,7 @@ def test_cloudfoundry_config_has_different_defaults():
 def test_queue_names_all_queues_correct():
     # Need to ensure that all_queues() only returns queue names used in API
     queues = QueueNames.all_queues()
-    assert len(queues) == 14
+    assert len(queues) == 15
     assert set([
         QueueNames.PRIORITY,
         QueueNames.PERIODIC,
@@ -81,4 +81,5 @@ def test_queue_names_all_queues_correct():
         QueueNames.CALLBACKS,
         QueueNames.LETTERS,
         QueueNames.SMS_CALLBACKS,
+        QueueNames.SAVE_API_EMAIL
     ]) == set(queues)

tests/app/v2/notifications/test_post_notifications.py

@@ -976,4 +976,58 @@ def test_post_notifications_saves_email_to_queue(client, notify_db_session, mock
     assert json_resp['id']
     assert json_resp['content']['body'] == "Dear citizen, have a nice day"
     assert json_resp['template']['id'] == str(template.id)
-    save_email_task.assert_called_once_with([mock.ANY], queue='save-api-email')
+    save_email_task.assert_called_once_with([mock.ANY], queue='save-api-email-tasks')
+
+
+def test_post_notifications_doesnt_save_email_to_queue_for_test_emails(client, notify_db_session, mocker):
+    save_email_task = mocker.patch("app.celery.tasks.save_api_email.apply_async")
+    mocked_send_task = mocker.patch('app.celery.provider_tasks.deliver_email.apply_async')
+
+    service = create_service(service_id='539d63a1-701d-400d-ab11-f3ee2319d4d4', service_name='high volume service')
+    # create_api_key(service=service, key_type='test')
+    template = create_template(service=service, content='((message))', template_type=EMAIL_TYPE)
+    data = {
+        "email_address": "joe.citizen@example.com",
+        "template_id": template.id,
+        "personalisation": {"message": "Dear citizen, have a nice day"}
+    }
+    response = client.post(
+        path='/v2/notifications/email',
+        data=json.dumps(data),
+        headers=[('Content-Type', 'application/json'),
+                 create_authorization_header(service_id=service.id, key_type='test')]
+    )
+
+    json_resp = response.get_json()
+
+    assert response.status_code == 201
+    assert json_resp['id']
+    assert json_resp['content']['body'] == "Dear citizen, have a nice day"
+    assert json_resp['template']['id'] == str(template.id)
+    assert mocked_send_task.called
+    assert not save_email_task.called
+
+
+def test_post_notifications_doesnt_save_email_to_queue_for_sms(client, notify_db_session, mocker):
+    save_email_task = mocker.patch("app.celery.tasks.save_api_email.apply_async")
+    mocked_send_task = mocker.patch('app.celery.provider_tasks.deliver_sms.apply_async')
+
+    service = create_service(service_id='539d63a1-701d-400d-ab11-f3ee2319d4d4', service_name='high volume service')
+    template = create_template(service=service, content='((message))', template_type=SMS_TYPE)
+    data = {
+        "phone_number": '+447700900855',
+        "template_id": template.id,
+        "personalisation": {"message": "Dear citizen, have a nice day"}
+    }
+    response = client.post(
+        path='/v2/notifications/sms',
+        data=json.dumps(data),
+        headers=[('Content-Type', 'application/json'), create_authorization_header(service_id=service.id)]
+    )
+
+    json_resp = response.get_json()
+
+    assert response.status_code == 201
+    assert json_resp['id']
+    assert mocked_send_task.called
+    assert not save_email_task.called