diff --git a/common/lib/xmodule/xmodule/video_module/video_module.py b/common/lib/xmodule/xmodule/video_module/video_module.py index d10b60b22c..2ebbbe31d5 100644 --- a/common/lib/xmodule/xmodule/video_module/video_module.py +++ b/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, diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index a52ad4f16b..6e9523ecf3 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/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): """