diff --git a/cms/djangoapps/contentstore/tests/test_transcripts_utils.py b/cms/djangoapps/contentstore/tests/test_transcripts_utils.py index ef762471d1..82d7a2e91c 100644 --- a/cms/djangoapps/contentstore/tests/test_transcripts_utils.py +++ b/cms/djangoapps/contentstore/tests/test_transcripts_utils.py @@ -270,6 +270,107 @@ class TestDownloadYoutubeSubs(ModuleStoreTestCase): self.clear_sub_content(good_youtube_sub) + @patch('xmodule.video_module.transcripts_utils.requests.get') + def test_get_transcript_name_youtube_server_success(self, mock_get): + """ + Get transcript name from transcript_list fetch from youtube server api + depends on language code, default language in YOUTUBE Text Api is "en" + """ + youtube_text_api = copy.deepcopy(settings.YOUTUBE['TEXT_API']) + youtube_text_api['params']['v'] = 'dummy_video_id' + response_success = """ + + + + + """ + mock_get.return_value = Mock(status_code=200, text=response_success, content=response_success) + + transcript_name = transcripts_utils.youtube_video_transcript_name(youtube_text_api) + self.assertEqual(transcript_name, 'Custom') + + @patch('xmodule.video_module.transcripts_utils.requests.get') + def test_get_transcript_name_youtube_server_no_transcripts(self, mock_get): + """ + When there are no transcripts of video transcript name will be None + """ + youtube_text_api = copy.deepcopy(settings.YOUTUBE['TEXT_API']) + youtube_text_api['params']['v'] = 'dummy_video_id' + response_success = "" + mock_get.return_value = Mock(status_code=200, text=response_success, content=response_success) + + transcript_name = transcripts_utils.youtube_video_transcript_name(youtube_text_api) + self.assertIsNone(transcript_name) + + @patch('xmodule.video_module.transcripts_utils.requests.get') + def test_get_transcript_name_youtube_server_language_not_exist(self, mock_get): + """ + When the language does not exist in transcript_list transcript name will be None + """ + youtube_text_api = copy.deepcopy(settings.YOUTUBE['TEXT_API']) + youtube_text_api['params']['v'] = 'dummy_video_id' + youtube_text_api['params']['lang'] = 'abc' + response_success = """ + + + + + """ + mock_get.return_value = Mock(status_code=200, text=response_success, content=response_success) + + transcript_name = transcripts_utils.youtube_video_transcript_name(youtube_text_api) + self.assertIsNone(transcript_name) + + def mocked_requests_get(*args, **kwargs): + """ + This method will be used by the mock to replace requests.get + """ + # pylint: disable=no-method-argument + response_transcript_list = """ + + + + + """ + response_transcript = textwrap.dedent(""" + + + Test text 1. + Test text 2. + Test text 3. + + """) + + if kwargs == {'params': {'lang': 'en', 'v': 'good_id_2'}}: + return Mock(status_code=200, text='') + elif kwargs == {'params': {'type': 'list', 'v': 'good_id_2'}}: + return Mock(status_code=200, text=response_transcript_list, content=response_transcript_list) + elif kwargs == {'params': {'lang': 'en', 'v': 'good_id_2', 'name': 'Custom'}}: + return Mock(status_code=200, text=response_transcript, content=response_transcript) + + return Mock(status_code=404, text='') + + @patch('xmodule.video_module.transcripts_utils.requests.get', side_effect=mocked_requests_get) + def test_downloading_subs_using_transcript_name(self, mock_get): + """ + Download transcript using transcript name in url + """ + good_youtube_sub = 'good_id_2' + self.clear_sub_content(good_youtube_sub) + + transcripts_utils.download_youtube_subs(good_youtube_sub, self.course, settings) + mock_get.assert_any_call( + 'http://video.google.com/timedtext', + params={'lang': 'en', 'v': 'good_id_2', 'name': 'Custom'} + ) + + # Check asset status after import of transcript. + filename = 'subs_{0}.srt.sjson'.format(good_youtube_sub) + content_location = StaticContent.compute_location(self.course.id, filename) + self.assertTrue(contentstore().find(content_location)) + + self.clear_sub_content(good_youtube_sub) + class TestGenerateSubsFromSource(TestDownloadYoutubeSubs): """Tests for `generate_subs_from_source` function.""" diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index ae197ee0f2..f0d7b80128 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -332,13 +332,14 @@ def xblock_outline_handler(request, usage_key_string): response_format = request.REQUEST.get('format', 'html') if response_format == 'json' or 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'): store = modulestore() - root_xblock = store.get_item(usage_key) - return JsonResponse(create_xblock_info( - root_xblock, - include_child_info=True, - course_outline=True, - include_children_predicate=lambda xblock: not xblock.category == 'vertical' - )) + with store.bulk_operations(usage_key.course_key): + root_xblock = store.get_item(usage_key) + return JsonResponse(create_xblock_info( + root_xblock, + include_child_info=True, + course_outline=True, + include_children_predicate=lambda xblock: not xblock.category == 'vertical' + )) else: return Http404 diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 3632fd4d75..c29bee1f1b 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -1410,6 +1410,28 @@ class TestXBlockInfo(ItemTest): json_response = json.loads(resp.content) self.validate_course_xblock_info(json_response, course_outline=True) + def test_xblock_outline_handler_mongo_calls(self): + expected_calls = 5 + with self.store.default_store(ModuleStoreEnum.Type.split): + course = CourseFactory.create() + chapter = ItemFactory.create( + parent_location=course.location, category='chapter', display_name='Week 1' + ) + outline_url = reverse_usage_url('xblock_outline_handler', chapter.location) + with check_mongo_calls(expected_calls): + self.client.get(outline_url, HTTP_ACCEPT='application/json') + + sequential = ItemFactory.create( + parent_location=chapter.location, category='sequential', display_name='Sequential 1' + ) + + ItemFactory.create( + parent_location=sequential.location, category='vertical', display_name='Vertical 1' + ) + # calls should be same after adding two new children. + with check_mongo_calls(expected_calls): + self.client.get(outline_url, HTTP_ACCEPT='application/json') + def test_entrance_exam_chapter_xblock_info(self): chapter = ItemFactory.create( parent_location=self.course.location, category='chapter', display_name="Entrance Exam", diff --git a/common/djangoapps/enrollment/tests/test_views.py b/common/djangoapps/enrollment/tests/test_views.py index a9d97308de..5d4355fd04 100644 --- a/common/djangoapps/enrollment/tests/test_views.py +++ b/common/djangoapps/enrollment/tests/test_views.py @@ -2,6 +2,7 @@ Tests for user enrollment. """ import json +import itertools import unittest import datetime @@ -91,11 +92,11 @@ class EnrollmentTestMixin(object): return response - def assert_enrollment_activation(self, expected_activation, expected_mode=CourseMode.VERIFIED): + def assert_enrollment_activation(self, expected_activation, expected_mode): """Change an enrollment's activation and verify its activation and mode are as expected.""" self.assert_enrollment_status( as_server=True, - mode=None, + mode=expected_mode, is_active=expected_activation, expected_status=status.HTTP_200_OK ) @@ -637,6 +638,58 @@ class EnrollmentTest(EnrollmentTestMixin, ModuleStoreTestCase, APITestCase): self.assertTrue(is_active) self.assertEqual(course_mode, CourseMode.HONOR) + @ddt.data(*itertools.product( + (CourseMode.HONOR, CourseMode.VERIFIED), + (CourseMode.HONOR, CourseMode.VERIFIED), + (True, False), + (True, False), + )) + @ddt.unpack + def test_change_mode_from_server(self, old_mode, new_mode, old_is_active, new_is_active): + """ + Server-to-server calls should be allowed to change the mode of any + enrollment, as long as the enrollment is not being deactivated during + the same call (this is assumed to be an error on the client's side). + """ + for mode in [CourseMode.HONOR, CourseMode.VERIFIED]: + CourseModeFactory.create( + course_id=self.course.id, + mode_slug=mode, + mode_display_name=mode, + ) + + # Set up the initial enrollment + self.assert_enrollment_status(as_server=True, mode=old_mode, is_active=old_is_active) + course_mode, is_active = CourseEnrollment.enrollment_mode_for_user(self.user, self.course.id) + self.assertEqual(is_active, old_is_active) + self.assertEqual(course_mode, old_mode) + + expected_status = status.HTTP_400_BAD_REQUEST if ( + old_mode != new_mode and + old_is_active != new_is_active and + not new_is_active + ) else status.HTTP_200_OK + + # simulate the server-server api call under test + response = self.assert_enrollment_status( + as_server=True, + mode=new_mode, + is_active=new_is_active, + expected_status=expected_status, + ) + + course_mode, is_active = CourseEnrollment.enrollment_mode_for_user(self.user, self.course.id) + if expected_status == status.HTTP_400_BAD_REQUEST: + # nothing should have changed + self.assertEqual(is_active, old_is_active) + self.assertEqual(course_mode, old_mode) + # error message should contain specific text. Otto checks for this text in the message. + self.assertRegexpMatches(json.loads(response.content)['message'], 'Enrollment mode mismatch') + else: + # call should have succeeded + self.assertEqual(is_active, new_is_active) + self.assertEqual(course_mode, new_mode) + def test_change_mode_invalid_user(self): """ Attempts to change an enrollment for a non-existent user should result in an HTTP 404 for non-server users, diff --git a/common/djangoapps/enrollment/views.py b/common/djangoapps/enrollment/views.py index f8080bedf2..eef151a8b9 100644 --- a/common/djangoapps/enrollment/views.py +++ b/common/djangoapps/enrollment/views.py @@ -3,6 +3,8 @@ The Enrollment API Views should be simple, lean HTTP endpoints for API access. T consist primarily of authentication, request validation, and serialization. """ +import logging + from ipware.ip import get_ip from django.core.exceptions import ObjectDoesNotExist from django.utils.decorators import method_decorator @@ -31,6 +33,9 @@ from enrollment.errors import ( from student.models import User +log = logging.getLogger(__name__) + + class EnrollmentCrossDomainSessionAuth(SessionAuthenticationAllowInactiveUser, SessionAuthenticationCrossDomainCsrf): """Session authentication that allows inactive users and cross-domain requests. """ pass @@ -429,7 +434,18 @@ class EnrollmentListView(APIView, ApiKeyPermissionMixIn): ) enrollment = api.get_enrollment(username, unicode(course_id)) - if has_api_key_permissions and enrollment and enrollment['mode'] != mode: + mode_changed = enrollment and mode is not None and enrollment['mode'] != mode + active_changed = enrollment and is_active is not None and enrollment['is_active'] != is_active + if has_api_key_permissions and (mode_changed or active_changed): + if mode_changed and active_changed and not is_active: + # if the requester wanted to deactivate but specified the wrong mode, fail + # the request (on the assumption that the requester had outdated information + # about the currently active enrollment). + msg = u"Enrollment mode mismatch: active mode={}, requested mode={}. Won't deactivate.".format( + enrollment["mode"], mode + ) + log.warning(msg) + return Response(status=status.HTTP_400_BAD_REQUEST, data={"message": msg}) response = api.update_enrollment(username, unicode(course_id), mode=mode, is_active=is_active) else: # Will reactivate inactive enrollments. diff --git a/common/lib/xmodule/xmodule/js/src/video/01_initialize.js b/common/lib/xmodule/xmodule/js/src/video/01_initialize.js index ad7edc8b56..2a30ceb454 100644 --- a/common/lib/xmodule/xmodule/js/src/video/01_initialize.js +++ b/common/lib/xmodule/xmodule/js/src/video/01_initialize.js @@ -522,12 +522,9 @@ function (VideoPlayer, i18n) { this.youtubeXhr .always(function (json, status) { - var err = $.isPlainObject(json.error) || - ( - status !== 'success' && - status !== 'notmodified' - ); - if (err) { + // It will work for both if statusCode is 200 or 410. + var didSucceed = (json.error && json.error.code === 410) || status === 'success' || status === 'notmodified'; + if (!didSucceed) { console.log( '[Video info]: YouTube returned an error for ' + 'video with id "' + id + '".' diff --git a/common/lib/xmodule/xmodule/video_module/transcripts_utils.py b/common/lib/xmodule/xmodule/video_module/transcripts_utils.py index fd82ed3452..bf7d39b7c9 100644 --- a/common/lib/xmodule/xmodule/video_module/transcripts_utils.py +++ b/common/lib/xmodule/xmodule/video_module/transcripts_utils.py @@ -94,7 +94,32 @@ def save_subs_to_store(subs, subs_id, item, language='en'): return save_to_store(filedata, filename, 'application/json', item.location) -def get_transcripts_from_youtube(youtube_id, settings, i18n): +def youtube_video_transcript_name(youtube_text_api): + """ + Get the transcript name from available transcripts of video + with respect to language from youtube server + """ + # pylint: disable=no-member + utf8_parser = etree.XMLParser(encoding='utf-8') + + transcripts_param = {'type': 'list', 'v': youtube_text_api['params']['v']} + lang = youtube_text_api['params']['lang'] + # get list of transcripts of specific video + # url-form + # http://video.google.com/timedtext?type=list&v={VideoId} + youtube_response = requests.get('http://' + youtube_text_api['url'], params=transcripts_param) + if youtube_response.status_code == 200 and youtube_response.text: + # pylint: disable=no-member + youtube_data = etree.fromstring(youtube_response.content, parser=utf8_parser) + # iterate all transcripts information from youtube server + for element in youtube_data: + # search specific language code such as 'en' in transcripts info list + if element.tag == 'track' and element.get('lang_code', '') == lang: + return element.get('name') + return None + + +def get_transcripts_from_youtube(youtube_id, settings, i18n, youtube_transcript_name=''): """ Gets transcripts from youtube for youtube_id. @@ -109,6 +134,12 @@ def get_transcripts_from_youtube(youtube_id, settings, i18n): youtube_text_api = copy.deepcopy(settings.YOUTUBE['TEXT_API']) youtube_text_api['params']['v'] = youtube_id + # if the transcript name is not empty on youtube server we have to pass + # name param in url in order to get transcript + # example http://video.google.com/timedtext?lang=en&v={VideoId}&name={transcript_name} + youtube_transcript_name = youtube_video_transcript_name(youtube_text_api) + if youtube_transcript_name: + youtube_text_api['params']['name'] = youtube_transcript_name data = requests.get('http://' + youtube_text_api['url'], params=youtube_text_api['params']) if data.status_code != 200 or not data.text: diff --git a/lms/djangoapps/branding/tests/test_page.py b/lms/djangoapps/branding/tests/test_page.py index 199784da06..ac4357e4f0 100644 --- a/lms/djangoapps/branding/tests/test_page.py +++ b/lms/djangoapps/branding/tests/test_page.py @@ -198,6 +198,52 @@ class IndexPageCourseCardsSortingTests(ModuleStoreTestCase): ) self.factory = RequestFactory() + @patch('student.views.render_to_response', RENDER_MOCK) + @patch('courseware.views.render_to_response', RENDER_MOCK) + @patch.dict('django.conf.settings.FEATURES', {'ENABLE_COURSE_DISCOVERY': False}) + def test_course_discovery_off(self): + """ + Asserts that the Course Discovery UI elements follow the + feature flag settings + """ + response = self.client.get('/') + self.assertEqual(response.status_code, 200) + # assert that the course discovery UI is not present + self.assertNotIn('Search for a course', response.content) + + # check the /courses view + response = self.client.get(reverse('branding.views.courses')) + self.assertEqual(response.status_code, 200) + + # assert that the course discovery UI is not present + self.assertNotIn('Search for a course', response.content) + self.assertNotIn('