Merge pull request #333 from edx/andya/update-email

Improve learner email messages

edx_proctoring/api.py

@@ -13,6 +13,7 @@ import pytz
 
 from django.utils.translation import ugettext as _, ugettext_noop
 from django.conf import settings
+from django.contrib.auth.models import User
 from django.template import Context, loader
 from django.core.urlresolvers import reverse, NoReverseMatch
 from django.core.mail.message import EmailMessage
@@ -885,38 +886,31 @@ def update_attempt_status(exam_id, user_id, to_status,
                 cascade_effects=False
             )
 
-    # email will be send when the exam is proctored and not practice exam
-    # and the status is verified, submitted or rejected
-    should_send_status_email = (
-        exam_attempt_obj.taking_as_proctored and
-        not exam_attempt_obj.is_sample_attempt and
-        ProctoredExamStudentAttemptStatus.needs_status_change_email(exam_attempt_obj.status)
+    # call service to get course name.
+    credit_service = get_runtime_service('credit')
+    credit_state = credit_service.get_credit_state(
+        exam_attempt_obj.user_id,
+        exam_attempt_obj.proctored_exam.course_id,
+        return_course_info=True
     )
-    if should_send_status_email:
-        # trigger credit workflow, as needed
-        credit_service = get_runtime_service('credit')
 
-        # call service to get course name.
-        credit_state = credit_service.get_credit_state(
+    default_name = _('your course')
+    if credit_state:
+        course_name = credit_state.get('course_name', default_name)
+    else:
+        course_name = default_name
+        log.info(
+            "Could not find credit_state for user id %r in the course %r.",
             exam_attempt_obj.user_id,
-            exam_attempt_obj.proctored_exam.course_id,
-            return_course_info=True
-        )
-
-        default_name = _('your course')
-        if credit_state:
-            course_name = credit_state.get('course_name', default_name)
-        else:
-            course_name = default_name
-            log.info(
-                "Could not find credit_state for user id %r in the course %r.",
-                exam_attempt_obj.user_id,
-                exam_attempt_obj.proctored_exam.course_id
-            )
-        send_proctoring_attempt_status_email(
-            exam_attempt_obj,
-            course_name
+            exam_attempt_obj.proctored_exam.course_id
         )
+    email = create_proctoring_attempt_status_email(
+        user_id,
+        exam_attempt_obj,
+        course_name
+    )
+    if email:
+        email.send()
 
     # emit an anlytics event based on the state transition
     # we re-read this from the database in case fields got updated
@@ -929,20 +923,46 @@ def update_attempt_status(exam_id, user_id, to_status,
     return attempt['id']
 
 
-def send_proctoring_attempt_status_email(exam_attempt_obj, course_name):
+def create_proctoring_attempt_status_email(user_id, exam_attempt_obj, course_name):
     """
-    Sends an email about change in proctoring attempt status.
+    Creates an email about change in proctoring attempt status.
     """
+    # Don't send an email unless this is a non-practice proctored exam
+    if not exam_attempt_obj.taking_as_proctored or exam_attempt_obj.is_sample_attempt:
+        return None
 
+    user = User.objects.get(id=user_id)
     course_info_url = ''
-    email_template = loader.get_template('emails/proctoring_attempt_status_email.html')
+    email_subject = (
+        _('Proctoring Results For {course_name} {exam_name}').format(
+            course_name=course_name,
+            exam_name=exam_attempt_obj.proctored_exam.exam_name
+        )
+    )
+    status = exam_attempt_obj.status
+    if status == ProctoredExamStudentAttemptStatus.submitted:
+        email_template_path = 'emails/proctoring_attempt_submitted_email.html'
+        email_subject = (
+            _('Proctoring Review In Progress For {course_name} {exam_name}').format(
+                course_name=course_name,
+                exam_name=exam_attempt_obj.proctored_exam.exam_name
+            )
+        )
+    elif status == ProctoredExamStudentAttemptStatus.verified:
+        email_template_path = 'emails/proctoring_attempt_satisfactory_email.html'
+    elif status == ProctoredExamStudentAttemptStatus.rejected:
+        email_template_path = 'emails/proctoring_attempt_unsatisfactory_email.html'
+    else:
+        # Don't send an email for any other attempt status codes
+        return None
+    email_template = loader.get_template(email_template_path)
     try:
         course_info_url = reverse(
             'courseware.views.views.course_info',
             args=[exam_attempt_obj.proctored_exam.course_id]
         )
     except NoReverseMatch:
-        log.exception("Can't find Course Info url for course %s", exam_attempt_obj.proctored_exam.course_id)
+        log.exception("Can't find course info url for course %s", exam_attempt_obj.proctored_exam.course_id)
 
     scheme = 'https' if getattr(settings, 'HTTPS', 'on') == 'on' else 'http'
     course_url = '{scheme}://{site_name}{course_info_url}'.format(
@@ -950,33 +970,34 @@ def send_proctoring_attempt_status_email(exam_attempt_obj, course_name):
         site_name=constants.SITE_NAME,
         course_info_url=course_info_url
     )
+    exam_name = exam_attempt_obj.proctored_exam.exam_name
+    support_email_subject = _('Proctored exam {exam_name} in {course_name} for user {username}').format(
+        exam_name=exam_name,
+        course_name=course_name,
+        username=user.username,
+    )
 
     body = email_template.render(
         Context({
+            'username': user.username,
             'course_url': course_url,
             'course_name': course_name,
-            'exam_name': exam_attempt_obj.proctored_exam.exam_name,
-            'status': ProctoredExamStudentAttemptStatus.get_status_alias(exam_attempt_obj.status),
+            'exam_name': exam_name,
+            'status': status,
             'platform': constants.PLATFORM_NAME,
             'contact_email': constants.CONTACT_EMAIL,
+            'support_email_subject': support_email_subject,
         })
     )
 
-    subject = (
-        _('Proctoring Session Results Update for {course_name} {exam_name}').format(
-            course_name=course_name,
-            exam_name=exam_attempt_obj.proctored_exam.exam_name
-        )
-    )
-
     email = EmailMessage(
         body=body,
         from_email=constants.FROM_EMAIL,
         to=[exam_attempt_obj.user.email],
-        subject=subject
+        subject=email_subject,
     )
-    email.content_subtype = "html"
-    email.send()
+    email.content_subtype = 'html'
+    return email
 
 
 def remove_exam_attempt(attempt_id, requesting_user):

edx_proctoring/models.py

@@ -224,16 +224,6 @@ class ProctoredExamStudentAttemptStatus(object):
         ]
 
     @classmethod
-    def needs_status_change_email(cls, to_status):
-        """
-        We need to send out emails for rejected, verified and submitted statuses.
-        """
-
-        return to_status in [
-            cls.rejected, cls.submitted, cls.verified
-        ]
-
-    @classmethod
     def get_status_alias(cls, status):
         """
         Returns status alias used in email

edx_proctoring/templates/emails/proctoring_attempt_satisfactory_email.html

+{% load i18n %}
+
+<p>
+    {% blocktrans %}
+        Hi {{ username }},
+    {% endblocktrans %}
+</p>
+<p>
+    {% blocktrans %}
+        Your proctored exam "{{ exam_name }}" in
+        <a href="{{ course_url }}">{{ course_name }}</a> was reviewed and you
+        met all exam requirements. You can view your grade on the course
+        progress page.
+    {% endblocktrans %}
+</p>
+<p>
+    {% blocktrans %}
+        If you have any questions about your results, contact {{ platform }}
+        support at
+        <a href="mailto:{{ contact_email }}?Subject={{ support_email_subject }}">
+            {{ contact_email }}
+        </a>.
+    {% endblocktrans %}
+</p>

edx_proctoring/templates/emails/proctoring_attempt_status_email.html

-{% load i18n %}
-
-{% blocktrans %}
-
-This email is to let you know that the status of your proctoring session review for {{ exam_name }} in
-<a href="{{ course_url }}">{{ course_name }} </a> is {{ status }}. If you have any questions about proctoring,
-contact {{ platform }} support at {{ contact_email }}.
-
-{% endblocktrans %}
\ No newline at end of file

edx_proctoring/templates/emails/proctoring_attempt_submitted_email.html

+{% load i18n %}
+
+<p>
+    {% blocktrans %}
+        Hi {{ username }},
+    {% endblocktrans %}
+</p>
+<p>
+    {% blocktrans %}
+        Your proctored exam "{{ exam_name }}" in
+        <a href="{{ course_url }}">{{ course_name }}</a> was submitted
+        successfully and will now be reviewed to ensure all proctoring exam
+        rules were followed. You should receive an email with your updated exam
+        status within 5 business days.
+    {% endblocktrans %}
+</p>
+<p>
+    {% blocktrans %}
+        If you have any questions about proctoring, contact {{ platform }}
+        support at
+        <a href="mailto:{{ contact_email }}?Subject={{ support_email_subject }}">
+            {{ contact_email }}
+        </a>.
+    {% endblocktrans %}
+</p>

edx_proctoring/templates/emails/proctoring_attempt_unsatisfactory_email.html

+{% load i18n %}
+
+<p>
+    {% blocktrans %}
+        Hi {{ username }},
+    {% endblocktrans %}
+</p>
+<p>
+    {% blocktrans %}
+        Your proctored exam "{{ exam_name }}" in
+        <a href="{{ course_url }}">{{ course_name }}</a> was reviewed and the
+        team found one or more violations of the proctored exam rules. Examples
+        of behaviors that may result in a rules violation include browsing
+        the internet, using a phone, or getting help from another person. As a
+        result of the violation(s), you did not successfully meet the proctored
+        exam requirements.
+    {% endblocktrans %}
+</p>
+<p>
+    {% blocktrans %}
+        If you have any questions about your results, contact {{ platform }}
+        support at
+        <a href="mailto:{{ contact_email }}?Subject={{ support_email_subject }}">
+            {{ contact_email }}
+        </a>.
+    {% endblocktrans %}
+</p>

edx_proctoring/tests/test_email.py

@@ -31,22 +31,25 @@ class ProctoredExamEmailTests(ProctoredExamTestCase):
     All tests for proctored exam emails.
     """
 
-    def setUp(self):
-        """
-        Build out test harnessing
-        """
-        super(ProctoredExamEmailTests, self).setUp()
-
-        # Messages for get_student_view
-        self.proctored_exam_email_subject = 'Proctoring Session Results Update'
-        self.proctored_exam_email_body = 'the status of your proctoring session review'
-
     @ddt.data(
-        ProctoredExamStudentAttemptStatus.submitted,
-        ProctoredExamStudentAttemptStatus.verified,
-        ProctoredExamStudentAttemptStatus.rejected
+        [
+            ProctoredExamStudentAttemptStatus.submitted,
+            'Proctoring Review In Progress',
+            'was submitted successfully',
+        ],
+        [
+            ProctoredExamStudentAttemptStatus.verified,
+            'Proctoring Results',
+            'was reviewed and you met all exam requirements',
+        ],
+        [
+            ProctoredExamStudentAttemptStatus.rejected,
+            'Proctoring Results',
+            'the team found one or more violations',
+        ]
     )
-    def test_send_email(self, status):
+    @ddt.unpack
+    def test_send_email(self, status, expected_subject, expected_message_string):
         """
         Assert that email is sent on the following statuses of proctoring attempt.
         """
@@ -59,27 +62,18 @@ class ProctoredExamEmailTests(ProctoredExamTestCase):
             status
         )
         self.assertEquals(len(mail.outbox), 1)
-        self.assertIn(self.proctored_exam_email_subject, mail.outbox[0].subject)
-        self.assertIn(self.proctored_exam_email_body, mail.outbox[0].body)
-        self.assertIn(ProctoredExamStudentAttemptStatus.get_status_alias(status), mail.outbox[0].body)
-        self.assertIn(credit_state['course_name'], mail.outbox[0].body)
 
-    @ddt.data(
-        ProctoredExamStudentAttemptStatus.second_review_required,
-        ProctoredExamStudentAttemptStatus.error
-    )
-    def test_email_not_sent(self, status):
-        """
-        Assert than email is not sent on the following statuses of proctoring attempt
-        """
+        # Verify the subject
+        actual_subject = self._normalize_whitespace(mail.outbox[0].subject)
+        self.assertIn(expected_subject, actual_subject)
+        self.assertIn(self.exam_name, actual_subject)
 
-        exam_attempt = self._create_started_exam_attempt()
-        update_attempt_status(
-            exam_attempt.proctored_exam_id,
-            self.user.id,
-            status
-        )
-        self.assertEquals(len(mail.outbox), 0)
+        # Verify the body
+        actual_body = self._normalize_whitespace(mail.outbox[0].body)
+        self.assertIn('Hi tester,', actual_body)
+        self.assertIn('Your proctored exam "Test Exam"', actual_body)
+        self.assertIn(credit_state['course_name'], actual_body)
+        self.assertIn(expected_message_string, actual_body)
 
     def test_send_email_unicode(self):
         """
@@ -97,16 +91,16 @@ class ProctoredExamEmailTests(ProctoredExamTestCase):
             ProctoredExamStudentAttemptStatus.submitted
         )
         self.assertEquals(len(mail.outbox), 1)
-        self.assertIn(self.proctored_exam_email_subject, mail.outbox[0].subject)
-        self.assertIn(course_name, mail.outbox[0].subject)
-        self.assertIn(self.proctored_exam_email_body, mail.outbox[0].body)
-        self.assertIn(
-            ProctoredExamStudentAttemptStatus.get_status_alias(
-                ProctoredExamStudentAttemptStatus.submitted
-            ),
-            mail.outbox[0].body
-        )
-        self.assertIn(credit_state['course_name'], mail.outbox[0].body)
+
+        # Verify the subject
+        actual_subject = self._normalize_whitespace(mail.outbox[0].subject)
+        self.assertIn('Proctoring Review In Progress', actual_subject)
+        self.assertIn(course_name, actual_subject)
+
+        # Verify the body
+        actual_body = self._normalize_whitespace(mail.outbox[0].body)
+        self.assertIn('was submitted successfully', actual_body)
+        self.assertIn(credit_state['course_name'], actual_body)
 
     @ddt.data(
         ProctoredExamStudentAttemptStatus.eligible,
@@ -117,12 +111,13 @@ class ProctoredExamEmailTests(ProctoredExamTestCase):
         ProctoredExamStudentAttemptStatus.ready_to_submit,
         ProctoredExamStudentAttemptStatus.declined,
         ProctoredExamStudentAttemptStatus.timed_out,
-        ProctoredExamStudentAttemptStatus.error
+        ProctoredExamStudentAttemptStatus.second_review_required,
+        ProctoredExamStudentAttemptStatus.error,
     )
     @patch.dict('django.conf.settings.PROCTORING_SETTINGS', {'ALLOW_TIMED_OUT_STATE': True})
-    def test_not_send_email(self, status):
+    def test_email_not_sent(self, status):
         """
-        Assert that email is not sent on the following statuses of proctoring attempt.
+        Assert that an email is not sent for the following attempt status codes.
         """
 
         exam_attempt = self._create_started_exam_attempt()

edx_proctoring/tests/utils.py

@@ -341,3 +341,10 @@ class ProctoredExamTestCase(LoggedInTestCase):
             status=ProctoredExamStudentAttemptStatus.started,
             allowed_time_limit_mins=10
         )
+
+    @staticmethod
+    def _normalize_whitespace(string):
+        """
+        Replaces newlines and multiple spaces with a single space.
+        """
+        return ' '.join(string.replace('\n', '').split())