Commit f9d85e65 by David Ormsbee

Remove unnecessary VideoModule save_states.

Before this commit, roughly half the save_state AJAX calls for the
VideoModule were done at load time, in order to send information about
YouTube availability (the value for which was almost always true).

This commit sends the value for what the LMS already thinks YouTube
availability is for this user, so that the AJAX callback is only
used in the case where the client side sees something different.

[PERF-262]
parent ea522a17
......@@ -9,12 +9,14 @@
.createSpy('onTouchBasedDevice')
.and.returnValue(null);
state = jasmine.initializePlayer();
state = jasmine.initializePlayer({
recordedYoutubeIsAvailable: true
});
spyOn(state.storage, 'setItem');
});
afterEach(function () {
$('source').remove();
window.onTouchBasedDevice = oldOTBD;
state.storage.clear();
......@@ -199,7 +201,20 @@
expect(state.storage.setItem).toHaveBeenCalledWith('language', 'ua');
});
it('can save information about youtube availability', function () {
it('can save youtube availability', function () {
$.ajax.calls.reset();
// Test the cases where we shouldn't send anything at all -- client
// side code determines that YouTube availability is the same as
// what's already been recorded on the server side.
state.config.recordedYoutubeIsAvailable = true;
state.el.trigger('youtube_availability', [true]);
state.config.recordedYoutubeIsAvailable = false;
state.el.trigger('youtube_availability', [false]);
expect($.ajax).not.toHaveBeenCalled();
// Test that we can go from unavailable -> available
state.config.recordedYoutubeIsAvailable = false;
state.el.trigger('youtube_availability', [true]);
expect($.ajax).toHaveBeenCalledWith({
url: state.config.saveStateUrl,
......@@ -208,6 +223,17 @@
dataType: 'json',
data: {youtube_is_available: true}
});
// Test that we can go from available -> unavailable
state.config.recordedYoutubeIsAvailable = true;
state.el.trigger('youtube_availability', [false]);
expect($.ajax).toHaveBeenCalledWith({
url: state.config.saveStateUrl,
type: 'POST',
async: true,
dataType: 'json',
data: {youtube_is_available: false}
});
});
it('can destroy itself', function () {
......
......@@ -84,7 +84,12 @@ define('video/09_save_state_plugin.js', [], function() {
},
onYoutubeAvailability: function (event, youtubeIsAvailable) {
this.saveState(true, {youtube_is_available: youtubeIsAvailable});
// Compare what the client-side code has determined Youtube
// availability to be (true/false) vs. what the LMS recorded for
// this user. The LMS will assume YouTube is available by default.
if (youtubeIsAvailable !== this.state.config.recordedYoutubeIsAvailable) {
this.saveState(true, {youtube_is_available: youtubeIsAvailable});
}
},
saveState: function (async, data) {
......
......@@ -332,7 +332,12 @@ class VideoModule(VideoFields, VideoTranscriptsMixin, VideoStudentViewHandlers,
## There is no option in the "Advanced Editor" to set this option. However,
## this option will have an effect if changed to "True". The code on
## front-end exists.
'autohideHtml5': False
'autohideHtml5': False,
# This is the server's guess at whether youtube is available for
# this user, based on what was recorded the last time we saw the
# user, and defaulting to True.
'recordedYoutubeIsAvailable': self.youtube_is_available,
}
bumperize(self)
......
......@@ -79,6 +79,7 @@ class TestVideoYouTube(TestVideo):
self.item_descriptor, 'transcript', 'available_translations'
).rstrip('/?'),
"autohideHtml5": False,
"recordedYoutubeIsAvailable": True,
})),
'track': None,
'transcript_download_format': 'srt',
......@@ -157,6 +158,7 @@ class TestVideoNonYouTube(TestVideo):
self.item_descriptor, 'transcript', 'available_translations'
).rstrip('/?'),
"autohideHtml5": False,
"recordedYoutubeIsAvailable": True,
})),
'track': None,
'transcript_download_format': 'srt',
......@@ -211,6 +213,7 @@ class TestGetHtmlMethod(BaseTestXmodule):
self.item_descriptor, 'transcript', 'available_translations'
).rstrip('/?'),
"autohideHtml5": False,
"recordedYoutubeIsAvailable": True,
})
def test_get_html_track(self):
......@@ -1379,6 +1382,7 @@ class TestVideoWithBumper(TestVideo):
self.item_descriptor, 'transcript', 'available_translations'
).rstrip('/?'),
"autohideHtml5": False,
"recordedYoutubeIsAvailable": True,
})),
'track': None,
'transcript_download_format': 'srt',
......
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