From bdf2c2864169bc9454b98e2e4b726a7e65a890c1 Mon Sep 17 00:00:00 2001
From: zubair-arbi <zubair.afzal@arbisoft.com>
Date: Fri, 19 Sep 2014 16:37:12 +0500
Subject: [PATCH] update video module to always set download_video field on
 export

---
 .../lib/xmodule/xmodule/tests/test_video.py   | 41 ++-----------------
 .../xmodule/video_module/video_module.py      |  8 ++++
 .../commands/tests/test_dump_course.py        |  2 +-
 .../courseware/tests/test_video_mongo.py      |  4 +-
 4 files changed, 15 insertions(+), 40 deletions(-)

diff --git a/common/lib/xmodule/xmodule/tests/test_video.py b/common/lib/xmodule/xmodule/tests/test_video.py
index 0af8212c7f..8763c15225 100644
--- a/common/lib/xmodule/xmodule/tests/test_video.py
+++ b/common/lib/xmodule/xmodule/tests/test_video.py
@@ -202,40 +202,6 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
             'transcripts': {'ua': 'ukrainian_translation.srt', 'ge': 'german_translation.srt'}
         })
 
-    def test_constructor_for_download_video(self):
-        """
-        Test that "download_video" field is False (default value) if not explicitly set in xml
-        """
-        sample_xml = '''
-            <video display_name="Test Video" youtube="1.0:p2Q6BrNhdh8">
-              <source src="http://www.example.com/source.mp4"/>
-            </video>
-        '''
-        descriptor = instantiate_descriptor(data=sample_xml)
-        self.assert_attributes_equal(descriptor, {
-            'youtube_id_1_0': 'p2Q6BrNhdh8',
-            'download_video': False,
-            'html5_sources': ['http://www.example.com/source.mp4'],
-            'data': ''
-        })
-
-        # Now construct video from xml with explicitly set download_video and check that
-        # resulting video descriptor has same value for "download_video"
-        sample_xml = '''
-            <video display_name="Test Video"
-                youtube="1.0:p2Q6BrNhdh8"
-                download_video="true">
-              <source src="http://www.example.com/source.mp4"/>
-            </video>
-        '''
-        descriptor = instantiate_descriptor(data=sample_xml)
-        self.assert_attributes_equal(descriptor, {
-            'youtube_id_1_0': 'p2Q6BrNhdh8',
-            'download_video': True,
-            'html5_sources': ['http://www.example.com/source.mp4'],
-            'data': ''
-        })
-
     def test_from_xml(self):
         module_system = DummySystem(load_error_modules=True)
         xml_data = '''
@@ -296,7 +262,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
             'track': '',
             'handout': None,
             'download_track': False,
-            'download_video': False,
+            'download_video': True,
             'html5_sources': ['http://www.example.com/source.mp4'],
             'data': ''
         })
@@ -326,7 +292,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
             'end_time': datetime.timedelta(seconds=0.0),
             'track': 'http://www.example.com/track',
             'download_track': True,
-            'download_video': False,
+            'download_video': True,
             'html5_sources': ['http://www.example.com/source.mp4'],
             'data': '',
             'transcripts': {},
@@ -583,7 +549,8 @@ class VideoExportTestCase(VideoDescriptorTestBase):
         Test XML export with defaults.
         """
         xml = self.descriptor.definition_to_xml(None)
-        expected = '<video url_name="SampleProblem"/>\n'
+        # Check that download_video field is also set to default (False) in xml for backward compatibility
+        expected = '<video url_name="SampleProblem" download_video="false"/>\n'
         self.assertEquals(expected, etree.tostring(xml, pretty_print=True))
 
 
diff --git a/common/lib/xmodule/xmodule/video_module/video_module.py b/common/lib/xmodule/xmodule/video_module/video_module.py
index 55528c2a52..b19421d17c 100644
--- a/common/lib/xmodule/xmodule/video_module/video_module.py
+++ b/common/lib/xmodule/xmodule/video_module/video_module.py
@@ -230,13 +230,21 @@ class VideoDescriptor(VideoFields, VideoStudioViewHandlers, TabsEditingDescripto
         editable_fields = super(VideoDescriptor, self).editable_metadata_fields
 
         self.source_visible = False
+        # Set download_video field to default value if its not explicitly set for backward compatibility.
+        download_video = editable_fields['download_video']
+        if not download_video['explicitly_set']:
+            self.download_video = self.download_video
+
         if self.source:
             # If `source` field value exist in the `html5_sources` field values,
             # then delete `source` field value and use value from `html5_sources` field.
             if self.source in self.html5_sources:
                 self.source = ''  # Delete source field value.
+                self.download_video = True
             else:  # Otherwise, `source` field value will be used.
                 self.source_visible = True
+                if not download_video['explicitly_set']:
+                    self.download_video = True
 
         # for backward compatibility.
         # If course was existed and was not re-imported by the moment of adding `download_track` field,
diff --git a/lms/djangoapps/courseware/management/commands/tests/test_dump_course.py b/lms/djangoapps/courseware/management/commands/tests/test_dump_course.py
index 2ecf952841..56f6382095 100644
--- a/lms/djangoapps/courseware/management/commands/tests/test_dump_course.py
+++ b/lms/djangoapps/courseware/management/commands/tests/test_dump_course.py
@@ -104,7 +104,7 @@ class CommandsTestBase(TestCase):
 
         video_id = test_course_key.make_usage_key('video', 'Welcome').to_deprecated_string()
         self.assertEqual(dump[video_id]['category'], 'video')
-        self.assertEqual(len(dump[video_id]['metadata']), 4)
+        self.assertEqual(len(dump[video_id]['metadata']), 5)
         self.assertIn('youtube_id_1_0', dump[video_id]['metadata'])
 
         # Check if there are the right number of elements
diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py
index 963c861331..c620f3cd4e 100644
--- a/lms/djangoapps/courseware/tests/test_video_mongo.py
+++ b/lms/djangoapps/courseware/tests/test_video_mongo.py
@@ -487,7 +487,7 @@ class TestVideoDescriptorInitialization(BaseTestXmodule):
 
         self.assertIn('source', fields)
         self.assertEqual(self.item_descriptor.source, 'http://example.org/video.mp4')
-        self.assertFalse(self.item_descriptor.download_video)
+        self.assertTrue(self.item_descriptor.download_video)
         self.assertTrue(self.item_descriptor.source_visible)
 
     def test_source_in_html5sources(self):
@@ -500,7 +500,7 @@ class TestVideoDescriptorInitialization(BaseTestXmodule):
         fields = self.item_descriptor.editable_metadata_fields
 
         self.assertNotIn('source', fields)
-        self.assertFalse(self.item_descriptor.download_video)
+        self.assertTrue(self.item_descriptor.download_video)
         self.assertFalse(self.item_descriptor.source_visible)
 
     def test_download_video_is_explicitly_set(self):