BLD-364: Merge "video sources" and "download video" fields.

2013-12-23 13:03:18 +02:00
parent 98eecf1369
commit 645007f910
7 changed files with 457 additions and 203 deletions
--- a/common/lib/xmodule/xmodule/tests/test_video.py
+++ b/common/lib/xmodule/xmodule/tests/test_video.py
@@ -187,6 +187,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
                   youtube="1.0:p2Q6BrNhdh8,0.75:izygArpw-Qo,1.25:1EeWXzPdhSA,1.5:rABDYkeK0x8"
                   show_captions="false"
                   download_track="true"
+                   download_video="true"
                   start_time="00:00:01"
                   end_time="00:01:00">
              <source src="http://www.example.com/source.mp4"/>
@@ -207,6 +208,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
            'youtube_id_1_0': 'p2Q6BrNhdh8',
            'youtube_id_1_25': '1EeWXzPdhSA',
            'youtube_id_1_5': 'rABDYkeK0x8',
+            'download_video': True,
            'show_captions': False,
            'start_time': datetime.timedelta(seconds=1),
            'end_time': datetime.timedelta(seconds=60),
@@ -224,6 +226,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
                   show_captions="false"
                   download_track="false"
                   start_time="00:00:01"
+                   download_video="false"
                   end_time="00:01:00">
              <source src="http://www.example.com/source.mp4"/>
              <track src="http://www.example.com/track"/>
@@ -240,7 +243,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
            'end_time': datetime.timedelta(seconds=60),
            'track': 'http://www.example.com/track',
            'download_track': False,
-            'source': 'http://www.example.com/source.mp4',
+            'download_video': False,
            'html5_sources': ['http://www.example.com/source.mp4'],
            'data': ''
        })
@@ -269,7 +272,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
            'end_time': datetime.timedelta(seconds=0.0),
            'track': '',
            'download_track': False,
-            'source': 'http://www.example.com/source.mp4',
+            'download_video': False,
            'html5_sources': ['http://www.example.com/source.mp4'],
            'data': ''
        })
@@ -291,7 +294,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
            'end_time': datetime.timedelta(seconds=0.0),
            'track': '',
            'download_track': False,
-            'source': '',
+            'download_video': False,
            'html5_sources': [],
            'data': ''
        })
@@ -306,7 +309,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
            <video display_name="&quot;display_name&quot;"
                html5_sources="[&quot;source_1&quot;, &quot;source_2&quot;]"
                show_captions="false"
-                source="&quot;http://download_video&quot;"
+                download_video="true"
                sub="&quot;html5_subtitles&quot;"
                track="&quot;http://download_track&quot;"
                download_track="true"
@@ -327,7 +330,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
            'end_time': datetime.timedelta(seconds=0.0),
            'track': 'http://download_track',
            'download_track': True,
-            'source': 'http://download_video',
+            'download_video': True,
            'html5_sources': ["source_1", "source_2"],
            'data': ''
        })
@@ -350,7 +353,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
            'end_time': datetime.timedelta(seconds=0.0),
            'track': '',
            'download_track': False,
-            'source': '',
+            'download_video': False,
            'html5_sources': [],
            'data': ''
        })
@@ -364,6 +367,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
            <video display_name="Test Video"
                   youtube="1.0:p2Q6BrNhdh8,0.75:izygArpw-Qo,1.25:1EeWXzPdhSA,1.5:rABDYkeK0x8"
                   show_captions="false"
+                   source="http://www.example.com/source.mp4"
                   from="00:00:01"
                   to="00:01:00">
              <source src="http://www.example.com/source.mp4"/>
@@ -382,7 +386,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
            'track': 'http://www.example.com/track',
            'download_track': True,
            'html5_sources': ['http://www.example.com/source.mp4'],
-            'data': ''
+            'data': '',
        })

    def test_old_video_data(self):
@@ -473,10 +477,11 @@ class VideoExportTestCase(unittest.TestCase):
        desc.track = 'http://www.example.com/track'
        desc.download_track = True
        desc.html5_sources = ['http://www.example.com/source.mp4', 'http://www.example.com/source.ogg']
+        desc.download_video = True

        xml = desc.definition_to_xml(None)  # We don't use the `resource_fs` parameter
        expected = etree.fromstring('''\
-         <video url_name="SampleProblem1" 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_track="true">
+         <video url_name="SampleProblem1" 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"/>
           <track src="http://www.example.com/track"/>
@@ -501,10 +506,11 @@ class VideoExportTestCase(unittest.TestCase):
        desc.track = 'http://www.example.com/track'
        desc.download_track = True
        desc.html5_sources = ['http://www.example.com/source.mp4', 'http://www.example.com/source.ogg']
+        desc.download_video = True

        xml = desc.definition_to_xml(None)  # We don't use the `resource_fs` parameter
        expected = etree.fromstring('''\
-         <video url_name="SampleProblem1" start_time="0:00:05" youtube="0.75:izygArpw-Qo,1.00:p2Q6BrNhdh8,1.25:1EeWXzPdhSA,1.50:rABDYkeK0x8" show_captions="false" download_track="true">
+         <video url_name="SampleProblem1" start_time="0:00:05" youtube="0.75:izygArpw-Qo,1.00:p2Q6BrNhdh8,1.25:1EeWXzPdhSA,1.50:rABDYkeK0x8" show_captions="false" download_video="true" download_track="true">
           <source src="http://www.example.com/source.mp4"/>
           <source src="http://www.example.com/source.ogg"/>
           <track src="http://www.example.com/track"/>
--- a/common/lib/xmodule/xmodule/video_module.py
+++ b/common/lib/xmodule/xmodule/video_module.py
@@ -98,12 +98,20 @@ class VideoFields(object):
    )
    #front-end code of video player checks logical validity of (start_time, end_time) pair.

+    # `source` is deprecated field and should not be used in future.
+    # `download_video` is used instead.
    source = String(
-        help="The external URL to download the video. This appears as a link beneath the video.",
+        help="The external URL to download the video.",
        display_name="Download Video",
        scope=Scope.settings,
        default=""
    )
+    download_video = Boolean(
+        help="Show a link beneath the video to allow students to download the video. Note: You must add at least one video source below.",
+        display_name="Video Download Allowed",
+        scope=Scope.settings,
+        default=False
+    )
    html5_sources = List(
        help="A list of filenames to be used with HTML5 video. The first supported filetype will be displayed.",
        display_name="Video Sources",
@@ -181,7 +189,12 @@ class VideoModule(VideoFields, XModule):

        get_ext = lambda filename: filename.rpartition('.')[-1]
        sources = {get_ext(src): src for src in self.html5_sources}
-        sources['main'] = self.source
+
+        if self.download_video:
+            if self.source:
+                sources['main'] = self.source
+            elif self.html5_sources:
+                sources['main'] = self.html5_sources[0]

        if self.download_track:
            if self.track:
@@ -281,6 +294,14 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
        If `track` field exists show `track` field on front-end as not-editable
        but clearable. Dropdown `download_track` is a new field and it has value
        True.
+
+        `source` is deprecated field.
+        a) If `source` exists and `source` is not `html5_sources`: show `source`
+            field on front-end as not-editable but clearable. Dropdown is a new
+            field `download_video` and it has value True.
+        b) If `source` is cleared it is not shown anymore.
+        c) If `source` exists and `source` in `html5_sources`, do not show `source`
+            field. `download_video` field has value True.
        '''
        super(VideoDescriptor, self).__init__(*args, **kwargs)
        # For backwards compatibility -- if we've got XML data, parse
@@ -290,21 +311,43 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
            self._field_data.set_many(self, field_data)
            del self.data

+        editable_fields = self.editable_metadata_fields
+
        self.track_visible = False
        if self.track:
            self.track_visible = True
-            download_track = self.editable_metadata_fields['download_track']
+            download_track = editable_fields['download_track']
            if not download_track['explicitly_set']:
                self.download_track = True

+        self.source_visible = False
+        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
+                download_video = editable_fields['download_video']
+                if not download_video['explicitly_set']:
+                    self.download_video = True
+
    @property
    def editable_metadata_fields(self):
        editable_fields = super(VideoDescriptor, self).editable_metadata_fields

-        if self.track_visible:
-            editable_fields['track']['non_editable'] = True
-        else:
-            editable_fields.pop('track')
+        if hasattr(self, 'track_visible'):
+            if self.track_visible:
+                editable_fields['track']['non_editable'] = True
+            else:
+                editable_fields.pop('track')
+
+        if hasattr(self, 'source_visible'):
+            if self.source_visible:
+                editable_fields['source']['non_editable'] = True
+            else:
+                editable_fields.pop('source')

        return editable_fields

@@ -359,6 +402,7 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
            'end_time': self.end_time,
            'sub': self.sub,
            'download_track': json.dumps(self.download_track),
+            'download_video': json.dumps(self.download_video),
        }
        for key, value in attrs.items():
            # Mild workaround to ensure that tests pass -- if a field
@@ -468,7 +512,6 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
        sources = xml.findall('source')
        if sources:
            field_data['html5_sources'] = [ele.get('src') for ele in sources]
-            field_data['source'] = field_data['html5_sources'][0]

        track = xml.find('track')
        if track is not None: