" + email + "
Status:" + str(r.status_code) + " Body:" + r.text + "", - ) - @querylog.timed def check_password(password, hash): diff --git a/website/auth_pages.py b/website/auth_pages.py index d708fdad6d7..484c1ee3b98 100644 --- a/website/auth_pages.py +++ b/website/auth_pages.py @@ -7,8 +7,8 @@ from safe_format import safe_format from hedy_content import ALL_LANGUAGES, COUNTRIES from utils import extract_bcrypt_rounds, is_heroku, is_testing_request, timems, times, remove_class_preview +from website.newsletter import create_subscription from website.auth import ( - MAILCHIMP_API_URL, RESET_LENGTH, SESSION_LENGTH, TOKEN_COOKIE_NAME, @@ -20,13 +20,11 @@ forget_current_user, is_admin, is_teacher, - mailchimp_subscribe_user, make_salt, password_hash, prepare_user_db, remember_current_user, requires_login, - send_email, send_email_template, send_localized_email_template, validate_signup_data, @@ -180,17 +178,7 @@ def signup(self): user, resp = self.store_new_account(body, body["email"].strip().lower()) if not is_testing_request(request) and "subscribe" in body: - # If we have a Mailchimp API key, we use it to add the subscriber through the API - if MAILCHIMP_API_URL: - mailchimp_subscribe_user(user["email"], body["country"]) - # Otherwise, we send an email to notify about the subscription to the main email address - else: - send_email( - config["email"]["sender"], - "Subscription to Hedy newsletter on signup", - user["email"], - "
" + user["email"] + "
", - ) + create_subscription(user["email"], body.get("country")) # We automatically login the user cookie = make_salt() diff --git a/website/classes.py b/website/classes.py index 14c86ec7db0..0418278ccb7 100644 --- a/website/classes.py +++ b/website/classes.py @@ -9,6 +9,7 @@ from website.flask_helpers import render_template from website.auth import current_user, is_teacher, requires_login, requires_teacher, \ refresh_current_user_from_db, is_second_teacher +from website.newsletter import add_class_created_to_subscription from .database import Database from .website_module import WebsiteModule, route @@ -50,6 +51,7 @@ def create_class(self, user): } self.db.store_class(Class) + add_class_created_to_subscription(user['email']) response = {"id": Class["id"]} return make_response(response, 200) diff --git a/website/for_teachers.py b/website/for_teachers.py index 8e6ba5d9b9c..702a0e1f9a7 100644 --- a/website/for_teachers.py +++ b/website/for_teachers.py @@ -19,6 +19,7 @@ from website.server_types import SortedAdventure from website.flask_helpers import render_template +from website.newsletter import add_class_customized_to_subscription from website.auth import ( is_admin, is_teacher, @@ -899,6 +900,7 @@ def add_adventure(self, user, level): customizations["other_settings"].remove("hide_parsons") self.db.update_class_customizations(customizations) + add_class_customized_to_subscription(user['email']) available_adventures = self.get_unused_adventures(adventures, teacher_adventures, adventure_names) return render_partial('customize-class/partial-sortable-adventures.html', @@ -936,6 +938,7 @@ def remove_adventure_from_class(self, user): is_command_adventure=adventure_id in hedy_content.KEYWORDS_ADVENTURES) adventures[int(level)].remove(sorted_adventure) self.db.update_class_customizations(customizations) + add_class_customized_to_subscription(user['email']) available_adventures = self.get_unused_adventures(adventures, teacher_adventures, adventure_names) return render_partial('customize-class/partial-sortable-adventures.html', @@ -979,6 +982,7 @@ def sort_adventures_in_class(self, user): self.reorder_adventures(adventures[int(level)], from_sorted_adv_class=True) self.reorder_adventures(customizations['sorted_adventures'][level]) self.db.update_class_customizations(customizations) + add_class_customized_to_subscription(user['email']) return render_partial('customize-class/partial-sortable-adventures.html', level=level, @@ -1356,6 +1360,7 @@ def update_customizations(self, user, class_id): } self.db.update_class_customizations(customizations) + add_class_customized_to_subscription(user['email']) response = {"success": gettext("class_customize_success")} return make_response(response, 200) @@ -1784,6 +1789,7 @@ def add_adventure_to_class_level(self, user, class_id, adventure_id, level, remo self.reorder_adventures(customizations['sorted_adventures'][level]) self.db.update_class_customizations(customizations) + add_class_customized_to_subscription(user['email']) @route("/create-adventure/", methods=["POST"]) @route("/create-adventure/{email}
") + + +def update_subscription(current_email, new_email, new_country): + """ Updates the newsletter subscription when the user changes their email or/and their country """ + if not MAILCHIMP_API_URL: + # TODO: Why do we send an email to hello@hedy.org if a user subscribes and there is no mailchimp api key + # but if the user changes their email and we do not have a key, we do nothing? + return + r = get_mailchimp_subscriber(current_email) + if r.status_code == 200: + current_tags = r.json().get('tags', []) + if new_email != current_email: + # If user is subscribed, we remove the old email from the list and add the new one + new_tags = [t.get('name') for t in current_tags if t.get('name') not in COUNTRIES] + [new_country] + create_mailchimp_subscriber(new_email, new_tags) + delete_mailchimp_subscriber(current_email) + else: + # If the user did not change their email, check if the country needs to be updated + tags_to_update = get_country_tag_changes(current_tags, new_country) + if tags_to_update: + update_mailchimp_tags(current_email, tags_to_update) + + +def add_class_created_to_subscription(email): + create_subscription_event(email, MailchimpTag.CREATED_CLASS) + + +def add_class_customized_to_subscription(email): + create_subscription_event(email, MailchimpTag.CUSTOMIZED_CLASS) + + +def create_subscription_event(email, tag): + """ When certain events occur, e.g. a newsletter subscriber creates or customizes a class, these events + should be reflected in their subscription, so that we can send them relevant content """ + if not MAILCHIMP_API_URL: + return + r = get_mailchimp_subscriber(email) + if r.status_code == 200: + current_tags = r.json().get('tags', []) + if not any([t for t in current_tags if t.get('name') == tag]): + new_tags = current_tags + [to_mailchimp_tag(tag)] + update_mailchimp_tags(email, new_tags) + + +def get_country_tag_changes(current_tags, country): + """ Returns the necessary alterations to the tags of the newsletter subscriber when they change their country. + The old country tag, if existing, should be removed, and the new country, if existing, should be added. """ + current_country_tags = [t.get('name') for t in current_tags if t.get('name') in COUNTRIES] + + if country in current_country_tags: + return [] + + changes = [to_mailchimp_tag(t, active=False) for t in current_country_tags] + if country: + changes.append(to_mailchimp_tag(country)) + return changes + + +def to_mailchimp_tag(tag, active=True): + # https://mailchimp.com/developer/marketing/api/list-member-tags/ + status = 'active' if active else 'inactive' + return {'name': tag, 'status': status} + + +class MailchimpTag: + TEACHER = 'teacher' + CREATED_CLASS = "created_class" + CUSTOMIZED_CLASS = "customized_class" + + +def create_mailchimp_subscriber(email, tag_names): + tag_names = [t for t in tag_names if t] # the country can be None, so filter it + request_body = {"email_address": email, "status": "subscribed", "tags": tag_names} + request_path = MAILCHIMP_API_URL + "/members/" + r = requests.post(request_path, headers=MAILCHIMP_API_HEADERS, data=json.dumps(request_body)) + + subscription_error = None + if r.status_code != 200 and r.status_code != 400: + subscription_error = True + # We can get a 400 if the email is already subscribed to the list. We should ignore this error. + if r.status_code == 400 and "already a list member" not in r.text: + subscription_error = True + # If there's an error in subscription through the API, we report it to the main email address + if subscription_error: + send_email( + config["email"]["sender"], + "ERROR - Subscription to Hedy newsletter", + f"email: {email} status: {r.status_code} body: {r.text}", + f"{email}
Status:{r.status_code} Body:{r.text}")
+
+
+def get_mailchimp_subscriber(email):
+ request_path = f'{MAILCHIMP_API_URL}/members/{get_subscriber_hash(email)}'
+ return requests.get(request_path, headers=MAILCHIMP_API_HEADERS)
+
+
+def update_mailchimp_tags(email, tags):
+ request_path = f'{MAILCHIMP_API_URL}/members/{get_subscriber_hash(email)}/tags'
+ return requests.post(request_path, headers=MAILCHIMP_API_HEADERS, data=json.dumps({'tags': tags}))
+
+
+def delete_mailchimp_subscriber(email):
+ request_path = f'{MAILCHIMP_API_URL}/members/{get_subscriber_hash(email)}'
+ requests.delete(request_path, headers=MAILCHIMP_API_HEADERS)
+
+
+def get_subscriber_hash(email):
+ """ We hash the email with md5 to avoid emails with unescaped characters triggering errors """
+ return hashlib.md5(email.encode("utf-8")).hexdigest()
diff --git a/website/profile.py b/website/profile.py
index a68dbd1eadd..5d82a485e47 100644
--- a/website/profile.py
+++ b/website/profile.py
@@ -1,7 +1,5 @@
import datetime
-import hashlib
-import requests
from flask import make_response, request, session
from website.flask_helpers import gettext_with_fallback as gettext
@@ -9,17 +7,15 @@
from hedy_content import ALL_KEYWORD_LANGUAGES, ALL_LANGUAGES, COUNTRIES
from utils import is_testing_request, timems, valid_email
from website.auth import (
- MAILCHIMP_API_HEADERS,
- MAILCHIMP_API_URL,
SESSION_LENGTH,
create_verify_link,
- mailchimp_subscribe_user,
make_salt,
password_hash,
remember_current_user,
requires_login,
send_email_template,
)
+from website.newsletter import update_subscription
from .database import Database
from .website_module import WebsiteModule, route
@@ -71,7 +67,7 @@ def update_profile(self, user):
if "email" in body:
email = body["email"].strip().lower()
old_user_email = user.get("email")
- if email != user.get("email"):
+ if email != old_user_email:
exists = self.db.user_by_email(email)
if exists:
return make_response(gettext("exists_email"), 403)
@@ -95,19 +91,6 @@ def update_profile(self, user):
# Add a notification to the response, still process the changes
print(f"Profile changes processed for {user['username']}, mail sending invalid")
- # We check whether the user is in the Mailchimp list.
- if not is_testing_request(request) and MAILCHIMP_API_URL:
- # We hash the email with md5 to avoid emails with unescaped characters triggering errors
- request_path = (
- MAILCHIMP_API_URL + "/members/" + hashlib.md5(old_user_email.encode("utf-8")).hexdigest()
- )
- r = requests.get(request_path, headers=MAILCHIMP_API_HEADERS)
- # If user is subscribed, we remove the old email from the list and add the new one
- if r.status_code == 200:
- r = requests.delete(request_path, headers=MAILCHIMP_API_HEADERS)
- self.db.get_username_role(user["username"])
- mailchimp_subscribe_user(email, body["country"])
-
username = user["username"]
updates = {}
@@ -117,6 +100,14 @@ def update_profile(self, user):
else:
updates[field] = None
+ if not is_testing_request(request):
+ current_email = user.get('email')
+ current_country = user.get('country')
+ new_email = body.get('email', '').strip().lower()
+ new_country = body.get('country')
+ if current_email != new_email or current_country != new_country:
+ update_subscription(current_email, new_email, new_country)
+
if updates:
self.db.update_user(username, updates)
From 165ad0ae38abfa87afa73fce39e31148a04b8be2 Mon Sep 17 00:00:00 2001
From: Boryana Goncharenko
<3010723+boryanagoncharenko@users.noreply.github.com>
Date: Thu, 2 Jan 2025 15:31:47 +0200
Subject: [PATCH 2/4] Refactoring
---
website/newsletter.py | 43 ++++++++++++++++++++++++-------------------
1 file changed, 24 insertions(+), 19 deletions(-)
diff --git a/website/newsletter.py b/website/newsletter.py
index 635d6e6bda1..58d7eeb5539 100644
--- a/website/newsletter.py
+++ b/website/newsletter.py
@@ -3,6 +3,7 @@
import hashlib
import requests
from config import config
+from functools import wraps
from hedy_content import COUNTRIES
from website.auth import send_email
@@ -23,32 +24,36 @@
}
+def run_if_mailchimp_config_present(f):
+ """Similar to 'requires_login', but also tests that the user is an admin."""
+ @wraps(f)
+ def inner(*args, **kws):
+ if MAILCHIMP_API_URL:
+ return f(*args, **kws)
+
+ return inner
+
+
+@run_if_mailchimp_config_present
def create_subscription(email, country):
""" Subscribes the user to the newsletter. Currently, users can subscribe to the newsletter only on signup and
only if they are creating a teacher account. """
- # If there is a Mailchimp API key, use it to add the subscriber through the API
- if MAILCHIMP_API_URL:
- create_mailchimp_subscriber(email, [country, MailchimpTag.TEACHER])
- # Otherwise, email to notify about the subscription to the main email address
- else:
- recipient = config["email"]["sender"]
- send_email(recipient, "Subscription to Hedy newsletter on signup", email, f"{email}
") + tags = [country, MailchimpTag.TEACHER] + create_mailchimp_subscriber(email, tags) +@run_if_mailchimp_config_present def update_subscription(current_email, new_email, new_country): """ Updates the newsletter subscription when the user changes their email or/and their country """ - if not MAILCHIMP_API_URL: - # TODO: Why do we send an email to hello@hedy.org if a user subscribes and there is no mailchimp api key - # but if the user changes their email and we do not have a key, we do nothing? - return - r = get_mailchimp_subscriber(current_email) - if r.status_code == 200: - current_tags = r.json().get('tags', []) + response = get_mailchimp_subscriber(current_email) + if response.status_code == 200: + current_tags = response.json().get('tags', []) if new_email != current_email: # If user is subscribed, we remove the old email from the list and add the new one new_tags = [t.get('name') for t in current_tags if t.get('name') not in COUNTRIES] + [new_country] - create_mailchimp_subscriber(new_email, new_tags) - delete_mailchimp_subscriber(current_email) + successfully_created = create_mailchimp_subscriber(new_email, new_tags) + if successfully_created: + delete_mailchimp_subscriber(current_email) else: # If the user did not change their email, check if the country needs to be updated tags_to_update = get_country_tag_changes(current_tags, new_country) @@ -64,11 +69,10 @@ def add_class_customized_to_subscription(email): create_subscription_event(email, MailchimpTag.CUSTOMIZED_CLASS) +@run_if_mailchimp_config_present def create_subscription_event(email, tag): """ When certain events occur, e.g. a newsletter subscriber creates or customizes a class, these events should be reflected in their subscription, so that we can send them relevant content """ - if not MAILCHIMP_API_URL: - return r = get_mailchimp_subscriber(email) if r.status_code == 200: current_tags = r.json().get('tags', []) @@ -122,6 +126,7 @@ def create_mailchimp_subscriber(email, tag_names): "ERROR - Subscription to Hedy newsletter", f"email: {email} status: {r.status_code} body: {r.text}", f"{email}
Status:{r.status_code} Body:{r.text}")
+ return not subscription_error
def get_mailchimp_subscriber(email):
@@ -140,5 +145,5 @@ def delete_mailchimp_subscriber(email):
def get_subscriber_hash(email):
- """ We hash the email with md5 to avoid emails with unescaped characters triggering errors """
+ """ Hashes the email with md5 to avoid emails with unescaped characters triggering errors """
return hashlib.md5(email.encode("utf-8")).hexdigest()
From 45b9694a1a2b63345ae3e12871a41a4f9973a37f Mon Sep 17 00:00:00 2001
From: Boryana Goncharenko
<3010723+boryanagoncharenko@users.noreply.github.com>
Date: Wed, 15 Jan 2025 11:53:24 +0200
Subject: [PATCH 3/4] Improve Mailchimp calls
---
website/newsletter.py | 82 ++++++++++++++++++++++++++++---------------
1 file changed, 54 insertions(+), 28 deletions(-)
diff --git a/website/newsletter.py b/website/newsletter.py
index 58d7eeb5539..b41d21fbcd6 100644
--- a/website/newsletter.py
+++ b/website/newsletter.py
@@ -2,14 +2,18 @@
import json
import hashlib
import requests
+import logging
+import threading
from config import config
from functools import wraps
from hedy_content import COUNTRIES
from website.auth import send_email
+logger = logging.getLogger(__name__)
MAILCHIMP_API_URL = None
MAILCHIMP_API_HEADERS = {}
+MAILCHIMP_TIMEOUT_SECONDS = 15
if os.getenv("MAILCHIMP_API_KEY") and os.getenv("MAILCHIMP_AUDIENCE_ID"):
# The domain in the path is the server name, which is contained in the Mailchimp API key
MAILCHIMP_API_URL = (
@@ -25,40 +29,55 @@
def run_if_mailchimp_config_present(f):
- """Similar to 'requires_login', but also tests that the user is an admin."""
+ """ Decorator that runs a particular Mailchimp-dependent function only if there are credentials available. """
@wraps(f)
def inner(*args, **kws):
if MAILCHIMP_API_URL:
- return f(*args, **kws)
+ f(*args, **kws)
return inner
+def fire_and_forget(f):
+ def run():
+ try:
+ f()
+ except Exception as e:
+ logger.exception(f'Exception in background thread: {e}')
+
+ threading.Thread(target=run, daemon=True).start()
+
+
@run_if_mailchimp_config_present
def create_subscription(email, country):
""" Subscribes the user to the newsletter. Currently, users can subscribe to the newsletter only on signup and
only if they are creating a teacher account. """
- tags = [country, MailchimpTag.TEACHER]
- create_mailchimp_subscriber(email, tags)
+ def create():
+ tags = [country, MailchimpTag.TEACHER]
+ create_mailchimp_subscriber(email, tags)
+ fire_and_forget(create)
@run_if_mailchimp_config_present
def update_subscription(current_email, new_email, new_country):
- """ Updates the newsletter subscription when the user changes their email or/and their country """
- response = get_mailchimp_subscriber(current_email)
- if response.status_code == 200:
- current_tags = response.json().get('tags', [])
- if new_email != current_email:
- # If user is subscribed, we remove the old email from the list and add the new one
- new_tags = [t.get('name') for t in current_tags if t.get('name') not in COUNTRIES] + [new_country]
- successfully_created = create_mailchimp_subscriber(new_email, new_tags)
- if successfully_created:
- delete_mailchimp_subscriber(current_email)
- else:
- # If the user did not change their email, check if the country needs to be updated
- tags_to_update = get_country_tag_changes(current_tags, new_country)
- if tags_to_update:
- update_mailchimp_tags(current_email, tags_to_update)
+ def update():
+ """ Updates the newsletter subscription when the user changes their email or/and their country """
+ response = get_mailchimp_subscriber(current_email)
+ if response and response.status_code == 200:
+ current_tags = response.json().get('tags', [])
+ if new_email != current_email:
+ # If user is subscribed, we remove the old email from the list and add the new one
+ new_tags = [t.get('name') for t in current_tags if t.get('name') not in COUNTRIES] + [new_country]
+ successfully_created = create_mailchimp_subscriber(new_email, new_tags)
+ if successfully_created:
+ delete_mailchimp_subscriber(current_email)
+ else:
+ # If the user did not change their email, check if the country needs to be updated
+ tags_to_update = get_country_tag_changes(current_tags, new_country)
+ if tags_to_update:
+ update_mailchimp_tags(current_email, tags_to_update)
+
+ fire_and_forget(update)
def add_class_created_to_subscription(email):
@@ -73,12 +92,14 @@ def add_class_customized_to_subscription(email):
def create_subscription_event(email, tag):
""" When certain events occur, e.g. a newsletter subscriber creates or customizes a class, these events
should be reflected in their subscription, so that we can send them relevant content """
- r = get_mailchimp_subscriber(email)
- if r.status_code == 200:
- current_tags = r.json().get('tags', [])
- if not any([t for t in current_tags if t.get('name') == tag]):
- new_tags = current_tags + [to_mailchimp_tag(tag)]
- update_mailchimp_tags(email, new_tags)
+ def create():
+ r = get_mailchimp_subscriber(email)
+ if r.status_code == 200:
+ current_tags = r.json().get('tags', [])
+ if not any([t for t in current_tags if t.get('name') == tag]):
+ new_tags = current_tags + [to_mailchimp_tag(tag)]
+ update_mailchimp_tags(email, new_tags)
+ fire_and_forget(create)
def get_country_tag_changes(current_tags, country):
@@ -131,17 +152,22 @@ def create_mailchimp_subscriber(email, tag_names):
def get_mailchimp_subscriber(email):
request_path = f'{MAILCHIMP_API_URL}/members/{get_subscriber_hash(email)}'
- return requests.get(request_path, headers=MAILCHIMP_API_HEADERS)
+ return requests.get(request_path, headers=MAILCHIMP_API_HEADERS, timeout=MAILCHIMP_TIMEOUT_SECONDS)
def update_mailchimp_tags(email, tags):
request_path = f'{MAILCHIMP_API_URL}/members/{get_subscriber_hash(email)}/tags'
- return requests.post(request_path, headers=MAILCHIMP_API_HEADERS, data=json.dumps({'tags': tags}))
+ return requests.post(
+ request_path,
+ headers=MAILCHIMP_API_HEADERS,
+ data=json.dumps({'tags': tags}),
+ timeout=MAILCHIMP_TIMEOUT_SECONDS
+ )
def delete_mailchimp_subscriber(email):
request_path = f'{MAILCHIMP_API_URL}/members/{get_subscriber_hash(email)}'
- requests.delete(request_path, headers=MAILCHIMP_API_HEADERS)
+ requests.delete(request_path, headers=MAILCHIMP_API_HEADERS, timeout=MAILCHIMP_TIMEOUT_SECONDS)
def get_subscriber_hash(email):
From 7efa4035f7e1f43b8504931881de75f7d3fd1247 Mon Sep 17 00:00:00 2001
From: Boryana Goncharenko
<3010723+boryanagoncharenko@users.noreply.github.com>
Date: Mon, 3 Feb 2025 14:10:07 +0200
Subject: [PATCH 4/4] Add slides #3995
---
app.py | 5 +++++
website/newsletter.py | 5 +++++
2 files changed, 10 insertions(+)
diff --git a/app.py b/app.py
index b665919d169..cb7488a1c15 100644
--- a/app.py
+++ b/app.py
@@ -55,6 +55,7 @@
from website.log_fetcher import log_fetcher
from website.frontend_types import Adventure, Program, ExtraStory, SaveInfo
from website.flask_hedy import g_db
+from website.newsletter import add_used_slides_to_subscription
logConfig(LOGGING_CONFIG)
logger = logging.getLogger(__name__)
@@ -2573,6 +2574,10 @@ def get_slides(level):
if not SLIDES[g.lang].get_slides_for_level(level, keyword_language):
return utils.error_page(error=404, ui_message="Slides do not exist!")
+ email = current_user().get('email')
+ if email:
+ add_used_slides_to_subscription(email)
+
slides = SLIDES[g.lang].get_slides_for_level(level, keyword_language)
return render_template('slides.html', level=level, slides=slides)
diff --git a/website/newsletter.py b/website/newsletter.py
index b41d21fbcd6..fe058ea8820 100644
--- a/website/newsletter.py
+++ b/website/newsletter.py
@@ -88,6 +88,10 @@ def add_class_customized_to_subscription(email):
create_subscription_event(email, MailchimpTag.CUSTOMIZED_CLASS)
+def add_used_slides_to_subscription(email):
+ create_subscription_event(email, MailchimpTag.USED_SLIDES)
+
+
@run_if_mailchimp_config_present
def create_subscription_event(email, tag):
""" When certain events occur, e.g. a newsletter subscriber creates or customizes a class, these events
@@ -126,6 +130,7 @@ class MailchimpTag:
TEACHER = 'teacher'
CREATED_CLASS = "created_class"
CUSTOMIZED_CLASS = "customized_class"
+ USED_SLIDES = "used_slides"
def create_mailchimp_subscriber(email, tag_names):