Merge pull request #4105 from edx/alex/remove_unused_code_when_downloading_from_youtube

Remove unused code when download from youtube.

cms/djangoapps/contentstore/tests/test_transcripts_utils.py

@@ -164,16 +164,27 @@ class TestDownloadYoutubeSubs(ModuleStoreTestCase):
     number = '999'
     display_name = 'Test course'
 
+
+    def clear_sub_content(self, subs_id):
+        """
+        Remove, if subtitle content exists.
+        """
+        filename = 'subs_{0}.srt.sjson'.format(subs_id)
+        content_location = StaticContent.compute_location(self.course.id, filename)
+        try:
+            content = contentstore().find(content_location)
+            contentstore().delete(content.location)
+        except NotFoundError:
+            pass
+
     def clear_subs_content(self, youtube_subs):
-        """Remove, if subtitles content exists."""
+        """
+        Remove, if subtitles content exists.
+
+        youtube_subs: dict of '{speed: youtube_id}' format for different speeds.
+        """
         for subs_id in youtube_subs.values():
-            filename = 'subs_{0}.srt.sjson'.format(subs_id)
-            content_location = StaticContent.compute_location(self.course.id, filename)
-            try:
-                content = contentstore().find(content_location)
-                contentstore().delete(content.location)
-            except NotFoundError:
-                pass
+            self.clear_sub_content(subs_id)
 
     def setUp(self):
         self.course = CourseFactory.create(
@@ -193,29 +204,22 @@ class TestDownloadYoutubeSubs(ModuleStoreTestCase):
                     <text start="5.43" dur="1.73">Test text 3.</text>
                 </transcript>
         """)
-        good_youtube_subs = {
-            0.5: 'good_id_1',
-            1.0: 'good_id_2',
-            2.0: 'good_id_3'
-        }
-        self.clear_subs_content(good_youtube_subs)
+        good_youtube_sub = 'good_id_2'
+        self.clear_sub_content(good_youtube_sub)
 
         with patch('xmodule.video_module.transcripts_utils.requests.get') as mock_get:
             mock_get.return_value = Mock(status_code=200, text=response, content=response)
             # Check transcripts_utils.GetTranscriptsFromYouTubeException not thrown
-            transcripts_utils.download_youtube_subs(good_youtube_subs, self.course, settings)
+            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_1'})
         mock_get.assert_any_call('http://video.google.com/timedtext', params={'lang': 'en', 'v': 'good_id_2'})
-        mock_get.assert_any_call('http://video.google.com/timedtext', params={'lang': 'en', 'v': 'good_id_3'})
 
-        # Check assets status after importing subtitles.
-        for subs_id in good_youtube_subs.values():
-            filename = 'subs_{0}.srt.sjson'.format(subs_id)
-            content_location = StaticContent.compute_location(self.course.id, filename)
-            self.assertTrue(contentstore().find(content_location))
+        # 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_subs_content(good_youtube_subs)
+        self.clear_sub_content(good_youtube_sub)
 
     def test_subs_for_html5_vid_with_periods(self):
         """
@@ -235,25 +239,21 @@ class TestDownloadYoutubeSubs(ModuleStoreTestCase):
 
         mock_get.return_value = Mock(status_code=404, text='Error 404')
 
-        bad_youtube_subs = {
-            0.5: 'BAD_YOUTUBE_ID1',
-            1.0: 'BAD_YOUTUBE_ID2',
-            2.0: 'BAD_YOUTUBE_ID3'
-        }
-        self.clear_subs_content(bad_youtube_subs)
+        bad_youtube_sub = 'BAD_YOUTUBE_ID2'
+        self.clear_sub_content(bad_youtube_sub)
+
         with self.assertRaises(transcripts_utils.GetTranscriptsFromYouTubeException):
-            transcripts_utils.download_youtube_subs(bad_youtube_subs, self.course, settings)
+            transcripts_utils.download_youtube_subs(bad_youtube_sub, self.course, settings)
 
-        # Check assets status after importing subtitles.
-        for subs_id in bad_youtube_subs.values():
-            filename = 'subs_{0}.srt.sjson'.format(subs_id)
-            content_location = StaticContent.compute_location(
-                self.course.id, filename
-            )
-            with self.assertRaises(NotFoundError):
-                contentstore().find(content_location)
+        # Check asset status after import of transcript.
+        filename = 'subs_{0}.srt.sjson'.format(bad_youtube_sub)
+        content_location = StaticContent.compute_location(
+            self.course.id, filename
+        )
+        with self.assertRaises(NotFoundError):
+            contentstore().find(content_location)
 
-        self.clear_subs_content(bad_youtube_subs)
+        self.clear_sub_content(bad_youtube_sub)
 
     def test_success_downloading_chinese_transcripts(self):
 
@@ -262,13 +262,11 @@ class TestDownloadYoutubeSubs(ModuleStoreTestCase):
         # Re-enable when `requests.get` is patched using `mock.patch`
         raise SkipTest
 
-        good_youtube_subs = {
-            1.0: 'j_jEn79vS3g',  # Chinese, utf-8
-        }
-        self.clear_subs_content(good_youtube_subs)
+        good_youtube_sub = 'j_jEn79vS3g'  # Chinese, utf-8
+        self.clear_sub_content(good_youtube_sub)
 
         # Check transcripts_utils.GetTranscriptsFromYouTubeException not thrown
-        transcripts_utils.download_youtube_subs(good_youtube_subs, self.course, settings)
+        transcripts_utils.download_youtube_subs(good_youtube_sub, self.course, settings)
 
         # Check assets status after importing subtitles.
         for subs_id in good_youtube_subs.values():
@@ -278,7 +276,7 @@ class TestDownloadYoutubeSubs(ModuleStoreTestCase):
             )
             self.assertTrue(contentstore().find(content_location))
 
-        self.clear_subs_content(good_youtube_subs)
+        self.clear_sub_content(good_youtube_sub)
 
 
 class TestGenerateSubsFromSource(TestDownloadYoutubeSubs):

cms/djangoapps/contentstore/views/transcripts_ajax.py

@@ -402,7 +402,7 @@ def replace_transcripts(request):
         return error_response(response, 'YouTube id {} is not presented in request data.'.format(youtube_id))
 
     try:
-        download_youtube_subs({1.0: youtube_id}, item, settings)
+        download_youtube_subs(youtube_id, item, settings)
     except GetTranscriptsFromYouTubeException as e:
         return error_response(response, e.message)
 

common/lib/xmodule/xmodule/video_module/transcripts_utils.py

@@ -132,67 +132,29 @@ def get_transcripts_from_youtube(youtube_id, settings, i18n):
     return {'start': sub_starts, 'end': sub_ends, 'text': sub_texts}
 
 
-def download_youtube_subs(youtube_subs, item, settings):
+def download_youtube_subs(youtube_id, video_descriptor, settings):
     """
     Download transcripts from Youtube and save them to assets.
 
     Args:
-    youtube_subs: dictionary of `speed: youtube_id` key:value pairs.
-    item: video module instance.
+        youtube_id: str, actual youtube_id of the video.
+        video_descriptor: video descriptor instance.
 
-    Returns: None, if transcripts were successfully downloaded and saved.
-    Otherwise raises GetTranscriptsFromYouTubeException.
-    """
-    i18n = item.runtime.service(item, "i18n")
-    _ = i18n.ugettext
-
-    highest_speed = highest_speed_subs = None
-    missed_speeds = []
-    # Iterate from lowest to highest speed and try to do download transcripts
-    # from the Youtube service.
-    for speed, youtube_id in sorted(youtube_subs.iteritems()):
-        if not youtube_id:
-            continue
-        try:
-            subs = get_transcripts_from_youtube(youtube_id, settings, i18n)
-            if not subs:  # if empty subs are returned
-                raise GetTranscriptsFromYouTubeException
-        except GetTranscriptsFromYouTubeException:
-            missed_speeds.append(speed)
-            continue
-
-        save_subs_to_store(subs, youtube_id, item)
-
-        log.info(
-            "Transcripts for YouTube id %s (speed %s)"
-            "are downloaded and saved.", youtube_id, speed
-        )
+    We save transcripts for 1.0 speed, as for other speed conversion is done on front-end.
 
-        highest_speed = speed
-        highest_speed_subs = subs
+    Returns:
+        None, if transcripts were successfully downloaded and saved.
 
-    if not highest_speed:
-        raise GetTranscriptsFromYouTubeException(_("Can't find any transcripts on the Youtube service."))
+    Raises:
+        GetTranscriptsFromYouTubeException, if fails.
+    """
+    i18n = video_descriptor.runtime.service(video_descriptor, "i18n")
+    _ = i18n.ugettext
 
-    # When we exit from the previous loop, `highest_speed` and `highest_speed_subs`
-    # are the transcripts data for the highest speed available on the
-    # Youtube service. We use the highest speed as main speed for the
-    # generation other transcripts, cause during calculation timestamps
-    # for lower speeds we just use multiplication instead of division.
-    for speed in missed_speeds:  # Generate transcripts for missed speeds.
-        save_subs_to_store(
-            generate_subs(speed, highest_speed, highest_speed_subs),
-            youtube_subs[speed],
-            item
-        )
+    subs = get_transcripts_from_youtube(youtube_id, settings, i18n)
+    save_subs_to_store(subs, youtube_id, video_descriptor)
 
-        log.info(
-            "Transcripts for YouTube id %s (speed %s)"
-            "are generated from YouTube id %s (speed %s) and saved",
-            youtube_subs[speed], speed,
-            youtube_subs[highest_speed],
-            highest_speed
-        )
+    log.info("Transcripts for youtube_id %s for 1.0 speed are downloaded and saved.", youtube_id)
 
 
 def remove_subs_from_store(subs_id, item, lang='en'):