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'))) 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..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'), @@ -72,12 +72,12 @@ 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) - 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'), @@ -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 @@ -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 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 02e45c6131..9e207306e1 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 c8eef40c92..fbc97d6e35 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -392,8 +392,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