Fix to handle_complaints.

The json we were getting from SES was not quite as expected, the test data now reflects what we get.
New test added, fix a test that was passing regardless.

app/notifications/notifications_ses_callback.py

@@ -111,7 +111,6 @@ def remove_emails_from_bounce(bounce_dict):
 def handle_complaint(ses_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_message['mail']['messageId']
     except KeyError as e:
@@ -132,6 +131,7 @@ def handle_complaint(ses_message):
 
 def remove_emails_from_complaint(complaint_dict):
     complaint_dict['complaint'].pop('complainedRecipients')
+    complaint_dict['mail'].pop('destination')
 
 
 def _check_and_queue_callback_task(notification):

tests/app/celery/test_process_ses_receipts_tasks.py

@@ -50,4 +50,4 @@ def test_process_ses_results_in_complaint(sample_email_template, mocker):
 def test_remove_emails_from_complaint():
     test_json = json.loads(ses_complaint_callback()['Message'])
     remove_emails_from_complaint(test_json)
-    assert "recipient1@example.com" not in test_json
+    assert "recipient1@example.com" not in json.dumps(test_json)

tests/app/db.py

@@ -1,4 +1,3 @@
-import json
 from datetime import datetime, date
 import uuid
 
@@ -592,10 +591,11 @@ def ses_complaint_callback_malformed_message_id():
         'UnsubscribeUrl': 'https://sns.eu-west-1.amazonaws.com',
         'TopicArn': 'arn:ses_notifications', 'Type': 'Notification',
         'Timestamp': '2018-06-05T14:00:15.952Z', 'Subject': None,
-        'Message': '{"notificationType":"Complaint","complaint":{"complainedRecipients":[{"emailAddress":"someone@hotmail.com"}],"timestamp":"2018-06-05T13:59:58.000Z","feedbackId":"ses_feedback_id"},"mail":{"timestamp":"2018-06-05T14:00:15.950Z","source":"\\"Some Service\\" <someservicenotifications.service.gov.uk>","sourceArn":"arn:identity/notifications.service.gov.uk","sourceIp":"52.208.24.161","sendingAccountId":"888450439860","badMessageId":"ref1","destination":["someone@hotmail.com"]}}',  # noqa
+        'Message': '{"notificationType":"Complaint","complaint":{"complainedRecipients":[{"emailAddress":"recipient1@example.com"}],"timestamp":"2018-06-05T13:59:58.000Z","feedbackId":"ses_feedback_id"},"mail":{"timestamp":"2018-06-05T14:00:15.950Z","source":"\\"Some Service\\" <someservicenotifications.service.gov.uk>","sourceArn":"arn:identity/notifications.service.gov.uk","sourceIp":"52.208.24.161","sendingAccountId":"888450439860","badMessageId":"ref1","destination":["recipient1@example.com"]}}',  # noqa
         'SigningCertUrl': 'https://sns.pem'
     }
 
+
 def ses_complaint_callback_with_missing_complaint_type():
     """
     https://docs.aws.amazon.com/ses/latest/DeveloperGuide/notification-contents.html#complaint-object
@@ -606,10 +606,11 @@ def ses_complaint_callback_with_missing_complaint_type():
         'UnsubscribeUrl': 'https://sns.eu-west-1.amazonaws.com',
         'TopicArn': 'arn:ses_notifications', 'Type': 'Notification',
         'Timestamp': '2018-06-05T14:00:15.952Z', 'Subject': None,
-        'Message': '{"notificationType":"Complaint","complaint":{"complainedRecipients":[{"emailAddress":"someone@hotmail.com"}],"timestamp":"2018-06-05T13:59:58.000Z","feedbackId":"ses_feedback_id"},"mail":{"timestamp":"2018-06-05T14:00:15.950Z","source":"\\"Some Service\\" <someservicenotifications.service.gov.uk>","sourceArn":"arn:identity/notifications.service.gov.uk","sourceIp":"52.208.24.161","sendingAccountId":"888450439860","messageId":"ref1","destination":["someone@hotmail.com"]}}',  # noqa
+        'Message': '{"notificationType":"Complaint","complaint":{"complainedRecipients":[{"emailAddress":"recipient1@example.com"}],"timestamp":"2018-06-05T13:59:58.000Z","feedbackId":"ses_feedback_id"},"mail":{"timestamp":"2018-06-05T14:00:15.950Z","source":"\\"Some Service\\" <someservicenotifications.service.gov.uk>","sourceArn":"arn:identity/notifications.service.gov.uk","sourceIp":"52.208.24.161","sendingAccountId":"888450439860","messageId":"ref1","destination":["recipient1@example.com"]}}',  # noqa
         'SigningCertUrl': 'https://sns.pem'
     }
 
+
 def ses_complaint_callback():
     """
     https://docs.aws.amazon.com/ses/latest/DeveloperGuide/notification-contents.html#complaint-object
@@ -620,7 +621,7 @@ def ses_complaint_callback():
         'UnsubscribeUrl': 'https://sns.eu-west-1.amazonaws.com',
         'TopicArn': 'arn:ses_notifications', 'Type': 'Notification',
         'Timestamp': '2018-06-05T14:00:15.952Z', 'Subject': None,
-        'Message': '{"notificationType":"Complaint","complaint":{"complaintFeedbackType": "abuse", "complainedRecipients":[{"emailAddress":"someone@hotmail.com"}],"timestamp":"2018-06-05T13:59:58.000Z","feedbackId":"ses_feedback_id"},"mail":{"timestamp":"2018-06-05T14:00:15.950Z","source":"\\"Some Service\\" <someservicenotifications.service.gov.uk>","sourceArn":"arn:identity/notifications.service.gov.uk","sourceIp":"52.208.24.161","sendingAccountId":"888450439860","messageId":"ref1","destination":["someone@hotmail.com"]}}',  # noqa
+        'Message': '{"notificationType":"Complaint","complaint":{"complaintFeedbackType": "abuse", "complainedRecipients":[{"emailAddress":"recipient1@example.com"}],"timestamp":"2018-06-05T13:59:58.000Z","feedbackId":"ses_feedback_id"},"mail":{"timestamp":"2018-06-05T14:00:15.950Z","source":"\\"Some Service\\" <someservicenotifications.service.gov.uk>","sourceArn":"arn:identity/notifications.service.gov.uk","sourceIp":"52.208.24.161","sendingAccountId":"888450439860","messageId":"ref1","destination":["recipient1@example.com"]}}',  # noqa
         'SigningCertUrl': 'https://sns.pem'
     }
 

tests/app/notifications/test_notifications_ses_callback.py

@@ -1,7 +1,9 @@
 from datetime import datetime
 
+import pytest
 from flask import json
 from freezegun import freeze_time
+from sqlalchemy.exc import SQLAlchemyError
 
 from app import statsd_client
 from app.dao.notifications_dao import get_notification_by_id
@@ -201,7 +203,7 @@ def test_remove_emails_from_bounce():
 
 def test_process_ses_results_in_complaint(sample_email_template):
     notification = create_notification(template=sample_email_template, reference='ref1')
-    handle_complaint(ses_complaint_callback())
+    handle_complaint(json.loads(ses_complaint_callback()['Message']))
     complaints = Complaint.query.all()
     assert len(complaints) == 1
     assert complaints[0].notification_id == notification.id
@@ -213,6 +215,12 @@ def test_handle_complaint_does_not_raise_exception_if_reference_is_missing(notif
     assert len(Complaint.query.all()) == 0
 
 
+def test_handle_complaint_does_raise_exception_if_notification_not_found(notify_api):
+    response = json.loads(ses_complaint_callback()['Message'])
+    with pytest.raises(expected_exception=SQLAlchemyError):
+        handle_complaint(response)
+
+
 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')
     msg = json.loads(ses_complaint_callback_with_missing_complaint_type()['Message'])