Skip to content

E
edx-platform
Commit 9c844405 by Peter Fogg
Options

PR comment cleanup.

parent 3e654550
Showing with 168 additions and 121 deletions
cms/djangoapps/contentstore/features/common.py
......@@ -256,14 +256,14 @@ def open_new_unit(step):
world.css_click('a.new-unit-item')
@step('when I view the video it (.*) show the captions')
def shows_captions(step, show_captions):
@step('when I view the (video.*) it (.*) show the captions')
def shows_captions(_step, video_type, show_captions):
# Prevent cookies from overriding course settings
world.browser.cookies.delete('hide_captions')
if show_captions == 'does not':
assert world.css_has_class('.video', 'closed')
assert world.css_has_class('.%s' % video_type, 'closed')
else:
assert world.is_css_not_present('.video.closed')
assert world.is_css_not_present('.%s.closed' % video_type)
@step('the save button is disabled$')
......
cms/djangoapps/contentstore/features/component_settings_editor_helpers.py
......@@ -60,6 +60,12 @@ def edit_component_and_select_settings():
@world.absorb
def edit_component():
world.wait_for(lambda _driver: world.css_visible('a.edit-button'))
world.css_click('a.edit-button')
@world.absorb
def verify_setting_entry(setting, display_name, value, explicitly_set):
assert_equal(display_name, setting.find_by_css('.setting-label')[0].value)
assert_equal(value, setting.find_by_css('.setting-input')[0].value)
......
cms/djangoapps/contentstore/features/video-editor.py
......@@ -29,8 +29,10 @@ def correct_videoalpha_settings(_step):
world.verify_all_setting_entries([['Display Name', 'Video Alpha', False],
['Download Track', '', False],
['Download Video', '', False],
['End Time', '0', False],
['HTML5 Subtitles', '', False],
['Show Captions', 'True', False],
['Start Time', '0', False],
['Video Sources', '', False],
['Youtube ID', 'OEoXaMPEzfM', False],
['Youtube ID for .75x speed', '', False],
......
cms/djangoapps/contentstore/features/video.feature
......@@ -29,23 +29,21 @@ Feature: Video Component
Scenario: User can view Video Alpha metadata
Given I have created a Video Alpha component
And I edit and select Settings
And I edit the component
Then I see the correct videoalpha settings and default values
Scenario: User can modify Video Alpha display name
Given I have created a Video Alpha component
And I edit and select Settings
And I edit the component
Then I can modify the display name
And my display name change is persisted on save
And my videoalpha display name change is persisted on save
@skip
Scenario: Video Alpha captions are hidden when "show captions" is false
Given I have created a Video Alpha component
And I have set "show captions" to False
Then when I view the video it does not show the captions
Then when I view the videoalpha it does not show the captions
@skip
Scenario: Video Alpha captions are shown when "show captions" is true
Given I have created a Video Alpha component
And I have set "show captions" to True
Then when I view the video it does show the captions
Then when I view the videoalpha it does show the captions
cms/djangoapps/contentstore/features/video.py
#pylint: disable=C0111
from lettuce import world, step
from terrain.steps import reload_the_page
############### ACTIONS ####################
......@@ -31,3 +32,15 @@ def hide_or_show_captions(step, shown):
button = world.css_find(button_css)
button.mouse_out()
world.css_click(button_css)
@step('I edit the component')
def i_edit_the_component(_step):
world.edit_component()
@step('my videoalpha display name change is persisted on save')
def videoalpha_name_persisted(step):
world.css_click('a.save-button')
reload_the_page(step)
world.edit_component()
world.verify_setting_entry(world.get_setting_entry('Display Name'), 'Display Name', '3.4', True)
cms/static/coffee/spec/views/metadata_edit_spec.coffee
......@@ -107,7 +107,8 @@ describe "Test Metadata Editor", ->
view = new CMS.Views.Metadata.Editor({collection: @model})
childModels = view.collection.models
expect(childModels.length).toBe(6)
childViews = view.$el.find('.setting-input')
# Be sure to check list view as well as other input types
childViews = view.$el.find('.setting-input, .list-settings')
expect(childViews.length).toBe(6)
verifyEntry = (index, display_name, type) ->
......@@ -116,7 +117,7 @@ describe "Test Metadata Editor", ->
verifyEntry(0, 'Display Name', 'text')
verifyEntry(1, 'Inputs', 'number')
verifyEntry(2, 'List', 'text')
verifyEntry(2, 'List', '')
verifyEntry(3, 'Show Answer', 'select-one')
verifyEntry(4, 'Unknown', 'text')
verifyEntry(5, 'Weight', 'number')
......@@ -319,9 +320,7 @@ describe "Test Metadata Editor", ->
beforeEach ->
listModel = new CMS.Models.Metadata(listEntry)
@listView = new CMS.Views.Metadata.List({model: listModel})
it "uses a text input type", ->
assertInputType(@listView, 'text')
@el = @listView.$el
it "returns the initial value upon initialization", ->
assertValueInView(@listView, ['the first display value', 'the second'])
......@@ -337,10 +336,24 @@ describe "Test Metadata Editor", ->
it "can add an entry", ->
expect(@listView.model.get('value').length).toEqual(2)
@listView.$el.find('.setting-add').click()
expect(@listView.$el.find('input.input').length).toEqual(3)
@el.find('.create-setting').click()
expect(@el.find('input.input').length).toEqual(3)
it "can remove an entry", ->
expect(@listView.model.get('value').length).toEqual(2)
@listView.$el.find('.setting-remove').first().click()
@el.find('.remove-setting').first().click()
expect(@listView.model.get('value').length).toEqual(1)
it "only allows one blank entry at a time", ->
expect(@el.find('input').length).toEqual(2)
@el.find('.create-setting').click()
@el.find('.create-setting').click()
expect(@el.find('input').length).toEqual(3)
it "re-enables the add setting button after entering a new value", ->
expect(@el.find('input').length).toEqual(2)
@el.find('.create-setting').click()
expect(@el.find('.create-setting')).toHaveClass('is-disabled')
@el.find('input').last().val('third setting')
@el.find('input').last().trigger('input')
expect(@el.find('.create-setting')).not.toHaveClass('is-disabled')
cms/static/js/views/metadata_editor_view.js
......@@ -320,6 +320,7 @@ CMS.Views.Metadata.List = CMS.Views.Metadata.AbstractEditor.extend({
"click .setting-clear" : "clear",
"keypress .setting-input" : "showClearButton",
"change input" : "updateModel",
"input input" : "enableAdd",
"click .create-setting" : "addEntry",
"click .remove-setting" : "removeEntry"
},
......@@ -353,6 +354,7 @@ CMS.Views.Metadata.List = CMS.Views.Metadata.AbstractEditor.extend({
// change event
var list = this.model.get('value') || [];
this.setValueInEditor(list.concat(['']))
this.$el.find('.create-setting').addClass('is-disabled');
},
removeEntry: function(event) {
......@@ -360,5 +362,10 @@ CMS.Views.Metadata.List = CMS.Views.Metadata.AbstractEditor.extend({
var entry = $(event.currentTarget).siblings().val();
this.setValueInEditor(_.without(this.model.get('value'), entry));
this.updateModel();
this.$el.find('.create-setting').removeClass('is-disabled');
},
enableAdd: function() {
this.$el.find('.create-setting').removeClass('is-disabled');
}
});
cms/templates/js/metadata-list-entry.underscore
......@@ -6,7 +6,7 @@
</ol>
<a href="#" class="create-action create-setting">
<i class="icon-plus"></i>Add <span class="sr"><%= model.get('display_name')%></span>
<i class="icon-plus"></i><%= gettext("Add") %> <span class="sr"><%= model.get('display_name')%></span>
</a>
</div>
<button class="action setting-clear inactive" type="button" name="setting-clear" value="<%= gettext("Clear") %>" data-tooltip="<%= gettext("Clear") %>">
......
common/lib/xmodule/xmodule/js/src/videoalpha/01_initialize.js
......@@ -148,7 +148,7 @@ function (VideoPlayer) {
// Option
// this.config.show_captions = true | false
//
// defines whether to turn off/on the captions altogether. User will not have the ability to turn them on/off.
// Defines whether or not captions are shown on first viewing.
//
// Option
// this.hide_captions = true | false
......
common/lib/xmodule/xmodule/js/src/videoalpha/10_main.js
......@@ -60,10 +60,7 @@ function (
VideoProgressSlider(state);
VideoVolumeControl(state);
VideoSpeedControl(state);
if (state.config.show_captions) {
VideoCaption(state);
}
VideoCaption(state);
// Because the 'state' object is only available inside this closure, we will also make
// it available to the caller by returning it. This is necessary so that we can test
......
common/lib/xmodule/xmodule/tests/test_videoalpha.py
......@@ -148,6 +148,14 @@ class VideoAlphaDescriptorImportTestCase(unittest.TestCase):
Make sure that VideoAlphaDescriptor can import an old XML-based video correctly.
"""
def assert_attributes_equal(self, video, attrs):
"""
Assert that `video` has the correct attributes. `attrs` is a map
of {metadata_field: value}.
"""
for key, value in attrs.items():
self.assertEquals(getattr(video, key), value)
def test_constructor(self):
sample_xml = '''
<videoalpha display_name="Test Video"
......@@ -166,17 +174,18 @@ class VideoAlphaDescriptorImportTestCase(unittest.TestCase):
'location': location}
system = DummySystem(load_error_modules=True)
descriptor = VideoAlphaDescriptor(system, model_data)
self.assertEquals(descriptor.youtube_id_0_75, 'izygArpw-Qo')
self.assertEquals(descriptor.youtube_id_1_0, 'p2Q6BrNhdh8')
self.assertEquals(descriptor.youtube_id_1_25, '1EeWXzPdhSA')
self.assertEquals(descriptor.youtube_id_1_5, 'rABDYkeK0x8')
self.assertEquals(descriptor.show_captions, False)
self.assertEquals(descriptor.start_time, 1.0)
self.assertEquals(descriptor.end_time, 60)
self.assertEquals(descriptor.track, 'http://www.example.com/track')
self.assertEquals(descriptor.source, 'http://www.example.com/source.mp4')
self.assertEquals(descriptor.html5_sources, ['http://www.example.com/source.mp4', 'http://www.example.com/source.ogg'])
self.assertEquals(descriptor.data, '')
self.assert_attributes_equal(descriptor, {
'youtube_id_0_75': 'izygArpw-Qo',
'youtube_id_1_0': 'p2Q6BrNhdh8',
'youtube_id_1_25': '1EeWXzPdhSA',
'youtube_id_1_5': 'rABDYkeK0x8',
'show_captions': False,
'start_time': 1.0,
'end_time': 60,
'track': 'http://www.example.com/track',
'html5_sources': ['http://www.example.com/source.mp4', 'http://www.example.com/source.ogg'],
'data': ''
})
def test_from_xml(self):
module_system = DummySystem(load_error_modules=True)
......@@ -191,17 +200,19 @@ class VideoAlphaDescriptorImportTestCase(unittest.TestCase):
</videoalpha>
'''
output = VideoAlphaDescriptor.from_xml(xml_data, module_system)
self.assertEquals(output.youtube_id_0_75, 'izygArpw-Qo')
self.assertEquals(output.youtube_id_1_0, 'p2Q6BrNhdh8')
self.assertEquals(output.youtube_id_1_25, '1EeWXzPdhSA')
self.assertEquals(output.youtube_id_1_5, 'rABDYkeK0x8')
self.assertEquals(output.show_captions, False)
self.assertEquals(output.start_time, 1.0)
self.assertEquals(output.end_time, 60)
self.assertEquals(output.track, 'http://www.example.com/track')
self.assertEquals(output.source, 'http://www.example.com/source.mp4')
self.assertEquals(output.html5_sources, ['http://www.example.com/source.mp4'])
self.assertEquals(output.data, '')
self.assert_attributes_equal(output, {
'youtube_id_0_75': 'izygArpw-Qo',
'youtube_id_1_0': 'p2Q6BrNhdh8',
'youtube_id_1_25': '1EeWXzPdhSA',
'youtube_id_1_5': 'rABDYkeK0x8',
'show_captions': False,
'start_time': 1.0,
'end_time': 60,
'track': 'http://www.example.com/track',
'source': 'http://www.example.com/source.mp4',
'html5_sources': ['http://www.example.com/source.mp4'],
'data': ''
})
def test_from_xml_missing_attributes(self):
"""
......@@ -218,17 +229,19 @@ class VideoAlphaDescriptorImportTestCase(unittest.TestCase):
</videoalpha>
'''
output = VideoAlphaDescriptor.from_xml(xml_data, module_system)
self.assertEquals(output.youtube_id_0_75, '')
self.assertEquals(output.youtube_id_1_0, 'p2Q6BrNhdh8')
self.assertEquals(output.youtube_id_1_25, '1EeWXzPdhSA')
self.assertEquals(output.youtube_id_1_5, '')
self.assertEquals(output.show_captions, True)
self.assertEquals(output.start_time, 0.0)
self.assertEquals(output.end_time, 0.0)
self.assertEquals(output.track, 'http://www.example.com/track')
self.assertEquals(output.source, 'http://www.example.com/source.mp4')
self.assertEquals(output.html5_sources, ['http://www.example.com/source.mp4'])
self.assertEquals(output.data, '')
self.assert_attributes_equal(output, {
'youtube_id_0_75': '',
'youtube_id_1_0': 'p2Q6BrNhdh8',
'youtube_id_1_25': '1EeWXzPdhSA',
'youtube_id_1_5': '',
'show_captions': True,
'start_time': 0.0,
'end_time': 0.0,
'track': 'http://www.example.com/track',
'source': 'http://www.example.com/source.mp4',
'html5_sources': ['http://www.example.com/source.mp4'],
'data': ''
})
def test_from_xml_no_attributes(self):
"""
......@@ -237,17 +250,19 @@ class VideoAlphaDescriptorImportTestCase(unittest.TestCase):
module_system = DummySystem(load_error_modules=True)
xml_data = '<videoalpha></videoalpha>'
output = VideoAlphaDescriptor.from_xml(xml_data, module_system)
self.assertEquals(output.youtube_id_0_75, '')
self.assertEquals(output.youtube_id_1_0, 'OEoXaMPEzfM')
self.assertEquals(output.youtube_id_1_25, '')
self.assertEquals(output.youtube_id_1_5, '')
self.assertEquals(output.show_captions, True)
self.assertEquals(output.start_time, 0.0)
self.assertEquals(output.end_time, 0.0)
self.assertEquals(output.track, '')
self.assertEquals(output.source, '')
self.assertEquals(output.html5_sources, [])
self.assertEquals(output.data, '')
self.assert_attributes_equal(output, {
'youtube_id_0_75': '',
'youtube_id_1_0': 'OEoXaMPEzfM',
'youtube_id_1_25': '',
'youtube_id_1_5': '',
'show_captions': True,
'start_time': 0.0,
'end_time': 0.0,
'track': '',
'source': '',
'html5_sources': [],
'data': ''
})
def test_old_video_format(self):
"""
......@@ -265,19 +280,23 @@ class VideoAlphaDescriptorImportTestCase(unittest.TestCase):
</videoalpha>
"""
output = VideoAlphaDescriptor.from_xml(xml_data, module_system)
self.assertEquals(output.youtube_id_0_75, 'izygArpw-Qo')
self.assertEquals(output.youtube_id_1_0, 'p2Q6BrNhdh8')
self.assertEquals(output.youtube_id_1_25, '1EeWXzPdhSA')
self.assertEquals(output.youtube_id_1_5, 'rABDYkeK0x8')
self.assertEquals(output.show_captions, False)
self.assertEquals(output.start_time, 1.0)
self.assertEquals(output.end_time, 60)
self.assertEquals(output.track, 'http://www.example.com/track')
self.assertEquals(output.source, 'http://www.example.com/source.mp4')
self.assertEquals(output.html5_sources, ['http://www.example.com/source.mp4'])
self.assertEquals(output.data, '')
self.assert_attributes_equal(output, {
'youtube_id_0_75': 'izygArpw-Qo',
'youtube_id_1_0': 'p2Q6BrNhdh8',
'youtube_id_1_25': '1EeWXzPdhSA',
'youtube_id_1_5': 'rABDYkeK0x8',
'show_captions': False,
'start_time': 1.0,
'end_time': 60,
'track': 'http://www.example.com/track',
'html5_sources': ['http://www.example.com/source.mp4'],
'data': ''
})
def test_old_video_data(self):
"""
Ensure that Video Alpha is able to read VideoModule's model data.
"""
module_system = DummySystem(load_error_modules=True)
xml_data = """
<video display_name="Test Video"
......@@ -291,17 +310,18 @@ class VideoAlphaDescriptorImportTestCase(unittest.TestCase):
"""
video = VideoDescriptor.from_xml(xml_data, module_system)
video_alpha = VideoAlphaDescriptor(module_system, video._model_data)
self.assertEquals(video_alpha.youtube_id_0_75, 'izygArpw-Qo')
self.assertEquals(video_alpha.youtube_id_1_0, 'p2Q6BrNhdh8')
self.assertEquals(video_alpha.youtube_id_1_25, '1EeWXzPdhSA')
self.assertEquals(video_alpha.youtube_id_1_5, 'rABDYkeK0x8')
self.assertEquals(video_alpha.show_captions, False)
self.assertEquals(video_alpha.start_time, 1.0)
self.assertEquals(video_alpha.end_time, 60)
self.assertEquals(video_alpha.track, 'http://www.example.com/track')
self.assertEquals(video_alpha.source, 'http://www.example.com/source.mp4')
self.assertEquals(video_alpha.html5_sources, ['http://www.example.com/source.mp4'])
self.assertEquals(video_alpha.data, '')
self.assert_attributes_equal(video_alpha, {
'youtube_id_0_75': 'izygArpw-Qo',
'youtube_id_1_0': 'p2Q6BrNhdh8',
'youtube_id_1_25': '1EeWXzPdhSA',
'youtube_id_1_5': 'rABDYkeK0x8',
'show_captions': False,
'start_time': 1.0,
'end_time': 60,
'track': 'http://www.example.com/track',
'html5_sources': ['http://www.example.com/source.mp4'],
'data': ''
})
class VideoAlphaExportTestCase(unittest.TestCase):
......
common/lib/xmodule/xmodule/tests/test_xml_module.py
......@@ -39,7 +39,8 @@ class TestFields(object):
float_non_select = Float(scope=Scope.settings, default=.999, values={'min': 0, 'step': .3})
# Used for testing that Booleans get mapped to select type
boolean_select = Boolean(scope=Scope.settings)
# Used for testing Lists
list_field = List(scope=Scope.settings, default=[])
class EditableMetadataFieldsTest(unittest.TestCase):
def test_display_name_field(self):
......@@ -63,7 +64,7 @@ class EditableMetadataFieldsTest(unittest.TestCase):
def test_integer_field(self):
descriptor = self.get_descriptor({'max_attempts': '7'})
editable_fields = descriptor.editable_metadata_fields
self.assertEqual(6, len(editable_fields))
self.assertEqual(7, len(editable_fields))
self.assert_field_values(
editable_fields, 'max_attempts', TestFields.max_attempts,
explicitly_set=True, inheritable=False, value=7, default_value=1000, type='Integer',
......@@ -137,6 +138,12 @@ class EditableMetadataFieldsTest(unittest.TestCase):
type='Float', options={'min': 0, 'step': .3}
)
self.assert_field_values(
editable_fields, 'list_field', TestFields.list_field,
explicitly_set=False, inheritable=False, value=[], default_value=[],
type='List'
)
# Start of helper methods
def get_xml_editable_fields(self, model_data):
system = get_test_system()
......
common/lib/xmodule/xmodule/videoalpha_module.py
......@@ -51,6 +51,8 @@ class VideoAlphaFields(object):
scope=Scope.settings,
default=True
)
# TODO: This should be moved to Scope.content, but this will
# require data migration to support the old video module.
youtube_id_1_0 = String(
help="This is the Youtube ID reference for the normal speed video.",
display_name="Youtube ID",
......@@ -76,13 +78,13 @@ class VideoAlphaFields(object):
default=""
)
start_time = Float(
help="Time the video starts",
help="Start time for the video.",
display_name="Start Time",
scope=Scope.settings,
default=0.0
)
end_time = Float(
help="Time the video ends",
help="End time for the video.",
display_name="End Time",
scope=Scope.settings,
default=0.0
......@@ -94,7 +96,7 @@ class VideoAlphaFields(object):
default=""
)
html5_sources = List(
help="A list of filenames to be used with HTML5 video.",
help="A list of filenames to be used with HTML5 video. The first supported filetype will be displayed.",
display_name="Video Sources",
scope=Scope.settings,
default=[]
......@@ -179,7 +181,7 @@ class VideoAlphaModule(VideoAlphaFields, XModule):
# isn't on the filesystem
'data_dir': getattr(self, 'data_dir', None),
'caption_asset_path': caption_asset_path,
'show_captions': self.show_captions,
'show_captions': json.dumps(self.show_captions),
'start': self.start_time,
'end': self.end_time,
'autoplay': settings.MITX_FEATURES.get('AUTOPLAY_VIDEOS', True)
......@@ -211,11 +213,6 @@ class VideoAlphaDescriptor(VideoAlphaFields, TabsEditingDescriptor, RawDescripto
self._model_data.update(model_data)
del self.data
@property
def non_editable_metadata_fields(self):
non_editable_fields = super(TabsEditingDescriptor, self).non_editable_metadata_fields
return non_editable_fields + [VideoAlphaFields.start_time, VideoAlphaFields.end_time]
@classmethod
def from_xml(cls, xml_data, system, org=None, course=None):
"""
......@@ -234,16 +231,7 @@ class VideoAlphaDescriptor(VideoAlphaFields, TabsEditingDescriptor, RawDescripto
def export_to_xml(self, resource_fs):
"""
Returns an xml string representing this module, and all modules
underneath it. May also write required resources out to resource_fs
Assumes that modules have single parentage (that no module appears twice
in the same course), and that it is thus safe to nest modules as xml
children as appropriate.
The returned XML should be able to be parsed back into an identical
XModuleDescriptor using the from_xml method with the same system, org,
and course
Returns an xml string representing this module.
"""
xml = etree.Element('videoalpha')
attrs = {
......@@ -347,7 +335,7 @@ class VideoAlphaDescriptor(VideoAlphaFields, TabsEditingDescriptor, RawDescripto
def _parse_time(str_time):
"""Converts s in '12:34:45' format to seconds. If s is
None, returns empty string"""
if str_time is None or str_time == '':
if not str_time:
return ''
else:
obj_time = time.strptime(str_time, '%H:%M:%S')
......
lms/djangoapps/courseware/tests/test_videoalpha_mongo.py
......@@ -55,7 +55,7 @@ class TestVideo(BaseTestXmodule):
expected_context = {
'data_dir': getattr(self, 'data_dir', None),
'caption_asset_path': '/c4x/MITx/999/asset/subs_',
'show_captions': True,
'show_captions': 'true',
'display_name': 'A Name',
'end': 3610.0,
'id': self.item_module.location.html_id(),
......@@ -104,7 +104,7 @@ class TestVideoNonYouTube(TestVideo):
expected_context = {
'data_dir': getattr(self, 'data_dir', None),
'caption_asset_path': '/c4x/MITx/999/asset/subs_',
'show_captions': True,
'show_captions': 'true',
'display_name': 'A Name',
'end': 3610.0,
'id': self.item_module.location.html_id(),
......
lms/djangoapps/courseware/tests/test_videoalpha_xml.py
......@@ -86,7 +86,7 @@ class VideoAlphaModuleUnitTest(unittest.TestCase):
'end': 3610.0,
'start': 3603.0,
'id': module.location.html_id(),
'show_captions': True,
'show_captions': 'true',
'sources': sources,
'youtube_streams': _create_youtube_string(module),
'track': '',
......
lms/templates/videoalpha.html
......@@ -58,17 +58,13 @@
<a href="#" class="add-fullscreen" title="${_('Fill browser')}">${_('Fill browser')}</a>
<a href="#" class="quality_control" title="${_('HD')}">${_('HD')}</a>
% if show_captions == 'true':
<a href="#" class="hide-subtitles" title="${_('Turn off captions')}">${_('Captions')}</a>
% endif
<a href="#" class="hide-subtitles" title="${_('Turn off captions')}">Captions</a>
</div>
</div>
</section>
</article>
% if show_captions == 'true':
<ol class="subtitles"><li></li></ol>
% endif
</div>
</div>
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment