diff --git a/emails/tests/views_tests.py b/emails/tests/views_tests.py index bd781e2052..55f42d422d 100644 --- a/emails/tests/views_tests.py +++ b/emails/tests/views_tests.py @@ -1495,23 +1495,23 @@ def test_minimal_complaint(self): message = { "complaint": { "complainedRecipients": [{"emailAddress": "complainer@example.com"}], - "complaintFeedbackType": "abuse", }, "mail": {"commonHeaders": {"from": ["relay-mask@test.com"]}}, } - complaint_data = _get_complaint_data(message) + with self.assertNoLogs(ERROR_LOG): + complaint_data = _get_complaint_data(message) assert complaint_data == RawComplaintData( complained_recipients=[("complainer@example.com", {})], from_addresses=["relay-mask@test.com"], subtype="", user_agent="", - feedback_type="abuse", + feedback_type="", ) def test_no_complained_recipients_error_logged(self): """When complaint.complainedRecipients is missing, an error is logged.""" message = { - "complaint": {"complaintFeedbackType": "abuse"}, + "complaint": {}, "mail": {"commonHeaders": {"from": ["relay-mask@test.com"]}}, } with self.assertLogs(ERROR_LOG) as error_logs: @@ -1521,12 +1521,12 @@ def test_no_complained_recipients_error_logged(self): (err_log,) = error_logs.records assert err_log.msg == "_get_complaint_data: Unexpected message format" assert getattr(err_log, "missing_key") == "complainedRecipients" - assert getattr(err_log, "found_keys") == "complaintFeedbackType" + assert getattr(err_log, "found_keys") == "" def test_empty_complained_recipients_error_logged(self): """When complaint.complainedRecipients is empty, an error is logged.""" message = { - "complaint": {"complainedRecipients": [], "complaintFeedbackType": "abuse"}, + "complaint": {"complainedRecipients": []}, "mail": {"commonHeaders": {"from": ["relay-mask@test.com"]}}, } with self.assertLogs(ERROR_LOG) as error_logs: @@ -1544,7 +1544,6 @@ def test_wrong_complained_recipients_error_logged(self): message = { "complaint": { "complainedRecipients": [{"foo": "bar"}], - "complaintFeedbackType": "abuse", }, "mail": {"commonHeaders": {"from": ["relay-mask@test.com"]}}, } @@ -1562,7 +1561,6 @@ def test_no_mail_error_logged(self): message = { "complaint": { "complainedRecipients": [{"emailAddress": "complainer@example.com"}], - "complaintFeedbackType": "abuse", }, } with self.assertLogs(ERROR_LOG) as error_logs: @@ -1579,7 +1577,6 @@ def test_no_common_headers_error_logged(self): message = { "complaint": { "complainedRecipients": [{"emailAddress": "complainer@example.com"}], - "complaintFeedbackType": "abuse", }, "mail": {}, } @@ -1597,7 +1594,6 @@ def test_no_from_header_error_logged(self): message = { "complaint": { "complainedRecipients": [{"emailAddress": "complainer@example.com"}], - "complaintFeedbackType": "abuse", }, "mail": {"commonHeaders": {}}, } @@ -1610,23 +1606,18 @@ def test_no_from_header_error_logged(self): assert getattr(err_log, "missing_key") == "from" assert getattr(err_log, "found_keys") == "" - def test_no_feedback_type_error_logged(self): - """If the From header entry is missing, an error is logged.""" + def test_no_feedback_type_not_an_error(self): + """If the feedback type is missing, no error is logged""" message = { "complaint": { "complainedRecipients": [{"emailAddress": "complainer@example.com"}], }, "mail": {"commonHeaders": {"from": ["relay-mask@test.com"]}}, } - with self.assertLogs(ERROR_LOG) as error_logs: + with self.assertNoLogs(ERROR_LOG): complaint_data = _get_complaint_data(message) assert complaint_data.feedback_type == "" - (err_log,) = error_logs.records - assert err_log.msg == "_get_complaint_data: Unexpected message format" - assert getattr(err_log, "missing_key") == "complaintFeedbackType" - assert getattr(err_log, "found_keys") == "complainedRecipients" - class GatherComplainersTest(TestCase): """ diff --git a/emails/views.py b/emails/views.py index 0dc1b91812..9f1759f79c 100644 --- a/emails/views.py +++ b/emails/views.py @@ -1866,8 +1866,8 @@ def get_or_log( raw_from_addresses = [] from_addresses = [parseaddr(addr)[1] for addr in raw_from_addresses] - feedback_type, _ = get_or_log("complaintFeedbackType", complaint, str) - + # Only present when set + feedback_type = complaint.get("complaintFeedbackType", "") # Only present when destination is on account suppression list subtype = complaint.get("complaintSubType", "") # Only present for feedback reports