Fix #562: add subscribed field to newsletters Acoustic relational table

ctms/acoustic_service.py

@@ -290,8 +290,6 @@ def _newsletter_converter(self, acoustic_main_table, contact, newsletters_mappin
         newsletter_rows = []
         contact_newsletters: List[NewsletterSchema] = contact.newsletters
         contact_email_id = str(contact.email.email_id)
-        contact_email_format = contact.email.email_format
-        contact_email_lang = contact.email.email_lang
         skipped = []
 
         # populate with all the sub_flags set to false
@@ -300,27 +298,20 @@ def _newsletter_converter(self, acoustic_main_table, contact, newsletters_mappin
             acoustic_main_table[sub_flag] = "0"
 
         for newsletter in contact_newsletters:
-            newsletter_template = {
+            newsletter_row = {
                 "email_id": contact_email_id,
-                "newsletter_format": contact_email_format,
-                "newsletter_lang": contact_email_lang,
+                "newsletter_name": newsletter.name,
+                "newsletter_source": newsletter.source and str(newsletter.source),
+                "create_timestamp": newsletter.create_timestamp.date().isoformat(),
+                "update_timestamp": newsletter.update_timestamp.date().isoformat(),
+                "newsletter_format": newsletter.format,
+                "newsletter_lang": newsletter.lang,
+                "subscribed": newsletter.subscribed,
+                "newsletter_unsub_reason": newsletter.unsub_reason,
             }
+            newsletter_rows.append(newsletter_row)
 
             if newsletter.name in newsletters_mapping:
-                newsletter_template[
-                    "create_timestamp"
-                ] = newsletter.create_timestamp.date().isoformat()
-                newsletter_template[
-                    "update_timestamp"
-                ] = newsletter.update_timestamp.date().isoformat()
-                newsletter_template["newsletter_name"] = newsletter.name
-                newsletter_template["newsletter_unsub_reason"] = newsletter.unsub_reason
-                _source = newsletter.source
-                if _source is not None:
-                    _source = str(_source)
-                newsletter_template["newsletter_source"] = _source
-
-                newsletter_rows.append(newsletter_template)
                 # and finally flip the main table's sub_<newsletter> flag to true for each subscription
                 if newsletter.subscribed:
                     acoustic_main_table[newsletters_mapping[newsletter.name]] = "1"

tests/unit/test_acoustic_service.py

@@ -82,37 +82,59 @@ def test_ctms_to_acoustic_no_product(
 
 
 def test_ctms_to_acoustic_newsletters(
+    dbsession,
     base_ctms_acoustic_service,
-    minimal_contact,
+    email_factory,
     main_acoustic_fields,
     waitlist_acoustic_fields,
-    acoustic_newsletters_mapping,
 ):
-    main, newsletters, _, _ = base_ctms_acoustic_service.convert_ctms_to_acoustic(
-        minimal_contact,
+    email = email_factory(newsletters=5)
+    email.newsletters[2].subscribed = False
+    email.newsletters[2].unsub_reason = "not interested"
+    dbsession.commit()
+
+    contact = ContactSchema.from_email(email)
+
+    acoustic_newsletters_mapping = {
+        contact.newsletters[0].name: "sub_column_1",
+        contact.newsletters[1].name: "sub_column_2",
+        contact.newsletters[2].name: "sub_column_3",
+    }
+
+    (main, newsletter_rows, _, _) = base_ctms_acoustic_service.convert_ctms_to_acoustic(
+        contact,
         main_acoustic_fields,
         waitlist_acoustic_fields,
         acoustic_newsletters_mapping,
     )
 
-    assert len(minimal_contact.newsletters) == 4
-    # Newsletters records
-    assert len(newsletters) == 2
-    app_dev, mofo = newsletters
-    assert (
-        app_dev["email_id"] == mofo["email_id"] == str(minimal_contact.email.email_id)
+    # As many records in the newsletters relation table as in contact
+    assert len(newsletter_rows) == len(contact.newsletters)
+    # `newsletter_name` for all names
+    assert sorted(nl["newsletter_name"] for nl in newsletter_rows) == sorted(
+        nl.name for nl in contact.newsletters
+    )
+    # `email_id` column
+    assert all(nl["email_id"] == str(contact.email.email_id) for nl in newsletter_rows)
+    # `source` column
+    assert sorted(nl["newsletter_source"] for nl in newsletter_rows) == sorted(
+        nl.source for nl in contact.newsletters
+    )
+    # `subscribed` column
+    assert newsletter_rows[1]["subscribed"]
+    assert not newsletter_rows[2]["subscribed"]
+    assert newsletter_rows[3]["subscribed"]
+    # Newsletters in mapping are marked as subscribed in main table
+    assert main["sub_column_1"] == "1"
+    assert main["sub_column_2"] == "1"
+    assert main["sub_column_3"] == "0"
+    # Newsletters not in mapping are listed as skipped
+    assert sorted(base_ctms_acoustic_service.context["newsletters_skipped"]) == sorted(
+        [
+            contact.newsletters[3].name,
+            contact.newsletters[4].name,
+        ]
     )
-    assert app_dev["newsletter_source"] == mofo["newsletter_source"] == None
-    assert app_dev["newsletter_name"] == "app-dev"
-    assert mofo["newsletter_name"] == "mozilla-foundation"
-    # Newsletters are marked as subscribed in main table
-    assert main["sub_apps_and_hacks"] == "1"
-    assert main["sub_mozilla_foundation"] == "1"
-    # Newsletters not in mapping are skipped
-    assert base_ctms_acoustic_service.context["newsletters_skipped"] == [
-        "maker-party",
-        "mozilla-learning-network",
-    ]
 
 
 def test_ctms_to_acoustic_newsletter_timestamps(
@@ -296,8 +318,6 @@ def test_ctms_to_acoustic_mocked(
             waitlist_acoustic_fields,
             acoustic_newsletters_mapping,
         )
-    acoustic_mock.add_recipient.assert_called()
-    acoustic_mock.insert_update_relational_table.assert_called()
 
     acoustic_mock.add_recipient.assert_called_with(
         list_id=CTMS_ACOUSTIC_MAIN_TABLE_ID,
@@ -323,7 +343,7 @@ def test_ctms_to_acoustic_mocked(
     expected_log.update(
         {
             "email_id": "67e52c77-950f-4f28-accb-bb3ea1a2c51a",
-            "newsletter_count": 5,
+            "newsletter_count": len(newsletter_rows),
             "newsletters_skipped": ["ambassadors", "firefox-os"],
         }
     )
@@ -527,15 +547,13 @@ def test_ctms_to_acoustic_traced_email(
         )
 
     assert len(caplog) == 1
-    expected_log = EXPECTED_LOG.copy()
-    expected_log.update(
-        {
-            "email_id": "332de237-cab7-4461-bcc3-48e68f42bd5c",
-            "newsletters_skipped": ["firefox-welcome", "mozilla-welcome"],
-            "trace": email,
-        }
-    )
-    assert caplog[0] == expected_log
+    assert caplog[0] == {
+        **EXPECTED_LOG,
+        "email_id": "332de237-cab7-4461-bcc3-48e68f42bd5c",
+        "newsletter_count": 2,
+        "newsletters_skipped": ["firefox-welcome", "mozilla-welcome"],
+        "trace": email,
+    }
 
 
 @pytest.mark.parametrize(