Merge branch 'master' into jnater/courseware_tests

Conflicts:
	lms/djangoapps/courseware/tests/tests.py
	lms/djangoapps/instructor/tests/test_enrollment.py

CHANGELOG.rst

@@ -5,6 +5,15 @@ 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.
 
+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.
 
@@ -51,6 +60,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
@@ -141,3 +152,5 @@ Common: Updated CodeJail.
 
 Common: Allow setting of authentication session cookie name.
 
+LMS: Option to email students when enroll/un-enroll them.
+

cms/djangoapps/auth/authz.py

 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
@@ -36,10 +40,10 @@ def get_users_in_course_group_by_role(location, role):
     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)
 
@@ -56,10 +60,10 @@ 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():
@@ -72,10 +76,10 @@ def _delete_course_group(location):
         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 +98,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)
+
+
+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)
+
 
-        group = Group.objects.get(name=groupname)
+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,9 +151,27 @@ 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)
+
+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()
 
@@ -136,3 +182,26 @@ 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

cms/djangoapps/auth/tests/test_authz.py

+"""
+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
+
+
+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)

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

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

cms/djangoapps/contentstore/module_info_model.py

@@ -5,9 +5,6 @@ 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)
     except ItemNotFoundError:
         # create a new one

cms/djangoapps/contentstore/tests/test_contentstore.py

 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)
 
@@ -583,11 +594,9 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         self.assertRaises(InvalidVersionError, draft_store.clone_item, 'i4x://edx/templates/chapter/Empty',
                           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,
@@ -598,7 +607,6 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
         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,42 +874,73 @@ 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.assertEqual(resp.status_code, 200)
-        self.assertEqual(data['ErrMsg'],
+        self.assert_course_creation_failed(
             "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"""
         # Create a course so there is something to view

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/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))

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

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 in self._descriptor_model_data or key in self._session

cms/envs/acceptance.py

@@ -23,7 +23,7 @@ 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 = {
@@ -40,6 +40,21 @@ MODULESTORE = {
         '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/dev.py

@@ -22,7 +22,7 @@ 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 = {

cms/envs/test.py

@@ -48,7 +48,7 @@ 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 = {
@@ -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': {

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 ->
@@ -123,6 +133,35 @@ 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 ->

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",
@@ -106,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);
         }
@@ -121,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/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/emails/activation_email_subject.txt

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

cms/templates/new_item.html

-<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>

common/djangoapps/student/forms.py

+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/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 student.models import unique_id_for_user
-from student.views import process_survey_link, _cert_info
 
+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, 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

@@ -11,9 +11,9 @@ 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/util/testing.py

 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/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.
@@ -677,20 +677,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):
@@ -751,7 +751,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:
@@ -765,7 +765,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']
@@ -951,16 +951,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'.
@@ -974,7 +974,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/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="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": "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.
-        '''
+        """
         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")
+
+        `data` is POST dictionary (Django QueryDict).
 
-        The *get* dict has keys of the form 'x_y', which are mapped
+        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
 
+        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,8 +1014,9 @@ class CapaModule(CapaFields, XModule):
         return {'success': True,
                 'msg': msg}
 
-    def reset_problem(self, get):
-        ''' Changes problem state to unfinished -- removes student answers,
+    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:
@@ -945,8 +1024,8 @@ class CapaModule(CapaFields, XModule):
            'html': Problem HTML string }
 
         If an error occurs, the dictionary will also have an
-            'error' key containing an error message.
-        '''
+        `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/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/js/src/capa/display.coffee

@@ -138,7 +138,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
@@ -364,8 +365,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
@@ -379,7 +378,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/modulestore/draft.py

@@ -101,12 +101,12 @@ class DraftModuleStore(ModuleStoreBase):
         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)
+        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)
+                and item.location.replace(revision=None) not in draft_locs_found)
         ]
         return [wrap_draft(item) for item in draft_items + non_draft_items]
 

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

@@ -195,7 +195,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

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

@@ -43,6 +43,7 @@ class ModuleStoreTestCase(TestCase):
 
         # Remove everything except templates
         modulestore.collection.remove(query)
+        modulestore.collection.drop()
 
     @staticmethod
     def load_templates_if_necessary():

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/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

@@ -62,10 +62,10 @@ class SequenceModule(SequenceFields, XModule):
         progress = reduce(Progress.add_counts, progresses)
         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/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

-"""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,
+        exception_classes = [StudentInputError,
                              LoncapaProblemError,
-                                ResponseError]:
+                             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_import.py

@@ -157,9 +157,10 @@ class ImportTestCase(BaseCourseTestCase):
         self.assertEqual(child.lms.due, ImportTestCase.date.from_json(v))
         self.assertEqual(child._inheritable_metadata, child._inherited_metadata)
         self.assertEqual(2, len(child._inherited_metadata))
-        self.assertLessEqual(ImportTestCase.date.from_json(
-            child._inherited_metadata['start']),
-            datetime.datetime.now(UTC()))
+        self.assertLessEqual(
+            ImportTestCase.date.from_json(child._inherited_metadata['start']),
+            datetime.datetime.now(UTC())
+        )
         self.assertEqual(v, child._inherited_metadata['due'])
 
         # Now export and check things
@@ -221,7 +222,8 @@ class ImportTestCase(BaseCourseTestCase):
         # why do these tests look in the internal structure v just calling child.start?
         self.assertLessEqual(
             ImportTestCase.date.from_json(child._inherited_metadata['start']),
-            datetime.datetime.now(UTC()))
+            datetime.datetime.now(UTC())
+        )
 
     def test_metadata_override_default(self):
         """

common/lib/xmodule/xmodule/tests/test_logic.py

@@ -40,9 +40,9 @@ class LogicTest(unittest.TestCase):
             self.raw_model_data
         )
 
-    def ajax_request(self, dispatch, get):
+    def ajax_request(self, dispatch, data):
         """Call Xmodule.handle_ajax."""
-        return json.loads(self.xmodule.handle_ajax(dispatch, get))
+        return json.loads(self.xmodule.handle_ajax(dispatch, data))
 
 
 class PollModuleTest(LogicTest):

common/lib/xmodule/xmodule/tests/test_video_module.py

 # -*- coding: utf-8 -*-
 import unittest
 
+from xmodule.modulestore import Location
 from xmodule.video_module import VideoDescriptor
 from .test_import import DummySystem
 
@@ -10,6 +11,33 @@ class VideoDescriptorImportTestCase(unittest.TestCase):
     Make sure that VideoDescriptor can import an old XML-based video correctly.
     """
 
+    def test_constructor(self):
+        sample_xml = '''
+            <video display_name="Test Video"
+                   youtube="1.0:p2Q6BrNhdh8,0.75:izygArpw-Qo,1.25:1EeWXzPdhSA,1.5:rABDYkeK0x8"
+                   show_captions="false"
+                   from="00:00:01"
+                   to="00:01:00">
+              <source src="http://www.example.com/source.mp4"/>
+              <track src="http://www.example.com/track"/>
+            </video>
+        '''
+        location = Location(["i4x", "edX", "video", "default",
+                             "SampleProblem1"])
+        model_data = {'data': sample_xml,
+                      'location': location}
+        system = DummySystem(load_error_modules=True)
+        descriptor = VideoDescriptor(system, model_data)
+        self.assertEquals(descriptor.youtube_id_0_75, 'izygArpw-Qo')
+        self.assertEquals(descriptor.youtube_id_1_0, 'p2Q6BrNhdh8')
+        self.assertEquals(descriptor.youtube_id_1_25, '1EeWXzPdhSA')
+        self.assertEquals(descriptor.youtube_id_1_5, 'rABDYkeK0x8')
+        self.assertEquals(descriptor.show_captions, False)
+        self.assertEquals(descriptor.start_time, 1.0)
+        self.assertEquals(descriptor.end_time, 60)
+        self.assertEquals(descriptor.track, 'http://www.example.com/track')
+        self.assertEquals(descriptor.source, 'http://www.example.com/source.mp4')
+
     def test_from_xml(self):
         module_system = DummySystem(load_error_modules=True)
         xml_data = '''

common/lib/xmodule/xmodule/tests/test_xml_module.py

@@ -248,7 +248,7 @@ class TestDeserializeFloat(TestDeserialize):
     test_field = Float
 
     def test_deserialize(self):
-        self.assertDeserializeEqual( -2, '-2')
+        self.assertDeserializeEqual(-2, '-2')
         self.assertDeserializeEqual("450", '"450"')
         self.assertDeserializeEqual(-2.78, '-2.78')
         self.assertDeserializeEqual("0.45", '"0.45"')
@@ -256,7 +256,7 @@ class TestDeserializeFloat(TestDeserialize):
         # False can be parsed as a float (converts to 0)
         self.assertDeserializeEqual(False, 'false')
         # True can be parsed as a float (converts to 1)
-        self.assertDeserializeEqual( True, 'true')
+        self.assertDeserializeEqual(True, 'true')
 
     def test_deserialize_unsupported_types(self):
         self.assertDeserializeEqual('[3]', '[3]')

common/lib/xmodule/xmodule/timelimit_module.py

@@ -98,7 +98,7 @@ class TimeLimitModule(TimeLimitFields, XModule):
         progress = reduce(Progress.add_counts, progresses)
         return progress
 
-    def handle_ajax(self, dispatch, get):
+    def handle_ajax(self, _dispatch, _data):
         raise NotFoundError('Unexpected dispatch type')
 
     def render(self):
@@ -141,4 +141,3 @@ class TimeLimitDescriptor(TimeLimitFields, XMLEditingDescriptor, XmlDescriptor):
             xml_object.append(
                 etree.fromstring(child.export_to_xml(resource_fs)))
         return xml_object
-

common/lib/xmodule/xmodule/video_module.py

@@ -54,9 +54,9 @@ class VideoModule(VideoFields, XModule):
     def __init__(self, *args, **kwargs):
         XModule.__init__(self, *args, **kwargs)
 
-    def handle_ajax(self, dispatch, get):
+    def handle_ajax(self, dispatch, data):
         """This is not being called right now and we raise 404 error."""
-        log.debug(u"GET {0}".format(get))
+        log.debug(u"GET {0}".format(data))
         log.debug(u"DISPATCH {0}".format(dispatch))
         raise Http404()
 
@@ -88,6 +88,13 @@ class VideoDescriptor(VideoFields,
     module_class = VideoModule
     template_dir_name = "video"
 
+    def __init__(self, *args, **kwargs):
+        super(VideoDescriptor, self).__init__(*args, **kwargs)
+        # If we don't have a `youtube_id_1_0`, this is an XML course
+        # and we parse out the fields.
+        if self.data and 'youtube_id_1_0' not in self._model_data:
+            _parse_video_xml(self, self.data)
+
     @property
     def non_editable_metadata_fields(self):
         non_editable_fields = super(MetadataOnlyEditingDescriptor, self).non_editable_metadata_fields
@@ -108,6 +115,15 @@ class VideoDescriptor(VideoFields,
             url identifiers
         """
         video = super(VideoDescriptor, cls).from_xml(xml_data, system, org, course)
+        _parse_video_xml(video, xml_data)
+        return video
+
+
+def _parse_video_xml(video, xml_data):
+    """
+    Parse video fields out of xml_data. The fields are set if they are
+    present in the XML.
+    """
     xml = etree.fromstring(xml_data)
 
     display_name = xml.get('display_name')
@@ -146,8 +162,6 @@ class VideoDescriptor(VideoFields,
     if end_time:
         video.end_time = end_time
 
-        return video
-
 
 def _get_first_external(xmltree, tag):
     """

common/lib/xmodule/xmodule/videoalpha_module.py

@@ -125,9 +125,9 @@ class VideoAlphaModule(VideoAlphaFields, XModule):
 
         return parse_time(xmltree.get('start_time')), parse_time(xmltree.get('end_time'))
 
-    def handle_ajax(self, dispatch, get):
+    def handle_ajax(self, dispatch, data):
         """This is not being called right now and we raise 404 error."""
-        log.debug(u"GET {0}".format(get))
+        log.debug(u"GET {0}".format(data))
         log.debug(u"DISPATCH {0}".format(dispatch))
         raise Http404()
 

common/lib/xmodule/xmodule/word_cloud_module.py

@@ -168,12 +168,12 @@ class WordCloudModule(WordCloudFields, XModule):
             )[:amount]
         )
 
-    def handle_ajax(self, dispatch, post):
+    def handle_ajax(self, dispatch, data):
         """Ajax handler.
 
         Args:
             dispatch: string request slug
-            post: dict request get parameters
+            data: dict request get parameters
 
         Returns:
             json string
@@ -187,7 +187,7 @@ class WordCloudModule(WordCloudFields, XModule):
 
             # Student words from client.
             # FIXME: we must use raw JSON, not a post data (multipart/form-data)
-            raw_student_words = post.getlist('student_words[]')
+            raw_student_words = data.getlist('student_words[]')
             student_words = filter(None, map(self.good_word, raw_student_words))
 
             self.student_words = student_words

common/lib/xmodule/xmodule/x_module.py

@@ -272,9 +272,9 @@ class XModule(XModuleFields, HTMLSnippet, XBlock):
         '''
         return None
 
-    def handle_ajax(self, _dispatch, _get):
+    def handle_ajax(self, _dispatch, _data):
         ''' dispatch is last part of the URL.
-            get is a dictionary-like object '''
+            data is a dictionary-like object with the content of the request'''
         return ""
 
 

common/lib/xmodule/xmodule/xml_module.py

@@ -409,7 +409,6 @@ class XmlDescriptor(XModuleDescriptor):
             # don't want e.g. data_dir
             if attr not in self.metadata_to_strip and attr not in self.metadata_to_export_to_policy:
                 val = val_for_xml(attr)
-                #logging.debug('location.category = {0}, attr = {1}'.format(self.location.category, attr))
                 try:
                     xml_object.set(attr, val)
                 except Exception, e:

common/static/coffee/spec/logger_spec.coffee

@@ -3,10 +3,15 @@ describe 'Logger', ->
     expect(window.log_event).toBe Logger.log
 
   describe 'log', ->
-    it 'sends an event to Segment.io, if the event is whitelisted', ->
+    it 'sends an event to Segment.io, if the event is whitelisted and the data is not a dictionary', ->
       spyOn(analytics, 'track')
       Logger.log 'seq_goto', 'data'
-      expect(analytics.track).toHaveBeenCalledWith 'seq_goto', 'data'
+      expect(analytics.track).toHaveBeenCalledWith 'seq_goto', value: 'data'
+
+    it 'sends an event to Segment.io, if the event is whitelisted and the data is a dictionary', ->
+      spyOn(analytics, 'track')
+      Logger.log 'seq_goto', value: 'data'
+      expect(analytics.track).toHaveBeenCalledWith 'seq_goto', value: 'data'
 
     it 'send a request to log event', ->
       spyOn $, 'getWithPrefix'

common/static/coffee/src/logger.coffee

 class @Logger
   # events we want sent to Segment.io for tracking
-  SEGMENT_IO_WHITELIST = ["seq_goto", "seq_next", "seq_prev"]
+  SEGMENT_IO_WHITELIST = ["seq_goto", "seq_next", "seq_prev", "problem_check", "problem_reset", "problem_show", "problem_save"]
 
   @log: (event_type, data) ->
-    if event_type in SEGMENT_IO_WHITELIST
     # Segment.io event tracking
+    if event_type in SEGMENT_IO_WHITELIST
+      # to avoid changing the format of data sent to our servers, we only massage it here
+      if typeof data isnt 'object' or data is null
+        analytics.track event_type, value: data
+      else
         analytics.track event_type, data
 
     $.getWithPrefix '/event',

common/test/data/graphic_slider_tool/policies/2012_Fall.json

@@ -9,6 +9,6 @@
         "display_name": "Overview"
     },
     "graphical_slider_tool/sample_gst": {
-        "display_name": "Sample GST",
-    },
+        "display_name": "Sample GST"
+    }
 }

common/test/data/self_assessment/policies/2012_Fall.json

@@ -9,6 +9,6 @@
         "display_name": "Overview"
     },
     "selfassessment/SampleQuestion": {
-        "display_name": "Sample Question",
-    },
+        "display_name": "Sample Question"
+    }
 }

jenkins/test.sh

@@ -60,10 +60,7 @@ fi
 
 export PIP_DOWNLOAD_CACHE=/mnt/pip-cache
 
-# Allow django liveserver tests to use a range of ports
-export DJANGO_LIVE_TEST_SERVER_ADDRESS=${DJANGO_LIVE_TEST_SERVER_ADDRESS-localhost:8000-9000}
-
-source /mnt/virtualenvs/"$JOB_NAME"/bin/activate
+source $VIRTUALENV_DIR/bin/activate
 
 bundle install
 

lms/djangoapps/courseware/access.py

@@ -523,10 +523,8 @@ def _adjust_start_date_for_beta_testers(user, descriptor):
     beta_group = course_beta_test_group_name(descriptor.location)
     if beta_group in user_groups:
         debug("Adjust start time: user in group %s", beta_group)
-        start_as_datetime = descriptor.lms.start
         delta = timedelta(descriptor.lms.days_early_for_beta)
-        effective = start_as_datetime - delta
-        # ...and back to time_struct
+        effective = descriptor.lms.start - delta
         return effective
 
     return descriptor.lms.start

lms/djangoapps/courseware/courses.py

@@ -12,12 +12,11 @@ from xmodule.modulestore import Location
 from xmodule.modulestore.django import modulestore
 from xmodule.contentstore.content import StaticContent
 from xmodule.modulestore.xml import XMLModuleStore
-from xmodule.modulestore.exceptions import ItemNotFoundError
+from xmodule.modulestore.exceptions import ItemNotFoundError, InvalidLocationError
 from courseware.model_data import ModelDataCache
 from static_replace import replace_static_urls
 from courseware.access import has_access
 import branding
-from xmodule.modulestore.exceptions import ItemNotFoundError
 
 log = logging.getLogger(__name__)
 
@@ -49,7 +48,8 @@ def get_course_by_id(course_id, depth=0):
         return modulestore().get_instance(course_id, course_loc, depth=depth)
     except (KeyError, ItemNotFoundError):
         raise Http404("Course not found.")
-
+    except InvalidLocationError:
+        raise Http404("Invalid location")
 
 def get_course_with_access(user, course_id, action, depth=0):
     """

lms/djangoapps/courseware/features/problems.py

@@ -2,8 +2,8 @@
 Steps for problem.feature lettuce tests
 '''
 
-#pylint: disable=C0111
-#pylint: disable=W0621
+# pylint: disable=C0111
+# pylint: disable=W0621
 
 from lettuce import world, step
 from lettuce.django import django_url
@@ -135,7 +135,7 @@ def action_button_present(_step, buttonname, doesnt_appear):
 
 
 @step(u'the button with the label "([^"]*)" does( not)? appear')
-def button_with_label_present(step, buttonname, doesnt_appear):
+def button_with_label_present(_step, buttonname, doesnt_appear):
     if doesnt_appear:
         assert world.browser.is_text_not_present(buttonname, wait_time=5)
     else:

lms/djangoapps/courseware/module_render.py

@@ -2,8 +2,6 @@ import json
 import logging
 import re
 import sys
-import static_replace
-
 from functools import partial
 
 from django.conf import settings
@@ -15,27 +13,31 @@ from django.http import Http404
 from django.http import HttpResponse, HttpResponseBadRequest
 from django.views.decorators.csrf import csrf_exempt
 
+import pyparsing
 from requests.auth import HTTPBasicAuth
+from statsd import statsd
 
 from capa.xqueue_interface import XQueueInterface
-from courseware.masquerade import setup_masquerade
-from courseware.access import has_access
 from mitxmako.shortcuts import render_to_string
-from .models import StudentModule
-from psychometrics.psychoanalyze import make_psychometrics_data_update_handler
-from student.models import unique_id_for_user
+from xblock.runtime import DbModel
+from xmodule.error_module import ErrorDescriptor, NonStaffErrorDescriptor
 from xmodule.errortracker import exc_info_to_str
 from xmodule.exceptions import NotFoundError, ProcessingError
 from xmodule.modulestore import Location
 from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.exceptions import ItemNotFoundError
 from xmodule.x_module import ModuleSystem
-from xmodule.error_module import ErrorDescriptor, NonStaffErrorDescriptor
-from xblock.runtime import DbModel
 from xmodule_modifiers import replace_course_urls, replace_static_urls, add_histogram, wrap_xmodule
-from .model_data import LmsKeyValueStore, LmsUsage, ModelDataCache
 
-from xmodule.modulestore.exceptions import ItemNotFoundError
-from statsd import statsd
+import static_replace
+from psychometrics.psychoanalyze import make_psychometrics_data_update_handler
+from student.models import unique_id_for_user
+
+from courseware.access import has_access
+from courseware.masquerade import setup_masquerade
+from courseware.model_data import LmsKeyValueStore, LmsUsage, ModelDataCache
+from courseware.models import StudentModule
+
 
 log = logging.getLogger(__name__)
 
@@ -221,7 +223,7 @@ def get_module_for_descriptor_internal(user, descriptor, model_data_cache, cours
         relative_xqueue_callback_url = reverse('xqueue_callback',
                                                kwargs=dict(course_id=course_id,
                                                            userid=str(user.id),
-                                                           id=descriptor.location.url(),
+                                                           mod_id=descriptor.location.url(),
                                                            dispatch=dispatch),
                                        )
         return xqueue_callback_url_prefix + relative_xqueue_callback_url
@@ -397,40 +399,47 @@ def get_module_for_descriptor_internal(user, descriptor, model_data_cache, cours
 
 
 @csrf_exempt
-def xqueue_callback(request, course_id, userid, id, dispatch):
+def xqueue_callback(request, course_id, userid, mod_id, dispatch):
     '''
     Entry point for graded results from the queueing system.
     '''
+    data = request.POST.copy()
+
     # Test xqueue package, which we expect to be:
     #   xpackage = {'xqueue_header': json.dumps({'lms_key':'secretkey',...}),
     #               'xqueue_body'  : 'Message from grader'}
-    get = request.POST.copy()
     for key in ['xqueue_header', 'xqueue_body']:
-        if not get.has_key(key):
+        if key not in data:
             raise Http404
-    header = json.loads(get['xqueue_header'])
-    if not isinstance(header, dict) or not header.has_key('lms_key'):
+
+    header = json.loads(data['xqueue_header'])
+    if not isinstance(header, dict) or 'lms_key' not in header:
         raise Http404
 
     # Retrieve target StudentModule
     user = User.objects.get(id=userid)
-
-    model_data_cache = ModelDataCache.cache_for_descriptor_descendents(course_id,
-        user, modulestore().get_instance(course_id, id), depth=0, select_for_update=True)
-    instance = get_module(user, request, id, model_data_cache, course_id, grade_bucket_type='xqueue')
+    model_data_cache = ModelDataCache.cache_for_descriptor_descendents(
+        course_id,
+        user,
+        modulestore().get_instance(course_id, mod_id),
+        depth=0,
+        select_for_update=True
+    )
+    instance = get_module(user, request, mod_id, model_data_cache, course_id, grade_bucket_type='xqueue')
     if instance is None:
-        log.debug("No module {0} for user {1}--access denied?".format(id, user))
+        msg = "No module {0} for user {1}--access denied?".format(mod_id, user)
+        log.debug(msg)
         raise Http404
 
-    # Transfer 'queuekey' from xqueue response header to 'get'. This is required to
-    #   use the interface defined by 'handle_ajax'
-    get.update({'queuekey': header['lms_key']})
+    # Transfer 'queuekey' from xqueue response header to the data.
+    # This is required to use the interface defined by 'handle_ajax'
+    data.update({'queuekey': header['lms_key']})
 
     # We go through the "AJAX" path
     # So far, the only dispatch from xqueue will be 'score_update'
     try:
         # Can ignore the return value--not used for xqueue_callback
-        instance.handle_ajax(dispatch, get)
+        instance.handle_ajax(dispatch, data)
     except:
         log.exception("error processing ajax call")
         raise
@@ -464,23 +473,15 @@ def modx_dispatch(request, dispatch, location, course_id):
     if not request.user.is_authenticated():
         raise PermissionDenied
 
-    # Check for submitted files and basic file size checks
-    p = request.POST.copy()
-    if request.FILES:
-        for fileinput_id in request.FILES.keys():
-            inputfiles = request.FILES.getlist(fileinput_id)
-
-            if len(inputfiles) > settings.MAX_FILEUPLOADS_PER_INPUT:
-                too_many_files_msg = 'Submission aborted! Maximum %d files may be submitted at once' % \
-                    settings.MAX_FILEUPLOADS_PER_INPUT
-                return HttpResponse(json.dumps({'success': too_many_files_msg}))
+    # Get the submitted data
+    data = request.POST.copy()
 
-            for inputfile in inputfiles:
-                if inputfile.size > settings.STUDENT_FILEUPLOAD_MAX_SIZE:  # Bytes
-                    file_too_big_msg = 'Submission aborted! Your file "%s" is too large (max size: %d MB)' % \
-                                        (inputfile.name, settings.STUDENT_FILEUPLOAD_MAX_SIZE / (1000 ** 2))
-                    return HttpResponse(json.dumps({'success': file_too_big_msg}))
-            p[fileinput_id] = inputfiles
+    # Get and check submitted files
+    files = request.FILES or {}
+    error_msg = _check_files_limits(files)
+    if error_msg:
+        return HttpResponse(json.dumps({'success': error_msg}))
+    data.update(files)  # Merge files into data dictionary
 
     try:
         descriptor = modulestore().get_instance(course_id, location)
@@ -493,8 +494,11 @@ def modx_dispatch(request, dispatch, location, course_id):
         )
         raise Http404
 
-    model_data_cache = ModelDataCache.cache_for_descriptor_descendents(course_id,
-        request.user, descriptor)
+    model_data_cache = ModelDataCache.cache_for_descriptor_descendents(
+        course_id,
+        request.user,
+        descriptor
+    )
 
     instance = get_module(request.user, request, location, model_data_cache, course_id, grade_bucket_type='ajax')
     if instance is None:
@@ -505,7 +509,7 @@ def modx_dispatch(request, dispatch, location, course_id):
 
     # Let the module handle the AJAX
     try:
-        ajax_return = instance.handle_ajax(dispatch, p)
+        ajax_return = instance.handle_ajax(dispatch, data)
 
     # If we can't find the module, respond with a 404
     except NotFoundError:
@@ -527,7 +531,6 @@ def modx_dispatch(request, dispatch, location, course_id):
     return HttpResponse(ajax_return)
 
 
-
 def get_score_bucket(grade, max_grade):
     """
     Function to split arbitrary score ranges into 3 buckets.
@@ -540,3 +543,30 @@ def get_score_bucket(grade, max_grade):
         score_bucket = "correct"
 
     return score_bucket
+
+
+def _check_files_limits(files):
+    """
+    Check if the files in a request are under the limits defined by
+    `settings.MAX_FILEUPLOADS_PER_INPUT` and
+    `settings.STUDENT_FILEUPLOAD_MAX_SIZE`.
+
+    Returns None if files are correct or an error messages otherwise.
+    """
+    for fileinput_id in files.keys():
+        inputfiles = files.getlist(fileinput_id)
+
+        # Check number of files submitted
+        if len(inputfiles) > settings.MAX_FILEUPLOADS_PER_INPUT:
+            msg = 'Submission aborted! Maximum %d files may be submitted at once' %\
+                  settings.MAX_FILEUPLOADS_PER_INPUT
+            return msg
+
+        # Check file sizes
+        for inputfile in inputfiles:
+            if inputfile.size > settings.STUDENT_FILEUPLOAD_MAX_SIZE:   # Bytes
+                msg = 'Submission aborted! Your file "%s" is too large (max size: %d MB)' %\
+                      (inputfile.name, settings.STUDENT_FILEUPLOAD_MAX_SIZE / (1000 ** 2))
+                return msg
+
+    return None

lms/djangoapps/courseware/tests/test_courses.py

+# -*- coding: utf-8 -*-
+from django.test import TestCase
+from django.http import Http404
+from courseware.courses import get_course_by_id
+
+class CoursesTest(TestCase):
+    def test_get_course_by_id_invalid_chars(self):
+        """
+        Test that `get_course_by_id` throws a 404, rather than
+        an exception, when faced with unexpected characters 
+        (such as unicode characters, and symbols such as = and ' ')
+        """
+        with self.assertRaises(Http404):
+            get_course_by_id('MITx/foobar/statistics=introduction')
+            get_course_by_id('MITx/foobar/business and management')
+            get_course_by_id('MITx/foobar/NiñøJoséMaríáßç')

lms/djangoapps/django_comment_client/base/tests.py

 import logging
 
-from django.conf import settings
 from django.test.utils import override_settings
 from django.test.client import Client
 from django.contrib.auth.models import User
@@ -21,16 +20,13 @@ log = logging.getLogger(__name__)
 @override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE)
 @patch('comment_client.utils.requests.request')
 class ViewsTestCase(UrlResetMixin, ModuleStoreTestCase):
+
+    @patch.dict("django.conf.settings.MITX_FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
     def setUp(self):
 
-        # This feature affects the contents of urls.py, so we change
-        # it before the call to super.setUp() which reloads urls.py (because
+        # Patching the ENABLE_DISCUSSION_SERVICE value affects the contents of urls.py,
+        # so we need to call super.setUp() which reloads urls.py (because
         # of the UrlResetMixin)
-
-        # This setting is cleaned up at the end of the test by @override_settings, which
-        # restores all of the old settings
-        settings.MITX_FEATURES['ENABLE_DISCUSSION_SERVICE'] = True
-
         super(ViewsTestCase, self).setUp()
 
         # create a course

lms/djangoapps/instructor/tests/test_enrollment.py

-'''
+"""
 Unit tests for enrollment methods in views.py
 
-'''
+"""
 
 from django.test.utils import override_settings
-from django.contrib.auth.models import Group, User
+from django.contrib.auth.models import User
 from django.core.urlresolvers import reverse
 from courseware.access import _course_staff_group_name
 from courseware.tests.helpers import LoginEnrollmentTestCase
 from courseware.tests.modulestore_config import TEST_DATA_XML_MODULESTORE
 from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.tests.factories import CourseFactory
+from student.tests.factories import UserFactory, CourseEnrollmentFactory, AdminFactory
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE, LoginEnrollmentTestCase
 from student.models import CourseEnrollment, CourseEnrollmentAllowed
-from instructor.views import get_and_clean_student_list
+from instructor.views import get_and_clean_student_list, send_mail_to_student
+from django.core import mail
 
+USER_COUNT = 4
 
-@override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE)
-class TestInstructorEnrollsStudent(LoginEnrollmentTestCase):
-    '''
-    Check Enrollment/Unenrollment with/without auto-enrollment on activation
-    '''
+
+@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE)
+class TestInstructorEnrollsStudent(ModuleStoreTestCase, LoginEnrollmentTestCase):
+    """
+    Check Enrollment/Unenrollment with/without auto-enrollment on activation and with/without email notification
+    """
 
     def setUp(self):
 
-        self.full = modulestore().get_course("edX/full/6.002_Spring_2012")
-        self.toy = modulestore().get_course("edX/toy/2012_Fall")
-
-        #Create instructor and student accounts
-        self.instructor = 'instructor1@test.com'
-        self.student1 = 'student1@test.com'
-        self.student2 = 'student2@test.com'
-        self.password = 'foo'
-        self.create_account('it1', self.instructor, self.password)
-        self.create_account('st1', self.student1, self.password)
-        self.create_account('st2', self.student2, self.password)
-        self.activate_user(self.instructor)
-        self.activate_user(self.student1)
-        self.activate_user(self.student2)
+        instructor = AdminFactory.create()
+        self.client.login(username=instructor.username, password='test')
 
-        def make_instructor(course):
-            group_name = _course_staff_group_name(course.location)
-            g = Group.objects.create(name=group_name)
-            g.user_set.add(User.objects.get(email=self.instructor))
+        self.course = CourseFactory.create()
 
-        make_instructor(self.toy)
+        self.users = [
+            UserFactory.create(username="student%d" % i, email="student%d@test.com" % i)
+            for i in xrange(USER_COUNT)
+        ]
 
-        #Enroll Students
-        self.logout()
-        self.login(self.student1, self.password)
-        self.enroll(self.toy)
+        for user in self.users:
+            CourseEnrollmentFactory.create(user=user, course_id=self.course.id)
 
-        self.logout()
-        self.login(self.student2, self.password)
-        self.enroll(self.toy)
+        # Empty the test outbox
+        mail.outbox = []
 
-        #Enroll Instructor
-        self.logout()
-        self.login(self.instructor, self.password)
-        self.enroll(self.toy)
+    def test_unenrollment_email_off(self):
+        """
+        Do un-enrollment email off test
+        """
 
-    def test_unenrollment(self):
-        '''
-        Do un-enrollment test
-        '''
+        course = self.course
 
-        course = self.toy
+        #Run the Un-enroll students command
         url = reverse('instructor_dashboard', kwargs={'course_id': course.id})
-        response = self.client.post(url, {'action': 'Unenroll multiple students', 'multiple_students': 'student1@test.com, student2@test.com'})
+        response = self.client.post(url, {'action': 'Unenroll multiple students', 'multiple_students': 'student0@test.com student1@test.com'})
 
         #Check the page output
+        self.assertContains(response, '<td>student0@test.com</td>')
         self.assertContains(response, '<td>student1@test.com</td>')
-        self.assertContains(response, '<td>student2@test.com</td>')
         self.assertContains(response, '<td>un-enrolled</td>')
 
         #Check the enrollment table
-        user = User.objects.get(email='student1@test.com')
+        user = User.objects.get(email='student0@test.com')
         ce = CourseEnrollment.objects.filter(course_id=course.id, user=user)
         self.assertEqual(0, len(ce))
 
-        user = User.objects.get(email='student2@test.com')
+        user = User.objects.get(email='student1@test.com')
         ce = CourseEnrollment.objects.filter(course_id=course.id, user=user)
         self.assertEqual(0, len(ce))
 
-    def test_enrollment_new_student_autoenroll_on(self):
-        '''
-        Do auto-enroll on test
-        '''
+        #Check the outbox
+        self.assertEqual(len(mail.outbox), 0)
+
+    def test_enrollment_new_student_autoenroll_on_email_off(self):
+        """
+        Do auto-enroll on, email off test
+        """
+
+        course = self.course
 
         #Run the Enroll students command
-        course = self.toy
         url = reverse('instructor_dashboard', kwargs={'course_id': course.id})
-        response = self.client.post(url, {'action': 'Enroll multiple students', 'multiple_students': 'test1_1@student.com, test1_2@student.com', 'auto_enroll': 'on'})
+        response = self.client.post(url, {'action': 'Enroll multiple students', 'multiple_students': 'student1_1@test.com, student1_2@test.com', 'auto_enroll': 'on'})
 
         #Check the page output
-        self.assertContains(response, '<td>test1_1@student.com</td>')
-        self.assertContains(response, '<td>test1_2@student.com</td>')
+        self.assertContains(response, '<td>student1_1@test.com</td>')
+        self.assertContains(response, '<td>student1_2@test.com</td>')
         self.assertContains(response, '<td>user does not exist, enrollment allowed, pending with auto enrollment on</td>')
 
+        #Check the outbox
+        self.assertEqual(len(mail.outbox), 0)
+
         #Check the enrollmentallowed db entries
-        cea = CourseEnrollmentAllowed.objects.filter(email='test1_1@student.com', course_id=course.id)
+        cea = CourseEnrollmentAllowed.objects.filter(email='student1_1@test.com', course_id=course.id)
         self.assertEqual(1, cea[0].auto_enroll)
-        cea = CourseEnrollmentAllowed.objects.filter(email='test1_2@student.com', course_id=course.id)
+        cea = CourseEnrollmentAllowed.objects.filter(email='student1_2@test.com', course_id=course.id)
         self.assertEqual(1, cea[0].auto_enroll)
 
-        #Check there is no enrollment db entry other than for the setup instructor and students
+        #Check there is no enrollment db entry other than for the other students
         ce = CourseEnrollment.objects.filter(course_id=course.id)
-        self.assertEqual(3, len(ce))
+        self.assertEqual(4, len(ce))
 
         #Create and activate student accounts with same email
-        self.student1 = 'test1_1@student.com'
+        self.student1 = 'student1_1@test.com'
         self.password = 'bar'
         self.create_account('s1_1', self.student1, self.password)
         self.activate_user(self.student1)
 
-        self.student2 = 'test1_2@student.com'
+        self.student2 = 'student1_2@test.com'
         self.create_account('s1_2', self.student2, self.password)
         self.activate_user(self.student2)
 
         #Check students are enrolled
-        user = User.objects.get(email='test1_1@student.com')
+        user = User.objects.get(email='student1_1@test.com')
         ce = CourseEnrollment.objects.filter(course_id=course.id, user=user)
         self.assertEqual(1, len(ce))
 
-        user = User.objects.get(email='test1_2@student.com')
+        user = User.objects.get(email='student1_2@test.com')
         ce = CourseEnrollment.objects.filter(course_id=course.id, user=user)
         self.assertEqual(1, len(ce))
 
-    def test_enrollmemt_new_student_autoenroll_off(self):
-        '''
-        Do auto-enroll off test
-        '''
+    def test_repeat_enroll(self):
+        """
+        Try to enroll an already enrolled student
+        """
+
+        course = self.course
+
+        url = reverse('instructor_dashboard', kwargs={'course_id': course.id})
+        response = self.client.post(url, {'action': 'Enroll multiple students', 'multiple_students': 'student0@test.com', 'auto_enroll': 'on'})
+        self.assertContains(response, '<td>student0@test.com</td>')
+        self.assertContains(response, '<td>already enrolled</td>')
+
+    def test_enrollmemt_new_student_autoenroll_off_email_off(self):
+        """
+        Do auto-enroll off, email off test
+        """
+
+        course = self.course
 
         #Run the Enroll students command
-        course = self.toy
         url = reverse('instructor_dashboard', kwargs={'course_id': course.id})
-        response = self.client.post(url, {'action': 'Enroll multiple students', 'multiple_students': 'test2_1@student.com, test2_2@student.com'})
+        response = self.client.post(url, {'action': 'Enroll multiple students', 'multiple_students': 'student2_1@test.com, student2_2@test.com'})
 
         #Check the page output
-        self.assertContains(response, '<td>test2_1@student.com</td>')
-        self.assertContains(response, '<td>test2_2@student.com</td>')
+        self.assertContains(response, '<td>student2_1@test.com</td>')
+        self.assertContains(response, '<td>student2_2@test.com</td>')
         self.assertContains(response, '<td>user does not exist, enrollment allowed, pending with auto enrollment off</td>')
 
+        #Check the outbox
+        self.assertEqual(len(mail.outbox), 0)
+
         #Check the enrollmentallowed db entries
-        cea = CourseEnrollmentAllowed.objects.filter(email='test2_1@student.com', course_id=course.id)
+        cea = CourseEnrollmentAllowed.objects.filter(email='student2_1@test.com', course_id=course.id)
         self.assertEqual(0, cea[0].auto_enroll)
-        cea = CourseEnrollmentAllowed.objects.filter(email='test2_2@student.com', course_id=course.id)
+        cea = CourseEnrollmentAllowed.objects.filter(email='student2_2@test.com', course_id=course.id)
         self.assertEqual(0, cea[0].auto_enroll)
 
         #Check there is no enrollment db entry other than for the setup instructor and students
         ce = CourseEnrollment.objects.filter(course_id=course.id)
-        self.assertEqual(3, len(ce))
+        self.assertEqual(4, len(ce))
 
         #Create and activate student accounts with same email
-        self.student = 'test2_1@student.com'
+        self.student = 'student2_1@test.com'
         self.password = 'bar'
         self.create_account('s2_1', self.student, self.password)
         self.activate_user(self.student)
 
-        self.student = 'test2_2@student.com'
+        self.student = 'student2_2@test.com'
         self.create_account('s2_2', self.student, self.password)
         self.activate_user(self.student)
 
         #Check students are not enrolled
-        user = User.objects.get(email='test2_1@student.com')
+        user = User.objects.get(email='student2_1@test.com')
         ce = CourseEnrollment.objects.filter(course_id=course.id, user=user)
         self.assertEqual(0, len(ce))
-        user = User.objects.get(email='test2_2@student.com')
+        user = User.objects.get(email='student2_2@test.com')
         ce = CourseEnrollment.objects.filter(course_id=course.id, user=user)
         self.assertEqual(0, len(ce))
 
     def test_get_and_clean_student_list(self):
-        '''
+        """
         Clean user input test
-        '''
+        """
 
-        string = "abc@test.com, def@test.com ghi@test.com \n \n jkl@test.com      "
+        string = "abc@test.com, def@test.com ghi@test.com \n \n jkl@test.com   \n mno@test.com   "
         cleaned_string, cleaned_string_lc = get_and_clean_student_list(string)
-        self.assertEqual(cleaned_string, ['abc@test.com', 'def@test.com', 'ghi@test.com', 'jkl@test.com'])
+        self.assertEqual(cleaned_string, ['abc@test.com', 'def@test.com', 'ghi@test.com', 'jkl@test.com', 'mno@test.com'])
+
+    def test_enrollment_email_on(self):
+        """
+        Do email on enroll test
+        """
+
+        course = self.course
+
+        #Create activated, but not enrolled, user
+        UserFactory.create(username="student3_0", email="student3_0@test.com")
+
+        url = reverse('instructor_dashboard', kwargs={'course_id': course.id})
+        response = self.client.post(url, {'action': 'Enroll multiple students', 'multiple_students': 'student3_0@test.com, student3_1@test.com, student3_2@test.com', 'auto_enroll': 'on', 'email_students': 'on'})
+
+        #Check the page output
+        self.assertContains(response, '<td>student3_0@test.com</td>')
+        self.assertContains(response, '<td>student3_1@test.com</td>')
+        self.assertContains(response, '<td>student3_2@test.com</td>')
+        self.assertContains(response, '<td>added, email sent</td>')
+        self.assertContains(response, '<td>user does not exist, enrollment allowed, pending with auto enrollment on, email sent</td>')
+
+        #Check the outbox
+        self.assertEqual(len(mail.outbox), 3)
+        self.assertEqual(mail.outbox[0].subject, 'You have been enrolled in MITx/999/Robot_Super_Course')
+
+        self.assertEqual(mail.outbox[1].subject, 'You have been invited to register for MITx/999/Robot_Super_Course')
+        self.assertEqual(mail.outbox[1].body, "Dear student,\n\nYou have been invited to join MITx/999/Robot_Super_Course at edx.org by a member of the course staff.\n\n" +
+                                              "To finish your registration, please visit https://edx.org/register and fill out the registration form.\n" +
+                                              "Once you have registered and activated your account, you will see MITx/999/Robot_Super_Course listed on your dashboard.\n\n" +
+                                              "----\nThis email was automatically sent from edx.org to student3_1@test.com")
+
+    def test_unenrollment_email_on(self):
+        """
+        Do email on unenroll test
+        """
+
+        course = self.course
+
+        #Create invited, but not registered, user
+        cea = CourseEnrollmentAllowed(email='student4_0@test.com', course_id=course.id)
+        cea.save()
+
+        url = reverse('instructor_dashboard', kwargs={'course_id': course.id})
+        response = self.client.post(url, {'action': 'Unenroll multiple students', 'multiple_students': 'student4_0@test.com, student2@test.com, student3@test.com', 'email_students': 'on'})
+
+        #Check the page output
+        self.assertContains(response, '<td>student2@test.com</td>')
+        self.assertContains(response, '<td>student3@test.com</td>')
+        self.assertContains(response, '<td>un-enrolled, email sent</td>')
+
+        #Check the outbox
+        self.assertEqual(len(mail.outbox), 3)
+        self.assertEqual(mail.outbox[0].subject, 'You have been un-enrolled from MITx/999/Robot_Super_Course')
+        self.assertEqual(mail.outbox[0].body, "Dear Student,\n\nYou have been un-enrolled from course MITx/999/Robot_Super_Course by a member of the course staff. " +
+                         "Please disregard the invitation previously sent.\n\n" +
+                         "----\nThis email was automatically sent from edx.org to student4_0@test.com")
+        self.assertEqual(mail.outbox[1].subject, 'You have been un-enrolled from MITx/999/Robot_Super_Course')
+
+    def test_send_mail_to_student(self):
+        """
+        Do invalid mail template test
+        """
+
+        d = {'message': 'message_type_that_doesn\'t_exist'}
+
+        send_mail_ret = send_mail_to_student('student0@test.com', d)
+        self.assertFalse(send_mail_ret)

lms/djangoapps/instructor/views.py

@@ -20,6 +20,8 @@ from django.http import HttpResponse
 from django_future.csrf import ensure_csrf_cookie
 from django.views.decorators.cache import cache_control
 from django.core.urlresolvers import reverse
+from django.core.mail import send_mail
+
 import xmodule.graders as xmgraders
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.exceptions import ItemNotFoundError
@@ -45,6 +47,7 @@ from mitxmako.shortcuts import render_to_response
 from psychometrics import psychoanalyze
 from student.models import CourseEnrollment, CourseEnrollmentAllowed
 import track.views
+from mitxmako.shortcuts import render_to_string
 
 
 log = logging.getLogger(__name__)
@@ -634,13 +637,15 @@ def instructor_dashboard(request, course_id):
 
         students = request.POST.get('multiple_students', '')
         auto_enroll = bool(request.POST.get('auto_enroll'))
-        ret = _do_enroll_students(course, course_id, students, auto_enroll=auto_enroll)
+        email_students = bool(request.POST.get('email_students'))
+        ret = _do_enroll_students(course, course_id, students, auto_enroll=auto_enroll, email_students=email_students)
         datatable = ret['datatable']
 
     elif action == 'Unenroll multiple students':
 
         students = request.POST.get('multiple_students', '')
-        ret = _do_unenroll_students(course_id, students)
+        email_students = bool(request.POST.get('email_students'))
+        ret = _do_unenroll_students(course_id, students, email_students=email_students)
         datatable = ret['datatable']
 
     elif action == 'List sections available in remote gradebook':
@@ -1068,9 +1073,17 @@ def grade_summary(request, course_id):
 #-----------------------------------------------------------------------------
 # enrollment
 
-def _do_enroll_students(course, course_id, students, overload=False, auto_enroll=False):
-    """Do the actual work of enrolling multiple students, presented as a string
-    of emails separated by commas or returns"""
+def _do_enroll_students(course, course_id, students, overload=False, auto_enroll=False, email_students=False):
+    """
+    Do the actual work of enrolling multiple students, presented as a string
+    of emails separated by commas or returns
+    `course` is course object
+    `course_id` id of course (a `str`)
+    `students` string of student emails separated by commas or returns (a `str`)
+    `overload` un-enrolls all existing students (a `boolean`)
+    `auto_enroll` is user input preference (a `boolean`)
+    `email_students` is user input preference (a `boolean`)
+    """
 
     new_students, new_students_lc = get_and_clean_student_list(students)
     status = dict([x, 'unprocessed'] for x in new_students)
@@ -1088,12 +1101,22 @@ def _do_enroll_students(course, course_id, students, overload=False, auto_enroll
             status[cea.email] = 'removed from pending enrollment list'
         ceaset.delete()
 
+    if email_students:
+        registration_url = 'https://' + settings.SITE_NAME + reverse('student.views.register_user')
+        #Composition of email
+        d = {'site_name': settings.SITE_NAME,
+             'registration_url': registration_url,
+             'course_id': course_id,
+             'auto_enroll': auto_enroll,
+             'course_url': registration_url + '/courses/' + course_id,
+             }
+
     for student in new_students:
         try:
             user = User.objects.get(email=student)
         except User.DoesNotExist:
 
-            #User not signed up yet, put in pending enrollment allowed table
+            #Student not signed up yet, put in pending enrollment allowed table
             cea = CourseEnrollmentAllowed.objects.filter(email=student, course_id=course_id)
 
             #If enrollmentallowed already exists, update auto_enroll flag to however it was set in UI
@@ -1104,18 +1127,42 @@ def _do_enroll_students(course, course_id, students, overload=False, auto_enroll
                 status[student] = 'user does not exist, enrollment already allowed, pending with auto enrollment ' \
                     + ('on' if auto_enroll else 'off')
                 continue
+
+            #EnrollmentAllowed doesn't exist so create it
             cea = CourseEnrollmentAllowed(email=student, course_id=course_id, auto_enroll=auto_enroll)
             cea.save()
-            status[student] = 'user does not exist, enrollment allowed, pending with auto enrollment ' + ('on' if auto_enroll else 'off')
+
+            status[student] = 'user does not exist, enrollment allowed, pending with auto enrollment ' \
+                + ('on' if auto_enroll else 'off')
+
+            if email_students:
+                #User is allowed to enroll but has not signed up yet
+                d['email_address'] = student
+                d['message'] = 'allowed_enroll'
+                send_mail_ret = send_mail_to_student(student, d)
+                status[student] += (', email sent' if send_mail_ret else '')
             continue
 
+        #Student has already registered
         if CourseEnrollment.objects.filter(user=user, course_id=course_id):
             status[student] = 'already enrolled'
             continue
+
         try:
+            #Not enrolled yet
             ce = CourseEnrollment(user=user, course_id=course_id)
             ce.save()
             status[student] = 'added'
+
+            if email_students:
+                #User enrolled for first time, populate dict with user specific info
+                d['email_address'] = student
+                d['first_name'] = user.first_name
+                d['last_name'] = user.last_name
+                d['message'] = 'enrolled_enroll'
+                send_mail_ret = send_mail_to_student(student, d)
+                status[student] += (', email sent' if send_mail_ret else '')
+
         except:
             status[student] = 'rejected'
 
@@ -1133,13 +1180,23 @@ def _do_enroll_students(course, course_id, students, overload=False, auto_enroll
 
 
 #Unenrollment
-def _do_unenroll_students(course_id, students):
-    """Do the actual work of un-enrolling multiple students, presented as a string
-    of emails separated by commas or returns"""
+def _do_unenroll_students(course_id, students, email_students=False):
+    """
+    Do the actual work of un-enrolling multiple students, presented as a string
+    of emails separated by commas or returns
+    `course_id` is id of course (a `str`)
+    `students` is string of student emails separated by commas or returns (a `str`)
+    `email_students` is user input preference (a `boolean`)
+    """
 
     old_students, _ = get_and_clean_student_list(students)
     status = dict([x, 'unprocessed'] for x in old_students)
 
+    if email_students:
+        #Composition of email
+        d = {'site_name': settings.SITE_NAME,
+             'course_id': course_id}
+
     for student in old_students:
 
         isok = False
@@ -1153,6 +1210,14 @@ def _do_unenroll_students(course_id, students):
         try:
             user = User.objects.get(email=student)
         except User.DoesNotExist:
+
+            if isok and email_students:
+                #User was allowed to join but had not signed up yet
+                d['email_address'] = student
+                d['message'] = 'allowed_unenroll'
+                send_mail_ret = send_mail_to_student(student, d)
+                status[student] += (', email sent' if send_mail_ret else '')
+
             continue
 
         ce = CourseEnrollment.objects.filter(user=user, course_id=course_id)
@@ -1161,6 +1226,15 @@ def _do_unenroll_students(course_id, students):
             try:
                 ce[0].delete()
                 status[student] = "un-enrolled"
+                if email_students:
+                    #User was enrolled
+                    d['email_address'] = student
+                    d['first_name'] = user.first_name
+                    d['last_name'] = user.last_name
+                    d['message'] = 'enrolled_unenroll'
+                    send_mail_ret = send_mail_to_student(student, d)
+                    status[student] += (', email sent' if send_mail_ret else '')
+
             except Exception:
                 if not isok:
                     status[student] = "Error!  Failed to un-enroll"
@@ -1173,13 +1247,48 @@ def _do_unenroll_students(course_id, students):
     return data
 
 
+def send_mail_to_student(student, param_dict):
+    """
+    Construct the email using templates and then send it.
+    `student` is the student's email address (a `str`),
+    
+    `param_dict` is a `dict` with keys [
+    `site_name`: name given to edX instance (a `str`)
+    `registration_url`: url for registration (a `str`)
+    `course_id`: id of course (a `str`)
+    `auto_enroll`: user input option (a `str`)
+    `course_url`: url of course (a `str`)
+    `email_address`: email of student (a `str`)
+    `first_name`: student first name (a `str`)
+    `last_name`: student last name (a `str`)
+    `message`: type of email to send and template to use (a `str`)
+                                        ]
+    Returns a boolean indicating whether the email was sent successfully.
+    """
+
+    EMAIL_TEMPLATE_DICT = {'allowed_enroll': ('emails/enroll_email_allowedsubject.txt', 'emails/enroll_email_allowedmessage.txt'),
+                           'enrolled_enroll': ('emails/enroll_email_enrolledsubject.txt', 'emails/enroll_email_enrolledmessage.txt'),
+                           'allowed_unenroll': ('emails/unenroll_email_subject.txt', 'emails/unenroll_email_allowedmessage.txt'),
+                           'enrolled_unenroll': ('emails/unenroll_email_subject.txt', 'emails/unenroll_email_enrolledmessage.txt')}
+
+    subject_template, message_template = EMAIL_TEMPLATE_DICT.get(param_dict['message'], (None, None))
+    if subject_template is not None and message_template is not None:
+        subject = render_to_string(subject_template, param_dict)
+        message = render_to_string(message_template, param_dict)
+
+        # Email subject *must not* contain newlines
+        subject = ''.join(subject.splitlines())
+        send_mail(subject, message, settings.DEFAULT_FROM_EMAIL, [student], fail_silently=False)
+        return True
+    else:
+        return False
+
+
 def get_and_clean_student_list(students):
     """
     Separate out individual student email from the comma, or space separated string.
-
-    In:
-    students: string coming from the input text area
-    Return:
+    `students` is string of student emails separated by commas or returns (a `str`)
+    Returns:
     students: list of cleaned student emails
     students_lc: list of lower case cleaned student emails
     """

lms/djangoapps/instructor_task/api_helper.py

@@ -2,8 +2,6 @@ import hashlib
 import json
 import logging
 
-from django.db import transaction
-
 from celery.result import AsyncResult
 from celery.states import READY_STATES, SUCCESS, FAILURE, REVOKED
 
@@ -30,7 +28,6 @@ def _task_is_running(course_id, task_type, task_key):
     return len(runningTasks) > 0
 
 
-@transaction.autocommit
 def _reserve_task(course_id, task_type, task_key, task_input, requester):
     """
     Creates a database entry to indicate that a task is in progress.
@@ -39,9 +36,9 @@ def _reserve_task(course_id, task_type, task_key, task_input, requester):
     Includes the creation of an arbitrary value for task_id, to be
     submitted with the task call to celery.
 
-    Autocommit annotation makes sure the database entry is committed.
+    The InstructorTask.create method makes sure the InstructorTask entry is committed.
     When called from any view that is wrapped by TransactionMiddleware,
-    and thus in a "commit-on-success" transaction, this autocommit here
+    and thus in a "commit-on-success" transaction, an autocommit buried within here
     will cause any pending transaction to be committed by a successful
     save here.  Any future database operations will take place in a
     separate transaction.

lms/djangoapps/instructor_task/models.py

@@ -72,6 +72,16 @@ class InstructorTask(models.Model):
 
     @classmethod
     def create(cls, course_id, task_type, task_key, task_input, requester):
+        """
+        Create an instance of InstructorTask.
+
+        The InstructorTask.save_now method makes sure the InstructorTask entry is committed.
+        When called from any view that is wrapped by TransactionMiddleware,
+        and thus in a "commit-on-success" transaction, an autocommit buried within here
+        will cause any pending transaction to be committed by a successful
+        save here.  Any future database operations will take place in a
+        separate transaction.
+        """
         # create the task_id here, and pass it into celery:
         task_id = str(uuid4())
 
@@ -99,7 +109,16 @@ class InstructorTask(models.Model):
 
     @transaction.autocommit
     def save_now(self):
-        """Writes InstructorTask immediately, ensuring the transaction is committed."""
+        """
+        Writes InstructorTask immediately, ensuring the transaction is committed.
+
+        Autocommit annotation makes sure the database entry is committed.
+        When called from any view that is wrapped by TransactionMiddleware,
+        and thus in a "commit-on-success" transaction, this autocommit here
+        will cause any pending transaction to be committed by a successful
+        save here.  Any future database operations will take place in a
+        separate transaction.
+        """
         self.save()
 
     @staticmethod

lms/djangoapps/instructor_task/tests/test_api.py

@@ -22,7 +22,7 @@ from instructor_task.tests.test_base import (InstructorTaskTestCase,
 
 class InstructorTaskReportTest(InstructorTaskTestCase):
     """
-    Tests API and view methods that involve the reporting of status for background tasks.
+    Tests API methods that involve the reporting of status for background tasks.
     """
 
     def test_get_running_instructor_tasks(self):

lms/djangoapps/instructor_task/tests/test_integration.py

 """
-Integration Tests for LMS instructor-initiated background tasks
+Integration Tests for LMS instructor-initiated background tasks.
 
 Runs tasks on answers to course problems to validate that code
 paths actually work.

lms/djangoapps/instructor_task/tests/test_tasks.py

 """
-Unit tests for LMS instructor-initiated background tasks, 
+Unit tests for LMS instructor-initiated background tasks.
 
 Runs tasks on answers to course problems to validate that code
 paths actually work.
@@ -7,6 +7,7 @@ paths actually work.
 """
 import json
 from uuid import uuid4
+from unittest import skip
 
 from mock import Mock, patch
 
@@ -62,6 +63,7 @@ class TestInstructorTasks(InstructorTaskModuleTestCase):
                 }
 
     def _run_task_with_mock_celery(self, task_function, entry_id, task_id, expected_failure_message=None):
+        """Submit a task and mock how celery provides a current_task."""
         self.current_task = Mock()
         self.current_task.request = Mock()
         self.current_task.request.id = task_id
@@ -73,7 +75,7 @@ class TestInstructorTasks(InstructorTaskModuleTestCase):
             return task_function(entry_id, self._get_xmodule_instance_args())
 
     def _test_missing_current_task(self, task_function):
-        # run without (mock) Celery running
+        """Check that a task_function fails when celery doesn't provide a current_task."""
         task_entry = self._create_input_entry()
         with self.assertRaises(UpdateProblemModuleStateError):
             task_function(task_entry.id, self._get_xmodule_instance_args())
@@ -88,7 +90,7 @@ class TestInstructorTasks(InstructorTaskModuleTestCase):
         self._test_missing_current_task(delete_problem_state)
 
     def _test_undefined_problem(self, task_function):
-        # run with celery, but no problem defined
+        """Run with celery, but no problem defined."""
         task_entry = self._create_input_entry()
         with self.assertRaises(ItemNotFoundError):
             self._run_task_with_mock_celery(task_function, task_entry.id, task_entry.task_id)
@@ -103,7 +105,7 @@ class TestInstructorTasks(InstructorTaskModuleTestCase):
         self._test_undefined_problem(delete_problem_state)
 
     def _test_run_with_task(self, task_function, action_name, expected_num_updated):
-        # run with some StudentModules for the problem
+        """Run a task and check the number of StudentModules processed."""
         task_entry = self._create_input_entry()
         status = self._run_task_with_mock_celery(task_function, task_entry.id, task_entry.task_id)
         # check return value
@@ -118,7 +120,7 @@ class TestInstructorTasks(InstructorTaskModuleTestCase):
         self.assertEquals(entry.task_state, SUCCESS)
 
     def _test_run_with_no_state(self, task_function, action_name):
-        # run with no StudentModules for the problem
+        """Run with no StudentModules defined for the current problem."""
         self.define_option_problem(PROBLEM_URL_NAME)
         self._test_run_with_task(task_function, action_name, 0)
 
@@ -185,7 +187,7 @@ class TestInstructorTasks(InstructorTaskModuleTestCase):
                                           module_state_key=self.problem_url)
 
     def _test_reset_with_student(self, use_email):
-        # run with some StudentModules for the problem
+        """Run a reset task for one student, with several StudentModules for the problem defined."""
         num_students = 10
         initial_attempts = 3
         input_state = json.dumps({'attempts': initial_attempts})
@@ -233,8 +235,7 @@ class TestInstructorTasks(InstructorTaskModuleTestCase):
         self._test_reset_with_student(True)
 
     def _test_run_with_failure(self, task_function, expected_message):
-        # run with no StudentModules for the problem,
-        # because we will fail before entering the loop.
+        """Run a task and trigger an artificial failure with give message."""
         task_entry = self._create_input_entry()
         self.define_option_problem(PROBLEM_URL_NAME)
         with self.assertRaises(TestTaskFailure):
@@ -256,8 +257,10 @@ class TestInstructorTasks(InstructorTaskModuleTestCase):
         self._test_run_with_failure(delete_problem_state, 'We expected this to fail')
 
     def _test_run_with_long_error_msg(self, task_function):
-        # run with an error message that is so long it will require
-        # truncation (as well as the jettisoning of the traceback).
+        """
+        Run with an error message that is so long it will require
+        truncation (as well as the jettisoning of the traceback).
+        """
         task_entry = self._create_input_entry()
         self.define_option_problem(PROBLEM_URL_NAME)
         expected_message = "x" * 1500
@@ -282,9 +285,11 @@ class TestInstructorTasks(InstructorTaskModuleTestCase):
         self._test_run_with_long_error_msg(delete_problem_state)
 
     def _test_run_with_short_error_msg(self, task_function):
-        # run with an error message that is short enough to fit
-        # in the output, but long enough that the traceback won't.
-        # Confirm that the traceback is truncated.
+        """
+        Run with an error message that is short enough to fit
+        in the output, but long enough that the traceback won't.
+        Confirm that the traceback is truncated.
+        """
         task_entry = self._create_input_entry()
         self.define_option_problem(PROBLEM_URL_NAME)
         expected_message = "x" * 900
@@ -330,3 +335,43 @@ class TestInstructorTasks(InstructorTaskModuleTestCase):
         self.assertEquals(output['exception'], 'ValueError')
         self.assertTrue("Length of task output is too long" in output['message'])
         self.assertTrue('traceback' not in output)
+
+    @skip
+    def test_rescoring_unrescorable(self):
+        # TODO: this test needs to have Mako templates initialized
+        # to make sure that the creation of an XModule works.
+        input_state = json.dumps({'done': True})
+        num_students = 1
+        self._create_students_with_state(num_students, input_state)
+        task_entry = self._create_input_entry()
+        with self.assertRaises(UpdateProblemModuleStateError):
+            self._run_task_with_mock_celery(rescore_problem, task_entry.id, task_entry.task_id)
+        # check values stored in table:
+        entry = InstructorTask.objects.get(id=task_entry.id)
+        output = json.loads(entry.task_output)
+        self.assertEquals(output['exception'], "UpdateProblemModuleStateError")
+        self.assertEquals(output['message'], "Specified problem does not support rescoring.")
+        self.assertGreater(len(output['traceback']), 0)
+
+    @skip
+    def test_rescoring_success(self):
+        # TODO: this test needs to have Mako templates initialized
+        # to make sure that the creation of an XModule works.
+        input_state = json.dumps({'done': True})
+        num_students = 10
+        self._create_students_with_state(num_students, input_state)
+        task_entry = self._create_input_entry()
+        mock_instance = Mock()
+        mock_instance.rescore_problem = Mock({'success': 'correct'})
+        # TODO: figure out why this mock is not working....
+        with patch('courseware.module_render.get_module_for_descriptor_internal') as mock_get_module:
+            mock_get_module.return_value = mock_instance
+            self._run_task_with_mock_celery(rescore_problem, task_entry.id, task_entry.task_id)
+        # check return value
+        entry = InstructorTask.objects.get(id=task_entry.id)
+        output = json.loads(entry.task_output)
+        self.assertEquals(output.get('attempted'), num_students)
+        self.assertEquals(output.get('updated'), num_students)
+        self.assertEquals(output.get('total'), num_students)
+        self.assertEquals(output.get('action_name'), 'rescored')
+        self.assertGreater('duration_ms', 0)

lms/djangoapps/instructor_task/tests/test_views.py

 
 """
-Test for LMS instructor background task queue management
+Test for LMS instructor background task views.
 """
 import json
 from celery.states import SUCCESS, FAILURE, REVOKED, PENDING
@@ -18,7 +18,7 @@ from instructor_task.views import instructor_task_status, get_task_completion_in
 
 class InstructorTaskReportTest(InstructorTaskTestCase):
     """
-    Tests API and view methods that involve the reporting of status for background tasks.
+    Tests view methods that involve the reporting of status for background tasks.
     """
 
     def _get_instructor_task_status(self, task_id):
@@ -263,4 +263,3 @@ class InstructorTaskReportTest(InstructorTaskTestCase):
         succeeded, message = get_task_completion_info(instructor_task)
         self.assertFalse(succeeded)
         self.assertEquals(message, "Problem rescored for 2 of 3 students (out of 5)")
-

lms/envs/acceptance.py

@@ -24,7 +24,7 @@ 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 = {

lms/envs/cms/dev.py

@@ -21,7 +21,7 @@ modulestore_options = {
     'db': 'xmodule',
     'collection': 'modulestore',
     'fs_root': DATA_DIR,
-    'render_template': 'mitxmako.shortcuts.render_to_string'
+    'render_template': 'mitxmako.shortcuts.render_to_string',
 }
 
 MODULESTORE = {

lms/envs/dev_mongo.py

@@ -19,7 +19,7 @@ MODULESTORE = {
             'db': 'xmodule',
             'collection': 'modulestore',
             'fs_root': GITHUB_REPO_ROOT,
-            'render_template': 'mitxmako.shortcuts.render_to_string'
+            'render_template': 'mitxmako.shortcuts.render_to_string',
         }
     }
 }

lms/templates/courseware/instructor_dashboard.html

@@ -249,7 +249,7 @@ function goto( mode)
     <p>
       Then select an action:
       <input type="submit" name="action" value="Reset student's attempts">
-      %if settings.MITX_FEATURES.get('ENABLE_COURSE_BACKGROUND_TASKS'):
+      %if settings.MITX_FEATURES.get('ENABLE_INSTRUCTOR_BACKGROUND_TASKS'):
       <input type="submit" name="action" value="Rescore student's problem submission">
       %endif
     </p>
@@ -260,9 +260,9 @@ function goto( mode)
       <input type="submit" name="action" value="Delete student state for module">
     </p>
     %endif
-    %if settings.MITX_FEATURES.get('ENABLE_COURSE_BACKGROUND_TASKS'):
+    %if settings.MITX_FEATURES.get('ENABLE_INSTRUCTOR_BACKGROUND_TASKS'):
     <p>Rescoring runs in the background, and status for active tasks will appear in a table below.
-       To see status for all tasks submitted for this course and student, click on this button:
+       To see status for all tasks submitted for this problem and student, click on this button:
     </p>
     <p>
       <input type="submit" name="action" value="Show Background Task History for Student">
@@ -382,6 +382,8 @@ function goto( mode)
   <p>Enroll or un-enroll one or many students: enter emails, separated by new lines or commas;</p>
   <textarea rows="6" cols="70" name="multiple_students"></textarea>
   <p>
+  <input type="checkbox" name="email_students"> Notify students by email
+  <p>
   <input type="checkbox" name="auto_enroll"> Auto-enroll students when they activate
   <input type="submit" name="action" value="Enroll multiple students">
   <p>

lms/templates/emails/enroll_email_allowedmessage.txt

+Dear student,
+
+You have been invited to join ${course_id} at ${site_name} by a member of the course staff.
+
+To finish your registration, please visit ${registration_url} and fill out the registration form.
+% if auto_enroll:
+Once you have registered and activated your account, you will see ${course_id} listed on your dashboard.
+% else:
+Once you have registered and activated your account, visit ${course_url} to join the course.
+% endif
+
+----
+This email was automatically sent from ${site_name} to ${email_address}
\ No newline at end of file

lms/templates/emails/enroll_email_allowedsubject.txt

+You have been invited to register for ${course_id}
\ No newline at end of file

lms/templates/emails/enroll_email_enrolledmessage.txt

+Dear ${first_name} ${last_name}
+
+You have been enrolled in ${course_id} at ${site_name} by a member of the course staff. The course should now appear on your ${site_name} dashboard.
+
+To start accessing course materials, please visit ${course_url}
+
+----
+This email was automatically sent from ${site_name} to ${first_name} ${last_name}
\ No newline at end of file

lms/templates/emails/enroll_email_enrolledsubject.txt

+You have been enrolled in ${course_id}
\ No newline at end of file

lms/templates/emails/unenroll_email_allowedmessage.txt

+Dear Student,
+
+You have been un-enrolled from course ${course_id} by a member of the course staff. Please disregard the invitation previously sent.
+
+----
+This email was automatically sent from ${site_name} to ${email_address}
\ No newline at end of file

lms/templates/emails/unenroll_email_enrolledmessage.txt

+Dear ${first_name} ${last_name}
+
+You have been un-enrolled in ${course_id} at ${site_name} by a member of the course staff. The course will no longer appear on your ${site_name} dashboard.
+
+Your other courses have not been affected.
+
+----
+This email was automatically sent from  ${site_name} to ${first_name} ${last_name}
\ No newline at end of file

lms/templates/emails/unenroll_email_subject.txt

+You have been un-enrolled from ${course_id}
\ No newline at end of file

lms/templates/main.html

@@ -21,17 +21,17 @@
       <title>Home | class.stanford.edu</title>
     % else:
       <title>edX</title>
-    % endif
-  </%block>
+      
       <script type="text/javascript">
-  /* immediately break out of an iframe if coming
-     from the marketing website */
+        /* immediately break out of an iframe if coming from the marketing website */
         (function(window) {
           if (window.location !== window.top.location) {
             window.top.location = window.location;
           }
         })(this);
       </script>
+    % endif
+  </%block>
 
   <link rel="icon" type="image/x-icon" href="${static.url(settings.FAVICON_PATH)}" />
 

lms/templates/registration/password_reset_email.html

@@ -3,7 +3,7 @@
 
 {% trans "Please go to the following page and choose a new password:" %}
 {% block reset_link %}
-https://{{domain}}{% url 'django.contrib.auth.views.password_reset_confirm' uidb36=uid token=token %}
+https://{{domain}}{% url 'student.views.password_reset_confirm_wrapper' uidb36=uid token=token %}
 {% endblock %}
 
 If you didn't request this change, you can disregard this email - we have not yet reset your password.

lms/urls.py

@@ -51,7 +51,7 @@ urlpatterns = ('',  # nopep8
     url(r'^password_change_done/$', django.contrib.auth.views.password_change_done,
         name='auth_password_change_done'),
     url(r'^password_reset_confirm/(?P<uidb36>[0-9A-Za-z]+)-(?P<token>.+)/$',
-        django.contrib.auth.views.password_reset_confirm,
+        'student.views.password_reset_confirm_wrapper',
         name='auth_password_reset_confirm'),
     url(r'^password_reset_complete/$', django.contrib.auth.views.password_reset_complete,
         name='auth_password_reset_complete'),
@@ -188,7 +188,7 @@ if settings.COURSEWARE_ENABLED:
         # into the database.
         url(r'^software-licenses$', 'licenses.views.user_software_license', name="user_software_license"),
 
-        url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/xqueue/(?P<userid>[^/]*)/(?P<id>.*?)/(?P<dispatch>[^/]*)$',
+        url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/xqueue/(?P<userid>[^/]*)/(?P<mod_id>.*?)/(?P<dispatch>[^/]*)$',
             'courseware.module_render.xqueue_callback',
             name='xqueue_callback'),
         url(r'^change_setting$', 'student.views.change_setting',
@@ -438,5 +438,3 @@ if settings.DEBUG:
 #Custom error pages
 handler404 = 'static_template_view.views.render_404'
 handler500 = 'static_template_view.views.render_500'
-
-

rakelib/tests.rake

@@ -16,7 +16,7 @@ def run_tests(system, report_dir, test_id=nil, stop_on_failure=true)
     ENV['NOSE_XUNIT_FILE'] = File.join(report_dir, "nosetests.xml")
     dirs = Dir["common/djangoapps/*"] + Dir["#{system}/djangoapps/*"]
     test_id = dirs.join(' ') if test_id.nil? or test_id == ''
-    cmd = django_admin(system, :test, 'test', '--logging-clear-handlers', test_id)
+    cmd = django_admin(system, :test, 'test', '--logging-clear-handlers', '--liveserver=localhost:8000-9000', test_id)
     test_sh(run_under_coverage(cmd, system))
 end
 

requirements/edx/github.txt

@@ -10,4 +10,4 @@
 # Our libraries:
 -e git+https://github.com/edx/XBlock.git@4d8735e883#egg=XBlock
 -e git+https://github.com/edx/codejail.git@0a1b468#egg=codejail
--e git+https://github.com/edx/diff-cover.git@v0.1.2#egg=diff_cover
+-e git+https://github.com/edx/diff-cover.git@v0.1.3#egg=diff_cover

scripts/create-dev-env.sh

@@ -98,19 +98,23 @@ clone_repos() {
 
 set_base_default() {  # if PROJECT_HOME not set
     # 2 possibilities: this is from cloned repo, or not
-    # this script is in "./scripts" if a git clone
-    this_repo=$(cd "${BASH_SOURCE%/*}/.." && pwd)
-    if [[ "${this_repo##*/}" = "edx-platform" && -d "$this_repo/.git" ]]; then
-        # set BASE one-up from this_repo;
-        echo "${this_repo%/*}"
+
+    # See if remote's url is named edx-platform (this works for forks too, but
+    # not if the name was changed).
+    cd "$( dirname "${BASH_SOURCE[0]}" )" 
+    this_repo=$(basename $(git ls-remote --get-url 2>/dev/null) 2>/dev/null) ||
+        echo -n ""
+
+    if [[ "x$this_repo" = "xedx-platform.git" ]]; then
+        # We are in the edx repo and already have git installed. Let git do the
+        # work of finding base dir:
+        echo "$(dirname $(git rev-parse --show-toplevel))"
     else
         echo "$HOME/edx_all"
     fi
 }
 
 
-
-
 ### START
 
 PROG=${0##*/}