From b5ab766092c340da375d6accee111c4478508a6c Mon Sep 17 00:00:00 2001
From: Brian Wilson <brian@edx.org>
Date: Wed, 6 Mar 2013 17:57:33 -0500
Subject: [PATCH 01/27] initial framework for htmlbook

---
 common/lib/xmodule/xmodule/course_module.py |  7 ++++
 lms/djangoapps/courseware/tabs.py           | 11 ++++++
 lms/djangoapps/staticbook/views.py          | 40 +++++++++++++++++++--
 lms/urls.py                                 |  5 +++
 4 files changed, 60 insertions(+), 3 deletions(-)

diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py
index 72196f92a2..3923d3f056 100644
--- a/common/lib/xmodule/xmodule/course_module.py
+++ b/common/lib/xmodule/xmodule/course_module.py
@@ -358,6 +358,13 @@ class CourseDescriptor(SequenceDescriptor):
         """
         return self.metadata.get('pdf_textbooks')
 
+    @property
+    def html_textbooks(self):
+        """
+        Return the html_textbooks config, as a python object, or None if not specified.
+        """
+        return self.metadata.get('html_textbooks')
+
     @tabs.setter
     def tabs(self, value):
         self.metadata['tabs'] = value
diff --git a/lms/djangoapps/courseware/tabs.py b/lms/djangoapps/courseware/tabs.py
index b31aeb6b5a..4a9de0792f 100644
--- a/lms/djangoapps/courseware/tabs.py
+++ b/lms/djangoapps/courseware/tabs.py
@@ -130,6 +130,17 @@ def _pdf_textbooks(tab, user, course, active_page):
                 for index, textbook in enumerate(course.pdf_textbooks)]
     return []
 
+def _html_textbooks(tab, user, course, active_page):
+    """
+    Generates one tab per textbook.  Only displays if user is authenticated.
+    """
+    if user.is_authenticated():
+        # since there can be more than one textbook, active_page is e.g. "book/0".
+        return [CourseTab(textbook['tab_title'], reverse('html_book', args=[course.id, index]),
+                          active_page == "htmltextbook/{0}".format(index))
+                for index, textbook in enumerate(course.html_textbooks)]
+    return []
+
 def _staff_grading(tab, user, course, active_page):
     if has_access(user, course, 'staff'):
         link = reverse('staff_grading', args=[course.id])
diff --git a/lms/djangoapps/staticbook/views.py b/lms/djangoapps/staticbook/views.py
index a391b1cb32..8e973ac97d 100644
--- a/lms/djangoapps/staticbook/views.py
+++ b/lms/djangoapps/staticbook/views.py
@@ -57,12 +57,46 @@ def pdf_index(request, course_id, book_index, chapter=None, page=None):
     # then remap all the chapter URLs as well, if they are provided.
     if 'chapters' in textbook:
         for entry in textbook['chapters']:
-            entry['url'] = remap_static_url(entry['url'], course)            
+            entry['url'] = remap_static_url(entry['url'], course)
 
 
     return render_to_response('static_pdfbook.html',
-                              {'book_index': book_index, 
-                               'course': course, 
+                              {'book_index': book_index,
+                               'course': course,
+                               'textbook': textbook,
+                               'chapter': chapter,
+                               'page': page,
+                               'staff_access': staff_access})
+
+@login_required
+def html_index(request, course_id, book_index, chapter=None, page=None):
+    course = get_course_with_access(request.user, course_id, 'load')
+    staff_access = has_access(request.user, course, 'staff')
+
+    book_index = int(book_index)
+    textbook = course.html_textbooks[book_index]
+
+    def remap_static_url(original_url, course):
+        input_url = "'" + original_url + "'"
+        output_url = replace_static_urls(
+                    input_url,
+                    course.metadata['data_dir'],
+                    course_namespace=course.location
+                )
+        # strip off the quotes again...
+        return output_url[1:-1]
+
+    if 'url' in textbook:
+        textbook['url'] = remap_static_url(textbook['url'], course)
+    # then remap all the chapter URLs as well, if they are provided.
+    if 'chapters' in textbook:
+        for entry in textbook['chapters']:
+            entry['url'] = remap_static_url(entry['url'], course)
+
+
+    return render_to_response('static_htmlbook.html',
+                              {'book_index': book_index,
+                               'course': course,
                                'textbook': textbook,
                                'chapter': chapter,
                                'page': page,
diff --git a/lms/urls.py b/lms/urls.py
index 5e5ac9a7f2..b85bc3d458 100644
--- a/lms/urls.py
+++ b/lms/urls.py
@@ -285,6 +285,11 @@ if settings.COURSEWARE_ENABLED:
         url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/pdfbook/(?P<book_index>[^/]*)/chapter/(?P<chapter>[^/]*)/(?P<page>[^/]*)$',
             'staticbook.views.pdf_index'),
 
+        url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/htmlbook/(?P<book_index>[^/]*)/$',
+            'staticbook.views.html_index', name="html_book"),
+        url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/htmlbook/(?P<book_index>[^/]*)/(?P<page>[^/]*)$',
+            'staticbook.views.html_index'),
+
         url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/courseware/?$',
             'courseware.views.index', name="courseware"),
         url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/courseware/(?P<chapter>[^/]*)/$',

From aee7d856942b403b768f9a0a9781502777c99401 Mon Sep 17 00:00:00 2001
From: Brian Wilson <brian@edx.org>
Date: Thu, 7 Mar 2013 02:01:42 -0500
Subject: [PATCH 02/27] fix bug in tabs

---
 lms/djangoapps/courseware/tabs.py  |  1 +
 lms/templates/static_htmlbook.html | 71 ++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+)
 create mode 100644 lms/templates/static_htmlbook.html

diff --git a/lms/djangoapps/courseware/tabs.py b/lms/djangoapps/courseware/tabs.py
index 4a9de0792f..a956c03eeb 100644
--- a/lms/djangoapps/courseware/tabs.py
+++ b/lms/djangoapps/courseware/tabs.py
@@ -220,6 +220,7 @@ VALID_TAB_TYPES = {
     'external_link': TabImpl(key_checker(['name', 'link']), _external_link),
     'textbooks': TabImpl(null_validator, _textbooks),
     'pdf_textbooks': TabImpl(null_validator, _pdf_textbooks),
+    'html_textbooks': TabImpl(null_validator, _html_textbooks),
     'progress': TabImpl(need_name, _progress),
     'static_tab': TabImpl(key_checker(['name', 'url_slug']), _static_tab),
     'peer_grading': TabImpl(null_validator, _peer_grading),
diff --git a/lms/templates/static_htmlbook.html b/lms/templates/static_htmlbook.html
new file mode 100644
index 0000000000..e3caa9c0ce
--- /dev/null
+++ b/lms/templates/static_htmlbook.html
@@ -0,0 +1,71 @@
+<%inherit file="main.html" />
+<%namespace name='static' file='static_content.html'/>
+<%block name="title">
+    <meta charset="utf-8">
+    <meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1">
+    <title>${course.number} Textbook</title>
+</%block>
+
+<%block name="headextra">
+<%static:css group='course'/>
+<%static:js group='courseware'/>
+</%block>
+
+<%block name="js_extra">
+  <script type="text/javascript">
+
+    $(document).ready(function() {
+        var options = {};
+        %if 'url' in textbook:
+            options.url = "${textbook['url']}";
+        %endif
+        %if 'chapters' in textbook:
+            var chptrs = [];
+            %for chap in textbook['chapters']:
+                chptrs.push("${chap['url']}");
+            %endfor
+            options.chapters = chptrs;
+        %endif
+        %if chapter is not None:
+            options.chapterNum = ${chapter};
+        %endif
+        %if page is not None:
+            options.pageNum = ${page};
+        %endif
+
+    });
+  </script>  
+</%block>
+
+<%include file="/courseware/course_navigation.html" args="active_page='htmltextbook/{0}'.format(book_index)" />
+
+    <div id="outerContainer">
+      <div id="mainContainer" class="book-wrapper">
+
+        %if 'chapters' in textbook:
+	<section aria-label="Textbook Navigation" class="book-sidebar">
+	  <ul id="booknav" class="treeview-booknav">
+	      <%def name="print_entry(entry, index_value)">
+		  <li id="htmlchapter-${index_value}">
+		    <a class="chapter">
+		      ${entry.get('title')}
+		    </a>
+		  </li>
+	      </%def>
+
+	      % for (index, entry) in enumerate(textbook['chapters']):
+		${print_entry(entry, index+1)}
+	      % endfor
+	  </ul>
+	</section>
+        %endif
+
+        <section id="viewerContainer" class="book">
+<!--	  <section class="page"> -->
+            <iframe class="page" id="bookpage" src="${ textbook['chapters'][0]['url'] }">
+<!--          </section> -->
+        </section>
+
+      </div> <!-- mainContainer -->
+
+    </div> <!-- outerContainer -->

From 49dee5080afd551862e248205c1489e94e4a0081 Mon Sep 17 00:00:00 2001
From: Brian Wilson <brian@edx.org>
Date: Thu, 7 Mar 2013 18:45:00 -0500
Subject: [PATCH 03/27] first pass at chapter navigation

---
 lms/templates/static_htmlbook.html | 126 +++++++++++++++++++++++------
 1 file changed, 100 insertions(+), 26 deletions(-)

diff --git a/lms/templates/static_htmlbook.html b/lms/templates/static_htmlbook.html
index e3caa9c0ce..2cdec8c109 100644
--- a/lms/templates/static_htmlbook.html
+++ b/lms/templates/static_htmlbook.html
@@ -1,9 +1,6 @@
 <%inherit file="main.html" />
 <%namespace name='static' file='static_content.html'/>
-<%block name="title">
-    <meta charset="utf-8">
-    <meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1">
-    <title>${course.number} Textbook</title>
+<%block name="title"><title>${course.number} Textbook</title>
 </%block>
 
 <%block name="headextra">
@@ -13,6 +10,85 @@
 
 <%block name="js_extra">
   <script type="text/javascript">
+(function($) {
+    $.fn.myHTMLViewer = function(options) {
+        var urlToLoad = null;
+        if (options.url) {
+            urlToLoad = options.url;
+        }
+        var chapterUrls = null;
+        if (options.chapters) {
+            chapterUrls = options.chapters;
+        }
+        var chapterToLoad = 1;
+        if (options.chapterNum) {
+            // TODO: this should only be specified if there are 
+            // chapters, and it should be in-bounds.
+            chapterToLoad = options.chapterNum;
+        }
+        var anchorToLoad = null;
+
+        loadUrlAsIframe = function htmlViewLoadUrlAsIframe(url, anchorId) {
+            if (anchorId != null) {
+                var newurl = url + "#" + anchorId;
+                $("#bookpage").src = newurl;
+            } else {
+                // $("#bookpage").src = url;
+                // $("#bookpage").append("<iframe seamless id="bookpage-iframe" src=" + url + "\"></iframe>")
+                parentElement = document.getElementById('bookpage');
+                while (parentElement.hasChildNodes())
+                    parentElement.removeChild(parentElement.lastChild);
+
+                $('<iframe id="bookpage-iframe" src="' + url + '"></iframe>"')
+                    // .css({'height':'40px','width':'200px'})
+                    .appendTo('#bookpage');
+            }
+        }; 
+
+        loadUrl = function htmlViewLoadUrl(url, anchorId) {
+            var newurl = url;
+            if (anchorId != null) {
+                newurl = url + "#" + anchorId;
+            } 
+            // clear out previous load, if any:
+            parentElement = document.getElementById('bookpage');
+            while (parentElement.hasChildNodes())
+                parentElement.removeChild(parentElement.lastChild);
+
+            $('#bookpage').load(newurl);
+
+        }; 
+
+        loadChapterUrl = function htmlViewLoadChapterUrl(chapterNum, anchorId) {
+            if (chapterNum < 1 || chapterNum > chapterUrls.length) {
+                return;
+            }
+            var chapterUrl = chapterUrls[chapterNum-1];
+            loadUrl(chapterUrl, anchorId);
+        };
+
+        // define navigation links for chapters:  
+        if (chapterUrls != null) {
+            var loadChapterUrlHelper = function(i) {
+                return function(event) {
+                    // when opening a new chapter, always open to the top:
+                    loadChapterUrl(i, null);
+                };
+            };
+            for (var index = 1; index <= chapterUrls.length; index += 1) {
+                $("#htmlchapter-" + index).click(loadChapterUrlHelper(index));
+            }   
+        }
+
+        // finally, load the appropriate url/page
+        if (urlToLoad != null) {
+            loadUrl(urlToLoad, anchorToLoad);
+        } else {
+            loadChapterUrl(chapterToLoad, anchorToLoad);
+        }       
+
+    }
+})(jQuery);
 
     $(document).ready(function() {
         var options = {};
@@ -29,10 +105,8 @@
         %if chapter is not None:
             options.chapterNum = ${chapter};
         %endif
-        %if page is not None:
-            options.pageNum = ${page};
-        %endif
 
+        $('#outerContainer').myHTMLViewer(options);
     });
   </script>  
 </%block>
@@ -43,29 +117,29 @@
       <div id="mainContainer" class="book-wrapper">
 
         %if 'chapters' in textbook:
-	<section aria-label="Textbook Navigation" class="book-sidebar">
-	  <ul id="booknav" class="treeview-booknav">
-	      <%def name="print_entry(entry, index_value)">
-		  <li id="htmlchapter-${index_value}">
-		    <a class="chapter">
-		      ${entry.get('title')}
-		    </a>
-		  </li>
-	      </%def>
+        <section aria-label="Textbook Navigation" class="book-sidebar">
+          <ul id="booknav" class="treeview-booknav">
+            <%def name="print_entry(entry, index_value)">
+              <li id="htmlchapter-${index_value}">
+                <a class="chapter">
+                  ${entry.get('title')}
+                </a>
+              </li>
+            </%def>
 
-	      % for (index, entry) in enumerate(textbook['chapters']):
-		${print_entry(entry, index+1)}
-	      % endfor
-	  </ul>
-	</section>
+            %for (index, entry) in enumerate(textbook['chapters']):
+              ${print_entry(entry, index+1)}
+            % endfor
+          </ul>
+        </section>
         %endif
 
         <section id="viewerContainer" class="book">
-<!--	  <section class="page"> -->
-            <iframe class="page" id="bookpage" src="${ textbook['chapters'][0]['url'] }">
-<!--          </section> -->
+          <section class="page"> 
+            <div id="bookpage" />
+          </section> 
         </section>
 
-      </div> <!-- mainContainer -->
+      </div>
+    </div>
 
-    </div> <!-- outerContainer -->

From bed7dbdb6bf3bae7f51d8c1012f4fb82cebd44ae Mon Sep 17 00:00:00 2001
From: Brian Wilson <brian@edx.org>
Date: Fri, 8 Mar 2013 18:25:33 -0500
Subject: [PATCH 04/27] fix plumbing for anchors and chapters.

---
 lms/djangoapps/staticbook/views.py |  4 ++--
 lms/templates/static_htmlbook.html | 34 +++++++++++-------------------
 lms/urls.py                        |  6 +++++-
 3 files changed, 19 insertions(+), 25 deletions(-)

diff --git a/lms/djangoapps/staticbook/views.py b/lms/djangoapps/staticbook/views.py
index 8e973ac97d..72c72e3154 100644
--- a/lms/djangoapps/staticbook/views.py
+++ b/lms/djangoapps/staticbook/views.py
@@ -69,7 +69,7 @@ def pdf_index(request, course_id, book_index, chapter=None, page=None):
                                'staff_access': staff_access})
 
 @login_required
-def html_index(request, course_id, book_index, chapter=None, page=None):
+def html_index(request, course_id, book_index, chapter=None, anchor_id=None):
     course = get_course_with_access(request.user, course_id, 'load')
     staff_access = has_access(request.user, course, 'staff')
 
@@ -99,5 +99,5 @@ def html_index(request, course_id, book_index, chapter=None, page=None):
                                'course': course,
                                'textbook': textbook,
                                'chapter': chapter,
-                               'page': page,
+                               'anchor_id': anchor_id,
                                'staff_access': staff_access})
diff --git a/lms/templates/static_htmlbook.html b/lms/templates/static_htmlbook.html
index 2cdec8c109..9500a379ac 100644
--- a/lms/templates/static_htmlbook.html
+++ b/lms/templates/static_htmlbook.html
@@ -27,35 +27,22 @@
             chapterToLoad = options.chapterNum;
         }
         var anchorToLoad = null;
-
-        loadUrlAsIframe = function htmlViewLoadUrlAsIframe(url, anchorId) {
-            if (anchorId != null) {
-                var newurl = url + "#" + anchorId;
-                $("#bookpage").src = newurl;
-            } else {
-                // $("#bookpage").src = url;
-                // $("#bookpage").append("<iframe seamless id="bookpage-iframe" src=" + url + "\"></iframe>")
-                parentElement = document.getElementById('bookpage');
-                while (parentElement.hasChildNodes())
-                    parentElement.removeChild(parentElement.lastChild);
-
-                $('<iframe id="bookpage-iframe" src="' + url + '"></iframe>"')
-                    // .css({'height':'40px','width':'200px'})
-                    .appendTo('#bookpage');
-            }
-        }; 
+        if (options.chapters) {
+            anchorToLoad = options.anchor_id;
+        }
 
         loadUrl = function htmlViewLoadUrl(url, anchorId) {
-            var newurl = url;
-            if (anchorId != null) {
-                newurl = url + "#" + anchorId;
-            } 
             // clear out previous load, if any:
             parentElement = document.getElementById('bookpage');
             while (parentElement.hasChildNodes())
                 parentElement.removeChild(parentElement.lastChild);
+	    // load new URL in:
+            $('#bookpage').load(url);
 
-            $('#bookpage').load(newurl);
+	    // if there is an anchor set, then go to that location:
+            if (anchorId != null) {
+		// TODO: add implementation....
+            } 
 
         }; 
 
@@ -105,6 +92,9 @@
         %if chapter is not None:
             options.chapterNum = ${chapter};
         %endif
+        %if anchor_id is not None:
+            options.anchor_id = ${anchor_id};
+        %endif
 
         $('#outerContainer').myHTMLViewer(options);
     });
diff --git a/lms/urls.py b/lms/urls.py
index b85bc3d458..3377fa54c0 100644
--- a/lms/urls.py
+++ b/lms/urls.py
@@ -287,7 +287,11 @@ if settings.COURSEWARE_ENABLED:
 
         url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/htmlbook/(?P<book_index>[^/]*)/$',
             'staticbook.views.html_index', name="html_book"),
-        url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/htmlbook/(?P<book_index>[^/]*)/(?P<page>[^/]*)$',
+        url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/htmlbook/(?P<book_index>[^/]*)/chapter/(?P<chapter>[^/]*)/$',
+            'staticbook.views.html_index'),
+        url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/htmlbook/(?P<book_index>[^/]*)/chapter/(?P<chapter>[^/]*)/(?P<anchor_id>[^/]*)/$',
+            'staticbook.views.html_index'),
+        url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/htmlbook/(?P<book_index>[^/]*)/(?P<anchor_id>[^/]*)/$',
             'staticbook.views.html_index'),
 
         url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/courseware/?$',

From 18ee1018e63db73a8d19c3e9b86bd2515bbbd3d6 Mon Sep 17 00:00:00 2001
From: Chris Dodge <cdodge@edx.org>
Date: Sun, 10 Mar 2013 11:11:17 -0400
Subject: [PATCH 05/27] optimize forum page rendering as we don't pre-compute
 the link urls to inline discussions. We can use jump_to urls and then figure
 out the link path if/when end-user clicks on it. This saves a lot of
 unnecessary round trips to the DB as path computation is expensive,
 especially when it being done for every discussion module in a course in a
 loop.

---
 lms/djangoapps/django_comment_client/utils.py | 28 ++-----------------
 lms/envs/dev.py                               |  3 +-
 2 files changed, 4 insertions(+), 27 deletions(-)

diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py
index dde219294c..7cc36c491b 100644
--- a/lms/djangoapps/django_comment_client/utils.py
+++ b/lms/djangoapps/django_comment_client/utils.py
@@ -18,7 +18,6 @@ import pystache_custom as pystache
 
 from xmodule.modulestore import Location
 from xmodule.modulestore.django import modulestore
-from xmodule.modulestore.search import path_to_location
 
 log = logging.getLogger(__name__)
 
@@ -166,7 +165,6 @@ def initialize_discussion_info(course):
     # get all discussion models within this course_id
     all_modules = modulestore().get_items(['i4x', course.location.org, course.location.course, 'discussion', None], course_id=course_id)
 
-    path_to_locations = {}
     for module in all_modules:
         skip_module = False
         for key in ('id', 'discussion_category', 'for'):
@@ -174,14 +172,6 @@ def initialize_discussion_info(course):
                 log.warning("Required key '%s' not in discussion %s, leaving out of category map" % (key, module.location))
                 skip_module = True
 
-        # cdodge: pre-compute the path_to_location. Note this can throw an exception for any
-        # dangling discussion modules
-        try:
-            path_to_locations[module.location] = path_to_location(modulestore(), course.id, module.location)
-        except NoPathToItem:
-            log.warning("Could not compute path_to_location for {0}. Perhaps this is an orphaned discussion module?!? Skipping...".format(module.location))
-            skip_module = True
-
         if skip_module:
             continue
 
@@ -246,7 +236,6 @@ def initialize_discussion_info(course):
     _DISCUSSIONINFO[course.id]['id_map'] = discussion_id_map
     _DISCUSSIONINFO[course.id]['category_map'] = category_map
     _DISCUSSIONINFO[course.id]['timestamp'] = datetime.now()
-    _DISCUSSIONINFO[course.id]['path_to_location'] = path_to_locations
 
 
 class JsonResponse(HttpResponse):
@@ -403,21 +392,8 @@ def get_courseware_context(content, course):
         location = id_map[id]["location"].url()
         title = id_map[id]["title"]
 
-        # cdodge: did we pre-compute, if so, then let's use that rather than recomputing
-        if 'path_to_location' in _DISCUSSIONINFO[course.id] and location in _DISCUSSIONINFO[course.id]['path_to_location']:
-            (course_id, chapter, section, position) = _DISCUSSIONINFO[course.id]['path_to_location'][location]
-        else:
-            try:
-                (course_id, chapter, section, position) = path_to_location(modulestore(), course.id, location)
-            except NoPathToItem:
-                # Object is not in the graph any longer, let's just get path to the base of the course
-                # so that we can at least return something to the caller
-                (course_id, chapter, section, position) = path_to_location(modulestore(), course.id, course.location)
-
-        url = reverse('courseware_position', kwargs={"course_id":course_id,
-                                                     "chapter":chapter,
-                                                     "section":section,
-                                                     "position":position})
+        url = reverse('jump_to', kwargs={"course_id":course.location.course_id, 
+                                                    "location": location})
 
         content_info = {"courseware_url": url, "courseware_title": title}
     return content_info
diff --git a/lms/envs/dev.py b/lms/envs/dev.py
index 6ecbbb0f85..84cd80859c 100644
--- a/lms/envs/dev.py
+++ b/lms/envs/dev.py
@@ -104,7 +104,8 @@ SUBDOMAIN_BRANDING = {
 # have an actual course with that org set
 VIRTUAL_UNIVERSITIES = []
 
-COMMENTS_SERVICE_KEY = "PUT_YOUR_API_KEY_HERE"
+COMMENTS_SERVICE_KEY = "***REMOVED***"
+COMMENTS_SERVICE_URL = "https://comments-edge-stage.herokuapp.com"
 
 ############################## Course static files ##########################
 if os.path.isdir(DATA_DIR):

From cbcda0fc65a11961e87b3d2ff6df741bad4c9472 Mon Sep 17 00:00:00 2001
From: Chris Dodge <cdodge@edx.org>
Date: Sun, 10 Mar 2013 11:14:25 -0400
Subject: [PATCH 06/27] oops didn't mean to commit the test API key

---
 lms/envs/dev.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lms/envs/dev.py b/lms/envs/dev.py
index 84cd80859c..6ecbbb0f85 100644
--- a/lms/envs/dev.py
+++ b/lms/envs/dev.py
@@ -104,8 +104,7 @@ SUBDOMAIN_BRANDING = {
 # have an actual course with that org set
 VIRTUAL_UNIVERSITIES = []
 
-COMMENTS_SERVICE_KEY = "***REMOVED***"
-COMMENTS_SERVICE_URL = "https://comments-edge-stage.herokuapp.com"
+COMMENTS_SERVICE_KEY = "PUT_YOUR_API_KEY_HERE"
 
 ############################## Course static files ##########################
 if os.path.isdir(DATA_DIR):

From 0c949d20c9b98041d898900f91db3b966f313f13 Mon Sep 17 00:00:00 2001
From: Diana Huang <dkh@edx.org>
Date: Mon, 11 Mar 2013 10:09:56 -0400
Subject: [PATCH 07/27] Add the ability to hide correctness markers with a
 custom message for when they submit something.

---
 common/lib/capa/capa/inputtypes.py              | 6 ++++++
 common/lib/capa/capa/templates/choicegroup.html | 7 +++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py
index f614743e67..9781f10ae6 100644
--- a/common/lib/capa/capa/inputtypes.py
+++ b/common/lib/capa/capa/inputtypes.py
@@ -366,6 +366,12 @@ class ChoiceGroup(InputTypeBase):
 
         self.choices = self.extract_choices(self.xml)
 
+    @classmethod
+    def get_attributes(cls):
+        return [Attribute("show_correctness", "always"),
+                Attribute("submitted_message", "Answer received.")]
+
+
     def _extra_context(self):
         return {'input_type': self.html_input_type,
                 'choices': self.choices,
diff --git a/common/lib/capa/capa/templates/choicegroup.html b/common/lib/capa/capa/templates/choicegroup.html
index e4a3f1dc39..e1ff40b6a1 100644
--- a/common/lib/capa/capa/templates/choicegroup.html
+++ b/common/lib/capa/capa/templates/choicegroup.html
@@ -1,7 +1,7 @@
 <form class="choicegroup capa_inputtype" id="inputtype_${id}">
     <div class="indicator_container">
     % if input_type == 'checkbox' or not value:
-        % if status == 'unsubmitted':
+        % if status == 'unsubmitted' or show_correctness == 'never':
         <span class="unanswered" style="display:inline-block;" id="status_${id}"></span>
         % elif status == 'correct': 
         <span class="correct" id="status_${id}"></span>
@@ -26,7 +26,7 @@
                 else:
                     correctness = None
             %>
-            % if correctness:
+            % if correctness and not show_correctness=='never':
             class="choicegroup_${correctness}"
             % endif
             % endif
@@ -41,4 +41,7 @@
         <span id="answer_${id}"></span>
     </fieldset>
 
+    % if show_correctness == "never" and (value or status not in ['unsubmitted']):
+    <div class="capa_alert">${submitted_message}</div>
+    %endif
 </form>

From 421e169e1ee3d7536d78023fcbb15070e2246550 Mon Sep 17 00:00:00 2001
From: Diana Huang <dkh@edx.org>
Date: Mon, 11 Mar 2013 10:29:18 -0400
Subject: [PATCH 08/27] Update tests with new attributes.

---
 common/lib/capa/capa/tests/test_inputtypes.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/common/lib/capa/capa/tests/test_inputtypes.py b/common/lib/capa/capa/tests/test_inputtypes.py
index f670a38746..287caad28f 100644
--- a/common/lib/capa/capa/tests/test_inputtypes.py
+++ b/common/lib/capa/capa/tests/test_inputtypes.py
@@ -102,6 +102,8 @@ class ChoiceGroupTest(unittest.TestCase):
                     'choices': [('foil1', '<text>This is foil One.</text>'),
                                 ('foil2', '<text>This is foil Two.</text>'),
                                 ('foil3', 'This is foil Three.'), ],
+                    'show_correctness': 'always',
+                    'submitted_message': 'Answer received.',
                     'name_array_suffix': expected_suffix,   # what is this for??
                     }
 

From a1febce86bff258ea3495b7b3ada8209add1de39 Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Mon, 11 Mar 2013 10:33:09 -0400
Subject: [PATCH 09/27] Specify that mitx should use ruby 1.9.3-p374 for
 consistancy with prod

---
 .ruby-version | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/.ruby-version b/.ruby-version
index dd472cffa2..311baaf3e2 100644
--- a/.ruby-version
+++ b/.ruby-version
@@ -1 +1 @@
-1.8.7-p371
\ No newline at end of file
+1.9.3-p374

From fcb618f04eedd65602914582546c57aed77ad30b Mon Sep 17 00:00:00 2001
From: Brian Wilson <brian@edx.org>
Date: Mon, 11 Mar 2013 10:38:32 -0400
Subject: [PATCH 10/27] fix basic alignment issue with html textbook display

---
 lms/static/sass/course/_textbook.scss | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lms/static/sass/course/_textbook.scss b/lms/static/sass/course/_textbook.scss
index af9c2493fd..aba076af5b 100644
--- a/lms/static/sass/course/_textbook.scss
+++ b/lms/static/sass/course/_textbook.scss
@@ -158,6 +158,10 @@ div.book-wrapper {
       img {
         max-width: 100%;
       }
+
+      div {
+        text-align: left;
+      }
     }
   }
 

From d38b37f0b4964aaf56c23b47469aa7f7ddc19f37 Mon Sep 17 00:00:00 2001
From: Chris Dodge <cdodge@edx.org>
Date: Mon, 11 Mar 2013 10:47:22 -0400
Subject: [PATCH 11/27] delete_item should update the parent's list of children
 to reflect the delete operation. Also added unit test

---
 .../contentstore/tests/test_contentstore.py   | 31 +++++++++++++++++++
 cms/djangoapps/contentstore/views.py          | 11 +++++++
 2 files changed, 42 insertions(+)

diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py
index c0ab9ec60e..50bbb305f5 100644
--- a/cms/djangoapps/contentstore/tests/test_contentstore.py
+++ b/cms/djangoapps/contentstore/tests/test_contentstore.py
@@ -103,6 +103,37 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
         self.assertEqual(reverse_tabs, course_tabs)
 
+    def test_delete(self):
+        import_from_xml(modulestore(), 'common/test/data/', ['full'])
+
+        ms = modulestore('direct')
+        course = ms.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None]))
+
+        sequential = ms.get_item(Location(['i4x', 'edX', 'full', 'sequential','Administrivia_and_Circuit_Elements', None]))
+
+        chapter = ms.get_item(Location(['i4x', 'edX', 'full', 'chapter','Week_1', None]))
+
+        # make sure the parent no longer points to the child object which was deleted
+        self.assertTrue(sequential.location.url() in chapter.definition['children'])
+
+        resp = self.client.post(reverse('delete_item'), json.dumps({'id': sequential.location.url(), 'delete_children':'true'}), "application/json")
+
+        bFound = False
+        try:
+            sequential = ms.get_item(Location(['i4x', 'edX', 'full', 'sequential','Administrivia_and_Circuit_Elements', None]))
+            bFound = True
+        except ItemNotFoundError:
+            pass
+
+        self.assertFalse(bFound)
+
+        chapter = ms.get_item(Location(['i4x', 'edX', 'full', 'chapter','Week_1', None]))
+
+        # make sure the parent no longer points to the child object which was deleted
+        self.assertFalse(sequential.location.url() in chapter.definition['children'])
+
+        
+
     def test_about_overrides(self):
         '''
         This test case verifies that a course can use specialized override for about data, e.g. /about/Fall_2012/effort.html
diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py
index 34003d71a4..846c0625b1 100644
--- a/cms/djangoapps/contentstore/views.py
+++ b/cms/djangoapps/contentstore/views.py
@@ -636,6 +636,17 @@ def delete_item(request):
     if item.location.revision is None and item.location.category == 'vertical' and delete_all_versions:
         modulestore('direct').delete_item(item.location)
 
+    # cdodge: we need to remove our parent's pointer to us so that it is no longer dangling
+
+    parent_locs = modulestore('direct').get_parent_locations(item_loc, None)
+
+    for parent_loc in parent_locs:
+        parent = modulestore('direct').get_item(parent_loc)
+        item_url = item_loc.url()
+        if item_url in parent.definition["children"]:
+            parent.definition["children"].remove(item_url)
+            modulestore('direct').update_children(parent.location, parent.definition["children"])
+
     return HttpResponse()
 
 

From 2199ae673b1895ae5a004b6dcc88bca67beecf61 Mon Sep 17 00:00:00 2001
From: Brian Wilson <brian@edx.org>
Date: Mon, 11 Mar 2013 11:10:08 -0400
Subject: [PATCH 12/27] add checks that book_index is in range

---
 lms/djangoapps/staticbook/views.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lms/djangoapps/staticbook/views.py b/lms/djangoapps/staticbook/views.py
index 72c72e3154..ec34683997 100644
--- a/lms/djangoapps/staticbook/views.py
+++ b/lms/djangoapps/staticbook/views.py
@@ -1,7 +1,7 @@
 from lxml import etree
 
-# from django.conf import settings
 from django.contrib.auth.decorators import login_required
+from django.http import Http404
 from mitxmako.shortcuts import render_to_response
 
 from courseware.access import has_access
@@ -15,6 +15,8 @@ def index(request, course_id, book_index, page=None):
     staff_access = has_access(request.user, course, 'staff')
 
     book_index = int(book_index)
+    if book_index < 0 or book_index >= len(course.textbooks):
+        raise Http404("Invalid book index value: {0}".format(book_index))
     textbook = course.textbooks[book_index]
     table_of_contents = textbook.table_of_contents
 
@@ -40,6 +42,8 @@ def pdf_index(request, course_id, book_index, chapter=None, page=None):
     staff_access = has_access(request.user, course, 'staff')
 
     book_index = int(book_index)
+    if book_index < 0 or book_index >= len(course.pdf_textbooks):
+        raise Http404("Invalid book index value: {0}".format(book_index))
     textbook = course.pdf_textbooks[book_index]
 
     def remap_static_url(original_url, course):
@@ -74,6 +78,8 @@ def html_index(request, course_id, book_index, chapter=None, anchor_id=None):
     staff_access = has_access(request.user, course, 'staff')
 
     book_index = int(book_index)
+    if book_index < 0 or book_index >= len(course.html_textbooks):
+        raise Http404("Invalid book index value: {0}".format(book_index))
     textbook = course.html_textbooks[book_index]
 
     def remap_static_url(original_url, course):

From 588b27c9dc6911e9b8e0728f274b155cf9674c46 Mon Sep 17 00:00:00 2001
From: Julian Arni <julian@edx.org>
Date: Mon, 11 Mar 2013 11:17:12 -0400
Subject: [PATCH 13/27] Added Descriptor method, fixed order issue.

---
 common/lib/xmodule/xmodule/foldit_module.py | 14 ++++++++++----
 lms/djangoapps/foldit/models.py             |  2 +-
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/common/lib/xmodule/xmodule/foldit_module.py b/common/lib/xmodule/xmodule/foldit_module.py
index 920a5aed6d..2bcdda5bc2 100644
--- a/common/lib/xmodule/xmodule/foldit_module.py
+++ b/common/lib/xmodule/xmodule/foldit_module.py
@@ -86,7 +86,10 @@ class FolditModule(XModule):
         """
         from foldit.models import Score
 
-        return [(e['username'], e['score']) for e in Score.get_tops_n(10)]
+        leaders = [(e['username'], e['score']) for e in Score.get_tops_n(10)]
+        leaders.sort(key=lambda x: x[1])
+
+        return leaders
 
     def get_html(self):
         """
@@ -173,7 +176,10 @@ class FolditDescriptor(XmlDescriptor, EditingDescriptor):
 
     @classmethod
     def definition_from_xml(cls, xml_object, system):
-        """
-        Get the xml_object's attributes.
-        """
+        """ Get the xml_object's attributes.  """
+
         return {'metadata': xml_object.attrib}
+
+    def definition_to_xml(self):
+        xml_object = etree.Element('foldit')
+        return xml_object
diff --git a/lms/djangoapps/foldit/models.py b/lms/djangoapps/foldit/models.py
index 7041be1446..0dce956756 100644
--- a/lms/djangoapps/foldit/models.py
+++ b/lms/djangoapps/foldit/models.py
@@ -59,7 +59,7 @@ class Score(models.Model):
         scores = Score.objects \
             .filter(puzzle_id__in=puzzles) \
             .annotate(total_score=models.Sum('best_score')) \
-            .order_by('-total_score')[:n]
+            .order_by('total_score')[:n]
         num = len(puzzles)
 
         return [{'username': s.user.username,

From cd95872b52fa1f6b7b5055f5312573049e37a4f3 Mon Sep 17 00:00:00 2001
From: Brian Wilson <brian@edx.org>
Date: Mon, 11 Mar 2013 11:56:48 -0400
Subject: [PATCH 14/27] htmlbook-specific styling to make Heroes text
 presentable.

---
 lms/static/sass/course/_textbook.scss | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/lms/static/sass/course/_textbook.scss b/lms/static/sass/course/_textbook.scss
index aba076af5b..b1f3a863b8 100644
--- a/lms/static/sass/course/_textbook.scss
+++ b/lms/static/sass/course/_textbook.scss
@@ -161,6 +161,15 @@ div.book-wrapper {
 
       div {
         text-align: left;
+	line-height: 1.6em;
+	margin-left: 5px;
+	margin-right: 5px;
+	margin-top: 5px;
+	margin-bottom: 5px;
+	
+	.Paragraph, h2 {
+	   margin-top: 10px;
+	}	   
       }
     }
   }

From 81527d60d8d307f372743638c08285b4fc418c55 Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Mon, 11 Mar 2013 12:27:56 -0400
Subject: [PATCH 15/27] Move pep8 and pylint into test suite, so that we can
 rachet down the number of violations

---
 jenkins/quality.sh | 16 ----------------
 jenkins/test.sh    |  5 ++++-
 2 files changed, 4 insertions(+), 17 deletions(-)
 delete mode 100755 jenkins/quality.sh

diff --git a/jenkins/quality.sh b/jenkins/quality.sh
deleted file mode 100755
index 56217af874..0000000000
--- a/jenkins/quality.sh
+++ /dev/null
@@ -1,16 +0,0 @@
-#! /bin/bash
-
-set -e
-set -x
-
-git remote prune origin
-
-# Reset the submodule, in case it changed
-git submodule foreach 'git reset --hard HEAD'
-
-# Set the IO encoding to UTF-8 so that askbot will start
-export PYTHONIOENCODING=UTF-8
-
-rake clobber
-rake pep8 || echo "pep8 failed, continuing"
-rake pylint || echo "pylint failed, continuing"
diff --git a/jenkins/test.sh b/jenkins/test.sh
index 5b9a5ed9bd..3a572c9808 100755
--- a/jenkins/test.sh
+++ b/jenkins/test.sh
@@ -38,12 +38,15 @@ pip install -q -r test-requirements.txt
 yes w | pip install -q -r requirements.txt
 
 rake clobber
+rake pep8
+rake pylint
+
 TESTS_FAILED=0
 rake test_cms[false] || TESTS_FAILED=1
 rake test_lms[false] || TESTS_FAILED=1
 rake test_common/lib/capa || TESTS_FAILED=1
 rake test_common/lib/xmodule || TESTS_FAILED=1
-# Don't run the lms jasmine tests for now because 
+# Don't run the lms jasmine tests for now because
 # they mostly all fail anyhow
 # rake phantomjs_jasmine_lms || true
 rake phantomjs_jasmine_cms || TESTS_FAILED=1

From 5f95c6848cd9031df7a8b87ae0262b3c005610a3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Carlos=20Andr=C3=A9s=20Rocha?= <rocha@edx.org>
Date: Mon, 11 Mar 2013 12:53:01 -0400
Subject: [PATCH 16/27] Make numeric response handle plus sign exponential
 correctly

NumericResponse was failing to accept as correct an answer with a plus
sign in the exponential. For example: 5.0e+1 was marked as incorrect
if the answer was 50. However, 5.0e1 was marked as correct. This
caused confusion, since the answer shown to students included the plus
sign.

LMS Lighthouse [#242]
---
 common/lib/capa/capa/calc.py                  |  2 +-
 .../lib/capa/capa/tests/test_responsetypes.py | 55 +++++++++++--------
 2 files changed, 33 insertions(+), 24 deletions(-)

diff --git a/common/lib/capa/capa/calc.py b/common/lib/capa/capa/calc.py
index 0f062d17d5..c3fe6b656b 100644
--- a/common/lib/capa/capa/calc.py
+++ b/common/lib/capa/capa/calc.py
@@ -183,7 +183,7 @@ def evaluator(variables, functions, string, cs=False):
 
     # 0.33k or -17
     number = (Optional(minus | plus) + inner_number
-              + Optional(CaselessLiteral("E") + Optional("-") + number_part)
+              + Optional(CaselessLiteral("E") + Optional((plus | minus)) + number_part)
               + Optional(number_suffix))
     number = number.setParseAction(number_parse_action)  # Convert to number
 
diff --git a/common/lib/capa/capa/tests/test_responsetypes.py b/common/lib/capa/capa/tests/test_responsetypes.py
index 93a7e9628a..e024909d75 100644
--- a/common/lib/capa/capa/tests/test_responsetypes.py
+++ b/common/lib/capa/capa/tests/test_responsetypes.py
@@ -19,7 +19,7 @@ from capa.xqueue_interface import dateformat
 
 class ResponseTest(unittest.TestCase):
     """ Base class for tests of capa responses."""
-    
+
     xml_factory_class = None
 
     def setUp(self):
@@ -43,7 +43,7 @@ class ResponseTest(unittest.TestCase):
 
         for input_str in incorrect_answers:
             result = problem.grade_answers({'1_2_1': input_str}).get_correctness('1_2_1')
-            self.assertEqual(result, 'incorrect', 
+            self.assertEqual(result, 'incorrect',
                             msg="%s should be marked incorrect" % str(input_str))
 
 class MultiChoiceResponseTest(ResponseTest):
@@ -61,7 +61,7 @@ class MultiChoiceResponseTest(ResponseTest):
     def test_named_multiple_choice_grade(self):
         problem = self.build_problem(choices=[False, True, False],
                                     choice_names=["foil_1", "foil_2", "foil_3"])
-        
+
         # Ensure that we get the expected grades
         self.assert_grade(problem, 'choice_foil_1', 'incorrect')
         self.assert_grade(problem, 'choice_foil_2', 'correct')
@@ -117,7 +117,7 @@ class ImageResponseTest(ResponseTest):
 
         # Anything inside the rectangle (and along the borders) is correct
         # Everything else is incorrect
-        correct_inputs = ["[12,19]", "[10,10]", "[20,20]", 
+        correct_inputs = ["[12,19]", "[10,10]", "[20,20]",
                             "[10,15]", "[20,15]", "[15,10]", "[15,20]"]
         incorrect_inputs = ["[4,6]", "[25,15]", "[15,40]", "[15,4]"]
         self.assert_multiple_grade(problem, correct_inputs, incorrect_inputs)
@@ -259,7 +259,7 @@ class OptionResponseTest(ResponseTest):
     xml_factory_class = OptionResponseXMLFactory
 
     def test_grade(self):
-        problem = self.build_problem(options=["first", "second", "third"], 
+        problem = self.build_problem(options=["first", "second", "third"],
                                     correct_option="second")
 
         # Assert that we get the expected grades
@@ -374,8 +374,8 @@ class StringResponseTest(ResponseTest):
         hints = [("wisconsin", "wisc", "The state capital of Wisconsin is Madison"),
                 ("minnesota", "minn", "The state capital of Minnesota is St. Paul")]
 
-        problem = self.build_problem(answer="Michigan", 
-                                    case_sensitive=False, 
+        problem = self.build_problem(answer="Michigan",
+                                    case_sensitive=False,
                                     hints=hints)
 
         # We should get a hint for Wisconsin
@@ -543,7 +543,7 @@ class ChoiceResponseTest(ResponseTest):
     xml_factory_class = ChoiceResponseXMLFactory
 
     def test_radio_group_grade(self):
-        problem = self.build_problem(choice_type='radio', 
+        problem = self.build_problem(choice_type='radio',
                                         choices=[False, True, False])
 
         # Check that we get the expected results
@@ -601,17 +601,17 @@ class NumericalResponseTest(ResponseTest):
         correct_responses = ["4", "4.0", "4.00"]
         incorrect_responses = ["", "3.9", "4.1", "0"]
         self.assert_multiple_grade(problem, correct_responses, incorrect_responses)
-        
+
 
     def test_grade_decimal_tolerance(self):
         problem = self.build_problem(question_text="What is 2 + 2 approximately?",
                                         explanation="The answer is 4",
                                         answer=4,
                                         tolerance=0.1)
-        correct_responses = ["4.0", "4.00", "4.09", "3.91"] 
+        correct_responses = ["4.0", "4.00", "4.09", "3.91"]
         incorrect_responses = ["", "4.11", "3.89", "0"]
         self.assert_multiple_grade(problem, correct_responses, incorrect_responses)
-                        
+
     def test_grade_percent_tolerance(self):
         problem = self.build_problem(question_text="What is 2 + 2 approximately?",
                                         explanation="The answer is 4",
@@ -642,6 +642,15 @@ class NumericalResponseTest(ResponseTest):
         incorrect_responses = ["", "2.11", "1.89", "0"]
         self.assert_multiple_grade(problem, correct_responses, incorrect_responses)
 
+    def test_exponential_answer(self):
+        problem = self.build_problem(question_text="What 5 * 10?",
+                                     explanation="The answer is 50",
+                                     answer="5e+1")
+        correct_responses = ["50", "50.0", "5e1", "5e+1", "50e0", "500e-1"]
+        incorrect_responses = ["", "3.9", "4.1", "0", "5.01e1"]
+        self.assert_multiple_grade(problem, correct_responses, incorrect_responses)
+
+
 
 class CustomResponseTest(ResponseTest):
     from response_xml_factory import CustomResponseXMLFactory
@@ -667,7 +676,7 @@ class CustomResponseTest(ResponseTest):
         # The code can also set the global overall_message (str)
         # to pass a message that applies to the whole response
         inline_script = textwrap.dedent("""
-        messages[0] = "Test Message" 
+        messages[0] = "Test Message"
         overall_message = "Overall message"
         """)
         problem = self.build_problem(answer=inline_script)
@@ -687,14 +696,14 @@ class CustomResponseTest(ResponseTest):
     def test_function_code_single_input(self):
 
         # For function code, we pass in these arguments:
-        # 
+        #
         #   'expect' is the expect attribute of the <customresponse>
         #
         #   'answer_given' is the answer the student gave (if there is just one input)
         #       or an ordered list of answers (if there are multiple inputs)
-        #   
         #
-        # The function should return a dict of the form 
+        #
+        # The function should return a dict of the form
         # { 'ok': BOOL, 'msg': STRING }
         #
         script = textwrap.dedent("""
@@ -727,7 +736,7 @@ class CustomResponseTest(ResponseTest):
     def test_function_code_multiple_input_no_msg(self):
 
         # Check functions also have the option of returning
-        # a single boolean value 
+        # a single boolean value
         # If true, mark all the inputs correct
         # If false, mark all the inputs incorrect
         script = textwrap.dedent("""
@@ -736,7 +745,7 @@ class CustomResponseTest(ResponseTest):
                         answer_given[1] == expect)
         """)
 
-        problem = self.build_problem(script=script, cfn="check_func", 
+        problem = self.build_problem(script=script, cfn="check_func",
                                     expect="42", num_inputs=2)
 
         # Correct answer -- expect both inputs marked correct
@@ -764,10 +773,10 @@ class CustomResponseTest(ResponseTest):
 
         # If the <customresponse> has multiple inputs associated with it,
         # the check function can return a dict of the form:
-        # 
+        #
         # {'overall_message': STRING,
         #  'input_list': [{'ok': BOOL, 'msg': STRING}, ...] }
-        # 
+        #
         # 'overall_message' is displayed at the end of the response
         #
         # 'input_list' contains dictionaries representing the correctness
@@ -784,7 +793,7 @@ class CustomResponseTest(ResponseTest):
                             {'ok': check3,  'msg': 'Feedback 3'} ] }
             """)
 
-        problem = self.build_problem(script=script, 
+        problem = self.build_problem(script=script,
                                     cfn="check_func", num_inputs=3)
 
         # Grade the inputs (one input incorrect)
@@ -821,11 +830,11 @@ class CustomResponseTest(ResponseTest):
                 check1 = (int(answer_given[0]) == 1)
                 check2 = (int(answer_given[1]) == 2)
                 check3 = (int(answer_given[2]) == 3)
-                return {'ok': (check1 and check2 and check3),  
+                return {'ok': (check1 and check2 and check3),
                         'msg': 'Message text'}
             """)
 
-        problem = self.build_problem(script=script, 
+        problem = self.build_problem(script=script,
                                     cfn="check_func", num_inputs=3)
 
         # Grade the inputs (one input incorrect)
@@ -862,7 +871,7 @@ class CustomResponseTest(ResponseTest):
         # Expect that an exception gets raised when we check the answer
         with self.assertRaises(Exception):
             problem.grade_answers({'1_2_1': '42'})
-    
+
     def test_invalid_dict_exception(self):
 
         # Construct a script that passes back an invalid dict format

From 53f85f2a07438fe84cfc6cda77537419a619a7fd Mon Sep 17 00:00:00 2001
From: Brian Wilson <brian@edx.org>
Date: Mon, 11 Mar 2013 13:52:43 -0400
Subject: [PATCH 17/27] return empty list if no html or pdf textbook appears in
 policy.json

---
 common/lib/xmodule/xmodule/course_module.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py
index 3923d3f056..1c9928a502 100644
--- a/common/lib/xmodule/xmodule/course_module.py
+++ b/common/lib/xmodule/xmodule/course_module.py
@@ -356,14 +356,14 @@ class CourseDescriptor(SequenceDescriptor):
         """
         Return the pdf_textbooks config, as a python object, or None if not specified.
         """
-        return self.metadata.get('pdf_textbooks')
+        return self.metadata.get('pdf_textbooks', [])
 
     @property
     def html_textbooks(self):
         """
         Return the html_textbooks config, as a python object, or None if not specified.
         """
-        return self.metadata.get('html_textbooks')
+        return self.metadata.get('html_textbooks', [])
 
     @tabs.setter
     def tabs(self, value):

From 78c8358957d29b59a0972511f9c56db3ebfd0fac Mon Sep 17 00:00:00 2001
From: Chris Dodge <cdodge@edx.org>
Date: Mon, 11 Mar 2013 14:51:44 -0400
Subject: [PATCH 18/27] clean up pyling violations from views.py

---
 cms/djangoapps/contentstore/views.py | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py
index 34003d71a4..a4a3b620c4 100644
--- a/cms/djangoapps/contentstore/views.py
+++ b/cms/djangoapps/contentstore/views.py
@@ -86,12 +86,14 @@ def signup(request):
     csrf_token = csrf(request)['csrf_token']
     return render_to_response('signup.html', {'csrf': csrf_token})
 
+
 def old_login_redirect(request):
     '''
     Redirect to the active login url.
     '''
     return redirect('login', permanent=True)
 
+
 @ssl_login_shortcut
 @ensure_csrf_cookie
 def login_page(request):
@@ -104,6 +106,7 @@ def login_page(request):
         'forgot_password_link': "//{base}/#forgot-password-modal".format(base=settings.LMS_BASE),
     })
 
+
 def howitworks(request):
     if request.user.is_authenticated():
         return index(request)
@@ -112,6 +115,7 @@ def howitworks(request):
 
 # ==== Views for any logged-in user ==================================
 
+
 @login_required
 @ensure_csrf_cookie
 def index(request):
@@ -145,6 +149,7 @@ def index(request):
 
 # ==== Views with per-item permissions================================
 
+
 def has_access(user, location, role=STAFF_ROLE_NAME):
     '''
     Return True if user allowed to access this piece of data
@@ -393,6 +398,7 @@ def preview_component(request, location):
         'editor': wrap_xmodule(component.get_html, component, 'xmodule_edit.html')(),
     })
 
+
 @expect_json
 @login_required
 @ensure_csrf_cookie
@@ -709,6 +715,7 @@ def create_draft(request):
 
     return HttpResponse()
 
+
 @login_required
 @expect_json
 def publish_draft(request):
@@ -738,6 +745,7 @@ def unpublish_unit(request):
 
     return HttpResponse()
 
+
 @login_required
 @expect_json
 def clone_item(request):
@@ -768,8 +776,7 @@ def clone_item(request):
 
     return HttpResponse(json.dumps({'id': dest_location.url()}))
 
-#@login_required
-#@ensure_csrf_cookie
+
 def upload_asset(request, org, course, coursename):
     '''
     cdodge: this method allows for POST uploading of files into the course asset library, which will
@@ -831,6 +838,7 @@ def upload_asset(request, org, course, coursename):
     response['asset_url'] = StaticContent.get_url_path_from_location(content.location)
     return response
 
+
 '''
 This view will return all CMS users who are editors for the specified course
 '''
@@ -863,6 +871,7 @@ def create_json_response(errmsg = None):
 
     return resp
 
+
 '''
 This POST-back view will add a user - specified by email - to the list of editors for
 the specified course
@@ -895,6 +904,7 @@ def add_user(request, location):
 
     return create_json_response()
 
+
 '''
 This POST-back view will remove a user - specified by email - from the list of editors for
 the specified course
@@ -926,6 +936,7 @@ def remove_user(request, location):
 def landing(request, org, course, coursename):
     return render_to_response('temp-course-landing.html', {})
 
+
 @login_required
 @ensure_csrf_cookie
 def static_pages(request, org, course, coursename):
@@ -1029,6 +1040,7 @@ def edit_tabs(request, org, course, coursename):
         'components': components
         })
 
+
 def not_found(request):
     return render_to_response('error.html', {'error': '404'})
 
@@ -1064,6 +1076,7 @@ def course_info(request, org, course, name, provided_id=None):
         'handouts_location': Location(['i4x', org, course, 'course_info', 'handouts']).url()
     })
 
+
 @expect_json
 @login_required
 @ensure_csrf_cookie
@@ -1161,6 +1174,7 @@ def get_course_settings(request, org, course, name):
                                        "section": "details"})
     })
 
+
 @login_required
 @ensure_csrf_cookie
 def course_config_graders_page(request, org, course, name):
@@ -1184,6 +1198,7 @@ def course_config_graders_page(request, org, course, name):
         'course_details': json.dumps(course_details, cls=CourseSettingsEncoder)
     })
 
+
 @login_required
 @ensure_csrf_cookie
 def course_config_advanced_page(request, org, course, name):
@@ -1207,6 +1222,7 @@ def course_config_advanced_page(request, org, course, name):
         'advanced_dict' : json.dumps(CourseMetadata.fetch(location)),
     })
 
+
 @expect_json
 @login_required
 @ensure_csrf_cookie
@@ -1238,6 +1254,7 @@ def course_settings_updates(request, org, course, name, section):
         return HttpResponse(json.dumps(manager.update_from_json(request.POST), cls=CourseSettingsEncoder),
                             mimetype="application/json")
 
+
 @expect_json
 @login_required
 @ensure_csrf_cookie
@@ -1272,7 +1289,7 @@ def course_grader_updates(request, org, course, name, grader_index=None):
         return HttpResponse(json.dumps(CourseGradingModel.update_grader_from_json(Location(['i4x', org, course, 'course', name]), request.POST)),
                             mimetype="application/json")
 
-        
+
 ## NB: expect_json failed on ["key", "key2"] and json payload
 @login_required
 @ensure_csrf_cookie
@@ -1363,6 +1380,7 @@ def asset_index(request, org, course, name):
 def edge(request):
     return render_to_response('university_profiles/edge.html', {})
 
+
 @login_required
 @expect_json
 def create_new_course(request):
@@ -1418,6 +1436,7 @@ def create_new_course(request):
 
     return HttpResponse(json.dumps({'id': new_course.location.url()}))
 
+
 def initialize_course_tabs(course):
     # set up the default tabs
     # I've added this because when we add static tabs, the LMS either expects a None for the tabs list or
@@ -1435,6 +1454,7 @@ def initialize_course_tabs(course):
 
     modulestore('direct').update_metadata(course.location.url(), course.own_metadata)
 
+
 @ensure_csrf_cookie
 @login_required
 def import_course(request, org, course, name):
@@ -1512,6 +1532,7 @@ def import_course(request, org, course, name):
                         course_module.location.name])
         })
 
+
 @ensure_csrf_cookie
 @login_required
 def generate_export_course(request, org, course, name):
@@ -1563,6 +1584,7 @@ def export_course(request, org, course, name):
         'successful_import_redirect_url': ''
     })
 
+
 def event(request):
     '''
     A noop to swallow the analytics call so that cms methods don't spook and poor developers looking at

From 26f442458168d1b1247046fc0357a4fdf56d3861 Mon Sep 17 00:00:00 2001
From: Chris Dodge <cdodge@edx.org>
Date: Mon, 11 Mar 2013 14:55:37 -0400
Subject: [PATCH 19/27] clean up some pylint violations

---
 cms/djangoapps/contentstore/tests/test_contentstore.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py
index c0ab9ec60e..b62f691818 100644
--- a/cms/djangoapps/contentstore/tests/test_contentstore.py
+++ b/cms/djangoapps/contentstore/tests/test_contentstore.py
@@ -63,7 +63,6 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         self.client = Client()
         self.client.login(username=uname, password=password)
 
-
     def check_edit_unit(self, test_course_name):
         import_from_xml(modulestore(), 'common/test/data/', [test_course_name])
 

From 4ec3683c3c0bf159bf64a0b7b8c992e4febb8a47 Mon Sep 17 00:00:00 2001
From: Chris Dodge <cdodge@edx.org>
Date: Mon, 11 Mar 2013 14:57:04 -0400
Subject: [PATCH 20/27] clean up some pylint violations

---
 .../models/settings/course_metadata.py        | 26 +++++++++----------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py
index d088d75665..24245a39d5 100644
--- a/cms/djangoapps/models/settings/course_metadata.py
+++ b/cms/djangoapps/models/settings/course_metadata.py
@@ -10,7 +10,7 @@ class CourseMetadata(object):
     '''
     # __new_advanced_key__ is used by client not server; so, could argue against it being here
     FILTERED_LIST = XModuleDescriptor.system_metadata_fields + ['start', 'end', 'enrollment_start', 'enrollment_end', 'tabs', 'graceperiod', '__new_advanced_key__']
-    
+
     @classmethod
     def fetch(cls, course_location):
         """
@@ -18,17 +18,17 @@ class CourseMetadata(object):
         """
         if not isinstance(course_location, Location):
             course_location = Location(course_location)
-            
+
         course = {}
-        
+
         descriptor = get_modulestore(course_location).get_item(course_location)
-        
+
         for k, v in descriptor.metadata.iteritems():
             if k not in cls.FILTERED_LIST:
                 course[k] = v
-                
+    
         return course
-        
+
     @classmethod
     def update_from_json(cls, course_location, jsondict):
         """
@@ -37,7 +37,7 @@ class CourseMetadata(object):
         Ensures none of the fields are in the blacklist.
         """
         descriptor = get_modulestore(course_location).get_item(course_location)
-        
+
         dirty = False
 
         for k, v in jsondict.iteritems():
@@ -45,26 +45,26 @@ class CourseMetadata(object):
             if k not in cls.FILTERED_LIST and (k not in descriptor.metadata or descriptor.metadata[k] != v):
                 dirty = True
                 descriptor.metadata[k] = v
-            
+
         if dirty:
             get_modulestore(course_location).update_metadata(course_location, descriptor.metadata)
-            
+
         # 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 cls.fetch(course_location)
-    
+
     @classmethod
     def delete_key(cls, course_location, payload):
         '''
         Remove the given metadata key(s) from the course. payload can be a single key or [key..]
         '''
         descriptor = get_modulestore(course_location).get_item(course_location)
-        
+
         for key in payload['deleteKeys']:
             if key in descriptor.metadata:
                 del descriptor.metadata[key]
-    
+
         get_modulestore(course_location).update_metadata(course_location, descriptor.metadata)
-        
+
         return cls.fetch(course_location)
         
\ No newline at end of file

From 6e92666a203ed9ec149a44458c24e6d3d1a6be97 Mon Sep 17 00:00:00 2001
From: Julian Arni <julian@edx.org>
Date: Mon, 11 Mar 2013 14:57:19 -0400
Subject: [PATCH 21/27] Fixed score order.

  Less is more.
---
 lms/djangoapps/foldit/tests.py | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/lms/djangoapps/foldit/tests.py b/lms/djangoapps/foldit/tests.py
index 7127651601..afdd678f06 100644
--- a/lms/djangoapps/foldit/tests.py
+++ b/lms/djangoapps/foldit/tests.py
@@ -143,11 +143,12 @@ class FolditTestCase(TestCase):
     def test_SetPlayerPuzzleScores_manyplayers(self):
         """
         Check that when we send scores from multiple users, the correct order
-        of scores is displayed.
+        of scores is displayed. Note that, before being processed by
+        display_score, lower scores are better.
         """
         puzzle_id = ['1']
-        player1_score = 0.07
-        player2_score = 0.08
+        player1_score = 0.08
+        player2_score = 0.02
         response1 = self.make_puzzle_score_request(puzzle_id, player1_score,
                 self.user)
 
@@ -164,8 +165,12 @@ class FolditTestCase(TestCase):
         self.assertEqual(len(top_10), 2)
 
         # Top score should be player2_score. Second should be player1_score
-        self.assertEqual(top_10[0]['score'], Score.display_score(player2_score))
-        self.assertEqual(top_10[1]['score'], Score.display_score(player1_score))
+        self.assertAlmostEqual(top_10[0]['score'],
+                Score.display_score(player2_score),
+                delta=0.5)
+        self.assertAlmostEqual(top_10[1]['score'],
+                Score.display_score(player1_score),
+                delta=0.5)
 
         # Top score user should be self.user2.username
         self.assertEqual(top_10[0]['username'], self.user2.username)

From 1c98d5fc8d095d3b0ef9594cbacf20225e76ec1c Mon Sep 17 00:00:00 2001
From: Jay Zoldak <jzoldak@edx.org>
Date: Mon, 11 Mar 2013 15:16:09 -0400
Subject: [PATCH 22/27] Fix modulestore tests for pep8 violations

---
 .../xmodule/modulestore/tests/factories.py    |  8 +++----
 .../modulestore/tests/test_location.py        | 22 +++++++++----------
 .../modulestore/tests/test_modulestore.py     |  4 ++--
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py
index 1259da2690..f2a291d680 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py
@@ -46,10 +46,10 @@ class XModuleCourseFactory(Factory):
         new_course.metadata['start'] = stringify_time(gmtime())
 
         new_course.tabs = [{"type": "courseware"},
-            {"type": "course_info", "name": "Course Info"}, 
-            {"type": "discussion", "name": "Discussion"},
-            {"type": "wiki", "name": "Wiki"},
-            {"type": "progress", "name": "Progress"}]
+                           {"type": "course_info", "name": "Course Info"},
+                           {"type": "discussion", "name": "Discussion"},
+                           {"type": "wiki", "name": "Wiki"},
+                           {"type": "progress", "name": "Progress"}]
 
         # Update the data in the mongo datastore
         store.update_metadata(new_course.location.url(), new_course.own_metadata)
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_location.py b/common/lib/xmodule/xmodule/modulestore/tests/test_location.py
index 0772951884..f0f0e8bf48 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_location.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_location.py
@@ -119,11 +119,11 @@ def test_equality():
 
 # All the cleaning functions should do the same thing with these
 general_pairs = [('', ''),
-                  (' ', '_'),
-                  ('abc,', 'abc_'),
-                  ('ab    fg!@//\\aj', 'ab_fg_aj'),
-                  (u"ab\xA9", "ab_"),  # no unicode allowed for now
-                  ]
+                 (' ', '_'),
+                 ('abc,', 'abc_'),
+                 ('ab    fg!@//\\aj', 'ab_fg_aj'),
+                 (u"ab\xA9", "ab_"),  # no unicode allowed for now
+                 ]
 
 
 def test_clean():
@@ -131,7 +131,7 @@ def test_clean():
         ('a:b', 'a_b'),  # no colons in non-name components
         ('a-b', 'a-b'),  # dashes ok
         ('a.b', 'a.b'),  # dot ok
-        ]
+    ]
     for input, output in pairs:
         assert_equals(Location.clean(input), output)
 
@@ -141,17 +141,17 @@ def test_clean_for_url_name():
         ('a:b', 'a:b'),  # colons ok in names
         ('a-b', 'a-b'),  # dashes ok in names
         ('a.b', 'a.b'),  # dot ok in names
-        ]
+    ]
     for input, output in pairs:
         assert_equals(Location.clean_for_url_name(input), output)
 
 
 def test_clean_for_html():
     pairs = general_pairs + [
-              ("a:b", "a_b"),   # no colons for html use
-              ("a-b", "a-b"),   # dashes ok (though need to be replaced in various use locations. ugh.)
-              ('a.b', 'a_b'),   # no dots.
-              ]
+        ("a:b", "a_b"),   # no colons for html use
+        ("a-b", "a-b"),   # dashes ok (though need to be replaced in various use locations. ugh.)
+        ('a.b', 'a_b'),   # no dots.
+    ]
     for input, output in pairs:
         assert_equals(Location.clean_for_html(input), output)
 
diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore.py
index 94ea622907..469eedac05 100644
--- a/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore.py
+++ b/common/lib/xmodule/xmodule/modulestore/tests/test_modulestore.py
@@ -12,7 +12,7 @@ def check_path_to_location(modulestore):
          ("edX/toy/2012_Fall", "Overview", "Welcome", None)),
         ("i4x://edX/toy/chapter/Overview",
          ("edX/toy/2012_Fall", "Overview", None, None)),
-        )
+    )
     course_id = "edX/toy/2012_Fall"
 
     for location, expected in should_work:
@@ -20,6 +20,6 @@ def check_path_to_location(modulestore):
 
     not_found = (
         "i4x://edX/toy/video/WelcomeX", "i4x://edX/toy/course/NotHome"
-        )
+    )
     for location in not_found:
         assert_raises(ItemNotFoundError, path_to_location, modulestore, course_id, location)

From cc2c26b9240271eb79e334b4c2768ab7684dc592 Mon Sep 17 00:00:00 2001
From: John Hess <mgojohn@gmail.com>
Date: Mon, 11 Mar 2013 18:04:21 -0400
Subject: [PATCH 23/27] reverted change to definition to/from xml

---
 common/lib/xmodule/xmodule/foldit_module.py | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/common/lib/xmodule/xmodule/foldit_module.py b/common/lib/xmodule/xmodule/foldit_module.py
index 2bcdda5bc2..65632fa031 100644
--- a/common/lib/xmodule/xmodule/foldit_module.py
+++ b/common/lib/xmodule/xmodule/foldit_module.py
@@ -176,10 +176,9 @@ class FolditDescriptor(XmlDescriptor, EditingDescriptor):
 
     @classmethod
     def definition_from_xml(cls, xml_object, system):
-        """ Get the xml_object's attributes.  """
+        """ 
+        Get the xml_object's attributes.  
+        """
 
         return {'metadata': xml_object.attrib}
 
-    def definition_to_xml(self):
-        xml_object = etree.Element('foldit')
-        return xml_object

From 00c35c95a4140741da9669c7d01965cba98a7170 Mon Sep 17 00:00:00 2001
From: John Hess <mgojohn@gmail.com>
Date: Mon, 11 Mar 2013 18:05:10 -0400
Subject: [PATCH 24/27] whitespace back to original

---
 common/lib/xmodule/xmodule/foldit_module.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/common/lib/xmodule/xmodule/foldit_module.py b/common/lib/xmodule/xmodule/foldit_module.py
index 65632fa031..152930876d 100644
--- a/common/lib/xmodule/xmodule/foldit_module.py
+++ b/common/lib/xmodule/xmodule/foldit_module.py
@@ -176,9 +176,7 @@ class FolditDescriptor(XmlDescriptor, EditingDescriptor):
 
     @classmethod
     def definition_from_xml(cls, xml_object, system):
-        """ 
+        """
         Get the xml_object's attributes.  
         """
-
         return {'metadata': xml_object.attrib}
-

From ad92eb4d022da3c7aba6df37929905621b2d4610 Mon Sep 17 00:00:00 2001
From: John Hess <mgojohn@gmail.com>
Date: Mon, 11 Mar 2013 18:05:34 -0400
Subject: [PATCH 25/27] whitespace back to original. for real though.

---
 common/lib/xmodule/xmodule/foldit_module.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/lib/xmodule/xmodule/foldit_module.py b/common/lib/xmodule/xmodule/foldit_module.py
index 152930876d..37255bd5cb 100644
--- a/common/lib/xmodule/xmodule/foldit_module.py
+++ b/common/lib/xmodule/xmodule/foldit_module.py
@@ -177,6 +177,6 @@ class FolditDescriptor(XmlDescriptor, EditingDescriptor):
     @classmethod
     def definition_from_xml(cls, xml_object, system):
         """
-        Get the xml_object's attributes.  
+        Get the xml_object's attributes.
         """
         return {'metadata': xml_object.attrib}

From bf0e5a270183b90becc56ec7f709deef7bb26fd6 Mon Sep 17 00:00:00 2001
From: Chris Dodge <cdodge@edx.org>
Date: Mon, 11 Mar 2013 21:14:24 -0400
Subject: [PATCH 26/27] do a bunch of pylint improvements

---
 .../contentstore/tests/test_contentstore.py   | 159 +++++++++---------
 1 file changed, 78 insertions(+), 81 deletions(-)

diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py
index 58197b9762..99ef1169b1 100644
--- a/cms/djangoapps/contentstore/tests/test_contentstore.py
+++ b/cms/djangoapps/contentstore/tests/test_contentstore.py
@@ -9,10 +9,8 @@ from tempdir import mkdtemp_clean
 import json
 from fs.osfs import OSFS
 import copy
-from mock import Mock
-from json import dumps, loads
+from json import loads
 
-from student.models import Registration
 from django.contrib.auth.models import User
 from cms.djangoapps.contentstore.utils import get_modulestore
 
@@ -22,12 +20,11 @@ from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 from xmodule.modulestore import Location
 from xmodule.modulestore.store_utilities import clone_course
 from xmodule.modulestore.store_utilities import delete_course
-from xmodule.modulestore.django import modulestore, _MODULESTORES
+from xmodule.modulestore.django import modulestore
 from xmodule.contentstore.django import contentstore
 from xmodule.templates import update_templates
 from xmodule.modulestore.xml_exporter import export_to_xml
 from xmodule.modulestore.xml_importer import import_from_xml
-from xmodule.templates import update_templates
 
 from xmodule.capa_module import CapaDescriptor
 from xmodule.course_module import CourseDescriptor
@@ -81,8 +78,8 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
     def test_static_tab_reordering(self):
         import_from_xml(modulestore(), 'common/test/data/', ['full'])
 
-        ms = modulestore('direct')
-        course = ms.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None]))
+        module_store = modulestore('direct')
+        course = module_store.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None]))
 
         # reverse the ordering
         reverse_tabs = []
@@ -90,9 +87,9 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
             if tab['type'] == 'static_tab':
                 reverse_tabs.insert(0, 'i4x://edX/full/static_tab/{0}'.format(tab['url_slug']))
 
-        resp = self.client.post(reverse('reorder_static_tabs'), json.dumps({'tabs': reverse_tabs}), "application/json")
+        self.client.post(reverse('reorder_static_tabs'), json.dumps({'tabs': reverse_tabs}), "application/json")
 
-        course = ms.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None]))
+        course = module_store.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None]))
 
         # compare to make sure that the tabs information is in the expected order after the server call
         course_tabs = []
@@ -105,28 +102,29 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
     def test_delete(self):
         import_from_xml(modulestore(), 'common/test/data/', ['full'])
 
-        ms = modulestore('direct')
-        course = ms.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None]))
+        module_store = modulestore('direct')
 
-        sequential = ms.get_item(Location(['i4x', 'edX', 'full', 'sequential','Administrivia_and_Circuit_Elements', None]))
+        sequential = module_store.get_item(Location(['i4x', 'edX', 'full', 'sequential', 'Administrivia_and_Circuit_Elements', None]))
 
-        chapter = ms.get_item(Location(['i4x', 'edX', 'full', 'chapter','Week_1', None]))
+        chapter = module_store.get_item(Location(['i4x', 'edX', 'full', 'chapter','Week_1', None]))
 
         # make sure the parent no longer points to the child object which was deleted
         self.assertTrue(sequential.location.url() in chapter.definition['children'])
 
-        resp = self.client.post(reverse('delete_item'), json.dumps({'id': sequential.location.url(), 'delete_children':'true'}), "application/json")
+        self.client.post(reverse('delete_item'), 
+            json.dumps({'id': sequential.location.url(), 'delete_children':'true'}), 
+            "application/json")
 
-        bFound = False
+        found = False
         try:
-            sequential = ms.get_item(Location(['i4x', 'edX', 'full', 'sequential','Administrivia_and_Circuit_Elements', None]))
-            bFound = True
+            module_store.get_item(Location(['i4x', 'edX', 'full', 'sequential', 'Administrivia_and_Circuit_Elements', None]))
+            found = True
         except ItemNotFoundError:
             pass
 
-        self.assertFalse(bFound)
+        self.assertFalse(found)
 
-        chapter = ms.get_item(Location(['i4x', 'edX', 'full', 'chapter','Week_1', None]))
+        chapter = module_store.get_item(Location(['i4x', 'edX', 'full', 'chapter','Week_1', None]))
 
         # make sure the parent no longer points to the child object which was deleted
         self.assertFalse(sequential.location.url() in chapter.definition['children'])
@@ -139,22 +137,22 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         while there is a base definition in /about/effort.html
         '''
         import_from_xml(modulestore(), 'common/test/data/', ['full'])
-        ms = modulestore('direct')
-        effort = ms.get_item(Location(['i4x', 'edX', 'full', 'about', 'effort', None]))
+        module_store = modulestore('direct')
+        effort = module_store.get_item(Location(['i4x', 'edX', 'full', 'about', 'effort', None]))
         self.assertEqual(effort.definition['data'], '6 hours')
 
         # this one should be in a non-override folder
-        effort = ms.get_item(Location(['i4x', 'edX', 'full', 'about', 'end_date', None]))
+        effort = module_store.get_item(Location(['i4x', 'edX', 'full', 'about', 'end_date', None]))
         self.assertEqual(effort.definition['data'], 'TBD')
 
     def test_remove_hide_progress_tab(self):
         import_from_xml(modulestore(), 'common/test/data/', ['full'])
 
-        ms = modulestore('direct')
-        cs = contentstore()
+        module_store = modulestore('direct')
+        content_store = contentstore()
 
         source_location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012')
-        course = ms.get_item(source_location)
+        course = module_store.get_item(source_location)
         self.assertNotIn('hide_progress_tab', course.metadata)
 
     def test_clone_course(self):
@@ -173,19 +171,19 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         data = parse_json(resp)
         self.assertEqual(data['id'], 'i4x://MITx/999/course/Robot_Super_Course')
 
-        ms = modulestore('direct')
-        cs = contentstore()
+        module_store = modulestore('direct')
+        content_store = contentstore()
 
         source_location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012')
         dest_location = CourseDescriptor.id_to_location('MITx/999/Robot_Super_Course')
 
-        clone_course(ms, cs, source_location, dest_location)
+        clone_course(module_store, content_store, source_location, dest_location)
 
         # now loop through all the units in the course and verify that the clone can render them, which
         # means the objects are at least present
-        items = ms.get_items(Location(['i4x', 'edX', 'full', 'vertical', None]))
+        items = module_store.get_items(Location(['i4x', 'edX', 'full', 'vertical', None]))
         self.assertGreater(len(items), 0)
-        clone_items = ms.get_items(Location(['i4x', 'MITx', '999', 'vertical', None]))
+        clone_items = module_store.get_items(Location(['i4x', 'MITx', '999', 'vertical', None]))
         self.assertGreater(len(clone_items), 0)
         for descriptor in items:
             new_loc = descriptor.location._replace(org='MITx', course='999')
@@ -196,14 +194,14 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
     def test_delete_course(self):
         import_from_xml(modulestore(), 'common/test/data/', ['full'])
 
-        ms = modulestore('direct')
-        cs = contentstore()
+        module_store = modulestore('direct')
+        content_store = contentstore()
 
         location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012')
 
-        delete_course(ms, cs, location, commit=True)
+        delete_course(module_store, content_store, location, commit=True)
 
-        items = ms.get_items(Location(['i4x', 'edX', 'full', 'vertical', None]))
+        items = module_store.get_items(Location(['i4x', 'edX', 'full', 'vertical', None]))
         self.assertEqual(len(items), 0)
 
     def verify_content_existence(self, modulestore, root_dir, location, dirname, category_name, filename_suffix=''):
@@ -218,10 +216,10 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
             self.assertTrue(fs.exists(item.location.name + filename_suffix))
 
     def test_export_course(self):
-        ms = modulestore('direct')
-        cs = contentstore()
+        module_store = modulestore('direct')
+        content_store = contentstore()
 
-        import_from_xml(ms, 'common/test/data/', ['full'])
+        import_from_xml(module_store, 'common/test/data/', ['full'])
         location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012')
 
         root_dir = path(mkdtemp_clean())
@@ -229,43 +227,43 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         print 'Exporting to tempdir = {0}'.format(root_dir)
 
         # export out to a tempdir
-        export_to_xml(ms, cs, location, root_dir, 'test_export')
+        export_to_xml(module_store, content_store, location, root_dir, 'test_export')
 
         # check for static tabs
-        self.verify_content_existence(ms, root_dir, location, 'tabs', 'static_tab', '.html')
+        self.verify_content_existence(module_store, root_dir, location, 'tabs', 'static_tab', '.html')
 
         # check for custom_tags
-        self.verify_content_existence(ms, root_dir, location, 'info', 'course_info', '.html')
+        self.verify_content_existence(module_store, root_dir, location, 'info', 'course_info', '.html')
 
         # check for custom_tags
-        self.verify_content_existence(ms, root_dir, location, 'custom_tags', 'custom_tag_template')
+        self.verify_content_existence(module_store, root_dir, location, 'custom_tags', 'custom_tag_template')
 
         # check for graiding_policy.json
         fs = OSFS(root_dir / 'test_export/policies/6.002_Spring_2012')
         self.assertTrue(fs.exists('grading_policy.json'))
 
-        course = ms.get_item(location)
+        course = module_store.get_item(location)
         # compare what's on disk compared to what we have in our course
-        with fs.open('grading_policy.json','r') as grading_policy:
-            on_disk = loads(grading_policy.read())    
+        with fs.open('grading_policy.json', 'r') as grading_policy:
+            on_disk = loads(grading_policy.read())
             self.assertEqual(on_disk, course.definition['data']['grading_policy'])
 
         #check for policy.json
         self.assertTrue(fs.exists('policy.json'))
 
         # compare what's on disk to what we have in the course module
-        with fs.open('policy.json','r') as course_policy:
+        with fs.open('policy.json', 'r') as course_policy:
             on_disk = loads(course_policy.read())
             self.assertIn('course/6.002_Spring_2012', on_disk)
             self.assertEqual(on_disk['course/6.002_Spring_2012'], course.metadata)
 
         # remove old course
-        delete_course(ms, cs, location)
+        delete_course(module_store, content_store, location)
 
         # reimport
-        import_from_xml(ms, root_dir, ['test_export'])
+        import_from_xml(module_store, root_dir, ['test_export'])
 
-        items = ms.get_items(Location(['i4x', 'edX', 'full', 'vertical', None]))
+        items = module_store.get_items(Location(['i4x', 'edX', 'full', 'vertical', None]))
         self.assertGreater(len(items), 0)
         for descriptor in items:
             print "Checking {0}....".format(descriptor.location.url())
@@ -275,11 +273,11 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         shutil.rmtree(root_dir)
 
     def test_course_handouts_rewrites(self):
-        ms = modulestore('direct')
-        cs = contentstore()
+        module_store = modulestore('direct')
+        content_store = contentstore()
 
         # import a test course
-        import_from_xml(ms, 'common/test/data/', ['full'])
+        import_from_xml(module_store, 'common/test/data/', ['full'])
 
         handout_location = Location(['i4x', 'edX', 'full', 'course_info', 'handouts'])
 
@@ -294,32 +292,32 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         self.assertContains(resp, '/c4x/edX/full/asset/handouts_schematic_tutorial.pdf')
 
     def test_export_course_with_unknown_metadata(self):
-        ms = modulestore('direct')
-        cs = contentstore()
+        module_store = modulestore('direct')
+        content_store = contentstore()
 
-        import_from_xml(ms, 'common/test/data/', ['full'])
+        import_from_xml(module_store, 'common/test/data/', ['full'])
         location = CourseDescriptor.id_to_location('edX/full/6.002_Spring_2012')
 
         root_dir = path(mkdtemp_clean())
 
-        course = ms.get_item(location)
+        course = module_store.get_item(location)
 
         # add a bool piece of unknown metadata so we can verify we don't throw an exception
         course.metadata['new_metadata'] = True
 
-        ms.update_metadata(location, course.metadata)
+        module_store.update_metadata(location, course.metadata)
 
         print 'Exporting to tempdir = {0}'.format(root_dir)
 
         # export out to a tempdir
-        bExported = False
+        exported = False
         try:
-            export_to_xml(ms, cs, location, root_dir, 'test_export')
-            bExported = True
+            export_to_xml(module_store, content_store, location, root_dir, 'test_export')
+            exported = True
         except Exception:
             pass
 
-        self.assertTrue(bExported)
+        self.assertTrue(exported)
 
 class ContentStoreTest(ModuleStoreTestCase):
     """
@@ -458,7 +456,7 @@ class ContentStoreTest(ModuleStoreTestCase):
 
     def test_capa_module(self):
         """Test that a problem treats markdown specially."""
-        course = CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course')
+        CourseFactory.create(org='MITx', course='999', display_name='Robot Super Course')
 
         problem_data = {
             'parent_location': 'i4x://MITx/999/course/Robot_Super_Course',
@@ -480,10 +478,10 @@ class ContentStoreTest(ModuleStoreTestCase):
 
     def test_import_metadata_with_attempts_empty_string(self):
         import_from_xml(modulestore(), 'common/test/data/', ['simple'])
-        ms = modulestore('direct')
+        module_store = modulestore('direct')
         did_load_item = False
         try:       
-            ms.get_item(Location(['i4x', 'edX', 'simple', 'problem', 'ps01-simple', None]))
+            module_store.get_item(Location(['i4x', 'edX', 'simple', 'problem', 'ps01-simple', None]))
             did_load_item = True
         except ItemNotFoundError:
             pass
@@ -494,10 +492,10 @@ class ContentStoreTest(ModuleStoreTestCase):
     def test_metadata_inheritance(self):
         import_from_xml(modulestore(), 'common/test/data/', ['full'])
 
-        ms = modulestore('direct')
-        course = ms.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None]))
+        module_store = modulestore('direct')
+        course = module_store.get_item(Location(['i4x', 'edX', 'full', 'course', '6.002_Spring_2012', None]))
 
-        verticals = ms.get_items(['i4x', 'edX', 'full', 'vertical', None, None])
+        verticals = module_store.get_items(['i4x', 'edX', 'full', 'vertical', None, None])
 
         # let's assert on the metadata_inheritance on an existing vertical
         for vertical in verticals:
@@ -508,15 +506,15 @@ class ContentStoreTest(ModuleStoreTestCase):
 
         new_component_location = Location('i4x', 'edX', 'full', 'html', 'new_component')
         source_template_location = Location('i4x', 'edx', 'templates', 'html', 'Blank_HTML_Page')
-        
+
         # crate a new module and add it as a child to a vertical
-        ms.clone_item(source_template_location, new_component_location)
+        module_store.clone_item(source_template_location, new_component_location)
         parent = verticals[0]
-        ms.update_children(parent.location, parent.definition.get('children', []) + [new_component_location.url()])
+        module_store.update_children(parent.location, parent.definition.get('children', []) + [new_component_location.url()])
 
         # flush the cache
-        ms.get_cached_metadata_inheritance_tree(new_component_location, -1)
-        new_module = ms.get_item(new_component_location)
+        module_store.get_cached_metadata_inheritance_tree(new_component_location, -1)
+        new_module = module_store.get_item(new_component_location)
 
         # check for grace period definition which should be defined at the course level
         self.assertIn('graceperiod', new_module.metadata)
@@ -529,11 +527,11 @@ class ContentStoreTest(ModuleStoreTestCase):
         # now let's define an override at the leaf node level
         #
         new_module.metadata['graceperiod'] = '1 day'
-        ms.update_metadata(new_module.location, new_module.metadata)
+        module_store.update_metadata(new_module.location, new_module.metadata)
 
         # flush the cache and refetch
-        ms.get_cached_metadata_inheritance_tree(new_component_location, -1)
-        new_module = ms.get_item(new_component_location)
+        module_store.get_cached_metadata_inheritance_tree(new_component_location, -1)
+        new_module = module_store.get_item(new_component_location)
 
         self.assertIn('graceperiod', new_module.metadata)
         self.assertEqual('1 day', new_module.metadata['graceperiod'])
@@ -542,15 +540,15 @@ class ContentStoreTest(ModuleStoreTestCase):
 class TemplateTestCase(ModuleStoreTestCase):
 
     def test_template_cleanup(self):        
-        ms = modulestore('direct')
+        module_store = modulestore('direct')
 
         # insert a bogus template in the store
         bogus_template_location = Location('i4x', 'edx', 'templates', 'html', 'bogus')
         source_template_location = Location('i4x', 'edx', 'templates', 'html', 'Blank_HTML_Page')
-        
-        ms.clone_item(source_template_location, bogus_template_location)
 
-        verify_create = ms.get_item(bogus_template_location)
+        module_store.clone_item(source_template_location, bogus_template_location)
+
+        verify_create = module_store.get_item(bogus_template_location)
         self.assertIsNotNone(verify_create)
 
         # now run cleanup
@@ -559,10 +557,9 @@ class TemplateTestCase(ModuleStoreTestCase):
         # now try to find dangling template, it should not be in DB any longer
         asserted = False
         try:
-            verify_create = ms.get_item(bogus_template_location)
+            verify_create = module_store.get_item(bogus_template_location)
         except ItemNotFoundError:
             asserted = True
 
-        self.assertTrue(asserted)     
-
+        self.assertTrue(asserted)
 

From 40f134ed4de99e99a76fcb088dddea55ff1c4484 Mon Sep 17 00:00:00 2001
From: Calen Pennington <calen.pennington@gmail.com>
Date: Mon, 11 Mar 2013 22:23:23 -0400
Subject: [PATCH 27/27] Don't use mutable defaults for fields

---
 common/lib/xmodule/xmodule/course_module.py | 20 ++++++++++----------
 local-requirements.txt                      |  2 +-
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py
index 55e041e8f6..f3634d81b9 100644
--- a/common/lib/xmodule/xmodule/course_module.py
+++ b/common/lib/xmodule/xmodule/course_module.py
@@ -121,7 +121,7 @@ class Textbook(object):
         return table_of_contents
 
 
-class TextbookList(ModelType):
+class TextbookList(List):
     def from_json(self, values):
         textbooks = []
         for title, book_url in values:
@@ -150,19 +150,19 @@ class TextbookList(ModelType):
 class CourseDescriptor(SequenceDescriptor):
     module_class = SequenceModule
 
-    textbooks = TextbookList(help="List of pairs of (title, url) for textbooks used in this course", default=[], scope=Scope.content)
+    textbooks = TextbookList(help="List of pairs of (title, url) for textbooks used in this course", scope=Scope.content)
     wiki_slug = String(help="Slug that points to the wiki for this course", scope=Scope.content)
     enrollment_start = Date(help="Date that enrollment for this class is opened", scope=Scope.settings)
     enrollment_end = Date(help="Date that enrollment for this class is closed", scope=Scope.settings)
     start = Date(help="Start time when this module is visible", scope=Scope.settings)
     end = Date(help="Date that this class ends", scope=Scope.settings)
     advertised_start = StringOrDate(help="Date that this course is advertised to start", scope=Scope.settings)
-    grading_policy = Object(help="Grading policy definition for this class", scope=Scope.content, default={})
+    grading_policy = Object(help="Grading policy definition for this class", scope=Scope.content)
     show_calculator = Boolean(help="Whether to show the calculator in this course", default=False, scope=Scope.settings)
     display_name = String(help="Display name for this module", scope=Scope.settings)
     tabs = List(help="List of tabs to enable in this course", scope=Scope.settings)
     end_of_course_survey_url = String(help="Url for the end-of-course survey", scope=Scope.settings)
-    discussion_blackouts = List(help="List of pairs of start/end dates for discussion blackouts", scope=Scope.settings, default=[])
+    discussion_blackouts = List(help="List of pairs of start/end dates for discussion blackouts", scope=Scope.settings)
     discussion_topics = Object(
         help="Map of topics names to ids",
         scope=Scope.settings,
@@ -175,10 +175,10 @@ class CourseDescriptor(SequenceDescriptor):
     no_grade = Boolean(help="True if this course isn't graded", default=False, scope=Scope.settings)
     disable_progress_graph = Boolean(help="True if this course shouldn't display the progress graph", default=False, scope=Scope.settings)
     pdf_textbooks = List(help="List of dictionaries containing pdf_textbook configuration", default=None, scope=Scope.settings)
-    remote_gradebook = Object(scope=Scope.settings, default={})
+    remote_gradebook = Object(scope=Scope.settings)
     allow_anonymous = Boolean(scope=Scope.settings, default=True)
     allow_anonymous_to_peers = Boolean(scope=Scope.settings, default=False)
-    advanced_modules = List(help="Beta modules used in your course", default=[], scope=Scope.settings)
+    advanced_modules = List(help="Beta modules used in your course", scope=Scope.settings)
     has_children = True
 
     info_sidebar_name = String(scope=Scope.settings, default='Course Handouts')
@@ -256,27 +256,27 @@ class CourseDescriptor(SequenceDescriptor):
                     "min_count": 12,
                     "drop_count": 2,
                     "short_label": "HW",
-                    "weight": 15
+                    "weight": 0.15
                 },
                 {
                     "type": "Lab",
                     "min_count": 12,
                     "drop_count": 2,
-                    "weight": 15
+                    "weight": 0.15
                 },
                 {
                     "type": "Midterm Exam",
                     "short_label": "Midterm",
                     "min_count": 1,
                     "drop_count": 0,
-                    "weight": 30
+                    "weight": 0.3
                 },
                 {
                     "type": "Final Exam",
                     "short_label": "Final",
                     "min_count": 1,
                     "drop_count": 0,
-                    "weight": 40
+                    "weight": 0.4
                 }
             ],
             "GRADE_CUTOFFS": {
diff --git a/local-requirements.txt b/local-requirements.txt
index 199af35dc0..512c6b3ff6 100644
--- a/local-requirements.txt
+++ b/local-requirements.txt
@@ -6,4 +6,4 @@
 # XBlock:
 # Might change frequently, so put it in local-requirements.txt,
 # but conceptually is an external package, so it is in a separate repo.
--e git+ssh://git@github.com/MITx/xmodule-debugger@857dcfe8#egg=XBlock
+-e git+ssh://git@github.com/MITx/xmodule-debugger@6d5c2443#egg=XBlock