From 6f0e5d68d7b75fc954703582700bd5aa6e971f40 Mon Sep 17 00:00:00 2001 From: Pooja Kulkarni Date: Thu, 20 Dec 2018 11:56:43 +0530 Subject: [PATCH] Make video contents visible to unenrolled users This is based on PR #19284 and is part of the series of work related to the proposal #18134. Adds VideoModule.public_view() to enable unenrolled and anonymous users to view the video contents of a public course. When an unenrolled or anonymous user accesses the video content of a public course, the public_view() introduced in the previous PR is used instead of student_view() method. --- .../video/video_save_state_plugin_spec.js | 22 ++++++++++ .../js/src/video/09_save_state_plugin.js | 40 ++++++++++--------- common/lib/xmodule/xmodule/seq_module.py | 3 +- .../xmodule/video_module/video_module.py | 12 +++++- .../courseware/tests/test_video_mongo.py | 23 ++++++++++- lms/djangoapps/lms_xblock/runtime.py | 4 +- 6 files changed, 80 insertions(+), 24 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_save_state_plugin_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_save_state_plugin_spec.js index 83d26a5521..4679d4c0c6 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_save_state_plugin_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/video_save_state_plugin_spec.js @@ -128,6 +128,7 @@ import * as Time from 'time.js'; }); function itSpec(value) { + state.config.saveStateEnabled = true; var asyncVal = value.asyncVal, speedVal = value.speedVal, positionVal = value.positionVal, @@ -162,6 +163,11 @@ import * as Time from 'time.js'; }); it('can save state on speed change', function() { + state.el.trigger('speedchange', ['2.0']); + expect($.ajax).not.toHaveBeenCalledWith({ + url: state.config.saveStateUrl + }); + state.config.saveStateEnabled = true; state.el.trigger('speedchange', ['2.0']); expect($.ajax).toHaveBeenCalledWith({ url: state.config.saveStateUrl, @@ -173,6 +179,12 @@ import * as Time from 'time.js'; }); it('can save state on page unload', function() { + $.ajax.calls.reset(); + state.videoSaveStatePlugin.onUnload(); + expect($.ajax).not.toHaveBeenCalledWith({ + url: state.config.saveStateUrl + }) + state.config.saveStateEnabled = true; $.ajax.calls.reset(); state.videoSaveStatePlugin.onUnload(); expect($.ajax).toHaveBeenCalledWith({ @@ -185,6 +197,11 @@ import * as Time from 'time.js'; }); it('can save state on pause', function() { + state.el.trigger('pause'); + expect($.ajax).not.toHaveBeenCalledWith({ + url: state.config.saveStateUrl + }) + state.config.saveStateEnabled = true; state.el.trigger('pause'); expect($.ajax).toHaveBeenCalledWith({ url: state.config.saveStateUrl, @@ -213,6 +230,11 @@ import * as Time from 'time.js'; expect($.ajax).not.toHaveBeenCalled(); // Test that we can go from unavailable -> available + state.config.saveStateEnabled = false; + state.config.recordedYoutubeIsAvailable = false; + state.el.trigger('youtube_availability', [true]); + expect($.ajax).not.toHaveBeenCalled() + state.config.saveStateEnabled = true; state.config.recordedYoutubeIsAvailable = false; state.el.trigger('youtube_availability', [true]); expect($.ajax).toHaveBeenCalledWith({ diff --git a/common/lib/xmodule/xmodule/js/src/video/09_save_state_plugin.js b/common/lib/xmodule/xmodule/js/src/video/09_save_state_plugin.js index 3121961a23..726d07043d 100644 --- a/common/lib/xmodule/xmodule/js/src/video/09_save_state_plugin.js +++ b/common/lib/xmodule/xmodule/js/src/video/09_save_state_plugin.js @@ -99,28 +99,30 @@ }, saveState: function(async, data) { - if (!($.isPlainObject(data))) { - data = { - saved_video_position: this.state.videoPlayer.currentTime - }; - } + if (this.state.config.saveStateEnabled) { + if (!($.isPlainObject(data))) { + data = { + saved_video_position: this.state.videoPlayer.currentTime + }; + } - if (data.speed) { - this.state.storage.setItem('speed', data.speed, true); - } + if (data.speed) { + this.state.storage.setItem('speed', data.speed, true); + } - if (_.has(data, 'saved_video_position')) { - this.state.storage.setItem('savedVideoPosition', data.saved_video_position, true); - data.saved_video_position = Time.formatFull(data.saved_video_position); - } + if (_.has(data, 'saved_video_position')) { + this.state.storage.setItem('savedVideoPosition', data.saved_video_position, true); + data.saved_video_position = Time.formatFull(data.saved_video_position); + } - $.ajax({ - url: this.state.config.saveStateUrl, - type: 'POST', - async: !!async, - dataType: 'json', - data: data - }); + $.ajax({ + url: this.state.config.saveStateUrl, + type: 'POST', + async: !!async, + dataType: 'json', + data: data + }); + } } }; diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 6f8916ce1f..cf0fa057ef 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -497,7 +497,8 @@ class SequenceModule(SequenceFields, ProctoringFields, XModule): if is_user_authenticated: if item.location.block_type == 'vertical': - iteminfo['complete'] = completion_service.vertical_is_complete(item) + if completion_service: + iteminfo['complete'] = completion_service.vertical_is_complete(item) contents.append(iteminfo) diff --git a/common/lib/xmodule/xmodule/video_module/video_module.py b/common/lib/xmodule/xmodule/video_module/video_module.py index 1bc57e0d7f..6e2dc44790 100644 --- a/common/lib/xmodule/xmodule/video_module/video_module.py +++ b/common/lib/xmodule/xmodule/video_module/video_module.py @@ -38,7 +38,7 @@ from xmodule.modulestore.inheritance import InheritanceKeyValueStore, own_metada from xmodule.raw_module import EmptyDataRawDescriptor from xmodule.validation import StudioValidation, StudioValidationMessage from xmodule.video_module import manage_video_subtitles_save -from xmodule.x_module import XModule, module_attr +from xmodule.x_module import XModule, module_attr, PUBLIC_VIEW, STUDENT_VIEW from xmodule.xml_module import deserialize_field, is_pointer_tag, name_to_pathname from .bumper_utils import bumperize @@ -54,6 +54,7 @@ from .transcripts_utils import ( from .video_handlers import VideoStudentViewHandlers, VideoStudioViewHandlers from .video_utils import create_youtube_string, format_xml_exception_message, get_poster, rewrite_video_url from .video_xfields import VideoFields +from web_fragments.fragment import Fragment # The following import/except block for edxval is temporary measure until # edxval is a proper XBlock Runtime Service. @@ -208,7 +209,13 @@ class VideoModule(VideoFields, VideoTranscriptsMixin, VideoStudentViewHandlers, return False - def get_html(self): + def public_view(self, context): + """ + Returns a fragment that contains the html for the public view + """ + return Fragment(self.get_html(view=PUBLIC_VIEW)) + + def get_html(self, view=STUDENT_VIEW): track_status = (self.download_track and self.track) transcript_download_format = self.transcript_download_format if not track_status else None @@ -338,6 +345,7 @@ class VideoModule(VideoFields, VideoTranscriptsMixin, VideoStudentViewHandlers, autoadvance_this_video = self.auto_advance and autoadvance_enabled metadata = { + 'saveStateEnabled': view != PUBLIC_VIEW, 'saveStateUrl': self.system.ajax_url + '/save_user_state', 'autoplay': settings.FEATURES.get('AUTOPLAY_VIDEOS', False), 'streams': self.youtube_streams, diff --git a/lms/djangoapps/courseware/tests/test_video_mongo.py b/lms/djangoapps/courseware/tests/test_video_mongo.py index 9dd142d0f6..bfeafb6e4b 100644 --- a/lms/djangoapps/courseware/tests/test_video_mongo.py +++ b/lms/djangoapps/courseware/tests/test_video_mongo.py @@ -51,7 +51,7 @@ from xmodule.video_module.video_module import ( EXPORT_IMPORT_COURSE_DIR, EXPORT_IMPORT_STATIC_DIR, ) -from xmodule.x_module import STUDENT_VIEW +from xmodule.x_module import STUDENT_VIEW, PUBLIC_VIEW from .helpers import BaseTestXmodule from .test_video_handlers import TestVideo @@ -97,6 +97,7 @@ class TestVideoYouTube(TestVideo): 'id': self.item_descriptor.location.html_id(), 'metadata': json.dumps(OrderedDict({ 'autoAdvance': False, + 'saveStateEnabled': True, 'saveStateUrl': self.item_descriptor.xmodule_runtime.ajax_url + '/save_user_state', 'autoplay': False, 'streams': '0.75:jNCf2gIqpeE,1.00:ZwkTiUPN0mg,1.25:rsq9auxASqI,1.50:kMyNdzVHHgg', @@ -179,6 +180,7 @@ class TestVideoNonYouTube(TestVideo): 'id': self.item_descriptor.location.html_id(), 'metadata': json.dumps(OrderedDict({ 'autoAdvance': False, + 'saveStateEnabled': True, 'saveStateUrl': self.item_descriptor.xmodule_runtime.ajax_url + '/save_user_state', 'autoplay': False, 'streams': '1.00:3_yD_cEKoCk', @@ -238,6 +240,7 @@ class TestGetHtmlMethod(BaseTestXmodule): self.setup_course() self.default_metadata_dict = OrderedDict({ 'autoAdvance': False, + 'saveStateEnabled': True, 'saveStateUrl': '', 'autoplay': settings.FEATURES.get('AUTOPLAY_VIDEOS', True), 'streams': '1.00:3_yD_cEKoCk', @@ -971,6 +974,22 @@ class TestGetHtmlMethod(BaseTestXmodule): context = self.item_descriptor.render(STUDENT_VIEW).content self.assertIn("'download_video_link': None", context) + def test_html_student_public_view(self): + """ + Test the student and public views + """ + video_xml = """ + + """ + + self.initialize_module(data=video_xml) + context = self.item_descriptor.render(STUDENT_VIEW).content + self.assertIn('"saveStateEnabled": true', context) + context = self.item_descriptor.render(PUBLIC_VIEW).content + self.assertIn('"saveStateEnabled": false', context) + @patch('xmodule.video_module.video_module.edxval_api.get_course_video_image_url') def test_poster_image(self, get_course_video_image_url): """ @@ -2135,6 +2154,7 @@ class TestVideoWithBumper(TestVideo): 'id': self.item_descriptor.location.html_id(), 'metadata': json.dumps(OrderedDict({ 'autoAdvance': False, + 'saveStateEnabled': True, 'saveStateUrl': self.item_descriptor.xmodule_runtime.ajax_url + '/save_user_state', 'autoplay': False, 'streams': '0.75:jNCf2gIqpeE,1.00:ZwkTiUPN0mg,1.25:rsq9auxASqI,1.50:kMyNdzVHHgg', @@ -2208,6 +2228,7 @@ class TestAutoAdvanceVideo(TestVideo): 'bumper_metadata': 'null', 'metadata': json.dumps(OrderedDict({ 'autoAdvance': autoadvance_flag, + 'saveStateEnabled': True, 'saveStateUrl': self.item_descriptor.xmodule_runtime.ajax_url + '/save_user_state', 'autoplay': False, 'streams': '0.75:jNCf2gIqpeE,1.00:ZwkTiUPN0mg,1.25:rsq9auxASqI,1.50:kMyNdzVHHgg', diff --git a/lms/djangoapps/lms_xblock/runtime.py b/lms/djangoapps/lms_xblock/runtime.py index 6cbeabf5c5..d25516995b 100644 --- a/lms/djangoapps/lms_xblock/runtime.py +++ b/lms/djangoapps/lms_xblock/runtime.py @@ -137,7 +137,9 @@ class LmsModuleSystem(ModuleSystem): # pylint: disable=abstract-method store = modulestore() services = kwargs.setdefault('services', {}) - services['completion'] = CompletionService(user=kwargs.get('user'), course_key=kwargs.get('course_id')) + user = kwargs.get('user') + if user and user.is_authenticated: + services['completion'] = CompletionService(user=user, course_key=kwargs.get('course_id')) services['fs'] = xblock.reference.plugins.FSService() services['i18n'] = ModuleI18nService services['library_tools'] = LibraryToolsService(store)