Commit 5cc73bc7 by Tyler Hallada Committed by GitHub

Merge pull request #16276 from edx/thallada/ret-dedupe-upgrade-reminder

Send only one upgrade reminder email per user per day
parents 5940342f 4ab477ea
......@@ -34,8 +34,9 @@ NUM_QUERIES_NO_MATCHING_SCHEDULES = 2
NUM_QUERIES_WITH_MATCHES = NUM_QUERIES_NO_MATCHING_SCHEDULES + 1
# 1) Global dynamic deadline switch
# 2) Course dynamic deadline switch
# 2) E-commerce configuration
NUM_QUERIES_WITH_DEADLINE = 2
NUM_QUERIES_WITH_DEADLINE = 3
NUM_COURSE_MODES_QUERIES = 1
......@@ -233,7 +234,7 @@ class TestUpgradeReminder(FilteredQueryCountMixin, CacheIsolationTestCase):
bin_num=user.id % tasks.UPGRADE_REMINDER_NUM_BINS,
org_list=[schedules[0].enrollment.course.org],
)
self.assertEqual(mock_schedule_send.apply_async.call_count, 3)
self.assertEqual(mock_schedule_send.apply_async.call_count, 1)
self.assertFalse(mock_ace.send.called)
@ddt.data(*itertools.product((1, 10, 100), (2, 10)))
......@@ -281,7 +282,7 @@ class TestUpgradeReminder(FilteredQueryCountMixin, CacheIsolationTestCase):
# we execute one query per course to see if it's opted out of dynamic upgrade deadlines, however,
# since we create a new course for each schedule in this test, we expect there to be one per message
num_expected_queries = NUM_QUERIES_WITH_MATCHES + NUM_QUERIES_WITH_DEADLINE + message_count
num_expected_queries = NUM_QUERIES_WITH_MATCHES + NUM_QUERIES_WITH_DEADLINE
with self.assertNumQueries(num_expected_queries, table_blacklist=WAFFLE_TABLES):
tasks.upgrade_reminder_schedule_bin(
self.site_config.site.id, target_day_str=test_datetime_str, day_offset=day,
......@@ -289,15 +290,15 @@ class TestUpgradeReminder(FilteredQueryCountMixin, CacheIsolationTestCase):
org_list=[schedules[0].enrollment.course.org],
)
self.assertEqual(len(sent_messages), message_count)
self.assertEqual(len(sent_messages), 1)
# Load the site (which we query per message sent)
# Check the schedule config
with self.assertNumQueries(1 + message_count):
with self.assertNumQueries(2):
for args in sent_messages:
tasks._upgrade_reminder_schedule_send(*args)
self.assertEqual(mock_channel.deliver.call_count, message_count)
self.assertEqual(mock_channel.deliver.call_count, 1)
for (_name, (_msg, email), _kwargs) in mock_channel.deliver.mock_calls:
for template in attr.astuple(email):
self.assertNotIn("TEMPLATE WARNING", template)
......
......@@ -222,26 +222,28 @@ def _upgrade_reminder_schedules_for_bin(site, current_datetime, target_datetime,
exclude_orgs=exclude_orgs,
)
for schedule in schedules:
enrollment = schedule.enrollment
user = enrollment.user
course_id_str = str(enrollment.course_id)
# TODO: group by schedule and user like recurring nudge
course_id_strs = [course_id_str]
first_schedule = schedule
for (user, user_schedules) in groupby(schedules, lambda s: s.enrollment.user):
user_schedules = list(user_schedules)
course_id_strs = [str(schedule.enrollment.course_id) for schedule in user_schedules]
first_schedule = user_schedules[0]
template_context = get_base_template_context(site)
template_context.update({
'student_name': user.profile.name,
'user_personal_address': user.profile.name if user.profile.name else user.username,
'course_name': first_schedule.enrollment.course.display_name,
'course_url': absolute_url(site, reverse('course_root', args=[str(first_schedule.enrollment.course_id)])),
'course_links': [
{
'url': absolute_url(site, reverse('course_root', args=[str(s.enrollment.course_id)])),
'name': s.enrollment.course.display_name
} for s in user_schedules
],
'first_course_name': first_schedule.enrollment.course.display_name,
# This is used by the bulk email optout policy
'course_ids': course_id_strs,
'cert_image': absolute_url(site, static('course_experience/images/verified-cert.png')),
})
......
......@@ -3,12 +3,25 @@
{% load static %}
{% block preview_text %}
{% if course_ids|length > 1 %}
{% blocktrans trimmed %}
We hope you are enjoying learning with us so far in {{ course_name }}! A verified certificate
will allow you to highlight your new knowledge and skills. It's official, and easily shareable.
We hope you are enjoying learning with us so far on {{ platform_name }}! A verified certificate allows you to
highlight your new knowledge and skills. An {{ platform_name }} certificate is official and easily
shareable.
Upgrade by {{ user_schedule_upgrade_deadline_time }}.
{% endblocktrans %}
{% else %}
{% blocktrans trimmed %}
We hope you are enjoying learning with us so far in {{ first_course_name }}! A verified certificate allows
you to highlight your new knowledge and skills. An {{ platform_name }} certificate is official and easily
shareable.
Upgrade by {{ user_schedule_upgrade_deadline_time }}.
{% endblocktrans %}
{% endif %}
{% blocktrans trimmed %}
{% endblocktrans %}
{% endblock %}
{% block content %}
......@@ -18,11 +31,19 @@
<h1>{% trans "Upgrade now" %}</h1>
<p>
{% if course_ids|length > 1 %}
{% blocktrans trimmed %}
We hope you are enjoying learning with us so far in <strong>{{ course_name }}</strong>! A
verified certificate will allow you to highlight your new knowledge and skills. It's official,
and easily shareable.
We hope you are enjoying learning with us so far on <strong>{{ platform_name }}</strong>! A
verified certificate allows you to highlight your new knowledge and skills. An {{ platform_name
}} certificate is official and easily shareable.
{% endblocktrans %}
{% else %}
{% blocktrans trimmed %}
We hope you are enjoying learning with us so far in <strong>{{ first_course_name }}</strong>! A
verified certificate allows you to highlight your new knowledge and skills. An {{ platform_name
}} certificate is official and easily shareable.
{% endblocktrans %}
{% endif %}
</p>
<p>
{% blocktrans trimmed %}
......@@ -30,7 +51,19 @@
{% endblocktrans %}
</p>
<a href="{{ upsell_link }}">
{% if course_ids|length > 1 and course_ids|length < 10 %}
<p>
{% trans "You are eligible to upgrade in these courses:" %}
</p>
<ul style="margin-bottom: 30px;">
{% for course_link in course_links %}
<li>
<a href="{{ course_link.url }}">{{ course_link.name }}</a>
</li>
{% endfor %}
</ul>
{% endif %}
<img
src="{{ cert_image }}"
alt="{% trans 'Example print-out of a verified certificate' %}"
......@@ -45,12 +78,15 @@
border-right: 3px solid lightgray;
border-left: 1px solid lightgray;
" />
</a>
<p>
{# email client support for style sheets is pretty spotty, so we have to inline all of these styles #}
<a
{% if course_ids|length == 1 %}
href="{{ upsell_link }}"
{% else %}
href="{{ dashboard_url }}"
{% endif %}
style="
color: #ffffff;
text-decoration: none;
......
{% load i18n %}
{% if course_ids|length > 1 %}
{% blocktrans trimmed %}
We hope you are enjoying learning with us so far in {{ course_name }}! A verified certificate
will allow you to highlight your new knowledge and skills. It's official, and easily shareable.
We hope you are enjoying learning with us so far on {{ platform_name }}! A verified certificate
allows you to highlight your new knowledge and skills. An {{ platform_name }} certificate is
official and easily shareable.
Upgrade by {{ user_schedule_upgrade_deadline_time }}.
{% endblocktrans %}
{% if course_ids|length > 1 and course_ids|length < 10 %}
{% for course_link in course_links %}
* {{ course_link.name }} <{{ course_link.url }}>
{% endfor %}
{% endif %}
{% trans "Upgrade now at" %} <{{ dashboard_url }}>
{% else %}
{% blocktrans trimmed %}
We hope you are enjoying learning with us so far in {{ first_course_name }}! A verified certificate
allows you to highlight your new knowledge and skills. An {{ platform_name }} certificate is
official and easily shareable.
Upgrade by {{ user_schedule_upgrade_deadline_time }}.
{% endblocktrans %}
{% trans "Upgrade now at" %} <{{ upsell_link }}>
{% endif %}
{{ course_name }}
{% if course_ids|length > 1 %}
{{ platform_name }}
{% else %}
{{ first_course_name }}
{% endif %}
{% load i18n %}
{% blocktrans %}Upgrade to earn a verified certificate in {{ course_name }}{% endblocktrans %}
{% if course_ids|length > 1 %}
{% blocktrans %}Upgrade to earn a verified certificate on {{ platform_name }}{% endblocktrans %}
{% else %}
{% blocktrans %}Upgrade to earn a verified certificate in {{ first_course_name }}{% endblocktrans %}
{% endif %}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment