From 055c3deb683923a161e3eb8c712b53a1a1378f61 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Thu, 17 Nov 2022 21:13:54 +0100 Subject: [PATCH] Ref #256: Move newsletters mapping to database (#456) * Read Acoustic Newsletters mapping from database (ref #256) * Mention command in docs * Use official return codes * Use os.EX_DATAERR instead of os.EX_NOTFOUND (available in Unix only) * Address code review comments * Address changes on the acoustic_fields command too * Adjust usage info in commands --- ctms/acoustic_service.py | 62 ++++++---------- ctms/bin/acoustic_fields.py | 45 ++++++------ ctms/bin/acoustic_newsletters_mapping.py | 66 +++++++++++++++++ ctms/crud.py | 57 +++++++++++++++ ctms/models.py | 7 ++ ctms/sync.py | 32 +++++++-- docs/deployment_guide.md | 16 +++-- ...6aa0e96_add_acoustic_newsletter_mapping.py | 70 +++++++++++++++++++ tests/unit/conftest.py | 13 ++-- tests/unit/test_acoustic_service.py | 39 ++++++++--- tests/unit/test_sync.py | 43 ++++++++---- 11 files changed, 344 insertions(+), 106 deletions(-) create mode 100644 ctms/bin/acoustic_newsletters_mapping.py create mode 100644 migrations/versions/20221115_f8b8f6aa0e96_add_acoustic_newsletter_mapping.py diff --git a/ctms/acoustic_service.py b/ctms/acoustic_service.py index ead94424..058efd39 100644 --- a/ctms/acoustic_service.py +++ b/ctms/acoustic_service.py @@ -58,31 +58,6 @@ def force_bytes(s, encoding="utf-8", strings_only=False, errors="strict"): class AcousticResources: - MAIN_TABLE_SUBSCR_FLAGS = { - # maps the Basket/CTMS newsletter name to the Acoustic name - "about-mozilla": "sub_about_mozilla", - "app-dev": "sub_apps_and_hacks", - "common-voice": "sub_common_voice", - "firefox-accounts-journey": "sub_firefox_accounts_journey", - "firefox-news": "sub_firefox_news", - "firefox-sweepstakes": "sub_firefox_sweepstakes", - "hubs": "sub_hubs", - "internet-health-report": "sub_internet_health_report", - "knowledge-is-power": "sub_knowledge_is_power", - "miti": "sub_miti", - "mixed-reality": "sub_mixed_reality", - "mozilla-and-you": "sub_firefox_news", - "mozilla-fellowship-awardee-alumni": "sub_mozilla_fellowship_awardee_alumni", - "mozilla-festival": "sub_mozilla_festival", - "mozilla-foundation": "sub_mozilla_foundation", - "mozilla-rally": "sub_rally", - "mozilla-technology": "sub_mozilla_technology", - "mozillians-nda": "sub_mozillians_nda", - "security-privacy-news": "sub_security_privacy_news", - "take-action-for-the-internet": "sub_take_action_for_the_internet", - "test-pilot": "sub_test_pilot", - } - SKIP_FIELDS = set( ( # Known skipped fields from CTMS @@ -159,21 +134,21 @@ def __init__( self.context: Dict[str, Union[str, int, List[str]]] = {} self.metric_service = metric_service - def convert_ctms_to_acoustic(self, contact: ContactSchema, main_fields: set[str]): + def convert_ctms_to_acoustic( + self, + contact: ContactSchema, + main_fields: set[str], + newsletters_mapping: dict[str, str], + ): acoustic_main_table = self._main_table_converter(contact, main_fields) newsletter_rows, acoustic_main_table = self._newsletter_converter( - acoustic_main_table, contact + acoustic_main_table, contact, newsletters_mapping ) product_rows = self._product_converter(contact) return acoustic_main_table, newsletter_rows, product_rows def _main_table_converter(self, contact, main_fields): - acoustic_main_table = { - # populate with all the sub_flags set to false - # they'll get set to true below, as-needed - v: "0" - for v in AcousticResources.MAIN_TABLE_SUBSCR_FLAGS.values() - } + acoustic_main_table = {} acceptable_subdicts = ["email", "amo", "fxa", "vpn_waitlist", "relay_waitlist"] special_cases = { ("fxa", "fxa_id"): "fxa_id", @@ -234,7 +209,7 @@ def fxa_created_date_string_to_datetime(self, inner_value): self.context["fxa_created_date_converted"] = "skipped" return inner_value - def _newsletter_converter(self, acoustic_main_table, contact): + def _newsletter_converter(self, acoustic_main_table, contact, newsletters_mapping): # create the RT rows for the newsletter table in acoustic newsletter_rows = [] contact_newsletters: List[NewsletterSchema] = contact.newsletters @@ -242,6 +217,12 @@ def _newsletter_converter(self, acoustic_main_table, contact): contact_email_format = contact.email.email_format contact_email_lang = contact.email.email_lang skipped = [] + + # populate with all the sub_flags set to false + # they'll get set to true below, as-needed + for sub_flag in newsletters_mapping.values(): + acoustic_main_table[sub_flag] = "0" + for newsletter in contact_newsletters: newsletter_template = { "email_id": contact_email_id, @@ -249,7 +230,7 @@ def _newsletter_converter(self, acoustic_main_table, contact): "newsletter_lang": contact_email_lang, } - if newsletter.name in AcousticResources.MAIN_TABLE_SUBSCR_FLAGS: + if newsletter.name in newsletters_mapping: newsletter_dict = newsletter.dict() _today = datetime.date.today().isoformat() newsletter_template["create_timestamp"] = newsletter_dict.get( @@ -268,9 +249,7 @@ def _newsletter_converter(self, acoustic_main_table, contact): 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[ - AcousticResources.MAIN_TABLE_SUBSCR_FLAGS[newsletter.name] - ] = "1" + acoustic_main_table[newsletters_mapping[newsletter.name]] = "1" else: skipped.append(newsletter.name) if skipped: @@ -418,7 +397,10 @@ def _insert_update_relational_table(self, table_name, rows): self.context[f"{table_name}_duration_s"] = duration_s def attempt_to_upload_ctms_contact( - self, contact: ContactSchema, main_fields: set[str] + self, + contact: ContactSchema, + main_fields: set[str], + newsletters_mapping: dict[str, str], ) -> bool: """ @@ -428,7 +410,7 @@ def attempt_to_upload_ctms_contact( self.context = {} try: main_table_data, nl_data, prod_data = self.convert_ctms_to_acoustic( - contact, main_fields + contact, main_fields, newsletters_mapping ) main_table_id = str(self.acoustic_main_table_id) email_id = main_table_data["email_id"] diff --git a/ctms/bin/acoustic_fields.py b/ctms/bin/acoustic_fields.py index 10d2ff36..f3e05a42 100644 --- a/ctms/bin/acoustic_fields.py +++ b/ctms/bin/acoustic_fields.py @@ -1,29 +1,35 @@ #!/usr/bin/env python3 -"""Manage Acoustic fields: add and remove from the db.""" +"""Manage Acoustic fields: add, list, and remove from the db.""" import argparse import os import sys from ctms import config +from ctms.crud import ( + create_acoustic_field, + delete_acoustic_field, + get_all_acoustic_fields, +) from ctms.database import get_db_engine -from ctms.models import AcousticField def main(dbsession, args=None) -> int: parser = argparse.ArgumentParser( - description=""" - Manage Acoustic fields - """ + description="Manage Acoustic fields", ) subparsers = parser.add_subparsers(dest="action") - parser_add = subparsers.add_parser("add") + parser_add = subparsers.add_parser( + "add", usage="""python acoustic_fields.py add "fxaid" """ + ) parser_add.add_argument("field") parser_add.add_argument( "--tablename", "-t", help="Acoustic table name", default="main" ) - parser_remove = subparsers.add_parser("remove") + parser_remove = subparsers.add_parser( + "remove", usage="""python acoustic_fields.py remove "fxaid" """ + ) parser_remove.add_argument("field") parser_remove.add_argument( "--tablename", @@ -42,37 +48,26 @@ def main(dbsession, args=None) -> int: args = parser.parse_args(args=args) if args.action == "add": - dbsession.merge(AcousticField(tablename=args.tablename, field=args.field)) - dbsession.commit() - print("Added.") + row = create_acoustic_field(dbsession, args.tablename, args.field) + print(f"Added '{row.tablename}.{row.field}'.") elif args.action == "remove": - row = ( - dbsession.query(AcousticField) - .filter( - AcousticField.tablename == args.tablename, - AcousticField.field == args.field, - ) - .one_or_none() - ) + row = delete_acoustic_field(dbsession, args.tablename, args.field) if not row: print(f"Unknown field '{args.tablename}.{args.field}'. Give up.") return os.EX_DATAERR - dbsession.delete(row) - dbsession.commit() - print("Removed.") + print(f"Removed '{row.tablename}.{row.field}'.") else: - entries = dbsession.query(AcousticField).all() + entries = get_all_acoustic_fields(dbsession) print("\n".join(sorted(f"- {e.tablename}.{e.field}" for e in entries))) return os.EX_OK if __name__ == "__main__": - # Get the database config_settings = config.Settings() engine, session_factory = get_db_engine(config_settings) session = session_factory() with engine.connect() as connection: - ret = main(session) # pylint:disable = invalid-name + return_code = main(session) # pylint:disable = invalid-name - sys.exit(ret) + sys.exit(return_code) diff --git a/ctms/bin/acoustic_newsletters_mapping.py b/ctms/bin/acoustic_newsletters_mapping.py new file mode 100644 index 00000000..ac6c7bfb --- /dev/null +++ b/ctms/bin/acoustic_newsletters_mapping.py @@ -0,0 +1,66 @@ +#!/usr/bin/env python3 +"""Manage Acoustic newsletters mapping: add, list, and remove from the db.""" + +import argparse +import os +import sys + +from ctms import config +from ctms.crud import ( + create_acoustic_newsletters_mapping, + delete_acoustic_newsletters_mapping, + get_all_acoustic_newsletters_mapping, +) +from ctms.database import get_db_engine + + +def main(dbsession, test_args=None) -> int: + parser = argparse.ArgumentParser( + description="Manage Acoustic Newsletter mapping", + ) + subparsers = parser.add_subparsers(dest="action") + parser_add = subparsers.add_parser( + "add", + usage="""python acoustic_newsletters_mapping.py add "test-pilot:sub_new_test_pilot" """, + ) + parser_add.add_argument( + "mapping", help="Add newsletter mapping specified as 'source:destination'" + ) + + parser_remove = subparsers.add_parser( + "remove", + usage="""python acoustic_newsletters_mapping.py remove "test-pilot" """, + ) + parser_remove.add_argument( + "source", help="Remove newsletter mapping with specified 'source'" + ) + + subparsers.add_parser("list") + + args = parser.parse_args(args=test_args) + if args.action == "add": + source, destination = args.mapping.split(":") + # This will fail if mapping already exists. + create_acoustic_newsletters_mapping(dbsession, source, destination) + print(f"Added {source!r} → {destination!r}.") + elif args.action == "remove": + row = delete_acoustic_newsletters_mapping(dbsession, args.source) + if not row: + print(f"Unknown mapping '{args.source}'. Give up.") + return os.EX_DATAERR + print(f"Removed {row.source!r} → {row.destination!r}.") + else: + entries = get_all_acoustic_newsletters_mapping(dbsession) + print("\n".join(sorted(f"- {e.source!r} → {e.destination!r}" for e in entries))) + + return os.EX_OK + + +if __name__ == "__main__": + config_settings = config.Settings() + engine, session_factory = get_db_engine(config_settings) + session = session_factory() + with engine.connect() as connection: + return_code = main(session) # pylint:disable = invalid-name + + sys.exit(return_code) diff --git a/ctms/crud.py b/ctms/crud.py index aeee37aa..7bdcc5ab 100644 --- a/ctms/crud.py +++ b/ctms/crud.py @@ -14,6 +14,8 @@ from .auth import hash_password from .database import Base from .models import ( + AcousticField, + AcousticNewsletterMapping, AmoAccount, ApiClient, Email, @@ -843,3 +845,58 @@ def get_product_id(prod: ProductBaseSchema) -> str: products.sort(key=get_product_id) return products + + +def get_all_acoustic_fields(dbsession, tablename=None): + query = dbsession.query(AcousticField) + if tablename: + query = query.filter(AcousticField.tablename == tablename) + return query.all() + + +def create_acoustic_field(dbsession, tablename, field): + row = AcousticField(tablename=tablename, field=field) + dbsession.merge(row) + dbsession.commit() + return row + + +def delete_acoustic_field(dbsession, tablename, field): + row = ( + dbsession.query(AcousticField) + .filter( + AcousticField.tablename == tablename, + AcousticField.field == field, + ) + .one_or_none() + ) + if row is None: + return None + dbsession.delete(row) + dbsession.commit() + return row + + +def get_all_acoustic_newsletters_mapping(dbsession): + return dbsession.query(AcousticNewsletterMapping).all() + + +def create_acoustic_newsletters_mapping(dbsession, source, destination): + row = AcousticNewsletterMapping(source=source, destination=destination) + # This will fail if the mapping already exists. + dbsession.add(row) + dbsession.commit() + return row + + +def delete_acoustic_newsletters_mapping(dbsession, source): + row = ( + dbsession.query(AcousticNewsletterMapping) + .filter(AcousticNewsletterMapping.source == source) + .one_or_none() + ) + if not row: + return None + dbsession.delete(row) + dbsession.commit() + return row diff --git a/ctms/models.py b/ctms/models.py index 1bd52109..e6260c52 100644 --- a/ctms/models.py +++ b/ctms/models.py @@ -199,6 +199,13 @@ class AcousticField(Base): ) +class AcousticNewsletterMapping(Base): + __tablename__ = "acoustic_newsletter_mapping" + + source = Column(String, primary_key=True) + destination = Column(String) + + class VpnWaitlist(Base): __tablename__ = "vpn_waitlist" diff --git a/ctms/sync.py b/ctms/sync.py index d10fd744..0fbbc819 100644 --- a/ctms/sync.py +++ b/ctms/sync.py @@ -13,7 +13,7 @@ get_all_acoustic_retries_count, retry_acoustic_record, ) -from ctms.models import AcousticField, PendingAcousticRecord +from ctms.models import AcousticField, AcousticNewsletterMapping, PendingAcousticRecord from ctms.schemas import ContactSchema @@ -53,7 +53,12 @@ def __init__( self.is_acoustic_enabled = is_acoustic_enabled self.metric_service = metric_service - def sync_contact_with_acoustic(self, contact: ContactSchema, main_fields: set[str]): + def sync_contact_with_acoustic( + self, + contact: ContactSchema, + main_fields: set[str], + newsletters_mapping: dict[str, str], + ): """ :param contact: @@ -62,14 +67,18 @@ def sync_contact_with_acoustic(self, contact: ContactSchema, main_fields: set[st try: # Convert ContactSchema to Acoustic Readable, attempt API call return self.ctms_to_acoustic.attempt_to_upload_ctms_contact( - contact, main_fields + contact, main_fields, newsletters_mapping ) except Exception: # pylint: disable=W0703 self.logger.exception("Error executing sync.sync_contact_with_acoustic") return False def _sync_pending_record( - self, db, pending_record: PendingAcousticRecord, main_fields: set[str] + self, + db, + pending_record: PendingAcousticRecord, + main_fields: set[str], + newsletters_mapping: dict[str, str], ) -> str: state = "unknown" try: @@ -77,7 +86,9 @@ def _sync_pending_record( contact: ContactSchema = get_acoustic_record_as_contact( db, pending_record ) - is_success = self.sync_contact_with_acoustic(contact, main_fields) + is_success = self.sync_contact_with_acoustic( + contact, main_fields, newsletters_mapping + ) else: self.logger.debug( "Acoustic is not currently enabled. Records will be classified as successful and " @@ -129,6 +140,13 @@ def sync_records(self, db, end_time=None) -> Dict[str, Union[int, str]]: ) main_fields = {entry.field for entry in main_acoustic_fields} + newsletters_mapping_entries: List[AcousticNewsletterMapping] = db.query( + AcousticNewsletterMapping + ).all() + newsletters_mapping = { + entry.source: entry.destination for entry in newsletters_mapping_entries + } + # Get all Records before current time all_acoustic_records_before_now: List[ PendingAcousticRecord @@ -144,7 +162,9 @@ def sync_records(self, db, end_time=None) -> Dict[str, Union[int, str]]: states: Dict[str, int] = defaultdict(int) record_created = None for acoustic_record in all_acoustic_records_before_now: - state = self._sync_pending_record(db, acoustic_record, main_fields) + state = self._sync_pending_record( + db, acoustic_record, main_fields, newsletters_mapping + ) total += 1 states[state] += 1 diff --git a/docs/deployment_guide.md b/docs/deployment_guide.md index 6697828a..1107be0a 100644 --- a/docs/deployment_guide.md +++ b/docs/deployment_guide.md @@ -61,12 +61,18 @@ More information about CTMS operations is available on the The list of contact fields to be synchronized with Acoustic is controlled by the `acoustic_field` table in the database. -In order to add or remove certain fields, use the `acoustic_fields.py` script: +In order to add, list, or remove certain fields, use the `acoustic_fields.py` script: -``` -python ctms/bin/acoustic_fields.py remove fxaaa_id -python ctms/bin/acoustic_fields.py add fxa_id -``` +See `python ctms/bin/acoustic_fields.py --help` for usage details. + +## Acoustic Newsletters Fields + +The mapping between the Basket/CTMS newsletter name and the Acoustic field name is controlled by the `acoustic_newsletter_mapping` table in the database. + + +In order to add, list, or remove certain mappings, use the `acoustic_newsletters_mapping.py` script. + +See `python ctms/bin/acoustic_newsletters_mapping.py --help` for usage details. ## Logging diff --git a/migrations/versions/20221115_f8b8f6aa0e96_add_acoustic_newsletter_mapping.py b/migrations/versions/20221115_f8b8f6aa0e96_add_acoustic_newsletter_mapping.py new file mode 100644 index 00000000..e8fb596e --- /dev/null +++ b/migrations/versions/20221115_f8b8f6aa0e96_add_acoustic_newsletter_mapping.py @@ -0,0 +1,70 @@ +"""Add Acoustic Newsletter Mapping + +Revision ID: f8b8f6aa0e96 +Revises: 06f24cc01c09 +Create Date: 2022-11-15 12:20:10.096170 + +""" +# pylint: disable=no-member invalid-name +# no-member is triggered by alembic.op, which has dynamically added functions +# invalid-name is triggered by migration file names with a date prefix +# invalid-name is triggered by top-level alembic constants like revision instead of REVISION + +import sqlalchemy as sa +from alembic import op + + +# revision identifiers, used by Alembic. +revision = "f8b8f6aa0e96" # pragma: allowlist secret +down_revision = "06f24cc01c09" # pragma: allowlist secret +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + acoustic_mapping_table = op.create_table( + "acoustic_newsletter_mapping", + sa.Column("source", sa.String(), nullable=False), + sa.Column("destination", sa.String(), nullable=False), + sa.PrimaryKeyConstraint("source"), + ) + # ### end Alembic commands ### + + # This mapping used to be hard-coded. We now initialize the freshly created + # table with it, but the records of this table will likely be modified + # eventually. + MAIN_TABLE_SUBSCR_FLAGS = { + # maps the Basket/CTMS newsletter name to the Acoustic name + "about-mozilla": "sub_about_mozilla", + "app-dev": "sub_apps_and_hacks", + "common-voice": "sub_common_voice", + "firefox-accounts-journey": "sub_firefox_accounts_journey", + "firefox-news": "sub_firefox_news", + "firefox-sweepstakes": "sub_firefox_sweepstakes", + "hubs": "sub_hubs", + "internet-health-report": "sub_internet_health_report", + "knowledge-is-power": "sub_knowledge_is_power", + "miti": "sub_miti", + "mixed-reality": "sub_mixed_reality", + "mozilla-and-you": "sub_firefox_news", + "mozilla-fellowship-awardee-alumni": "sub_mozilla_fellowship_awardee_alumni", + "mozilla-festival": "sub_mozilla_festival", + "mozilla-foundation": "sub_mozilla_foundation", + "mozilla-rally": "sub_rally", + "mozilla-technology": "sub_mozilla_technology", + "mozillians-nda": "sub_mozillians_nda", + "security-privacy-news": "sub_security_privacy_news", + "take-action-for-the-internet": "sub_take_action_for_the_internet", + "test-pilot": "sub_test_pilot", + } + op.bulk_insert( + acoustic_mapping_table, + [{"source": k, "destination": v} for k, v in MAIN_TABLE_SUBSCR_FLAGS.items()], + ) + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_table("acoustic_newsletter_mapping") + # ### end Alembic commands ### diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 095ba399..42081af2 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -26,6 +26,8 @@ create_stripe_price, create_stripe_subscription, create_stripe_subscription_item, + get_all_acoustic_fields, + get_all_acoustic_newsletters_mapping, get_amo_by_email_id, get_contacts_by_any_id, get_email, @@ -35,7 +37,6 @@ get_stripe_products, get_vpn_by_email_id, ) -from ctms.models import AcousticField from ctms.schemas import ( ApiClientSchema, ContactSchema, @@ -164,12 +165,16 @@ def sample_contacts(minimal_contact, maximal_contact, example_contact): @pytest.fixture def main_acoustic_fields(dbsession): - records = ( - dbsession.query(AcousticField).filter(AcousticField.tablename == "main").all() - ) + records = get_all_acoustic_fields(dbsession, tablename="main") return {r.field for r in records} +@pytest.fixture +def acoustic_newsletters_mapping(dbsession): + records = get_all_acoustic_newsletters_mapping(dbsession) + return {r.source: r.destination for r in records} + + @pytest.fixture def anon_client(): """A test client with no authorization.""" diff --git a/tests/unit/test_acoustic_service.py b/tests/unit/test_acoustic_service.py index d79ef1e2..8a5e7562 100644 --- a/tests/unit/test_acoustic_service.py +++ b/tests/unit/test_acoustic_service.py @@ -61,6 +61,7 @@ def test_ctms_to_acoustic( maximal_contact, minimal_contact, main_acoustic_fields, + acoustic_newsletters_mapping, ): contact_list = [example_contact, maximal_contact, minimal_contact] example_contact_expected = [52, len(example_contact.newsletters) - 2] @@ -79,7 +80,7 @@ def test_ctms_to_acoustic( _newsletter, _product, ) = base_ctms_acoustic_service.convert_ctms_to_acoustic( - contact, main_acoustic_fields + contact, main_acoustic_fields, acoustic_newsletters_mapping ) assert _main is not None assert _newsletter is not None @@ -114,18 +115,19 @@ def test_ctms_to_acoustic_mocked( base_ctms_acoustic_service, maximal_contact, main_acoustic_fields, + acoustic_newsletters_mapping, ): acoustic_mock: MagicMock = MagicMock() base_ctms_acoustic_service.acoustic = acoustic_mock _main, _newsletter, _product = base_ctms_acoustic_service.convert_ctms_to_acoustic( - maximal_contact, main_acoustic_fields + maximal_contact, main_acoustic_fields, acoustic_newsletters_mapping ) # To be used as in testing, for expected inputs to downstream methods assert _main is not None assert _newsletter is not None assert len(_product) == 0 with capture_logs() as caplog: results = base_ctms_acoustic_service.attempt_to_upload_ctms_contact( - maximal_contact, main_acoustic_fields + maximal_contact, main_acoustic_fields, acoustic_newsletters_mapping ) assert results # success acoustic_mock.add_recipient.assert_called() @@ -159,19 +161,26 @@ def test_ctms_to_acoustic_mocked( def test_ctms_to_acoustic_with_subscription( - base_ctms_acoustic_service, contact_with_stripe_subscription, main_acoustic_fields + base_ctms_acoustic_service, + contact_with_stripe_subscription, + main_acoustic_fields, + acoustic_newsletters_mapping, ): acoustic_mock = MagicMock() base_ctms_acoustic_service.acoustic = acoustic_mock _main, _newsletter, _product = base_ctms_acoustic_service.convert_ctms_to_acoustic( - contact_with_stripe_subscription, main_acoustic_fields + contact_with_stripe_subscription, + main_acoustic_fields, + acoustic_newsletters_mapping, ) # To be used as in testing, for expected inputs to downstream methods assert _main is not None assert len(_newsletter) == 0 # None in Main Table Subscriber flags assert len(_product) == 1 with capture_logs() as caplog: results = base_ctms_acoustic_service.attempt_to_upload_ctms_contact( - contact_with_stripe_subscription, main_acoustic_fields + contact_with_stripe_subscription, + main_acoustic_fields, + acoustic_newsletters_mapping, ) assert results # success @@ -197,12 +206,15 @@ def test_ctms_to_acoustic_with_subscription_and_metrics( metrics_ctms_acoustic_service, contact_with_stripe_subscription, main_acoustic_fields, + acoustic_newsletters_mapping, ): acoustic_mock = MagicMock() acoustic_svc = metrics_ctms_acoustic_service acoustic_svc.acoustic = acoustic_mock _main, _newsletter, _product = acoustic_svc.convert_ctms_to_acoustic( - contact_with_stripe_subscription, main_acoustic_fields + contact_with_stripe_subscription, + main_acoustic_fields, + acoustic_newsletters_mapping, ) # To be used as in testing, for expected inputs to downstream methods assert _main is not None assert len(_newsletter) == 0 # None in Main Table Subscriber flags @@ -239,7 +251,9 @@ def test_ctms_to_acoustic_with_subscription_and_metrics( with capture_logs() as caplog: results = acoustic_svc.attempt_to_upload_ctms_contact( - contact_with_stripe_subscription, main_acoustic_fields + contact_with_stripe_subscription, + main_acoustic_fields, + acoustic_newsletters_mapping, ) assert results # success @@ -284,7 +298,10 @@ def test_ctms_to_acoustic_with_subscription_and_metrics( def test_ctms_to_acoustic_traced_email( - base_ctms_acoustic_service, example_contact, main_acoustic_fields + base_ctms_acoustic_service, + example_contact, + main_acoustic_fields, + acoustic_newsletters_mapping, ): """A contact requesting tracing is traced in the logs.""" email = "tester+trace-me-mozilla-nov24@example.com" @@ -292,14 +309,14 @@ def test_ctms_to_acoustic_traced_email( acoustic_mock: MagicMock = MagicMock() base_ctms_acoustic_service.acoustic = acoustic_mock _main, _newsletter, _product = base_ctms_acoustic_service.convert_ctms_to_acoustic( - example_contact, main_acoustic_fields + example_contact, main_acoustic_fields, acoustic_newsletters_mapping ) # To be used as in testing, for expected inputs to downstream methods assert _main is not None assert _newsletter is not None assert len(_product) == 0 with capture_logs() as caplog: results = base_ctms_acoustic_service.attempt_to_upload_ctms_contact( - example_contact, main_acoustic_fields + example_contact, main_acoustic_fields, acoustic_newsletters_mapping ) assert results # success diff --git a/tests/unit/test_sync.py b/tests/unit/test_sync.py index 3922dd23..30323dc2 100644 --- a/tests/unit/test_sync.py +++ b/tests/unit/test_sync.py @@ -56,17 +56,25 @@ def test_ctms_to_acoustic_sync_creation(sync_obj): assert sync_obj is not None -def test_sync_to_acoustic(sync_obj, maximal_contact, main_acoustic_fields): +def test_sync_to_acoustic( + sync_obj, maximal_contact, main_acoustic_fields, acoustic_newsletters_mapping +): sync_obj.ctms_to_acoustic = MagicMock() - result = sync_obj.sync_contact_with_acoustic(maximal_contact, main_acoustic_fields) + result = sync_obj.sync_contact_with_acoustic( + maximal_contact, main_acoustic_fields, acoustic_newsletters_mapping + ) assert result sync_obj.ctms_to_acoustic.attempt_to_upload_ctms_contact.assert_called_with( - maximal_contact, main_acoustic_fields + maximal_contact, main_acoustic_fields, acoustic_newsletters_mapping ) def test_sync_acoustic_record_retry_path( - dbsession, sync_obj, maximal_contact, main_acoustic_fields + dbsession, + sync_obj, + maximal_contact, + main_acoustic_fields, + acoustic_newsletters_mapping, ): sync_obj.ctms_to_acoustic = MagicMock( **{"attempt_to_upload_ctms_contact.return_value": False} @@ -78,7 +86,7 @@ def test_sync_acoustic_record_retry_path( context = sync_obj.sync_records(dbsession, end_time=end_time) dbsession.flush() sync_obj.ctms_to_acoustic.attempt_to_upload_ctms_contact.assert_called_with( - maximal_contact, main_acoustic_fields + maximal_contact, main_acoustic_fields, acoustic_newsletters_mapping ) expected_context = { "batch_limit": 20, @@ -88,14 +96,14 @@ def test_sync_acoustic_record_retry_path( "end_time": end_time.isoformat(), } if no_metrics: - assert ( - watcher.count == 5 - ) # Get Acoustic Fields, Get All Records, Get Contact(x2), Increment Retry + # Get Acoustic Fields, Get Acoustic newsletters mappings, Get All Records, + # Get Contact(x2), Increment Retry + assert watcher.count == 6 assert context == expected_context return # Metrics adds two DB queries (total records and retries) - assert watcher.count == 7 + assert watcher.count == 8 expected_context["retry_backlog"] = 0 expected_context["sync_backlog"] = 1 assert context == expected_context @@ -113,7 +121,12 @@ def test_sync_acoustic_record_retry_path( def test_sync_acoustic_record_delete_path( - dbsession, sync_obj, maximal_contact, settings, main_acoustic_fields + dbsession, + sync_obj, + maximal_contact, + settings, + main_acoustic_fields, + acoustic_newsletters_mapping, ): no_metrics = sync_obj.metric_service is None sync_obj.ctms_to_acoustic = MagicMock( @@ -126,7 +139,7 @@ def test_sync_acoustic_record_delete_path( context = sync_obj.sync_records(dbsession, end_time=end_time) dbsession.flush() sync_obj.ctms_to_acoustic.attempt_to_upload_ctms_contact.assert_called_with( - maximal_contact, main_acoustic_fields + maximal_contact, main_acoustic_fields, acoustic_newsletters_mapping ) expected_context = { "batch_limit": 20, @@ -136,14 +149,14 @@ def test_sync_acoustic_record_delete_path( "end_time": end_time.isoformat(), } if no_metrics: - assert ( - watcher.count == 5 - ) # Get Acoustic Fields, All Records, Get Contact(x2), Increment Retry + # Get Acoustic Fields, Get Acoustic newsletters mappings, Get All Records, + # Get Contact(x2), Increment Retry + assert watcher.count == 6 assert context == expected_context return # Metrics adds two DB queries (total records and retries) - assert watcher.count == 7 + assert watcher.count == 8 expected_context["retry_backlog"] = 0 expected_context["sync_backlog"] = 1 assert context == expected_context