When SES callback is for a complaint save that to the new complaints table.

When handling the complaint we don't want to throw an exception if the message is missing fields. Only log an exception if we are unable to tie a complaint to a notification.

app/dao/complaint_dao.py

@@ -0,0 +1,12 @@
+from app import db
+from app.dao.dao_utils import transactional
+from app.models import Complaint
+
+
+@transactional
+def save_complaint(complaint):
+    db.session.add(complaint)
+
+
+def fetch_complaints_by_service(service_id):
+    return Complaint.query.filter_by(service_id=service_id).all()

app/notifications/notifications_ses_callback.py

@@ -10,7 +10,10 @@ from app.clients.email.aws_ses import get_aws_responses
 from app.dao import (
     notifications_dao
 )
+from app.dao.complaint_dao import save_complaint
+from app.dao.notifications_dao import dao_get_notification_history_by_reference
 from app.dao.service_callback_api_dao import get_service_callback_api_for_service
+from app.models import Complaint
 from app.notifications.process_client_response import validate_callback_data
 from app.celery.service_callback_tasks import (
     send_delivery_status_to_service,
@@ -35,10 +38,7 @@ def process_ses_response(ses_request):
         if notification_type == 'Bounce':
             notification_type = determine_notification_bounce_type(notification_type, ses_message)
         elif notification_type == 'Complaint':
-            # Complaints are going to be stored in a table of it's own,
+            handle_complaint(ses_request)
-            # this will no longer update the status of a notification as it does now.
-            remove_emails_from_complaint(ses_message)
-            current_app.logger.info("Complaint from SES: \n{}".format(ses_message))
             return
 
         try:
@@ -108,6 +108,29 @@ def remove_emails_from_bounce(bounce_dict):
         recip.pop('emailAddress')
 
 
+def handle_complaint(ses_request):
+    ses_message = json.loads(ses_request['Message'])
+    remove_emails_from_complaint(ses_message)
+    current_app.logger.info("Complaint from SES: \n{}".format(ses_message))
+    # It is possible that the we get a key error, let this fail so we can investigate.
+    try:
+        reference = ses_request['MessageId']
+    except KeyError as e:
+        current_app.logger.exception("Complaint from SES failed to get reference from message", e)
+        return
+    notification = dao_get_notification_history_by_reference(reference)
+    ses_complaint = ses_message.get('complaint', None)
+
+    complaint = Complaint(
+        notification_id=notification.id,
+        service_id=notification.service_id,
+        ses_feedback_id=ses_complaint.get('feedbackId', None) if ses_complaint else None,
+        complaint_type=ses_complaint.get('complaintFeedbackType', None) if ses_complaint else None,
+        complaint_date=ses_complaint.get('timestamp', None) if ses_complaint else None
+    )
+    save_complaint(complaint)
+
+
 def remove_emails_from_complaint(complaint_dict):
     complaint_dict['complaint'].pop('complainedRecipients')
 

tests/app/celery/test_process_ses_receipts_tasks.py

@@ -2,9 +2,13 @@ import json
 from datetime import datetime
 
 from app.celery.process_ses_receipts_tasks import process_ses_results
+from app.models import Complaint
 from app.notifications.notifications_ses_callback import remove_emails_from_complaint
 
-from tests.app.db import create_notification
+from tests.app.db import (
+    create_notification, ses_complaint_callback,
+    ses_notification_callback
+)
 
 
 def test_process_ses_results(sample_email_template):
@@ -33,11 +37,15 @@ def test_process_ses_results_retry_called(notify_db, mocker):
     assert mocked.call_count != 0
 
 
-def test_process_ses_results_in_complaint(notify_db, mocker):
+def test_process_ses_results_in_complaint(sample_email_template, mocker):
+    notification = create_notification(template=sample_email_template, reference='ref1')
     mocked = mocker.patch("app.dao.notifications_dao.update_notification_status_by_reference")
     response = json.loads(ses_complaint_callback())
     process_ses_results(response=response)
     assert mocked.call_count == 0
+    complaints = Complaint.query.all()
+    assert len(complaints) == 1
+    assert complaints[0].notification_id == notification.id
 
 
 def test_remove_emails_from_complaint():
@@ -45,44 +53,3 @@ def test_remove_emails_from_complaint():
     test_json = json.loads(json.loads(test_message)['Message'])
     remove_emails_from_complaint(test_json)
     assert "recipient1@example.com" not in test_json
-
-
-def ses_notification_callback():
-    return '{\n  "Type" : "Notification",\n  "MessageId" : "ref1",' \
-           '\n  "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",' \
-           '\n  "Message" : "{\\"notificationType\\":\\"Delivery\\",' \
-           '\\"mail\\":{\\"timestamp\\":\\"2016-03-14T12:35:25.909Z\\",' \
-           '\\"source\\":\\"test@test-domain.com\\",' \
-           '\\"sourceArn\\":\\"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify\\",' \
-           '\\"sendingAccountId\\":\\"123456789012\\",' \
-           '\\"messageId\\":\\"ref1\\",' \
-           '\\"destination\\":[\\"testing@digital.cabinet-office.gov.uk\\"]},' \
-           '\\"delivery\\":{\\"timestamp\\":\\"2016-03-14T12:35:26.567Z\\",' \
-           '\\"processingTimeMillis\\":658,' \
-           '\\"recipients\\":[\\"testing@digital.cabinet-office.gov.uk\\"],' \
-           '\\"smtpResponse\\":\\"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp\\",' \
-           '\\"reportingMTA\\":\\"a6-238.smtp-out.eu-west-1.amazonses.com\\"}}",' \
-           '\n  "Timestamp" : "2016-03-14T12:35:26.665Z",\n  "SignatureVersion" : "1",' \
-           '\n  "Signature" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUt' \
-           'OowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYL' \
-           'VSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMA' \
-           'PmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==",' \
-           '\n  "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750' \
-           'dd426d95ee9390147a5624348ee.pem",' \
-           '\n  "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&S' \
-           'ubscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"\n}'
-
-
-def ses_complaint_callback():
-    """
-    https://docs.aws.amazon.com/ses/latest/DeveloperGuide/notification-contents.html#complaint-object
-    """
-    return '{\n  "Type" : "Notification",\n  "MessageId" : "ref1",' \
-           '\n  "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",' \
-           '\n  "Message" : "{\\"notificationType\\":\\"Complaint\\",' \
-           '\\"complaint\\": {\\"userAgent\\":\\"AnyCompany Feedback Loop (V0.01)\\",' \
-           '\\"complainedRecipients\\":[{\\"emailAddress\\":\\"recipient1@example.com\\"}],' \
-           '\\"complaintFeedbackType\\":\\"abuse\\", ' \
-           '\\"arrivalDate\\":\\"2009-12-03T04:24:21.000-05:00\\", ' \
-           '\\"timestamp\\":\\"2012-05-25T14:59:38.623Z\\", ' \
-           '\\"feedbackId\\":\\"someSESID\\"}}"\n}'

tests/app/dao/test_complaint_dao.py

@@ -0,0 +1,64 @@
+import uuid
+
+from datetime import datetime
+
+from app.dao.complaint_dao import save_complaint, fetch_complaints_by_service
+from app.models import Complaint
+from tests.app.db import create_service, create_template, create_notification
+
+
+def test_fetch_complaint_by_service_returns_one(sample_service, sample_email_notification):
+    complaint = Complaint(notification_id=sample_email_notification.id,
+                          service_id=sample_service.id,
+                          ses_feedback_id=str(uuid.uuid4()),
+                          complaint_type='abuse',
+                          complaint_date=datetime.utcnow()
+                          )
+
+    save_complaint(complaint)
+
+    complaints = fetch_complaints_by_service(service_id=sample_service.id)
+    assert len(complaints) == 1
+    assert complaints[0] == complaint
+
+
+def test_fetch_complaint_by_service_returns_empty_list(sample_service, sample_email_notification):
+    complaints = fetch_complaints_by_service(service_id=sample_service.id)
+    assert len(complaints) == 0
+
+
+def test_fetch_complaint_by_service_return_many(notify_db_session):
+    service_1 = create_service(service_name='first')
+    service_2 = create_service(service_name='second')
+    template_1 = create_template(service=service_1, template_type='email')
+    template_2 = create_template(service=service_2, template_type='email')
+    notification_1 = create_notification(template=template_1)
+    notification_2 = create_notification(template=template_2)
+    notification_3 = create_notification(template=template_2)
+    complaint_1 = Complaint(notification_id=notification_1.id,
+                            service_id=service_1.id,
+                            ses_feedback_id=str(uuid.uuid4()),
+                            complaint_type='abuse',
+                            complaint_date=datetime.utcnow()
+                            )
+    complaint_2 = Complaint(notification_id=notification_2.id,
+                            service_id=service_2.id,
+                            ses_feedback_id=str(uuid.uuid4()),
+                            complaint_type='abuse',
+                            complaint_date=datetime.utcnow()
+                            )
+    complaint_3 = Complaint(notification_id=notification_3.id,
+                            service_id=service_2.id,
+                            ses_feedback_id=str(uuid.uuid4()),
+                            complaint_type='abuse',
+                            complaint_date=datetime.utcnow()
+                            )
+
+    save_complaint(complaint_1)
+    save_complaint(complaint_2)
+    save_complaint(complaint_3)
+
+    complaints = fetch_complaints_by_service(service_id=service_2.id)
+    assert len(complaints) == 2
+    assert complaints[0] == complaint_2
+    assert complaints[1] == complaint_3

tests/app/db.py

@@ -562,3 +562,69 @@ def create_ft_billing(bst_date,
     db.session.add(data)
     db.session.commit()
     return data
+
+
+def ses_complaint_callback_malformed_message_id():
+    return '{\n  "Type" : "Notification",\n  "msgId" : "ref1",' \
+           '\n  "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",' \
+           '\n  "Message" : "{\\"notificationType\\":\\"Complaint\\",' \
+           '\\"complaint\\": {\\"userAgent\\":\\"AnyCompany Feedback Loop (V0.01)\\",' \
+           '\\"complainedRecipients\\":[{\\"emailAddress\\":\\"recipient1@example.com\\"}],' \
+           '\\"arrivalDate\\":\\"2009-12-03T04:24:21.000-05:00\\", ' \
+           '\\"timestamp\\":\\"2012-05-25T14:59:38.623Z\\", ' \
+           '\\"feedbackId\\":\\"someSESID\\"}}"\n}'
+
+
+def ses_complaint_callback_with_missing_complaint_type():
+    """
+    https://docs.aws.amazon.com/ses/latest/DeveloperGuide/notification-contents.html#complaint-object
+    """
+    return '{\n  "Type" : "Notification",\n  "MessageId" : "ref1",' \
+           '\n  "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",' \
+           '\n  "Message" : "{\\"notificationType\\":\\"Complaint\\",' \
+           '\\"complaint\\": {\\"userAgent\\":\\"AnyCompany Feedback Loop (V0.01)\\",' \
+           '\\"complainedRecipients\\":[{\\"emailAddress\\":\\"recipient1@example.com\\"}],' \
+           '\\"arrivalDate\\":\\"2009-12-03T04:24:21.000-05:00\\", ' \
+           '\\"timestamp\\":\\"2012-05-25T14:59:38.623Z\\", ' \
+           '\\"feedbackId\\":\\"someSESID\\"}}"\n}'
+
+
+def ses_complaint_callback():
+    """
+    https://docs.aws.amazon.com/ses/latest/DeveloperGuide/notification-contents.html#complaint-object
+    """
+    return '{\n  "Type" : "Notification",\n  "MessageId" : "ref1",' \
+           '\n  "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",' \
+           '\n  "Message" : "{\\"notificationType\\":\\"Complaint\\",' \
+           '\\"complaint\\": {\\"userAgent\\":\\"AnyCompany Feedback Loop (V0.01)\\",' \
+           '\\"complainedRecipients\\":[{\\"emailAddress\\":\\"recipient1@example.com\\"}],' \
+           '\\"complaintFeedbackType\\":\\"abuse\\", ' \
+           '\\"arrivalDate\\":\\"2009-12-03T04:24:21.000-05:00\\", ' \
+           '\\"timestamp\\":\\"2012-05-25T14:59:38.623Z\\", ' \
+           '\\"feedbackId\\":\\"someSESID\\"}}"\n}'
+
+
+def ses_notification_callback():
+    return '{\n  "Type" : "Notification",\n  "MessageId" : "ref1",' \
+           '\n  "TopicArn" : "arn:aws:sns:eu-west-1:123456789012:testing",' \
+           '\n  "Message" : "{\\"notificationType\\":\\"Delivery\\",' \
+           '\\"mail\\":{\\"timestamp\\":\\"2016-03-14T12:35:25.909Z\\",' \
+           '\\"source\\":\\"test@test-domain.com\\",' \
+           '\\"sourceArn\\":\\"arn:aws:ses:eu-west-1:123456789012:identity/testing-notify\\",' \
+           '\\"sendingAccountId\\":\\"123456789012\\",' \
+           '\\"messageId\\":\\"ref1\\",' \
+           '\\"destination\\":[\\"testing@digital.cabinet-office.gov.uk\\"]},' \
+           '\\"delivery\\":{\\"timestamp\\":\\"2016-03-14T12:35:26.567Z\\",' \
+           '\\"processingTimeMillis\\":658,' \
+           '\\"recipients\\":[\\"testing@digital.cabinet-office.gov.uk\\"],' \
+           '\\"smtpResponse\\":\\"250 2.0.0 OK 1457958926 uo5si26480932wjc.221 - gsmtp\\",' \
+           '\\"reportingMTA\\":\\"a6-238.smtp-out.eu-west-1.amazonses.com\\"}}",' \
+           '\n  "Timestamp" : "2016-03-14T12:35:26.665Z",\n  "SignatureVersion" : "1",' \
+           '\n  "Signature" : "X8d7eTAOZ6wlnrdVVPYanrAlsX0SMPfOzhoTEBnQqYkrNWTqQY91C0f3bxtPdUhUt' \
+           'OowyPAOkTQ4KnZuzphfhVb2p1MyVYMxNKcBFB05/qaCX99+92fjw4x9LeUOwyGwMv5F0Vkfi5qZCcEw69uVrhYL' \
+           'VSTFTrzi/yCtru+yFULMQ6UhbY09GwiP6hjxZMVr8aROQy5lLHglqQzOuSZ4KeD85JjifHdKzlx8jjQ+uj+FLzHXPMA' \
+           'PmPU1JK9kpoHZ1oPshAFgPDpphJe+HwcJ8ezmk+3AEUr3wWli3xF+49y8Z2anASSVp6YI2YP95UT8Rlh3qT3T+V9V8rbSVislxA==",' \
+           '\n  "SigningCertURL" : "https://sns.eu-west-1.amazonaws.com/SimpleNotificationService-bb750' \
+           'dd426d95ee9390147a5624348ee.pem",' \
+           '\n  "UnsubscribeURL" : "https://sns.eu-west-1.amazonaws.com/?Action=Unsubscribe&S' \
+           'subscriptionArn=arn:aws:sns:eu-west-1:302763885840:preview-emails:d6aad3ef-83d6-4cf3-a470-54e2e75916da"\n}'

tests/app/notifications/test_notifications_ses_callback.py

@@ -5,13 +5,20 @@ from freezegun import freeze_time
 
 from app import statsd_client
 from app.dao.notifications_dao import get_notification_by_id
-from app.models import Notification
+from app.models import Notification, Complaint
-from app.notifications.notifications_ses_callback import process_ses_response, remove_emails_from_bounce
+from app.notifications.notifications_ses_callback import (
+    process_ses_response, remove_emails_from_bounce,
+    handle_complaint
+)
 from app.celery.research_mode_tasks import ses_hard_bounce_callback, ses_soft_bounce_callback, ses_notification_callback
 from app.celery.service_callback_tasks import create_encrypted_callback_data
 
 from tests.app.conftest import sample_notification as create_sample_notification
-from tests.app.db import create_service_callback_api
+from tests.app.db import (
+    create_service_callback_api, create_notification, ses_complaint_callback_malformed_message_id,
+    ses_complaint_callback_with_missing_complaint_type,
+    ses_complaint_callback
+)
 
 
 def test_ses_callback_should_update_notification_status(
@@ -190,3 +197,28 @@ def test_remove_emails_from_bounce():
     remove_emails_from_bounce(message_dict['bounce'])
 
     assert 'not-real@gmail.com' not in json.dumps(message_dict)
+
+
+def test_process_ses_results_in_complaint(sample_email_template):
+    notification = create_notification(template=sample_email_template, reference='ref1')
+    response = json.loads(ses_complaint_callback())
+    handle_complaint(response)
+    complaints = Complaint.query.all()
+    assert len(complaints) == 1
+    assert complaints[0].notification_id == notification.id
+
+
+def test_handle_complaint_does_not_raise_exception_if_reference_is_missing(notify_api):
+    response = json.loads(ses_complaint_callback_malformed_message_id())
+    handle_complaint(response)
+    assert len(Complaint.query.all()) == 0
+
+
+def test_process_ses_results_in_complaint_save_complaint_with_null_complaint_type(notify_api, sample_email_template):
+    notification = create_notification(template=sample_email_template, reference='ref1')
+    response = json.loads(ses_complaint_callback_with_missing_complaint_type())
+    handle_complaint(response)
+    complaints = Complaint.query.all()
+    assert len(complaints) == 1
+    assert complaints[0].notification_id == notification.id
+    assert not complaints[0].complaint_type