From 5cc6acae18528ed150a333c5d3a032ba01dc2a56 Mon Sep 17 00:00:00 2001 From: Pooja Kulkarni Date: Wed, 4 Jan 2023 14:00:42 -0500 Subject: [PATCH] refactor: rename descriptor -> block within lms/djangoapps/instructor_task Co-authored-by: Agrendalath --- lms/djangoapps/instructor_task/api.py | 6 +- lms/djangoapps/instructor_task/api_helper.py | 30 +++---- lms/djangoapps/instructor_task/tasks.py | 2 +- .../tasks_helper/module_state.py | 38 ++++----- .../instructor_task/tests/test_base.py | 8 +- .../instructor_task/tests/test_integration.py | 80 +++++++++---------- .../instructor_task/tests/test_tasks.py | 2 +- 7 files changed, 83 insertions(+), 83 deletions(-) diff --git a/lms/djangoapps/instructor_task/api.py b/lms/djangoapps/instructor_task/api.py index bbf5bf7b28..62095c3ae3 100644 --- a/lms/djangoapps/instructor_task/api.py +++ b/lms/djangoapps/instructor_task/api.py @@ -210,7 +210,7 @@ def submit_reset_problem_attempts_for_all_students(request, usage_key): # pylin if the problem is already being reset. """ # check arguments: make sure that the usage_key is defined - # (since that's currently typed in). If the corresponding module descriptor doesn't exist, + # (since that's currently typed in). If the corresponding block doesn't exist, # an exception will be raised. Let it pass up to the caller. modulestore().get_item(usage_key) @@ -256,7 +256,7 @@ def submit_delete_problem_state_for_all_students(request, usage_key): # pylint: if the particular problem's state is already being deleted. """ # check arguments: make sure that the usage_key is defined - # (since that's currently typed in). If the corresponding module descriptor doesn't exist, + # (since that's currently typed in). If the corresponding block doesn't exist, # an exception will be raised. Let it pass up to the caller. modulestore().get_item(usage_key) @@ -270,7 +270,7 @@ def submit_delete_entrance_exam_state_for_student(request, usage_key, student): """ Requests reset of state for entrance exam as a background task. - Module state for all problems in entrance exam will be deleted + Block state for all problems in entrance exam will be deleted for specified student. All User Milestones of entrance exam will be removed for the specified student diff --git a/lms/djangoapps/instructor_task/api_helper.py b/lms/djangoapps/instructor_task/api_helper.py index 50718580ae..9c3f900fee 100644 --- a/lms/djangoapps/instructor_task/api_helper.py +++ b/lms/djangoapps/instructor_task/api_helper.py @@ -149,14 +149,14 @@ def _get_xblock_instance_args(request, task_id): return xblock_instance_args -def _supports_rescore(descriptor): +def _supports_rescore(block): """ Helper method to determine whether a given item supports rescoring. In order to accommodate both XModules and XBlocks, we have to check - the descriptor itself then fall back on its module class. + the block itself then fall back on its module class. """ - return hasattr(descriptor, 'rescore') or ( - hasattr(descriptor, 'module_class') and hasattr(descriptor.module_class, 'rescore') + return hasattr(block, 'rescore') or ( + hasattr(block, 'module_class') and hasattr(block.module_class, 'rescore') ) @@ -343,49 +343,49 @@ def get_status_from_instructor_task(instructor_task): def check_arguments_for_rescoring(usage_key): """ - Do simple checks on the descriptor to confirm that it supports rescoring. + Do simple checks on the block to confirm that it supports rescoring. Confirms first that the usage_key is defined (since that's currently typed in). An ItemNotFoundException is raised if the corresponding module - descriptor doesn't exist. NotImplementedError is raised if the + block doesn't exist. NotImplementedError is raised if the corresponding module doesn't support rescoring calls. Note: the string returned here is surfaced as the error message on the instructor dashboard when a rescore is submitted for a non-rescorable block. """ - descriptor = modulestore().get_item(usage_key) - if not _supports_rescore(descriptor): + block = modulestore().get_item(usage_key) + if not _supports_rescore(block): msg = _("This component cannot be rescored.") raise NotImplementedError(msg) def check_arguments_for_overriding(usage_key, score): """ - Do simple checks on the descriptor to confirm that it supports overriding + Do simple checks on the block to confirm that it supports overriding the problem score and the score passed in is not greater than the value of the problem or less than 0. """ - descriptor = modulestore().get_item(usage_key) + block = modulestore().get_item(usage_key) score = float(score) - # some weirdness around initializing the descriptor requires this - if not hasattr(descriptor.__class__, 'set_score'): + # some weirdness around initializing the block requires this + if not hasattr(block.__class__, 'set_score'): msg = _("This component does not support score override.") raise NotImplementedError(msg) - if score < 0 or score > descriptor.max_score(): + if score < 0 or score > block.max_score(): msg = _("Scores must be between 0 and the value of the problem.") raise ValueError(msg) def check_entrance_exam_problems_for_rescoring(exam_key): # pylint: disable=invalid-name """ - Grabs all problem descriptors in exam and checks each descriptor to + Grabs all problem blocks in exam and checks each block to confirm that it supports re-scoring. An ItemNotFoundException is raised if the corresponding module - descriptor doesn't exist for exam_key. NotImplementedError is raised if + block doesn't exist for exam_key. NotImplementedError is raised if any of the problem in entrance exam doesn't support re-scoring calls. """ problems = list(get_problems_in_section(exam_key).values()) diff --git a/lms/djangoapps/instructor_task/tasks.py b/lms/djangoapps/instructor_task/tasks.py index f142b90cc4..9603a4fa18 100644 --- a/lms/djangoapps/instructor_task/tasks.py +++ b/lms/djangoapps/instructor_task/tasks.py @@ -4,7 +4,7 @@ running state of a course. At present, these tasks all operate on StudentModule objects in one way or another, so they share a visitor architecture. Each task defines an "update function" that -takes a module_descriptor, a particular StudentModule object, and xblock_instance_args. +takes a block, a particular StudentModule object, and xblock_instance_args. A task may optionally specify a "filter function" that takes a query for StudentModule objects, and adds additional filter clauses. diff --git a/lms/djangoapps/instructor_task/tasks_helper/module_state.py b/lms/djangoapps/instructor_task/tasks_helper/module_state.py index ec7d8b89d4..a39ef8066c 100644 --- a/lms/djangoapps/instructor_task/tasks_helper/module_state.py +++ b/lms/djangoapps/instructor_task/tasks_helper/module_state.py @@ -37,7 +37,7 @@ def perform_module_state_update(update_fcn, filter_fcn, _entry_id, course_id, ta Performs generic update by visiting StudentModule instances with the update_fcn provided. The student modules are fetched for update the `update_fcn` is called on each StudentModule - that passes the resulting filtering. It is passed four arguments: the module_descriptor for + that passes the resulting filtering. It is passed four arguments: the block for the module pointed to by the module_state_key, the particular StudentModule to update, the xblock_instance_args, and the task_input being passed through. If the value returned by the update function evaluates to a boolean True, the update is successful; False indicates the update @@ -73,9 +73,9 @@ def perform_module_state_update(update_fcn, filter_fcn, _entry_id, course_id, ta usage_key = UsageKey.from_string(problem_url).map_into_course(course_id) usage_keys.append(usage_key) - # find the problem descriptor: - problem_descriptor = modulestore().get_item(usage_key) - problems[str(usage_key)] = problem_descriptor + # find the problem block: + problem_block = modulestore().get_item(usage_key) + problems[str(usage_key)] = problem_block # if entrance_exam is present grab all problems in it if entrance_exam_url: @@ -91,10 +91,10 @@ def perform_module_state_update(update_fcn, filter_fcn, _entry_id, course_id, ta for module_to_update in modules_to_update: task_progress.attempted += 1 - module_descriptor = problems[str(module_to_update.module_state_key)] + block = problems[str(module_to_update.module_state_key)] # There is no try here: if there's an error, we let it throw, and the task will # be marked as FAILED, with a stack trace. - update_status = update_fcn(module_descriptor, module_to_update, task_input) + update_status = update_fcn(block, module_to_update, task_input) if update_status == UPDATE_STATUS_SUCCEEDED: # If the update_fcn returns true, then it performed some kind of work. # Logging of failures is left to the update_fcn itself. @@ -110,9 +110,9 @@ def perform_module_state_update(update_fcn, filter_fcn, _entry_id, course_id, ta @outer_atomic -def rescore_problem_module_state(xblock_instance_args, module_descriptor, student_module, task_input): +def rescore_problem_module_state(xblock_instance_args, block, student_module, task_input): ''' - Takes an XModule descriptor and a corresponding StudentModule object, and + Takes an XBlock and a corresponding StudentModule object, and performs rescoring on the student's problem submission. Throws exceptions if the rescoring is fatal and should be aborted if in a loop. @@ -135,7 +135,7 @@ def rescore_problem_module_state(xblock_instance_args, module_descriptor, studen instance = _get_module_instance_for_task( course_id, student, - module_descriptor, + block, xblock_instance_args, grade_bucket_type='rescore', course=course @@ -208,9 +208,9 @@ def rescore_problem_module_state(xblock_instance_args, module_descriptor, studen @outer_atomic -def override_score_module_state(xblock_instance_args, module_descriptor, student_module, task_input): +def override_score_module_state(xblock_instance_args, block, student_module, task_input): ''' - Takes an XModule descriptor and a corresponding StudentModule object, and + Takes an XBlock and a corresponding StudentModule object, and performs an override on the student's problem score. Throws exceptions if the override is fatal and should be aborted if in a loop. @@ -232,7 +232,7 @@ def override_score_module_state(xblock_instance_args, module_descriptor, student instance = _get_module_instance_for_task( course_id, student, - module_descriptor, + block, xblock_instance_args, course=course ) @@ -288,7 +288,7 @@ def override_score_module_state(xblock_instance_args, module_descriptor, student @outer_atomic -def reset_attempts_module_state(xblock_instance_args, _module_descriptor, student_module, _task_input): +def reset_attempts_module_state(xblock_instance_args, _block, student_module, _task_input): """ Resets problem attempts to zero for specified `student_module`. @@ -315,7 +315,7 @@ def reset_attempts_module_state(xblock_instance_args, _module_descriptor, studen @outer_atomic -def delete_problem_module_state(xblock_instance_args, _module_descriptor, student_module, _task_input): +def delete_problem_module_state(xblock_instance_args, _block, student_module, _task_input): """ Delete the StudentModule entry. @@ -329,17 +329,17 @@ def delete_problem_module_state(xblock_instance_args, _module_descriptor, studen return UPDATE_STATUS_SUCCEEDED -def _get_module_instance_for_task(course_id, student, module_descriptor, xblock_instance_args=None, +def _get_module_instance_for_task(course_id, student, block, xblock_instance_args=None, grade_bucket_type=None, course=None): """ - Fetches a StudentModule instance for a given `course_id`, `student` object, and `module_descriptor`. + Fetches a StudentModule instance for a given `course_id`, `student` object, and `block`. `xblock_instance_args` is used to provide information for creating a track function and an XQueue callback. These are passed, along with `grade_bucket_type`, to get_block_for_descriptor_internal, which sidesteps the need for a Request object when instantiating an xblock instance. """ - # reconstitute the problem's corresponding XModule: - field_data_cache = FieldDataCache.cache_for_descriptor_descendents(course_id, student, module_descriptor) + # reconstitute the problem's corresponding XBlock: + field_data_cache = FieldDataCache.cache_for_block_descendents(course_id, student, block) student_data = KvsFieldData(DjangoKeyValueStore(field_data_cache)) # get request-related tracking information from args passthrough, and supplement with task-specific @@ -359,7 +359,7 @@ def _get_module_instance_for_task(course_id, student, module_descriptor, xblock_ return get_block_for_descriptor_internal( user=student, - descriptor=module_descriptor, + block=block, student_data=student_data, course_id=course_id, track_function=make_track_function(), diff --git a/lms/djangoapps/instructor_task/tests/test_base.py b/lms/djangoapps/instructor_task/tests/test_base.py index 2f1fcfdc5e..dbcac033f7 100644 --- a/lms/djangoapps/instructor_task/tests/test_base.py +++ b/lms/djangoapps/instructor_task/tests/test_base.py @@ -254,12 +254,12 @@ class InstructorTaskModuleTestCase(InstructorTaskCourseTestCase): self.module_store.update_item(item, self.user.id) self.module_store.publish(location, self.user.id) - def get_student_module(self, username, descriptor): - """Get StudentModule object for test course, given the `username` and the problem's `descriptor`.""" + def get_student_module(self, username, block): + """Get StudentModule object for test course, given the `username` and the problem's `block`.""" return StudentModule.objects.get(course_id=self.course.id, student=User.objects.get(username=username), - module_type=descriptor.location.block_type, - module_state_key=descriptor.location, + module_type=block.location.block_type, + module_state_key=block.location, ) def submit_student_answer(self, username, problem_url_name, responses): diff --git a/lms/djangoapps/instructor_task/tests/test_integration.py b/lms/djangoapps/instructor_task/tests/test_integration.py index f3f6a9c7c9..ff4f929ea8 100644 --- a/lms/djangoapps/instructor_task/tests/test_integration.py +++ b/lms/djangoapps/instructor_task/tests/test_integration.py @@ -110,15 +110,15 @@ class TestRescoringTask(TestIntegrationTask): resp = self.client.post(modx_url, {}) return resp - def check_state(self, user, descriptor, expected_score, expected_max_score, expected_attempts=1): + def check_state(self, user, block, expected_score, expected_max_score, expected_attempts=1): """ Check that the StudentModule state contains the expected values. - The student module is found for the test course, given the `username` and problem `descriptor`. + The student module is found for the test course, given the `username` and problem `block`. Values checked include the number of attempts, the score, and the max score for a problem. """ - module = self.get_student_module(user.username, descriptor) + module = self.get_student_module(user.username, block) assert module.grade == expected_score assert module.max_grade == expected_max_score state = json.loads(module.state) @@ -160,11 +160,11 @@ class TestRescoringTask(TestIntegrationTask): Common helper to verify the results of rescoring for a single student and all students are as expected. """ - # get descriptor: + # get block: problem_url_name = 'H1P1' self.define_option_problem(problem_url_name) location = InstructorTaskModuleTestCase.problem_location(problem_url_name) - descriptor = self.module_store.get_item(location) + block = self.module_store.get_item(location) # first store answers for each of the separate users: self.submit_student_answer('u1', problem_url_name, [OPTION_1, OPTION_1]) @@ -176,25 +176,25 @@ class TestRescoringTask(TestIntegrationTask): expected_original_scores = (2, 1, 1, 0) expected_original_max = 2 for i, user in enumerate(self.users): - self.check_state(user, descriptor, expected_original_scores[i], expected_original_max) + self.check_state(user, block, expected_original_scores[i], expected_original_max) # update the data in the problem definition so the answer changes. self.redefine_option_problem(problem_url_name, **problem_edit) # confirm that simply rendering the problem again does not change the grade self.render_problem('u1', problem_url_name) - self.check_state(self.user1, descriptor, expected_original_scores[0], expected_original_max) + self.check_state(self.user1, block, expected_original_scores[0], expected_original_max) # rescore the problem for only one student -- only that student's grade should change: self.submit_rescore_one_student_answer('instructor', problem_url_name, self.user1, rescore_if_higher) - self.check_state(self.user1, descriptor, new_expected_scores[0], new_expected_max) + self.check_state(self.user1, block, new_expected_scores[0], new_expected_max) for i, user in enumerate(self.users[1:], start=1): # everyone other than user1 - self.check_state(user, descriptor, expected_original_scores[i], expected_original_max) + self.check_state(user, block, expected_original_scores[i], expected_original_max) # rescore the problem for all students self.submit_rescore_all_student_answers('instructor', problem_url_name, rescore_if_higher) for i, user in enumerate(self.users): - self.check_state(user, descriptor, new_expected_scores[i], new_expected_max) + self.check_state(user, block, new_expected_scores[i], new_expected_max) RescoreTestData = namedtuple('RescoreTestData', 'edit, new_expected_scores, new_expected_max') @@ -240,31 +240,31 @@ class TestRescoringTask(TestIntegrationTask): problem_url_name = 'H1P1' self.define_option_problem(problem_url_name) location = InstructorTaskModuleTestCase.problem_location(problem_url_name) - descriptor = self.module_store.get_item(location) + block = self.module_store.get_item(location) # first store answers for each of the separate users: self.submit_student_answer('u1', problem_url_name, [OPTION_1, OPTION_1]) self.submit_student_answer('u2', problem_url_name, [OPTION_2, OPTION_2]) # verify each user's grade - self.check_state(self.user1, descriptor, 2, 2) # user 1 has a 2/2 - self.check_state(self.user2, descriptor, 0, 2) # user 2 has a 0/2 + self.check_state(self.user1, block, 2, 2) # user 1 has a 2/2 + self.check_state(self.user2, block, 0, 2) # user 2 has a 0/2 # update the data in the problem definition so the answer changes. self.redefine_option_problem(problem_url_name, **problem_edit) # confirm that simply rendering the problem again does not change the grade self.render_problem('u1', problem_url_name) - self.check_state(self.user1, descriptor, 2, 2) - self.check_state(self.user2, descriptor, 0, 2) + self.check_state(self.user1, block, 2, 2) + self.check_state(self.user2, block, 0, 2) # rescore the problem for all students self.submit_rescore_all_student_answers('instructor', problem_url_name, True) # user 1's score would go down, so it remains 2/2. user 2's score was 0/2, which is equivalent to the new score # of 0/4, so user 2's score changes to 0/4. - self.check_state(self.user1, descriptor, 2, unchanged_max) - self.check_state(self.user2, descriptor, 0, new_max) + self.check_state(self.user1, block, 2, unchanged_max) + self.check_state(self.user2, block, 0, new_max) def test_rescoring_failure(self): """Simulate a failure in rescoring a problem""" @@ -365,13 +365,13 @@ class TestRescoringTask(TestIntegrationTask): """ % ('!=' if redefine else '==')) problem_xml = factory.build_xml(script=script, cfn="check_func", expect="42", num_responses=1) if redefine: - descriptor = self.module_store.get_item( + block = self.module_store.get_item( InstructorTaskModuleTestCase.problem_location(problem_url_name) ) - descriptor.data = problem_xml - with self.module_store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, descriptor.location.course_key): # lint-amnesty, pylint: disable=line-too-long - self.module_store.update_item(descriptor, self.user.id) - self.module_store.publish(descriptor.location, self.user.id) + block.data = problem_xml + with self.module_store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, block.location.course_key): # lint-amnesty, pylint: disable=line-too-long + self.module_store.update_item(block, self.user.id) + self.module_store.publish(block.location, self.user.id) else: # Use "per-student" rerandomization so that check-problem can be called more than once. # Using "always" means we cannot check a problem twice, but we want to call once to get the @@ -390,7 +390,7 @@ class TestRescoringTask(TestIntegrationTask): problem_url_name = 'H1P1' self.define_randomized_custom_response_problem(problem_url_name) location = InstructorTaskModuleTestCase.problem_location(problem_url_name) - descriptor = self.module_store.get_item(location) + block = self.module_store.get_item(location) # run with more than one user for user in self.users: # first render the problem, so that a seed will be created for this user @@ -399,9 +399,9 @@ class TestRescoringTask(TestIntegrationTask): dummy_answer = "1000" self.submit_student_answer(user.username, problem_url_name, [dummy_answer, dummy_answer]) # we should have gotten the problem wrong, since we're way out of range: - self.check_state(user, descriptor, 0, 1, expected_attempts=1) + self.check_state(user, block, 0, 1, expected_attempts=1) # dig the correct answer out of the problem's message - module = self.get_student_module(user.username, descriptor) + module = self.get_student_module(user.username, block) state = json.loads(module.state) correct_map = state['correct_map'] log.info("Correct Map: %s", correct_map) @@ -409,28 +409,28 @@ class TestRescoringTask(TestIntegrationTask): answer = list(correct_map.values())[0]['msg'] self.submit_student_answer(user.username, problem_url_name, [answer, answer]) # we should now get the problem right, with a second attempt: - self.check_state(user, descriptor, 1, 1, expected_attempts=2) + self.check_state(user, block, 1, 1, expected_attempts=2) # redefine the problem (as stored in Mongo) so that the definition of correct changes self.define_randomized_custom_response_problem(problem_url_name, redefine=True) # confirm that simply rendering the problem again does not result in a change # in the grade (or the attempts): self.render_problem('u1', problem_url_name) - self.check_state(self.user1, descriptor, 1, 1, expected_attempts=2) + self.check_state(self.user1, block, 1, 1, expected_attempts=2) # rescore the problem for only one student -- only that student's grade should change # (and none of the attempts): self.submit_rescore_one_student_answer('instructor', problem_url_name, User.objects.get(username='u1')) for user in self.users: expected_score = 0 if user.username == 'u1' else 1 - self.check_state(user, descriptor, expected_score, 1, expected_attempts=2) + self.check_state(user, block, expected_score, 1, expected_attempts=2) # rescore the problem for all students self.submit_rescore_all_student_answers('instructor', problem_url_name) # all grades should change to being wrong (with no change in attempts) for user in self.users: - self.check_state(user, descriptor, 0, 1, expected_attempts=2) + self.check_state(user, block, 0, 1, expected_attempts=2) @override_settings(RATELIMIT_ENABLE=False) @@ -450,9 +450,9 @@ class TestResetAttemptsTask(TestIntegrationTask): self.create_student(username) self.logout() - def get_num_attempts(self, username, descriptor): - """returns number of attempts stored for `username` on problem `descriptor` for test course""" - module = self.get_student_module(username, descriptor) + def get_num_attempts(self, username, block): + """returns number of attempts stored for `username` on problem `block` for test course""" + module = self.get_student_module(username, block) state = json.loads(module.state) return state['attempts'] @@ -463,11 +463,11 @@ class TestResetAttemptsTask(TestIntegrationTask): def test_reset_attempts_on_problem(self): """Run reset-attempts scenario on option problem""" - # get descriptor: + # get block: problem_url_name = 'H1P1' self.define_option_problem(problem_url_name) location = InstructorTaskModuleTestCase.problem_location(problem_url_name) - descriptor = self.module_store.get_item(location) + block = self.module_store.get_item(location) num_attempts = 3 # first store answers for each of the separate users: for _ in range(num_attempts): @@ -475,12 +475,12 @@ class TestResetAttemptsTask(TestIntegrationTask): self.submit_student_answer(username, problem_url_name, [OPTION_1, OPTION_1]) for username in self.userlist: - assert self.get_num_attempts(username, descriptor) == num_attempts + assert self.get_num_attempts(username, block) == num_attempts self.reset_problem_attempts('instructor', location) for username in self.userlist: - assert self.get_num_attempts(username, descriptor) == 0 + assert self.get_num_attempts(username, block) == 0 def test_reset_failure(self): """Simulate a failure in resetting attempts on a problem""" @@ -528,23 +528,23 @@ class TestDeleteProblemTask(TestIntegrationTask): def test_delete_problem_state(self): """Run delete-state scenario on option problem""" - # get descriptor: + # get block: problem_url_name = 'H1P1' self.define_option_problem(problem_url_name) location = InstructorTaskModuleTestCase.problem_location(problem_url_name) - descriptor = self.module_store.get_item(location) + block = self.module_store.get_item(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: - assert self.get_student_module(username, descriptor) is not None + assert self.get_student_module(username, block) is not None # run delete task: self.delete_problem_state('instructor', location) # confirm that no state can be found: for username in self.userlist: with pytest.raises(StudentModule.DoesNotExist): - self.get_student_module(username, descriptor) + self.get_student_module(username, block) def test_delete_failure(self): """Simulate a failure in deleting state of a problem""" diff --git a/lms/djangoapps/instructor_task/tests/test_tasks.py b/lms/djangoapps/instructor_task/tests/test_tasks.py index 4c66f7271c..07798935a1 100644 --- a/lms/djangoapps/instructor_task/tests/test_tasks.py +++ b/lms/djangoapps/instructor_task/tests/test_tasks.py @@ -283,7 +283,7 @@ class TestOverrideScoreInstructorTask(TestInstructorTasks): def test_overriding_non_scorable(self): """ - Tests that override problem score raises an error if module descriptor has not `set_score` method. + Tests that override problem score raises an error if block has not `set_score` method. """ input_state = json.dumps({'done': True}) num_students = 1