Fix #730: Add default schema for waitlists (#731)

mozilla-it · Jul 10, 2023 · dd11a1f · dd11a1f
1 parent 865283c
commit dd11a1f
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 23 deletions.
diff --git a/ctms/schemas/waitlist.py b/ctms/schemas/waitlist.py
@@ -46,9 +46,7 @@ def __lt__(self, other):
 
     @root_validator
     def check_fields(cls, values):  # pylint:disable = no-self-argument
-        if "subscribed" in values and values["subscribed"] and "name" in values:
-            validate_waitlist_fields(values["name"], values.get("fields", {}))
-        # If subscribed is False, we don't need to validate fields.
+        validate_waitlist_fields(values.get("name"), values.get("fields", {}))
         return values
 
     class Config:
@@ -80,20 +78,33 @@ class Config:
         extra = "forbid"
 
 
-def validate_waitlist_fields(name: str, fields: dict):
+def CountryField():  # pylint:disable = invalid-name
+    return Field(
+        default=None,
+        max_length=100,
+        description="Waitlist country",
+        example="fr",
+    )
+
+
+def PlatformField():  # pylint:disable = invalid-name
+    return Field(
+        default=None,
+        max_length=100,
+        description="VPN waitlist platforms as comma-separated list",
+        example="ios,mac",
+    )
+
+
+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.
     """
     if name == "relay":
 
         class RelayFieldsSchema(ComparableBase):
-            geo: Optional[str] = Field(
-                default=None,
-                max_length=100,
-                description="Waitlist country",
-                example="fr",
-            )
+            geo: Optional[str] = CountryField()
 
             class Config:
                 extra = "forbid"
@@ -103,24 +114,26 @@ class Config:
     elif name == "vpn":
 
         class VPNFieldsSchema(ComparableBase):
-            geo: Optional[str] = Field(
-                default=None,
-                max_length=100,
-                description="Waitlist country",
-                example="fr",
-            )
-            platform: Optional[str] = Field(
-                default=None,
-                max_length=100,
-                description="VPN waitlist platforms as comma-separated list",
-                example="ios,mac",
-            )
+            geo: Optional[str] = CountryField()
+            platform: Optional[str] = PlatformField()
 
             class Config:
                 extra = "forbid"
 
         VPNFieldsSchema(**fields)
 
+    else:
+        # Default schema for any waitlist.
+        # Only the known fields are validated. Any extra field would
+        # be accepted as is.
+        # This should allow us to onboard most waitlists without specific
+        # code change and service redeployment.
+        class DefaultFieldsSchema(ComparableBase):
+            geo: Optional[str] = CountryField()
+            platform: Optional[str] = PlatformField()
+
+        DefaultFieldsSchema(**fields)
+
 
 def validate_waitlist_newsletters(values):
     """

diff --git a/tests/unit/schemas/test_waitlist.py b/tests/unit/schemas/test_waitlist.py
@@ -13,7 +13,7 @@
         {
             "name": "a",
             "source": "http://website.com",
-            "fields": {"foo": "bar", "geo": "b"},
+            "fields": {"foo": "bar", "geo": "br"},
         },
     ],
 )
@@ -31,6 +31,16 @@ def test_waitlist_with_valid_input_data(data):
         {"name": "a", "fields": None},
         {"name": "a", "fields": "foo"},
         {"name": "a", "fields": [{}]},
+        {
+            "name": "a",
+            "source": "http://website.com",
+            "fields": {"geo": "s" * 101},
+        },
+        {
+            "name": "a",
+            "source": "http://website.com",
+            "fields": {"platform": "s" * 101},
+        },
     ],
 )
 def test_waitlist_with_invalid_input_data(data):