Instructors can view previously sent email content

CHANGELOG.rst

@@ -5,6 +5,8 @@ These are notable changes in edx-platform.  This is a rolling list of changes,
 in roughly chronological order, most recent first.  Add your entries at or near
 the top.  Include a label indicating the component affected.
 
+LMS: Instructors can request and see content of previous bulk emails sent in the instructor dashboard. 
+
 Studio: New advanced setting "invitation_only" for courses. This setting overrides the enrollment start/end dates
   if set. LMS-2670
 

lms/djangoapps/instructor/tests/test_api.py

@@ -8,6 +8,7 @@
 import requests
 import datetime
 import ddt
+import random
 from urllib import quote
 from django.test import TestCase
 from nose.tools import raises
@@ -31,6 +32,8 @@
 from courseware.tests.factories import StaffFactory, InstructorFactory, BetaTesterFactory
 from student.roles import CourseBetaTesterRole
 from microsite_configuration import microsite
+from util.date_utils import get_default_time_display
+from instructor.tests.utils import FakeContentTask, FakeEmail, FakeEmailInfo
 
 from student.models import CourseEnrollment, CourseEnrollmentAllowed
 from courseware.models import StudentModule
@@ -1321,7 +1324,6 @@ def assert_update_forum_role_membership(self, unique_student_identifier, rolenam
             self.assertNotIn(rolename, user_roles)
 
 
-
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
 class TestInstructorAPILevelsDataDump(ModuleStoreTestCase, LoginEnrollmentTestCase):
     """
@@ -1802,7 +1804,7 @@ def test_list_instructor_tasks_running(self, act):
         act.return_value = self.tasks
         url = reverse('list_instructor_tasks', kwargs={'course_id': self.course.id.to_deprecated_string()})
         mock_factory = MockCompletionInfo()
-        with patch('instructor.views.api.get_task_completion_info') as mock_completion_info:
+        with patch('instructor.views.instructor_task_helpers.get_task_completion_info') as mock_completion_info:
             mock_completion_info.side_effect = mock_factory.mock_get_task_completion_info
             response = self.client.get(url, {})
         self.assertEqual(response.status_code, 200)
@@ -1821,7 +1823,7 @@ def test_list_background_email_tasks(self, act):
         act.return_value = self.tasks
         url = reverse('list_background_email_tasks', kwargs={'course_id': self.course.id.to_deprecated_string()})
         mock_factory = MockCompletionInfo()
-        with patch('instructor.views.api.get_task_completion_info') as mock_completion_info:
+        with patch('instructor.views.instructor_task_helpers.get_task_completion_info') as mock_completion_info:
             mock_completion_info.side_effect = mock_factory.mock_get_task_completion_info
             response = self.client.get(url, {})
         self.assertEqual(response.status_code, 200)
@@ -1840,7 +1842,7 @@ def test_list_instructor_tasks_problem(self, act):
         act.return_value = self.tasks
         url = reverse('list_instructor_tasks', kwargs={'course_id': self.course.id.to_deprecated_string()})
         mock_factory = MockCompletionInfo()
-        with patch('instructor.views.api.get_task_completion_info') as mock_completion_info:
+        with patch('instructor.views.instructor_task_helpers.get_task_completion_info') as mock_completion_info:
             mock_completion_info.side_effect = mock_factory.mock_get_task_completion_info
             response = self.client.get(url, {
                 'problem_location_str': self.problem_urlname,
@@ -1861,7 +1863,7 @@ def test_list_instructor_tasks_problem_student(self, act):
         act.return_value = self.tasks
         url = reverse('list_instructor_tasks', kwargs={'course_id': self.course.id.to_deprecated_string()})
         mock_factory = MockCompletionInfo()
-        with patch('instructor.views.api.get_task_completion_info') as mock_completion_info:
+        with patch('instructor.views.instructor_task_helpers.get_task_completion_info') as mock_completion_info:
             mock_completion_info.side_effect = mock_factory.mock_get_task_completion_info
             response = self.client.get(url, {
                 'problem_location_str': self.problem_urlname,
@@ -1879,6 +1881,104 @@ def test_list_instructor_tasks_problem_student(self, act):
         self.assertEqual(actual_tasks, expected_tasks)
 
 
+@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
+@patch.object(instructor_task.api, 'get_instructor_task_history')
+class TestInstructorEmailContentList(ModuleStoreTestCase, LoginEnrollmentTestCase):
+    """
+    Test the instructor email content history endpoint.
+    """
+    def setUp(self):
+        self.course = CourseFactory.create()
+        self.instructor = InstructorFactory(course_key=self.course.id)
+        self.client.login(username=self.instructor.username, password='test')
+
+    def tearDown(self):
+        """
+        Undo all patches.
+        """
+        patch.stopall()
+
+    def setup_fake_email_info(self, num_emails):
+        """ Initialize the specified number of fake emails """
+        self.tasks = {}
+        self.emails = {}
+        self.emails_info = {}
+        for email_id in range(num_emails):
+            num_sent = random.randint(1, 15401)
+            self.tasks[email_id] = FakeContentTask(email_id, num_sent, 'expected')
+            self.emails[email_id] = FakeEmail(email_id)
+            self.emails_info[email_id] = FakeEmailInfo(self.emails[email_id], num_sent)
+
+    def get_matching_mock_email(self, *args, **kwargs):
+        """ Returns the matching mock emails for the given id """
+        email_id = kwargs.get('id', 0)
+        return self.emails[email_id]
+
+    def get_email_content_response(self, num_emails, task_history_request):
+        """ Calls the list_email_content endpoint and returns the repsonse """
+        self.setup_fake_email_info(num_emails)
+        task_history_request.return_value = self.tasks.values()
+        url = reverse('list_email_content', kwargs={'course_id': self.course.id.to_deprecated_string()})
+        with patch('instructor.views.api.CourseEmail.objects.get') as mock_email_info:
+            mock_email_info.side_effect = self.get_matching_mock_email
+            response = self.client.get(url, {})
+        self.assertEqual(response.status_code, 200)
+        return response
+
+    def test_content_list_one_email(self, task_history_request):
+        """ Test listing of bulk emails when email list has one email """
+        response = self.get_email_content_response(1, task_history_request)
+        self.assertTrue(task_history_request.called)
+        email_info = json.loads(response.content)['emails']
+
+        # Emails list should have one email
+        self.assertEqual(len(email_info), 1)
+
+        # Email content should be what's expected
+        expected_message = self.emails[0].html_message
+        returned_email_info = email_info[0]
+        received_message = returned_email_info[u'email'][u'html_message']
+        self.assertEqual(expected_message, received_message)
+
+    def test_content_list_no_emails(self, task_history_request):
+        """ Test listing of bulk emails when email list empty """
+        response = self.get_email_content_response(0, task_history_request)
+        self.assertTrue(task_history_request.called)
+        email_info = json.loads(response.content)['emails']
+
+        # Emails list should be empty
+        self.assertEqual(len(email_info), 0)
+
+    def test_content_list_email_content_many(self, task_history_request):
+        """ Test listing of bulk emails sent large amount of emails """
+        response = self.get_email_content_response(50, task_history_request)
+        self.assertTrue(task_history_request.called)
+        expected_email_info = [email_info.to_dict() for email_info in self.emails_info.values()]
+        actual_email_info = json.loads(response.content)['emails']
+
+        self.assertEqual(len(actual_email_info), 50)
+        for exp_email, act_email in zip(expected_email_info, actual_email_info):
+            self.assertDictEqual(exp_email, act_email)
+
+        self.assertEqual(actual_email_info, expected_email_info)
+
+    def test_list_email_content_error(self, task_history_request):
+        """ Test handling of error retrieving email """
+        self.invalid_task = FakeContentTask(0, 0, 'test')
+        self.invalid_task.make_invalid_input()
+        task_history_request.return_value = [self.invalid_task]
+        url = reverse('list_email_content', kwargs={'course_id': self.course.id.to_deprecated_string()})
+        response = self.client.get(url, {})
+        self.assertEqual(response.status_code, 200)
+
+        self.assertTrue(task_history_request.called)
+        returned_email_info = json.loads(response.content)['emails']
+        self.assertEqual(len(returned_email_info), 1)
+        returned_info = returned_email_info[0]
+        for info in ['created', 'sent_to', 'email', 'number_sent']:
+            self.assertEqual(returned_info[info], None)
+
+
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
 @override_settings(ANALYTICS_SERVER_URL="http://robotanalyticsserver.netbot:900/")
 @override_settings(ANALYTICS_API_KEY="robot_api_key")

lms/djangoapps/instructor/tests/utils.py

@@ -0,0 +1,84 @@
+"""
+Utilities for instructor unit tests
+"""
+import datetime
+import json
+import random
+from django.utils.timezone import utc
+from util.date_utils import get_default_time_display
+
+
+class FakeInfo(object):
+    """Parent class for faking objects used in tests"""
+    FEATURES = []
+
+    def __init__(self):
+        for feature in self.FEATURES:
+            setattr(self, feature, u'expected')
+
+    def to_dict(self):
+        """ Returns a dict representation of the object """
+        return {key: getattr(self, key) for key in self.FEATURES}
+
+
+class FakeContentTask(FakeInfo):
+    """ Fake task info needed for email content list """
+    FEATURES = [
+        'task_input',
+        'task_output',
+    ]
+
+    def __init__(self, email_id, num_sent, sent_to):
+        super(FakeContentTask, self).__init__()
+        self.task_input = {'email_id': email_id, 'to_option': sent_to}
+        self.task_input = json.dumps(self.task_input)
+        self.task_output = {'total': num_sent}
+        self.task_output = json.dumps(self.task_output)
+
+    def make_invalid_input(self):
+        """Corrupt the task input field to test errors"""
+        self.task_input = "THIS IS INVALID JSON"
+
+
+class FakeEmail(FakeInfo):
+    """ Corresponding fake email for a fake task """
+    FEATURES = [
+        'subject',
+        'html_message',
+        'id',
+        'created',
+    ]
+
+    def __init__(self, email_id):
+        super(FakeEmail, self).__init__()
+        self.id = unicode(email_id)
+        # Select a random data for create field
+        year = random.choice(range(1950, 2000))
+        month = random.choice(range(1, 12))
+        day = random.choice(range(1, 28))
+        hour = random.choice(range(0, 23))
+        minute = random.choice(range(0, 59))
+        self.created = datetime.datetime(year, month, day, hour, minute, tzinfo=utc)
+
+
+class FakeEmailInfo(FakeInfo):
+    """ Fake email information object """
+    FEATURES = [
+        u'created',
+        u'sent_to',
+        u'email',
+        u'number_sent'
+    ]
+
+    EMAIL_FEATURES = [
+        u'subject',
+        u'html_message',
+        u'id'
+    ]
+
+    def __init__(self, fake_email, num_sent):
+        super(FakeEmailInfo, self).__init__()
+        self.created = get_default_time_display(fake_email.created)
+        self.number_sent = num_sent
+        fake_email_dict = fake_email.to_dict()
+        self.email = {feature: fake_email_dict[feature] for feature in self.EMAIL_FEATURES}

lms/djangoapps/instructor/views/api.py

@@ -20,6 +20,8 @@
 from django.http import HttpResponse, HttpResponseBadRequest, HttpResponseForbidden
 from django.utils.html import strip_tags
 from util.json_request import JsonResponse
+from util.date_utils import get_default_time_display
+from instructor.views.instructor_task_helpers import extract_email_features, extract_task_features
 
 from courseware.access import has_access
 from courseware.courses import get_course_with_access, get_course_by_id
@@ -36,7 +38,6 @@
 from student.models import CourseEnrollment, unique_id_for_user, anonymous_id_for_user
 import instructor_task.api
 from instructor_task.api_helper import AlreadyRunningError
-from instructor_task.views import get_task_completion_info
 from instructor_task.models import ReportStore
 import instructor.enrollment as enrollment
 from instructor.enrollment import (
@@ -52,7 +53,7 @@
 import instructor_analytics.csvs
 import csv
 
-from submissions import api as sub_api # installed from the edx-submissions repository
+from submissions import api as sub_api  # installed from the edx-submissions repository
 
 from bulk_email.models import CourseEmail
 
@@ -610,9 +611,10 @@ def get_anon_ids(request, course_id):  # pylint: disable=W0613
     Respond with 2-column CSV output of user-id, anonymized-user-id
     """
     # TODO: the User.objects query and CSV generation here could be
-    # centralized into instructor_analytics. Currently instructor_analytics 
+    # centralized into instructor_analytics. Currently instructor_analytics
     # has similar functionality but not quite what's needed.
     course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id)
+
     def csv_response(filename, header, rows):
         """Returns a CSV http response for the given header and rows (excel/utf-8)."""
         response = HttpResponse(mimetype='text/csv')
@@ -850,45 +852,6 @@ def rescore_problem(request, course_id):
     return JsonResponse(response_payload)
 
 
-def extract_task_features(task):
-    """
-    Convert task to dict for json rendering.
-    Expects tasks have the following features:
-    * task_type (str, type of task)
-    * task_input (dict, input(s) to the task)
-    * task_id (str, celery id of the task)
-    * requester (str, username who submitted the task)
-    * task_state (str, state of task eg PROGRESS, COMPLETED)
-    * created (datetime, when the task was completed)
-    * task_output (optional)
-    """
-    # Pull out information from the task
-    features = ['task_type', 'task_input', 'task_id', 'requester', 'task_state']
-    task_feature_dict = {feature: str(getattr(task, feature)) for feature in features}
-    # Some information (created, duration, status, task message) require additional formatting
-    task_feature_dict['created'] = task.created.isoformat()
-
-    # Get duration info, if known
-    duration_sec = 'unknown'
-    if hasattr(task, 'task_output') and task.task_output is not None:
-        try:
-            task_output = json.loads(task.task_output)
-        except ValueError:
-            log.error("Could not parse task output as valid json; task output: %s", task.task_output)
-        else:
-            if 'duration_ms' in task_output:
-                duration_sec = int(task_output['duration_ms'] / 1000.0)
-    task_feature_dict['duration_sec'] = duration_sec
-
-    # Get progress status message & success information
-    success, task_message = get_task_completion_info(task)
-    status = _("Complete") if success else _("Incomplete")
-    task_feature_dict['status'] = status
-    task_feature_dict['task_message'] = task_message
-
-    return task_feature_dict
-
-
 @ensure_csrf_cookie
 @cache_control(no_cache=True, no_store=True, must_revalidate=True)
 @require_level('staff')
@@ -907,6 +870,24 @@ def list_background_email_tasks(request, course_id):  # pylint: disable=unused-a
     return JsonResponse(response_payload)
 
 
+@ensure_csrf_cookie
+@cache_control(no_cache=True, no_store=True, must_revalidate=True)
+@require_level('staff')
+def list_email_content(requests, course_id):
+    """
+    List the content of bulk emails sent
+    """
+    course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id)
+    task_type = 'bulk_course_email'
+    # First get tasks list of bulk emails sent
+    emails = instructor_task.api.get_instructor_task_history(course_id, task_type=task_type)
+
+    response_payload = {
+        'emails': map(extract_email_features, emails),
+    }
+    return JsonResponse(response_payload)
+
+
 @ensure_csrf_cookie
 @cache_control(no_cache=True, no_store=True, must_revalidate=True)
 @require_level('staff')

lms/djangoapps/instructor/views/api_urls.py

@@ -31,6 +31,8 @@
         'instructor.views.api.list_instructor_tasks', name="list_instructor_tasks"),
     url(r'^list_background_email_tasks$',
         'instructor.views.api.list_background_email_tasks', name="list_background_email_tasks"),
+    url(r'^list_email_content$',
+        'instructor.views.api.list_email_content', name="list_email_content"),
     url(r'^list_forum_members$',
         'instructor.views.api.list_forum_members', name="list_forum_members"),
     url(r'^update_forum_role_membership$',

lms/djangoapps/instructor/views/instructor_dashboard.py

@@ -92,7 +92,6 @@ def instructor_dashboard_2(request, course_id):
     if course_mode_has_price:
         sections.append(_section_e_commerce(course_key, access))
 
-
     studio_url = None
     if is_studio_course:
         studio_url = get_cms_course_link(course)
@@ -278,6 +277,9 @@ def _section_send_email(course_key, access, course):
         'email_background_tasks_url': reverse(
             'list_background_email_tasks', kwargs={'course_id': course_key.to_deprecated_string()}
         ),
+        'email_content_history_url': reverse(
+            'list_email_content', kwargs={'course_id': course_key.to_deprecated_string()}
+        ),
     }
     return section_data