From 845bb7b88426c315c07b20c69a8e904e9c2bb336 Mon Sep 17 00:00:00 2001
From: muhammad-ammar <mammar@gmail.com>
Date: Mon, 11 Sep 2017 15:06:11 +0500
Subject: [PATCH] transcript import export

EDUCATOR-1216
---
 .../test_cross_modulestore_import_export.py   |   1 +
 .../xmodule/modulestore/tests/test_mongo.py   |   3 +
 .../lib/xmodule/xmodule/tests/test_export.py  |   1 +
 .../lib/xmodule/xmodule/tests/test_video.py   |  31 +++--
 .../xmodule/video_module/video_module.py      |  64 ++++++----
 .../courseware/tests/test_video_mongo.py      | 111 +++++++++++++++++-
 6 files changed, 173 insertions(+), 38 deletions(-)

diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py b/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py
index 042198113f..3fce2798e3 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py
@@ -55,6 +55,7 @@ class CrossStoreXMLRoundtrip(CourseComparisonTest, PartitionTestCase):
         self.export_dir = mkdtemp()
         self.addCleanup(rmtree, self.export_dir, ignore_errors=True)
 
+    @patch('xmodule.video_module.video_module.edxval_api', None)
     @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
     @ddt.data(*itertools.product(
         MODULESTORE_SETUPS,
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py
index ca1239c402..e1801de0ec 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py
@@ -557,6 +557,7 @@ class TestMongoModuleStore(TestMongoModuleStoreBase):
         check_xblock_fields()
         check_mongo_fields()
 
+    @patch('xmodule.video_module.video_module.edxval_api', None)
     @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
     def test_export_course_image(self, _from_json):
         """
@@ -575,6 +576,7 @@ class TestMongoModuleStore(TestMongoModuleStoreBase):
         self.assertTrue(path(root_dir / 'test_export/static/images/course_image.jpg').isfile())
         self.assertTrue(path(root_dir / 'test_export/static/images_course_image.jpg').isfile())
 
+    @patch('xmodule.video_module.video_module.edxval_api', None)
     @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json)
     def test_export_course_image_nondefault(self, _from_json):
         """
@@ -590,6 +592,7 @@ class TestMongoModuleStore(TestMongoModuleStoreBase):
         self.assertTrue(path(root_dir / 'test_export/static/just_a_test.jpg').isfile())
         self.assertFalse(path(root_dir / 'test_export/static/images/course_image.jpg').isfile())
 
+    @patch('xmodule.video_module.video_module.edxval_api', None)
     def test_course_without_image(self):
         """
         Make sure we elegantly passover our code when there isn't a static
diff --git a/common/lib/xmodule/xmodule/tests/test_export.py b/common/lib/xmodule/xmodule/tests/test_export.py
index 4e5c330419..c43e3e4cb1 100644
--- a/common/lib/xmodule/xmodule/tests/test_export.py
+++ b/common/lib/xmodule/xmodule/tests/test_export.py
@@ -68,6 +68,7 @@ class RoundTripTestCase(unittest.TestCase):
         self.temp_dir = mkdtemp()
         self.addCleanup(shutil.rmtree, self.temp_dir)
 
+    @mock.patch('xmodule.video_module.video_module.edxval_api', None)
     @mock.patch('xmodule.course_module.requests.get')
     @ddt.data(
         "toy",
diff --git a/common/lib/xmodule/xmodule/tests/test_video.py b/common/lib/xmodule/xmodule/tests/test_video.py
index 6df6a9f0a0..63b98eda64 100644
--- a/common/lib/xmodule/xmodule/tests/test_video.py
+++ b/common/lib/xmodule/xmodule/tests/test_video.py
@@ -646,7 +646,11 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
         video = VideoDescriptor.from_xml(xml_data, module_system, id_generator)
 
         self.assert_attributes_equal(video, {'edx_video_id': 'test_edx_video_id'})
-        mock_val_api.import_from_xml.assert_called_once_with(ANY, 'test_edx_video_id', course_id='test_course_id')
+        mock_val_api.import_from_xml.assert_called_once_with(
+            ANY,
+            'test_edx_video_id',
+            course_id='test_course_id'
+        )
 
     @patch('xmodule.video_module.video_module.edxval_api')
     def test_import_val_data_invalid(self, mock_val_api):
@@ -673,14 +677,7 @@ class VideoExportTestCase(VideoDescriptorTestBase):
         """
         Test that we write the correct XML on export.
         """
-        def mock_val_export(edx_video_id, course_id):
-            """Mock edxval.api.export_to_xml"""
-            return etree.Element(
-                'video_asset',
-                attrib={'export_edx_video_id': edx_video_id}
-            )
-
-        mock_val_api.export_to_xml = mock_val_export
+        mock_val_api.export_to_xml = Mock(return_value=etree.Element('video_asset'))
         self.descriptor.youtube_id_0_75 = 'izygArpw-Qo'
         self.descriptor.youtube_id_1_0 = 'p2Q6BrNhdh8'
         self.descriptor.youtube_id_1_25 = '1EeWXzPdhSA'
@@ -691,7 +688,7 @@ class VideoExportTestCase(VideoDescriptorTestBase):
         self.descriptor.track = 'http://www.example.com/track'
         self.descriptor.handout = 'http://www.example.com/handout'
         self.descriptor.download_track = True
-        self.descriptor.html5_sources = ['http://www.example.com/source.mp4', 'http://www.example.com/source.ogg']
+        self.descriptor.html5_sources = ['http://www.example.com/source.mp4', 'http://www.example.com/source1.ogg']
         self.descriptor.download_video = True
         self.descriptor.transcripts = {'ua': 'ukrainian_translation.srt', 'ge': 'german_translation.srt'}
         self.descriptor.edx_video_id = 'test_edx_video_id'
@@ -702,16 +699,21 @@ class VideoExportTestCase(VideoDescriptorTestBase):
         xml_string = '''\
          <video url_name="SampleProblem" start_time="0:00:01" youtube="0.75:izygArpw-Qo,1.00:p2Q6BrNhdh8,1.25:1EeWXzPdhSA,1.50:rABDYkeK0x8" show_captions="false" end_time="0:01:00" download_video="true" download_track="true">
            <source src="http://www.example.com/source.mp4"/>
-           <source src="http://www.example.com/source.ogg"/>
+           <source src="http://www.example.com/source1.ogg"/>
            <track src="http://www.example.com/track"/>
            <handout src="http://www.example.com/handout"/>
            <transcript language="ge" src="german_translation.srt" />
            <transcript language="ua" src="ukrainian_translation.srt" />
-           <video_asset export_edx_video_id="test_edx_video_id"/>
+           <video_asset />
          </video>
         '''
         expected = etree.XML(xml_string, parser=parser)
         self.assertXmlEqual(expected, xml)
+        mock_val_api.export_to_xml.assert_called_once_with(
+            [u'test_edx_video_id', u'p2Q6BrNhdh8', 'source', 'source1'],
+            ANY,
+            external=False
+        )
 
     @patch('xmodule.video_module.video_module.edxval_api')
     def test_export_to_xml_val_error(self, mock_val_api):
@@ -727,6 +729,7 @@ class VideoExportTestCase(VideoDescriptorTestBase):
         expected = etree.XML(xml_string, parser=parser)
         self.assertXmlEqual(expected, xml)
 
+    @patch('xmodule.video_module.video_module.edxval_api', None)
     def test_export_to_xml_empty_end_time(self):
         """
         Test that we write the correct XML on export.
@@ -755,6 +758,7 @@ class VideoExportTestCase(VideoDescriptorTestBase):
         expected = etree.XML(xml_string, parser=parser)
         self.assertXmlEqual(expected, xml)
 
+    @patch('xmodule.video_module.video_module.edxval_api', None)
     def test_export_to_xml_empty_parameters(self):
         """
         Test XML export with defaults.
@@ -764,6 +768,7 @@ class VideoExportTestCase(VideoDescriptorTestBase):
         expected = '<video url_name="SampleProblem" download_video="false"/>\n'
         self.assertEquals(expected, etree.tostring(xml, pretty_print=True))
 
+    @patch('xmodule.video_module.video_module.edxval_api', None)
     def test_export_to_xml_with_transcripts_as_none(self):
         """
         Test XML export with transcripts being overridden to None.
@@ -773,6 +778,7 @@ class VideoExportTestCase(VideoDescriptorTestBase):
         expected = '<video url_name="SampleProblem" download_video="false"/>\n'
         self.assertEquals(expected, etree.tostring(xml, pretty_print=True))
 
+    @patch('xmodule.video_module.video_module.edxval_api', None)
     def test_export_to_xml_invalid_characters_in_attributes(self):
         """
         Test XML export will *not* raise TypeError by lxml library if contains illegal characters.
@@ -782,6 +788,7 @@ class VideoExportTestCase(VideoDescriptorTestBase):
         xml = self.descriptor.definition_to_xml(None)
         self.assertEqual(xml.get('display_name'), 'DisplayName')
 
+    @patch('xmodule.video_module.video_module.edxval_api', None)
     def test_export_to_xml_unicode_characters(self):
         """
         Test XML export handles the unicode characters.
diff --git a/common/lib/xmodule/xmodule/video_module/video_module.py b/common/lib/xmodule/xmodule/video_module/video_module.py
index ece09b0e49..59a728c526 100644
--- a/common/lib/xmodule/xmodule/video_module/video_module.py
+++ b/common/lib/xmodule/xmodule/video_module/video_module.py
@@ -41,7 +41,9 @@ from xmodule.x_module import XModule, module_attr
 from xmodule.xml_module import deserialize_field, is_pointer_tag, name_to_pathname
 
 from .bumper_utils import bumperize
-from .transcripts_utils import Transcript, VideoTranscriptsMixin, get_html5_ids
+from .transcripts_utils import (
+    Transcript, VideoTranscriptsMixin, get_html5_ids, get_video_ids_info
+)
 from .video_handlers import VideoStudentViewHandlers, VideoStudioViewHandlers
 from .video_utils import create_youtube_string, format_xml_exception_message, get_poster, rewrite_video_url
 from .video_xfields import VideoFields
@@ -595,6 +597,10 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
             ScopeIds(None, block_type, definition_id, usage_id),
             field_data,
         )
+
+        # update val with info extracted from `xml_object`
+        video.import_video_info_into_val(xml_object, getattr(id_generator, 'target_course_id', None))
+
         return video
 
     def definition_to_xml(self, resource_fs):
@@ -658,14 +664,19 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
                 ele.set('src', self.transcripts[transcript_language])
                 xml.append(ele)
 
-        if self.edx_video_id and edxval_api:
-            try:
-                xml.append(edxval_api.export_to_xml(
-                    self.edx_video_id,
-                    unicode(self.runtime.course_id.for_branch(None)))
-                )
-            except edxval_api.ValVideoNotFoundError:
-                pass
+        if edxval_api:
+            external, video_ids = get_video_ids_info(self.edx_video_id, self.youtube_id_1_0, self.html5_sources)
+            if video_ids:
+                try:
+                    xml.append(
+                        edxval_api.export_to_xml(
+                            video_ids,
+                            unicode(self.runtime.course_id.for_branch(None)),
+                            external=external
+                        )
+                    )
+                except edxval_api.ValVideoNotFoundError:
+                    pass
 
         # handle license specifically
         self.add_license_to_xml(xml)
@@ -864,24 +875,33 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
         if 'download_track' not in field_data and track is not None:
             field_data['download_track'] = True
 
-        video_asset_elem = xml.find('video_asset')
-        if (
-                edxval_api and
-                video_asset_elem is not None and
-                'edx_video_id' in field_data
-        ):
-            # Allow ValCannotCreateError to escape
-            edxval_api.import_from_xml(
-                video_asset_elem,
-                field_data['edx_video_id'],
-                course_id=course_id
-            )
-
         # load license if it exists
         field_data = LicenseMixin.parse_license_from_xml(field_data, xml)
 
         return field_data
 
+    def import_video_info_into_val(self, xml, course_id):
+        """
+        Import parsed video info from `xml` into edxval.
+
+        Arguments:
+            xml (lxml object): xml representation of video to be imported
+            course_id (str): course id
+        """
+        if self.edx_video_id is not None:
+            edx_video_id = self.edx_video_id.strip()
+
+        video_asset_elem = xml.find('video_asset')
+        if edxval_api and video_asset_elem is not None:
+            # Always pass the edx_video_id, Whether the video is internal or external
+            # In case of external, we only need to import transcripts and for that
+            # purpose video id is already present in the xml
+            edxval_api.import_from_xml(
+                video_asset_elem,
+                edx_video_id,
+                course_id=course_id
+            )
+
     def index_dictionary(self):
         xblock_body = super(VideoDescriptor, self).index_dictionary()
         video_body = {
diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py
index 01fed42433..a495f27a46 100644
--- a/lms/djangoapps/courseware/tests/test_video_mongo.py
+++ b/lms/djangoapps/courseware/tests/test_video_mongo.py
@@ -9,7 +9,15 @@ import ddt
 from django.conf import settings
 from django.test import TestCase
 from django.test.utils import override_settings
-from edxval.api import ValCannotCreateError, ValVideoNotFoundError, create_profile, create_video, get_video_info
+from edxval.api import (
+    ValCannotCreateError,
+    ValVideoNotFoundError,
+    create_or_update_video_transcript,
+    create_profile,
+    create_video,
+    get_video_info,
+    get_video_transcript
+)
 from lxml import etree
 from mock import MagicMock, Mock, patch
 from nose.plugins.attrib import attr
@@ -1453,6 +1461,15 @@ class VideoDescriptorTest(TestCase, VideoDescriptorTestBase):
         self.descriptor.runtime.handler_url = MagicMock()
         self.descriptor.runtime.course_id = MagicMock()
 
+    def get_video_transcript_data(self, video_id):
+        return dict(
+            video_id=video_id,
+            language_code='ar',
+            url='/media/ext101.srt',
+            provider='Cielo24',
+            file_format='srt',
+        )
+
     def test_get_context(self):
         """"
         Test get_context.
@@ -1480,7 +1497,7 @@ class VideoDescriptorTest(TestCase, VideoDescriptorTestBase):
             self.descriptor.editable_metadata_fields['edx_video_id']
         )
 
-    def test_export_val_data(self):
+    def test_export_val_data_with_internal(self):
         self.descriptor.edx_video_id = 'test_edx_video_id'
         create_profile('mobile')
         create_video({
@@ -1495,15 +1512,52 @@ class VideoDescriptorTest(TestCase, VideoDescriptorTestBase):
                 'bitrate': 333,
             }],
         })
+        create_or_update_video_transcript(
+            video_id=self.descriptor.edx_video_id,
+            language_code='ar',
+            file_name='ext101.srt',
+            file_format='srt',
+            provider='Cielo24',
+        )
 
         actual = self.descriptor.definition_to_xml(resource_fs=None)
         expected_str = """
             <video download_video="false" url_name="SampleProblem">
                 <video_asset client_video_id="test_client_video_id" duration="111.0" image="">
                     <encoded_video profile="mobile" url="http://example.com/video" file_size="222" bitrate="333"/>
+                    <transcripts>
+                        <transcript file_format="srt" file_name="ext101.srt" language_code="ar" provider="Cielo24" video_id="{video_id}"/>
+                    </transcripts>
                 </video_asset>
             </video>
+        """.format(video_id=self.descriptor.edx_video_id)
+        parser = etree.XMLParser(remove_blank_text=True)
+        expected = etree.XML(expected_str, parser=parser)
+        self.assertXmlEqual(expected, actual)
+
+    def test_export_val_data_with_external(self):
         """
+        Tests exported val data for external video.
+        """
+        external_video_id = '3_yD_cEKoCk'
+        create_or_update_video_transcript(
+            video_id=external_video_id,
+            language_code='ar',
+            file_name='ext101.srt',
+            file_format='srt',
+            provider='Cielo24',
+        )
+
+        actual = self.descriptor.definition_to_xml(resource_fs=None)
+        expected_str = """
+            <video url_name="SampleProblem" download_video="false">
+                <video_asset>
+                    <transcripts>
+                        <transcript file_format="srt" file_name="ext101.srt" language_code="ar" provider="Cielo24" video_id="{video_id}"/>
+                    </transcripts>
+                </video_asset>
+            </video>
+        """.format(video_id=external_video_id)
         parser = etree.XMLParser(remove_blank_text=True)
         expected = etree.XML(expected_str, parser=parser)
         self.assertXmlEqual(expected, actual)
@@ -1516,7 +1570,21 @@ class VideoDescriptorTest(TestCase, VideoDescriptorTestBase):
         expected = etree.XML(expected_str, parser=parser)
         self.assertXmlEqual(expected, actual)
 
-    def test_import_val_data(self):
+    @patch('xmodule.video_module.transcripts_utils.get_video_ids_info')
+    def test_export_no_video_ids(self, mock_get_video_ids_info):
+        """
+        Tests export when there are no video ids
+        """
+        mock_get_video_ids_info.return_value = True, []
+
+        actual = self.descriptor.definition_to_xml(resource_fs=None)
+        expected_str = '<video url_name="SampleProblem" download_video="false"><video_asset/></video>'
+
+        parser = etree.XMLParser(remove_blank_text=True)
+        expected = etree.XML(expected_str, parser=parser)
+        self.assertXmlEqual(expected, actual)
+
+    def test_import_val_data_internal(self):
         create_profile('mobile')
         module_system = DummySystem(load_error_modules=True)
 
@@ -1524,12 +1592,15 @@ class VideoDescriptorTest(TestCase, VideoDescriptorTestBase):
             <video edx_video_id="test_edx_video_id">
                 <video_asset client_video_id="test_client_video_id" duration="111.0">
                     <encoded_video profile="mobile" url="http://example.com/video" file_size="222" bitrate="333"/>
+                    <transcripts>
+                        <transcript file_format="srt" file_name="ext101.srt" language_code="ar" provider="Cielo24"  video_id="test_edx_video_id"/>
+                    </transcripts>
                 </video_asset>
             </video>
         """
         id_generator = Mock()
         id_generator.target_course_id = "test_course_id"
-        video = VideoDescriptor.from_xml(xml_data, module_system, id_generator)
+        video = self.descriptor.from_xml(xml_data, module_system, id_generator)
         self.assertEqual(video.edx_video_id, 'test_edx_video_id')
         video_data = get_video_info(video.edx_video_id)
         self.assertEqual(video_data['client_video_id'], 'test_client_video_id')
@@ -1540,6 +1611,38 @@ class VideoDescriptorTest(TestCase, VideoDescriptorTestBase):
         self.assertEqual(video_data['encoded_videos'][0]['url'], 'http://example.com/video')
         self.assertEqual(video_data['encoded_videos'][0]['file_size'], 222)
         self.assertEqual(video_data['encoded_videos'][0]['bitrate'], 333)
+        # verify transcript data
+        self.assertDictEqual(
+            get_video_transcript(video.edx_video_id, 'ar'),
+            self.get_video_transcript_data('test_edx_video_id')
+        )
+
+    def test_import_val_data_external(self):
+        """
+        Tests video import with external video.
+        """
+        external_video_id = 'external_video_id'
+        module_system = DummySystem(load_error_modules=True)
+
+        xml_data = """
+            <video>
+                <video_asset>
+                    <transcripts>
+                        <transcript file_format="srt" file_name="ext101.srt" language_code="ar" provider="Cielo24" video_id="{video_id}"/>
+                    </transcripts>
+                </video_asset>
+            </video>
+        """.format(video_id=external_video_id)
+
+        id_generator = Mock()
+        id_generator.target_course_id = "test_course_id"
+        self.descriptor.from_xml(xml_data, module_system, id_generator)
+
+        # verify transcript data
+        self.assertDictEqual(
+            get_video_transcript(external_video_id, 'ar'),
+            self.get_video_transcript_data(external_video_id)
+        )
 
     def test_import_val_data_invalid(self):
         create_profile('mobile')