Merge pull request #2906 from edx/anton/fix-for-different-videoids

Video: Fix issues related to videos that have separate YouTube IDs for the different video speeds.
openedx · Mar 11, 2014 · 14d0378 · 14d0378
2 parents e117957 + 40c1090
commit 14d0378
Show file tree

Hide file tree

Showing 13 changed files with 269 additions and 134 deletions.
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
@@ -5,6 +5,9 @@ These are notable changes in edx-platform.  This is a rolling list of changes,
 in roughly chronological order, most recent first.  Add your entries at or near
 the top.  Include a label indicating the component affected.
 
+Blades: Fix issues related to videos that have separate YouTube IDs for the
+different video speeds. BLD-915, BLD-901.
+
 Blades: Add .txt and .srt options to the "download transcript" button. BLD-844.
 
 Blades: Fix bug when transcript cutting off view in full view mode. BLD-852.

diff --git a/common/djangoapps/terrain/start_stubs.py b/common/djangoapps/terrain/start_stubs.py
@@ -15,8 +15,8 @@
 }
 
 
-@before.all
-def start_stubs():
+@before.each_scenario
+def start_stubs(_):
     """
     Start each stub service running on a local port.
     """
@@ -25,7 +25,7 @@ def start_stubs():
         setattr(world, name, fake_server)
 
 
-@after.all
+@after.each_scenario
 def stop_stubs(_):
     """
     Shut down each stub service.

diff --git a/common/djangoapps/terrain/stubs/youtube.py b/common/djangoapps/terrain/stubs/youtube.py
@@ -5,7 +5,6 @@
 from .http import StubHttpRequestHandler, StubHttpService
 import json
 import time
-import requests
 from urlparse import urlparse
 from collections import OrderedDict
 
@@ -80,7 +79,7 @@ def _send_video_response(self, youtube_id, message):
             'data': OrderedDict({
                 'id': youtube_id,
                 'message': message,
-                'duration': 60,
+                'duration': 60 if youtube_id == 'OEoXaMPEzfM' else 77,
             })
         })
         response = "{cb}({data})".format(cb=callback, data=json.dumps(data))

diff --git a/common/lib/xmodule/xmodule/js/spec/video/initialize_spec.js b/common/lib/xmodule/xmodule/js/spec/video/initialize_spec.js
@@ -233,25 +233,42 @@ function (Initialize) {
                         '1.0': 'testId',
                         '1.50': 'videoId'
                     },
-                    youtubeId: Initialize.prototype.youtubeId
+                    youtubeId: Initialize.prototype.youtubeId,
+                    isFlashMode: jasmine.createSpy().andReturn(false)
                 };
             });
 
-            it('returns duration for current video', function () {
-                var expected = Initialize.prototype.getDuration.call(state);
-
-                expect(expected).toEqual(100);
-            });
-
-            var msg = 'returns duration for the 1.0 speed as a fallback';
+            var msg = 'returns duration for the 1.0 speed if speed is not 1.0';
             it(msg, function () {
                 var expected;
 
-                state.speed = '2.0';
+                state.speed = '1.50';
                 expected = Initialize.prototype.getDuration.call(state);
 
                 expect(expected).toEqual(400);
             });
+
+            describe('Flash mode', function () {
+                it('returns duration for current video', function () {
+                    var expected;
+
+                    state.isFlashMode.andReturn(true);
+                    expected = Initialize.prototype.getDuration.call(state);
+
+                    expect(expected).toEqual(100);
+                });
+
+                var msg = 'returns duration for the 1.0 speed as a fall-back';
+                it(msg, function () {
+                    var expected;
+
+                    state.isFlashMode.andReturn(true);
+                    state.speed = '2.0';
+                    expected = Initialize.prototype.getDuration.call(state);
+
+                    expect(expected).toEqual(400);
+                });
+            });
         });
 
         describe('youtubeId', function () {
@@ -262,7 +279,8 @@ function (Initialize) {
                         '0.50': '7tqY6eQzVhE',
                         '1.0': 'cogebirgzzM',
                         '1.50': 'abcdefghijkl'
-                    }
+                    },
+                    isFlashMode: jasmine.createSpy().andReturn(false)
                 };
             });
 
@@ -278,14 +296,25 @@ function (Initialize) {
                 });
             });
 
-            describe('without speed', function () {
+            describe('without speed for flash mode', function () {
                 it('return the video id for current speed', function () {
-                    var expected = Initialize.prototype.youtubeId.call(state);
+                    var expected;
+
+                    state.isFlashMode.andReturn(true);
+                    expected = Initialize.prototype.youtubeId.call(state);
 
                     expect(expected).toEqual('abcdefghijkl');
                 });
             });
 
+            describe('without speed for youtube html5 mode', function () {
+                it('return the video id for 1.0x speed', function () {
+                    var expected = Initialize.prototype.youtubeId.call(state);
+
+                    expect(expected).toEqual('cogebirgzzM');
+                });
+            });
+
             describe('speed is absent in the list of video speeds', function () {
                 it('return the video id for 1.0x speed', function () {
                     var expected = Initialize.prototype.youtubeId.call(state, '0.0');

diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js
@@ -55,11 +55,7 @@
                     });
 
                     waitsFor(function () {
-                        if (state.videoCaption.loaded === true) {
-                            return true;
-                        }
-
-                        return false;
+                        return state.videoCaption.loaded;
                     }, 'Expect captions to be loaded.', WAIT_TIMEOUT);
 
                     runs(function () {
@@ -77,17 +73,15 @@
                     });
                 });
 
-                it('fetch the caption in Youtube mode', function () {
+                it('fetch the caption in Flash mode', function () {
                     runs(function () {
                         state = jasmine.initializePlayerYouTube();
+                        spyOn(state, 'isFlashMode').andReturn(true);
+                        state.videoCaption.fetchCaption();
                     });
 
                     waitsFor(function () {
-                        if (state.videoCaption.loaded === true) {
-                            return true;
-                        }
-
-                        return false;
+                        return state.videoCaption.loaded;
                     }, 'Expect captions to be loaded.', WAIT_TIMEOUT);
 
                     runs(function () {
@@ -106,6 +100,31 @@
                     });
                 });
 
+                it('fetch the caption in Youtube mode', function () {
+                    runs(function () {
+                        state = jasmine.initializePlayerYouTube();
+                    });
+
+                    waitsFor(function () {
+                        return state.videoCaption.loaded;
+                    }, 'Expect captions to be loaded.', WAIT_TIMEOUT);
+
+                    runs(function () {
+                        expect($.ajaxWithPrefix).toHaveBeenCalledWith({
+                            url: '/transcript/translation',
+                            notifyOnError: false,
+                            data: jasmine.any(Object),
+                            success: jasmine.any(Function),
+                            error: jasmine.any(Function)
+                        });
+                        expect($.ajaxWithPrefix.mostRecentCall.args[0].data)
+                            .toEqual({
+                                language: 'en',
+                                videoId: 'cogebirgzzM'
+                            });
+                    });
+                });
+
                 it('bind the hide caption button', function () {
                     state = jasmine.initializePlayer();
                     expect($('.hide-subtitles')).toHandleWith(

diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js
@@ -105,6 +105,7 @@ function (VideoPlayer) {
             it('create Flash player', function () {
                 var player;
 
+                spyOn($.fn, 'trigger');
                 state = jasmine.initializePlayerYouTube();
                 state.videoEl = state.el.find('video, iframe').width(100);
                 player = state.videoPlayer.player;
@@ -715,7 +716,8 @@ function (VideoPlayer) {
                     },
                     currentPlayerMode: 'html5',
                     trigger: function () {},
-                    browserIsFirefox: false
+                    browserIsFirefox: false,
+                    isFlashMode: jasmine.createSpy().andReturn(false)
                 };
             });
 
@@ -1015,7 +1017,8 @@ function (VideoPlayer) {
                         updatePlayTime: jasmine.createSpy(),
                         setPlaybackRate: jasmine.createSpy(),
                         player: jasmine.createSpyObj('player', ['setPlaybackRate'])
-                    }
+                    },
+                    isFlashMode: jasmine.createSpy().andReturn(false)
                 };
             });
 
@@ -1033,7 +1036,7 @@ function (VideoPlayer) {
                 });
 
                 it('convert the current time to the new speed', function () {
-                    state.currentPlayerMode = 'flash';
+                    state.isFlashMode.andReturn(true);
                     VideoPlayer.prototype.onSpeedChange.call(state, '0.75', false);
                     expect(state.videoPlayer.currentTime).toBe('120.000');
                 });

diff --git a/common/lib/xmodule/xmodule/js/src/video/01_initialize.js b/common/lib/xmodule/xmodule/js/src/video/01_initialize.js
@@ -65,6 +65,7 @@ function (VideoPlayer, VideoStorage) {
         getDuration: getDuration,
         getVideoMetadata: getVideoMetadata,
         initialize: initialize,
+        isFlashMode: isFlashMode,
         parseSpeed: parseSpeed,
         parseVideoSources: parseVideoSources,
         parseYoutubeStreams: parseYoutubeStreams,
@@ -550,7 +551,7 @@ function (VideoPlayer, VideoStorage) {
             _this.videos[speed] = video[1];
         });
 
-        return true;
+        return _.isString(this.videos['1.0']);
     }
 
     // function parseVideoSources(, mp4Source, webmSource, oggSource)
@@ -702,7 +703,11 @@ function (VideoPlayer, VideoStorage) {
     }
 
     function youtubeId(speed) {
-        return this.videos[speed || this.speed] || this.videos['1.0'];
+        var currentSpeed = this.isFlashMode() ? this.speed : '1.0';
+
+        return  this.videos[speed] ||
+                this.videos[currentSpeed] ||
+                this.videos['1.0'];
     }
 
     function getDuration() {
@@ -713,6 +718,10 @@ function (VideoPlayer, VideoStorage) {
         }
     }
 
+    function isFlashMode() {
+        return this.currentPlayerMode === 'flash';
+    }
+
     function getCurrentLanguage() {
         var keys = _.keys(this.config.transcriptLanguages);