Merge branch 'release'

cms/djangoapps/contentstore/features/video-editor.py

@@ -40,11 +40,12 @@ def correct_video_settings(_step):
 
         # advanced
         ['Display Name', 'Video', False],
+        ['Download Transcript', '', False],
         ['End Time', '00:00:00', False],
         ['HTML5 Transcript', '', False],
         ['Show Transcript', 'True', False],
         ['Start Time', '00:00:00', False],
-        ['Transcript Download Allowed', 'False', False],
+        # ['Transcript Download Allowed', 'False', False],
         ['Video Download Allowed', 'False', False],
         ['Video Sources', '', False],
         ['Youtube ID', 'OEoXaMPEzfM', False],

cms/djangoapps/contentstore/views/item.py

@@ -110,7 +110,8 @@ def xblock_handler(request, tag=None, package_id=None, branch=None, version_guid
             accept_header = request.META.get('HTTP_ACCEPT', 'application/json')
 
             if 'application/x-fragment+json' in accept_header:
-                component = modulestore().get_item(old_location)
+                store = get_modulestore(old_location)
+                component = store.get_item(old_location)
 
                 # Wrap the generated fragment in the xmodule_editor div so that the javascript
                 # can bind to it correctly
@@ -125,7 +126,7 @@ def xblock_handler(request, tag=None, package_id=None, branch=None, version_guid
                     log.debug("Unable to render studio_view for %r", component, exc_info=True)
                     editor_fragment = Fragment(render_to_string('html_error.html', {'message': str(exc)}))
 
-                modulestore().save_xmodule(component)
+                store.save_xmodule(component)
 
                 preview_fragment = get_preview_fragment(request, component)
 

cms/djangoapps/contentstore/views/tests/test_item.py

@@ -538,6 +538,41 @@ class TestEditItem(ItemTest):
         draft = self.get_item_from_modulestore(self.problem_locator, True)
         self.assertEqual(draft.due, datetime(2077, 10, 10, 4, 0, tzinfo=UTC))
 
+    def test_published_and_draft_contents_with_update(self):
+        """ Create a draft and publish it then modify the draft and check that published content is not modified """
+
+        # Make problem public.
+        resp = self.client.ajax_post(
+            self.problem_update_url,
+            data={'publish': 'make_public'}
+        )
+        self.assertIsNotNone(self.get_item_from_modulestore(self.problem_locator, False))
+
+        # Now make a draft
+        resp = self.client.ajax_post(
+            self.problem_update_url,
+            data={
+                'id': self.problem_locator,
+                'metadata': {},
+                'data': "<p>Problem content draft.</p>",
+                'publish': 'create_draft'
+            }
+        )
+
+        # Both published and draft content should be different
+        published = self.get_item_from_modulestore(self.problem_locator, False)
+        draft = self.get_item_from_modulestore(self.problem_locator, True)
+        self.assertNotEqual(draft.data, published.data)
+
+        # Get problem by 'xblock_handler'
+        resp = self.client.get('/xblock/' + self.problem_locator, HTTP_ACCEPT='application/x-fragment+json')
+        self.assertEqual(resp.status_code, 200)
+
+        # Both published and draft content should still be different
+        published = self.get_item_from_modulestore(self.problem_locator, False)
+        draft = self.get_item_from_modulestore(self.problem_locator, True)
+        self.assertNotEqual(draft.data, published.data)
+
 
 @ddt.ddt
 class TestComponentHandler(TestCase):

common/lib/xmodule/xmodule/modulestore/mongo/draft.py

@@ -106,6 +106,20 @@ class DraftModuleStore(MongoModuleStore):
             raise InvalidVersionError(location)
         return super(DraftModuleStore, self).create_xmodule(draft_loc, definition_data, metadata, system)
 
+    def save_xmodule(self, xmodule):
+        """
+        Save the given xmodule (will either create or update based on whether id already exists).
+        Pulls out the data definition v metadata v children locally but saves it all.
+
+        :param xmodule:
+        """
+        orig_location = xmodule.location
+
+        xmodule.location = as_draft(orig_location)
+        try:
+            super(DraftModuleStore, self).save_xmodule(xmodule)
+        finally:
+            xmodule.location = orig_location
 
     def get_items(self, location, course_id=None, depth=0, qualifiers=None):
         """

common/lib/xmodule/xmodule/tests/test_video.py

@@ -272,7 +272,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
             'end_time': datetime.timedelta(seconds=0.0),
             'track': '',
             'download_track': False,
-            'download_video': False,
+            'download_video': True,
             'html5_sources': ['http://www.example.com/source.mp4'],
             'data': ''
         })
@@ -384,7 +384,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
             'start_time': datetime.timedelta(seconds=1),
             'end_time': datetime.timedelta(seconds=60),
             'track': 'http://www.example.com/track',
-            'download_track': True,
+            # 'download_track': True,
             'html5_sources': ['http://www.example.com/source.mp4'],
             'data': '',
         })
@@ -414,7 +414,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
             'start_time': datetime.timedelta(seconds=1),
             'end_time': datetime.timedelta(seconds=60),
             'track': 'http://www.example.com/track',
-            'download_track': True,
+            # 'download_track': True,
             'html5_sources': ['http://www.example.com/source.mp4'],
             'data': ''
         })
@@ -444,7 +444,7 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
             'start_time': datetime.timedelta(seconds=1),
             'end_time': datetime.timedelta(seconds=60),
             'track': 'http://www.example.com/track',
-            'download_track': True,
+            # 'download_track': True,
             'html5_sources': ['http://www.example.com/source.mp4'],
             'data': ''
         })

common/lib/xmodule/xmodule/video_module.py

@@ -119,7 +119,7 @@ class VideoFields(object):
     # `track` is deprecated field and should not be used in future.
     # `download_track` is used instead.
     track = String(
-        help="The external URL to download the timed transcript track.",
+        help="The external URL to download the timed transcript track. This appears as a link beneath the video.",
         display_name="Download Transcript",
         scope=Scope.settings,
         default=''
@@ -221,11 +221,14 @@ class VideoModule(VideoFields, XModule):
             elif self.html5_sources:
                 sources['main'] = self.html5_sources[0]
 
-        if self.download_track:
-            if self.track:
-                track_url = self.track
-            elif self.sub:
-                track_url = self.runtime.handler_url(self, 'download_transcript')
+        # Commented due to the reason described in BLD-811.
+        # if self.download_track:
+        #     if self.track:
+        #         track_url = self.track
+        #     elif self.sub:
+        #         track_url = self.runtime.handler_url(self, 'download_transcript')
+
+        track_url = self.track
 
         return self.system.render_template('video.html', {
             'ajax_url': self.system.ajax_url + '/save_user_state',
@@ -320,10 +323,16 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
 
     def __init__(self, *args, **kwargs):
         '''
+        Mostly handles backward compatibility issues.
+
+        Track was deprecated field, but functionality was reverted,
+        this is commented out because might be used in future.
+        ###
         `track` is deprecated field.
         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`
@@ -343,12 +352,13 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
 
         editable_fields = self.editable_metadata_fields
 
-        self.track_visible = False
-        if self.track:
-            self.track_visible = True
-            download_track = editable_fields['download_track']
-            if not download_track['explicitly_set']:
-                self.download_track = True
+        # Commented due to the reason described in BLD-811.
+        # self.track_visible = False
+        # if self.track:
+        #     self.track_visible = True
+        #     download_track = editable_fields['download_track']
+        #     if not download_track['explicitly_set']:
+        #         self.download_track = True
 
         self.source_visible = False
         if self.source:
@@ -367,11 +377,15 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
     def editable_metadata_fields(self):
         editable_fields = super(VideoDescriptor, self).editable_metadata_fields
 
-        if hasattr(self, 'track_visible'):
-            if self.track_visible:
-                editable_fields['track']['non_editable'] = True
-            else:
-                editable_fields.pop('track')
+        # Commented due to the reason described in BLD-811.
+        # if hasattr(self, 'track_visible'):
+        #     if self.track_visible:
+        #         editable_fields['track']['non_editable'] = True
+        #     else:
+        #         editable_fields.pop('track')
+
+        if 'download_track' in editable_fields:
+            editable_fields.pop('download_track')
 
         if hasattr(self, 'source_visible'):
             if self.source_visible:
@@ -571,6 +585,10 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
                     value = deserialize_field(cls.fields[attr], value)
                 field_data[attr] = value
 
+        # Add `source` for backwards compatibility if xml doesn't  have `download_video`.
+        if 'download_video' not in field_data and sources:
+            field_data['source'] = field_data['html5_sources'][0]
+
         return field_data
 
 

lms/djangoapps/courseware/tests/test_video_mongo.py

@@ -5,6 +5,7 @@ from mock import patch, PropertyMock
 import os
 import tempfile
 import textwrap
+import unittest
 from functools import partial
 
 from xmodule.contentstore.content import StaticContent
@@ -71,7 +72,7 @@ class TestVideoYouTube(TestVideo):
             'start': 3603.0,
             'saved_video_position': 0.0,
             'sub': u'a_sub_file.srt.sjson',
-            'track': None,
+            'track': '',
             'youtube_streams': _create_youtube_string(self.item_module),
             'autoplay': settings.FEATURES.get('AUTOPLAY_VIDEOS', False),
             'yt_test_timeout': 1500,
@@ -127,7 +128,7 @@ class TestVideoNonYouTube(TestVideo):
             'start': 3603.0,
             'saved_video_position': 0.0,
             'sub': u'a_sub_file.srt.sjson',
-            'track': None,
+            'track': '',
             'youtube_streams': '1.00:OEoXaMPEzfM',
             'autoplay': settings.FEATURES.get('AUTOPLAY_VIDEOS', True),
             'yt_test_timeout': 1500,
@@ -200,6 +201,7 @@ class TestGetHtmlMethod(BaseTestXmodule):
             'end': 3610.0,
             'id': None,
             'sources': {
+                'main': u'example.mp4',
                 u'mp4': u'example.mp4',
                 u'webm': u'example.webm'
             },
@@ -208,7 +210,7 @@ class TestGetHtmlMethod(BaseTestXmodule):
             'sub': u'a_sub_file.srt.sjson',
             'speed': 'null',
             'general_speed': 1.0,
-            'track': None,
+            'track': u'http://www.example.com/track',
             'youtube_streams': '1.00:OEoXaMPEzfM',
             'autoplay': settings.FEATURES.get('AUTOPLAY_VIDEOS', True),
             'yt_test_timeout': 1500,
@@ -223,13 +225,14 @@ class TestGetHtmlMethod(BaseTestXmodule):
             )
 
             self.initialize_module(data=DATA)
-            track_url = self.item_descriptor.xmodule_runtime.handler_url(self.item_module, 'download_transcript')
+            # track_url = self.item_descriptor.xmodule_runtime.handler_url(self.item_module, 'download_transcript')
 
             context = self.item_module.render('student_view').content
 
             expected_context.update({
                 'ajax_url': self.item_descriptor.xmodule_runtime.ajax_url + '/save_user_state',
-                'track': track_url if data['expected_track_url'] == u'a_sub_file.srt.sjson' else data['expected_track_url'],
+                # 'track': track_url if data['expected_track_url'] == u'a_sub_file.srt.sjson' else data['expected_track_url'],
+                'track': u'http://www.example.com/track' if data['track'] else '',
                 'sub': data['sub'],
                 'id': self.item_module.location.html_id(),
             })
@@ -313,7 +316,7 @@ class TestGetHtmlMethod(BaseTestXmodule):
             'start': 3603.0,
             'saved_video_position': 0.0,
             'sub': u'a_sub_file.srt.sjson',
-            'track': None,
+            'track': '',
             'youtube_streams': '1.00:OEoXaMPEzfM',
             'autoplay': settings.FEATURES.get('AUTOPLAY_VIDEOS', True),
             'yt_test_timeout': 1500,
@@ -451,6 +454,7 @@ class TestVideoDescriptorInitialization(BaseTestXmodule):
         self.assertNotIn('source', fields)
         self.assertFalse(self.item_module.download_video)
 
+    @unittest.skip('Skipped due to the reason described in BLD-811')
     def test_track_is_not_empty(self):
         metatdata = {
             'track': 'http://example.org/track',
@@ -464,6 +468,7 @@ class TestVideoDescriptorInitialization(BaseTestXmodule):
         self.assertTrue(self.item_module.download_track)
         self.assertTrue(self.item_module.track_visible)
 
+    @unittest.skip('Skipped due to the reason described in BLD-811')
     @patch('xmodule.x_module.XModuleDescriptor.editable_metadata_fields', new_callable=PropertyMock)
     def test_download_track_is_explicitly_set(self, mock_editable_fields):
         mock_editable_fields.return_value = {
@@ -514,7 +519,7 @@ class TestVideoDescriptorInitialization(BaseTestXmodule):
         self.assertFalse(self.item_module.download_track)
         self.assertTrue(self.item_module.track_visible)
 
-
+    @unittest.skip('Skipped due to the reason described in BLD-811')
     def test_track_is_empty(self):
         metatdata = {
             'track': '',