Merge pull request #29105 from edx/ddumesnil/reset-completion-exam-aa-1063
fix: AA-1063: Reset Completion data for problems on exam reset
This commit is contained in:
@@ -16,7 +16,7 @@ from common.djangoapps.student.models import get_user_by_username_or_email
|
||||
from common.djangoapps.student.roles import CourseStaffRole
|
||||
from lms.djangoapps.commerce.utils import create_zendesk_ticket
|
||||
from lms.djangoapps.courseware.models import StudentModule
|
||||
from lms.djangoapps.instructor.tasks import complete_student_attempt_task
|
||||
from lms.djangoapps.instructor.tasks import update_exam_completion_task
|
||||
from xmodule.modulestore.django import modulestore
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
@@ -80,9 +80,15 @@ class InstructorService:
|
||||
)
|
||||
log.error(err_msg)
|
||||
|
||||
# In some cases, reset_student_attempts does not clear the entire exam's completion state.
|
||||
# One example of this is an exam with multiple units (verticals) within it and the learner
|
||||
# never viewing one of the units. All of the content in that unit will still be marked complete,
|
||||
# but the reset code is unable to handle clearing the completion in that scenario.
|
||||
update_exam_completion_task.apply_async((student_identifier, content_id, 0.0))
|
||||
|
||||
def complete_student_attempt(self, user_identifier: str, content_id: str) -> None:
|
||||
"""
|
||||
Calls the complete_student_attempt_task
|
||||
Calls the update_exam_completion_task, marking the exam as complete.
|
||||
|
||||
The task submits all completable xblocks inside of the content_id block to the
|
||||
Completion Service to mark them as complete. One use case of this function is
|
||||
@@ -94,7 +100,7 @@ class InstructorService:
|
||||
user_identifier (str): username or email of a user
|
||||
content_id (str): the block key for a piece of content
|
||||
"""
|
||||
complete_student_attempt_task.apply_async((user_identifier, content_id))
|
||||
update_exam_completion_task.apply_async((user_identifier, content_id, 1.0))
|
||||
|
||||
def is_course_staff(self, user, course_id):
|
||||
"""
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
""" Celery Tasks for the Instructor App """
|
||||
""" Celery Tasks for the Instructor App. """
|
||||
|
||||
import logging
|
||||
|
||||
@@ -22,9 +22,9 @@ log = logging.getLogger(__name__)
|
||||
|
||||
@shared_task(base=LoggedTask, ignore_result=True)
|
||||
@set_code_owner_attribute
|
||||
def complete_student_attempt_task(user_identifier: str, content_id: str) -> None:
|
||||
def update_exam_completion_task(user_identifier: str, content_id: str, completion: float) -> None:
|
||||
"""
|
||||
Marks all completable children of content_id as complete for the user
|
||||
Marks all completable children of content_id as complete for the user.
|
||||
|
||||
Submits all completable xblocks inside of the content_id block to the
|
||||
Completion Service to mark them as complete. One use case of this function is
|
||||
@@ -35,6 +35,7 @@ def complete_student_attempt_task(user_identifier: str, content_id: str) -> None
|
||||
params:
|
||||
user_identifier (str): username or email of a user
|
||||
content_id (str): the block key for a piece of content
|
||||
completion (float): the completion percentage to send to the Completion service (either 1.0 or 0.0)
|
||||
"""
|
||||
err_msg_prefix = (
|
||||
'Error occurred while attempting to complete student attempt for user '
|
||||
@@ -78,13 +79,13 @@ def complete_student_attempt_task(user_identifier: str, content_id: str) -> None
|
||||
log.error(err_msg)
|
||||
return
|
||||
|
||||
def _submit_completions(block, user):
|
||||
def _submit_completions(block, user, completion):
|
||||
"""
|
||||
Recursively submits the children for completion to the Completion Service
|
||||
"""
|
||||
mode = XBlockCompletionMode.get_mode(block)
|
||||
if mode == XBlockCompletionMode.COMPLETABLE:
|
||||
block.runtime.publish(block, 'completion', {'completion': 1.0, 'user_id': user.id})
|
||||
block.runtime.publish(block, 'completion', {'completion': completion, 'user_id': user.id})
|
||||
elif mode == XBlockCompletionMode.AGGREGATOR:
|
||||
# I know this looks weird, but at the time of writing at least, there isn't a good
|
||||
# single way to get the children assigned for a partcular user. Some blocks define the
|
||||
@@ -96,6 +97,6 @@ def complete_student_attempt_task(user_identifier: str, content_id: str) -> None
|
||||
or (hasattr(block, 'get_children') and block.get_children())
|
||||
or [])
|
||||
for child in block_children:
|
||||
_submit_completions(child, user)
|
||||
_submit_completions(child, user, completion)
|
||||
|
||||
_submit_completions(root_module, user)
|
||||
_submit_completions(root_module, user, completion)
|
||||
|
||||
@@ -15,7 +15,6 @@ from common.djangoapps.student.tests.factories import UserFactory
|
||||
from lms.djangoapps.courseware.models import StudentModule
|
||||
from lms.djangoapps.instructor.access import allow_access
|
||||
from lms.djangoapps.instructor.services import InstructorService
|
||||
from lms.djangoapps.instructor.tests.test_tools import msk_from_problem_urlname
|
||||
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
|
||||
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
|
||||
from xmodule.partitions.partitions import Group, UserPartition
|
||||
@@ -31,16 +30,12 @@ class InstructorServiceTests(SharedModuleStoreTestCase):
|
||||
super().setUpClass()
|
||||
cls.email = 'escalation@test.com'
|
||||
cls.course = CourseFactory.create(proctoring_escalation_email=cls.email)
|
||||
cls.problem_location = msk_from_problem_urlname(
|
||||
cls.course.id,
|
||||
'robot-some-problem-urlname'
|
||||
)
|
||||
cls.other_problem_location = msk_from_problem_urlname(
|
||||
cls.course.id,
|
||||
'robot-some-other_problem-urlname'
|
||||
)
|
||||
cls.problem_urlname = str(cls.problem_location)
|
||||
cls.other_problem_urlname = str(cls.other_problem_location)
|
||||
cls.section = ItemFactory.create(parent=cls.course, category='chapter')
|
||||
cls.subsection = ItemFactory.create(parent=cls.section, category='sequential')
|
||||
cls.unit = ItemFactory.create(parent=cls.subsection, category='vertical')
|
||||
cls.problem = ItemFactory.create(parent=cls.unit, category='problem')
|
||||
cls.unit_2 = ItemFactory.create(parent=cls.subsection, category='vertical')
|
||||
cls.problem_2 = ItemFactory.create(parent=cls.unit_2, category='problem')
|
||||
cls.complete_error_prefix = ('Error occurred while attempting to complete student attempt for '
|
||||
'user {user} for content_id {content_id}. ')
|
||||
|
||||
@@ -54,12 +49,13 @@ class InstructorServiceTests(SharedModuleStoreTestCase):
|
||||
self.module_to_reset = StudentModule.objects.create(
|
||||
student=self.student,
|
||||
course_id=self.course.id,
|
||||
module_state_key=self.problem_location,
|
||||
module_state_key=self.problem.location,
|
||||
state=json.dumps({'attempts': 2}),
|
||||
)
|
||||
|
||||
@mock.patch('lms.djangoapps.grades.signals.handlers.PROBLEM_WEIGHTED_SCORE_CHANGED.send')
|
||||
def test_reset_student_attempts_delete(self, _mock_signal):
|
||||
@mock.patch('completion.handlers.BlockCompletion.objects.submit_completion')
|
||||
def test_reset_student_attempts_delete(self, mock_submit, _mock_signal):
|
||||
"""
|
||||
Test delete student state.
|
||||
"""
|
||||
@@ -68,17 +64,23 @@ class InstructorServiceTests(SharedModuleStoreTestCase):
|
||||
assert StudentModule.objects.filter(student=self.module_to_reset.student, course_id=self.course.id,
|
||||
module_state_key=self.module_to_reset.module_state_key).count() == 1
|
||||
|
||||
self.service.delete_student_attempt(
|
||||
self.student.username,
|
||||
str(self.course.id),
|
||||
self.problem_urlname,
|
||||
requesting_user=self.student,
|
||||
)
|
||||
with override_waffle_switch(ENABLE_COMPLETION_TRACKING_SWITCH, True):
|
||||
self.service.delete_student_attempt(
|
||||
self.student.username,
|
||||
str(self.course.id),
|
||||
str(self.subsection.location),
|
||||
requesting_user=self.student,
|
||||
)
|
||||
|
||||
# make sure the module has been deleted
|
||||
assert StudentModule.objects.filter(student=self.module_to_reset.student, course_id=self.course.id,
|
||||
module_state_key=self.module_to_reset.module_state_key).count() == 0
|
||||
|
||||
# Assert we send completion == 0.0 for both problems even though the second problem was never viewed
|
||||
assert mock_submit.call_count == 2
|
||||
mock_submit.assert_any_call(user=self.student, block_key=self.problem.location, completion=0.0)
|
||||
mock_submit.assert_any_call(user=self.student, block_key=self.problem_2.location, completion=0.0)
|
||||
|
||||
def test_reset_bad_content_id(self):
|
||||
"""
|
||||
Negative test of trying to reset attempts with bad content_id
|
||||
@@ -113,7 +115,7 @@ class InstructorServiceTests(SharedModuleStoreTestCase):
|
||||
result = self.service.delete_student_attempt( # lint-amnesty, pylint: disable=assignment-from-none
|
||||
self.student.username,
|
||||
str(self.course.id),
|
||||
self.other_problem_urlname,
|
||||
str(self.problem_2.location),
|
||||
requesting_user=self.student,
|
||||
)
|
||||
assert result is None
|
||||
@@ -198,9 +200,10 @@ class InstructorServiceTests(SharedModuleStoreTestCase):
|
||||
Assert complete_student_attempt with a bad user raises error and returns None
|
||||
"""
|
||||
username = 'bad_user'
|
||||
self.service.complete_student_attempt(username, self.problem_urlname)
|
||||
block_id = str(self.problem.location)
|
||||
self.service.complete_student_attempt(username, block_id)
|
||||
mock_logger.assert_called_once_with(
|
||||
self.complete_error_prefix.format(user=username, content_id=self.problem_urlname) + 'User does not exist!'
|
||||
self.complete_error_prefix.format(user=username, content_id=block_id) + 'User does not exist!'
|
||||
)
|
||||
|
||||
@mock.patch('lms.djangoapps.instructor.tasks.log.error')
|
||||
@@ -221,7 +224,7 @@ class InstructorServiceTests(SharedModuleStoreTestCase):
|
||||
raises error and returns None
|
||||
"""
|
||||
username = self.student.username
|
||||
block = self.problem_urlname
|
||||
block = 'i4x://org.0/course_0/problem/fake_problem'
|
||||
self.service.complete_student_attempt(username, block)
|
||||
mock_logger.assert_called_once_with(
|
||||
self.complete_error_prefix.format(user=username, content_id=block) + 'Block not found in the modulestore!'
|
||||
|
||||
Reference in New Issue
Block a user