Skip to content

Commit

Permalink
fix: log correct time and duration for video subsections (#36019)
Browse files Browse the repository at this point in the history
* fix: log correct time and duration for video subsections

If a subsection of a video is added to a course using advanced settings, show and log correct start and end times to the users.

* test: add unit tests
  • Loading branch information
a-asad authored Dec 23, 2024
1 parent 644e7e7 commit bfcaa13
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 5 deletions.
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

0 comments on commit bfcaa13

Please sign in to comment.