Skip to content

Commit

Permalink
Drop format support
Browse files Browse the repository at this point in the history
When unsent, this defaults to "H" in CTMS.
  • Loading branch information
robhudson committed Nov 12, 2024
1 parent 56e6a1b commit 454f126
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 35 deletions.
10 changes: 4 additions & 6 deletions basket/news/backends/ctms.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ class CTMSError(Exception):
"first_name": "first_name",
"last_name": "last_name",
"mailing_country": "country",
"email_format": "format",
"email_format": None,
"email_lang": "lang",
"has_opted_out_of_email": "optout",
"unsubscribe_reason": "reason",
Expand Down Expand Up @@ -111,7 +111,7 @@ def from_vendor(contact):
data[basket_name] = group[ctms_name]
elif group_name == "newsletters":
# Import newsletter names
# Unimported per-newsletter data: format, language, source, unsub_reason
# Unimported per-newsletter data: language, source, unsub_reason
for newsletter in group:
if newsletter["subscribed"]:
data.setdefault("newsletters", []).append(newsletter["name"])
Expand Down Expand Up @@ -287,8 +287,6 @@ def to_vendor(data, existing_data=None):
if "lang" in existing_data:
default_lang = process_lang(existing_data["lang"])
newsletter_subscription_default["lang"] = default_lang
if "format" in existing_data:
newsletter_subscription_default["format"] = existing_data["format"]

cleaned_data = {}
for name, raw_value in data.items():
Expand Down Expand Up @@ -319,7 +317,7 @@ def to_vendor(data, existing_data=None):
if name in BASKET_TO_CTMS_NAMES:
group_name, key = BASKET_TO_CTMS_NAMES[name]
ctms_data.setdefault(group_name, {})[key] = value
if name in {"lang", "format"}:
if name in {"lang"}:
newsletter_subscription_default[name] = value
elif name == "source_url":
newsletter_subscription_default["source"] = value
Expand Down Expand Up @@ -381,7 +379,7 @@ def to_vendor(data, existing_data=None):
output_waitlists.append(wl_sub)
else:
# Regular newsletter, which may include extra data from the
# email group like `format`, `source`, and `lang`, e.g.
# email group like `source` and `lang`.
if subscribed:
nl_sub = newsletter_subscription_default.copy()
nl_sub.update({"name": slug, "subscribed": True})
Expand Down
2 changes: 0 additions & 2 deletions basket/news/tests/test_confirm.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ def test_normal(self, get_user_data, ctms_mock):
"status": "ok",
"optin": False,
"newsletters": Mock(),
"format": "ZZ",
"email": "[email protected]",
"token": token,
"email_id": "some-email-id",
Expand All @@ -31,7 +30,6 @@ def test_already_confirmed(self, get_user_data, ctms_mock):
"status": "ok",
"optin": True,
"newsletters": Mock(),
"format": "ZZ",
}
get_user_data.return_value = user_data
token = "TOKEN"
Expand Down
35 changes: 12 additions & 23 deletions basket/news/tests/test_ctms.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@
"email": "[email protected]",
"email_id": "332de237-cab7-4461-bcc3-48e68f42bd5c",
"first_name": "Jane",
"format": "H",
"fpn_country": "fr",
"fpn_platform": "ios,mac",
"relay_country": "fr",
Expand Down Expand Up @@ -224,7 +223,6 @@ def test_sample_format(self, mock_nl_slugs, mock_wl_slugs):
"basket_token": "c4a7d759-bb52-457b-896b-90f1d3ef8433",
"create_timestamp": "2020-03-28T15:41:00.000Z",
"double_opt_in": True,
"email_format": "H",
"email_id": "332de237-cab7-4461-bcc3-48e68f42bd5c",
"email_lang": "en",
"first_name": "Jane",
Expand All @@ -249,7 +247,6 @@ def test_sample_format(self, mock_nl_slugs, mock_wl_slugs):
{
"name": "mozilla-welcome",
"subscribed": True,
"format": "H",
"lang": "en",
},
],
Expand Down Expand Up @@ -415,30 +412,26 @@ def test_newsletter_list_with_extra_data(self, mock_nl_slugs, mock_langs):
"newsletters": ["slug1", "slug2", "slug3", "other"],
"source_url": " https://example.com",
"lang": "es",
"format": "T",
}
prepared = to_vendor(data)
assert prepared == {
"email": {"email_format": "T", "email_lang": "es"},
"email": {"email_lang": "es"},
"newsletters": [
{
"name": "slug1",
"subscribed": True,
"format": "T",
"lang": "es",
"source": "https://example.com",
},
{
"name": "slug2",
"subscribed": True,
"format": "T",
"lang": "es",
"source": "https://example.com",
},
{
"name": "slug3",
"subscribed": True,
"format": "T",
"lang": "es",
"source": "https://example.com",
},
Expand All @@ -448,27 +441,27 @@ def test_newsletter_list_with_extra_data(self, mock_nl_slugs, mock_langs):
@patch("basket.news.newsletters.newsletter_languages", return_value=["en", "fr"])
@patch("basket.news.backends.ctms.newsletter_slugs", return_value=["slug1"])
def test_newsletter_list_with_defaults(self, mock_nl_slugs, mock_langs):
"""A newsletter list uses the default language and format"""
"""A newsletter list uses the default language"""
data = {"newsletters": ["slug1"]}
existing_data = {"lang": "fr", "format": "H"}
existing_data = {"lang": "fr"}
prepared = to_vendor(data, existing_data)
assert prepared == {
"newsletters": [
{"name": "slug1", "subscribed": True, "format": "H", "lang": "fr"},
{"name": "slug1", "subscribed": True, "lang": "fr"},
],
}

@patch("basket.news.newsletters.newsletter_languages", return_value=["en", "fr"])
@patch("basket.news.backends.ctms.newsletter_slugs", return_value=["slug1"])
def test_newsletter_list_with_defaults_override(self, mock_nl_slugs, mock_langs):
"""A newsletter list uses the updated data rather than the defaults"""
data = {"lang": "en", "format": "T", "newsletters": ["slug1"]}
existing_data = {"lang": "fr", "format": "H"}
data = {"lang": "en", "newsletters": ["slug1"]}
existing_data = {"lang": "fr"}
prepared = to_vendor(data, existing_data)
assert prepared == {
"email": {"email_format": "T", "email_lang": "en"},
"email": {"email_lang": "en"},
"newsletters": [
{"name": "slug1", "subscribed": True, "format": "T", "lang": "en"},
{"name": "slug1", "subscribed": True, "lang": "en"},
],
}

Expand Down Expand Up @@ -527,24 +520,21 @@ def test_newsletter_map_with_extra_data(self, mock_nl_slugs, mock_langs):
},
"source_url": " https://example.com",
"lang": "es",
"format": "T",
}
prepared = to_vendor(data)
assert prepared == {
"email": {"email_format": "T", "email_lang": "es"},
"email": {"email_lang": "es"},
"newsletters": [
{
"name": "slug1",
"subscribed": True,
"format": "T",
"lang": "es",
"source": "https://example.com",
},
{"name": "slug2", "subscribed": False},
{
"name": "slug3",
"subscribed": True,
"format": "T",
"lang": "es",
"source": "https://example.com",
},
Expand Down Expand Up @@ -1514,8 +1504,8 @@ def test_update_email_id_not_in_existing_data(self):

@patch("basket.news.newsletters.newsletter_languages", return_value=["en", "fr"])
@patch("basket.news.backends.ctms.newsletter_slugs", return_value=["slug1"])
def test_update_use_existing_lang_and_format(self, mock_slugs, mock_langs):
"""CTMS.update uses the existing language and format"""
def test_update_use_existing_lang(self, mock_slugs, mock_langs):
"""CTMS.update uses the existing language"""
updated = {"updated": "fake_response"}
interface = mock_interface("PATCH", 200, updated)
ctms = CTMS(interface)
Expand All @@ -1524,15 +1514,14 @@ def test_update_use_existing_lang_and_format(self, mock_slugs, mock_langs):
"token": "an-existing-user",
"email_id": "an-existing-id",
"lang": "fr",
"format": "T",
}
update_data = {"newsletters": ["slug1"]}
assert ctms.update(user_data, update_data) == updated
interface.session.patch.assert_called_once_with(
"/ctms/an-existing-id",
json={
"newsletters": [
{"name": "slug1", "subscribed": True, "lang": "fr", "format": "T"},
{"name": "slug1", "subscribed": True, "lang": "fr"},
],
},
)
Expand Down
2 changes: 2 additions & 0 deletions basket/news/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,8 @@ def test_success_with_email(self):
"newsletters": "news,lets",
"optin": "N",
"sync": "N",
# Throwing `format` in here to ensure backwards compatibility.
"format": "H",
"email": "[email protected]",
"first_name": "The",
"last_name": "Dude",
Expand Down
2 changes: 0 additions & 2 deletions basket/news/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,6 @@ def language_code_is_valid(code):
"created_date",
"email",
"first_name",
"format",
"fxa_primary_email",
"lang",
"last_modified_date",
Expand Down Expand Up @@ -268,7 +267,6 @@ def get_user_data(
'country': country code,
'lang': language code,
'token': basket token, a UUID,
'format': 'T' | 'H',
'mofo_relevant': subscribed to a mofo newsletter,
'has_fxa': has an fxa account,
'optin': double opted in,
Expand Down
2 changes: 0 additions & 2 deletions docs/newsletter_api.rst
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ The following URLs are available (assuming "/news" is app url):
returns: {
status: ok,
email: <email>,
format: <format>,
country: <country>,
lang: <lang>,
newsletters: [<newsletter>, ...]
Expand Down Expand Up @@ -214,7 +213,6 @@ The following URLs are available (assuming "/news" is app url):
'status': 'error', # errors talking to CTMS, see next field
'desc': 'error message' # details if status is error
'email': 'email@address',
'format': 'T'|'H',
'country': country code,
'lang': language code,
'token': UUID,
Expand Down

0 comments on commit 454f126

Please sign in to comment.