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 d0e27b0. * Save timestamps manually in tests, instead of transparently in crud.py * Update ctms/crud.py Co-authored-by: grahamalama <gbeckley@mozilla.com> * Add Graham's comment on waitlist crud function --------- Co-authored-by: grahamalama <gbeckley@mozilla.com>
mozilla-it · Jun 28, 2023 · 1342217 · 1342217
1 parent 57c4477
commit 1342217
Show file tree

Hide file tree

Showing 12 changed files with 388 additions and 148 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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,22 +165,54 @@ 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",
                 "example": [
                     {
                         "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
@@ -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),