Merge pull request #5177 from mozilla/complaint-feedback-type-mpp-3948

MPP-3948: `complaintFeedbackType` is not required

emails/tests/views_tests.py

@@ -1495,23 +1495,23 @@ def test_minimal_complaint(self):
         message = {
             "complaint": {
                 "complainedRecipients": [{"emailAddress": "[email protected]"}],
-                "complaintFeedbackType": "abuse",
             },
             "mail": {"commonHeaders": {"from": ["[email protected]"]}},
         }
-        complaint_data = _get_complaint_data(message)
+        with self.assertNoLogs(ERROR_LOG):
+            complaint_data = _get_complaint_data(message)
         assert complaint_data == RawComplaintData(
             complained_recipients=[("[email protected]", {})],
             from_addresses=["[email protected]"],
             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": ["[email protected]"]}},
         }
         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": ["[email protected]"]}},
         }
         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": ["[email protected]"]}},
         }
@@ -1562,7 +1561,6 @@ def test_no_mail_error_logged(self):
         message = {
             "complaint": {
                 "complainedRecipients": [{"emailAddress": "[email protected]"}],
-                "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": "[email protected]"}],
-                "complaintFeedbackType": "abuse",
             },
             "mail": {},
         }
@@ -1597,7 +1594,6 @@ def test_no_from_header_error_logged(self):
         message = {
             "complaint": {
                 "complainedRecipients": [{"emailAddress": "[email protected]"}],
-                "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": "[email protected]"}],
             },
             "mail": {"commonHeaders": {"from": ["[email protected]"]}},
         }
-        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):
     """

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