diff --git a/lms/djangoapps/bulk_email/models.py b/lms/djangoapps/bulk_email/models.py index 7dc398197f..39cefa71f2 100644 --- a/lms/djangoapps/bulk_email/models.py +++ b/lms/djangoapps/bulk_email/models.py @@ -41,7 +41,7 @@ SEND_TO_ALL = 'all' TO_OPTIONS = [SEND_TO_MYSELF, SEND_TO_STAFF, SEND_TO_ALL] -class CourseEmail(Email, models.Model): +class CourseEmail(Email): """ Stores information for an email to a course. """ @@ -103,7 +103,7 @@ class CourseEmail(Email, models.Model): @transaction.autocommit def save_now(self): """ - Writes InstructorTask immediately, ensuring the transaction is committed. + Writes CourseEmail immediately, ensuring the transaction is committed. Autocommit annotation makes sure the database entry is committed. When called from any view that is wrapped by TransactionMiddleware, diff --git a/lms/djangoapps/bulk_email/tasks.py b/lms/djangoapps/bulk_email/tasks.py index 292825efcc..81bbe2918f 100644 --- a/lms/djangoapps/bulk_email/tasks.py +++ b/lms/djangoapps/bulk_email/tasks.py @@ -188,12 +188,7 @@ def perform_delegate_email_batches(entry_id, course_id, task_input, action_name) num_tasks_this_query = int(math.ceil(float(num_emails_this_query) / float(settings.EMAILS_PER_TASK))) chunk = int(math.ceil(float(num_emails_this_query) / float(num_tasks_this_query))) for i in range(num_tasks_this_query): - if i == num_tasks_this_query - 1: - # Avoid cutting off the very last email when chunking a task that divides perfectly - # (e.g. num_emails_this_query = 297 and EMAILS_PER_TASK is 100) - to_list = recipient_sublist[i * chunk:] - else: - to_list = recipient_sublist[i * chunk:i * chunk + chunk] + to_list = recipient_sublist[i * chunk:i * chunk + chunk] subtask_id = str(uuid4()) subtask_id_list.append(subtask_id) subtask_status = create_subtask_status(subtask_id) @@ -480,6 +475,8 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas except INFINITE_RETRY_ERRORS as exc: dog_stats_api.increment('course_email.infinite_retry', tags=[_statsd_tag(course_title)]) + # Increment the "retried_nomax" counter, update other counters with progress to date, + # and set the state to RETRY: subtask_progress = increment_subtask_status( subtask_status, succeeded=num_sent, @@ -497,6 +494,8 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas # without popping the current recipient off of the existing list. # Errors caught are those that indicate a temporary condition that might succeed on retry. dog_stats_api.increment('course_email.limited_retry', tags=[_statsd_tag(course_title)]) + # Increment the "retried_withmax" counter, update other counters with progress to date, + # and set the state to RETRY: subtask_progress = increment_subtask_status( subtask_status, succeeded=num_sent, @@ -514,6 +513,8 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas num_pending = len(to_list) log.exception('Task %s: email with id %d caused send_course_email task to fail with "fatal" exception. %d emails unsent.', task_id, email_id, num_pending) + # Update counters with progress to date, counting unsent emails as failures, + # and set the state to FAILURE: subtask_progress = increment_subtask_status( subtask_status, succeeded=num_sent, @@ -531,6 +532,8 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas dog_stats_api.increment('course_email.limited_retry', tags=[_statsd_tag(course_title)]) log.exception('Task %s: email with id %d caused send_course_email task to fail with unexpected exception. Generating retry.', task_id, email_id) + # Increment the "retried_withmax" counter, update other counters with progress to date, + # and set the state to RETRY: subtask_progress = increment_subtask_status( subtask_status, succeeded=num_sent, @@ -544,7 +547,8 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas ) else: - # Successful completion is marked by an exception value of None. + # All went well. Update counters with progress to date, + # and set the state to SUCCESS: subtask_progress = increment_subtask_status( subtask_status, succeeded=num_sent, @@ -552,6 +556,7 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas skipped=num_optout, state=SUCCESS ) + # Successful completion is marked by an exception value of None. return subtask_progress, None finally: # Clean up at the end. @@ -559,7 +564,15 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas def _get_current_task(): - """Stub to make it easier to test without actually running Celery""" + """ + Stub to make it easier to test without actually running Celery. + + This is a wrapper around celery.current_task, which provides access + to the top of the stack of Celery's tasks. When running tests, however, + it doesn't seem to work to mock current_task directly, so this wrapper + is used to provide a hook to mock in tests, while providing the real + `current_task` in production. + """ return current_task diff --git a/lms/djangoapps/instructor_task/tasks.py b/lms/djangoapps/instructor_task/tasks.py index 9291d7dd16..f30ffe3af2 100644 --- a/lms/djangoapps/instructor_task/tasks.py +++ b/lms/djangoapps/instructor_task/tasks.py @@ -19,6 +19,7 @@ a problem URL and optionally a student. These are used to set up the initial va of the query for traversing StudentModule objects. """ +from django.utils.translation import ugettext_noop from celery import task from functools import partial from instructor_task.tasks_helper import ( @@ -51,7 +52,8 @@ def rescore_problem(entry_id, xmodule_instance_args): `xmodule_instance_args` provides information needed by _get_module_instance_for_task() to instantiate an xmodule instance. """ - action_name = 'rescored' + # Translators: This is a past-tense verb that is inserted into task progress messages as {action}. + action_name = ugettext_noop('rescored') update_fcn = partial(rescore_problem_module_state, xmodule_instance_args) def filter_fcn(modules_to_update): @@ -77,7 +79,8 @@ def reset_problem_attempts(entry_id, xmodule_instance_args): `xmodule_instance_args` provides information needed by _get_module_instance_for_task() to instantiate an xmodule instance. """ - action_name = 'reset' + # Translators: This is a past-tense verb that is inserted into task progress messages as {action}. + action_name = ugettext_noop('reset') update_fcn = partial(reset_attempts_module_state, xmodule_instance_args) visit_fcn = partial(perform_module_state_update, update_fcn, None) return run_main_task(entry_id, visit_fcn, action_name) @@ -98,7 +101,8 @@ def delete_problem_state(entry_id, xmodule_instance_args): `xmodule_instance_args` provides information needed by _get_module_instance_for_task() to instantiate an xmodule instance. """ - action_name = 'deleted' + # Translators: This is a past-tense verb that is inserted into task progress messages as {action}. + action_name = ugettext_noop('deleted') update_fcn = partial(delete_problem_module_state, xmodule_instance_args) visit_fcn = partial(perform_module_state_update, update_fcn, None) return run_main_task(entry_id, visit_fcn, action_name) @@ -119,6 +123,7 @@ def send_bulk_course_email(entry_id, _xmodule_instance_args): `_xmodule_instance_args` provides information needed by _get_module_instance_for_task() to instantiate an xmodule instance. This is unused here. """ - action_name = 'emailed' + # Translators: This is a past-tense verb that is inserted into task progress messages as {action}. + action_name = ugettext_noop('emailed') visit_fcn = perform_delegate_email_batches return run_main_task(entry_id, visit_fcn, action_name) diff --git a/lms/djangoapps/instructor_task/tests/test_tasks.py b/lms/djangoapps/instructor_task/tests/test_tasks.py index 448054a13d..37bb81ae2c 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks.py @@ -40,13 +40,10 @@ class TestInstructorTasks(InstructorTaskModuleTestCase): self.instructor = self.create_instructor('instructor') self.problem_url = InstructorTaskModuleTestCase.problem_location(PROBLEM_URL_NAME) - def _create_input_entry(self, student_ident=None, use_problem_url=True, course_id=None, task_input=None): + def _create_input_entry(self, student_ident=None, use_problem_url=True, course_id=None): """Creates a InstructorTask entry for testing.""" task_id = str(uuid4()) - if task_input is None: - task_input = {} - else: - task_input = dict(task_input) + task_input = {} if use_problem_url: task_input['problem_url'] = self.problem_url if student_ident is not None: diff --git a/lms/djangoapps/instructor_task/views.py b/lms/djangoapps/instructor_task/views.py index d345e4c4e7..9ec18f31e5 100644 --- a/lms/djangoapps/instructor_task/views.py +++ b/lms/djangoapps/instructor_task/views.py @@ -3,6 +3,7 @@ import json import logging from django.http import HttpResponse +from django.utils.translation import ugettext as _ from celery.states import FAILURE, REVOKED, READY_STATES @@ -105,36 +106,38 @@ def get_task_completion_info(instructor_task): succeeded = False if instructor_task.task_state not in STATES_WITH_STATUS: - return (succeeded, "No status information available") + return (succeeded, _("No status information available")) # we're more surprised if there is no output for a completed task, but just warn: if instructor_task.task_output is None: - log.warning("No task_output information found for instructor_task {0}".format(instructor_task.task_id)) - return (succeeded, "No status information available") + log.warning(_("No task_output information found for instructor_task {0}").format(instructor_task.task_id)) + return (succeeded, _("No status information available")) try: task_output = json.loads(instructor_task.task_output) except ValueError: - fmt = "No parsable task_output information found for instructor_task {0}: {1}" + fmt = _("No parsable task_output information found for instructor_task {0}: {1}") log.warning(fmt.format(instructor_task.task_id, instructor_task.task_output)) - return (succeeded, "No parsable status information available") + return (succeeded, _("No parsable status information available")) if instructor_task.task_state in [FAILURE, REVOKED]: - return (succeeded, task_output.get('message', 'No message provided')) + return (succeeded, task_output.get('message', _('No message provided'))) if any([key not in task_output for key in ['action_name', 'attempted', 'total']]): - fmt = "Invalid task_output information found for instructor_task {0}: {1}" + fmt = _("Invalid task_output information found for instructor_task {0}: {1}") log.warning(fmt.format(instructor_task.task_id, instructor_task.task_output)) - return (succeeded, "No progress status information available") + return (succeeded, _("No progress status information available")) - action_name = task_output['action_name'] + action_name = _(task_output['action_name']) num_attempted = task_output['attempted'] num_total = task_output['total'] - # old tasks may still have 'updated' instead of the preferred 'succeeded': + # In earlier versions of this code, the key 'updated' was used instead of + # (the more general) 'succeeded'. In order to support history that may contain + # output with the old key, we check for values with both the old and the current + # key, and simply sum them. num_succeeded = task_output.get('updated', 0) + task_output.get('succeeded', 0) num_skipped = task_output.get('skipped', 0) - # num_failed = task_output.get('failed', 0) student = None problem_url = None @@ -142,7 +145,7 @@ def get_task_completion_info(instructor_task): try: task_input = json.loads(instructor_task.task_input) except ValueError: - fmt = "No parsable task_input information found for instructor_task {0}: {1}" + fmt = _("No parsable task_input information found for instructor_task {0}: {1}") log.warning(fmt.format(instructor_task.task_id, instructor_task.task_input)) else: student = task_input.get('student') @@ -151,47 +154,61 @@ def get_task_completion_info(instructor_task): if instructor_task.task_state == PROGRESS: # special message for providing progress updates: - msg_format = "Progress: {action} {succeeded} of {attempted} so far" + msg_format = _("Progress: {action} {succeeded} of {attempted} so far") elif student is not None and problem_url is not None: # this reports on actions on problems for a particular student: if num_attempted == 0: - msg_format = "Unable to find submission to be {action} for student '{student}'" + # Translators: {action} is a past-tense verb that is localized separately. {student} is a student identifier. + msg_format = _("Unable to find submission to be {action} for student '{student}'") elif num_succeeded == 0: - msg_format = "Problem failed to be {action} for student '{student}'" + # Translators: {action} is a past-tense verb that is localized separately. {student} is a student identifier. + msg_format = _("Problem failed to be {action} for student '{student}'") else: succeeded = True - msg_format = "Problem successfully {action} for student '{student}'" + # Translators: {action} is a past-tense verb that is localized separately. {student} is a student identifier. + msg_format = _("Problem successfully {action} for student '{student}'") elif student is None and problem_url is not None: # this reports on actions on problems for all students: if num_attempted == 0: - msg_format = "Unable to find any students with submissions to be {action}" + # Translators: {action} is a past-tense verb that is localized separately. + msg_format = _("Unable to find any students with submissions to be {action}") elif num_succeeded == 0: - msg_format = "Problem failed to be {action} for any of {attempted} students" + # Translators: {action} is a past-tense verb that is localized separately. {attempted} is a count. + msg_format = _("Problem failed to be {action} for any of {attempted} students") elif num_succeeded == num_attempted: succeeded = True - msg_format = "Problem successfully {action} for {attempted} students" + # Translators: {action} is a past-tense verb that is localized separately. {attempted} is a count. + msg_format = _("Problem successfully {action} for {attempted} students") else: # num_succeeded < num_attempted - msg_format = "Problem {action} for {succeeded} of {attempted} students" + # Translators: {action} is a past-tense verb that is localized separately. {succeeded} and {attempted} are counts. + msg_format = _("Problem {action} for {succeeded} of {attempted} students") elif email_id is not None: # this reports on actions on bulk emails if num_attempted == 0: - msg_format = "Unable to find any recipients to be {action}" + # Translators: {action} is a past-tense verb that is localized separately. + msg_format = _("Unable to find any recipients to be {action}") elif num_succeeded == 0: - msg_format = "Message failed to be {action} for any of {attempted} recipients " + # Translators: {action} is a past-tense verb that is localized separately. {attempted} is a count. + msg_format = _("Message failed to be {action} for any of {attempted} recipients ") elif num_succeeded == num_attempted: succeeded = True - msg_format = "Message successfully {action} for {attempted} recipients" + # Translators: {action} is a past-tense verb that is localized separately. {attempted} is a count. + msg_format = _("Message successfully {action} for {attempted} recipients") else: # num_succeeded < num_attempted - msg_format = "Message {action} for {succeeded} of {attempted} recipients" + # Translators: {action} is a past-tense verb that is localized separately. {succeeded} and {attempted} are counts. + msg_format = _("Message {action} for {succeeded} of {attempted} recipients") else: # provide a default: - msg_format = "Status: {action} {succeeded} of {attempted}" + # Translators: {action} is a past-tense verb that is localized separately. {succeeded} and {attempted} are counts. + msg_format = _("Status: {action} {succeeded} of {attempted}") if num_skipped > 0: - msg_format += " (skipping {skipped})" + # Translators: {skipped} is a count. This message is appended to task progress status messages. + msg_format += _(" (skipping {skipped})") if student is None and num_attempted != num_total: - msg_format += " (out of {total})" + # Translators: {total} is a count. This message is appended to task progress status messages. + msg_format += _(" (out of {total})") # Update status in task result object itself: message = msg_format.format( @@ -200,5 +217,6 @@ def get_task_completion_info(instructor_task): attempted=num_attempted, total=num_total, skipped=num_skipped, - student=student) + student=student + ) return (succeeded, message)