Merge pull request #9924 from edx/shr/bug/TNL-3168-import_from_youtube_html5_subs

html5 transcript exists

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

@@ -37,7 +37,7 @@ from xmodule.raw_module import EmptyDataRawDescriptor
 from xmodule.xml_module import is_pointer_tag, name_to_pathname, deserialize_field
 from xmodule.exceptions import NotFoundError
 
-from .transcripts_utils import VideoTranscriptsMixin
+from .transcripts_utils import VideoTranscriptsMixin, Transcript, get_html5_ids
 from .video_utils import create_youtube_string, get_video_from_cdn, get_poster
 from .bumper_utils import bumperize
 from .video_xfields import VideoFields
@@ -428,6 +428,23 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
         This should be fixed too.
         """
         metadata_was_changed_by_user = old_metadata != own_metadata(self)
+
+        # There is an edge case when old_metadata and own_metadata are same and we are importing transcript from youtube
+        # then there is a syncing issue where html5_subs are not syncing with youtube sub, We can make sync better by
+        # checking if transcript is present for the video and if any html5_ids transcript is not present then trigger
+        # the manage_video_subtitles_save to create the missing transcript with particular html5_id.
+        if not metadata_was_changed_by_user and self.sub and hasattr(self, 'html5_sources'):
+            html5_ids = get_html5_ids(self.html5_sources)
+            for subs_id in html5_ids:
+                try:
+                    Transcript.asset(self.location, subs_id)
+                except NotFoundError:
+                    # If a transcript does not not exist with particular html5_id then there is no need to check other
+                    # html5_ids because we have to create a new transcript with this missing html5_id by turning on
+                    # metadata_was_changed_by_user flag.
+                    metadata_was_changed_by_user = True
+                    break
+
         if metadata_was_changed_by_user:
             manage_video_subtitles_save(
                 self,

lms/djangoapps/courseware/tests/test_video_mongo.py

@@ -4,6 +4,7 @@ import ddt
 import itertools
 import json
 from collections import OrderedDict
+from path import Path as path
 
 from lxml import etree
 from mock import patch, MagicMock, Mock
@@ -17,7 +18,13 @@ from xmodule.video_module import VideoDescriptor, bumper_utils, video_utils
 from xmodule.x_module import STUDENT_VIEW
 from xmodule.tests.test_video import VideoDescriptorTestBase, instantiate_descriptor
 from xmodule.tests.test_import import DummySystem
-
+from xmodule.video_module.transcripts_utils import save_to_store, Transcript
+from xmodule.modulestore.inheritance import own_metadata
+from xmodule.contentstore.content import StaticContent
+from xmodule.exceptions import NotFoundError
+from xmodule.modulestore.tests.django_utils import (
+    TEST_DATA_MONGO_MODULESTORE, TEST_DATA_SPLIT_MODULESTORE
+)
 from edxval.api import (
     create_profile, create_video, get_video_info, ValCannotCreateError, ValVideoNotFoundError
 )
@@ -866,6 +873,73 @@ class TestVideoDescriptorInitialization(BaseTestXmodule):
         self.assertFalse(self.item_descriptor.download_video)
 
 
+@attr('shard_1')
+@ddt.ddt
+class TestEditorSavedMethod(BaseTestXmodule):
+    """
+    Make sure that `editor_saved` method works correctly.
+    """
+    CATEGORY = "video"
+    DATA = SOURCE_XML
+    METADATA = {}
+
+    def setUp(self):
+        super(TestEditorSavedMethod, self).setUp()
+        self.setup_course()
+        self.metadata = {
+            'source': 'http://youtu.be/3_yD_cEKoCk',
+            'html5_sources': ['http://example.org/video.mp4'],
+        }
+        # path to subs_3_yD_cEKoCk.srt.sjson file
+        self.file_name = 'subs_3_yD_cEKoCk.srt.sjson'
+        # pylint: disable=no-value-for-parameter
+        self.test_dir = path(__file__).abspath().dirname().dirname().dirname().dirname().dirname()
+        self.file_path = self.test_dir + '/common/test/data/uploads/' + self.file_name
+
+    @ddt.data(TEST_DATA_MONGO_MODULESTORE, TEST_DATA_SPLIT_MODULESTORE)
+    def test_editor_saved_when_html5_sub_not_exist(self, default_store):
+        """
+        When there is youtube_sub exist but no html5_sub present for
+        html5_sources, editor_saved function will generate new html5_sub
+        for video.
+        """
+        self.MODULESTORE = default_store  # pylint: disable=invalid-name
+        self.initialize_module(metadata=self.metadata)
+        item = self.store.get_item(self.item_descriptor.location)
+        with open(self.file_path, "r") as myfile:
+            save_to_store(myfile.read(), self.file_name, 'text/sjson', item.location)
+        item.sub = "3_yD_cEKoCk"
+        # subs_video.srt.sjson does not exist before calling editor_saved function
+        with self.assertRaises(NotFoundError):
+            Transcript.get_asset(item.location, 'subs_video.srt.sjson')
+        old_metadata = own_metadata(item)
+        # calling editor_saved will generate new file subs_video.srt.sjson for html5_sources
+        item.editor_saved(self.user, old_metadata, None)
+        self.assertIsInstance(Transcript.get_asset(item.location, 'subs_3_yD_cEKoCk.srt.sjson'), StaticContent)
+        self.assertIsInstance(Transcript.get_asset(item.location, 'subs_video.srt.sjson'), StaticContent)
+
+    @ddt.data(TEST_DATA_MONGO_MODULESTORE, TEST_DATA_SPLIT_MODULESTORE)
+    def test_editor_saved_when_youtube_and_html5_subs_exist(self, default_store):
+        """
+        When both youtube_sub and html5_sub already exist then no new
+        sub will be generated by editor_saved function.
+        """
+        self.MODULESTORE = default_store
+        self.initialize_module(metadata=self.metadata)
+        item = self.store.get_item(self.item_descriptor.location)
+        with open(self.file_path, "r") as myfile:
+            save_to_store(myfile.read(), self.file_name, 'text/sjson', item.location)
+            save_to_store(myfile.read(), 'subs_video.srt.sjson', 'text/sjson', item.location)
+        item.sub = "3_yD_cEKoCk"
+        # subs_3_yD_cEKoCk.srt.sjson and subs_video.srt.sjson already exist
+        self.assertIsInstance(Transcript.get_asset(item.location, self.file_name), StaticContent)
+        self.assertIsInstance(Transcript.get_asset(item.location, 'subs_video.srt.sjson'), StaticContent)
+        old_metadata = own_metadata(item)
+        with patch('xmodule.video_module.video_module.manage_video_subtitles_save') as manage_video_subtitles_save:
+            item.editor_saved(self.user, old_metadata, None)
+            self.assertFalse(manage_video_subtitles_save.called)
+
+
 @ddt.ddt
 class TestVideoDescriptorStudentViewJson(TestCase):
     """