From b2d90eb74343950c2ef9039d71ff6c9aa7b72121 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Mon, 26 Jun 2023 10:43:30 +0200 Subject: [PATCH 1/7] Use NewsletterTableSchema and WaitlistTableSchema in ContactSchema --- ctms/crud.py | 34 +++++++--- ctms/schemas/__init__.py | 8 ++- ctms/schemas/contact.py | 58 +++++++++++------ ctms/schemas/newsletter.py | 26 +++++++- tests/unit/conftest.py | 117 +++++++++++++++++++++++++++-------- tests/unit/test_api.py | 31 +++++++++- tests/unit/test_api_patch.py | 33 +++++++--- tests/unit/test_bulk.py | 9 +++ tests/unit/test_crud.py | 14 +++-- 9 files changed, 252 insertions(+), 78 deletions(-) diff --git a/ctms/crud.py b/ctms/crud.py index f865087e..1ced1368 100644 --- a/ctms/crud.py +++ b/ctms/crud.py @@ -10,6 +10,10 @@ from sqlalchemy.dialects.postgresql import insert from sqlalchemy.orm import Session, joinedload, load_only, selectinload +from ctms.schemas.email import EmailSchema +from ctms.schemas.newsletter import NewsletterTableSchema +from ctms.schemas.waitlist import WaitlistTableSchema + from .auth import hash_password from .backport_legacy_waitlists import format_legacy_vpn_relay_waitlist_input from .database import Base @@ -369,7 +373,7 @@ def create_or_update_amo(db: Session, email_id: UUID4, amo: Optional[AddOnsInSch db.execute(stmt) -def create_email(db: Session, email: EmailInSchema): +def create_email(db: Session, email: EmailInSchema | EmailSchema): db_email = Email(**email.dict()) db.add(db_email) @@ -439,11 +443,14 @@ def create_or_update_mofo( def create_newsletter( - db: Session, email_id: UUID4, newsletter: NewsletterInSchema + db: Session, email_id: UUID4, newsletter: NewsletterInSchema | NewsletterTableSchema ) -> Optional[Newsletter]: if newsletter.is_default(): return None - db_newsletter = Newsletter(email_id=email_id, **newsletter.dict()) + # This is called from API input data with `NewsletterInSchema`, and from tests fixtures + # with `NewsletterTableSchema` + # Unlike `NewsletterTableSchema`, `NewsletterInSchema` has no `email_id` field. + db_newsletter = Newsletter(**{"email_id": email_id, **newsletter.dict()}) db.add(db_newsletter) return db_newsletter @@ -473,15 +480,19 @@ def create_or_update_newsletters( def create_waitlist( - db: Session, email_id: UUID4, waitlist: WaitlistInSchema + db: Session, email_id: UUID4, waitlist: WaitlistInSchema | WaitlistTableSchema ) -> 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()) + + # This is called from API input data with `WaitlistInSchema`, and from tests fixtures + # with `WaitlistTableSchema` + # Unlike `WaitlistTableSchema`, `WaitlistInSchema` has no `email_id`, `subscribed` fields. + attrs = {"email_id": email_id, **waitlist.dict()} + if "subscribed" in attrs: + del attrs["subscribed"] + + db_waitlist = Waitlist(**attrs) db.add(db_waitlist) return db_waitlist @@ -517,7 +528,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 | ContactSchema, + 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 328ecfc9..fae681c3 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -189,28 +189,49 @@ def most_minimal_contact(dbsession): @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): +def minimal_contact(minimal_contact_data, dbsession) -> ContactSchema: create_contact( dbsession, minimal_contact_data.email.email_id, @@ -222,7 +243,8 @@ def minimal_contact(minimal_contact_data, dbsession): @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", @@ -240,7 +262,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", @@ -250,7 +272,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( @@ -266,58 +288,101 @@ 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): +def maximal_contact(dbsession, maximal_contact_data) -> ContactSchema: create_contact( dbsession, maximal_contact_data.email.email_id, @@ -329,7 +394,7 @@ def maximal_contact(dbsession, 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)), @@ -338,14 +403,14 @@ 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): +def example_contact(dbsession, example_contact_data) -> ContactSchema: create_contact( dbsession, example_contact_data.email.email_id, diff --git a/tests/unit/test_api.py b/tests/unit/test_api.py index 5e5d3d01..a63d55d4 100644 --- a/tests/unit/test_api.py +++ b/tests/unit/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/test_api_patch.py b/tests/unit/test_api_patch.py index 0af4681a..1e530e5d 100644 --- a/tests/unit/test_api_patch.py +++ b/tests/unit/test_api_patch.py @@ -17,6 +17,7 @@ MozillaFoundationInSchema, MozillaFoundationSchema, ) +from ctms.schemas.waitlist import WaitlistInSchema 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 `ContactTableSchema` + # 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 @@ -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/test_bulk.py b/tests/unit/test_bulk.py index 8460c164..784776c9 100644 --- a/tests/unit/test_bulk.py +++ b/tests/unit/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_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): From 4c4c85372efaf29b05686d9bd68b4152c2a607c8 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Mon, 26 Jun 2023 10:44:22 +0200 Subject: [PATCH 2/7] Reproduce #611 in a dedicated test --- ctms/acoustic_service.py | 14 ++++++-------- tests/unit/test_acoustic_service.py | 28 +++++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 9 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/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, From d0e27b0256f0d6153b11a78296d924bbc22d7006 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Mon, 26 Jun 2023 17:14:01 +0200 Subject: [PATCH 3/7] Rename ContactSchema to ContactTableSchema for consistency --- ctms/acoustic_service.py | 6 ++--- ctms/app.py | 6 ++--- ctms/bin/acoustic.py | 4 +-- ctms/crud.py | 16 +++++++----- ctms/schemas/__init__.py | 2 +- ctms/schemas/contact.py | 10 ++++---- tests/unit/conftest.py | 40 +++++++++++++++-------------- tests/unit/test_acoustic_service.py | 6 ++--- tests/unit/test_api.py | 4 +-- 9 files changed, 49 insertions(+), 45 deletions(-) diff --git a/ctms/acoustic_service.py b/ctms/acoustic_service.py index cf7efffc..008e4a4e 100644 --- a/ctms/acoustic_service.py +++ b/ctms/acoustic_service.py @@ -12,7 +12,7 @@ from ctms.background_metrics import BackgroundMetricService from ctms.config import re_trace_email -from ctms.schemas import ContactSchema, NewsletterSchema +from ctms.schemas import ContactTableSchema, NewsletterSchema # Start cherry-picked from django.utils.encoding _PROTECTED_TYPES = ( @@ -140,7 +140,7 @@ def __init__( def convert_ctms_to_acoustic( self, - contact: ContactSchema, + contact: ContactTableSchema, main_fields: set[str], newsletters_mapping: dict[str, str], ): @@ -428,7 +428,7 @@ def _insert_update_relational_table(self, table_name, rows): def attempt_to_upload_ctms_contact( self, - contact: ContactSchema, + contact: ContactTableSchema, main_fields: set[str], newsletters_mapping: dict[str, str], ): # raises AcousticUploadError diff --git a/ctms/app.py b/ctms/app.py index 7a5e06f5..0f5e0f3b 100644 --- a/ctms/app.py +++ b/ctms/app.py @@ -70,7 +70,7 @@ ContactInSchema, ContactPatchSchema, ContactPutSchema, - ContactSchema, + ContactTableSchema, CTMSBulkResponse, CTMSResponse, CTMSSingleResponse, @@ -152,10 +152,10 @@ def get_email_or_404(db: Session, email_id) -> Email: return email -def get_contact_or_404(db: Session, email_id) -> ContactSchema: +def get_contact_or_404(db: Session, email_id) -> ContactTableSchema: """Get a contact by email_ID, or raise a 404 exception.""" email = get_email_or_404(db, email_id) - return ContactSchema.from_email(email) + return ContactTableSchema.from_email(email) def all_ids( diff --git a/ctms/bin/acoustic.py b/ctms/bin/acoustic.py index c8581f04..a95fd673 100755 --- a/ctms/bin/acoustic.py +++ b/ctms/bin/acoustic.py @@ -27,7 +27,7 @@ from ctms.database import SessionLocal from ctms.exception_capture import init_sentry from ctms.log import configure_logging -from ctms.schemas.contact import ContactSchema +from ctms.schemas.contact import ContactTableSchema def confirm(msg): @@ -258,7 +258,7 @@ def do_dump(dbsession, contacts, output: TextIO): fieldnames = None writer = None for email in contacts: - contact = ContactSchema.from_email(email) + contact = ContactTableSchema.from_email(email) main_table_row, _, _ = service.convert_ctms_to_acoustic( contact, main_fields, newsletters_mapping ) diff --git a/ctms/crud.py b/ctms/crud.py index 1ced1368..28894629 100644 --- a/ctms/crud.py +++ b/ctms/crud.py @@ -41,7 +41,7 @@ ApiClientSchema, ContactInSchema, ContactPutSchema, - ContactSchema, + ContactTableSchema, EmailInSchema, EmailPutSchema, FirefoxAccountsInSchema, @@ -161,7 +161,7 @@ def get_bulk_contacts( .all() ) - return [ContactSchema.from_email(email) for email in bulk_contacts] + return [ContactTableSchema.from_email(email) for email in bulk_contacts] def get_email(db: Session, email_id: UUID4) -> Optional[Email]: @@ -172,12 +172,14 @@ def get_email(db: Session, email_id: UUID4) -> Optional[Email]: ) -def get_contact_by_email_id(db: Session, email_id: UUID4) -> Optional[ContactSchema]: +def get_contact_by_email_id( + db: Session, email_id: UUID4 +) -> Optional[ContactTableSchema]: """Return a Contact object for a given email id""" email = get_email(db, email_id) if email is None: return None - return ContactSchema.from_email(email) + return ContactTableSchema.from_email(email) def get_contacts_by_any_id( @@ -191,7 +193,7 @@ def get_contacts_by_any_id( amo_user_id: Optional[str] = None, fxa_id: Optional[str] = None, fxa_primary_email: Optional[str] = None, -) -> List[ContactSchema]: +) -> List[ContactTableSchema]: """ Get all the data for multiple contacts by ID as a list of Contacts. @@ -239,7 +241,7 @@ def get_contacts_by_any_id( fxa_primary_email_insensitive_comparator=fxa_primary_email ) emails = cast(List[Email], statement.all()) - return [ContactSchema.from_email(email) for email in emails] + return [ContactTableSchema.from_email(email) for email in emails] def _acoustic_sync_retry_query(db: Session): @@ -530,7 +532,7 @@ def create_or_update_waitlists( def create_contact( db: Session, email_id: UUID4, - contact: ContactInSchema | ContactSchema, + contact: ContactInSchema | ContactTableSchema, metrics: Optional[Dict], ): create_email(db, contact.email) diff --git a/ctms/schemas/__init__.py b/ctms/schemas/__init__.py index 76b4a3c5..ce12a5a6 100644 --- a/ctms/schemas/__init__.py +++ b/ctms/schemas/__init__.py @@ -5,7 +5,7 @@ ContactInSchema, ContactPatchSchema, ContactPutSchema, - ContactSchema, + ContactTableSchema, CTMSBulkResponse, CTMSResponse, CTMSSingleResponse, diff --git a/ctms/schemas/contact.py b/ctms/schemas/contact.py index 1874a6c4..dbbfc08a 100644 --- a/ctms/schemas/contact.py +++ b/ctms/schemas/contact.py @@ -138,7 +138,7 @@ def get_stripe_products(email: "Email") -> List[ProductBaseSchema]: return products -class ContactSchema(ComparableBase): +class ContactTableSchema(ComparableBase): """A complete contact.""" amo: Optional[AddOnsSchema] = None @@ -150,7 +150,7 @@ class ContactSchema(ComparableBase): products: List[ProductBaseSchema] = [] @classmethod - def from_email(cls, email: "Email") -> "ContactSchema": + def from_email(cls, email: "Email") -> "ContactTableSchema": return cls( amo=email.amo, email=email, @@ -247,7 +247,7 @@ class ContactInBase(ComparableBase): relay_waitlist: Optional[RelayWaitlistInSchema] = None class Config: - fields = ContactSchema.Config.fields + fields = ContactTableSchema.Config.fields @root_validator def check_fields(cls, values): # pylint:disable = no-self-argument @@ -353,7 +353,7 @@ class CTMSResponse(BaseModel): """ Response for GET /ctms/ by alternate IDs - Similar to ContactSchema, but groups are required + Similar to ContactTableSchema, but groups are required """ amo: AddOnsSchema @@ -414,7 +414,7 @@ class CTMSSingleResponse(CTMSResponse): """ Response for /ctms/ - Similar to ContactSchema, but groups are required and includes status: OK + Similar to ContactTableSchema, but groups are required and includes status: OK """ status: Literal["ok"] = Field( diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index fae681c3..c73ff34c 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -35,7 +35,7 @@ get_waitlists_by_email_id, ) from ctms.database import ScopedSessionLocal, SessionLocal -from ctms.schemas import ApiClientSchema, ContactSchema +from ctms.schemas import ApiClientSchema, ContactTableSchema from tests import factories from tests.data import fake_stripe_id @@ -177,7 +177,7 @@ def end_savepoint(*args): @pytest.fixture def most_minimal_contact(dbsession): - contact = ContactSchema( + contact = ContactTableSchema( email=schemas.EmailSchema( email_id=UUID("62d8d3c6-95f3-4ed6-b176-7f69acff22f6"), primary_email="ctms-most-minimal-user@example.com", @@ -189,9 +189,9 @@ def most_minimal_contact(dbsession): @pytest.fixture -def minimal_contact_data() -> ContactSchema: +def minimal_contact_data() -> ContactTableSchema: email_id = UUID("93db83d4-4119-4e0c-af87-a713786fa81d") - return ContactSchema( + return ContactTableSchema( email=schemas.EmailSchema( basket_token="142e20b6-1ef5-43d8-b5f4-597430e956d7", create_timestamp="2014-01-22T15:24:00.000+0000", @@ -231,7 +231,7 @@ def minimal_contact_data() -> ContactSchema: @pytest.fixture -def minimal_contact(minimal_contact_data, dbsession) -> ContactSchema: +def minimal_contact(minimal_contact_data, dbsession) -> ContactTableSchema: create_contact( dbsession, minimal_contact_data.email.email_id, @@ -243,9 +243,9 @@ def minimal_contact(minimal_contact_data, dbsession) -> ContactSchema: @pytest.fixture -def maximal_contact_data() -> ContactSchema: +def maximal_contact_data() -> ContactTableSchema: email_id = UUID("67e52c77-950f-4f28-accb-bb3ea1a2c51a") - return ContactSchema( + return ContactTableSchema( amo=schemas.AddOnsSchema( add_on_ids="fanfox,foxfan", display_name="#1 Mozilla Fan", @@ -382,7 +382,7 @@ def maximal_contact_data() -> ContactSchema: @pytest.fixture -def maximal_contact(dbsession, maximal_contact_data) -> ContactSchema: +def maximal_contact(dbsession, maximal_contact_data) -> ContactTableSchema: create_contact( dbsession, maximal_contact_data.email.email_id, @@ -394,23 +394,25 @@ def maximal_contact(dbsession, maximal_contact_data) -> ContactSchema: @pytest.fixture -def example_contact_data() -> ContactSchema: - return ContactSchema( +def example_contact_data() -> ContactTableSchema: + return ContactTableSchema( amo=schemas.AddOnsSchema(**_gather_examples(schemas.AddOnsSchema)), email=schemas.EmailSchema(**_gather_examples(schemas.EmailSchema)), fxa=schemas.FirefoxAccountsSchema( **_gather_examples(schemas.FirefoxAccountsSchema) ), - newsletters=ContactSchema.schema()["properties"]["newsletters"]["example"], + newsletters=ContactTableSchema.schema()["properties"]["newsletters"]["example"], waitlists=[ schemas.WaitlistTableSchema(**example) - for example in ContactSchema.schema()["properties"]["waitlists"]["example"] + for example in ContactTableSchema.schema()["properties"]["waitlists"][ + "example" + ] ], ) @pytest.fixture -def example_contact(dbsession, example_contact_data) -> ContactSchema: +def example_contact(dbsession, example_contact_data) -> ContactTableSchema: create_contact( dbsession, example_contact_data.email.email_id, @@ -423,7 +425,7 @@ def example_contact(dbsession, example_contact_data) -> ContactSchema: @pytest.fixture def to_add_contact_data(): - return ContactSchema( + return ContactTableSchema( email=schemas.EmailInSchema( basket_token="21aeb466-4003-4c2b-a27e-e6651c13d231", email_id=UUID("d1da1c99-fe09-44db-9c68-78a75752574d"), @@ -448,7 +450,7 @@ def to_add_contact(dbsession, to_add_contact_data): @pytest.fixture def simple_default_contact_data(): - return ContactSchema( + return ContactTableSchema( email=schemas.EmailInSchema( basket_token="d3a827b5-747c-41c2-8381-59ce9ad63050", email_id=UUID("4f98b303-8863-421f-95d3-847cd4d83c9f"), @@ -474,7 +476,7 @@ def simple_default_contact(dbsession, simple_default_contact_data): @pytest.fixture def default_newsletter_contact_data(): - contact = ContactSchema( + contact = ContactTableSchema( email=schemas.EmailInSchema( basket_token="b5487fbf-86ae-44b9-a638-bbb037ce61a6", email_id=UUID("dd2bc52c-49e4-4df9-95a8-197d3a7794cd"), @@ -570,7 +572,7 @@ def post_contact(client, dbsession, request): email_id = contact_fixture.email.email_id def _add( - modifier: Callable[[ContactSchema], ContactSchema] = lambda x: x, + modifier: Callable[[ContactTableSchema], ContactTableSchema] = lambda x: x, code: int = 201, stored_contacts: int = 1, check_redirect: bool = True, @@ -650,12 +652,12 @@ def put_contact(client, dbsession, request): sample_email_id = contact_fixture.email.email_id def _add( - modifier: Callable[[ContactSchema], ContactSchema] = lambda x: x, + modifier: Callable[[ContactTableSchema], ContactTableSchema] = lambda x: x, code: int = 201, stored_contacts: int = 1, query_fields: Optional[dict] = None, check_written: bool = True, - record: Optional[ContactSchema] = None, + record: Optional[ContactTableSchema] = None, new_default_fields: Optional[set] = None, ): if record: diff --git a/tests/unit/test_acoustic_service.py b/tests/unit/test_acoustic_service.py index 20c5f5de..3cb74258 100644 --- a/tests/unit/test_acoustic_service.py +++ b/tests/unit/test_acoustic_service.py @@ -8,7 +8,7 @@ from ctms import acoustic_service from ctms.acoustic_service import CTMSToAcousticService from ctms.crud import get_contact_by_email_id, get_newsletters_by_email_id -from ctms.schemas.contact import ContactSchema +from ctms.schemas.contact import ContactTableSchema CTMS_ACOUSTIC_MAIN_TABLE_ID = "1" CTMS_ACOUSTIC_NEWSLETTER_TABLE_ID = "9" @@ -264,7 +264,7 @@ def test_ctms_to_acoustic_with_subscription( dbsession.commit() contact = get_contact_by_email_id(dbsession, email_id=subscription.get_email_id()) - contact = ContactSchema.parse_obj(contact) + contact = ContactTableSchema.parse_obj(contact) acoustic_mock = MagicMock() base_ctms_acoustic_service.acoustic = acoustic_mock @@ -319,7 +319,7 @@ def test_ctms_to_acoustic_with_subscription_and_metrics( dbsession.commit() contact = get_contact_by_email_id(dbsession, email_id=subscription.get_email_id()) - contact = ContactSchema.parse_obj(contact) + contact = ContactTableSchema.parse_obj(contact) acoustic_mock = MagicMock() acoustic_svc = metrics_ctms_acoustic_service diff --git a/tests/unit/test_api.py b/tests/unit/test_api.py index a63d55d4..2c744c46 100644 --- a/tests/unit/test_api.py +++ b/tests/unit/test_api.py @@ -4,7 +4,7 @@ import pytest -from ctms.schemas import ContactInSchema, ContactSchema, NewsletterInSchema +from ctms.schemas import ContactInSchema, ContactTableSchema, NewsletterInSchema from tests.unit.conftest import SAMPLE_CONTACT_PARAMS API_TEST_CASES: Tuple[Tuple[str, str, Any], ...] = ( @@ -155,7 +155,7 @@ def _compare_written_contacts( assert saved_contact.idempotent_equal(sample) -def find_default_fields(contact: ContactSchema) -> Set[str]: +def find_default_fields(contact: ContactTableSchema) -> 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(): From c433f838d5e64f1d789669a57a3b4db60fe7e9d3 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Tue, 27 Jun 2023 10:35:48 +0200 Subject: [PATCH 4/7] Revert "Rename ContactSchema to ContactTableSchema for consistency" Because there is not contact table in the database :) This reverts commit d0e27b0256f0d6153b11a78296d924bbc22d7006. --- ctms/acoustic_service.py | 6 ++--- ctms/app.py | 6 ++--- ctms/bin/acoustic.py | 4 +-- ctms/crud.py | 16 +++++------- ctms/schemas/__init__.py | 2 +- ctms/schemas/contact.py | 10 ++++---- tests/unit/conftest.py | 40 ++++++++++++++--------------- tests/unit/test_acoustic_service.py | 6 ++--- tests/unit/test_api.py | 4 +-- 9 files changed, 45 insertions(+), 49 deletions(-) diff --git a/ctms/acoustic_service.py b/ctms/acoustic_service.py index 008e4a4e..cf7efffc 100644 --- a/ctms/acoustic_service.py +++ b/ctms/acoustic_service.py @@ -12,7 +12,7 @@ from ctms.background_metrics import BackgroundMetricService from ctms.config import re_trace_email -from ctms.schemas import ContactTableSchema, NewsletterSchema +from ctms.schemas import ContactSchema, NewsletterSchema # Start cherry-picked from django.utils.encoding _PROTECTED_TYPES = ( @@ -140,7 +140,7 @@ def __init__( def convert_ctms_to_acoustic( self, - contact: ContactTableSchema, + contact: ContactSchema, main_fields: set[str], newsletters_mapping: dict[str, str], ): @@ -428,7 +428,7 @@ def _insert_update_relational_table(self, table_name, rows): def attempt_to_upload_ctms_contact( self, - contact: ContactTableSchema, + contact: ContactSchema, main_fields: set[str], newsletters_mapping: dict[str, str], ): # raises AcousticUploadError diff --git a/ctms/app.py b/ctms/app.py index 0f5e0f3b..7a5e06f5 100644 --- a/ctms/app.py +++ b/ctms/app.py @@ -70,7 +70,7 @@ ContactInSchema, ContactPatchSchema, ContactPutSchema, - ContactTableSchema, + ContactSchema, CTMSBulkResponse, CTMSResponse, CTMSSingleResponse, @@ -152,10 +152,10 @@ def get_email_or_404(db: Session, email_id) -> Email: return email -def get_contact_or_404(db: Session, email_id) -> ContactTableSchema: +def get_contact_or_404(db: Session, email_id) -> ContactSchema: """Get a contact by email_ID, or raise a 404 exception.""" email = get_email_or_404(db, email_id) - return ContactTableSchema.from_email(email) + return ContactSchema.from_email(email) def all_ids( diff --git a/ctms/bin/acoustic.py b/ctms/bin/acoustic.py index a95fd673..c8581f04 100755 --- a/ctms/bin/acoustic.py +++ b/ctms/bin/acoustic.py @@ -27,7 +27,7 @@ from ctms.database import SessionLocal from ctms.exception_capture import init_sentry from ctms.log import configure_logging -from ctms.schemas.contact import ContactTableSchema +from ctms.schemas.contact import ContactSchema def confirm(msg): @@ -258,7 +258,7 @@ def do_dump(dbsession, contacts, output: TextIO): fieldnames = None writer = None for email in contacts: - contact = ContactTableSchema.from_email(email) + contact = ContactSchema.from_email(email) main_table_row, _, _ = service.convert_ctms_to_acoustic( contact, main_fields, newsletters_mapping ) diff --git a/ctms/crud.py b/ctms/crud.py index 28894629..1ced1368 100644 --- a/ctms/crud.py +++ b/ctms/crud.py @@ -41,7 +41,7 @@ ApiClientSchema, ContactInSchema, ContactPutSchema, - ContactTableSchema, + ContactSchema, EmailInSchema, EmailPutSchema, FirefoxAccountsInSchema, @@ -161,7 +161,7 @@ def get_bulk_contacts( .all() ) - return [ContactTableSchema.from_email(email) for email in bulk_contacts] + return [ContactSchema.from_email(email) for email in bulk_contacts] def get_email(db: Session, email_id: UUID4) -> Optional[Email]: @@ -172,14 +172,12 @@ def get_email(db: Session, email_id: UUID4) -> Optional[Email]: ) -def get_contact_by_email_id( - db: Session, email_id: UUID4 -) -> Optional[ContactTableSchema]: +def get_contact_by_email_id(db: Session, email_id: UUID4) -> Optional[ContactSchema]: """Return a Contact object for a given email id""" email = get_email(db, email_id) if email is None: return None - return ContactTableSchema.from_email(email) + return ContactSchema.from_email(email) def get_contacts_by_any_id( @@ -193,7 +191,7 @@ def get_contacts_by_any_id( amo_user_id: Optional[str] = None, fxa_id: Optional[str] = None, fxa_primary_email: Optional[str] = None, -) -> List[ContactTableSchema]: +) -> List[ContactSchema]: """ Get all the data for multiple contacts by ID as a list of Contacts. @@ -241,7 +239,7 @@ def get_contacts_by_any_id( fxa_primary_email_insensitive_comparator=fxa_primary_email ) emails = cast(List[Email], statement.all()) - return [ContactTableSchema.from_email(email) for email in emails] + return [ContactSchema.from_email(email) for email in emails] def _acoustic_sync_retry_query(db: Session): @@ -532,7 +530,7 @@ def create_or_update_waitlists( def create_contact( db: Session, email_id: UUID4, - contact: ContactInSchema | ContactTableSchema, + contact: ContactInSchema | ContactSchema, metrics: Optional[Dict], ): create_email(db, contact.email) diff --git a/ctms/schemas/__init__.py b/ctms/schemas/__init__.py index ce12a5a6..76b4a3c5 100644 --- a/ctms/schemas/__init__.py +++ b/ctms/schemas/__init__.py @@ -5,7 +5,7 @@ ContactInSchema, ContactPatchSchema, ContactPutSchema, - ContactTableSchema, + ContactSchema, CTMSBulkResponse, CTMSResponse, CTMSSingleResponse, diff --git a/ctms/schemas/contact.py b/ctms/schemas/contact.py index dbbfc08a..1874a6c4 100644 --- a/ctms/schemas/contact.py +++ b/ctms/schemas/contact.py @@ -138,7 +138,7 @@ def get_stripe_products(email: "Email") -> List[ProductBaseSchema]: return products -class ContactTableSchema(ComparableBase): +class ContactSchema(ComparableBase): """A complete contact.""" amo: Optional[AddOnsSchema] = None @@ -150,7 +150,7 @@ class ContactTableSchema(ComparableBase): products: List[ProductBaseSchema] = [] @classmethod - def from_email(cls, email: "Email") -> "ContactTableSchema": + def from_email(cls, email: "Email") -> "ContactSchema": return cls( amo=email.amo, email=email, @@ -247,7 +247,7 @@ class ContactInBase(ComparableBase): relay_waitlist: Optional[RelayWaitlistInSchema] = None class Config: - fields = ContactTableSchema.Config.fields + fields = ContactSchema.Config.fields @root_validator def check_fields(cls, values): # pylint:disable = no-self-argument @@ -353,7 +353,7 @@ class CTMSResponse(BaseModel): """ Response for GET /ctms/ by alternate IDs - Similar to ContactTableSchema, but groups are required + Similar to ContactSchema, but groups are required """ amo: AddOnsSchema @@ -414,7 +414,7 @@ class CTMSSingleResponse(CTMSResponse): """ Response for /ctms/ - Similar to ContactTableSchema, but groups are required and includes status: OK + Similar to ContactSchema, but groups are required and includes status: OK """ status: Literal["ok"] = Field( diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index c73ff34c..fae681c3 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -35,7 +35,7 @@ get_waitlists_by_email_id, ) from ctms.database import ScopedSessionLocal, SessionLocal -from ctms.schemas import ApiClientSchema, ContactTableSchema +from ctms.schemas import ApiClientSchema, ContactSchema from tests import factories from tests.data import fake_stripe_id @@ -177,7 +177,7 @@ def end_savepoint(*args): @pytest.fixture def most_minimal_contact(dbsession): - contact = ContactTableSchema( + contact = ContactSchema( email=schemas.EmailSchema( email_id=UUID("62d8d3c6-95f3-4ed6-b176-7f69acff22f6"), primary_email="ctms-most-minimal-user@example.com", @@ -189,9 +189,9 @@ def most_minimal_contact(dbsession): @pytest.fixture -def minimal_contact_data() -> ContactTableSchema: +def minimal_contact_data() -> ContactSchema: email_id = UUID("93db83d4-4119-4e0c-af87-a713786fa81d") - return ContactTableSchema( + return ContactSchema( email=schemas.EmailSchema( basket_token="142e20b6-1ef5-43d8-b5f4-597430e956d7", create_timestamp="2014-01-22T15:24:00.000+0000", @@ -231,7 +231,7 @@ def minimal_contact_data() -> ContactTableSchema: @pytest.fixture -def minimal_contact(minimal_contact_data, dbsession) -> ContactTableSchema: +def minimal_contact(minimal_contact_data, dbsession) -> ContactSchema: create_contact( dbsession, minimal_contact_data.email.email_id, @@ -243,9 +243,9 @@ def minimal_contact(minimal_contact_data, dbsession) -> ContactTableSchema: @pytest.fixture -def maximal_contact_data() -> ContactTableSchema: +def maximal_contact_data() -> ContactSchema: email_id = UUID("67e52c77-950f-4f28-accb-bb3ea1a2c51a") - return ContactTableSchema( + return ContactSchema( amo=schemas.AddOnsSchema( add_on_ids="fanfox,foxfan", display_name="#1 Mozilla Fan", @@ -382,7 +382,7 @@ def maximal_contact_data() -> ContactTableSchema: @pytest.fixture -def maximal_contact(dbsession, maximal_contact_data) -> ContactTableSchema: +def maximal_contact(dbsession, maximal_contact_data) -> ContactSchema: create_contact( dbsession, maximal_contact_data.email.email_id, @@ -394,25 +394,23 @@ def maximal_contact(dbsession, maximal_contact_data) -> ContactTableSchema: @pytest.fixture -def example_contact_data() -> ContactTableSchema: - return ContactTableSchema( +def example_contact_data() -> ContactSchema: + return ContactSchema( amo=schemas.AddOnsSchema(**_gather_examples(schemas.AddOnsSchema)), email=schemas.EmailSchema(**_gather_examples(schemas.EmailSchema)), fxa=schemas.FirefoxAccountsSchema( **_gather_examples(schemas.FirefoxAccountsSchema) ), - newsletters=ContactTableSchema.schema()["properties"]["newsletters"]["example"], + newsletters=ContactSchema.schema()["properties"]["newsletters"]["example"], waitlists=[ schemas.WaitlistTableSchema(**example) - for example in ContactTableSchema.schema()["properties"]["waitlists"][ - "example" - ] + for example in ContactSchema.schema()["properties"]["waitlists"]["example"] ], ) @pytest.fixture -def example_contact(dbsession, example_contact_data) -> ContactTableSchema: +def example_contact(dbsession, example_contact_data) -> ContactSchema: create_contact( dbsession, example_contact_data.email.email_id, @@ -425,7 +423,7 @@ def example_contact(dbsession, example_contact_data) -> ContactTableSchema: @pytest.fixture def to_add_contact_data(): - return ContactTableSchema( + return ContactSchema( email=schemas.EmailInSchema( basket_token="21aeb466-4003-4c2b-a27e-e6651c13d231", email_id=UUID("d1da1c99-fe09-44db-9c68-78a75752574d"), @@ -450,7 +448,7 @@ def to_add_contact(dbsession, to_add_contact_data): @pytest.fixture def simple_default_contact_data(): - return ContactTableSchema( + return ContactSchema( email=schemas.EmailInSchema( basket_token="d3a827b5-747c-41c2-8381-59ce9ad63050", email_id=UUID("4f98b303-8863-421f-95d3-847cd4d83c9f"), @@ -476,7 +474,7 @@ def simple_default_contact(dbsession, simple_default_contact_data): @pytest.fixture def default_newsletter_contact_data(): - contact = ContactTableSchema( + contact = ContactSchema( email=schemas.EmailInSchema( basket_token="b5487fbf-86ae-44b9-a638-bbb037ce61a6", email_id=UUID("dd2bc52c-49e4-4df9-95a8-197d3a7794cd"), @@ -572,7 +570,7 @@ def post_contact(client, dbsession, request): email_id = contact_fixture.email.email_id def _add( - modifier: Callable[[ContactTableSchema], ContactTableSchema] = lambda x: x, + modifier: Callable[[ContactSchema], ContactSchema] = lambda x: x, code: int = 201, stored_contacts: int = 1, check_redirect: bool = True, @@ -652,12 +650,12 @@ def put_contact(client, dbsession, request): sample_email_id = contact_fixture.email.email_id def _add( - modifier: Callable[[ContactTableSchema], ContactTableSchema] = lambda x: x, + modifier: Callable[[ContactSchema], ContactSchema] = lambda x: x, code: int = 201, stored_contacts: int = 1, query_fields: Optional[dict] = None, check_written: bool = True, - record: Optional[ContactTableSchema] = None, + record: Optional[ContactSchema] = None, new_default_fields: Optional[set] = None, ): if record: diff --git a/tests/unit/test_acoustic_service.py b/tests/unit/test_acoustic_service.py index 3cb74258..20c5f5de 100644 --- a/tests/unit/test_acoustic_service.py +++ b/tests/unit/test_acoustic_service.py @@ -8,7 +8,7 @@ from ctms import acoustic_service from ctms.acoustic_service import CTMSToAcousticService from ctms.crud import get_contact_by_email_id, get_newsletters_by_email_id -from ctms.schemas.contact import ContactTableSchema +from ctms.schemas.contact import ContactSchema CTMS_ACOUSTIC_MAIN_TABLE_ID = "1" CTMS_ACOUSTIC_NEWSLETTER_TABLE_ID = "9" @@ -264,7 +264,7 @@ def test_ctms_to_acoustic_with_subscription( dbsession.commit() contact = get_contact_by_email_id(dbsession, email_id=subscription.get_email_id()) - contact = ContactTableSchema.parse_obj(contact) + contact = ContactSchema.parse_obj(contact) acoustic_mock = MagicMock() base_ctms_acoustic_service.acoustic = acoustic_mock @@ -319,7 +319,7 @@ def test_ctms_to_acoustic_with_subscription_and_metrics( dbsession.commit() contact = get_contact_by_email_id(dbsession, email_id=subscription.get_email_id()) - contact = ContactTableSchema.parse_obj(contact) + contact = ContactSchema.parse_obj(contact) acoustic_mock = MagicMock() acoustic_svc = metrics_ctms_acoustic_service diff --git a/tests/unit/test_api.py b/tests/unit/test_api.py index 2c744c46..a63d55d4 100644 --- a/tests/unit/test_api.py +++ b/tests/unit/test_api.py @@ -4,7 +4,7 @@ import pytest -from ctms.schemas import ContactInSchema, ContactTableSchema, NewsletterInSchema +from ctms.schemas import ContactInSchema, ContactSchema, NewsletterInSchema from tests.unit.conftest import SAMPLE_CONTACT_PARAMS API_TEST_CASES: Tuple[Tuple[str, str, Any], ...] = ( @@ -155,7 +155,7 @@ def _compare_written_contacts( assert saved_contact.idempotent_equal(sample) -def find_default_fields(contact: ContactTableSchema) -> Set[str]: +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(): From da1beea05328265f5df53ddfe806b4c2467f3d68 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Tue, 27 Jun 2023 11:22:36 +0200 Subject: [PATCH 5/7] Save timestamps manually in tests, instead of transparently in crud.py --- ctms/crud.py | 24 ++-- tests/unit/conftest.py | 115 +++++++++++-------- tests/unit/test_api_patch.py | 10 +- tests/unit/test_backport_legacy_waitlists.py | 63 +++++++--- 4 files changed, 127 insertions(+), 85 deletions(-) diff --git a/ctms/crud.py b/ctms/crud.py index 1ced1368..d5deded6 100644 --- a/ctms/crud.py +++ b/ctms/crud.py @@ -10,10 +10,6 @@ from sqlalchemy.dialects.postgresql import insert from sqlalchemy.orm import Session, joinedload, load_only, selectinload -from ctms.schemas.email import EmailSchema -from ctms.schemas.newsletter import NewsletterTableSchema -from ctms.schemas.waitlist import WaitlistTableSchema - from .auth import hash_password from .backport_legacy_waitlists import format_legacy_vpn_relay_waitlist_input from .database import Base @@ -373,7 +369,7 @@ def create_or_update_amo(db: Session, email_id: UUID4, amo: Optional[AddOnsInSch db.execute(stmt) -def create_email(db: Session, email: EmailInSchema | EmailSchema): +def create_email(db: Session, email: EmailInSchema): db_email = Email(**email.dict()) db.add(db_email) @@ -443,14 +439,11 @@ def create_or_update_mofo( def create_newsletter( - db: Session, email_id: UUID4, newsletter: NewsletterInSchema | NewsletterTableSchema + db: Session, email_id: UUID4, newsletter: NewsletterInSchema ) -> Optional[Newsletter]: if newsletter.is_default(): return None - # This is called from API input data with `NewsletterInSchema`, and from tests fixtures - # with `NewsletterTableSchema` - # Unlike `NewsletterTableSchema`, `NewsletterInSchema` has no `email_id` field. - db_newsletter = Newsletter(**{"email_id": email_id, **newsletter.dict()}) + db_newsletter = Newsletter(email_id=email_id, **newsletter.dict()) db.add(db_newsletter) return db_newsletter @@ -480,19 +473,16 @@ def create_or_update_newsletters( def create_waitlist( - db: Session, email_id: UUID4, waitlist: WaitlistInSchema | WaitlistTableSchema + db: Session, email_id: UUID4, waitlist: WaitlistInSchema ) -> Optional[Waitlist]: if waitlist.is_default(): return None - # This is called from API input data with `WaitlistInSchema`, and from tests fixtures - # with `WaitlistTableSchema` - # Unlike `WaitlistTableSchema`, `WaitlistInSchema` has no `email_id`, `subscribed` fields. - attrs = {"email_id": email_id, **waitlist.dict()} + attrs = waitlist.dict() if "subscribed" in attrs: del attrs["subscribed"] - db_waitlist = Waitlist(**attrs) + db_waitlist = Waitlist(email_id=email_id, **attrs) db.add(db_waitlist) return db_waitlist @@ -530,7 +520,7 @@ def create_or_update_waitlists( def create_contact( db: Session, email_id: UUID4, - contact: ContactInSchema | ContactSchema, + contact: ContactInSchema, metrics: Optional[Dict], ): create_email(db, contact.email) diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index fae681c3..ff3d3386 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -29,6 +29,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, @@ -36,6 +37,7 @@ ) from ctms.database import ScopedSessionLocal, SessionLocal from ctms.schemas import ApiClientSchema, ContactSchema +from ctms.schemas.contact import ContactInSchema from tests import factories from tests.data import fake_stripe_id @@ -175,6 +177,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( @@ -183,8 +231,7 @@ 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 @@ -231,14 +278,8 @@ def minimal_contact_data() -> ContactSchema: @pytest.fixture -def minimal_contact(minimal_contact_data, dbsession) -> ContactSchema: - 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 @@ -382,14 +423,8 @@ def maximal_contact_data() -> ContactSchema: @pytest.fixture -def maximal_contact(dbsession, maximal_contact_data) -> ContactSchema: - 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 @@ -411,18 +446,12 @@ def example_contact_data() -> ContactSchema: @pytest.fixture def example_contact(dbsession, example_contact_data) -> ContactSchema: - create_contact( - dbsession, - example_contact_data.email.email_id, - example_contact_data, - get_metrics(), - ) - dbsession.commit() + 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", @@ -435,14 +464,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 @@ -461,14 +484,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 @@ -488,14 +507,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/test_api_patch.py b/tests/unit/test_api_patch.py index 1e530e5d..d72887aa 100644 --- a/tests/unit/test_api_patch.py +++ b/tests/unit/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, @@ -18,6 +17,7 @@ MozillaFoundationSchema, ) from ctms.schemas.waitlist import WaitlistInSchema +from tests.unit.conftest import create_full_contact def swap_bool(existing): @@ -188,7 +188,7 @@ 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 `ContactTableSchema` + # `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`. @@ -253,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, @@ -269,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) 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", + ), ], } ) From 830098eb1ac66663eb80d4e418397f9e10cab1f8 Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Wed, 28 Jun 2023 14:23:40 +0200 Subject: [PATCH 6/7] Update ctms/crud.py Co-authored-by: grahamalama --- ctms/crud.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/ctms/crud.py b/ctms/crud.py index d5deded6..a9851ac0 100644 --- a/ctms/crud.py +++ b/ctms/crud.py @@ -478,11 +478,7 @@ def create_waitlist( if waitlist.is_default(): return None - attrs = waitlist.dict() - if "subscribed" in attrs: - del attrs["subscribed"] - - db_waitlist = Waitlist(email_id=email_id, **attrs) + db_waitlist = Waitlist(email_id=email_id, **waitlist.dict(exclude={"subscribed"})) db.add(db_waitlist) return db_waitlist From 4dfceb0fb422d6ec6d951da86de331147b0d790e Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Wed, 28 Jun 2023 14:39:19 +0200 Subject: [PATCH 7/7] Add Graham's comment on waitlist crud function --- ctms/crud.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ctms/crud.py b/ctms/crud.py index a9851ac0..e71f7073 100644 --- a/ctms/crud.py +++ b/ctms/crud.py @@ -478,6 +478,10 @@ def create_waitlist( if waitlist.is_default(): return None + # `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