From a2dd5d91f6a6311cf7e35cd6e4432b207297b56c Mon Sep 17 00:00:00 2001 From: zubair-arbi Date: Thu, 23 Jul 2015 18:49:10 +0500 Subject: [PATCH] update caching for eligibility email content and edx-logo ECOM-1525 --- .../credit_eligibility_email.html | 43 +++++------ .../credit_eligibility_email.txt | 14 ++-- openedx/core/djangoapps/credit/email_utils.py | 75 +++++++++++++------ .../core/djangoapps/credit/tests/test_api.py | 46 +++++++++++- 4 files changed, 128 insertions(+), 50 deletions(-) diff --git a/lms/templates/credit_notifications/credit_eligibility_email.html b/lms/templates/credit_notifications/credit_eligibility_email.html index 3d5ee38bbb..1d05b71fd1 100644 --- a/lms/templates/credit_notifications/credit_eligibility_email.html +++ b/lms/templates/credit_notifications/credit_eligibility_email.html @@ -26,33 +26,32 @@

- ${_("Hi {name},").format(name=full_name)} + ${_(u"Hi {name},").format(name=full_name)}

- ${_("Congratulations! You are eligible to receive university credit from edX partners! Click {link} to get your credit now.").format( - link=u'here'.format( - dashboard_url=dashboard_link - )) - } -

- -

- ${_("Credit from can help you get a jump start on your university degree, finish a degree already started, or fulfill requirements at a different academic institution.")} -

- -

- ${_('To get university credit for {course_name}, simply go to your {link} and click the yellow "Get Credit" button. No application, transcript, or grade report is required.').format( - course_name=course_name, - link=u'edX dashboard'.format( - dashboard_url=dashboard_link - ) + ${_(u"Congratulations! You are eligible to receive course credit for successfully completing your edX course! Click {link} to get your credit now.").format( + link=u'here'.format( + dashboard_url=dashboard_link + ) )}

- ${_("We hope you enjoyed the course, and we hope to see you in future edX courses!")}
- ${_("The edX team")} + ${_(u"Course credit can help you get a jump start on your university degree, finish a degree already started, or fulfill requirements at a different academic institution.")} +

+ +

+ ${_(u'To get course credit, simply go to your {link} and click the Get Credit button. No application, transcript, or grade report is required.').format( + link=u'edX dashboard'.format( + dashboard_url=dashboard_link + ) + )} +

+ +

+ ${_(u"We hope you enjoyed the course, and we hope to see you in future edX courses!")}
+ ${_(u"The edX team")}

@@ -62,7 +61,9 @@

- ${_("Find more edX courses you can take for university credit.")} + ${_(u"For more information on credit at edX, click {link}.").format( + link=u'here' + )}

diff --git a/lms/templates/credit_notifications/credit_eligibility_email.txt b/lms/templates/credit_notifications/credit_eligibility_email.txt index 2dde4b702f..572188e4cc 100644 --- a/lms/templates/credit_notifications/credit_eligibility_email.txt +++ b/lms/templates/credit_notifications/credit_eligibility_email.txt @@ -1,16 +1,16 @@ <%! from django.utils.translation import ugettext as _ %> -${_("Hi {name},").format(name=full_name)} +${_(u"Hi {name},").format(name=full_name)} -${_("Congratulations! You are eligible to receive university credit from edX and our partners!")} +${_(u"Congratulations! You are eligible to receive course credit for successfully completing your edX course!")} -${_("Click on the link below to get your credit now")} +${_(u"Click on the link below to get your credit now:")} ${dashboard_link} -${_("Credit from can help you get a jump start on your university degree, finish a degree already started, or fulfill requirements at a different academic institution.")} +${_(u"Course credit can help you get a jump start on your university degree, finish a degree already started, or fulfill requirements at a different academic institution.")} -${_('To get university credit for {course_name}, simply go to your edX dashboard and click the yellow "Get Credit" button. No application, transcript, or grade report is required.').format(course_name=course_name)} +${_(u'To get course credit, simply go to your edX dashboard and click the "Get Credit" button. No application, transcript, or grade report is required.')} -${_("We hope you enjoyed the course, and we hope to see you in future edX courses!")} +${_(u"We hope you enjoyed the course, and we hope to see you in future edX courses!")} -${_("The edX team")} +${_(u"The edX team")} diff --git a/openedx/core/djangoapps/credit/email_utils.py b/openedx/core/djangoapps/credit/email_utils.py index d3dd58ff38..54c6c8f8f6 100644 --- a/openedx/core/djangoapps/credit/email_utils.py +++ b/openedx/core/djangoapps/credit/email_utils.py @@ -8,6 +8,7 @@ import logging import pynliner import urlparse import uuid +import HTMLParser from django.conf import settings from django.contrib.auth.models import User @@ -23,6 +24,7 @@ from email.mime.text import MIMEText from eventtracking import tracker from edxmako.shortcuts import render_to_string +from edxmako.template import Template from microsite_configuration import microsite from xmodule.modulestore.django import modulestore @@ -42,19 +44,35 @@ def send_credit_notifications(username, course_key): course = modulestore().get_course(course_key, depth=0) course_display_name = course.display_name - branded_logo = dict(title='Logo', path=settings.NOTIFICATION_EMAIL_EDX_LOGO, cid=str(uuid.uuid4())) tracking_context = tracker.get_tracker().resolve_context() tracking_id = str(tracking_context.get('user_id')) client_id = str(tracking_context.get('client_id')) events = '&t=event&ec=email&ea=open' tracking_pixel = 'https://www.google-analytics.com/collect?v=1&tid' + tracking_id + '&cid' + client_id + events dashboard_link = _email_url_parser('dashboard') - credit_course_link = _email_url_parser('courses', "?type=credit") + credit_course_link = _email_url_parser('courses', '?type=credit') + + # get attached branded logo + logo_image = cache.get('credit.email.attached-logo') + if logo_image is None: + branded_logo = { + 'title': 'Logo', + 'path': settings.NOTIFICATION_EMAIL_EDX_LOGO, + 'cid': str(uuid.uuid4()) + } + logo_image_id = branded_logo['cid'] + logo_image = attach_image(branded_logo, 'Header Logo') + if logo_image: + cache.set('credit.email.attached-logo', logo_image, settings.CREDIT_NOTIFICATION_CACHE_TIMEOUT) + else: + # strip enclosing angle brackets from 'logo_image' cache 'Content-ID' + logo_image_id = logo_image.get('Content-ID', '')[1:-1] + context = { 'full_name': user.get_full_name(), 'platform_name': settings.PLATFORM_NAME, 'course_name': course_display_name, - 'branded_logo': branded_logo['cid'], + 'branded_logo': logo_image_id, 'dashboard_link': dashboard_link, 'credit_course_link': credit_course_link, 'tracking_pixel': tracking_pixel, @@ -67,29 +85,37 @@ def send_credit_notifications(username, course_key): msg_alternative = MIMEMultipart('alternative') notification_msg.attach(msg_alternative) # render the credit notification templates - subject = _("Course Credit Eligibility") + subject = _(u'Course Credit Eligibility') # add alternative plain text message email_body_plain = render_to_string('credit_notifications/credit_eligibility_email.txt', context) - msg_alternative.attach(MIMEText(email_body_plain, _subtype='plain')) + msg_alternative.attach(MIMEText(email_body_plain, _subtype='plain', _charset='utf-8')) # add alternative html message - email_body = cache.get('css-email-body') - if not email_body: - email_body = with_inline_css( - render_to_string("credit_notifications/credit_eligibility_email.html", context) - ) - cache.set('css-email-body', email_body, settings.CREDIT_NOTIFICATION_CACHE_TIMEOUT) + email_body_content = cache.get('credit.email.css-email-body') + if email_body_content is None: + html_file_path = file_path_finder('templates/credit_notifications/credit_eligibility_email.html') + if html_file_path: + with open(html_file_path, 'r') as cur_file: + cur_text = cur_file.read() + # use html parser to unescape html characters which are changed + # by the 'pynliner' while adding inline css to html content + html_parser = HTMLParser.HTMLParser() + email_body_content = html_parser.unescape(with_inline_css(cur_text)) + # cache the email body content before rendering it since the + # email context will change for each user e.g., 'full_name' + cache.set('credit.email.css-email-body', email_body_content, settings.CREDIT_NOTIFICATION_CACHE_TIMEOUT) + else: + email_body_content = '' - msg_alternative.attach(MIMEText(email_body, _subtype='html')) - # add images - logo_image = cache.get('attached-logo-email') - if not logo_image: - logo_image = attach_image(branded_logo, 'Header Logo') - if logo_image: - notification_msg.attach(logo_image) - cache.set('attached-logo-email', logo_image, settings.CREDIT_NOTIFICATION_CACHE_TIMEOUT) + email_body = Template(email_body_content).render([context]) + msg_alternative.attach(MIMEText(email_body, _subtype='html', _charset='utf-8')) + # attach logo image + if logo_image: + notification_msg.attach(logo_image) + + # add email addresses of sender and receiver from_address = microsite.get_value('default_from_email', settings.DEFAULT_FROM_EMAIL) to_address = user.email @@ -105,7 +131,7 @@ def with_inline_css(html_without_css): """ css_filepath = settings.NOTIFICATION_EMAIL_CSS if not css_filepath.startswith('/'): - css_filepath = finders.FileSystemFinder().find(settings.NOTIFICATION_EMAIL_CSS) + css_filepath = file_path_finder(settings.NOTIFICATION_EMAIL_CSS) if css_filepath: with open(css_filepath, "r") as _file: @@ -124,7 +150,7 @@ def attach_image(img_dict, filename): """ img_path = img_dict['path'] if not img_path.startswith('/'): - img_path = finders.FileSystemFinder().find(img_path) + img_path = file_path_finder(img_path) if img_path: with open(img_path, 'rb') as img: @@ -134,6 +160,13 @@ def attach_image(img_dict, filename): return msg_image +def file_path_finder(path): + """ + Return physical path of file if found. + """ + return finders.FileSystemFinder().find(path) + + def _email_url_parser(url_name, extra_param=None): """Parse url according to 'SITE_NAME' which will be used in the mail. diff --git a/openedx/core/djangoapps/credit/tests/test_api.py b/openedx/core/djangoapps/credit/tests/test_api.py index 1711d6fad7..7737d246c7 100644 --- a/openedx/core/djangoapps/credit/tests/test_api.py +++ b/openedx/core/djangoapps/credit/tests/test_api.py @@ -316,6 +316,9 @@ class CreditRequirementApiTests(CreditApiTestBase): self.assertEqual(req_status[0]["status"], "failed") def test_satisfy_all_requirements(self): + """ Test the credit requirements, eligibility notification, email + content caching for a credit course. + """ # Configure a course with two credit requirements self.add_credit_course() CourseFactory.create(org='edX', number='DemoX', display_name='Demo_Course') @@ -364,10 +367,51 @@ class CreditRequirementApiTests(CreditApiTestBase): # Now the user should be eligible self.assertTrue(api.is_user_eligible_for_credit("bob", self.course_key)) - # Credit eligible mail should be sent + # Credit eligibility email should be sent self.assertEqual(len(mail.outbox), 1) self.assertEqual(mail.outbox[0].subject, 'Course Credit Eligibility') + # Now verify them email content + email_payload_first = mail.outbox[0].attachments[0]._payload # pylint: disable=protected-access + + # Test that email has two payloads [multipart (plain text and html + # content), attached image] + self.assertEqual(len(email_payload_first), 2) + # pylint: disable=protected-access + self.assertIn('text/plain', email_payload_first[0]._payload[0]['Content-Type']) + # pylint: disable=protected-access + self.assertIn('text/html', email_payload_first[0]._payload[1]['Content-Type']) + self.assertIn('image/png', email_payload_first[1]['Content-Type']) + + # Now check that html email content has same logo image 'Content-ID' + # as the attached logo image 'Content-ID' + email_image = email_payload_first[1] + html_content_first = email_payload_first[0]._payload[1]._payload # pylint: disable=protected-access + + # strip enclosing angle brackets from 'logo_image' cache 'Content-ID' + image_id = email_image.get('Content-ID', '')[1:-1] + self.assertIsNotNone(image_id) + self.assertIn(image_id, html_content_first) + + # Delete the eligibility entries and satisfy the user's eligibility + # requirement again to trigger eligibility notification + CreditEligibility.objects.all().delete() + with self.assertNumQueries(12): + api.set_credit_requirement_status( + "bob", + self.course_key, + requirements[1]["namespace"], + requirements[1]["name"] + ) + + # Credit eligibility email should be sent + self.assertEqual(len(mail.outbox), 2) + # Now check that on sending eligibility notification again cached + # logo image is used + email_payload_second = mail.outbox[1].attachments[0]._payload # pylint: disable=protected-access + html_content_second = email_payload_second[0]._payload[1]._payload # pylint: disable=protected-access + self.assertIn(image_id, html_content_second) + # The user should remain eligible even if the requirement status is later changed api.set_credit_requirement_status( "bob",