Merge pull request #284 from edx/peter-fogg/fix-default-videos

Peter fogg/fix default videos

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.
 

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 = Group.objects.get(name=groupname)
+    (group, created) = Group.objects.get_or_create(name=COURSE_CREATOR_GROUP_NAME)
+    if created:
+        group.save()
+    return _add_user_to_group(user, group)
+
+
+def _add_user_to_group(user, group):
+    """
+    This is to be called only by either a command line code path or through an app which has already
+    asserted permissions to do this action
+    """
+    if user.is_active and user.is_authenticated:
         user.groups.add(group)
         user.save()
         return True
@@ -123,11 +151,29 @@ def remove_user_from_course_group(caller, user, location, role):
 
     # see if the user is actually in that role, if not then we don't have to do anything
     if is_user_in_course_group_role(user, location, role):
-        groupname = get_course_groupname_for_role(location, role)
+        _remove_user_from_group(user, get_course_groupname_for_role(location, role))
 
-        group = Group.objects.get(name=groupname)
-        user.groups.remove(group)
-        user.save()
+
+def remove_user_from_creator_group(caller, user):
+    """
+    Removes user from the course creator group.
+
+    The caller must have staff access to perform this operation.
+    """
+    if not caller.is_active or not caller.is_authenticated or not caller.is_staff:
+        raise PermissionDenied
+
+    _remove_user_from_group(user, COURSE_CREATOR_GROUP_NAME)
+
+
+def _remove_user_from_group(user, group_name):
+    """
+    This is to be called only by either a command line code path or through an app which has already
+    asserted permissions to do this action
+    """
+    group = Group.objects.get(name=group_name)
+    user.groups.remove(group)
+    user.save()
 
 
 def is_user_in_course_group_role(user, location, role):
@@ -136,3 +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/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/envs/acceptance.py

@@ -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/test.py

@@ -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/templates/emails/activation_email_subject.txt

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

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

@@ -519,11 +519,11 @@ class CapaModule(CapaFields, XModule):
         # 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,
@@ -547,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):
         """
@@ -633,32 +634,32 @@ 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:
@@ -666,28 +667,30 @@ class CapaModule(CapaFields, XModule):
 
         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.
 
@@ -717,10 +720,9 @@ class CapaModule(CapaFields, XModule):
         return {'answers': new_answers}
 
     # Figure out if we should move these to capa_problem?
-    def get_problem(self, get):
+    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
@@ -729,27 +731,27 @@ class CapaModule(CapaFields, XModule):
         return {'html': self.get_problem_html(encapsulate=False)}
 
     @staticmethod
-    def make_dict_of_responses(get):
+    def make_dict_of_responses(data):
         """
         Make dictionary of student responses (aka "answers")
 
-        `get` is POST dictionary (Django QueryDict).
+        `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 at least one underscore
+        -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.
@@ -758,7 +760,7 @@ class CapaModule(CapaFields, XModule):
         """
         answers = dict()
 
-        for key in get:
+        for key in data:
             # e.g. input_resistor_1 ==> resistor_1
             _, _, name = key.partition('_')
 
@@ -777,9 +779,9 @@ 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
@@ -801,7 +803,7 @@ class CapaModule(CapaFields, XModule):
             'max_value': score['total'],
         })
 
-    def check_problem(self, get):
+    def check_problem(self, data):
         """
         Checks whether answers to a problem are correct
 
@@ -813,8 +815,9 @@ 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'] = convert_files_to_filenames(answers)
+
         # Too late. Cannot submit
         if self.closed():
             event_info['failure'] = 'closed'
@@ -972,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 }
@@ -982,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
@@ -1011,7 +1014,7 @@ class CapaModule(CapaFields, XModule):
         return {'success': True,
                 'msg': msg}
 
-    def reset_problem(self, get):
+    def reset_problem(self, _data):
         """
         Changes problem state to unfinished -- removes student answers,
         and causes problem to rerender itself.

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

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

@@ -27,6 +27,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,30 +618,30 @@ 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 {
@@ -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/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/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,47 +115,54 @@ class VideoDescriptor(VideoFields,
             url identifiers
         """
         video = super(VideoDescriptor, cls).from_xml(xml_data, system, org, course)
-        xml = etree.fromstring(xml_data)
-
-        display_name = xml.get('display_name')
-        if display_name:
-            video.display_name = display_name
-
-        youtube = xml.get('youtube')
-        if youtube:
-            speeds = _parse_youtube(youtube)
-            if speeds['0.75']:
-                video.youtube_id_0_75 = speeds['0.75']
-            if speeds['1.00']:
-                video.youtube_id_1_0 = speeds['1.00']
-            if speeds['1.25']:
-                video.youtube_id_1_25 = speeds['1.25']
-            if speeds['1.50']:
-                video.youtube_id_1_5 = speeds['1.50']
-
-        show_captions = xml.get('show_captions')
-        if show_captions:
-            video.show_captions = json.loads(show_captions)
-
-        source = _get_first_external(xml, 'source')
-        if source:
-            video.source = source
-
-        track = _get_first_external(xml, 'track')
-        if track:
-            video.track = track
-
-        start_time = _parse_time(xml.get('from'))
-        if start_time:
-            video.start_time = start_time
-
-        end_time = _parse_time(xml.get('to'))
-        if end_time:
-            video.end_time = end_time
-
+        _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')
+    if display_name:
+        video.display_name = display_name
+
+    youtube = xml.get('youtube')
+    if youtube:
+        speeds = _parse_youtube(youtube)
+        if speeds['0.75']:
+            video.youtube_id_0_75 = speeds['0.75']
+        if speeds['1.00']:
+            video.youtube_id_1_0 = speeds['1.00']
+        if speeds['1.25']:
+            video.youtube_id_1_25 = speeds['1.25']
+        if speeds['1.50']:
+            video.youtube_id_1_5 = speeds['1.50']
+
+    show_captions = xml.get('show_captions')
+    if show_captions:
+        video.show_captions = json.loads(show_captions)
+
+    source = _get_first_external(xml, 'source')
+    if source:
+        video.source = source
+
+    track = _get_first_external(xml, 'track')
+    if track:
+        video.track = track
+
+    start_time = _parse_time(xml.get('from'))
+    if start_time:
+        video.start_time = start_time
+
+    end_time = _parse_time(xml.get('to'))
+    if end_time:
+        video.end_time = end_time
+
+
 def _get_first_external(xmltree, tag):
     """
     Returns the src attribute of the nested `tag` in `xmltree`, if it

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/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) ->
+    # Segment.io event tracking
     if event_type in SEGMENT_IO_WHITELIST
-      # Segment.io event tracking
-      analytics.track event_type, data
+      # 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',
       event_type: event_type

jenkins/test.sh

@@ -60,9 +60,6 @@ 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
 
 bundle install

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'
+    # 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)
+    # Get the submitted data
+    data = request.POST.copy()
 
-            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}))
-
-            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/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##*/}