diff --git a/lms/djangoapps/courseware/tasks.py b/lms/djangoapps/courseware/tasks.py index af01403e38..292abc8ba8 100644 --- a/lms/djangoapps/courseware/tasks.py +++ b/lms/djangoapps/courseware/tasks.py @@ -1,30 +1,33 @@ import json -from time import sleep, time - +from time import time from django.contrib.auth.models import User from django.db import transaction - from celery import task, current_task from celery.utils.log import get_task_logger +from xmodule.modulestore.django import modulestore + import mitxmako.middleware as middleware +from track.views import task_track from courseware.models import StudentModule from courseware.model_data import ModelDataCache from courseware.module_render import get_module_for_descriptor_internal -from xmodule.modulestore.django import modulestore - -from track.views import task_track - # define different loggers for use within tasks and on client side task_log = get_task_logger(__name__) class UpdateProblemModuleStateError(Exception): + """ + Error signaling a fatal condition while updating problem modules. + + Used when the current module cannot be processed and that no more + modules should be attempted. + """ pass @@ -33,7 +36,20 @@ def _update_problem_module_state(course_id, module_state_key, student, update_fc """ Performs generic update by visiting StudentModule instances with the update_fcn provided. - If student is None, performs update on modules for all students on the specified problem. + StudentModule instances are those that match the specified `course_id` and `module_state_key`. + If `student` is not None, it is used as an additional filter to limit the modules to those belonging + to that student. If `student` is None, performs update on modules for all students on the specified problem. + + If a `filter_fcn` is not None, it is applied to the query that has been constructed. It takes one + argument, which is the query being filtered. + + The `update_fcn` is called on each StudentModule that passes the resulting filtering. + It is passed three arguments: the module_descriptor for the module pointed to by the + module_state_key, the particular StudentModule to update, and the xmodule_instance_args being + passed through. + + Because this is run internal to a task, it does not catch exceptions. These are allowed to pass up to the + task-running level, so that it can set the failure modes and capture the error trace in the result object. """ task_id = current_task.request.id fmt = 'Starting to update problem modules as task "{task_id}": course "{course_id}" problem "{state_key}": nothing {action} yet' @@ -74,9 +90,10 @@ def _update_problem_module_state(course_id, module_state_key, student, update_fc # perform the main loop num_updated = 0 num_attempted = 0 - num_total = len(modules_to_update) # TODO: make this more efficient. Count()? + num_total = modules_to_update.count() def get_task_progress(): + """Return a dict containing info about current task""" current_time = time() progress = {'action_name': action_name, 'attempted': num_attempted, @@ -101,9 +118,6 @@ def _update_problem_module_state(course_id, module_state_key, student, update_fc # -- may depend on each iteration's duration current_task.update_state(state='PROGRESS', meta=get_task_progress()) - # TODO: remove this once done with manual testing - sleep(5) # in seconds - task_progress = get_task_progress() current_task.update_state(state='PROGRESS', meta=task_progress) @@ -114,6 +128,9 @@ def _update_problem_module_state(course_id, module_state_key, student, update_fc def _update_problem_module_state_for_student(course_id, problem_url, student_identifier, update_fcn, action_name, filter_fcn=None, xmodule_instance_args=None): + """ + Update the StudentModule for a given student. See _update_problem_module_state(). + """ msg = '' success = False # try to uniquely id student by email address or username @@ -131,16 +148,22 @@ def _update_problem_module_state_for_student(course_id, problem_url, student_ide def _update_problem_module_state_for_all_students(course_id, problem_url, update_fcn, action_name, filter_fcn=None, xmodule_instance_args=None): + """ + Update the StudentModule for all students. See _update_problem_module_state(). + """ return _update_problem_module_state(course_id, problem_url, None, update_fcn, action_name, filter_fcn, xmodule_instance_args) def _get_module_instance_for_task(course_id, student, module_descriptor, module_state_key, xmodule_instance_args=None, grade_bucket_type=None): + """ + Fetches a StudentModule instance for a given course_id, student, and module_state_key. + + Includes providing information for creating a track function and an XQueue callback, + but does not require passing in a Request object. + """ # reconstitute the problem's corresponding XModule: model_data_cache = ModelDataCache.cache_for_descriptor_descendents(course_id, student, module_descriptor) - # Note that the request is passed to get_module() to provide xqueue-related URL information -# instance = get_module(student, request, module_state_key, model_data_cache, -# course_id, grade_bucket_type='regrade') # get request-related tracking information from args passthrough, and supplement with task-specific # information: @@ -211,11 +234,13 @@ def _regrade_problem_module_state(module_descriptor, student_module, xmodule_ins def filter_problem_module_state_for_done(modules_to_update): + """Filter to apply for regrading, to limit module instances to those marked as done""" return modules_to_update.filter(state__contains='"done": true') @task def regrade_problem_for_student(course_id, problem_url, student_identifier, xmodule_instance_args): + """Regrades problem `problem_url` in `course_id` for specified student.""" action_name = 'regraded' update_fcn = _regrade_problem_module_state filter_fcn = filter_problem_module_state_for_done @@ -225,8 +250,7 @@ def regrade_problem_for_student(course_id, problem_url, student_identifier, xmod @task def regrade_problem_for_all_students(course_id, problem_url, xmodule_instance_args): -# factory = RequestFactory(**request_environ) -# request = factory.get('/') + """Regrades problem `problem_url` in `course_id` for all students.""" action_name = 'regraded' update_fcn = _regrade_problem_module_state filter_fcn = filter_problem_module_state_for_done @@ -236,8 +260,11 @@ def regrade_problem_for_all_students(course_id, problem_url, xmodule_instance_ar @transaction.autocommit def _reset_problem_attempts_module_state(module_descriptor, student_module, xmodule_instance_args=None): - # modify the problem's state - # load the state json and change state + """ + Resets problem attempts to zero for specified `student_module`. + + Always returns true, if it doesn't throw an exception. + """ problem_state = json.loads(student_module.state) if 'attempts' in problem_state: old_number_of_attempts = problem_state["attempts"] @@ -246,6 +273,13 @@ def _reset_problem_attempts_module_state(module_descriptor, student_module, xmod # convert back to json and save student_module.state = json.dumps(problem_state) student_module.save() + # get request-related tracking information from args passthrough, + # and supplement with task-specific information: + request_info = xmodule_instance_args.get('request_info', {}) if xmodule_instance_args is not None else {} + task_id = xmodule_instance_args['task_id'] if xmodule_instance_args is not None else "unknown-task_id" + task_info = {"student": student_module.student.username, "task_id": task_id} + event_info = {"old_attempts": old_number_of_attempts, "new_attempts": 0} + task_track(request_info, task_info, 'problem_reset_attempts', event_info, page='x_module_task') # consider the reset to be successful, even if no update was performed. (It's just "optimized".) return True @@ -253,6 +287,7 @@ def _reset_problem_attempts_module_state(module_descriptor, student_module, xmod @task def reset_problem_attempts_for_student(course_id, problem_url, student_identifier, xmodule_instance_args): + """Resets problem attempts to zero for `problem_url` in `course_id` for specified student.""" action_name = 'reset' update_fcn = _reset_problem_attempts_module_state return _update_problem_module_state_for_student(course_id, problem_url, student_identifier, @@ -262,6 +297,7 @@ def reset_problem_attempts_for_student(course_id, problem_url, student_identifie @task def reset_problem_attempts_for_all_students(course_id, problem_url, xmodule_instance_args): + """Resets problem attempts to zero for `problem_url` in `course_id` for all students.""" action_name = 'reset' update_fcn = _reset_problem_attempts_module_state return _update_problem_module_state_for_all_students(course_id, problem_url, @@ -273,11 +309,18 @@ def reset_problem_attempts_for_all_students(course_id, problem_url, xmodule_inst def _delete_problem_module_state(module_descriptor, student_module, xmodule_instance_args=None): """Delete the StudentModule entry.""" student_module.delete() + # get request-related tracking information from args passthrough, + # and supplement with task-specific information: + request_info = xmodule_instance_args.get('request_info', {}) if xmodule_instance_args is not None else {} + task_id = xmodule_instance_args['task_id'] if xmodule_instance_args is not None else "unknown-task_id" + task_info = {"student": student_module.student.username, "task_id": task_id} + task_track(request_info, task_info, 'problem_delete_state', {}, page='x_module_task') return True @task def delete_problem_state_for_student(course_id, problem_url, student_ident, xmodule_instance_args): + """Deletes problem state entirely for `problem_url` in `course_id` for specified student.""" action_name = 'deleted' update_fcn = _delete_problem_module_state return _update_problem_module_state_for_student(course_id, problem_url, student_ident, @@ -287,6 +330,7 @@ def delete_problem_state_for_student(course_id, problem_url, student_ident, xmod @task def delete_problem_state_for_all_students(course_id, problem_url, xmodule_instance_args): + """Deletes problem state entirely for `problem_url` in `course_id` for all students.""" action_name = 'deleted' update_fcn = _delete_problem_module_state return _update_problem_module_state_for_all_students(course_id, problem_url, diff --git a/lms/djangoapps/courseware/tests/test_tasks.py b/lms/djangoapps/courseware/tests/test_tasks.py index 094f1632d2..e5b299c3a7 100644 --- a/lms/djangoapps/courseware/tests/test_tasks.py +++ b/lms/djangoapps/courseware/tests/test_tasks.py @@ -1,6 +1,6 @@ -''' +""" Test for LMS courseware background tasks -''' +""" import logging import json from mock import Mock, patch @@ -20,7 +20,8 @@ from courseware.model_data import StudentModule from courseware.task_queue import (submit_regrade_problem_for_all_students, submit_regrade_problem_for_student, course_task_log_status, - submit_reset_problem_attempts_for_all_students) + submit_reset_problem_attempts_for_all_students, + submit_delete_problem_state_for_all_students) from courseware.tests.tests import LoginEnrollmentTestCase, TEST_DATA_MONGO_MODULESTORE @@ -169,7 +170,7 @@ class TestRegradingBase(LoginEnrollmentTestCase, ModuleStoreTestCase): }) return resp - def _create_task_request(self, requester_username): + def create_task_request(self, requester_username): """Generate request that can be used for submitting tasks""" request = Mock() request.user = User.objects.get(username=requester_username) @@ -180,12 +181,12 @@ class TestRegradingBase(LoginEnrollmentTestCase, ModuleStoreTestCase): def regrade_all_student_answers(self, instructor, problem_url_name): """Submits the current problem for regrading""" - return submit_regrade_problem_for_all_students(self._create_task_request(instructor), self.course.id, + return submit_regrade_problem_for_all_students(self.create_task_request(instructor), self.course.id, TestRegradingBase.problem_location(problem_url_name)) def regrade_one_student_answer(self, instructor, problem_url_name, student): """Submits the current problem for regrading for a particular student""" - return submit_regrade_problem_for_student(self._create_task_request(instructor), self.course.id, + return submit_regrade_problem_for_student(self.create_task_request(instructor), self.course.id, TestRegradingBase.problem_location(problem_url_name), student) @@ -358,7 +359,7 @@ class TestResetAttempts(TestRegradingBase): def reset_problem_attempts(self, instructor, problem_url_name): """Submits the current problem for resetting""" - return submit_reset_problem_attempts_for_all_students(self._create_task_request(instructor), self.course.id, + return submit_reset_problem_attempts_for_all_students(self.create_task_request(instructor), self.course.id, TestRegradingBase.problem_location(problem_url_name)) def test_reset_attempts_on_problem(self): @@ -420,3 +421,78 @@ class TestResetAttempts(TestRegradingBase): problem_url_name = 'NonexistentProblem' with self.assertRaises(ItemNotFoundError): self.reset_problem_attempts('instructor', problem_url_name) + + +class TestDeleteProblem(TestRegradingBase): + userlist = ['u1', 'u2', 'u3', 'u4'] + + def setUp(self): + self.initialize_course() + self.create_instructor('instructor') + for username in self.userlist: + self.create_student(username) + self.logout() + + def delete_problem_state(self, instructor, problem_url_name): + """Submits the current problem for deletion""" + return submit_delete_problem_state_for_all_students(self.create_task_request(instructor), self.course.id, + TestRegradingBase.problem_location(problem_url_name)) + + def test_delete_problem_state(self): + '''Run delete-state scenario on option problem''' + # get descriptor: + problem_url_name = 'H1P1' + self.define_option_problem(problem_url_name) + location = TestRegradingBase.problem_location(problem_url_name) + descriptor = self.module_store.get_instance(self.course.id, location) + # first store answers for each of the separate users: + for username in self.userlist: + self.submit_student_answer(username, problem_url_name, ['Option 1', 'Option 1']) + # confirm that state exists: + for username in self.userlist: + self.assertTrue(self.get_student_module(username, descriptor) is not None) + # run delete task: + self.delete_problem_state('instructor', problem_url_name) + # confirm that no state can be found: + for username in self.userlist: + with self.assertRaises(StudentModule.DoesNotExist): + self.get_student_module(username, descriptor) + + def test_delete_failure(self): + """Simulate a failure in deleting state of a problem""" + problem_url_name = 'H1P1' + self.define_option_problem(problem_url_name) + self.submit_student_answer('u1', problem_url_name, ['Option 1', 'Option 1']) + + expected_message = "bad things happened" + with patch('courseware.models.StudentModule.delete') as mock_delete: + mock_delete.side_effect = ZeroDivisionError(expected_message) + course_task_log = self.delete_problem_state('instructor', problem_url_name) + + # check task_log returned + self.assertEqual(course_task_log.task_state, 'FAILURE') + self.assertEqual(course_task_log.student, None) + self.assertEqual(course_task_log.requester.username, 'instructor') + self.assertEqual(course_task_log.task_name, 'delete_problem_state') + self.assertEqual(course_task_log.task_args, TestRegrading.problem_location(problem_url_name)) + status = json.loads(course_task_log.task_progress) + self.assertEqual(status['exception'], 'ZeroDivisionError') + self.assertEqual(status['message'], expected_message) + + # check status returned: + mock_request = Mock() + response = course_task_log_status(mock_request, task_id=course_task_log.task_id) + status = json.loads(response.content) + self.assertEqual(status['message'], expected_message) + + def test_delete_non_problem(self): + """confirm that a non-problem can still be successfully deleted""" + problem_url_name = self.problem_section.location.url() + course_task_log = self.delete_problem_state('instructor', problem_url_name) + self.assertEqual(course_task_log.task_state, 'SUCCESS') + + def test_delete_nonexistent_module(self): + """confirm that a non-existent module will not submit""" + problem_url_name = 'NonexistentProblem' + with self.assertRaises(ItemNotFoundError): + self.delete_problem_state('instructor', problem_url_name)