diff --git a/lms/djangoapps/discussion/tasks.py b/lms/djangoapps/discussion/tasks.py index 7b13112b30..1cc5e0eec6 100644 --- a/lms/djangoapps/discussion/tasks.py +++ b/lms/djangoapps/discussion/tasks.py @@ -101,10 +101,9 @@ def _get_course_language(course_id): def _build_message_context(context): - message_context = get_base_template_context(Site.objects.get(id=context['site_id'])) + message_context = get_base_template_context(context['site']) message_context.update(_deserialize_context_dates(context)) message_context['post_link'] = _get_thread_url(context) - message_context['ga_tracking_pixel_url'] = _generate_ga_pixel_url(context) return message_context @@ -122,25 +121,3 @@ def _get_thread_url(context): 'id': context['thread_id'], } return urljoin(context['site'].domain, permalink(thread_content)) - - -def _generate_ga_pixel_url(context): - # used for analytics - query_params = { - 'v': '1', # version, required for GA - 't': 'event', # - 'ec': 'email', # event category - 'ea': 'edx.bi.email.opened', # event action: in this case, the user opened the email - 'tid': get_value("GOOGLE_ANALYTICS_TRACKING_ID", getattr(settings, "GOOGLE_ANALYTICS_TRACKING_ID", None)), # tracking ID to associate this link with our GA instance - 'uid': context['thread_author_id'], - 'cs': 'sailthru', # Campaign source - what sent the email - 'cm': 'email', # Campaign medium - how the content is being delivered - 'cn': 'triggered_discussionnotification', # Campaign name - human-readable name for this particular class of message - 'dp': '/email/ace/discussions/responsenotification/{0}/'.format(context['course_id']), # document path, used for drilling down into specific events - 'dt': 'Reply to {0} at {1}'.format(context['thread_title'], context['comment_created_at']), # document title, should match the title of the email - } - - return u"{url}?{params}".format( - url="https://www.google-analytics.com/collect", - params=urlencode(query_params) - ) diff --git a/lms/djangoapps/discussion/templates/discussion/edx_ace/response_notification/email/body.html b/lms/djangoapps/discussion/templates/discussion/edx_ace/response_notification/email/body.html deleted file mode 100644 index 1b3ba81124..0000000000 --- a/lms/djangoapps/discussion/templates/discussion/edx_ace/response_notification/email/body.html +++ /dev/null @@ -1,62 +0,0 @@ -{% load i18n %} -{% load static %} - -{% block preview_text %} - {% blocktrans trimmed %} - Hi {{ thread_username }} - {% endblocktrans %} -{% endblock %} - -{% block content %} - - - - -
- {% blocktrans trimmed %} -

- Hi {{ thread_username }}, -

- -

- {{ comment_username }} made the following reply to {{ thread_title }} at {{ comment_created_at }}. -

- -

- {{ comment_body }} -

- -

- View the discussion. -

- {% endblocktrans %} - -

- {# email client support for style sheets is pretty spotty, so we have to inline all of these styles #} - - -

-
-{% endblock %} - -{% block google_analytics_pixel %} - -{% endblock %} diff --git a/lms/djangoapps/discussion/templates/discussion/edx_ace/response_notification/email/head.html b/lms/djangoapps/discussion/templates/discussion/edx_ace/response_notification/email/head.html deleted file mode 100644 index cf607a4521..0000000000 --- a/lms/djangoapps/discussion/templates/discussion/edx_ace/response_notification/email/head.html +++ /dev/null @@ -1,29 +0,0 @@ - -{% block title %}Reply to {{ thread_title }} at {{ comment_created_at }} {% endblock %} - - - \ No newline at end of file diff --git a/lms/djangoapps/discussion/templates/discussion/edx_ace/responsenotification/email/body.html b/lms/djangoapps/discussion/templates/discussion/edx_ace/responsenotification/email/body.html new file mode 100644 index 0000000000..1b299b51d2 --- /dev/null +++ b/lms/djangoapps/discussion/templates/discussion/edx_ace/responsenotification/email/body.html @@ -0,0 +1,35 @@ +{% extends 'schedules/edx_ace/common/base_body.html' %} + +{% load i18n %} +{% load static %} + +{% block preview_text %} + {% blocktrans trimmed %} + {{ comment_username }} replied to your thread. + {% endblocktrans %} +{% endblock %} + +{% block content %} + + + + +
+ {% blocktrans trimmed %} +

+ Hi {{ thread_username }}, +

+ +

+ {{ comment_username }} made the following reply to {{ thread_title }} at {{ comment_created_at }}. +

+ +

+ {{ comment_body }} +

+ {% endblocktrans %} + + {% trans "View the discussion" as course_cta_text %} + {% include "schedules/edx_ace/common/return_to_course_cta.html" with course_cta_text=course_cta_text course_cta_url=post_link%} +
+{% endblock %} diff --git a/lms/djangoapps/discussion/templates/discussion/edx_ace/response_notification/email/body.txt b/lms/djangoapps/discussion/templates/discussion/edx_ace/responsenotification/email/body.txt similarity index 100% rename from lms/djangoapps/discussion/templates/discussion/edx_ace/response_notification/email/body.txt rename to lms/djangoapps/discussion/templates/discussion/edx_ace/responsenotification/email/body.txt diff --git a/lms/djangoapps/discussion/templates/discussion/edx_ace/response_notification/email/from_name.txt b/lms/djangoapps/discussion/templates/discussion/edx_ace/responsenotification/email/from_name.txt similarity index 100% rename from lms/djangoapps/discussion/templates/discussion/edx_ace/response_notification/email/from_name.txt rename to lms/djangoapps/discussion/templates/discussion/edx_ace/responsenotification/email/from_name.txt diff --git a/lms/djangoapps/discussion/templates/discussion/edx_ace/responsenotification/email/head.html b/lms/djangoapps/discussion/templates/discussion/edx_ace/responsenotification/email/head.html new file mode 100644 index 0000000000..588357ec65 --- /dev/null +++ b/lms/djangoapps/discussion/templates/discussion/edx_ace/responsenotification/email/head.html @@ -0,0 +1 @@ +{% extends 'schedules/edx_ace/common/base_head.html' %} diff --git a/lms/djangoapps/discussion/templates/discussion/edx_ace/response_notification/email/subject.txt b/lms/djangoapps/discussion/templates/discussion/edx_ace/responsenotification/email/subject.txt similarity index 100% rename from lms/djangoapps/discussion/templates/discussion/edx_ace/response_notification/email/subject.txt rename to lms/djangoapps/discussion/templates/discussion/edx_ace/responsenotification/email/subject.txt diff --git a/lms/djangoapps/discussion/tests/test_tasks.py b/lms/djangoapps/discussion/tests/test_tasks.py index ccaf9d784e..0adfab68eb 100644 --- a/lms/djangoapps/discussion/tests/test_tasks.py +++ b/lms/djangoapps/discussion/tests/test_tasks.py @@ -1,25 +1,23 @@ """ Tests the execution of forum notification tasks. """ -from contextlib import contextmanager from datetime import datetime, timedelta import json import math -from urlparse import urljoin import ddt -from django.conf import settings from django.contrib.sites.models import Site import mock +import lms.lib.comment_client as cc + from django_comment_common.models import ForumsConfig from django_comment_common.signals import comment_created from edx_ace.recipient import Recipient from edx_ace.utils import date from lms.djangoapps.discussion.config.waffle import waffle, FORUM_RESPONSE_NOTIFICATIONS, SEND_NOTIFICATIONS_FOR_COURSE from lms.djangoapps.discussion.signals.handlers import ENABLE_FORUM_NOTIFICATIONS_FOR_SITE_KEY -from lms.djangoapps.discussion.tasks import _should_send_message, _generate_ga_pixel_url -import lms.lib.comment_client as cc +from lms.djangoapps.discussion.tasks import _should_send_message from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from openedx.core.djangoapps.schedules.template_context import get_base_template_context from openedx.core.djangoapps.site_configuration.tests.factories import SiteConfigurationFactory @@ -204,11 +202,10 @@ class TaskTestCase(ModuleStoreTestCase): 'thread_title': 'thread-title', 'thread_username': self.thread_author.username, 'thread_commentable_id': self.thread['commentable_id'], - 'post_link': urljoin(site.domain, self.mock_permalink.return_value), + 'post_link': self.mock_permalink.return_value, 'site': site, 'site_id': site.id }) - expected_message_context['ga_tracking_pixel_url'] = _generate_ga_pixel_url(expected_message_context) expected_recipient = Recipient(self.thread_author.username, self.thread_author.email) actual_message = self.mock_ace_send.call_args_list[0][0][0] self.assertEqual(expected_message_context, actual_message.context) diff --git a/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py b/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py index d9cc13ac06..6194bfe3d3 100644 --- a/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py +++ b/openedx/core/djangoapps/schedules/management/commands/tests/send_email_base.py @@ -400,7 +400,9 @@ class ScheduleSendEmailTestBase(FilteredQueryCountMixin, CacheIsolationTestCase) self.assertEqual(len(sent_messages), num_expected_messages) with self.assertNumQueries(NUM_QUERIES_PER_MESSAGE_DELIVERY): - self.deliver_task(*sent_messages[0]) + with patch('analytics.track') as mock_analytics_track: + self.deliver_task(*sent_messages[0]) + self.assertEqual(mock_analytics_track.call_count, 1) self.assertEqual(mock_channel.deliver.call_count, 1) for (_name, (_msg, email), _kwargs) in mock_channel.deliver.mock_calls: diff --git a/openedx/core/djangoapps/schedules/resolvers.py b/openedx/core/djangoapps/schedules/resolvers.py index bd46f27819..61f6a7b911 100644 --- a/openedx/core/djangoapps/schedules/resolvers.py +++ b/openedx/core/djangoapps/schedules/resolvers.py @@ -20,10 +20,7 @@ from openedx.core.djangoapps.schedules.content_highlights import get_week_highli from openedx.core.djangoapps.schedules.exceptions import CourseUpdateDoesNotExist from openedx.core.djangoapps.schedules.models import Schedule, ScheduleExperience from openedx.core.djangoapps.schedules.utils import PrefixedDebugLoggerMixin -from openedx.core.djangoapps.schedules.template_context import ( - absolute_url, - get_base_template_context -) +from openedx.core.djangoapps.schedules.template_context import get_base_template_context from openedx.core.djangoapps.site_configuration.models import SiteConfiguration @@ -247,9 +244,7 @@ class RecurringNudgeResolver(BinnedSchedulesBaseResolver): first_schedule = user_schedules[0] context = { 'course_name': first_schedule.enrollment.course.display_name, - 'course_url': absolute_url( - self.site, reverse('course_root', args=[str(first_schedule.enrollment.course_id)]) - ), + 'course_url': reverse('course_root', args=[str(first_schedule.enrollment.course_id)]), } # Information for including upsell messaging in template. @@ -289,7 +284,7 @@ class UpgradeReminderResolver(BinnedSchedulesBaseResolver): course_id_str = str(schedule.enrollment.course_id) course_id_strs.append(course_id_str) course_links.append({ - 'url': absolute_url(self.site, reverse('course_root', args=[course_id_str])), + 'url': reverse('course_root', args=[course_id_str]), 'name': schedule.enrollment.course.display_name }) @@ -300,7 +295,7 @@ class UpgradeReminderResolver(BinnedSchedulesBaseResolver): context = { 'course_links': course_links, 'first_course_name': first_schedule.enrollment.course.display_name, - 'cert_image': absolute_url(self.site, static('course_experience/images/verified-cert.png')), + 'cert_image': static('course_experience/images/verified-cert.png'), 'course_ids': course_id_strs, } context.update(first_valid_upsell_context) @@ -365,9 +360,7 @@ class CourseUpdateResolver(BinnedSchedulesBaseResolver): course_id_str = str(enrollment.course_id) template_context.update({ 'course_name': schedule.enrollment.course.display_name, - 'course_url': absolute_url( - self.site, reverse('course_root', args=[course_id_str]) - ), + 'course_url': reverse('course_root', args=[course_id_str]), 'week_num': week_num, 'week_highlights': week_highlights, diff --git a/openedx/core/djangoapps/schedules/tasks.py b/openedx/core/djangoapps/schedules/tasks.py index 88b866905b..b33a0ad628 100644 --- a/openedx/core/djangoapps/schedules/tasks.py +++ b/openedx/core/djangoapps/schedules/tasks.py @@ -1,6 +1,7 @@ import datetime import logging +import analytics from celery.task import task, Task from crum import CurrentRequestUserMiddleware from django.conf import settings @@ -180,7 +181,7 @@ class ScheduleCourseUpdate(ScheduleMessageBaseTask): def _schedule_send(msg_str, site_id, delivery_config_var, log_prefix): - site = Site.objects.get(pk=site_id) + site = Site.objects.select_related('configuration').get(pk=site_id) if _is_delivery_enabled(site, delivery_config_var, log_prefix): msg = Message.from_string(msg_str) @@ -193,6 +194,31 @@ def _schedule_send(msg_str, site_id, delivery_config_var, log_prefix): _annonate_send_task_for_monitoring(msg) LOG.debug('%s: Sending message = %s', log_prefix, msg_str) ace.send(msg) + _track_message_sent(site, user, msg) + + +def _track_message_sent(site, user, msg): + properties = { + 'site': site.domain, + 'app_label': msg.app_label, + 'name': msg.name, + 'language': msg.language, + 'uuid': msg.uuid, + 'send_uuid': msg.send_uuid, + } + course_ids = msg.context.get('course_ids', []) + if len(course_ids) > 0: + properties['course_ids'] = course_ids[:10] + properties['primary_course_id'] = course_ids[0] + + if len(course_ids) > 1: + properties['num_courses'] = len(course_ids) + + analytics.track( + user_id=user.id, + event='edx.bi.email.sent', + properties=properties + ) def _is_delivery_enabled(site, delivery_config_var, log_prefix): diff --git a/openedx/core/djangoapps/schedules/template_context.py b/openedx/core/djangoapps/schedules/template_context.py index f896a485e8..79f1b2ec81 100644 --- a/openedx/core/djangoapps/schedules/template_context.py +++ b/openedx/core/djangoapps/schedules/template_context.py @@ -1,65 +1,20 @@ -from urlparse import urlparse - from django.conf import settings from django.core.urlresolvers import reverse -from django.utils.http import urlquote from edxmako.shortcuts import marketing_link +from openedx.core.djangoapps.schedules.utils import get_config_value_from_site_or_settings def get_base_template_context(site): """Dict with entries needed for all templates that use the base template""" return { # Platform information - 'homepage_url': encode_url(marketing_link('ROOT')), - 'dashboard_url': absolute_url(site, reverse('dashboard')), - 'template_revision': settings.EDX_PLATFORM_REVISION, - 'platform_name': site.configuration.get_value('platform_name', settings.PLATFORM_NAME), - 'contact_mailing_address': site.configuration.get_value( - 'contact_mailing_address', - settings.CONTACT_MAILING_ADDRESS - ), - 'social_media_urls': encode_urls_in_dict( - site.configuration.get_value( - 'SOCIAL_MEDIA_FOOTER_URLS', - getattr(settings, 'SOCIAL_MEDIA_FOOTER_URLS', {}) - ) - ), - 'mobile_store_urls': encode_urls_in_dict( - site.configuration.get_value( - 'MOBILE_STORE_URLS', - getattr(settings, 'MOBILE_STORE_URLS', {}) - ) - - ), + 'homepage_url': marketing_link('ROOT'), + 'dashboard_url': reverse('dashboard'), + 'template_revision': getattr(settings, 'EDX_PLATFORM_REVISION', None), + 'platform_name': get_config_value_from_site_or_settings('PLATFORM_NAME', site=site, site_config_name='platform_name'), + 'contact_mailing_address': get_config_value_from_site_or_settings( + 'CONTACT_MAILING_ADDRESS', site=site, site_config_name='contact_mailing_address'), + 'social_media_urls': get_config_value_from_site_or_settings('SOCIAL_MEDIA_FOOTER_URLS', site=site), + 'mobile_store_urls': get_config_value_from_site_or_settings('MOBILE_STORE_URLS', site=site), } - - -def encode_url(url): - # Sailthru has a bug where URLs that contain "+" characters in their path components are misinterpreted - # when GA instrumentation is enabled. We need to percent-encode the path segments of all URLs that are - # injected into our templates to work around this issue. - parsed_url = urlparse(url) - modified_url = parsed_url._replace(path=urlquote(parsed_url.path)) - return modified_url.geturl() - - -def absolute_url(site, relative_path): - """ - Add site.domain to the beginning of the given relative path. - - If the given URL is already absolute (has a netloc part), then it is just returned. - """ - if bool(urlparse(relative_path).netloc): - # Given URL is already absolute - return relative_path - root = site.domain.rstrip('/') - relative_path = relative_path.lstrip('/') - return encode_url(u'https://{root}/{path}'.format(root=root, path=relative_path)) - - -def encode_urls_in_dict(mapping): - urls = {} - for key, value in mapping.iteritems(): - urls[key] = encode_url(value) - return urls diff --git a/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/common/base_body.html b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/common/base_body.html index 23f4144393..db47420c9b 100644 --- a/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/common/base_body.html +++ b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/common/base_body.html @@ -1,4 +1,5 @@ {% load i18n %} +{% load ace %} {% get_current_language as LANGUAGE_CODE %} {% get_current_language_bidi as LANGUAGE_BIDI %} @@ -23,6 +24,8 @@ {# Sailthru when the email is sent. Other email providers would need to replace this variable in the HTML as well. #} +{% google_analytics_tracking_pixel %} +
- {% blocktrans %}Go to {{ platform_name }} Home Page{% endblocktrans %} - {% trans "Sign In" %} + {% trans "Sign In" %} @@ -90,7 +93,7 @@ {% if social_media_urls.linkedin %} - + {% blocktrans %}{{ platform_name }} on LinkedIn{% endblocktrans %} @@ -98,7 +101,7 @@ {% endif %} {% if social_media_urls.twitter %} - + {% blocktrans %}{{ platform_name }} on Twitter{% endblocktrans %} @@ -106,7 +109,7 @@ {% endif %} {% if social_media_urls.facebook %} - + {% blocktrans %}{{ platform_name }} on Facebook{% endblocktrans %} @@ -114,7 +117,7 @@ {% endif %} {% if social_media_urls.google_plus %} - + {% blocktrans %}{{ platform_name }} on Google Plus{% endblocktrans %} @@ -122,7 +125,7 @@ {% endif %} {% if social_media_urls.reddit %} - + {% blocktrans %}{{ platform_name }} on Reddit{% endblocktrans %} @@ -136,14 +139,14 @@ {% if mobile_store_urls.apple %} - + {% trans {% endif %} {% if mobile_store_urls.google %} - + {% trans diff --git a/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/common/return_to_course_cta.html b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/common/return_to_course_cta.html index eb6e6b0c0b..fcf93ea3af 100644 --- a/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/common/return_to_course_cta.html +++ b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/common/return_to_course_cta.html @@ -1,12 +1,17 @@ {% load i18n %} +{% load ace %}

{# email client support for style sheets is pretty spotty, so we have to inline all of these styles #} 1 %} - href="{{ dashboard_url }}" + {% if course_cta_url %} + href="{% with_link_tracking course_cta_url %}" {% else %} - href="{{ course_url }}" + {%if course_ids|length > 1 %} + href="{% with_link_tracking dashboard_url %}" + {% else %} + href="{% with_link_tracking course_url %}" + {% endif %} {% endif %} style=" color: #ffffff; diff --git a/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/common/upsell_cta.html b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/common/upsell_cta.html index b134529583..913a9a58d0 100644 --- a/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/common/upsell_cta.html +++ b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/common/upsell_cta.html @@ -1,4 +1,5 @@ {% load i18n %} +{% load ace %} {% if show_upsell %}

@@ -8,7 +9,7 @@ {% endblocktrans %}

- {% endif %} diff --git a/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/courseupdate/email/body.html b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/courseupdate/email/body.html index decd420ccb..4be7dcd00e 100644 --- a/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/courseupdate/email/body.html +++ b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/courseupdate/email/body.html @@ -1,6 +1,5 @@ {% extends 'schedules/edx_ace/common/base_body.html' %} {% load i18n %} -{% load static %} {% block preview_text %} {% blocktrans trimmed %} diff --git a/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/recurringnudge_day10/email/body.txt b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/recurringnudge_day10/email/body.txt index 9a6e8cf8eb..584470c3f4 100644 --- a/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/recurringnudge_day10/email/body.txt +++ b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/recurringnudge_day10/email/body.txt @@ -1,15 +1,16 @@ {% load i18n %} +{% load ace %} {% if course_ids|length > 1 %} {% blocktrans trimmed %} Many edX learners are completing more problems every week, and participating in the discussion forums. What do you want to do to keep learning? {% endblocktrans %} - {% trans "Keep learning" %} <{{dashboard_url}}> + {% trans "Keep learning" %} <{% with_link_tracking dashboard_url %}> {% else %} {% blocktrans trimmed %} Many edX learners in {{course_name}} are completing more problems every week, and participating in the discussion forums. What do you want to do to keep learning? {% endblocktrans %} - {% trans "Keep learning" %} <{{course_url}}> + {% trans "Keep learning" %} <{% with_link_tracking course_url %}> {% endif %} {% include "schedules/edx_ace/common/upsell_cta.txt"%} diff --git a/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/recurringnudge_day3/email/body.txt b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/recurringnudge_day3/email/body.txt index efcfeba1bf..910f9e14ba 100644 --- a/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/recurringnudge_day3/email/body.txt +++ b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/recurringnudge_day3/email/body.txt @@ -1,18 +1,18 @@ {% load i18n %} - +{% load ace %} {% if course_ids|length > 1 %} {% blocktrans trimmed %} Remember when you enrolled in {{ course_name }}, and other courses on edX.org? We do, and we’re glad to have you! Come see what everyone is learning. {% endblocktrans %} -{% trans "Start learning now" %} <{{ dashboard_url }}> +{% trans "Start learning now" %} <{% with_link_tracking dashboard_url %}> {% else %} {% blocktrans trimmed %} Remember when you enrolled in {{ course_name }} on edX.org? We do, and we’re glad to have you! Come see what everyone is learning. {% endblocktrans %} -{% trans "Start learning now" %} <{{ course_url }}> +{% trans "Start learning now" %} <{% with_link_tracking course_url %}> {% endif %} {% include "schedules/edx_ace/common/upsell_cta.txt"%} diff --git a/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/body.html b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/body.html index b14570d5fe..5c1af75a7a 100644 --- a/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/body.html +++ b/openedx/core/djangoapps/schedules/templates/schedules/edx_ace/upgradereminder/email/body.html @@ -1,6 +1,7 @@ {% extends 'schedules/edx_ace/common/base_body.html' %} {% load i18n %} {% load static %} +{% load ace %} {% block preview_text %} {% if course_ids|length > 1 %} @@ -56,7 +57,7 @@

@@ -82,9 +83,9 @@ {# email client support for style sheets is pretty spotty, so we have to inline all of these styles #} +{% trans "Upgrade now at" %} <{% with_link_tracking dashboard_url %}> {% else %} {% blocktrans trimmed %} We hope you are enjoying learning with us so far in {{ first_course_name }}! A verified certificate @@ -25,5 +25,5 @@ official and easily shareable. Upgrade by {{ user_schedule_upgrade_deadline_time }}. {% endblocktrans %} -{% trans "Upgrade now at" %} <{{ upsell_link }}> +{% trans "Upgrade now at" %} <{% with_link_tracking upsell_link %}> {% endif %} diff --git a/openedx/core/djangoapps/schedules/templatetags/__init__.py b/openedx/core/djangoapps/schedules/templatetags/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/openedx/core/djangoapps/schedules/templatetags/ace.py b/openedx/core/djangoapps/schedules/templatetags/ace.py new file mode 100644 index 0000000000..aff2ad45eb --- /dev/null +++ b/openedx/core/djangoapps/schedules/templatetags/ace.py @@ -0,0 +1,150 @@ +from urlparse import urlparse, parse_qs + +from crum import get_current_request +from django import template +from django.utils.safestring import mark_safe + +from openedx.core.djangoapps.schedules.tracking import CampaignTrackingInfo, GoogleAnalyticsTrackingPixel +from openedx.core.djangolib.markup import HTML + +register = template.Library() + + +@register.simple_tag(takes_context=True) +def with_link_tracking(context, url): + """ + Modifies the provided URL to ensure it is safe for usage in an email template and adds UTM parameters to it. + + The provided URL can be relative or absolute. If it is relative, it will be made absolute. + + All URLs will be augmented to include UTM parameters so that clicks can be tracked. + + Args: + context (dict): The template context. Must include a "request" and "message". + url (str): The url to rewrite. + + Returns: + str: The URL as an absolute URL with appropriate query string parameters that allow clicks to be tracked. + + """ + site, _user, message = _get_variables_from_context(context, 'with_link_tracking') + + campaign = CampaignTrackingInfo( + source=message.app_label, + campaign=message.name, + content=message.uuid, + ) + course_ids = context.get('course_ids') + if course_ids is not None and len(course_ids) > 0: + campaign.term = course_ids[0] + + return mark_safe( + modify_url_to_track_clicks( + ensure_url_is_absolute(site, url), + campaign=campaign + ) + ) + + +def _get_variables_from_context(context, tag_name): + if 'request' in context: + request = context['request'] + else: + request = get_current_request() + + if request is None: + raise template.VariableDoesNotExist( + 'The {0} template tag requires a "request" to be present in the template context. Consider using ' + '"emulate_http_request" if you are rendering the template in a celery task.'.format(tag_name) + ) + + message = context.get('message') + if message is None: + raise template.VariableDoesNotExist( + 'The {0} template tag requires a "message" to be present in the template context.'.format(tag_name) + ) + + return request.site, request.user, message + + +@register.simple_tag(takes_context=True) +def google_analytics_tracking_pixel(context): + """ + If configured, inject a google analytics tracking pixel into the template. + + This tracking pixel will allow email open events to be tracked. + + Args: + context (dict): The template context. Must include a "request" and "message". + + Returns: + str: A string containing an HTML image tag that implements the GA measurement protocol or an empty string if + GA is not configured. For this to work, the site or settings must include the GA tracking ID. + """ + image_url = _get_google_analytics_tracking_url(context) + if image_url is not None: + return mark_safe( + HTML('').format(HTML(image_url)) + ) + else: + return '' + + +def _get_google_analytics_tracking_url(context): + site, user, message = _get_variables_from_context(context, 'google_analytics_tracking_pixel') + + pixel = GoogleAnalyticsTrackingPixel( + site=site, + user_id=user.id, + campaign_source=message.app_label, + campaign_name=message.name, + campaign_content=message.uuid, + document_path='/email/{0}/{1}/{2}/{3}'.format( + message.app_label, + message.name, + message.send_uuid, + message.uuid, + ), + ) + course_ids = context.get('course_ids') + if course_ids is not None and len(course_ids) > 0: + pixel.course_id = course_ids[0] + + return pixel.image_url + + +def modify_url_to_track_clicks(url, campaign=None): + """ + Given a URL, this method modifies the query string parameters to include UTM tracking parameters. + + These UTM codes are used to by Google Analytics to identify the source of traffic. This will help us better + understand how users behave when they come to the site by clicking a link in this email. + + Arguments: + url (str): pass + campaign (CampaignTrackingInfo): pass + + Returns: + str: The url with appropriate query string parameters. + """ + parsed_url = urlparse(url) + if campaign is None: + campaign = CampaignTrackingInfo() + modified_url = parsed_url._replace(query=campaign.to_query_string(parsed_url.query)) + return modified_url.geturl() + + +def ensure_url_is_absolute(site, relative_path): + """ + Add site.domain to the beginning of the given relative path. + + If the given URL is already absolute (has a netloc part), then it is just returned. + """ + if bool(urlparse(relative_path).netloc): + # Given URL is already absolute + url = relative_path + else: + root = site.domain.rstrip('/') + relative_path = relative_path.lstrip('/') + url = u'https://{root}/{path}'.format(root=root, path=relative_path) + return url diff --git a/openedx/core/djangoapps/schedules/tests/mixins.py b/openedx/core/djangoapps/schedules/tests/mixins.py new file mode 100644 index 0000000000..e738743b94 --- /dev/null +++ b/openedx/core/djangoapps/schedules/tests/mixins.py @@ -0,0 +1,53 @@ +from urlparse import parse_qs, urlparse + + +class QueryStringAssertionMixin(object): + + def assert_query_string_equal(self, expected_qs, actual_qs): + """ + Compares two query strings to see if they are equivalent. Note that order of parameters is not significant. + + Args: + expected_qs (str): The expected query string. + actual_qs (str): The actual query string. + + Raises: + AssertionError: If the two query strings are not equal. + """ + self.assertDictEqual(parse_qs(expected_qs), parse_qs(actual_qs)) + + def assert_url_components_equal(self, url, **kwargs): + """ + Assert that the provided URL has the expected components with the expected values. + + Args: + url (str): The URL to parse and make assertions about. + **kwargs: The expected component values. For example: scheme='https' would assert that the URL scheme was + https. + + Raises: + AssertionError: If any of the expected components do not match. + """ + parsed_url = urlparse(url) + for expected_component, expected_value in kwargs.items(): + if expected_component == 'query': + self.assert_query_string_equal(expected_value, parsed_url.query) + else: + self.assertEqual(expected_value, getattr(parsed_url, expected_component)) + + def assert_query_string_parameters_equal(self, url, **kwargs): + """ + Assert that the provided URL has query string paramters that match the kwargs. + + Args: + url (str): The URL to parse and make assertions about. + **kwargs: The expected query string parameter values. For example: foo='bar' would assert that foo=bar + appeared in the query string. + + Raises: + AssertionError: If any of the expected parameters values do not match. + """ + parsed_url = urlparse(url) + parsed_qs = parse_qs(parsed_url.query) + for expected_key, expected_value in kwargs.items(): + self.assertEqual(parsed_qs[expected_key], [str(expected_value)]) diff --git a/openedx/core/djangoapps/schedules/tests/test_template_context.py b/openedx/core/djangoapps/schedules/tests/test_template_context.py deleted file mode 100644 index d116530c79..0000000000 --- a/openedx/core/djangoapps/schedules/tests/test_template_context.py +++ /dev/null @@ -1,23 +0,0 @@ -from openedx.core.djangoapps.schedules.template_context import absolute_url -from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory -from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms - - -@skip_unless_lms -class TestTemplateContext(CacheIsolationTestCase): - def setUp(self): - self.site = SiteFactory.create() - self.site.domain = 'example.com' - - def test_absolute_url(self): - absolute = absolute_url(self.site, '/foo/bar') - self.assertEqual(absolute, 'https://example.com/foo/bar') - - def test_absolute_url_domain_lstrip(self): - self.site.domain = 'example.com/' - absolute = absolute_url(self.site, 'foo/bar') - self.assertEqual(absolute, 'https://example.com/foo/bar') - - def test_absolute_url_already_absolute(self): - absolute = absolute_url(self.site, 'https://some-cdn.com/foo/bar') - self.assertEqual(absolute, 'https://some-cdn.com/foo/bar') diff --git a/openedx/core/djangoapps/schedules/tests/test_templatetags.py b/openedx/core/djangoapps/schedules/tests/test_templatetags.py new file mode 100644 index 0000000000..f223fc76c6 --- /dev/null +++ b/openedx/core/djangoapps/schedules/tests/test_templatetags.py @@ -0,0 +1,172 @@ +import uuid + +from django.http import HttpRequest +from django.template import VariableDoesNotExist +from django.test import override_settings +from mock import patch + +from edx_ace import Message, Recipient +from openedx.core.djangoapps.schedules.templatetags.ace import ( + ensure_url_is_absolute, + with_link_tracking, + google_analytics_tracking_pixel, + _get_google_analytics_tracking_url +) +from openedx.core.djangoapps.schedules.tests.mixins import QueryStringAssertionMixin +from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory +from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms +from student.tests.factories import UserFactory + + +@skip_unless_lms +class TestAbsoluteUrl(CacheIsolationTestCase): + def setUp(self): + self.site = SiteFactory.create() + self.site.domain = 'example.com' + + def test_absolute_url(self): + absolute = ensure_url_is_absolute(self.site, '/foo/bar') + self.assertEqual(absolute, 'https://example.com/foo/bar') + + def test_absolute_url_domain_lstrip(self): + self.site.domain = 'example.com/' + absolute = ensure_url_is_absolute(self.site, 'foo/bar') + self.assertEqual(absolute, 'https://example.com/foo/bar') + + def test_absolute_url_already_absolute(self): + absolute = ensure_url_is_absolute(self.site, 'https://some-cdn.com/foo/bar') + self.assertEqual(absolute, 'https://some-cdn.com/foo/bar') + + +class EmailTemplateTagMixin(object): + + def setUp(self): + patcher = patch('openedx.core.djangoapps.schedules.templatetags.ace.get_current_request') + self.mock_get_current_request = patcher.start() + self.addCleanup(patcher.stop) + + self.fake_request = HttpRequest() + self.fake_request.user = UserFactory.create() + self.fake_request.site = SiteFactory.create() + self.fake_request.site.domain = 'example.com' + self.mock_get_current_request.return_value = self.fake_request + + self.message = Message( + app_label='test_app_label', + name='test_name', + recipient=Recipient(username='test_user'), + context={}, + send_uuid=uuid.uuid4(), + ) + self.context = { + 'message': self.message + } + + +@skip_unless_lms +class TestLinkTrackingTag(QueryStringAssertionMixin, EmailTemplateTagMixin, CacheIsolationTestCase): + + def test_default(self): + result_url = str(with_link_tracking(self.context, 'http://example.com/foo')) + self.assert_url_components_equal( + result_url, + scheme='http', + netloc='example.com', + path='/foo', + query='utm_source=test_app_label&utm_campaign=test_name&utm_medium=email&utm_content={uuid}'.format( + uuid=self.message.uuid + ) + ) + + def test_missing_request(self): + self.mock_get_current_request.return_value = None + + with self.assertRaises(VariableDoesNotExist): + with_link_tracking(self.context, 'http://example.com/foo') + + def test_missing_message(self): + del self.context['message'] + + with self.assertRaises(VariableDoesNotExist): + with_link_tracking(self.context, 'http://example.com/foo') + + def test_course_id(self): + self.context['course_ids'] = ['foo/bar/baz'] + result_url = str(with_link_tracking(self.context, 'http://example.com/foo')) + self.assert_query_string_parameters_equal( + result_url, + utm_term='foo/bar/baz', + ) + + def test_multiple_course_ids(self): + self.context['course_ids'] = ['foo/bar/baz', 'course-v1:FooX+bar+baz'] + result_url = str(with_link_tracking(self.context, 'http://example.com/foo')) + self.assert_query_string_parameters_equal( + result_url, + utm_term='foo/bar/baz', + ) + + def test_relative_url(self): + result_url = str(with_link_tracking(self.context, '/foobar')) + self.assert_url_components_equal( + result_url, + scheme='https', + netloc='example.com', + path='/foobar' + ) + + +@skip_unless_lms +@override_settings(GOOGLE_ANALYTICS_TRACKING_ID='UA-123456-1') +class TestGoogleAnalyticsPixelTag(QueryStringAssertionMixin, EmailTemplateTagMixin, CacheIsolationTestCase): + + def test_default(self): + result_url = _get_google_analytics_tracking_url(self.context) + self.assert_query_string_parameters_equal( + result_url, + uid=self.fake_request.user.id, + cs=self.message.app_label, + cn=self.message.name, + cc=self.message.uuid, + dp='/email/test_app_label/test_name/{send_uuid}/{uuid}'.format( + send_uuid=self.message.send_uuid, + uuid=self.message.uuid, + ) + ) + + def test_missing_request(self): + self.mock_get_current_request.return_value = None + + with self.assertRaises(VariableDoesNotExist): + google_analytics_tracking_pixel(self.context) + + def test_missing_message(self): + del self.context['message'] + + with self.assertRaises(VariableDoesNotExist): + google_analytics_tracking_pixel(self.context) + + def test_course_id(self): + self.context['course_ids'] = ['foo/bar/baz'] + result_url = _get_google_analytics_tracking_url(self.context) + self.assert_query_string_parameters_equal( + result_url, + el='foo/bar/baz', + ) + + def test_multiple_course_ids(self): + self.context['course_ids'] = ['foo/bar/baz', 'course-v1:FooX+bar+baz'] + result_url = _get_google_analytics_tracking_url(self.context) + self.assert_query_string_parameters_equal( + result_url, + el='foo/bar/baz', + ) + + def test_html_emitted(self): + result_html = google_analytics_tracking_pixel(self.context) + self.assertIn(' +{% google_analytics_tracking_pixel %} +
- {% blocktrans %}Go to {{ platform_name }} Home Page{% endblocktrans %} - + @@ -92,7 +95,7 @@ {% if social_media_urls.linkedin %} - + {% blocktrans %}{{ platform_name }} on LinkedIn{% endblocktrans %} @@ -100,7 +103,7 @@ {% endif %} {% if social_media_urls.twitter %} - + {% blocktrans %}{{ platform_name }} on Twitter{% endblocktrans %} @@ -108,7 +111,7 @@ {% endif %} {% if social_media_urls.facebook %} - + {% blocktrans %}{{ platform_name }} on Facebook{% endblocktrans %} @@ -116,7 +119,7 @@ {% endif %} {% if social_media_urls.google_plus %} - + {% blocktrans %}{{ platform_name }} on Google Plus{% endblocktrans %} @@ -124,7 +127,7 @@ {% endif %} {% if social_media_urls.reddit %} - + {% blocktrans %}{{ platform_name }} on Reddit{% endblocktrans %} @@ -138,14 +141,14 @@ {% if mobile_store_urls.apple %} - + {% trans {% endif %} {% if mobile_store_urls.google %} - + {% trans diff --git a/themes/red-theme/lms/templates/schedules/edx_ace/common/return_to_course_cta.html b/themes/red-theme/lms/templates/schedules/edx_ace/common/return_to_course_cta.html index 4251c14cbd..ef4bfa5160 100644 --- a/themes/red-theme/lms/templates/schedules/edx_ace/common/return_to_course_cta.html +++ b/themes/red-theme/lms/templates/schedules/edx_ace/common/return_to_course_cta.html @@ -1,12 +1,17 @@ {% load i18n %} +{% load ace %}

{# email client support for style sheets is pretty spotty, so we have to inline all of these styles #} 1 %} - href="{{ dashboard_url }}" + {% if course_cta_url %} + href="{% with_link_tracking course_cta_url %}" {% else %} - href="{{ course_url }}" + {%if course_ids|length > 1 %} + href="{% with_link_tracking dashboard_url %}" + {% else %} + href="{% with_link_tracking course_url %}" + {% endif %} {% endif %} style=" color: #ffffff; diff --git a/themes/red-theme/lms/templates/schedules/edx_ace/recurringnudge_day3/email/body.txt b/themes/red-theme/lms/templates/schedules/edx_ace/recurringnudge_day3/email/body.txt index 4e9484ac03..19b5365f08 100644 --- a/themes/red-theme/lms/templates/schedules/edx_ace/recurringnudge_day3/email/body.txt +++ b/themes/red-theme/lms/templates/schedules/edx_ace/recurringnudge_day3/email/body.txt @@ -1,4 +1,5 @@ {% load i18n %} +{% load ace %} This is the RED theme! {% if course_ids|length > 1 %} @@ -7,13 +8,13 @@ This is the RED theme! to have you! Come see what everyone is learning. {% endblocktrans %} -{% trans "Start learning now" %} <{{ dashboard_url }}> +{% trans "Start learning now" %} <{% with_link_tracking dashboard_url %}> {% else %} {% blocktrans trimmed %} Remember when you enrolled in {{ course_name }} on edX.org? We do, and we’re glad to have you! Come see what everyone is learning. {% endblocktrans %} -{% trans "Start learning now" %} <{{ course_url }}> +{% trans "Start learning now" %} <{% with_link_tracking course_url %}> {% endif %} {% include "schedules/edx_ace/common/upsell_cta.txt"%}