Merge branch 'master' into feature/server_split

common/djangoapps/pipeline_mako/templates/static_content.html

@@ -3,7 +3,13 @@ from staticfiles.storage import staticfiles_storage
 from pipeline_mako import compressed_css, compressed_js
 %>
 
-<%def name='url(file)'>${staticfiles_storage.url(file)}</%def>
+<%def name='url(file)'>
+<%
+try:
+    url = staticfiles_storage.url(file)
+except:
+    url = file
+%>${url}</%def>
 
 <%def name='css(group)'>
   % if settings.MITX_FEATURES['USE_DJANGO_PIPELINE']:

common/djangoapps/static_replace.py

 from staticfiles.storage import staticfiles_storage
+from staticfiles import finders
+from django.conf import settings
 import re
 
 
@@ -9,7 +11,15 @@ def replace(static_url, prefix=None):
         prefix = prefix + '/'
 
     quote = static_url.group('quote')
-    if staticfiles_storage.exists(static_url.group('rest')):
+
+    servable = (
+        # If in debug mode, we'll serve up anything that the finders can find
+        (settings.DEBUG and finders.find(static_url.group('rest'), True)) or
+        # Otherwise, we'll only serve up stuff that the storages can find
+        staticfiles_storage.exists(static_url.group('rest'))
+    )
+
+    if servable:
         return static_url.group(0)
     else:
         url = staticfiles_storage.url(prefix + static_url.group('rest'))

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

+##
+## A script to create some dummy users
+
+from django.core.management.base import BaseCommand
+from django.conf import settings
+from django.contrib.auth.models import User
+from student.models import UserProfile, CourseEnrollment
+
+from student.views import _do_create_account, get_random_post_override
+
+def create(n, course_id):
+    """Create n users, enrolling them in course_id if it's not None"""
+    for i in range(n):
+        (user, user_profile, _) = _do_create_account(get_random_post_override())
+        if course_id is not None:
+            CourseEnrollment.objects.create(user=user, course_id=course_id)
+
+class Command(BaseCommand):
+    help = """Create N new users, with random parameters.
+
+Usage: create_random_users.py N [course_id_to_enroll_in].
+
+Examples:
+  create_random_users.py 1
+  create_random_users.py 10 MITx/6.002x/2012_Fall
+  create_random_users.py 100 HarvardX/CS50x/2012
+"""
+
+    def handle(self, *args, **options):
+        if len(args) < 1 or len(args) > 2:
+            print Command.help
+            return
+
+        n = int(args[0])
+        course_id = args[1] if len(args) == 2 else None
+        create(n, course_id)

common/djangoapps/student/views.py

@@ -94,8 +94,9 @@ def main_index(extra_context = {}, user=None):
     context.update(extra_context)
     return render_to_response('index.html', context)
 
-def course_from_id(id):
-    course_loc = CourseDescriptor.id_to_location(id)
+def course_from_id(course_id):
+    """Return the CourseDescriptor corresponding to this course_id"""
+    course_loc = CourseDescriptor.id_to_location(course_id)
     return modulestore().get_item(course_loc)
 
 
@@ -158,15 +159,19 @@ def try_change_enrollment(request):
 
 @login_required
 def change_enrollment_view(request):
+    """Delegate to change_enrollment to actually do the work."""
     return HttpResponse(json.dumps(change_enrollment(request)))
 
-
 def change_enrollment(request):
     if request.method != "POST":
         raise Http404
 
-    action = request.POST.get("enrollment_action", "")
     user = request.user
+    if not user.is_authenticated():
+        raise Http404
+
+    action = request.POST.get("enrollment_action", "")
+
     course_id = request.POST.get("course_id", None)
     if course_id == None:
         return HttpResponse(json.dumps({'success': False, 'error': 'There was an error receiving the course id.'}))
@@ -184,7 +189,7 @@ def change_enrollment(request):
         if settings.MITX_FEATURES.get('ACCESS_REQUIRE_STAFF_FOR_COURSE'):
             # require that user be in the staff_* group (or be an overall admin) to be able to enroll
             # eg staff_6.002x or staff_6.00x
-            if not has_staff_access_to_course(user,course):
+            if not has_staff_access_to_course(user, course):
                 staff_group = course_staff_group_name(course)
                 log.debug('user %s denied enrollment to %s ; not in %s' % (user,course.location.url(),staff_group))
                 return {'success': False, 'error' : '%s membership required to access course.' % staff_group}
@@ -264,6 +269,7 @@ def logout_user(request):
 def change_setting(request):
     ''' JSON call to change a profile setting: Right now, location
     '''
+    # TODO (vshnayder): location is no longer used
     up = UserProfile.objects.get(user=request.user)  # request.user.profile_cache
     if 'location' in request.POST:
         up.location = request.POST['location']
@@ -272,6 +278,58 @@ def change_setting(request):
     return HttpResponse(json.dumps({'success': True,
                                     'location': up.location, }))
 
+def _do_create_account(post_vars):
+    """
+    Given cleaned post variables, create the User and UserProfile objects, as well as the
+    registration for this user.
+
+    Returns a tuple (User, UserProfile, Registration).
+
+    Note: this function is also used for creating test users.
+    """
+    user = User(username=post_vars['username'],
+             email=post_vars['email'],
+             is_active=False)
+    user.set_password(post_vars['password'])
+    registration = Registration()
+    # TODO: Rearrange so that if part of the process fails, the whole process fails.
+    # Right now, we can have e.g. no registration e-mail sent out and a zombie account
+    try:
+        user.save()
+    except IntegrityError:
+        # Figure out the cause of the integrity error
+        if len(User.objects.filter(username=post_vars['username'])) > 0:
+            js['value'] = "An account with this username already exists."
+            js['field'] = 'username'
+            return HttpResponse(json.dumps(js))
+
+        if len(User.objects.filter(email=post_vars['email'])) > 0:
+            js['value'] = "An account with this e-mail already exists."
+            js['field'] = 'email'
+            return HttpResponse(json.dumps(js))
+
+        raise
+
+    registration.register(user)
+
+    profile = UserProfile(user=user)
+    profile.name = post_vars['name']
+    profile.level_of_education = post_vars.get('level_of_education')
+    profile.gender = post_vars.get('gender')
+    profile.mailing_address = post_vars.get('mailing_address')
+    profile.goals = post_vars.get('goals')
+
+    try:
+        profile.year_of_birth = int(post_vars['year_of_birth'])
+    except (ValueError, KeyError):
+        profile.year_of_birth = None  # If they give us garbage, just ignore it instead
+                                # of asking them to put an integer.
+    try:
+        profile.save()
+    except Exception:
+        log.exception("UserProfile creation failed for user {0}.".format(user.id))
+    return (user, profile, registration)
+
 
 @ensure_csrf_cookie
 def create_account(request, post_override=None):
@@ -343,50 +401,11 @@ def create_account(request, post_override=None):
         js['field'] = 'username'
         return HttpResponse(json.dumps(js))
 
-    u = User(username=post_vars['username'],
-             email=post_vars['email'],
-             is_active=False)
-    u.set_password(post_vars['password'])
-    r = Registration()
-    # TODO: Rearrange so that if part of the process fails, the whole process fails.
-    # Right now, we can have e.g. no registration e-mail sent out and a zombie account
-    try:
-        u.save()
-    except IntegrityError:
-        # Figure out the cause of the integrity error
-        if len(User.objects.filter(username=post_vars['username'])) > 0:
-            js['value'] = "An account with this username already exists."
-            js['field'] = 'username'
-            return HttpResponse(json.dumps(js))
-
-        if len(User.objects.filter(email=post_vars['email'])) > 0:
-            js['value'] = "An account with this e-mail already exists."
-            js['field'] = 'email'
-            return HttpResponse(json.dumps(js))
-
-        raise
-
-    r.register(u)
-
-    up = UserProfile(user=u)
-    up.name = post_vars['name']
-    up.level_of_education = post_vars.get('level_of_education')
-    up.gender = post_vars.get('gender')
-    up.mailing_address = post_vars.get('mailing_address')
-    up.goals = post_vars.get('goals')
-
-    try:
-        up.year_of_birth = int(post_vars['year_of_birth'])
-    except (ValueError, KeyError):
-        up.year_of_birth = None  # If they give us garbage, just ignore it instead
-                                # of asking them to put an integer.
-    try:
-        up.save()
-    except Exception:
-        log.exception("UserProfile creation failed for user {0}.".format(u.id))
+    # Ok, looks like everything is legit.  Create the account.
+    (user, profile, registration) = _do_create_account(post_vars)
 
     d = {'name': post_vars['name'],
-         'key': r.activation_key,
+         'key': registration.activation_key,
          }
 
     # composes activation email
@@ -398,10 +417,11 @@ def create_account(request, post_override=None):
     try:
         if settings.MITX_FEATURES.get('REROUTE_ACTIVATION_EMAIL'):
             dest_addr = settings.MITX_FEATURES['REROUTE_ACTIVATION_EMAIL']
-            message = "Activation for %s (%s): %s\n" % (u, u.email, up.name) + '-' * 80 + '\n\n' + message
+            message = ("Activation for %s (%s): %s\n" % (user, user.email, profile.name) +
+                       '-' * 80 + '\n\n' + message)
             send_mail(subject, message, settings.DEFAULT_FROM_EMAIL, [dest_addr], fail_silently=False)
         elif not settings.GENERATE_RANDOM_USER_CREDENTIALS:
-            res = u.email_user(subject, message, settings.DEFAULT_FROM_EMAIL)
+            res = user.email_user(subject, message, settings.DEFAULT_FROM_EMAIL)
     except:
         log.exception(sys.exc_info())
         js['value'] = 'Could not send activation e-mail.'
@@ -431,24 +451,30 @@ def create_account(request, post_override=None):
     return HttpResponse(json.dumps(js), mimetype="application/json")
 
 
-def create_random_account(create_account_function):
-
+def get_random_post_override():
+    """
+    Return a dictionary suitable for passing to post_vars of _do_create_account or post_override
+    of create_account, with random user info.
+    """
     def id_generator(size=6, chars=string.ascii_uppercase + string.ascii_lowercase + string.digits):
         return ''.join(random.choice(chars) for x in range(size))
 
-    def inner_create_random_account(request):
-        post_override = {'username': "random_" + id_generator(),
+    return {'username': "random_" + id_generator(),
             'email': id_generator(size=10, chars=string.ascii_lowercase) + "_dummy_test@mitx.mit.edu",
             'password': id_generator(),
-                            'location': id_generator(size=5, chars=string.ascii_uppercase),
-                            'name': id_generator(size=5, chars=string.ascii_lowercase) + " " + id_generator(size=7, chars=string.ascii_lowercase),
+            'name': (id_generator(size=5, chars=string.ascii_lowercase) + " " +
+                     id_generator(size=7, chars=string.ascii_lowercase)),
                      'honor_code': u'true',
                      'terms_of_service': u'true', }
 
-        return create_account_function(request, post_override=post_override)
+
+def create_random_account(create_account_function):
+    def inner_create_random_account(request):
+        return create_account_function(request, post_override=get_random_post_override())
 
     return inner_create_random_account
 
+# TODO (vshnayder): do we need GENERATE_RANDOM_USER_CREDENTIALS for anything?
 if settings.GENERATE_RANDOM_USER_CREDENTIALS:
     create_account = create_random_account(create_account)
 
@@ -514,7 +540,7 @@ def reactivation_email(request):
     subject = ''.join(subject.splitlines())
     message = render_to_string('reactivation_email.txt', d)
 
-    res = u.email_user(subject, message, settings.DEFAULT_FROM_EMAIL)
+    res = user.email_user(subject, message, settings.DEFAULT_FROM_EMAIL)
 
     return HttpResponse(json.dumps({'success': True}))
 

common/lib/capa/capa/inputtypes.py

@@ -307,7 +307,17 @@ def filesubmission(element, value, status, render_template, msg=''):
     Upload a single file (e.g. for programming assignments)
     '''
     eid = element.get('id')
-    context = { 'id': eid, 'state': status, 'msg': msg, 'value': value, }
+
+    # Check if problem has been queued
+    queue_len = 0
+    if status == 'incomplete': # Flag indicating that the problem has been queued, 'msg' is length of queue
+        status = 'queued'
+        queue_len = msg
+        msg = 'Submitted to grader. (Queue length: %s)' % queue_len 
+
+    context = { 'id': eid, 'state': status, 'msg': msg, 'value': value, 
+                'queue_len': queue_len
+              }
     html = render_template("filesubmission.html", context)
     return etree.XML(html) 
 
@@ -330,9 +340,16 @@ def textbox(element, value, status, render_template, msg=''):
 
     if not value: value = element.text	 # if no student input yet, then use the default input given by the problem
     
+    # Check if problem has been queued
+    queue_len = 0
+    if status == 'incomplete': # Flag indicating that the problem has been queued, 'msg' is length of queue
+        status = 'queued' 
+        queue_len = msg
+        msg = 'Submitted to grader. (Queue length: %s)' % queue_len 
+
     # For CodeMirror
-    mode = element.get('mode')	or 'python' # mode, eg "python" or "xml"
-    linenumbers = element.get('linenumbers','true')	 # for CodeMirror
+    mode = element.get('mode','python') 
+    linenumbers = element.get('linenumbers','true')
     tabsize = element.get('tabsize','4')
     tabsize = int(tabsize)
 
@@ -340,6 +357,7 @@ def textbox(element, value, status, render_template, msg=''):
                'mode': mode, 'linenumbers': linenumbers,
                'rows': rows, 'cols': cols,
                'hidden': hidden, 'tabsize': tabsize,
+               'queue_len': queue_len,
                }
     html = render_template("textbox.html", context)
     try:

common/lib/capa/capa/responsetypes.py

@@ -898,7 +898,7 @@ class CodeResponse(LoncapaResponse):
                     'processor': self.code,
                     }
         
-        # Submit request
+        # Submit request. When successful, 'msg' is the prior length of the queue
         if is_file(submission):
             contents.update({'edX_student_response': submission.name})
             (error, msg) = qinterface.send_to_queue(header=xheader,
@@ -914,8 +914,11 @@ class CodeResponse(LoncapaResponse):
             cmap.set(self.answer_id, queuekey=None,
                      msg='Unable to deliver your submission to grader. (Reason: %s.) Please try again later.' % msg)
         else:
-            # Non-null CorrectMap['queuekey'] indicates that the problem has been queued 
-            cmap.set(self.answer_id, queuekey=queuekey, msg='Submitted to grader. (Queue length: %s)' % msg)
+            # Queueing mechanism flags:
+            #   1) Backend: Non-null CorrectMap['queuekey'] indicates that the problem has been queued
+            #   2) Frontend: correctness='incomplete' eventually trickles down through inputtypes.textbox 
+            #       and .filesubmission to inform the browser to poll the LMS
+            cmap.set(self.answer_id, queuekey=queuekey, correctness='incomplete', msg=msg)
 
         return cmap
 

common/lib/capa/capa/templates/filesubmission.html

@@ -6,8 +6,9 @@
     <span class="correct" id="status_${id}"></span>
     % elif state == 'incorrect':
     <span class="incorrect" id="status_${id}"></span>
-    % elif state == 'incomplete':
-    <span class="incorrect" id="status_${id}"></span>
+    % elif state == 'queued':
+    <span class="processing" id="status_${id}"></span>
+    <span style="display:none;" class="xqueue" id="${id}" >${queue_len}</span>
     % endif
     <span class="debug">(${state})</span>
     <br/>

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

@@ -13,8 +13,9 @@
   <span class="correct" id="status_${id}"></span>
   % elif state == 'incorrect':
   <span class="incorrect" id="status_${id}"></span>
-  % elif state == 'incomplete':
-  <span class="incorrect" id="status_${id}"></span>
+  % elif state == 'queued':
+  <span class="processing" id="status_${id}"></span>
+  <span style="display:none;" class="xqueue" id="${id}" >${queue_len}</span>
   % endif
   % if hidden:
   <div style="display:none;" name="${hidden}" inputid="input_${id}" />

common/lib/xmodule/xmodule/abtest_module.py

@@ -11,18 +11,19 @@ DEFAULT = "_DEFAULT_GROUP"
 
 
 def group_from_value(groups, v):
-    ''' Given group: (('a',0.3),('b',0.4),('c',0.3)) And random value
+    """
+    Given group: (('a', 0.3), ('b', 0.4), ('c', 0.3)) and random value v
     in [0,1], return the associated group (in the above case, return
-    'a' if v<0.3, 'b' if 0.3<=v<0.7, and 'c' if v>0.7
-'''
+    'a' if v < 0.3, 'b' if 0.3 <= v < 0.7, and 'c' if v > 0.7
+    """
     sum = 0
     for (g, p) in groups:
         sum = sum + p
         if sum > v:
             return g
 
-    # Round off errors might cause us to run to the end of the list
-    # If the do, return the last element
+    # Round off errors might cause us to run to the end of the list.
+    # If the do, return the last element.
     return g
 
 

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

@@ -49,6 +49,8 @@ padding-left: flex-gutter(9);
   }
 }
 
+
+
 div {
   p.status {
     text-indent: -9999px;
@@ -64,6 +66,16 @@ div {
     }
   }
 
+  &.processing {
+    p.status {
+      @include inline-block();
+      background: url('../images/spinner.gif') center center no-repeat;
+      height: 20px;
+      width: 20px;
+      text-indent: -9999px;
+    }
+  }
+
   &.correct, &.ui-icon-check {
     p.status {
       @include inline-block();
@@ -134,6 +146,15 @@ div {
       width: 14px;
     }
 
+    &.processing, &.ui-icon-check {
+      @include inline-block();
+      background: url('../images/spinner.gif') center center no-repeat;
+      height: 20px;
+      position: relative;
+      top: 6px;
+      width: 25px;
+    }
+
     &.correct, &.ui-icon-check {
       @include inline-block();
       background: url('../images/correct-icon.png') center center no-repeat;

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

@@ -12,7 +12,10 @@ class @Problem
   bind: =>
     MathJax.Hub.Queue ["Typeset", MathJax.Hub]
     window.update_schematics()
-    @inputs = @$("[id^=input_#{@element_id.replace(/problem_/, '')}_]")
+
+    problem_prefix = @element_id.replace(/problem_/,'')
+    @inputs = @$("[id^=input_#{problem_prefix}_]")
+    
     @$('section.action input:button').click @refreshAnswers
     @$('section.action input.check').click @check_fd
     #@$('section.action input.check').click @check
@@ -26,15 +29,37 @@ class @Problem
         @el.attr progress: response.progress_status
         @el.trigger('progressChanged')
 
+  queueing: =>
+    @queued_items = @$(".xqueue")
+    if @queued_items.length > 0
+      if window.queuePollerID # Only one poller 'thread' per Problem
+        window.clearTimeout(window.queuePollerID)
+      window.queuePollerID = window.setTimeout(@poll, 100)
+
+  poll: =>
+    $.postWithPrefix "#{@url}/problem_get", (response) =>
+      @el.html(response.html)
+      @executeProblemScripts()
+      @bind()
+
+      @queued_items = @$(".xqueue")
+      if @queued_items.length == 0 
+        delete window.queuePollerID
+      else
+        # TODO: Dynamically adjust timeout interval based on @queued_items.value
+        window.queuePollerID = window.setTimeout(@poll, 1000)
+
   render: (content) ->
     if content
       @el.html(content)
       @bind()
+      @queueing()
     else
       $.postWithPrefix "#{@url}/problem_get", (response) =>
         @el.html(response.html)
         @executeProblemScripts()
         @bind()
+        @queueing()
 
   executeProblemScripts: ->
     @el.find(".script_placeholder").each (index, placeholder) ->

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

@@ -91,6 +91,13 @@ class @Sequence
     event.preventDefault()
     new_position = $(event.target).data('element')
     Logger.log "seq_goto", old: @position, new: new_position, id: @id
+
+    # On Sequence chage, destroy any existing polling thread 
+    #   for queued submissions, see ../capa/display.coffee
+    if window.queuePollerID
+      window.clearTimeout(window.queuePollerID)
+      delete window.queuePollerID
+
     @render new_position
 
   next: (event) =>

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

@@ -55,6 +55,9 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
         if json_data is None:
             return self.modulestore.get_item(location)
         else:
+            # TODO (vshnayder): metadata inheritance is somewhat broken because mongo, doesn't
+            # always load an entire course.  We're punting on this until after launch, and then
+            # will build a proper course policy framework.
             return XModuleDescriptor.load_from_json(json_data, self, self.default_class)
 
 

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

@@ -213,6 +213,12 @@ class XMLModuleStore(ModuleStoreBase):
             system = ImportSystem(self, org, course, course_dir, tracker)
 
             course_descriptor = system.process_xml(etree.tostring(course_data))
+            # NOTE: The descriptors end up loading somewhat bottom up, which
+            # breaks metadata inheritance via get_children().  Instead
+            # (actually, in addition to, for now), we do a final inheritance pass
+            # after we have the course descriptor.
+            XModuleDescriptor.compute_inherited_metadata(course_descriptor)
+
             log.debug('========> Done with course import from {0}'.format(course_dir))
             return course_descriptor
 

common/lib/xmodule/xmodule/tests/test_import.py

@@ -8,8 +8,11 @@ from xmodule.x_module import XMLParsingSystem, XModuleDescriptor
 from xmodule.xml_module import is_pointer_tag
 from xmodule.errortracker import make_error_tracker
 from xmodule.modulestore import Location
+from xmodule.modulestore.xml import XMLModuleStore
 from xmodule.modulestore.exceptions import ItemNotFoundError
 
+from .test_export import DATA_DIR
+
 ORG = 'test_org'
 COURSE = 'test_course'
 
@@ -185,3 +188,22 @@ class ImportTestCase(unittest.TestCase):
             chapter_xml = etree.fromstring(f.read())
         self.assertEqual(chapter_xml.tag, 'chapter')
         self.assertFalse('graceperiod' in chapter_xml.attrib)
+
+    def test_metadata_inherit(self):
+        """Make sure that metadata is inherited properly"""
+
+        print "Starting import"
+        initial_import = XMLModuleStore(DATA_DIR, eager=True, course_dirs=['toy'])
+
+        courses = initial_import.get_courses()
+        self.assertEquals(len(courses), 1)
+        course = courses[0]
+
+        def check_for_key(key, node):
+            "recursive check for presence of key"
+            print "Checking {}".format(node.location.url())
+            self.assertTrue(key in node.metadata)
+            for c in node.get_children():
+                check_for_key(key, c)
+
+        check_for_key('graceperiod', course)

common/lib/xmodule/xmodule/x_module.py

@@ -403,6 +403,18 @@ class XModuleDescriptor(Plugin, HTMLSnippet):
         return dict((k,v) for k,v in self.metadata.items()
                     if k not in self._inherited_metadata)
 
+    @staticmethod
+    def compute_inherited_metadata(node):
+        """Given a descriptor, traverse all of its descendants and do metadata
+        inheritance.  Should be called on a CourseDescriptor after importing a
+        course.
+
+        NOTE: This means that there is no such thing as lazy loading at the
+        moment--this accesses all the children."""
+        for c in node.get_children():
+            c.inherit_metadata(node.metadata)
+            XModuleDescriptor.compute_inherited_metadata(c)
+
     def inherit_metadata(self, metadata):
         """
         Updates this module with metadata inherited from a containing module.
@@ -423,6 +435,9 @@ class XModuleDescriptor(Plugin, HTMLSnippet):
             self._child_instances = []
             for child_loc in self.definition.get('children', []):
                 child = self.system.load_item(child_loc)
+                # TODO (vshnayder): this should go away once we have
+                # proper inheritance support in mongo.  The xml
+                # datastore does all inheritance on course load.
                 child.inherit_metadata(self.metadata)
                 self._child_instances.append(child)
 

lms/djangoapps/courseware/courses.py

@@ -145,6 +145,8 @@ def has_staff_access_to_course(user, course):
     '''
     Returns True if the given user has staff access to the course.
     This means that user is in the staff_* group, or is an overall admin.
+    TODO (vshnayder): this needs to be changed to allow per-course_id permissions, not per-course
+    (e.g. staff in 2012 is different from 2013, but maybe some people always have access)
 
     course is the course field of the location being accessed.
     '''
@@ -156,13 +158,18 @@ def has_staff_access_to_course(user, course):
     # note this is the Auth group, not UserTestGroup
     user_groups = [x[1] for x in user.groups.values_list()]
     staff_group = course_staff_group_name(course)
-    log.debug('course %s, staff_group %s, user %s, groups %s' % (
-        course, staff_group, user, user_groups))
     if staff_group in user_groups:
         return True
     return False
 
-def has_access_to_course(user,course):
+def has_staff_access_to_course_id(user, course_id):
+    """Helper method that takes a course_id instead of a course name"""
+    loc = CourseDescriptor.id_to_location(course_id)
+    return has_staff_access_to_course(user, loc.course)
+
+
+def has_access_to_course(user, course):
+    '''course is the .course element of a location'''
     if course.metadata.get('ispublic'):
         return True
     return has_staff_access_to_course(user,course)

lms/djangoapps/courseware/grades.py

+# Compute grades using real division, with no integer truncation
+from __future__ import division
+
 import random
 import logging
 
@@ -179,7 +182,7 @@ def progress_summary(student, course, grader, student_module_cache):
 
 def get_score(user, problem, student_module_cache):
     """
-    Return the score for a user on a problem
+    Return the score for a user on a problem, as a tuple (correct, total).
 
     user: a Student object
     problem: an XModule

lms/djangoapps/courseware/module_render.py

@@ -47,7 +47,8 @@ def toc_for_course(user, request, course, active_chapter, active_section):
        'format': format, 'due': due, 'active' : bool}, ...]
 
     active is set for the section and chapter corresponding to the passed
-    parameters.  Everything else comes from the xml, or defaults to "".
+    parameters, which are expected to be url_names of the chapter+section.
+    Everything else comes from the xml, or defaults to "".
 
     chapters with name 'hidden' are skipped.
     '''
@@ -60,8 +61,8 @@ def toc_for_course(user, request, course, active_chapter, active_section):
         sections = list()
         for section in chapter.get_display_items():
 
-            active = (chapter.display_name == active_chapter and
-                      section.display_name == active_section)
+            active = (chapter.url_name == active_chapter and
+                      section.url_name == active_section)
             hide_from_toc = section.metadata.get('hide_from_toc', 'false').lower() == 'true'
 
             if not hide_from_toc:
@@ -74,7 +75,7 @@ def toc_for_course(user, request, course, active_chapter, active_section):
         chapters.append({'display_name': chapter.display_name,
                          'url_name': chapter.url_name,
                          'sections': sections,
-                         'active': chapter.display_name == active_chapter})
+                         'active': chapter.url_name == active_chapter})
     return chapters
 
 

lms/djangoapps/courseware/tests/tests.py

@@ -13,8 +13,9 @@ from django.core.urlresolvers import reverse
 from mock import patch, Mock
 from override_settings import override_settings
 
-from django.contrib.auth.models import User
+from django.contrib.auth.models import User, Group
 from student.models import Registration
+from courseware.courses import course_staff_group_name
 
 from xmodule.modulestore.django import modulestore
 import xmodule.modulestore.django
@@ -88,6 +89,13 @@ class ActivateLoginTestCase(TestCase):
         self.assertTrue(data['success'])
         return resp
 
+    def logout(self):
+        '''Logout, check that it worked.'''
+        resp = self.client.get(reverse('logout'), {})
+        # should redirect
+        self.assertEqual(resp.status_code, 302)
+        return resp
+
     def _create_account(self, username, email, pw):
         '''Try to create an account.  No error checking'''
         resp = self.client.post('/create_account', {
@@ -131,12 +139,16 @@ class ActivateLoginTestCase(TestCase):
         '''The setup function does all the work'''
         pass
 
+    def test_logout(self):
+        '''Setup function does login'''
+        self.logout()
+
 
 class PageLoader(ActivateLoginTestCase):
     ''' Base class that adds a function to load all pages in a modulestore '''
 
-
     def enroll(self, course):
+        """Enroll the currently logged-in user, and check that it worked."""
         resp = self.client.post('/change_enrollment', {
             'enrollment_action': 'enroll',
             'course_id': course.id,
@@ -193,7 +205,99 @@ class TestCoursesLoadTestCase(PageLoader):
         self.check_pages_load('full', TEST_DATA_DIR, modulestore())
 
 
-    # ========= TODO: check ajax interaction here too?
+@override_settings(MODULESTORE=TEST_DATA_MODULESTORE)
+class TestInstructorAuth(PageLoader):
+    """Check that authentication works properly"""
+
+    # NOTE: setUpClass() runs before override_settings takes effect, so
+    # can't do imports there without manually hacking settings.
+
+    def setUp(self):
+        xmodule.modulestore.django._MODULESTORES = {}
+        modulestore().collection.drop()
+        import_from_xml(modulestore(), TEST_DATA_DIR, ['toy'])
+        import_from_xml(modulestore(), TEST_DATA_DIR, ['full'])
+        courses = modulestore().get_courses()
+        # get the two courses sorted out
+        courses.sort(key=lambda c: c.location.course)
+        [self.full, self.toy] = courses
+
+        # Create two accounts
+        self.student = 'view@test.com'
+        self.instructor = 'view2@test.com'
+        self.password = 'foo'
+        self.create_account('u1', self.student, self.password)
+        self.create_account('u2', self.instructor, self.password)
+        self.activate_user(self.student)
+        self.activate_user(self.instructor)
+
+    def check_for_get_code(self, code, url):
+        resp = self.client.get(url)
+        # HACK: workaround the bug that returns 200 instead of 404.
+        # TODO (vshnayder): once we're returning 404s, get rid of this if.
+        if code != 404:
+            self.assertEqual(resp.status_code, code)
+        else:
+            # look for "page not found" instead of the status code
+            self.assertTrue(resp.content.lower().find('page not found') != -1)
+
+    def test_instructor_page(self):
+        "Make sure only instructors can load it"
+
+        # First, try with an enrolled student
+        self.login(self.student, self.password)
+        # shouldn't work before enroll
+        self.check_for_get_code(302, reverse('courseware', kwargs={'course_id': self.toy.id}))
+        self.enroll(self.toy)
+        self.enroll(self.full)
+        # should work now
+        self.check_for_get_code(200, reverse('courseware', kwargs={'course_id': self.toy.id}))
+
+        def instructor_urls(course):
+            "list of urls that only instructors/staff should be able to see"
+            urls = [reverse(name, kwargs={'course_id': course.id}) for name in (
+                'instructor_dashboard',
+                'gradebook',
+                'grade_summary',)]
+            urls.append(reverse('student_profile', kwargs={'course_id': course.id,
+                                                     'student_id': user(self.student).id}))
+            return urls
+
+        # shouldn't be able to get to the instructor pages
+        for url in instructor_urls(self.toy) + instructor_urls(self.full):
+            print 'checking for 404 on {}'.format(url)
+            self.check_for_get_code(404, url)
+
+        # Make the instructor staff in the toy course
+        group_name = course_staff_group_name(self.toy)
+        g = Group.objects.create(name=group_name)
+        g.user_set.add(user(self.instructor))
+
+        self.logout()
+        self.login(self.instructor, self.password)
+
+        # Now should be able to get to the toy course, but not the full course
+        for url in instructor_urls(self.toy):
+            print 'checking for 200 on {}'.format(url)
+            self.check_for_get_code(200, url)
+
+        for url in instructor_urls(self.full):
+            print 'checking for 404 on {}'.format(url)
+            self.check_for_get_code(404, url)
+
+
+        # now also make the instructor staff
+        u = user(self.instructor)
+        u.is_staff = True
+        u.save()
+
+        # and now should be able to load both
+        for url in instructor_urls(self.toy) + instructor_urls(self.full):
+            print 'checking for 200 on {}'.format(url)
+            self.check_for_get_code(200, url)
+
+
+
 
 
 @override_settings(MODULESTORE=REAL_DATA_MODULESTORE)

lms/envs/test.py

@@ -25,6 +25,7 @@ SOUTH_TESTS_MIGRATE = False # To disable migrations and use syncdb instead
 # Nose Test Runner
 INSTALLED_APPS += ('django_nose',)
 NOSE_ARGS = ['--cover-erase', '--with-xunit', '--with-xcoverage', '--cover-html',
+             # '-v', '--pdb',     # When really stuck, uncomment to start debugger on error
              '--cover-inclusive', '--cover-html-dir',
              os.environ.get('NOSE_COVER_HTML_DIR', 'cover_html')]
 for app in os.listdir(PROJECT_ROOT / 'djangoapps'):

lms/static/coffee/src/navigation.coffee

 class @Navigation
   constructor: ->
     if $('#accordion').length
+      # First look for an active section
       active = $('#accordion ul:has(li.active)').index('#accordion ul')
+      # if we didn't find one, look for an active chapter
+      if active < 0
+        active = $('#accordion h3.active').index('#accordion h3')
+      # if that didn't work either, default to 0
+      if active < 0
+        active = 0
       $('#accordion').bind('accordionchange', @log).accordion
-        active: if active >= 0 then active else 1
+        active: active
         header: 'h3'
         autoHeight: false
       $('#open_close_accordion a').click @toggle

lms/static/sass/base/_extends.scss

@@ -89,3 +89,9 @@
   border: 1px solid rgb(6, 65, 18);
   color: rgb(255, 255, 255);
 }
+
+.global {
+  h2 {
+    display: none;
+  }
+}
\ No newline at end of file

lms/static/sass/course/_textbook.scss

@@ -9,6 +9,27 @@ div.book-wrapper {
     ul#booknav {
       font-size: em(14);
 
+      .chapter-number {
+        
+      }
+
+      .chapter {
+        float: left;
+        width: 87%;
+        line-height: 1.4em;
+      }
+
+      .page-number {
+        float: right;
+        width: 12%;
+        font-size: .8em;
+        line-height: 2.1em;
+        text-align: right;
+        color: #9a9a9a;
+        opacity: 0;
+        @include transition(opacity .15s);
+      }
+
       li {
         background: none;
         border-bottom: 0;
@@ -16,9 +37,14 @@ div.book-wrapper {
 
         a {
           padding: 0;
+          @include clearfix;
 
           &:hover {
             background-color: transparent;
+
+            .page-number {
+              opacity: 1;
+            }
           }
         }
 

lms/static/sass/course/base/_base.scss

+body {
+  min-width: 980px;
+}
+
 body, h1, h2, h3, h4, h5, h6, p, p a:link, p a:visited, a {
   font-family: $sans-serif;
 }

lms/static/sass/course/base/_extends.scss

@@ -185,3 +185,30 @@ h1.top-header {
 .tran {
   @include transition( all, .2s, $ease-in-out-quad);
 }
+
+.global {
+  .find-courses-button {
+    display: none;
+  }
+
+  h2 {
+    display: block;
+    width: 700px;
+    float: left;
+    font-size: 0.9em;
+    font-weight: 600;
+    line-height: 40px;
+    letter-spacing: 0;
+    text-transform: none;
+    text-shadow: 0 1px 0 #fff;
+    white-space: nowrap;
+    text-overflow: ellipsis;
+    overflow: hidden;
+
+    .provider {
+      font: inherit;
+      font-weight: bold;
+      color: #6d6d6d;
+    }
+  }
+}
\ No newline at end of file

lms/templates/accordion.html

 <%! from django.core.urlresolvers import reverse %>
 
 <%def name="make_chapter(chapter)">
-  <h3><a href="#">${chapter['display_name']}</a></h3>
+  <h3 ${' class="active"' if 'active' in chapter and chapter['active'] else ''}><a href="#">${chapter['display_name']}</a>
+  </h3>
 
   <ul>
     % for section in chapter['sections']:

lms/templates/course_navigation.html

@@ -7,6 +7,7 @@ def url_class(url):
   return ""
 %>
 <%! from django.core.urlresolvers import reverse %>
+<%! from courseware.courses import has_staff_access_to_course_id %>
 
 <nav class="${active_page} course-material">
   <div class="inner-wrapper">
@@ -27,6 +28,10 @@ def url_class(url):
 % if user.is_authenticated():
       <li class="profile"><a href="${reverse('profile', args=[course.id])}" class="${url_class('profile')}">Profile</a></li>
 % endif
+% if has_staff_access_to_course_id(user, course.id):
+      <li class="instructor"><a href="${reverse('instructor_dashboard', args=[course.id])}" class="${url_class('instructor')}">Instructor</a></li>
+% endif
+
     </ol>
   </div>
 </nav>
\ No newline at end of file

lms/templates/grade_summary.html

+<%inherit file="main.html" />
+<%! from django.core.urlresolvers import reverse %>
+<%namespace name='static' file='static_content.html'/>
+
+<%include file="course_navigation.html" args="active_page=''" />
+
+<section class="container">
+<div class="gradebook-summary-wrapper">
+  <section class="gradebook-summary-content">
+    <h1>Grade summary</h1>
+
+    <p>Not implemented yet</p>
+
+  </section>
+</div>
+</section>

lms/templates/gradebook.html

 <%inherit file="main.html" />
+<%! from django.core.urlresolvers import reverse %>
 <%namespace name='static' file='static_content.html'/>
 
 <%block name="js_extra">
@@ -42,24 +43,27 @@
         <th>Total</th>
       </tr>
 
-      <%def name="percent_data(percentage)">
+      <%def name="percent_data(fraction)">
         <%
           letter_grade = 'None'
-          if percentage > 0:
+          if fraction > 0:
             letter_grade = 'F'
             for grade in ['A', 'B', 'C']:
-              if percentage >= course.grade_cutoffs[grade]:
+              if fraction >= course.grade_cutoffs[grade]:
                 letter_grade = grade
                 break
 
           data_class = "grade_" + letter_grade
         %>
-        <td class="${data_class}" data-percent="${percentage}">${ "{0:.0%}".format( percentage ) }</td>
+        <td class="${data_class}" data-percent="${fraction}">${ "{0:.0f}".format( 100 * fraction ) }</td>
       </%def>
 
       %for student in students:
       <tr>
-        <td><a href="/profile/${student['id']}/">${student['username']}</a></td>
+        <td><a href="${reverse('student_profile',
+                               kwargs={'course_id' : course_id,
+                                       'student_id': student['id']})}">
+                      ${student['username']}</a></td>
         %for section in student['grade_summary']['section_breakdown']:
           ${percent_data( section['percent'] )}
         %endfor

lms/templates/instructor_dashboard.html

+<%inherit file="main.html" />
+<%! from django.core.urlresolvers import reverse %>
+<%namespace name='static' file='static_content.html'/>
+
+<%block name="headextra">
+  <%static:css group='course'/>
+</%block>
+
+<%include file="course_navigation.html" args="active_page='instructor'" />
+
+<section class="container">
+<div class="instructor-dashboard-wrapper">
+  <section class="instructor-dashboard-content">
+    <h1>Instructor Dashboard</h1>
+
+    <p>
+    <a href="${reverse('gradebook', kwargs={'course_id': course.id})}">Gradebook</a>
+
+    <p>
+    <a href="${reverse('grade_summary', kwargs={'course_id': course.id})}">Grade summary</a>
+
+  </section>
+</div>
+</section>

lms/templates/navigation.html

@@ -7,7 +7,12 @@
 <header class="global" aria-label="Global Navigation">
   <nav>
     <h1 class="logo"><a href="${reverse('root')}"></a></h1>
-    <ol class="left">
+
+    %if course:
+      <h2><span class="provider">${course.org}:</span> ${course.number} ${course.title}</h2>
+    %endif
+
+    <ol class="left find-courses-button">
       <li class="primary">
         <a href="${reverse('courses')}">Find Courses</a>
       </li>

lms/templates/staticbook.html

@@ -75,7 +75,14 @@ $("#open_close_accordion a").click(function(){
           <%def name="print_entry(entry)">
               <li>
                   <a href="javascript:goto_page(${entry.get('page')})">
-                      ${' '.join(entry.get(attribute, '') for attribute in ['chapter', 'name', 'page_label'])}
+                      <span class="chapter">
+                        %if entry.get('chapter'):
+                          <span class="chapter-number">${entry.get('chapter')}.</span> ${entry.get('name')}
+                        %else:
+                          ${entry.get('name')}
+                        %endif
+                      </span>
+                      <span class="page-number">${entry.get('page_label')}</span>
                   </a>
                   % if len(entry) > 0:
                       <ul>

lms/urls.py

@@ -84,7 +84,6 @@ urlpatterns = ('',
     (r'^pressrelease$', 'django.views.generic.simple.redirect_to', {'url': '/press/uc-berkeley-joins-edx'}),
 
 
-
     (r'^favicon\.ico$', 'django.views.generic.simple.redirect_to', {'url': '/static/images/favicon.ico'}),
 
     # TODO: These urls no longer work. They need to be updated before they are re-enabled
@@ -133,16 +132,24 @@ if settings.COURSEWARE_ENABLED:
             'staticbook.views.index_shifted'),
         url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/courseware/?$',
             'courseware.views.index', name="courseware"),
+        url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/courseware/(?P<chapter>[^/]*)/$',
+            'courseware.views.index', name="courseware_chapter"),
         url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/courseware/(?P<chapter>[^/]*)/(?P<section>[^/]*)/$',
             'courseware.views.index', name="courseware_section"),
         url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/profile$',
             'courseware.views.profile', name="profile"),
+        # Takes optional student_id for instructor use--shows profile as that student sees it.
         url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/profile/(?P<student_id>[^/]*)/$',
-            'courseware.views.profile'),
+            'courseware.views.profile', name="student_profile"),
 
         # For the instructor
+        url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/instructor$',
+            'courseware.views.instructor_dashboard', name="instructor_dashboard"),
         url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/gradebook$',
-            'courseware.views.gradebook'),        
+            'courseware.views.gradebook', name='gradebook'),
+        url(r'^courses/(?P<course_id>[^/]+/[^/]+/[^/]+)/grade_summary$',
+            'courseware.views.grade_summary', name='grade_summary'),
+
     )
 
     # Multicourse wiki