diff --git a/cms/djangoapps/contentstore/features/video-editor.py b/cms/djangoapps/contentstore/features/video-editor.py index 4036f10351..12b2a5225f 100644 --- a/cms/djangoapps/contentstore/features/video-editor.py +++ b/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], diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 372ba75969..7b5c57019d 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/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) diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 238660f510..376f8ff82e 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/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': "

Problem content draft.

", + '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): diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py b/common/lib/xmodule/xmodule/modulestore/mongo/draft.py index df31badf80..5956b00513 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/draft.py +++ b/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): """ diff --git a/common/lib/xmodule/xmodule/tests/test_video.py b/common/lib/xmodule/xmodule/tests/test_video.py index 8fc4aabef3..d15886cac0 100644 --- a/common/lib/xmodule/xmodule/tests/test_video.py +++ b/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': '' }) diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py index 8949e525c1..cebd57806b 100644 --- a/common/lib/xmodule/xmodule/video_module.py +++ b/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 diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index e00a0230e6..a82c304f14 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/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': '',