Merge branch 'master' into will/use-mixed-modulestore-in-tests

Conflicts:
	common/lib/xmodule/xmodule/modulestore/django.py
	lms/djangoapps/courseware/tests/test_views.py
	lms/djangoapps/courseware/tests/tests.py

CHANGELOG.rst

@@ -7,6 +7,10 @@ the top.  Include a label indicating the component affected.
 
 Blades: Took videoalpha out of alpha, replacing the old video player
 
+Common: Allow instructors to input complicated expressions as answers to
+`NumericalResponse`s. Prior to the change only numbers were allowed, now any
+answer from '1/3' to 'sqrt(12)*(1-1/3^2+1/5/3^2)' are valid.
+
 LMS: Enable beta instructor dashboard. The beta dashboard is a rearchitecture
 of the existing instructor dashboard and is available by clicking a link at
 the top right of the existing dashboard.

cms/djangoapps/contentstore/features/grading.feature

@@ -93,3 +93,9 @@ Feature: Course Grading
         And I press the "Save" notification button
         And I reload the page
         Then I see the highest grade range is "Good"
+
+    Scenario: User cannot edit failing grade range name
+        Given I have opened a new course in Studio
+        And I have populated the course
+        And I am viewing the grading settings
+        Then I cannot edit the "Fail" grade range

cms/djangoapps/contentstore/features/grading.py

@@ -4,6 +4,7 @@
 from lettuce import world, step
 from common import *
 from terrain.steps import reload_the_page
+from selenium.common.exceptions import InvalidElementStateException
 
 
 @step(u'I am viewing the grading settings')
@@ -130,6 +131,18 @@ def i_see_highest_grade_range(_step, range_name):
     grade = world.css_find(range_css).first
     assert grade.value == range_name
 
+
+@step(u'I cannot edit the "Fail" grade range$')
+def cannot_edit_fail(_step):
+    range_css = 'span.letter-grade'
+    ranges = world.css_find(range_css)
+    assert len(ranges) == 2
+    try:
+        ranges.last.value = 'Failure'
+        assert False, "Should not be able to edit failing range"
+    except InvalidElementStateException:
+        pass  # We should get this exception on failing to edit the element
+
 def get_type_index(name):
     name_id = '#course-grading-assignment-name'
     all_types = world.css_find(name_id)

cms/djangoapps/contentstore/features/video-editor.py

@@ -5,7 +5,7 @@ from lettuce import world, step
 from terrain.steps import reload_the_page
 
 
-@step('I have set "show captions" to (.*)')
+@step('I have set "show captions" to (.*)$')
 def set_show_captions(step, setting):
     world.css_click('a.edit-button')
     world.wait_for(lambda _driver: world.css_visible('a.save-button'))
@@ -13,7 +13,7 @@ def set_show_captions(step, setting):
     world.css_click('a.save-button')
 
 
-@step('when I view the (video.*) it (.*) show the captions')
+@step('when I view the (video.*) it (.*) show the captions$')
 def shows_captions(_step, video_type, show_captions):
     # Prevent cookies from overriding course settings
     world.browser.cookies.delete('hide_captions')
@@ -39,7 +39,7 @@ def correct_video_settings(_step):
                                       ['Youtube ID for 1.5x speed', '', False]])
 
 
-@step('my video display name change is persisted on save')
+@step('my video display name change is persisted on save$')
 def video_name_persisted(step):
     world.css_click('a.save-button')
     reload_the_page(step)

cms/djangoapps/contentstore/features/video.py

@@ -19,25 +19,25 @@ def i_created_a_video_component(step):
     )
 
 
-@step('when I view the (.*) it does not have autoplay enabled')
+@step('when I view the (.*) it does not have autoplay enabled$')
 def does_not_autoplay(_step, video_type):
     assert world.css_find('.%s' % video_type)[0]['data-autoplay'] == 'False'
     assert world.css_has_class('.video_control', 'play')
 
 
-@step('creating a video takes a single click')
+@step('creating a video takes a single click$')
 def video_takes_a_single_click(_step):
     assert(not world.is_css_present('.xmodule_VideoModule'))
     world.css_click("a[data-category='video']")
     assert(world.is_css_present('.xmodule_VideoModule'))
 
 
-@step('I edit the component')
+@step('I edit the component$')
 def i_edit_the_component(_step):
     world.edit_component()
 
 
-@step('I have (hidden|toggled) captions')
+@step('I have (hidden|toggled) captions$')
 def hide_or_show_captions(step, shown):
     button_css = 'a.hide-subtitles'
     if shown == 'hidden':
@@ -54,7 +54,7 @@ def hide_or_show_captions(step, shown):
         world.css_click(button_css)
 
 
-@step('I have created a video with only XML data')
+@step('I have created a video with only XML data$')
 def xml_only_video(step):
     # Create a new video *without* metadata. This requires a certain
     # amount of rummaging to make sure all the correct data is present
@@ -84,7 +84,7 @@ def xml_only_video(step):
     reload_the_page(step)
 
 
-@step('The correct Youtube video is shown')
+@step('The correct Youtube video is shown$')
 def the_youtube_video_is_shown(_step):
     ele = world.css_find('.video').first
     assert ele['data-streams'].split(':')[1] == world.scenario_dict['YOUTUBE_ID']

cms/static/js/views/settings/settings_grading_view.js

@@ -8,7 +8,7 @@ CMS.Views.Settings.Grading = CMS.Views.ValidatingView.extend({
         // Leaving change in as fallback for older browsers
         "change input" : "updateModel",
         "change textarea" : "updateModel",
-        "input span[contenteditable]" : "updateDesignation",
+        "input span[contenteditable=true]" : "updateDesignation",
         "click .settings-extra header" : "showSettingsExtras",
         "click .new-grade-button" : "addNewGrade",
         "click .remove-button" : "removeGrade",
@@ -20,7 +20,7 @@ CMS.Views.Settings.Grading = CMS.Views.ValidatingView.extend({
     initialize : function() {
         //  load template for grading view
         var self = this;
-        this.gradeCutoffTemplate = _.template('<li class="grade-specific-bar" style="width:<%= width %>%"><span class="letter-grade" contenteditable>' +
+        this.gradeCutoffTemplate = _.template('<li class="grade-specific-bar" style="width:<%= width %>%"><span class="letter-grade" contenteditable="true">' +
                 '<%= descriptor %>' +
                 '</span><span class="range"></span>' +
                 '<% if (removable) {%><a href="#" class="remove-button">remove</a><% ;} %>' +
@@ -168,9 +168,12 @@ CMS.Views.Settings.Grading = CMS.Views.ValidatingView.extend({
         },
         this);
         // add fail which is not in data
-        var failBar = this.gradeCutoffTemplate({ descriptor : this.failLabel(),
-            width : nextWidth, removable : false});
-        $(failBar).find("span[contenteditable=true]").attr("contenteditable", false);
+        var failBar = $(this.gradeCutoffTemplate({
+            descriptor : this.failLabel(),
+            width : nextWidth,
+            removable : false
+        }));
+        failBar.find("span[contenteditable=true]").attr("contenteditable", false);
         gradelist.append(failBar);
         gradelist.children().last().resizable({
             handles: "e",

common/djangoapps/static_replace/__init__.py

@@ -119,7 +119,15 @@ def replace_static_urls(text, data_directory, course_id=None):
         elif course_id and modulestore().get_modulestore_type(course_id) != XML_MODULESTORE_TYPE:
             # first look in the static file pipeline and see if we are trying to reference
             # a piece of static content which is in the mitx repo (e.g. JS associated with an xmodule)
-            if staticfiles_storage.exists(rest):
+
+            exists_in_staticfiles_storage = False
+            try:
+                exists_in_staticfiles_storage = staticfiles_storage.exists(rest)
+            except Exception as err:
+                log.warning("staticfiles_storage couldn't find path {0}: {1}".format(
+                    rest, str(err)))
+
+            if exists_in_staticfiles_storage:
                 url = staticfiles_storage.url(rest)
             else:
                 # if not, then assume it's courseware specific content and then look in the
@@ -142,6 +150,7 @@ def replace_static_urls(text, data_directory, course_id=None):
 
         return "".join([quote, url, quote])
 
+
     return re.sub(
         _url_replace_regex('/static/(?!{data_dir})'.format(data_dir=data_directory)),
         replace_static_url,

common/lib/capa/capa/responsetypes.py

@@ -17,6 +17,7 @@ import logging
 import numbers
 import numpy
 import os
+from pyparsing import ParseException
 import sys
 import random
 import re
@@ -826,45 +827,89 @@ class NumericalResponse(LoncapaResponse):
     required_attributes = ['answer']
     max_inputfields = 1
 
+    def __init__(self, *args, **kwargs):
+        self.correct_answer = ''
+        self.tolerance = '0'  # Default value
+        super(NumericalResponse, self).__init__(*args, **kwargs)
+
     def setup_response(self):
         xml = self.xml
         context = self.context
         self.correct_answer = contextualize_text(xml.get('answer'), context)
+
+        # Find the tolerance
+        tolerance_xml = xml.xpath(
+            '//*[@id=$id]//responseparam[@type="tolerance"]/@default',
+            id=xml.get('id')
+        )
+        if tolerance_xml:  # If it isn't an empty list...
+            self.tolerance = contextualize_text(tolerance_xml[0], context)
+
+    def get_staff_ans(self):
+        """
+        Given the staff answer as a string, find its float value.
+
+        Use `evaluator` for this, but for backward compatability, try the
+        built-in method `complex` (which used to be the standard).
+        """
+
         try:
-            self.tolerance_xml = xml.xpath(
-                '//*[@id=$id]//responseparam[@type="tolerance"]/@default',
-                id=xml.get('id'))[0]
-            self.tolerance = contextualize_text(self.tolerance_xml, context)
-        except IndexError:  # xpath found an empty list, so (...)[0] is the error
-            self.tolerance = '0'
+            correct_ans = complex(self.correct_answer)
+        except ValueError:
+            # When `correct_answer` is not of the form X+Yj, it raises a
+            # `ValueError`. Then test if instead it is a math expression.
+            # `complex` seems to only generate `ValueErrors`, only catch these.
+            try:
+                correct_ans = evaluator({}, {}, self.correct_answer)
+            except Exception:
+                log.debug("Content error--answer '%s' is not a valid number", self.correct_answer)
+                raise StudentInputError(
+                    "There was a problem with the staff answer to this problem"
+                )
+
+        return correct_ans
 
     def get_score(self, student_answers):
         '''Grade a numeric response '''
         student_answer = student_answers[self.answer_id]
 
+        correct_float = self.get_staff_ans()
+
+        general_exception = StudentInputError(
+            u"Could not interpret '{0}' as a number".format(cgi.escape(student_answer))
+        )
+
+        # Begin `evaluator` block
+        # Catch a bunch of exceptions and give nicer messages to the student.
         try:
-            correct_ans = complex(self.correct_answer)
-        except ValueError:
-            log.debug("Content error--answer '{0}' is not a valid complex number".format(
-                self.correct_answer))
+            student_float = evaluator({}, {}, student_answer)
+        except UndefinedVariable as undef_var:
             raise StudentInputError(
-                "There was a problem with the staff answer to this problem")
-
-        try:
-            correct = compare_with_tolerance(
-                evaluator(dict(), dict(), student_answer),
-                correct_ans, self.tolerance)
-        # We should catch this explicitly.
-        # I think this is just pyparsing.ParseException, calc.UndefinedVariable:
-        # But we'd need to confirm
-        except:
-            # Use the traceback-preserving version of re-raising with a
-            # different type
-            type, value, traceback = sys.exc_info()
-
-            raise StudentInputError, ("Could not interpret '%s' as a number" %
-                                      cgi.escape(student_answer)), traceback
+                u"You may not use variables ({0}) in numerical problems".format(undef_var.message)
+            )
+        except ValueError as val_err:
+            if 'factorial' in val_err.message:
+                # This is thrown when fact() or factorial() is used in an answer
+                #   that evaluates on negative and/or non-integer inputs
+                # ve.message will be: `factorial() only accepts integral values` or
+                # `factorial() not defined for negative values`
+                raise StudentInputError(
+                    ("factorial function evaluated outside its domain:"
+                     "'{0}'").format(cgi.escape(student_answer))
+                )
+            else:
+                raise general_exception
+        except ParseException:
+            raise StudentInputError(
+                u"Invalid math syntax: '{0}'".format(cgi.escape(student_answer))
+            )
+        except Exception:
+            raise general_exception
+        # End `evaluator` block -- we figured out the student's answer!
 
+        correct = compare_with_tolerance(
+            student_float, correct_float, self.tolerance
+        )
         if correct:
             return CorrectMap(self.answer_id, 'correct')
         else:
@@ -1691,18 +1736,26 @@ class FormulaResponse(LoncapaResponse):
     required_attributes = ['answer', 'samples']
     max_inputfields = 1
 
+    def __init__(self, *args, **kwargs):
+        self.correct_answer = ''
+        self.samples = ''
+        self.tolerance = '1e-5'  # Default value
+        self.case_sensitive = False
+        super(FormulaResponse, self).__init__(*args, **kwargs)
+
     def setup_response(self):
         xml = self.xml
         context = self.context
         self.correct_answer = contextualize_text(xml.get('answer'), context)
         self.samples = contextualize_text(xml.get('samples'), context)
-        try:
-            self.tolerance_xml = xml.xpath(
-                '//*[@id=$id]//responseparam[@type="tolerance"]/@default',
-                id=xml.get('id'))[0]
-            self.tolerance = contextualize_text(self.tolerance_xml, context)
-        except Exception:
-            self.tolerance = '0.00001'
+
+        # Find the tolerance
+        tolerance_xml = xml.xpath(
+            '//*[@id=$id]//responseparam[@type="tolerance"]/@default',
+            id=xml.get('id')
+        )
+        if tolerance_xml:  # If it isn't an empty list...
+            self.tolerance = contextualize_text(tolerance_xml[0], context)
 
         ts = xml.get('type')
         if ts is None:
@@ -1734,7 +1787,7 @@ class FormulaResponse(LoncapaResponse):
         ranges = dict(zip(variables, sranges))
         for _ in range(numsamples):
             instructor_variables = self.strip_dict(dict(self.context))
-            student_variables = dict()
+            student_variables = {}
             # ranges give numerical ranges for testing
             for var in ranges:
                 # TODO: allow specified ranges (i.e. integers and complex numbers) for random variables
@@ -1746,7 +1799,7 @@ class FormulaResponse(LoncapaResponse):
 
             # Call `evaluator` on the instructor's answer and get a number
             instructor_result = evaluator(
-                instructor_variables, dict(),
+                instructor_variables, {},
                 expected, case_sensitive=self.case_sensitive
             )
             try:
@@ -1756,7 +1809,7 @@ class FormulaResponse(LoncapaResponse):
                 # Call `evaluator` on the student's answer; look for exceptions
                 student_result = evaluator(
                     student_variables,
-                    dict(),
+                    {},
                     given,
                     case_sensitive=self.case_sensitive
                 )
@@ -2422,7 +2475,7 @@ class ChoiceTextResponse(LoncapaResponse):
             # if all that is important is verifying numericality
             try:
                 partial_correct = compare_with_tolerance(
-                    evaluator(dict(), dict(), answer_value),
+                    evaluator({}, {}, answer_value),
                     correct_ans,
                     tolerance
                 )

common/lib/capa/capa/tests/test_responsetypes.py

@@ -5,12 +5,14 @@ Tests of responsetypes
 from datetime import datetime
 import json
 import os
+import pyparsing
 import random
 import unittest
 import textwrap
 import mock
 
 from . import new_loncapa_problem, test_system
+import calc
 
 from capa.responsetypes import LoncapaProblemError, \
     StudentInputError, ResponseError
@@ -22,7 +24,7 @@ from pytz import UTC
 
 
 class ResponseTest(unittest.TestCase):
-    """ Base class for tests of capa responses."""
+    """Base class for tests of capa responses."""
 
     xml_factory_class = None
 
@@ -442,91 +444,6 @@ class FormulaResponseTest(ResponseTest):
         self.assert_grade(problem, '2*x', 'correct')
         self.assert_grade(problem, '3*x', 'incorrect')
 
-    def test_parallel_resistors(self):
-        """
-        Test parallel resistors
-        """
-        sample_dict = {'R1': (10, 10), 'R2': (2, 2), 'R3': (5, 5), 'R4': (1, 1)}
-
-        # Test problem
-        problem = self.build_problem(sample_dict=sample_dict,
-                                     num_samples=10,
-                                     tolerance=0.01,
-                                     answer="R1||R2")
-        # Expect answer to be marked correct
-        input_formula = "R1||R2"
-        self.assert_grade(problem, input_formula, "correct")
-
-        # Expect random number to be marked incorrect
-        input_formula = "13"
-        self.assert_grade(problem, input_formula, "incorrect")
-
-        # Expect incorrect answer marked incorrect
-        input_formula = "R3||R4"
-        self.assert_grade(problem, input_formula, "incorrect")
-
-    def test_default_variables(self):
-        """
-        Test the default variables provided in calc.py
-
-        which are: j (complex number), e, pi, k, c, T, q
-        """
-
-        # Sample x in the range [-10,10]
-        sample_dict = {'x': (-10, 10)}
-        default_variables = [('j', 2, 3), ('e', 2, 3), ('pi', 2, 3), ('c', 2, 3), ('T', 2, 3),
-                             ('k', 2 * 10 ** 23, 3 * 10 ** 23),   # note k = scipy.constants.k = 1.3806488e-23
-                             ('q', 2 * 10 ** 19, 3 * 10 ** 19)]   # note k = scipy.constants.e = 1.602176565e-19
-        for (var, cscalar, iscalar) in default_variables:
-            # The expected solution is numerically equivalent to cscalar*var
-            correct = '{0}*x*{1}'.format(cscalar, var)
-            incorrect = '{0}*x*{1}'.format(iscalar, var)
-            problem = self.build_problem(sample_dict=sample_dict,
-                                         num_samples=10,
-                                         tolerance=0.01,
-                                         answer=correct)
-
-            # Expect that the inputs are graded correctly
-            self.assert_grade(problem, correct, 'correct',
-                              msg="Failed on variable {0}; the given, correct answer was {1} but graded 'incorrect'".format(var, correct))
-            self.assert_grade(problem, incorrect, 'incorrect',
-                              msg="Failed on variable {0}; the given, incorrect answer was {1} but graded 'correct'".format(var, incorrect))
-
-    def test_default_functions(self):
-        """
-        Test the default functions provided in common/lib/capa/capa/calc.py
-
-        which are:
-          sin, cos, tan, sqrt, log10, log2, ln,
-          arccos, arcsin, arctan, abs,
-          fact, factorial
-        """
-        w = random.randint(3, 10)
-        sample_dict = {'x': (-10, 10),  # Sample x in the range [-10,10]
-                       'y': (1, 10),    # Sample y in the range [1,10] - logs, arccos need positive inputs
-                       'z': (-1, 1),    # Sample z in the range [1,10] - for arcsin, arctan
-                       'w': (w, w)}     # Sample w is a random, positive integer - factorial needs a positive, integer input,
-                                        # and the way formularesponse is defined, we can only specify a float range
-
-        default_functions = [('sin', 2, 3, 'x'), ('cos', 2, 3, 'x'), ('tan', 2, 3, 'x'), ('sqrt', 2, 3, 'y'), ('log10', 2, 3, 'y'),
-                             ('log2', 2, 3, 'y'), ('ln', 2, 3, 'y'), ('arccos', 2, 3, 'z'), ('arcsin', 2, 3, 'z'), ('arctan', 2, 3, 'x'),
-                             ('abs', 2, 3, 'x'), ('fact', 2, 3, 'w'), ('factorial', 2, 3, 'w')]
-        for (func, cscalar, iscalar, var) in default_functions:
-            print 'func is: {0}'.format(func)
-            # The expected solution is numerically equivalent to cscalar*func(var)
-            correct = '{0}*{1}({2})'.format(cscalar, func, var)
-            incorrect = '{0}*{1}({2})'.format(iscalar, func, var)
-            problem = self.build_problem(sample_dict=sample_dict,
-                                         num_samples=10,
-                                         tolerance=0.01,
-                                         answer=correct)
-
-            # Expect that the inputs are graded correctly
-            self.assert_grade(problem, correct, 'correct',
-                              msg="Failed on function {0}; the given, correct answer was {1} but graded 'incorrect'".format(func, correct))
-            self.assert_grade(problem, incorrect, 'incorrect',
-                              msg="Failed on function {0}; the given, incorrect answer was {1} but graded 'correct'".format(func, incorrect))
-
     def test_grade_infinity(self):
         """
         Test that a large input on a problem with relative tolerance isn't
@@ -885,92 +802,118 @@ class NumericalResponseTest(ResponseTest):
     from capa.tests.response_xml_factory import NumericalResponseXMLFactory
     xml_factory_class = NumericalResponseXMLFactory
 
+    # We blend the line between integration (using evaluator) and exclusively
+    # unit testing the NumericalResponse (mocking out the evaluator)
+    # For simple things its not worth the effort.
     def test_grade_exact(self):
-        problem = self.build_problem(question_text="What is 2 + 2?",
-                                     explanation="The answer is 4",
-                                     answer=4)
+        problem = self.build_problem(answer=4)
         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)
+        problem = self.build_problem(answer=4, tolerance=0.1)
         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",
-                                     answer=4,
-                                     tolerance="10%")
+        problem = self.build_problem(answer=4, tolerance="10%")
         correct_responses = ["4.0", "4.3", "3.7", "4.30", "3.70"]
         incorrect_responses = ["", "4.5", "3.5", "0"]
         self.assert_multiple_grade(problem, correct_responses, incorrect_responses)
 
-    def test_grade_infinity(self):
-        # This resolves a bug where a problem with relative tolerance would
-        # pass with any arbitrarily large student answer.
-        problem = self.build_problem(question_text="What is 2 + 2 approximately?",
-                                     explanation="The answer is 4",
-                                     answer=4,
-                                     tolerance="10%")
-        correct_responses = []
-        incorrect_responses = ["1e999", "-1e999"]
-        self.assert_multiple_grade(problem, correct_responses, incorrect_responses)
-
-    def test_grade_nan(self):
-        # Attempt to produce a value which causes the student's answer to be
-        # evaluated to nan. See if this is resolved correctly.
-        problem = self.build_problem(question_text="What is 2 + 2 approximately?",
-                                     explanation="The answer is 4",
-                                     answer=4,
-                                     tolerance="10%")
-        correct_responses = []
-        # Right now these evaluate to `nan`
-        # `4 + nan` should be incorrect
-        incorrect_responses = ["0*1e999", "4 + 0*1e999"]
-        self.assert_multiple_grade(problem, correct_responses, incorrect_responses)
-
     def test_grade_with_script(self):
         script_text = "computed_response = math.sqrt(4)"
-        problem = self.build_problem(question_text="What is sqrt(4)?",
-                                     explanation="The answer is 2",
-                                     answer="$computed_response",
-                                     script=script_text)
+        problem = self.build_problem(answer="$computed_response", script=script_text)
         correct_responses = ["2", "2.0"]
         incorrect_responses = ["", "2.01", "1.99", "0"]
         self.assert_multiple_grade(problem, correct_responses, incorrect_responses)
 
-    def test_grade_with_script_and_tolerance(self):
-        script_text = "computed_response = math.sqrt(4)"
-        problem = self.build_problem(question_text="What is sqrt(4)?",
-                                     explanation="The answer is 2",
-                                     answer="$computed_response",
-                                     tolerance="0.1",
-                                     script=script_text)
-        correct_responses = ["2", "2.0", "2.05", "1.95"]
-        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)
-
     def test_raises_zero_division_err(self):
-        """See if division by zero is handled correctly"""
-        problem = self.build_problem(question_text="What 5 * 10?",
-                                     explanation="The answer is 50",
-                                     answer="5e+1")  # Answer doesn't matter
+        """See if division by zero is handled correctly."""
+        problem = self.build_problem(answer="1")  # Answer doesn't matter
         input_dict = {'1_2_1': '1/0'}
-        self.assertRaises(StudentInputError, problem.grade_answers, input_dict)
+        with self.assertRaises(StudentInputError):
+            problem.grade_answers(input_dict)
+
+    def test_staff_inputs_expressions(self):
+        """Test that staff may enter in an expression as the answer."""
+        problem = self.build_problem(answer="1/3", tolerance=1e-3)
+        correct_responses = ["1/3", "0.333333"]
+        incorrect_responses = []
+        self.assert_multiple_grade(problem, correct_responses, incorrect_responses)
+
+    def test_staff_inputs_expressions_legacy(self):
+        """Test that staff may enter in a complex number as the answer."""
+        problem = self.build_problem(answer="1+1j", tolerance=1e-3)
+        self.assert_grade(problem, '1+j', 'correct')
+
+    @mock.patch('capa.responsetypes.log')
+    def test_staff_inputs_bad_syntax(self, mock_log):
+        """Test that staff may enter in a complex number as the answer."""
+        staff_ans = "clearly bad syntax )[+1e"
+        problem = self.build_problem(answer=staff_ans, tolerance=1e-3)
+
+        msg = "There was a problem with the staff answer to this problem"
+        with self.assertRaisesRegexp(StudentInputError, msg):
+            self.assert_grade(problem, '1+j', 'correct')
+
+        mock_log.debug.assert_called_once_with(
+            "Content error--answer '%s' is not a valid number", staff_ans
+        )
+
+    def test_grade_infinity(self):
+        """
+        Check that infinity doesn't automatically get marked correct.
+
+        This resolves a bug where a problem with relative tolerance would
+        pass with any arbitrarily large student answer.
+        """
+        mapping = {
+            'some big input': float('inf'),
+            'some neg input': -float('inf'),
+            'weird NaN input': float('nan'),
+            '4': 4
+        }
+
+        def evaluator_side_effect(_, __, math_string):
+            """Look up the given response for `math_string`."""
+            return mapping[math_string]
+
+        problem = self.build_problem(answer=4, tolerance='10%')
+
+        with mock.patch('capa.responsetypes.evaluator') as mock_eval:
+            mock_eval.side_effect = evaluator_side_effect
+            self.assert_grade(problem, 'some big input', 'incorrect')
+            self.assert_grade(problem, 'some neg input', 'incorrect')
+            self.assert_grade(problem, 'weird NaN input', 'incorrect')
+
+    def test_err_handling(self):
+        """
+        See that `StudentInputError`s are raised when things go wrong.
+        """
+        problem = self.build_problem(answer=4)
+
+        errors = [  # (exception raised, message to student)
+            (calc.UndefinedVariable("x"), r"You may not use variables \(x\) in numerical problems"),
+            (ValueError("factorial() mess-up"), "factorial function evaluated outside its domain"),
+            (ValueError(), "Could not interpret '.*' as a number"),
+            (pyparsing.ParseException("oopsie"), "Invalid math syntax"),
+            (ZeroDivisionError(), "Could not interpret '.*' as a number")
+        ]
+
+        with mock.patch('capa.responsetypes.evaluator') as mock_eval:
+            for err, msg_regex in errors:
+
+                def evaluator_side_effect(_, __, math_string):
+                    """Raise an error only for the student input."""
+                    if math_string != '4':
+                        raise err
+                mock_eval.side_effect = evaluator_side_effect
+
+                with self.assertRaisesRegexp(StudentInputError, msg_regex):
+                    problem.grade_answers({'1_2_1': 'foobar'})
 
 
 class CustomResponseTest(ResponseTest):

common/lib/xmodule/xmodule/css/capa/display.scss

@@ -577,7 +577,7 @@ section.problem {
   section.action {
     margin-top: 20px;
 
-    .save, .check, .show {
+    .save, .check, .show, .reset {
       height: ($baseline*2);
       font-weight: 600;
       vertical-align: middle;

common/lib/xmodule/xmodule/modulestore/django.py

@@ -8,6 +8,7 @@ from __future__ import absolute_import
 from importlib import import_module
 
 from django.conf import settings
+from xmodule.modulestore.loc_mapper_store import LocMapperStore
 
 _MODULESTORES = {}
 
@@ -55,6 +56,21 @@ def modulestore(name='default'):
     return _MODULESTORES[name]
 
 
+_loc_singleton = None
+def loc_mapper():
+    """
+    Get the loc mapper which bidirectionally maps Locations to Locators. Used like modulestore() as
+    a singleton accessor.
+    """
+    # pylint: disable=W0603
+    global _loc_singleton
+    # pylint: disable=W0212
+    if _loc_singleton is None:
+        # instantiate
+        _loc_singleton = LocMapperStore(settings.modulestore_options)
+    return _loc_singleton
+
+
 def clear_existing_modulestores():
     """
     Clear the existing modulestore instances, causing
@@ -93,3 +109,4 @@ def editable_modulestore(name='default'):
 
     else:
         return None
+

common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py

@@ -0,0 +1,382 @@
+'''
+Method for converting among our differing Location/Locator whatever reprs
+'''
+from random import randint
+import re
+import pymongo
+
+from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError, DuplicateItemError
+from xmodule.modulestore.locator import BlockUsageLocator
+from xmodule.modulestore.mongo import draft
+from xmodule.modulestore import Location
+
+
+class LocMapperStore(object):
+    '''
+    This store persists mappings among the addressing schemes. At this time, it's between the old i4x Location
+    tuples and the split mongo Course and Block Locator schemes.
+
+    edX has used several different addressing schemes. The original ones were organically created based on
+    immediate needs and were overly restrictive esp wrt course ids. These were slightly extended to support
+    some types of blocks may need to have draft states during editing to keep live courses from seeing the wip.
+    A later refactoring generalized course ids to enable governance and more complex naming, branch naming with
+    anything able to be in any branch.
+
+    The expectation is that the configuration will have this use the same store as whatever is the default
+    or dominant store, but that's not a requirement. This store creates its own connection.
+    '''
+
+    # C0103: varnames and attrs must be >= 3 chars, but db defined by long time usage
+    # pylint: disable = C0103
+    def __init__(self, host, db, collection, port=27017, user=None, password=None, **kwargs):
+        '''
+        Constructor
+        '''
+        self.db = pymongo.database.Database(
+            pymongo.MongoClient(
+                host=host,
+                port=port,
+                tz_aware=True,
+                **kwargs
+            ),
+            db
+        )
+        if user is not None and password is not None:
+            self.db.authenticate(user, password)
+
+        self.location_map = self.db[collection + '.location_map']
+        self.location_map.write_concern = {'w': 1}
+
+    # location_map functions
+    def create_map_entry(self, course_location, course_id=None, draft_branch='draft', prod_branch='published',
+                         block_map=None):
+        """
+        Add a new entry to map this course_location to the new style CourseLocator.course_id. If course_id is not
+        provided, it creates the default map of using org.course.name from the location (just like course_id) if
+        the location.cateogry = 'course'; otherwise, it uses org.course.
+
+        You can create more than one mapping to the
+        same course_id target. In that case, the reverse translate will be arbitrary (no guarantee of which wins).
+        The use
+        case for more than one mapping is to map both org/course/run and org/course to the same new course_id thus
+        making a default for org/course. When querying for just org/course, the translator will prefer any entry
+        which does not have a name in the _id; otherwise, it will return an arbitrary match.
+
+        Note: the opposite is not true. That is, it never makes sense to use 2 different CourseLocator.course_id
+        keys to index the same old Locator org/course/.. pattern. There's no checking to ensure you don't do this.
+
+        NOTE: if there's already an entry w the given course_location, this may either overwrite that entry or
+        throw an error depending on how mongo is configured.
+
+        :param course_location: a Location preferably whose category is 'course'. Unlike the other
+        map methods, this one doesn't take the old-style course_id.  It should be called with
+        a course location not a block location; however, if called w/ a non-course Location, it creates
+        a "default" map for the org/course pair to a new course_id.
+        :param course_id: the CourseLocator style course_id
+        :param draft_branch: the branch name to assign for drafts. This is hardcoded because old mongo had
+        a fixed notion that there was 2 and only 2 versions for modules: draft and production. The old mongo
+        did not, however, require that a draft version exist. The new one, however, does require a draft to
+        exist.
+        :param prod_branch: the branch name to assign for the production (live) copy. In old mongo, every course
+        had to have a production version (whereas new split mongo does not require that until the author's ready
+        to publish).
+        :param block_map: an optional map to specify preferred names for blocks where the keys are the
+        Location block names and the values are the BlockUsageLocator.block_id.
+        """
+        if course_id is None:
+            if course_location.category == 'course':
+                course_id = "{0.org}.{0.course}.{0.name}".format(course_location)
+            else:
+                course_id = "{0.org}.{0.course}".format(course_location)
+        # very like _interpret_location_id but w/o the _id
+        location_id = {'org': course_location.org, 'course': course_location.course}
+        if course_location.category == 'course':
+            location_id['name'] = course_location.name
+
+        self.location_map.insert({
+            '_id': location_id,
+            'course_id': course_id,
+            'draft_branch': draft_branch,
+            'prod_branch': prod_branch,
+            'block_map': block_map or {},
+        })
+
+    def translate_location(self, old_style_course_id, location, published=True, add_entry_if_missing=True):
+        """
+        Translate the given module location to a Locator. If the mapping has the run id in it, then you
+        should provide old_style_course_id with that run id in it to disambiguate the mapping if there exists more
+        than one entry in the mapping table for the org.course.
+
+        The rationale for auto adding entries was that there should be a reasonable default translation
+        if the code just trips into this w/o creating translations. The downfall is that ambiguous course
+        locations may generate conflicting block_ids.
+
+        Will raise ItemNotFoundError if there's no mapping and add_entry_if_missing is False.
+
+        :param old_style_course_id: the course_id used in old mongo not the new one (optional, will use location)
+        :param location:  a Location pointing to a module
+        :param published: a boolean to indicate whether the caller wants the draft or published branch.
+        :param add_entry_if_missing: a boolean as to whether to raise ItemNotFoundError or to create an entry if
+        the course
+        or block is not found in the map.
+
+        NOTE: unlike old mongo, draft branches contain the whole course; so, it applies to all category
+        of locations including course.
+        """
+        location_id = self._interpret_location_course_id(old_style_course_id, location)
+
+        maps = self.location_map.find(location_id).sort('_id.name', pymongo.ASCENDING)
+        if maps.count() == 0:
+            if add_entry_if_missing:
+                # create a new map
+                course_location = location.replace(category='course', name=location_id['_id.name'])
+                self.create_map_entry(course_location)
+                entry = self.location_map.find_one(location_id)
+            else:
+                raise ItemNotFoundError()
+        elif maps.count() > 1:
+            # if more than one, prefer the one w/o a name if that exists. Otherwise, choose the first (alphabetically)
+            entry = maps[0]
+        else:
+            entry = maps[0]
+
+        if published:
+            branch = entry['prod_branch']
+        else:
+            branch = entry['draft_branch']
+
+        usage_id = entry['block_map'].get(location.name)
+        if usage_id is None:
+            if add_entry_if_missing:
+                usage_id = self._add_to_block_map(location, location_id, entry['block_map'])
+            else:
+                raise ItemNotFoundError()
+        elif isinstance(usage_id, dict):
+            # name is not unique, look through for the right category
+            if location.category in usage_id:
+                usage_id = usage_id[location.category]
+            elif add_entry_if_missing:
+                usage_id = self._add_to_block_map(location, location_id, entry['block_map'])
+            else:
+                raise ItemNotFoundError()
+        else:
+            raise InvalidLocationError()
+
+        return BlockUsageLocator(course_id=entry['course_id'], branch=branch, usage_id=usage_id)
+
+    def translate_locator_to_location(self, locator):
+        """
+        Returns an old style Location for the given Locator if there's an appropriate entry in the
+        mapping collection. Note, it requires that the course was previously mapped (a side effect of
+        translate_location or explicitly via create_map_entry) and
+        the block's usage_id was previously stored in the
+        map (a side effect of translate_location or via add|update_block_location).
+
+        If there are no matches, it returns None.
+
+        If there's more than one location to locator mapping to the same course_id, it looks for the first
+        one with a mapping for the block usage_id and picks that arbitrary course location.
+
+        :param locator: a BlockUsageLocator
+        """
+        # This does not require that the course exist in any modulestore
+        # only that it has a mapping entry.
+        maps = self.location_map.find({'course_id': locator.course_id})
+        # look for one which maps to this block usage_id
+        if maps.count() == 0:
+            return None
+        for candidate in maps:
+            for old_name, cat_to_usage in candidate['block_map'].iteritems():
+                for category, usage_id in cat_to_usage.iteritems():
+                    if usage_id == locator.usage_id:
+                        # figure out revision
+                        # enforce the draft only if category in [..] logic
+                        if category in draft.DIRECT_ONLY_CATEGORIES:
+                            revision = None
+                        elif locator.branch == candidate['draft_branch']:
+                            revision = draft.DRAFT
+                        else:
+                            revision = None
+                        return Location(
+                            'i4x',
+                            candidate['_id']['org'],
+                            candidate['_id']['course'],
+                            category,
+                            old_name,
+                            revision)
+        return None
+
+    def add_block_location_translator(self, location, old_course_id=None, usage_id=None):
+        """
+        Similar to translate_location which adds an entry if none is found, but this cannot create a new
+        course mapping entry, only a block within such a mapping entry. If it finds no existing
+        course maps, it raises ItemNotFoundError.
+
+        In the case that there are more than one mapping record for the course identified by location, this
+        method adds the mapping to all matching records! (translate_location only adds to one)
+
+        It allows the caller to specify
+        the new-style usage_id for the target rather than having the translate concoct its own.
+        If the provided usage_id already exists in one of the found maps for the org/course, this function
+        raises DuplicateItemError unless the old item id == the new one.
+
+        If the caller does not provide a usage_id and there exists an entry in one of the course variants,
+        it will use that entry. If more than one variant uses conflicting entries, it will raise DuplicateItemError.
+
+        Returns the usage_id used in the mapping
+
+        :param location: a fully specified Location
+        :param old_course_id: the old-style org/course or org/course/run string (optional)
+        :param usage_id: the desired new block_id. If left as None, this will generate one as per translate_location
+        """
+        location_id = self._interpret_location_course_id(old_course_id, location)
+
+        maps = self.location_map.find(location_id)
+        if maps.count() == 0:
+            raise ItemNotFoundError()
+
+        # turn maps from cursor to list
+        map_list = list(maps)
+        # check whether there's already a usage_id for this location (and it agrees w/ any passed in or found)
+        for map_entry in map_list:
+            if (location.name in map_entry['block_map'] and
+                    location.category in map_entry['block_map'][location.name]):
+                if usage_id is None:
+                    usage_id = map_entry['block_map'][location.name][location.category]
+                elif usage_id != map_entry['block_map'][location.name][location.category]:
+                    raise DuplicateItemError()
+
+        computed_usage_id = usage_id
+
+        # update the maps (and generate a usage_id if it's not been set yet)
+        for map_entry in map_list:
+            if computed_usage_id is None:
+                computed_usage_id = self._add_to_block_map(location, location_id, map_entry['block_map'])
+            elif (location.name not in map_entry['block_map'] or
+                    location.category not in map_entry['block_map'][location.name]):
+                alt_usage_id = self._verify_uniqueness(computed_usage_id, map_entry['block_map'])
+                if alt_usage_id != computed_usage_id:
+                    if usage_id is not None:
+                        raise DuplicateItemError()
+                    else:
+                        # revise already set ones and add to remaining ones
+                        computed_usage_id = self.update_block_location_translator(
+                            location,
+                            alt_usage_id,
+                            old_course_id,
+                            True
+                        )
+
+                map_entry['block_map'].setdefault(location.name, {})[location.category] = computed_usage_id
+                self.location_map.update({'_id': map_entry['_id']}, {'$set': {'block_map': map_entry['block_map']}})
+
+        return computed_usage_id
+
+    def update_block_location_translator(self, location, usage_id, old_course_id=None, autogenerated_usage_id=False):
+        """
+        Update all existing maps from location's block to the new usage_id. Used for changing the usage_id,
+        thus the usage_id is required.
+
+        Returns the usage_id. (which is primarily useful in the case of autogenerated_usage_id)
+
+        :param location: a fully specified Location
+        :param usage_id: the desired new block_id.
+        :param old_course_id: the old-style org/course or org/course/run string (optional)
+        :param autogenerated_usage_id: a flag used mostly for internal calls to indicate that this usage_id
+        was autogenerated and thus can be overridden if it's not unique. If you set this flag, the stored
+        usage_id may not be the one you submitted.
+        """
+        location_id = self._interpret_location_course_id(old_course_id, location)
+
+        maps = self.location_map.find(location_id)
+        for map_entry in maps:
+            # handle noop of renaming to same name
+            if (location.name in map_entry['block_map'] and
+                    map_entry['block_map'][location.name].get(location.category) == usage_id):
+                continue
+            alt_usage_id = self._verify_uniqueness(usage_id, map_entry['block_map'])
+            if alt_usage_id != usage_id:
+                if autogenerated_usage_id:
+                    # revise already set ones and add to remaining ones
+                    usage_id = self.update_block_location_translator(location, alt_usage_id, old_course_id, True)
+                    return usage_id
+                else:
+                    raise DuplicateItemError()
+
+            if location.category in map_entry['block_map'].setdefault(location.name, {}):
+                map_entry['block_map'][location.name][location.category] = usage_id
+                self.location_map.update({'_id': map_entry['_id']}, {'$set': {'block_map': map_entry['block_map']}})
+
+        return usage_id
+
+    def delete_block_location_translator(self, location, old_course_id=None):
+        """
+        Remove all existing maps from location's block.
+
+        :param location: a fully specified Location
+        :param old_course_id: the old-style org/course or org/course/run string (optional)
+        """
+        location_id = self._interpret_location_course_id(old_course_id, location)
+
+        maps = self.location_map.find(location_id)
+        for map_entry in maps:
+            if location.category in map_entry['block_map'].setdefault(location.name, {}):
+                if len(map_entry['block_map'][location.name]) == 1:
+                    del map_entry['block_map'][location.name]
+                else:
+                    del map_entry['block_map'][location.name][location.category]
+                self.location_map.update({'_id': map_entry['_id']}, {'$set': {'block_map': map_entry['block_map']}})
+
+    def _add_to_block_map(self, location, location_id, block_map):
+        '''add the given location to the block_map and persist it'''
+        if self._block_id_is_guid(location.name):
+            # This makes the ids more meaningful with a small probability of name collision.
+            # The downside is that if there's more than one course mapped to from the same org/course root
+            # the block ids will likely be out of sync and collide from an id perspective. HOWEVER,
+            # if there are few == org/course roots or their content is unrelated, this will work well.
+            usage_id = self._verify_uniqueness(location.category + location.name[:3], block_map)
+        block_map.setdefault(location.name, {})[location.category] = usage_id
+        self.location_map.update(location_id, {'$set': {'block_map': block_map}})
+        return usage_id
+
+    def _interpret_location_course_id(self, course_id, location):
+        """
+        Take the old style course id (org/course/run) and return a dict for querying the mapping table.
+        If the course_id is empty, it uses location, but this may result in an inadequate id.
+
+        :param course_id: old style 'org/course/run' id from Location.course_id where Location.category = 'course'
+        :param location: a Location object which may be to a module or a course. Provides partial info
+        if course_id is omitted.
+        """
+        if course_id:
+            # re doesn't allow ?P<_id.org> and ilk
+            matched = re.match(r'([^/]+)/([^/]+)/([^/]+)', course_id)
+            return dict(zip(['_id.org', '_id.course', '_id.name'], matched.groups()))
+
+        location_id = {'_id.org': location.org, '_id.course': location.course}
+        if location.category == 'course':
+            location_id['_id.name'] = location.name
+        return location_id
+
+    def _block_id_is_guid(self, name):
+        """
+        Does the given name look like it's a guid?
+        """
+        return len(name) == 32 and re.search(r'[^0-9A-Fa-f]', name) is None
+
+    def _verify_uniqueness(self, name, block_map):
+        '''
+        Verify that the name doesn't occur elsewhere in block_map. If it does, keep adding to it until
+        it's unique.
+        '''
+        for targets in block_map.itervalues():
+            if isinstance(targets, dict):
+                for values in targets.itervalues():
+                    if values == name:
+                        name += str(randint(0, 9))
+                        return self._verify_uniqueness(name, block_map)
+
+            elif targets == name:
+                name += str(randint(0, 9))
+                return self._verify_uniqueness(name, block_map)
+        return name

common/lib/xmodule/xmodule/modulestore/mixed.py

@@ -7,7 +7,7 @@ IMPORTANT: This modulestore only supports READONLY applications, e.g. LMS
 """
 
 from . import ModuleStoreBase
-from django import create_modulestore_instance
+from xmodule.modulestore.django import create_modulestore_instance
 import logging
 
 log = logging.getLogger(__name__)

common/lib/xmodule/xmodule/modulestore/parsers.py

@@ -19,10 +19,10 @@ def parse_url(string):
 
     Examples:
         'edx://version/0123FFFF'
-        'edx://edu.mit.eecs.6002x'
-        'edx://edu.mit.eecs.6002x/branch/published'
-        'edx://edu.mit.eecs.6002x/branch/published/version/519665f6223ebd6980884f2b/block/HW3'
-        'edx://edu.mit.eecs.6002x/branch/published/block/HW3'
+        'edx://mit.eecs.6002x'
+        'edx://mit.eecs.6002x;published'
+        'edx://mit.eecs.6002x;published/block/HW3'
+        'edx://mit.eecs.6002x;published/version/000eee12345/block/HW3'
 
     This returns None if string cannot be parsed.
 
@@ -97,11 +97,11 @@ def parse_course_id(string):
 
     Examples of valid course_ids:
 
-      'edu.mit.eecs.6002x'
-      'edu.mit.eecs.6002x/branch/published'
-      'edu.mit.eecs.6002x/block/HW3'
-      'edu.mit.eecs.6002x/branch/published/block/HW3'
-      'edu.mit.eecs.6002x/branch/published/version/519665f6223ebd6980884f2b/block/HW3'
+      'mit.eecs.6002x'
+      'mit.eecs.6002x/branch/published'
+      'mit.eecs.6002x/block/HW3'
+      'mit.eecs.6002x/branch/published/block/HW3'
+      'mit.eecs.6002x/branch/published/version/519665f6223ebd6980884f2b/block/HW3'
 
 
     Syntax:

common/lib/xmodule/xmodule/modulestore/split_mongo/split.py

@@ -10,9 +10,8 @@ from xmodule.errortracker import null_error_tracker
 from xmodule.x_module import XModuleDescriptor
 from xmodule.modulestore.locator import BlockUsageLocator, DescriptionLocator, CourseLocator, VersionTree
 from xmodule.modulestore.exceptions import InsufficientSpecificationError, VersionConflictError
-from xmodule.modulestore import inheritance
+from xmodule.modulestore import inheritance, ModuleStoreBase
 
-from .. import ModuleStoreBase
 from ..exceptions import ItemNotFoundError
 from .definition_lazy_loader import DefinitionLazyLoader
 from .caching_descriptor_system import CachingDescriptorSystem
@@ -62,14 +61,11 @@ class SplitMongoModuleStore(ModuleStoreBase):
             **kwargs
         ), db)
 
-        # TODO add caching of structures to thread_cache to prevent repeated fetches (but not index b/c
-        # it changes w/o having a change in id)
         self.course_index = self.db[collection + '.active_versions']
         self.structures = self.db[collection + '.structures']
         self.definitions = self.db[collection + '.definitions']
 
-        # ??? Code review question: those familiar w/ python threading. Should I instead
-        # use django cache? How should I expire entries?
+        # Code review question: How should I expire entries?
         # _add_cache could use a lru mechanism to control the cache size?
         self.thread_cache = threading.local()
 
@@ -1178,6 +1174,7 @@ class SplitMongoModuleStore(ModuleStoreBase):
         else:
             return DescriptionLocator(definition['_id'])
 
+
     def _block_matches(self, value, qualifiers):
         '''
         Return True or False depending on whether the value (block contents)

common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py

@@ -0,0 +1,507 @@
+'''
+Created on Aug 5, 2013
+
+@author: dmitchell
+'''
+import unittest
+import uuid
+from xmodule.modulestore import Location
+from xmodule.modulestore.locator import BlockUsageLocator
+from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateItemError
+from xmodule.modulestore.loc_mapper_store import LocMapperStore
+
+
+class TestLocationMapper(unittest.TestCase):
+    """
+    Test the location to locator mapper
+    """
+
+    def setUp(self):
+        modulestore_options = {
+            'host': 'localhost',
+            'db': 'test_xmodule',
+            'collection': 'modulestore{0}'.format(uuid.uuid4().hex),
+        }
+
+        # pylint: disable=W0142
+        TestLocationMapper.loc_store = LocMapperStore(**modulestore_options)
+
+    def tearDown(self):
+        dbref = TestLocationMapper.loc_store.db
+        dbref.drop_collection(TestLocationMapper.loc_store.location_map)
+        dbref.connection.close()
+        TestLocationMapper.loc_store = None
+
+    def test_create_map(self):
+        org = 'foo_org'
+        course = 'bar_course'
+        loc_mapper().create_map_entry(Location('i4x', org, course, 'course', 'baz_run'))
+        entry = loc_mapper().location_map.find_one({
+            '_id': {'org': org, 'course': course, 'name': 'baz_run'}
+        })
+        self.assertIsNotNone(entry, "Didn't find entry")
+        self.assertEqual(entry['course_id'], '{}.{}.baz_run'.format(org, course))
+        self.assertEqual(entry['draft_branch'], 'draft')
+        self.assertEqual(entry['prod_branch'], 'published')
+        self.assertEqual(entry['block_map'], {})
+
+        # ensure create_entry does the right thing when not given a course (creates org/course
+        # rather than org/course/run course_id)
+        loc_mapper().create_map_entry(Location('i4x', org, course, 'vertical', 'baz_vert'))
+        entry = loc_mapper().location_map.find_one({
+            '_id': {'org': org, 'course': course}
+        })
+        self.assertIsNotNone(entry, "Didn't find entry")
+        self.assertEqual(entry['course_id'], '{}.{}'.format(org, course))
+
+        course = 'quux_course'
+        # oldname: {category: newname}
+        block_map = {'abc123': {'problem': 'problem2'}}
+        loc_mapper().create_map_entry(
+            Location('i4x', org, course, 'problem', 'abc123', 'draft'),
+            'foo_org.geek_dept.quux_course.baz_run',
+            'wip',
+            'live',
+            block_map)
+        entry = loc_mapper().location_map.find_one({
+            '_id': {'org': org, 'course': course}
+        })
+        self.assertIsNotNone(entry, "Didn't find entry")
+        self.assertEqual(entry['course_id'], 'foo_org.geek_dept.quux_course.baz_run')
+        self.assertEqual(entry['draft_branch'], 'wip')
+        self.assertEqual(entry['prod_branch'], 'live')
+        self.assertEqual(entry['block_map'], block_map)
+
+    def test_translate_location_read_only(self):
+        """
+        Test the variants of translate_location which don't create entries, just decode
+        """
+        # lookup before there are any maps
+        org = 'foo_org'
+        course = 'bar_course'
+        old_style_course_id = '{}/{}/{}'.format(org, course, 'baz_run')
+        with self.assertRaises(ItemNotFoundError):
+            _ = loc_mapper().translate_location(
+                old_style_course_id,
+                Location('i4x', org, course, 'problem', 'abc123'),
+                add_entry_if_missing=False
+            )
+
+        new_style_course_id = '{}.geek_dept.{}.baz_run'.format(org, course)
+        block_map = {'abc123': {'problem': 'problem2'}}
+        loc_mapper().create_map_entry(
+            Location('i4x', org, course, 'course', 'baz_run'),
+            new_style_course_id,
+            block_map=block_map
+        )
+        # only one course matches
+        prob_locator = loc_mapper().translate_location(
+            old_style_course_id,
+            Location('i4x', org, course, 'problem', 'abc123'),
+            add_entry_if_missing=False
+        )
+        self.assertEqual(prob_locator.course_id, new_style_course_id)
+        self.assertEqual(prob_locator.branch, 'published')
+        self.assertEqual(prob_locator.usage_id, 'problem2')
+        # look for w/ only the Location (works b/c there's only one possible course match)
+        prob_locator = loc_mapper().translate_location(
+            None,
+            Location('i4x', org, course, 'problem', 'abc123'),
+            add_entry_if_missing=False
+        )
+        self.assertEqual(prob_locator.course_id, new_style_course_id)
+        # look for non-existent problem
+        with self.assertRaises(ItemNotFoundError):
+            prob_locator = loc_mapper().translate_location(
+                None,
+                Location('i4x', org, course, 'problem', '1def23'),
+                add_entry_if_missing=False
+            )
+
+        # add a distractor course
+        block_map = {'abc123': {'problem': 'problem3'}}
+        loc_mapper().create_map_entry(
+            Location('i4x', org, course, 'course', 'delta_run'),
+            '{}.geek_dept.{}.{}'.format(org, course, 'delta_run'),
+            block_map=block_map
+        )
+        prob_locator = loc_mapper().translate_location(
+            old_style_course_id,
+            Location('i4x', org, course, 'problem', 'abc123'),
+            add_entry_if_missing=False
+        )
+        self.assertEqual(prob_locator.course_id, new_style_course_id)
+        self.assertEqual(prob_locator.usage_id, 'problem2')
+        # look for w/ only the Location (not unique; so, just verify it returns something)
+        prob_locator = loc_mapper().translate_location(
+            None,
+            Location('i4x', org, course, 'problem', 'abc123'),
+            add_entry_if_missing=False
+        )
+        self.assertIsNotNone(prob_locator, "couldn't find ambiguous location")
+
+        # add a default course pointing to the delta_run
+        loc_mapper().create_map_entry(
+            Location('i4x', org, course, 'problem', '789abc123efg456'),
+            '{}.geek_dept.{}.{}'.format(org, course, 'delta_run'),
+            block_map=block_map
+        )
+        # now the ambiguous query should return delta
+        prob_locator = loc_mapper().translate_location(
+            None,
+            Location('i4x', org, course, 'problem', 'abc123'),
+            add_entry_if_missing=False
+        )
+        self.assertEqual(prob_locator.course_id, '{}.geek_dept.{}.{}'.format(org, course, 'delta_run'))
+        self.assertEqual(prob_locator.usage_id, 'problem3')
+
+        # get the draft one (I'm sorry this is getting long)
+        prob_locator = loc_mapper().translate_location(
+            None,
+            Location('i4x', org, course, 'problem', 'abc123'),
+            published=False,
+            add_entry_if_missing=False
+        )
+        self.assertEqual(prob_locator.course_id, '{}.geek_dept.{}.{}'.format(org, course, 'delta_run'))
+        self.assertEqual(prob_locator.usage_id, 'problem3')
+        self.assertEqual(prob_locator.branch, 'draft')
+
+    def test_translate_location_dwim(self):
+        """
+        Test the location translation mechanisms which try to do-what-i-mean by creating new
+        entries for never seen queries.
+        """
+        org = 'foo_org'
+        course = 'bar_course'
+        old_style_course_id = '{}/{}/{}'.format(org, course, 'baz_run')
+        problem_name = 'abc123abc123abc123abc123abc123f9'
+        location = Location('i4x', org, course, 'problem', problem_name)
+        prob_locator = loc_mapper().translate_location(
+            old_style_course_id,
+            location,
+            add_entry_if_missing=True
+        )
+        new_style_course_id = '{}.{}.{}'.format(org, course, 'baz_run')
+        self.assertEqual(prob_locator.course_id, new_style_course_id)
+        self.assertEqual(prob_locator.branch, 'published')
+        self.assertEqual(prob_locator.usage_id, 'problemabc')
+        # look for w/ only the Location (works b/c there's only one possible course match)
+        prob_locator = loc_mapper().translate_location(
+            None,
+            location,
+            add_entry_if_missing=True
+        )
+        self.assertEqual(prob_locator.course_id, new_style_course_id)
+
+        # add a distractor course
+        loc_mapper().create_map_entry(
+            Location('i4x', org, course, 'course', 'delta_run'),
+            '{}.geek_dept.{}.{}'.format(org, course, 'delta_run'),
+            block_map={problem_name: {'problem': 'problem3'}}
+        )
+        prob_locator = loc_mapper().translate_location(
+            old_style_course_id,
+            location,
+            add_entry_if_missing=True
+        )
+        self.assertEqual(prob_locator.course_id, new_style_course_id)
+        self.assertEqual(prob_locator.usage_id, 'problemabc')
+        # look for w/ only the Location (not unique; so, just verify it returns something)
+        prob_locator = loc_mapper().translate_location(
+            None,
+            location,
+            add_entry_if_missing=True
+        )
+        self.assertIsNotNone(prob_locator, "couldn't find ambiguous location")
+
+        # add a default course pointing to the delta_run
+        loc_mapper().create_map_entry(
+            Location('i4x', org, course, 'problem', '789abc123efg456'),
+            '{}.geek_dept.{}.{}'.format(org, course, 'delta_run'),
+            block_map={problem_name: {'problem': 'problem3'}}
+        )
+        # now the ambiguous query should return delta
+        prob_locator = loc_mapper().translate_location(
+            None,
+            location,
+            add_entry_if_missing=False
+        )
+        self.assertEqual(prob_locator.course_id, '{}.geek_dept.{}.{}'.format(org, course, 'delta_run'))
+        self.assertEqual(prob_locator.usage_id, 'problem3')
+
+    def test_translate_locator(self):
+        """
+        tests translate_locator_to_location(BlockUsageLocator)
+        """
+        # lookup for non-existent course
+        org = 'foo_org'
+        course = 'bar_course'
+        new_style_course_id = '{}.geek_dept.{}.baz_run'.format(org, course)
+        prob_locator = BlockUsageLocator(
+            course_id=new_style_course_id,
+            usage_id='problem2'
+        )
+        prob_location = loc_mapper().translate_locator_to_location(prob_locator)
+        self.assertIsNone(prob_location, 'found entry in empty map table')
+
+        loc_mapper().create_map_entry(
+            Location('i4x', org, course, 'course', 'baz_run'),
+            new_style_course_id,
+            block_map={
+                'abc123': {'problem': 'problem2'},
+                '48f23a10395384929234': {'chapter': 'chapter48f'}
+            }
+        )
+        # only one course matches
+        prob_location = loc_mapper().translate_locator_to_location(prob_locator)
+        # default branch
+        self.assertEqual(prob_location, Location('i4x', org, course, 'problem', 'abc123', None))
+        # explicit branch
+        prob_locator = BlockUsageLocator(prob_locator, branch='draft')
+        prob_location = loc_mapper().translate_locator_to_location(prob_locator)
+        self.assertEqual(prob_location, Location('i4x', org, course, 'problem', 'abc123', 'draft'))
+        prob_locator = BlockUsageLocator(
+            course_id=new_style_course_id, usage_id='problem2', branch='production'
+        )
+        prob_location = loc_mapper().translate_locator_to_location(prob_locator)
+        self.assertEqual(prob_location, Location('i4x', org, course, 'problem', 'abc123', None))
+        # same for chapter except chapter cannot be draft in old system
+        chap_locator = BlockUsageLocator(
+            course_id=new_style_course_id,
+            usage_id='chapter48f'
+        )
+        chap_location = loc_mapper().translate_locator_to_location(chap_locator)
+        self.assertEqual(chap_location, Location('i4x', org, course, 'chapter', '48f23a10395384929234'))
+        # explicit branch
+        chap_locator = BlockUsageLocator(chap_locator, branch='draft')
+        chap_location = loc_mapper().translate_locator_to_location(chap_locator)
+        self.assertEqual(chap_location, Location('i4x', org, course, 'chapter', '48f23a10395384929234'))
+        chap_locator = BlockUsageLocator(
+            course_id=new_style_course_id, usage_id='chapter48f', branch='production'
+        )
+        chap_location = loc_mapper().translate_locator_to_location(chap_locator)
+        self.assertEqual(chap_location, Location('i4x', org, course, 'chapter', '48f23a10395384929234'))
+
+        # look for non-existent problem
+        prob_locator2 = BlockUsageLocator(
+            course_id=new_style_course_id,
+            branch='draft',
+            usage_id='problem3'
+        )
+        prob_location = loc_mapper().translate_locator_to_location(prob_locator2)
+        self.assertIsNone(prob_location, 'Found non-existent problem')
+
+        # add a distractor course
+        new_style_course_id = '{}.geek_dept.{}.{}'.format(org, course, 'delta_run')
+        loc_mapper().create_map_entry(
+            Location('i4x', org, course, 'course', 'delta_run'),
+            new_style_course_id,
+            block_map={'abc123': {'problem': 'problem3'}}
+        )
+        prob_location = loc_mapper().translate_locator_to_location(prob_locator)
+        self.assertEqual(prob_location, Location('i4x', org, course, 'problem', 'abc123', None))
+
+        # add a default course pointing to the delta_run
+        loc_mapper().create_map_entry(
+            Location('i4x', org, course, 'problem', '789abc123efg456'),
+            new_style_course_id,
+            block_map={'abc123': {'problem': 'problem3'}}
+        )
+        # now query delta (2 entries point to it)
+        prob_locator = BlockUsageLocator(
+            course_id=new_style_course_id,
+            branch='production',
+            usage_id='problem3'
+        )
+        prob_location = loc_mapper().translate_locator_to_location(prob_locator)
+        self.assertEqual(prob_location, Location('i4x', org, course, 'problem', 'abc123'))
+
+    def test_add_block(self):
+        """
+        Test add_block_location_translator(location, old_course_id=None, usage_id=None)
+        """
+        # call w/ no matching courses
+        org = 'foo_org'
+        course = 'bar_course'
+        old_style_course_id = '{}/{}/{}'.format(org, course, 'baz_run')
+        problem_name = 'abc123abc123abc123abc123abc123f9'
+        location = Location('i4x', org, course, 'problem', problem_name)
+        with self.assertRaises(ItemNotFoundError):
+            loc_mapper().add_block_location_translator(location)
+        with self.assertRaises(ItemNotFoundError):
+            loc_mapper().add_block_location_translator(location, old_style_course_id)
+
+        # w/ one matching course
+        new_style_course_id = '{}.{}.{}'.format(org, course, 'baz_run')
+        loc_mapper().create_map_entry(
+            Location('i4x', org, course, 'course', 'baz_run'),
+            new_style_course_id,
+        )
+        new_usage_id = loc_mapper().add_block_location_translator(location)
+        self.assertEqual(new_usage_id, 'problemabc')
+        # look it up
+        translated_loc = loc_mapper().translate_location(old_style_course_id, location, add_entry_if_missing=False)
+        self.assertEqual(translated_loc.course_id, new_style_course_id)
+        self.assertEqual(translated_loc.usage_id, new_usage_id)
+
+        # w/ one distractor which has one entry already
+        new_style_course_id = '{}.geek_dept.{}.{}'.format(org, course, 'delta_run')
+        loc_mapper().create_map_entry(
+            Location('i4x', org, course, 'course', 'delta_run'),
+            new_style_course_id,
+            block_map={'48f23a10395384929234': {'chapter': 'chapter48f'}}
+        )
+        # try adding the one added before
+        new_usage_id2 = loc_mapper().add_block_location_translator(location)
+        self.assertEqual(new_usage_id, new_usage_id2)
+        # it should be in the distractor now
+        new_location = loc_mapper().translate_locator_to_location(
+            BlockUsageLocator(course_id=new_style_course_id, usage_id=new_usage_id2)
+        )
+        self.assertEqual(new_location, location)
+        # add one close to the existing chapter (cause name collision)
+        location = Location('i4x', org, course, 'chapter', '48f23a103953849292341234567890ab')
+        new_usage_id = loc_mapper().add_block_location_translator(location)
+        self.assertRegexpMatches(new_usage_id, r'^chapter48f\d')
+        # retrievable from both courses
+        new_location = loc_mapper().translate_locator_to_location(
+            BlockUsageLocator(course_id=new_style_course_id, usage_id=new_usage_id)
+        )
+        self.assertEqual(new_location, location)
+        new_location = loc_mapper().translate_locator_to_location(
+            BlockUsageLocator(course_id='{}.{}.{}'.format(org, course, 'baz_run'), usage_id=new_usage_id)
+        )
+        self.assertEqual(new_location, location)
+
+        # provoke duplicate item errors
+        location = location.replace(name='44f23a103953849292341234567890ab')
+        with self.assertRaises(DuplicateItemError):
+            loc_mapper().add_block_location_translator(location, usage_id=new_usage_id)
+        new_usage_id = loc_mapper().add_block_location_translator(location, old_course_id=old_style_course_id)
+        other_course_old_style = '{}/{}/{}'.format(org, course, 'delta_run')
+        new_usage_id2 = loc_mapper().add_block_location_translator(
+            location,
+            old_course_id=other_course_old_style,
+            usage_id='{}b'.format(new_usage_id)
+        )
+        with self.assertRaises(DuplicateItemError):
+            loc_mapper().add_block_location_translator(location)
+
+    def test_update_block(self):
+        """
+        test update_block_location_translator(location, usage_id, old_course_id=None)
+        """
+        org = 'foo_org'
+        course = 'bar_course'
+        new_style_course_id = '{}.geek_dept.{}.baz_run'.format(org, course)
+        loc_mapper().create_map_entry(
+            Location('i4x', org, course, 'course', 'baz_run'),
+            new_style_course_id,
+            block_map={
+                'abc123': {'problem': 'problem2'},
+                '48f23a10395384929234': {'chapter': 'chapter48f'},
+                '1': {'chapter': 'chapter1', 'problem': 'problem1'},
+            }
+        )
+        new_style_course_id2 = '{}.geek_dept.{}.delta_run'.format(org, course)
+        loc_mapper().create_map_entry(
+            Location('i4x', org, course, 'course', 'delta_run'),
+            new_style_course_id2,
+            block_map={
+                'abc123': {'problem': 'problem3'},
+                '48f23a10395384929234': {'chapter': 'chapter48b'},
+                '1': {'chapter': 'chapter2', 'problem': 'problem2'},
+            }
+        )
+        location = Location('i4x', org, course, 'problem', '1')
+        # change in all courses to same value
+        loc_mapper().update_block_location_translator(location, 'problem1')
+        trans_loc = loc_mapper().translate_locator_to_location(
+            BlockUsageLocator(course_id=new_style_course_id, usage_id='problem1')
+        )
+        self.assertEqual(trans_loc, location)
+        trans_loc = loc_mapper().translate_locator_to_location(
+            BlockUsageLocator(course_id=new_style_course_id2, usage_id='problem1')
+        )
+        self.assertEqual(trans_loc, location)
+        # try to change to overwrite used usage_id
+        location = Location('i4x', org, course, 'chapter', '48f23a10395384929234')
+        with self.assertRaises(DuplicateItemError):
+            loc_mapper().update_block_location_translator(location, 'chapter2')
+        # just change the one course
+        loc_mapper().update_block_location_translator(location, 'chapter2', '{}/{}/{}'.format(org, course, 'baz_run'))
+        trans_loc = loc_mapper().translate_locator_to_location(
+            BlockUsageLocator(course_id=new_style_course_id, usage_id='chapter2')
+        )
+        self.assertEqual(trans_loc.name, '48f23a10395384929234')
+        # but this still points to the old
+        trans_loc = loc_mapper().translate_locator_to_location(
+            BlockUsageLocator(course_id=new_style_course_id2, usage_id='chapter2')
+        )
+        self.assertEqual(trans_loc.name, '1')
+
+    def test_delete_block(self):
+        """
+        test delete_block_location_translator(location, old_course_id=None)
+        """
+        org = 'foo_org'
+        course = 'bar_course'
+        new_style_course_id = '{}.geek_dept.{}.baz_run'.format(org, course)
+        loc_mapper().create_map_entry(
+            Location('i4x', org, course, 'course', 'baz_run'),
+            new_style_course_id,
+            block_map={
+                'abc123': {'problem': 'problem2'},
+                '48f23a10395384929234': {'chapter': 'chapter48f'},
+                '1': {'chapter': 'chapter1', 'problem': 'problem1'},
+            }
+        )
+        new_style_course_id2 = '{}.geek_dept.{}.delta_run'.format(org, course)
+        loc_mapper().create_map_entry(
+            Location('i4x', org, course, 'course', 'delta_run'),
+            new_style_course_id2,
+            block_map={
+                'abc123': {'problem': 'problem3'},
+                '48f23a10395384929234': {'chapter': 'chapter48b'},
+                '1': {'chapter': 'chapter2', 'problem': 'problem2'},
+            }
+        )
+        location = Location('i4x', org, course, 'problem', '1')
+        # delete from all courses
+        loc_mapper().delete_block_location_translator(location)
+        self.assertIsNone(loc_mapper().translate_locator_to_location(
+            BlockUsageLocator(course_id=new_style_course_id, usage_id='problem1')
+        ))
+        self.assertIsNone(loc_mapper().translate_locator_to_location(
+            BlockUsageLocator(course_id=new_style_course_id2, usage_id='problem2')
+        ))
+        # delete from one course
+        location = location.replace(name='abc123')
+        loc_mapper().delete_block_location_translator(location, '{}/{}/{}'.format(org, course, 'baz_run'))
+        with self.assertRaises(ItemNotFoundError):
+            loc_mapper().translate_location(
+                '{}/{}/{}'.format(org, course, 'baz_run'),
+                location,
+                add_entry_if_missing=False
+            )
+        locator = loc_mapper().translate_location(
+            '{}/{}/{}'.format(org, course, 'delta_run'),
+            location,
+            add_entry_if_missing=False
+        )
+        self.assertEqual(locator.usage_id, 'problem3')
+
+
+#==================================
+# functions to mock existing services
+def loc_mapper():
+    """
+    Mocks the global location mapper.
+    """
+    return TestLocationMapper.loc_store
+
+
+def render_to_template_mock(*_args):
+    """
+    Mocks the mako render_to_template w/ a noop
+    """

common/lib/xmodule/xmodule/modulestore/tests/test_locators.py

@@ -252,12 +252,18 @@ class LocatorTest(TestCase):
 
     def check_course_locn_fields(self, testobj, msg, version_guid=None,
                                  course_id=None, branch=None):
+        """
+        Checks the version, course_id, and branch in testobj
+        """
         self.assertEqual(testobj.version_guid, version_guid, msg)
         self.assertEqual(testobj.course_id, course_id, msg)
         self.assertEqual(testobj.branch, branch, msg)
 
     def check_block_locn_fields(self, testobj, msg, version_guid=None,
                                 course_id=None, branch=None, block=None):
+        """
+        Does adds a block id check over and above the check_course_locn_fields tests
+        """
         self.check_course_locn_fields(testobj, msg, version_guid, course_id,
                                       branch)
         self.assertEqual(testobj.usage_id, block)

common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py

@@ -13,7 +13,7 @@ HOST = 'localhost'
 PORT = 27017
 DB = 'test_mongo_%s' % uuid4().hex
 COLLECTION = 'modulestore'
-FS_ROOT = DATA_DIR  # TODO (vshnayder): will need a real fs_root for testing load_item
+FS_ROOT = DATA_DIR
 DEFAULT_CLASS = 'xmodule.raw_module.RawDescriptor'
 RENDER_TEMPLATE = lambda t_n, d, ctx = None, nsp = 'main': ''
 
@@ -54,6 +54,9 @@ class TestMixedModuleStore(object):
     '''Tests!'''
     @classmethod
     def setupClass(cls):
+        """
+        Set up the database for testing
+        """
         cls.connection = pymongo.connection.Connection(HOST, PORT)
         cls.connection.drop_database(DB)
         cls.fake_location = Location(['i4x', 'foo', 'bar', 'vertical', 'baz'])
@@ -66,10 +69,16 @@ class TestMixedModuleStore(object):
 
     @classmethod
     def teardownClass(cls):
+        """
+        Clear out database after test has completed
+        """
         cls.destroy_db(cls.connection)
 
     @staticmethod
     def initdb():
+        """
+        Initialize the database and import one test course into it
+        """
         # connect to the db
         _options = {}
         _options.update(OPTIONS)
@@ -92,7 +101,9 @@ class TestMixedModuleStore(object):
 
     @staticmethod
     def destroy_db(connection):
-        # Destroy the test db.
+        """
+        Destroy the test db.
+        """
         connection.drop_database(DB)
 
     def setUp(self):
@@ -204,6 +215,7 @@ class TestMixedModuleStore(object):
         module = self.store.get_course(XML_COURSEID2)
         assert_equals(module.location.course, 'simple')
 
+    # pylint: disable=E1101
     def test_get_parent_locations(self):
         parents = self.store.get_parent_locations(
             Location(['i4x', self.import_org, self.import_course, 'chapter', 'Overview']),
@@ -223,6 +235,7 @@ class TestMixedModuleStore(object):
         assert_equals(Location(parents[0]).course, 'toy')
         assert_equals(Location(parents[0]).name, '2012_Fall')
 
+    # pylint: disable=W0212
     def test_set_modulestore_configuration(self):
         config = {'foo': 'bar'}
         self.store.set_modulestore_configuration(config)

common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py

@@ -120,8 +120,30 @@ class TestMongoModuleStore(object):
                 '{0} is a template course'.format(course)
             )
 
+    def test_static_tab_names(self):
+        courses = self.store.get_courses()
+
+        def get_tab_name(index):
+            """
+            Helper function for pulling out the name of a given static tab.
+
+            Assumes the information is desired for courses[1] ('toy' course).
+            """
+            return courses[1].tabs[index]['name']
+
+        # There was a bug where model.save was not getting called after the static tab name
+        # was set set for tabs that have a URL slug. 'Syllabus' and 'Resources' fall into that
+        # category, but for completeness, I'm also testing 'Course Info' and 'Discussion' (no url slug).
+        assert_equals('Course Info', get_tab_name(1))
+        assert_equals('Syllabus', get_tab_name(2))
+        assert_equals('Resources', get_tab_name(3))
+        assert_equals('Discussion', get_tab_name(4))
+
 
 class TestMongoKeyValueStore(object):
+    """
+    Tests for MongoKeyValueStore.
+    """
 
     def setUp(self):
         self.data = {'foo': 'foo_value'}
@@ -131,6 +153,9 @@ class TestMongoKeyValueStore(object):
         self.kvs = MongoKeyValueStore(self.data, self.children, self.metadata, self.location, 'category')
 
     def _check_read(self, key, expected_value):
+        """
+        Asserts the get and has methods.
+        """
         assert_equals(expected_value, self.kvs.get(key))
         assert self.kvs.has(key)
 

common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py

@@ -992,8 +992,8 @@ class TestInheritance(SplitModuleTest):
 # This mocks the django.modulestore() function and is intended purely to disentangle
 # the tests from django
 def modulestore():
-    def load_function(path):
-        module_path, _, name = path.rpartition('.')
+    def load_function(engine_path):
+        module_path, _, name = engine_path.rpartition('.')
         return getattr(import_module(module_path), name)
 
     if SplitModuleTest.modulestore is None:

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

@@ -479,6 +479,7 @@ class XMLModuleStore(ModuleStoreBase):
                             if tab.get('url_slug') == slug:
                                 module.display_name = tab['name']
                     module.data_dir = course_dir
+                    module.save()
                     self.modules[course_descriptor.id][module.location] = module
                 except Exception, e:
                     logging.exception("Failed to load {0}. Skipping... Exception: {1}".format(filepath, str(e)))

common/lib/xmodule/xmodule/modulestore/xml_importer.py

@@ -242,9 +242,37 @@ def import_course_draft(xml_module_store, store, draft_store, course_data_path,
     for dirname, dirnames, filenames in os.walk(draft_dir + "/vertical"):
         for filename in filenames:
             module_path = os.path.join(dirname, filename)
-            with open(module_path) as f:
+            with open(module_path, 'r') as f:
                 try:
-                    xml = f.read().decode('utf-8')
+                    # note, on local dev it seems like OSX will put some extra files in
+                    # the directory with "quarantine" information. These files are
+                    # binary files and will throw exceptions when we try to parse
+                    # the file as an XML string. Let's make sure we're
+                    # dealing with a string before ingesting
+                    data = f.read()
+
+                    try:
+                        xml = data.decode('utf-8')
+                    except UnicodeDecodeError, err:
+                        # seems like on OSX localdev, the OS is making quarantine files
+                        # in the unzip directory when importing courses
+                        # so if we blindly try to enumerate through the directory, we'll try
+                        # to process a bunch of binary quarantine files (which are prefixed with a '._' character
+                        # which will dump a bunch of exceptions to the output, although they are harmless.
+                        #
+                        # Reading online docs there doesn't seem to be a good means to detect a 'hidden'
+                        # file that works well across all OS environments. So for now, I'm using
+                        # OSX's utilization of a leading '.' in the filename to indicate a system hidden
+                        # file.
+                        #
+                        # Better yet would be a way to figure out if this is a binary file, but I
+                        # haven't found a good way to do this yet.
+                        #
+                        if filename.startswith('._'):
+                            continue
+                        # Not a 'hidden file', then re-raise exception
+                        raise err
+
                     descriptor = system.process_xml(xml)
 
                     def _import_module(module):

common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py

@@ -527,7 +527,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild):
                 feedback = "".join(feedback_items)
             else:
                 feedback = feedback_items
-            score = int(median(score_result['score']))
+            score = int(round(median(score_result['score'])))
         else:
             # This is for instructor and ML grading
             feedback, rubric_score = self._format_feedback(score_result, system)

common/lib/xmodule/xmodule/tests/test_combined_open_ended.py

@@ -291,6 +291,30 @@ class OpenEndedModuleTest(unittest.TestCase):
                'xqueue_body': json.dumps(score_msg)}
         self.openendedmodule.update_score(get, self.test_system)
 
+    def update_score_multiple(self):
+        self.openendedmodule.new_history_entry("New Entry")
+        feedback = {
+            "success": True,
+            "feedback": "Grader Feedback"
+        }
+        score_msg = {
+            'correct': True,
+            'score': [0, 1],
+            'msg': 'Grader Message',
+            'feedback': [json.dumps(feedback), json.dumps(feedback)],
+            'grader_type': 'PE',
+            'grader_id': ['1', '2'],
+            'submission_id': '1',
+            'success': True,
+            'rubric_scores': [[0], [0]],
+            'rubric_scores_complete': [True, True],
+            'rubric_xml': [etree.tostring(self.rubric), etree.tostring(self.rubric)]
+        }
+        get = {'queuekey': "abcd",
+               'xqueue_body': json.dumps(score_msg)}
+        self.openendedmodule.update_score(get, self.test_system)
+
+
     def test_latest_post_assessment(self):
         self.update_score_single()
         assessment = self.openendedmodule.latest_post_assessment(self.test_system)
@@ -298,11 +322,19 @@ class OpenEndedModuleTest(unittest.TestCase):
         # check for errors
         self.assertFalse('errors' in assessment)
 
-    def test_update_score(self):
+    def test_update_score_single(self):
         self.update_score_single()
         score = self.openendedmodule.latest_score()
         self.assertEqual(score, 4)
 
+    def test_update_score_multiple(self):
+        """
+        Tests that a score of [0, 1] gets aggregated to 1.  A change in behavior added by @jbau
+        """
+        self.update_score_multiple()
+        score = self.openendedmodule.latest_score()
+        self.assertEquals(score, 1)
+
 
 class CombinedOpenEndedModuleTest(unittest.TestCase):
     """

common/static/coffee/src/logger.coffee

@@ -42,5 +42,7 @@ class @Logger
           page: window.location.href
         async: false
 
-# Keeping this for compatibility issue only.
+
+# log_event exists for compatibility reasons
+# and will soon be deprecated.
 @log_event = Logger.log

common/static/js/pdfviewer.js

@@ -157,7 +157,7 @@ PDFJS.disableWorker = true;
             }
 
             // Update logging:
-            log_event("book", { "type" : "gotopage", "old" : pageNum, "new" : num });
+            Logger.log("book", { "type" : "gotopage", "old" : pageNum, "new" : num });
 
             parentElement = viewerElement;
             while (parentElement.hasChildNodes())
@@ -207,7 +207,7 @@ PDFJS.disableWorker = true;
             if (pageNum <= 1)
                 return;
             renderPage(pageNum - 1);
-            log_event("book", { "type" : "prevpage", "new" : pageNum });
+            Logger.log("book", { "type" : "prevpage", "new" : pageNum });
         }
 
         // Go to next page
@@ -215,7 +215,7 @@ PDFJS.disableWorker = true;
             if (pageNum >= pdfDocument.numPages)
                 return;
             renderPage(pageNum + 1);
-            log_event("book", { "type" : "nextpage", "new" : pageNum });
+            Logger.log("book", { "type" : "nextpage", "new" : pageNum });
         }
 
         selectScaleOption = function(value) {

common/test/data/toy/course/2012_Fall.xml

@@ -7,6 +7,7 @@
       <html url_name="toyhtml"/>
       <html url_name="nonportable"/>
       <html url_name="nonportable_link"/>
+      <html url_name="badlink"/>
       <video url_name="Video_Resources" youtube_id_1_0="1bK-WdDi6Qw" display_name="Video Resources"/>
     </videosequence>
     <video url_name="Welcome" youtube_id_1_0="p2Q6BrNhdh8" display_name="Welcome"/>

common/test/data/toy/html/badlink.html

@@ -0,0 +1 @@
+<img src="/static//file.jpg" />

common/test/data/toy/html/badlink.xml

@@ -0,0 +1 @@
+<html filename="badlink.html"/>

docs/Makefile

@@ -1,7 +1,13 @@
 
 .PHONY: html
 
+Q_FLAG =
+
+ifeq ($(quiet), true)
+Q_FLAG = quiet=true
+endif
+
 html:
-	@cd $(CURDIR)/data && make html
-	@cd $(CURDIR)/course_authors && make html
-	@cd $(CURDIR)/developers && make html
+	@cd $(CURDIR)/data && make html $(Q_FLAG)
+	@cd $(CURDIR)/course_authors && make html $(Q_FLAG)
+	@cd $(CURDIR)/developers && make html $(Q_FLAG)

docs/course_authors/Makefile

@@ -12,10 +12,16 @@ ifeq ($(shell which $(SPHINXBUILD) >/dev/null 2>&1; echo $$?), 1)
 $(error The '$(SPHINXBUILD)' command was not found. Make sure you have Sphinx installed, then set the SPHINXBUILD environment variable to point to the full path of the '$(SPHINXBUILD)' executable. Alternatively you can add the directory with the executable to your PATH. If you don't have Sphinx installed, grab it from http://sphinx-doc.org/)
 endif
 
+Q_FLAG        = 
+
+ifeq ($(quiet), true)
+Q_FLAG = -q
+endif
+
 # Internal variables.
 PAPEROPT_a4     = -D latex_paper_size=a4
 PAPEROPT_letter = -D latex_paper_size=letter
-ALLSPHINXOPTS   = -d $(BUILDDIR)/doctrees -c source $(PAPEROPT_$(PAPER)) $(SPHINXOPTS) source
+ALLSPHINXOPTS   = -q -d $(BUILDDIR)/doctrees -c source $(PAPEROPT_$(PAPER)) $(SPHINXOPTS) source
 # the i18n builder cannot share the environment and doctrees with the others
 I18NSPHINXOPTS  = $(PAPEROPT_$(PAPER)) $(SPHINXOPTS) source
 

docs/course_authors/source/appendices/e.rst

@@ -473,12 +473,39 @@ Answers with scripts
 
   .. image:: ../Images/numericalresponse.png
 
-<numericalresponse>
+
+``<numericalresponse>``
+
++------------+----------------------------------------------+-------------------------------+
+| Attribute  |                 Description                  |              Notes            |
++============+==============================================+===============================+
+| ``answer`` | A value to which student input must be       | Note that any numeric         |
+|            | equivalent. Note that this expression can be | expression provided by the    |
+|            | expressed in terms of a variable that is     | student will be automatically |
+|            | computed in a script provided in the problem | simplified on the grader's    |
+|            | by preceding the appropriate variable name   | backend.                      |
+|            | with a dollar sign.                          |                               |
+|            |                                              |                               |
+|            | This answer will be evaluated similar to a   |                               |
+|            | student's input. Thus '1/3' and 'sin(pi/5)'  |                               |
+|            | are valid, as well as simpler expressions,   |                               |
+|            | such as '0.3' and '42'                       |                               |
++------------+----------------------------------------------+-------------------------------+
 
 
-  .. image:: ../Images/numericalresponse2.png
++------------------------+--------------------------------------------+--------------------------------------+
+|       Children         |                 Description                |                 Notes                |
++========================+============================================+======================================+
+| ``responseparam``      | used to specify a tolerance on the accepted|                                      |
+|                        | values of a number. See description below. |                                      |
++------------------------+--------------------------------------------+--------------------------------------+
+|``formulaequationinput``| An input specifically for taking math      |                                      |
+|                        | input from students. See below.            |                                      |
++------------------------+--------------------------------------------+--------------------------------------+
+| ``textline``           | A format to take input from students, see  | Deprecated for NumericalResponse.    |
+|                        | description below.                         | Use ``formulaequationinput`` instead.|
++------------------------+--------------------------------------------+--------------------------------------+
 
-Children may include ``<formulaequationinput/>``.
 
 <responseparam>
 
@@ -494,7 +521,9 @@ size      (optional) defines the size (i.e. the width)
           typing their math expression.
 ========= ============================================= =====
 
-<textline> (While <textline /> is supported, its use is extremely discouraged. We urge usage of <formulaequationinput />. See the opening paragraphs of the Numerical Response section for more information.)
+<textline> (While <textline /> is supported, its use is extremely discouraged.
+We urge usage of <formulaequationinput />. See the opening paragraphs of the
+`Numerical Response`_ section for more information.)
 
   .. image:: ../Images/numericalresponse5.png
 
@@ -563,7 +592,8 @@ by ``||``. For example, an input of ``1 || 2`` would represent the resistance
 of a pair of parallel resistors (of resistance 1 and 2 ohms), evaluating to 2/3
 (ohms).
 
-At the time of writing, factorials written in the form '3!' are invalid, but there is a workaround. Students can specify ``fact(3)`` or ``factorial(3)`` to
+At the time of writing, factorials written in the form '3!' are invalid, but
+there is a workaround. Students can specify ``fact(3)`` or ``factorial(3)`` to
 access the factorial function.
 
 The default included functions are the following:
@@ -573,7 +603,8 @@ The default included functions are the following:
 - Other common functions: sqrt, log10, log2, ln, exp, abs
 - Factorial: ``fact(3)`` or ``factorial(3)`` are valid. However, you must take
   care to only input integers. For example, ``fact(1.5)`` would fail.
-- Hyperbolic trig functions and their inverses: sinh, cosh, tanh, sech, csch, coth, arcsinh, arccosh, arctanh, arcsech, arccsch, arccoth
+- Hyperbolic trig functions and their inverses: sinh, cosh, tanh, sech, csch,
+  coth, arcsinh, arccosh, arctanh, arcsech, arccsch, arccoth
 
 .. raw:: latex
   

docs/course_authors/source/establish_course_settings.rst

@@ -1,221 +1,232 @@
 *************************
 Establish Course Settings
 *************************
- 
+
 Add Collaborators
 *****************
- 
- 
+
+
     Studio has support for rudimentary collaborative editing of a course. Users must have registered at studio.edge.edx.org, and must have activated their account via the mail link. If a user is not found, you will be notified.
- 
- 
+
+
     Before you add a new user, consider the following.
- 
- 
+
+
     · Invited users have full permissions to edit your course, including deleting content created by anyone else.
- 
- 
+
+
     · Invited users cannot currently grant new permissions on the course.
- 
- 
+
+
     · Editing conflicts are currently not managed. Thus, the state of the course might change between refreshes of the page.
- 
- 
+
+
     To give another user permission to edit your course:
- 
- 
+
+
     1. On the navigation bar, click **Course Settings**, and then click **Course Team**.
- 
- 
+
+
     .. image:: Images/image115.png
-  
- 
- 
+
+
+
     2. Click **New User**.
- 
- 
+
+
     .. image:: Images/image117.png
- 
- 
+
+
     3. In the **email** box, type the  mail address of the user, and then click **Add User**.
 
 
 .. raw:: latex
-  
+
       \newpage %
 
- 
+
 
 Add Manual Policy Data
 **********************
 
 
- 
+
     You can add manual policy data on the **Advanced Settings** page. These advanced configuration options are specified using JSON key and value
     pairs.
- 
- 
+
+
     You should only add manual policy data if you are very familiar with valid configuration key value pairs and the ways these pairs will affect your course.
     Errors on this page can cause significant problems with your course.
- 
- 
+
+
     The edX program managers can help you learn about how to apply these settings.
- 
- 
+
+
     1. On the navigation bar, click **Course Settings**, and then click **Advanced Settings**.
- 
- 
+
+
     2. Click **New Manual Policy** .
- 
- 
-      .. image:: Images/image119.png  
- 
- 
+
+
+      .. image:: Images/image119.png
+
+
     3. In the **Policy Key** box, enter the policy key.
- 
- 
+
+
     4. In the **Policy Value** box, enter the value of the policy.
 
 
 .. raw:: latex
-  
+
       \newpage %
- 
- 
-Add Course Catalog Information
-******************************
- 
- 
+
+
+Add About Page Information
+***************************
+
+
     To add scheduling information, a description, and other information for your course, use the **Course Settings** menu.
- 
- 
-     .. image:: Images/image121.png  
- 
- 
-    This takes you to the 
+
+
+     .. image:: Images/image121.png
+
+
+    This takes you to the
 
 Schedule and Details Page
 =========================
- 
- 
-1. At the top of this page, you will find a section with the **Basic Information** for your course. It is here that you can locate the title of your course and find the URL for your course, which you can  mail to students to invite students to enroll in your course. 
 
- .. image:: Images/image281.png   
- 
- 
+
+1. At the top of this page, you will find a section with the **Basic Information** for your course. It is here that you can locate the title of your course and find the URL for your course, which you can  mail to students to invite students to enroll in your course.
+
+ .. image:: Images/image281.png
+
+
 2. In the **Course Schedule** section, enter the date you want your course to start in the **Course Start Date** box, and then enter the time you want your course to start in the **Course** **Start Time** box.
- 
- 
-.. note::
-    
-    The Course Start Time on this screen will reflect the current time zone in your browser, depending on your geography. Course start times for students will show as UTC on Edge.
-        
- 
-3. In the **Course Schedule** section, enter the date you want your course to end in the **Course** **End Date **
-    box, and then enter the time you want your course to end in the **Course** **End Time** box.
- 
 
-Add Enrollment Information 
+
+.. note::
+
+    The Course Start Time on this screen will reflect the current time zone in your browser, depending on your geography. Course start times for students will show as UTC on Edge.
+
+
+3. In the **Course Schedule** section, enter the date you want your course to end in the **Course** **End Date**
+    box, and then enter the time you want your course to end in the **Course** **End Time** box.
+
+
+Add Enrollment Information
 ==========================
- 
- 
+
+
 1. On the navigation bar, click **Course **Settings, and then click **Schedule & Details** .
- 
- 
+
+
 2. In the **Course Schedule** section, enter the date you want enrollment for your course to start in the **Enrollment Start Date** box, and then enter the time you want enrollment for your course to start in the **Enrollment Start Time** box.
- 
- 
+
+
 3. In the **Course Schedule** section, enter the date you want enrollment for your course to end in the **Enrollment End Date**
 box, and then enter the time you want enrollment for your course to end in the **Enrollment End Time** box.
- 
- 
+
+
 .. note::
 
     The Enrollment dates on this screen will reflect the current time zone in your browser, depending on your geography. Enrollment times for students will show as UTC on Edge.
-        
 
-  
-Add a Course Overview 
+
+
+Add a Course Overview
 =====================
- 
- 
-1. On the navigation bar, click **Course Settings**, and then click   ** Schedule & Details   ** .
- 
- 
+
+
+1. On the navigation bar, click **Course Settings**, and then click   **Schedule & Details** .
+
+
 2. Scroll down to the **Introducing Your Course** section, and then locate the **Course Overview** box.
 
 .. image:: Images/image123.png
-   
 
- 
- 
-3. In the **Course Overview** box, enter a description of your course. 
+
+
+
+3. In the **Course Overview** box, enter a description of your course.
 
 
 The content for this box must be formated in HTML. For a template that you
 can use that includes placeholders, see :doc:`appendices/a`.
 
-    
- 
+
+
 If your course has prerequisites, you can include that information in the course overview.
- 
- 
+
+
 .. note::
 
     There is no save button. Studio automatically saves your changes.
- 
- 
-The following is example content for the **Course Overview** box:
- 
- 
-.. image:: Images/image125.png   
 
-  
+
+The following is example content for the **Course Overview** box:
+
+
+.. image:: Images/image125.png
+
+Add a Descriptive Picture
+=========================
+
+1.  Select a high-resolution image that is a minimum of 660 pixels in width by 240 pixels in height.
+
+2.  Change the file name of the picture that you want to use to **images_course_image.jpg**.
+
+3.  Upload the file to the **Files & Uploads** page.
+
+
+The picture that is named **images_course_image.jpg** automatically appears on the course About page.
+
 Add an About Video
 ==================
- 
- 
+
+
 You can create an About video that will appear on the **About** page for your course.
- 
- 
+
+
 1. Upload the video that you want to YouTube. Make note of the code that appears between   ** watch?v =**  and   ** &feature**  in the URL. This code appears in the green box below.
 
 
-.. image:: Images/image127.png      
- 
- 
+.. image:: Images/image127.png
+
+
 2. On the navigation bar, click **Course Settings**, and then click **Schedule & Details** .
- 
- 
+
+
 3. Scroll down to the **Introducing Your Course** section, and then locate the **Course** **Introduction Video**
     field. If you have not already added a video, you see a blank field above an **id** box.
- 
- 
-.. image:: Images/image129.png    
- 
- 
+
+
+.. image:: Images/image129.png
+
+
 4. In the **your YouTube video's ID**  box, enter your video code. When you add the code, the video automatically appears in the field above the   **your YouTube video's ID**  box.
- 
- 
+
+
 .. note::
 
     There is no save button. Studio automatically saves your changes.
- 
- 
+
+
 For example, your course introduction video appears as follows.
- 
- 
-.. image:: Images/image131.png 
- 
-  
+
+
+.. image:: Images/image131.png
+
+
 Add Weekly Time Requirements Information
-======================================== 
- 
- 
+========================================
+
+
 1. On the navigation bar, click **Course Settings**, and then click **Schedule & Details** .
- 
- 
+
+
 2. Scroll down to the **Requirments** section.
- 
- 
+
+
 3. In the **Hours of Effort per Week** box, enter the number of hours you expect students to work on this course each week.

docs/course_authors/source/get_started.rst

@@ -13,13 +13,22 @@ Introduction
 
 Since the launch of edX to our original partners, we have been working to provide opportunities for additional educators to create courses on our platform. The fruits of our efforts are Edge and Studio. These tools are available not only to our edX partners, but to all faculty at consortium universities.
 
+EdX (http://edx.org) is our original, premiere learning portal. Publication to
+edX is available on a limited basis, depending on your university’s agreement
+with edX. You need specific approval from your university to release your
+course on the edX portal. Once a course is released on the edX portal, it
+becomes a publicly available massively open online course (MOOC).
 
 
-EdX (http://edx.org) is our original, premiere learning portal. Publication to edX is available on a limited basis, depending on your university’s agreement with edX. You need specific approval from your university to release your course on the edX portal.
-
 Edge (http://edge.edx.org) is our newest online learning portal. It is almost identical to edX.org both visibly and functionally. 
 
-Edge is where you view the content you create with Studio, our course authoring tool, and where students will view your course. Instructors are encouraged to use Edge to experiment with creating courses. You do not need approval to release a course on Edge—you can create a course and release it immediately.
+Edge is where you view the content you create with Studio, our course authoring
+tool. Courses on Edge cannot be seen publicly; rather, only you, your
+colleagues, and the students with whom you explicitly share a course link can
+see your course. Instructors are encouraged to use Edge to experiment with
+creating courses. You do not need approval to release a course on Edge--you can
+create a course and release it immediately.
+
 
 Studio (http://studio.edge.edx.org) is our web-based course authoring tool. It is the easiest way for educators to develop courses for the edX platform. You can create courses in Studio and view and enroll in them instantly on Edge—even before you have finished creating the course.
 

docs/data/Makefile

@@ -7,10 +7,21 @@ SPHINXBUILD   = sphinx-build
 PAPER         =
 BUILDDIR      = build
 
+# User-friendly check for sphinx-build
+ifeq ($(shell which $(SPHINXBUILD) >/dev/null 2>&1; echo $$?), 1)
+$(error The '$(SPHINXBUILD)' command was not found. Make sure you have Sphinx installed, then set the SPHINXBUILD environment variable to point to the full path of the '$(SPHINXBUILD)' executable. Alternatively you can add the directory with the executable to your PATH. If you don't have Sphinx installed, grab it from http://sphinx-doc.org/)
+endif
+
+Q_FLAG        = 
+
+ifeq ($(quiet), true)
+Q_FLAG = -q
+endif
+
 # Internal variables.
 PAPEROPT_a4     = -D latex_paper_size=a4
 PAPEROPT_letter = -D latex_paper_size=letter
-ALLSPHINXOPTS   = -d $(BUILDDIR)/doctrees -c source $(PAPEROPT_$(PAPER)) $(SPHINXOPTS) source
+ALLSPHINXOPTS   = -q -d $(BUILDDIR)/doctrees -c source $(PAPEROPT_$(PAPER)) $(SPHINXOPTS) source
 # the i18n builder cannot share the environment and doctrees with the others
 I18NSPHINXOPTS  = $(PAPEROPT_$(PAPER)) $(SPHINXOPTS) source
 

docs/developers/Makefile

@@ -7,10 +7,21 @@ SPHINXBUILD   = sphinx-build
 PAPER         =
 BUILDDIR      = build
 
+# User-friendly check for sphinx-build
+ifeq ($(shell which $(SPHINXBUILD) >/dev/null 2>&1; echo $$?), 1)
+$(error The '$(SPHINXBUILD)' command was not found. Make sure you have Sphinx installed, then set the SPHINXBUILD environment variable to point to the full path of the '$(SPHINXBUILD)' executable. Alternatively you can add the directory with the executable to your PATH. If you don't have Sphinx installed, grab it from http://sphinx-doc.org/)
+endif
+
+Q_FLAG        = 
+
+ifeq ($(quiet), true)
+Q_FLAG = -q
+endif
+
 # Internal variables.
 PAPEROPT_a4     = -D latex_paper_size=a4
 PAPEROPT_letter = -D latex_paper_size=letter
-ALLSPHINXOPTS   = -d $(BUILDDIR)/doctrees -c source $(PAPEROPT_$(PAPER)) $(SPHINXOPTS) source
+ALLSPHINXOPTS   = $(Q_FLAG) -d $(BUILDDIR)/doctrees -c source $(PAPEROPT_$(PAPER)) $(SPHINXOPTS) source
 # the i18n builder cannot share the environment and doctrees with the others
 I18NSPHINXOPTS  = $(PAPEROPT_$(PAPER)) $(SPHINXOPTS) source
 

docs/developers/source/conf.py

@@ -3,7 +3,7 @@
 #pylint: disable=W0622
 #pylint: disable=W0212
 #pylint: disable=W0613
-  
+
 import sys, os
 
 on_rtd = os.environ.get('READTHEDOCS', None) == 'True'
@@ -26,7 +26,7 @@ html_static_path.append('source/_static')
 # If extensions (or modules to document with autodoc) are in another directory,
 # add these directories to sys.path here. If the directory is relative to the
 # documentation root, use os.path.abspath to make it absolute, like shown here.
-#sys.path.insert(0, os.path.abspath('../../..'))
+sys.path.insert(0, os.path.abspath('../../..'))
 root = os.path.abspath('../../..')
 
 sys.path.append(root)
@@ -53,8 +53,9 @@ else:
 # Add any Sphinx extension module names here, as strings. They can be extensions
 # coming with Sphinx (named 'sphinx.ext.*') or your custom ones.
 extensions = [
-    'sphinx.ext.autodoc', 'sphinx.ext.doctest', 'sphinx.ext.intersphinx', 'sphinx.ext.todo', 'sphinx.ext.coverage',
-    'sphinx.ext.pngmath', 'sphinx.ext.mathjax', 'sphinx.ext.viewcode']
+    'sphinx.ext.autodoc', 'sphinx.ext.doctest', 'sphinx.ext.intersphinx',
+    'sphinx.ext.todo', 'sphinx.ext.coverage', 'sphinx.ext.pngmath',
+    'sphinx.ext.mathjax', 'sphinx.ext.viewcode']
 
 # List of patterns, relative to source directory, that match files and
 # directories to ignore when looking for source files.
@@ -64,6 +65,46 @@ exclude_patterns = ['build']
 # Output file base name for HTML help builder.
 htmlhelp_basename = 'edXDocs'
 
+# --- Mock modules ------------------------------------------------------------
+
+# Mock all the modules that the readthedocs build can't import
+import mock
+
+class Mock(object):
+    def __init__(self, *args, **kwargs):
+        pass
+
+    def __call__(self, *args, **kwargs):
+        return Mock()
+
+    @classmethod
+    def __getattr__(cls, name):
+        if name in ('__file__', '__path__'):
+            return '/dev/null'
+        elif name[0] == name[0].upper():
+            mockType = type(name, (), {})
+            mockType.__module__ = __name__
+            return mockType
+        else:
+            return Mock()
+
+# The list of modules and submodules that we know give RTD trouble.
+# Make sure you've tried including the relevant package in
+# docs/share/requirements.txt before adding to this list.
+MOCK_MODULES = [
+    'numpy',
+    'matplotlib',
+    'matplotlib.pyplot',
+    'scipy.interpolate',
+    'scipy.constants',
+    'scipy.optimize',
+    ]
+
+if on_rtd:
+    for mod_name in MOCK_MODULES:
+        sys.modules[mod_name] = Mock()
+
+# -----------------------------------------------------------------------------
 
 # from http://djangosnippets.org/snippets/2533/
 # autogenerate models definitions
@@ -109,27 +150,7 @@ def strip_tags(html):
     s.feed(html)
     return s.get_data()
 
-class Mock(object):
-    def __init__(self, *args, **kwargs):
-        pass
 
-    def __call__(self, *args, **kwargs):
-        return Mock()
-
-    @classmethod
-    def __getattr__(cls, name):
-        if name in ('__file__', '__path__'):
-            return '/dev/null'
-        elif name[0] == name[0].upper():
-            mockType = type(name, (), {})
-            mockType.__module__ = __name__
-            return mockType
-        else:
-            return Mock()
-
-MOCK_MODULES = ['scipy', 'numpy']
-for mod_name in MOCK_MODULES:
-    sys.modules[mod_name] = Mock()
 
 def process_docstring(app, what, name, obj, options, lines):
     """Autodoc django models"""
@@ -165,34 +186,6 @@ def process_docstring(app, what, name, obj, options, lines):
 
             # Add the field's type to the docstring
             lines.append(u':type %s: %s' % (field.attname, type(field).__name__))
-        # Only look at objects that inherit from Django's base FORM class
-    # elif (inspect.isclass(obj) and issubclass(obj, forms.ModelForm) or issubclass(obj, forms.ModelForm) or issubclass(obj, BaseInlineFormSet)):
-    #     pass
-        # # Grab the field list from the meta class
-        # import ipdb; ipdb.set_trace()
-        # fields = obj._meta._fields()
-        # import ipdb; ipdb.set_trace()
-        # for field in fields:
-        #     import ipdb; ipdb.set_trace()
-        #     # Decode and strip any html out of the field's help text
-        #     help_text = strip_tags(force_unicode(field.help_text))
-
-        #     # Decode and capitalize the verbose name, for use if there isn't
-        #     # any help text
-        #     verbose_name = force_unicode(field.verbose_name).capitalize()
-
-        #     if help_text:
-        #         # Add the model field to the end of the docstring as a param
-        #         # using the help text as the description
-        #         lines.append(u':param %s: %s' % (field.attname, help_text))
-        #     else:
-        #         # Add the model field to the end of the docstring as a param
-        #         # using the verbose name as the description
-        #         lines.append(u':param %s: %s' % (field.attname, verbose_name))
-
-        #     # Add the field's type to the docstring
-        #     lines.append(u':type %s: %s' % (field.attname, type(field).__name__))
-    # Return the extended docstring
     return lines
 
 

lms/djangoapps/course_modes/migrations/0002_auto__add_field_coursemode_currency.py

@@ -0,0 +1,35 @@
+# -*- coding: utf-8 -*-
+import datetime
+from south.db import db
+from south.v2 import SchemaMigration
+from django.db import models
+
+
+class Migration(SchemaMigration):
+
+    def forwards(self, orm):
+        # Adding field 'CourseMode.currency'
+        db.add_column('course_modes_coursemode', 'currency',
+                      self.gf('django.db.models.fields.CharField')(default='usd', max_length=8),
+                      keep_default=False)
+
+
+    def backwards(self, orm):
+        # Deleting field 'CourseMode.currency'
+        db.delete_column('course_modes_coursemode', 'currency')
+
+
+    models = {
+        'course_modes.coursemode': {
+            'Meta': {'object_name': 'CourseMode'},
+            'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'currency': ('django.db.models.fields.CharField', [], {'default': "'usd'", 'max_length': '8'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'min_price': ('django.db.models.fields.IntegerField', [], {'default': '0'}),
+            'mode_display_name': ('django.db.models.fields.CharField', [], {'max_length': '255'}),
+            'mode_slug': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'suggested_prices': ('django.db.models.fields.CommaSeparatedIntegerField', [], {'default': "''", 'max_length': '255', 'blank': 'True'})
+        }
+    }
+
+    complete_apps = ['course_modes']

lms/djangoapps/course_modes/migrations/0003_auto__add_unique_coursemode_course_id_currency_mode_slug.py

@@ -0,0 +1,33 @@
+# -*- coding: utf-8 -*-
+import datetime
+from south.db import db
+from south.v2 import SchemaMigration
+from django.db import models
+
+
+class Migration(SchemaMigration):
+
+    def forwards(self, orm):
+        # Adding unique constraint on 'CourseMode', fields ['course_id', 'currency', 'mode_slug']
+        db.create_unique('course_modes_coursemode', ['course_id', 'currency', 'mode_slug'])
+
+
+    def backwards(self, orm):
+        # Removing unique constraint on 'CourseMode', fields ['course_id', 'currency', 'mode_slug']
+        db.delete_unique('course_modes_coursemode', ['course_id', 'currency', 'mode_slug'])
+
+
+    models = {
+        'course_modes.coursemode': {
+            'Meta': {'unique_together': "(('course_id', 'mode_slug', 'currency'),)", 'object_name': 'CourseMode'},
+            'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'currency': ('django.db.models.fields.CharField', [], {'default': "'usd'", 'max_length': '8'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'min_price': ('django.db.models.fields.IntegerField', [], {'default': '0'}),
+            'mode_display_name': ('django.db.models.fields.CharField', [], {'max_length': '255'}),
+            'mode_slug': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'suggested_prices': ('django.db.models.fields.CommaSeparatedIntegerField', [], {'default': "''", 'max_length': '255', 'blank': 'True'})
+        }
+    }
+
+    complete_apps = ['course_modes']

lms/djangoapps/course_modes/models.py

@@ -5,7 +5,7 @@ from django.db import models
 from collections import namedtuple
 from django.utils.translation import ugettext as _
 
-Mode = namedtuple('Mode', ['slug', 'name', 'min_price', 'suggested_prices'])
+Mode = namedtuple('Mode', ['slug', 'name', 'min_price', 'suggested_prices', 'currency'])
 
 
 class CourseMode(models.Model):
@@ -29,7 +29,14 @@ class CourseMode(models.Model):
     # the suggested prices for this mode
     suggested_prices = models.CommaSeparatedIntegerField(max_length=255, blank=True, default='')
 
-    DEFAULT_MODE = Mode('honor', _('Honor Code Certificate'), 0, '')
+    # the currency these prices are in, using lower case ISO currency codes
+    currency = models.CharField(default="usd", max_length=8)
+
+    DEFAULT_MODE = Mode('honor', _('Honor Code Certificate'), 0, '', 'usd')
+
+    class Meta:
+        """ meta attributes of this model """
+        unique_together = ('course_id', 'mode_slug', 'currency')
 
     @classmethod
     def modes_for_course(cls, course_id):
@@ -39,7 +46,7 @@ class CourseMode(models.Model):
         If no modes have been set in the table, returns the default mode
         """
         found_course_modes = cls.objects.filter(course_id=course_id)
-        modes = ([Mode(mode.mode_slug, mode.mode_display_name, mode.min_price, mode.suggested_prices)
+        modes = ([Mode(mode.mode_slug, mode.mode_display_name, mode.min_price, mode.suggested_prices, mode.currency)
                   for mode in found_course_modes])
         if not modes:
             modes = [cls.DEFAULT_MODE]

lms/djangoapps/course_modes/tests.py

@@ -18,7 +18,7 @@ class CourseModeModelTest(TestCase):
         self.course_id = 'TestCourse'
         CourseMode.objects.all().delete()
 
-    def create_mode(self, mode_slug, mode_name, min_price=0, suggested_prices=''):
+    def create_mode(self, mode_slug, mode_name, min_price=0, suggested_prices='', currency='usd'):
         """
         Create a new course mode
         """
@@ -27,7 +27,8 @@ class CourseModeModelTest(TestCase):
             mode_display_name=mode_name,
             mode_slug=mode_slug,
             min_price=min_price,
-            suggested_prices=suggested_prices
+            suggested_prices=suggested_prices,
+            currency=currency
         )
 
     def test_modes_for_course_empty(self):
@@ -45,14 +46,14 @@ class CourseModeModelTest(TestCase):
 
         self.create_mode('verified', 'Verified Certificate')
         modes = CourseMode.modes_for_course(self.course_id)
-        self.assertEqual([Mode(u'verified', u'Verified Certificate', 0, '')], modes)
+        self.assertEqual([Mode(u'verified', u'Verified Certificate', 0, '', 'usd')], modes)
 
     def test_modes_for_course_multiple(self):
         """
         Finding the modes when there's multiple modes
         """
-        mode1 = Mode(u'honor', u'Honor Code Certificate', 0, '')
-        mode2 = Mode(u'verified', u'Verified Certificate', 0, '')
+        mode1 = Mode(u'honor', u'Honor Code Certificate', 0, '', 'usd')
+        mode2 = Mode(u'verified', u'Verified Certificate', 0, '', 'usd')
         set_modes = [mode1, mode2]
         for mode in set_modes:
             self.create_mode(mode.slug, mode.name, mode.min_price, mode.suggested_prices)

lms/djangoapps/courseware/features/common.py

@@ -66,6 +66,13 @@ def add_tab_to_course(_step, course, extra_tab_name):
         display_name=str(extra_tab_name))
 
 
+@step(u'I am in a course$')
+def go_into_course(step):
+    step.given('I am registered for the course "6.002x"')
+    step.given('And I am logged in')
+    step.given('And I click on View Courseware')
+
+
 def course_id(course_num):
     return "%s/%s/%s" % (world.scenario_dict['COURSE'].org, course_num,
                          world.scenario_dict['COURSE'].display_name.replace(" ", "_"))

lms/djangoapps/courseware/features/help.feature

@@ -0,0 +1,31 @@
+Feature: The help module should work
+  In order to get help
+  As a student
+  I want to be able to report a problem
+
+
+  Scenario: I can submit a problem when I am not logged in
+    Given I visit the homepage
+    When I open the help form
+    And I report a "<FeedbackType>"
+    Then I should see confirmation that the issue was received
+
+        Examples:
+        | FeedbackType      |
+        | problem           |
+        | suggestion        |
+        | question          |
+
+
+  Scenario: I can submit a problem when I am logged in
+    Given I am in a course
+    When I open the help form
+    And I report a "<FeedbackType>" without saying who I am
+    Then I should see confirmation that the issue was received
+
+        Examples:
+        | FeedbackType      |
+        | problem           |
+        | suggestion        |
+        | question          |
+

lms/djangoapps/courseware/features/help.py

@@ -0,0 +1,45 @@
+#pylint: disable=C0111
+#pylint: disable=W0621
+
+from lettuce import world, step
+
+
+@step(u'I open the help form')
+def open_help_modal(step):
+    help_css = 'div.help-tab'
+    world.css_click(help_css)
+
+
+@step(u'I report a "([^"]*)"$')
+def submit_problem_type(step, submission_type):
+    type_css = '#feedback_link_{}'.format(submission_type)
+    world.css_click(type_css)
+    fill_field('name', 'Robot')
+    fill_field('email', 'robot@edx.org')
+    fill_field('subject', 'Test Issue')
+    fill_field('details', 'I am having a problem')
+    submit_css = 'div.submit'
+    world.css_click(submit_css)
+
+
+@step(u'I report a "([^"]*)" without saying who I am$')
+def submit_partial_problem_type(step, submission_type):
+    type_css = '#feedback_link_{}'.format(submission_type)
+    world.css_click(type_css)
+    fill_field('subject', 'Test Issue')
+    fill_field('details', 'I am having a problem')
+    submit_css = 'div.submit'
+    world.css_click(submit_css)
+
+
+@step(u'I should see confirmation that the issue was received')
+def see_confirmation(step):
+    assert world.browser.evaluate_script("$('input[value=\"Submit\"]').attr('disabled')") == 'disabled'
+
+
+def fill_field(name, info):
+    def fill_info():
+        form_css = 'form.feedback_form'
+        form = world.css_find(form_css)
+        form.find_by_name(name).fill(info)
+    world.retry_on_exception(fill_info)

lms/djangoapps/courseware/features/video.py

@@ -12,12 +12,12 @@ HTML5_SOURCES = [
     'https://s3.amazonaws.com/edx-course-videos/edx-intro/edX-FA12-cware-1_100.ogv'
 ]
 
-@step('when I view the (.*) it has autoplay enabled')
+@step('when I view the (.*) it has autoplay enabled$')
 def does_autoplay_video(_step, video_type):
     assert(world.css_find('.%s' % video_type)[0]['data-autoplay'] == 'True')
 
 
-@step('the course has a Video component in (.*) mode')
+@step('the course has a Video component in (.*) mode$')
 def view_video(_step, player_mode):
     coursenum = 'test_course'
     i_am_registered_for_the_course(step, coursenum)
@@ -55,18 +55,16 @@ def add_video_to_course(course, player_mode):
     world.ItemFactory.create(**kwargs)
 
 
-@step('when I view the video it has rendered in (.*) mode')
+@step('when I view the video it has rendered in (.*) mode$')
 def video_is_rendered(_step, mode):
     modes = {
         'html5': 'video',
         'youtube': 'iframe'
     }
-    if mode.lower() in modes:
-        assert world.css_find('.video {0}'.format(modes[mode.lower()])).first
-    else:
-        assert False
+    html_tag = modes[mode.lower()]
+    assert world.css_find('.video {0}'.format(html_tag)).first
 
-@step('all sources are correct')
+@step('all sources are correct$')
 def all_sources_are_correct(_step):
     sources = world.css_find('.video video source')
     assert set(source['src'] for source in sources) == set(HTML5_SOURCES)

lms/djangoapps/courseware/tests/test_module_render.py

@@ -275,10 +275,11 @@ class TestHtmlModifiers(ModuleStoreTestCase):
         self.course = CourseFactory.create()
         self.content_string = '<p>This is the content<p>'
         self.rewrite_link = '<a href="/static/foo/content">Test rewrite</a>'
+        self.rewrite_bad_link = '<img src="/static//file.jpg" />'
         self.course_link = '<a href="/course/bar/content">Test course rewrite</a>'
         self.descriptor = ItemFactory.create(
             category='html',
-            data=self.content_string + self.rewrite_link + self.course_link
+            data=self.content_string + self.rewrite_link + self.rewrite_bad_link + self.course_link
         )
         self.location = self.descriptor.location
         self.model_data_cache = ModelDataCache.cache_for_descriptor_descendents(
@@ -331,6 +332,24 @@ class TestHtmlModifiers(ModuleStoreTestCase):
             result_fragment.content
         )
 
+    def test_static_badlink_rewrite(self):
+        module = render.get_module(
+            self.user,
+            self.request,
+            self.location,
+            self.model_data_cache,
+            self.course.id,
+        )
+        result_fragment = module.runtime.render(module, None, 'student_view')
+
+        self.assertIn(
+            '/c4x/{org}/{course}/asset/_file.jpg'.format(
+                org=self.course.location.org,
+                course=self.course.location.course,
+            ),
+            result_fragment.content
+        )
+
     def test_course_link_rewrite(self):
         module = render.get_module(
             self.user,

lms/djangoapps/courseware/tests/test_video_xml.py

@@ -20,8 +20,7 @@ import unittest
 
 from django.conf import settings
 
-from xmodule.video_module import (
-    VideoDescriptor, _create_youtube_string)
+from xmodule.video_module import VideoDescriptor, _create_youtube_string
 from xmodule.modulestore import Location
 from xmodule.tests import get_test_system, LogicTest
 

lms/djangoapps/courseware/tests/test_views.py

@@ -12,12 +12,14 @@ from django.core.urlresolvers import reverse
 
 from student.models import CourseEnrollment
 from student.tests.factories import AdminFactory
+
 from xmodule.modulestore.django import modulestore
 
 import courseware.views as views
 from xmodule.modulestore import Location
 from pytz import UTC
 from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
+from course_modes.models import CourseMode
 
 
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
@@ -162,6 +164,36 @@ class ViewsTestCase(TestCase):
         #       generate/store a real password.
         self.assertEquals(chat_settings['password'], "johndoe@%s" % domain)
 
+    def test_course_mktg_about_coming_soon(self):
+        # we should not be able to find this course
+        url = reverse('mktg_about_course', kwargs={'course_id': 'no/course/here'})
+        response = self.client.get(url)
+        self.assertIn('Coming Soon', response.content)
+
+    def test_course_mktg_register(self):
+        admin = AdminFactory()
+        self.client.login(username=admin.username, password='test')
+        url = reverse('mktg_about_course', kwargs={'course_id': self.course_id})
+        response = self.client.get(url)
+        self.assertIn('Register for', response.content)
+        self.assertNotIn('and choose your student track', response.content)
+
+    def test_course_mktg_register_multiple_modes(self):
+        admin = AdminFactory()
+        CourseMode.objects.get_or_create(mode_slug='honor',
+                                         mode_display_name='Honor Code Certificate',
+                                         course_id=self.course_id)
+        CourseMode.objects.get_or_create(mode_slug='verified',
+                                         mode_display_name='Verified Certificate',
+                                         course_id=self.course_id)
+        self.client.login(username=admin.username, password='test')
+        url = reverse('mktg_about_course', kwargs={'course_id': self.course_id})
+        response = self.client.get(url)
+        self.assertIn('Register for', response.content)
+        self.assertIn('and choose your student track', response.content)
+        # clean up course modes
+        CourseMode.objects.all().delete()
+
     def test_submission_history_xss(self):
         # log into a staff account
         admin = AdminFactory()

lms/djangoapps/courseware/views.py

@@ -25,6 +25,7 @@ from courseware.masquerade import setup_masquerade
 from courseware.model_data import ModelDataCache
 from .module_render import toc_for_course, get_module_for_descriptor, get_module
 from courseware.models import StudentModule, StudentModuleHistory
+from course_modes.models import CourseMode
 
 from django_comment_client.utils import get_discussion_title
 
@@ -457,7 +458,10 @@ def jump_to_id(request, course_id, module_id):
 
     course_location = CourseDescriptor.id_to_location(course_id)
 
-    items = modulestore().get_items(['i4x', course_location.org, course_location.course, None, module_id])
+    items = modulestore().get_items(
+        ['i4x', course_location.org, course_location.course, None, module_id],
+        course_id=course_id
+    )
 
     if len(items) == 0:
         raise Http404("Could not find id = {0} in course_id = {1}. Referer = {2}".
@@ -600,9 +604,14 @@ def course_about(request, course_id):
                                'registered': registered,
                                'course_target': course_target,
                                'show_courseware_link': show_courseware_link})
+
+
 @ensure_csrf_cookie
 @cache_if_anonymous
 def mktg_course_about(request, course_id):
+    """
+    This is the button that gets put into an iframe on the Drupal site
+    """
 
     try:
         course = get_course_with_access(request.user, course_id, 'see_exists')
@@ -610,7 +619,7 @@ def mktg_course_about(request, course_id):
         # if a course does not exist yet, display a coming
         # soon button
         return render_to_response('courseware/mktg_coming_soon.html',
-                              {'course_id': course_id})
+                                  {'course_id': course_id})
 
     registered = registered_for_course(course, request.user)
 
@@ -623,13 +632,17 @@ def mktg_course_about(request, course_id):
 
     show_courseware_link = (has_access(request.user, course, 'load') or
                             settings.MITX_FEATURES.get('ENABLE_LMS_MIGRATION'))
+    course_modes = CourseMode.modes_for_course(course.id)
 
     return render_to_response('courseware/mktg_course_about.html',
-                              {'course': course,
-                               'registered': registered,
-                               'allow_registration': allow_registration,
-                               'course_target': course_target,
-                               'show_courseware_link': show_courseware_link})
+                              {
+                                  'course': course,
+                                  'registered': registered,
+                                  'allow_registration': allow_registration,
+                                  'course_target': course_target,
+                                  'show_courseware_link': show_courseware_link,
+                                  'course_modes': course_modes,
+                              })
 
 
 def render_notifications(request, course, notifications):

lms/djangoapps/instructor/hint_manager.py

@@ -89,7 +89,7 @@ def get_hints(request, course_id, field):
 
     for hints_by_problem in all_hints:
         loc = Location(hints_by_problem.definition_id)
-        name = location_to_problem_name(loc)
+        name = location_to_problem_name(course_id, loc)
         if name is None:
             continue
         id_to_name[hints_by_problem.definition_id] = name
@@ -119,13 +119,13 @@ def get_hints(request, course_id, field):
     return render_dict
 
 
-def location_to_problem_name(loc):
+def location_to_problem_name(course_id, loc):
     """
     Given the location of a crowdsource_hinter module, try to return the name of the
     problem it wraps around.  Return None if the hinter no longer exists.
     """
     try:
-        descriptor = modulestore().get_items(loc)[0]
+        descriptor = modulestore().get_items(loc, course_id=course_id)[0]
         return descriptor.get_children()[0].display_name
     except IndexError:
         # Sometimes, the problem is no longer in the course.  Just

lms/djangoapps/instructor/tests/test_hint_manager.py

@@ -42,7 +42,7 @@ class HintManagerTest(ModuleStoreTestCase):
                               value=5)
         # Mock out location_to_problem_name, which ordinarily accesses the modulestore.
         # (I can't figure out how to get fake structures into the modulestore.)
-        view.location_to_problem_name = lambda loc: "Test problem"
+        view.location_to_problem_name = lambda course_id, loc: "Test problem"
 
     def test_student_block(self):
         """

lms/djangoapps/instructor/views/legacy.py

@@ -342,7 +342,7 @@ def instructor_dashboard(request, course_id):
                     student_module.delete()
                     msg += "<font color='red'>Deleted student module state for {0}!</font>".format(module_state_key)
                     event = {
-                        "problem": problem_url,
+                        "problem": module_state_key,
                         "student": unique_student_identifier,
                         "course": course_id
                     }

lms/envs/acceptance.py

@@ -87,6 +87,9 @@ MITX_FEATURES['AUTOMATIC_AUTH_FOR_TESTING'] = True
 # We do not yet understand why this occurs. Setting this to true is a stopgap measure
 USE_I18N = True
 
+MITX_FEATURES['ENABLE_FEEDBACK_SUBMISSION'] = True
+FEEDBACK_SUBMISSION_EMAIL = 'dummy@example.com'
+
 # Include the lettuce app for acceptance testing, including the 'harvest' django-admin command
 INSTALLED_APPS += ('lettuce.django',)
 LETTUCE_APPS = ('courseware',)

lms/envs/cms/mixed_dev.py

@@ -6,7 +6,7 @@ This configuration is to run the MixedModuleStore on a localdev environment
 # want to import all variables from base settings files
 # pylint: disable=W0401, W0614
 
-from .dev import *, DATA_DIR
+from .dev import *
 
 MODULESTORE = {
     'default': {

lms/static/coffee/spec/navigation_spec.coffee

@@ -57,8 +57,7 @@ describe 'Navigation', ->
 
   describe 'log', ->
     beforeEach ->
-      window.log_event = ->
-      spyOn window, 'log_event'
+      spyOn Logger, 'log'
 
     it 'submit event log', ->
       @navigation.log {}, {
@@ -68,6 +67,6 @@ describe 'Navigation', ->
           text: -> "old"
       }
 
-      expect(window.log_event).toHaveBeenCalledWith 'accordion',
+      expect(Logger.log).toHaveBeenCalledWith 'accordion',
         newheader: 'new'
         oldheader: 'old'

lms/static/coffee/src/navigation.coffee

@@ -20,7 +20,7 @@ class @Navigation
       $('#accordion a').click @setChapter
 
   log: (event, ui) ->
-    log_event 'accordion',
+    Logger.log 'accordion',
       newheader: ui.newHeader.text()
       oldheader: ui.oldHeader.text()
 

lms/static/sass/_shame.scss

@@ -156,6 +156,19 @@
     &.action-register, &.access-courseware {
       @extend .m-btn-primary;
       display: block;
+
+      .track {
+        @include transition(all 0.25s ease-in-out);
+        color: $white;
+        display: block;
+        font-size: 13px;
+        line-height: 2em;
+        opacity: 0.6;
+      }
+
+      &:hover .track {
+        opacity: 1.0;
+      }
     }
 
     // already registered but course not started or registration is closed

lms/templates/courseware/mktg_course_about.html

@@ -52,7 +52,13 @@
             <div class="action is-registered">${_("You Are Registered")}</div>
           %endif
         %elif allow_registration:
-          <a class="action action-register register" href="#">${_("Register for")} <strong>${course.display_number_with_default | h}</strong></a>
+        <a class="action action-register register" href="#">${_("Register for")} <strong>${course.display_number_with_default | h}</strong>
+            %if len(course_modes) > 1:
+            <span class="track">
+            and choose your student track
+            </span>
+            %endif
+        </a>
         %else:
           <div class="action registration-closed is-disabled">${_("Registration Is Closed")}</div>
         %endif

lms/templates/dashboard.html

@@ -140,11 +140,11 @@
 
           % if course.id in show_courseware_links_for:
             <a href="${course_target}" class="cover">
-              <img src="${course_image_url(course)}" alt="${_('{course_number} {course_name} Cover Image').format(course_number='${course.number}', course_name='${course.display_name_with_default}') |h}" />
+              <img src="${course_image_url(course)}" alt="${_('{course_number} {course_name} Cover Image').format(course_number=course.number, course_name=course.display_name_with_default) |h}" />
             </a>
           % else:
             <div class="cover">
-              <img src="${course_image_url(course)}" alt="${_('{course_number} {course_name} Cover Image').format(course_number='${course.number}', course_name='${course.display_name_with_default}')}" />
+              <img src="${course_image_url(course)}" alt="${_('{course_number} {course_name} Cover Image').format(course_number=course.number, course_name=course.display_name_with_default) | h}" />
             </div>
           % endif
 

lms/templates/help_modal.html

@@ -31,7 +31,13 @@ discussion_link = get_discussion_link(course) if course else None
     </p>
 % endif
 
-    <p>${_('Have <strong>general questions about {platform_name}</strong>? You can find lots of helpful information in the {platform_name} {link_start}FAQ{link_end}.').format(link_start='<a href="/help" target="_blank">', link_end='</a>', platform_name=settings.PLATFORM_NAME)}</p>
+    <p>${_('Have <strong>general questions about {platform_name}</strong>? You can find lots of helpful information in the {platform_name} {link_start}FAQ{link_end}.').format(
+        link_start='<a href="{url}" target="_blank">'.format(
+          url=marketing_link('FAQ')
+        ),
+        link_end='</a>',
+        platform_name=settings.PLATFORM_NAME)}
+      </p>
 
     <p>${_('Have a <strong>question about something specific</strong>? You can contact the {platform_name} general support team directly:').format(platform_name=settings.PLATFORM_NAME)}</p>
     <hr>

lms/templates/staticbook.html

@@ -25,7 +25,7 @@ $(document).ready(function(){
 });
 
 function goto_page(n) {
-  log_event("book", {"type":"gotopage","old":page,"new":n});
+  Logger.log("book", {"type":"gotopage","old":page,"new":n});
   page=n;
   var prefix = "";
   if(n<100) {
@@ -42,14 +42,14 @@ function prev_page() {
   var newpage=page-1;
   if(newpage< ${start_page}) newpage=${start_page};
   goto_page(newpage);
-  log_event("book", {"type":"prevpage","new":page});
+  Logger.log("book", {"type":"prevpage","new":page});
 }
 
 function next_page() {
   var newpage=page+1;
   if(newpage> ${end_page}) newpage=${end_page};
   goto_page(newpage);
-  log_event("book", {"type":"nextpage","new":page});
+  Logger.log("book", {"type":"nextpage","new":page});
 }
 
 $("#open_close_accordion a").click(function(){

pylintrc

@@ -100,6 +100,11 @@ ignore-mixin-members=yes
 # (useful for classes with attributes dynamically set).
 ignored-classes=SQLObject
 
+# See http://stackoverflow.com/questions/17156240/nose-tools-and-pylint
+# Pylint does not like the way that nose tools inspects and makes available
+# the assert classes
+ignored-classes=nose.tools,nose.tools.trivial
+
 # When zope mode is activated, add a predefined set of Zope acquired attributes
 # to generated-members.
 zope=no

rakelib/docs.rake

@@ -2,24 +2,24 @@ require 'launchy'
 
 # --- Develop and public documentation ---
 desc "Invoke sphinx 'make build' to generate docs."
-task :builddocs, [:options] do |t, args|
-    if args.options == 'dev'
+task :builddocs, [:type, :quiet] do |t, args|
+    args.with_defaults(:quiet => "quiet")
+    if args.type == 'dev'
         path = "docs/developer"
-    elsif args.options == 'author'
+    elsif args.type == 'author'
         path = "docs/course_authors"
-    elsif args.options == 'data'
+    elsif args.type == 'data'
         path = "docs/data"
     else
         path = "docs"
     end
 
     Dir.chdir(path) do
-        sh('make html')
-    end
-    path = "docs"
-
-    Dir.chdir(path) do
-        sh('make html')
+        if args.quiet == 'verbose'
+            sh('make html quiet=false')
+        else
+            sh('make html')
+        end
     end
 end
 
@@ -39,7 +39,7 @@ task :showdocs, [:options] do |t, args|
 end
 
 desc "Build docs and show them in browser"
-task :doc, [:options] =>  :builddocs do |t, args|
-    Rake::Task["showdocs"].invoke(args.options)
+task :doc, [:type, :quiet] =>  :builddocs do |t, args|
+    Rake::Task["showdocs"].invoke(args.type, args.quiet)
 end
 # --- Develop and public documentation ---

rakelib/tests.rake

@@ -40,12 +40,10 @@ end
 desc "Run documentation tests"
 task :test_docs do
     # Be sure that sphinx can build docs w/o exceptions.
-    test_message = "If test fails, you shoud run %s and look at whole output and fix exceptions.
+    test_message = "If test fails, you shoud run '%s' and look at whole output and fix exceptions.
 (You shouldn't fix rst warnings and errors for this to pass, just get rid of exceptions.)"
-    puts (test_message  % ["rake doc"]).colorize( :light_green )
+    puts (test_message  % ["rake doc[docs,verbose]"]).colorize( :light_green )
     test_sh('rake builddocs')
-    puts  (test_message  % ["rake doc[pub]"]).colorize( :light_green )
-    test_sh('rake builddocs[pub]')
 end
 
 task :clean_test_files do
@@ -161,4 +159,4 @@ task :coverage => :report_dirs do
     if not found_coverage_info
         puts "No coverage info found.  Run `rake test` before running `rake coverage`."
     end
-end
+end