From f9476ab4ff189d35e4dd55629fa5796695b01428 Mon Sep 17 00:00:00 2001
From: Qubad786 <muhammadrehan69@gmail.com>
Date: Mon, 21 May 2018 18:35:38 +0500
Subject: [PATCH 1/2] Update old transcripts metadata present in video xml with
 the new transcripts for backward compatibility.

---
 .../lib/xmodule/xmodule/tests/test_video.py   | 20 +++++-
 .../xmodule/video_module/video_module.py      | 47 +++++++++----
 common/lib/xmodule/xmodule/xml_module.py      | 21 ++++--
 .../courseware/tests/test_video_mongo.py      | 70 ++++++++++++++++++-
 requirements/edx/base.in                      |  2 +-
 requirements/edx/base.txt                     |  2 +-
 requirements/edx/development.txt              |  2 +-
 requirements/edx/github.in                    |  2 +
 requirements/edx/testing.txt                  |  5 +-
 9 files changed, 144 insertions(+), 27 deletions(-)

diff --git a/common/lib/xmodule/xmodule/tests/test_video.py b/common/lib/xmodule/xmodule/tests/test_video.py
index 0e2db392a2..1addb31451 100644
--- a/common/lib/xmodule/xmodule/tests/test_video.py
+++ b/common/lib/xmodule/xmodule/tests/test_video.py
@@ -716,7 +716,12 @@ class VideoExportTestCase(VideoDescriptorTestBase):
         Test that we write the correct XML on export.
         """
         edx_video_id = u'test_edx_video_id'
-        mock_val_api.export_to_xml = Mock(return_value=etree.Element('video_asset'))
+        mock_val_api.export_to_xml = Mock(
+            return_value=dict(
+                xml=etree.Element('video_asset'),
+                transcripts={}
+            )
+        )
         self.descriptor.youtube_id_0_75 = 'izygArpw-Qo'
         self.descriptor.youtube_id_1_0 = 'p2Q6BrNhdh8'
         self.descriptor.youtube_id_1_25 = '1EeWXzPdhSA'
@@ -736,14 +741,23 @@ class VideoExportTestCase(VideoDescriptorTestBase):
         xml = self.descriptor.definition_to_xml(self.file_system)
         parser = etree.XMLParser(remove_blank_text=True)
         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">
+         <video
+            url_name="SampleProblem"
+            start_time="0:00:01"
+            show_captions="false"
+            end_time="0:01:00"
+            download_video="true"
+            download_track="true"
+            youtube="0.75:izygArpw-Qo,1.00:p2Q6BrNhdh8,1.25:1EeWXzPdhSA,1.50:rABDYkeK0x8"
+            transcripts='{"ge": "german_translation.srt", "ua": "ukrainian_translation.srt"}'
+         >
            <source src="http://www.example.com/source.mp4"/>
            <source src="http://www.example.com/source1.ogg"/>
            <track src="http://www.example.com/track"/>
            <handout src="http://www.example.com/handout"/>
+           <video_asset />
            <transcript language="ge" src="german_translation.srt" />
            <transcript language="ua" src="ukrainian_translation.srt" />
-           <video_asset />
          </video>
         '''
         expected = etree.XML(xml_string, parser=parser)
diff --git a/common/lib/xmodule/xmodule/video_module/video_module.py b/common/lib/xmodule/xmodule/video_module/video_module.py
index 34dc33938d..ee5e29be1e 100644
--- a/common/lib/xmodule/xmodule/video_module/video_module.py
+++ b/common/lib/xmodule/xmodule/video_module/video_module.py
@@ -739,13 +739,9 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
             ele.set('src', self.handout)
             xml.append(ele)
 
+        transcripts = {}
         if self.transcripts is not None:
-            # sorting for easy testing of resulting xml
-            for transcript_language in sorted(self.transcripts.keys()):
-                ele = etree.Element('transcript')
-                ele.set('language', transcript_language)
-                ele.set('src', self.transcripts[transcript_language])
-                xml.append(ele)
+            transcripts.update(self.transcripts)
 
         edx_video_id = clean_video_id(self.edx_video_id)
         if edxval_api and edx_video_id:
@@ -753,17 +749,42 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
                 # Create static dir if not created earlier.
                 resource_fs.makedirs(EXPORT_IMPORT_STATIC_DIR, recreate=True)
 
-                xml.append(
-                    edxval_api.export_to_xml(
-                        video_id=edx_video_id,
-                        resource_fs=resource_fs,
-                        static_dir=EXPORT_IMPORT_STATIC_DIR,
-                        course_id=unicode(self.runtime.course_id.for_branch(None))
-                    )
+                # Backward compatible exports
+                # edxval exports new transcripts into the course OLX and returns a transcript
+                # files map so that it can also be rewritten in old transcript metadata fields
+                # (i.e. `self.transcripts`) on import and older open-releases (<= ginkgo),
+                # who do not have deprecated contentstore yet, can also import and use new-style
+                # transcripts into their openedX instances.
+
+                exported_metadata = edxval_api.export_to_xml(
+                    video_id=edx_video_id,
+                    resource_fs=resource_fs,
+                    static_dir=EXPORT_IMPORT_STATIC_DIR,
+                    course_id=unicode(self.runtime.course_id.for_branch(None))
                 )
+                # Update xml with edxval metadata
+                xml.append(exported_metadata['xml'])
+
+                # we don't need sub if english transcript
+                # is also in new transcripts.
+                new_transcripts = exported_metadata['transcripts']
+                transcripts.update(new_transcripts)
+                if new_transcripts.get('en'):
+                    xml.set('sub', '')
+
+                # Update `transcripts` attribute in the xml
+                xml.set('transcripts', json.dumps(transcripts))
+
             except edxval_api.ValVideoNotFoundError:
                 pass
 
+            # Sorting transcripts for easy testing of resulting xml
+            for transcript_language in sorted(transcripts.keys()):
+                ele = etree.Element('transcript')
+                ele.set('language', transcript_language)
+                ele.set('src', transcripts[transcript_language])
+                xml.append(ele)
+
         # handle license specifically
         self.add_license_to_xml(xml)
 
diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py
index b56847532f..1c6e16fe01 100644
--- a/common/lib/xmodule/xmodule/xml_module.py
+++ b/common/lib/xmodule/xmodule/xml_module.py
@@ -139,6 +139,14 @@ class XmlParserMixin(object):
                          # Used for storing xml attributes between import and export, for roundtrips
                          'xml_attributes')
 
+    # This is a categories to fields map that contains the block category specific fields which should not be
+    # cleaned and/or override while adding xml to node.
+    metadata_to_not_to_clean = {
+        # A category `video` having `sub` and `transcripts` fields
+        # which should not be cleaned/override in an xml object.
+        'video': ('sub', 'transcripts')
+    }
+
     metadata_to_export_to_policy = ('discussion_topics',)
 
     @staticmethod
@@ -165,13 +173,15 @@ class XmlParserMixin(object):
         raise NotImplementedError("%s does not implement definition_from_xml" % cls.__name__)
 
     @classmethod
-    def clean_metadata_from_xml(cls, xml_object):
+    def clean_metadata_from_xml(cls, xml_object, excluded_fields=()):
         """
         Remove any attribute named for a field with scope Scope.settings from the supplied
         xml_object
         """
         for field_name, field in cls.fields.items():
-            if field.scope == Scope.settings and xml_object.get(field_name) is not None:
+            if (field.scope == Scope.settings
+                    and field_name not in excluded_fields
+                    and xml_object.get(field_name) is not None):
                 del xml_object.attrib[field_name]
 
     @classmethod
@@ -448,7 +458,8 @@ class XmlParserMixin(object):
                 aside.add_xml_to_node(aside_node)
                 xml_object.append(aside_node)
 
-        self.clean_metadata_from_xml(xml_object)
+        not_to_clean_fields = self.metadata_to_not_to_clean.get(self.category, ())
+        self.clean_metadata_from_xml(xml_object, excluded_fields=not_to_clean_fields)
 
         # Set the tag on both nodes so we get the file path right.
         xml_object.tag = self.category
@@ -457,7 +468,9 @@ class XmlParserMixin(object):
         # Add the non-inherited metadata
         for attr in sorted(own_metadata(self)):
             # don't want e.g. data_dir
-            if attr not in self.metadata_to_strip and attr not in self.metadata_to_export_to_policy:
+            if (attr not in self.metadata_to_strip
+                    and attr not in self.metadata_to_export_to_policy
+                    and attr not in not_to_clean_fields):
                 val = serialize_field(self._field_data.get(self, attr))
                 try:
                     xml_object.set(attr, val)
diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py
index c799fb219c..ca6450e1a9 100644
--- a/lms/djangoapps/courseware/tests/test_video_mongo.py
+++ b/lms/djangoapps/courseware/tests/test_video_mongo.py
@@ -20,6 +20,7 @@ from fs.path import combine
 from edxval.api import (
     ValCannotCreateError,
     ValVideoNotFoundError,
+    create_video_transcript,
     create_or_update_video_transcript,
     create_profile,
     create_video,
@@ -54,7 +55,7 @@ MODULESTORES = {
     ModuleStoreEnum.Type.split: TEST_DATA_SPLIT_MODULESTORE,
 }
 
-TRANSCRIPT_FILE_SRT_DATA = """
+TRANSCRIPT_FILE_SRT_DATA = u"""
 1
 00:00:14,370 --> 00:00:16,530
 I am overwatch.
@@ -1589,16 +1590,19 @@ class VideoDescriptorTest(TestCase, VideoDescriptorTestBase):
 
         actual = self.descriptor.definition_to_xml(resource_fs=self.file_system)
         expected_str = """
-            <video download_video="false" url_name="SampleProblem">
+            <video download_video="false" url_name="SampleProblem" transcripts='{transcripts}'>
                 <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" language_code="{language_code}" provider="Cielo24"/>
                     </transcripts>
                 </video_asset>
+                <transcript language="{language_code}" src="{transcript_file}"/>
             </video>
         """.format(
-            language_code=language_code
+            language_code=language_code,
+            transcript_file=transcript_file_name,
+            transcripts=json.dumps({language_code: transcript_file_name})
         )
         parser = etree.XMLParser(remove_blank_text=True)
         expected = etree.XML(expected_str, parser=parser)
@@ -1612,6 +1616,66 @@ class VideoDescriptorTest(TestCase, VideoDescriptorTestBase):
         transcript = get_video_transcript_data(video_id=self.descriptor.edx_video_id, language_code=language_code)
         self.assertEqual(transcript['content'], expected_transcript_content)
 
+    @ddt.data(
+        (['en', 'da'], 'test_sub', ''),
+        (['da'], 'test_sub', 'test_sub')
+    )
+    @ddt.unpack
+    def test_export_val_transcripts_backward_compatibility(self, languages, sub, expected_sub):
+        """
+        Tests new transcripts export for backward compatibility.
+        """
+        self.descriptor.edx_video_id = 'test_video_id'
+        self.descriptor.sub = sub
+
+        # Setup VAL encode profile, video and transcripts
+        create_profile('mobile')
+        create_video({
+            'edx_video_id': self.descriptor.edx_video_id,
+            'client_video_id': 'test_client_video_id',
+            'duration': 111.0,
+            'status': 'dummy',
+            'encoded_videos': [{
+                'profile': 'mobile',
+                'url': 'http://example.com/video',
+                'file_size': 222,
+                'bitrate': 333,
+            }],
+        })
+
+        for language in languages:
+            create_video_transcript(
+                video_id=self.descriptor.edx_video_id,
+                language_code=language,
+                file_format=Transcript.SRT,
+                content=ContentFile(TRANSCRIPT_FILE_SRT_DATA)
+            )
+
+        # Export the video module into xml
+        video_xml = self.descriptor.definition_to_xml(resource_fs=self.file_system)
+
+        # Assert `sub` and `transcripts` attribute in the xml
+        self.assertEqual(video_xml.get('sub'), expected_sub)
+
+        expected_transcripts = {
+            language: "{edx_video_id}-{language}.srt".format(
+                edx_video_id=self.descriptor.edx_video_id,
+                language=language
+            )
+            for language in languages
+        }
+        self.assertDictEqual(json.loads(video_xml.get('transcripts')), expected_transcripts)
+
+        # Assert transcript content from course OLX
+        for language in languages:
+            expected_transcript_path = combine(
+                combine(self.temp_dir, EXPORT_IMPORT_COURSE_DIR),
+                combine(EXPORT_IMPORT_STATIC_DIR, expected_transcripts[language])
+            )
+            expected_transcript_content = File(open(expected_transcript_path)).read()
+            transcript = get_video_transcript_data(video_id=self.descriptor.edx_video_id, language_code=language)
+            self.assertEqual(transcript['content'], expected_transcript_content)
+
     def test_export_val_data_not_found(self):
         """
         Tests that external video export works as expected.
diff --git a/requirements/edx/base.in b/requirements/edx/base.in
index 9b5e6593d0..eb2374cb82 100644
--- a/requirements/edx/base.in
+++ b/requirements/edx/base.in
@@ -82,7 +82,7 @@ edx-rest-api-client
 edx-search
 edx-submissions
 edx-user-state-client
-edxval
+#edxval
 enum34==1.1.6                       # Backport of Enum from Python 3.4+
 event-tracking
 feedparser==5.1.3
diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt
index f862b54004..fa178fcd2c 100644
--- a/requirements/edx/base.txt
+++ b/requirements/edx/base.txt
@@ -127,7 +127,7 @@ edx-rest-api-client==1.7.1
 edx-search==1.1.0
 edx-submissions==2.0.12
 edx-user-state-client==1.0.4
-edxval==0.1.14
+#edxval==0.1.14
 elasticsearch==1.9.0      # via edx-search
 enum34==1.1.6
 event-tracking==0.2.4
diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt
index d70b10c854..27bb82080d 100644
--- a/requirements/edx/development.txt
+++ b/requirements/edx/development.txt
@@ -149,7 +149,7 @@ edx-search==1.1.0
 edx-sphinx-theme==1.3.0
 edx-submissions==2.0.12
 edx-user-state-client==1.0.4
-edxval==0.1.14
+#edxval==0.1.14
 elasticsearch==1.9.0
 enum34==1.1.6
 event-tracking==0.2.4
diff --git a/requirements/edx/github.in b/requirements/edx/github.in
index 798bec4791..6bda38b321 100644
--- a/requirements/edx/github.in
+++ b/requirements/edx/github.in
@@ -98,6 +98,8 @@
 -e git+https://github.com/edx-solutions/xblock-google-drive.git@138e6fa0bf3a2013e904a085b9fed77dab7f3f21#egg=xblock-google-drive
 -e git+https://github.com/edx/xblock-lti-consumer.git@v1.1.8#egg=lti_consumer-xblock==1.1.8
 
+-e git+https://github.com/edx/edx-val.git@mrehan/backward-transcript-export#egg=edxval
+
 # Third Party XBlocks
 
 -e git+https://github.com/mitodl/edx-sga.git@6b2f7aa2a18206023c8407e2c46f86d4b4c3ac96#egg=edx-sga==0.8.0
diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt
index 7df6c40b91..ecccd57678 100644
--- a/requirements/edx/testing.txt
+++ b/requirements/edx/testing.txt
@@ -41,6 +41,9 @@ git+https://github.com/edx-solutions/xblock-drag-and-drop-v2@v2.1.6#egg=xblock-d
 git+https://github.com/open-craft/xblock-poll@7ba819b968fe8faddb78bb22e1fe7637005eb414#egg=xblock-poll==1.2.7
 git+https://github.com/edx/xblock-utils.git@v1.1.1#egg=xblock-utils==1.1.1
 -e common/lib/xmodule
+
+-e git+https://github.com/edx/edx-val.git@mrehan/backward-transcript-export#egg=edxval
+
 amqp==1.4.9
 analytics-python==1.1.0
 anyjson==0.3.3
@@ -143,7 +146,7 @@ edx-rest-api-client==1.7.1
 edx-search==1.1.0
 edx-submissions==2.0.12
 edx-user-state-client==1.0.4
-edxval==0.1.14
+#edxval==0.1.14
 elasticsearch==1.9.0
 enum34==1.1.6
 event-tracking==0.2.4

From 3d01a5b24a39efe2265ca73b4d84a7908ed3335f Mon Sep 17 00:00:00 2001
From: Qubad786 <muhammadrehan69@gmail.com>
Date: Fri, 25 May 2018 16:54:38 +0500
Subject: [PATCH 2/2] bump VAL

---
 requirements/edx/base.in         | 2 +-
 requirements/edx/base.txt        | 2 +-
 requirements/edx/development.txt | 2 +-
 requirements/edx/github.in       | 2 --
 requirements/edx/testing.txt     | 4 +---
 5 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/requirements/edx/base.in b/requirements/edx/base.in
index eb2374cb82..9b5e6593d0 100644
--- a/requirements/edx/base.in
+++ b/requirements/edx/base.in
@@ -82,7 +82,7 @@ edx-rest-api-client
 edx-search
 edx-submissions
 edx-user-state-client
-#edxval
+edxval
 enum34==1.1.6                       # Backport of Enum from Python 3.4+
 event-tracking
 feedparser==5.1.3
diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt
index fa178fcd2c..c5b7991ed2 100644
--- a/requirements/edx/base.txt
+++ b/requirements/edx/base.txt
@@ -127,7 +127,7 @@ edx-rest-api-client==1.7.1
 edx-search==1.1.0
 edx-submissions==2.0.12
 edx-user-state-client==1.0.4
-#edxval==0.1.14
+edxval==0.1.15
 elasticsearch==1.9.0      # via edx-search
 enum34==1.1.6
 event-tracking==0.2.4
diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt
index 27bb82080d..7816b8473b 100644
--- a/requirements/edx/development.txt
+++ b/requirements/edx/development.txt
@@ -149,7 +149,7 @@ edx-search==1.1.0
 edx-sphinx-theme==1.3.0
 edx-submissions==2.0.12
 edx-user-state-client==1.0.4
-#edxval==0.1.14
+edxval==0.1.15
 elasticsearch==1.9.0
 enum34==1.1.6
 event-tracking==0.2.4
diff --git a/requirements/edx/github.in b/requirements/edx/github.in
index 6bda38b321..798bec4791 100644
--- a/requirements/edx/github.in
+++ b/requirements/edx/github.in
@@ -98,8 +98,6 @@
 -e git+https://github.com/edx-solutions/xblock-google-drive.git@138e6fa0bf3a2013e904a085b9fed77dab7f3f21#egg=xblock-google-drive
 -e git+https://github.com/edx/xblock-lti-consumer.git@v1.1.8#egg=lti_consumer-xblock==1.1.8
 
--e git+https://github.com/edx/edx-val.git@mrehan/backward-transcript-export#egg=edxval
-
 # Third Party XBlocks
 
 -e git+https://github.com/mitodl/edx-sga.git@6b2f7aa2a18206023c8407e2c46f86d4b4c3ac96#egg=edx-sga==0.8.0
diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt
index ecccd57678..940554f8f4 100644
--- a/requirements/edx/testing.txt
+++ b/requirements/edx/testing.txt
@@ -42,8 +42,6 @@ git+https://github.com/open-craft/xblock-poll@7ba819b968fe8faddb78bb22e1fe763700
 git+https://github.com/edx/xblock-utils.git@v1.1.1#egg=xblock-utils==1.1.1
 -e common/lib/xmodule
 
--e git+https://github.com/edx/edx-val.git@mrehan/backward-transcript-export#egg=edxval
-
 amqp==1.4.9
 analytics-python==1.1.0
 anyjson==0.3.3
@@ -146,7 +144,7 @@ edx-rest-api-client==1.7.1
 edx-search==1.1.0
 edx-submissions==2.0.12
 edx-user-state-client==1.0.4
-#edxval==0.1.14
+edxval==0.1.15
 elasticsearch==1.9.0
 enum34==1.1.6
 event-tracking==0.2.4