Convert VideoModule to VideoBlock.

Some deprecated functionality has been removed:

- Reading data field and transforms being applied in the init() method.
- The source field.
- The source_visible attribute.
This commit is contained in:
Usman Khalid
2019-05-02 21:43:31 +05:00
parent 392cc97ce9
commit 0b27c62ca6
34 changed files with 379 additions and 454 deletions

View File

@@ -18,8 +18,6 @@ XMODULES = [
"section = xmodule.backcompat_module:SemanticSectionDescriptor",
"sequential = xmodule.seq_module:SequenceDescriptor",
"slides = xmodule.backcompat_module:TranslateCustomTagDescriptor",
"video = xmodule.video_module:VideoDescriptor",
"videoalpha = xmodule.video_module:VideoDescriptor",
"videodev = xmodule.backcompat_module:TranslateCustomTagDescriptor",
"videosequence = xmodule.seq_module:SequenceDescriptor",
"course_info = xmodule.html_module:CourseInfoDescriptor",
@@ -36,6 +34,8 @@ XBLOCKS = [
"library = xmodule.library_root_xblock:LibraryRoot",
"problem = xmodule.capa_module:ProblemBlock",
"vertical = xmodule.vertical_block:VerticalBlock",
"video = xmodule.video_module:VideoBlock",
"videoalpha = xmodule.video_module:VideoBlock",
"wrapper = xmodule.wrapper_module:WrapperBlock",
]
XBLOCKS_ASIDES = [

View File

@@ -51,17 +51,11 @@ class EditingDescriptor(EditingMixin, MakoModuleDescriptor):
pass
class TabsEditingDescriptor(EditingFields, MakoModuleDescriptor):
class TabsEditingMixin(EditingFields, MakoTemplateBlockBase):
"""
Module that provides a raw editing view of its data and children. It does not
perform any validation on its definition---just passes it along to the browser.
This class is intended to be used as a mixin.
Engine (module_edit.js) wants for metadata editor
template to be always loaded, so don't forget to include
settings tab in your module descriptor.
Common code between TabsEditingDescriptor and XBlocks converted from XModules.
"""
mako_template = "widgets/tabs-aggregator.html"
css = {'scss': [resource_string(__name__, 'css/tabs/tabs.scss')]}
js = {'js': [resource_string(
@@ -70,7 +64,7 @@ class TabsEditingDescriptor(EditingFields, MakoModuleDescriptor):
tabs = []
def get_context(self):
_context = super(TabsEditingDescriptor, self).get_context()
_context = MakoTemplateBlockBase.get_context(self)
_context.update({
'tabs': self.tabs,
'html_id': self.location.html_id(), # element_id
@@ -91,6 +85,20 @@ class TabsEditingDescriptor(EditingFields, MakoModuleDescriptor):
return cls.css
class TabsEditingDescriptor(TabsEditingMixin, MakoModuleDescriptor):
"""
Module that provides a raw editing view of its data and children. It does not
perform any validation on its definition---just passes it along to the browser.
This class is intended to be used as a mixin.
Engine (module_edit.js) wants for metadata editor
template to be always loaded, so don't forget to include
settings tab in your module descriptor.
"""
pass
class XMLEditingDescriptor(EditingDescriptor):
"""
Module that provides a raw editing view of its data as XML. It does not perform

View File

@@ -69,10 +69,9 @@ class RawDescriptor(RawMixin, XmlDescriptor, XMLEditingDescriptor):
pass
class EmptyDataRawDescriptor(XmlDescriptor, XMLEditingDescriptor):
class EmptyDataRawMixin(object):
"""
Version of RawDescriptor for modules which may have no XML data,
but use XMLEditingDescriptor for import/export handling.
Common code between EmptyDataRawDescriptor and XBlocks converted from XModules.
"""
resources_dir = None
@@ -88,3 +87,11 @@ class EmptyDataRawDescriptor(XmlDescriptor, XMLEditingDescriptor):
if self.data:
return etree.fromstring(self.data)
return etree.Element(self.category)
class EmptyDataRawDescriptor(EmptyDataRawMixin, XmlDescriptor, XMLEditingDescriptor):
"""
Version of RawDescriptor for modules which may have no XML data,
but use XMLEditingDescriptor for import/export handling.
"""
pass

View File

@@ -14,22 +14,57 @@ import os
import sys
import textwrap
from collections import defaultdict
from pkg_resources import resource_string
import django
import six
from docopt import docopt
from path import Path as path
from xmodule.x_module import XModuleDescriptor
from .capa_module import ProblemBlock
from xmodule.capa_module import ProblemBlock
from xmodule.x_module import XModuleDescriptor, HTMLSnippet
LOG = logging.getLogger(__name__)
class VideoBlock(HTMLSnippet):
"""
Static assets for VideoBlock.
Kept here because importing VideoBlock code requires Django to be setup.
"""
preview_view_js = {
'js': [
resource_string(__name__, 'js/src/video/10_main.js'),
],
'xmodule_js': resource_string(__name__, 'js/src/xmodule.js')
}
preview_view_css = {
'scss': [
resource_string(__name__, 'css/video/display.scss'),
resource_string(__name__, 'css/video/accessible_menu.scss'),
],
}
studio_view_js = {
'js': [
resource_string(__name__, 'js/src/tabs/tabs-aggregator.js'),
],
'xmodule_js': resource_string(__name__, 'js/src/xmodule.js'),
}
studio_view_css = {
'scss': [
resource_string(__name__, 'css/tabs/tabs.scss'),
]
}
# List of XBlocks which use this static content setup.
# Should only be used for XModules being converted to XBlocks.
XBLOCK_CLASSES = [
ProblemBlock,
VideoBlock,
]

View File

@@ -179,8 +179,8 @@ def mock_render_template(*args, **kwargs):
class ModelsTest(unittest.TestCase):
def test_load_class(self):
vc = XModuleDescriptor.load_class('video')
vc_str = "<class 'xmodule.video_module.video_module.VideoDescriptor'>"
vc = XModuleDescriptor.load_class('sequential')
vc_str = "<class 'xmodule.seq_module.SequenceDescriptor'>"
self.assertEqual(str(vc), vc_str)

View File

@@ -36,9 +36,8 @@ from xblock.fields import ScopeIds
from xmodule.tests import get_test_descriptor_system
from xmodule.validation import StudioValidationMessage
from xmodule.video_module import VideoDescriptor, create_youtube_string, EXPORT_IMPORT_STATIC_DIR
from xmodule.video_module import VideoBlock, create_youtube_string, EXPORT_IMPORT_STATIC_DIR
from xmodule.video_module.transcripts_utils import download_youtube_subs, save_to_store, save_subs_to_store
from . import LogicTest
from .test_import import DummySystem
SRT_FILEDATA = '''
@@ -96,11 +95,13 @@ def instantiate_descriptor(**field_data):
"""
Instantiate descriptor with most properties.
"""
if field_data.get('data', None):
field_data = VideoBlock.parse_video_xml(field_data['data'])
system = get_test_descriptor_system()
course_key = CourseLocator('org', 'course', 'run')
usage_key = course_key.make_usage_key('video', 'SampleProblem')
return system.construct_xblock_from_class(
VideoDescriptor,
VideoBlock,
scope_ids=ScopeIds(None, None, usage_key, usage_key),
field_data=DictFieldData(field_data),
)
@@ -119,9 +120,8 @@ class _MockValCannotCreateError(Exception):
pass
class VideoModuleTest(LogicTest):
"""Logic tests for Video Xmodule."""
descriptor_class = VideoDescriptor
class VideoBlockTest(unittest.TestCase):
"""Logic tests for Video XBlock."""
raw_field_data = {
'data': '<video />'
@@ -130,7 +130,7 @@ class VideoModuleTest(LogicTest):
def test_parse_youtube(self):
"""Test parsing old-style Youtube ID strings into a dict."""
youtube_str = '0.75:jNCf2gIqpeE,1.00:ZwkTiUPN0mg,1.25:rsq9auxASqI,1.50:kMyNdzVHHgg'
output = VideoDescriptor._parse_youtube(youtube_str)
output = VideoBlock._parse_youtube(youtube_str)
self.assertEqual(output, {'0.75': 'jNCf2gIqpeE',
'1.00': 'ZwkTiUPN0mg',
'1.25': 'rsq9auxASqI',
@@ -142,7 +142,7 @@ class VideoModuleTest(LogicTest):
empty string.
"""
youtube_str = '0.75:jNCf2gIqpeE'
output = VideoDescriptor._parse_youtube(youtube_str)
output = VideoBlock._parse_youtube(youtube_str)
self.assertEqual(output, {'0.75': 'jNCf2gIqpeE',
'1.00': '',
'1.25': '',
@@ -152,14 +152,14 @@ class VideoModuleTest(LogicTest):
"""Ensure that ids that are invalid return an empty dict"""
# invalid id
youtube_str = 'thisisaninvalidid'
output = VideoDescriptor._parse_youtube(youtube_str)
output = VideoBlock._parse_youtube(youtube_str)
self.assertEqual(output, {'0.75': '',
'1.00': '',
'1.25': '',
'1.50': ''})
# another invalid id
youtube_str = ',::,:,,'
output = VideoDescriptor._parse_youtube(youtube_str)
output = VideoBlock._parse_youtube(youtube_str)
self.assertEqual(output, {'0.75': '',
'1.00': '',
'1.25': '',
@@ -167,7 +167,7 @@ class VideoModuleTest(LogicTest):
# and another one, partially invalid
youtube_str = '0.75_BAD!!!,1.0:AXdE34_U,1.25:KLHF9K_Y,1.5:VO3SxfeD,'
output = VideoDescriptor._parse_youtube(youtube_str)
output = VideoBlock._parse_youtube(youtube_str)
self.assertEqual(output, {'0.75': '',
'1.00': 'AXdE34_U',
'1.25': 'KLHF9K_Y',
@@ -180,8 +180,8 @@ class VideoModuleTest(LogicTest):
youtube_str = '1.00:p2Q6BrNhdh8'
youtube_str_hack = '1.0:p2Q6BrNhdh8'
self.assertEqual(
VideoDescriptor._parse_youtube(youtube_str),
VideoDescriptor._parse_youtube(youtube_str_hack)
VideoBlock._parse_youtube(youtube_str),
VideoBlock._parse_youtube(youtube_str_hack)
)
def test_parse_youtube_empty(self):
@@ -190,7 +190,7 @@ class VideoModuleTest(LogicTest):
that well.
"""
self.assertEqual(
VideoDescriptor._parse_youtube(''),
VideoBlock._parse_youtube(''),
{'0.75': '',
'1.00': '',
'1.25': '',
@@ -198,13 +198,13 @@ class VideoModuleTest(LogicTest):
)
class VideoDescriptorTestBase(unittest.TestCase):
class VideoBlockTestBase(unittest.TestCase):
"""
Base class for tests for VideoDescriptor
Base class for tests for VideoBlock
"""
def setUp(self):
super(VideoDescriptorTestBase, self).setUp()
super(VideoBlockTestBase, self).setUp()
self.descriptor = instantiate_descriptor()
def assertXmlEqual(self, expected, xml):
@@ -223,7 +223,7 @@ class VideoDescriptorTestBase(unittest.TestCase):
self.assertXmlEqual(left, right)
class TestCreateYoutubeString(VideoDescriptorTestBase):
class TestCreateYoutubeString(VideoBlockTestBase):
"""
Checks that create_youtube_string correcty extracts information from Video descriptor.
"""
@@ -250,7 +250,7 @@ class TestCreateYoutubeString(VideoDescriptorTestBase):
self.assertEqual(create_youtube_string(self.descriptor), expected)
class TestCreateYouTubeUrl(VideoDescriptorTestBase):
class TestCreateYouTubeUrl(VideoBlockTestBase):
"""
Tests for helper method `create_youtube_url`.
"""
@@ -264,9 +264,9 @@ class TestCreateYouTubeUrl(VideoDescriptorTestBase):
@ddt.ddt
class VideoDescriptorImportTestCase(TestCase):
class VideoBlockImportTestCase(TestCase):
"""
Make sure that VideoDescriptor can import an old XML-based video correctly.
Make sure that VideoBlock can import an old XML-based video correctly.
"""
def assert_attributes_equal(self, video, attrs):
@@ -328,7 +328,7 @@ class VideoDescriptorImportTestCase(TestCase):
<transcript language="de" src="german_translation.srt" />
</video>
'''
output = VideoDescriptor.from_xml(xml_data, module_system, Mock())
output = VideoBlock.from_xml(xml_data, module_system, Mock())
self.assert_attributes_equal(output, {
'youtube_id_0_75': 'izygArpw-Qo',
'youtube_id_1_0': 'p2Q6BrNhdh8',
@@ -376,7 +376,7 @@ class VideoDescriptorImportTestCase(TestCase):
id_generator = Mock()
id_generator.target_course_id = course_id
output = VideoDescriptor.from_xml(xml_data, module_system, id_generator)
output = VideoBlock.from_xml(xml_data, module_system, id_generator)
self.assert_attributes_equal(output, {
'youtube_id_0_75': 'izygArpw-Qo',
'youtube_id_1_0': 'p2Q6BrNhdh8',
@@ -407,7 +407,7 @@ class VideoDescriptorImportTestCase(TestCase):
<source src="http://www.example.com/source.mp4"/>
</video>
'''
output = VideoDescriptor.from_xml(xml_data, module_system, Mock())
output = VideoBlock.from_xml(xml_data, module_system, Mock())
self.assert_attributes_equal(output, {
'youtube_id_0_75': '',
'youtube_id_1_0': 'p2Q6BrNhdh8',
@@ -419,7 +419,7 @@ class VideoDescriptorImportTestCase(TestCase):
'track': '',
'handout': None,
'download_track': False,
'download_video': True,
'download_video': False,
'html5_sources': ['http://www.example.com/source.mp4'],
'data': ''
})
@@ -438,7 +438,7 @@ class VideoDescriptorImportTestCase(TestCase):
<track src="http://www.example.com/track"/>
</video>
'''
output = VideoDescriptor.from_xml(xml_data, module_system, Mock())
output = VideoBlock.from_xml(xml_data, module_system, Mock())
self.assert_attributes_equal(output, {
'youtube_id_0_75': '',
'youtube_id_1_0': 'p2Q6BrNhdh8',
@@ -449,7 +449,7 @@ class VideoDescriptorImportTestCase(TestCase):
'end_time': datetime.timedelta(seconds=0.0),
'track': 'http://www.example.com/track',
'download_track': True,
'download_video': True,
'download_video': False,
'html5_sources': ['http://www.example.com/source.mp4'],
'data': '',
'transcripts': {},
@@ -461,7 +461,7 @@ class VideoDescriptorImportTestCase(TestCase):
"""
module_system = DummySystem(load_error_modules=True)
xml_data = '<video></video>'
output = VideoDescriptor.from_xml(xml_data, module_system, Mock())
output = VideoBlock.from_xml(xml_data, module_system, Mock())
self.assert_attributes_equal(output, {
'youtube_id_0_75': '',
'youtube_id_1_0': '3_yD_cEKoCk',
@@ -500,7 +500,7 @@ class VideoDescriptorImportTestCase(TestCase):
youtube_id_1_0="&quot;OEoXaMPEzf10&quot;"
/>
'''
output = VideoDescriptor.from_xml(xml_data, module_system, Mock())
output = VideoBlock.from_xml(xml_data, module_system, Mock())
self.assert_attributes_equal(output, {
'youtube_id_0_75': 'OEoXaMPEzf65',
'youtube_id_1_0': 'OEoXaMPEzf10',
@@ -524,7 +524,7 @@ class VideoDescriptorImportTestCase(TestCase):
youtube="1.0:&quot;p2Q6BrNhdh8&quot;,1.25:&quot;1EeWXzPdhSA&quot;">
</video>
'''
output = VideoDescriptor.from_xml(xml_data, module_system, Mock())
output = VideoBlock.from_xml(xml_data, module_system, Mock())
self.assert_attributes_equal(output, {
'youtube_id_0_75': '',
'youtube_id_1_0': 'p2Q6BrNhdh8',
@@ -543,7 +543,7 @@ class VideoDescriptorImportTestCase(TestCase):
def test_old_video_format(self):
"""
Test backwards compatibility with VideoModule's XML format.
Test backwards compatibility with VideoBlock's XML format.
"""
module_system = DummySystem(load_error_modules=True)
xml_data = """
@@ -557,7 +557,7 @@ class VideoDescriptorImportTestCase(TestCase):
<track src="http://www.example.com/track"/>
</video>
"""
output = VideoDescriptor.from_xml(xml_data, module_system, Mock())
output = VideoBlock.from_xml(xml_data, module_system, Mock())
self.assert_attributes_equal(output, {
'youtube_id_0_75': 'izygArpw-Qo',
'youtube_id_1_0': 'p2Q6BrNhdh8',
@@ -574,7 +574,7 @@ class VideoDescriptorImportTestCase(TestCase):
def test_old_video_data(self):
"""
Ensure that Video is able to read VideoModule's model data.
Ensure that Video is able to read VideoBlock's model data.
"""
module_system = DummySystem(load_error_modules=True)
xml_data = """
@@ -587,7 +587,7 @@ class VideoDescriptorImportTestCase(TestCase):
<track src="http://www.example.com/track"/>
</video>
"""
video = VideoDescriptor.from_xml(xml_data, module_system, Mock())
video = VideoBlock.from_xml(xml_data, module_system, Mock())
self.assert_attributes_equal(video, {
'youtube_id_0_75': 'izygArpw-Qo',
'youtube_id_1_0': 'p2Q6BrNhdh8',
@@ -604,7 +604,7 @@ class VideoDescriptorImportTestCase(TestCase):
def test_import_with_float_times(self):
"""
Ensure that Video is able to read VideoModule's model data.
Ensure that Video is able to read VideoBlock's model data.
"""
module_system = DummySystem(load_error_modules=True)
xml_data = """
@@ -617,7 +617,7 @@ class VideoDescriptorImportTestCase(TestCase):
<track src="http://www.example.com/track"/>
</video>
"""
video = VideoDescriptor.from_xml(xml_data, module_system, Mock())
video = VideoBlock.from_xml(xml_data, module_system, Mock())
self.assert_attributes_equal(video, {
'youtube_id_0_75': 'izygArpw-Qo',
'youtube_id_1_0': 'p2Q6BrNhdh8',
@@ -665,7 +665,7 @@ class VideoDescriptorImportTestCase(TestCase):
)
id_generator = Mock()
id_generator.target_course_id = 'test_course_id'
video = VideoDescriptor.from_xml(xml_data, module_system, id_generator)
video = VideoBlock.from_xml(xml_data, module_system, id_generator)
self.assert_attributes_equal(video, {'edx_video_id': edx_video_id})
mock_val_api.import_from_xml.assert_called_once_with(
@@ -690,12 +690,12 @@ class VideoDescriptorImportTestCase(TestCase):
</video>
"""
with self.assertRaises(mock_val_api.ValCannotCreateError):
VideoDescriptor.from_xml(xml_data, module_system, id_generator=Mock())
VideoBlock.from_xml(xml_data, module_system, id_generator=Mock())
class VideoExportTestCase(VideoDescriptorTestBase):
class VideoExportTestCase(VideoBlockTestBase):
"""
Make sure that VideoDescriptor can export itself to XML correctly.
Make sure that VideoBlock can export itself to XML correctly.
"""
def setUp(self):
@@ -773,7 +773,7 @@ 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" download_video="false"/>'
xml_string = '<video url_name="SampleProblem"/>'
expected = etree.XML(xml_string, parser=parser)
self.assertXmlEqual(expected, xml)
@@ -813,7 +813,7 @@ class VideoExportTestCase(VideoDescriptorTestBase):
"""
xml = self.descriptor.definition_to_xml(self.file_system)
# 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'
expected = '<video url_name="SampleProblem"/>\n'
self.assertEquals(expected, etree.tostring(xml, pretty_print=True))
@patch('xmodule.video_module.video_module.edxval_api', None)
@@ -823,7 +823,7 @@ class VideoExportTestCase(VideoDescriptorTestBase):
"""
self.descriptor.transcripts = None
xml = self.descriptor.definition_to_xml(self.file_system)
expected = '<video url_name="SampleProblem" download_video="false"/>\n'
expected = '<video url_name="SampleProblem"/>\n'
self.assertEquals(expected, etree.tostring(xml, pretty_print=True))
@patch('xmodule.video_module.video_module.edxval_api', None)
@@ -850,9 +850,9 @@ class VideoExportTestCase(VideoDescriptorTestBase):
@patch.object(settings, 'FEATURES', create=True, new={
'FALLBACK_TO_ENGLISH_TRANSCRIPTS': False,
})
class VideoDescriptorStudentViewDataTestCase(unittest.TestCase):
class VideoBlockStudentViewDataTestCase(unittest.TestCase):
"""
Make sure that VideoDescriptor returns the expected student_view_data.
Make sure that VideoBlock returns the expected student_view_data.
"""
VIDEO_URL_1 = 'http://www.example.com/source_low.mp4'
@@ -865,41 +865,6 @@ class VideoDescriptorStudentViewDataTestCase(unittest.TestCase):
{'only_on_web': True},
{'only_on_web': True},
),
# Ensure that the deprecated `source` attribute is included in the `all_sources` list.
(
{
'only_on_web': False,
'youtube_id_1_0': None,
'source': VIDEO_URL_1,
},
{
'only_on_web': False,
'duration': None,
'transcripts': {},
'encoded_videos': {
'fallback': {'url': VIDEO_URL_1, 'file_size': 0},
},
'all_sources': [VIDEO_URL_1],
},
),
# Ensure that `html5_sources` take precendence over deprecated `source` url
(
{
'only_on_web': False,
'youtube_id_1_0': None,
'source': VIDEO_URL_1,
'html5_sources': [VIDEO_URL_2, VIDEO_URL_3],
},
{
'only_on_web': False,
'duration': None,
'transcripts': {},
'encoded_videos': {
'fallback': {'url': VIDEO_URL_2, 'file_size': 0},
},
'all_sources': [VIDEO_URL_2, VIDEO_URL_3, VIDEO_URL_1],
},
),
# Ensure that YouTube URLs are included in `encoded_videos`, but not `all_sources`.
(
{
@@ -1002,9 +967,9 @@ class VideoDescriptorStudentViewDataTestCase(unittest.TestCase):
# The default value in {lms,cms}/envs/common.py and xmodule/tests/test_video.py should be consistent.
'FALLBACK_TO_ENGLISH_TRANSCRIPTS': True,
})
class VideoDescriptorIndexingTestCase(unittest.TestCase):
class VideoBlockIndexingTestCase(unittest.TestCase):
"""
Make sure that VideoDescriptor can format data for indexing as expected.
Make sure that VideoBlock can format data for indexing as expected.
"""
def test_video_with_no_subs_index_dictionary(self):

View File

@@ -34,7 +34,6 @@ from xmodule.course_module import CourseDescriptor
from xmodule.html_module import HtmlDescriptor
from xmodule.poll_module import PollDescriptor
from xmodule.word_cloud_module import WordCloudDescriptor
#from xmodule.video_module import VideoDescriptor
from xmodule.seq_module import SequenceDescriptor
from xmodule.conditional_module import ConditionalDescriptor
from xmodule.randomize_module import RandomizeDescriptor
@@ -51,8 +50,6 @@ LEAF_XMODULES = {
HtmlDescriptor: [{}],
PollDescriptor: [{'display_name': 'Poll Display Name'}],
WordCloudDescriptor: [{}],
# This is being excluded because it has dependencies on django
#VideoDescriptor,
}

View File

@@ -127,7 +127,7 @@ def bumper_metadata(video, sources):
unused_track_url, bumper_transcript_language, bumper_languages = video.get_transcripts_for_student(transcripts)
metadata = OrderedDict({
'saveStateUrl': video.system.ajax_url + '/save_user_state',
'saveStateUrl': video.ajax_url + '/save_user_state',
'showCaptions': json.dumps(video.show_captions),
'sources': sources,
'streams': '',

View File

@@ -730,7 +730,7 @@ class Transcript(object):
class VideoTranscriptsMixin(object):
"""Mixin class for transcript functionality.
This is necessary for both VideoModule and VideoDescriptor.
This is necessary for VideoBlock.
"""
def available_translations(self, transcripts, verify_assets=None, is_bumper=False):
@@ -740,7 +740,7 @@ class VideoTranscriptsMixin(object):
Arguments:
verify_assets (boolean): If True, checks to ensure that the transcripts
really exist in the contentstore. If False, we just look at the
VideoDescriptor fields and do not query the contentstore. One reason
VideoBlock fields and do not query the contentstore. One reason
we might do this is to avoid slamming contentstore() with queries
when trying to make a listing of videos and their languages.

View File

@@ -203,7 +203,7 @@ class VideoStudentViewHandlers(object):
if transcript_name:
# Get the asset path for course
asset_path = None
course = self.descriptor.runtime.modulestore.get_course(self.course_id)
course = self.runtime.modulestore.get_course(self.course_id)
if course.static_asset_path:
asset_path = course.static_asset_path
else:

View File

@@ -24,7 +24,6 @@ import six
from django.conf import settings
from lxml import etree
from opaque_keys.edx.locator import AssetLocator
from pkg_resources import resource_string
from web_fragments.fragment import Fragment
from xblock.completable import XBlockCompletionMode
from xblock.core import XBlock
@@ -36,14 +35,19 @@ from openedx.core.djangoapps.video_pipeline.config.waffle import DEPRECATE_YOUTU
from openedx.core.lib.cache_utils import request_cached
from openedx.core.lib.license import LicenseMixin
from xmodule.contentstore.content import StaticContent
from xmodule.editing_module import TabsEditingDescriptor
from xmodule.editing_module import EditingMixin, TabsEditingMixin
from xmodule.exceptions import NotFoundError
from xmodule.modulestore.inheritance import InheritanceKeyValueStore, own_metadata
from xmodule.raw_module import EmptyDataRawDescriptor
from xmodule.raw_module import EmptyDataRawMixin
from xmodule.validation import StudioValidation, StudioValidationMessage
from xmodule.util.xmodule_django import add_webpack_to_fragment
from xmodule.video_module import manage_video_subtitles_save
from xmodule.x_module import PUBLIC_VIEW, STUDENT_VIEW, XModule, module_attr
from xmodule.xml_module import deserialize_field, is_pointer_tag, name_to_pathname
from xmodule.x_module import (
PUBLIC_VIEW, STUDENT_VIEW,
HTMLSnippet, ResourceTemplates, shim_xmodule_js,
XModuleMixin, XModuleToXBlockMixin, XModuleDescriptorToXBlockMixin,
)
from xmodule.xml_module import XmlMixin, deserialize_field, is_pointer_tag, name_to_pathname
from .bumper_utils import bumperize
from .transcripts_utils import (
@@ -61,25 +65,25 @@ from .video_xfields import VideoFields
# The following import/except block for edxval is temporary measure until
# edxval is a proper XBlock Runtime Service.
#
# Here's the deal: the VideoModule should be able to take advantage of edx-val
# Here's the deal: the VideoBlock should be able to take advantage of edx-val
# (https://github.com/edx/edx-val) to figure out what URL to give for video
# resources that have an edx_video_id specified. edx-val is a Django app, and
# including it causes tests to fail because we run common/lib tests standalone
# without Django dependencies. The alternatives seem to be:
#
# 1. Move VideoModule out of edx-platform.
# 1. Move VideoBlock out of edx-platform.
# 2. Accept the Django dependency in common/lib.
# 3. Try to import, catch the exception on failure, and check for the existence
# of edxval_api before invoking it in the code.
# 4. Make edxval an XBlock Runtime Service
#
# (1) is a longer term goal. VideoModule should be made into an XBlock and
# (1) is a longer term goal. VideoBlock should be made into an XBlock and
# extracted from edx-platform entirely. But that's expensive to do because of
# the various dependencies (like templates). Need to sort this out.
# (2) is explicitly discouraged.
# (3) is what we're doing today. The code is still functional when called within
# the context of the LMS, but does not cause failure on import when running
# standalone tests. Most VideoModule tests tend to be in the LMS anyway,
# standalone tests. Most VideoBlock tests tend to be in the LMS anyway,
# probably for historical reasons, so we're not making things notably worse.
# (4) is one of the next items on the backlog for edxval, and should get rid
# of this particular import silliness. It's just that I haven't made one before,
@@ -104,8 +108,12 @@ EXPORT_IMPORT_COURSE_DIR = u'course'
EXPORT_IMPORT_STATIC_DIR = u'static'
@XBlock.wants('settings', 'completion')
class VideoModule(VideoFields, VideoTranscriptsMixin, VideoStudentViewHandlers, XModule, LicenseMixin):
@XBlock.wants('settings', 'completion', 'i18n', 'request_cache')
class VideoBlock(
VideoFields, VideoTranscriptsMixin, VideoStudioViewHandlers, VideoStudentViewHandlers,
TabsEditingMixin, EmptyDataRawMixin, XmlMixin, EditingMixin,
XModuleDescriptorToXBlockMixin, XModuleToXBlockMixin, HTMLSnippet, ResourceTemplates, XModuleMixin,
LicenseMixin):
"""
XML source example:
<video show_captions="true"
@@ -123,27 +131,22 @@ class VideoModule(VideoFields, VideoTranscriptsMixin, VideoStudentViewHandlers,
video_time = 0
icon_class = 'video'
# To make sure that js files are called in proper order we use numerical
# index. We do that to avoid issues that occurs in tests.
module = __name__.replace('.video_module', '', 2)
show_in_read_only_mode = True
#TODO: For each of the following, ensure that any generated html is properly escaped.
js = {
'js': [
resource_string(module, 'js/src/video/10_main.js'),
]
}
css = {'scss': [
resource_string(module, 'css/video/display.scss'),
resource_string(module, 'css/video/accessible_menu.scss'),
]}
js_module_name = "Video"
tabs = [
{
'name': _("Basic"),
'template': "video/transcripts.html",
'current': True
},
{
'name': _("Advanced"),
'template': "tabs/metadata-edit-tab.html"
}
]
def validate(self):
"""
Validates the state of this Video Module Instance.
"""
return self.descriptor.validate()
uses_xmodule_styles_setup = True
requires_per_student_anonymous_id = True
def get_transcripts_for_student(self, transcripts):
"""Return transcript information necessary for rendering the XModule student view.
@@ -211,6 +214,32 @@ class VideoModule(VideoFields, VideoTranscriptsMixin, VideoStudentViewHandlers,
return False
def student_view(self, _context):
"""
Return the student view.
"""
fragment = Fragment(self.get_html())
add_webpack_to_fragment(fragment, 'VideoBlockPreview')
shim_xmodule_js(fragment, 'Video')
return fragment
def author_view(self, context):
"""
Renders the Studio preview view.
"""
return self.student_view(context)
def studio_view(self, _context):
"""
Return the studio view.
"""
fragment = Fragment(
self.system.render_template(self.mako_template, self.get_context())
)
add_webpack_to_fragment(fragment, 'VideoBlockStudio')
shim_xmodule_js(fragment, 'TabsEditingDescriptor')
return fragment
def public_view(self, context):
"""
Returns a fragment that contains the html for the public view
@@ -279,7 +308,7 @@ class VideoModule(VideoFields, VideoTranscriptsMixin, VideoStudentViewHandlers,
except (edxval_api.ValInternalError, edxval_api.ValVideoNotFoundError):
# VAL raises this exception if it can't find data for the edx video ID. This can happen if the
# course data is ported to a machine that does not have the VAL data. So for now, pass on this
# exception and fallback to whatever we find in the VideoDescriptor.
# exception and fallback to whatever we find in the VideoBlock.
log.warning("Could not retrieve information from VAL for edx Video ID: %s.", self.edx_video_id)
# If the user comes from China use China CDN for html5 videos.
@@ -296,11 +325,9 @@ class VideoModule(VideoFields, VideoTranscriptsMixin, VideoStudentViewHandlers,
sources[index] = new_url
# If there was no edx_video_id, or if there was no download specified
# for it, we fall back on whatever we find in the VideoDescriptor
# for it, we fall back on whatever we find in the VideoBlock.
if not download_video_link and self.download_video:
if self.source:
download_video_link = self.source
elif self.html5_sources:
if self.html5_sources:
download_video_link = self.html5_sources[0]
# don't give the option to download HLS video urls
@@ -351,7 +378,7 @@ class VideoModule(VideoFields, VideoTranscriptsMixin, VideoStudentViewHandlers,
metadata = {
'saveStateEnabled': view != PUBLIC_VIEW,
'saveStateUrl': self.system.ajax_url + '/save_user_state',
'saveStateUrl': self.ajax_url + '/save_user_state',
'autoplay': settings.FEATURES.get('AUTOPLAY_VIDEOS', False),
'streams': self.youtube_streams,
'sources': sources,
@@ -421,85 +448,18 @@ class VideoModule(VideoFields, VideoTranscriptsMixin, VideoStudentViewHandlers,
'download_video_link': download_video_link,
'track': track_url,
'transcript_download_format': transcript_download_format,
'transcript_download_formats_list': self.descriptor.fields['transcript_download_format'].values,
'transcript_download_formats_list': self.fields['transcript_download_format'].values,
'license': getattr(self, "license", None),
}
return self.system.render_template('video.html', context)
@XBlock.wants("request_cache", "settings", "completion")
class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandlers,
TabsEditingDescriptor, EmptyDataRawDescriptor, LicenseMixin):
"""
Descriptor for `VideoModule`.
"""
module_class = VideoModule
transcript = module_attr('transcript')
publish_completion = module_attr('publish_completion')
has_custom_completion = module_attr('has_custom_completion')
show_in_read_only_mode = True
tabs = [
{
'name': _("Basic"),
'template': "video/transcripts.html",
'current': True
},
{
'name': _("Advanced"),
'template': "tabs/metadata-edit-tab.html"
}
]
def __init__(self, *args, **kwargs):
"""
Mostly handles backward compatibility issues.
`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 it out and set the metadata fields
if self.data:
field_data = self._parse_video_xml(etree.fromstring(self.data))
self._field_data.set_many(self, field_data)
del self.data
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
if not self.fields['download_video'].is_set_on(self):
self.download_video = True
# Force download_video field to default value if it's not explicitly set for backward compatibility.
if not self.fields['download_video'].is_set_on(self):
self.download_video = self.download_video
self.force_save_fields(['download_video'])
# for backward compatibility.
# If course was existed and was not re-imported by the moment of adding `download_track` field,
# we should enable `download_track` if following is true:
if not self.fields['download_track'].is_set_on(self) and self.track:
self.download_track = True
def validate(self):
"""
Validates the state of this video Module Instance. This
Validates the state of this Video XBlock instance. This
is the override of the general XBlock method, and it will also ask
its superclass to validate.
"""
validation = super(VideoDescriptor, self).validate()
validation = super(VideoBlock, self).validate()
if not isinstance(validation, StudioValidation):
validation = StudioValidation.copy(validation)
@@ -585,7 +545,7 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
@property
def editable_metadata_fields(self):
editable_fields = super(VideoDescriptor, self).editable_metadata_fields
editable_fields = super(VideoBlock, self).editable_metadata_fields
settings_service = self.runtime.service(self, 'settings')
if settings_service:
@@ -593,11 +553,6 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
if not xb_settings.get("licensing_enabled", False) and "license" in editable_fields:
del editable_fields["license"]
if self.source_visible:
editable_fields['source']['non_editable'] = True
else:
editable_fields.pop('source')
# Default Timed Transcript a.k.a `sub` has been deprecated and end users shall
# not be able to modify it.
editable_fields.pop('sub')
@@ -659,7 +614,7 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
filepath = cls._format_filepath(xml_object.tag, name_to_pathname(url_name))
xml_object = cls.load_file(filepath, system.resources_fs, usage_id)
system.parse_asides(xml_object, definition_id, usage_id, id_generator)
field_data = cls._parse_video_xml(xml_object, id_generator)
field_data = cls.parse_video_xml(xml_object, id_generator)
kvs = InheritanceKeyValueStore(initial_values=field_data)
field_data = KvsFieldData(kvs)
video = system.construct_xblock_from_class(
@@ -802,7 +757,7 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
"""
Extend context by data for transcript basic tab.
"""
_context = super(VideoDescriptor, self).get_context()
_context = super(VideoBlock, self).get_context()
metadata_fields = copy.deepcopy(self.editable_metadata_fields)
@@ -896,7 +851,7 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
return ret
@classmethod
def _parse_video_xml(cls, xml, id_generator=None):
def parse_video_xml(cls, xml, id_generator=None):
"""
Parse video fields out of xml_data. The fields are set if they are
present in the XML.
@@ -904,6 +859,9 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
Arguments:
id_generator is used to generate course-specific urls and identifiers
"""
if isinstance(xml, str) or isinstance(xml, unicode):
xml = etree.fromstring(xml)
field_data = {}
# Convert between key types for certain attributes --
@@ -1026,7 +984,7 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
return edx_video_id
def index_dictionary(self):
xblock_body = super(VideoDescriptor, self).index_dictionary()
xblock_body = super(VideoBlock, self).index_dictionary()
video_body = {
"display_name": self.display_name,
}
@@ -1092,10 +1050,6 @@ class VideoDescriptor(VideoFields, VideoTranscriptsMixin, VideoStudioViewHandler
val_video_data = {}
all_sources = self.html5_sources or []
# `source` is a deprecated field, but we include it for backwards compatibility.
if self.source:
all_sources.append(self.source)
# Check in VAL data first if edx_video_id exists
if self.edx_video_id:
video_profile_names = context.get("profiles", ["mobile_low"])

View File

@@ -103,7 +103,7 @@ def get_poster(video):
def format_xml_exception_message(location, key, value):
"""
Generate exception message for VideoDescriptor class which will use for ValueError and UnicodeDecodeError
Generate exception message for VideoBlock class which will use for ValueError and UnicodeDecodeError
when setting xml attributes.
"""
exception_message = "Block-location:{location}, Key:{key}, Value:{value}".format(

View File

@@ -15,7 +15,7 @@ _ = lambda text: text
class VideoFields(object):
"""Fields for `VideoModule` and `VideoDescriptor`."""
"""Fields for `VideoBlock`."""
display_name = String(
help=_("The display name for this component."),
display_name=_("Component Display Name"),
@@ -76,14 +76,6 @@ 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."),
display_name=_("Download Video"),
scope=Scope.settings,
default=""
)
download_video = Boolean(
help=_("Allow students to download versions of this video in different formats if they cannot use the edX video player or do not have access to YouTube. You must add at least one non-YouTube URL in the Video File URLs field."), # pylint: disable=line-too-long
display_name=_("Video Download Allowed"),

View File

@@ -43,7 +43,7 @@ CSS_CLASS_NAMES = {
'video_container': '.video',
'video_sources': '.video-player video source',
'video_spinner': '.video-wrapper .spinner',
'video_xmodule': '.xmodule_VideoModule',
'video_xmodule': '.xmodule_VideoBlock',
'video_init': '.is-initialized',
'video_time': '.vidtime',
'video_display_name': '.vert h3',

View File

@@ -17,7 +17,7 @@ from common.test.acceptance.tests.helpers import YouTubeStubConfig
CLASS_SELECTORS = {
'video_container': '.video',
'video_init': '.is-initialized',
'video_xmodule': '.xmodule_VideoModule',
'video_xmodule': '.xmodule_VideoBlock',
'video_spinner': '.video-wrapper .spinner',
'video_controls': '.video-controls',
'attach_asset': '.upload-dialog > input[type="file"]',

View File

@@ -57,7 +57,7 @@ class VideoLicenseTest(StudioCourseTest):
self.lms_courseware.visit()
video = self.lms_courseware.q(css=".vert .xblock .video")
self.assertTrue(video.is_present())
video_license = self.lms_courseware.q(css=".vert .xblock.xmodule_VideoModule .xblock-license")
video_license = self.lms_courseware.q(css=".vert .xblock.xmodule_VideoBlock .xblock-license")
self.assertFalse(video_license.is_present())
def test_arr_license(self):
@@ -83,7 +83,7 @@ class VideoLicenseTest(StudioCourseTest):
self.lms_courseware.visit()
video = self.lms_courseware.q(css=".vert .xblock .video")
self.assertTrue(video.is_present())
video_license_css = ".vert .xblock.xmodule_VideoModule .xblock-license"
video_license_css = ".vert .xblock.xmodule_VideoBlock .xblock-license"
self.lms_courseware.wait_for_element_presence(
video_license_css, "Video module license block is present"
)
@@ -113,7 +113,7 @@ class VideoLicenseTest(StudioCourseTest):
self.lms_courseware.visit()
video = self.lms_courseware.q(css=".vert .xblock .video")
self.assertTrue(video.is_present())
video_license_css = ".vert .xblock.xmodule_VideoModule .xblock-license"
video_license_css = ".vert .xblock.xmodule_VideoBlock .xblock-license"
self.lms_courseware.wait_for_element_presence(
video_license_css, "Video module license block is present"
)

View File

@@ -1013,7 +1013,7 @@ class YouTubeQualityTest(VideoBaseTest):
@attr('a11y')
class LMSVideoModuleA11yTest(VideoBaseTest):
class LMSVideoBlockA11yTest(VideoBaseTest):
"""
LMS Video Accessibility Test Class
"""
@@ -1030,7 +1030,7 @@ class LMSVideoModuleA11yTest(VideoBaseTest):
browser = 'firefox'
with patch.dict(os.environ, {'SELENIUM_BROWSER': browser}):
super(LMSVideoModuleA11yTest, self).setUp()
super(LMSVideoBlockA11yTest, self).setUp()
def test_video_player_a11y(self):
# load transcripts so we can test skipping to