Merge pull request #9012 from edx/zub/bugfix/ecom-1525-update-eligibility-email-caching

update caching for eligibility email content and edx-logo

lms/templates/credit_notifications/credit_eligibility_email.html

@@ -26,33 +26,32 @@
         <tr>
           <td class="cn-content">
             <p>
-              ${_("Hi {name},").format(name=full_name)}
+              ${_(u"Hi {name},").format(name=full_name)}
             </p>
 
             <p>
-              ${_("Congratulations! You are eligible to receive university credit from edX partners! Click {link} to get your credit now.").format(
-                    link=u'<a href="{dashboard_url}">here</a>'.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'<a href="{dashboard_url}">here</a>'.format(
+                    dashboard_url=dashboard_link
+                )
+              )}
             </p>
 
             <p>
-              ${_("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.")}
             </p>
 
             <p>
-              ${_('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'<a href="{dashboard_url}">edX dashboard</a>'.format(
-                        dashboard_url=dashboard_link
-                  )
+              ${_(u'To get course credit, simply go to your {link} and click the <b>Get Credit</b> button. No application, transcript, or grade report is required.').format(
+                link=u'<a href="{dashboard_url}">edX dashboard</a>'.format(
+                    dashboard_url=dashboard_link
+                )
               )}
             </p>
 
             <p>
-              ${_("We hope you enjoyed the course, and we hope to see you in future edX courses!")}<br/>
-              ${_("The edX team")}
+              ${_(u"We hope you enjoyed the course, and we hope to see you in future edX courses!")}<br/>
+              ${_(u"The edX team")}
             </p>
           </td>
         </tr>
@@ -62,7 +61,9 @@
         <tr>
           <td class="cn-footer-content">
             <p>
-              <a href="${credit_course_link}"> ${_("Find more edX courses you can take for university credit.")} </a>
+              ${_(u"For more information on credit at edX, click {link}.").format(
+                link=u'<a href="https://www.edx.org/gfa">here</a>'
+              )}
             </p>
           </td>
         </tr>

lms/templates/credit_notifications/credit_eligibility_email.txt

 <%! 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")}

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)
-
-    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_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 = ''
+
+    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.
 

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",