Merge branch 'master' into HEAD

AUTHORS

@@ -78,3 +78,4 @@ Peter Fogg <peter.p.fogg@gmail.com>
 Bethany LaPenta <lapentab@mit.edu>
 Renzo Lucioni <renzolucioni@gmail.com>
 Felix Sun <felixsun@mit.edu>
+Adam Palay <adam@edx.org>

CHANGELOG.rst

@@ -5,6 +5,24 @@ These are notable changes in edx-platform.  This is a rolling list of changes,
 in roughly chronological order, most recent first.  Add your entries at or near
 the top.  Include a label indicating the component affected.
 
+Common: Student information is now passed to the tracking log via POST instead of GET.
+
+Common: Add tests for documentation generation to test suite
+
+Blades: User answer now preserved (and changeable) after clicking "show answer" in choice problems
+
+LMS: Users are no longer auto-activated if they click "reset password"
+This is now done when they click on the link in the reset password
+email they receive (along with usual path through activation email).
+
+LMS: Problem rescoring.  Added options on the Grades tab of the
+Instructor Dashboard to allow a particular student's submission for a
+particular problem to be rescored.  Provides an option to see a
+history of background tasks for a given problem and student.
+
+Blades: Small UX fix on capa multiple-choice problems.  Make labels only
+as wide as the text to reduce accidental choice selections.
+
 Studio: Remove XML from the video component editor. All settings are
 moved to be edited as metadata.
 
@@ -48,6 +66,8 @@ setting now run entirely outside the Python sandbox.
 
 Blades: Added tests for Video Alpha player.
 
+Common: Have the capa module handle unicode better (especially errors)
+
 Blades: Video Alpha bug fix for speed changing to 1.0 in Firefox.
 
 Blades: Additional event tracking added to Video Alpha: fullscreen switch, show/hide
@@ -138,3 +158,5 @@ Common: Updated CodeJail.
 
 Common: Allow setting of authentication session cookie name.
 
+LMS: Option to email students when enroll/un-enroll them.
+

README.md

@@ -152,6 +152,12 @@ otherwise noted.
 
 Please see ``LICENSE.txt`` for details.
 
+Documentation
+------------
+
+High-level documentation of the code is located in the `doc` subdirectory. Start
+with `overview.md` to get an introduction to the architecture of the system.
+
 How to Contribute
 -----------------
 

cms/djangoapps/auth/authz.py

@@ -1,5 +1,6 @@
 from django.contrib.auth.models import User, Group
 from django.core.exceptions import PermissionDenied
+from django.conf import settings
 
 from xmodule.modulestore import Location
 
@@ -12,6 +13,9 @@ but this implementation should be data compatible with the LMS implementation
 INSTRUCTOR_ROLE_NAME = 'instructor'
 STAFF_ROLE_NAME = 'staff'
 
+# This is the group of people who have permission to create new courses on edge or edx.
+COURSE_CREATOR_GROUP_NAME = "course_creator_group"
+
 # we're just making a Django group for each location/role combo
 # to do this we're just creating a Group name which is a formatted string
 # of those two variables
@@ -32,14 +36,14 @@ def get_course_groupname_for_role(location, role):
 
 def get_users_in_course_group_by_role(location, role):
     groupname = get_course_groupname_for_role(location, role)
-    (group, created) = Group.objects.get_or_create(name=groupname)
+    (group, _created) = Group.objects.get_or_create(name=groupname)
     return group.user_set.all()
 
 
-'''
-Create all permission groups for a new course and subscribe the caller into those roles
-'''
 def create_all_course_groups(creator, location):
+    """
+    Create all permission groups for a new course and subscribe the caller into those roles
+    """
     create_new_course_group(creator, location, INSTRUCTOR_ROLE_NAME)
     create_new_course_group(creator, location, STAFF_ROLE_NAME)
 
@@ -55,11 +59,12 @@ def create_new_course_group(creator, location, role):
 
     return
 
+
 def _delete_course_group(location):
-    '''
+    """
     This is to be called only by either a command line code path or through a app which has already
     asserted permissions
-    '''
+    """
     # remove all memberships
     instructors = Group.objects.get(name=get_course_groupname_for_role(location, INSTRUCTOR_ROLE_NAME))
     for user in instructors.user_set.all():
@@ -71,11 +76,12 @@ def _delete_course_group(location):
         user.groups.remove(staff)
         user.save()
 
+
 def _copy_course_group(source, dest):
-    '''
+    """
     This is to be called only by either a command line code path or through an app which has already
     asserted permissions to do this action
-    '''
+    """
     instructors = Group.objects.get(name=get_course_groupname_for_role(source, INSTRUCTOR_ROLE_NAME))
     new_instructors_group = Group.objects.get(name=get_course_groupname_for_role(dest, INSTRUCTOR_ROLE_NAME))
     for user in instructors.user_set.all():
@@ -94,10 +100,34 @@ def add_user_to_course_group(caller, user, location, role):
     if not is_user_in_course_group_role(caller, location, INSTRUCTOR_ROLE_NAME):
         raise PermissionDenied
 
-    if user.is_active and user.is_authenticated:
-        groupname = get_course_groupname_for_role(location, role)
+    group = Group.objects.get(name=get_course_groupname_for_role(location, role))
+    return _add_user_to_group(user, group)
 
-        group = Group.objects.get(name=groupname)
+
+def add_user_to_creator_group(caller, user):
+    """
+    Adds the user to the group of course creators.
+
+    The caller must have staff access to perform this operation.
+
+    Note that on the edX site, we currently limit course creators to edX staff, and this
+    method is a no-op in that environment.
+    """
+    if not caller.is_active or not caller.is_authenticated or not caller.is_staff:
+        raise PermissionDenied
+
+    (group, created) = Group.objects.get_or_create(name=COURSE_CREATOR_GROUP_NAME)
+    if created:
+        group.save()
+    return _add_user_to_group(user, group)
+
+
+def _add_user_to_group(user, group):
+    """
+    This is to be called only by either a command line code path or through an app which has already
+    asserted permissions to do this action
+    """
+    if user.is_active and user.is_authenticated:
         user.groups.add(group)
         user.save()
         return True
@@ -123,11 +153,29 @@ def remove_user_from_course_group(caller, user, location, role):
 
     # see if the user is actually in that role, if not then we don't have to do anything
     if is_user_in_course_group_role(user, location, role):
-        groupname = get_course_groupname_for_role(location, role)
+        _remove_user_from_group(user, get_course_groupname_for_role(location, role))
 
-        group = Group.objects.get(name=groupname)
-        user.groups.remove(group)
-        user.save()
+
+def remove_user_from_creator_group(caller, user):
+    """
+    Removes user from the course creator group.
+
+    The caller must have staff access to perform this operation.
+    """
+    if not caller.is_active or not caller.is_authenticated or not caller.is_staff:
+        raise PermissionDenied
+
+    _remove_user_from_group(user, COURSE_CREATOR_GROUP_NAME)
+
+
+def _remove_user_from_group(user, group_name):
+    """
+    This is to be called only by either a command line code path or through an app which has already
+    asserted permissions to do this action
+    """
+    group = Group.objects.get(name=group_name)
+    user.groups.remove(group)
+    user.save()
 
 
 def is_user_in_course_group_role(user, location, role):
@@ -136,3 +184,40 @@ def is_user_in_course_group_role(user, location, role):
         return user.is_staff or user.groups.filter(name=get_course_groupname_for_role(location, role)).count() > 0
 
     return False
+
+
+def is_user_in_creator_group(user):
+    """
+    Returns true if the user has permissions to create a course.
+
+    Will always return True if user.is_staff is True.
+
+    Note that on the edX site, we currently limit course creators to edX staff. On
+    other sites, this method checks that the user is in the course creator group.
+    """
+    if user.is_staff:
+        return True
+
+    # On edx, we only allow edX staff to create courses. This may be relaxed in the future.
+    if settings.MITX_FEATURES.get('DISABLE_COURSE_CREATION', False):
+        return False
+
+    # Feature flag for using the creator group setting. Will be removed once the feature is complete.
+    if settings.MITX_FEATURES.get('ENABLE_CREATOR_GROUP', False):
+        return user.groups.filter(name=COURSE_CREATOR_GROUP_NAME).count() > 0
+
+    return True
+
+
+def _grant_instructors_creator_access(caller):
+    """
+    This is to be called only by either a command line code path or through an app which has already
+    asserted permissions to do this action.
+
+    Gives all users with instructor role course creator rights.
+    This is only intended to be run once on a given environment.
+    """
+    for group in Group.objects.all():
+        if group.name.startswith(INSTRUCTOR_ROLE_NAME + "_"):
+            for user in group.user_set.all():
+                add_user_to_creator_group(caller, user)

cms/djangoapps/auth/tests/test_authz.py

@@ -0,0 +1,215 @@
+"""
+Tests authz.py
+"""
+import mock
+
+from django.test import TestCase
+from django.contrib.auth.models import User
+from django.core.exceptions import PermissionDenied
+
+from auth.authz import add_user_to_creator_group, remove_user_from_creator_group, is_user_in_creator_group,\
+    create_all_course_groups, add_user_to_course_group, STAFF_ROLE_NAME, INSTRUCTOR_ROLE_NAME,\
+    is_user_in_course_group_role, remove_user_from_course_group, _grant_instructors_creator_access
+
+
+class CreatorGroupTest(TestCase):
+    """
+    Tests for the course creator group.
+    """
+
+    def setUp(self):
+        """ Test case setup """
+        self.user = User.objects.create_user('testuser', 'test+courses@edx.org', 'foo')
+        self.admin = User.objects.create_user('Mark', 'admin+courses@edx.org', 'foo')
+        self.admin.is_staff = True
+
+    def test_creator_group_not_enabled(self):
+        """
+        Tests that is_user_in_creator_group always returns True if ENABLE_CREATOR_GROUP
+        and DISABLE_COURSE_CREATION are both not turned on.
+        """
+        self.assertTrue(is_user_in_creator_group(self.user))
+
+    def test_creator_group_enabled_but_empty(self):
+        """ Tests creator group feature on, but group empty. """
+        with mock.patch.dict('django.conf.settings.MITX_FEATURES', {"ENABLE_CREATOR_GROUP": True}):
+            self.assertFalse(is_user_in_creator_group(self.user))
+
+            # Make user staff. This will cause is_user_in_creator_group to return True.
+            self.user.is_staff = True
+            self.assertTrue(is_user_in_creator_group(self.user))
+
+    def test_creator_group_enabled_nonempty(self):
+        """ Tests creator group feature on, user added. """
+        with mock.patch.dict('django.conf.settings.MITX_FEATURES', {"ENABLE_CREATOR_GROUP": True}):
+            self.assertTrue(add_user_to_creator_group(self.admin, self.user))
+            self.assertTrue(is_user_in_creator_group(self.user))
+
+            # check that a user who has not been added to the group still returns false
+            user_not_added = User.objects.create_user('testuser2', 'test+courses2@edx.org', 'foo2')
+            self.assertFalse(is_user_in_creator_group(user_not_added))
+
+            # remove first user from the group and verify that is_user_in_creator_group now returns false
+            remove_user_from_creator_group(self.admin, self.user)
+            self.assertFalse(is_user_in_creator_group(self.user))
+
+    def test_add_user_not_authenticated(self):
+        """
+        Tests that adding to creator group fails if user is not authenticated
+        """
+        self.user.is_authenticated = False
+        self.assertFalse(add_user_to_creator_group(self.admin, self.user))
+
+    def test_add_user_not_active(self):
+        """
+        Tests that adding to creator group fails if user is not active
+        """
+        self.user.is_active = False
+        self.assertFalse(add_user_to_creator_group(self.admin, self.user))
+
+    def test_course_creation_disabled(self):
+        """ Tests that the COURSE_CREATION_DISABLED flag overrides course creator group settings. """
+        with mock.patch.dict('django.conf.settings.MITX_FEATURES',
+                             {'DISABLE_COURSE_CREATION': True, "ENABLE_CREATOR_GROUP": True}):
+            # Add user to creator group.
+            self.assertTrue(add_user_to_creator_group(self.admin, self.user))
+
+            # DISABLE_COURSE_CREATION overrides (user is not marked as staff).
+            self.assertFalse(is_user_in_creator_group(self.user))
+
+            # Mark as staff. Now is_user_in_creator_group returns true.
+            self.user.is_staff = True
+            self.assertTrue(is_user_in_creator_group(self.user))
+
+            # Remove user from creator group. is_user_in_creator_group still returns true because is_staff=True
+            remove_user_from_creator_group(self.admin, self.user)
+            self.assertTrue(is_user_in_creator_group(self.user))
+
+    def test_add_user_to_group_requires_staff_access(self):
+        with self.assertRaises(PermissionDenied):
+            self.admin.is_staff = False
+            add_user_to_creator_group(self.admin, self.user)
+
+        with self.assertRaises(PermissionDenied):
+            add_user_to_creator_group(self.user, self.user)
+
+    def test_add_user_to_group_requires_active(self):
+        with self.assertRaises(PermissionDenied):
+            self.admin.is_active = False
+            add_user_to_creator_group(self.admin, self.user)
+
+    def test_add_user_to_group_requires_authenticated(self):
+        with self.assertRaises(PermissionDenied):
+            self.admin.is_authenticated = False
+            add_user_to_creator_group(self.admin, self.user)
+
+    def test_remove_user_from_group_requires_staff_access(self):
+        with self.assertRaises(PermissionDenied):
+            self.admin.is_staff = False
+            remove_user_from_creator_group(self.admin, self.user)
+
+    def test_remove_user_from_group_requires_active(self):
+        with self.assertRaises(PermissionDenied):
+            self.admin.is_active = False
+            remove_user_from_creator_group(self.admin, self.user)
+
+    def test_remove_user_from_group_requires_authenticated(self):
+        with self.assertRaises(PermissionDenied):
+            self.admin.is_authenticated = False
+            remove_user_from_creator_group(self.admin, self.user)
+
+
+class CourseGroupTest(TestCase):
+    """
+    Tests for instructor and staff groups for a particular course.
+    """
+
+    def setUp(self):
+        """ Test case setup """
+        self.creator = User.objects.create_user('testcreator', 'testcreator+courses@edx.org', 'foo')
+        self.staff = User.objects.create_user('teststaff', 'teststaff+courses@edx.org', 'foo')
+        self.location = 'i4x', 'mitX', '101', 'course', 'test'
+
+    def test_add_user_to_course_group(self):
+        """
+        Tests adding user to course group (happy path).
+        """
+        # Create groups for a new course (and assign instructor role to the creator).
+        self.assertFalse(is_user_in_course_group_role(self.creator, self.location, INSTRUCTOR_ROLE_NAME))
+        create_all_course_groups(self.creator, self.location)
+        self.assertTrue(is_user_in_course_group_role(self.creator, self.location, INSTRUCTOR_ROLE_NAME))
+
+        # Add another user to the staff role.
+        self.assertFalse(is_user_in_course_group_role(self.staff, self.location, STAFF_ROLE_NAME))
+        self.assertTrue(add_user_to_course_group(self.creator, self.staff, self.location, STAFF_ROLE_NAME))
+        self.assertTrue(is_user_in_course_group_role(self.staff, self.location, STAFF_ROLE_NAME))
+
+    def test_add_user_to_course_group_permission_denied(self):
+        """
+        Verifies PermissionDenied if caller of add_user_to_course_group is not instructor role.
+        """
+        create_all_course_groups(self.creator, self.location)
+        with self.assertRaises(PermissionDenied):
+            add_user_to_course_group(self.staff, self.staff, self.location, STAFF_ROLE_NAME)
+
+    def test_remove_user_from_course_group(self):
+        """
+        Tests removing user from course group (happy path).
+        """
+        create_all_course_groups(self.creator, self.location)
+
+        self.assertTrue(add_user_to_course_group(self.creator, self.staff, self.location, STAFF_ROLE_NAME))
+        self.assertTrue(is_user_in_course_group_role(self.staff, self.location, STAFF_ROLE_NAME))
+
+        remove_user_from_course_group(self.creator, self.staff, self.location, STAFF_ROLE_NAME)
+        self.assertFalse(is_user_in_course_group_role(self.staff, self.location, STAFF_ROLE_NAME))
+
+        remove_user_from_course_group(self.creator, self.creator, self.location, INSTRUCTOR_ROLE_NAME)
+        self.assertFalse(is_user_in_course_group_role(self.creator, self.location, INSTRUCTOR_ROLE_NAME))
+
+    def test_remove_user_from_course_group_permission_denied(self):
+        """
+        Verifies PermissionDenied if caller of remove_user_from_course_group is not instructor role.
+        """
+        create_all_course_groups(self.creator, self.location)
+        with self.assertRaises(PermissionDenied):
+            remove_user_from_course_group(self.staff, self.staff, self.location, STAFF_ROLE_NAME)
+
+
+class GrantInstructorsCreatorAccessTest(TestCase):
+    """
+    Tests granting existing instructors course creator rights.
+    """
+    def create_course(self, index):
+        """
+        Creates a course with one instructor and one staff member.
+        """
+        creator = User.objects.create_user('testcreator' + str(index), 'testcreator+courses@edx.org', 'foo')
+        staff = User.objects.create_user('teststaff' + str(index), 'teststaff+courses@edx.org', 'foo')
+        location = 'i4x', 'mitX', str(index), 'course', 'test'
+        create_all_course_groups(creator, location)
+        add_user_to_course_group(creator, staff, location, STAFF_ROLE_NAME)
+        return [creator, staff]
+
+    def test_grant_creator_access(self):
+        """
+        Test for _grant_instructors_creator_access.
+        """
+        [creator1, staff1] = self.create_course(1)
+        [creator2, staff2] = self.create_course(2)
+        with mock.patch.dict('django.conf.settings.MITX_FEATURES', {"ENABLE_CREATOR_GROUP": True}):
+            # Initially no creators.
+            self.assertFalse(is_user_in_creator_group(creator1))
+            self.assertFalse(is_user_in_creator_group(creator2))
+            self.assertFalse(is_user_in_creator_group(staff1))
+            self.assertFalse(is_user_in_creator_group(staff2))
+
+            admin = User.objects.create_user('populate_creators_command', 'grant+creator+access@edx.org', 'foo')
+            admin.is_staff = True
+            _grant_instructors_creator_access(admin)
+
+            # Now instructors only are creators.
+            self.assertTrue(is_user_in_creator_group(creator1))
+            self.assertTrue(is_user_in_creator_group(creator2))
+            self.assertFalse(is_user_in_creator_group(staff1))
+            self.assertFalse(is_user_in_creator_group(staff2))

cms/djangoapps/contentstore/features/advanced-settings.py

@@ -27,7 +27,7 @@ def i_am_on_advanced_course_settings(step):
 
 @step(u'I press the "([^"]*)" notification button$')
 def press_the_notification_button(step, name):
-    css = 'a.%s-button' % name.lower()
+    css = 'a.action-%s' % name.lower()
 
     # Save was clicked if either the save notification bar is gone, or we have a error notification
     # overlaying it (expected in the case of typing Object into display_name).

cms/djangoapps/contentstore/features/component.feature

@@ -0,0 +1,71 @@
+Feature: Component Adding
+    As a course author, I want to be able to add a wide variety of components
+
+    Scenario: I can add components
+        Given I have opened a new course in studio
+        And I am editing a new unit
+        When I add the following components:
+            | Component    |
+            | Discussion   |
+            | Announcement |
+            | Blank HTML   |
+            | LaTex        |
+            | Blank Problem|
+            | Dropdown     |
+            | Multi Choice |
+            | Numerical    |
+            | Text Input   |
+            | Advanced     |
+            | Circuit      |
+            | Custom Python|
+            | Image Mapped |
+            | Math Input   |
+            | Problem LaTex|
+            | Adaptive Hint|
+            | Video        |
+        Then I see the following components:
+            | Component    |
+            | Discussion   |
+            | Announcement |
+            | Blank HTML   |
+            | LaTex        |
+            | Blank Problem|
+            | Dropdown     |
+            | Multi Choice |
+            | Numerical    |
+            | Text Input   |
+            | Advanced     |
+            | Circuit      |
+            | Custom Python|
+            | Image Mapped |
+            | Math Input   |
+            | Problem LaTex|
+            | Adaptive Hint|
+            | Video        |
+
+
+    Scenario: I can delete Components
+        Given I have opened a new course in studio
+        And I am editing a new unit
+        And I add the following components:
+            | Component    |
+            | Discussion   |
+            | Announcement |
+            | Blank HTML   |
+            | LaTex        |
+            | Blank Problem|
+            | Dropdown     |
+            | Multi Choice |
+            | Numerical    |
+            | Text Input   |
+            | Advanced     |
+            | Circuit      |
+            | Custom Python|
+            | Image Mapped |
+            | Math Input   |
+            | Problem LaTex|
+            | Adaptive Hint|
+            | Video        |
+        When I will confirm all alerts
+        And I delete all components
+        Then I see no components

cms/djangoapps/contentstore/features/component.py

@@ -0,0 +1,129 @@
+#pylint: disable=C0111
+#pylint: disable=W0621
+
+from lettuce import world, step
+
+DATA_LOCATION = 'i4x://edx/templates'
+
+
+@step(u'I am editing a new unit')
+def add_unit(step):
+    css_selectors = ['a.new-courseware-section-button', 'input.new-section-name-save', 'a.new-subsection-item',
+                    'input.new-subsection-name-save', 'div.section-item a.expand-collapse-icon', 'a.new-unit-item']
+    for selector in css_selectors:
+        world.css_click(selector)
+
+
+@step(u'I add the following components:')
+def add_components(step):
+    for component in [step_hash['Component'] for step_hash in step.hashes]:
+        assert component in COMPONENT_DICTIONARY
+        for css in COMPONENT_DICTIONARY[component]['steps']:
+            world.css_click(css)
+
+
+@step(u'I see the following components')
+def check_components(step):
+    for component in [step_hash['Component'] for step_hash in step.hashes]:
+        assert component in COMPONENT_DICTIONARY
+        assert COMPONENT_DICTIONARY[component]['found_func']()
+
+
+@step(u'I delete all components')
+def delete_all_components(step):
+    for _ in range(len(COMPONENT_DICTIONARY)):
+        world.css_click('a.delete-button')
+
+
+@step(u'I see no components')
+def see_no_components(steps):
+    assert world.is_css_not_present('li.component')
+
+
+def step_selector_list(data_type, path, index=1):
+    selector_list = ['a[data-type="{}"]'.format(data_type)]
+    if index != 1:
+        selector_list.append('a[id="ui-id-{}"]'.format(index))
+    if path is not None:
+        selector_list.append('a[data-location="{}/{}/{}"]'.format(DATA_LOCATION, data_type, path))
+    return selector_list
+
+
+def found_text_func(text):
+    return lambda: world.browser.is_text_present(text)
+
+
+def found_css_func(css):
+    return lambda: world.is_css_present(css, wait_time=2)
+
+COMPONENT_DICTIONARY = {
+    'Discussion': {
+        'steps': step_selector_list('discussion', None),
+        'found_func': found_css_func('section.xmodule_DiscussionModule')
+    },
+    'Announcement': {
+        'steps': step_selector_list('html', 'Announcement'),
+        'found_func': found_text_func('Heading of document')
+    },
+    'Blank HTML': {
+        'steps': step_selector_list('html', 'Blank_HTML_Page'),
+        #this one is a blank html so a more refined search is being done
+        'found_func': lambda: '\n    \n' in [x.html for x in world.css_find('section.xmodule_HtmlModule')]
+    },
+    'LaTex': {
+        'steps': step_selector_list('html', 'E-text_Written_in_LaTeX'),
+        'found_func': found_text_func('EXAMPLE: E-TEXT PAGE')
+    },
+    'Blank Problem': {
+        'steps': step_selector_list('problem', 'Blank_Common_Problem'),
+        'found_func': found_text_func('BLANK COMMON PROBLEM')
+    },
+    'Dropdown': {
+        'steps': step_selector_list('problem', 'Dropdown'),
+        'found_func': found_text_func('DROPDOWN')
+    },
+    'Multi Choice': {
+        'steps': step_selector_list('problem', 'Multiple_Choice'),
+        'found_func': found_text_func('MULTIPLE CHOICE')
+    },
+    'Numerical': {
+        'steps': step_selector_list('problem', 'Numerical_Input'),
+        'found_func': found_text_func('NUMERICAL INPUT')
+    },
+    'Text Input': {
+        'steps': step_selector_list('problem', 'Text_Input'),
+        'found_func': found_text_func('TEXT INPUT')
+    },
+    'Advanced': {
+        'steps': step_selector_list('problem', 'Blank_Advanced_Problem', index=2),
+        'found_func': found_text_func('BLANK ADVANCED PROBLEM')
+    },
+    'Circuit': {
+        'steps': step_selector_list('problem', 'Circuit_Schematic_Builder', index=2),
+        'found_func': found_text_func('CIRCUIT SCHEMATIC BUILDER')
+    },
+    'Custom Python': {
+        'steps': step_selector_list('problem', 'Custom_Python-Evaluated_Input', index=2),
+        'found_func': found_text_func('CUSTOM PYTHON-EVALUATED INPUT')
+    },
+    'Image Mapped': {
+        'steps': step_selector_list('problem', 'Image_Mapped_Input', index=2),
+        'found_func': found_text_func('IMAGE MAPPED INPUT')
+    },
+    'Math Input': {
+        'steps': step_selector_list('problem', 'Math_Expression_Input', index=2),
+        'found_func': found_text_func('MATH EXPRESSION INPUT')
+    },
+    'Problem LaTex': {
+        'steps': step_selector_list('problem', 'Problem_Written_in_LaTeX', index=2),
+        'found_func': found_text_func('PROBLEM WRITTEN IN LATEX')
+    },
+    'Adaptive Hint': {
+        'steps': step_selector_list('problem', 'Problem_with_Adaptive_Hint', index=2),
+        'found_func': found_text_func('PROBLEM WITH ADAPTIVE HINT')
+    },
+    'Video': {
+        'steps': step_selector_list('video', None),
+        'found_func': found_css_func('section.xmodule_VideoModule')
+    }
+}

cms/djangoapps/contentstore/features/upload.feature

@@ -21,6 +21,7 @@ Feature: Upload Files
         When I upload the file "test"
         And I delete the file "test"
         Then I should not see the file "test" was uploaded
+        And I see a confirmation that the file was deleted
 
     Scenario: Users can download files
         Given I have opened a new course in studio

cms/djangoapps/contentstore/features/upload.py

@@ -90,6 +90,12 @@ def modify_upload(_step, file_name):
         cur_file.write(new_text)
 
 
+@step('I see a confirmation that the file was deleted')
+def i_see_a_delete_confirmation(_step):
+    alert_css = '#notification-confirmation'
+    assert world.is_css_present(alert_css)
+
+
 def get_index(file_name):
     names_css = 'td.name-col > a.filename'
     all_names = world.css_find(names_css)

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

@@ -1,5 +1,5 @@
 # disable missing docstring
-#pylint: disable=C0111
+# pylint: disable=C0111
 
 from lettuce import world, step
 

cms/djangoapps/contentstore/management/commands/populate_creators.py

@@ -0,0 +1,35 @@
+"""
+Script for granting existing course instructors course creator privileges.
+
+This script is only intended to be run once on a given environment.
+"""
+from auth.authz import _grant_instructors_creator_access
+from django.core.management.base import BaseCommand
+
+from django.contrib.auth.models import User
+from django.db.utils import IntegrityError
+
+
+class Command(BaseCommand):
+    """
+    Script for granting existing course instructors course creator privileges.
+    """
+    help = 'Grants all users with INSTRUCTOR role permission to create courses'
+
+    def handle(self, *args, **options):
+        """
+        The logic of the command.
+        """
+        username = 'populate_creators_command'
+        email = 'grant+creator+access@edx.org'
+        try:
+            admin = User.objects.create_user(username, email, 'foo')
+            admin.is_staff = True
+            admin.save()
+        except IntegrityError:
+            # If the script did not complete the last time it was run,
+            # the admin user will already exist.
+            admin = User.objects.get(username=username, email=email)
+
+        _grant_instructors_creator_access(admin)
+        admin.delete()

cms/djangoapps/contentstore/module_info_model.py

@@ -5,10 +5,7 @@ from xmodule.modulestore import Location
 
 def get_module_info(store, location, parent_location=None, rewrite_static_links=False):
     try:
-        if location.revision is None:
-            module = store.get_item(location)
-        else:
-            module = store.get_item(location)
+        module = store.get_item(location)
     except ItemNotFoundError:
         # create a new one
         template_location = Location(['i4x', 'edx', 'templates', location.category, 'Empty'])

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -1,5 +1,6 @@
 import json
 import shutil
+import mock
 from django.test.client import Client
 from django.test.utils import override_settings
 from django.conf import settings
@@ -16,6 +17,8 @@ from django.dispatch import Signal
 from contentstore.utils import get_modulestore
 from contentstore.tests.utils import parse_json
 
+from auth.authz import add_user_to_creator_group
+
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 
@@ -23,7 +26,7 @@ from xmodule.modulestore import Location
 from xmodule.modulestore.store_utilities import clone_course
 from xmodule.modulestore.store_utilities import delete_course
 from xmodule.modulestore.django import modulestore
-from xmodule.contentstore.django import contentstore
+from xmodule.contentstore.django import contentstore, _CONTENTSTORE
 from xmodule.templates import update_templates
 from xmodule.modulestore.xml_exporter import export_to_xml
 from xmodule.modulestore.xml_importer import import_from_xml, perform_xlint
@@ -43,10 +46,12 @@ from django_comment_common.utils import are_permissions_roles_seeded
 from xmodule.exceptions import InvalidVersionError
 import datetime
 from pytz import UTC
+from uuid import uuid4
+from pymongo import MongoClient
 
-TEST_DATA_MODULESTORE = copy.deepcopy(settings.MODULESTORE)
-TEST_DATA_MODULESTORE['default']['OPTIONS']['fs_root'] = path('common/test/data')
-TEST_DATA_MODULESTORE['direct']['OPTIONS']['fs_root'] = path('common/test/data')
+
+TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE)
+TEST_DATA_CONTENTSTORE['OPTIONS']['db'] = 'test_xcontent_%s' % uuid4().hex
 
 
 class MongoCollectionFindWrapper(object):
@@ -59,13 +64,16 @@ class MongoCollectionFindWrapper(object):
         return self.original(query, *args, **kwargs)
 
 
-@override_settings(MODULESTORE=TEST_DATA_MODULESTORE)
+@override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE)
 class ContentStoreToyCourseTest(ModuleStoreTestCase):
     """
     Tests that rely on the toy courses.
     TODO: refactor using CourseFactory so they do not.
     """
     def setUp(self):
+
+        settings.MODULESTORE['default']['OPTIONS']['fs_root'] = path('common/test/data')
+        settings.MODULESTORE['direct']['OPTIONS']['fs_root'] = path('common/test/data')
         uname = 'testuser'
         email = 'test+courses@edx.org'
         password = 'foo'
@@ -83,6 +91,11 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         self.client = Client()
         self.client.login(username=uname, password=password)
 
+    def tearDown(self):
+        mongo = MongoClient()
+        mongo.drop_database(TEST_DATA_CONTENTSTORE['OPTIONS']['db'])
+        _CONTENTSTORE.clear()
+
     def check_components_on_page(self, component_types, expected_types):
         """
         Ensure that the right types end up on the page.
@@ -403,7 +416,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         self.assertGreater(len(all_assets), 0)
 
         # make sure we have some thumbnails in our contentstore
-        all_thumbnails = content_store.get_all_content_thumbnails_for_course(course_location)
+        content_store.get_all_content_thumbnails_for_course(course_location)
 
         #
         # cdodge: temporarily comment out assertion on thumbnails because many environments
@@ -442,7 +455,6 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         content_store = contentstore()
         trash_store = contentstore('trashcan')
         module_store = modulestore('direct')
-
         import_from_xml(module_store, 'common/test/data/', ['full'], static_content_store=content_store)
 
         # look up original (and thumbnail) in content store, should be there after import
@@ -519,7 +531,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         self.assertGreater(len(all_assets), 0)
 
         # make sure we have some thumbnails in our trashcan
-        all_thumbnails = trash_store.get_all_content_thumbnails_for_course(course_location)
+        _all_thumbnails = trash_store.get_all_content_thumbnails_for_course(course_location)
         #
         # cdodge: temporarily comment out assertion on thumbnails because many environments
         # will not have the jpeg converter installed and this test will fail
@@ -533,7 +545,6 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         all_assets = trash_store.get_all_content_for_course(course_location)
         self.assertEqual(len(all_assets), 0)
 
-
         all_thumbnails = trash_store.get_all_content_thumbnails_for_course(course_location)
         self.assertEqual(len(all_thumbnails), 0)
 
@@ -581,24 +592,21 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
         location = Location('i4x://MITx/999/chapter/neuvo')
         self.assertRaises(InvalidVersionError, draft_store.clone_item, 'i4x://edx/templates/chapter/Empty',
-            location)
+                          location)
         direct_store.clone_item('i4x://edx/templates/chapter/Empty', location)
-        self.assertRaises(InvalidVersionError, draft_store.clone_item, location,
-            location)
+        self.assertRaises(InvalidVersionError, draft_store.clone_item, location, location)
 
-        self.assertRaises(InvalidVersionError, draft_store.update_item, location,
-            'chapter data')
+        self.assertRaises(InvalidVersionError, draft_store.update_item, location, 'chapter data')
 
         # taking advantage of update_children and other functions never checking that the ids are valid
         self.assertRaises(InvalidVersionError, draft_store.update_children, location,
-            ['i4x://MITx/999/problem/doesntexist'])
+                          ['i4x://MITx/999/problem/doesntexist'])
 
         self.assertRaises(InvalidVersionError, draft_store.update_metadata, location,
-            {'due': datetime.datetime.now(UTC)})
+                          {'due': datetime.datetime.now(UTC)})
 
         self.assertRaises(InvalidVersionError, draft_store.unpublish, location)
 
-
     def test_bad_contentstore_request(self):
         resp = self.client.get('http://localhost:8001/c4x/CDX/123123/asset/&images_circuits_Lab7Solution2.png')
         self.assertEqual(resp.status_code, 400)
@@ -809,6 +817,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         export_to_xml(module_store, content_store, location, root_dir, 'test_export')
 
 
+@override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE)
 class ContentStoreTest(ModuleStoreTestCase):
     """
     Tests for the CMS ContentStore application.
@@ -845,8 +854,19 @@ class ContentStoreTest(ModuleStoreTestCase):
             'display_name': 'Robot Super Course',
         }
 
+    def tearDown(self):
+        mongo = MongoClient()
+        mongo.drop_database(TEST_DATA_CONTENTSTORE['OPTIONS']['db'])
+        _CONTENTSTORE.clear()
+
     def test_create_course(self):
         """Test new course creation - happy path"""
+        self.assert_created_course()
+
+    def assert_created_course(self):
+        """
+        Checks that the course was created properly.
+        """
         resp = self.client.post(reverse('create_new_course'), self.course_data)
         self.assertEqual(resp.status_code, 200)
         data = parse_json(resp)
@@ -854,41 +874,72 @@ class ContentStoreTest(ModuleStoreTestCase):
 
     def test_create_course_check_forum_seeding(self):
         """Test new course creation and verify forum seeding """
-        resp = self.client.post(reverse('create_new_course'), self.course_data)
-        self.assertEqual(resp.status_code, 200)
-        data = parse_json(resp)
-        self.assertEqual(data['id'], 'i4x://MITx/999/course/Robot_Super_Course')
+        self.assert_created_course()
         self.assertTrue(are_permissions_roles_seeded('MITx/999/Robot_Super_Course'))
 
     def test_create_course_duplicate_course(self):
         """Test new course creation - error path"""
         self.client.post(reverse('create_new_course'), self.course_data)
+        self.assert_course_creation_failed('There is already a course defined with this name.')
+
+    def assert_course_creation_failed(self, error_message):
+        """
+        Checks that the course did not get created
+        """
         resp = self.client.post(reverse('create_new_course'), self.course_data)
-        data = parse_json(resp)
         self.assertEqual(resp.status_code, 200)
-        self.assertEqual(data['ErrMsg'], 'There is already a course defined with this name.')
+        data = parse_json(resp)
+        self.assertEqual(data['ErrMsg'], error_message)
 
     def test_create_course_duplicate_number(self):
         """Test new course creation - error path"""
         self.client.post(reverse('create_new_course'), self.course_data)
         self.course_data['display_name'] = 'Robot Super Course Two'
 
-        resp = self.client.post(reverse('create_new_course'), self.course_data)
-        data = parse_json(resp)
-
-        self.assertEqual(resp.status_code, 200)
-        self.assertEqual(data['ErrMsg'],
-                         'There is already a course defined with the same organization and course number.')
+        self.assert_course_creation_failed('There is already a course defined with the same organization and course number.')
 
     def test_create_course_with_bad_organization(self):
         """Test new course creation - error path for bad organization name"""
         self.course_data['org'] = 'University of California, Berkeley'
-        resp = self.client.post(reverse('create_new_course'), self.course_data)
-        data = parse_json(resp)
+        self.assert_course_creation_failed(
+            "Unable to create course 'Robot Super Course'.\n\nInvalid characters in 'University of California, Berkeley'.")
 
-        self.assertEqual(resp.status_code, 200)
-        self.assertEqual(data['ErrMsg'],
-                         "Unable to create course 'Robot Super Course'.\n\nInvalid characters in 'University of California, Berkeley'.")
+    def test_create_course_with_course_creation_disabled_staff(self):
+        """Test new course creation -- course creation disabled, but staff access."""
+        with mock.patch.dict('django.conf.settings.MITX_FEATURES', {'DISABLE_COURSE_CREATION': True}):
+            self.assert_created_course()
+
+    def test_create_course_with_course_creation_disabled_not_staff(self):
+        """Test new course creation -- error path for course creation disabled, not staff access."""
+        with mock.patch.dict('django.conf.settings.MITX_FEATURES', {'DISABLE_COURSE_CREATION': True}):
+            self.user.is_staff = False
+            self.user.save()
+            self.assert_course_permission_denied()
+
+    def test_create_course_no_course_creators_staff(self):
+        """Test new course creation -- course creation group enabled, staff, group is empty."""
+        with mock.patch.dict('django.conf.settings.MITX_FEATURES', {'ENABLE_CREATOR_GROUP': True}):
+            self.assert_created_course()
+
+    def test_create_course_no_course_creators_not_staff(self):
+        """Test new course creation -- error path for course creator group enabled, not staff, group is empty."""
+        with mock.patch.dict('django.conf.settings.MITX_FEATURES', {"ENABLE_CREATOR_GROUP": True}):
+            self.user.is_staff = False
+            self.user.save()
+            self.assert_course_permission_denied()
+
+    def test_create_course_with_course_creator(self):
+        """Test new course creation -- use course creator group"""
+        with mock.patch.dict('django.conf.settings.MITX_FEATURES', {"ENABLE_CREATOR_GROUP": True}):
+            add_user_to_creator_group(self.user, self.user)
+            self.assert_created_course()
+
+    def assert_course_permission_denied(self):
+        """
+        Checks that the course did not get created due to a PermissionError.
+        """
+        resp = self.client.post(reverse('create_new_course'), self.course_data)
+        self.assertEqual(resp.status_code, 403)
 
     def test_course_index_view_with_no_courses(self):
         """Test viewing the index page with no courses"""

cms/djangoapps/contentstore/tests/test_course_settings.py

@@ -105,7 +105,6 @@ class CourseDetailsTestCase(CourseTestCase):
         self.assertEqual(jsondetails['string'], 'string')
 
     def test_update_and_fetch(self):
-        # # NOTE: I couldn't figure out how to validly test time setting w/ all the conversions
         jsondetails = CourseDetails.fetch(self.course_location)
         jsondetails.syllabus = "<a href='foo'>bar</a>"
         # encode - decode to convert date fields and other data which changes form
@@ -128,6 +127,11 @@ class CourseDetailsTestCase(CourseTestCase):
             CourseDetails.update_from_json(jsondetails.__dict__).effort,
             jsondetails.effort, "After set effort"
         )
+        jsondetails.start_date = datetime.datetime(2010, 10, 1, 0, tzinfo=UTC())
+        self.assertEqual(
+            CourseDetails.update_from_json(jsondetails.__dict__).start_date,
+            jsondetails.start_date
+        )
 
     @override_settings(MKTG_URLS={'ROOT': 'dummy-root'})
     def test_marketing_site_fetch(self):
@@ -235,8 +239,7 @@ class CourseDetailsViewTest(CourseTestCase):
                 dt1 = date.from_json(encoded[field])
                 dt2 = details[field]
 
-                expected_delta = datetime.timedelta(0)
-                self.assertEqual(dt1 - dt2, expected_delta, str(dt1) + "!=" + str(dt2) + " at " + context)
+                self.assertEqual(dt1, dt2, msg="{} != {} at {}".format(dt1, dt2, context))
             else:
                 self.fail(field + " missing from encoded but in details at " + context)
         elif field in encoded and encoded[field] is not None:

cms/djangoapps/contentstore/tests/test_request_event.py

@@ -0,0 +1,36 @@
+"""Tests for CMS's requests to logs"""
+from django.test import TestCase
+from django.core.urlresolvers import reverse
+from contentstore.views.requests import event as cms_user_track
+
+
+class CMSLogTest(TestCase):
+    """
+    Tests that request to logs from CMS return 204s
+    """
+
+    def test_post_answers_to_log(self):
+        """
+        Checks that student answer requests submitted to cms's "/event" url
+        via POST are correctly returned as 204s
+        """
+        requests = [
+            {"event": "my_event", "event_type": "my_event_type", "page": "my_page"},
+            {"event": "{'json': 'object'}", "event_type": unichr(512), "page": "my_page"}
+        ]
+        for request_params in requests:
+            response = self.client.post(reverse(cms_user_track), request_params)
+            self.assertEqual(response.status_code, 204)
+
+    def test_get_answers_to_log(self):
+        """
+        Checks that student answer requests submitted to cms's "/event" url
+        via GET are correctly returned as 204s
+        """
+        requests = [
+            {"event": "my_event", "event_type": "my_event_type", "page": "my_page"},
+            {"event": "{'json': 'object'}", "event_type": unichr(512), "page": "my_page"}
+        ]
+        for request_params in requests:
+            response = self.client.get(reverse(cms_user_track), request_params)
+            self.assertEqual(response.status_code, 204)

cms/djangoapps/contentstore/utils.py

@@ -10,7 +10,7 @@ from xmodule.modulestore.draft import DIRECT_ONLY_CATEGORIES
 
 log = logging.getLogger(__name__)
 
-#In order to instantiate an open ended tab automatically, need to have this data
+# In order to instantiate an open ended tab automatically, need to have this data
 OPEN_ENDED_PANEL = {"name": "Open Ended Panel", "type": "open_ended"}
 NOTES_PANEL = {"name": "My Notes", "type": "notes"}
 EXTRA_TAB_PANELS = dict([(p['type'], p) for p in [OPEN_ENDED_PANEL, NOTES_PANEL]])

cms/djangoapps/contentstore/views/access.py

@@ -2,12 +2,13 @@ from auth.authz import STAFF_ROLE_NAME, INSTRUCTOR_ROLE_NAME
 from auth.authz import is_user_in_course_group_role
 from django.core.exceptions import PermissionDenied
 from ..utils import get_course_location_for_item
+from xmodule.modulestore import Location
 
 
 def get_location_and_verify_access(request, org, course, name):
     """
-    Create the location tuple verify that the user has permissions
-    to view the location. Returns the location.
+    Create the location, verify that the user has permissions
+    to view the location. Returns the location as a Location
     """
     location = ['i4x', org, course, 'course', name]
 
@@ -15,7 +16,7 @@ def get_location_and_verify_access(request, org, course, name):
     if not has_access(request.user, location):
         raise PermissionDenied()
 
-    return location
+    return Location(location)
 
 
 def has_access(user, location, role=STAFF_ROLE_NAME):

cms/djangoapps/contentstore/views/assets.py

@@ -240,13 +240,13 @@ def import_course(request, org, course, name):
         # find the 'course.xml' file
 
         for dirpath, _dirnames, filenames in os.walk(course_dir):
-            for files in filenames:
-                if files == 'course.xml':
+            for filename in filenames:
+                if filename == 'course.xml':
                     break
-            if files == 'course.xml':
+            if filename == 'course.xml':
                 break
 
-        if files != 'course.xml':
+        if filename != 'course.xml':
             return HttpResponse(json.dumps({'ErrMsg': 'Could not find the course.xml file in the package.'}))
 
         logging.debug('found course.xml at {0}'.format(dirpath))
@@ -258,7 +258,7 @@ def import_course(request, org, course, name):
         _module_store, course_items = import_from_xml(modulestore('direct'), settings.GITHUB_REPO_ROOT,
                                                       [course_subdir], load_error_modules=False,
                                                       static_content_store=contentstore(),
-                                                      target_location_namespace=Location(location),
+                                                      target_location_namespace=location,
                                                       draft_store=modulestore())
 
         # we can blow this away when we're done importing.

cms/djangoapps/contentstore/views/checklist.py

@@ -67,7 +67,9 @@ def update_checklist(request, org, course, name, checklist_index=None):
         if checklist_index is not None and 0 <= int(checklist_index) < len(course_module.checklists):
             index = int(checklist_index)
             course_module.checklists[index] = json.loads(request.body)
-            checklists, modified = expand_checklist_action_urls(course_module)
+            # seeming noop which triggers kvs to record that the metadata is not default
+            course_module.checklists = course_module.checklists
+            checklists, _ = expand_checklist_action_urls(course_module)
             modulestore.update_metadata(location, own_metadata(course_module))
             return HttpResponse(json.dumps(checklists[index]), mimetype="application/json")
         else:

cms/djangoapps/contentstore/views/component.py

@@ -38,7 +38,8 @@ __all__ = ['OPEN_ENDED_COMPONENT_TYPES',
 
 log = logging.getLogger(__name__)
 
-COMPONENT_TYPES = ['customtag', 'discussion', 'html', 'problem', 'video']
+# NOTE: edit_unit assumes this list is disjoint from ADVANCED_COMPONENT_TYPES
+COMPONENT_TYPES = ['discussion', 'html', 'problem', 'video']
 
 OPEN_ENDED_COMPONENT_TYPES = ["combinedopenended", "peergrading"]
 NOTE_COMPONENT_TYPES = ['notes']
@@ -220,7 +221,7 @@ def edit_unit(request, location):
         'section': containing_section,
         'create_new_unit_template': Location('i4x', 'edx', 'templates', 'vertical', 'Empty'),
         'unit_state': unit_state,
-        'published_date': item.cms.published_date.strftime('%B %d, %Y') if item.cms.published_date is not None else None,
+        'published_date': get_default_time_display(item.cms.published_date) if item.cms.published_date is not None else None
     })
 
 

cms/djangoapps/contentstore/views/course.py

@@ -21,7 +21,7 @@ from contentstore.utils import get_lms_link_for_item, add_extra_panel_tab, remov
 from models.settings.course_details import CourseDetails, CourseSettingsEncoder
 from models.settings.course_grading import CourseGradingModel
 from models.settings.course_metadata import CourseMetadata
-from auth.authz import create_all_course_groups
+from auth.authz import create_all_course_groups, is_user_in_creator_group
 from util.json_request import expect_json
 
 from .access import has_access, get_location_and_verify_access
@@ -81,7 +81,7 @@ def course_index(request, org, course, name):
 @expect_json
 def create_new_course(request):
 
-    if settings.MITX_FEATURES.get('DISABLE_COURSE_CREATION', False) and not request.user.is_staff:
+    if not is_user_in_creator_group(request.user):
         raise PermissionDenied()
 
     # This logic is repeated in xmodule/modulestore/tests/factories.py
@@ -153,7 +153,7 @@ def course_info(request, org, course, name, provided_id=None):
     course_module = modulestore().get_item(location)
 
     # get current updates
-    location = ['i4x', org, course, 'course_info', "updates"]
+    location = Location(['i4x', org, course, 'course_info', "updates"])
 
     return render_to_response('course_info.html', {
         'active_tab': 'courseinfo-tab',

cms/djangoapps/contentstore/views/error.py

@@ -1,20 +1,45 @@
-from django.http import HttpResponseServerError, HttpResponseNotFound
+from django.http import (HttpResponse, HttpResponseServerError,
+                         HttpResponseNotFound)
 from mitxmako.shortcuts import render_to_string, render_to_response
+import functools
+import json
 
 __all__ = ['not_found', 'server_error', 'render_404', 'render_500']
 
 
+def jsonable_error(status=500, message="The Studio servers encountered an error"):
+    """
+    A decorator to make an error view return an JSON-formatted message if
+    it was requested via AJAX.
+    """
+    def outer(func):
+        @functools.wraps(func)
+        def inner(request, *args, **kwargs):
+            if request.is_ajax():
+                content = json.dumps({"error": message})
+                return HttpResponse(content, content_type="application/json",
+                                    status=status)
+            else:
+                return func(request, *args, **kwargs)
+        return inner
+    return outer
+
+
+@jsonable_error(404, "Resource not found")
 def not_found(request):
     return render_to_response('error.html', {'error': '404'})
 
 
+@jsonable_error(500, "The Studio servers encountered an error")
 def server_error(request):
     return render_to_response('error.html', {'error': '500'})
 
 
+@jsonable_error(404, "Resource not found")
 def render_404(request):
     return HttpResponseNotFound(render_to_string('404.html', {}))
 
 
+@jsonable_error(500, "The Studio servers encountered an error")
 def render_500(request):
     return HttpResponseServerError(render_to_string('500.html', {}))

cms/djangoapps/contentstore/views/session_kv_store.py

@@ -2,27 +2,27 @@ from xblock.runtime import KeyValueStore, InvalidScopeError
 
 
 class SessionKeyValueStore(KeyValueStore):
-    def __init__(self, request, model_data):
-        self._model_data = model_data
+    def __init__(self, request, descriptor_model_data):
+        self._descriptor_model_data = descriptor_model_data
         self._session = request.session
 
     def get(self, key):
         try:
-            return self._model_data[key.field_name]
+            return self._descriptor_model_data[key.field_name]
         except (KeyError, InvalidScopeError):
             return self._session[tuple(key)]
 
     def set(self, key, value):
         try:
-            self._model_data[key.field_name] = value
+            self._descriptor_model_data[key.field_name] = value
         except (KeyError, InvalidScopeError):
             self._session[tuple(key)] = value
 
     def delete(self, key):
         try:
-            del self._model_data[key.field_name]
+            del self._descriptor_model_data[key.field_name]
         except (KeyError, InvalidScopeError):
             del self._session[tuple(key)]
 
     def has(self, key):
-        return key in self._model_data or key in self._session
+        return key.field_name in self._descriptor_model_data or tuple(key) in self._session

cms/djangoapps/models/settings/course_details.py

@@ -74,7 +74,7 @@ class CourseDetails(object):
         Decode the json into CourseDetails and save any changed attrs to the db
         """
         # TODO make it an error for this to be undefined & for it to not be retrievable from modulestore
-        course_location = jsondict['course_location']
+        course_location = Location(jsondict['course_location'])
         # Will probably want to cache the inflight courses because every blur generates an update
         descriptor = get_modulestore(course_location).get_item(course_location)
 

cms/envs/acceptance.py

@@ -23,12 +23,12 @@ MODULESTORE_OPTIONS = {
     'db': 'test_xmodule',
     'collection': 'acceptance_modulestore',
     'fs_root': TEST_ROOT / "data",
-    'render_template': 'mitxmako.shortcuts.render_to_string'
+    'render_template': 'mitxmako.shortcuts.render_to_string',
 }
 
 MODULESTORE = {
     'default': {
-        'ENGINE': 'xmodule.modulestore.mongo.DraftMongoModuleStore',
+        'ENGINE': 'xmodule.modulestore.draft.DraftModuleStore',
         'OPTIONS': MODULESTORE_OPTIONS
     },
     'direct': {
@@ -36,10 +36,25 @@ MODULESTORE = {
         'OPTIONS': MODULESTORE_OPTIONS
     },
     'draft': {
-        'ENGINE': 'xmodule.modulestore.mongo.DraftMongoModuleStore',
+        'ENGINE': 'xmodule.modulestore.draft.DraftModuleStore',
         'OPTIONS': MODULESTORE_OPTIONS
     }
 }
+
+CONTENTSTORE = {
+    'ENGINE': 'xmodule.contentstore.mongo.MongoContentStore',
+    'OPTIONS': {
+        'host': 'localhost',
+        'db': 'acceptance_xcontent',
+    },
+    # allow for additional options that can be keyed on a name, e.g. 'trashcan'
+    'ADDITIONAL_OPTIONS': {
+        'trashcan': {
+            'bucket': 'trash_fs'
+        }
+    }
+}
+
 # Set this up so that rake lms[acceptance] and running the
 # harvest command both use the same (test) database
 # which they can flush without messing up your dev db

cms/envs/common.py

@@ -54,7 +54,11 @@ MITX_FEATURES = {
     'ENABLE_SERVICE_STATUS': False,
 
     # Don't autoplay videos for course authors
-    'AUTOPLAY_VIDEOS': False
+    'AUTOPLAY_VIDEOS': False,
+
+    # If set to True, new Studio users won't be able to author courses unless
+    # edX has explicitly added them to the course creator group.
+    'ENABLE_CREATOR_GROUP': False
 }
 ENABLE_JASMINE = False
 

cms/envs/dev.py

@@ -22,12 +22,12 @@ modulestore_options = {
     'db': 'xmodule',
     'collection': 'modulestore',
     'fs_root': GITHUB_REPO_ROOT,
-    'render_template': 'mitxmako.shortcuts.render_to_string'
+    'render_template': 'mitxmako.shortcuts.render_to_string',
 }
 
 MODULESTORE = {
     'default': {
-        'ENGINE': 'xmodule.modulestore.mongo.DraftMongoModuleStore',
+        'ENGINE': 'xmodule.modulestore.draft.DraftModuleStore',
         'OPTIONS': modulestore_options
     },
     'direct': {
@@ -181,6 +181,6 @@ if SEGMENT_IO_KEY:
 #####################################################################
 # Lastly, see if the developer has any local overrides.
 try:
-    from .private import *
+    from .private import *      # pylint: disable=F0401
 except ImportError:
     pass

cms/envs/test.py

@@ -48,12 +48,12 @@ MODULESTORE_OPTIONS = {
     'db': 'test_xmodule',
     'collection': 'test_modulestore',
     'fs_root': TEST_ROOT / "data",
-    'render_template': 'mitxmako.shortcuts.render_to_string'
+    'render_template': 'mitxmako.shortcuts.render_to_string',
 }
 
 MODULESTORE = {
     'default': {
-        'ENGINE': 'xmodule.modulestore.mongo.DraftMongoModuleStore',
+        'ENGINE': 'xmodule.modulestore.draft.DraftModuleStore',
         'OPTIONS': MODULESTORE_OPTIONS
     },
     'direct': {
@@ -61,7 +61,7 @@ MODULESTORE = {
         'OPTIONS': MODULESTORE_OPTIONS
     },
     'draft': {
-        'ENGINE': 'xmodule.modulestore.mongo.DraftMongoModuleStore',
+        'ENGINE': 'xmodule.modulestore.draft.DraftModuleStore',
         'OPTIONS': MODULESTORE_OPTIONS
     }
 }
@@ -70,7 +70,7 @@ CONTENTSTORE = {
     'ENGINE': 'xmodule.contentstore.mongo.MongoContentStore',
     'OPTIONS': {
         'host': 'localhost',
-        'db': 'test_xmodule',
+        'db': 'test_xcontent',
     },
     # allow for additional options that can be keyed on a name, e.g. 'trashcan'
     'ADDITIONAL_OPTIONS': {
@@ -140,3 +140,6 @@ SEGMENT_IO_KEY = '***REMOVED***'
 MITX_FEATURES['STUDIO_NPS_SURVEY'] = False
 
 MITX_FEATURES['ENABLE_SERVICE_STATUS'] = True
+
+# Enabling SQL tracking logs for testing on common/djangoapps/track
+MITX_FEATURES['ENABLE_SQL_TRACKING_LOGS'] = True

cms/static/coffee/spec/views/feedback_spec.coffee

@@ -17,6 +17,16 @@ beforeEach ->
                 return text.test(trimmedText)
             else
                 return trimmedText.indexOf(text) != -1;
+        toHaveBeenPrevented: ->
+            # remove this when we upgrade jasmine-jquery
+            eventName = @actual.eventName
+            selector = @actual.selector
+            @message = ->
+                [
+                  "Expected event #{eventName} to have been prevented on #{selector}",
+                  "Expected event #{eventName} not to have been prevented on #{selector}"
+                ]
+            return jasmine.JQuery.events.wasPrevented(selector, eventName)
 
 describe "CMS.Views.SystemFeedback", ->
     beforeEach ->
@@ -100,11 +110,10 @@ describe "CMS.Views.SystemFeedback click events", ->
                     text: "Save",
                     class: "save-button",
                     click: @primaryClickSpy
-                secondary: [{
+                secondary:
                     text: "Revert",
                     class: "cancel-button",
                     click: @secondaryClickSpy
-                }]
         )
         @view.show()
 
@@ -124,6 +133,75 @@ describe "CMS.Views.SystemFeedback click events", ->
     it "should apply class to secondary action", ->
         expect(@view.$(".action-secondary")).toHaveClass("cancel-button")
 
+    it "should preventDefault on primary action", ->
+        spyOnEvent(".action-primary", "click")
+        @view.$(".action-primary").click()
+        expect("click").toHaveBeenPreventedOn(".action-primary")
+
+    it "should preventDefault on secondary action", ->
+        spyOnEvent(".action-secondary", "click")
+        @view.$(".action-secondary").click()
+        expect("click").toHaveBeenPreventedOn(".action-secondary")
+
+describe "CMS.Views.SystemFeedback not preventing events", ->
+    beforeEach ->
+        @clickSpy = jasmine.createSpy('clickSpy')
+        @view = new CMS.Views.Alert.Confirmation(
+            title: "It's all good"
+            message: "No reason for this alert"
+            actions:
+                primary:
+                    text: "Whatever"
+                    click: @clickSpy
+                    preventDefault: false
+        )
+        @view.show()
+
+    it "should not preventDefault", ->
+        spyOnEvent(".action-primary", "click")
+        @view.$(".action-primary").click()
+        expect("click").not.toHaveBeenPreventedOn(".action-primary")
+        expect(@clickSpy).toHaveBeenCalled()
+
+describe "CMS.Views.SystemFeedback multiple secondary actions", ->
+    beforeEach ->
+        @secondarySpyOne = jasmine.createSpy('secondarySpyOne')
+        @secondarySpyTwo = jasmine.createSpy('secondarySpyTwo')
+        @view = new CMS.Views.Notification.Warning(
+            title: "No Primary",
+            message: "Pick a secondary action",
+            actions:
+                secondary: [
+                    {
+                        text: "Option One"
+                        class: "option-one"
+                        click: @secondarySpyOne
+                    }, {
+                        text: "Option Two"
+                        class: "option-two"
+                        click: @secondarySpyTwo
+                    }
+                ]
+        )
+        @view.show()
+
+    it "should render both", ->
+        expect(@view.el).toContain(".action-secondary.option-one")
+        expect(@view.el).toContain(".action-secondary.option-two")
+        expect(@view.el).not.toContain(".action-secondary.option-one.option-two")
+        expect(@view.$(".action-secondary.option-one")).toContainText("Option One")
+        expect(@view.$(".action-secondary.option-two")).toContainText("Option Two")
+
+    it "should differentiate clicks (1)", ->
+        @view.$(".option-one").click()
+        expect(@secondarySpyOne).toHaveBeenCalled()
+        expect(@secondarySpyTwo).not.toHaveBeenCalled()
+
+    it "should differentiate clicks (2)", ->
+        @view.$(".option-two").click()
+        expect(@secondarySpyOne).not.toHaveBeenCalled()
+        expect(@secondarySpyTwo).toHaveBeenCalled()
+
 describe "CMS.Views.Notification minShown and maxShown", ->
     beforeEach ->
         @showSpy = spyOn(CMS.Views.Notification.Saving.prototype, 'show')

cms/static/coffee/src/main.coffee

@@ -19,12 +19,15 @@ $ ->
     if ajaxSettings.notifyOnError is false
       return
     if jqXHR.responseText
+      try
+        message = JSON.parse(jqXHR.responseText).error
+      catch error
         message = _.str.truncate(jqXHR.responseText, 300)
     else
-        message = gettext("This may be happening because of an error with our server or your internet connection. Try refreshing the page or making sure you are online.")
+      message = gettext("This may be happening because of an error with our server or your internet connection. Try refreshing the page or making sure you are online.")
     msg = new CMS.Views.Notification.Error(
-        "title": gettext("Studio's having trouble saving your work")
-        "message": message
+      "title": gettext("Studio's having trouble saving your work")
+      "message": message
     )
     msg.show()
 

cms/static/js/views/assets.js

@@ -23,7 +23,12 @@ function removeAsset(e){
                         { 'location': row.data('id') },
                         function() {
                             // show the post-commit confirmation
-                            $(".wrapper-alert-confirmation").addClass("is-shown").attr('aria-hidden','false');
+                            var deleted = new CMS.Views.Notification.Confirmation({
+                                title: gettext("Your file has been deleted."),
+                                closeIcon: false,
+                                maxShown: 2000
+                            });
+                            deleted.show();
                             row.remove();
                             analytics.track('Deleted Asset', {
                                 'course': course_location_analytics,

cms/static/js/views/feedback.js

@@ -10,8 +10,12 @@ CMS.Views.SystemFeedback = Backbone.View.extend({
         minShown: 0,  // length of time after this view has been shown before it can be hidden (milliseconds)
         maxShown: Infinity  // length of time after this view has been shown before it will be automatically hidden (milliseconds)
 
-        /* could also have an "actions" hash: here is an example demonstrating
-           the expected structure
+        /* Could also have an "actions" hash: here is an example demonstrating
+           the expected structure. For each action, by default the framework
+           will call preventDefault on the click event before the function is
+           run; to make it not do that, just pass `preventDefault: false` in
+           the action object.
+
         actions: {
             primary: {
                 "text": "Save",
@@ -49,6 +53,11 @@ CMS.Views.SystemFeedback = Backbone.View.extend({
         }
         this.template = _.template(tpl);
         this.setElement($("#page-"+this.options.type));
+        // handle single "secondary" action
+        if (this.options.actions && this.options.actions.secondary &&
+            !_.isArray(this.options.actions.secondary)) {
+            this.options.actions.secondary = [this.options.actions.secondary];
+        }
         return this;
     },
     // public API: show() and hide()
@@ -101,6 +110,9 @@ CMS.Views.SystemFeedback = Backbone.View.extend({
         if(!actions) { return; }
         var primary = actions.primary;
         if(!primary) { return; }
+        if(primary.preventDefault !== false) {
+            event.preventDefault();
+        }
         if(primary.click) {
             primary.click.call(event.target, this, event);
         }
@@ -116,6 +128,9 @@ CMS.Views.SystemFeedback = Backbone.View.extend({
             i = _.indexOf(this.$(".action-secondary"), event.target);
         }
         var secondary = secondaryList[i];
+        if(secondary.preventDefault !== false) {
+            event.preventDefault();
+        }
         if(secondary.click) {
             secondary.click.call(event.target, this, event);
         }

cms/static/js/views/settings/advanced_view.js

@@ -20,9 +20,6 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({
                 self.render();
             }
         );
-        // because these are outside of this.$el, they can't be in the event hash
-        $('.save-button').on('click', this, this.saveView);
-        $('.cancel-button').on('click', this, this.revertView);
         this.listenTo(this.model, 'invalid', this.handleValidationError);
     },
     render: function() {
@@ -45,7 +42,6 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({
 
         var policyValues = listEle$.find('.json');
         _.each(policyValues, this.attachJSONEditor, this);
-        this.showMessage();
         return this;
     },
     attachJSONEditor : function (textarea) {
@@ -61,7 +57,9 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({
             mode: "application/json", lineNumbers: false, lineWrapping: false,
             onChange: function(instance, changeobj) {
                 // this event's being called even when there's no change :-(
-                if (instance.getValue() !== oldValue) self.showSaveCancelButtons();
+                if (instance.getValue() !== oldValue && !self.notificationBarShowing) {
+                    self.showNotificationBar();
+                }
             },
             onFocus : function(mirror) {
               $(textarea).parent().children('label').addClass("is-focused");
@@ -99,59 +97,65 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({
             }
         });
     },
-    showMessage: function (type) {
-        $(".wrapper-alert").removeClass("is-shown");
-        if (type) {
-            if (type === this.error_saving) {
-                $(".wrapper-alert-error").addClass("is-shown").attr('aria-hidden','false');
-            }
-            else if (type === this.successful_changes) {
-                $(".wrapper-alert-confirmation").addClass("is-shown").attr('aria-hidden','false');
-                this.hideSaveCancelButtons();
-            }
-        }
-        else {
-            // This is the case of the page first rendering, or when Cancel is pressed.
-            this.hideSaveCancelButtons();
+    showNotificationBar: function() {
+        var self = this;
+        var message = gettext("Your changes will not take effect until you save your progress. Take care with key and value formatting, as validation is not implemented.")
+        var confirm = new CMS.Views.Notification.Warning({
+            title: gettext("You've Made Some Changes"),
+            message: message,
+            actions: {
+                primary: {
+                    "text": gettext("Save Changes"),
+                    "class": "action-save",
+                    "click": function() {
+                        self.saveView();
+                        confirm.hide();
+                        self.notificationBarShowing = false;
+                    }
+                },
+                secondary: [{
+                    "text": gettext("Cancel"),
+                    "class": "action-cancel",
+                    "click": function() {
+                        self.revertView();
+                        confirm.hide();
+                        self.notificationBarShowing = false;
+                    }
+                }]
+            }});
+        this.notificationBarShowing = true;
+        confirm.show();
+        if(this.saved) {
+            this.saved.hide();
         }
     },
-    showSaveCancelButtons: function(event) {
-        if (!this.notificationBarShowing) {
-            this.$el.find(".message-status").removeClass("is-shown");
-            $('.wrapper-notification').removeClass('is-hiding').addClass('is-shown').attr('aria-hidden','false');
-            this.notificationBarShowing = true;
-        }
-    },
-    hideSaveCancelButtons: function() {
-        if (this.notificationBarShowing) {
-            $('.wrapper-notification').removeClass('is-shown').addClass('is-hiding').attr('aria-hidden','true');
-            this.notificationBarShowing = false;
-        }
-    },
-    saveView : function(event) {
-        window.CmsUtils.smoothScrollTop(event);
+    saveView : function() {
         // TODO one last verification scan:
         //    call validateKey on each to ensure proper format
         //    check for dupes
-        var self = event.data;
-        self.model.save({},
+        var self = this;
+        this.model.save({},
             {
             success : function() {
                 self.render();
-                self.showMessage(self.successful_changes);
+                var message = gettext("Please note that validation of your policy key and value pairs is not currently in place yet. If you are having difficulties, please review your policy pairs.");
+                self.saved = new CMS.Views.Alert.Confirmation({
+                    title: gettext("Your policy changes have been saved."),
+                    message: message,
+                    closeIcon: false
+                });
+                self.saved.show();
                 analytics.track('Saved Advanced Settings', {
                     'course': course_location_analytics
                 });
-
             }
         });
     },
-    revertView : function(event) {
-        event.preventDefault();
-        var self = event.data;
-        self.model.deleteKeys = [];
-        self.model.clear({silent : true});
-        self.model.fetch({
+    revertView : function() {
+        var self = this;
+        this.model.deleteKeys = [];
+        this.model.clear({silent : true});
+        this.model.fetch({
             success : function() { self.render(); },
             reset: true
         });

cms/static/sass/_variables.scss

@@ -24,16 +24,16 @@ $f-monospace: 'Bitstream Vera Sans Mono', Consolas, Courier, monospace;
 
 // colors - new for re-org
 $black: rgb(0,0,0);
-$black-t0: rgba(0,0,0,0.125);
-$black-t1: rgba(0,0,0,0.25);
-$black-t2: rgba(0,0,0,0.50);
-$black-t3: rgba(0,0,0,0.75);
+$black-t0: rgba($black, 0.125);
+$black-t1: rgba($black, 0.25);
+$black-t2: rgba($black, 0.5);
+$black-t3: rgba($black, 0.75);
 
 $white: rgb(255,255,255);
-$white-t0: rgba(255,255,255,0.125);
-$white-t1: rgba(255,255,255,0.25);
-$white-t2: rgba(255,255,255,0.50);
-$white-t3: rgba(255,255,255,0.75);
+$white-t0: rgba($white, 0.125);
+$white-t1: rgba($white, 0.25);
+$white-t2: rgba($white, 0.5);
+$white-t3: rgba($white, 0.75);
 
 $gray: rgb(127,127,127);
 $gray-l1: tint($gray,20%);
@@ -63,10 +63,10 @@ $blue-s3: saturate($blue,45%);
 $blue-u1: desaturate($blue,15%);
 $blue-u2: desaturate($blue,30%);
 $blue-u3: desaturate($blue,45%);
-$blue-t0: rgba(85, 151, 221,0.125);
-$blue-t1: rgba(85, 151, 221,0.25);
-$blue-t2: rgba(85, 151, 221,0.50);
-$blue-t3: rgba(85, 151, 221,0.75);
+$blue-t0: rgba($blue, 0.125);
+$blue-t1: rgba($blue, 0.25);
+$blue-t2: rgba($blue, 0.50);
+$blue-t3: rgba($blue, 0.75);
 
 $pink: rgb(183, 37, 103);
 $pink-l1: tint($pink,20%);
@@ -153,10 +153,11 @@ $orange-u1: desaturate($orange,15%);
 $orange-u2: desaturate($orange,30%);
 $orange-u3: desaturate($orange,45%);
 
-$shadow: rgba(0,0,0,0.2);
-$shadow-l1: rgba(0,0,0,0.1);
-$shadow-l2: rgba(0,0,0,0.05);
-$shadow-d1: rgba(0,0,0,0.4);
+$shadow: rgba($black, 0.2);
+$shadow-l1: rgba($black, 0.1);
+$shadow-l2: rgba($black, 0.05);
+$shadow-d1: rgba($black, 0.4);
+$shadow-d2: rgba($black, 0.6);
 
 // ====================
 
@@ -186,4 +187,3 @@ $error-red: rgb(253, 87, 87);
 // type
 $sans-serif: $f-sans-serif;
 $body-line-height: golden-ratio(.875em, 1);
-

cms/templates/base.html

@@ -61,8 +61,6 @@
     <div class="wrapper wrapper-view">
       <%include file="widgets/header.html" />
 
-      ## remove this block after advanced settings notification is rewritten
-      <%block name="view_alerts"></%block>
       <div id="page-alert"></div>
 
       <%block name="content"></%block>
@@ -74,13 +72,9 @@
       <%include file="widgets/footer.html" />
       <%include file="widgets/tender.html" />
 
-      ## remove this block after advanced settings notification is rewritten
-      <%block name="view_notifications"></%block>
       <div id="page-notification"></div>
     </div>
 
-    ## remove this block after advanced settings notification is rewritten
-    <%block name="view_prompts"></%block>
     <div id="page-prompt"></div>
     <%block name="jsextra"></%block>
   </body>

cms/templates/edit_subsection.html

@@ -1,7 +1,7 @@
 <%inherit file="base.html" />
 <%!
    import logging
-   from xmodule.util.date_utils import get_default_time_display
+   from xmodule.util.date_utils import get_default_time_display, almost_same_datetime
 %>
 
 <%! from django.core.urlresolvers import reverse %>
@@ -47,9 +47,10 @@
                 placeholder="HH:MM" class="time" size='10' autocomplete="off"/>
           </div>
         </div>
-        % if subsection.lms.start != parent_item.lms.start and subsection.lms.start:
+        % if subsection.lms.start and not almost_same_datetime(subsection.lms.start, parent_item.lms.start):
             % if parent_item.lms.start is None:
-            <p class="notice">The date above differs from the release date of ${parent_item.display_name_with_default}, which is unset.
+            <p class="notice">The date above differs from the release date of 
+            ${parent_item.display_name_with_default}, which is unset.
             % else:
             <p class="notice">The date above differs from the release date of ${parent_item.display_name_with_default} –
                 ${get_default_time_display(parent_item.lms.start)}.

cms/templates/emails/activation_email_subject.txt

@@ -1 +1 @@
-Your account for edX edge
+Your account for edX Studio

cms/templates/new_item.html

@@ -1,19 +0,0 @@
-<section>
-  <div>${parent_name}</div>
-  <div>${parent_location}</div>
-  <input type="text" class="name"/>
-  <div>
-    % for module_type, module_templates in templates:
-    <div>
-      <div>${module_type}</div>
-      <div>
-        % for template in module_templates:
-        <a class="save" data-template-id="${template.location.url()}">${template.display_name_with_default}</a>
-        % endfor
-      </div>
-    </div>
-    % endfor
-  </div>
-  <a class='cancel'>Cancel</a>
-</section>
-

cms/templates/overview.html

@@ -167,7 +167,8 @@
                 %else:
                   <span class="published-status"><strong>Will Release:</strong> 
                   ${date_utils.get_default_time_display(section.lms.start)}</span>
-                  <a href="#" class="edit-button" data-date="${start_date_str}" data-time="${start_time_str}" data-id="${section.location}">Edit</a>
+                  <a href="#" class="edit-button" data-date="${start_date_str}" 
+                    data-time="${start_time_str}" data-id="${section.location}">Edit</a>
                 %endif
               </div>
             </div>

cms/templates/settings_advanced.html

@@ -104,60 +104,3 @@ editor.render();
   </section>
 </div>
 </%block>
-
-<%block name="view_notifications">
-<!-- notification: change has been made and a save is needed -->
-<div class="wrapper wrapper-notification wrapper-notification-warning" aria-hidden="true" role="dialog" aria-labelledby="notification-changesMade-title" aria-describedby="notification-changesMade-description">
-  <div class="notification warning has-actions">
-    <i class="icon-warning-sign"></i>
-
-    <div class="copy">
-      <h2 class="title title-3" id="notification-changesMade-title">You've Made Some Changes</h2>
-      <p id="notification-changesMade-description">Your changes will not take effect until you <strong>save your progress</strong>. Take care with key and value formatting, as validation is <strong>not implemented</strong>.</p>
-    </div>
-
-    <nav class="nav-actions">
-      <h3 class="sr">Notification Actions</h3>
-      <ul>
-        <li class="nav-item">
-          <a href="" class="action-primary save-button">Save Changes</a>
-        </li>
-        <li class="nav-item">
-          <a href="" class="action-secondary cancel-button">Cancel</a>
-        </li>
-      </ul>
-    </nav>
-  </div>
-</div>
-</%block>
-
-<%block name="view_alerts">
-<!-- alert: save confirmed with close -->
-<div class="wrapper wrapper-alert wrapper-alert-confirmation" role="status">
-  <div class="alert confirmation">
-    <i class="icon-ok"></i>
-
-    <div class="copy">
-      <h2 class="title title-3">Your policy changes have been saved.</h2>
-      <p>Please note that validation of your policy key and value pairs is not currently in place yet. If you are having difficulties, please review your policy pairs.</p>
-    </div>
-
-    <a href="" rel="view" class="action action-alert-close">
-      <i class="icon-remove-sign"></i>
-      <span class="label">close alert</span>
-    </a>
-  </div>
-</div>
-
-<!-- alert: error -->
-<div class="wrapper wrapper-alert wrapper-alert-error" role="status">
-  <div class="alert error">
-    <i class="icon-warning-sign"></i>
-
-    <div class="copy">
-      <h2 class="title title-3">There was an error saving your information</h2>
-      <p>Please see the error below and correct it to ensure there are no problems in rendering your course.</p>
-    </div>
-  </div>
-</div>
-</%block>

common/djangoapps/course_groups/tests/tests.py

@@ -10,22 +10,12 @@ from course_groups.cohorts import (get_cohort, get_course_cohorts,
 
 from xmodule.modulestore.django import modulestore, _MODULESTORES
 
+from xmodule.modulestore.tests.django_utils import xml_store_config
+
 # NOTE: running this with the lms.envs.test config works without
 # manually overriding the modulestore.  However, running with
 # cms.envs.test doesn't.
 
-
-def xml_store_config(data_dir):
-    return {
-    'default': {
-        'ENGINE': 'xmodule.modulestore.xml.XMLModuleStore',
-        'OPTIONS': {
-            'data_dir': data_dir,
-            'default_class': 'xmodule.hidden_module.HiddenDescriptor',
-        }
-    }
-}
-
 TEST_DATA_DIR = settings.COMMON_TEST_DATA_ROOT
 TEST_DATA_XML_MODULESTORE = xml_store_config(TEST_DATA_DIR)
 
@@ -33,7 +23,6 @@ TEST_DATA_XML_MODULESTORE = xml_store_config(TEST_DATA_DIR)
 @override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE)
 class TestCohorts(django.test.TestCase):
 
-
     @staticmethod
     def topic_name_to_id(course, name):
         """
@@ -44,7 +33,6 @@ class TestCohorts(django.test.TestCase):
                                               run=course.url_name,
                                               name=name)
 
-
     @staticmethod
     def config_course_cohorts(course, discussions,
                               cohorted,
@@ -90,7 +78,6 @@ class TestCohorts(django.test.TestCase):
 
         course.cohort_config = d
 
-
     def setUp(self):
         """
         Make sure that course is reloaded every time--clear out the modulestore.
@@ -99,7 +86,6 @@ class TestCohorts(django.test.TestCase):
         # to course.  We don't have a course.clone() method.
         _MODULESTORES.clear()
 
-
     def test_get_cohort(self):
         """
         Make sure get_cohort() does the right thing when the course is cohorted
@@ -115,7 +101,7 @@ class TestCohorts(django.test.TestCase):
 
         cohort = CourseUserGroup.objects.create(name="TestCohort",
                                                 course_id=course.id,
-                               group_type=CourseUserGroup.COHORT)
+                                                group_type=CourseUserGroup.COHORT)
 
         cohort.users.add(user)
 
@@ -145,7 +131,7 @@ class TestCohorts(django.test.TestCase):
 
         cohort = CourseUserGroup.objects.create(name="TestCohort",
                                                 course_id=course.id,
-                               group_type=CourseUserGroup.COHORT)
+                                                group_type=CourseUserGroup.COHORT)
 
         # user1 manually added to a cohort
         cohort.users.add(user1)
@@ -179,7 +165,6 @@ class TestCohorts(django.test.TestCase):
         self.assertEquals(get_cohort(user2, course.id).name, "AutoGroup",
                           "user2 should still be in originally placed cohort")
 
-
     def test_auto_cohorting_randomization(self):
         """
         Make sure get_cohort() randomizes properly.
@@ -209,8 +194,6 @@ class TestCohorts(django.test.TestCase):
             self.assertGreater(num_users, 1)
             self.assertLess(num_users, 50)
 
-
-
     def test_get_course_cohorts(self):
         course1_id = 'a/b/c'
         course2_id = 'e/f/g'
@@ -224,14 +207,12 @@ class TestCohorts(django.test.TestCase):
                                                 course_id=course1_id,
                                                 group_type=CourseUserGroup.COHORT)
 
-
         # second course should have no cohorts
         self.assertEqual(get_course_cohorts(course2_id), [])
 
         cohorts = sorted([c.name for c in get_course_cohorts(course1_id)])
         self.assertEqual(cohorts, ['TestCohort', 'TestCohort2'])
 
-
     def test_is_commentable_cohorted(self):
         course = modulestore().get_course("edX/toy/2012_Fall")
         self.assertFalse(course.is_cohorted)

common/djangoapps/external_auth/tests/test_openid_provider.py

@@ -9,9 +9,11 @@ from urlparse import parse_qs
 
 from django.conf import settings
 from django.test import TestCase, LiveServerTestCase
+from django.test.utils import override_settings
 # from django.contrib.auth.models import User
 from django.core.urlresolvers import reverse
 from django.test.client import RequestFactory
+from unittest import skipUnless
 
 
 class MyFetcher(HTTPFetcher):
@@ -59,21 +61,17 @@ class MyFetcher(HTTPFetcher):
             final_url=final_url,
             headers=response_headers,
             status=status,
-            )
+        )
 
 
 class OpenIdProviderTest(TestCase):
+    """
+    Tests of the OpenId login
+    """
 
-#    def setUp(self):
-#        username = 'viewtest'
-#        email = 'view@test.com'
-#        password = 'foo'
-#        user = User.objects.create_user(username, email, password)
-
-    def testBeginLoginWithXrdsUrl(self):
-        # skip the test if openid is not enabled (as in cms.envs.test):
-        if not settings.MITX_FEATURES.get('AUTH_USE_OPENID') or not settings.MITX_FEATURES.get('AUTH_USE_OPENID_PROVIDER'):
-            return
+    @skipUnless(settings.MITX_FEATURES.get('AUTH_USE_OPENID') or
+                settings.MITX_FEATURES.get('AUTH_USE_OPENID_PROVIDER'), True)
+    def test_begin_login_with_xrds_url(self):
 
         # the provider URL must be converted to an absolute URL in order to be
         # used as an openid provider.
@@ -99,10 +97,9 @@ class OpenIdProviderTest(TestCase):
                              "got code {0} for url '{1}'. Expected code {2}"
                              .format(resp.status_code, url, code))
 
-    def testBeginLoginWithLoginUrl(self):
-        # skip the test if openid is not enabled (as in cms.envs.test):
-        if not settings.MITX_FEATURES.get('AUTH_USE_OPENID') or not settings.MITX_FEATURES.get('AUTH_USE_OPENID_PROVIDER'):
-            return
+    @skipUnless(settings.MITX_FEATURES.get('AUTH_USE_OPENID') or
+                settings.MITX_FEATURES.get('AUTH_USE_OPENID_PROVIDER'), True)
+    def test_begin_login_with_login_url(self):
 
         # the provider URL must be converted to an absolute URL in order to be
         # used as an openid provider.
@@ -150,49 +147,70 @@ class OpenIdProviderTest(TestCase):
             # <input name="openid.return_to" type="hidden" value="http://testserver/openid/complete/?janrain_nonce=2013-01-23T06%3A20%3A17ZaN7j6H" />
             # <input name="openid.assoc_handle" type="hidden" value="{HMAC-SHA1}{50ff8120}{rh87+Q==}" />
 
-
-    def testOpenIdSetup(self):
-        if not settings.MITX_FEATURES.get('AUTH_USE_OPENID_PROVIDER'):
-            return
+    def attempt_login(self, expected_code, **kwargs):
+        """ Attempt to log in through the open id provider login """
         url = reverse('openid-provider-login')
         post_args = {
-                     "openid.mode": "checkid_setup",
-                     "openid.return_to": "http://testserver/openid/complete/?janrain_nonce=2013-01-23T06%3A20%3A17ZaN7j6H",
-                     "openid.assoc_handle": "{HMAC-SHA1}{50ff8120}{rh87+Q==}",
-                     "openid.claimed_id": "http://specs.openid.net/auth/2.0/identifier_select",
-                     "openid.ns": "http://specs.openid.net/auth/2.0",
-                     "openid.realm": "http://testserver/",
-                     "openid.identity": "http://specs.openid.net/auth/2.0/identifier_select",
-                     "openid.ns.ax": "http://openid.net/srv/ax/1.0",
-                     "openid.ax.mode": "fetch_request",
-                     "openid.ax.required": "email,fullname,old_email,firstname,old_nickname,lastname,old_fullname,nickname",
-                     "openid.ax.type.fullname": "http://axschema.org/namePerson",
-                     "openid.ax.type.lastname": "http://axschema.org/namePerson/last",
-                     "openid.ax.type.firstname": "http://axschema.org/namePerson/first",
-                     "openid.ax.type.nickname": "http://axschema.org/namePerson/friendly",
-                     "openid.ax.type.email": "http://axschema.org/contact/email",
-                     "openid.ax.type.old_email": "http://schema.openid.net/contact/email",
-                     "openid.ax.type.old_nickname": "http://schema.openid.net/namePerson/friendly",
-                     "openid.ax.type.old_fullname": "http://schema.openid.net/namePerson",
-                     }
+            "openid.mode": "checkid_setup",
+            "openid.return_to": "http://testserver/openid/complete/?janrain_nonce=2013-01-23T06%3A20%3A17ZaN7j6H",
+            "openid.assoc_handle": "{HMAC-SHA1}{50ff8120}{rh87+Q==}",
+            "openid.claimed_id": "http://specs.openid.net/auth/2.0/identifier_select",
+            "openid.ns": "http://specs.openid.net/auth/2.0",
+            "openid.realm": "http://testserver/",
+            "openid.identity": "http://specs.openid.net/auth/2.0/identifier_select",
+            "openid.ns.ax": "http://openid.net/srv/ax/1.0",
+            "openid.ax.mode": "fetch_request",
+            "openid.ax.required": "email,fullname,old_email,firstname,old_nickname,lastname,old_fullname,nickname",
+            "openid.ax.type.fullname": "http://axschema.org/namePerson",
+            "openid.ax.type.lastname": "http://axschema.org/namePerson/last",
+            "openid.ax.type.firstname": "http://axschema.org/namePerson/first",
+            "openid.ax.type.nickname": "http://axschema.org/namePerson/friendly",
+            "openid.ax.type.email": "http://axschema.org/contact/email",
+            "openid.ax.type.old_email": "http://schema.openid.net/contact/email",
+            "openid.ax.type.old_nickname": "http://schema.openid.net/namePerson/friendly",
+            "openid.ax.type.old_fullname": "http://schema.openid.net/namePerson",
+        }
+        # override the default args with any given arguments
+        for key in kwargs:
+            post_args["openid." + key] = kwargs[key]
+
         resp = self.client.post(url, post_args)
-        code = 200
+        code = expected_code
         self.assertEqual(resp.status_code, code,
                          "got code {0} for url '{1}'. Expected code {2}"
                          .format(resp.status_code, url, code))
 
+    @skipUnless(settings.MITX_FEATURES.get('AUTH_USE_OPENID') or
+                settings.MITX_FEATURES.get('AUTH_USE_OPENID_PROVIDER'), True)
+    def test_open_id_setup(self):
+        """ Attempt a standard successful login """
+        self.attempt_login(200)
+
+    @skipUnless(settings.MITX_FEATURES.get('AUTH_USE_OPENID') or
+                settings.MITX_FEATURES.get('AUTH_USE_OPENID_PROVIDER'), True)
+    def test_invalid_namespace(self):
+        """ Test for 403 error code when the namespace of the request is invalid"""
+        self.attempt_login(403, ns="http%3A%2F%2Fspecs.openid.net%2Fauth%2F2.0")
+
+    @override_settings(OPENID_PROVIDER_TRUSTED_ROOTS=['http://apps.cs50.edx.org'])
+    @skipUnless(settings.MITX_FEATURES.get('AUTH_USE_OPENID') or
+                settings.MITX_FEATURES.get('AUTH_USE_OPENID_PROVIDER'), True)
+    def test_invalid_return_url(self):
+        """ Test for 403 error code when the url"""
+        self.attempt_login(403, return_to="http://apps.cs50.edx.or")
+
 
-# In order for this absolute URL to work (i.e. to get xrds, then authentication)
-# in the test environment, we either need a live server that works with the default
-# fetcher (i.e. urlopen2), or a test server that is reached through a custom fetcher.
-# Here we do the former.
 class OpenIdProviderLiveServerTest(LiveServerTestCase):
+    """
+    In order for this absolute URL to work (i.e. to get xrds, then authentication)
+    in the test environment, we either need a live server that works with the default
+    fetcher (i.e. urlopen2), or a test server that is reached through a custom fetcher.
+    Here we do the former.
+    """
 
-    def testBeginLogin(self):
-        # skip the test if openid is not enabled (as in cms.envs.test):
-        if not settings.MITX_FEATURES.get('AUTH_USE_OPENID') or not settings.MITX_FEATURES.get('AUTH_USE_OPENID_PROVIDER'):
-            return
-
+    @skipUnless(settings.MITX_FEATURES.get('AUTH_USE_OPENID') or
+                settings.MITX_FEATURES.get('AUTH_USE_OPENID_PROVIDER'), True)
+    def test_begin_login(self):
         # the provider URL must be converted to an absolute URL in order to be
         # used as an openid provider.
         provider_url = reverse('openid-provider-xrds')

common/djangoapps/external_auth/views.py

@@ -3,7 +3,7 @@ import json
 import logging
 import random
 import re
-import string
+import string       # pylint: disable=W0402
 import fnmatch
 
 from textwrap import dedent
@@ -36,7 +36,7 @@ import django_openid_auth.views as openid_views
 from django_openid_auth import auth as openid_auth
 from openid.consumer.consumer import SUCCESS
 
-from openid.server.server import Server
+from openid.server.server import Server, ProtocolError, UntrustedReturnURL
 from openid.server.trustroot import TrustRoot
 from openid.extensions import ax, sreg
 
@@ -102,7 +102,7 @@ def openid_login_complete(request,
         oid_backend = openid_auth.OpenIDBackend()
         details = oid_backend._extract_user_details(openid_response)
 
-        log.debug('openid success, details=%s' % details)
+        log.debug('openid success, details=%s', details)
 
         url = getattr(settings, 'OPENID_SSO_SERVER_URL', None)
         external_domain = "openid:%s" % url
@@ -132,7 +132,7 @@ def external_login_or_signup(request,
     try:
         eamap = ExternalAuthMap.objects.get(external_id=external_id,
                                             external_domain=external_domain)
-        log.debug('Found eamap=%s' % eamap)
+        log.debug('Found eamap=%s', eamap)
     except ExternalAuthMap.DoesNotExist:
         # go render form for creating edX user
         eamap = ExternalAuthMap(external_id=external_id,
@@ -141,11 +141,11 @@ def external_login_or_signup(request,
         eamap.external_email = email
         eamap.external_name = fullname
         eamap.internal_password = generate_password()
-        log.debug('Created eamap=%s' % eamap)
+        log.debug('Created eamap=%s', eamap)
 
         eamap.save()
 
-    log.info("External_Auth login_or_signup for %s : %s : %s : %s" % (external_domain, external_id, email, fullname))
+    log.info(u"External_Auth login_or_signup for %s : %s : %s : %s", external_domain, external_id, email, fullname)
     internal_user = eamap.user
     if internal_user is None:
         if settings.MITX_FEATURES.get('AUTH_USE_SHIB'):
@@ -157,7 +157,7 @@ def external_login_or_signup(request,
                     eamap.user = link_user
                     eamap.save()
                     internal_user = link_user
-                    log.info('SHIB: Linking existing account for %s' % eamap.external_email)
+                    log.info('SHIB: Linking existing account for %s', eamap.external_email)
                     # now pass through to log in
                 else:
                     # otherwise, there must have been an error, b/c we've already linked a user with these external
@@ -168,10 +168,10 @@ def external_login_or_signup(request,
                                            % getattr(settings, 'TECH_SUPPORT_EMAIL', 'techsupport@class.stanford.edu')))
                     return default_render_failure(request, failure_msg)
             except User.DoesNotExist:
-                log.info('SHIB: No user for %s yet, doing signup' % eamap.external_email)
+                log.info('SHIB: No user for %s yet, doing signup', eamap.external_email)
                 return signup(request, eamap)
         else:
-            log.info('No user for %s yet, doing signup' % eamap.external_email)
+            log.info('No user for %s yet. doing signup', eamap.external_email)
             return signup(request, eamap)
 
     # We trust shib's authentication, so no need to authenticate using the password again
@@ -183,17 +183,17 @@ def external_login_or_signup(request,
         else:
             auth_backend = 'django.contrib.auth.backends.ModelBackend'
         user.backend = auth_backend
-        log.info('SHIB: Logging in linked user %s' % user.email)
+        log.info('SHIB: Logging in linked user %s', user.email)
     else:
         uname = internal_user.username
         user = authenticate(username=uname, password=eamap.internal_password)
     if user is None:
-        log.warning("External Auth Login failed for %s / %s" %
-                    (uname, eamap.internal_password))
+        log.warning("External Auth Login failed for %s / %s",
+                    uname, eamap.internal_password)
         return signup(request, eamap)
 
     if not user.is_active:
-        log.warning("User %s is not active" % (uname))
+        log.warning("User %s is not active", uname)
         # TODO: improve error page
         msg = 'Account not yet activated: please look for link in your email'
         return default_render_failure(request, msg)
@@ -208,7 +208,7 @@ def external_login_or_signup(request,
         student_views.try_change_enrollment(enroll_request)
     else:
         student_views.try_change_enrollment(request)
-    log.info("Login success - {0} ({1})".format(user.username, user.email))
+    log.info("Login success - %s (%s)", user.username, user.email)
     if retfun is None:
         return redirect('/')
     return retfun()
@@ -261,7 +261,7 @@ def signup(request, eamap=None):
     except ValidationError:
         context['ask_for_email'] = True
 
-    log.info('EXTAUTH: Doing signup for %s' % eamap.external_id)
+    log.info('EXTAUTH: Doing signup for %s', eamap.external_id)
 
     return student_views.register_user(request, extra_context=context)
 
@@ -405,7 +405,7 @@ def shib_login(request):
         shib['sn'] = shib['sn'].split(";")[0].strip().capitalize().decode('utf-8')
         shib['givenName'] = shib['givenName'].split(";")[0].strip().capitalize().decode('utf-8')
 
-    log.info("SHIB creds returned: %r" % shib)
+    log.info("SHIB creds returned: %r", shib)
 
     return external_login_or_signup(request,
                                     external_id=shib['REMOTE_USER'],
@@ -640,7 +640,10 @@ def provider_login(request):
     error = False
     if 'openid.mode' in request.GET or 'openid.mode' in request.POST:
         # decode request
-        openid_request = server.decodeRequest(querydict)
+        try:
+            openid_request = server.decodeRequest(querydict)
+        except (UntrustedReturnURL, ProtocolError):
+            openid_request = None
 
         if not openid_request:
             return default_render_failure(request, "Invalid OpenID request")
@@ -697,8 +700,8 @@ def provider_login(request):
             user = User.objects.get(email=email)
         except User.DoesNotExist:
             request.session['openid_error'] = True
-            msg = "OpenID login failed - Unknown user email: {0}".format(email)
-            log.warning(msg)
+            msg = "OpenID login failed - Unknown user email: %s"
+            log.warning(msg, email)
             return HttpResponseRedirect(openid_request_url)
 
         # attempt to authenticate user (but not actually log them in...)
@@ -708,9 +711,8 @@ def provider_login(request):
         user = authenticate(username=username, password=password)
         if user is None:
             request.session['openid_error'] = True
-            msg = "OpenID login failed - password for {0} is invalid"
-            msg = msg.format(email)
-            log.warning(msg)
+            msg = "OpenID login failed - password for %s is invalid"
+            log.warning(msg, email)
             return HttpResponseRedirect(openid_request_url)
 
         # authentication succeeded, so fetch user information
@@ -720,10 +722,8 @@ def provider_login(request):
             if 'openid_error' in request.session:
                 del request.session['openid_error']
 
-            # fullname field comes from user profile
-            profile = UserProfile.objects.get(user=user)
-            log.info("OpenID login success - {0} ({1})".format(user.username,
-                                                               user.email))
+            log.info("OpenID login success - %s (%s)",
+                     user.username, user.email)
 
             # redirect user to return_to location
             url = endpoint + urlquote(user.username)
@@ -753,8 +753,8 @@ def provider_login(request):
 
         # the account is not active, so redirect back to the login page:
         request.session['openid_error'] = True
-        msg = "Login failed - Account not active for user {0}".format(username)
-        log.warning(msg)
+        msg = "Login failed - Account not active for user %s"
+        log.warning(msg, username)
         return HttpResponseRedirect(openid_request_url)
 
     # determine consumer domain if applicable

common/djangoapps/heartbeat/urls.py

@@ -1,4 +1,4 @@
-from django.conf.urls import *
+from django.conf.urls import url, patterns
 
 urlpatterns = patterns('',  # nopep8
     url(r'^$', 'heartbeat.views.heartbeat', name='heartbeat'),

common/djangoapps/student/admin.py

@@ -2,9 +2,9 @@
 django admin pages for courseware model
 '''
 
-from student.models import *
+from student.models import UserProfile, UserTestGroup, CourseEnrollmentAllowed
+from student.models import CourseEnrollment, Registration, PendingNameChange
 from django.contrib import admin
-from django.contrib.auth.models import User
 
 admin.site.register(UserProfile)
 

common/djangoapps/student/forms.py

@@ -0,0 +1,21 @@
+from django import forms
+from django.contrib.auth.models import User
+from django.contrib.auth.forms import PasswordResetForm
+from django.contrib.auth.hashers import UNUSABLE_PASSWORD
+
+class PasswordResetFormNoActive(PasswordResetForm):
+    def clean_email(self):
+        """
+        This is a literal copy from Django 1.4.5's django.contrib.auth.forms.PasswordResetForm
+        Except removing the requirement of active users
+        Validates that a user exists with the given email address.
+        """
+        email = self.cleaned_data["email"]
+        #The line below contains the only change, removing is_active=True
+        self.users_cache = User.objects.filter(email__iexact=email)
+        if not len(self.users_cache):
+            raise forms.ValidationError(self.error_messages['unknown'])
+        if any((user.password == UNUSABLE_PASSWORD)
+               for user in self.users_cache):
+            raise forms.ValidationError(self.error_messages['unusable'])
+        return email

common/djangoapps/student/management/commands/massemailtxt.py

@@ -37,7 +37,6 @@ rate -- messages per second
         self.log_file.write(datetime.datetime.utcnow().isoformat() + ' -- ' + text + '\n')
 
     def handle(self, *args, **options):
-        global log_file
         (user_file, message_base, logfilename, ratestr) = args
 
         users = [u.strip() for u in open(user_file).readlines()]

common/djangoapps/student/tests/tests.py

@@ -5,18 +5,127 @@ when you run "manage.py test".
 Replace this with more appropriate tests for your application.
 """
 import logging
+import json
+import re
+import unittest
 
+from django import forms
+from django.conf import settings
 from django.test import TestCase
-from mock import Mock
+from django.test.client import RequestFactory
+from django.contrib.auth.models import User
+from django.contrib.auth.hashers import UNUSABLE_PASSWORD
+from django.contrib.auth.tokens import default_token_generator
+from django.template.loader import render_to_string, get_template, TemplateDoesNotExist
+from django.core.urlresolvers import is_valid_path
+from django.utils.http import int_to_base36
+
+
+from mock import Mock, patch
+from textwrap import dedent
 
 from student.models import unique_id_for_user
-from student.views import process_survey_link, _cert_info
-
+from student.views import process_survey_link, _cert_info, password_reset, password_reset_confirm_wrapper
+from student.tests.factories import UserFactory
+from student.tests.test_email import mock_render_to_string
 COURSE_1 = 'edX/toy/2012_Fall'
 COURSE_2 = 'edx/full/6.002_Spring_2012'
 
 log = logging.getLogger(__name__)
 
+try:
+    get_template('registration/password_reset_email.html')
+    project_uses_password_reset = True
+except TemplateDoesNotExist:
+    project_uses_password_reset = False
+
+
+class ResetPasswordTests(TestCase):
+    """ Tests that clicking reset password sends email, and doesn't activate the user
+    """
+    request_factory = RequestFactory()
+
+    def setUp(self):
+        self.user = UserFactory.create()
+        self.user.is_active = False
+        self.user.save()
+        self.token = default_token_generator.make_token(self.user)
+        self.uidb36 = int_to_base36(self.user.id)
+
+        self.user_bad_passwd = UserFactory.create()
+        self.user_bad_passwd.is_active = False
+        self.user_bad_passwd.password = UNUSABLE_PASSWORD
+        self.user_bad_passwd.save()
+
+    def test_user_bad_password_reset(self):
+        """Tests password reset behavior for user with password marked UNUSABLE_PASSWORD"""
+
+        bad_pwd_req = self.request_factory.post('/password_reset/', {'email': self.user_bad_passwd.email})
+        bad_pwd_resp = password_reset(bad_pwd_req)
+        self.assertEquals(bad_pwd_resp.status_code, 200)
+        self.assertEquals(bad_pwd_resp.content, json.dumps({'success': False,
+                                                            'error': 'Invalid e-mail or user'}))
+
+    def test_nonexist_email_password_reset(self):
+        """Now test the exception cases with of reset_password called with invalid email."""
+
+        bad_email_req = self.request_factory.post('/password_reset/', {'email': self.user.email+"makeItFail"})
+        bad_email_resp = password_reset(bad_email_req)
+        self.assertEquals(bad_email_resp.status_code, 200)
+        self.assertEquals(bad_email_resp.content, json.dumps({'success': False,
+                                                              'error': 'Invalid e-mail or user'}))
+
+    @unittest.skipUnless(project_uses_password_reset,
+                         dedent("""Skipping Test because CMS has not provided necessary templates for password reset.
+                                If LMS tests print this message, that needs to be fixed."""))
+    @patch('django.core.mail.send_mail')
+    @patch('student.views.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True))
+    def test_reset_password_email(self, send_email):
+        """Tests contents of reset password email, and that user is not active"""
+
+        good_req = self.request_factory.post('/password_reset/', {'email': self.user.email})
+        good_resp = password_reset(good_req)
+        self.assertEquals(good_resp.status_code, 200)
+        self.assertEquals(good_resp.content,
+                          json.dumps({'success': True,
+                                      'value': "('registration/password_reset_done.html', [])"}))
+
+        ((subject, msg, from_addr, to_addrs), sm_kwargs) = send_email.call_args
+        self.assertIn("Password reset", subject)
+        self.assertIn("You're receiving this e-mail because you requested a password reset", msg)
+        self.assertEquals(from_addr, settings.DEFAULT_FROM_EMAIL)
+        self.assertEquals(len(to_addrs), 1)
+        self.assertIn(self.user.email, to_addrs)
+
+        #test that the user is not active
+        self.user = User.objects.get(pk=self.user.pk)
+        self.assertFalse(self.user.is_active)
+        reset_match = re.search(r'password_reset_confirm/(?P<uidb36>[0-9A-Za-z]+)-(?P<token>.+)/', msg).groupdict()
+
+    @patch('student.views.password_reset_confirm')
+    def test_reset_password_bad_token(self, reset_confirm):
+        """Tests bad token and uidb36 in password reset"""
+
+        bad_reset_req = self.request_factory.get('/password_reset_confirm/NO-OP/')
+        password_reset_confirm_wrapper(bad_reset_req, 'NO', 'OP')
+        (confirm_args, confirm_kwargs) = reset_confirm.call_args
+        self.assertEquals(confirm_kwargs['uidb36'], 'NO')
+        self.assertEquals(confirm_kwargs['token'], 'OP')
+        self.user = User.objects.get(pk=self.user.pk)
+        self.assertFalse(self.user.is_active)
+
+    @patch('student.views.password_reset_confirm')
+    def test_reset_password_good_token(self, reset_confirm):
+        """Tests good token and uidb36 in password reset"""
+
+        good_reset_req = self.request_factory.get('/password_reset_confirm/{0}-{1}/'.format(self.uidb36, self.token))
+        password_reset_confirm_wrapper(good_reset_req, self.uidb36, self.token)
+        (confirm_args, confirm_kwargs) = reset_confirm.call_args
+        self.assertEquals(confirm_kwargs['uidb36'], self.uidb36)
+        self.assertEquals(confirm_kwargs['token'], self.token)
+        self.user = User.objects.get(pk=self.user.pk)
+        self.assertTrue(self.user.is_active)
+
 
 class CourseEndingTest(TestCase):
     """Test things related to course endings: certificates, surveys, etc"""

common/djangoapps/student/views.py

@@ -4,16 +4,16 @@ import json
 import logging
 import random
 import re
-import string
+import string       # pylint: disable=W0402
 import urllib
 import uuid
 import time
 
 from django.conf import settings
 from django.contrib.auth import logout, authenticate, login
-from django.contrib.auth.forms import PasswordResetForm
 from django.contrib.auth.models import User
 from django.contrib.auth.decorators import login_required
+from django.contrib.auth.views import password_reset_confirm
 from django.core.cache import cache
 from django.core.context_processors import csrf
 from django.core.mail import send_mail
@@ -24,6 +24,7 @@ from django.http import HttpResponse, HttpResponseBadRequest, HttpResponseForbid
 from django.shortcuts import redirect
 from django_future.csrf import ensure_csrf_cookie
 from django.utils.http import cookie_date
+from django.utils.http import base36_to_int
 
 from mitxmako.shortcuts import render_to_response, render_to_string
 from bs4 import BeautifulSoup
@@ -34,6 +35,8 @@ from student.models import (Registration, UserProfile, TestCenterUser, TestCente
                             CourseEnrollment, unique_id_for_user,
                             get_testcenter_registration, CourseEnrollmentAllowed)
 
+from student.forms import PasswordResetFormNoActive
+
 from certificates.models import CertificateStatuses, certificate_status_for_student
 
 from xmodule.course_module import CourseDescriptor
@@ -962,17 +965,7 @@ def password_reset(request):
     if request.method != "POST":
         raise Http404
 
-    # By default, Django doesn't allow Users with is_active = False to reset their passwords,
-    # but this bites people who signed up a long time ago, never activated, and forgot their
-    # password. So for their sake, we'll auto-activate a user for whom password_reset is called.
-    try:
-        user = User.objects.get(email=request.POST['email'])
-        user.is_active = True
-        user.save()
-    except:
-        log.exception("Tried to auto-activate user to enable password reset, but failed.")
-
-    form = PasswordResetForm(request.POST)
+    form = PasswordResetFormNoActive(request.POST)
     if form.is_valid():
         form.save(use_https=request.is_secure(),
                   from_email=settings.DEFAULT_FROM_EMAIL,
@@ -982,7 +975,21 @@ def password_reset(request):
                                         'value': render_to_string('registration/password_reset_done.html', {})}))
     else:
         return HttpResponse(json.dumps({'success': False,
-                                        'error': 'Invalid e-mail'}))
+                                        'error': 'Invalid e-mail or user'}))
+
+def password_reset_confirm_wrapper(request, uidb36=None, token=None):
+    ''' A wrapper around django.contrib.auth.views.password_reset_confirm.
+        Needed because we want to set the user as active at this step.
+    '''
+    #cribbed from django.contrib.auth.views.password_reset_confirm
+    try:
+        uid_int = base36_to_int(uidb36)
+        user = User.objects.get(id=uid_int)
+        user.is_active = True
+        user.save()
+    except (ValueError, User.DoesNotExist):
+        pass
+    return password_reset_confirm(request, uidb36=uidb36, token=token)
 
 
 def reactivation_email_for_user(user):

common/djangoapps/terrain/factories.py

@@ -44,7 +44,7 @@ class GroupFactory(sf.GroupFactory):
 
 
 @world.absorb
-class CourseEnrollmentAllowedFactory(sf.CourseEnrollmentAllowed):
+class CourseEnrollmentAllowedFactory(sf.CourseEnrollmentAllowedFactory):
     """
     Users allowed to enroll in the course outside of the usual window
     """

common/djangoapps/track/admin.py

@@ -2,7 +2,7 @@
 django admin pages for courseware model
 '''
 
-from track.models import *
+from track.models import TrackingLog
 from django.contrib import admin
 
 admin.site.register(TrackingLog)

common/djangoapps/track/models.py

@@ -1,9 +1,8 @@
 from django.db import models
 
-from django.db import models
-
 
 class TrackingLog(models.Model):
+    """Defines the fields that are stored in the tracking log database"""
     dtcreated = models.DateTimeField('creation date', auto_now_add=True)
     username = models.CharField(max_length=32, blank=True)
     ip = models.CharField(max_length=32, blank=True)
@@ -16,6 +15,9 @@ class TrackingLog(models.Model):
     host = models.CharField(max_length=64, blank=True)
 
     def __unicode__(self):
-        s = "[%s] %s@%s: %s | %s | %s | %s" % (self.time, self.username, self.ip, self.event_source,
-                                               self.event_type, self.page, self.event)
-        return s
+        fmt = (
+            u"[{self.time}] {self.username}@{self.ip}: "
+            u"{self.event_source}| {self.event_type} | "
+            u"{self.page} | {self.event}"
+        )
+        return fmt.format(self=self)

common/djangoapps/track/tests.py

@@ -1,16 +1,56 @@
-"""
-This file demonstrates writing tests using the unittest module. These will pass
-when you run "manage.py test".
-
-Replace this with more appropriate tests for your application.
-"""
-
+"""Tests for student tracking"""
 from django.test import TestCase
+from django.core.urlresolvers import reverse, NoReverseMatch
+from track.models import TrackingLog
+from track.views import user_track
+from nose.plugins.skip import SkipTest
 
 
-class SimpleTest(TestCase):
-    def test_basic_addition(self):
+class TrackingTest(TestCase):
+    """
+    Tests that tracking logs correctly handle events
+    """
+
+    def test_post_answers_to_log(self):
         """
-        Tests that 1 + 1 always equals 2.
+        Checks that student answer requests submitted to track.views via POST
+        are correctly logged in the TrackingLog db table
         """
-        self.assertEqual(1 + 1, 2)
+        requests = [
+            {"event": "my_event", "event_type": "my_event_type", "page": "my_page"},
+            {"event": "{'json': 'object'}", "event_type": unichr(512), "page": "my_page"}
+        ]
+        for request_params in requests:
+            try:  # because /event maps to two different views in lms and cms, we're only going to test lms here
+                response = self.client.post(reverse(user_track), request_params)
+            except NoReverseMatch:
+                raise SkipTest()
+            self.assertEqual(response.status_code, 200)
+            self.assertEqual(response.content, 'success')
+            tracking_logs = TrackingLog.objects.order_by('-dtcreated')
+            log = tracking_logs[0]
+            self.assertEqual(log.event, request_params["event"])
+            self.assertEqual(log.event_type, request_params["event_type"])
+            self.assertEqual(log.page, request_params["page"])
+
+    def test_get_answers_to_log(self):
+        """
+        Checks that student answer requests submitted to track.views via GET
+        are correctly logged in the TrackingLog db table
+        """
+        requests = [
+            {"event": "my_event", "event_type": "my_event_type", "page": "my_page"},
+            {"event": "{'json': 'object'}", "event_type": unichr(512), "page": "my_page"}
+        ]
+        for request_params in requests:
+            try:  # because /event maps to two different views in lms and cms, we're only going to test lms here
+                response = self.client.get(reverse(user_track), request_params)
+            except NoReverseMatch:
+                raise SkipTest()
+            self.assertEqual(response.status_code, 200)
+            self.assertEqual(response.content, 'success')
+            tracking_logs = TrackingLog.objects.order_by('-dtcreated')
+            log = tracking_logs[0]
+            self.assertEqual(log.event, request_params["event"])
+            self.assertEqual(log.event_type, request_params["event_type"])
+            self.assertEqual(log.page, request_params["page"])

common/djangoapps/track/views.py

@@ -34,9 +34,10 @@ def log_event(event):
 
 def user_track(request):
     """
-    Log when GET call to "event" URL is made by a user.
+    Log when POST call to "event" URL is made by a user. Uses request.REQUEST
+    to allow for GET calls.
 
-    GET call should provide "event_type", "event", and "page" arguments.
+    GET or POST call should provide "event_type", "event", and "page" arguments.
     """
     try:  # TODO: Do the same for many of the optional META parameters
         username = request.user.username
@@ -59,13 +60,14 @@ def user_track(request):
         "session": scookie,
         "ip": request.META['REMOTE_ADDR'],
         "event_source": "browser",
-        "event_type": request.GET['event_type'],
-        "event": request.GET['event'],
+        "event_type": request.REQUEST['event_type'],
+        "event": request.REQUEST['event'],
         "agent": agent,
-        "page": request.GET['page'],
+        "page": request.REQUEST['page'],
         "time": datetime.datetime.now(UTC).isoformat(),
         "host": request.META['SERVER_NAME'],
-        }
+    }
+
     log_event(event)
     return HttpResponse('success')
 
@@ -92,7 +94,7 @@ def server_track(request, event_type, event, page=None):
         "page": page,
         "time": datetime.datetime.now(UTC).isoformat(),
         "host": request.META['SERVER_NAME'],
-        }
+    }
 
     if event_type.startswith("/event_logs") and request.user.is_staff:  # don't log
         return
@@ -136,7 +138,7 @@ def task_track(request_info, task_info, event_type, event, page=None):
         "page": page,
         "time": datetime.datetime.utcnow().isoformat(),
         "host": request_info.get('host', 'unknown')
-        }
+    }
 
     log_event(event)
 

common/djangoapps/util/json_request.py

@@ -15,8 +15,7 @@ def expect_json(view_function):
         # e.g. 'charset', so we can't do a direct string compare
         if request.META.get('CONTENT_TYPE', '').lower().startswith("application/json"):
             cloned_request = copy.copy(request)
-            cloned_request.POST = cloned_request.POST.copy()
-            cloned_request.POST.update(json.loads(request.body))
+            cloned_request.POST = json.loads(request.body)
             return view_function(cloned_request, *args, **kwargs)
         else:
             return view_function(request, *args, **kwargs)

common/djangoapps/util/testing.py

@@ -1,7 +1,7 @@
 import sys
 
 from django.conf import settings
-from django.core.urlresolvers import clear_url_caches
+from django.core.urlresolvers import clear_url_caches, resolve
 
 
 class UrlResetMixin(object):
@@ -27,6 +27,9 @@ class UrlResetMixin(object):
             reload(sys.modules[urlconf])
         clear_url_caches()
 
+        # Resolve a URL so that the new urlconf gets loaded
+        resolve('/')
+
     def setUp(self):
         """Reset django default urlconf before tests and after tests"""
         super(UrlResetMixin, self).setUp()

common/djangoapps/util/views.py

@@ -4,7 +4,10 @@ import sys
 
 from django.conf import settings
 from django.core.validators import ValidationError, validate_email
-from django.http import Http404, HttpResponse, HttpResponseNotAllowed
+from django.views.decorators.csrf import requires_csrf_token
+from django.views.defaults import server_error
+from django.http import (Http404, HttpResponse, HttpResponseNotAllowed,
+                         HttpResponseServerError)
 from dogapi import dog_stats_api
 from mitxmako.shortcuts import render_to_response
 import zendesk
@@ -16,6 +19,19 @@ import track.views
 log = logging.getLogger(__name__)
 
 
+@requires_csrf_token
+def jsonable_server_error(request, template_name='500.html'):
+    """
+    500 error handler that serves JSON on an AJAX request, and proxies
+    to the Django default `server_error` view otherwise.
+    """
+    if request.is_ajax():
+        msg = {"error": "The edX servers encountered an error"}
+        return HttpResponseServerError(json.dumps(msg))
+    else:
+        return server_error(request, template_name=template_name)
+
+
 def calculate(request):
     ''' Calculator in footer of every page. '''
     equation = request.GET['equation']
@@ -228,4 +244,3 @@ def accepts(request, media_type):
     """Return whether this request has an Accept header that matches type"""
     accept = parse_accept_header(request.META.get("HTTP_ACCEPT", ""))
     return media_type in [t for (t, p, q) in accept]
-

common/lib/calc/calc.py

@@ -93,7 +93,7 @@ def check_variables(string, variables):
     Pyparsing uses a left-to-right parser, which makes a more
     elegant approach pretty hopeless.
     """
-    general_whitespace = re.compile('[^\\w]+')
+    general_whitespace = re.compile('[^\\w]+')  # TODO consider non-ascii
     # List of all alnums in string
     possible_variables = re.split(general_whitespace, string)
     bad_variables = []

common/lib/capa/capa/capa_problem.py

@@ -373,7 +373,7 @@ class LoncapaProblem(object):
         html = contextualize_text(etree.tostring(self._extract_html(self.tree)), self.context)
         return html
 
-    def handle_input_ajax(self, get):
+    def handle_input_ajax(self, data):
         '''
         InputTypes can support specialized AJAX calls. Find the correct input and pass along the correct data
 
@@ -381,10 +381,10 @@ class LoncapaProblem(object):
         '''
 
         # pull out the id
-        input_id = get['input_id']
+        input_id = data['input_id']
         if self.inputs[input_id]:
-            dispatch = get['dispatch']
-            return self.inputs[input_id].handle_ajax(dispatch, get)
+            dispatch = data['dispatch']
+            return self.inputs[input_id].handle_ajax(dispatch, data)
         else:
             log.warning("Could not find matching input for id: %s" % input_id)
             return {}

common/lib/capa/capa/inputtypes.py

@@ -223,13 +223,13 @@ class InputTypeBase(object):
         """
         pass
 
-    def handle_ajax(self, dispatch, get):
+    def handle_ajax(self, dispatch, data):
         """
         InputTypes that need to handle specialized AJAX should override this.
 
         Input:
             dispatch: a string that can be used to determine how to handle the data passed in
-            get: a dictionary containing the data that was sent with the ajax call
+            data: a dictionary containing the data that was sent with the ajax call
 
         Output:
             a dictionary object that can be serialized into JSON. This will be sent back to the Javascript.
@@ -739,20 +739,20 @@ class MatlabInput(CodeInput):
             self.queue_len = 1
             self.msg = self.plot_submitted_msg
 
-    def handle_ajax(self, dispatch, get):
+    def handle_ajax(self, dispatch, data):
         '''
         Handle AJAX calls directed to this input
 
         Args:
             - dispatch (str) - indicates how we want this ajax call to be handled
-            - get (dict) - dictionary of key-value pairs that contain useful data
+            - data (dict) - dictionary of key-value pairs that contain useful data
         Returns:
             dict - 'success' - whether or not we successfully queued this submission
                  - 'message' - message to be rendered in case of error
         '''
 
         if dispatch == 'plot':
-            return self._plot_data(get)
+            return self._plot_data(data)
         return {}
 
     def ungraded_response(self, queue_msg, queuekey):
@@ -813,7 +813,7 @@ class MatlabInput(CodeInput):
         msg = result['msg']
         return msg
 
-    def _plot_data(self, get):
+    def _plot_data(self, data):
         '''
         AJAX handler for the plot button
         Args:
@@ -827,7 +827,7 @@ class MatlabInput(CodeInput):
             return {'success': False, 'message': 'Cannot connect to the queue'}
 
         # pull relevant info out of get
-        response = get['submission']
+        response = data['submission']
 
         # construct xqueue headers
         qinterface = self.system.xqueue['interface']
@@ -1013,16 +1013,16 @@ class ChemicalEquationInput(InputTypeBase):
         """
         return {'previewer': '/static/js/capa/chemical_equation_preview.js', }
 
-    def handle_ajax(self, dispatch, get):
+    def handle_ajax(self, dispatch, data):
         '''
         Since we only have chemcalc preview this input, check to see if it
         matches the corresponding dispatch and send it through if it does
         '''
         if dispatch == 'preview_chemcalc':
-            return self.preview_chemcalc(get)
+            return self.preview_chemcalc(data)
         return {}
 
-    def preview_chemcalc(self, get):
+    def preview_chemcalc(self, data):
         """
         Render an html preview of a chemical formula or equation.  get should
         contain a key 'formula' and value 'some formula string'.
@@ -1036,7 +1036,7 @@ class ChemicalEquationInput(InputTypeBase):
 
         result = {'preview': '',
                   'error': ''}
-        formula = get['formula']
+        formula = data['formula']
         if formula is None:
             result['error'] = "No formula specified."
             return result

common/lib/capa/capa/safe_exec/safe_exec.py

@@ -18,7 +18,6 @@ import random as random_module
 import sys
 random = random_module.Random(%r)
 random.Random = random_module.Random
-del random_module
 sys.modules['random'] = random
 """
 

common/lib/capa/capa/tests/test_inputtypes.py

@@ -467,8 +467,8 @@ class MatlabTest(unittest.TestCase):
         self.assertEqual(context, expected)
 
     def test_plot_data(self):
-        get = {'submission': 'x = 1234;'}
-        response = self.the_input.handle_ajax("plot", get)
+        data = {'submission': 'x = 1234;'}
+        response = self.the_input.handle_ajax("plot", data)
 
         test_system().xqueue['interface'].send_to_queue.assert_called_with(header=ANY, body=ANY)
 
@@ -477,10 +477,10 @@ class MatlabTest(unittest.TestCase):
         self.assertEqual(self.the_input.input_state['queuestate'], 'queued')
 
     def test_plot_data_failure(self):
-        get = {'submission': 'x = 1234;'}
+        data = {'submission': 'x = 1234;'}
         error_message = 'Error message!'
         test_system().xqueue['interface'].send_to_queue.return_value = (1, error_message)
-        response = self.the_input.handle_ajax("plot", get)
+        response = self.the_input.handle_ajax("plot", data)
         self.assertFalse(response['success'])
         self.assertEqual(response['message'], error_message)
         self.assertTrue('queuekey' not in self.the_input.input_state)

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

@@ -1266,6 +1266,24 @@ class CustomResponseTest(ResponseTest):
         msg = correct_map.get_msg('1_2_1')
         self.assertEqual(msg, self._get_random_number_result(problem.seed))
 
+    def test_random_isnt_none(self):
+        # Bug LMS-500 says random.seed(10) fails with:
+        #     File "<string>", line 61, in <module>
+        #     File "/usr/lib/python2.7/random.py", line 116, in seed
+        #       super(Random, self).seed(a)
+        #   TypeError: must be type, not None
+
+        r = random.Random()
+        r.seed(10)
+        num = r.randint(0, 1e9)
+
+        script = textwrap.dedent("""
+            random.seed(10)
+            num = random.randint(0, 1e9)
+            """)
+        problem = self.build_problem(script=script)
+        self.assertEqual(problem.context['num'], num)
+
     def test_module_imports_inline(self):
         '''
         Check that the correct modules are available to custom

common/lib/symmath/symmath/formula.py

@@ -10,7 +10,7 @@
 # Provides sympy representation.
 
 import os
-import string
+import string       # pylint: disable=W0402
 import re
 import logging
 import operator

common/lib/xmodule/setup.py

@@ -55,6 +55,7 @@ setup(
             "word_cloud = xmodule.word_cloud_module:WordCloudDescriptor",
             "hidden = xmodule.hidden_module:HiddenDescriptor",
             "raw = xmodule.raw_module:RawDescriptor",
+            "crowdsource_hinter = xmodule.crowdsource_hinter:CrowdsourceHinterDescriptor",
         ],
         'console_scripts': [
             'xmodule_assets = xmodule.static_content:main',

common/lib/xmodule/xmodule/capa_module.py

@@ -47,6 +47,9 @@ def randomization_bin(seed, problem_id):
 
 
 class Randomization(String):
+    """
+    Define a field to store how to randomize a problem.
+    """
     def from_json(self, value):
         if value in ("", "true"):
             return "always"
@@ -58,24 +61,39 @@ class Randomization(String):
 
 
 class ComplexEncoder(json.JSONEncoder):
+    """
+    Extend the JSON encoder to correctly handle complex numbers
+    """
     def default(self, obj):
+        """
+        Print a nicely formatted complex number, or default to the JSON encoder
+        """
         if isinstance(obj, complex):
-            return "{real:.7g}{imag:+.7g}*j".format(real=obj.real, imag=obj.imag)
+            return u"{real:.7g}{imag:+.7g}*j".format(real=obj.real, imag=obj.imag)
         return json.JSONEncoder.default(self, obj)
 
 
 class CapaFields(object):
-    attempts = Integer(help="Number of attempts taken by the student on this problem", default=0, scope=Scope.user_state)
+    """
+    Define the possible fields for a Capa problem
+    """
+    attempts = Integer(help="Number of attempts taken by the student on this problem",
+                       default=0, scope=Scope.user_state)
     max_attempts = Integer(
         display_name="Maximum Attempts",
-        help="Defines the number of times a student can try to answer this problem. If the value is not set, infinite attempts are allowed.",
+        help=("Defines the number of times a student can try to answer this problem. "
+              "If the value is not set, infinite attempts are allowed."),
         values={"min": 0}, scope=Scope.settings
     )
     due = Date(help="Date that this problem is due by", scope=Scope.settings)
-    graceperiod = Timedelta(help="Amount of time after the due date that submissions will be accepted", scope=Scope.settings)
+    graceperiod = Timedelta(
+        help="Amount of time after the due date that submissions will be accepted",
+        scope=Scope.settings
+    )
     showanswer = String(
         display_name="Show Answer",
-        help="Defines when to show the answer to the problem. A default value can be set in Advanced Settings.",
+        help=("Defines when to show the answer to the problem. "
+              "A default value can be set in Advanced Settings."),
         scope=Scope.settings, default="closed",
         values=[
             {"display_name": "Always", "value": "always"},
@@ -86,23 +104,33 @@ class CapaFields(object):
             {"display_name": "Past Due", "value": "past_due"},
             {"display_name": "Never", "value": "never"}]
     )
-    force_save_button = Boolean(help="Whether to force the save button to appear on the page", scope=Scope.settings, default=False)
+    force_save_button = Boolean(
+        help="Whether to force the save button to appear on the page",
+        scope=Scope.settings, default=False
+    )
     rerandomize = Randomization(
-        display_name="Randomization", help="Defines how often inputs are randomized when a student loads the problem. This setting only applies to problems that can have randomly generated numeric values. A default value can be set in Advanced Settings.",
-        default="always", scope=Scope.settings, values=[{"display_name": "Always", "value": "always"},
-                                                        {"display_name": "On Reset", "value": "onreset"},
-                                                        {"display_name": "Never", "value": "never"},
-                                                        {"display_name": "Per Student", "value": "per_student"}]
+        display_name="Randomization",
+        help="Defines how often inputs are randomized when a student loads the problem. "
+        "This setting only applies to problems that can have randomly generated numeric values. "
+        "A default value can be set in Advanced Settings.",
+        default="always", scope=Scope.settings, values=[
+            {"display_name": "Always", "value": "always"},
+            {"display_name": "On Reset", "value": "onreset"},
+            {"display_name": "Never", "value": "never"},
+            {"display_name": "Per Student", "value": "per_student"}
+        ]
     )
     data = String(help="XML data for the problem", scope=Scope.content)
-    correct_map = Dict(help="Dictionary with the correctness of current student answers", scope=Scope.user_state, default={})
+    correct_map = Dict(help="Dictionary with the correctness of current student answers",
+                       scope=Scope.user_state, default={})
     input_state = Dict(help="Dictionary for maintaining the state of inputtypes", scope=Scope.user_state)
     student_answers = Dict(help="Dictionary with the current student responses", scope=Scope.user_state)
     done = Boolean(help="Whether the student has answered the problem", scope=Scope.user_state)
     seed = Integer(help="Random seed for this student", scope=Scope.user_state)
     weight = Float(
         display_name="Problem Weight",
-        help="Defines the number of points each problem is worth. If the value is not set, each response field in the problem is worth one point.",
+        help=("Defines the number of points each problem is worth. "
+              "If the value is not set, each response field in the problem is worth one point."),
         values={"min": 0, "step": .1},
         scope=Scope.settings
     )
@@ -114,12 +142,12 @@ class CapaFields(object):
 
 
 class CapaModule(CapaFields, XModule):
-    '''
+    """
     An XModule implementing LonCapa format problems, implemented by way of
     capa.capa_problem.LoncapaProblem
 
     CapaModule.__init__ takes the same arguments as xmodule.x_module:XModule.__init__
-    '''
+    """
     icon_class = 'problem'
 
     js = {'coffee': [resource_string(__name__, 'js/src/capa/display.coffee'),
@@ -134,7 +162,9 @@ class CapaModule(CapaFields, XModule):
     css = {'scss': [resource_string(__name__, 'css/capa/display.scss')]}
 
     def __init__(self, *args, **kwargs):
-        """ Accepts the same arguments as xmodule.x_module:XModule.__init__ """
+        """
+        Accepts the same arguments as xmodule.x_module:XModule.__init__
+        """
         XModule.__init__(self, *args, **kwargs)
 
         due_date = self.due
@@ -167,7 +197,7 @@ class CapaModule(CapaFields, XModule):
                 self.seed = self.lcp.seed
 
         except Exception as err:
-            msg = 'cannot create LoncapaProblem {loc}: {err}'.format(
+            msg = u'cannot create LoncapaProblem {loc}: {err}'.format(
                 loc=self.location.url(), err=err)
             # TODO (vshnayder): do modules need error handlers too?
             # We shouldn't be switching on DEBUG.
@@ -176,12 +206,15 @@ class CapaModule(CapaFields, XModule):
                 # TODO (vshnayder): This logic should be general, not here--and may
                 # want to preserve the data instead of replacing it.
                 # e.g. in the CMS
-                msg = '<p>%s</p>' % msg.replace('<', '&lt;')
-                msg += '<p><pre>%s</pre></p>' % traceback.format_exc().replace('<', '&lt;')
+                msg = u'<p>{msg}</p>'.format(msg=cgi.escape(msg))
+                msg += u'<p><pre>{tb}</pre></p>'.format(
+                    tb=cgi.escape(traceback.format_exc()))
                 # create a dummy problem with error message instead of failing
-                problem_text = ('<problem><text><span class="inline-error">'
-                                'Problem %s has an error:</span>%s</text></problem>' %
-                                (self.location.url(), msg))
+                problem_text = (u'<problem><text><span class="inline-error">'
+                                u'Problem {url} has an error:</span>{msg}</text></problem>'.format(
+                                    url=self.location.url(),
+                                    msg=msg)
+                                )
                 self.lcp = self.new_lcp(self.get_state_for_lcp(), text=problem_text)
             else:
                 # add extra info and raise
@@ -192,7 +225,9 @@ class CapaModule(CapaFields, XModule):
         assert self.seed is not None
 
     def choose_new_seed(self):
-        """Choose a new seed."""
+        """
+        Choose a new seed.
+        """
         if self.rerandomize == 'never':
             self.seed = 1
         elif self.rerandomize == "per_student" and hasattr(self.system, 'seed'):
@@ -206,6 +241,9 @@ class CapaModule(CapaFields, XModule):
             self.seed %= MAX_RANDOMIZATION_BINS
 
     def new_lcp(self, state, text=None):
+        """
+        Generate a new Loncapa Problem
+        """
         if text is None:
             text = self.data
 
@@ -218,6 +256,9 @@ class CapaModule(CapaFields, XModule):
         )
 
     def get_state_for_lcp(self):
+        """
+        Give a dictionary holding the state of the module
+        """
         return {
             'done': self.done,
             'correct_map': self.correct_map,
@@ -227,6 +268,9 @@ class CapaModule(CapaFields, XModule):
         }
 
     def set_state_from_lcp(self):
+        """
+        Set the module's state from the settings in `self.lcp`
+        """
         lcp_state = self.lcp.get_state()
         self.done = lcp_state['done']
         self.correct_map = lcp_state['correct_map']
@@ -235,26 +279,36 @@ class CapaModule(CapaFields, XModule):
         self.seed = lcp_state['seed']
 
     def get_score(self):
+        """
+        Access the problem's score
+        """
         return self.lcp.get_score()
 
     def max_score(self):
+        """
+        Access the problem's max score
+        """
         return self.lcp.get_max_score()
 
     def get_progress(self):
-        ''' For now, just return score / max_score
-        '''
+        """
+        For now, just return score / max_score
+        """
         d = self.get_score()
         score = d['score']
         total = d['total']
         if total > 0:
             try:
                 return Progress(score, total)
-            except Exception:
+            except (TypeError, ValueError):
                 log.exception("Got bad progress")
                 return None
         return None
 
     def get_html(self):
+        """
+        Return some html with data about the module
+        """
         return self.system.render_template('problem_ajax.html', {
             'element_id': self.location.html_id(),
             'id': self.id,
@@ -265,6 +319,7 @@ class CapaModule(CapaFields, XModule):
     def check_button_name(self):
         """
         Determine the name for the "check" button.
+
         Usually it is just "Check", but if this is the student's
         final attempt, change the name to "Final Check"
         """
@@ -350,27 +405,26 @@ class CapaModule(CapaFields, XModule):
 
     def handle_problem_html_error(self, err):
         """
-        Change our problem to a dummy problem containing
-        a warning message to display to users.
+        Create a dummy problem to represent any errors.
 
-        Returns the HTML to show to users
+        Change our problem to a dummy problem containing a warning message to
+        display to users. Returns the HTML to show to users
 
-        *err* is the Exception encountered while rendering the problem HTML.
+        `err` is the Exception encountered while rendering the problem HTML.
         """
-        log.exception(err)
+        log.exception(err.message)
 
         # TODO (vshnayder): another switch on DEBUG.
         if self.system.DEBUG:
             msg = (
-                '[courseware.capa.capa_module] <font size="+1" color="red">'
-                'Failed to generate HTML for problem %s</font>' %
-                (self.location.url()))
-            msg += '<p>Error:</p><p><pre>%s</pre></p>' % str(err).replace('<', '&lt;')
-            msg += '<p><pre>%s</pre></p>' % traceback.format_exc().replace('<', '&lt;')
+                u'[courseware.capa.capa_module] <font size="+1" color="red">'
+                u'Failed to generate HTML for problem {url}</font>'.format(
+                    url=cgi.escape(self.location.url()))
+            )
+            msg += u'<p>Error:</p><p><pre>{msg}</pre></p>'.format(msg=cgi.escape(err.message))
+            msg += u'<p><pre>{tb}</pre></p>'.format(tb=cgi.escape(traceback.format_exc()))
             html = msg
 
-        # We're in non-debug mode, and possibly even in production. We want
-        #   to avoid bricking of problem as much as possible
         else:
             # We're in non-debug mode, and possibly even in production. We want
             #   to avoid bricking of problem as much as possible
@@ -416,8 +470,12 @@ class CapaModule(CapaFields, XModule):
         return html
 
     def get_problem_html(self, encapsulate=True):
-        '''Return html for the problem.  Adds check, reset, save buttons
-        as necessary based on the problem config and state.'''
+        """
+        Return html for the problem.
+
+        Adds check, reset, save buttons as necessary based on the problem config
+        and state.
+        """
 
         try:
             html = self.lcp.get_html()
@@ -454,22 +512,24 @@ class CapaModule(CapaFields, XModule):
 
         html = self.system.render_template('problem.html', context)
         if encapsulate:
-            html = '<div id="problem_{id}" class="problem" data-url="{ajax_url}">'.format(
-                id=self.location.html_id(), ajax_url=self.system.ajax_url) + html + "</div>"
+            html = u'<div id="problem_{id}" class="problem" data-url="{ajax_url}">'.format(
+                id=self.location.html_id(), ajax_url=self.system.ajax_url
+            ) + html + "</div>"
 
         # now do the substitutions which are filesystem based, e.g. '/static/' prefixes
         return self.system.replace_urls(html)
 
-    def handle_ajax(self, dispatch, get):
-        '''
+    def handle_ajax(self, dispatch, data):
+        """
         This is called by courseware.module_render, to handle an AJAX call.
-        "get" is request.POST.
+
+        `data` is request.POST.
 
         Returns a json dictionary:
         { 'progress_changed' : True/False,
           'progress' : 'none'/'in_progress'/'done',
           <other request-specific values here > }
-        '''
+        """
         handlers = {
             'problem_get': self.get_problem,
             'problem_check': self.check_problem,
@@ -487,18 +547,19 @@ class CapaModule(CapaFields, XModule):
         before = self.get_progress()
 
         try:
-            d = handlers[dispatch](get)
-
+            result = handlers[dispatch](data)
         except Exception as err:
             _, _, traceback_obj = sys.exc_info()
-            raise ProcessingError, err.message, traceback_obj
+            raise ProcessingError(err.message, traceback_obj)
 
         after = self.get_progress()
-        d.update({
+
+        result.update({
             'progress_changed': after != before,
             'progress_status': Progress.to_js_status_str(after),
         })
-        return json.dumps(d, cls=ComplexEncoder)
+
+        return json.dumps(result, cls=ComplexEncoder)
 
     def is_past_due(self):
         """
@@ -508,7 +569,9 @@ class CapaModule(CapaFields, XModule):
                 datetime.datetime.now(UTC()) > self.close_date)
 
     def closed(self):
-        ''' Is the student still allowed to submit answers? '''
+        """
+        Is the student still allowed to submit answers?
+        """
         if self.max_attempts is not None and self.attempts >= self.max_attempts:
             return True
         if self.is_past_due():
@@ -527,18 +590,24 @@ class CapaModule(CapaFields, XModule):
         return self.lcp.done
 
     def is_attempted(self):
-        """Used by conditional module"""
+        """
+        Has the problem been attempted?
+
+        used by conditional module
+        """
         return self.attempts > 0
 
     def is_correct(self):
-        """True if full points"""
+        """
+        True iff full points
+        """
         d = self.get_score()
         return d['score'] == d['total']
 
     def answer_available(self):
-        '''
+        """
         Is the user allowed to see an answer?
-        '''
+        """
         if self.showanswer == '':
             return False
         elif self.showanswer == "never":
@@ -565,66 +634,68 @@ class CapaModule(CapaFields, XModule):
 
         return False
 
-    def update_score(self, get):
+    def update_score(self, data):
         """
         Delivers grading response (e.g. from asynchronous code checking) to
             the capa problem, so its score can be updated
 
-        'get' must have a field 'response' which is a string that contains the
+        'data' must have a key 'response' which is a string that contains the
             grader's response
 
         No ajax return is needed. Return empty dict.
         """
-        queuekey = get['queuekey']
-        score_msg = get['xqueue_body']
+        queuekey = data['queuekey']
+        score_msg = data['xqueue_body']
         self.lcp.update_score(score_msg, queuekey)
         self.set_state_from_lcp()
         self.publish_grade()
 
         return dict()  # No AJAX return is needed
 
-    def handle_ungraded_response(self, get):
-        '''
+    def handle_ungraded_response(self, data):
+        """
         Delivers a response from the XQueue to the capa problem
 
         The score of the problem will not be updated
 
         Args:
-            - get (dict) must contain keys:
+            - data (dict) must contain keys:
                             queuekey - a key specific to this response
                             xqueue_body - the body of the response
         Returns:
             empty dictionary
 
         No ajax return is needed, so an empty dict is returned
-        '''
-        queuekey = get['queuekey']
-        score_msg = get['xqueue_body']
+        """
+        queuekey = data['queuekey']
+        score_msg = data['xqueue_body']
+
         # pass along the xqueue message to the problem
         self.lcp.ungraded_response(score_msg, queuekey)
         self.set_state_from_lcp()
         return dict()
 
-    def handle_input_ajax(self, get):
-        '''
+    def handle_input_ajax(self, data):
+        """
         Handle ajax calls meant for a particular input in the problem
 
         Args:
-            - get (dict) - data that should be passed to the input
+            - data (dict) - data that should be passed to the input
         Returns:
             - dict containing the response from the input
-        '''
-        response = self.lcp.handle_input_ajax(get)
+        """
+        response = self.lcp.handle_input_ajax(data)
+
         # save any state changes that may occur
         self.set_state_from_lcp()
         return response
 
-    def get_answer(self, get):
-        '''
+    def get_answer(self, data):
+        """
         For the "show answer" button.
 
         Returns the answers: {'answers' : answers}
-        '''
+        """
         event_info = dict()
         event_info['problem_id'] = self.location.url()
         self.system.track_function('showanswer', event_info)
@@ -641,51 +712,55 @@ class CapaModule(CapaFields, XModule):
             try:
                 new_answer = {answer_id: self.system.replace_urls(answers[answer_id])}
             except TypeError:
-                log.debug('Unable to perform URL substitution on answers[%s]: %s' % (answer_id, answers[answer_id]))
+                log.debug(u'Unable to perform URL substitution on answers[%s]: %s',
+                          answer_id, answers[answer_id])
                 new_answer = {answer_id: answers[answer_id]}
             new_answers.update(new_answer)
 
         return {'answers': new_answers}
 
     # Figure out if we should move these to capa_problem?
-    def get_problem(self, get):
-        ''' Return results of get_problem_html, as a simple dict for json-ing.
+    def get_problem(self, _data):
+        """
+        Return results of get_problem_html, as a simple dict for json-ing.
         { 'html': <the-html> }
 
-            Used if we want to reconfirm we have the right thing e.g. after
-            several AJAX calls.
-        '''
+        Used if we want to reconfirm we have the right thing e.g. after
+        several AJAX calls.
+        """
         return {'html': self.get_problem_html(encapsulate=False)}
 
     @staticmethod
-    def make_dict_of_responses(get):
-        '''Make dictionary of student responses (aka "answers")
-        get is POST dictionary (Django QueryDict).
+    def make_dict_of_responses(data):
+        """
+        Make dictionary of student responses (aka "answers")
 
-        The *get* dict has keys of the form 'x_y', which are mapped
+        `data` is POST dictionary (Django QueryDict).
+
+        The `data` dict has keys of the form 'x_y', which are mapped
         to key 'y' in the returned dict.  For example,
         'input_1_2_3' would be mapped to '1_2_3' in the returned dict.
 
         Some inputs always expect a list in the returned dict
         (e.g. checkbox inputs).  The convention is that
-        keys in the *get* dict that end with '[]' will always
+        keys in the `data` dict that end with '[]' will always
         have list values in the returned dict.
-        For example, if the *get* dict contains {'input_1[]': 'test' }
+        For example, if the `data` dict contains {'input_1[]': 'test' }
         then the output dict would contain {'1': ['test'] }
         (the value is a list).
 
         Raises an exception if:
 
-            A key in the *get* dictionary does not contain >= 1 underscores
-            (e.g. "input" is invalid; "input_1" is valid)
+        -A key in the `data` dictionary does not contain at least one underscore
+          (e.g. "input" is invalid, but "input_1" is valid)
 
-            Two keys end up with the same name in the returned dict.
-            (e.g. 'input_1' and 'input_1[]', which both get mapped
-            to 'input_1' in the returned dict)
-        '''
+        -Two keys end up with the same name in the returned dict.
+          (e.g. 'input_1' and 'input_1[]', which both get mapped to 'input_1'
+           in the returned dict)
+        """
         answers = dict()
 
-        for key in get:
+        for key in data:
             # e.g. input_resistor_1 ==> resistor_1
             _, _, name = key.partition('_')
 
@@ -693,7 +768,7 @@ class CapaModule(CapaFields, XModule):
             # will return (key, '', '')
             # We detect this and raise an error
             if not name:
-                raise ValueError("%s must contain at least one underscore" % str(key))
+                raise ValueError(u"{key} must contain at least one underscore".format(key=key))
 
             else:
                 # This allows for answers which require more than one value for
@@ -704,14 +779,14 @@ class CapaModule(CapaFields, XModule):
                 name = name[:-2] if is_list_key else name
 
                 if is_list_key:
-                    val = get.getlist(key)
+                    val = data.getlist(key)
                 else:
-                    val = get[key]
+                    val = data[key]
 
                 # If the name already exists, then we don't want
                 # to override it.  Raise an error instead
                 if name in answers:
-                    raise ValueError("Key %s already exists in answers dict" % str(name))
+                    raise ValueError(u"Key {name} already exists in answers dict".format(name=name))
                 else:
                     answers[name] = val
 
@@ -728,19 +803,21 @@ class CapaModule(CapaFields, XModule):
             'max_value': score['total'],
         })
 
-    def check_problem(self, get):
-        ''' Checks whether answers to a problem are correct, and
-            returns a map of correct/incorrect answers:
+    def check_problem(self, data):
+        """
+        Checks whether answers to a problem are correct
 
-            {'success' : 'correct' | 'incorrect' | AJAX alert msg string,
-             'contents' : html}
-            '''
+        Returns a map of correct/incorrect answers:
+          {'success' : 'correct' | 'incorrect' | AJAX alert msg string,
+           'contents' : html}
+        """
         event_info = dict()
         event_info['state'] = self.lcp.get_state()
         event_info['problem_id'] = self.location.url()
 
-        answers = self.make_dict_of_responses(get)
+        answers = self.make_dict_of_responses(data)
         event_info['answers'] = convert_files_to_filenames(answers)
+
         # Too late. Cannot submit
         if self.closed():
             event_info['failure'] = 'closed'
@@ -759,7 +836,8 @@ class CapaModule(CapaFields, XModule):
             prev_submit_time = self.lcp.get_recentmost_queuetime()
             waittime_between_requests = self.system.xqueue['waittime']
             if (current_time - prev_submit_time).total_seconds() < waittime_between_requests:
-                msg = 'You must wait at least %d seconds between submissions' % waittime_between_requests
+                msg = u'You must wait at least {wait} seconds between submissions'.format(
+                    wait=waittime_between_requests)
                 return {'success': msg, 'html': ''}  # Prompts a modal dialog in ajax callback
 
         try:
@@ -776,19 +854,19 @@ class CapaModule(CapaFields, XModule):
             # the full exception, including traceback,
             # in the response
             if self.system.user_is_staff:
-                msg = "Staff debug info: %s" % traceback.format_exc()
+                msg = u"Staff debug info: {tb}".format(tb=cgi.escape(traceback.format_exc()))
 
             # Otherwise, display just an error message,
             # without a stack trace
             else:
-                msg = "Error: %s" % str(inst.message)
+                msg = u"Error: {msg}".format(msg=inst.message)
 
             return {'success': msg}
 
         except Exception as err:
             if self.system.DEBUG:
-                msg = "Error checking problem: " + str(err)
-                msg += '\nTraceback:\n' + traceback.format_exc()
+                msg = u"Error checking problem: {}".format(err.message)
+                msg += u'\nTraceback:\n{}'.format(traceback.format_exc())
                 return {'success': msg}
             raise
 
@@ -897,7 +975,7 @@ class CapaModule(CapaFields, XModule):
 
         return {'success': success}
 
-    def save_problem(self, get):
+    def save_problem(self, data):
         """
         Save the passed in answers.
         Returns a dict { 'success' : bool, 'msg' : message }
@@ -907,7 +985,7 @@ class CapaModule(CapaFields, XModule):
         event_info['state'] = self.lcp.get_state()
         event_info['problem_id'] = self.location.url()
 
-        answers = self.make_dict_of_responses(get)
+        answers = self.make_dict_of_responses(data)
         event_info['answers'] = answers
 
         # Too late. Cannot submit
@@ -936,17 +1014,18 @@ class CapaModule(CapaFields, XModule):
         return {'success': True,
                 'msg': msg}
 
-    def reset_problem(self, get):
-        ''' Changes problem state to unfinished -- removes student answers,
-            and causes problem to rerender itself.
+    def reset_problem(self, _data):
+        """
+        Changes problem state to unfinished -- removes student answers,
+        and causes problem to rerender itself.
 
-            Returns a dictionary of the form:
-            {'success': True/False,
-            'html': Problem HTML string }
+        Returns a dictionary of the form:
+          {'success': True/False,
+           'html': Problem HTML string }
 
-            If an error occurs, the dictionary will also have an
-            'error' key containing an error message.
-        '''
+        If an error occurs, the dictionary will also have an
+        `error` key containing an error message.
+        """
         event_info = dict()
         event_info['old_state'] = self.lcp.get_state()
         event_info['problem_id'] = self.location.url()
@@ -993,7 +1072,8 @@ class CapaDescriptor(CapaFields, RawDescriptor):
     mako_template = "widgets/problem-edit.html"
     js = {'coffee': [resource_string(__name__, 'js/src/problem/edit.coffee')]}
     js_module_name = "MarkdownEditingDescriptor"
-    css = {'scss': [resource_string(__name__, 'css/editor/edit.scss'), resource_string(__name__, 'css/problem/edit.scss')]}
+    css = {'scss': [resource_string(__name__, 'css/editor/edit.scss'),
+                    resource_string(__name__, 'css/problem/edit.scss')]}
 
     # Capa modules have some additional metadata:
     # TODO (vshnayder): do problems have any other metadata?  Do they

common/lib/xmodule/xmodule/combined_open_ended_module.py

@@ -204,9 +204,9 @@ class CombinedOpenEndedModule(CombinedOpenEndedFields, XModule):
         return_value = self.child_module.get_html()
         return return_value
 
-    def handle_ajax(self, dispatch, get):
+    def handle_ajax(self, dispatch, data):
         self.save_instance_data()
-        return_value = self.child_module.handle_ajax(dispatch, get)
+        return_value = self.child_module.handle_ajax(dispatch, data)
         self.save_instance_data()
         return return_value
 
@@ -266,4 +266,3 @@ class CombinedOpenEndedDescriptor(CombinedOpenEndedFields, RawDescriptor):
         non_editable_fields.extend([CombinedOpenEndedDescriptor.due, CombinedOpenEndedDescriptor.graceperiod,
                                     CombinedOpenEndedDescriptor.markdown, CombinedOpenEndedDescriptor.version])
         return non_editable_fields
-

common/lib/xmodule/xmodule/conditional_module.py

@@ -135,7 +135,7 @@ class ConditionalModule(ConditionalFields, XModule):
             'depends': ';'.join(self.required_html_ids)
         })
 
-    def handle_ajax(self, dispatch, post):
+    def handle_ajax(self, _dispatch, _data):
         """This is called by courseware.moduleodule_render, to handle
         an AJAX call.
         """

common/lib/xmodule/xmodule/contentstore/django.py

@@ -18,8 +18,6 @@ def load_function(path):
 
 
 def contentstore(name='default'):
-    global _CONTENTSTORE
-
     if name not in _CONTENTSTORE:
         class_ = load_function(settings.CONTENTSTORE['ENGINE'])
         options = {}

common/lib/xmodule/xmodule/contentstore/mongo.py

@@ -2,7 +2,8 @@ from pymongo import Connection
 import gridfs
 from gridfs.errors import NoFile
 
-from xmodule.modulestore.mongo import location_to_query, Location
+from xmodule.modulestore import Location
+from xmodule.modulestore.mongo.base import location_to_query
 from xmodule.contentstore.content import XASSET_LOCATION_TAG
 
 import logging

common/lib/xmodule/xmodule/course_module.py

@@ -212,6 +212,9 @@ class CourseDescriptor(CourseFields, SequenceDescriptor):
     template_dir_name = 'course'
 
     def __init__(self, *args, **kwargs):
+        """
+        Expects the same arguments as XModuleDescriptor.__init__
+        """
         super(CourseDescriptor, self).__init__(*args, **kwargs)
 
         if self.wiki_slug is None:

common/lib/xmodule/xmodule/crowdsource_hinter.py

@@ -0,0 +1,311 @@
+"""
+Adds crowdsourced hinting functionality to lon-capa numerical response problems.
+
+Currently experimental - not for instructor use, yet.
+"""
+
+import logging
+import json
+import random
+
+from pkg_resources import resource_string
+
+from lxml import etree
+
+from xmodule.x_module import XModule
+from xmodule.xml_module import XmlDescriptor
+from xblock.core import Scope, String, Integer, Boolean, Dict, List
+
+from django.utils.html import escape
+
+log = logging.getLogger(__name__)
+
+
+class CrowdsourceHinterFields(object):
+    """Defines fields for the crowdsource hinter module."""
+    has_children = True
+
+    moderate = String(help='String "True"/"False" - activates moderation', scope=Scope.content,
+                      default='False')
+    debug = String(help='String "True"/"False" - allows multiple voting', scope=Scope.content,
+                   default='False')
+    # Usage: hints[answer] = {str(pk): [hint_text, #votes]}
+    # hints is a dictionary that takes answer keys.
+    # Each value is itself a dictionary, accepting hint_pk strings as keys,
+    # and returning [hint text, #votes] pairs as values
+    hints = Dict(help='A dictionary containing all the active hints.', scope=Scope.content, default={})
+    mod_queue = Dict(help='A dictionary containing hints still awaiting approval', scope=Scope.content,
+                     default={})
+    hint_pk = Integer(help='Used to index hints.', scope=Scope.content, default=0)
+    # A list of previous answers this student made to this problem.
+    # Of the form [answer, [hint_pk_1, hint_pk_2, hint_pk_3]] for each problem.  hint_pk's are
+    # None if the hint was not given.
+    previous_answers = List(help='A list of previous submissions.', scope=Scope.user_state, default=[])
+    user_voted = Boolean(help='Specifies if the user has voted on this problem or not.',
+                         scope=Scope.user_state, default=False)
+
+
+class CrowdsourceHinterModule(CrowdsourceHinterFields, XModule):
+    """
+    An Xmodule that makes crowdsourced hints.
+    Currently, only works on capa problems with exactly one numerical response,
+    and no other parts.
+
+    Example usage:
+    <crowdsource_hinter>
+        <problem blah blah />
+    </crowdsource_hinter>
+
+    XML attributes:
+    -moderate="True" will not display hints until staff approve them in the hint manager.
+    -debug="True" will let users vote as often as they want.
+    """
+    icon_class = 'crowdsource_hinter'
+    css = {'scss': [resource_string(__name__, 'css/crowdsource_hinter/display.scss')]}
+    js = {'coffee': [resource_string(__name__, 'js/src/crowdsource_hinter/display.coffee')],
+          'js': []}
+    js_module_name = "Hinter"
+
+    def __init__(self, *args, **kwargs):
+        XModule.__init__(self, *args, **kwargs)
+
+    def get_html(self):
+        """
+        Puts a wrapper around the problem html.  This wrapper includes ajax urls of the
+        hinter and of the problem.
+        - Dependent on lon-capa problem.
+        """
+        if self.debug == 'True':
+            # Reset the user vote, for debugging only!
+            self.user_voted = False
+        if self.hints == {}:
+            # Force self.hints to be written into the database.  (When an xmodule is initialized,
+            # fields are not added to the db until explicitly changed at least once.)
+            self.hints = {}
+
+        try:
+            child = self.get_display_items()[0]
+            out = child.get_html()
+            # The event listener uses the ajax url to find the child.
+            child_url = child.system.ajax_url
+        except IndexError:
+            out = 'Error in loading crowdsourced hinter - can\'t find child problem.'
+            child_url = ''
+
+        # Wrap the module in a <section>.  This lets us pass data attributes to the javascript.
+        out += '<section class="crowdsource-wrapper" data-url="' + self.system.ajax_url +\
+            '" data-child-url = "' + child_url + '"> </section>'
+
+        return out
+
+    def capa_answer_to_str(self, answer):
+        """
+        Converts capa answer format to a string representation
+        of the answer.
+        -Lon-capa dependent.
+        -Assumes that the problem only has one part.
+        """
+        return str(float(answer.values()[0]))
+
+    def handle_ajax(self, dispatch, get):
+        """
+        This is the landing method for AJAX calls.
+        """
+        if dispatch == 'get_hint':
+            out = self.get_hint(get)
+        elif dispatch == 'get_feedback':
+            out = self.get_feedback(get)
+        elif dispatch == 'vote':
+            out = self.tally_vote(get)
+        elif dispatch == 'submit_hint':
+            out = self.submit_hint(get)
+        else:
+            return json.dumps({'contents': 'Error - invalid operation.'})
+
+        if out is None:
+            out = {'op': 'empty'}
+        else:
+            out.update({'op': dispatch})
+        return json.dumps({'contents': self.system.render_template('hinter_display.html', out)})
+
+    def get_hint(self, get):
+        """
+        The student got the incorrect answer found in get.  Give him a hint.
+
+        Called by hinter javascript after a problem is graded as incorrect.
+        Args:
+        `get` -- must be interpretable by capa_answer_to_str.
+        Output keys:
+            - 'best_hint' is the hint text with the most votes.
+            - 'rand_hint_1' and 'rand_hint_2' are two random hints to the answer in `get`.
+            - 'answer' is the parsed answer that was submitted.
+        """
+        answer = self.capa_answer_to_str(get)
+        # Look for a hint to give.
+        # Make a local copy of self.hints - this means we only need to do one json unpacking.
+        # (This is because xblocks storage makes the following command a deep copy.)
+        local_hints = self.hints
+        if (answer not in local_hints) or (len(local_hints[answer]) == 0):
+            # No hints to give.  Return.
+            self.previous_answers += [[answer, [None, None, None]]]
+            return
+        # Get the top hint, plus two random hints.
+        n_hints = len(local_hints[answer])
+        best_hint_index = max(local_hints[answer], key=lambda key: local_hints[answer][key][1])
+        best_hint = local_hints[answer][best_hint_index][0]
+        if len(local_hints[answer]) == 1:
+            rand_hint_1 = ''
+            rand_hint_2 = ''
+            self.previous_answers += [[answer, [best_hint_index, None, None]]]
+        elif n_hints == 2:
+            best_hint = local_hints[answer].values()[0][0]
+            best_hint_index = local_hints[answer].keys()[0]
+            rand_hint_1 = local_hints[answer].values()[1][0]
+            hint_index_1 = local_hints[answer].keys()[1]
+            rand_hint_2 = ''
+            self.previous_answers += [[answer, [best_hint_index, hint_index_1, None]]]
+        else:
+            (hint_index_1, rand_hint_1), (hint_index_2, rand_hint_2) =\
+                random.sample(local_hints[answer].items(), 2)
+            rand_hint_1 = rand_hint_1[0]
+            rand_hint_2 = rand_hint_2[0]
+            self.previous_answers += [[answer, [best_hint_index, hint_index_1, hint_index_2]]]
+
+        return {'best_hint': best_hint,
+                'rand_hint_1': rand_hint_1,
+                'rand_hint_2': rand_hint_2,
+                'answer': answer}
+
+    def get_feedback(self, get):
+        """
+        The student got it correct.  Ask him to vote on hints, or submit a hint.
+
+        Args:
+        `get` -- not actually used.  (It is assumed that the answer is correct.)
+        Output keys:
+            - 'index_to_hints' maps previous answer indices to hints that the user saw earlier.
+            - 'index_to_answer' maps previous answer indices to the actual answer submitted.
+        """
+        # The student got it right.
+        # Did he submit at least one wrong answer?
+        if len(self.previous_answers) == 0:
+            # No.  Nothing to do here.
+            return
+        # Make a hint-voting interface for each wrong answer.  The student will only
+        # be allowed to make one vote / submission, but he can choose which wrong answer
+        # he wants to look at.
+        # index_to_hints[previous answer #] = [(hint text, hint pk), + ]
+        index_to_hints = {}
+        # index_to_answer[previous answer #] = answer text
+        index_to_answer = {}
+
+        # Go through each previous answer, and populate index_to_hints and index_to_answer.
+        for i in xrange(len(self.previous_answers)):
+            answer, hints_offered = self.previous_answers[i]
+            index_to_hints[i] = []
+            index_to_answer[i] = answer
+            if answer in self.hints:
+                # Go through each hint, and add to index_to_hints
+                for hint_id in hints_offered:
+                    if hint_id is not None:
+                        try:
+                            index_to_hints[i].append((self.hints[answer][str(hint_id)][0], hint_id))
+                        except KeyError:
+                            # Sometimes, the hint that a user saw will have been deleted by the instructor.
+                            continue
+
+        return {'index_to_hints': index_to_hints, 'index_to_answer': index_to_answer}
+
+    def tally_vote(self, get):
+        """
+        Tally a user's vote on his favorite hint.
+
+        Args:
+        `get` -- expected to have the following keys:
+            'answer': ans_no (index in previous_answers)
+            'hint': hint_pk
+        Returns key 'hint_and_votes', a list of (hint_text, #votes) pairs.
+        """
+        if self.user_voted:
+            return json.dumps({'contents': 'Sorry, but you have already voted!'})
+        ans_no = int(get['answer'])
+        hint_no = str(get['hint'])
+        answer = self.previous_answers[ans_no][0]
+        # We use temp_dict because we need to do a direct write for the database to update.
+        temp_dict = self.hints
+        temp_dict[answer][hint_no][1] += 1
+        self.hints = temp_dict
+        # Don't let the user vote again!
+        self.user_voted = True
+
+        # Return a list of how many votes each hint got.
+        hint_and_votes = []
+        for hint_no in self.previous_answers[ans_no][1]:
+            if hint_no is None:
+                continue
+            hint_and_votes.append(temp_dict[answer][str(hint_no)])
+
+        # Reset self.previous_answers.
+        self.previous_answers = []
+        return {'hint_and_votes': hint_and_votes}
+
+    def submit_hint(self, get):
+        """
+        Take a hint submission and add it to the database.
+
+        Args:
+        `get` -- expected to have the following keys:
+            'answer': answer index in previous_answers
+            'hint': text of the new hint that the user is adding
+        Returns a thank-you message.
+        """
+        # Do html escaping.  Perhaps in the future do profanity filtering, etc. as well.
+        hint = escape(get['hint'])
+        answer = self.previous_answers[int(get['answer'])][0]
+        # Only allow a student to vote or submit a hint once.
+        if self.user_voted:
+            return {'message': 'Sorry, but you have already voted!'}
+        # Add the new hint to self.hints or self.mod_queue.  (Awkward because a direct write
+        # is necessary.)
+        if self.moderate == 'True':
+            temp_dict = self.mod_queue
+        else:
+            temp_dict = self.hints
+        if answer in temp_dict:
+            temp_dict[answer][self.hint_pk] = [hint, 1]     # With one vote (the user himself).
+        else:
+            temp_dict[answer] = {self.hint_pk: [hint, 1]}
+        self.hint_pk += 1
+        if self.moderate == 'True':
+            self.mod_queue = temp_dict
+        else:
+            self.hints = temp_dict
+        # Mark the user has having voted; reset previous_answers
+        self.user_voted = True
+        self.previous_answers = []
+        return {'message': 'Thank you for your hint!'}
+
+
+class CrowdsourceHinterDescriptor(CrowdsourceHinterFields, XmlDescriptor):
+    module_class = CrowdsourceHinterModule
+    stores_state = True
+
+    @classmethod
+    def definition_from_xml(cls, xml_object, system):
+        children = []
+        for child in xml_object:
+            try:
+                children.append(system.process_xml(etree.tostring(child, encoding='unicode')).location.url())
+            except Exception as e:
+                log.exception("Unable to load child when parsing CrowdsourceHinter. Continuing...")
+                if system.error_tracker is not None:
+                    system.error_tracker("ERROR: " + str(e))
+                continue
+        return {}, children
+
+    def definition_to_xml(self, resource_fs):
+        xml_object = etree.Element('crowdsource_hinter')
+        for child in self.get_children():
+            xml_object.append(
+                etree.fromstring(child.export_to_xml(resource_fs)))
+        return xml_object

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

@@ -0,0 +1,65 @@
+.crowdsource-wrapper {
+  @include box-shadow(inset 0 1px 2px 1px rgba(0,0,0,0.1));
+  @include border-radius(2px);
+  display: none;
+  margin-top: 20px;
+  padding: (15px);
+  background: rgb(253, 248, 235);
+}
+
+#answer-tabs {
+  background: #FFFFFF;
+  border: none;
+  margin-bottom: 20px;
+  padding-bottom: 20px;
+}
+
+#answer-tabs .ui-widget-header {
+  border-bottom: 1px solid #DCDCDC;
+  background: #F3F3F3;
+}
+
+#answer-tabs .ui-tabs-nav .ui-state-default {
+  border: 1px solid #DCDCDC;
+  background: #F8F8F8;
+  margin-bottom: 0px;
+}
+
+#answer-tabs .ui-tabs-nav .ui-state-default:hover {
+  background: #FFFFFF;
+}
+
+#answer-tabs .ui-tabs-nav .ui-state-active:hover {
+  background: #FFFFFF;
+}
+
+#answer-tabs .ui-tabs-nav .ui-state-active {
+  border: 1px solid #DCDCDC;
+  background: #FFFFFF;
+}
+
+#answer-tabs .ui-tabs-nav .ui-state-active a {
+  color: #222222;
+  background: #FFFFFF;
+}
+
+#answer-tabs .ui-tabs-nav .ui-state-default a:hover {
+  color: #222222;
+  background: #FFFFFF;
+}
+
+#answer-tabs .custom-hint {
+  height: 100px;
+  width: 100%;
+}
+
+.hint-inner-container {
+  padding-left: 15px;
+  padding-right: 15px;
+  font-size: 16px;
+}
+
+.vote {
+  padding-top: 0px !important;
+  padding-bottom: 0px !important;
+}

common/lib/xmodule/xmodule/js/src/capa/display.coffee

@@ -162,7 +162,8 @@ class @Problem
   #       maybe preferable to consolidate all dispatches to use FormData
   ###
   check_fd: =>
-    Logger.log 'problem_check', @answers
+    # Calling check from check_fd will result in firing the 'problem_check' event twice, since it is also called in the check function.
+    #Logger.log 'problem_check', @answers
 
     # If there are no file inputs in the problem, we can fall back on @check
     if $('input:file').length == 0
@@ -247,6 +248,7 @@ class @Problem
             @el.removeClass 'showed'
         else
           @gentle_alert response.success
+      Logger.log 'problem_graded', [@answers, response.contents], @url
 
   reset: =>
     Logger.log 'problem_reset', @answers
@@ -389,8 +391,6 @@ class @Problem
     choicegroup: (element, display, answers) =>
       element = $(element)
 
-      element.find('input').attr('disabled', 'disabled')
-
       input_id = element.attr('id').replace(/inputtype_/,'')
       answer = answers[input_id]
       for choice in answer
@@ -404,7 +404,6 @@ class @Problem
   inputtypeHideAnswerMethods:
     choicegroup: (element, display) =>
       element = $(element)
-      element.find('input').attr('disabled', null)
       element.find('label').removeClass('choicegroup_correct')
 
     javascriptinput: (element, display) =>

common/lib/xmodule/xmodule/js/src/crowdsource_hinter/display.coffee

@@ -0,0 +1,74 @@
+class @Hinter
+  # The client side code for the crowdsource_hinter.
+  # Contains code for capturing problem checks and making ajax calls to
+  # the server component.  Also contains styling code to clear default
+  # text on a textarea.
+
+  constructor: (element) ->
+    @el = $(element).find('.crowdsource-wrapper')
+    @url = @el.data('url')
+    Logger.listen('problem_graded', @el.data('child-url'), @capture_problem)
+    @render()
+
+  capture_problem: (event_type, data, element) =>
+    # After a problem gets graded, we get the info here.
+    # We want to send this info to the server in another AJAX
+    # request.
+    answers = data[0]
+    response = data[1]
+    if response.search(/class="correct/) == -1
+      # Incorrect.  Get hints.
+      $.postWithPrefix "#{@url}/get_hint", answers, (response) =>
+        @render(response.contents)
+    else
+      # Correct.  Get feedback from students.
+      $.postWithPrefix "#{@url}/get_feedback", answers, (response) =>
+        @render(response.contents)
+
+  $: (selector) ->
+    $(selector, @el)
+
+  bind: =>
+    window.update_schematics()
+    @$('input.vote').click @vote
+    @$('input.submit-hint').click @submit_hint
+    @$('.custom-hint').click @clear_default_text
+    @$('#answer-tabs').tabs({active: 0})
+    @$('.expand-goodhint').click @expand_goodhint
+
+  expand_goodhint: =>
+    if @$('.goodhint').css('display') == 'none'
+      @$('.goodhint').css('display', 'block')
+    else
+      @$('.goodhint').css('display', 'none')
+
+  vote: (eventObj) =>
+    target = @$(eventObj.currentTarget)
+    post_json = {'answer': target.data('answer'), 'hint': target.data('hintno')}
+    $.postWithPrefix "#{@url}/vote", post_json, (response) =>
+      @render(response.contents)
+
+  submit_hint: (eventObj) =>
+    target = @$(eventObj.currentTarget)
+    textarea_id = '#custom-hint-' + target.data('answer')
+    post_json = {'answer': target.data('answer'), 'hint': @$(textarea_id).val()}
+    $.postWithPrefix "#{@url}/submit_hint",post_json, (response) =>
+      @render(response.contents)
+
+  clear_default_text: (eventObj) =>
+    target = @$(eventObj.currentTarget)
+    if target.data('cleared') == undefined
+      target.val('')
+      target.data('cleared', true)
+
+  render: (content) ->
+    if content
+      # Trim leading and trailing whitespace
+      content = content.replace /^\s+|\s+$/g, ""
+
+    if content
+      @el.html(content)
+      @el.show()
+      JavascriptLoader.executeModuleScripts @el, () =>
+        @bind()
+      @$('#previous-answer-0').css('display', 'inline')

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

@@ -16,16 +16,7 @@ log = logging.getLogger('mitx.' + 'modulestore')
 
 
 URL_RE = re.compile("""
-    (?P<tag>[^:]+)://
-    (?P<org>[^/]+)/
-    (?P<course>[^/]+)/
-    (?P<category>[^/]+)/
-    (?P<name>[^@]+)
-    (@(?P<revision>[^/]+))?
-    """, re.VERBOSE)
-
-MISSING_SLASH_URL_RE = re.compile("""
-    (?P<tag>[^:]+):/
+    (?P<tag>[^:]+)://?
     (?P<org>[^/]+)/
     (?P<course>[^/]+)/
     (?P<category>[^/]+)/
@@ -52,8 +43,8 @@ class Location(_LocationBase):
     Locations representations of URLs of the
     form {tag}://{org}/{course}/{category}/{name}[@{revision}]
 
-    However, they can also be represented a dictionaries (specifying each component),
-    tuples or list (specified in order), or as strings of the url
+    However, they can also be represented as dictionaries (specifying each component),
+    tuples or lists (specified in order), or as strings of the url
     '''
     __slots__ = ()
 
@@ -180,13 +171,8 @@ class Location(_LocationBase):
         if isinstance(location, basestring):
             match = URL_RE.match(location)
             if match is None:
-                # cdodge:
-                # check for a dropped slash near the i4x:// element of the location string. This can happen with some
-                # redirects (e.g. edx.org -> www.edx.org which I think happens in Nginx)
-                match = MISSING_SLASH_URL_RE.match(location)
-                if match is None:
-                    log.debug('location is instance of %s but no URL match' % basestring)
-                    raise InvalidLocationError(location)
+                log.debug('location is instance of %s but no URL match' % basestring)
+                raise InvalidLocationError(location)
             groups = match.groupdict()
             check_dict(groups)
             return _LocationBase.__new__(_cls, **groups)

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

@@ -26,8 +26,6 @@ def load_function(path):
 
 
 def modulestore(name='default'):
-    global _MODULESTORES
-
     if name not in _MODULESTORES:
         class_ = load_function(settings.MODULESTORE[name]['ENGINE'])
 

common/lib/xmodule/xmodule/modulestore/draft.py

@@ -1,248 +1,7 @@
-from datetime import datetime
+"""
+Backwards compatibility for old pointers to draft module store
 
-from . import ModuleStoreBase, Location, namedtuple_to_son
-from .exceptions import ItemNotFoundError
-from .inheritance import own_metadata
-from xmodule.exceptions import InvalidVersionError
-from pytz import UTC
+This modulestore has been moved to xmodule.modulestore.mongo.draft
+"""
 
-DRAFT = 'draft'
-# Things w/ these categories should never be marked as version='draft'
-DIRECT_ONLY_CATEGORIES = ['course', 'chapter', 'sequential', 'about', 'static_tab', 'course_info']
-
-
-def as_draft(location):
-    """
-    Returns the Location that is the draft for `location`
-    """
-    return Location(location).replace(revision=DRAFT)
-
-
-def as_published(location):
-    """
-    Returns the Location that is the published version for `location`
-    """
-    return Location(location).replace(revision=None)
-
-
-def wrap_draft(item):
-    """
-    Sets `item.is_draft` to `True` if the item is a
-    draft, and `False` otherwise. Sets the item's location to the
-    non-draft location in either case
-    """
-    setattr(item, 'is_draft', item.location.revision == DRAFT)
-    item.location = item.location.replace(revision=None)
-    return item
-
-
-class DraftModuleStore(ModuleStoreBase):
-    """
-    This mixin modifies a modulestore to give it draft semantics.
-    That is, edits made to units are stored to locations that have the revision DRAFT,
-    and when reads are made, they first read with revision DRAFT, and then fall back
-    to the baseline revision only if DRAFT doesn't exist.
-
-    This module also includes functionality to promote DRAFT modules (and optionally
-    their children) to published modules.
-    """
-
-    def get_item(self, location, depth=0):
-        """
-        Returns an XModuleDescriptor instance for the item at location.
-        If location.revision is None, returns the item with the most
-        recent revision
-
-        If any segment of the location is None except revision, raises
-            xmodule.modulestore.exceptions.InsufficientSpecificationError
-
-        If no object is found at that location, raises
-            xmodule.modulestore.exceptions.ItemNotFoundError
-
-        location: Something that can be passed to Location
-
-        depth (int): An argument that some module stores may use to prefetch
-            descendents of the queried modules for more efficient results later
-            in the request. The depth is counted in the number of calls to
-            get_children() to cache. None indicates to cache all descendents
-        """
-
-        try:
-            return wrap_draft(super(DraftModuleStore, self).get_item(as_draft(location), depth=depth))
-        except ItemNotFoundError:
-            return wrap_draft(super(DraftModuleStore, self).get_item(location, depth=depth))
-
-    def get_instance(self, course_id, location, depth=0):
-        """
-        Get an instance of this location, with policy for course_id applied.
-        TODO (vshnayder): this may want to live outside the modulestore eventually
-        """
-
-        try:
-            return wrap_draft(super(DraftModuleStore, self).get_instance(course_id, as_draft(location), depth=depth))
-        except ItemNotFoundError:
-            return wrap_draft(super(DraftModuleStore, self).get_instance(course_id, location, depth=depth))
-
-    def get_items(self, location, course_id=None, depth=0):
-        """
-        Returns a list of XModuleDescriptor instances for the items
-        that match location. Any element of location that is None is treated
-        as a wildcard that matches any value
-
-        location: Something that can be passed to Location
-
-        depth: An argument that some module stores may use to prefetch
-            descendents of the queried modules for more efficient results later
-            in the request. The depth is counted in the number of calls to
-            get_children() to cache. None indicates to cache all descendents
-        """
-        draft_loc = as_draft(location)
-
-        draft_items = super(DraftModuleStore, self).get_items(draft_loc, course_id=course_id, depth=depth)
-        items = super(DraftModuleStore, self).get_items(location, course_id=course_id, depth=depth)
-
-        draft_locs_found = set(item.location._replace(revision=None) for item in draft_items)
-        non_draft_items = [
-            item
-            for item in items
-            if (item.location.revision != DRAFT
-                and item.location._replace(revision=None) not in draft_locs_found)
-        ]
-        return [wrap_draft(item) for item in draft_items + non_draft_items]
-
-    def clone_item(self, source, location):
-        """
-        Clone a new item that is a copy of the item at the location `source`
-        and writes it to `location`
-        """
-        if Location(location).category in DIRECT_ONLY_CATEGORIES:
-            raise InvalidVersionError(location)
-        return wrap_draft(super(DraftModuleStore, self).clone_item(source, as_draft(location)))
-
-    def update_item(self, location, data, allow_not_found=False):
-        """
-        Set the data in the item specified by the location to
-        data
-
-        location: Something that can be passed to Location
-        data: A nested dictionary of problem data
-        """
-        draft_loc = as_draft(location)
-        try:
-            draft_item = self.get_item(location)
-            if not getattr(draft_item, 'is_draft', False):
-                self.clone_item(location, draft_loc)
-        except ItemNotFoundError, e:
-            if not allow_not_found:
-                raise e
-
-        return super(DraftModuleStore, self).update_item(draft_loc, data)
-
-    def update_children(self, location, children):
-        """
-        Set the children for the item specified by the location to
-        children
-
-        location: Something that can be passed to Location
-        children: A list of child item identifiers
-        """
-        draft_loc = as_draft(location)
-        draft_item = self.get_item(location)
-        if not getattr(draft_item, 'is_draft', False):
-            self.clone_item(location, draft_loc)
-
-        return super(DraftModuleStore, self).update_children(draft_loc, children)
-
-    def update_metadata(self, location, metadata):
-        """
-        Set the metadata for the item specified by the location to
-        metadata
-
-        location: Something that can be passed to Location
-        metadata: A nested dictionary of module metadata
-        """
-        draft_loc = as_draft(location)
-        draft_item = self.get_item(location)
-
-        if not getattr(draft_item, 'is_draft', False):
-            self.clone_item(location, draft_loc)
-
-        if 'is_draft' in metadata:
-            del metadata['is_draft']
-
-        return super(DraftModuleStore, self).update_metadata(draft_loc, metadata)
-
-    def delete_item(self, location, delete_all_versions=False):
-        """
-        Delete an item from this modulestore
-
-        location: Something that can be passed to Location
-        """
-        super(DraftModuleStore, self).delete_item(as_draft(location))
-        if delete_all_versions:
-            super(DraftModuleStore, self).delete_item(as_published(location))
-
-        return
-
-    def get_parent_locations(self, location, course_id):
-        '''Find all locations that are the parents of this location.  Needed
-        for path_to_location().
-
-        returns an iterable of things that can be passed to Location.
-        '''
-        return super(DraftModuleStore, self).get_parent_locations(location, course_id)
-
-    def publish(self, location, published_by_id):
-        """
-        Save a current draft to the underlying modulestore
-        """
-        draft = self.get_item(location)
-
-        draft.cms.published_date = datetime.now(UTC)
-        draft.cms.published_by = published_by_id
-        super(DraftModuleStore, self).update_item(location, draft._model_data._kvs._data)
-        super(DraftModuleStore, self).update_children(location, draft._model_data._kvs._children)
-        super(DraftModuleStore, self).update_metadata(location, own_metadata(draft))
-        self.delete_item(location)
-
-    def unpublish(self, location):
-        """
-        Turn the published version into a draft, removing the published version
-        """
-        if Location(location).category in DIRECT_ONLY_CATEGORIES:
-            raise InvalidVersionError(location)
-        super(DraftModuleStore, self).clone_item(location, as_draft(location))
-        super(DraftModuleStore, self).delete_item(location)
-
-    def _query_children_for_cache_children(self, items):
-        # first get non-draft in a round-trip
-        queried_children = []
-        to_process_non_drafts = super(DraftModuleStore, self)._query_children_for_cache_children(items)
-
-        to_process_dict = {}
-        for non_draft in to_process_non_drafts:
-            to_process_dict[Location(non_draft["_id"])] = non_draft
-
-        # now query all draft content in another round-trip
-        query = {
-            '_id': {'$in': [namedtuple_to_son(as_draft(Location(item))) for item in items]}
-        }
-        to_process_drafts = list(self.collection.find(query))
-
-        # now we have to go through all drafts and replace the non-draft
-        # with the draft. This is because the semantics of the DraftStore is to
-        # always return the draft - if available
-        for draft in to_process_drafts:
-            draft_loc = Location(draft["_id"])
-            draft_as_non_draft_loc = draft_loc.replace(revision=None)
-
-            # does non-draft exist in the collection
-            # if so, replace it
-            if draft_as_non_draft_loc in to_process_dict:
-                to_process_dict[draft_as_non_draft_loc] = draft
-
-        # convert the dict - which is used for look ups - back into a list
-        for key, value in to_process_dict.iteritems():
-            queried_children.append(value)
-
-        return queried_children
+from xmodule.modulestore.mongo.draft import DIRECT_ONLY_CATEGORIES, DraftModuleStore

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

@@ -0,0 +1,5 @@
+from xmodule.modulestore.mongo.base import MongoModuleStore, MongoKeyValueStore, MongoUsage
+
+# Backwards compatibility for prod systems that refererence
+# xmodule.modulestore.mongo.DraftMongoModuleStore
+from xmodule.modulestore.mongo.draft import DraftModuleStore as DraftMongoModuleStore

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

@@ -18,11 +18,10 @@ from xmodule.error_module import ErrorDescriptor
 from xblock.runtime import DbModel, KeyValueStore, InvalidScopeError
 from xblock.core import Scope
 
-from . import ModuleStoreBase, Location, namedtuple_to_son
-from .draft import DraftModuleStore
-from .exceptions import (ItemNotFoundError,
+from xmodule.modulestore import ModuleStoreBase, Location, namedtuple_to_son
+from xmodule.modulestore.exceptions import (ItemNotFoundError,
                          DuplicateItemError)
-from .inheritance import own_metadata, INHERITABLE_METADATA, inherit_metadata
+from xmodule.modulestore.inheritance import own_metadata, INHERITABLE_METADATA, inherit_metadata
 
 log = logging.getLogger(__name__)
 
@@ -195,7 +194,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
                 if self.cached_metadata is not None:
                     # parent container pointers don't differentiate between draft and non-draft
                     # so when we do the lookup, we should do so with a non-draft location
-                    non_draft_loc = location._replace(revision=None)
+                    non_draft_loc = location.replace(revision=None)
                     metadata_to_inherit = self.cached_metadata.get(non_draft_loc.url(), {})
                     inherit_metadata(module, metadata_to_inherit)
                 return module
@@ -761,12 +760,3 @@ class MongoModuleStore(ModuleStoreBase):
         return {}
 
 
-# DraftModuleStore is first, because it needs to intercept calls to MongoModuleStore
-class DraftMongoModuleStore(DraftModuleStore, MongoModuleStore):
-    """
-    Version of MongoModuleStore with draft capability mixed in
-    """
-    """
-    Version of MongoModuleStore with draft capability mixed in
-    """
-    pass

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

@@ -0,0 +1,249 @@
+from datetime import datetime
+
+from xmodule.modulestore import Location, namedtuple_to_son
+from xmodule.modulestore.exceptions import ItemNotFoundError
+from xmodule.modulestore.inheritance import own_metadata
+from xmodule.exceptions import InvalidVersionError
+from xmodule.modulestore.mongo.base import MongoModuleStore
+from pytz import UTC
+
+DRAFT = 'draft'
+# Things w/ these categories should never be marked as version='draft'
+DIRECT_ONLY_CATEGORIES = ['course', 'chapter', 'sequential', 'about', 'static_tab', 'course_info']
+
+
+def as_draft(location):
+    """
+    Returns the Location that is the draft for `location`
+    """
+    return Location(location).replace(revision=DRAFT)
+
+
+def as_published(location):
+    """
+    Returns the Location that is the published version for `location`
+    """
+    return Location(location).replace(revision=None)
+
+
+def wrap_draft(item):
+    """
+    Sets `item.is_draft` to `True` if the item is a
+    draft, and `False` otherwise. Sets the item's location to the
+    non-draft location in either case
+    """
+    setattr(item, 'is_draft', item.location.revision == DRAFT)
+    item.location = item.location.replace(revision=None)
+    return item
+
+
+class DraftModuleStore(MongoModuleStore):
+    """
+    This mixin modifies a modulestore to give it draft semantics.
+    That is, edits made to units are stored to locations that have the revision DRAFT,
+    and when reads are made, they first read with revision DRAFT, and then fall back
+    to the baseline revision only if DRAFT doesn't exist.
+
+    This module also includes functionality to promote DRAFT modules (and optionally
+    their children) to published modules.
+    """
+
+    def get_item(self, location, depth=0):
+        """
+        Returns an XModuleDescriptor instance for the item at location.
+        If location.revision is None, returns the item with the most
+        recent revision
+
+        If any segment of the location is None except revision, raises
+            xmodule.modulestore.exceptions.InsufficientSpecificationError
+
+        If no object is found at that location, raises
+            xmodule.modulestore.exceptions.ItemNotFoundError
+
+        location: Something that can be passed to Location
+
+        depth (int): An argument that some module stores may use to prefetch
+            descendents of the queried modules for more efficient results later
+            in the request. The depth is counted in the number of calls to
+            get_children() to cache. None indicates to cache all descendents
+        """
+
+        try:
+            return wrap_draft(super(DraftModuleStore, self).get_item(as_draft(location), depth=depth))
+        except ItemNotFoundError:
+            return wrap_draft(super(DraftModuleStore, self).get_item(location, depth=depth))
+
+    def get_instance(self, course_id, location, depth=0):
+        """
+        Get an instance of this location, with policy for course_id applied.
+        TODO (vshnayder): this may want to live outside the modulestore eventually
+        """
+
+        try:
+            return wrap_draft(super(DraftModuleStore, self).get_instance(course_id, as_draft(location), depth=depth))
+        except ItemNotFoundError:
+            return wrap_draft(super(DraftModuleStore, self).get_instance(course_id, location, depth=depth))
+
+    def get_items(self, location, course_id=None, depth=0):
+        """
+        Returns a list of XModuleDescriptor instances for the items
+        that match location. Any element of location that is None is treated
+        as a wildcard that matches any value
+
+        location: Something that can be passed to Location
+
+        depth: An argument that some module stores may use to prefetch
+            descendents of the queried modules for more efficient results later
+            in the request. The depth is counted in the number of calls to
+            get_children() to cache. None indicates to cache all descendents
+        """
+        draft_loc = as_draft(location)
+
+        draft_items = super(DraftModuleStore, self).get_items(draft_loc, course_id=course_id, depth=depth)
+        items = super(DraftModuleStore, self).get_items(location, course_id=course_id, depth=depth)
+
+        draft_locs_found = set(item.location.replace(revision=None) for item in draft_items)
+        non_draft_items = [
+            item
+            for item in items
+            if (item.location.revision != DRAFT
+                and item.location.replace(revision=None) not in draft_locs_found)
+        ]
+        return [wrap_draft(item) for item in draft_items + non_draft_items]
+
+    def clone_item(self, source, location):
+        """
+        Clone a new item that is a copy of the item at the location `source`
+        and writes it to `location`
+        """
+        if Location(location).category in DIRECT_ONLY_CATEGORIES:
+            raise InvalidVersionError(location)
+        return wrap_draft(super(DraftModuleStore, self).clone_item(source, as_draft(location)))
+
+    def update_item(self, location, data, allow_not_found=False):
+        """
+        Set the data in the item specified by the location to
+        data
+
+        location: Something that can be passed to Location
+        data: A nested dictionary of problem data
+        """
+        draft_loc = as_draft(location)
+        try:
+            draft_item = self.get_item(location)
+            if not getattr(draft_item, 'is_draft', False):
+                self.clone_item(location, draft_loc)
+        except ItemNotFoundError, e:
+            if not allow_not_found:
+                raise e
+
+        return super(DraftModuleStore, self).update_item(draft_loc, data)
+
+    def update_children(self, location, children):
+        """
+        Set the children for the item specified by the location to
+        children
+
+        location: Something that can be passed to Location
+        children: A list of child item identifiers
+        """
+        draft_loc = as_draft(location)
+        draft_item = self.get_item(location)
+        if not getattr(draft_item, 'is_draft', False):
+            self.clone_item(location, draft_loc)
+
+        return super(DraftModuleStore, self).update_children(draft_loc, children)
+
+    def update_metadata(self, location, metadata):
+        """
+        Set the metadata for the item specified by the location to
+        metadata
+
+        location: Something that can be passed to Location
+        metadata: A nested dictionary of module metadata
+        """
+        draft_loc = as_draft(location)
+        draft_item = self.get_item(location)
+
+        if not getattr(draft_item, 'is_draft', False):
+            self.clone_item(location, draft_loc)
+
+        if 'is_draft' in metadata:
+            del metadata['is_draft']
+
+        return super(DraftModuleStore, self).update_metadata(draft_loc, metadata)
+
+    def delete_item(self, location, delete_all_versions=False):
+        """
+        Delete an item from this modulestore
+
+        location: Something that can be passed to Location
+        """
+        super(DraftModuleStore, self).delete_item(as_draft(location))
+        if delete_all_versions:
+            super(DraftModuleStore, self).delete_item(as_published(location))
+
+        return
+
+    def get_parent_locations(self, location, course_id):
+        '''Find all locations that are the parents of this location.  Needed
+        for path_to_location().
+
+        returns an iterable of things that can be passed to Location.
+        '''
+        return super(DraftModuleStore, self).get_parent_locations(location, course_id)
+
+    def publish(self, location, published_by_id):
+        """
+        Save a current draft to the underlying modulestore
+        """
+        draft = self.get_item(location)
+
+        draft.cms.published_date = datetime.now(UTC)
+        draft.cms.published_by = published_by_id
+        super(DraftModuleStore, self).update_item(location, draft._model_data._kvs._data)
+        super(DraftModuleStore, self).update_children(location, draft._model_data._kvs._children)
+        super(DraftModuleStore, self).update_metadata(location, own_metadata(draft))
+        self.delete_item(location)
+
+    def unpublish(self, location):
+        """
+        Turn the published version into a draft, removing the published version
+        """
+        if Location(location).category in DIRECT_ONLY_CATEGORIES:
+            raise InvalidVersionError(location)
+        super(DraftModuleStore, self).clone_item(location, as_draft(location))
+        super(DraftModuleStore, self).delete_item(location)
+
+    def _query_children_for_cache_children(self, items):
+        # first get non-draft in a round-trip
+        queried_children = []
+        to_process_non_drafts = super(DraftModuleStore, self)._query_children_for_cache_children(items)
+
+        to_process_dict = {}
+        for non_draft in to_process_non_drafts:
+            to_process_dict[Location(non_draft["_id"])] = non_draft
+
+        # now query all draft content in another round-trip
+        query = {
+            '_id': {'$in': [namedtuple_to_son(as_draft(Location(item))) for item in items]}
+        }
+        to_process_drafts = list(self.collection.find(query))
+
+        # now we have to go through all drafts and replace the non-draft
+        # with the draft. This is because the semantics of the DraftStore is to
+        # always return the draft - if available
+        for draft in to_process_drafts:
+            draft_loc = Location(draft["_id"])
+            draft_as_non_draft_loc = draft_loc.replace(revision=None)
+
+            # does non-draft exist in the collection
+            # if so, replace it
+            if draft_as_non_draft_loc in to_process_dict:
+                to_process_dict[draft_as_non_draft_loc] = draft
+
+        # convert the dict - which is used for look ups - back into a list
+        for key, value in to_process_dict.iteritems():
+            queried_children.append(value)
+
+        return queried_children

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

@@ -8,17 +8,97 @@ import xmodule.modulestore.django
 from xmodule.templates import update_templates
 
 
+def mongo_store_config(data_dir):
+    """
+    Defines default module store using MongoModuleStore.
+
+    Use of this config requires mongo to be running.
+    """
+    store = {
+        'default': {
+            'ENGINE': 'xmodule.modulestore.mongo.MongoModuleStore',
+            'OPTIONS': {
+                'default_class': 'xmodule.raw_module.RawDescriptor',
+                'host': 'localhost',
+                'db': 'test_xmodule',
+                'collection': 'modulestore_%s' % uuid4().hex,
+                'fs_root': data_dir,
+                'render_template': 'mitxmako.shortcuts.render_to_string'
+            }
+        }
+    }
+    store['direct'] = store['default']
+    return store
+
+
+def draft_mongo_store_config(data_dir):
+    """
+    Defines default module store using DraftMongoModuleStore.
+    """
+
+    modulestore_options = {
+        'default_class': 'xmodule.raw_module.RawDescriptor',
+        'host': 'localhost',
+        'db': 'test_xmodule',
+        'collection': 'modulestore_%s' % uuid4().hex,
+        'fs_root': data_dir,
+        'render_template': 'mitxmako.shortcuts.render_to_string'
+    }
+
+    return {
+        'default': {
+            'ENGINE': 'xmodule.modulestore.mongo.DraftMongoModuleStore',
+            'OPTIONS': modulestore_options
+        },
+        'direct': {
+            'ENGINE': 'xmodule.modulestore.mongo.MongoModuleStore',
+            'OPTIONS': modulestore_options
+        }
+    }
+
+
+def xml_store_config(data_dir):
+    """
+    Defines default module store using XMLModuleStore.
+    """
+    return {
+        'default': {
+            'ENGINE': 'xmodule.modulestore.xml.XMLModuleStore',
+            'OPTIONS': {
+                'data_dir': data_dir,
+                'default_class': 'xmodule.hidden_module.HiddenDescriptor',
+            }
+        }
+    }
+
+
 class ModuleStoreTestCase(TestCase):
     """ Subclass for any test case that uses the mongodb
     module store. This populates a uniquely named modulestore
     collection with templates before running the TestCase
     and drops it they are finished. """
 
+    @staticmethod
+    def update_course(course, data):
+        """
+        Updates the version of course in the modulestore
+        with the metadata in 'data' and returns the updated version.
+
+        'course' is an instance of CourseDescriptor for which we want
+        to update metadata.
+
+        'data' is a dictionary with an entry for each CourseField we want to update.
+        """
+        store = xmodule.modulestore.django.modulestore()
+        store.update_metadata(course.location, data)
+        updated_course = store.get_instance(course.id, course.location)
+        return updated_course
+
     @staticmethod
     def flush_mongo_except_templates():
-        '''
-        Delete everything in the module store except templates
-        '''
+        """
+        Delete everything in the module store except templates.
+        """
         modulestore = xmodule.modulestore.django.modulestore()
 
         # This query means: every item in the collection
@@ -27,14 +107,15 @@ class ModuleStoreTestCase(TestCase):
 
         # Remove everything except templates
         modulestore.collection.remove(query)
+        modulestore.collection.drop()
 
     @staticmethod
     def load_templates_if_necessary():
-        '''
+        """
         Load templates into the direct modulestore only if they do not already exist.
         We need the templates, because they are copied to create
-        XModules such as sections and problems
-        '''
+        XModules such as sections and problems.
+        """
         modulestore = xmodule.modulestore.django.modulestore('direct')
 
         # Count the number of templates
@@ -46,9 +127,9 @@ class ModuleStoreTestCase(TestCase):
 
     @classmethod
     def setUpClass(cls):
-        '''
-        Flush the mongo store and set up templates
-        '''
+        """
+        Flush the mongo store and set up templates.
+        """
 
         # Use a uuid to differentiate
         # the mongo collections on jenkins.
@@ -66,9 +147,9 @@ class ModuleStoreTestCase(TestCase):
 
     @classmethod
     def tearDownClass(cls):
-        '''
-        Revert to the old modulestore settings
-        '''
+        """
+        Revert to the old modulestore settings.
+        """
 
         # Clean up by dropping the collection
         modulestore = xmodule.modulestore.django.modulestore()
@@ -80,9 +161,9 @@ class ModuleStoreTestCase(TestCase):
         settings.MODULESTORE = cls.orig_modulestore
 
     def _pre_setup(self):
-        '''
-        Remove everything but the templates before each test
-        '''
+        """
+        Remove everything but the templates before each test.
+        """
 
         # Flush anything that is not a template
         ModuleStoreTestCase.flush_mongo_except_templates()
@@ -94,9 +175,9 @@ class ModuleStoreTestCase(TestCase):
         super(ModuleStoreTestCase, self)._pre_setup()
 
     def _post_teardown(self):
-        '''
-        Flush everything we created except the templates
-        '''
+        """
+        Flush everything we created except the templates.
+        """
         # Flush anything that is not a template
         ModuleStoreTestCase.flush_mongo_except_templates()
 

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

@@ -1,12 +1,13 @@
 from factory import Factory, lazy_attribute_sequence, lazy_attribute
 from uuid import uuid4
+import datetime
+
 from xmodule.modulestore import Location
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.inheritance import own_metadata
 from xmodule.x_module import ModuleSystem
 from mitxmako.shortcuts import render_to_string
 from xblock.runtime import InvalidScopeError
-import datetime
 from pytz import UTC
 
 
@@ -59,6 +60,10 @@ class XModuleCourseFactory(Factory):
         if data is not None:
             store.update_item(new_course.location, data)
 
+        # update_item updates the the course as it exists in the modulestore, but doesn't
+        # update the instance we are working with, so have to refetch the course after updating it.
+        new_course = store.get_instance(new_course.id, new_course.location)
+
         return new_course
 
 
@@ -147,6 +152,10 @@ class XModuleItemFactory(Factory):
         if new_item.location.category not in DETACHED_CATEGORIES:
             store.update_children(parent_location, parent.children + [new_item.location.url()])
 
+        # update_children updates the the item as it exists in the modulestore, but doesn't
+        # update the instance we are working with, so have to refetch the item after updating it.
+        new_item = store.get_item(new_item.location)
+
         return new_item
 
 
@@ -181,6 +190,7 @@ def get_test_xmodule_for_descriptor(descriptor):
     )
     return descriptor.xmodule(module_sys)
 
+
 def _test_xblock_model_data_accessor(descriptor):
     simple_map = {}
     for field in descriptor.fields:

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

@@ -13,11 +13,12 @@ from xmodule.templates import update_templates
 
 from .test_modulestore import check_path_to_location
 from . import DATA_DIR
+from uuid import uuid4
 
 
 HOST = 'localhost'
 PORT = 27017
-DB = 'test'
+DB = 'test_mongo_%s' % uuid4().hex
 COLLECTION = 'modulestore'
 FS_ROOT = DATA_DIR  # TODO (vshnayder): will need a real fs_root for testing load_item
 DEFAULT_CLASS = 'xmodule.raw_module.RawDescriptor'
@@ -39,7 +40,8 @@ class TestMongoModuleStore(object):
 
     @classmethod
     def teardownClass(cls):
-        pass
+        cls.connection = pymongo.connection.Connection(HOST, PORT)
+        cls.connection.drop_database(DB)
 
     @staticmethod
     def initdb():

common/lib/xmodule/xmodule/modulestore/xml_exporter.py

@@ -3,7 +3,24 @@ from xmodule.modulestore import Location
 from xmodule.modulestore.inheritance import own_metadata
 from fs.osfs import OSFS
 from json import dumps
+import json
+from json.encoder import JSONEncoder
+import datetime
 
+class EdxJSONEncoder(json.JSONEncoder):
+    def default(self, obj):
+        if isinstance(obj, Location):
+            return obj.url()
+        elif isinstance(obj, datetime.datetime):
+            if obj.tzinfo is not None:
+                if obj.utcoffset() is None:
+                    return obj.isoformat() + 'Z'
+                else:
+                    return obj.isoformat()
+            else:
+                return obj.isoformat()
+        else:
+            return super(EdxJSONEncoder, self).default(obj)
 
 def export_to_xml(modulestore, contentstore, course_location, root_dir, course_dir, draft_modulestore=None):
 
@@ -35,12 +52,12 @@ def export_to_xml(modulestore, contentstore, course_location, root_dir, course_d
     policies_dir = export_fs.makeopendir('policies')
     course_run_policy_dir = policies_dir.makeopendir(course.location.name)
     with course_run_policy_dir.open('grading_policy.json', 'w') as grading_policy:
-        grading_policy.write(dumps(course.grading_policy))
+        grading_policy.write(dumps(course.grading_policy, cls=EdxJSONEncoder))
 
     # export all of the course metadata in policy.json
     with course_run_policy_dir.open('policy.json', 'w') as course_policy:
         policy = {'course/' + course.location.name: own_metadata(course)}
-        course_policy.write(dumps(policy))
+        course_policy.write(dumps(policy, cls=EdxJSONEncoder))
 
     # export draft content
     # NOTE: this code assumes that verticals are the top most draftable container

common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py

@@ -500,10 +500,10 @@ class CombinedOpenEndedV1Module():
             pass
         return return_html
 
-    def get_rubric(self, get):
+    def get_rubric(self, _data):
         """
         Gets the results of a given grader via ajax.
-        Input: AJAX get dictionary
+        Input: AJAX data dictionary
         Output: Dictionary to be rendered via ajax that contains the result html.
         """
         all_responses = []
@@ -532,10 +532,10 @@ class CombinedOpenEndedV1Module():
         html = self.system.render_template('{0}/combined_open_ended_results.html'.format(self.TEMPLATE_DIR), context)
         return {'html': html, 'success': True}
 
-    def get_legend(self, get):
+    def get_legend(self, _data):
         """
         Gets the results of a given grader via ajax.
-        Input: AJAX get dictionary
+        Input: AJAX data dictionary
         Output: Dictionary to be rendered via ajax that contains the result html.
         """
         context = {
@@ -544,10 +544,10 @@ class CombinedOpenEndedV1Module():
         html = self.system.render_template('{0}/combined_open_ended_legend.html'.format(self.TEMPLATE_DIR), context)
         return {'html': html, 'success': True}
 
-    def get_results(self, get):
+    def get_results(self, _data):
         """
         Gets the results of a given grader via ajax.
-        Input: AJAX get dictionary
+        Input: AJAX data dictionary
         Output: Dictionary to be rendered via ajax that contains the result html.
         """
         self.update_task_states()
@@ -588,19 +588,19 @@ class CombinedOpenEndedV1Module():
         html = self.system.render_template('{0}/combined_open_ended_results.html'.format(self.TEMPLATE_DIR), context)
         return {'html': html, 'success': True}
 
-    def get_status_ajax(self, get):
+    def get_status_ajax(self, _data):
         """
         Gets the results of a given grader via ajax.
-        Input: AJAX get dictionary
+        Input: AJAX data dictionary
         Output: Dictionary to be rendered via ajax that contains the result html.
         """
         html = self.get_status(True)
         return {'html': html, 'success': True}
 
-    def handle_ajax(self, dispatch, get):
+    def handle_ajax(self, dispatch, data):
         """
         This is called by courseware.module_render, to handle an AJAX call.
-        "get" is request.POST.
+        "data" is request.POST.
 
         Returns a json dictionary:
         { 'progress_changed' : True/False,
@@ -618,35 +618,35 @@ class CombinedOpenEndedV1Module():
         }
 
         if dispatch not in handlers:
-            return_html = self.current_task.handle_ajax(dispatch, get, self.system)
+            return_html = self.current_task.handle_ajax(dispatch, data, self.system)
             return self.update_task_states_ajax(return_html)
 
-        d = handlers[dispatch](get)
+        d = handlers[dispatch](data)
         return json.dumps(d, cls=ComplexEncoder)
 
-    def next_problem(self, get):
+    def next_problem(self, _data):
         """
         Called via ajax to advance to the next problem.
-        Input: AJAX get request.
+        Input: AJAX data request.
         Output: Dictionary to be rendered
         """
         self.update_task_states()
         return {'success': True, 'html': self.get_html_nonsystem(), 'allow_reset': self.ready_to_reset}
 
-    def reset(self, get):
+    def reset(self, data):
         """
         If resetting is allowed, reset the state of the combined open ended module.
-        Input: AJAX get dictionary
+        Input: AJAX data dictionary
         Output: AJAX dictionary to tbe rendered
         """
         if self.state != self.DONE:
             if not self.ready_to_reset:
-                return self.out_of_sync_error(get)
+                return self.out_of_sync_error(data)
 
         if self.student_attempts > self.attempts:
             return {
                 'success': False,
-                #This is a student_facing_error
+                # This is a student_facing_error
                 'error': (
                     'You have attempted this question {0} times.  '
                     'You are only allowed to attempt it {1} times.'
@@ -789,13 +789,13 @@ class CombinedOpenEndedV1Module():
 
         return progress_object
 
-    def out_of_sync_error(self, get, msg=''):
+    def out_of_sync_error(self, data, msg=''):
         """
         return dict out-of-sync error message, and also log.
         """
         #This is a dev_facing_error
-        log.warning("Combined module state out sync. state: %r, get: %r. %s",
-                    self.state, get, msg)
+        log.warning("Combined module state out sync. state: %r, data: %r. %s",
+                    self.state, data, msg)
         #This is a student_facing_error
         return {'success': False,
                 'error': 'The problem state got out-of-sync.  Please try reloading the page.'}

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

@@ -122,17 +122,17 @@ class OpenEndedModule(openendedchild.OpenEndedChild):
 
         self.payload = {'grader_payload': updated_grader_payload}
 
-    def skip_post_assessment(self, get, system):
+    def skip_post_assessment(self, _data, system):
         """
         Ajax function that allows one to skip the post assessment phase
-        @param get: AJAX dictionary
+        @param data: AJAX dictionary
         @param system: ModuleSystem
         @return: Success indicator
         """
         self.child_state = self.DONE
         return {'success': True}
 
-    def message_post(self, get, system):
+    def message_post(self, data, system):
         """
         Handles a student message post (a reaction to the grade they received from an open ended grader type)
         Returns a boolean success/fail and an error message
@@ -141,7 +141,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild):
         event_info = dict()
         event_info['problem_id'] = self.location_string
         event_info['student_id'] = system.anonymous_student_id
-        event_info['survey_responses'] = get
+        event_info['survey_responses'] = data
 
         survey_responses = event_info['survey_responses']
         for tag in ['feedback', 'submission_id', 'grader_id', 'score']:
@@ -587,10 +587,10 @@ class OpenEndedModule(openendedchild.OpenEndedChild):
         html = system.render_template('{0}/open_ended_evaluation.html'.format(self.TEMPLATE_DIR), context)
         return html
 
-    def handle_ajax(self, dispatch, get, system):
+    def handle_ajax(self, dispatch, data, system):
         '''
         This is called by courseware.module_render, to handle an AJAX call.
-        "get" is request.POST.
+        "data" is request.POST.
 
         Returns a json dictionary:
         { 'progress_changed' : True/False,
@@ -612,7 +612,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild):
             return json.dumps({'error': 'Error handling action.  Please try again.', 'success': False})
 
         before = self.get_progress()
-        d = handlers[dispatch](get, system)
+        d = handlers[dispatch](data, system)
         after = self.get_progress()
         d.update({
             'progress_changed': after != before,
@@ -620,20 +620,20 @@ class OpenEndedModule(openendedchild.OpenEndedChild):
         })
         return json.dumps(d, cls=ComplexEncoder)
 
-    def check_for_score(self, get, system):
+    def check_for_score(self, _data, system):
         """
         Checks to see if a score has been received yet.
-        @param get: AJAX get dictionary
+        @param data: AJAX dictionary
         @param system: Modulesystem (needed to align with other ajax functions)
         @return: Returns the current state
         """
         state = self.child_state
         return {'state': state}
 
-    def save_answer(self, get, system):
+    def save_answer(self, data, system):
         """
         Saves a student answer
-        @param get: AJAX get dictionary
+        @param data: AJAX dictionary
         @param system: modulesystem
         @return: Success indicator
         """
@@ -644,17 +644,17 @@ class OpenEndedModule(openendedchild.OpenEndedChild):
             return msg
 
         if self.child_state != self.INITIAL:
-            return self.out_of_sync_error(get)
+            return self.out_of_sync_error(data)
 
         # add new history element with answer and empty score and hint.
-        success, get = self.append_image_to_student_answer(get)
+        success, data = self.append_image_to_student_answer(data)
         error_message = ""
         if success:
             success, allowed_to_submit, error_message = self.check_if_student_can_submit()
             if allowed_to_submit:
-                get['student_answer'] = OpenEndedModule.sanitize_html(get['student_answer'])
-                self.new_history_entry(get['student_answer'])
-                self.send_to_grader(get['student_answer'], system)
+                data['student_answer'] = OpenEndedModule.sanitize_html(data['student_answer'])
+                self.new_history_entry(data['student_answer'])
+                self.send_to_grader(data['student_answer'], system)
                 self.change_state(self.ASSESSING)
             else:
                 # Error message already defined
@@ -666,17 +666,17 @@ class OpenEndedModule(openendedchild.OpenEndedChild):
         return {
             'success': success,
             'error': error_message,
-            'student_response': get['student_answer']
+            'student_response': data['student_answer']
         }
 
-    def update_score(self, get, system):
+    def update_score(self, data, system):
         """
         Updates the current score via ajax.  Called by xqueue.
-        Input: AJAX get dictionary, modulesystem
+        Input: AJAX data dictionary, modulesystem
         Output: None
         """
-        queuekey = get['queuekey']
-        score_msg = get['xqueue_body']
+        queuekey = data['queuekey']
+        score_msg = data['xqueue_body']
         # TODO: Remove need for cmap
         self._update_score(score_msg, queuekey, system)
 

common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py

@@ -272,13 +272,13 @@ class OpenEndedChild(object):
                 return None
         return None
 
-    def out_of_sync_error(self, get, msg=''):
+    def out_of_sync_error(self, data, msg=''):
         """
         return dict out-of-sync error message, and also log.
         """
         # This is a dev_facing_error
-        log.warning("Open ended child state out sync. state: %r, get: %r. %s",
-                    self.child_state, get, msg)
+        log.warning("Open ended child state out sync. state: %r, data: %r. %s",
+                    self.child_state, data, msg)
         # This is a student_facing_error
         return {'success': False,
                 'error': 'The problem state got out-of-sync.  Please try reloading the page.'}
@@ -345,24 +345,24 @@ class OpenEndedChild(object):
 
         return success, image_ok, s3_public_url
 
-    def check_for_image_and_upload(self, get_data):
+    def check_for_image_and_upload(self, data):
         """
         Checks to see if an image was passed back in the AJAX query.  If so, it will upload it to S3
-        @param get_data: AJAX get data
-        @return: Success, whether or not a file was in the get dictionary,
+        @param data: AJAX data
+        @return: Success, whether or not a file was in the data dictionary,
         and the html corresponding to the uploaded image
         """
         has_file_to_upload = False
         uploaded_to_s3 = False
         image_tag = ""
         image_ok = False
-        if 'can_upload_files' in get_data:
-            if get_data['can_upload_files'] in ['true', '1']:
+        if 'can_upload_files' in data:
+            if data['can_upload_files'] in ['true', '1']:
                 has_file_to_upload = True
-                file = get_data['student_file'][0]
-                uploaded_to_s3, image_ok, s3_public_url = self.upload_image_to_s3(file)
+                student_file = data['student_file'][0]
+                uploaded_to_s3, image_ok, s3_public_url = self.upload_image_to_s3(student_file)
                 if uploaded_to_s3:
-                    image_tag = self.generate_image_tag_from_url(s3_public_url, file.name)
+                    image_tag = self.generate_image_tag_from_url(s3_public_url, student_file.name)
 
         return has_file_to_upload, uploaded_to_s3, image_ok, image_tag
 
@@ -371,27 +371,27 @@ class OpenEndedChild(object):
         Makes an image tag from a given URL
         @param s3_public_url: URL of the image
         @param image_name: Name of the image
-        @return: Boolean success, updated AJAX get data
+        @return: Boolean success, updated AJAX data
         """
         image_template = """
                         <a href="{0}" target="_blank">{1}</a>
                          """.format(s3_public_url, image_name)
         return image_template
 
-    def append_image_to_student_answer(self, get_data):
+    def append_image_to_student_answer(self, data):
         """
         Adds an image to a student answer after uploading it to S3
-        @param get_data: AJAx get data
-        @return: Boolean success, updated AJAX get data
+        @param data: AJAx data
+        @return: Boolean success, updated AJAX data
         """
         overall_success = False
         if not self.accept_file_upload:
             # If the question does not accept file uploads, do not do anything
-            return True, get_data
+            return True, data
 
-        has_file_to_upload, uploaded_to_s3, image_ok, image_tag = self.check_for_image_and_upload(get_data)
+        has_file_to_upload, uploaded_to_s3, image_ok, image_tag = self.check_for_image_and_upload(data)
         if uploaded_to_s3 and has_file_to_upload and image_ok:
-            get_data['student_answer'] += image_tag
+            data['student_answer'] += image_tag
             overall_success = True
         elif has_file_to_upload and not uploaded_to_s3 and image_ok:
             # In this case, an image was submitted by the student, but the image could not be uploaded to S3.  Likely
@@ -403,12 +403,12 @@ class OpenEndedChild(object):
             overall_success = True
         elif not has_file_to_upload:
             # If there is no file to upload, probably the student has embedded the link in the answer text
-            success, get_data['student_answer'] = self.check_for_url_in_text(get_data['student_answer'])
+            success, data['student_answer'] = self.check_for_url_in_text(data['student_answer'])
             overall_success = success
 
         # log.debug("Has file: {0} Uploaded: {1} Image Ok: {2}".format(has_file_to_upload, uploaded_to_s3, image_ok))
 
-        return overall_success, get_data
+        return overall_success, data
 
     def check_for_url_in_text(self, string):
         """

common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py

@@ -75,10 +75,10 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild):
         html = system.render_template('{0}/self_assessment_prompt.html'.format(self.TEMPLATE_DIR), context)
         return html
 
-    def handle_ajax(self, dispatch, get, system):
+    def handle_ajax(self, dispatch, data, system):
         """
         This is called by courseware.module_render, to handle an AJAX call.
-        "get" is request.POST.
+        "data" is request.POST.
 
         Returns a json dictionary:
         { 'progress_changed' : True/False,
@@ -99,7 +99,7 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild):
             return json.dumps({'error': 'Error handling action.  Please try again.', 'success': False})
 
         before = self.get_progress()
-        d = handlers[dispatch](get, system)
+        d = handlers[dispatch](data, system)
         after = self.get_progress()
         d.update({
             'progress_changed': after != before,
@@ -160,12 +160,12 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild):
 
         return system.render_template('{0}/self_assessment_hint.html'.format(self.TEMPLATE_DIR), context)
 
-    def save_answer(self, get, system):
+    def save_answer(self, data, system):
         """
         After the answer is submitted, show the rubric.
 
         Args:
-            get: the GET dictionary passed to the ajax request.  Should contain
+            data: the request dictionary passed to the ajax request.  Should contain
                 a key 'student_answer'
 
         Returns:
@@ -178,16 +178,16 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild):
             return msg
 
         if self.child_state != self.INITIAL:
-            return self.out_of_sync_error(get)
+            return self.out_of_sync_error(data)
 
         error_message = ""
         # add new history element with answer and empty score and hint.
-        success, get = self.append_image_to_student_answer(get)
+        success, data = self.append_image_to_student_answer(data)
         if success:
             success, allowed_to_submit, error_message = self.check_if_student_can_submit()
             if allowed_to_submit:
-                get['student_answer'] = SelfAssessmentModule.sanitize_html(get['student_answer'])
-                self.new_history_entry(get['student_answer'])
+                data['student_answer'] = SelfAssessmentModule.sanitize_html(data['student_answer'])
+                self.new_history_entry(data['student_answer'])
                 self.change_state(self.ASSESSING)
             else:
                 # Error message already defined
@@ -200,10 +200,10 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild):
             'success': success,
             'rubric_html': self.get_rubric_html(system),
             'error': error_message,
-            'student_response': get['student_answer'],
+            'student_response': data['student_answer'],
         }
 
-    def save_assessment(self, get, system):
+    def save_assessment(self, data, _system):
         """
         Save the assessment.  If the student said they're right, don't ask for a
         hint, and go straight to the done state.  Otherwise, do ask for a hint.
@@ -219,11 +219,11 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild):
         """
 
         if self.child_state != self.ASSESSING:
-            return self.out_of_sync_error(get)
+            return self.out_of_sync_error(data)
 
         try:
-            score = int(get['assessment'])
-            score_list = get.getlist('score_list[]')
+            score = int(data['assessment'])
+            score_list = data.getlist('score_list[]')
             for i in xrange(0, len(score_list)):
                 score_list[i] = int(score_list[i])
         except ValueError:
@@ -244,7 +244,7 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild):
         d['state'] = self.child_state
         return d
 
-    def save_hint(self, get, system):
+    def save_hint(self, data, _system):
         '''
         Not used currently, as hints have been removed from the system.
         Save the hint.
@@ -258,9 +258,9 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild):
         if self.child_state != self.POST_ASSESSMENT:
             # Note: because we only ask for hints on wrong answers, may not have
             # the same number of hints and answers.
-            return self.out_of_sync_error(get)
+            return self.out_of_sync_error(data)
 
-        self.record_latest_post_assessment(get['hint'])
+        self.record_latest_post_assessment(data['hint'])
         self.change_state(self.DONE)
 
         return {'success': True,

common/lib/xmodule/xmodule/peer_grading_module.py

@@ -133,8 +133,8 @@ class PeerGradingModule(PeerGradingFields, XModule):
         """
         return {'success': False, 'error': msg}
 
-    def _check_required(self, get, required):
-        actual = set(get.keys())
+    def _check_required(self, data, required):
+        actual = set(data.keys())
         missing = required - actual
         if len(missing) > 0:
             return False, "Missing required keys: {0}".format(', '.join(missing))
@@ -153,7 +153,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
         else:
             return self.peer_grading_problem({'location': self.link_to_location})['html']
 
-    def handle_ajax(self, dispatch, get):
+    def handle_ajax(self, dispatch, data):
         """
         Needs to be implemented by child modules.  Handles AJAX events.
         @return:
@@ -173,7 +173,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
             # This is a dev_facing_error
             return json.dumps({'error': 'Error handling action.  Please try again.', 'success': False})
 
-        d = handlers[dispatch](get)
+        d = handlers[dispatch](data)
 
         return json.dumps(d, cls=ComplexEncoder)
 
@@ -244,7 +244,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
             max_grade = self.max_grade
         return max_grade
 
-    def get_next_submission(self, get):
+    def get_next_submission(self, data):
         """
         Makes a call to the grading controller for the next essay that should be graded
         Returns a json dict with the following keys:
@@ -263,11 +263,11 @@ class PeerGradingModule(PeerGradingFields, XModule):
         'error': if success is False, will have an error message with more info.
         """
         required = set(['location'])
-        success, message = self._check_required(get, required)
+        success, message = self._check_required(data, required)
         if not success:
             return self._err_response(message)
         grader_id = self.system.anonymous_student_id
-        location = get['location']
+        location = data['location']
 
         try:
             response = self.peer_gs.get_next_submission(location, grader_id)
@@ -280,7 +280,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
             return {'success': False,
                     'error': EXTERNAL_GRADER_NO_CONTACT_ERROR}
 
-    def save_grade(self, get):
+    def save_grade(self, data):
         """
         Saves the grade of a given submission.
         Input:
@@ -298,18 +298,18 @@ class PeerGradingModule(PeerGradingFields, XModule):
 
         required = set(['location', 'submission_id', 'submission_key', 'score', 'feedback', 'rubric_scores[]',
                         'submission_flagged'])
-        success, message = self._check_required(get, required)
+        success, message = self._check_required(data, required)
         if not success:
             return self._err_response(message)
         grader_id = self.system.anonymous_student_id
 
-        location = get.get('location')
-        submission_id = get.get('submission_id')
-        score = get.get('score')
-        feedback = get.get('feedback')
-        submission_key = get.get('submission_key')
-        rubric_scores = get.getlist('rubric_scores[]')
-        submission_flagged = get.get('submission_flagged')
+        location = data.get('location')
+        submission_id = data.get('submission_id')
+        score = data.get('score')
+        feedback = data.get('feedback')
+        submission_key = data.get('submission_key')
+        rubric_scores = data.getlist('rubric_scores[]')
+        submission_flagged = data.get('submission_flagged')
 
         try:
             response = self.peer_gs.save_grade(location, grader_id, submission_id,
@@ -328,7 +328,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
                 'error': EXTERNAL_GRADER_NO_CONTACT_ERROR
             }
 
-    def is_student_calibrated(self, get):
+    def is_student_calibrated(self, data):
         """
         Calls the grading controller to see if the given student is calibrated
         on the given problem
@@ -347,12 +347,12 @@ class PeerGradingModule(PeerGradingFields, XModule):
         """
 
         required = set(['location'])
-        success, message = self._check_required(get, required)
+        success, message = self._check_required(data, required)
         if not success:
             return self._err_response(message)
         grader_id = self.system.anonymous_student_id
 
-        location = get['location']
+        location = data['location']
 
         try:
             response = self.peer_gs.is_student_calibrated(location, grader_id)
@@ -367,7 +367,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
                 'error': EXTERNAL_GRADER_NO_CONTACT_ERROR
             }
 
-    def show_calibration_essay(self, get):
+    def show_calibration_essay(self, data):
         """
         Fetch the next calibration essay from the grading controller and return it
         Inputs:
@@ -392,13 +392,13 @@ class PeerGradingModule(PeerGradingFields, XModule):
         """
 
         required = set(['location'])
-        success, message = self._check_required(get, required)
+        success, message = self._check_required(data, required)
         if not success:
             return self._err_response(message)
 
         grader_id = self.system.anonymous_student_id
 
-        location = get['location']
+        location = data['location']
         try:
             response = self.peer_gs.show_calibration_essay(location, grader_id)
             return response
@@ -417,8 +417,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
             return {'success': False,
                     'error': 'Error displaying submission.  Please notify course staff.'}
 
-
-    def save_calibration_essay(self, get):
+    def save_calibration_essay(self, data):
         """
         Saves the grader's grade of a given calibration.
         Input:
@@ -437,17 +436,17 @@ class PeerGradingModule(PeerGradingFields, XModule):
         """
 
         required = set(['location', 'submission_id', 'submission_key', 'score', 'feedback', 'rubric_scores[]'])
-        success, message = self._check_required(get, required)
+        success, message = self._check_required(data, required)
         if not success:
             return self._err_response(message)
         grader_id = self.system.anonymous_student_id
 
-        location = get.get('location')
-        calibration_essay_id = get.get('submission_id')
-        submission_key = get.get('submission_key')
-        score = get.get('score')
-        feedback = get.get('feedback')
-        rubric_scores = get.getlist('rubric_scores[]')
+        location = data.get('location')
+        calibration_essay_id = data.get('submission_id')
+        submission_key = data.get('submission_key')
+        score = data.get('score')
+        feedback = data.get('feedback')
+        rubric_scores = data.getlist('rubric_scores[]')
 
         try:
             response = self.peer_gs.save_calibration_essay(location, grader_id, calibration_essay_id,
@@ -473,8 +472,7 @@ class PeerGradingModule(PeerGradingFields, XModule):
         })
         return html
 
-
-    def peer_grading(self, get=None):
+    def peer_grading(self, _data=None):
         '''
         Show a peer grading interface
         '''
@@ -553,11 +551,11 @@ class PeerGradingModule(PeerGradingFields, XModule):
 
         return html
 
-    def peer_grading_problem(self, get=None):
+    def peer_grading_problem(self, data=None):
         '''
         Show individual problem interface
         '''
-        if get is None or get.get('location') is None:
+        if data is None or data.get('location') is None:
             if not self.use_for_single_location:
                 # This is an error case, because it must be set to use a single location to be called without get parameters
                 # This is a dev_facing_error
@@ -566,8 +564,8 @@ class PeerGradingModule(PeerGradingFields, XModule):
                 return {'html': "", 'success': False}
             problem_location = self.link_to_location
 
-        elif get.get('location') is not None:
-            problem_location = get.get('location')
+        elif data.get('location') is not None:
+            problem_location = data.get('location')
 
         ajax_url = self.ajax_url
         html = self.system.render_template('peer_grading/peer_grading_problem.html', {
@@ -617,4 +615,3 @@ class PeerGradingDescriptor(PeerGradingFields, RawDescriptor):
         non_editable_fields.extend([PeerGradingFields.due_date, PeerGradingFields.grace_period_string,
                                     PeerGradingFields.max_grade])
         return non_editable_fields
-

common/lib/xmodule/xmodule/poll_module.py

@@ -47,12 +47,12 @@ class PollModule(PollFields, XModule):
     css = {'scss': [resource_string(__name__, 'css/poll/display.scss')]}
     js_module_name = "Poll"
 
-    def handle_ajax(self, dispatch, get):
+    def handle_ajax(self, dispatch, data):
         """Ajax handler.
 
         Args:
             dispatch: string request slug
-            get: dict request get parameters
+            data: dict request data parameters
 
         Returns:
             json string

common/lib/xmodule/xmodule/seq_module.py

@@ -59,13 +59,13 @@ class SequenceModule(SequenceFields, XModule):
         # TODO: Cache progress or children array?
         children = self.get_children()
         progresses = [child.get_progress() for child in children]
-        progress = reduce(Progress.add_counts, progresses)
+        progress = reduce(Progress.add_counts, progresses, None)
         return progress
 
-    def handle_ajax(self, dispatch, get):  # TODO: bounds checking
+    def handle_ajax(self, dispatch, data):  # TODO: bounds checking
         ''' get = request.POST instance '''
         if dispatch == 'goto_position':
-            self.position = int(get['position'])
+            self.position = int(data['position'])
             return json.dumps({'success': True})
         raise NotFoundError('Unexpected dispatch type')
 

common/lib/xmodule/xmodule/template_module.py

@@ -49,7 +49,7 @@ class CustomTagDescriptor(RawDescriptor):
             else:
                 # TODO (vshnayder): better exception type
                 raise Exception("Could not find impl attribute in customtag {0}"
-                                .format(location))
+                                .format(self.location))
 
         params = dict(xmltree.items())
 

common/lib/xmodule/xmodule/templates/problem/customgrader.yaml

@@ -13,15 +13,16 @@ data: |
 
   <script type="loncapa/python">
 
-  def test_add_to_ten(expect,ans):
-    a1=float(ans[0])
-    a2=float(ans[1])
-    return (a1+a2)==10
+  def test_add(expect, ans):
+      try:
+          a1=int(ans[0])
+          a2=int(ans[1])
+          return (a1+a2) == int(expect)
+      except ValueError:
+          return False
 
-  def test_add(expect,ans):
-    a1=float(ans[0])
-    a2=float(ans[1])
-    return (a1+a2)== float(expect)
+  def test_add_to_ten(expect, ans):
+      return test_add(10, ans)
 
   </script>
 
@@ -40,7 +41,7 @@ data: |
       <solution>
           <div class="detailed-solution">
               <p>Explanation</p>
-              <p>Any set of values on the line \(y = 10 - x\) and \(y = 20 - x\) satisfy these constraints.</p>
+              <p>Any set of integers on the line \(y = 10 - x\) and \(y = 20 - x\) satisfy these constraints.</p>
               <img src="/static/images/simple_graph.png"/>
           </div>
       </solution>

common/lib/xmodule/xmodule/tests/test_capa_module.py

@@ -1,4 +1,7 @@
-"""Tests of the Capa XModule"""
+# -*- coding: utf-8 -*-
+"""
+Tests of the Capa XModule
+"""
 #pylint: disable=C0111
 #pylint: disable=R0904
 #pylint: disable=C0103
@@ -8,11 +11,12 @@ import datetime
 from mock import Mock, patch
 import unittest
 import random
+import json
 
 import xmodule
-from capa.responsetypes import StudentInputError, \
-    LoncapaProblemError, ResponseError
-from xmodule.capa_module import CapaModule
+from capa.responsetypes import (StudentInputError, LoncapaProblemError,
+                                ResponseError)
+from xmodule.capa_module import CapaModule, ComplexEncoder
 from xmodule.modulestore import Location
 
 from django.http import QueryDict
@@ -47,12 +51,16 @@ class CapaFactory(object):
 
     @staticmethod
     def input_key():
-        """ Return the input key to use when passing GET parameters """
+        """
+        Return the input key to use when passing GET parameters
+        """
         return ("input_" + CapaFactory.answer_key())
 
     @staticmethod
     def answer_key():
-        """ Return the key stored in the capa problem answer dict """
+        """
+        Return the key stored in the capa problem answer dict
+        """
         return ("-".join(['i4x', 'edX', 'capa_test', 'problem',
                          'SampleProblem%d' % CapaFactory.num]) +
                 "_2_1")
@@ -361,7 +369,9 @@ class CapaModuleTest(unittest.TestCase):
             result = CapaModule.make_dict_of_responses(invalid_get_dict)
 
     def _querydict_from_dict(self, param_dict):
-        """ Create a Django QueryDict from a Python dictionary """
+        """
+        Create a Django QueryDict from a Python dictionary
+        """
 
         # QueryDict objects are immutable by default, so we make
         # a copy that we can update.
@@ -496,9 +506,10 @@ class CapaModuleTest(unittest.TestCase):
     def test_check_problem_error(self):
 
         # Try each exception that capa_module should handle
-        for exception_class in [StudentInputError,
-                                LoncapaProblemError,
-                                ResponseError]:
+        exception_classes = [StudentInputError,
+                             LoncapaProblemError,
+                             ResponseError]
+        for exception_class in exception_classes:
 
             # Create the module
             module = CapaFactory.create(attempts=1)
@@ -520,6 +531,60 @@ class CapaModuleTest(unittest.TestCase):
             # Expect that the number of attempts is NOT incremented
             self.assertEqual(module.attempts, 1)
 
+    def test_check_problem_other_errors(self):
+        """
+        Test that errors other than the expected kinds give an appropriate message.
+
+        See also `test_check_problem_error` for the "expected kinds" or errors.
+        """
+        # Create the module
+        module = CapaFactory.create(attempts=1)
+
+        # Ensure that the user is NOT staff
+        module.system.user_is_staff = False
+
+        # Ensure that DEBUG is on
+        module.system.DEBUG = True
+
+        # Simulate answering a problem that raises the exception
+        with patch('capa.capa_problem.LoncapaProblem.grade_answers') as mock_grade:
+            error_msg = u"Superterrible error happened: ☠"
+            mock_grade.side_effect = Exception(error_msg)
+
+            get_request_dict = {CapaFactory.input_key(): '3.14'}
+            result = module.check_problem(get_request_dict)
+
+        # Expect an AJAX alert message in 'success'
+        self.assertTrue(error_msg in result['success'])
+
+    def test_check_problem_error_nonascii(self):
+
+        # Try each exception that capa_module should handle
+        exception_classes = [StudentInputError,
+                             LoncapaProblemError,
+                             ResponseError]
+        for exception_class in exception_classes:
+
+            # Create the module
+            module = CapaFactory.create(attempts=1)
+
+            # Ensure that the user is NOT staff
+            module.system.user_is_staff = False
+
+            # Simulate answering a problem that raises the exception
+            with patch('capa.capa_problem.LoncapaProblem.grade_answers') as mock_grade:
+                mock_grade.side_effect = exception_class(u"ȧƈƈḗƞŧḗḓ ŧḗẋŧ ƒǿř ŧḗşŧīƞɠ")
+
+                get_request_dict = {CapaFactory.input_key(): '3.14'}
+                result = module.check_problem(get_request_dict)
+
+            # Expect an AJAX alert message in 'success'
+            expected_msg = u'Error: ȧƈƈḗƞŧḗḓ ŧḗẋŧ ƒǿř ŧḗşŧīƞɠ'
+            self.assertEqual(expected_msg, result['success'])
+
+            # Expect that the number of attempts is NOT incremented
+            self.assertEqual(module.attempts, 1)
+
     def test_check_problem_error_with_staff_user(self):
 
         # Try each exception that capa module should handle
@@ -1021,6 +1086,33 @@ class CapaModuleTest(unittest.TestCase):
         # Expect that the module has created a new dummy problem with the error
         self.assertNotEqual(original_problem, module.lcp)
 
+    def test_get_problem_html_error_w_debug(self):
+        """
+        Test the html response when an error occurs with DEBUG on
+        """
+        module = CapaFactory.create()
+
+        # Simulate throwing an exception when the capa problem
+        # is asked to render itself as HTML
+        error_msg = u"Superterrible error happened: ☠"
+        module.lcp.get_html = Mock(side_effect=Exception(error_msg))
+
+        # Stub out the get_test_system rendering function
+        module.system.render_template = Mock(return_value="<div>Test Template HTML</div>")
+
+        # Make sure DEBUG is on
+        module.system.DEBUG = True
+
+        # Try to render the module with DEBUG turned on
+        html = module.get_problem_html()
+
+        self.assertTrue(html is not None)
+
+        # Check the rendering context
+        render_args, _ = module.system.render_template.call_args
+        context = render_args[1]
+        self.assertTrue(error_msg in context['problem']['html'])
+
     def test_random_seed_no_change(self):
 
         # Run the test for each possible rerandomize value
@@ -1126,3 +1218,28 @@ class CapaModuleTest(unittest.TestCase):
             for i in range(200):
                 module = CapaFactory.create(rerandomize=rerandomize)
                 assert 0 <= module.seed < 1000
+
+    @patch('xmodule.capa_module.log')
+    @patch('xmodule.capa_module.Progress')
+    def test_get_progress_error(self, mock_progress, mock_log):
+        """
+        Check that an exception given in `Progress` produces a `log.exception` call.
+        """
+        error_types = [TypeError, ValueError]
+        for error_type in error_types:
+            mock_progress.side_effect = error_type
+            module = CapaFactory.create()
+            self.assertIsNone(module.get_progress())
+            mock_log.exception.assert_called_once_with('Got bad progress')
+            mock_log.reset_mock()
+
+
+class ComplexEncoderTest(unittest.TestCase):
+    def test_default(self):
+        """
+        Check that complex numbers can be encoded into JSON.
+        """
+        complex_num = 1 - 1j
+        expected_str = '1-1*j'
+        json_str = json.dumps(complex_num, cls=ComplexEncoder)
+        self.assertEqual(expected_str, json_str[1:-1])  # ignore quotes

common/lib/xmodule/xmodule/tests/test_crowdsource_hinter.py

@@ -0,0 +1,439 @@
+"""
+Tests the crowdsourced hinter xmodule.
+"""
+
+from mock import Mock
+import unittest
+import copy
+
+from xmodule.crowdsource_hinter import CrowdsourceHinterModule
+from xmodule.vertical_module import VerticalModule, VerticalDescriptor
+
+from . import get_test_system
+
+import json
+
+
+class CHModuleFactory(object):
+    """
+    Helps us make a CrowdsourceHinterModule with the specified internal
+    state.
+    """
+
+    sample_problem_xml = """
+    <?xml version="1.0"?>
+    <crowdsource_hinter>
+        <problem display_name="Numerical Input" markdown="A numerical input problem accepts a line of text input from the student, and evaluates the input for correctness based on its numerical value.&#10;&#10;The answer is correct if it is within a specified numerical tolerance of the expected answer.&#10;&#10;Enter the number of fingers on a human hand:&#10;= 5&#10;&#10;[explanation]&#10;If you look at your hand, you can count that you have five fingers. [explanation] " rerandomize="never" showanswer="finished">
+          <p>A numerical input problem accepts a line of text input from the student, and evaluates the input for correctness based on its numerical value.</p>
+          <p>The answer is correct if it is within a specified numerical tolerance of the expected answer.</p>
+          <p>Enter the number of fingers on a human hand:</p>
+          <numericalresponse answer="5">
+            <textline/>
+          </numericalresponse>
+          <solution>
+            <div class="detailed-solution">
+              <p>Explanation</p>
+              <p>If you look at your hand, you can count that you have five fingers. </p>
+            </div>
+          </solution>
+        </problem>
+    </crowdsource_hinter>
+    """
+
+    num = 0
+
+    @staticmethod
+    def next_num():
+        """
+        Helps make unique names for our mock CrowdsourceHinterModule's
+        """
+        CHModuleFactory.num += 1
+        return CHModuleFactory.num
+
+    @staticmethod
+    def create(hints=None,
+               previous_answers=None,
+               user_voted=None,
+               moderate=None,
+               mod_queue=None):
+        """
+        A factory method for making CHM's
+        """
+        model_data = {'data': CHModuleFactory.sample_problem_xml}
+
+        if hints is not None:
+            model_data['hints'] = hints
+        else:
+            model_data['hints'] = {
+                '24.0': {'0': ['Best hint', 40],
+                         '3': ['Another hint', 30],
+                         '4': ['A third hint', 20],
+                         '6': ['A less popular hint', 3]},
+                '25.0': {'1': ['Really popular hint', 100]}
+            }
+
+        if mod_queue is not None:
+            model_data['mod_queue'] = mod_queue
+        else:
+            model_data['mod_queue'] = {
+                '24.0': {'2': ['A non-approved hint']},
+                '26.0': {'5': ['Another non-approved hint']}
+            }
+
+        if previous_answers is not None:
+            model_data['previous_answers'] = previous_answers
+        else:
+            model_data['previous_answers'] = [
+                ['24.0', [0, 3, 4]],
+                ['29.0', [None, None, None]]
+            ]
+
+        if user_voted is not None:
+            model_data['user_voted'] = user_voted
+
+        if moderate is not None:
+            model_data['moderate'] = moderate
+
+        descriptor = Mock(weight="1")
+        system = get_test_system()
+        module = CrowdsourceHinterModule(system, descriptor, model_data)
+
+        return module
+
+
+class VerticalWithModulesFactory(object):
+    """
+    Makes a vertical with several crowdsourced hinter modules inside.
+    Used to make sure that several crowdsourced hinter modules can co-exist
+    on one vertical.
+    """
+
+    sample_problem_xml = """<?xml version="1.0"?>
+    <vertical display_name="Test vertical">
+        <crowdsource_hinter>
+            <problem display_name="Numerical Input" markdown=" " rerandomize="never" showanswer="finished">
+              <p>Test numerical problem.</p>
+              <numericalresponse answer="5">
+                <textline/>
+              </numericalresponse>
+              <solution>
+                <div class="detailed-solution">
+                  <p>Explanation</p>
+                  <p>If you look at your hand, you can count that you have five fingers. </p>
+                </div>
+              </solution>
+            </problem>
+        </crowdsource_hinter>
+
+        <crowdsource_hinter>
+            <problem display_name="Numerical Input" markdown=" " rerandomize="never" showanswer="finished">
+              <p>Another test numerical problem.</p>
+              <numericalresponse answer="5">
+                <textline/>
+              </numericalresponse>
+              <solution>
+                <div class="detailed-solution">
+                  <p>Explanation</p>
+                  <p>If you look at your hand, you can count that you have five fingers. </p>
+                </div>
+              </solution>
+            </problem>
+        </crowdsource_hinter>
+    </vertical>
+    """
+
+    num = 0
+
+    @staticmethod
+    def next_num():
+        CHModuleFactory.num += 1
+        return CHModuleFactory.num
+
+    @staticmethod
+    def create():
+        model_data = {'data': VerticalWithModulesFactory.sample_problem_xml}
+        system = get_test_system()
+        descriptor = VerticalDescriptor.from_xml(VerticalWithModulesFactory.sample_problem_xml, system)
+        module = VerticalModule(system, descriptor, model_data)
+
+        return module
+
+
+class FakeChild(object):
+    """
+    A fake Xmodule.
+    """
+    def __init__(self):
+        self.system = Mock()
+        self.system.ajax_url = 'this/is/a/fake/ajax/url'
+
+    def get_html(self):
+        """
+        Return a fake html string.
+        """
+        return 'This is supposed to be test html.'
+
+
+class CrowdsourceHinterTest(unittest.TestCase):
+    """
+    In the below tests, '24.0' represents a wrong answer, and '42.5' represents
+    a correct answer.
+    """
+
+    def test_gethtml(self):
+        """
+        A simple test of get_html - make sure it returns the html of the inner
+        problem.
+        """
+        m = CHModuleFactory.create()
+
+        def fake_get_display_items():
+            """
+            A mock of get_display_items
+            """
+            return [FakeChild()]
+        m.get_display_items = fake_get_display_items
+        out_html = m.get_html()
+        self.assertTrue('This is supposed to be test html.' in out_html)
+        self.assertTrue('this/is/a/fake/ajax/url' in out_html)
+
+    def test_gethtml_nochild(self):
+        """
+        get_html, except the module has no child :(  Should return a polite
+        error message.
+        """
+        m = CHModuleFactory.create()
+
+        def fake_get_display_items():
+            """
+            Returns no children.
+            """
+            return []
+        m.get_display_items = fake_get_display_items
+        out_html = m.get_html()
+        self.assertTrue('Error in loading crowdsourced hinter' in out_html)
+
+    @unittest.skip("Needs to be finished.")
+    def test_gethtml_multiple(self):
+        """
+        Makes sure that multiple crowdsourced hinters play nice, when get_html
+        is called.
+        NOT WORKING RIGHT NOW
+        """
+        m = VerticalWithModulesFactory.create()
+        out_html = m.get_html()
+        print out_html
+        self.assertTrue('Test numerical problem.' in out_html)
+        self.assertTrue('Another test numerical problem.' in out_html)
+
+    def test_gethint_0hint(self):
+        """
+        Someone asks for a hint, when there's no hint to give.
+        - Output should be blank.
+        - New entry should be added to previous_answers
+        """
+        m = CHModuleFactory.create()
+        json_in = {'problem_name': '26.0'}
+        out = m.get_hint(json_in)
+        self.assertTrue(out is None)
+        self.assertTrue(['26.0', [None, None, None]] in m.previous_answers)
+
+    def test_gethint_1hint(self):
+        """
+        Someone asks for a hint, with exactly one hint in the database.
+        Output should contain that hint.
+        """
+        m = CHModuleFactory.create()
+        json_in = {'problem_name': '25.0'}
+        out = m.get_hint(json_in)
+        self.assertTrue(out['best_hint'] == 'Really popular hint')
+
+    def test_gethint_manyhints(self):
+        """
+        Someone asks for a hint, with many matching hints in the database.
+        - The top-rated hint should be returned.
+        - Two other random hints should be returned.
+        Currently, the best hint could be returned twice - need to fix this
+        in implementation.
+        """
+        m = CHModuleFactory.create()
+        json_in = {'problem_name': '24.0'}
+        out = m.get_hint(json_in)
+        self.assertTrue(out['best_hint'] == 'Best hint')
+        self.assertTrue('rand_hint_1' in out)
+        self.assertTrue('rand_hint_2' in out)
+
+    def test_getfeedback_0wronganswers(self):
+        """
+        Someone has gotten the problem correct on the first try.
+        Output should be empty.
+        """
+        m = CHModuleFactory.create(previous_answers=[])
+        json_in = {'problem_name': '42.5'}
+        out = m.get_feedback(json_in)
+        self.assertTrue(out is None)
+
+    def test_getfeedback_1wronganswer_nohints(self):
+        """
+        Someone has gotten the problem correct, with one previous wrong
+        answer.  However, we don't actually have hints for this problem.
+        There should be a dialog to submit a new hint.
+        """
+        m = CHModuleFactory.create(previous_answers=[['26.0', [None, None, None]]])
+        json_in = {'problem_name': '42.5'}
+        out = m.get_feedback(json_in)
+        print out['index_to_answer']
+        self.assertTrue(out['index_to_hints'][0] == [])
+        self.assertTrue(out['index_to_answer'][0] == '26.0')
+
+    def test_getfeedback_1wronganswer_withhints(self):
+        """
+        Same as above, except the user did see hints.  There should be
+        a voting dialog, with the correct choices, plus a hint submission
+        dialog.
+        """
+        m = CHModuleFactory.create(previous_answers=[['24.0', [0, 3, None]]])
+        json_in = {'problem_name': '42.5'}
+        out = m.get_feedback(json_in)
+        print out['index_to_hints']
+        self.assertTrue(len(out['index_to_hints'][0]) == 2)
+
+    def test_getfeedback_missingkey(self):
+        """
+        Someone gets a problem correct, but one of the hints that he saw
+        earlier (pk=100) has been deleted.  Should just skip that hint.
+        """
+        m = CHModuleFactory.create(
+            previous_answers=[['24.0', [0, 100, None]]])
+        json_in = {'problem_name': '42.5'}
+        out = m.get_feedback(json_in)
+        self.assertTrue(len(out['index_to_hints'][0]) == 1)
+
+    def test_vote_nopermission(self):
+        """
+        A user tries to vote for a hint, but he has already voted!
+        Should not change any vote tallies.
+        """
+        m = CHModuleFactory.create(user_voted=True)
+        json_in = {'answer': 0, 'hint': 1}
+        old_hints = copy.deepcopy(m.hints)
+        m.tally_vote(json_in)
+        self.assertTrue(m.hints == old_hints)
+
+    def test_vote_withpermission(self):
+        """
+        A user votes for a hint.
+        Also tests vote result rendering.
+        """
+        m = CHModuleFactory.create(
+            previous_answers=[['24.0', [0, 3, None]]])
+        json_in = {'answer': 0, 'hint': 3}
+        dict_out = m.tally_vote(json_in)
+        self.assertTrue(m.hints['24.0']['0'][1] == 40)
+        self.assertTrue(m.hints['24.0']['3'][1] == 31)
+        self.assertTrue(['Best hint', 40] in dict_out['hint_and_votes'])
+        self.assertTrue(['Another hint', 31] in dict_out['hint_and_votes'])
+
+    def test_submithint_nopermission(self):
+        """
+        A user tries to submit a hint, but he has already voted.
+        """
+        m = CHModuleFactory.create(user_voted=True)
+        json_in = {'answer': 1, 'hint': 'This is a new hint.'}
+        print m.user_voted
+        m.submit_hint(json_in)
+        print m.hints
+        self.assertTrue('29.0' not in m.hints)
+
+    def test_submithint_withpermission_new(self):
+        """
+        A user submits a hint to an answer for which no hints
+        exist yet.
+        """
+        m = CHModuleFactory.create()
+        json_in = {'answer': 1, 'hint': 'This is a new hint.'}
+        m.submit_hint(json_in)
+        self.assertTrue('29.0' in m.hints)
+
+    def test_submithint_withpermission_existing(self):
+        """
+        A user submits a hint to an answer that has other hints
+        already.
+        """
+        m = CHModuleFactory.create(previous_answers=[['25.0', [1, None, None]]])
+        json_in = {'answer': 0, 'hint': 'This is a new hint.'}
+        m.submit_hint(json_in)
+        # Make a hint request.
+        json_in = {'problem name': '25.0'}
+        out = m.get_hint(json_in)
+        self.assertTrue((out['best_hint'] == 'This is a new hint.')
+                        or (out['rand_hint_1'] == 'This is a new hint.'))
+
+    def test_submithint_moderate(self):
+        """
+        A user submits a hint, but moderation is on.  The hint should
+        show up in the mod_queue, not the public-facing hints
+        dict.
+        """
+        m = CHModuleFactory.create(moderate='True')
+        json_in = {'answer': 1, 'hint': 'This is a new hint.'}
+        m.submit_hint(json_in)
+        self.assertTrue('29.0' not in m.hints)
+        self.assertTrue('29.0' in m.mod_queue)
+
+    def test_submithint_escape(self):
+        """
+        Make sure that hints are being html-escaped.
+        """
+        m = CHModuleFactory.create()
+        json_in = {'answer': 1, 'hint': '<script> alert("Trololo"); </script>'}
+        m.submit_hint(json_in)
+        print m.hints
+        self.assertTrue(m.hints['29.0'][0][0] == u'&lt;script&gt; alert(&quot;Trololo&quot;); &lt;/script&gt;')
+
+    def test_template_gethint(self):
+        """
+        Test the templates for get_hint.
+        """
+        m = CHModuleFactory.create()
+
+        def fake_get_hint(get):
+            """
+            Creates a rendering dictionary, with which we can test
+            the templates.
+            """
+            return {'best_hint': 'This is the best hint.',
+                    'rand_hint_1': 'A random hint',
+                    'rand_hint_2': 'Another random hint',
+                    'answer': '42.5'}
+
+        m.get_hint = fake_get_hint
+        json_in = {'problem_name': '42.5'}
+        out = json.loads(m.handle_ajax('get_hint', json_in))['contents']
+        self.assertTrue('This is the best hint.' in out)
+        self.assertTrue('A random hint' in out)
+        self.assertTrue('Another random hint' in out)
+
+    def test_template_feedback(self):
+        """
+        Test the templates for get_feedback.
+        NOT FINISHED
+
+        from lxml import etree
+        m = CHModuleFactory.create()
+
+        def fake_get_feedback(get):
+            index_to_answer = {'0': '42.0', '1': '9000.01'}
+            index_to_hints = {'0': [('A hint for 42', 12),
+                                    ('Another hint for 42', 14)],
+                              '1': [('A hint for 9000.01', 32)]}
+            return {'index_to_hints': index_to_hints, 'index_to_answer': index_to_answer}
+
+        m.get_feedback = fake_get_feedback
+        json_in = {'problem_name': '42.5'}
+        out = json.loads(m.handle_ajax('get_feedback', json_in))['contents']
+        html_tree = etree.XML(out)
+        # To be continued...
+
+        """
+        pass

common/lib/xmodule/xmodule/tests/test_date_utils.py

@@ -1,54 +1,81 @@
 # Tests for xmodule.util.date_utils
 
-from nose.tools import assert_equals
-from xmodule.util import date_utils
-import datetime
+from nose.tools import assert_equals, assert_false
+from xmodule.util.date_utils import get_default_time_display, almost_same_datetime
+from datetime import datetime, timedelta, tzinfo
 from pytz import UTC
 
 
 def test_get_default_time_display():
-    assert_equals("", date_utils.get_default_time_display(None))
-    test_time = datetime.datetime(1992, 3, 12, 15, 3, 30, tzinfo=UTC)
+    assert_equals("", get_default_time_display(None))
+    test_time = datetime(1992, 3, 12, 15, 3, 30, tzinfo=UTC)
     assert_equals(
         "Mar 12, 1992 at 15:03 UTC",
-        date_utils.get_default_time_display(test_time))
+        get_default_time_display(test_time))
     assert_equals(
         "Mar 12, 1992 at 15:03 UTC",
-        date_utils.get_default_time_display(test_time, True))
+        get_default_time_display(test_time, True))
     assert_equals(
         "Mar 12, 1992 at 15:03",
-        date_utils.get_default_time_display(test_time, False))
+        get_default_time_display(test_time, False))
 
 def test_get_default_time_display_notz():
-    test_time = datetime.datetime(1992, 3, 12, 15, 3, 30)
+    test_time = datetime(1992, 3, 12, 15, 3, 30)
     assert_equals(
         "Mar 12, 1992 at 15:03 UTC",
-        date_utils.get_default_time_display(test_time))
+        get_default_time_display(test_time))
     assert_equals(
         "Mar 12, 1992 at 15:03 UTC",
-        date_utils.get_default_time_display(test_time, True))
+        get_default_time_display(test_time, True))
     assert_equals(
         "Mar 12, 1992 at 15:03",
-        date_utils.get_default_time_display(test_time, False))
+        get_default_time_display(test_time, False))
 
 # pylint: disable=W0232
-class NamelessTZ(datetime.tzinfo):
+class NamelessTZ(tzinfo):
 
     def utcoffset(self, _dt):
-        return datetime.timedelta(hours=-3)
+        return timedelta(hours=-3)
 
     def dst(self, _dt):
-        return datetime.timedelta(0)
+        return timedelta(0)
 
 def test_get_default_time_display_no_tzname():
-    assert_equals("", date_utils.get_default_time_display(None))
-    test_time = datetime.datetime(1992, 3, 12, 15, 3, 30, tzinfo=NamelessTZ())
+    assert_equals("", get_default_time_display(None))
+    test_time = datetime(1992, 3, 12, 15, 3, 30, tzinfo=NamelessTZ())
     assert_equals(
         "Mar 12, 1992 at 15:03-0300",
-        date_utils.get_default_time_display(test_time))
+        get_default_time_display(test_time))
     assert_equals(
         "Mar 12, 1992 at 15:03-0300",
-        date_utils.get_default_time_display(test_time, True))
+        get_default_time_display(test_time, True))
     assert_equals(
         "Mar 12, 1992 at 15:03",
-        date_utils.get_default_time_display(test_time, False))
+        get_default_time_display(test_time, False))
+
+def test_almost_same_datetime():
+    assert almost_same_datetime(
+        datetime(2013, 5, 3, 10, 20, 30),
+        datetime(2013, 5, 3, 10, 21, 29)
+    )
+
+    assert almost_same_datetime(
+        datetime(2013, 5, 3, 11, 20, 30),
+        datetime(2013, 5, 3, 10, 21, 29),
+        timedelta(hours=1)
+    )
+
+    assert_false(
+        almost_same_datetime(
+            datetime(2013, 5, 3, 11, 20, 30),
+            datetime(2013, 5, 3, 10, 21, 29)
+        )
+    )
+
+    assert_false(
+        almost_same_datetime(
+            datetime(2013, 5, 3, 11, 20, 30),
+            datetime(2013, 5, 3, 10, 21, 29),
+            timedelta(minutes=10)
+        )
+    )

common/lib/xmodule/xmodule/tests/test_export.py

@@ -1,11 +1,17 @@
 import unittest
+import pytz
 
+from datetime import datetime, timedelta, tzinfo
 from fs.osfs import OSFS
+from mock import Mock
 from path import path
 from tempfile import mkdtemp
 import shutil
 
 from xmodule.modulestore.xml import XMLModuleStore
+from xmodule.modulestore.xml_exporter import EdxJSONEncoder
+
+from xmodule.modulestore import Location
 
 # from ~/mitx_all/mitx/common/lib/xmodule/xmodule/tests/
 # to   ~/mitx_all/mitx/common/test
@@ -127,3 +133,61 @@ class RoundTripTestCase(unittest.TestCase):
 
     def test_word_cloud_roundtrip(self):
         self.check_export_roundtrip(DATA_DIR, "word_cloud")
+
+
+class TestEdxJsonEncoder(unittest.TestCase):
+    def setUp(self):
+        self.encoder = EdxJSONEncoder()
+
+        class OffsetTZ(tzinfo):
+            """A timezone with non-None utcoffset"""
+            def utcoffset(self, dt):
+                return timedelta(hours=4)
+
+        self.offset_tz = OffsetTZ()
+
+        class NullTZ(tzinfo):
+            """A timezone with None as its utcoffset"""
+            def utcoffset(self, dt):
+                return None
+        self.null_utc_tz = NullTZ()
+
+    def test_encode_location(self):
+        loc = Location('i4x', 'org', 'course', 'category', 'name')
+        self.assertEqual(loc.url(), self.encoder.default(loc))
+
+        loc = Location('i4x', 'org', 'course', 'category', 'name', 'version')
+        self.assertEqual(loc.url(), self.encoder.default(loc))
+
+    def test_encode_naive_datetime(self):
+        self.assertEqual(
+            "2013-05-03T10:20:30.000100",
+            self.encoder.default(datetime(2013, 5, 3, 10, 20, 30, 100))
+        )
+        self.assertEqual(
+            "2013-05-03T10:20:30",
+            self.encoder.default(datetime(2013, 5, 3, 10, 20, 30))
+        )
+
+    def test_encode_utc_datetime(self):
+        self.assertEqual(
+            "2013-05-03T10:20:30+00:00",
+            self.encoder.default(datetime(2013, 5, 3, 10, 20, 30, 0, pytz.UTC))
+        )
+
+        self.assertEqual(
+            "2013-05-03T10:20:30+04:00",
+            self.encoder.default(datetime(2013, 5, 3, 10, 20, 30, 0, self.offset_tz))
+        )
+
+        self.assertEqual(
+            "2013-05-03T10:20:30Z",
+            self.encoder.default(datetime(2013, 5, 3, 10, 20, 30, 0, self.null_utc_tz))
+        )
+
+    def test_fallthrough(self):
+        with self.assertRaises(TypeError):
+            self.encoder.default(None)
+
+        with self.assertRaises(TypeError):
+            self.encoder.default({})