From ce198051880d7f3653342e0d08b784ba47aaeb2a Mon Sep 17 00:00:00 2001 From: Mathieu Leplatre Date: Fri, 8 Sep 2023 11:56:31 +0200 Subject: [PATCH] Fix #814: add timestamps to newsletters and waitlists in contact responses --- ctms/schemas/contact.py | 12 +++++-- ctms/schemas/newsletter.py | 13 ++++--- ctms/schemas/waitlist.py | 13 ++++--- .../test_basket_waitlist_subscription.py | 12 +++++++ tests/unit/routers/contacts/test_api_get.py | 34 +++++++++++++++++++ tests/unit/routers/contacts/test_api_patch.py | 30 +++++++++++++--- tests/unit/routers/contacts/test_bulk.py | 4 --- 7 files changed, 97 insertions(+), 21 deletions(-) diff --git a/ctms/schemas/contact.py b/ctms/schemas/contact.py index 200dd34a..8085d945 100644 --- a/ctms/schemas/contact.py +++ b/ctms/schemas/contact.py @@ -16,7 +16,12 @@ ) from .fxa import FirefoxAccountsInSchema, FirefoxAccountsSchema from .mofo import MozillaFoundationInSchema, MozillaFoundationSchema -from .newsletter import NewsletterInSchema, NewsletterSchema, NewsletterTableSchema +from .newsletter import ( + NewsletterInSchema, + NewsletterSchema, + NewsletterTableSchema, + NewsletterTimestampedSchema, +) from .product import ProductBaseSchema, ProductSegmentEnum from .waitlist import ( RelayWaitlistInSchema, @@ -26,6 +31,7 @@ WaitlistInSchema, WaitlistSchema, WaitlistTableSchema, + WaitlistTimestampedSchema, validate_waitlist_newsletters, ) @@ -360,8 +366,8 @@ class CTMSResponse(BaseModel): email: EmailSchema fxa: FirefoxAccountsSchema mofo: MozillaFoundationSchema - newsletters: List[NewsletterSchema] - waitlists: List[WaitlistSchema] + newsletters: List[NewsletterTimestampedSchema] + waitlists: List[WaitlistTimestampedSchema] # Retro-compat fields vpn_waitlist: VpnWaitlistSchema relay_waitlist: RelayWaitlistSchema diff --git a/ctms/schemas/newsletter.py b/ctms/schemas/newsletter.py index 85308a37..be67cc29 100644 --- a/ctms/schemas/newsletter.py +++ b/ctms/schemas/newsletter.py @@ -50,11 +50,7 @@ class Config: NewsletterSchema = NewsletterBase -class NewsletterTableSchema(NewsletterBase): - email_id: UUID4 = Field( - description=EMAIL_ID_DESCRIPTION, - example=EMAIL_ID_EXAMPLE, - ) +class NewsletterTimestampedSchema(NewsletterBase): create_timestamp: datetime = Field( description="Newsletter data creation timestamp", example="2020-12-05T19:21:50.908000+00:00", @@ -64,5 +60,12 @@ class NewsletterTableSchema(NewsletterBase): example="2021-02-04T15:36:57.511000+00:00", ) + +class NewsletterTableSchema(NewsletterTimestampedSchema): + email_id: UUID4 = Field( + description=EMAIL_ID_DESCRIPTION, + example=EMAIL_ID_EXAMPLE, + ) + class Config: extra = "forbid" diff --git a/ctms/schemas/waitlist.py b/ctms/schemas/waitlist.py index ed73ea71..5719ada0 100644 --- a/ctms/schemas/waitlist.py +++ b/ctms/schemas/waitlist.py @@ -60,11 +60,7 @@ class Config: WaitlistInSchema = WaitlistBase -class WaitlistTableSchema(WaitlistBase): - email_id: UUID4 = Field( - description=EMAIL_ID_DESCRIPTION, - example=EMAIL_ID_EXAMPLE, - ) +class WaitlistTimestampedSchema(WaitlistBase): create_timestamp: datetime = Field( description="Waitlist data creation timestamp", example="2020-12-05T19:21:50.908000+00:00", @@ -74,6 +70,13 @@ class WaitlistTableSchema(WaitlistBase): example="2021-02-04T15:36:57.511000+00:00", ) + +class WaitlistTableSchema(WaitlistTimestampedSchema): + email_id: UUID4 = Field( + description=EMAIL_ID_DESCRIPTION, + example=EMAIL_ID_EXAMPLE, + ) + class Config: extra = "forbid" diff --git a/tests/integration/test_basket_waitlist_subscription.py b/tests/integration/test_basket_waitlist_subscription.py index a0bd834d..36d70672 100644 --- a/tests/integration/test_basket_waitlist_subscription.py +++ b/tests/integration/test_basket_waitlist_subscription.py @@ -127,6 +127,8 @@ def fetch_created(): contact_details = fetch_created() assert contact_details["newsletters"] == [] + del contact_details["waitlists"][0]["create_timestamp"] + del contact_details["waitlists"][0]["update_timestamp"] assert contact_details["waitlists"] == [ { "name": "vpn", @@ -163,6 +165,8 @@ def fetch_created(): # Request the full contact details again. contact_details = ctms_fetch(email, ctms_headers) assert contact_details["newsletters"] == [] + del contact_details["waitlists"][0]["create_timestamp"] + del contact_details["waitlists"][0]["update_timestamp"] assert contact_details["waitlists"] == [ { "name": "vpn", @@ -222,6 +226,8 @@ def fetch_created(): contact_details = fetch_created() # 3. CTMS should show both formats (legacy `relay_waitlist` field, and entry in `waitlists` list) + del contact_details["waitlists"][0]["create_timestamp"] + del contact_details["waitlists"][0]["update_timestamp"] assert contact_details["waitlists"] == [ { "name": "relay", @@ -256,6 +262,10 @@ def check_subscribed(): contact_details = check_subscribed() assert contact_details["newsletters"] == [] + del contact_details["waitlists"][0]["create_timestamp"] + del contact_details["waitlists"][0]["update_timestamp"] + del contact_details["waitlists"][1]["create_timestamp"] + del contact_details["waitlists"][1]["update_timestamp"] assert contact_details["waitlists"] == [ { "name": "relay", @@ -298,6 +308,8 @@ def check_unsubscribed(): contact_details = check_unsubscribed() # And only one newsletter subscribed. assert contact_details["newsletters"] == [] + del contact_details["waitlists"][0]["create_timestamp"] + del contact_details["waitlists"][0]["update_timestamp"] assert contact_details["waitlists"] == [ { "fields": {"geo": "es"}, diff --git a/tests/unit/routers/contacts/test_api_get.py b/tests/unit/routers/contacts/test_api_get.py index 9bdefebd..8289d67b 100644 --- a/tests/unit/routers/contacts/test_api_get.py +++ b/tests/unit/routers/contacts/test_api_get.py @@ -67,6 +67,8 @@ def test_get_ctms_for_minimal_contact(client, dbsession, email_factory): "source": newsletter.source, "subscribed": newsletter.subscribed, "unsub_reason": newsletter.unsub_reason, + "create_timestamp": newsletter.create_timestamp.isoformat(), + "update_timestamp": newsletter.update_timestamp.isoformat(), } ], "status": "ok", @@ -133,6 +135,8 @@ def test_get_ctms_for_maximal_contact(client, maximal_contact): "source": "https://www.mozilla.org/en-US/contribute/studentambassadors/", "subscribed": False, "unsub_reason": "Graduated, don't have time for FSA", + "create_timestamp": "2010-01-01T08:04:00+00:00", + "update_timestamp": "2020-01-28T14:50:00+00:00", }, { "format": "T", @@ -141,6 +145,8 @@ def test_get_ctms_for_maximal_contact(client, maximal_contact): "source": "https://commonvoice.mozilla.org/fr", "subscribed": True, "unsub_reason": None, + "create_timestamp": "2010-01-01T08:04:00+00:00", + "update_timestamp": "2020-01-28T14:50:00+00:00", }, { "format": "H", @@ -149,6 +155,8 @@ def test_get_ctms_for_maximal_contact(client, maximal_contact): "source": "https://www.mozilla.org/fr/firefox/accounts/", "subscribed": False, "unsub_reason": "done with this mailing list", + "create_timestamp": "2010-01-01T08:04:00+00:00", + "update_timestamp": "2020-01-28T14:50:00+00:00", }, { "format": "H", @@ -157,6 +165,8 @@ def test_get_ctms_for_maximal_contact(client, maximal_contact): "source": None, "subscribed": True, "unsub_reason": None, + "create_timestamp": "2010-01-01T08:04:00+00:00", + "update_timestamp": "2020-01-28T14:50:00+00:00", }, { "format": "H", @@ -165,6 +175,8 @@ def test_get_ctms_for_maximal_contact(client, maximal_contact): "source": None, "subscribed": True, "unsub_reason": None, + "create_timestamp": "2010-01-01T08:04:00+00:00", + "update_timestamp": "2020-01-28T14:50:00+00:00", }, { "format": "H", @@ -173,6 +185,8 @@ def test_get_ctms_for_maximal_contact(client, maximal_contact): "source": None, "subscribed": True, "unsub_reason": None, + "create_timestamp": "2010-01-01T08:04:00+00:00", + "update_timestamp": "2020-01-28T14:50:00+00:00", }, { "format": "H", @@ -181,6 +195,8 @@ def test_get_ctms_for_maximal_contact(client, maximal_contact): "source": None, "subscribed": True, "unsub_reason": None, + "create_timestamp": "2010-01-01T08:04:00+00:00", + "update_timestamp": "2020-01-28T14:50:00+00:00", }, ], "status": "ok", @@ -193,6 +209,8 @@ def test_get_ctms_for_maximal_contact(client, maximal_contact): "source": "https://a-software.mozilla.org/", "subscribed": True, "unsub_reason": None, + "create_timestamp": "2010-01-01T08:04:00+00:00", + "update_timestamp": "2020-01-28T14:50:00+00:00", }, { "fields": {"geo": "cn"}, @@ -200,6 +218,8 @@ def test_get_ctms_for_maximal_contact(client, maximal_contact): "source": None, "subscribed": True, "unsub_reason": None, + "create_timestamp": "2010-01-01T08:04:00+00:00", + "update_timestamp": "2020-01-28T14:50:00+00:00", }, { "fields": {"geo": "fr", "platform": "win64"}, @@ -207,6 +227,8 @@ def test_get_ctms_for_maximal_contact(client, maximal_contact): "source": "https://super-product.mozilla.org/", "subscribed": True, "unsub_reason": None, + "create_timestamp": "2010-01-01T08:04:00+00:00", + "update_timestamp": "2020-01-28T14:50:00+00:00", }, { "fields": {"geo": "ca", "platform": "windows,android"}, @@ -214,6 +236,8 @@ def test_get_ctms_for_maximal_contact(client, maximal_contact): "source": None, "subscribed": True, "unsub_reason": None, + "create_timestamp": "2010-01-01T08:04:00+00:00", + "update_timestamp": "2020-01-28T14:50:00+00:00", }, ], } @@ -277,6 +301,8 @@ def test_get_ctms_for_api_example(client, example_contact): "source": None, "subscribed": True, "unsub_reason": None, + "create_timestamp": "2020-12-05T19:21:50.908000+00:00", + "update_timestamp": "2021-02-04T15:36:57.511000+00:00", }, { "format": "H", @@ -285,6 +311,8 @@ def test_get_ctms_for_api_example(client, example_contact): "source": None, "subscribed": True, "unsub_reason": None, + "create_timestamp": "2020-12-05T19:21:50.908000+00:00", + "update_timestamp": "2021-02-04T15:36:57.511000+00:00", }, ], "status": "ok", @@ -297,6 +325,8 @@ def test_get_ctms_for_api_example(client, example_contact): "source": None, "subscribed": True, "unsub_reason": None, + "create_timestamp": "2020-12-05T19:21:50.908000+00:00", + "update_timestamp": "2021-02-04T15:36:57.511000+00:00", }, { "fields": {"geo": "fr"}, @@ -304,6 +334,8 @@ def test_get_ctms_for_api_example(client, example_contact): "source": None, "subscribed": True, "unsub_reason": None, + "create_timestamp": "2020-12-05T19:21:50.908000+00:00", + "update_timestamp": "2021-02-04T15:36:57.511000+00:00", }, { "fields": {"geo": "fr", "platform": "ios,mac"}, @@ -311,6 +343,8 @@ def test_get_ctms_for_api_example(client, example_contact): "source": None, "subscribed": True, "unsub_reason": None, + "create_timestamp": "2020-12-05T19:21:50.908000+00:00", + "update_timestamp": "2021-02-04T15:36:57.511000+00:00", }, ], } diff --git a/tests/unit/routers/contacts/test_api_patch.py b/tests/unit/routers/contacts/test_api_patch.py index d7eac6ad..e96ab7b0 100644 --- a/tests/unit/routers/contacts/test_api_patch.py +++ b/tests/unit/routers/contacts/test_api_patch.py @@ -194,12 +194,8 @@ def test_patch_cannot_set_timestamps(client, maximal_contact): # `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"] == [] @@ -301,6 +297,8 @@ def test_patch_to_subscribe(client, maximal_contact): assert resp.status_code == 200 actual = resp.json() assert len(actual["newsletters"]) == len(maximal_contact.newsletters) + 1 + del actual["newsletters"][-1]["create_timestamp"] + del actual["newsletters"][-1]["update_timestamp"] assert actual["newsletters"][-1] == { "format": "H", "lang": "en", @@ -330,6 +328,8 @@ def test_patch_to_update_subscription(client, dbsession, newsletter_factory): "source": existing_newsletter.source, "subscribed": existing_newsletter.subscribed, "unsub_reason": existing_newsletter.unsub_reason, + "create_timestamp": existing_newsletter.create_timestamp.isoformat(), + "update_timestamp": existing_newsletter.update_timestamp.isoformat(), } @@ -353,6 +353,8 @@ def test_patch_to_unsubscribe(client, maximal_contact): assert resp.status_code == 200 actual = resp.json() assert len(actual["newsletters"]) == len(maximal_contact.newsletters) + del actual["newsletters"][1]["create_timestamp"] + del actual["newsletters"][1]["update_timestamp"] assert actual["newsletters"][1] == { "format": "T", "lang": "fr", @@ -466,6 +468,8 @@ def test_patch_to_add_a_waitlist(client, maximal_contact): new_waitlists = actual["waitlists"] assert len(new_waitlists) == len(maximal_contact.waitlists) + 1 new_waitlist = next((wl for wl in new_waitlists if wl["name"] == "future-tech")) + del new_waitlist["create_timestamp"] + del new_waitlist["update_timestamp"] assert new_waitlist == { "name": "future-tech", "source": None, @@ -551,6 +555,8 @@ def test_patch_vpn_waitlist_legacy_add(client, minimal_contact): resp = client.patch(f"/ctms/{email_id}", json=patch_data, allow_redirects=True) assert resp.status_code == 200 actual = resp.json() + del actual["waitlists"][0]["create_timestamp"] + del actual["waitlists"][0]["update_timestamp"] assert actual["waitlists"] == [ { "name": "vpn", @@ -600,6 +606,8 @@ def test_patch_vpn_waitlist_legacy_update(client, dbsession, waitlist_factory): ) assert resp.status_code == 200 actual = resp.json() + del actual["waitlists"][0]["create_timestamp"] + del actual["waitlists"][0]["update_timestamp"] assert actual["waitlists"] == [ { "name": "vpn", @@ -624,6 +632,8 @@ def test_patch_vpn_waitlist_legacy_update_full(client, dbsession, waitlist_facto ) assert resp.status_code == 200 actual = resp.json() + del actual["waitlists"][0]["create_timestamp"] + del actual["waitlists"][0]["update_timestamp"] assert actual["waitlists"] == [ { "name": "vpn", @@ -641,6 +651,8 @@ def test_patch_relay_waitlist_legacy_add(client, minimal_contact): resp = client.patch(f"/ctms/{email_id}", json=patch_data, allow_redirects=True) assert resp.status_code == 200 actual = resp.json() + del actual["waitlists"][0]["create_timestamp"] + del actual["waitlists"][0]["update_timestamp"] assert actual["waitlists"] == [ { "name": "relay", @@ -687,6 +699,8 @@ def test_patch_relay_waitlist_legacy_update(client, dbsession, waitlist_factory) ) assert resp.status_code == 200 actual = resp.json() + del actual["waitlists"][0]["create_timestamp"] + del actual["waitlists"][0]["update_timestamp"] assert actual["waitlists"] == [ { "name": "relay", @@ -736,6 +750,10 @@ def test_patch_relay_waitlist_legacy_update_all( ) assert resp.status_code == 200 actual = resp.json() + del actual["waitlists"][0]["create_timestamp"] + del actual["waitlists"][0]["update_timestamp"] + del actual["waitlists"][1]["create_timestamp"] + del actual["waitlists"][1]["update_timestamp"] assert actual["waitlists"] == [ { "name": "relay", @@ -765,6 +783,8 @@ def test_subscribe_to_relay_newsletter_turned_into_relay_waitlist( resp = client.patch(f"/ctms/{email_id}", json=patch_data, allow_redirects=True) assert resp.status_code == 200 actual = resp.json() + del actual["waitlists"][0]["create_timestamp"] + del actual["waitlists"][0]["update_timestamp"] assert actual["waitlists"] == [ { "name": "relay-vpn-bundle", @@ -818,6 +838,8 @@ def test_unsubscribe_from_relay_newsletter_removes_relay_waitlist( } resp = client.patch(f"/ctms/{email_id}", json=patch_data, allow_redirects=True) current = resp.json() + del current["waitlists"][0]["create_timestamp"] + del current["waitlists"][0]["update_timestamp"] assert current["waitlists"] == [ { "fields": {"geo": "ru"}, diff --git a/tests/unit/routers/contacts/test_bulk.py b/tests/unit/routers/contacts/test_bulk.py index 784776c9..0d60a2bf 100644 --- a/tests/unit/routers/contacts/test_bulk.py +++ b/tests/unit/routers/contacts/test_bulk.py @@ -121,12 +121,8 @@ def test_get_ctms_bulk_by_timerange( # 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