From 94c8d86a42d6114cd219c06ccaabaff70b7e8ee8 Mon Sep 17 00:00:00 2001 From: Calen Pennington Date: Fri, 9 May 2014 16:37:17 -0400 Subject: [PATCH] Fix failing tests from merge with master --- common/djangoapps/student/tests/tests.py | 4 ++-- .../xmodule/video_module/video_handlers.py | 3 +-- .../xmodule/video_module/video_xfields.py | 2 +- .../bulk_email/tests/test_course_optout.py | 2 +- lms/djangoapps/bulk_email/tests/test_email.py | 2 +- lms/djangoapps/courseware/module_render.py | 2 +- .../courseware/tests/test_lti_integration.py | 21 +++++-------------- .../courseware/tests/test_module_render.py | 11 +++++----- lms/djangoapps/courseware/views.py | 15 ++++++++----- .../instructor/tests/test_legacy_email.py | 4 ++-- .../shoppingcart/tests/test_models.py | 12 +++++------ .../verify_student/tests/test_views.py | 8 +++---- lms/djangoapps/verify_student/views.py | 1 - 13 files changed, 40 insertions(+), 47 deletions(-) diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index 7065df11bf..862ea388a5 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -157,7 +157,7 @@ class DashboardTest(TestCase): """ Check that the css class and the status message are in the dashboard html. """ - CourseEnrollment.enroll(self.user, self.course.location.course_id, mode=mode) + CourseEnrollment.enroll(self.user, self.course.location.course_key, mode=mode) try: response = self.client.get(reverse('dashboard')) except NoReverseMatch: @@ -179,7 +179,7 @@ class DashboardTest(TestCase): """ Check that the css class and the status message are not in the dashboard html. """ - CourseEnrollment.enroll(self.user, self.course.location.course_id, mode=mode) + CourseEnrollment.enroll(self.user, self.course.location.course_key, mode=mode) try: response = self.client.get(reverse('dashboard')) except NoReverseMatch: diff --git a/common/lib/xmodule/xmodule/video_module/video_handlers.py b/common/lib/xmodule/xmodule/video_module/video_handlers.py index a51655395c..d59e9061e1 100644 --- a/common/lib/xmodule/xmodule/video_module/video_handlers.py +++ b/common/lib/xmodule/xmodule/video_module/video_handlers.py @@ -194,8 +194,7 @@ class VideoStudentViewHandlers(object): transcript_name = self.sub if transcript_name: - course_location = CourseDescriptor.id_to_location(self.course_id) - course = self.descriptor.runtime.modulestore.get_item(course_location) + course = self.descriptor.runtime.modulestore.get_course(self.course_id) if course.static_asset_path: response = Response( status=307, diff --git a/common/lib/xmodule/xmodule/video_module/video_xfields.py b/common/lib/xmodule/xmodule/video_module/video_xfields.py index 40332aae01..a4ff2acf07 100644 --- a/common/lib/xmodule/xmodule/video_module/video_xfields.py +++ b/common/lib/xmodule/xmodule/video_module/video_xfields.py @@ -79,7 +79,7 @@ class VideoFields(object): default=False ) html5_sources = List( - help="The URL or URLs where you’ve posted non-YouTube versions of the video. Each URL must end in .mpeg, .mp4, .ogg, or .webm and cannot be a YouTube URL. Students will be able to view the first listed video that's compatible with the student's computer. To allow students to download these videos, set Video Download Allowed to True.", + help="The URL or URLs where you've posted non-YouTube versions of the video. Each URL must end in .mpeg, .mp4, .ogg, or .webm and cannot be a YouTube URL. Students will be able to view the first listed video that's compatible with the student's computer. To allow students to download these videos, set Video Download Allowed to True.", display_name="Video File URLs", scope=Scope.settings, ) diff --git a/lms/djangoapps/bulk_email/tests/test_course_optout.py b/lms/djangoapps/bulk_email/tests/test_course_optout.py index 4eaa449232..34fd112670 100644 --- a/lms/djangoapps/bulk_email/tests/test_course_optout.py +++ b/lms/djangoapps/bulk_email/tests/test_course_optout.py @@ -55,7 +55,7 @@ class TestOptoutCourseEmails(ModuleStoreTestCase): # Select the Email view of the instructor dash session = self.client.session - session[u'idash_mode:{0}'.format(self.course.location.course_id)] = 'Email' + session[u'idash_mode:{0}'.format(self.course.location.course_key.to_deprecated_string())] = 'Email' session.save() response = self.client.get(url) selected_email_link = 'Email' diff --git a/lms/djangoapps/bulk_email/tests/test_email.py b/lms/djangoapps/bulk_email/tests/test_email.py index 21aec45dfa..12208a717a 100644 --- a/lms/djangoapps/bulk_email/tests/test_email.py +++ b/lms/djangoapps/bulk_email/tests/test_email.py @@ -77,7 +77,7 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): # Select the Email view of the instructor dash session = self.client.session - session[u'idash_mode:{0}'.format(self.course.location.course_id)] = 'Email' + session[u'idash_mode:{0}'.format(self.course.location.course_key.to_deprecated_string())] = 'Email' session.save() response = self.client.get(self.url) selected_email_link = 'Email' diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 3aae5ea841..51b4bb7de8 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -530,7 +530,7 @@ def get_module_for_descriptor_internal(user, descriptor, field_data_cache, cours # Do not check access when it's a noauth request. if getattr(user, 'known', True): # Short circuit--if the user shouldn't have access, bail without doing any work - if not has_access(user, descriptor, 'load', course_id): + if not has_access(user, 'load', descriptor, course_id): return None (system, student_data) = get_module_system_for_user( diff --git a/lms/djangoapps/courseware/tests/test_lti_integration.py b/lms/djangoapps/courseware/tests/test_lti_integration.py index 01e4867029..86c7fff1d3 100644 --- a/lms/djangoapps/courseware/tests/test_lti_integration.py +++ b/lms/djangoapps/courseware/tests/test_lti_integration.py @@ -148,29 +148,18 @@ class TestLTIModuleListing(ModuleStoreTestCase): display_name="section2", category='sequential') - self.published_location_dict = {'tag': 'i4x', - 'org': self.course.location.org, - 'category': 'lti', - 'course': self.course.location.course, - 'name': 'lti_published'} - self.draft_location_dict = {'tag': 'i4x', - 'org': self.course.location.org, - 'category': 'lti', - 'course': self.course.location.course, - 'name': 'lti_draft', - 'revision': 'draft'} # creates one draft and one published lti module, in different sections self.lti_published = ItemFactory.create( parent_location=self.section1.location, display_name="lti published", category="lti", - location=Location(self.published_location_dict) + location=self.course.id.make_usage_key('lti', 'lti_published'), ) self.lti_draft = ItemFactory.create( parent_location=self.section2.location, display_name="lti draft", category="lti", - location=Location(self.draft_location_dict) + location=self.course.id.make_usage_key('lti', 'lti_published').replace(revision='draft'), ) def expected_handler_url(self, handler): @@ -178,7 +167,7 @@ class TestLTIModuleListing(ModuleStoreTestCase): return "https://{}{}".format(settings.SITE_NAME, reverse( 'courseware.module_render.handle_xblock_callback_noauth', args=[ - self.course.id, + self.course.id.to_deprecated_string(), quote_slashes(unicode(self.lti_published.scope_ids.usage_id).encode('utf-8')), handler ] @@ -197,7 +186,7 @@ class TestLTIModuleListing(ModuleStoreTestCase): """tests that the draft lti module is not a part of the endpoint response, but the published one is""" request = mock.Mock() request.method = 'GET' - response = get_course_lti_endpoints(request, self.course.id) + response = get_course_lti_endpoints(request, self.course.id.to_deprecated_string()) self.assertEqual(200, response.status_code) self.assertEqual('application/json', response['Content-Type']) @@ -216,5 +205,5 @@ class TestLTIModuleListing(ModuleStoreTestCase): for method in DISALLOWED_METHODS: request = mock.Mock() request.method = method - response = get_course_lti_endpoints(request, self.course.id) + response = get_course_lti_endpoints(request, self.course.id.to_deprecated_string()) self.assertEqual(405, response.status_code) diff --git a/lms/djangoapps/courseware/tests/test_module_render.py b/lms/djangoapps/courseware/tests/test_module_render.py index e408677500..44e36c46c3 100644 --- a/lms/djangoapps/courseware/tests/test_module_render.py +++ b/lms/djangoapps/courseware/tests/test_module_render.py @@ -925,9 +925,10 @@ class TestXmoduleRuntimeEvent(TestSubmittingProblems): return render.get_module( # pylint: disable=protected-access user, mock_request, - self.problem.id, + self.problem.location, field_data_cache, - self.course.id)._xmodule + self.course.id + )._xmodule def set_module_grade_using_publish(self, grade_dict): """Publish the user's grade, takes grade_dict as input""" @@ -938,7 +939,7 @@ class TestXmoduleRuntimeEvent(TestSubmittingProblems): def test_xmodule_runtime_publish(self): """Tests the publish mechanism""" self.set_module_grade_using_publish(self.grade_dict) - student_module = StudentModule.objects.get(student=self.student_user, module_state_key=self.problem.id) + student_module = StudentModule.objects.get(student=self.student_user, module_state_key=self.problem.location) self.assertEqual(student_module.grade, self.grade_dict['value']) self.assertEqual(student_module.max_grade, self.grade_dict['max_value']) @@ -946,7 +947,7 @@ class TestXmoduleRuntimeEvent(TestSubmittingProblems): """Test deleting the grade using the publish mechanism""" module = self.set_module_grade_using_publish(self.grade_dict) module.system.publish(module, 'grade', self.delete_dict) - student_module = StudentModule.objects.get(student=self.student_user, module_state_key=self.problem.id) + student_module = StudentModule.objects.get(student=self.student_user, module_state_key=self.problem.location) self.assertIsNone(student_module.grade) self.assertIsNone(student_module.max_grade) @@ -973,7 +974,7 @@ class TestRebindModule(TestSubmittingProblems): return render.get_module( # pylint: disable=protected-access user, mock_request, - self.lti.id, + self.lti.location, field_data_cache, self.course.id)._xmodule diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 342fec5a54..c7df0ca3fc 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -853,13 +853,18 @@ def get_course_lti_endpoints(request, course_id): (django response object): HTTP response. 404 if course is not found, otherwise 200 with JSON body. """ try: - course = get_course(course_id, depth=2) - except ValueError: # get_course raises ValueError if course_id is invalid or doesn't refer to a course + course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + except InvalidKeyError: + return HttpResponse(status=404) + + try: + course = get_course(course_key, depth=2) + except ValueError: return HttpResponse(status=404) anonymous_user = AnonymousUser() anonymous_user.known = False # make these "noauth" requests like module_render.handle_xblock_callback_noauth - lti_descriptors = modulestore().get_items(Location("i4x", course.org, course.number, "lti", None), course.id) + lti_descriptors = modulestore().get_items(course.id, category='lti') lti_noauth_modules = [ get_module_for_descriptor( @@ -867,11 +872,11 @@ def get_course_lti_endpoints(request, course_id): request, descriptor, FieldDataCache.cache_for_descriptor_descendents( - course_id, + course_key, anonymous_user, descriptor ), - course_id + course_key ) for descriptor in lti_descriptors ] diff --git a/lms/djangoapps/instructor/tests/test_legacy_email.py b/lms/djangoapps/instructor/tests/test_legacy_email.py index dd42dd97c5..abeddc2a55 100644 --- a/lms/djangoapps/instructor/tests/test_legacy_email.py +++ b/lms/djangoapps/instructor/tests/test_legacy_email.py @@ -50,7 +50,7 @@ class TestInstructorDashboardEmailView(ModuleStoreTestCase): # Select the Email view of the instructor dash session = self.client.session - session[u'idash_mode:{0}'.format(self.course.location.course_id)] = 'Email' + session[u'idash_mode:{0}'.format(self.course.location.course_key)] = 'Email' session.save() response = self.client.get(self.url) @@ -131,7 +131,7 @@ class TestInstructorDashboardEmailView(ModuleStoreTestCase): course_authorization.save() session = self.client.session - session[u'idash_mode:{0}'.format(self.course.location.course_id)] = 'Email' + session[u'idash_mode:{0}'.format(self.course.location.course_key)] = 'Email' session.save() response = self.client.post( diff --git a/lms/djangoapps/shoppingcart/tests/test_models.py b/lms/djangoapps/shoppingcart/tests/test_models.py index a29d963d3e..26c9ad9d1d 100644 --- a/lms/djangoapps/shoppingcart/tests/test_models.py +++ b/lms/djangoapps/shoppingcart/tests/test_models.py @@ -409,25 +409,25 @@ class CertificateItemTest(ModuleStoreTestCase): def test_refund_cert_callback_before_expiration_email(self): """ Test that refund emails are being sent correctly. """ - course_id = "refund_before_expiration/test/one" + course = CourseFactory.create(org='refund_before_expiration', number='test', run='course', display_name='one') + course_key = course.id many_days = datetime.timedelta(days=60) - CourseFactory.create(org='refund_before_expiration', number='test', run='course', display_name='one') - course_mode = CourseMode(course_id=course_id, + course_mode = CourseMode(course_id=course_key, mode_slug="verified", mode_display_name="verified cert", min_price=self.cost, expiration_datetime=datetime.datetime.now(pytz.utc) + many_days) course_mode.save() - CourseEnrollment.enroll(self.user, course_id, 'verified') + CourseEnrollment.enroll(self.user, course_key, 'verified') cart = Order.get_cart_for_user(user=self.user) - CertificateItem.add_to_order(cart, course_id, self.cost, 'verified') + CertificateItem.add_to_order(cart, course_key, self.cost, 'verified') cart.purchase() mail.outbox = [] with patch('shoppingcart.models.log.error') as mock_error_logger: - CourseEnrollment.unenroll(self.user, course_id) + CourseEnrollment.unenroll(self.user, course_key) self.assertFalse(mock_error_logger.called) self.assertEquals(len(mail.outbox), 1) self.assertEquals('[Refund] User-Requested Refund', mail.outbox[0].subject) diff --git a/lms/djangoapps/verify_student/tests/test_views.py b/lms/djangoapps/verify_student/tests/test_views.py index aa3c39c385..0d68f04164 100644 --- a/lms/djangoapps/verify_student/tests/test_views.py +++ b/lms/djangoapps/verify_student/tests/test_views.py @@ -89,14 +89,14 @@ class TestVerifiedView(TestCase): def setUp(self): self.user = UserFactory.create(username="abc", password="test") self.client.login(username="abc", password="test") - self.course_id = "MITx/999.1x/Verified_Course" self.course = CourseFactory.create(org='MITx', number='999.1x', display_name='Verified Course') + self.course_id = self.course.id def test_verified_course_mode_none(self): """ Test VerifiedView when there is no active verified mode for course. """ - url = reverse('verify_student_verified', kwargs={"course_id": self.course_id}) + url = reverse('verify_student_verified', kwargs={"course_id": self.course_id.to_deprecated_string()}) verify_mode = CourseMode.mode_for_course(self.course_id, "verified") # Verify mode should be None. @@ -161,8 +161,8 @@ class TestPhotoVerificationResultsCallback(TestCase): Tests for the results_callback view. """ def setUp(self): - self.course_id = 'Robot/999/Test_Course' - CourseFactory.create(org='Robot', number='999', display_name='Test Course') + self.course = CourseFactory.create(org='Robot', number='999', display_name='Test Course') + self.course_id = self.course.id self.user = UserFactory.create() self.attempt = SoftwareSecurePhotoVerification( status="submitted", diff --git a/lms/djangoapps/verify_student/views.py b/lms/djangoapps/verify_student/views.py index f0a2673ae3..e5b08ad318 100644 --- a/lms/djangoapps/verify_student/views.py +++ b/lms/djangoapps/verify_student/views.py @@ -271,7 +271,6 @@ def results_callback(request): # If this is a reverification, log an event if attempt.window: course_id = attempt.window.course_id - course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id) course = course_from_id(course_id) course_enrollment = CourseEnrollment.get_or_create_enrollment(attempt.user, course_id) course_enrollment.emit_event(EVENT_NAME_USER_REVERIFICATION_REVIEWED_BY_SOFTWARESECURE)