From c6a02870e1fc7917134e794d60b559d349024789 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Tue, 13 May 2014 11:53:58 -0400 Subject: [PATCH 1/4] Update instructor dash to support ora2 submissions API with opaque keys --- lms/djangoapps/instructor/enrollment.py | 4 ++-- .../instructor/tests/test_enrollment.py | 20 +++++++++---------- .../instructor/tests/test_legacy_reset.py | 10 +++------- lms/djangoapps/instructor/views/legacy.py | 4 ++-- 4 files changed, 16 insertions(+), 22 deletions(-) diff --git a/lms/djangoapps/instructor/enrollment.py b/lms/djangoapps/instructor/enrollment.py index 67442f74ba..ce9cab4869 100644 --- a/lms/djangoapps/instructor/enrollment.py +++ b/lms/djangoapps/instructor/enrollment.py @@ -191,8 +191,8 @@ def reset_student_attempts(course_id, student, module_state_key, delete_module=F if delete_module: sub_api.reset_score( anonymous_id_for_user(student, course_id), - course_id, - module_state_key, + course_id.to_deprecated_string(), + module_state_key.to_deprecated_string(), ) module_to_reset = StudentModule.objects.get( diff --git a/lms/djangoapps/instructor/tests/test_enrollment.py b/lms/djangoapps/instructor/tests/test_enrollment.py index 8b09ff5331..4ce954ae2a 100644 --- a/lms/djangoapps/instructor/tests/test_enrollment.py +++ b/lms/djangoapps/instructor/tests/test_enrollment.py @@ -315,33 +315,31 @@ class TestInstructorEnrollmentStudentModule(TestCase): def test_delete_submission_scores(self): user = UserFactory() - course = CourseFactory.create() - problem_location = msk_from_problem_urlname( - course.id, - 'b3dce2586c9c4876b73e7f390e42ef8f', - block_type='openassessment' - ) + problem_location = self.course_key.make_usage_key('dummy', 'module') # Create a student module for the user StudentModule.objects.create( student=user, - course_id=course.id, + course_id=self.course_key, module_state_key=problem_location, state=json.dumps({}) ) # Create a submission and score for the student using the submissions API student_item = { - 'student_id': anonymous_id_for_user(user, course.id), - 'course_id': course.id, - 'item_id': problem_location, + 'student_id': anonymous_id_for_user(user, self.course_key), + 'course_id': self.course_key.to_deprecated_string(), + 'item_id': problem_location.to_deprecated_string(), 'item_type': 'openassessment' } submission = sub_api.create_submission(student_item, 'test answer') sub_api.set_score(submission['uuid'], 1, 2) # Delete student state using the instructor dash - reset_student_attempts(course.id, user, problem_location, delete_module=True) + reset_student_attempts( + self.course_key, user, problem_location, + delete_module=True + ) # Verify that the student's scores have been reset in the submissions API score = sub_api.get_score(student_item) diff --git a/lms/djangoapps/instructor/tests/test_legacy_reset.py b/lms/djangoapps/instructor/tests/test_legacy_reset.py index 4687fa2f7e..40875db38e 100644 --- a/lms/djangoapps/instructor/tests/test_legacy_reset.py +++ b/lms/djangoapps/instructor/tests/test_legacy_reset.py @@ -36,11 +36,7 @@ class InstructorResetStudentStateTest(ModuleStoreTestCase, LoginEnrollmentTestCa CourseEnrollmentFactory.create(user=self.student, course_id=self.course.id) def test_delete_student_state_resets_scores(self): - problem_location = msk_from_problem_urlname( - self.course.id, - 'b3dce2586c9c4876b73e7f390e42ef8f', - block_type='openassessment' - ) + problem_location = self.course.id.make_usage_key('dummy', 'module') # Create a student module for the user StudentModule.objects.create( @@ -53,8 +49,8 @@ class InstructorResetStudentStateTest(ModuleStoreTestCase, LoginEnrollmentTestCa # Create a submission and score for the student using the submissions API student_item = { 'student_id': anonymous_id_for_user(self.student, self.course.id), - 'course_id': self.course.id, - 'item_id': problem_location, + 'course_id': self.course.id.to_deprecated_string(), + 'item_id': problem_location.to_deprecated_string(), 'item_type': 'openassessment' } submission = sub_api.create_submission(student_item, 'test answer') diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index 00efa1b8e5..9fdf3007e8 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -388,8 +388,8 @@ def instructor_dashboard(request, course_id): try: sub_api.reset_score( anonymous_id_for_user(student, course_key), - course_key, - module_state_key, + course_key.to_deprecated_string(), + module_state_key.to_deprecated_string(), ) except sub_api.SubmissionError: # Trust the submissions API to log the error From b39278309d2606235691aae87a354e02e37d1514 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Tue, 13 May 2014 16:39:56 -0400 Subject: [PATCH 2/4] More ORA fixes for opaque-keys. --- .../open_ended_grading_classes/controller_query_service.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py index b3cb9d7496..4822e576c2 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py @@ -72,7 +72,7 @@ class ControllerQueryService(GradingService): def get_flagged_problem_list(self, course_id): params = { - 'course_id': course_id, + 'course_id': course_id.to_deprecated_string(), } data = self.get(self.flagged_problem_list_url, params) @@ -87,7 +87,7 @@ class ControllerQueryService(GradingService): def take_action_on_flags(self, course_id, student_id, submission_id, action_type): params = { - 'course_id': course_id, + 'course_id': course_id.to_deprecated_string(), 'student_id': student_id, 'submission_id': submission_id, 'action_type': action_type From 6c46fb98caffad94cef294e8478a3563bc2d7dee Mon Sep 17 00:00:00 2001 From: Julia Hansbrough Date: Tue, 13 May 2014 16:07:48 +0000 Subject: [PATCH 3/4] Compare stringified course_ids to regexes for courses allowed to execute unsafe code --- common/djangoapps/util/sandboxing.py | 7 ++++++- common/djangoapps/util/tests/test_sandboxing.py | 11 ++++++----- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/common/djangoapps/util/sandboxing.py b/common/djangoapps/util/sandboxing.py index 2024f8fa27..e3ec2eef23 100644 --- a/common/djangoapps/util/sandboxing.py +++ b/common/djangoapps/util/sandboxing.py @@ -16,7 +16,12 @@ def can_execute_unsafe_code(course_id): # a list of regexes configured on the server. # If this is not defined in the environment variables then default to the most restrictive, which # is 'no unsafe courses' + # TODO: This should be a database configuration, where we can mark individual courses as being + # safe/unsafe. Someone in the future should switch us over to that rather than using regexes + # in a settings file + # To others using this: the code as-is is brittle and likely to be changed in the future, + # as per the TODO, so please consider carefully before adding more values to COURSES_WITH_UNSAFE_CODE for regex in getattr(settings, 'COURSES_WITH_UNSAFE_CODE', []): - if re.match(regex, course_id): + if re.match(regex, course_id.to_deprecated_string()): return True return False diff --git a/common/djangoapps/util/tests/test_sandboxing.py b/common/djangoapps/util/tests/test_sandboxing.py index c76132696a..55daa6681c 100644 --- a/common/djangoapps/util/tests/test_sandboxing.py +++ b/common/djangoapps/util/tests/test_sandboxing.py @@ -5,6 +5,7 @@ Tests for sandboxing.py in util app from django.test import TestCase from util.sandboxing import can_execute_unsafe_code from django.test.utils import override_settings +from xmodule.modulestore.locations import SlashSeparatedCourseKey class SandboxingTest(TestCase): @@ -16,19 +17,19 @@ class SandboxingTest(TestCase): """ Test to make sure that a non-match returns false """ - self.assertFalse(can_execute_unsafe_code('edX/notful/empty')) + self.assertFalse(can_execute_unsafe_code(SlashSeparatedCourseKey('edX', 'notful', 'empty'))) @override_settings(COURSES_WITH_UNSAFE_CODE=['edX/full/.*']) def test_sandbox_inclusion(self): """ Test to make sure that a match works across course runs """ - self.assertTrue(can_execute_unsafe_code('edX/full/2012_Fall')) - self.assertTrue(can_execute_unsafe_code('edX/full/2013_Spring')) + self.assertTrue(can_execute_unsafe_code(SlashSeparatedCourseKey('edX', 'full', '2012_Fall'))) + self.assertTrue(can_execute_unsafe_code(SlashSeparatedCourseKey('edX', 'full', '2013_Spring'))) def test_courses_with_unsafe_code_default(self): """ Test that the default setting for COURSES_WITH_UNSAFE_CODE is an empty setting, e.g. we don't use @override_settings in these tests """ - self.assertFalse(can_execute_unsafe_code('edX/full/2012_Fall')) - self.assertFalse(can_execute_unsafe_code('edX/full/2013_Spring')) + self.assertFalse(can_execute_unsafe_code(SlashSeparatedCourseKey('edX', 'full', '2012_Fall'))) + self.assertFalse(can_execute_unsafe_code(SlashSeparatedCourseKey('edX', 'full', '2013_Spring'))) From 25e026438d8ccc515022cbcf381f0982c34cc7bb Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Tue, 13 May 2014 17:46:07 -0400 Subject: [PATCH 4/4] More missing ORA fixes. --- .../controller_query_service.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py index 4822e576c2..1ef75a20ef 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/controller_query_service.py @@ -44,7 +44,7 @@ class ControllerQueryService(GradingService): log.debug(self.combined_notifications_url) data = self.get(self.combined_notifications_url, params) - tags = [u'course_id:{}'.format(course_id), u'user_is_staff:{}'.format(user_is_staff)] + tags = [u'course_id:{}'.format(course_id.to_deprecated_string()), u'user_is_staff:{}'.format(user_is_staff)] tags.extend( u'{}:{}'.format(key, value) for key, value in data.items() @@ -56,12 +56,12 @@ class ControllerQueryService(GradingService): def get_grading_status_list(self, course_id, student_id): params = { 'student_id': student_id, - 'course_id': course_id, + 'course_id': course_id.to_deprecated_string(), } data = self.get(self.grading_status_list_url, params) - tags = [u'course_id:{}'.format(course_id)] + tags = [u'course_id:{}'.format(course_id.to_deprecated_string())] self._record_result('get_grading_status_list', data, tags) dog_stats_api.histogram( self._metric_name('get_grading_status_list.length'), @@ -77,7 +77,7 @@ class ControllerQueryService(GradingService): data = self.get(self.flagged_problem_list_url, params) - tags = [u'course_id:{}'.format(course_id)] + tags = [u'course_id:{}'.format(course_id.to_deprecated_string())] self._record_result('get_flagged_problem_list', data, tags) dog_stats_api.histogram( self._metric_name('get_flagged_problem_list.length'), @@ -95,7 +95,7 @@ class ControllerQueryService(GradingService): data = self.post(self.take_action_on_flags_url, params) - tags = [u'course_id:{}'.format(course_id), u'action_type:{}'.format(action_type)] + tags = [u'course_id:{}'.format(course_id.to_deprecated_string()), u'action_type:{}'.format(action_type)] self._record_result('take_action_on_flags', data, tags) return data