From 72b822466587bad7b8860016e0ce830bc55bac47 Mon Sep 17 00:00:00 2001 From: grahamalama Date: Tue, 1 Oct 2024 09:43:23 -0400 Subject: [PATCH] Remove more complex / indirect fixtures used in tests (#975) * Move fixtures and constants from conftest to modules * Remove use of `sample_contacts` fixture * Remove test get for maximal contact test This is largely covered by the "example" test * Switch default email format to HTML in email factory * Remove minimal / maximal contacts * Remove now-unused fixtures --- tests/factories/models.py | 2 +- tests/unit/conftest.py | 57 ------ tests/unit/routers/contacts/test_api.py | 12 +- tests/unit/routers/contacts/test_api_get.py | 167 +----------------- tests/unit/routers/contacts/test_api_patch.py | 126 ++++++++----- tests/unit/routers/contacts/test_api_put.py | 1 - .../unit/routers/contacts/test_private_api.py | 52 ++++-- tests/unit/test_crud.py | 87 ++++----- 8 files changed, 174 insertions(+), 330 deletions(-) diff --git a/tests/factories/models.py b/tests/factories/models.py index b401814e..5dfce618 100644 --- a/tests/factories/models.py +++ b/tests/factories/models.py @@ -111,7 +111,7 @@ class Meta: first_name = factory.Faker("first_name") last_name = factory.Faker("last_name") mailing_country = factory.Faker("country_code") - email_format = "T" + email_format = "H" email_lang = factory.Faker("language_code") double_opt_in = False has_opted_out_of_email = False diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index c82e9789..5e54479d 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -52,20 +52,6 @@ }, } -# Common test cases to use in parameterized tests -# List of tuples comprised of name fixture name (defined below) and the list -# of fields that are given a default value in that fixture, and therefore -# should not be overwritten on update. -SAMPLE_CONTACT_PARAMS = [ - ("minimal_contact_data", set()), - ("maximal_contact_data", set()), - ("example_contact_data", set()), - ("to_add_contact_data", set()), - ("simple_default_contact_data", {"amo"}), - ("default_newsletter_contact_data", {"newsletters"}), - ("default_waitlist_contact_data", {"waitlists"}), -] - def _gather_examples(schema_class) -> dict[str, str]: """Gather the examples from a schema definition""" @@ -256,12 +242,6 @@ def minimal_contact_data() -> ContactSchema: ) -@pytest.fixture -def minimal_contact(minimal_contact_data: ContactSchema, dbsession) -> ContactSchema: - create_full_contact(dbsession, minimal_contact_data) - return minimal_contact_data - - @pytest.fixture def maximal_contact_data() -> ContactSchema: email_id = UUID("67e52c77-950f-4f28-accb-bb3ea1a2c51a") @@ -405,12 +385,6 @@ def maximal_contact_data() -> ContactSchema: ) -@pytest.fixture -def maximal_contact(dbsession, maximal_contact_data: ContactSchema) -> ContactSchema: - create_full_contact(dbsession, maximal_contact_data) - return maximal_contact_data - - @pytest.fixture def example_contact_data() -> ContactSchema: return ContactSchema( @@ -450,12 +424,6 @@ def to_add_contact_data() -> ContactSchema: ) -@pytest.fixture -def to_add_contact(dbsession, to_add_contact_data: ContactSchema) -> ContactSchema: - create_full_contact(dbsession, to_add_contact_data) - return to_add_contact_data - - @pytest.fixture def simple_default_contact_data(): return ContactSchema( @@ -470,14 +438,6 @@ def simple_default_contact_data(): ) -@pytest.fixture -def simple_default_contact( - dbsession, simple_default_contact_data: ContactSchema -) -> ContactSchema: - create_full_contact(dbsession, simple_default_contact_data) - return simple_default_contact_data - - @pytest.fixture def default_newsletter_contact_data(): contact = ContactSchema( @@ -493,14 +453,6 @@ def default_newsletter_contact_data(): return contact -@pytest.fixture -def default_newsletter_contact( - dbsession, default_newsletter_contact_data: ContactSchema -) -> ContactSchema: - create_full_contact(dbsession, default_newsletter_contact_data) - return default_newsletter_contact_data - - @pytest.fixture def default_waitlist_contact_data(): contact = ContactSchema( @@ -516,15 +468,6 @@ def default_waitlist_contact_data(): return contact -@pytest.fixture -def sample_contacts(minimal_contact, maximal_contact, example_contact): - return { - "minimal": (minimal_contact.email.email_id, minimal_contact), - "maximal": (maximal_contact.email.email_id, maximal_contact), - "example": (example_contact.email.email_id, example_contact), - } - - @pytest.fixture def anon_client(dbsession): """A test client with no authorization.""" diff --git a/tests/unit/routers/contacts/test_api.py b/tests/unit/routers/contacts/test_api.py index 4853edce..0bd23e85 100644 --- a/tests/unit/routers/contacts/test_api.py +++ b/tests/unit/routers/contacts/test_api.py @@ -7,7 +7,6 @@ from ctms.schemas import ContactInSchema, ContactSchema, NewsletterInSchema from ctms.schemas.waitlist import WaitlistInSchema -from tests.unit.conftest import SAMPLE_CONTACT_PARAMS API_TEST_CASES = ( ("GET", "/ctms", {"primary_email": "contact@example.com"}), @@ -176,6 +175,17 @@ def find_default_fields(contact: ContactSchema) -> Set[str]: return default_fields +SAMPLE_CONTACT_PARAMS = [ + ("minimal_contact_data", set()), + ("maximal_contact_data", set()), + ("example_contact_data", set()), + ("to_add_contact_data", set()), + ("simple_default_contact_data", {"amo"}), + ("default_newsletter_contact_data", {"newsletters"}), + ("default_waitlist_contact_data", {"waitlists"}), +] + + @pytest.mark.parametrize("post_contact", SAMPLE_CONTACT_PARAMS, indirect=True) def test_post_get_put(client, post_contact, put_contact, update_fetched): """This encompasses the entire expected flow for basket""" diff --git a/tests/unit/routers/contacts/test_api_get.py b/tests/unit/routers/contacts/test_api_get.py index 5dd54cd4..fbe332d7 100644 --- a/tests/unit/routers/contacts/test_api_get.py +++ b/tests/unit/routers/contacts/test_api_get.py @@ -3,7 +3,7 @@ import pytest -def test_get_ctms_for_minimal_contact(client, email_factory): +def test_get_ctms_for_mostly_empty(client, email_factory): """GET /ctms/{email_id} returns a contact with most fields unset.""" contact = email_factory(newsletters=1) newsletter = contact.newsletters[0] @@ -74,171 +74,6 @@ def test_get_ctms_for_minimal_contact(client, email_factory): } -def test_get_ctms_for_maximal_contact(client, maximal_contact): - """GET /ctms/{email_id} returns a contact with almost all fields set.""" - email_id = maximal_contact.email.email_id - resp = client.get(f"/ctms/{email_id}") - assert resp.status_code == 200 - assert resp.json() == { - "amo": { - "add_on_ids": "fanfox,foxfan", - "create_timestamp": "2017-05-12T15:16:00+00:00", - "display_name": "#1 Mozilla Fan", - "email_opt_in": True, - "language": "fr,en", - "last_login": "2020-01-27", - "location": "The Inter", - "profile_url": "firefox/user/14508209", - "update_timestamp": "2020-01-27T14:25:43+00:00", - "user": True, - "user_id": "123", - "username": "Mozilla1Fan", - }, - "email": { - "basket_token": "d9ba6182-f5dd-4728-a477-2cc11bf62b69", - "create_timestamp": "2010-01-01T08:04:00+00:00", - "double_opt_in": True, - "email_format": "H", - "email_id": "67e52c77-950f-4f28-accb-bb3ea1a2c51a", - "email_lang": "fr", - "first_name": "Fan", - "has_opted_out_of_email": False, - "last_name": "of Mozilla", - "mailing_country": "ca", - "primary_email": "mozilla-fan@example.com", - "sfdc_id": "001A000001aMozFan", - "unsubscribe_reason": "done with this mailing list", - "update_timestamp": "2020-01-28T14:50:00+00:00", - }, - "fxa": { - "created_date": "2019-05-22T08:29:31.906094+00:00", - "account_deleted": False, - "first_service": "monitor", - "fxa_id": "611b6788-2bba-42a6-98c9-9ce6eb9cbd34", - "lang": "fr,fr-CA", - "primary_email": "fxa-firefox-fan@example.com", - }, - "mofo": { - "mofo_contact_id": "5e499cc0-eeb5-4f0e-aae6-a101721874b8", - "mofo_email_id": "195207d2-63f2-4c9f-b149-80e9c408477a", - "mofo_relevant": True, - }, - "newsletters": [ - { - "format": "H", - "lang": "en", - "name": "ambassadors", - "source": "https://www.mozilla.org/en-US/contribute/studentambassadors/", - "subscribed": False, - "unsub_reason": "Graduated, don't have time for FSA", - "create_timestamp": "2010-01-01T08:04:00+00:00", - "update_timestamp": "2020-01-28T14:50:00+00:00", - }, - { - "format": "T", - "lang": "fr", - "name": "common-voice", - "source": "https://commonvoice.mozilla.org/fr", - "subscribed": True, - "unsub_reason": None, - "create_timestamp": "2010-01-01T08:04:00+00:00", - "update_timestamp": "2020-01-28T14:50:00+00:00", - }, - { - "format": "H", - "lang": "fr", - "name": "firefox-accounts-journey", - "source": "https://www.mozilla.org/fr/firefox/accounts/", - "subscribed": False, - "unsub_reason": "done with this mailing list", - "create_timestamp": "2010-01-01T08:04:00+00:00", - "update_timestamp": "2020-01-28T14:50:00+00:00", - }, - { - "format": "H", - "lang": "en", - "name": "firefox-os", - "source": None, - "subscribed": True, - "unsub_reason": None, - "create_timestamp": "2010-01-01T08:04:00+00:00", - "update_timestamp": "2020-01-28T14:50:00+00:00", - }, - { - "format": "H", - "lang": "fr", - "name": "hubs", - "source": None, - "subscribed": True, - "unsub_reason": None, - "create_timestamp": "2010-01-01T08:04:00+00:00", - "update_timestamp": "2020-01-28T14:50:00+00:00", - }, - { - "format": "H", - "lang": "en", - "name": "mozilla-festival", - "source": None, - "subscribed": True, - "unsub_reason": None, - "create_timestamp": "2010-01-01T08:04:00+00:00", - "update_timestamp": "2020-01-28T14:50:00+00:00", - }, - { - "format": "H", - "lang": "fr", - "name": "mozilla-foundation", - "source": None, - "subscribed": True, - "unsub_reason": None, - "create_timestamp": "2010-01-01T08:04:00+00:00", - "update_timestamp": "2020-01-28T14:50:00+00:00", - }, - ], - "status": "ok", - "vpn_waitlist": {"geo": "ca", "platform": "windows,android"}, - "relay_waitlist": {"geo": "cn"}, - "waitlists": [ - { - "fields": {"geo": "fr"}, - "name": "a-software", - "source": "https://a-software.mozilla.org/", - "subscribed": True, - "unsub_reason": None, - "create_timestamp": "2010-01-01T08:04:00+00:00", - "update_timestamp": "2020-01-28T14:50:00+00:00", - }, - { - "fields": {"geo": "cn"}, - "name": "relay", - "source": None, - "subscribed": True, - "unsub_reason": None, - "create_timestamp": "2010-01-01T08:04:00+00:00", - "update_timestamp": "2020-01-28T14:50:00+00:00", - }, - { - "fields": {"geo": "fr", "platform": "win64"}, - "name": "super-product", - "source": "https://super-product.mozilla.org/", - "subscribed": True, - "unsub_reason": None, - "create_timestamp": "2010-01-01T08:04:00+00:00", - "update_timestamp": "2020-01-28T14:50:00+00:00", - }, - { - "fields": {"geo": "ca", "platform": "windows,android"}, - "name": "vpn", - "source": None, - "subscribed": True, - "unsub_reason": None, - "create_timestamp": "2010-01-01T08:04:00+00:00", - "update_timestamp": "2020-01-28T14:50:00+00:00", - }, - ], - } - - def test_get_ctms_for_api_example(client, example_contact): """The API examples represent a valid contact with many fields set. Test that the API examples are valid.""" diff --git a/tests/unit/routers/contacts/test_api_patch.py b/tests/unit/routers/contacts/test_api_patch.py index 951f2cd0..8c8156c0 100644 --- a/tests/unit/routers/contacts/test_api_patch.py +++ b/tests/unit/routers/contacts/test_api_patch.py @@ -28,53 +28,52 @@ def swap_bool(existing): return not existing -@pytest.mark.parametrize("contact_name", ("minimal_contact", "maximal_contact")) -@pytest.mark.parametrize( - "group_name,key,value", - ( - ("amo", "add_on_ids", "new-addon-id"), - ("amo", "display_name", "New Display Name"), - ("amo", "email_opt_in", swap_bool), - ("amo", "language", "es"), - ("amo", "last_login", "2020-04-07"), - ("amo", "location", "New Location"), - ("amo", "profile_url", "firefox/user/14612345"), - ("amo", "user", swap_bool), - ("amo", "user_id", "987"), - ("amo", "username", "NewUsername"), - ("email", "primary_email", "new-email@example.com"), - ("email", "basket_token", "8b359504-d1b4-4691-9175-cfee3059b171"), - ("email", "double_opt_in", swap_bool), - ("email", "sfdc_id", "001A000001aNewUser"), - ("email", "first_name", "New"), - ("email", "last_name", "LastName"), - ("email", "mailing_country", "uk"), - ("email", "email_format", "T"), - ("email", "email_lang", "eo"), - ("email", "has_opted_out_of_email", swap_bool), - ("email", "unsubscribe_reason", "new reason"), - ("fxa", "fxa_id", "8b359504-d1b4-4691-9175-cfee3059b171"), - ("fxa", "primary_email", "new-fxa-email@example.com"), - ("fxa", "created_date", "2021-04-07T10:00:00+00:00"), - ("fxa", "lang", "es"), - ("fxa", "first_service", "vpn"), - ("fxa", "account_deleted", swap_bool), - ("mofo", "mofo_email_id", "8b359504-d1b4-4691-9175-cfee3059b171"), - ("mofo", "mofo_contact_id", "8b359504-d1b4-4691-9175-cfee3059b171"), - ("mofo", "mofo_relevant", swap_bool), - ), +patch_single_value_params = ( + ("amo", "add_on_ids", "new-addon-id"), + ("amo", "display_name", "New Display Name"), + ("amo", "email_opt_in", swap_bool), + ("amo", "language", "es"), + ("amo", "last_login", "2020-04-07"), + ("amo", "location", "New Location"), + ("amo", "profile_url", "firefox/user/14612345"), + ("amo", "user", swap_bool), + ("amo", "user_id", "987"), + ("amo", "username", "NewUsername"), + ("email", "primary_email", "new-email@example.com"), + ("email", "basket_token", "8b359504-d1b4-4691-9175-cfee3059b171"), + ("email", "double_opt_in", swap_bool), + ("email", "sfdc_id", "001A000001aNewUser"), + ("email", "first_name", "New"), + ("email", "last_name", "LastName"), + ("email", "mailing_country", "uk"), + ("email", "email_format", "T"), + ("email", "email_lang", "eo"), + ("email", "has_opted_out_of_email", swap_bool), + ("email", "unsubscribe_reason", "new reason"), + ("fxa", "fxa_id", "8b359504-d1b4-4691-9175-cfee3059b171"), + ("fxa", "primary_email", "new-fxa-email@example.com"), + ("fxa", "created_date", "2021-04-07T10:00:00+00:00"), + ("fxa", "lang", "es"), + ("fxa", "first_service", "vpn"), + ("fxa", "account_deleted", swap_bool), + ("mofo", "mofo_email_id", "8b359504-d1b4-4691-9175-cfee3059b171"), + ("mofo", "mofo_contact_id", "8b359504-d1b4-4691-9175-cfee3059b171"), + ("mofo", "mofo_relevant", swap_bool), ) -def test_patch_one_new_value(client, contact_name, group_name, key, value, request): + + +@pytest.mark.parametrize("group_name,key,value", (patch_single_value_params)) +def test_patch_one_new_value_mostly_empty( + client, email_factory, group_name, key, value +): """PATCH can update a single value.""" - contact = request.getfixturevalue(contact_name) + email = email_factory() + contact = ContactSchema.from_email(email) expected = json.loads(CTMSResponse(**contact.model_dump()).model_dump_json()) existing_value = expected[group_name][key] # Set dynamic test values - if callable(value): - new_value = value(existing_value) - else: - new_value = value + new_value = value(existing_value) if callable(value) else value patch_data = {group_name: {key: new_value}} expected[group_name][key] = new_value @@ -87,14 +86,55 @@ def test_patch_one_new_value(client, contact_name, group_name, key, value, reque del actual["status"] # Timestamps should be set for newly created amo group - if group_name == "amo" and contact.amo is None: + if group_name == "amo": assert expected["amo"]["create_timestamp"] is None assert expected["amo"]["update_timestamp"] is None assert actual["amo"]["create_timestamp"] is not None assert actual["amo"]["update_timestamp"] is not None assert actual["amo"]["create_timestamp"] == actual["amo"]["update_timestamp"] expected["amo"]["create_timestamp"] = actual["amo"]["create_timestamp"] - expected["amo"]["update_timestamp"] = actual["amo"]["update_timestamp"] + expected["amo"]["update_timestamp"] = actual["amo"]["update_timestamp"] + + # Timestamps are not included for fxa or mofo objects + for group in ("fxa", "mofo"): + assert expected[group].get("create_timestamp") is None + assert expected[group].get("update_timestamp") is None + assert actual[group].get("create_timestamp") is None + assert actual[group].get("update_timestamp") is None + + expected["email"]["update_timestamp"] = actual["email"]["update_timestamp"] + assert actual == expected + + +@pytest.mark.parametrize("group_name,key,value", (patch_single_value_params)) +def test_patch_one_new_value_mostly_full(client, email_factory, group_name, key, value): + """PATCH can update a single value.""" + email = email_factory( + mailing_country="xx", + email_lang="xx", + with_amo=True, + amo__language="xx", + with_fxa=True, + fxa__lang="xx", + with_mofo=True, + ) + contact = ContactSchema.from_email(email) + expected = json.loads(CTMSResponse(**contact.model_dump()).model_dump_json()) + existing_value = expected[group_name][key] + + # Set dynamic test values + new_value = value(existing_value) if callable(value) else value + + patch_data = {group_name: {key: new_value}} + expected[group_name][key] = new_value + assert existing_value != new_value + + resp = client.patch(f"/ctms/{contact.email.email_id}", json=patch_data) + assert resp.status_code == 200 + actual = resp.json() + assert actual["status"] == "ok" + del actual["status"] + expected["email"]["update_timestamp"] = actual["email"]["update_timestamp"] assert actual == expected diff --git a/tests/unit/routers/contacts/test_api_put.py b/tests/unit/routers/contacts/test_api_put.py index ed757d53..8df44b3b 100644 --- a/tests/unit/routers/contacts/test_api_put.py +++ b/tests/unit/routers/contacts/test_api_put.py @@ -8,7 +8,6 @@ from ctms import models from ctms.schemas import ContactPutSchema, EmailInSchema -from tests.unit.conftest import SAMPLE_CONTACT_PARAMS def test_create_or_update_basic_id_is_different(client): diff --git a/tests/unit/routers/contacts/test_private_api.py b/tests/unit/routers/contacts/test_private_api.py index 5e535242..caa25918 100644 --- a/tests/unit/routers/contacts/test_private_api.py +++ b/tests/unit/routers/contacts/test_private_api.py @@ -48,13 +48,11 @@ def identity_response_for_contact(contact): } -@pytest.mark.parametrize( - "name", ("minimal_contact", "maximal_contact", "example_contact") -) -def test_get_identity_by_email_id(client, name, request): +def test_get_identity_by_email_id(client, email_factory): """GET /identity/{email_id} returns the identity object.""" - contact = request.getfixturevalue(name) + email = email_factory(with_fxa=True, with_mofo=True) + contact = ContactSchema.from_email(email) resp = client.get(f"/identity/{contact.email.email_id}") assert resp.status_code == 200 assert resp.json() == identity_response_for_contact(contact) @@ -69,25 +67,41 @@ def test_get_identity_not_found(client, dbsession): @pytest.mark.parametrize( - "name,ident", + ("id_name", "id_value"), ( - ("example_contact", "email_id"), - ("minimal_contact", "primary_email"), - ("maximal_contact", "basket_token"), - ("minimal_contact", "sfdc_id"), - ("maximal_contact", "amo_user_id"), - ("maximal_contact", "fxa_id"), - ("example_contact", "fxa_primary_email"), - ("maximal_contact", "mofo_contact_id"), - ("maximal_contact", "mofo_email_id"), + ("email_id", "67e52c77-950f-4f28-accb-bb3ea1a2c51a"), + ("primary_email", "mozilla-fan@example.com"), + ("amo_user_id", 123), + ("basket_token", "d9ba6182-f5dd-4728-a477-2cc11bf62b69"), + ("fxa_id", "611b6788-2bba-42a6-98c9-9ce6eb9cbd34"), + ("fxa_primary_email", "fxa-firefox-fan@example.com"), + ("sfdc_id", "001A000001aMozFan"), + ("mofo_contact_id", "5e499cc0-eeb5-4f0e-aae6-a101721874b8"), + ("mofo_email_id", "195207d2-63f2-4c9f-b149-80e9c408477a"), ), ) -def test_get_identities_by_alt_id(client, request, name, ident): +def test_get_identities_by_alt_id(client, email_factory, id_name, id_value): """GET /identities?alt_id=value returns a one-item identities list.""" - contact = request.getfixturevalue(name) + + email = email_factory( + email_id="67e52c77-950f-4f28-accb-bb3ea1a2c51a", + primary_email="mozilla-fan@example.com", + basket_token="d9ba6182-f5dd-4728-a477-2cc11bf62b69", + sfdc_id="001A000001aMozFan", + with_amo=True, + amo__user_id="123", + with_fxa=True, + fxa__fxa_id="611b6788-2bba-42a6-98c9-9ce6eb9cbd34", + fxa__primary_email="fxa-firefox-fan@example.com", + with_mofo=True, + mofo__mofo_contact_id="5e499cc0-eeb5-4f0e-aae6-a101721874b8", + mofo__mofo_email_id="195207d2-63f2-4c9f-b149-80e9c408477a", + ) + contact = ContactSchema.from_email(email) identity = identity_response_for_contact(contact) - assert identity[ident] - resp = client.get(f"/identities?{ident}={identity[ident]}") + + resp = client.get(f"/identities?{id_name}={id_value}") + assert resp.status_code == 200 assert resp.json() == [identity] diff --git a/tests/unit/test_crud.py b/tests/unit/test_crud.py index 22477d2c..62b2eab4 100644 --- a/tests/unit/test_crud.py +++ b/tests/unit/test_crud.py @@ -204,14 +204,31 @@ def test_get_bulk_contacts_none(dbsession): ("mofo_email_id", "195207d2-63f2-4c9f-b149-80e9c408477a"), ], ) -def test_get_contact_by_any_id(dbsession, sample_contacts, alt_id_name, alt_id_value): - contacts = get_contacts_by_any_id(dbsession, **{alt_id_name: alt_id_value}) - assert len(contacts) == 1 - newsletter_names = [nl.name for nl in contacts[0].newsletters] - assert sorted(newsletter_names) == newsletter_names +def test_get_contact_by_any_id(dbsession, email_factory, alt_id_name, alt_id_value): + email = email_factory( + email_id="67e52c77-950f-4f28-accb-bb3ea1a2c51a", + primary_email="mozilla-fan@example.com", + basket_token="d9ba6182-f5dd-4728-a477-2cc11bf62b69", + sfdc_id="001A000001aMozFan", + with_fxa=True, + fxa__fxa_id="611b6788-2bba-42a6-98c9-9ce6eb9cbd34", + fxa__primary_email="fxa-firefox-fan@example.com", + with_amo=True, + amo__user_id="123", + with_mofo=True, + mofo__mofo_contact_id="5e499cc0-eeb5-4f0e-aae6-a101721874b8", + mofo__mofo_email_id="195207d2-63f2-4c9f-b149-80e9c408477a", + ) + email_factory.create_batch(2, with_fxa=True, with_amo=True, with_mofo=True) + + [contact] = get_contacts_by_any_id(dbsession, **{alt_id_name: alt_id_value}) + assert contact.email.email_id == email.email_id -def test_get_contact_by_any_id_missing(dbsession, sample_contacts): +def test_get_contact_by_any_id_missing(dbsession, email_factory): + # create an email + email_factory() + # but use a different (random) UUID as the basket token contact = get_contacts_by_any_id(dbsession, basket_token=str(uuid4())) assert len(contact) == 0 @@ -226,44 +243,30 @@ def test_get_contact_by_any_id_missing(dbsession, sample_contacts): ], ) def test_get_multiple_contacts_by_any_id( - dbsession, sample_contacts, alt_id_name, alt_id_value + dbsession, email_factory, alt_id_name, alt_id_value ): - dupe_id = str(uuid4()) - create_email( - dbsession, - EmailInSchema( - email_id=dupe_id, - primary_email="dupe@example.com", - basket_token=str(uuid4()), - sfdc_id=( - alt_id_value if alt_id_name == "sfdc_id" else "other_sdfc_alt_id_value" - ), - ), + """Two contacts can share the same: + - fxa primary_email + - amo user_id + - sfdc_id + - mofo_contact_id + + And when we query users by these ids, we might get multiple contacts back + """ + email_factory.create_batch( + 2, + sfdc_id="001A000001aMozFan", + with_amo=True, + amo__user_id=123, + with_fxa=True, + fxa__primary_email="fxa-firefox-fan@example.com", + with_mofo=True, + mofo__mofo_contact_id="5e499cc0-eeb5-4f0e-aae6-a101721874b8", ) - if alt_id_name == "amo_user_id": - create_amo(dbsession, dupe_id, AddOnsInSchema(user_id=alt_id_value)) - if alt_id_name == "fxa_primary_email": - create_fxa( - dbsession, dupe_id, FirefoxAccountsInSchema(primary_email=alt_id_value) - ) - if alt_id_name == "mofo_contact_id": - create_mofo( - dbsession, - dupe_id, - MozillaFoundationInSchema( - mofo_email_id=str(uuid4()), mofo_contact_id=alt_id_value - ), - ) - - create_newsletter(dbsession, dupe_id, NewsletterInSchema(name="zzz_sleepy_news")) - create_newsletter(dbsession, dupe_id, NewsletterInSchema(name="aaa_game_news")) - dbsession.flush() - - contacts = get_contacts_by_any_id(dbsession, **{alt_id_name: alt_id_value}) - assert len(contacts) == 2 - for contact in contacts: - newsletter_names = [nl.name for nl in contact.newsletters] - assert sorted(newsletter_names) == newsletter_names + [contact_a, contact_b] = get_contacts_by_any_id( + dbsession, **{alt_id_name: alt_id_value} + ) + assert contact_a.email.email_id != contact_b.email.email_id def test_create_or_update_contact_related_objects(dbsession, email_factory):