From 1342217aeff84d00c056d9ef62ae10ed4d81b7dd Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Wed, 28 Jun 2023 15:54:05 +0200 Subject: [PATCH] Fix #611: `ContactSchema` now reflects what is stored in database (#711) * Use NewsletterTableSchema and WaitlistTableSchema in ContactSchema * Reproduce #611 in a dedicated test * Rename ContactSchema to ContactTableSchema for consistency * Revert "Rename ContactSchema to ContactTableSchema for consistency" Because there is not contact table in the database :) This reverts commit d0e27b0256f0d6153b11a78296d924bbc22d7006. * Save timestamps manually in tests, instead of transparently in crud.py * Update ctms/crud.py Co-authored-by: grahamalama * Add Graham's comment on waitlist crud function --------- Co-authored-by: grahamalama --- ctms/acoustic_service.py | 14 +- ctms/crud.py | 16 +- ctms/schemas/__init__.py | 8 +- ctms/schemas/contact.py | 58 +++-- ctms/schemas/newsletter.py | 26 +- tests/unit/conftest.py | 228 ++++++++++++------ tests/unit/routers/contacts/test_api.py | 31 ++- tests/unit/routers/contacts/test_api_patch.py | 41 ++-- tests/unit/routers/contacts/test_bulk.py | 9 + tests/unit/test_acoustic_service.py | 28 ++- tests/unit/test_backport_legacy_waitlists.py | 63 ++++- tests/unit/test_crud.py | 14 +- 12 files changed, 388 insertions(+), 148 deletions(-) diff --git a/ctms/acoustic_service.py b/ctms/acoustic_service.py index 63158718..cf7efffc 100644 --- a/ctms/acoustic_service.py +++ b/ctms/acoustic_service.py @@ -240,14 +240,12 @@ def _newsletter_converter(self, acoustic_main_table, contact, newsletters_mappin } if newsletter.name in newsletters_mapping: - newsletter_dict = newsletter.dict() - _today = datetime.date.today().isoformat() - newsletter_template["create_timestamp"] = newsletter_dict.get( - "create_timestamp", _today - ) - newsletter_template["update_timestamp"] = newsletter_dict.get( - "update_timestamp", _today - ) + 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 diff --git a/ctms/crud.py b/ctms/crud.py index f865087e..e71f7073 100644 --- a/ctms/crud.py +++ b/ctms/crud.py @@ -477,11 +477,12 @@ def create_waitlist( ) -> Optional[Waitlist]: if waitlist.is_default(): return None - if not isinstance(waitlist, WaitlistInSchema): - # Sample data are used as both input (`WaitlistInSchema`) and internal (`WaitlistSchema`) - # representations. - waitlist = WaitlistInSchema(**waitlist.dict()) - db_waitlist = Waitlist(email_id=email_id, **waitlist.orm_dict()) + + # `create_waitlist` uses the `WaitlistInSchema``, which has a `subscribed`` property + # this makes sense because we're receiving this payload from Basket, and waitlists + # are just newsletters with naming conventions, so it has this property. + # Our Waitlist ORM model currently doesn't have an update property, so we need to remove it. + db_waitlist = Waitlist(email_id=email_id, **waitlist.dict(exclude={"subscribed"})) db.add(db_waitlist) return db_waitlist @@ -517,7 +518,10 @@ def create_or_update_waitlists( def create_contact( - db: Session, email_id: UUID4, contact: ContactInSchema, metrics: Optional[Dict] + db: Session, + email_id: UUID4, + contact: ContactInSchema, + metrics: Optional[Dict], ): create_email(db, contact.email) if contact.amo: diff --git a/ctms/schemas/__init__.py b/ctms/schemas/__init__.py index 267f70aa..76b4a3c5 100644 --- a/ctms/schemas/__init__.py +++ b/ctms/schemas/__init__.py @@ -24,7 +24,12 @@ UpdatedFirefoxAccountsInSchema, ) from .mofo import MozillaFoundationInSchema, MozillaFoundationSchema -from .newsletter import NewsletterInSchema, NewsletterSchema, UpdatedNewsletterInSchema +from .newsletter import ( + NewsletterInSchema, + NewsletterSchema, + NewsletterTableSchema, + UpdatedNewsletterInSchema, +) from .product import ProductBaseSchema from .stripe_customer import ( StripeCustomerCreateSchema, @@ -68,6 +73,7 @@ VpnWaitlistInSchema, WaitlistInSchema, WaitlistSchema, + WaitlistTableSchema, ) from .web import ( BadRequestResponse, diff --git a/ctms/schemas/contact.py b/ctms/schemas/contact.py index 889309a6..1874a6c4 100644 --- a/ctms/schemas/contact.py +++ b/ctms/schemas/contact.py @@ -1,6 +1,6 @@ from collections import defaultdict from datetime import datetime -from typing import TYPE_CHECKING, List, Literal, Optional, Set, Union, cast +from typing import TYPE_CHECKING, List, Literal, Optional, Union, cast from uuid import UUID from pydantic import AnyUrl, BaseModel, Field, root_validator, validator @@ -16,7 +16,7 @@ ) from .fxa import FirefoxAccountsInSchema, FirefoxAccountsSchema from .mofo import MozillaFoundationInSchema, MozillaFoundationSchema -from .newsletter import NewsletterInSchema, NewsletterSchema +from .newsletter import NewsletterInSchema, NewsletterSchema, NewsletterTableSchema from .product import ProductBaseSchema, ProductSegmentEnum from .waitlist import ( RelayWaitlistInSchema, @@ -25,6 +25,7 @@ VpnWaitlistSchema, WaitlistInSchema, WaitlistSchema, + WaitlistTableSchema, validate_waitlist_newsletters, ) @@ -144,8 +145,8 @@ class ContactSchema(ComparableBase): email: EmailSchema fxa: Optional[FirefoxAccountsSchema] = None mofo: Optional[MozillaFoundationSchema] = None - newsletters: List[NewsletterSchema] = [] - waitlists: List[WaitlistSchema] = [] + newsletters: List[NewsletterTableSchema] = [] + waitlists: List[WaitlistTableSchema] = [] products: List[ProductBaseSchema] = [] @classmethod @@ -164,7 +165,24 @@ class Config: fields = { "newsletters": { "description": "List of newsletters for which the contact is or was subscribed", - "example": [{"name": "firefox-welcome"}, {"name": "mozilla-welcome"}], + "example": [ + { + "name": "firefox-welcome", + "create_timestamp": "2020-12-05T19:21:50.908000+00:00", + "update_timestamp": "2021-02-04T15:36:57.511000+00:00", + "email_id": EmailSchema.schema()["properties"]["email_id"][ + "example" + ], + }, + { + "name": "mozilla-welcome", + "create_timestamp": "2020-12-05T19:21:50.908000+00:00", + "update_timestamp": "2021-02-04T15:36:57.511000+00:00", + "email_id": EmailSchema.schema()["properties"]["email_id"][ + "example" + ], + }, + ], }, "waitlists": { "description": "List of waitlists for which the contact is or was subscribed", @@ -172,14 +190,29 @@ class Config: { "name": "example-product", "fields": {"geo": "fr", "platform": "win64"}, + "create_timestamp": "2020-12-05T19:21:50.908000+00:00", + "update_timestamp": "2021-02-04T15:36:57.511000+00:00", + "email_id": EmailSchema.schema()["properties"]["email_id"][ + "example" + ], }, { "name": "relay", "fields": {"geo": "fr"}, + "create_timestamp": "2020-12-05T19:21:50.908000+00:00", + "update_timestamp": "2021-02-04T15:36:57.511000+00:00", + "email_id": EmailSchema.schema()["properties"]["email_id"][ + "example" + ], }, { "name": "vpn", "fields": {"geo": "fr", "platform": "ios,mac"}, + "create_timestamp": "2020-12-05T19:21:50.908000+00:00", + "update_timestamp": "2021-02-04T15:36:57.511000+00:00", + "email_id": EmailSchema.schema()["properties"]["email_id"][ + "example" + ], }, ], }, @@ -199,21 +232,6 @@ def as_identity_response(self) -> "IdentityResponse": sfdc_id=getattr(self.email, "sfdc_id", None), ) - def find_default_fields(self) -> Set[str]: - """Return names of fields that contain default values only""" - default_fields = set() - if hasattr(self, "amo") and self.amo and self.amo.is_default(): - default_fields.add("amo") - if hasattr(self, "fxa") and self.fxa and self.fxa.is_default(): - default_fields.add("fxa") - if hasattr(self, "mofo") and self.mofo and self.mofo.is_default(): - default_fields.add("mofo") - if all(n.is_default() for n in self.newsletters): - default_fields.add("newsletters") - if all(n.is_default() for n in self.waitlists): - default_fields.add("waitlists") - return default_fields - class ContactInBase(ComparableBase): """A contact as provided by callers.""" diff --git a/ctms/schemas/newsletter.py b/ctms/schemas/newsletter.py index 1feed151..54f6b151 100644 --- a/ctms/schemas/newsletter.py +++ b/ctms/schemas/newsletter.py @@ -1,9 +1,13 @@ from datetime import datetime, timezone -from typing import Literal, Optional +from typing import TYPE_CHECKING, Literal, Optional -from pydantic import AnyUrl, Field +from pydantic import UUID4, AnyUrl, Field from .base import ComparableBase +from .email import EMAIL_ID_DESCRIPTION, EMAIL_ID_EXAMPLE + +if TYPE_CHECKING: + from ctms.models import Newsletter class NewsletterBase(ComparableBase): @@ -46,6 +50,24 @@ class Config: NewsletterSchema = NewsletterBase +class NewsletterTableSchema(NewsletterBase): + email_id: UUID4 = Field( + description=EMAIL_ID_DESCRIPTION, + example=EMAIL_ID_EXAMPLE, + ) + create_timestamp: datetime = Field( + description="Newsletter data creation timestamp", + example="2020-12-05T19:21:50.908000+00:00", + ) + update_timestamp: datetime = Field( + description="Newsletter data update timestamp", + example="2021-02-04T15:36:57.511000+00:00", + ) + + class Config: + extra = "forbid" + + class UpdatedNewsletterInSchema(NewsletterInSchema): update_timestamp: datetime = Field( default_factory=lambda: datetime.now(timezone.utc), diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 105963f0..cabc78e8 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -30,6 +30,7 @@ get_all_acoustic_newsletters_mapping, get_amo_by_email_id, get_contacts_by_any_id, + get_email, get_fxa_by_email_id, get_mofo_by_email_id, get_newsletters_by_email_id, @@ -39,6 +40,7 @@ from ctms.dependencies import get_api_client, get_db from ctms.metrics import get_metrics from ctms.schemas import ApiClientSchema, ContactSchema +from ctms.schemas.contact import ContactInSchema from tests import factories from tests.data import fake_stripe_id @@ -178,6 +180,52 @@ def end_savepoint(*args): register(factories.stripe.StripeCustomerDataFactory) +def create_full_contact(db, contact: ContactSchema): + """Helper to save a whole contact object into the database. + Unlike the `crud.py` functions, this takes a `ContactSchema` + instead of a `ContactInSchema` as input, and will save the specified + timestamps. + """ + contact_input = ContactInSchema(**contact.dict()) + create_contact(db, contact.email.email_id, contact_input, get_metrics()) + db.flush() + + if contact.email.create_timestamp and contact.email.update_timestamp: + email_in_db = get_email(db, contact.email.email_id) + email_in_db.create_timestamp = contact.email.create_timestamp + email_in_db.update_timestamp = contact.email.update_timestamp + + if contact.amo and contact.amo.create_timestamp and contact.amo.create_timestamp: + amo_in_db = get_amo_by_email_id(db, contact.email.email_id) + amo_in_db.create_timestamp = contact.amo.create_timestamp + amo_in_db.update_timestamp = contact.amo.update_timestamp + db.add(amo_in_db) + + specified_newsletters_by_name = {nl.name: nl for nl in contact.newsletters} + if specified_newsletters_by_name: + for newsletter_in_db in get_newsletters_by_email_id(db, contact.email.email_id): + newsletter_in_db.create_timestamp = specified_newsletters_by_name[ + newsletter_in_db.name + ].create_timestamp + newsletter_in_db.update_timestamp = specified_newsletters_by_name[ + newsletter_in_db.name + ].update_timestamp + db.add(newsletter_in_db) + + specified_waitlists_by_name = {wl.name: wl for wl in contact.waitlists} + if specified_waitlists_by_name: + for waitlist_in_db in get_waitlists_by_email_id(db, contact.email.email_id): + waitlist_in_db.create_timestamp = specified_waitlists_by_name[ + waitlist_in_db.name + ].create_timestamp + waitlist_in_db.update_timestamp = specified_waitlists_by_name[ + waitlist_in_db.name + ].update_timestamp + db.add(waitlist_in_db) + + db.commit() + + @pytest.fixture def most_minimal_contact(dbsession): contact = ContactSchema( @@ -186,46 +234,61 @@ def most_minimal_contact(dbsession): primary_email="ctms-most-minimal-user@example.com", ), ) - create_contact(dbsession, contact.email.email_id, contact, get_metrics()) - dbsession.commit() + create_full_contact(dbsession, contact) return contact @pytest.fixture -def minimal_contact_data(): +def minimal_contact_data() -> ContactSchema: + email_id = UUID("93db83d4-4119-4e0c-af87-a713786fa81d") return ContactSchema( email=schemas.EmailSchema( basket_token="142e20b6-1ef5-43d8-b5f4-597430e956d7", - create_timestamp="2014-01-22T15:24:00+00:00", - email_id=UUID("93db83d4-4119-4e0c-af87-a713786fa81d"), + create_timestamp="2014-01-22T15:24:00.000+0000", + email_id=email_id, mailing_country="us", primary_email="ctms-user@example.com", sfdc_id="001A000001aABcDEFG", update_timestamp="2020-01-22T15:24:00.000+0000", ), newsletters=[ - {"name": "app-dev"}, - {"name": "maker-party"}, - {"name": "mozilla-foundation"}, - {"name": "mozilla-learning-network"}, + schemas.NewsletterTableSchema( + name="app-dev", + email_id=email_id, + create_timestamp="2014-01-22T15:24:00.000+0000", + update_timestamp="2020-01-22T15:24:00.000+0000", + ), + schemas.NewsletterTableSchema( + name="maker-party", + email_id=email_id, + create_timestamp="2014-01-22T15:24:00.000+0000", + update_timestamp="2020-01-22T15:24:00.000+0000", + ), + schemas.NewsletterTableSchema( + name="mozilla-foundation", + email_id=email_id, + create_timestamp="2014-01-22T15:24:00.000+0000", + update_timestamp="2020-01-22T15:24:00.000+0000", + ), + schemas.NewsletterTableSchema( + name="mozilla-learning-network", + email_id=email_id, + create_timestamp="2014-01-22T15:24:00.000+0000", + update_timestamp="2020-01-22T15:24:00.000+0000", + ), ], ) @pytest.fixture -def minimal_contact(minimal_contact_data, dbsession): - create_contact( - dbsession, - minimal_contact_data.email.email_id, - minimal_contact_data, - get_metrics(), - ) - dbsession.commit() +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(): +def maximal_contact_data() -> ContactSchema: + email_id = UUID("67e52c77-950f-4f28-accb-bb3ea1a2c51a") return ContactSchema( amo=schemas.AddOnsSchema( add_on_ids="fanfox,foxfan", @@ -243,7 +306,7 @@ def maximal_contact_data(): update_timestamp="2020-01-27T14:25:43+00:00", ), email=schemas.EmailSchema( - email_id=UUID("67e52c77-950f-4f28-accb-bb3ea1a2c51a"), + email_id=email_id, primary_email="mozilla-fan@example.com", basket_token=UUID("d9ba6182-f5dd-4728-a477-2cc11bf62b69"), first_name="Fan", @@ -253,7 +316,7 @@ def maximal_contact_data(): sfdc_id="001A000001aMozFan", double_opt_in=True, unsubscribe_reason="done with this mailing list", - create_timestamp="2010-01-01T08:04:00+00:00", + create_timestamp="2010-01-01T08:04:00.000+0000", update_timestamp="2020-01-28T14:50:00.000+0000", ), fxa=schemas.FirefoxAccountsSchema( @@ -269,70 +332,107 @@ def maximal_contact_data(): mofo_relevant=True, ), newsletters=[ - schemas.NewsletterSchema( + schemas.NewsletterTableSchema( + email_id=email_id, 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.000+0000", + update_timestamp="2020-01-28T14:50:00.000+0000", ), - schemas.NewsletterSchema( + schemas.NewsletterTableSchema( + email_id=email_id, name="common-voice", format="T", lang="fr", source="https://commonvoice.mozilla.org/fr", + create_timestamp="2010-01-01T08:04:00.000+0000", + update_timestamp="2020-01-28T14:50:00.000+0000", ), - schemas.NewsletterSchema( + schemas.NewsletterTableSchema( + email_id=email_id, name="firefox-accounts-journey", source="https://www.mozilla.org/fr/firefox/accounts/", lang="fr", subscribed=False, unsub_reason="done with this mailing list", + create_timestamp="2010-01-01T08:04:00.000+0000", + update_timestamp="2020-01-28T14:50:00.000+0000", + ), + schemas.NewsletterTableSchema( + email_id=email_id, + name="firefox-os", + create_timestamp="2010-01-01T08:04:00.000+0000", + update_timestamp="2020-01-28T14:50:00.000+0000", + ), + schemas.NewsletterTableSchema( + email_id=email_id, + name="hubs", + lang="fr", + create_timestamp="2010-01-01T08:04:00.000+0000", + update_timestamp="2020-01-28T14:50:00.000+0000", + ), + schemas.NewsletterTableSchema( + email_id=email_id, + name="mozilla-festival", + create_timestamp="2010-01-01T08:04:00.000+0000", + update_timestamp="2020-01-28T14:50:00.000+0000", + ), + schemas.NewsletterTableSchema( + email_id=email_id, + name="mozilla-foundation", + lang="fr", + create_timestamp="2010-01-01T08:04:00.000+0000", + update_timestamp="2020-01-28T14:50:00.000+0000", ), - schemas.NewsletterSchema(name="firefox-os"), - schemas.NewsletterSchema(name="hubs", lang="fr"), - schemas.NewsletterSchema(name="mozilla-festival"), - schemas.NewsletterSchema(name="mozilla-foundation", lang="fr"), ], waitlists=[ - schemas.WaitlistSchema( + schemas.WaitlistTableSchema( + email_id=email_id, name="a-software", source="https://a-software.mozilla.org/", fields={"geo": "fr"}, + create_timestamp="2010-01-01T08:04:00.000+0000", + update_timestamp="2020-01-28T14:50:00.000+0000", ), - schemas.WaitlistSchema( + schemas.WaitlistTableSchema( + email_id=email_id, name="relay", fields={"geo": "cn"}, + create_timestamp="2010-01-01T08:04:00.000+0000", + update_timestamp="2020-01-28T14:50:00.000+0000", ), - schemas.WaitlistSchema( + schemas.WaitlistTableSchema( + email_id=email_id, name="super-product", source="https://super-product.mozilla.org/", fields={"geo": "fr", "platform": "win64"}, + create_timestamp="2010-01-01T08:04:00.000+0000", + update_timestamp="2020-01-28T14:50:00.000+0000", ), - schemas.WaitlistSchema( + schemas.WaitlistTableSchema( + email_id=email_id, name="vpn", fields={ "geo": "ca", "platform": "windows,android", }, + create_timestamp="2010-01-01T08:04:00.000+0000", + update_timestamp="2020-01-28T14:50:00.000+0000", ), ], ) @pytest.fixture -def maximal_contact(dbsession, maximal_contact_data): - create_contact( - dbsession, - maximal_contact_data.email.email_id, - maximal_contact_data, - get_metrics(), - ) - dbsession.commit() +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(): +def example_contact_data() -> ContactSchema: return ContactSchema( amo=schemas.AddOnsSchema(**_gather_examples(schemas.AddOnsSchema)), email=schemas.EmailSchema(**_gather_examples(schemas.EmailSchema)), @@ -341,26 +441,20 @@ def example_contact_data(): ), newsletters=ContactSchema.schema()["properties"]["newsletters"]["example"], waitlists=[ - schemas.WaitlistSchema(**example) + schemas.WaitlistTableSchema(**example) for example in ContactSchema.schema()["properties"]["waitlists"]["example"] ], ) @pytest.fixture -def example_contact(dbsession, example_contact_data): - create_contact( - dbsession, - example_contact_data.email.email_id, - example_contact_data, - get_metrics(), - ) - dbsession.commit() +def example_contact(dbsession, example_contact_data) -> ContactSchema: + create_full_contact(dbsession, example_contact_data) return example_contact_data @pytest.fixture -def to_add_contact_data(): +def to_add_contact_data() -> ContactSchema: return ContactSchema( email=schemas.EmailInSchema( basket_token="21aeb466-4003-4c2b-a27e-e6651c13d231", @@ -373,14 +467,8 @@ def to_add_contact_data(): @pytest.fixture -def to_add_contact(dbsession, to_add_contact_data): - create_contact( - dbsession, - to_add_contact_data.email.email_id, - to_add_contact_data, - get_metrics(), - ) - dbsession.commit() +def to_add_contact(dbsession, to_add_contact_data: ContactSchema) -> ContactSchema: + create_full_contact(dbsession, to_add_contact_data) return to_add_contact_data @@ -399,14 +487,10 @@ def simple_default_contact_data(): @pytest.fixture -def simple_default_contact(dbsession, simple_default_contact_data): - create_contact( - dbsession, - simple_default_contact_data.email.email_id, - simple_default_contact_data, - get_metrics(), - ) - dbsession.commit() +def simple_default_contact( + dbsession, simple_default_contact_data: ContactSchema +) -> ContactSchema: + create_full_contact(dbsession, simple_default_contact_data) return simple_default_contact_data @@ -426,14 +510,10 @@ def default_newsletter_contact_data(): @pytest.fixture -def default_newsletter_contact(dbsession, default_newsletter_contact_data): - create_contact( - dbsession, - default_newsletter_contact_data.email.email_id, - default_newsletter_contact_data, - get_metrics(), - ) - dbsession.commit() +def default_newsletter_contact( + dbsession, default_newsletter_contact_data: ContactSchema +) -> ContactSchema: + create_full_contact(dbsession, default_newsletter_contact_data) return default_newsletter_contact_data diff --git a/tests/unit/routers/contacts/test_api.py b/tests/unit/routers/contacts/test_api.py index 5e5d3d01..a63d55d4 100644 --- a/tests/unit/routers/contacts/test_api.py +++ b/tests/unit/routers/contacts/test_api.py @@ -1,5 +1,5 @@ """Unit tests for cross-API functionality""" -from typing import Any, Optional, Tuple +from typing import Any, Optional, Set, Tuple from uuid import uuid4 import pytest @@ -155,6 +155,22 @@ def _compare_written_contacts( assert saved_contact.idempotent_equal(sample) +def find_default_fields(contact: ContactSchema) -> Set[str]: + """Return names of fields that contain default values only""" + default_fields = set() + if hasattr(contact, "amo") and contact.amo and contact.amo.is_default(): + default_fields.add("amo") + if hasattr(contact, "fxa") and contact.fxa and contact.fxa.is_default(): + default_fields.add("fxa") + if hasattr(contact, "mofo") and contact.mofo and contact.mofo.is_default(): + default_fields.add("mofo") + if all(n.is_default() for n in contact.newsletters): + default_fields.add("newsletters") + if all(n.is_default() for n in contact.waitlists): + default_fields.add("waitlists") + return default_fields + + @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""" @@ -164,9 +180,18 @@ def test_post_get_put(client, post_contact, put_contact, update_fetched): resp = client.get(f"/ctms/{email_id}") assert resp.status_code == 200 - fetched = ContactSchema(**resp.json()) + # TODO: remove this once we remove support of `vpn_waitlist` and + # `relay_waitlist` as input. + # If we don't strip these two fields before turning the data into + # a `ContactInSchema`, they will create waitlist objects. + without_alias_fields = { + k: v + for k, v in resp.json().items() + if k not in ("vpn_waitlist", "relay_waitlist") + } + fetched = ContactInSchema(**without_alias_fields) update_fetched(fetched) - new_default_fields = fetched.find_default_fields() + new_default_fields = find_default_fields(fetched) # We set new_default_fields here because the returned response above # _includes_ defaults for many fields and we want to not write # them when the record is PUT again diff --git a/tests/unit/routers/contacts/test_api_patch.py b/tests/unit/routers/contacts/test_api_patch.py index 0af4681a..d72887aa 100644 --- a/tests/unit/routers/contacts/test_api_patch.py +++ b/tests/unit/routers/contacts/test_api_patch.py @@ -5,11 +5,10 @@ import pytest from structlog.testing import capture_logs -from ctms.crud import create_contact from ctms.schemas import ( AddOnsInSchema, AddOnsSchema, - ContactInSchema, + ContactSchema, CTMSResponse, EmailSchema, FirefoxAccountsInSchema, @@ -17,6 +16,8 @@ MozillaFoundationInSchema, MozillaFoundationSchema, ) +from ctms.schemas.waitlist import WaitlistInSchema +from tests.unit.conftest import create_full_contact def swap_bool(existing): @@ -187,6 +188,19 @@ def test_patch_cannot_set_timestamps(client, maximal_contact): assert actual["amo"]["update_timestamp"] != new_ts expected["amo"]["update_timestamp"] = actual["amo"]["update_timestamp"] expected["email"]["update_timestamp"] = actual["email"]["update_timestamp"] + # `actual` comes from a `CTMSResponse`, and `expected` is a `ContactSchema` + # that has timestamps. + # Since this test compares the two instances directly, we strip the timestamps from + # `expected`. + for newsletter in expected["newsletters"]: + del newsletter["email_id"] + del newsletter["create_timestamp"] + del newsletter["update_timestamp"] + for waitlist in expected["waitlists"]: + del waitlist["email_id"] + del waitlist["create_timestamp"] + del waitlist["update_timestamp"] + # products list is not (yet) in output schema assert expected["products"] == [] assert "products" not in actual @@ -239,7 +253,7 @@ def test_patch_error_on_id_conflict( ): """PATCH returns an error on ID conflicts, and makes none of the changes.""" conflict_id = str(uuid4()) - conflicting_data = ContactInSchema( + conflicting_data = ContactSchema( amo=AddOnsInSchema(user_id=1337), email=EmailSchema( email_id=conflict_id, @@ -255,7 +269,7 @@ def test_patch_error_on_id_conflict( fxa_id=1337, primary_email="fxa-conflict@example.com" ), ) - create_contact(dbsession, conflict_id, conflicting_data, metrics=None) + create_full_contact(dbsession, conflicting_data) existing_value = getattr(getattr(maximal_contact, group_name), key) conflicting_value = getattr(getattr(conflicting_data, group_name), key) @@ -323,14 +337,9 @@ def test_patch_to_unsubscribe(client, maximal_contact): """PATCH can unsubscribe by setting a newsletter field.""" email_id = maximal_contact.email.email_id existing_news_data = maximal_contact.newsletters[1].dict() - assert existing_news_data == { - "format": "T", - "lang": "fr", - "name": "common-voice", - "source": "https://commonvoice.mozilla.org/fr", - "subscribed": True, - "unsub_reason": None, - } + assert existing_news_data["subscribed"] + assert existing_news_data["name"] == "common-voice" + assert existing_news_data["unsub_reason"] is None patch_data = { "newsletters": [ { @@ -476,7 +485,9 @@ def test_patch_does_not_add_an_unsubscribed_waitlist(client, maximal_contact): def test_patch_to_update_a_waitlist(client, maximal_contact): """PATCH can update a waitlist.""" email_id = maximal_contact.email.email_id - existing = [wl.dict() for wl in maximal_contact.waitlists] + existing = [ + WaitlistInSchema(**wl.dict()).dict() for wl in maximal_contact.waitlists + ] existing[0]["fields"]["geo"] = "ca" patch_data = {"waitlists": existing} resp = client.patch(f"/ctms/{email_id}", json=patch_data, allow_redirects=True) @@ -492,7 +503,9 @@ def test_patch_to_remove_a_waitlist(client, maximal_contact): """PATCH can remove a single waitlist.""" email_id = maximal_contact.email.email_id existing = [wl.dict() for wl in maximal_contact.waitlists] - patch_data = {"waitlists": [{**existing[-1], "subscribed": False}]} + patch_data = { + "waitlists": [WaitlistInSchema(**existing[-1], subscribed=False).dict()] + } resp = client.patch(f"/ctms/{email_id}", json=patch_data, allow_redirects=True) assert resp.status_code == 200 actual = resp.json() diff --git a/tests/unit/routers/contacts/test_bulk.py b/tests/unit/routers/contacts/test_bulk.py index 8460c164..784776c9 100644 --- a/tests/unit/routers/contacts/test_bulk.py +++ b/tests/unit/routers/contacts/test_bulk.py @@ -118,6 +118,15 @@ def test_get_ctms_bulk_by_timerange( # does not have them. del dict_contact_actual["vpn_waitlist"] del dict_contact_actual["relay_waitlist"] + # The reponse does not show `email_id` and timestamp fields. + for newsletter in dict_contact_expected["newsletters"]: + del newsletter["email_id"] + del newsletter["create_timestamp"] + del newsletter["update_timestamp"] + for waitlist in dict_contact_expected["waitlists"]: + del waitlist["email_id"] + del waitlist["create_timestamp"] + del waitlist["update_timestamp"] assert dict_contact_expected == dict_contact_actual assert results["next"] is not None diff --git a/tests/unit/test_acoustic_service.py b/tests/unit/test_acoustic_service.py index 34583ad2..20c5f5de 100644 --- a/tests/unit/test_acoustic_service.py +++ b/tests/unit/test_acoustic_service.py @@ -7,7 +7,7 @@ from ctms import acoustic_service from ctms.acoustic_service import CTMSToAcousticService -from ctms.crud import get_contact_by_email_id +from ctms.crud import get_contact_by_email_id, get_newsletters_by_email_id from ctms.schemas.contact import ContactSchema CTMS_ACOUSTIC_MAIN_TABLE_ID = "1" @@ -103,6 +103,32 @@ def test_ctms_to_acoustic_newsletters( ] +def test_ctms_to_acoustic_newsletter_timestamps( + dbsession, + base_ctms_acoustic_service, + minimal_contact, + main_acoustic_fields, + acoustic_newsletters_mapping, +): + # Set timestamps on DB objects. + newsletters = get_newsletters_by_email_id(dbsession, minimal_contact.email.email_id) + app_dev_nl = [nl for nl in newsletters if nl.name == "app-dev"][0] + app_dev_nl.create_timestamp = "1982-05-08T13:20" + app_dev_nl.update_timestamp = "2023-06-19T12:17" + dbsession.add(app_dev_nl) + dbsession.commit() + # Reload contact from DB. + minimal_contact = get_contact_by_email_id(dbsession, minimal_contact.email.email_id) + + (_, newsletters_rows, _) = base_ctms_acoustic_service.convert_ctms_to_acoustic( + minimal_contact, main_acoustic_fields, acoustic_newsletters_mapping + ) + + app_dev_row = [r for r in newsletters_rows if r["newsletter_name"] == "app-dev"][0] + assert app_dev_row["create_timestamp"] == "1982-05-08" + assert app_dev_row["update_timestamp"] == "2023-06-19" + + def test_ctms_to_acoustic_waitlists_minimal( base_ctms_acoustic_service, minimal_contact, diff --git a/tests/unit/test_backport_legacy_waitlists.py b/tests/unit/test_backport_legacy_waitlists.py index b75997dc..b50f0541 100644 --- a/tests/unit/test_backport_legacy_waitlists.py +++ b/tests/unit/test_backport_legacy_waitlists.py @@ -8,25 +8,35 @@ from sqlalchemy.orm import sessionmaker from ctms.crud import ( - create_contact, create_or_update_contact, get_email, get_waitlists_by_email_id, update_contact, ) -from ctms.schemas import ContactPatchSchema, NewsletterInSchema, WaitlistInSchema -from tests.unit.conftest import APP_FOLDER +from ctms.schemas import ContactPatchSchema, NewsletterInSchema +from ctms.schemas.contact import ContactSchema +from ctms.schemas.newsletter import NewsletterTableSchema +from ctms.schemas.waitlist import WaitlistTableSchema +from tests.unit.conftest import APP_FOLDER, create_full_contact @pytest.fixture -def minimal_contact_with_relay(dbsession, minimal_contact_data): +def minimal_contact_with_relay(dbsession, minimal_contact_data: ContactSchema): email_id = minimal_contact_data.email.email_id contact = minimal_contact_data.copy( update={ - "waitlists": [WaitlistInSchema(name="relay", fields={"geo": "es"})], + "waitlists": [ + WaitlistTableSchema( + email_id=email_id, + name="relay", + fields={"geo": "es"}, + create_timestamp="2014-01-22T15:24:00.000+0000", + update_timestamp="2020-01-22T15:24:00.000+0000", + ) + ], } ) - create_contact(dbsession, email_id, contact, metrics={}) + create_full_contact(dbsession, contact) dbsession.commit() return contact @@ -37,12 +47,24 @@ def minimal_contact_with_relay_phone(dbsession, minimal_contact_data): contact = minimal_contact_data.copy( update={ "waitlists": [ - WaitlistInSchema(name="relay-vpn", fields={"geo": "es"}), - WaitlistInSchema(name="relay-phone-masking", fields={"geo": "es"}), + WaitlistTableSchema( + email_id=email_id, + name="relay-vpn", + fields={"geo": "es"}, + create_timestamp="2014-01-22T15:24:00.000+0000", + update_timestamp="2020-01-22T15:24:00.000+0000", + ), + WaitlistTableSchema( + email_id=email_id, + name="relay-phone-masking", + fields={"geo": "es"}, + create_timestamp="2014-01-22T15:24:00.000+0000", + update_timestamp="2020-01-22T15:24:00.000+0000", + ), ], } ) - create_contact(dbsession, email_id, contact, metrics={}) + create_full_contact(dbsession, contact) dbsession.commit() return contact @@ -55,12 +77,22 @@ def test_relay_waitlist_created_on_newsletter_subscribe( update={ "relay_waitlist": {"geo": "fr"}, "newsletters": [ - NewsletterInSchema(name="amazing-product"), - NewsletterInSchema(name="relay-phone-masking-waitlist"), + NewsletterTableSchema( + email_id=email_id, + name="amazing-product", + create_timestamp="2014-01-22T15:24:00.000+0000", + update_timestamp="2020-01-22T15:24:00.000+0000", + ), + NewsletterTableSchema( + email_id=email_id, + name="relay-phone-masking-waitlist", + create_timestamp="2014-01-22T15:24:00.000+0000", + update_timestamp="2020-01-22T15:24:00.000+0000", + ), ], } ) - create_contact(dbsession, email_id, contact, metrics={}) + create_full_contact(dbsession, contact) dbsession.flush() waitlists_by_name = { @@ -79,7 +111,12 @@ def test_relay_waitlist_created_on_newsletter_updated( update={ "relay_waitlist": {"geo": "es"}, "newsletters": [ - NewsletterInSchema(name="relay-phone-masking-waitlist"), + NewsletterTableSchema( + email_id=email_id, + name="relay-phone-masking-waitlist", + create_timestamp="2014-01-22T15:24:00.000+0000", + update_timestamp="2020-01-22T15:24:00.000+0000", + ), ], } ) diff --git a/tests/unit/test_crud.py b/tests/unit/test_crud.py index 5e1ec8c1..07bff35a 100644 --- a/tests/unit/test_crud.py +++ b/tests/unit/test_crud.py @@ -454,8 +454,9 @@ def test_get_bulk_contacts_some_after_higher_limit( after_email_id=after_id, ) assert len(bulk_contact_list) == 2 - assert last_contact in bulk_contact_list - assert sorted_list[-2] in bulk_contact_list + bulk_contact_list_ids = [c.email.email_id for c in bulk_contact_list] + assert last_contact.email.email_id in bulk_contact_list_ids + assert sorted_list[-2].email.email_id in bulk_contact_list_ids def test_get_bulk_contacts_some_after( @@ -482,7 +483,7 @@ def test_get_bulk_contacts_some_after( after_email_id=after_id, ) assert len(bulk_contact_list) == 1 - assert last_contact in bulk_contact_list + assert last_contact.email.email_id == bulk_contact_list[0].email.email_id def test_get_bulk_contacts_some( @@ -502,9 +503,10 @@ def test_get_bulk_contacts_some( limit=10, ) assert len(bulk_contact_list) >= 3 - assert example_contact in bulk_contact_list - assert maximal_contact in bulk_contact_list - assert minimal_contact in bulk_contact_list + bulk_contact_list_ids = [c.email.email_id for c in bulk_contact_list] + assert example_contact.email.email_id in bulk_contact_list_ids + assert maximal_contact.email.email_id in bulk_contact_list_ids + assert minimal_contact.email.email_id in bulk_contact_list_ids def test_get_bulk_contacts_one(dbsession, example_contact):