diff --git a/ctms/acoustic_service.py b/ctms/acoustic_service.py index 36461848..feddcac5 100644 --- a/ctms/acoustic_service.py +++ b/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_ flag to true for each subscription if newsletter.subscribed: acoustic_main_table[newsletters_mapping[newsletter.name]] = "1" diff --git a/tests/unit/test_acoustic_service.py b/tests/unit/test_acoustic_service.py index 5956cc25..b6d80fe1 100644 --- a/tests/unit/test_acoustic_service.py +++ b/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(