merge conflict

cms/djangoapps/github_sync/tests/test_views.py

@@ -11,33 +11,33 @@ class PostReceiveTestCase(TestCase):
     def setUp(self):
         self.client = Client()
 
-    @patch('github_sync.views.sync_with_github')
-    def test_non_branch(self, sync_with_github):
+    @patch('github_sync.views.import_from_github')
+    def test_non_branch(self, import_from_github):
         self.client.post('/github_service_hook', {'payload': json.dumps({
             'ref': 'refs/tags/foo'})
         })
-        self.assertFalse(sync_with_github.called)
+        self.assertFalse(import_from_github.called)
 
-    @patch('github_sync.views.sync_with_github')
-    def test_non_watched_repo(self, sync_with_github):
+    @patch('github_sync.views.import_from_github')
+    def test_non_watched_repo(self, import_from_github):
         self.client.post('/github_service_hook', {'payload': json.dumps({
             'ref': 'refs/heads/branch',
             'repository': {'name': 'bad_repo'}})
         })
-        self.assertFalse(sync_with_github.called)
+        self.assertFalse(import_from_github.called)
 
-    @patch('github_sync.views.sync_with_github')
-    def test_non_tracked_branch(self, sync_with_github):
+    @patch('github_sync.views.import_from_github')
+    def test_non_tracked_branch(self, import_from_github):
         self.client.post('/github_service_hook', {'payload': json.dumps({
             'ref': 'refs/heads/non_branch',
             'repository': {'name': 'repo'}})
         })
-        self.assertFalse(sync_with_github.called)
+        self.assertFalse(import_from_github.called)
 
-    @patch('github_sync.views.sync_with_github')
-    def test_tracked_branch(self, sync_with_github):
+    @patch('github_sync.views.import_from_github')
+    def test_tracked_branch(self, import_from_github):
         self.client.post('/github_service_hook', {'payload': json.dumps({
             'ref': 'refs/heads/branch',
             'repository': {'name': 'repo'}})
         })
-        sync_with_github.assert_called_with(load_repo_settings('repo'))
+        import_from_github.assert_called_with(load_repo_settings('repo'))

cms/djangoapps/github_sync/views.py

@@ -5,7 +5,7 @@ from django.http import HttpResponse
 from django.conf import settings
 from django_future.csrf import csrf_exempt
 
-from . import sync_with_github, load_repo_settings
+from . import import_from_github, load_repo_settings
 
 log = logging.getLogger()
 
@@ -46,6 +46,6 @@ def github_post_receive(request):
         log.info('Ignoring changes to non-tracked branch %s in repo %s' % (branch_name, repo_name))
         return HttpResponse('Ignoring non-tracked branch')
 
-    sync_with_github(repo)
+    import_from_github(repo)
 
     return HttpResponse('Push received')

common/djangoapps/xmodule_modifiers.py

@@ -89,8 +89,8 @@ def add_histogram(get_html, module):
         else:
             edit_link = False
 
-        staff_context = {'definition': dict(module.definition),
-                         'metadata': dict(module.metadata),
+        staff_context = {'definition': json.dumps(module.definition, indent=4),
+                         'metadata': json.dumps(module.metadata, indent=4),
                          'element_id': module.location.html_id(),
                          'edit_link': edit_link,
                          'histogram': json.dumps(histogram),

common/lib/supertrace.py

+"""
+A handy util to print a django-debug-screen-like stack trace with
+values of local variables.
+"""
+
+import sys, traceback
+from django.utils.encoding import smart_unicode
+
+
+def supertrace(max_len=160):
+    """
+    Print the usual traceback information, followed by a listing of all the
+    local variables in each frame.  Should be called from an exception handler.
+
+    if max_len is not None, will print up to max_len chars for each local variable.
+
+    (cite: modified from somewhere on stackoverflow)
+    """
+    tb = sys.exc_info()[2]
+    while True:
+        if not tb.tb_next:
+            break
+        tb = tb.tb_next
+    stack = []
+    frame = tb.tb_frame
+    while frame:
+        stack.append(f)
+        frame = frame.f_back
+    stack.reverse()
+    # First print the regular traceback
+    traceback.print_exc()
+
+    print "Locals by frame, innermost last"
+    for frame in stack:
+        print
+        print "Frame %s in %s at line %s" % (frame.f_code.co_name,
+                                             frame.f_code.co_filename,
+                                             frame.f_lineno)
+        for key, value in frame.f_locals.items():
+            print ("\t%20s = " % smart_unicode(key, errors='ignore')),
+            # We have to be careful not to cause a new error in our error
+            # printer! Calling str() on an unknown object could cause an
+            # error.
+            try:
+                s = smart_unicode(value, errors='ignore')
+                if max_len is not None:
+                    s = s[:max_len]
+                print s
+            except:
+                print "<ERROR WHILE PRINTING VALUE>"
+
+

common/lib/xmodule/lazy_dict.py

-from collections import MutableMapping
-
-class LazyLoadingDict(MutableMapping):
-    """
-    A dictionary object that lazily loads its contents from a provided
-    function on reads (of members that haven't already been set).
-    """
-
-    def __init__(self, loader):
-        '''
-        On the first read from this dictionary, it will call loader() to
-        populate its contents.  loader() must return something dict-like. Any
-        elements set before the first read will be preserved.
-        '''
-        self._contents = {}
-        self._loaded = False
-        self._loader = loader
-        self._deleted = set()
-
-    def __getitem__(self, name):
-        if not (self._loaded or name in self._contents or name in self._deleted):
-            self.load()
-
-        return self._contents[name]
-
-    def __setitem__(self, name, value):
-        self._contents[name] = value
-        self._deleted.discard(name)
-
-    def __delitem__(self, name):
-        del self._contents[name]
-        self._deleted.add(name)
-
-    def __contains__(self, name):
-        self.load()
-        return name in self._contents
-
-    def __len__(self):
-        self.load()
-        return len(self._contents)
-
-    def __iter__(self):
-        self.load()
-        return iter(self._contents)
-
-    def __repr__(self):
-        self.load()
-        return repr(self._contents)
-
-    def load(self):
-        if self._loaded:
-            return
-
-        loaded_contents = self._loader()
-        loaded_contents.update(self._contents)
-        self._contents = loaded_contents
-        self._loaded = True
-

common/lib/xmodule/progress.py

-'''
-Progress class for modules.  Represents where a student is in a module.
-
-Useful things to know:
- - Use Progress.to_js_status_str() to convert a progress into a simple
-   status string to pass to js.
- - Use Progress.to_js_detail_str() to convert a progress into a more detailed
-   string to pass to js.
-
-In particular, these functions have a canonical handing of None.
-
-For most subclassing needs, you should only need to reimplement
-frac() and __str__().
-'''
-
-from collections import namedtuple
-import numbers
-
-
-class Progress(object):
-    '''Represents a progress of a/b (a out of b done)
-
-    a and b must be numeric, but not necessarily integer, with
-    0 <= a <= b and b > 0.
-
-    Progress can only represent Progress for modules where that makes sense.  Other
-    modules (e.g. html) should return None from get_progress().
-
-    TODO: add tag for module type?  Would allow for smarter merging.
-    '''
-
-    def __init__(self, a, b):
-        '''Construct a Progress object.  a and b must be numbers, and must have
-              0 <= a <= b and b > 0
-        '''
-
-        # Want to do all checking at construction time, so explicitly check types
-        if not (isinstance(a, numbers.Number) and
-                isinstance(b, numbers.Number)):
-            raise TypeError('a and b must be numbers.  Passed {0}/{1}'.format(a, b))
-
-        if not (0 <= a <= b and b > 0):
-            raise ValueError(
-                'fraction a/b = {0}/{1} must have 0 <= a <= b and b > 0'.format(a, b))
-
-        self._a = a
-        self._b = b
-
-    def frac(self):
-        ''' Return tuple (a,b) representing progress of a/b'''
-        return (self._a, self._b)
-
-    def percent(self):
-        ''' Returns a percentage progress as a float between 0 and 100.
-
-        subclassing note: implemented in terms of frac(), assumes sanity
-        checking is done at construction time.
-        '''
-        (a, b) = self.frac()
-        return 100.0 * a / b
-
-    def started(self):
-        ''' Returns True if fractional progress is greater than 0.
-
-        subclassing note: implemented in terms of frac(), assumes sanity
-        checking is done at construction time.
-        '''
-        return self.frac()[0] > 0
-
-    def inprogress(self):
-        ''' Returns True if fractional progress is strictly between 0 and 1.
-
-        subclassing note: implemented in terms of frac(), assumes sanity
-        checking is done at construction time.
-        '''
-        (a, b) = self.frac()
-        return a > 0 and a < b
-
-    def done(self):
-        ''' Return True if this represents done.
-
-        subclassing note: implemented in terms of frac(), assumes sanity
-        checking is done at construction time.
-        '''
-        (a, b) = self.frac()
-        return a == b
-
-    def ternary_str(self):
-        ''' Return a string version of this progress: either
-           "none", "in_progress", or "done".
-
-        subclassing note: implemented in terms of frac()
-        '''
-        (a, b) = self.frac()
-        if a == 0:
-            return "none"
-        if a < b:
-            return "in_progress"
-        return "done"
-
-    def __eq__(self, other):
-        ''' Two Progress objects are equal if they have identical values.
-        Implemented in terms of frac()'''
-        if not isinstance(other, Progress):
-            return False
-        (a, b) = self.frac()
-        (a2, b2) = other.frac()
-        return a == a2 and b == b2
-
-    def __ne__(self, other):
-        ''' The opposite of equal'''
-        return not self.__eq__(other)
-
-    def __str__(self):
-        ''' Return a string representation of this string.
-
-        subclassing note: implemented in terms of frac().
-        '''
-        (a, b) = self.frac()
-        return "{0}/{1}".format(a, b)
-
-    @staticmethod
-    def add_counts(a, b):
-        '''Add two progress indicators, assuming that each represents items done:
-        (a / b) + (c / d) = (a + c) / (b + d).
-        If either is None, returns the other.
-        '''
-        if a is None:
-            return b
-        if b is None:
-            return a
-        # get numerators + denominators
-        (n, d) = a.frac()
-        (n2, d2) = b.frac()
-        return Progress(n + n2, d + d2)
-
-    @staticmethod
-    def to_js_status_str(progress):
-        '''
-        Return the "status string" version of the passed Progress
-        object that should be passed to js.  Use this function when
-        sending Progress objects to js to limit dependencies.
-        '''
-        if progress is None:
-            return "NA"
-        return progress.ternary_str()
-
-    @staticmethod
-    def to_js_detail_str(progress):
-        '''
-        Return the "detail string" version of the passed Progress
-        object that should be passed to js.  Use this function when
-        passing Progress objects to js to limit dependencies.
-        '''
-        if progress is None:
-            return "NA"
-        return str(progress)

common/lib/xmodule/xmodule/editing_module.py

@@ -20,5 +20,10 @@ class EditingDescriptor(MakoModuleDescriptor):
     def get_context(self):
         return {
             'module': self,
-            'data': self.definition['data'],
+            'data': self.definition.get('data', ''),
+    # TODO (vshnayder): allow children and metadata to be edited.
+    #'children' : self.definition.get('children, ''),
+
+    # TODO: show both own metadata and inherited?
+    #'metadata' : self.own_metadata,
         }

common/lib/xmodule/xmodule/error_module.py

+import sys
+import logging
+
 from pkg_resources import resource_string
 from lxml import etree
 from xmodule.x_module import XModule
 from xmodule.mako_module import MakoModuleDescriptor
 from xmodule.xml_module import XmlDescriptor
 from xmodule.editing_module import EditingDescriptor
+from xmodule.errortracker import exc_info_to_str
 
-import logging
 
 log = logging.getLogger(__name__)
 
@@ -14,14 +17,11 @@ class ErrorModule(XModule):
         '''Show an error.
         TODO (vshnayder): proper style, divs, etc.
         '''
-        if not self.system.is_staff:
-            return self.system.render_template('module-error.html', {})
-
         # staff get to see all the details
-        return self.system.render_template('module-error-staff.html', {
-            'data' : self.definition['data'],
-            # TODO (vshnayder): need to get non-syntax errors in here somehow
-            'error' : self.definition.get('error', 'Error not available')
+        return self.system.render_template('module-error.html', {
+            'data' : self.definition['data']['contents'],
+            'error' : self.definition['data']['error_msg'],
+            'is_staff' : self.system.is_staff,
             })
 
 class ErrorDescriptor(EditingDescriptor):
@@ -31,29 +31,36 @@ class ErrorDescriptor(EditingDescriptor):
     module_class = ErrorModule
 
     @classmethod
-    def from_xml(cls, xml_data, system, org=None, course=None, err=None):
+    def from_xml(cls, xml_data, system, org=None, course=None,
+                 error_msg='Error not available'):
         '''Create an instance of this descriptor from the supplied data.
 
         Does not try to parse the data--just stores it.
 
         Takes an extra, optional, parameter--the error that caused an
-        issue.
+        issue.  (should be a string, or convert usefully into one).
         '''
-
-        definition = {}
-        if err is not None:
-            definition['error'] = err
+        # Use a nested inner dictionary because 'data' is hardcoded
+        inner = {}
+        definition = {'data': inner}
+        inner['error_msg'] = str(error_msg)
 
         try:
             # If this is already an error tag, don't want to re-wrap it.
             xml_obj = etree.fromstring(xml_data)
             if xml_obj.tag == 'error':
                 xml_data = xml_obj.text
-        except etree.XMLSyntaxError as err:
+                error_node = xml_obj.find('error_msg')
+                if error_node is not None:
+                    inner['error_msg'] = error_node.text
+                else:
+                    inner['error_msg'] = 'Error not available'
+
+        except etree.XMLSyntaxError:
             # Save the error to display later--overrides other problems
-            definition['error'] = err
+            inner['error_msg'] = exc_info_to_str(sys.exc_info())
 
-        definition['data'] = xml_data
+        inner['contents'] = xml_data
         # TODO (vshnayder): Do we need a unique slug here?  Just pick a random
         # 64-bit num?
         location = ['i4x', org, course, 'error', 'slug']
@@ -71,10 +78,12 @@ class ErrorDescriptor(EditingDescriptor):
         files, etc.  That would just get re-wrapped on import.
         '''
         try:
-           xml = etree.fromstring(self.definition['data'])
+           xml = etree.fromstring(self.definition['data']['contents'])
            return etree.tostring(xml)
         except etree.XMLSyntaxError:
             # still not valid.
             root = etree.Element('error')
-            root.text = self.definition['data']
+            root.text = self.definition['data']['contents']
+            err_node = etree.SubElement(root, 'error_msg')
+            err_node.text = self.definition['data']['error_msg']
             return etree.tostring(root)

common/lib/xmodule/xmodule/errortracker.py

@@ -8,6 +8,12 @@ log = logging.getLogger(__name__)
 
 ErrorLog = namedtuple('ErrorLog', 'tracker errors')
 
+def exc_info_to_str(exc_info):
+    """Given some exception info, convert it into a string using
+    the traceback.format_exception() function.
+    """
+    return ''.join(traceback.format_exception(*exc_info))
+
 def in_exception_handler():
     '''Is there an active exception?'''
     return sys.exc_info() != (None, None, None)
@@ -27,7 +33,7 @@ def make_error_tracker():
         '''Log errors'''
         exc_str = ''
         if in_exception_handler():
-            exc_str = ''.join(traceback.format_exception(*sys.exc_info()))
+            exc_str = exc_info_to_str(sys.exc_info())
 
         errors.append((msg, exc_str))
 

common/lib/xmodule/xmodule/html_module.py

@@ -5,11 +5,11 @@ import os
 import sys
 from lxml import etree
 
-from xmodule.x_module import XModule
-from xmodule.xml_module import XmlDescriptor
-from xmodule.editing_module import EditingDescriptor
-from stringify import stringify_children
-from html_checker import check_html
+from .x_module import XModule
+from .xml_module import XmlDescriptor
+from .editing_module import EditingDescriptor
+from .stringify import stringify_children
+from .html_checker import check_html
 
 log = logging.getLogger("mitx.courseware")
 

common/lib/xmodule/xmodule/modulestore/__init__.py

@@ -3,10 +3,13 @@ This module provides an abstraction for working with XModuleDescriptors
 that are stored in a database an accessible using their Location as an identifier
 """
 
+import logging
 import re
+
 from collections import namedtuple
+
 from .exceptions import InvalidLocationError, InsufficientSpecificationError
-import logging
+from xmodule.errortracker import ErrorLog, make_error_tracker
 
 log = logging.getLogger('mitx.' + 'modulestore')
 
@@ -290,3 +293,38 @@ class ModuleStore(object):
         '''
         raise NotImplementedError
 
+
+class ModuleStoreBase(ModuleStore):
+    '''
+    Implement interface functionality that can be shared.
+    '''
+    def __init__(self):
+        '''
+        Set up the error-tracking logic.
+        '''
+        self._location_errors = {}    # location -> ErrorLog
+
+    def _get_errorlog(self, location):
+        """
+        If we already have an errorlog for this location, return it.  Otherwise,
+        create one.
+        """
+        location = Location(location)
+        if location not in self._location_errors:
+            self._location_errors[location] = make_error_tracker()
+        return self._location_errors[location]
+
+    def get_item_errors(self, location):
+        """
+        Return list of errors for this location, if any.  Raise the same
+        errors as get_item if location isn't present.
+
+        NOTE: For now, the only items that track errors are CourseDescriptors in
+        the xml datastore.  This will return an empty list for all other items
+        and datastores.
+        """
+        # check that item is present and raise the promised exceptions if needed
+        self.get_item(location)
+
+        errorlog = self._get_errorlog(location)
+        return errorlog.errors

common/lib/xmodule/xmodule/modulestore/mongo.py

@@ -11,7 +11,7 @@ from xmodule.x_module import XModuleDescriptor
 from xmodule.mako_module import MakoDescriptorSystem
 from mitxmako.shortcuts import render_to_string
 
-from . import ModuleStore, Location
+from . import ModuleStoreBase, Location
 from .exceptions import (ItemNotFoundError,
                          NoPathToItem, DuplicateItemError)
 
@@ -38,7 +38,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
 
         resources_fs: a filesystem, as per MakoDescriptorSystem
 
-        error_tracker:
+        error_tracker: a function that logs errors for later display to users
 
         render_template: a function for rendering templates, as per
             MakoDescriptorSystem
@@ -73,7 +73,7 @@ def location_to_query(location):
     return query
 
 
-class MongoModuleStore(ModuleStore):
+class MongoModuleStore(ModuleStoreBase):
     """
     A Mongodb backed ModuleStore
     """
@@ -81,6 +81,9 @@ class MongoModuleStore(ModuleStore):
     # TODO (cpennington): Enable non-filesystem filestores
     def __init__(self, host, db, collection, fs_root, port=27017, default_class=None,
                  error_tracker=null_error_tracker):
+
+        ModuleStoreBase.__init__(self)
+
         self.collection = pymongo.connection.Connection(
             host=host,
             port=port

common/lib/xmodule/xmodule/modulestore/xml.py

@@ -12,7 +12,7 @@ from xmodule.course_module import CourseDescriptor
 from xmodule.mako_module import MakoDescriptorSystem
 from cStringIO import StringIO
 
-from . import ModuleStore, Location
+from . import ModuleStoreBase, Location
 from .exceptions import ItemNotFoundError
 
 etree.set_default_parser(
@@ -98,7 +98,7 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
                                   error_tracker, process_xml, **kwargs)
 
 
-class XMLModuleStore(ModuleStore):
+class XMLModuleStore(ModuleStoreBase):
     """
     An XML backed ModuleStore
     """
@@ -118,13 +118,12 @@ class XMLModuleStore(ModuleStore):
         course_dirs: If specified, the list of course_dirs to load. Otherwise,
             load all course dirs
         """
+        ModuleStoreBase.__init__(self)
 
         self.eager = eager
         self.data_dir = path(data_dir)
         self.modules = {}  # location -> XModuleDescriptor
         self.courses = {}  # course_dir -> XModuleDescriptor for the course
-        self.location_errors = {}    # location -> ErrorLog
-
 
         if default_class is None:
             self.default_class = None
@@ -148,12 +147,14 @@ class XMLModuleStore(ModuleStore):
 
         for course_dir in course_dirs:
             try:
-                # make a tracker, then stick in the right place once the course loads
-                # and we know its location
+                # Special-case code here, since we don't have a location for the
+                # course before it loads.
+                # So, make a tracker to track load-time errors, then put in the right
+                # place after the course loads and we have its location
                 errorlog = make_error_tracker()
                 course_descriptor = self.load_course(course_dir, errorlog.tracker)
                 self.courses[course_dir] = course_descriptor
-                self.location_errors[course_descriptor.location] = errorlog
+                self._location_errors[course_descriptor.location] = errorlog
             except:
                 msg = "Failed to load course '%s'" % course_dir
                 log.exception(msg)
@@ -221,23 +222,6 @@ class XMLModuleStore(ModuleStore):
             raise ItemNotFoundError(location)
 
 
-    def get_item_errors(self, location):
-        """
-        Return list of errors for this location, if any.  Raise the same
-        errors as get_item if location isn't present.
-
-        NOTE: This only actually works for courses in the xml datastore--
-        will return an empty list for all other modules.
-        """
-        location = Location(location)
-        # check that item is present
-        self.get_item(location)
-
-        # now look up errors
-        if location in self.location_errors:
-            return self.location_errors[location].errors
-        return []
-
     def get_courses(self, depth=0):
         """
         Returns a list of course descriptors.  If there were errors on loading,
@@ -245,9 +229,11 @@ class XMLModuleStore(ModuleStore):
         """
         return self.courses.values()
 
+
     def create_item(self, location):
         raise NotImplementedError("XMLModuleStores are read-only")
 
+
     def update_item(self, location, data):
         """
         Set the data in the item specified by the location to
@@ -258,6 +244,7 @@ class XMLModuleStore(ModuleStore):
         """
         raise NotImplementedError("XMLModuleStores are read-only")
 
+
     def update_children(self, location, children):
         """
         Set the children for the item specified by the location to
@@ -268,6 +255,7 @@ class XMLModuleStore(ModuleStore):
         """
         raise NotImplementedError("XMLModuleStores are read-only")
 
+
     def update_metadata(self, location, metadata):
         """
         Set the metadata for the item specified by the location to

common/lib/xmodule/tests/test_import.py → common/lib/xmodule/xmodule/tests/test_import.py

@@ -73,6 +73,7 @@ class ImportTestCase(unittest.TestCase):
     def test_reimport(self):
         '''Make sure an already-exported error xml tag loads properly'''
 
+        self.maxDiff = None
         bad_xml = '''<sequential display_name="oops"><video url="hi"></sequential>'''
         system = self.get_system()
         descriptor = XModuleDescriptor.load_from_xml(bad_xml, system, 'org', 'course',

common/lib/xmodule/tests/test_stringify.py → common/lib/xmodule/xmodule/tests/test_stringify.py

 from nose.tools import assert_equals
 from lxml import etree
-from stringify import stringify_children
+from xmodule.stringify import stringify_children
 
 def test_stringify():
-    html = '''<html a="b" foo="bar">Hi <div x="foo">there <span>Bruce</span><b>!</b></div></html>'''
+    text = 'Hi <div x="foo">there <span>Bruce</span><b>!</b></div>'
+    html = '''<html a="b" foo="bar">{0}</html>'''.format(text)
     xml = etree.fromstring(html)
     out = stringify_children(xml)
-    assert_equals(out, '''Hi <div x="foo">there <span>Bruce</span><b>!</b></div>''')
+    assert_equals(out, text)

common/lib/xmodule/xmodule/x_module.py

-from lxml import etree
-from lxml.etree import XMLSyntaxError
-import pkg_resources
 import logging
+import pkg_resources
+import sys
+
 from fs.errors import ResourceNotFoundError
 from functools import partial
+from lxml import etree
+from lxml.etree import XMLSyntaxError
 
 from xmodule.modulestore import Location
-
+from xmodule.errortracker import exc_info_to_str
 
 log = logging.getLogger('mitx.' + __name__)
 
@@ -471,7 +473,9 @@ class XModuleDescriptor(Plugin, HTMLSnippet):
             msg = "Error loading from xml."
             log.exception(msg)
             system.error_tracker(msg)
-            descriptor = ErrorDescriptor.from_xml(xml_data, system, org, course, err)
+            err_msg = msg + "\n" + exc_info_to_str(sys.exc_info())
+            descriptor = ErrorDescriptor.from_xml(xml_data, system, org, course,
+                                                  err_msg)
 
         return descriptor
 

lms/djangoapps/courseware/views.py

@@ -257,6 +257,7 @@ def index(request, course_id, chapter=None, section=None,
 
     return result
 
+
 @ensure_csrf_cookie
 def jump_to(request, location):
     '''
@@ -283,7 +284,7 @@ def jump_to(request, location):
     except NoPathToItem:
         raise Http404("This location is not in any class: {0}".format(location))
 
-
+    # Rely on index to do all error handling
     return index(request, course_id, chapter, section, position)
 
 @ensure_csrf_cookie

lms/djangoapps/django_comment_client/base/views.py

@@ -15,6 +15,7 @@ from django.core.files.storage import get_storage_class
 from django.utils.translation import ugettext as _
 from django.conf import settings
 
+from mitxmako.shortcuts import render_to_response, render_to_string
 from django_comment_client.utils import JsonResponse, JsonError, extract
 
 def thread_author_only(fn):
@@ -58,6 +59,16 @@ def create_thread(request, course_id, commentable_id):
     if request.POST.get('autowatch', 'false').lower() == 'true':
         attributes['auto_subscribe'] = True
     response = comment_client.create_thread(commentable_id, attributes)
+    if request.is_ajax():
+        context = {
+            'course_id': course_id,
+            'thread': response,
+        }
+        html = render_to_string('discussion/ajax_thread_only.html', context)
+        return JsonResponse({
+            'html': html,
+        })
+    else:
         return JsonResponse(response)
 
 @thread_author_only
@@ -68,9 +79,7 @@ def update_thread(request, course_id, thread_id):
     response = comment_client.update_thread(thread_id, attributes)
     return JsonResponse(response)
 
-@login_required
-@require_POST
-def create_comment(request, course_id, thread_id):
+def _create_comment(request, course_id, _response_from_attributes):
     attributes = extract(request.POST, ['body'])
     attributes['user_id'] = request.user.id
     attributes['course_id'] = course_id
@@ -78,9 +87,25 @@ def create_comment(request, course_id, thread_id):
         attributes['anonymous'] = True
     if request.POST.get('autowatch', 'false').lower() == 'true':
         attributes['auto_subscribe'] = True
-    response = comment_client.create_comment(thread_id, attributes)
+    response = _response_from_attributes(attributes)
+    if request.is_ajax():
+        context = {
+            'comment': response,
+        }
+        html = render_to_string('discussion/ajax_comment_only.html', context)
+        return JsonResponse({
+            'html': html,
+        })
+    else:
         return JsonResponse(response)
 
+@login_required
+@require_POST
+def create_comment(request, course_id, thread_id):
+    def _response_from_attributes(attributes):
+        return comment_client.create_comment(thread_id, attributes)
+    return _create_comment(request, course_id, _response_from_attributes)
+
 @thread_author_only
 @login_required
 @require_POST
@@ -107,15 +132,9 @@ def endorse_comment(request, course_id, comment_id):
 @login_required
 @require_POST
 def create_sub_comment(request, course_id, comment_id):
-    attributes = extract(request.POST, ['body'])
-    attributes['user_id'] = request.user.id
-    attributes['course_id'] = course_id
-    if request.POST.get('anonymous', 'false').lower() == 'true':
-        attributes['anonymous'] = True
-    if request.POST.get('autowatch', 'false').lower() == 'true':
-        attributes['auto_subscribe'] = True
-    response = comment_client.create_sub_comment(comment_id, attributes)
-    return JsonResponse(response)
+    def _response_from_attributes(attributes):
+        return comment_client.create_sub_comment(comment_id, attributes)
+    return _create_comment(request, course_id, _response_from_attributes)
 
 @comment_author_only
 @login_required

lms/djangoapps/django_comment_client/forum/views.py

@@ -11,9 +11,7 @@ from courseware.courses import check_course
 from dateutil.tz import tzlocal
 from datehelper import time_ago_in_words
 
-from django_comment_client.utils import get_categorized_discussion_info, \
-                                        extract, strip_none, \
-                                        JsonResponse
+import django_comment_client.utils as utils
 from urllib import urlencode
 
 import json
@@ -24,13 +22,9 @@ import dateutil
 THREADS_PER_PAGE = 20
 PAGES_NEARBY_DELTA = 2
 
-class HtmlResponse(HttpResponse):
-    def __init__(self, html=''):
-        super(HtmlResponse, self).__init__(html, content_type='text/plain')
-
 def render_accordion(request, course, discussion_id):
 
-    discussion_info = get_categorized_discussion_info(request, course)
+    discussion_info = utils.get_categorized_discussion_info(request, course)
 
     context = {
         'course': course,
@@ -63,7 +57,7 @@ def render_discussion(request, course_id, threads, discussion_id=None, \
         'pages_nearby_delta': PAGES_NEARBY_DELTA,
         'discussion_type': discussion_type,
         'base_url': base_url,
-        'query_params': strip_none(extract(query_params, ['page', 'sort_key', 'sort_order', 'tags', 'text'])),
+        'query_params': utils.strip_none(utils.extract(query_params, ['page', 'sort_key', 'sort_order', 'tags', 'text'])),
     }
     context = dict(context.items() + query_params.items())
     return render_to_string(template, context)
@@ -86,9 +80,9 @@ def get_threads(request, course_id, discussion_id):
 
     if query_params['text'] or query_params['tags']: #TODO do tags search without sunspot
         query_params['commentable_id'] = discussion_id
-        threads, page, num_pages = comment_client.search_threads(course_id, recursive=False, query_params=strip_none(query_params))
+        threads, page, num_pages = comment_client.search_threads(course_id, recursive=False, query_params=utils.strip_none(query_params))
     else:
-        threads, page, num_pages = comment_client.get_threads(discussion_id, recursive=False, query_params=strip_none(query_params))
+        threads, page, num_pages = comment_client.get_threads(discussion_id, recursive=False, query_params=utils.strip_none(query_params))
 
     query_params['page'] = page
     query_params['num_pages'] = num_pages
@@ -162,7 +156,7 @@ def single_thread(request, course_id, discussion_id, thread_id):
         context = {'thread': thread}
         html = render_to_string('discussion/_ajax_single_thread.html', context)
 
-        return JsonResponse({
+        return utils.JsonResponse({
             'html': html,
             'annotated_content_info': annotated_content_info,
         })

lms/djangoapps/django_comment_client/utils.py

@@ -116,3 +116,7 @@ class JsonError(HttpResponse):
                                    ensure_ascii=False)
         super(JsonError, self).__init__(content,
                                         mimetype='application/json; charset=utf8')
+
+class HtmlResponse(HttpResponse):
+    def __init__(self, html=''):
+        super(HtmlResponse, self).__init__(html, content_type='text/plain')

lms/static/coffee/src/customwmd.coffee

@@ -174,3 +174,4 @@ $ ->
           text: text
           previewSetter: previewSet
       editor.run()
+      editor

lms/static/coffee/src/discussion/content.coffee

@@ -18,12 +18,13 @@ Discussion = @Discussion
       if $replyView.length
         $replyView.show()
       else
+        thread_id = $discussionContent.parents(".thread").attr("_id")
         view = {
           id: id
-          showWatchCheckbox: $discussionContent.parents(".thread").attr("_id") not in $$user_info.subscribed_thread_ids
+          showWatchCheckbox: not Discussion.isSubscribed(thread_id, "thread")
         }
         $discussionContent.append Mustache.render Discussion.replyTemplate, view
-        Markdown.makeWmdEditor $local(".reply-body"), "-reply-body-#{id}", Discussion.urlFor('upload')
+        Discussion.makeWmdEditor $content, $local, "reply-body"
         $local(".discussion-submit-post").click -> handleSubmitReply(this)
         $local(".discussion-cancel-post").click -> handleCancelReply(this)
       $local(".discussion-link").hide()
@@ -33,8 +34,8 @@ Discussion = @Discussion
       $replyView = $local(".discussion-reply-new")
       if $replyView.length
         $replyView.hide()
-      reply = Discussion.generateDiscussionLink("discussion-reply", "Reply", handleReply)
-      $(elem).replaceWith(reply)
+      #reply = Discussion.generateDiscussionLink("discussion-reply", "Reply", handleReply)
+      #$(elem).replaceWith(reply)
       $discussionContent.attr("status", "normal")
 
     handleSubmitReply = (elem) ->
@@ -45,26 +46,30 @@ Discussion = @Discussion
       else
         return
 
-      body = $local("#wmd-input-reply-body-#{id}").val()
+      body = Discussion.getWmdContent $content, $local, "reply-body"
 
       anonymous = false || $local(".discussion-post-anonymously").is(":checked")
       autowatch = false || $local(".discussion-auto-watch").is(":checked")
 
       Discussion.safeAjax
+        $elem: $(elem)
         url: url
         type: "POST"
+        dataType: 'json'
         data:
           body: body
           anonymous: anonymous
           autowatch: autowatch
-        success: (response, textStatus) ->
-          if response.errors? and response.errors.length > 0
-            errorsField = $local(".discussion-errors").empty()
-            for error in response.errors
-              errorsField.append($("<li>").addClass("new-post-form-error").html(error))
-          else
-            Discussion.handleAnchorAndReload(response)
-        dataType: 'json'
+        success: Discussion.formErrorHandler($local(".discussion-errors"), (response, textStatus) ->
+          console.log response
+          $comment = $(response.html)
+          $content.children(".comments").prepend($comment)
+          Discussion.setWmdContent $content, $local, "reply-body", ""
+          Discussion.initializeContent($comment)
+          Discussion.bindContentEvents($comment)
+          $local(".discussion-reply-new").hide()
+          $discussionContent.attr("status", "normal")
+        )
 
     handleVote = (elem, value) ->
       contentType = if $content.hasClass("thread") then "thread" else "comment"
@@ -91,7 +96,7 @@ Discussion = @Discussion
           tags: $local(".thread-raw-tags").html()
         }
         $discussionContent.append Mustache.render Discussion.editThreadTemplate, view
-        Markdown.makeWmdEditor $local(".thread-body-edit"), "-thread-body-edit-#{id}", Discussion.urlFor('update_thread', id)
+        Discussion.makeWmdEditor $content, $local, "thread-body-edit"
         $local(".thread-tags-edit").tagsInput
           autocomplete_url: Discussion.urlFor('tags_autocomplete')
           autocomplete:
@@ -107,16 +112,16 @@ Discussion = @Discussion
     handleSubmitEditThread = (elem) ->
       url = Discussion.urlFor('update_thread', id)
       title = $local(".thread-title-edit").val()
-      body = $local("#wmd-input-thread-body-edit-#{id}").val()
+      body = Discussion.getWmdContent $content, $local, "thread-body-edit"
       tags = $local(".thread-tags-edit").val()
-      $.post url, {title: title, body: body, tags: tags}, (response, textStatus) ->
-        if response.errors
-          errorsField = $local(".discussion-update-errors").empty()
-          for error in response.errors
-            errorsField.append($("<li>").addClass("new-post-form-error").html(error))
-        else
+      $.ajax
+        url: url
+        type: "POST"
+        dataType: 'json'
+        data: {title: title, body: body, tags: tags},
+        success: Discussion.formErrorHandler($local(".discussion-update-errors"), (response, textStatus) ->
           Discussion.handleAnchorAndReload(response)
-      , 'json'
+        )
 
     handleEditComment = (elem) ->
       $local(".discussion-content-wrapper").hide()
@@ -124,26 +129,23 @@ Discussion = @Discussion
       if $editView.length
         $editView.show()
       else
-        view = {
-          id: id
-          body: $local(".comment-raw-body").html()
-        }
+        view = { id: id, body: $local(".comment-raw-body").html() }
         $discussionContent.append Mustache.render Discussion.editCommentTemplate, view
-        Markdown.makeWmdEditor $local(".comment-body-edit"), "-comment-body-edit-#{id}", Discussion.urlFor('update_comment', id)
+        Discussion.makeWmdEditor $content, $local, "comment-body-edit"
         $local(".discussion-submit-update").unbind("click").click -> handleSubmitEditComment(this)
         $local(".discussion-cancel-update").unbind("click").click -> handleCancelEdit(this)
 
     handleSubmitEditComment= (elem) ->
       url = Discussion.urlFor('update_comment', id)
-      body = $local("#wmd-input-comment-body-edit-#{id}").val()
-      $.post url, {body: body}, (response, textStatus) ->
-        if response.errors
-          errorsField = $local(".discussion-update-errors").empty()
-          for error in response.errors
-            errorsField.append($("<li>").addClass("new-post-form-error").html(error))
-        else
+      body = Discussion.getWmdContent $content, $local, "comment-body-edit"
+      $.ajax
+        url: url
+        type: "POST"
+        dataType: "json"
+        data: {body: body}
+        success: Discussion.formErrorHandler($local(".discussion-update-errors"), (response, textStatus) ->
           Discussion.handleAnchorAndReload(response)
-      , 'json'
+        )
 
     handleEndorse = (elem) ->
       url = Discussion.urlFor('endorse_comment', id)
@@ -166,6 +168,9 @@ Discussion = @Discussion
       $threadTitle = $local(".thread-title")
       $showComments = $local(".discussion-show-comments")
 
+      if not $showComments.length or not $threadTitle.length
+        return
+
       rebindHideEvents = ->
         $threadTitle.unbind('click').click handleHideSingleThread
         $showComments.unbind('click').click handleHideSingleThread
@@ -182,6 +187,7 @@ Discussion = @Discussion
           $elem: $.merge($threadTitle, $showComments)
           url: url
           type: "GET"
+          dataType: 'json'
           success: (response, textStatus) ->
             if not $$annotated_content_info?
               window.$$annotated_content_info = {}
@@ -191,33 +197,35 @@ Discussion = @Discussion
               Discussion.initializeContent(comment)
               Discussion.bindContentEvents(comment)
             rebindHideEvents()
-          dataType: 'json'
       
+    Discussion.bindLocalEvents $local,
 
-    $local(".thread-title").click handleShowSingleThread
+      "click .thread-title": ->
+        handleShowSingleThread(this)
 
-    $local(".discussion-show-comments").click handleShowSingleThread
+      "click .discussion-show-comments": ->
+        handleShowSingleThread(this)
 
-    $local(".discussion-reply-thread").click ->
+      "click .discussion-reply-thread": ->
         handleShowSingleThread($local(".thread-title"))
         handleReply(this)
 
-    $local(".discussion-reply-comment").click ->
+      "click .discussion-reply-comment": ->
         handleReply(this)
 
-    $local(".discussion-cancel-reply").click ->
+      "click .discussion-cancel-reply": ->
         handleCancelReply(this)
 
-    $local(".discussion-vote-up").click ->
+      "click .discussion-vote-up": ->
         handleVote(this, "up")
 
-    $local(".discussion-vote-down").click ->
+      "click .discussion-vote-down": ->
         handleVote(this, "down")
 
-    $local(".discussion-endorse").click ->
+      "click .discussion-endorse": ->
         handleEndorse(this)
 
-    $local(".discussion-edit").click ->
+      "click .discussion-edit": ->
         if $content.hasClass("thread")
           handleEditThread(this)
         else

lms/static/coffee/src/discussion/discussion.coffee

@@ -91,17 +91,28 @@ initializeFollowThread = (index, thread) ->
 
     handleSubmitNewPost = (elem) ->
       title = $local(".new-post-title").val()
-      body = $local("#wmd-input-new-post-body-#{id}").val()
+      body = Discussion.getWmdContent $discussion, $local, "new-post-body"
       tags = $local(".new-post-tags").val()
       url = Discussion.urlFor('create_thread', $local(".new-post-form").attr("_id"))
-      $.post url, {title: title, body: body, tags: tags}, (response, textStatus) ->
-        if response.errors
-          errorsField = $local(".discussion-errors").empty()
-          for error in response.errors
-            errorsField.append($("<li>").addClass("new-post-form-error").html(error))
-        else
-          Discussion.handleAnchorAndReload(response)
-      , 'json'
+      Discussion.safeAjax
+        $elem: $(elem)
+        url: url
+        type: "POST"
+        dataType: 'json'
+        data:
+          title: title
+          body: body
+          tags: tags
+        success: Discussion.formErrorHandler($local(".new-post-form-error"), (response, textStatus) ->
+          console.log response
+          $thread = $(response.html)
+          $discussion.children(".threads").prepend($thread)
+          Discussion.setWmdContent $discussion, $local, "new-post-body", ""
+          Discussion.initializeContent($thread)
+          Discussion.bindContentEvents($thread)
+          $(".new-post-form").hide()
+          $local(".discussion-new-post").show()
+        )
 
     handleCancelNewPost = (elem) ->
       $local(".new-post-form").hide()
@@ -115,9 +126,9 @@ initializeFollowThread = (index, thread) ->
       else
         view = { discussion_id: id }
         $discussionNonContent.append Mustache.render Discussion.newPostTemplate, view
-        newPostBody = $(discussion).find(".new-post-body")
+        newPostBody = $discussion.find(".new-post-body")
         if newPostBody.length
-          Markdown.makeWmdEditor newPostBody, "-new-post-body-#{$(discussion).attr('_id')}", Discussion.urlFor('upload')
+          Discussion.makeWmdEditor $discussion, $local, "new-post-body"
 
         $local(".new-post-tags").tagsInput Discussion.tagsInputOptions()
 
@@ -128,8 +139,6 @@ initializeFollowThread = (index, thread) ->
 
         $(elem).hide()
 
-    handleUpdateDiscussionContent = ($elem, $discussion, url) ->
-
     handleAjaxSearch = (elem) ->
       handle
       $elem = $(elem)

lms/static/coffee/src/discussion/templates.coffee

@@ -3,7 +3,9 @@ if not @Discussion?
 
 Discussion = @Discussion
 
+
 @Discussion = $.extend @Discussion,
+
   newPostTemplate: """
     <form class="new-post-form" _id="{{discussion_id}}">
       <ul class="discussion-errors"></ul>    

lms/static/coffee/src/discussion/utils.coffee

@@ -3,6 +3,8 @@ if not @Discussion?
 
 Discussion = @Discussion
 
+wmdEditors = {}
+
 @Discussion = $.extend @Discussion,
 
   generateLocal: (elem) ->
@@ -65,3 +67,45 @@ Discussion = @Discussion
     height: "30px"
     width: "100%"
     removeWithBackspace: true
+
+  isSubscribed: (id, type) ->
+    if type == "thread"
+      id in $$user_info.subscribed_thread_ids
+    else if type == "commentable" or type == "discussion"
+      id in $$user_info.subscribed_commentable_ids
+    else
+      id in $$user_info.subscribed_user_ids
+
+  formErrorHandler: (errorsField, success) ->
+    (response, textStatus, xhr) ->
+      if response.errors? and response.errors.length > 0
+        errorsField.empty()
+        for error in response.errors
+          errorsField.append($("<li>").addClass("new-post-form-error").html(error))
+      else
+        success(response, textStatus, xhr)
+
+  makeWmdEditor: ($content, $local, cls_identifier) ->
+    elem = $local(".#{cls_identifier}")
+    id = $content.attr("_id")
+    appended_id = "-#{cls_identifier}-#{id}"
+    imageUploadUrl = Discussion.urlFor('upload')
+    editor = Markdown.makeWmdEditor elem, appended_id, imageUploadUrl
+    wmdEditors["#{cls_identifier}-#{id}"] = editor
+    console.log wmdEditors
+    editor
+
+  getWmdEditor: ($content, $local, cls_identifier) ->
+    id = $content.attr("_id")
+    wmdEditors["#{cls_identifier}-#{id}"]
+
+  getWmdContent: ($content, $local, cls_identifier) ->
+    id = $content.attr("_id")
+    $local("#wmd-input-#{cls_identifier}-#{id}").val()
+
+  setWmdContent: ($content, $local, cls_identifier, text) ->
+    id = $content.attr("_id")
+    $local("#wmd-input-#{cls_identifier}-#{id}").val(text)
+    console.log wmdEditors
+    console.log "#{cls_identifier}-#{id}"
+    wmdEditors["#{cls_identifier}-#{id}"].refreshPreview()

lms/templates/discussion/_forum.html

@@ -15,9 +15,11 @@
     <div class="discussion-new-post control-button" href="javascript:void(0)">New Post</div>
   </div>
   <%include file="_sort.html" />
+  <div class="threads">
     % for thread in threads:
       ${renderer.render_thread(course_id, thread, edit_thread=False, show_comments=False)}
     % endfor
+  </div>
   <%include file="_paginator.html" />
 </section>
 

lms/templates/discussion/_inline.html

@@ -8,9 +8,11 @@
     <div class="discussion-new-post control-button" href="javascript:void(0)">New Post</div>
     <%include file="_sort.html" />
   </div>
+  <div class="threads">
     % for thread in threads:
       ${renderer.render_thread(course_id, thread, edit_thread=False, show_comments=False)}
     % endfor
+  </div>
   <%include file="_paginator.html" />
 </section>
 

lms/templates/discussion/_thread.html

@@ -7,14 +7,12 @@
   <div class="thread" _id="${thread['id']}">
     ${render_content(thread, "thread", edit_thread=edit_thread, show_comments=show_comments)}
     % if show_comments:
-      ${render_comments(thread['children'])}
+      ${render_comments(thread.get('children', []))}
     % endif
   </div>
 </%def>
 
-<%def name="render_comments(comments)">
-  <div class="comments">
-    % for comment in comments:
+<%def name="render_comment(comment)">
   % if comment['endorsed']:
   <div class="comment endorsed" _id="${comment['id']}">
   % else:
@@ -22,9 +20,15 @@
   % endif
     ${render_content(comment, "comment")}
     <div class="comments">
-          ${render_comments(comment['children'])}
+      ${render_comments(comment.get('children', []))}
     </div>
   </div>
+</%def>
+
+<%def name="render_comments(comments)">
+  <div class="comments">
+    % for comment in comments:
+      ${render_comment(comment)}
     % endfor
   </div>
 </%def>

lms/templates/discussion/ajax_comment_only.html

+<%namespace name="renderer" file="_thread.html"/>
+
+${renderer.render_comment(comment)}

lms/templates/discussion/ajax_thread_only.html

+<%namespace name="renderer" file="_thread.html"/>
+
+${renderer.render_thread(course_id, thread, edit_thread=True, show_comments=False)}

lms/templates/module-error-staff.html

-<section class="outside-app">
-  <h1>There has been an error on the <em>MITx</em> servers</h1>
-  <p>We're sorry, this module is temporarily unavailable. Our staff is working to fix it as soon as possible. Please email us at <a href="mailto:technical@mitx.mit.edu">technical@mitx.mit.edu</a> to report any problems or downtime.</p>
-
-<h1>Staff-only details below:</h1>
-
-<p>Error: ${error}</p>
-
-<p>Raw data: ${data}</p>
-
-</section>

lms/templates/module-error.html

 <section class="outside-app">
   <h1>There has been an error on the <em>MITx</em> servers</h1>
   <p>We're sorry, this module is temporarily unavailable. Our staff is working to fix it as soon as possible. Please email us at <a href="mailto:technical@mitx.mit.edu">technical@mitx.mit.edu</a> to report any problems or downtime.</p>
+
+% if is_staff:
+<h1>Staff-only details below:</h1>
+
+<p>Error: ${error | h}</p>
+
+<p>Raw data: ${data | h}</p>
+% endif
+
 </section>