Merge pull request #1241 from MITx/bug/dhm/jan13

Fix iframe conversion problem (111)

cms/djangoapps/contentstore/course_info_model.py

 from xmodule.modulestore.exceptions import ItemNotFoundError
 from xmodule.modulestore import Location
 from xmodule.modulestore.django import modulestore
-from lxml import etree
+from lxml import html
 import re
 from django.http import HttpResponseBadRequest
 import logging
@@ -24,9 +24,9 @@ def get_course_updates(location):
     
     # purely to handle free formed updates not done via editor. Actually kills them, but at least doesn't break.
     try:
-        course_html_parsed = etree.fromstring(course_updates.definition['data'])
-    except etree.XMLSyntaxError:
-        course_html_parsed = etree.fromstring("<ol></ol>")
+        course_html_parsed = html.fromstring(course_updates.definition['data'])
+    except:
+        course_html_parsed = html.fromstring("<ol></ol>")
         
     # Confirm that root is <ol>, iterate over <li>, pull out <h2> subs and then rest of val
     course_upd_collection = []
@@ -39,7 +39,7 @@ def get_course_updates(location):
                 # could enforce that update[0].tag == 'h2'
                 content = update[0].tail
             else:
-                content = "\n".join([etree.tostring(ele) for ele in update[1:]])
+                content = "\n".join([html.tostring(ele) for ele in update[1:]])
                 
             # make the id on the client be 1..len w/ 1 being the oldest and len being the newest
             course_upd_collection.append({"id" : location_base + "/" + str(len(course_html_parsed) - idx),
@@ -61,17 +61,17 @@ def update_course_updates(location, update, passed_id=None):
     
     # purely to handle free formed updates not done via editor. Actually kills them, but at least doesn't break.
     try:
-        course_html_parsed = etree.fromstring(course_updates.definition['data'])
-    except etree.XMLSyntaxError:
-        course_html_parsed = etree.fromstring("<ol></ol>")
+        course_html_parsed = html.fromstring(course_updates.definition['data'])
+    except:
+        course_html_parsed = html.fromstring("<ol></ol>")
 
     # No try/catch b/c failure generates an error back to client
-    new_html_parsed = etree.fromstring('<li><h2>' + update['date'] + '</h2>' + update['content'] + '</li>')
+    new_html_parsed = html.fromstring('<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':
         # ??? Should this use the id in the json or in the url or does it matter?
-        if passed_id:
+        if passed_id is not None:
             idx = get_idx(passed_id)
             # idx is count from end of list
             course_html_parsed[-idx] = new_html_parsed
@@ -82,7 +82,7 @@ def update_course_updates(location, update, passed_id=None):
             passed_id = course_updates.location.url() + "/" + str(idx)
         
         # update db record
-        course_updates.definition['data'] = etree.tostring(course_html_parsed)
+        course_updates.definition['data'] = html.tostring(course_html_parsed)
         modulestore('direct').update_item(location, course_updates.definition['data'])
         
         return {"id" : passed_id,
@@ -105,9 +105,9 @@ def delete_course_update(location, update, passed_id):
     # TODO use delete_blank_text parser throughout and cache as a static var in a class
     # purely to handle free formed updates not done via editor. Actually kills them, but at least doesn't break.
     try:
-        course_html_parsed = etree.fromstring(course_updates.definition['data'])
-    except etree.XMLSyntaxError:
-        course_html_parsed = etree.fromstring("<ol></ol>")
+        course_html_parsed = html.fromstring(course_updates.definition['data'])
+    except:
+        course_html_parsed = html.fromstring("<ol></ol>")
         
     if course_html_parsed.tag == 'ol':
         # ??? Should this use the id in the json or in the url or does it matter?
@@ -118,7 +118,7 @@ def delete_course_update(location, update, passed_id):
             course_html_parsed.remove(element_to_delete)
 
         # update db record
-        course_updates.definition['data'] = etree.tostring(course_html_parsed)
+        course_updates.definition['data'] = html.tostring(course_html_parsed)
         store = modulestore('direct')
         store.update_item(location, course_updates.definition['data'])  
           

cms/djangoapps/contentstore/tests/test_course_updates.py

+from cms.djangoapps.contentstore.tests.test_course_settings import CourseTestCase
+from django.core.urlresolvers import reverse
+import json
+from cms.djangoapps.contentstore.course_info_model import update_course_updates
+
+
+class CourseUpdateTest(CourseTestCase):
+    def test_course_update(self):
+        # first get the update to force the creation
+        url = reverse('course_info', kwargs={'org' : self.course_location.org, 'course' : self.course_location.course, 
+                                       'name' : self.course_location.name })
+        self.client.get(url)
+
+        content = '<iframe width="560" height="315" src="http://www.youtube.com/embed/RocY-Jd93XU" frameborder="0"></iframe>'
+        payload = { 'content' : content,
+                   'date' : 'January 8, 2013'}
+        # No means to post w/ provided_id missing. django doesn't handle. So, go direct for the create
+        payload = update_course_updates(['i4x', self.course_location.org, self.course_location.course, 'course_info', "updates"] , payload)
+        
+        url = reverse('course_info', kwargs={'org' : self.course_location.org, 'course' : self.course_location.course,
+                                             'provided_id' : payload['id']})
+         
+        self.assertHTMLEqual(content, payload['content'], "single iframe")
+        
+        content += '<div>div <p>p</p></div>'
+        payload['content'] = content
+        resp = self.client.post(url, json.dumps(payload), "application/json")
+        
+        self.assertHTMLEqual(content, json.loads(resp.content)['content'], "iframe w/ div")

cms/djangoapps/contentstore/views.py

@@ -991,7 +991,7 @@ def course_info_updates(request, org, course, provided_id=None):
     elif request.method == 'POST':
         try:
             return HttpResponse(json.dumps(update_course_updates(location, request.POST, provided_id)), mimetype="application/json")
-        except etree.XMLSyntaxError:
+        except:
             return HttpResponseBadRequest("Failed to save: malformed html", content_type="text/plain")