From 5709ff34f5c275be5070cf77b410f027ce7433a1 Mon Sep 17 00:00:00 2001
From: Don Mitchell <dmitchell@edx.org>
Date: Tue, 18 Dec 2012 12:46:54 -0500
Subject: [PATCH] Refactored videos to a much simpler impl b/c about videos
 don't have speeds and don't use the video/default.yaml format.

---
 .../models/settings/course_details.py         |  38 +++++-
 .../js/models/settings/course_details.js      | 109 ++----------------
 .../js/views/settings/main_settings_view.js   |   9 +-
 cms/templates/settings.html                   |   2 +-
 lms/templates/staff_problem_info.html         |  26 +++--
 5 files changed, 69 insertions(+), 115 deletions(-)

diff --git a/cms/djangoapps/models/settings/course_details.py b/cms/djangoapps/models/settings/course_details.py
index e16dcc35c9..b8d15041b8 100644
--- a/cms/djangoapps/models/settings/course_details.py
+++ b/cms/djangoapps/models/settings/course_details.py
@@ -8,6 +8,8 @@ from contentstore.utils import get_modulestore
 from util.converters import jsdate_to_time, time_to_date
 from cms.djangoapps.models.settings import course_grading
 from cms.djangoapps.contentstore.utils import update_item
+import re
+
 
 class CourseDetails:
     def __init__(self, location):
@@ -58,7 +60,8 @@ class CourseDetails:
         
         temploc = temploc._replace(name='video')
         try:
-            course.intro_video = get_modulestore(temploc).get_item(temploc).definition['data']
+            raw_video = get_modulestore(temploc).get_item(temploc).definition['data']
+            course.intro_video = CourseDetails.parse_video_tag(raw_video) 
         except ItemNotFoundError:
             pass
         
@@ -127,12 +130,43 @@ class CourseDetails:
         update_item(temploc, jsondict['effort'])
         
         temploc = temploc._replace(name='video')
-        update_item(temploc, jsondict['intro_video'])
+        recomposed_video_tag = CourseDetails.recompose_video_tag(jsondict['intro_video'])
+        update_item(temploc, recomposed_video_tag)
         
                     
         # Could just generate and return a course obj w/o doing any db reads, but I put the reads in as a means to confirm
         # it persisted correctly
         return CourseDetails.fetch(course_location)
+    
+    @staticmethod
+    def parse_video_tag(raw_video):
+        """
+        Because the client really only wants the author to specify the youtube key, that's all we send to and get from the client.
+        The problem is that the db stores the html markup as well (which, of course, makes any sitewide changes to how we do videos
+        next to impossible.)
+        """
+        if not raw_video:
+            return None
+         
+        keystring_matcher = re.search('(?<=embed/)[a-zA-Z0-9_-]+', raw_video)
+        if keystring_matcher is None:
+            keystring_matcher = re.search('<?=\d+:[a-zA-Z0-9_-]+', raw_video)
+        
+        if keystring_matcher:
+            return keystring_matcher.group(0)
+        else:
+            # TODO should this be None or the raw_video? It would be good to at least log this
+            return None
+    
+    @staticmethod
+    def recompose_video_tag(video_key):
+        # TODO should this use a mako template? Of course, my hope is that this is a short-term workaround for the db not storing
+        # the right thing
+        result = '<iframe width="560" height="315" src="http://www.youtube.com/embed/' + \
+            video_key + '?autoplay=1&rel=0" frameborder="0" allowfullscreen=""></iframe>'
+        return result
+
+    
 
 # TODO move to a more general util? Is there a better way to do the isinstance model check?
 class CourseSettingsEncoder(json.JSONEncoder):
diff --git a/cms/static/js/models/settings/course_details.js b/cms/static/js/models/settings/course_details.js
index f0dc87be0d..d9beae2b7a 100644
--- a/cms/static/js/models/settings/course_details.js
+++ b/cms/static/js/models/settings/course_details.js
@@ -50,19 +50,10 @@ CMS.Models.Settings.CourseDetails = Backbone.Model.extend({
 			errors.enrollment_end = "The enrollment end date cannot be after the course end date.";
 		}
 		if (newattrs.intro_video && newattrs.intro_video != this.get('intro_video')) {
-			var videos = this.parse_videosource(newattrs.intro_video);
-			var vid_errors = new Array();
-			var cachethis = this;
-			for (var i=0; i<videos.length; i++) {
-				// doesn't call parseFloat or Number b/c they stop on first non parsable and return what they have
-				if (!isFinite(videos[i].speed)) vid_errors.push(videos[i].speed + " is not a valid speed.");
-				else if (!videos[i].key) vid_errors.push(videos[i].speed + " does not have a video id");
-				// can't use get from client to test if video exists b/c of CORS (crossbrowser get not allowed)
-				// GET "http://gdata.youtube.com/feeds/api/videos/" + videokey
-			}
-			if (!_.isEmpty(vid_errors)) {
-				errors.intro_video = vid_errors.join(' ');
+			if (this._videokey_illegal_chars.exec(newattrs.intro_video)) {
+				errors.intro_video = "Key should only contain letters, numbers, _, or -";
 			}
+			// TODO check if key points to a real video using google's youtube api
 		}
 		if (!_.isEmpty(errors)) return errors;
 		// NOTE don't return empty errors as that will be interpreted as an error state
@@ -73,100 +64,20 @@ CMS.Models.Settings.CourseDetails = Backbone.Model.extend({
 		return '/' + location.get('org') + "/" + location.get('course') + '/settings/' + location.get('name') + '/section/details';
 	},
 	
-	_videoprefix : /\s*<video\s*youtube="/g,
-	// the below is lax to enable validation
-	_videospeedparse : /[^:]*/g, // /\d+\.?\d*(?=:)/g,
-	_videokeyparse : /([^,\/>]+)/g,
-	_videonosuffix : /[^"\/>]+/g,
-	_getNextMatch : function (regex, string, cursor) {
-		regex.lastIndex = cursor;
-		var result = regex.exec(string);
-		if (_.isArray(result)) return result[0];
-		else return result;
-	},
-	// the whole string for editing (put in edit box)
-	getVideoSource: function() {
-		if (this.get('intro_video')) {
-			var cursor = 0;
-			var videostring = this.get('intro_video');
-			this._getNextMatch(this._videoprefix, videostring, cursor);
-			cursor = this._videoprefix.lastIndex;
-			return this._getNextMatch(this._videonosuffix, videostring, cursor);
-		}
-		else return "";
-	},
-	// the source closest to 1.0 speed
-	videosourceSample: function() {
-		if (this.get('intro_video')) {
-			var cursor = 0;
-			var videostring = this.get('intro_video');
-			this._getNextMatch(this._videoprefix, videostring, cursor);
-			cursor = this._videoprefix.lastIndex;
-			
-			// parse from [speed:id,/s?]* to find 1.0 or take first
-			var parsedspeed = this._getNextMatch(this._videospeedparse, videostring, cursor);
-			var bestkey;
-			if (parsedspeed) {
-				cursor = this._videospeedparse.lastIndex + 1;
-				var bestspeed = Number(parsedspeed);
-				bestkey = this._getNextMatch(this._videokeyparse, videostring, cursor);
-				cursor = this._videokeyparse.lastIndex + 1;
-				while (cursor < videostring.length && bestspeed != 1.0) {
-					parsedspeed = this._getNextMatch(this._videospeedparse, videostring, cursor);
-					if (parsedspeed) cursor = this._videospeedparse.lastIndex + 1;
-					else break;
-					if (Math.abs(Number(parsedspeed) - 1.0) < Math.abs(bestspeed - 1.0)) {
-						bestspeed = Number(parsedspeed);
-						bestkey = this._getNextMatch(this._videokeyparse, videostring, cursor);
-					}
-					else this._getNextMatch(this._videokeyparse, videostring, cursor);
-					if (this._videokeyparse.lastIndex > cursor)	cursor = this._videokeyparse.lastIndex + 1;
-					else cursor++;
-				}
-			}
-			else {
-				bestkey = this._getNextMatch(this._videokeyparse, videostring, cursor);
-			}
-			if (bestkey) {
-				// WTF? for some reason bestkey is an array [key, key] (same one repeated)
-				if (_.isArray(bestkey)) bestkey = bestkey[0];
-				return "http://www.youtube.com/embed/" + bestkey;
-			}
-			else return "";
-		}
-	},
-	parse_videosource: function(videostring) {
-		// used to validate before set so cannot get from model attr. Returns [{ speed: fff, key: sss }]
-		var cursor = 0;
-		this._getNextMatch(this._videoprefix, videostring, cursor);
-		cursor = this._videoprefix.lastIndex;
-		videostring = this._getNextMatch(this._videonosuffix, videostring, cursor);
-		cursor = 0;
-		// parsed to "fff:kkk,fff:kkk"
-		var result = new Array();
-		if (!videostring || videostring.length == 0) return result;
-		while (cursor < videostring.length) {
-			var speed = this._getNextMatch(this._videospeedparse, videostring, cursor);
-			if (speed) cursor = this._videospeedparse.lastIndex + 1;
-			else return result;
-			var key = this._getNextMatch(this._videokeyparse, videostring, cursor);
-			if (key) cursor = this._videokeyparse.lastIndex + 1;
-			// See the WTF above
-			if (_.isArray(key)) key = key[0];
-			result.push({speed: speed, key: key});
-		}
-		return result;
-	},
+	_videokey_illegal_chars : /[^a-zA-Z0-9_-]/g,
 	save_videosource: function(newsource) {
 		// newsource either is <video youtube="speed:key, *"/> or just the "speed:key, *" string
 		// returns the videosource for the preview which iss the key whose speed is closest to 1
-		if (newsource == null) this.save({'intro_video': null}); 
+		if (_.isEmpty(newsource) && !_.isEmpty(this.get('intro_video'))) this.save({'intro_video': null}); 
 		// TODO remove all whitespace w/in string
 		else {
-			var newVal = ((this._getNextMatch(this._videoprefix, newsource, 0)) ? newsource : '<video youtube="' + newsource + '"/>');
-			if (this.get('intro_video') != newVal) this.save('intro_video', newVal);
+			if (this.get('intro_video') != newsource) this.save('intro_video', newsource);
 		}
 		
 		return this.videosourceSample();
+	},
+	videosourceSample : function() {
+		 if (this.has('intro_video')) return "http://www.youtube.com/embed/" + this.get('intro_video');
+		 else return "";
 	}
 });
diff --git a/cms/static/js/views/settings/main_settings_view.js b/cms/static/js/views/settings/main_settings_view.js
index 1709f96b1c..31b8586d5a 100644
--- a/cms/static/js/views/settings/main_settings_view.js
+++ b/cms/static/js/views/settings/main_settings_view.js
@@ -193,7 +193,7 @@ CMS.Views.Settings.Details = CMS.Views.ValidatingView.extend({
 		this.$el.find('.current-course-introduction-video iframe').attr('src', this.model.videosourceSample());
 		if (this.model.has('intro_video')) {
 			this.$el.find('.remove-course-introduction-video').show();
-			this.$el.find('#' + this.fieldToSelectorMap['intro_video']).val(this.model.getVideoSource());
+			this.$el.find('#' + this.fieldToSelectorMap['intro_video']).val(this.model.get('intro_video'));
 		}
 		else this.$el.find('.remove-course-introduction-video').hide();
 		
@@ -261,6 +261,12 @@ CMS.Views.Settings.Details = CMS.Views.ValidatingView.extend({
 			this.clearValidationErrors();
 			var previewsource = this.model.save_videosource($(event.currentTarget).val());
 			this.$el.find(".current-course-introduction-video iframe").attr("src", previewsource);
+			if (this.model.has('intro_video')) {
+				this.$el.find('.remove-course-introduction-video').show();
+			}
+			else {
+				this.$el.find('.remove-course-introduction-video').hide();
+			}
 			break;
 			
 		default:
@@ -282,6 +288,7 @@ CMS.Views.Settings.Details = CMS.Views.ValidatingView.extend({
 			this.model.save_videosource(null);
 			this.$el.find(".current-course-introduction-video iframe").attr("src", "");
 			this.$el.find('#' + this.fieldToSelectorMap['intro_video']).val("");
+			this.$el.find('.remove-course-introduction-video').hide();
 		}
 	},
 	codeMirrors : {},
diff --git a/cms/templates/settings.html b/cms/templates/settings.html
index 65a84ee062..1266213906 100644
--- a/cms/templates/settings.html
+++ b/cms/templates/settings.html
@@ -230,7 +230,7 @@ from contentstore import utils
                   </div>
 
                   <div class="input">
-                    <input type="text" class="long new-course-introduction-video add-video-data" id="course-introduction-video" value="" placeholder="speed:id,speed:id" autocomplete="off">
+                    <input type="text" class="long new-course-introduction-video add-video-data" id="course-introduction-video" value="" placeholder="id" autocomplete="off">
                     <span class="tip tip-stacked">Video restrictions go here</span>
                   </div>
                 </div>
diff --git a/lms/templates/staff_problem_info.html b/lms/templates/staff_problem_info.html
index 0f1893ee4f..21044c1f80 100644
--- a/lms/templates/staff_problem_info.html
+++ b/lms/templates/staff_problem_info.html
@@ -61,16 +61,18 @@ category = ${category | h}
 
 <script type="text/javascript">
 // assumes courseware.html's loaded this method.
-setup_debug('${element_id}', 
-	%if edit_link:
-		'${edit_link}',
-	%else:
-		null,
-	%endif
-	{
-        'location': '${location}',
-        'xqa_key': '${xqa_key}',
-        'category': '${category}',
-        'user': '${user}'
-     });
+ % if staff_access:
+    setup_debug('${element_id}', 
+    	%if edit_link:
+    		'${edit_link}',
+    	%else:
+       		null,
+    	%endif
+    	{
+           'location': '${location}',
+           'xqa_key': '${xqa_key}',
+          'category': '${category}',
+          'user': '${user}'
+       });
+ % endif
 </script>