Merge branch 'master' into feature/kevin/edit_commentable_id

cms/djangoapps/auth/authz.py

+import logging
+import sys
+
+from django.contrib.auth.models import User, Group
+from django.core.exceptions import PermissionDenied
+
+from xmodule.modulestore import Location
+
+# define a couple of simple roles, we just need ADMIN and EDITOR now for our purposes
+ADMIN_ROLE_NAME = 'admin'
+EDITOR_ROLE_NAME = 'editor'
+
+# 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
+def get_course_groupname_for_role(location, role):
+    loc = Location(location)
+    groupname = loc.course_id  + ':' + role
+    return groupname
+
+def get_users_in_course_group_by_role(location, role):
+    groupname = get_course_groupname_for_role(location, role)
+    group = Group.objects.get(name=groupname)
+    return group.user_set.all()
+
+
+'''
+Create all permission groups for a new course and subscribe the caller into those roles
+'''
+def create_all_course_groups(creator, location):
+    create_new_course_group(creator, location, ADMIN_GROUP_NAME)
+    create_new_course_group(creator, location, EDITOR_GROUP_NAME)
+
+
+def create_new_course_group(creator, location, role):
+    groupname = get_course_groupname_for_role(location, role)
+    (group, created) =Group.get_or_create(name=groupname)
+    if created:
+        group.save()
+
+    creator.groups.add(group)
+    creator.save()
+
+    return
+
+
+def add_user_to_course_group(caller, user, location, role):
+    # only admins can add/remove other users
+    if not is_user_in_course_group_role(caller, location, ADMIN_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=groupname)
+        user.groups.add(group)
+        user.save()
+        return True
+
+    return False
+
+
+def get_user_by_email(email):
+    user = None
+    # try to look up user, return None if not found
+    try:
+        user = User.objects.get(email=email)
+    except:
+        pass
+
+    return user
+
+
+def remove_user_from_course_group(caller, user, location, role):
+    # only admins can add/remove other users
+    if not is_user_in_course_group_role(caller, location, ADMIN_ROLE_NAME):
+        raise PermissionDenied
+
+    # 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) == True:
+        groupname = get_course_groupname_for_role(location, role)
+
+        group = Group.objects.get(name=groupname)
+        user.groups.remove(group)
+        user.save()
+
+
+def is_user_in_course_group_role(user, location, role):
+    if user.is_active and user.is_authenticated:
+        return user.groups.filter(name=get_course_groupname_for_role(location,role)).count() > 0
+
+    return False
+
+    

cms/djangoapps/contentstore/utils.py

+from xmodule.modulestore import Location
+from xmodule.modulestore.django import modulestore
+
+'''
+cdodge: for a given Xmodule, return the course that it belongs to
+NOTE: This makes a lot of assumptions about the format of the course location
+Also we have to assert that this module maps to only one course item - it'll throw an
+assert if not
+'''
+def get_course_location_for_item(location):
+    item_loc = Location(location)
+
+    # check to see if item is already a course, if so we can skip this
+    if item_loc.category != 'course':
+        # @hack! We need to find the course location however, we don't
+        # know the 'name' parameter in this context, so we have
+        # to assume there's only one item in this query even though we are not specifying a name
+        course_search_location = ['i4x', item_loc.org, item_loc.course, 'course', None]
+        courses = modulestore().get_items(course_search_location)
+
+        # make sure we found exactly one match on this above course search
+        found_cnt = len(courses)
+        if found_cnt == 0:
+            raise BaseException('Could not find course at {0}'.format(course_search_location))
+
+        if found_cnt > 1:
+            raise BaseException('Found more than one course at {0}. There should only be one!!!'.format(course_search_location))
+
+        location = courses[0].location
+
+    return location

cms/djangoapps/contentstore/views.py

@@ -5,6 +5,7 @@ import logging
 import sys
 import mimetypes
 import StringIO
+import exceptions
 from collections import defaultdict
 
 # to install PIL on MacOSX: 'easy_install http://dist.repoze.org/PIL-1.1.6.tar.gz'
@@ -12,6 +13,7 @@ from PIL import Image
 
 from django.http import HttpResponse, Http404, HttpResponseBadRequest, HttpResponseForbidden
 from django.contrib.auth.decorators import login_required
+from django.core.exceptions import PermissionDenied
 from django.core.context_processors import csrf
 from django_future.csrf import ensure_csrf_cookie
 from django.core.urlresolvers import reverse
@@ -36,9 +38,11 @@ from operator import attrgetter
 from xmodule.contentstore.django import contentstore
 from xmodule.contentstore.content import StaticContent
 
-#from django.core.cache import cache
-
 from cache_toolbox.core import set_cached_content, get_cached_content, del_cached_content
+from auth.authz import is_user_in_course_group_role, get_users_in_course_group_by_role
+from auth.authz import get_user_by_email, add_user_to_course_group, remove_user_from_course_group
+from auth.authz import ADMIN_ROLE_NAME, EDITOR_ROLE_NAME
+from .utils import get_course_location_for_item
 
 log = logging.getLogger(__name__)
 
@@ -72,6 +76,10 @@ def index(request):
     List all courses available to the logged in user
     """
     courses = modulestore().get_items(['i4x', None, None, 'course', None])
+
+    # filter out courses that we don't have access to
+    courses = filter(lambda course: has_access(request.user, course.location), courses)
+
     return render_to_response('index.html', {
         'courses': [(course.metadata.get('display_name'),
                     reverse('course_index', args=[
@@ -84,10 +92,10 @@ def index(request):
 
 # ==== Views with per-item permissions================================
 
-def has_access(user, location):
+def has_access(user, location, role=EDITOR_ROLE_NAME):
     '''Return True if user allowed to access this piece of data'''
-    # TODO (vshnayder): actually check perms
-    return user.is_active and user.is_authenticated
+    '''Note that the CMS permissions model is with respect to courses'''
+    return is_user_in_course_group_role(user, get_course_location_for_item(location), role)
 
 
 @login_required
@@ -99,8 +107,10 @@ def course_index(request, org, course, name):
     org, course, name: Attributes of the Location for the item to edit
     """
     location = ['i4x', org, course, 'course', name]
+    
+    # check that logged in user has permissions to this item
     if not has_access(request.user, location):
-        raise Http404  # TODO (vshnayder): better error
+        raise PermissionDenied()
 
     # TODO (cpennington): These need to be read in from the active user
     _course = modulestore().get_item(location)
@@ -134,10 +144,12 @@ def edit_item(request):
 
     id: A Location URL
     """
-    # TODO (vshnayder): change name from id to location in coffee+html as well.
+    
     item_location = request.GET['id']
+
+    # check that we have permissions to edit this item
     if not has_access(request.user, item_location):
-        raise Http404  # TODO (vshnayder): better error
+        raise PermissionDenied()
 
     item = modulestore().get_item(item_location)
     item.get_html = wrap_xmodule(item.get_html, item, "xmodule_edit.html")
@@ -359,8 +371,10 @@ def get_module_previews(request, descriptor):
 @expect_json
 def save_item(request):
     item_location = request.POST['id']
+
+    # check permissions for this user within this course
     if not has_access(request.user, item_location):
-        raise Http404  # TODO (vshnayder): better error
+        raise PermissionDenied()
 
     if request.POST['data']:
         data = request.POST['data']
@@ -407,7 +421,7 @@ def clone_item(request):
     display_name = request.POST['name']
 
     if not has_access(request.user, parent_location):
-        raise Http404  # TODO (vshnayder): better error
+        raise PermissionDenied()
 
     parent = modulestore().get_item(parent_location)
     dest_location = parent_location._replace(category=template.category, name=Location.clean_for_url_name(display_name))
@@ -513,3 +527,84 @@ def upload_asset(request, org, course, coursename):
 
     return HttpResponse('Upload completed')
 
+'''
+This view will return all CMS users who are editors for the specified course
+'''
+@login_required
+@ensure_csrf_cookie
+def manage_users(request, org, course, name):
+    location = ['i4x', org, course, 'course', name]
+    
+    # check that logged in user has permissions to this item
+    if not has_access(request.user, location, role=ADMIN_ROLE_NAME):
+        raise PermissionDenied()
+
+    return render_to_response('manage_users.html', {
+        'editors': get_users_in_course_group_by_role(location, EDITOR_ROLE_NAME)
+    })
+    
+
+def create_json_response(errmsg = None):
+    if errmsg is not None:
+        resp = HttpResponse(json.dumps({'Status': 'Failed', 'ErrMsg' : errmsg}))
+    else:
+        resp = HttpResponse(json.dumps({'Status': 'OK'}))
+
+    return resp
+
+'''
+This POST-back view will add a user - specified by email - to the list of editors for
+the specified course
+'''
+@login_required
+@ensure_csrf_cookie
+def add_user(request, org, course, name):
+    email = request.POST["email"]
+
+    if email=='':
+        return create_json_response('Please specify an email address.')
+
+    location = ['i4x', org, course, 'course', name]
+    
+    # check that logged in user has admin permissions to this course
+    if not has_access(request.user, location, role=ADMIN_ROLE_NAME):
+        raise PermissionDenied()
+    
+    user = get_user_by_email(email)
+    
+    # user doesn't exist?!? Return error.
+    if user is None:
+        return create_json_response('Could not find user by email address \'{0}\'.'.format(email))
+
+    # user exists, but hasn't activated account?!?
+    if not user.is_active:
+        return create_json_response('User {0} has registered but has not yet activated his/her account.'.format(email))
+
+    # ok, we're cool to add to the course group
+    add_user_to_course_group(request.user, user, location, EDITOR_ROLE_NAME)
+
+    return create_json_response()
+
+'''
+This POST-back view will remove a user - specified by email - from the list of editors for
+the specified course
+'''
+@login_required
+@ensure_csrf_cookie
+def remove_user(request, org, course, name):
+    email = request.POST["email"]
+
+    location = ['i4x', org, course, 'course', name]
+    
+    # check that logged in user has admin permissions on this course
+    if not has_access(request.user, location, role=ADMIN_ROLE_NAME):
+        raise PermissionDenied()
+
+    user = get_user_by_email(email)
+    if user is None:
+        return create_json_response('Could not find user by email address \'{0}\'.'.format(email))
+
+    remove_user_from_course_group(request.user, user, location, EDITOR_ROLE_NAME)
+
+    return create_json_response()
+

cms/envs/common.py

@@ -325,6 +325,7 @@ INSTALLED_APPS = (
 
     # For CMS
     'contentstore',
+    'auth',
     'github_sync',
     'student',  # misleading name due to sharing with lms
 

cms/templates/manage_users.html

+<%inherit file="base.html" />
+<%block name="title">Course Editor Manager</%block>
+<%include file="widgets/header.html"/>
+
+<%block name="content">
+<section class="main-container">
+
+  <h2>Course Editors</h2>
+  <ul>
+    % for user in editors:
+    <li>${user.email} (${user.username})</li>
+    % endfor
+  </ul>
+
+  <form action="add_user" id="addEditorsForm">
+    <label>email:&nbsp;</label><input type="text" name="email" placeholder="email@example.com..." />
+    <input type="submit" value="add editor" />
+  </form>
+  <div id="result"></div>
+
+<script>
+  $("#addEditorsForm").submit(function(event) {
+  event.preventDefault();
+  var $form = $(this),
+  email = $form.find('input[name="email"]').val(),
+  url = $form.attr('action');
+  $.post(url, {email:email},
+  function(data) {
+  if(data['Status'] != 'OK')
+  $("#result").empty().append(data['ErrMsg']);
+  else
+  location.reload();
+  });
+  });
+</script>
+
+</section>
+</%block>

cms/urls.py

@@ -19,7 +19,14 @@ urlpatterns = ('',
     url(r'^preview/modx/(?P<preview_id>[^/]*)/(?P<location>.*?)/(?P<dispatch>[^/]*)$',
         'contentstore.views.preview_dispatch', name='preview_dispatch'),
     url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/course/(?P<coursename>[^/]+)/upload_asset$', 
-        'contentstore.views.upload_asset', name='upload_asset')
+        'contentstore.views.upload_asset', name='upload_asset'),
+    url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/course/(?P<name>[^/]+)/manage_users$',
+        'contentstore.views.manage_users', name='manage_users'),
+    url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/course/(?P<name>[^/]+)/add_user$',
+        'contentstore.views.add_user', name='add_user'),
+    url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/course/(?P<name>[^/]+)/remove_user$',
+        'contentstore.views.remove_user', name='remove_user')
+
 )
 
 # User creation and updating views

common/djangoapps/external_auth/djangostore.py

+"""A openid store using django cache"""
+
+from openid.store.interface import OpenIDStore
+from openid.store import nonce
+
+from django.core.cache import cache
+
+import logging
+import time
+
+DEFAULT_ASSOCIATIONS_TIMEOUT = 60
+DEFAULT_NONCE_TIMEOUT = 600
+
+ASSOCIATIONS_KEY_PREFIX = 'openid.provider.associations.'
+NONCE_KEY_PREFIX = 'openid.provider.nonce.'
+
+log = logging.getLogger('DjangoOpenIDStore')
+
+
+def get_url_key(server_url):
+    key = ASSOCIATIONS_KEY_PREFIX + server_url
+    return key
+
+
+def get_nonce_key(server_url, timestamp, salt):
+    key = '{prefix}{url}.{ts}.{salt}'.format(prefix=NONCE_KEY_PREFIX,
+                                             url=server_url,
+                                             ts=timestamp,
+                                             salt=salt)
+    return key
+
+
+class DjangoOpenIDStore(OpenIDStore):
+    def __init__(self):
+        log.info('DjangoStore cache:' + str(cache.__class__))
+
+    def storeAssociation(self, server_url, assoc):
+        key = get_url_key(server_url)
+
+        log.info('storeAssociation {0}'.format(key))
+
+        associations = cache.get(key, {})
+        associations[assoc.handle] = assoc
+
+        cache.set(key, associations, DEFAULT_ASSOCIATIONS_TIMEOUT)
+
+    def getAssociation(self, server_url, handle=None):
+        key = get_url_key(server_url)
+
+        log.info('getAssociation {0}'.format(key))
+
+        associations = cache.get(key, {})
+
+        assoc = None
+
+        if handle is None:
+            # get best association
+            valid_assocs = [a for a in associations if a.getExpiresIn() > 0]
+            if valid_assocs:
+                valid_assocs.sort(lambda a: a.getExpiresIn(), reverse=True)
+                assoc = valid_assocs.sort[0]
+        else:
+            assoc = associations.get(handle)
+
+        # check expiration and remove if it has expired
+        if assoc and assoc.getExpiresIn() <= 0:
+            if handle is None:
+                cache.delete(key)
+            else:
+                associations.pop(handle)
+                cache.set(key, associations, DEFAULT_ASSOCIATIONS_TIMEOUT)
+            assoc = None
+
+        return assoc
+
+    def removeAssociation(self, server_url, handle):
+        key = get_url_key(server_url)
+
+        log.info('removeAssociation {0}'.format(key))
+
+        associations = cache.get(key, {})
+
+        removed = False
+
+        if associations:
+            if handle is None:
+                cache.delete(key)
+                removed = True
+            else:
+                assoc = associations.pop(handle)
+                if assoc:
+                    cache.set(key, associations, DEFAULT_ASSOCIATIONS_TIMEOUT)
+                    removed = True
+
+        return removed
+
+    def useNonce(self, server_url, timestamp, salt):
+        key = get_nonce_key(server_url, timestamp, salt)
+
+        log.info('useNonce {0}'.format(key))
+
+        if abs(timestamp - time.time()) > nonce.SKEW:
+            return False
+
+        anonce = cache.get(key)
+
+        found = False
+
+        if anonce is None:
+            cache.set(key, '-', DEFAULT_NONCE_TIMEOUT)
+            found = False
+        else:
+            found = True
+
+        return found
+
+    def cleanupNonces(self):
+        # not necesary, keys will timeout
+        return 0
+
+    def cleanupAssociations(self):
+        # not necesary, keys will timeout
+        return 0

common/djangoapps/external_auth/views.py

@@ -7,6 +7,7 @@ import string
 import fnmatch
 
 from external_auth.models import ExternalAuthMap
+from external_auth.djangostore import DjangoOpenIDStore
 
 from django.conf import settings
 from django.contrib.auth import REDIRECT_FIELD_NAME, authenticate, login
@@ -30,7 +31,6 @@ from openid.consumer.consumer import SUCCESS
 
 from openid.server.server import Server
 from openid.server.trustroot import TrustRoot
-from openid.store.filestore import FileOpenIDStore
 from openid.extensions import ax, sreg
 
 import student.views as student_views
@@ -271,10 +271,7 @@ def get_xrds_url(resource, request):
     """
     Return the XRDS url for a resource
     """
-    host = request.META['HTTP_HOST']
-
-    if not host.endswith('edx.org'):
-        return None
+    host = request.get_host()
 
     location = host + '/openid/provider/' + resource + '/'
 
@@ -296,6 +293,8 @@ def add_openid_simple_registration(request, response, data):
                 sreg_data['email'] = data['email']
             elif field == 'fullname' and 'fullname' in data:
                 sreg_data['fullname'] = data['fullname']
+            elif field == 'nickname' and 'nickname' in data:
+                sreg_data['nickname'] = data['nickname']
 
         # construct sreg response
         sreg_response = sreg.SRegResponse.extractResponse(sreg_request,
@@ -400,7 +399,7 @@ def provider_login(request):
         return default_render_failure(request, "Invalid OpenID request")
 
     # initialize store and server
-    store = FileOpenIDStore('/tmp/openid_provider')
+    store = DjangoOpenIDStore()
     server = Server(store, endpoint)
 
     # handle OpenID request
@@ -489,13 +488,22 @@ def provider_login(request):
             url = endpoint + urlquote(user.username)
             response = openid_request.answer(True, None, url)
 
-            return provider_respond(server,
-                                    openid_request,
-                                    response,
-                                    {
-                                        'fullname': profile.name,
-                                        'email': user.email
-                                    })
+            # TODO: for CS50 we are forcibly returning the username
+            # instead of fullname. In the OpenID simple registration
+            # extension, we don't have to return any fields we don't
+            # want to, even if they were marked as required by the
+            # Consumer. The behavior of what to do when there are
+            # missing fields is up to the Consumer. The proper change
+            # should only return the username, however this will likely
+            # break the CS50 client. Temporarily we will be returning
+            # username filling in for fullname in addition to username 
+            # as sreg nickname.
+            results = {
+                'nickname': user.username,
+                'email': user.email,
+                'fullname': user.username
+                }
+            return provider_respond(server, openid_request, response, results)
 
         request.session['openid_error'] = True
         msg = "Login failed - Account not active for user {0}".format(username)

common/djangoapps/status/__init__.py

+

common/djangoapps/status/status.py

+"""
+A tiny app that checks for a status message.
+"""
+
+from django.conf import settings
+import json
+import logging
+import os
+import sys
+
+log = logging.getLogger(__name__)
+
+def get_site_status_msg(course_id):
+    """
+    Look for a file settings.STATUS_MESSAGE_PATH.  If found, read it,
+    parse as json, and do the following:
+
+    * if there is a key 'global', include that in the result list.
+    * if course is not None, and there is a key for course.id, add that to the result list.
+    * return "<br/>".join(result)
+
+    Otherwise, return None.
+
+    If something goes wrong, returns None.  ("is there a status msg?" logic is
+    not allowed to break the entire site).
+    """
+    try:
+        if os.path.isfile(settings.STATUS_MESSAGE_PATH):
+            with open(settings.STATUS_MESSAGE_PATH) as f:
+                content = f.read()
+        else:
+            return None
+
+        status_dict = json.loads(content)
+        msg = status_dict.get('global', None)
+        if course_id in status_dict:
+            msg = msg + "<br>" if msg else ''
+            msg += status_dict[course_id]
+
+        return msg
+    except:
+        log.exception("Error while getting a status message.")
+        return None

common/djangoapps/status/tests.py

+from django.conf import settings
+from django.test import TestCase
+import os
+from override_settings import override_settings
+from tempfile import NamedTemporaryFile
+
+from status import get_site_status_msg
+
+# Get a name where we can put test files
+TMP_FILE = NamedTemporaryFile(delete=False)
+TMP_NAME = TMP_FILE.name
+# Close it--we just want the path.
+TMP_FILE.close()
+
+
+@override_settings(STATUS_MESSAGE_PATH=TMP_NAME)
+class TestStatus(TestCase):
+    """Test that the get_site_status_msg function does the right thing"""
+
+    no_file = None
+
+    invalid_json = """{
+        "global" : "Hello, Globe",
+        }"""
+
+    global_only = """{
+        "global" : "Hello, Globe"
+        }"""
+
+    toy_only = """{
+        "edX/toy/2012_Fall" : "A toy story"
+        }"""
+
+    global_and_toy = """{
+        "global" : "Hello, Globe",
+        "edX/toy/2012_Fall" : "A toy story"
+        }"""
+
+
+    # json to use, expected results for course=None (e.g. homepage),
+    # for toy course, for full course.  Note that get_site_status_msg
+    # is supposed to return global message even if course=None.  The
+    # template just happens to not display it outside the courseware
+    # at the moment...
+    checks = [
+        (no_file, None, None, None),
+        (invalid_json, None, None, None),
+        (global_only, "Hello, Globe", "Hello, Globe", "Hello, Globe"),
+        (toy_only, None, "A toy story", None),
+        (global_and_toy, "Hello, Globe", "Hello, Globe<br>A toy story", "Hello, Globe"),
+        ]
+
+    def setUp(self):
+        """
+        Fake course ids, since we don't have to have full django
+        settings (common tests run without the lms settings imported)
+        """
+        self.full_id = 'edX/full/2012_Fall'
+        self.toy_id = 'edX/toy/2012_Fall'
+
+    def create_status_file(self, contents):
+        """
+        Write contents to settings.STATUS_MESSAGE_PATH.
+        """
+        with open(settings.STATUS_MESSAGE_PATH, 'w') as f:
+            f.write(contents)
+
+    def remove_status_file(self):
+        """Delete the status file if it exists"""
+        if os.path.exists(settings.STATUS_MESSAGE_PATH):
+            os.remove(settings.STATUS_MESSAGE_PATH)
+
+    def tearDown(self):
+        self.remove_status_file()
+
+    def test_get_site_status_msg(self):
+        """run the tests"""
+        for (json_str, exp_none, exp_toy, exp_full) in self.checks:
+
+            self.remove_status_file()
+            if json_str:
+                self.create_status_file(json_str)
+
+            print "checking results for {0}".format(json_str)
+            print "course=None:"
+            self.assertEqual(get_site_status_msg(None), exp_none)
+            print "course=toy:"
+            self.assertEqual(get_site_status_msg(self.toy_id), exp_toy)
+            print "course=full:"
+            self.assertEqual(get_site_status_msg(self.full_id), exp_full)

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

+from optparse import make_option
+
+from django.core.management.base import BaseCommand, CommandError
+from django.contrib.auth.models import User, Group
+
+class Command(BaseCommand):
+    option_list = BaseCommand.option_list + (
+        make_option('--list',
+                    action='store_true',
+                    dest='list',
+                    default=False,
+                    help='List available groups'),
+        make_option('--create',
+                    action='store_true',
+                    dest='create',
+                    default=False,
+                    help='Create the group if it does not exist'),
+        make_option('--remove',
+                    action='store_true',
+                    dest='remove',
+                    default=False,
+                    help='Remove the user from the group instead of adding it'),
+        )
+
+    args = '<user|email> <group>'
+    help = 'Add a user to a group'
+
+    def print_groups(self):
+        print 'Groups available:'
+        for group in Group.objects.all().distinct():
+            print '  ', group.name
+
+    def handle(self, *args, **options):
+        if options['list']:
+            self.print_groups()
+            return
+
+        if len(args) != 2:
+            raise CommandError('Usage is add_to_group {0}'.format(self.args))
+
+        name_or_email, group_name = args
+
+        if '@' in name_or_email:
+            user = User.objects.get(email=name_or_email)
+        else:
+            user = User.objects.get(username=name_or_email)
+
+        try:
+            group = Group.objects.get(name=group_name)
+        except Group.DoesNotExist:
+            if options['create']:
+                group = Group(name=group_name)
+                group.save()
+            else:
+                raise CommandError('Group {} does not exist'.format(group_name))
+
+        if options['remove']:
+            user.groups.remove(group)
+        else:
+            user.groups.add(group)
+
+        print 'Success!'

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

+from optparse import make_option
+
+from django.contrib.auth.models import User
+from django.core.management.base import BaseCommand, CommandError
+import re
+
+
+class Command(BaseCommand):
+    option_list = BaseCommand.option_list + (
+        make_option('--unset',
+                    action='store_true',
+                    dest='unset',
+                    default=False,
+                    help='Set is_staff to False instead of True'),
+        )
+
+    args = '<user|email> [user|email ...]>'
+    help = """
+    This command will set is_staff to true for one or more users.
+    Lookup by username or email address, assumes usernames
+    do not look like email addresses.
+    """
+
+    def handle(self, *args, **options):
+        if len(args) < 1:
+            raise CommandError('Usage is set_staff {0}'.format(self.args))
+
+        for user in args:
+            if re.match('[^@]+@[^@]+\.[^@]+', user):
+                try:
+                    v = User.objects.get(email=user)
+                except:
+                    raise CommandError("User {0} does not exist".format(user))
+            else:
+                try:
+                    v = User.objects.get(username=user)
+                except:
+                    raise CommandError("User {0} does not exist".format(user))
+
+            if options['unset']:
+                v.is_staff = False
+            else:
+                v.is_staff = True
+
+            v.save()
+
+        print 'Success!'

common/djangoapps/student/views.py

@@ -39,6 +39,8 @@ from collections import namedtuple
 from courseware.courses import get_courses_by_university
 from courseware.access import has_access
 
+from statsd import statsd
+
 log = logging.getLogger("mitx.student")
 Article = namedtuple('Article', 'title url author image deck publication publish_date')
 
@@ -204,7 +206,13 @@ def change_enrollment(request):
             return {'success': False,
                     'error': 'enrollment in {} not allowed at this time'
                     .format(course.display_name)}
-
+                    
+        org, course_num, run=course_id.split("/")        
+        statsd.increment("common.student.enrollment",
+                        tags=["org:{0}".format(org),
+                              "course:{0}".format(course_num),
+                              "run:{0}".format(run)])
+        
         enrollment, created = CourseEnrollment.objects.get_or_create(user=user, course_id=course.id)
         return {'success': True}
 
@@ -212,6 +220,13 @@ def change_enrollment(request):
         try:
             enrollment = CourseEnrollment.objects.get(user=user, course_id=course_id)
             enrollment.delete()
+            
+            org, course_num, run=course_id.split("/")        
+            statsd.increment("common.student.unenrollment",
+                            tags=["org:{0}".format(org),
+                                  "course:{0}".format(course_num),
+                                  "run:{0}".format(run)])
+                              
             return {'success': True}
         except CourseEnrollment.DoesNotExist:
             return {'success': False, 'error': 'You are not enrolled for this course.'}
@@ -260,12 +275,19 @@ def login_user(request, error=""):
         log.info("Login success - {0} ({1})".format(username, email))
 
         try_change_enrollment(request)
-
+        
+        statsd.increment("common.student.successful_login")
+        
         return HttpResponse(json.dumps({'success': True}))
-
-    log.warning("Login failed - Account not active for user {0}".format(username))
+    
+    log.warning("Login failed - Account not active for user {0}, resending activation".format(username))
+    
+    reactivation_email_for_user(user)
+    not_activated_msg = "This account has not been activated. We have " + \
+                        "sent another activation message. Please check your " + \
+                        "e-mail for the activation instructions."
     return HttpResponse(json.dumps({'success': False,
-                                    'value': 'This account has not been activated. Please check your e-mail for the activation instructions.'}))
+                                    'value': not_activated_msg}))
 
 
 @ensure_csrf_cookie
@@ -461,7 +483,9 @@ def create_account(request, post_override=None):
             log.debug('bypassing activation email')
             login_user.is_active = True
             login_user.save()
-
+            
+    statsd.increment("common.student.account_created")
+    
     js = {'success': True}
     return HttpResponse(json.dumps(js), mimetype="application/json")
 
@@ -517,6 +541,17 @@ def password_reset(request):
     ''' Attempts to send a password reset e-mail. '''
     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 whome 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)
     if form.is_valid():
         form.save(use_https = request.is_secure(),
@@ -529,7 +564,6 @@ def password_reset(request):
         return HttpResponse(json.dumps({'success': False,
                                         'error': 'Invalid e-mail'}))
 
-
 @ensure_csrf_cookie
 def reactivation_email(request):
     ''' Send an e-mail to reactivate a deactivated account, or to
@@ -540,25 +574,22 @@ def reactivation_email(request):
     except User.DoesNotExist:
         return HttpResponse(json.dumps({'success': False,
                                         'error': 'No inactive user with this e-mail exists'}))
+    return reactivation_email_for_user(user)
 
-    if user.is_active:
-        return HttpResponse(json.dumps({'success': False,
-                                        'error': 'User is already active'}))
-
+def reactivation_email_for_user(user):
     reg = Registration.objects.get(user=user)
-    reg.register(user)
 
-    d = {'name': UserProfile.get(user=user).name,
-       'key': r.activation_key}
+    d = {'name': user.profile.name,
+         'key': reg.activation_key}
 
-    subject = render_to_string('reactivation_email_subject.txt', d)
+    subject = render_to_string('emails/activation_email_subject.txt', d)
     subject = ''.join(subject.splitlines())
-    message = render_to_string('reactivation_email.txt', d)
+    message = render_to_string('emails/activation_email.txt', d)
 
     res = user.email_user(subject, message, settings.DEFAULT_FROM_EMAIL)
 
     return HttpResponse(json.dumps({'success': True}))
-
+    
 
 @ensure_csrf_cookie
 def change_email_request(request):
@@ -642,9 +673,12 @@ def confirm_email_change(request, key):
     meta['old_emails'].append([user.email, datetime.datetime.now().isoformat()])
     up.set_meta(meta)
     up.save()
+    # Send it to the old email...
+    user.email_user(subject, message, settings.DEFAULT_FROM_EMAIL)
     user.email = pec.new_email
     user.save()
     pec.delete()
+    # And send it to the new email...
     user.email_user(subject, message, settings.DEFAULT_FROM_EMAIL)
 
     return render_to_response("email_change_successful.html", d)
@@ -665,9 +699,12 @@ def change_name_request(request):
     pnc.rationale = request.POST['rationale']
     if len(pnc.new_name) < 2:
         return HttpResponse(json.dumps({'success': False, 'error': 'Name required'}))
-    if len(pnc.rationale) < 2:
-        return HttpResponse(json.dumps({'success': False, 'error': 'Rationale required'}))
     pnc.save()
+
+    # The following automatically accepts name change requests. Remove this to
+    # go back to the old system where it gets queued up for admin approval.
+    accept_name_change_by_id(pnc.id)
+
     return HttpResponse(json.dumps({'success': True}))
 
 
@@ -702,14 +739,9 @@ def reject_name_change(request):
     return HttpResponse(json.dumps({'success': True}))
 
 
-@ensure_csrf_cookie
-def accept_name_change(request):
-    ''' JSON: Name change process. Course staff clicks 'accept' on a given name change '''
-    if not request.user.is_staff:
-        raise Http404
-
+def accept_name_change_by_id(id):
     try:
-        pnc = PendingNameChange.objects.get(id=int(request.POST['id']))
+        pnc = PendingNameChange.objects.get(id=id)
     except PendingNameChange.DoesNotExist:
         return HttpResponse(json.dumps({'success': False, 'error': 'Invalid ID'}))
 
@@ -728,3 +760,17 @@ def accept_name_change(request):
     pnc.delete()
 
     return HttpResponse(json.dumps({'success': True}))
+
+
+@ensure_csrf_cookie
+def accept_name_change(request):
+    ''' JSON: Name change process. Course staff clicks 'accept' on a given name change 
+    
+    We used this during the prototype but now we simply record name changes instead
+    of manually approving them. Still keeping this around in case we want to go
+    back to this approval method.
+    '''
+    if not request.user.is_staff:
+        raise Http404
+
+    return accept_name_change_by_id(int(request.POST['id']))

common/lib/capa/capa/calc.py

@@ -48,7 +48,7 @@ general_whitespace = re.compile('[^\w]+')
 
 
 def check_variables(string, variables):
-    '''  Confirm the only variables in string are defined.
+    '''Confirm the only variables in string are defined.
 
     Pyparsing uses a left-to-right parser, which makes the more
     elegant approach pretty hopeless.
@@ -56,7 +56,8 @@ def check_variables(string, variables):
     achar = reduce(lambda a,b:a|b ,map(Literal,alphas)) # Any alphabetic character
     undefined_variable = achar + Word(alphanums)
     undefined_variable.setParseAction(lambda x:UndefinedVariable("".join(x)).raiseself())
-    varnames = varnames | undefined_variable'''
+    varnames = varnames | undefined_variable
+    '''
     possible_variables = re.split(general_whitespace, string)  # List of all alnums in string
     bad_variables = list()
     for v in possible_variables:
@@ -71,7 +72,8 @@ def check_variables(string, variables):
 
 
 def evaluator(variables, functions, string, cs=False):
-    ''' Evaluate an expression. Variables are passed as a dictionary
+    '''
+    Evaluate an expression. Variables are passed as a dictionary
     from string to value. Unary functions are passed as a dictionary
     from string to function. Variables must be floats.
     cs: Case sensitive
@@ -108,6 +110,7 @@ def evaluator(variables, functions, string, cs=False):
 
     if string.strip() == "":
         return float('nan')
+
     ops = {"^": operator.pow,
             "*": operator.mul,
             "/": operator.truediv,
@@ -169,14 +172,19 @@ def evaluator(variables, functions, string, cs=False):
     def func_parse_action(x):
         return [all_functions[x[0]](x[1])]
 
-    number_suffix = reduce(lambda a, b: a | b, map(Literal, suffixes.keys()), NoMatch())  # SI suffixes and percent
+    # SI suffixes and percent
+    number_suffix = reduce(lambda a, b: a | b, map(Literal, suffixes.keys()), NoMatch())
     (dot, minus, plus, times, div, lpar, rpar, exp) = map(Literal, ".-+*/()^")
 
     number_part = Word(nums)
-    inner_number = (number_part + Optional("." + number_part)) | ("." + number_part)  # 0.33 or 7 or .34
-    number = Optional(minus | plus) + inner_number + \
-        Optional(CaselessLiteral("E") + Optional("-") + number_part) + \
-        Optional(number_suffix)  # 0.33k or -17
+
+    # 0.33 or 7 or .34
+    inner_number = (number_part + Optional("." + number_part)) | ("." + number_part)
+
+    # 0.33k or -17
+    number = (Optional(minus | plus) + inner_number
+              + Optional(CaselessLiteral("E") + Optional("-") + number_part)
+              + Optional(number_suffix))
     number = number.setParseAction(number_parse_action)  # Convert to number
 
     # Predefine recursive variables
@@ -201,9 +209,11 @@ def evaluator(variables, functions, string, cs=False):
         varnames.setParseAction(lambda x: map(lambda y: all_variables[y], x))
     else:
         varnames = NoMatch()
+
     # Same thing for functions.
     if len(all_functions) > 0:
-        funcnames = sreduce(lambda x, y: x | y, map(lambda x: CasedLiteral(x), all_functions.keys()))
+        funcnames = sreduce(lambda x, y: x | y,
+                            map(lambda x: CasedLiteral(x), all_functions.keys()))
         function = funcnames + lpar.suppress() + expr + rpar.suppress()
         function.setParseAction(func_parse_action)
     else:

common/lib/capa/capa/chem/__init__.py

+

common/lib/capa/capa/chem/chemtools.py

+from collections import OrderedDict
+import json
+import unittest
+
+
+def vsepr_parse_user_answer(user_input):
+    d = OrderedDict(json.loads(user_input))
+    d['atoms'] = OrderedDict(sorted(d['atoms'].items()))
+    return d
+
+
+def vsepr_build_correct_answer(geometry, atoms):
+    correct_answer = OrderedDict()
+    correct_answer['geometry'] = geometry
+    correct_answer['atoms'] = OrderedDict(sorted(atoms.items()))
+    return correct_answer
+
+
+def vsepr_grade(user_input, correct_answer, ignore_p_order=False, ignore_a_order=False, ignore_e_order=False):
+    """ Flags ignore_(a,p,e)_order are for checking order in axial, perepherial or equatorial positions.
+        Allowed cases:
+            c0, a, e
+            c0, p
+        Not implemented and not tested cases when p with a or e (no need for now)
+    """
+    # print user_input, type(user_input)
+    # print correct_answer, type(correct_answer)
+    if user_input['geometry'] != correct_answer['geometry']:
+        return False
+
+    if user_input['atoms']['c0'] != correct_answer['atoms']['c0']:
+        return False
+
+    # not order-aware comparisons
+    for ignore in [(ignore_p_order, 'p'), (ignore_e_order, 'e'), (ignore_a_order, 'a')]:
+        if ignore[0]:
+            # collecting atoms:
+            a_user = [v for k, v in user_input['atoms'].items() if k.startswith(ignore[1])]
+            a_correct = [v for k, v in correct_answer['atoms'].items() if k.startswith(ignore[1])]
+            # print ignore[0], ignore[1], a_user, a_correct
+            if len(a_user) != len(a_correct):
+                return False
+            if sorted(a_user) != sorted(a_correct):
+                return False
+
+    # order-aware comparisons
+    for ignore in [(ignore_p_order, 'p'), (ignore_e_order, 'e'), (ignore_a_order, 'a')]:
+        if not ignore[0]:
+            # collecting atoms:
+            a_user = [v for k, v in user_input['atoms'].items() if k.startswith(ignore[1])]
+            a_correct = [v for k, v in correct_answer['atoms'].items() if k.startswith(ignore[1])]
+            # print '2nd', ignore[0], ignore[1], a_user, a_correct
+            if len(a_user) != len(a_correct):
+                return False
+            if len(a_correct) == 0:
+                continue
+            if a_user != a_correct:
+                return False
+
+    return True
+
+
+class Test_Grade(unittest.TestCase):
+    ''' test grade function '''
+
+    def test_incorrect_geometry(self):
+        correct_answer = vsepr_build_correct_answer(geometry="AX4E0", atoms={"c0": "N", "p0": "H", "p1": "(ep)", "p2": "H", "p3": "H"})
+        user_answer = vsepr_parse_user_answer(u'{"geometry":"AX3E0","atoms":{"c0":"B","p0":"F","p1":"B","p2":"F"}}')
+        self.assertFalse(vsepr_grade(user_answer, correct_answer))
+
+    def test_incorrect_positions(self):
+        correct_answer = vsepr_build_correct_answer(geometry="AX4E0", atoms={"c0": "N", "p0": "H", "p1": "(ep)", "p2": "H", "p3": "H"})
+        user_answer = vsepr_parse_user_answer(u'{"geometry":"AX4E0","atoms":{"c0":"B","p0":"F","p1":"B","p2":"F"}}')
+        self.assertFalse(vsepr_grade(user_answer, correct_answer))
+
+    def test_correct_answer(self):
+        correct_answer = vsepr_build_correct_answer(geometry="AX4E0", atoms={"c0": "N", "p0": "H", "p1": "(ep)", "p2": "H", "p3": "H"})
+        user_answer = vsepr_parse_user_answer(u'{"geometry":"AX4E0","atoms":{"c0":"N","p0":"H","p1":"(ep)","p2":"H", "p3":"H"}}')
+        self.assertTrue(vsepr_grade(user_answer, correct_answer))
+
+    def test_incorrect_position_order_p(self):
+        correct_answer = vsepr_build_correct_answer(geometry="AX4E0", atoms={"c0": "N", "p0": "H", "p1": "(ep)", "p2": "H", "p3": "H"})
+        user_answer = vsepr_parse_user_answer(u'{"geometry":"AX4E0","atoms":{"c0":"N","p0":"H","p1":"H","p2":"(ep)", "p3":"H"}}')
+        self.assertFalse(vsepr_grade(user_answer, correct_answer))
+
+    def test_correct_position_order_with_ignore_p_order(self):
+        correct_answer = vsepr_build_correct_answer(geometry="AX4E0", atoms={"c0": "N", "p0": "H", "p1": "(ep)", "p2": "H", "p3": "H"})
+        user_answer = vsepr_parse_user_answer(u'{"geometry":"AX4E0","atoms":{"c0":"N","p0":"H","p1":"H","p2":"(ep)", "p3":"H"}}')
+        self.assertTrue(vsepr_grade(user_answer, correct_answer, ignore_p_order=True))
+
+    def test_incorrect_position_order_ae(self):
+        correct_answer = vsepr_build_correct_answer(geometry="AX6E0", atoms={"c0": "Br", "a0": "test", "a1": "(ep)", "e0": "H", "e1": "H", "e2": "(ep)", "e3": "(ep)"})
+        user_answer = vsepr_parse_user_answer(u'{"geometry":"AX6E0","atoms":{"c0":"Br","a0":"test","a1":"(ep)","e0":"H","e1":"(ep)","e2":"(ep)","e3":"(ep)"}}')
+        self.assertFalse(vsepr_grade(user_answer, correct_answer))
+
+    def test_correct_position_order_with_ignore_a_order_not_e(self):
+        correct_answer = vsepr_build_correct_answer(geometry="AX6E0", atoms={"c0": "Br", "a0": "(ep)", "a1": "test", "e0": "H", "e1": "H", "e2": "(ep)", "e3": "(ep)"})
+        user_answer = vsepr_parse_user_answer(u'{"geometry":"AX6E0","atoms":{"c0":"Br","a0":"test","a1":"(ep)","e0":"H","e1":"H","e2":"(ep)","e3":"(ep)"}}')
+        self.assertTrue(vsepr_grade(user_answer, correct_answer, ignore_a_order=True))
+
+    def test_incorrect_position_order_with_ignore_a_order_not_e(self):
+        correct_answer = vsepr_build_correct_answer(geometry="AX6E0", atoms={"c0": "Br", "a0": "(ep)", "a1": "test", "e0": "H", "e1": "H", "e2": "H", "e3": "(ep)"})
+        user_answer = vsepr_parse_user_answer(u'{"geometry":"AX6E0","atoms":{"c0":"Br","a0":"test","a1":"(ep)","e0":"H","e1":"H","e2":"(ep)","e3":"H"}}')
+        self.assertFalse(vsepr_grade(user_answer, correct_answer, ignore_a_order=True))
+
+    def test_correct_position_order_with_ignore_e_order_not_a(self):
+        correct_answer = vsepr_build_correct_answer(geometry="AX6E0", atoms={"c0": "Br", "a0": "(ep)", "a1": "test", "e0": "H", "e1": "H", "e2": "H", "e3": "(ep)"})
+        user_answer = vsepr_parse_user_answer(u'{"geometry":"AX6E0","atoms":{"c0":"Br","a0":"(ep)","a1":"test","e0":"H","e1":"H","e2":"(ep)","e3":"H"}}')
+        self.assertTrue(vsepr_grade(user_answer, correct_answer, ignore_e_order=True))
+
+    def test_incorrect_position_order_with_ignore_e_order__not_a(self):
+        correct_answer = vsepr_build_correct_answer(geometry="AX6E0", atoms={"c0": "Br", "a0": "(ep)", "a1": "test", "e0": "H", "e1": "H", "e2": "H", "e3": "(ep)"})
+        user_answer = vsepr_parse_user_answer(u'{"geometry":"AX6E0","atoms":{"c0":"Br","a0":"test","a1":"(ep)","e0":"H","e1":"H","e2":"(ep)","e3":"H"}}')
+        self.assertFalse(vsepr_grade(user_answer, correct_answer, ignore_e_order=True))
+
+    def test_correct_position_order_with_ignore_ae_order(self):
+        correct_answer = vsepr_build_correct_answer(geometry="AX6E0", atoms={"c0": "Br", "a0": "(ep)", "a1": "test", "e0": "H", "e1": "H", "e2": "H", "e3": "(ep)"})
+        user_answer = vsepr_parse_user_answer(u'{"geometry":"AX6E0","atoms":{"c0":"Br","a0":"test","a1":"(ep)","e0":"H","e1":"H","e2":"(ep)","e3":"H"}}')
+        self.assertTrue(vsepr_grade(user_answer, correct_answer, ignore_e_order=True, ignore_a_order=True))
+
+    def test_incorrect_c0(self):
+        correct_answer = vsepr_build_correct_answer(geometry="AX6E0", atoms={"c0": "Br", "a0": "(ep)", "a1": "test", "e0": "H", "e1": "H", "e2": "H", "e3": "(ep)"})
+        user_answer = vsepr_parse_user_answer(u'{"geometry":"AX6E0","atoms":{"c0":"H","a0":"test","a1":"(ep)","e0":"H","e1":"H","e2":"(ep)","e3":"H"}}')
+        self.assertFalse(vsepr_grade(user_answer, correct_answer, ignore_e_order=True, ignore_a_order=True))
+
+
+def suite():
+
+    testcases = [Test_Grade]
+    suites = []
+    for testcase in testcases:
+        suites.append(unittest.TestLoader().loadTestsFromTestCase(testcase))
+    return unittest.TestSuite(suites)
+
+if __name__ == "__main__":
+    unittest.TextTestRunner(verbosity=2).run(suite())

common/lib/capa/capa/correctmap.py

@@ -3,25 +3,27 @@
 #
 # Used by responsetypes and capa_problem
 
-
 class CorrectMap(object):
-    '''
+    """
     Stores map between answer_id and response evaluation result for each question
     in a capa problem.  The response evaluation result for each answer_id includes
     (correctness, npoints, msg, hint, hintmode).
 
     - correctness : either 'correct' or 'incorrect'
     - npoints     : None, or integer specifying number of points awarded for this answer_id
-    - msg         : string (may have HTML) giving extra message response (displayed below textline or textbox)
-    - hint        : string (may have HTML) giving optional hint (displayed below textline or textbox, above msg)
+    - msg         : string (may have HTML) giving extra message response
+                    (displayed below textline or textbox)
+    - hint        : string (may have HTML) giving optional hint
+                    (displayed below textline or textbox, above msg)
     - hintmode    : one of (None,'on_request','always') criteria for displaying hint
     - queuestate  : Dict {key:'', time:''} where key is a secret string, and time is a string dump
                     of a DateTime object in the format '%Y%m%d%H%M%S'. Is None when not queued
 
     Behaves as a dict.
-    '''
+    """
     def __init__(self, *args, **kwargs):
-        self.cmap = dict()		# start with empty dict
+        # start with empty dict
+        self.cmap = dict()
         self.items = self.cmap.items
         self.keys = self.cmap.keys
         self.set(*args, **kwargs)
@@ -33,7 +35,15 @@ class CorrectMap(object):
         return self.cmap.__iter__()
 
     # See the documentation for 'set_dict' for the use of kwargs
-    def set(self, answer_id=None, correctness=None, npoints=None, msg='', hint='', hintmode=None, queuestate=None, **kwargs):
+    def set(self,
+            answer_id=None,
+            correctness=None,
+            npoints=None,
+            msg='',
+            hint='',
+            hintmode=None,
+            queuestate=None, **kwargs):
+
         if answer_id is not None:
             self.cmap[answer_id] = {'correctness': correctness,
                                     'npoints': npoints,
@@ -56,12 +66,13 @@ class CorrectMap(object):
         '''
         Set internal dict of CorrectMap to provided correct_map dict
 
-        correct_map is saved by LMS as a plaintext JSON dump of the correctmap dict. This means that
-        when the definition of CorrectMap (e.g. its properties) are altered, existing correct_map dict
-        not coincide with the newest CorrectMap format as defined by self.set.
+        correct_map is saved by LMS as a plaintext JSON dump of the correctmap dict. This
+        means that when the definition of CorrectMap (e.g. its properties) are altered,
+        an existing correct_map dict not coincide with the newest CorrectMap format as
+        defined by self.set.
 
         For graceful migration, feed the contents of each correct map to self.set, rather than
-        making a direct copy of the given correct_map dict. This way, the common keys between 
+        making a direct copy of the given correct_map dict. This way, the common keys between
         the incoming correct_map dict and the new CorrectMap instance will be written, while
         mismatched keys will be gracefully ignored.
 
@@ -69,14 +80,20 @@ class CorrectMap(object):
             If correct_map is a one-level dict, then convert it to the new dict of dicts format.
         '''
         if correct_map and not (type(correct_map[correct_map.keys()[0]]) == dict):
-            self.__init__()	                                  # empty current dict
-            for k in correct_map: self.set(k, correct_map[k]) # create new dict entries
+            # empty current dict
+            self.__init__()
+
+            # create new dict entries
+            for k in correct_map:
+                self.set(k, correct_map[k])
         else:
             self.__init__()
-            for k in correct_map: self.set(k, **correct_map[k])
+            for k in correct_map:
+                self.set(k, **correct_map[k])
 
     def is_correct(self, answer_id):
-        if answer_id in self.cmap: return self.cmap[answer_id]['correctness'] == 'correct'
+        if answer_id in self.cmap:
+            return self.cmap[answer_id]['correctness'] == 'correct'
         return None
 
     def is_queued(self, answer_id):
@@ -94,14 +111,18 @@ class CorrectMap(object):
             return npoints
         elif self.is_correct(answer_id):
             return 1
-        return 0 # if not correct and no points have been assigned, return 0
+         # if not correct and no points have been assigned, return 0
+        return 0
 
     def set_property(self, answer_id, property, value):
-        if answer_id in self.cmap: self.cmap[answer_id][property] = value
-        else: self.cmap[answer_id] = {property: value}
+        if answer_id in self.cmap:
+            self.cmap[answer_id][property] = value
+        else:
+            self.cmap[answer_id] = {property: value}
 
     def get_property(self, answer_id, property, default=None):
-        if answer_id in self.cmap: return self.cmap[answer_id].get(property, default)
+        if answer_id in self.cmap:
+            return self.cmap[answer_id].get(property, default)
         return default
 
     def get_correctness(self, answer_id):

common/lib/capa/capa/eia.py

-""" Standard resistor codes.
+"""
+Standard resistor codes.
 http://en.wikipedia.org/wiki/Electronic_color_code
 """
 E6 = [10, 15, 22, 33, 47, 68]
+
 E12 = [10, 12, 15, 18, 22, 27, 33, 39, 47, 56, 68, 82]
+
 E24 = [10, 12, 15, 18, 22, 27, 33, 39, 47, 56, 68, 82, 11, 13, 16, 20, 24, 30, 36, 43, 51, 62, 75, 91]
+
 E48 = [100, 121, 147, 178, 215, 261, 316, 383, 464, 562, 681, 825, 105, 127, 154, 187, 226, 274, 332, 402, 487, 590, 715, 866, 110, 133, 162, 196, 237, 287, 348, 422, 511, 619, 750, 909, 115, 140, 169, 205, 249, 301, 365, 442, 536, 649, 787, 953]
+
 E96 = [100, 121, 147, 178, 215, 261, 316, 383, 464, 562, 681, 825, 102, 124, 150, 182, 221, 267, 324, 392, 475, 576, 698, 845, 105, 127, 154, 187, 226, 274, 332, 402, 487, 590, 715, 866, 107, 130, 158, 191, 232, 280, 340, 412, 499, 604, 732, 887, 110, 133, 162, 196, 237, 287, 348, 422, 511, 619, 750, 909, 113, 137, 165, 200, 243, 294, 357, 432, 523, 634, 768, 931, 115, 140, 169, 205, 249, 301, 365, 442, 536, 649, 787, 953, 118, 143, 174, 210, 255, 309, 374, 453, 549, 665, 806, 976]
+
 E192 = [100, 121, 147, 178, 215, 261, 316, 383, 464, 562, 681, 825, 101, 123, 149, 180, 218, 264, 320, 388, 470, 569, 690, 835, 102, 124, 150, 182, 221, 267, 324, 392, 475, 576, 698, 845, 104, 126, 152, 184, 223, 271, 328, 397, 481, 583, 706, 856, 105, 127, 154, 187, 226, 274, 332, 402, 487, 590, 715, 866, 106, 129, 156, 189, 229, 277, 336, 407, 493, 597, 723, 876, 107, 130, 158, 191, 232, 280, 340, 412, 499, 604, 732, 887, 109, 132, 160, 193, 234, 284, 344, 417, 505, 612, 741, 898, 110, 133, 162, 196, 237, 287, 348, 422, 511, 619, 750, 909, 111, 135, 164, 198, 240, 291, 352, 427, 517, 626, 759, 920, 113, 137, 165, 200, 243, 294, 357, 432, 523, 634, 768, 931, 114, 138, 167, 203, 246, 298, 361, 437, 530, 642, 777, 942, 115, 140, 169, 205, 249, 301, 365, 442, 536, 649, 787, 953, 117, 142, 172, 208, 252, 305, 370, 448, 542, 657, 796, 965, 118, 143, 174, 210, 255, 309, 374, 453, 549, 665, 806, 976, 120, 145, 176, 213, 258, 312, 379, 459, 556, 673, 816, 988]

common/lib/capa/capa/templates/chemicalequationinput.html

+<section id="chemicalequationinput_${id}" class="chemicalequationinput">
+    <div class="script_placeholder" data-src="${previewer}"/>
+
+    % if status == 'unsubmitted':
+    <div class="unanswered" id="status_${id}">
+    % elif status == 'correct':
+    <div class="correct" id="status_${id}">
+    % elif status == 'incorrect':
+    <div class="incorrect" id="status_${id}">
+    % elif status == 'incomplete':
+    <div class="incorrect" id="status_${id}">
+    % endif
+
+  <input type="text" name="input_${id}" id="input_${id}" value="${value|h}"
+   % if size:
+   size="${size}"
+   % endif
+   />
+
+      <p class="status">
+        % if status == 'unsubmitted':
+        unanswered
+        % elif status == 'correct':
+        correct
+        % elif status == 'incorrect':
+        incorrect
+        % elif status == 'incomplete':
+        incomplete
+        % endif
+      </p>
+
+      <div id="input_${id}_preview" class="equation">
+      </div>
+      
+  <p id="answer_${id}" class="answer"></p>
+
+% if status in ['unsubmitted', 'correct', 'incorrect', 'incomplete']:
+</div>
+% endif
+</section>

common/lib/capa/capa/templates/crystallography.html

+<% doinline = "inline" if inline else "" %>
+
+<section id="textinput_${id}" class="textinput ${doinline}" >
+     <div id="holder" style="width:${width};height:${height}"></div>
+    <div class="script_placeholder" data-src="/static/js/raphael.js"></div><div class="script_placeholder" data-src="/static/js/sylvester.js"></div><div class="script_placeholder" data-src="/static/js/underscore-min.js"></div>
+    <div class="script_placeholder" data-src="/static/js/crystallography.js"></div>
+
+
+    % if state == 'unsubmitted':
+    <div class="unanswered ${doinline}" id="status_${id}">
+    % elif state == 'correct':
+    <div class="correct ${doinline}" id="status_${id}">
+    % elif state == 'incorrect':
+    <div class="incorrect ${doinline}" id="status_${id}">
+    % elif state == 'incomplete':
+    <div class="incorrect ${doinline}" id="status_${id}">
+    % endif
+    % if hidden:
+      <div style="display:none;" name="${hidden}" inputid="input_${id}" />
+    % endif
+
+  <input type="text" name="input_${id}" id="input_${id}" value="${value}"
+   % if size:
+   size="${size}"
+   % endif
+   % if hidden:
+    style="display:none;"
+   % endif
+   />
+
+      <p class="status">
+        % if state == 'unsubmitted':
+        unanswered
+        % elif state == 'correct':
+        correct
+        % elif state == 'incorrect':
+        incorrect
+        % elif state == 'incomplete':
+        incomplete
+        % endif
+      </p>
+
+  <p id="answer_${id}" class="answer"></p>
+
+  % if msg:
+      <span class="message">${msg|n}</span>
+  % endif
+% if state in ['unsubmitted', 'correct', 'incorrect', 'incomplete'] or hidden:
+</div>
+% endif
+</section>

common/lib/capa/capa/templates/textbox.html

@@ -5,8 +5,6 @@
        % endif
   >${value|h}</textarea>
 
-  <span id="answer_${id}"></span>
-
   <div class="grader-status">
     % if state == 'unsubmitted':
       <span class="unanswered" style="display:inline-block;" id="status_${id}">Unanswered</span>
@@ -26,6 +24,8 @@
     <p class="debug">${state}</p>
   </div>
 
+  <span id="answer_${id}"></span>
+
   <div class="external-grader-message">
     ${msg|n}
   </div>
@@ -42,7 +42,12 @@
         lineWrapping: true,
         indentUnit: "${tabsize}",
         tabSize: "${tabsize}",
-        indentWithTabs: true,
+        indentWithTabs: false,
+        extraKeys: {
+            "Tab": function(cm) {
+                cm.replaceSelection("${' '*tabsize}", "end");
+            }
+        },
         smartIndent: false
       });
     });

common/lib/capa/capa/templates/vsepr_input.html

+<% doinline = "inline" if inline else "" %>
+
+<section id="textinput_${id}" class="textinput ${doinline}" >
+     <table><tr><td height='600'>
+    <div id="vsepr_div_${id}" style="position:relative;" data-molecules="${molecules}" data-geometries="${geometries}">
+    <canvas id="vsepr_canvas_${id}" width="${width}" height="${height}">
+    </canvas>
+    </div>
+    </td><td valign ='top'>
+    <select class="molecule_select" id="molecule_select_${id}" size="18">
+    </select>
+    </td></tr></table>
+
+    <div class="script_placeholder" data-src="/static/js/vsepr/vsepr.js"></div>
+
+    % if state == 'unsubmitted':
+    <div class="unanswered ${doinline}" id="status_${id}">
+    % elif state == 'correct':
+    <div class="correct ${doinline}" id="status_${id}">
+    % elif state == 'incorrect':
+    <div class="incorrect ${doinline}" id="status_${id}">
+    % elif state == 'incomplete':
+    <div class="incorrect ${doinline}" id="status_${id}">
+    % endif
+    % if hidden:
+      <div style="display:none;" name="${hidden}" inputid="input_${id}" />
+    % endif
+
+  <input type="text" name="input_${id}" id="input_${id}" value="${value}"
+   % if size:
+   size="${size}"
+   % endif
+   % if hidden:
+    style="display:none;"
+   % endif
+   />
+
+      <p class="status">
+        % if state == 'unsubmitted':
+        unanswered
+        % elif state == 'correct':
+        correct
+        % elif state == 'incorrect':
+        incorrect
+        % elif state == 'incomplete':
+        incomplete
+        % endif
+      </p>
+
+  <p id="answer_${id}" class="answer"></p>
+
+  % if msg:
+      <span class="message">${msg|n}</span>
+  % endif
+% if state in ['unsubmitted', 'correct', 'incorrect', 'incomplete'] or hidden:
+</div>
+% endif
+</section>

common/lib/capa/capa/tests/__init__.py

+import fs
+import fs.osfs
+import os
+
+from mock import Mock
+
+TEST_DIR = os.path.dirname(os.path.realpath(__file__))
+
+test_system = Mock(
+    ajax_url='courses/course_id/modx/a_location',
+    track_function=Mock(),
+    get_module=Mock(),
+    render_template=Mock(),
+    replace_urls=Mock(),
+    user=Mock(),
+    filestore=fs.osfs.OSFS(os.path.join(TEST_DIR, "test_files")),
+    debug=True,
+    xqueue={'interface':None, 'callback_url':'/', 'default_queuename': 'testqueue', 'waittime': 10},
+    node_path=os.environ.get("NODE_PATH", "/usr/local/lib/node_modules"),
+    anonymous_student_id = 'student'
+)

common/lib/xmodule/xmodule/tests/test_files/imageresponse.xml → common/lib/capa/capa/tests/test_files/imageresponse.xml

@@ -8,8 +8,14 @@ Hello</p></text>
 <text>Click on the image where the top skier will stop momentarily if the top skier starts from rest.</text>
 <imageinput src="/static/Physics801/Figures/Skier-conservation of energy.jpg" width="560" height="388" rectangle="(242,202)-(296,276)"/>
 <text>Click on the image where the lower skier will stop momentarily if the lower skier starts from rest.</text>
+<imageinput src="/static/Physics801/Figures/Skier-conservation of energy.jpg" width="560" height="388" rectangle="(490,11)-(556,98);(242,202)-(296,276)"/>
+<text>Click on either of the two positions as discussed previously.</text>
+<imageinput src="/static/Physics801/Figures/Skier-conservation of energy.jpg" width="560" height="388" rectangle="(490,11)-(556,98);(242,202)-(296,276)"/>
+<text>Click on either of the two positions as discussed previously.</text>
+<imageinput src="/static/Physics801/Figures/Skier-conservation of energy.jpg" width="560" height="388" rectangle="(490,11)-(556,98);(242,202)-(296,276)"/>
+<text>Click on either of the two positions as discussed previously.</text>
 <hintgroup showoncorrect="no">
 <text><p>Use conservation of energy.</p></text>
 </hintgroup>
 </imageresponse>
-</problem>
\ No newline at end of file
+</problem>

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

+"""
+Tests of input types (and actually responsetypes too)
+"""
+
+from datetime import datetime
+import json
+from mock import Mock
+from nose.plugins.skip import SkipTest
+import os
+import unittest
+
+from . import test_system
+from capa import inputtypes
+
+from lxml import etree
+
+def tst_render_template(template, context):
+    """
+    A test version of render to template.  Renders to the repr of the context, completely ignoring the template name.
+    """
+    return repr(context)
+
+
+system = Mock(render_template=tst_render_template)
+
+class OptionInputTest(unittest.TestCase):
+    '''
+    Make sure option inputs work
+    '''
+    def test_rendering_new(self):
+        xml = """<optioninput options="('Up','Down')" id="sky_input" correct="Up"/>"""
+        element = etree.fromstring(xml)
+
+        value = 'Down'
+        status = 'answered'
+        context = inputtypes._optioninput(element, value, status, test_system.render_template)
+        print 'context: ', context
+
+        expected = {'value': 'Down',
+                    'options': [('Up', 'Up'), ('Down', 'Down')],
+                    'state': 'answered',
+                    'msg': '',
+                    'inline': '',
+                    'id': 'sky_input'}
+
+        self.assertEqual(context, expected)
+
+
+    def test_rendering(self):
+        xml_str = """<optioninput options="('Up','Down')" id="sky_input" correct="Up"/>"""
+        element = etree.fromstring(xml_str)
+
+        state = {'value': 'Down',
+                 'id': 'sky_input',
+                 'status': 'answered'}
+        option_input = inputtypes.OptionInput(system, element, state)
+
+        context = option_input._get_render_context()
+
+        expected = {'value': 'Down',
+                    'options': [('Up', 'Up'), ('Down', 'Down')],
+                    'state': 'answered',
+                    'msg': '',
+                    'inline': '',
+                    'id': 'sky_input'}
+
+        self.assertEqual(context, expected)
+

common/lib/capa/capa/util.py

@@ -11,7 +11,7 @@ def compare_with_tolerance(v1, v2, tol):
 
      - v1    :  student result (number)
      - v2    :  instructor result (number)
-     - tol   :  tolerance (string or number)
+     - tol   :  tolerance (string representing a number)
 
     '''
     relative = tol.endswith('%')
@@ -26,9 +26,20 @@ def compare_with_tolerance(v1, v2, tol):
 def contextualize_text(text, context):  # private
     ''' Takes a string with variables. E.g. $a+$b.
     Does a substitution of those variables from the context '''
-    if not text: return text
+    if not text:
+        return text
     for key in sorted(context, lambda x, y: cmp(len(y), len(x))):
-        text = text.replace('$' + key, str(context[key]))
+        # TODO (vshnayder): This whole replacement thing is a big hack
+        # right now--context contains not just the vars defined in the
+        # program, but also e.g. a reference to the numpy module.
+        # Should be a separate dict of variables that should be
+        # replaced.
+        if '$' + key in text:
+            try:
+                s = str(context[key])
+            except UnicodeEncodeError:
+                s = context[key].encode('utf8', errors='ignore')
+            text = text.replace('$' + key, s)
     return text
 
 
@@ -53,8 +64,4 @@ def is_file(file_to_test):
     '''
     Duck typing to check if 'file_to_test' is a File object
     '''
-    is_file = True
-    for method in ['read', 'name']:
-        if not hasattr(file_to_test, method):
-            is_file = False
-    return is_file 
+    return all(hasattr(file_to_test, method) for method in ['read', 'name'])

common/lib/capa/capa/xqueue_interface.py

@@ -12,7 +12,7 @@ dateformat = '%Y%m%d%H%M%S'
 
 def make_hashkey(seed):
     '''
-    Generate a string key by hashing 
+    Generate a string key by hashing
     '''
     h = hashlib.md5()
     h.update(str(seed))
@@ -20,27 +20,27 @@ def make_hashkey(seed):
 
 
 def make_xheader(lms_callback_url, lms_key, queue_name):
-    '''
+    """
     Generate header for delivery and reply of queue request.
 
     Xqueue header is a JSON-serialized dict:
         { 'lms_callback_url': url to which xqueue will return the request (string),
-          'lms_key': secret key used by LMS to protect its state (string), 
+          'lms_key': secret key used by LMS to protect its state (string),
           'queue_name': designate a specific queue within xqueue server, e.g. 'MITx-6.00x' (string)
         }
-    '''
+    """
     return json.dumps({ 'lms_callback_url': lms_callback_url,
                         'lms_key': lms_key,
                         'queue_name': queue_name })
 
 
 def parse_xreply(xreply):
-    '''
+    """
     Parse the reply from xqueue. Messages are JSON-serialized dict:
         { 'return_code': 0 (success), 1 (fail)
           'content': Message from xqueue (string)
         }
-    '''
+    """
     try:
         xreply = json.loads(xreply)
     except ValueError, err:
@@ -61,11 +61,11 @@ class XQueueInterface(object):
         self.url  = url
         self.auth = django_auth
         self.session = requests.session(auth=requests_auth)
-        
+
     def send_to_queue(self, header, body, files_to_upload=None):
-        '''
+        """
         Submit a request to xqueue.
-        
+
         header: JSON-serialized dict in the format described in 'xqueue_interface.make_xheader'
 
         body: Serialized data for the receipient behind the queueing service. The operation of
@@ -74,14 +74,16 @@ class XQueueInterface(object):
         files_to_upload: List of file objects to be uploaded to xqueue along with queue request
 
         Returns (error_code, msg) where error_code != 0 indicates an error
-        '''
+        """
         # Attempt to send to queue
         (error, msg) = self._send_to_queue(header, body, files_to_upload)
 
-        if error and (msg == 'login_required'): # Log in, then try again
+        # Log in, then try again
+        if error and (msg == 'login_required'):
             self._login()
             if files_to_upload is not None:
-                for f in files_to_upload: # Need to rewind file pointers
+                # Need to rewind file pointers
+                for f in files_to_upload:
                     f.seek(0)
             (error, msg) = self._send_to_queue(header, body, files_to_upload)
 
@@ -91,18 +93,18 @@ class XQueueInterface(object):
     def _login(self):
         payload = { 'username': self.auth['username'],
                     'password': self.auth['password'] }
-        return self._http_post(self.url+'/xqueue/login/', payload)
+        return self._http_post(self.url + '/xqueue/login/', payload)
 
 
     def _send_to_queue(self, header, body, files_to_upload):
         payload = {'xqueue_header': header,
                    'xqueue_body'  : body}
-        files = {} 
+        files = {}
         if files_to_upload is not None:
             for f in files_to_upload:
                 files.update({ f.name: f })
 
-        return self._http_post(self.url+'/xqueue/submit/', payload, files=files)
+        return self._http_post(self.url + '/xqueue/submit/', payload, files=files)
 
 
     def _http_post(self, url, data, files=None):
@@ -111,7 +113,7 @@ class XQueueInterface(object):
         except requests.exceptions.ConnectionError, err:
             log.error(err)
             return (1, 'cannot connect to server')
-        
+
         if r.status_code not in [200]:
             return (1, 'unexpected HTTP status code [%d]' % r.status_code)
 

common/lib/xmodule/setup.py

@@ -26,7 +26,7 @@ setup(
             "image = xmodule.backcompat_module:TranslateCustomTagDescriptor",
             "error = xmodule.error_module:ErrorDescriptor",
             "problem = xmodule.capa_module:CapaDescriptor",
-            "problemset = xmodule.vertical_module:VerticalDescriptor",
+            "problemset = xmodule.seq_module:SequenceDescriptor",
             "section = xmodule.backcompat_module:SemanticSectionDescriptor",
             "sequential = xmodule.seq_module:SequenceDescriptor",
             "slides = xmodule.backcompat_module:TranslateCustomTagDescriptor",

common/lib/xmodule/xmodule/capa_module.py

@@ -76,9 +76,13 @@ class CapaModule(XModule):
     '''
     icon_class = 'problem'
 
-    js = {'coffee': [resource_string(__name__, 'js/src/capa/display.coffee')],
+    js = {'coffee': [resource_string(__name__, 'js/src/capa/display.coffee'),
+                     resource_string(__name__, 'js/src/collapsible.coffee'),
+                     resource_string(__name__, 'js/src/javascript_loader.coffee'),
+                    ],
           'js': [resource_string(__name__, 'js/src/capa/imageinput.js'),
                  resource_string(__name__, 'js/src/capa/schematic.js')]}
+
     js_module_name = "Problem"
     css = {'scss': [resource_string(__name__, 'css/capa/display.scss')]}
 
@@ -116,6 +120,8 @@ class CapaModule(XModule):
 
         self.show_answer = self.metadata.get('showanswer', 'closed')
 
+        self.force_save_button = self.metadata.get('force_save_button', 'false')
+
         if self.show_answer == "":
             self.show_answer = "closed"
 
@@ -129,6 +135,11 @@ class CapaModule(XModule):
         if self.rerandomize == 'never':
             self.seed = 1
         elif self.rerandomize == "per_student" and hasattr(self.system, 'id'):
+            # TODO: This line is badly broken:
+            # (1) We're passing student ID to xmodule.
+            # (2) There aren't bins of students.  -- we only want 10 or 20 randomizations, and want to assign students
+            # to these bins, and may not want cohorts.  So e.g. hash(your-id, problem_id) % num_bins.
+            #     - analytics really needs small number of bins.
             self.seed = system.id
         else:
             self.seed = None
@@ -311,9 +322,10 @@ class CapaModule(XModule):
         if not self.lcp.done:
             reset_button = False
 
-        # We don't need a "save" button if infinite number of attempts and
-        # non-randomized
-        if self.max_attempts is None and self.rerandomize != "always":
+        # We may not need a "save" button if infinite number of attempts and
+        # non-randomized. The problem author can force it. It's a bit weird for
+        # randomization to control this; should perhaps be cleaned up.
+        if (self.force_save_button == "false") and (self.max_attempts is None and self.rerandomize != "always"):
             save_button = False
 
         context = {'problem': content,
@@ -615,12 +627,14 @@ class CapaModule(XModule):
         if self.closed():
             event_info['failure'] = 'closed'
             self.system.track_function('reset_problem_fail', event_info)
-            return "Problem is closed"
+            return {'success': False,
+                    'error': "Problem is closed"}
 
         if not self.lcp.done:
             event_info['failure'] = 'not_done'
             self.system.track_function('reset_problem_fail', event_info)
-            return "Refresh the page and make an attempt before resetting."
+            return {'success': False,
+                    'error': "Refresh the page and make an attempt before resetting."}
 
         self.lcp.do_reset()
         if self.rerandomize in ["always", "onreset"]:

common/lib/xmodule/xmodule/course_module.py

@@ -22,13 +22,13 @@ class CourseDescriptor(SequenceDescriptor):
             self.book_url = book_url
             self.table_of_contents = self._get_toc_from_s3()
             self.start_page = int(self.table_of_contents[0].attrib['page'])
-            
+
             # The last page should be the last element in the table of contents,
             # but it may be nested. So recurse all the way down the last element
             last_el = self.table_of_contents[-1]
             while last_el.getchildren():
                 last_el = last_el[-1]
-            
+
             self.end_page = int(last_el.attrib['page'])
 
         @property
@@ -87,6 +87,7 @@ class CourseDescriptor(SequenceDescriptor):
 
         self.enrollment_start = self._try_parse_time("enrollment_start")
         self.enrollment_end = self._try_parse_time("enrollment_end")
+        self.end = self._try_parse_time("end")
 
         # NOTE: relies on the modulestore to call set_grading_policy() right after
         # init.  (Modulestore is in charge of figuring out where to load the policy from)
@@ -127,6 +128,16 @@ class CourseDescriptor(SequenceDescriptor):
 
         return definition
 
+    def has_ended(self):
+        """
+        Returns True if the current time is after the specified course end date.
+        Returns False if there is no end date specified.
+        """
+        if self.end_date is None:
+            return False
+
+        return time.gmtime() > self.end
+
     def has_started(self):
         return time.gmtime() > self.start
 

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

@@ -44,8 +44,8 @@ section.problem {
       }
     }
 
-    min-width:100px; 
-    width: auto !important; 
+    min-width:100px;
+    width: auto !important;
     width: 100px;
 
     .indicator_container {
@@ -299,7 +299,7 @@ section.problem {
   form.option-input {
     margin: -10px 0 20px;
     padding-bottom: 20px;
-    
+
     select {
       margin-right: flex-gutter();
     }
@@ -421,7 +421,7 @@ section.problem {
         background-position: right;
         background-repeat: no-repeat;
     }
-    
+
     pre {
       @include border-radius(0);
       border-radius: 0;
@@ -572,7 +572,7 @@ section.problem {
         }
       }
 
-      section {
+      > section {
         padding: 9px;
       }
     }
@@ -622,4 +622,70 @@ section.problem {
       }
     }
   }
+
+  .external-grader-message {
+    section {
+      padding-left: 20px;
+      background-color: #FAFAFA;
+      color: #2C2C2C;
+      font-family: monospace;
+      font-size: 1em;
+
+      .shortform {
+        font-weight: bold;
+      }
+
+      .longform {
+        padding: 0px;
+        margin: 0px;
+
+        .result-errors {
+          margin: 5px;
+          padding: 10px 10px 10px 40px;
+          background: url('../images/incorrect-icon.png') center left no-repeat;
+          li {
+            color: #B00;
+            }
+        }
+
+        .result-output {
+          margin: 5px;
+          padding: 20px 0px 15px 50px;
+          border-top: 1px solid #DDD;
+          border-left: 20px solid #FAFAFA;
+
+          h4 {
+            font-family: monospace;
+            font-size: 1em;
+          }
+
+          dl {
+            margin: 0px;
+          }
+
+          dt {
+            margin-top: 20px;
+          }
+
+          dd {
+            margin-left: 24pt;
+          }
+        }
+
+        .result-correct {
+          background: url('../images/correct-icon.png') left 20px no-repeat;
+          .result-actual-output {
+            color: #090;
+          }
+        }
+
+        .result-incorrect {
+          background: url('../images/incorrect-icon.png') left 20px no-repeat;
+          .result-actual-output {
+            color: #B00;
+          }
+        }
+      }
+    }
+  }
 }

common/lib/xmodule/xmodule/graders.py

 import abc
+import inspect
 import json
 import logging
 import random
@@ -103,6 +104,15 @@ def aggregate_scores(scores, section_name="summary"):
     return all_total, graded_total
 
 
+def invalid_args(func, argdict):
+    """
+    Given a function and a dictionary of arguments, returns a set of arguments
+    from argdict that aren't accepted by func
+    """
+    args, varargs, keywords, defaults = inspect.getargspec(func)
+    if keywords: return set()  # All accepted
+    return set(argdict) - set(args)
+
 def grader_from_conf(conf):
     """
     This creates a CourseGrader from a configuration (such as in course_settings.py).
@@ -122,14 +132,21 @@ def grader_from_conf(conf):
         try:
             if 'min_count' in subgraderconf:
                 #This is an AssignmentFormatGrader
-                subgrader = AssignmentFormatGrader(**subgraderconf)
-                subgraders.append((subgrader, subgrader.category, weight))
+                subgrader_class = AssignmentFormatGrader
             elif 'name' in subgraderconf:
                 #This is an SingleSectionGrader
-                subgrader = SingleSectionGrader(**subgraderconf)
-                subgraders.append((subgrader, subgrader.category, weight))
+                subgrader_class = SingleSectionGrader
             else:
                 raise ValueError("Configuration has no appropriate grader class.")
+            
+            bad_args = invalid_args(subgrader_class.__init__, subgraderconf)
+            if len(bad_args) > 0:
+                log.warning("Invalid arguments for a subgrader: %s", bad_args)
+                for key in bad_args:
+                    del subgraderconf[key]
+            
+            subgrader = subgrader_class(**subgraderconf)
+            subgraders.append((subgrader, subgrader.category, weight))
 
         except (TypeError, ValueError) as error:
             # Add info and re-raise
@@ -294,9 +311,12 @@ class AssignmentFormatGrader(CourseGrader):
 
     short_label is similar to section_type, but shorter. For example, for Homework it would be
     "HW".
+    
+    starting_index is the first number that will appear. For example, starting_index=3 and
+    min_count = 2 would produce the labels "Assignment 3", "Assignment 4"
 
     """
-    def __init__(self, type, min_count, drop_count, category=None, section_type=None, short_label=None, show_only_average=False):
+    def __init__(self, type, min_count, drop_count, category=None, section_type=None, short_label=None, show_only_average=False, starting_index=1):
         self.type = type
         self.min_count = min_count
         self.drop_count = drop_count
@@ -304,6 +324,7 @@ class AssignmentFormatGrader(CourseGrader):
         self.section_type = section_type or self.type
         self.short_label = short_label or self.type
         self.show_only_average = show_only_average
+        self.starting_index = starting_index
 
     def grade(self, grade_sheet, generate_random_scores=False):
         def totalWithDrops(breakdown, drop_count):
@@ -339,7 +360,7 @@ class AssignmentFormatGrader(CourseGrader):
                     section_name = scores[i].section
                 
                 percentage = earned / float(possible)
-                summary = "{section_type} {index} - {name} - {percent:.0%} ({earned:.3n}/{possible:.3n})".format(index=i + 1,
+                summary = "{section_type} {index} - {name} - {percent:.0%} ({earned:.3n}/{possible:.3n})".format(index=i + self.starting_index,
                                                                 section_type=self.section_type,
                                                                 name=section_name,
                                                                 percent=percentage,
@@ -347,9 +368,9 @@ class AssignmentFormatGrader(CourseGrader):
                                                                 possible=float(possible))
             else:
                 percentage = 0
-                summary = "{section_type} {index} Unreleased - 0% (?/?)".format(index=i + 1, section_type=self.section_type)
+                summary = "{section_type} {index} Unreleased - 0% (?/?)".format(index=i + self.starting_index, section_type=self.section_type)
 
-            short_label = "{short_label} {index:02d}".format(index=i + 1, short_label=self.short_label)
+            short_label = "{short_label} {index:02d}".format(index=i + self.starting_index, short_label=self.short_label)
             
             breakdown.append({'percent': percentage, 'label': short_label, 'detail': summary, 'category': self.category})
 

common/lib/xmodule/xmodule/html_module.py

@@ -21,7 +21,11 @@ log = logging.getLogger("mitx.courseware")
 
 
 class HtmlModule(XModule):
-    js = {'coffee': [resource_string(__name__, 'js/src/html/display.coffee')]}
+    js = {'coffee': [resource_string(__name__, 'js/src/javascript_loader.coffee'),
+                     resource_string(__name__, 'js/src/collapsible.coffee'),
+                     resource_string(__name__, 'js/src/html/display.coffee')
+                    ]
+         }
     js_module_name = "HTMLModule"
     
     def get_html(self):

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

@@ -27,11 +27,7 @@ class @Problem
     @$('section.action input.save').click @save
 
     # Collapsibles
-    @$('.longform').hide();
-    @$('.shortform').append('<a href="#" class="full">See full output</a>');
-    @$('.collapsible section').hide();
-    @$('.full').click @toggleFull
-    @$('.collapsible header a').click @toggleHint
+    Collapsible.setCollapsibles(@el)
 
     # Dynamath
     @$('input.math').keyup(@refreshMath)
@@ -67,7 +63,7 @@ class @Problem
       @new_queued_items = $(response.html).find(".xqueue")
       if @new_queued_items.length isnt @num_queued_items
         @el.html(response.html)
-        @executeProblemScripts () =>
+        JavascriptLoader.executeModuleScripts @el, () =>
           @setupInputTypes()
           @bind()
       
@@ -81,18 +77,19 @@ class @Problem
   render: (content) ->
     if content
       @el.html(content)
-      @executeProblemScripts () =>
+      JavascriptLoader.executeModuleScripts @el, () =>
         @setupInputTypes()
         @bind()
         @queueing()
     else
       $.postWithPrefix "#{@url}/problem_get", (response) =>
         @el.html(response.html)
-        @executeProblemScripts () =>
+        JavascriptLoader.executeModuleScripts @el, () =>
           @setupInputTypes()
           @bind()
           @queueing()
 
+
   # TODO add hooks for problem types here by inspecting response.html and doing
   # stuff if a div w a class is found
 
@@ -106,50 +103,6 @@ class @Problem
         if setupMethod?
           @inputtypeDisplays[id] = setupMethod(inputtype)
 
-  executeProblemScripts: (callback=null) ->
-
-    placeholders = @el.find(".script_placeholder")
-
-    if placeholders.length == 0
-      callback()
-      return
-
-    completed      = (false for i in [1..placeholders.length])
-    callbackCalled = false
-
-    # This is required for IE8 support.
-    completionHandlerGeneratorIE = (index) =>
-      return () ->
-        if (this.readyState == 'complete' || this.readyState == 'loaded')
-          #completionHandlerGenerator.call(self, index)()
-          completionHandlerGenerator(index)()
-
-    completionHandlerGenerator = (index) =>
-      return () =>
-        allComplete = true
-        completed[index] = true
-        for flag in completed
-          if not flag
-            allComplete = false
-            break
-        if allComplete and not callbackCalled
-          callbackCalled = true
-          callback() if callback?
-
-    placeholders.each (index, placeholder) ->
-      s = document.createElement('script')
-      s.setAttribute('src', $(placeholder).attr("data-src"))
-      s.setAttribute('type', "text/javascript")
-
-      s.onload             = completionHandlerGenerator(index)
-
-      # s.onload does not fire in IE8; this does.
-      s.onreadystatechange = completionHandlerGeneratorIE(index)
-
-      # Need to use the DOM elements directly or the scripts won't execute
-      # properly.
-      $('head')[0].appendChild(s)
-      $(placeholder).remove()
 
   ###
   # 'check_fd' uses FormData to allow file submissions in the 'problem_check' dispatch,
@@ -263,7 +216,9 @@ class @Problem
             for choice in value
               @$("label[for='input_#{key}_#{choice}']").attr correct_answer: 'true'
           else
-            @$("#answer_#{key}, #solution_#{key}").html(value)
+            answer = @$("#answer_#{key}, #solution_#{key}")
+            answer.html(value)
+            Collapsible.setCollapsibles(answer)
 
         # TODO remove the above once everything is extracted into its own
         # inputtype functions.
@@ -340,17 +295,6 @@ class @Problem
       element.CodeMirror.save() if element.CodeMirror.save
     @answers = @inputs.serialize()
 
-  toggleFull: (event) =>
-    $(event.target).parent().siblings().slideToggle()
-    $(event.target).parent().parent().toggleClass('open')
-    text = $(event.target).text() == 'See full output' ? 'Hide output' : 'See full output'
-    $(this).text(text)
-
-  toggleHint: (event) =>
-    event.preventDefault()
-    $(event.target).parent().siblings().slideToggle()
-    $(event.target).parent().parent().toggleClass('open')
-
   inputtypeSetupMethods:
 
     'text-input-dynamath': (element) =>
@@ -392,10 +336,13 @@ class @Problem
   inputtypeShowAnswerMethods:
     choicegroup: (element, display, answers) =>
       element = $(element)
-      for key, value of answers
-        element.find('input').attr('disabled', 'disabled')
-        for choice in value
-          element.find("label[for='input_#{key}_#{choice}']").addClass 'choicegroup_correct'
+
+      element.find('input').attr('disabled', 'disabled')
+
+      input_id = element.attr('id').replace(/inputtype_/,'')
+      answer = answers[input_id]
+      for choice in answer
+        element.find("label[for='input_#{input_id}_#{choice}']").addClass 'choicegroup_correct'
 
     javascriptinput: (element, display, answers) =>
       answer_id = $(element).attr('id').split("_")[1...].join("_")

common/lib/xmodule/xmodule/js/src/capa/imageinput.js

@@ -11,8 +11,8 @@
 
 function image_input_click(id,event){
     iidiv = document.getElementById("imageinput_"+id);
-    pos_x = event.offsetX?(event.offsetX):event.pageX-document.iidiv.offsetLeft;
-    pos_y = event.offsetY?(event.offsetY):event.pageY-document.iidiv.offsetTop;
+    pos_x = event.offsetX?(event.offsetX):event.pageX-iidiv.offsetLeft;
+    pos_y = event.offsetY?(event.offsetY):event.pageY-iidiv.offsetTop;
     result = "[" + pos_x + "," + pos_y + "]";
     cx = (pos_x-15) +"px";
     cy = (pos_y-15)  +"px" ;

common/lib/xmodule/xmodule/js/src/collapsible.coffee

+class @Collapsible
+
+  # Set of library functions that provide a simple way to add collapsible
+  # functionality to elements. 
+
+  # setCollapsibles:
+  #   Scan element's content for generic collapsible containers
+  @setCollapsibles: (el) =>
+    ###
+    el: container
+    ###
+    el.find('.longform').hide()
+    el.find('.shortform').append('<a href="#" class="full">See full output</a>')
+    el.find('.collapsible header + section').hide()
+    el.find('.full').click @toggleFull
+    el.find('.collapsible header a').click @toggleHint
+
+  @toggleFull: (event) =>
+    event.preventDefault()
+    $(event.target).parent().siblings().slideToggle()
+    $(event.target).parent().parent().toggleClass('open')
+    if $(event.target).text() == 'See full output'
+      new_text = 'Hide output'
+    else
+      new_text = 'See full ouput'
+    $(event.target).text(new_text)
+
+  @toggleHint: (event) =>
+    event.preventDefault()
+    $(event.target).parent().siblings().slideToggle()
+    $(event.target).parent().parent().toggleClass('open')

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

 class @HTMLModule
 
   constructor: (@element) ->
-  	@el = $(@element)
-  	@setCollapsibles()
+    @el = $(@element)
+    JavascriptLoader.executeModuleScripts(@el)
+    Collapsible.setCollapsibles(@el)
 
   $: (selector) ->
     $(selector, @el)
-
-  setCollapsibles: =>
-    $('.longform').hide();
-    $('.shortform').append('<a href="#" class="full">See full output</a>');
-    $('.collapsible section').hide();
-    $('.full').click @toggleFull
-    $('.collapsible header a').click @toggleHint
-
-  toggleFull: (event) =>
-    $(event.target).parent().siblings().slideToggle()
-    $(event.target).parent().parent().toggleClass('open')
-    text = $(event.target).text() == 'See full output' ? 'Hide output' : 'See full output'
-    $(this).text(text)
-
-  toggleHint: (event) =>
-    event.preventDefault()
-    $(event.target).parent().siblings().slideToggle()
-    $(event.target).parent().parent().toggleClass('open')
\ No newline at end of file

common/lib/xmodule/xmodule/js/src/javascript_loader.coffee

+class @JavascriptLoader
+
+  # Set of library functions that provide common interface for javascript loading
+  # for all module types. All functionality provided by JavascriptLoader should take
+  # place at module scope, i.e. don't run jQuery over entire page
+
+  # executeModuleScripts:
+  #   Scan the module ('el') for "script_placeholder"s, then:
+  #     1) Fetch each script from server
+  #     2) Explicitly attach the script to the <head> of document
+  #     3) Explicitly wait for each script to be loaded
+  #     4) Return to callback function when all scripts loaded
+  @executeModuleScripts: (el, callback=null) ->
+
+    placeholders = el.find(".script_placeholder")
+
+    if placeholders.length == 0
+      callback() if callback?
+      return
+
+    # TODO: Verify the execution order of multiple placeholders
+    completed      = (false for i in [1..placeholders.length])
+    callbackCalled = false
+
+    # This is required for IE8 support.
+    completionHandlerGeneratorIE = (index) =>
+      return () ->
+        if (this.readyState == 'complete' || this.readyState == 'loaded')
+          #completionHandlerGenerator.call(self, index)()
+          completionHandlerGenerator(index)()
+
+    completionHandlerGenerator = (index) =>
+      return () =>
+        allComplete = true
+        completed[index] = true
+        for flag in completed
+          if not flag
+            allComplete = false
+            break
+        if allComplete and not callbackCalled
+          callbackCalled = true
+          callback() if callback?
+
+    # Keep a map of what sources we're loaded from, and don't do it twice.
+    loaded = {}
+    placeholders.each (index, placeholder) ->
+      # TODO: Check if the script already exists in DOM. If so, (1) copy it
+      #         into memory; (2) delete the DOM script element; (3) reappend it.
+      #       This would prevent memory bloat and save a network request.
+      src = $(placeholder).attr("data-src")
+      if src not of loaded
+        loaded[src] = true
+        s = document.createElement('script')
+        s.setAttribute('src', src)
+        s.setAttribute('type', "text/javascript")
+  
+        s.onload             = completionHandlerGenerator(index)
+
+        # s.onload does not fire in IE8; this does.
+        s.onreadystatechange = completionHandlerGeneratorIE(index)
+
+        # Need to use the DOM elements directly or the scripts won't execute
+        # properly.
+        $('head')[0].appendChild(s)
+      else
+        # just call the completion callback directly, without reloading the file
+        completionHandlerGenerator(index)()
+      $(placeholder).remove()
+        
+      

common/lib/xmodule/xmodule/tests/test_progress.py

+"""Module progress tests"""
+
+import unittest
+
+from xmodule.progress import Progress
+from xmodule import x_module
+
+from . import i4xs
+
+class ProgressTest(unittest.TestCase):
+    ''' Test that basic Progress objects work.  A Progress represents a
+    fraction between 0 and 1.
+    '''
+    not_started = Progress(0, 17)
+    part_done = Progress(2, 6)
+    half_done = Progress(3, 6)
+    also_half_done = Progress(1, 2)
+    done = Progress(7, 7)
+
+    def test_create_object(self):
+        # These should work:
+        p = Progress(0, 2)
+        p = Progress(1, 2)
+        p = Progress(2, 2)
+
+        p = Progress(2.5, 5.0)
+        p = Progress(3.7, 12.3333)
+
+        # These shouldn't
+        self.assertRaises(ValueError, Progress, 0, 0)
+        self.assertRaises(ValueError, Progress, 2, 0)
+        self.assertRaises(ValueError, Progress, 1, -2)
+
+        self.assertRaises(TypeError, Progress, 0, "all")
+        # check complex numbers just for the heck of it :)
+        self.assertRaises(TypeError, Progress, 2j, 3)
+
+    def test_clamp(self):
+        self.assertEqual((2, 2), Progress(3, 2).frac())
+        self.assertEqual((0, 2), Progress(-2, 2).frac())
+
+    def test_frac(self):
+        p = Progress(1, 2)
+        (a, b) = p.frac()
+        self.assertEqual(a, 1)
+        self.assertEqual(b, 2)
+
+    def test_percent(self):
+        self.assertEqual(self.not_started.percent(), 0)
+        self.assertAlmostEqual(self.part_done.percent(), 33.33333333333333)
+        self.assertEqual(self.half_done.percent(), 50)
+        self.assertEqual(self.done.percent(), 100)
+
+        self.assertEqual(self.half_done.percent(), self.also_half_done.percent())
+
+    def test_started(self):
+        self.assertFalse(self.not_started.started())
+
+        self.assertTrue(self.part_done.started())
+        self.assertTrue(self.half_done.started())
+        self.assertTrue(self.done.started())
+
+    def test_inprogress(self):
+        # only true if working on it
+        self.assertFalse(self.done.inprogress())
+        self.assertFalse(self.not_started.inprogress())
+
+        self.assertTrue(self.part_done.inprogress())
+        self.assertTrue(self.half_done.inprogress())
+
+    def test_done(self):
+        self.assertTrue(self.done.done())
+        self.assertFalse(self.half_done.done())
+        self.assertFalse(self.not_started.done())
+
+    def test_str(self):
+        self.assertEqual(str(self.not_started), "0/17")
+        self.assertEqual(str(self.part_done), "2/6")
+        self.assertEqual(str(self.done), "7/7")
+
+    def test_ternary_str(self):
+        self.assertEqual(self.not_started.ternary_str(), "none")
+        self.assertEqual(self.half_done.ternary_str(), "in_progress")
+        self.assertEqual(self.done.ternary_str(), "done")
+
+    def test_to_js_status(self):
+        '''Test the Progress.to_js_status_str() method'''
+
+        self.assertEqual(Progress.to_js_status_str(self.not_started), "none")
+        self.assertEqual(Progress.to_js_status_str(self.half_done), "in_progress")
+        self.assertEqual(Progress.to_js_status_str(self.done), "done")
+        self.assertEqual(Progress.to_js_status_str(None), "NA")
+
+    def test_to_js_detail_str(self):
+        '''Test the Progress.to_js_detail_str() method'''
+        f = Progress.to_js_detail_str
+        for p in (self.not_started, self.half_done, self.done):
+            self.assertEqual(f(p), str(p))
+        # But None should be encoded as NA
+        self.assertEqual(f(None), "NA")
+
+    def test_add(self):
+        '''Test the Progress.add_counts() method'''
+        p = Progress(0, 2)
+        p2 = Progress(1, 3)
+        p3 = Progress(2, 5)
+        pNone = None
+        add = lambda a, b: Progress.add_counts(a, b).frac()
+
+        self.assertEqual(add(p, p), (0, 4))
+        self.assertEqual(add(p, p2), (1, 5))
+        self.assertEqual(add(p2, p3), (3, 8))
+
+        self.assertEqual(add(p2, pNone), p2.frac())
+        self.assertEqual(add(pNone, p2), p2.frac())
+
+    def test_equality(self):
+        '''Test that comparing Progress objects for equality
+        works correctly.'''
+        p = Progress(1, 2)
+        p2 = Progress(2, 4)
+        p3 = Progress(1, 2)
+        self.assertTrue(p == p3)
+        self.assertFalse(p == p2)
+
+        # Check != while we're at it
+        self.assertTrue(p != p2)
+        self.assertFalse(p != p3)
+
+
+class ModuleProgressTest(unittest.TestCase):
+    ''' Test that get_progress() does the right thing for the different modules
+    '''
+    def test_xmodule_default(self):
+        '''Make sure default get_progress exists, returns None'''
+        xm = x_module.XModule(i4xs, 'a://b/c/d/e', None, {})
+        p = xm.get_progress()
+        self.assertEqual(p, None)

common/lib/xmodule/xmodule/video_module.py

@@ -31,12 +31,23 @@ class VideoModule(XModule):
         self.youtube = xmltree.get('youtube')
         self.position = 0
         self.show_captions = xmltree.get('show_captions', 'true')
+        self.source = self._get_source(xmltree)
 
         if instance_state is not None:
             state = json.loads(instance_state)
             if 'position' in state:
                 self.position = int(float(state['position']))
 
+    def _get_source(self, xmltree):
+        # find the first valid source
+        source = None
+        for element in xmltree.findall('source'):
+            src = element.get('src')
+            if src:
+                source = src
+                break
+        return source
+
     def handle_ajax(self, dispatch, get):
         '''
         Handle ajax calls to this video.
@@ -73,6 +84,7 @@ class VideoModule(XModule):
             'streams': self.video_list(),
             'id': self.location.html_id(),
             'position': self.position,
+            'source': self.source,
             'display_name': self.display_name,
             # TODO (cpennington): This won't work when we move to data that isn't on the filesystem
             'data_dir': self.metadata['data_dir'],
@@ -82,6 +94,5 @@ class VideoModule(XModule):
 
 class VideoDescriptor(RawDescriptor):
     module_class = VideoModule
-    
     stores_state = True
     template_dir_name = "video"