Commit 62bbea6b by Eric Fischer

Merge pull request #11666 from edx/efischer/xblock_binding

Fix for ORA delete state TNL-4152
parents 5b5b4eb4 25e67370
...@@ -202,7 +202,7 @@ def send_beta_role_email(action, user, email_params): ...@@ -202,7 +202,7 @@ def send_beta_role_email(action, user, email_params):
send_mail_to_student(user.email, email_params, language=get_user_email_language(user)) send_mail_to_student(user.email, email_params, language=get_user_email_language(user))
def reset_student_attempts(course_id, student, module_state_key, delete_module=False): def reset_student_attempts(course_id, student, module_state_key, requesting_user, delete_module=False):
""" """
Reset student attempts for a problem. Optionally deletes all student state for the specified problem. Reset student attempts for a problem. Optionally deletes all student state for the specified problem.
...@@ -219,26 +219,42 @@ def reset_student_attempts(course_id, student, module_state_key, delete_module=F ...@@ -219,26 +219,42 @@ def reset_student_attempts(course_id, student, module_state_key, delete_module=F
submissions.SubmissionError: unexpected error occurred while resetting the score in the submissions API. submissions.SubmissionError: unexpected error occurred while resetting the score in the submissions API.
""" """
user_id = anonymous_id_for_user(student, course_id)
requesting_user_id = anonymous_id_for_user(requesting_user, course_id)
submission_cleared = False
try: try:
# A block may have children. Clear state on children first. # A block may have children. Clear state on children first.
block = modulestore().get_item(module_state_key) block = modulestore().get_item(module_state_key)
if block.has_children: if block.has_children:
for child in block.children: for child in block.children:
try: try:
reset_student_attempts(course_id, student, child, delete_module=delete_module) reset_student_attempts(course_id, student, child, requesting_user, delete_module=delete_module)
except StudentModule.DoesNotExist: except StudentModule.DoesNotExist:
# If a particular child doesn't have any state, no big deal, as long as the parent does. # If a particular child doesn't have any state, no big deal, as long as the parent does.
pass pass
if delete_module:
# Some blocks (openassessment) use StudentModule data as a key for internal submission data.
# Inform these blocks of the reset and allow them to handle their data.
clear_student_state = getattr(block, "clear_student_state", None)
if callable(clear_student_state):
clear_student_state(
user_id=user_id,
course_id=unicode(course_id),
item_id=unicode(module_state_key),
requesting_user_id=requesting_user_id
)
submission_cleared = True
except ItemNotFoundError: except ItemNotFoundError:
log.warning("Could not find %s in modulestore when attempting to reset attempts.", module_state_key) log.warning("Could not find %s in modulestore when attempting to reset attempts.", module_state_key)
# Reset the student's score in the submissions API # Reset the student's score in the submissions API, if xblock.clear_student_state has not done so already.
# Currently this is used only by open assessment (ORA 2) # We need to do this before retrieving the `StudentModule` model, because a score may exist with no student module.
# We need to do this *before* retrieving the `StudentModule` model,
# because it's possible for a score to exist even if no student module exists. # TODO: Should the LMS know about sub_api and call this reset, or should it generically call it on all of its
if delete_module: # xblock services as well? See JIRA ARCH-26.
if delete_module and not submission_cleared:
sub_api.reset_score( sub_api.reset_score(
anonymous_id_for_user(student, course_id), user_id,
course_id.to_deprecated_string(), course_id.to_deprecated_string(),
module_state_key.to_deprecated_string(), module_state_key.to_deprecated_string(),
) )
......
...@@ -27,9 +27,9 @@ class InstructorService(object): ...@@ -27,9 +27,9 @@ class InstructorService(object):
and attempt counts if there had been an earlier attempt. and attempt counts if there had been an earlier attempt.
""" """
def delete_student_attempt(self, student_identifier, course_id, content_id): def delete_student_attempt(self, student_identifier, course_id, content_id, requesting_user):
""" """
Deletes student state for a problem. Deletes student state for a problem. requesting_user may be kept as an audit trail.
Takes some of the following query parameters Takes some of the following query parameters
- student_identifier is an email or username - student_identifier is an email or username
...@@ -63,7 +63,13 @@ class InstructorService(object): ...@@ -63,7 +63,13 @@ class InstructorService(object):
if student: if student:
try: try:
enrollment.reset_student_attempts(course_id, student, module_state_key, delete_module=True) enrollment.reset_student_attempts(
course_id,
student,
module_state_key,
requesting_user=requesting_user,
delete_module=True,
)
except (StudentModule.DoesNotExist, enrollment.sub_api.SubmissionError): except (StudentModule.DoesNotExist, enrollment.sub_api.SubmissionError):
err_msg = ( err_msg = (
'Error occurred while attempting to reset student attempts for user ' 'Error occurred while attempting to reset student attempts for user '
......
...@@ -371,7 +371,7 @@ class TestInstructorEnrollmentStudentModule(SharedModuleStoreTestCase): ...@@ -371,7 +371,7 @@ class TestInstructorEnrollmentStudentModule(SharedModuleStoreTestCase):
# lambda to reload the module state from the database # lambda to reload the module state from the database
module = lambda: StudentModule.objects.get(student=self.user, course_id=self.course_key, module_state_key=msk) module = lambda: StudentModule.objects.get(student=self.user, course_id=self.course_key, module_state_key=msk)
self.assertEqual(json.loads(module().state)['attempts'], 32) self.assertEqual(json.loads(module().state)['attempts'], 32)
reset_student_attempts(self.course_key, self.user, msk) reset_student_attempts(self.course_key, self.user, msk, requesting_user=self.user)
self.assertEqual(json.loads(module().state)['attempts'], 0) self.assertEqual(json.loads(module().state)['attempts'], 0)
def test_delete_student_attempts(self): def test_delete_student_attempts(self):
...@@ -389,7 +389,7 @@ class TestInstructorEnrollmentStudentModule(SharedModuleStoreTestCase): ...@@ -389,7 +389,7 @@ class TestInstructorEnrollmentStudentModule(SharedModuleStoreTestCase):
course_id=self.course_key, course_id=self.course_key,
module_state_key=msk module_state_key=msk
).count(), 1) ).count(), 1)
reset_student_attempts(self.course_key, self.user, msk, delete_module=True) reset_student_attempts(self.course_key, self.user, msk, requesting_user=self.user, delete_module=True)
self.assertEqual( self.assertEqual(
StudentModule.objects.filter( StudentModule.objects.filter(
student=self.user, student=self.user,
...@@ -425,7 +425,8 @@ class TestInstructorEnrollmentStudentModule(SharedModuleStoreTestCase): ...@@ -425,7 +425,8 @@ class TestInstructorEnrollmentStudentModule(SharedModuleStoreTestCase):
# Delete student state using the instructor dash # Delete student state using the instructor dash
reset_student_attempts( reset_student_attempts(
self.course_key, user, problem_location, self.course_key, user, problem_location,
delete_module=True requesting_user=user,
delete_module=True,
) )
# Verify that the student's scores have been reset in the submissions API # Verify that the student's scores have been reset in the submissions API
...@@ -451,7 +452,7 @@ class TestInstructorEnrollmentStudentModule(SharedModuleStoreTestCase): ...@@ -451,7 +452,7 @@ class TestInstructorEnrollmentStudentModule(SharedModuleStoreTestCase):
self.assertEqual(unrelated_state['attempts'], 12) self.assertEqual(unrelated_state['attempts'], 12)
self.assertEqual(unrelated_state['brains'], 'zombie') self.assertEqual(unrelated_state['brains'], 'zombie')
reset_student_attempts(self.course_key, self.user, self.parent.location) reset_student_attempts(self.course_key, self.user, self.parent.location, requesting_user=self.user)
parent_state = json.loads(self.get_state(self.parent.location)) parent_state = json.loads(self.get_state(self.parent.location))
self.assertEqual(json.loads(self.get_state(self.parent.location))['attempts'], 0) self.assertEqual(json.loads(self.get_state(self.parent.location))['attempts'], 0)
...@@ -478,7 +479,13 @@ class TestInstructorEnrollmentStudentModule(SharedModuleStoreTestCase): ...@@ -478,7 +479,13 @@ class TestInstructorEnrollmentStudentModule(SharedModuleStoreTestCase):
self.assertEqual(unrelated_state['attempts'], 12) self.assertEqual(unrelated_state['attempts'], 12)
self.assertEqual(unrelated_state['brains'], 'zombie') self.assertEqual(unrelated_state['brains'], 'zombie')
reset_student_attempts(self.course_key, self.user, self.parent.location, delete_module=True) reset_student_attempts(
self.course_key,
self.user,
self.parent.location,
requesting_user=self.user,
delete_module=True,
)
self.assertRaises(StudentModule.DoesNotExist, self.get_state, self.parent.location) self.assertRaises(StudentModule.DoesNotExist, self.get_state, self.parent.location)
self.assertRaises(StudentModule.DoesNotExist, self.get_state, self.child.location) self.assertRaises(StudentModule.DoesNotExist, self.get_state, self.child.location)
......
...@@ -67,7 +67,8 @@ class InstructorServiceTests(SharedModuleStoreTestCase): ...@@ -67,7 +67,8 @@ class InstructorServiceTests(SharedModuleStoreTestCase):
self.service.delete_student_attempt( self.service.delete_student_attempt(
self.student.username, self.student.username,
unicode(self.course.id), unicode(self.course.id),
self.problem_urlname self.problem_urlname,
requesting_user=self.student,
) )
# make sure the module has been deleted # make sure the module has been deleted
...@@ -88,7 +89,8 @@ class InstructorServiceTests(SharedModuleStoreTestCase): ...@@ -88,7 +89,8 @@ class InstructorServiceTests(SharedModuleStoreTestCase):
result = self.service.delete_student_attempt( result = self.service.delete_student_attempt(
self.student.username, self.student.username,
unicode(self.course.id), unicode(self.course.id),
'foo/bar/baz' 'foo/bar/baz',
requesting_user=self.student,
) )
self.assertIsNone(result) self.assertIsNone(result)
...@@ -100,7 +102,8 @@ class InstructorServiceTests(SharedModuleStoreTestCase): ...@@ -100,7 +102,8 @@ class InstructorServiceTests(SharedModuleStoreTestCase):
result = self.service.delete_student_attempt( result = self.service.delete_student_attempt(
'bad_student', 'bad_student',
unicode(self.course.id), unicode(self.course.id),
'foo/bar/baz' 'foo/bar/baz',
requesting_user=self.student,
) )
self.assertIsNone(result) self.assertIsNone(result)
...@@ -112,7 +115,8 @@ class InstructorServiceTests(SharedModuleStoreTestCase): ...@@ -112,7 +115,8 @@ class InstructorServiceTests(SharedModuleStoreTestCase):
result = self.service.delete_student_attempt( result = self.service.delete_student_attempt(
self.student.username, self.student.username,
unicode(self.course.id), unicode(self.course.id),
self.other_problem_urlname self.other_problem_urlname,
requesting_user=self.student,
) )
self.assertIsNone(result) self.assertIsNone(result)
......
...@@ -1980,7 +1980,13 @@ def reset_student_attempts(request, course_id): ...@@ -1980,7 +1980,13 @@ def reset_student_attempts(request, course_id):
if student: if student:
try: try:
enrollment.reset_student_attempts(course_id, student, module_state_key, delete_module=delete_module) enrollment.reset_student_attempts(
course_id,
student,
module_state_key,
requesting_user=request.user,
delete_module=delete_module
)
except StudentModule.DoesNotExist: except StudentModule.DoesNotExist:
return HttpResponseBadRequest(_("Module does not exist.")) return HttpResponseBadRequest(_("Module does not exist."))
except sub_api.SubmissionError: except sub_api.SubmissionError:
......
...@@ -90,7 +90,7 @@ git+https://github.com/edx/xblock-utils.git@v1.0.2#egg=xblock-utils==1.0.2 ...@@ -90,7 +90,7 @@ git+https://github.com/edx/xblock-utils.git@v1.0.2#egg=xblock-utils==1.0.2
-e git+https://github.com/edx-solutions/xblock-google-drive.git@138e6fa0bf3a2013e904a085b9fed77dab7f3f21#egg=xblock-google-drive -e git+https://github.com/edx-solutions/xblock-google-drive.git@138e6fa0bf3a2013e904a085b9fed77dab7f3f21#egg=xblock-google-drive
-e git+https://github.com/edx/edx-reverification-block.git@0.0.5#egg=edx-reverification-block==0.0.5 -e git+https://github.com/edx/edx-reverification-block.git@0.0.5#egg=edx-reverification-block==0.0.5
git+https://github.com/edx/edx-user-state-client.git@1.0.1#egg=edx-user-state-client==1.0.1 git+https://github.com/edx/edx-user-state-client.git@1.0.1#egg=edx-user-state-client==1.0.1
git+https://github.com/edx/edx-proctoring.git@0.12.13#egg=edx-proctoring==0.12.13 git+https://github.com/edx/edx-proctoring.git@0.12.14#egg=edx-proctoring==0.12.14
git+https://github.com/edx/xblock-lti-consumer.git@v1.0.3#egg=xblock-lti-consumer==1.0.3 git+https://github.com/edx/xblock-lti-consumer.git@v1.0.3#egg=xblock-lti-consumer==1.0.3
# Third Party XBlocks # Third Party XBlocks
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment