Fixed course_info parsing bug.

Addressed some of Christina's review comments.

cms/djangoapps/contentstore/course_info_model.py

@@ -4,6 +4,7 @@ from xmodule.modulestore.django import modulestore
 from lxml import etree
 import re
 from django.http import HttpResponseBadRequest
+import logging
 
 ## TODO store as array of { date, content } and override  course_info_module.definition_from_xml
 ## This should be in a class which inherits from XmlDescriptor
@@ -65,9 +66,11 @@ def update_course_updates(location, update, passed_id=None):
         course_html_parsed = etree.fromstring("<ol></ol>")
 
     try:
-        new_html_parsed = etree.fromstring(update['content'], etree.XMLParser(remove_blank_text=True))
+        new_html_parsed = etree.fromstring('<li><h2>' + update['date'] + '</h2>' + update['content'] + '</li>', 
+                                           etree.XMLParser(remove_blank_text=True))
     except etree.XMLSyntaxError:
-        new_html_parsed = None
+        logging.debug("Mashing malformed update")
+        new_html_parsed = '<li><h2>' + update['date'] + '</h2>' + update['content'] + '</li>'
         
     # Confirm that root is <ol>, iterate over <li>, pull out <h2> subs and then rest of val
     if course_html_parsed.tag == 'ol':
@@ -75,29 +78,9 @@ def update_course_updates(location, update, passed_id=None):
         if passed_id:
             idx = get_idx(passed_id)
             # idx is count from end of list
-            element = course_html_parsed[-idx]
-            element[0].text = update['date']
-            if (len(element) == 1):
-                if new_html_parsed is not None:
-                    element[0].tail = None
-                    element.append(new_html_parsed)
-                else:
-                    element[0].tail = update['content']
-            else:
-                if new_html_parsed is not None:
-                    element[1] = new_html_parsed
-                else:
-                    element.pop(1)
-                    element[0].tail = update['content']
+            course_html_parsed[-idx] = new_html_parsed
         else:
-            element =  etree.Element("li")
-            course_html_parsed.insert(0, element)
-            date_element = etree.SubElement(element, "h2")
-            date_element.text = update['date']
-            if new_html_parsed is not None:
-                element.append(new_html_parsed)
-            else:
-                date_element.tail = update['content']
+            course_html_parsed.insert(0, new_html_parsed)
 
             idx = len(course_html_parsed)
             passed_id = course_updates.location.url() + "/" + str(idx)

cms/djangoapps/models/settings/course_details.py

@@ -9,6 +9,7 @@ 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
+import logging
 
 
 class CourseDetails:
@@ -155,7 +156,7 @@ class CourseDetails:
         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
+            logging.warn("ignoring the content because it doesn't not conform to expected pattern: " + raw_video)
             return None
     
     @staticmethod

cms/static/js/models/settings/course_details.js

@@ -49,7 +49,7 @@ CMS.Models.Settings.CourseDetails = Backbone.Model.extend({
 		if (newattrs.end_date && newattrs.enrollment_end && newattrs.end_date < newattrs.enrollment_end) {
 			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')) {
+		if (newattrs.intro_video && newattrs.intro_video !== this.get('intro_video')) {
 			if (this._videokey_illegal_chars.exec(newattrs.intro_video)) {
 				errors.intro_video = "Key should only contain letters, numbers, _, or -";
 			}
@@ -71,7 +71,7 @@ CMS.Models.Settings.CourseDetails = Backbone.Model.extend({
 		if (_.isEmpty(newsource) && !_.isEmpty(this.get('intro_video'))) this.save({'intro_video': null}); 
 		// TODO remove all whitespace w/in string
 		else {
-			if (this.get('intro_video') != newsource) this.save('intro_video', newsource);
+			if (this.get('intro_video') !== newsource) this.save('intro_video', newsource);
 		}
 		
 		return this.videosourceSample();