diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index bacaeb9e4a..59227e9825 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -1980,22 +1980,27 @@ class TestInstructorEmailContentList(ModuleStoreTestCase, LoginEnrollmentTestCas """ patch.stopall() - def setup_fake_email_info(self, num_emails): + def setup_fake_email_info(self, num_emails, with_failures=False): """ Initialize the specified number of fake emails """ for email_id in range(num_emails): num_sent = random.randint(1, 15401) - self.tasks[email_id] = FakeContentTask(email_id, num_sent, 'expected') + if with_failures: + failed = random.randint(1, 15401) + else: + failed = 0 + + self.tasks[email_id] = FakeContentTask(email_id, num_sent, failed, 'expected') self.emails[email_id] = FakeEmail(email_id) - self.emails_info[email_id] = FakeEmailInfo(self.emails[email_id], num_sent) + self.emails_info[email_id] = FakeEmailInfo(self.emails[email_id], num_sent, failed) def get_matching_mock_email(self, **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): + def get_email_content_response(self, num_emails, task_history_request, with_failures=False): """ Calls the list_email_content endpoint and returns the repsonse """ - self.setup_fake_email_info(num_emails) + self.setup_fake_email_info(num_emails, with_failures) 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: @@ -2004,6 +2009,19 @@ class TestInstructorEmailContentList(ModuleStoreTestCase, LoginEnrollmentTestCas self.assertEqual(response.status_code, 200) return response + def check_emails_sent(self, num_emails, task_history_request, with_failures=False): + """ Tests sending emails with or without failures """ + response = self.get_email_content_response(num_emails, task_history_request, with_failures) + 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), num_emails) + for exp_email, act_email in zip(expected_email_info, actual_email_info): + self.assertDictEqual(exp_email, act_email) + + self.assertEqual(expected_email_info, actual_email_info) + 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) @@ -2030,20 +2048,11 @@ class TestInstructorEmailContentList(ModuleStoreTestCase, LoginEnrollmentTestCas 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) + self.check_emails_sent(50, task_history_request) def test_list_email_content_error(self, task_history_request): """ Test handling of error retrieving email """ - invalid_task = FakeContentTask(0, 0, 'test') + invalid_task = FakeContentTask(0, 0, 0, 'test') invalid_task.make_invalid_input() task_history_request.return_value = [invalid_task] url = reverse('list_email_content', kwargs={'course_id': self.course.id.to_deprecated_string()}) @@ -2054,9 +2063,36 @@ class TestInstructorEmailContentList(ModuleStoreTestCase, LoginEnrollmentTestCas 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']: + for info in ['created', 'sent_to', 'email', 'number_sent', 'requester']: self.assertEqual(returned_info[info], None) + def test_list_email_with_failure(self, task_history_request): + """ Test the handling of email task that had failures """ + self.check_emails_sent(1, task_history_request, True) + + def test_list_many_emails_with_failures(self, task_history_request): + """ Test the handling of many emails with failures """ + self.check_emails_sent(50, task_history_request, True) + + def test_list_email_with_no_successes(self, task_history_request): + task_info = FakeContentTask(0, 0, 10, 'expected') + email = FakeEmail(0) + email_info = FakeEmailInfo(email, 0, 10) + task_history_request.return_value = [task_info] + 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.return_value = email + response = self.client.get(url, {}) + self.assertEqual(response.status_code, 200) + + self.assertTrue(task_history_request.called) + returned_info_list = json.loads(response.content)['emails'] + + self.assertEqual(len(returned_info_list), 1) + returned_info = returned_info_list[0] + expected_info = email_info.to_dict() + self.assertDictEqual(expected_info, returned_info) + @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) @override_settings(ANALYTICS_SERVER_URL="http://robotanalyticsserver.netbot:900/") diff --git a/lms/djangoapps/instructor/tests/utils.py b/lms/djangoapps/instructor/tests/utils.py index 884fb95ba1..b2444fe1b1 100644 --- a/lms/djangoapps/instructor/tests/utils.py +++ b/lms/djangoapps/instructor/tests/utils.py @@ -26,14 +26,16 @@ class FakeContentTask(FakeInfo): FEATURES = [ 'task_input', 'task_output', + 'requester', ] - def __init__(self, email_id, num_sent, sent_to): + def __init__(self, email_id, num_sent, num_failed, 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 = {'succeeded': num_sent, 'failed': num_failed} self.task_output = json.dumps(self.task_output) + self.requester = 'expected' def make_invalid_input(self): """Corrupt the task input field to test errors""" @@ -67,7 +69,8 @@ class FakeEmailInfo(FakeInfo): u'created', u'sent_to', u'email', - u'number_sent' + u'number_sent', + u'requester', ] EMAIL_FEATURES = [ @@ -76,9 +79,15 @@ class FakeEmailInfo(FakeInfo): u'id' ] - def __init__(self, fake_email, num_sent): + def __init__(self, fake_email, num_sent, num_failed): super(FakeEmailInfo, self).__init__() self.created = get_default_time_display(fake_email.created) - self.number_sent = num_sent + + number_sent = str(num_sent) + ' sent' + if num_failed > 0: + number_sent += ', ' + str(num_failed) + " failed" + + self.number_sent = number_sent fake_email_dict = fake_email.to_dict() self.email = {feature: fake_email_dict[feature] for feature in self.EMAIL_FEATURES} + self.requester = u'expected' diff --git a/lms/djangoapps/instructor/views/instructor_task_helpers.py b/lms/djangoapps/instructor/views/instructor_task_helpers.py index 4c89e6addd..11d0e5e399 100644 --- a/lms/djangoapps/instructor/views/instructor_task_helpers.py +++ b/lms/djangoapps/instructor/views/instructor_task_helpers.py @@ -7,6 +7,7 @@ import logging from util.date_utils import get_default_time_display from bulk_email.models import CourseEmail from django.utils.translation import ugettext as _ +from django.utils.translation import ungettext from instructor_task.views import get_task_completion_info log = logging.getLogger(__name__) @@ -21,7 +22,8 @@ def email_error_information(): 'created', 'sent_to', 'email', - 'number_sent' + 'number_sent', + 'requester', ] return {info: None for info in expected_info} @@ -33,6 +35,7 @@ def extract_email_features(email_task): Expects that the given task has the following attributes: * task_input (dict containing email_id and to_option) * task_output (optional, dict containing total emails sent) + * requester, the user who executed the task With this information, gets the corresponding email object from the bulk emails table, and loads up a dict containing the following: @@ -40,6 +43,7 @@ def extract_email_features(email_task): * sent_to, the group the email was delivered to * email, dict containing the subject, id, and html_message of an email * number_sent, int number of emails sent + * requester, the user who sent the emails If task_input cannot be loaded, then the email cannot be loaded and None is returned for these fields. """ @@ -51,26 +55,43 @@ def extract_email_features(email_task): return email_error_information() email = CourseEmail.objects.get(id=task_input_information['email_id']) - - creation_time = get_default_time_display(email.created) - email_feature_dict = {'created': creation_time, 'sent_to': task_input_information['to_option']} + email_feature_dict = { + 'created': get_default_time_display(email.created), + 'sent_to': task_input_information['to_option'], + 'requester': str(getattr(email_task, 'requester')), + } features = ['subject', 'html_message', 'id'] email_info = {feature: unicode(getattr(email, feature)) for feature in features} # Pass along email as an object with the information we desire email_feature_dict['email'] = email_info - number_sent = None + # Translators: number sent refers to the number of emails sent + number_sent = _('0 sent') if hasattr(email_task, 'task_output') and email_task.task_output is not None: try: task_output = json.loads(email_task.task_output) except ValueError: log.error("Could not parse task output as valid json; task output: %s", email_task.task_output) else: - if 'total' in task_output: - number_sent = int(task_output['total']) - email_feature_dict['number_sent'] = number_sent + if 'succeeded' in task_output and task_output['succeeded'] > 0: + num_emails = task_output['succeeded'] + number_sent = ungettext( + "{num_emails} sent", + "{num_emails} sent", + num_emails + ).format(num_emails=num_emails) + if 'failed' in task_output and task_output['failed'] > 0: + num_emails = task_output['failed'] + number_sent += ", " + number_sent += ungettext( + "{num_emails} failed", + "{num_emails} failed", + num_emails + ).format(num_emails=num_emails) + + email_feature_dict['number_sent'] = number_sent return email_feature_dict diff --git a/lms/static/coffee/src/instructor_dashboard/util.coffee b/lms/static/coffee/src/instructor_dashboard/util.coffee index 1010474651..a9ace5f643 100644 --- a/lms/static/coffee/src/instructor_dashboard/util.coffee +++ b/lms/static/coffee/src/instructor_dashboard/util.coffee @@ -121,17 +121,21 @@ create_task_list_table = ($table_tasks, tasks_data) -> # Formats the subject field for email content history table subject_formatter = (row, cell, value, columnDef, dataContext) -> - if !value then return gettext("An error occurred retrieving your email. Please try again later, and contact technical support if the problem persists.") + if value is null then return gettext("An error occurred retrieving your email. Please try again later, and contact technical support if the problem persists.") subject_text = $('').text(value['subject']).html() return '

' + subject_text + '

' +# Formats the author field for the email content history table +sent_by_formatter = (row, cell, value, columnDef, dataContext) -> + if value is null then return "

" + gettext("Unknown") + "

" else return '

' + value + '

' + # Formats the created field for the email content history table created_formatter = (row, cell, value, columnDef, dataContext) -> - if !value then return "

" + gettext("Unknown") + "

" else return '

' + value + '

' + if value is null then return "

" + gettext("Unknown") + "

" else return '

' + value + '

' # Formats the number sent field for the email content history table number_sent_formatter = (row, cell, value, columndDef, dataContext) -> - if !value then return "

" + gettext("Unknown") + "

" else return '

' + value + '

' + if value is null then return "

" + gettext("Unknown") + "

" else return '

' + value + '

' # Creates a table to display the content of bulk course emails # sent in the past @@ -153,6 +157,14 @@ create_email_content_table = ($table_emails, $table_emails_inner, email_data) -> minWidth: 80 cssClass: "email-content-cell" formatter: subject_formatter + , + id: 'requester' + field: 'requester' + name: gettext('Sent By') + minWidth: 80 + maxWidth: 100 + cssClass: "email-content-cell" + formatter: sent_by_formatter , id: 'created' field: 'created' @@ -203,7 +215,7 @@ create_email_message_views = ($messages_wrapper, emails) -> # HTML escape the subject line subject_text = $('').text(email_info.email['subject']).html() $email_header.append $('

', class: "message-bold").html('' + gettext('Subject:') + ' ' + subject_text) - + $email_header.append $('

', class: "message-bold").html('' + gettext('Sent By:') + ' ' + email_info.requester) $email_header.append $('

', class: "message-bold").html('' + gettext('Time Sent:') + ' ' + email_info.created) $email_header.append $('

', class: "message-bold").html('' + gettext('Sent To:') + ' ' + email_info.sent_to) $email_wrapper.append $email_header