Ref #492, Fix #734: drop write support of legacy vpn_waitlist and relay_waitlist fields

ctms/crud.py

@@ -11,7 +11,6 @@
 from sqlalchemy.orm import Session, joinedload, load_only, selectinload
 
 from .auth import hash_password
-from .backport_legacy_waitlists import format_legacy_vpn_relay_waitlist_input
 from .database import Base
 from .models import (
     AcousticField,
@@ -539,10 +538,6 @@ def create_contact(
     if contact.mofo:
         create_mofo(db, email_id, contact.mofo)
 
-    contact = format_legacy_vpn_relay_waitlist_input(
-        db, email_id, contact, ContactInSchema, metrics
-    )
-
     for newsletter in contact.newsletters:
         create_newsletter(db, email_id, newsletter)
 
@@ -558,10 +553,6 @@ def create_or_update_contact(
     create_or_update_fxa(db, email_id, contact.fxa)
     create_or_update_mofo(db, email_id, contact.mofo)
 
-    contact = format_legacy_vpn_relay_waitlist_input(
-        db, email_id, contact, ContactPutSchema, metrics
-    )
-
     create_or_update_newsletters(db, email_id, contact.newsletters)
     create_or_update_waitlists(db, email_id, contact.waitlists)
 
@@ -621,10 +612,6 @@ def update_contact(
                         db.delete(existing)
                         setattr(email, group_name, None)
 
-    update_data = format_legacy_vpn_relay_waitlist_input(
-        db, email_id, update_data, dict, metrics
-    )
-
     if "newsletters" in update_data:
         if update_data["newsletters"] == "UNSUBSCRIBE":
             for newsletter in getattr(email, "newsletters", []):

ctms/metrics.py

@@ -56,13 +56,6 @@
             "documentation": "Total count of contacts added to Acoustic sync backlog",
         },
     ),
-    "legacy_waitlists_requests": (
-        Counter,
-        {
-            "name": "ctms_legacy_waitlists_requests_total",
-            "documentation": "Total count of API calls that use the legacy waitlists format",
-        },
-    ),
 }
 
 # We could use the default prometheus_client.REGISTRY, but it makes tests

ctms/schemas/__init__.py

@@ -62,13 +62,7 @@
     StripeSubscriptionItemOutputSchema,
     StripeSubscriptionItemUpsertSchema,
 )
-from .waitlist import (
-    RelayWaitlistInSchema,
-    VpnWaitlistInSchema,
-    WaitlistInSchema,
-    WaitlistSchema,
-    WaitlistTableSchema,
-)
+from .waitlist import WaitlistInSchema, WaitlistSchema, WaitlistTableSchema
 from .web import (
     BadRequestResponse,
     NotFoundResponse,

ctms/schemas/contact.py

@@ -19,14 +19,11 @@
 from .newsletter import NewsletterInSchema, NewsletterSchema, NewsletterTableSchema
 from .product import ProductBaseSchema, ProductSegmentEnum
 from .waitlist import (
-    RelayWaitlistInSchema,
     RelayWaitlistSchema,
-    VpnWaitlistInSchema,
     VpnWaitlistSchema,
     WaitlistInSchema,
     WaitlistSchema,
     WaitlistTableSchema,
-    validate_waitlist_newsletters,
 )
 
 if TYPE_CHECKING:
@@ -242,23 +239,10 @@ class ContactInBase(ComparableBase):
     mofo: Optional[MozillaFoundationInSchema] = None
     newsletters: List[NewsletterInSchema] = []
     waitlists: List[WaitlistInSchema] = []
-    # TODO waitlist: remove once Basket leverages the `waitlists` field.
-    vpn_waitlist: Optional[VpnWaitlistInSchema] = None
-    relay_waitlist: Optional[RelayWaitlistInSchema] = None
 
     class Config:
         fields = ContactSchema.Config.fields
 
-    @root_validator
-    def check_fields(cls, values):  # pylint:disable = no-self-argument
-        """
-        This makes sure a Relay country is specified when one of the `relay-*-waitlist`
-        newsletter is subscribed.
-
-        TODO waitlist: remove once Basket leverages the `waitlists` field.
-        """
-        return validate_waitlist_newsletters(values)
-
     def idempotent_equal(self, other):
         def _noneify(field):
             if not field:
@@ -301,19 +285,6 @@ class ContactPatchSchema(ComparableBase):
     mofo: Optional[Union[Literal["DELETE"], MozillaFoundationInSchema]]
     newsletters: Optional[Union[List[NewsletterSchema], Literal["UNSUBSCRIBE"]]]
     waitlists: Optional[Union[List[WaitlistInSchema], Literal["UNSUBSCRIBE"]]]
-    # TODO waitlist: remove once Basket leverages the `waitlists` field.
-    vpn_waitlist: Optional[Union[Literal["DELETE"], VpnWaitlistInSchema]]
-    relay_waitlist: Optional[Union[Literal["DELETE"], RelayWaitlistInSchema]]
-
-    @root_validator
-    def check_fields(cls, values):  # pylint:disable = no-self-argument
-        """
-        This makes sure a Relay country is specified when one of the `relay-*-waitlist`
-        newsletter is subscribed.
-
-        TODO waitlist: remove once Basket leverages the `waitlists` field.
-        """
-        return validate_waitlist_newsletters(values)
 
     class Config:
         fields = {

ctms/schemas/waitlist.py

@@ -13,12 +13,6 @@ class WaitlistBase(ComparableBase):
 
     This is meant to serve as the common and generic schemas to
     all waitlists.
-
-    In this implementation phase, it cohabits with individual (non-generic)
-    schemas of Relay and VPN.
-
-    TODO waitlist: once Basket leverages the `waitlists` field, we can drop
-    `RelayWaitlistBase` and `VpnWaitlistBase`.
     """
 
     name: str = Field(
@@ -98,8 +92,10 @@ def PlatformField():  # pylint:disable = invalid-name
 
 def validate_waitlist_fields(name: Optional[str], fields: dict):
     """
-    Once waitlists will have been migrated to a full N-N relationship,
-    this will be the only remaining VPN specific piece of code.
+    This is the only place where specific code has
+    to be added for custom validation of extra waitlist
+    fields.
+    In the future, a JSON schema for each waitlist could be put in the database instead.
     """
     if name == "relay":
 
@@ -135,53 +131,9 @@ class DefaultFieldsSchema(ComparableBase):
         DefaultFieldsSchema(**fields)
 
 
-def validate_waitlist_newsletters(values):
-    """
-    This helper validates that when subscribing to `relay-*-waitlist`
-    newsletters, the country is provided.
-    # TODO waitlist: remove once Basket leverages the `waitlists` field.
-    """
-    if "newsletters" not in values:
-        return values
-
-    newsletters = values["newsletters"]
-    if not isinstance(newsletters, list):
-        return values
-
-    relay_newsletter_found = False
-    for newsletter in newsletters:
-        if newsletter.subscribed and newsletter.name.startswith("relay-"):
-            relay_newsletter_found = True
-            break
-
-    if not relay_newsletter_found:
-        return values
-
-    # If specified using the legacy `relay_waitlist`
-    relay_country = None
-    relay_waitlist = values.get("relay_waitlist")
-    if relay_waitlist:
-        relay_country = relay_waitlist.geo
-    elif "waitlists" in values:
-        # If specified using the `waitlists` field (unlikely, but in our tests we do)
-        waitlists = values["waitlists"]
-        if isinstance(waitlists, list):
-            for waitlist in waitlists:
-                if waitlist.name == "relay":
-                    relay_country = waitlist.fields.get("geo")
-
-    # Relay country not specified, check if a relay newsletter is being subscribed.
-    if not relay_country:
-        raise ValueError("Relay country missing")
-
-    return values
-
-
-class RelayWaitlistBase(ComparableBase):
+class RelayWaitlistSchema(ComparableBase):
     """
-    The Mozilla Relay Waitlist schema.
-
-    TODO waitlist: remove once Basket leverages the `waitlists` field.
+    The Mozilla Relay Waitlist schema for the read-only `relay_waitlist` field.
     """
 
     geo: Optional[str] = Field(
@@ -195,19 +147,9 @@ class Config:
         orm_mode = True
 
 
-# No need to change anything, just extend if you want to
-RelayWaitlistInSchema = RelayWaitlistBase
-RelayWaitlistSchema = RelayWaitlistBase
-
-
-class VpnWaitlistBase(ComparableBase):
+class VpnWaitlistSchema(ComparableBase):
     """
-    The Mozilla VPN Waitlist schema.
-
-    This was previously the Firefox Private Network (fpn) waitlist data,
-    with a similar purpose.
-
-    TODO waitlist: remove once Basket leverages the `waitlists` field.
+    The Mozilla VPN Waitlist schema for the read-only `vpn_waitlist` field
     """
 
     geo: Optional[str] = Field(
@@ -228,8 +170,3 @@ class VpnWaitlistBase(ComparableBase):
 
     class Config:
         orm_mode = True
-
-
-# No need to change anything, just extend if you want to
-VpnWaitlistInSchema = VpnWaitlistBase
-VpnWaitlistSchema = VpnWaitlistBase