Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: log correct time and duration for video subsections #36019

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions xmodule/js/spec/video/video_control_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,26 @@
});

describe('constructor with end-time', function() {
it('displays the correct time when startTime and endTime are specified', function(done) {
state = jasmine.initializePlayer({
start: 10,
end: 20
});
spyOn(state.videoPlayer, 'duration').and.returnValue(60);

state.videoControl.updateVcrVidTime({
time: 15,
duration: 60
});

jasmine.waitUntil(function() {
var expectedValue = $('.video-controls').find('.vidtime');
return expectedValue.text().indexOf('0:05 / 0:20') !== -1; // Expecting 15 seconds - 10 seconds = 5 seconds
}).then(function() {
expect($('.video-controls').find('.vidtime')).toHaveText('0:05 / 0:20');
}).always(done);
});

it(
'saved position is 0, timer slider and VCR set to 0:00 '
+ 'and ending at specified end-time',
Expand Down
49 changes: 49 additions & 0 deletions xmodule/js/spec/video/video_events_plugin_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,55 @@ import '../helper.js';
destroy: plugin.destroy
});
});

describe('getCurrentTime method', function() {
it('returns current time adjusted by startTime if video starts from a subsection', function() {
spyOn(state.videoPlayer, 'currentTime', 'get').and.returnValue(120);
state.config.startTime = 30;
expect(state.videoEventsPlugin.getCurrentTime()).toBe(90); // 120 - 30 = 90
});

it('returns 0 if currentTime is undefined', function() {
spyOn(state.videoPlayer, 'currentTime', 'get').and.returnValue(undefined);
state.config.startTime = 30; // Start time is irrelevant since current time is undefined
expect(state.videoEventsPlugin.getCurrentTime()).toBe(0);
});

it('returns unadjusted current time if startTime is not defined', function() {
spyOn(state.videoPlayer, 'currentTime', 'get').and.returnValue(60);
expect(state.videoEventsPlugin.getCurrentTime()).toBe(60); // Returns current time as is
});
});

describe('log method', function() {
it('logs event with adjusted duration when startTime and endTime are defined', function() {
state.config.startTime = 30;
state.config.endTime = 150;
state.duration = 200;

state.videoEventsPlugin.log('test_event', {});

expect(Logger.log).toHaveBeenCalledWith('test_event', {
id: 'id',
code: this.code,
duration: 120, // 150 - 30 = 120
});
});

it('logs event with full duration when startTime and endTime are not defined', function() {
state.config.startTime = undefined;
state.config.endTime = undefined;
state.duration = 200;

state.videoEventsPlugin.log('test_event', {});

expect(Logger.log).toHaveBeenCalledWith('test_event', {
id: 'id',
code: this.code,
duration: 200 // Full duration as no start/end time adjustment is needed
});
});
});
});

describe('VideoPlayer Events plugin', function() {
Expand Down
11 changes: 9 additions & 2 deletions xmodule/js/src/video/04_video_control.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,17 @@
}

function updateVcrVidTime(params) {
var endTime = (this.config.endTime !== null) ? this.config.endTime : params.duration;
var endTime = (this.config.endTime !== null) ? this.config.endTime : params.duration,
startTime, currentTime;
// in case endTime is accidentally specified as being greater than the video
endTime = Math.min(endTime, params.duration);
this.videoControl.vidTimeEl.text(Time.format(params.time) + ' / ' + Time.format(endTime));
startTime = this.config.startTime > 0 ? this.config.startTime : 0;
// if it's a subsection of video, use the clip duration as endTime
if (startTime && this.config.endTime) {
endTime = this.config.endTime - startTime;
}
currentTime = startTime ? params.time - startTime : params.time;
this.videoControl.vidTimeEl.text(Time.format(currentTime) + ' / ' + Time.format(endTime));
}
}
);
Expand Down
17 changes: 14 additions & 3 deletions xmodule/js/src/video/09_events_plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,8 +143,15 @@
},

getCurrentTime: function() {
var player = this.state.videoPlayer;
return player ? player.currentTime : 0;
var player = this.state.videoPlayer,
startTime = this.state.config.startTime,
currentTime;
currentTime = player ? player.currentTime : 0;
// if video didn't start from 0(it's a subsection of video), subtract the additional time at start
if (startTime) {
currentTime = currentTime ? currentTime - startTime : 0;
}
return currentTime;
},

getCurrentLanguage: function() {
Expand All @@ -153,11 +160,15 @@
},

log: function(eventName, data) {
// use startTime and endTime to calculate the duration to handle the case where only a subsection of video is used
var endTime = this.state.config.endTime || this.state.duration,
startTime = this.state.config.startTime;

var logInfo = _.extend({
id: this.state.id,
// eslint-disable-next-line no-nested-ternary
code: this.state.isYoutubeType() ? this.state.youtubeId() : this.state.canPlayHLS ? 'hls' : 'html5',
duration: this.state.duration
duration: endTime - startTime
}, data, this.options.data);
Logger.log(eventName, logInfo);
}
Expand Down
Loading