Fix #761: fix waitlist boolean data type (#762)

* Fix #761: fix waitlist boolean data type

* Rewrite transform field test

* lint

* Use Yes/No instead of 0/1

.secrets.baseline

@@ -156,7 +156,7 @@
         "filename": "tests/unit/test_acoustic_service.py",
         "hashed_secret": "3c3b274d119ff5a5ec6c1e215c1cb794d9973ac1",
         "is_verified": false,
-        "line_number": 22
+        "line_number": 23
       }
     ],
     "tests/unit/test_auth.py": [
@@ -185,5 +185,5 @@
       }
     ]
   },
-  "generated_at": "2023-07-13T13:27:23Z"
+  "generated_at": "2023-07-17T09:04:13Z"
 }

ctms/acoustic_service.py

@@ -65,6 +65,24 @@ def force_bytes(s, encoding="utf-8", strings_only=False, errors="strict"):
 # End cherry-picked from django.utils.encoding
 
 
+def transform_field_for_acoustic(data):
+    """Transform data type for Acoustic."""
+    if isinstance(data, bool):
+        if data:
+            return "Yes"
+        return "No"
+    if isinstance(data, datetime.datetime):
+        # Acoustic doesn't have timestamps, so make timestamps into dates.
+        data = data.date()
+    if isinstance(data, datetime.date):
+        return data.strftime("%m/%d/%Y")
+    if isinstance(data, UUID):
+        return str(data)
+    if data is None:
+        return ""
+    return data
+
+
 def acoustic_heartbeat(
     db: Session, settings: Settings = get_settings()
 ) -> tuple[bool, dict[str, Any]]:
@@ -245,7 +263,7 @@ def _main_table_converter(self, contact, main_fields):
 
                         acoustic_main_table[
                             acoustic_field_name
-                        ] = self.transform_field_for_acoustic(inner_value)
+                        ] = transform_field_for_acoustic(inner_value)
                     elif (contact_attr, inner_attr) in AcousticResources.SKIP_FIELDS:
                         pass
                     else:
@@ -326,7 +344,6 @@ def _waitlist_converter(
         """
         waitlist_rows = []
         contact_waitlists: List[WaitlistBase] = contact.waitlists
-        contact_email_id = str(contact.email.email_id)
 
         waitlists_by_name = {wl.name: wl for wl in contact_waitlists}
         for acoustic_field_name in main_fields:
@@ -337,56 +354,33 @@ def _waitlist_converter(
             if name in waitlists_by_name:
                 waitlist = waitlists_by_name[name]
                 value = getattr(waitlist, field, waitlist.fields.get(field, None))
-            acoustic_main_table[
-                acoustic_field_name
-            ] = self.transform_field_for_acoustic(value)
+            acoustic_main_table[acoustic_field_name] = transform_field_for_acoustic(
+                value
+            )
 
         # Waitlist relational table
         for waitlist in contact_waitlists:
-            waitlist_row = {
-                "email_id": contact_email_id,
-                "waitlist_name": waitlist.name,
-                "waitlist_source": str(waitlist.source or ""),
-                "subscribed": waitlist.subscribed,
-                "unsub_reason": waitlist.unsub_reason or "",
-                # Timestamps
-                "create_timestamp": waitlist.create_timestamp.date().isoformat(),
-                "update_timestamp": waitlist.update_timestamp.date().isoformat(),
-            }
+            waitlist_row = {}
+            for column, field in (
+                ("email_id", "email_id"),
+                ("waitlist_name", "name"),
+                ("waitlist_source", "source"),
+                ("subscribed", "subscribed"),
+                ("unsub_reason", "unsub_reason"),
+                ("create_timestamp", "create_timestamp"),
+                ("update_timestamp", "update_timestamp"),
+            ):
+                value = getattr(waitlist, field)
+                waitlist_row[column] = transform_field_for_acoustic(value)
             # Extra optional fields (eg. "geo", "platform", ...)
             for field in waitlist_fields:
-                waitlist_row[f"waitlist_{field}"] = str(
-                    waitlist.fields.get(field) or ""
+                waitlist_row[f"waitlist_{field}"] = transform_field_for_acoustic(
+                    waitlist.fields.get(field)
                 )
-
-            # TODO: manage sync of unsubscribed waitlists (currently not possible)
             waitlist_rows.append(waitlist_row)
 
         return waitlist_rows, acoustic_main_table
 
-    @staticmethod
-    def transform_field_for_acoustic(data):
-        """Transform data for main contact table."""
-        if isinstance(data, bool):
-            if data:
-                return "1"
-            return "0"
-        if isinstance(data, datetime.datetime):
-            # Acoustic doesn't have timestamps, so make timestamps into dates.
-            data = data.date()
-        if isinstance(data, datetime.date):
-            return data.strftime("%m/%d/%Y")
-        if isinstance(data, UUID):
-            return str(data)
-        return data
-
-    @staticmethod
-    def to_acoustic_bool(bool_str):
-        """Transform bool for products relational table."""
-        if bool_str in (True, "true"):
-            return "Yes"
-        return "No"
-
     @staticmethod
     def to_acoustic_timestamp(dt_val):
         """Transform datetime for products relational table."""
@@ -429,8 +423,8 @@ def _product_converter(self, contact):
                     "current_period_start": to_ts(product.current_period_start),
                     "current_period_end": to_ts(product.current_period_end),
                     "canceled_at": to_ts(product.canceled_at),
-                    "cancel_at_period_end": CTMSToAcousticService.to_acoustic_bool(
-                        product.cancel_at_period_end
+                    "cancel_at_period_end": transform_field_for_acoustic(
+                        product.cancel_at_period_end in (True, "true")
                     ),
                     "ended_at": to_ts(product.ended_at),
                 }

tests/unit/test_acoustic_service.py

@@ -1,12 +1,13 @@
 import datetime
 from unittest import mock
 from unittest.mock import MagicMock
+from uuid import UUID
 
 import pytest
 from structlog.testing import capture_logs
 
 from ctms import acoustic_service
-from ctms.acoustic_service import CTMSToAcousticService
+from ctms.acoustic_service import CTMSToAcousticService, transform_field_for_acoustic
 from ctms.crud import get_contact_by_email_id, get_newsletters_by_email_id
 from ctms.schemas.contact import ContactSchema
 
@@ -158,9 +159,9 @@ def test_ctms_to_acoustic_waitlists_minimal(
         acoustic_newsletters_mapping,
     )
     assert len(minimal_contact.waitlists) == 0
-    assert main["vpn_waitlist_geo"] is None
-    assert main["vpn_waitlist_platform"] is None
-    assert main["relay_waitlist_geo"] is None
+    assert main["vpn_waitlist_geo"] == ""
+    assert main["vpn_waitlist_platform"] == ""
+    assert main["relay_waitlist_geo"] == ""
 
 
 def test_ctms_to_acoustic_waitlists_maximal(
@@ -537,49 +538,26 @@ def test_ctms_to_acoustic_traced_email(
     assert caplog[0] == expected_log
 
 
-def test_transform_field(base_ctms_acoustic_service):
-    is_true = base_ctms_acoustic_service.transform_field_for_acoustic(True)
-    assert is_true == "1"
-    is_false = base_ctms_acoustic_service.transform_field_for_acoustic(False)
-    assert is_false == "0"
-    transformed_from_datetime = base_ctms_acoustic_service.transform_field_for_acoustic(
-        datetime.datetime.now()
-    )
-    assert (
-        transformed_from_datetime is not None
-    ), "Error when using method to transform datetime object"
-    transformed_from_date = base_ctms_acoustic_service.transform_field_for_acoustic(
-        datetime.date.today()
-    )
-    assert (
-        transformed_from_date is not None
-    ), "Error when using method to transform date object"
-
-    assert transformed_from_datetime == transformed_from_date, (
-        "The result of the transformation process of a "
-        "date and datetime should be identical, "
-        "when starting values are equivalent in date "
-    )
-
-    is_datetime_parsed = datetime.datetime.strptime(
-        transformed_from_datetime, "%m/%d/%Y"
-    )
-    assert isinstance(
-        is_datetime_parsed, datetime.date
-    ), "The result should be in MM/DD/YYYY format, to be able to be processed to a date"
-    is_date_parsed = datetime.datetime.strptime(transformed_from_date, "%m/%d/%Y")
-    assert isinstance(
-        is_date_parsed, datetime.date
-    ), "The result should be in MM/DD/YYYY format, to be able to be processed to a date"
-
-
 @pytest.mark.parametrize(
-    "value,expected",
-    (("true", "Yes"), (True, "Yes"), (False, "No"), ("false", "No"), ("", "No")),
+    "value, expected",
+    (
+        ("string", "string"),
+        (True, "Yes"),
+        (False, "No"),
+        (None, ""),
+        (
+            UUID("62d8d3c6-95f3-4ed6-b176-7f69acff22f6"),
+            "62d8d3c6-95f3-4ed6-b176-7f69acff22f6",
+        ),
+        (
+            datetime.datetime(2021, 11, 8, 9, 6, tzinfo=datetime.timezone.utc),
+            "11/08/2021",
+        ),
+        (datetime.date(2021, 11, 8), "11/08/2021"),
+    ),
 )
-def test_to_acoustic_bool(value, expected):
-    """Python and JS booleans are converted to Acoustic Yes/No bools."""
-    assert CTMSToAcousticService.to_acoustic_bool(value) == expected
+def test_transform_field_for_acoustic(value, expected):
+    assert transform_field_for_acoustic(value) == expected
 
 
 def test_to_acoustic_timestamp():