From bfcaa138ff786682bd0644fd1c6b726657975aeb Mon Sep 17 00:00:00 2001 From: Abdur Rahman Asad <51022010+a-asad@users.noreply.github.com> Date: Mon, 23 Dec 2024 12:21:00 +0500 Subject: [PATCH] fix: log correct time and duration for video subsections (#36019) * 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 --- xmodule/js/spec/video/video_control_spec.js | 20 ++++++++ .../js/spec/video/video_events_plugin_spec.js | 49 +++++++++++++++++++ xmodule/js/src/video/04_video_control.js | 11 ++++- xmodule/js/src/video/09_events_plugin.js | 17 +++++-- 4 files changed, 92 insertions(+), 5 deletions(-) diff --git a/xmodule/js/spec/video/video_control_spec.js b/xmodule/js/spec/video/video_control_spec.js index 248c3c31d2be..49326290716b 100644 --- a/xmodule/js/spec/video/video_control_spec.js +++ b/xmodule/js/spec/video/video_control_spec.js @@ -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', diff --git a/xmodule/js/spec/video/video_events_plugin_spec.js b/xmodule/js/spec/video/video_events_plugin_spec.js index 860cfb5e07ba..ac809d054edc 100644 --- a/xmodule/js/spec/video/video_events_plugin_spec.js +++ b/xmodule/js/spec/video/video_events_plugin_spec.js @@ -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() { diff --git a/xmodule/js/src/video/04_video_control.js b/xmodule/js/src/video/04_video_control.js index b8cec53cae6f..c6f2ba240745 100644 --- a/xmodule/js/src/video/04_video_control.js +++ b/xmodule/js/src/video/04_video_control.js @@ -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)); } } ); diff --git a/xmodule/js/src/video/09_events_plugin.js b/xmodule/js/src/video/09_events_plugin.js index d407534c983e..6116f26e22c5 100644 --- a/xmodule/js/src/video/09_events_plugin.js +++ b/xmodule/js/src/video/09_events_plugin.js @@ -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() { @@ -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); }