Merge branch 'master' into jkarni/docs-merge

CHANGELOG.rst

@@ -9,6 +9,9 @@ Studio: Send e-mails to new Studio users (on edge only) when their course creato
 status has changed. This will not be in use until the course creator table
 is enabled.
 
+Studio: Added improvements to Course Creation: richer error messaging, tip
+text, and fourth field for course run.
+
 LMS: Added user preferences (arbitrary user/key/value tuples, for which
 which user/key is unique) and a REST API for reading users and
 preferences. Access to the REST API is restricted by use of the
@@ -18,6 +21,9 @@ the setting is not present, the API is disabled).
 LMS: Added endpoints for AJAX requests to enable/disable notifications
 (which are not yet implemented) and a one-click unsubscribe page.
 
+Studio: Allow instructors of a course to designate other staff as instructors;
+this allows instructors to hand off management of a course to someone else.
+
 Common: Add a manage.py that knows about edx-platform specific settings and projects
 
 Common: Added *experimental* support for jsinput type.

cms/djangoapps/auth/authz.py

@@ -178,10 +178,12 @@ def _remove_user_from_group(user, group_name):
     user.save()
 
 
-def is_user_in_course_group_role(user, location, role):
+def is_user_in_course_group_role(user, location, role, check_staff=True):
     if user.is_active and user.is_authenticated:
         # all "is_staff" flagged accounts belong to all groups
-        return user.is_staff or user.groups.filter(name=get_course_groupname_for_role(location, role)).count() > 0
+        if check_staff and user.is_staff:
+            return True
+        return user.groups.filter(name=get_course_groupname_for_role(location, role)).count() > 0
 
     return False
 

cms/djangoapps/contentstore/course_info_model.py

@@ -1,7 +1,7 @@
 from xmodule.modulestore.exceptions import ItemNotFoundError
 from xmodule.modulestore import Location
 from xmodule.modulestore.django import modulestore
-from lxml import html
+from lxml import html, etree
 import re
 from django.http import HttpResponseBadRequest
 import logging
@@ -74,34 +74,44 @@ def update_course_updates(location, update, passed_id=None):
         escaped = django.utils.html.escape(course_updates.data)
         course_html_parsed = html.fromstring("<ol><li>" + escaped + "</li></ol>")
 
+    # if there's no ol, create it
+    if course_html_parsed.tag != 'ol':
+        # surround whatever's there w/ an ol
+        if course_html_parsed.tag != 'li':
+            # but first wrap in an li
+            li = etree.Element('li')
+            li.append(course_html_parsed)
+            course_html_parsed = li
+        ol = etree.Element('ol')
+        ol.append(course_html_parsed)
+        course_html_parsed = ol
+
     # No try/catch b/c failure generates an error back to client
     new_html_parsed = html.fromstring('<li><h2>' + update['date'] + '</h2>' + update['content'] + '</li>')
 
-    # Confirm that root is <ol>, iterate over <li>, pull out <h2> subs and then rest of val
-    if course_html_parsed.tag == 'ol':
-        # ??? Should this use the id in the json or in the url or does it matter?
-        if passed_id is not None:
-            idx = get_idx(passed_id)
-            # idx is count from end of list
-            course_html_parsed[-idx] = new_html_parsed
-        else:
-            course_html_parsed.insert(0, new_html_parsed)
+    # ??? Should this use the id in the json or in the url or does it matter?
+    if passed_id is not None:
+        idx = get_idx(passed_id)
+        # idx is count from end of list
+        course_html_parsed[-idx] = new_html_parsed
+    else:
+        course_html_parsed.insert(0, new_html_parsed)
 
-            idx = len(course_html_parsed)
-            passed_id = course_updates.location.url() + "/" + str(idx)
+        idx = len(course_html_parsed)
+        passed_id = course_updates.location.url() + "/" + str(idx)
 
-        # update db record
-        course_updates.data = html.tostring(course_html_parsed)
-        modulestore('direct').update_item(location, course_updates.data)
+    # update db record
+    course_updates.data = html.tostring(course_html_parsed)
+    modulestore('direct').update_item(location, course_updates.data)
 
-        if (len(new_html_parsed) == 1):
-            content = new_html_parsed[0].tail
-        else:
-            content = "\n".join([html.tostring(ele) for ele in new_html_parsed[1:]])
+    if (len(new_html_parsed) == 1):
+        content = new_html_parsed[0].tail
+    else:
+        content = "\n".join([html.tostring(ele) for ele in new_html_parsed[1:]])
 
-        return {"id": passed_id,
-                "date": update['date'],
-                "content": content}
+    return {"id": passed_id,
+            "date": update['date'],
+            "content": content}
 
 
 def delete_course_update(location, update, passed_id):

cms/djangoapps/contentstore/features/common.py

@@ -53,6 +53,14 @@ def i_have_opened_a_new_course(_step):
     open_new_course()
 
 
+@step('(I select|s?he selects) the new course')
+def select_new_course(_step, whom):
+    course_link_xpath = '//div[contains(@class, "courses")]//a[contains(@class, "class-link")]//span[contains(., "{name}")]/..'.format(
+        name="Robot Super Course")
+    element = world.browser.find_by_xpath(course_link_xpath)
+    element.click()
+
+
 @step(u'I press the "([^"]*)" notification button$')
 def press_the_notification_button(_step, name):
     css = 'a.action-%s' % name.lower()
@@ -118,14 +126,18 @@ def create_studio_user(
     registration.register(studio_user)
     registration.activate()
 
+    return studio_user
+
 
 def fill_in_course_info(
         name='Robot Super Course',
         org='MITx',
-        num='999'):
+        num='101',
+        run='2013_Spring'):
     world.css_fill('.new-course-name', name)
     world.css_fill('.new-course-org', org)
     world.css_fill('.new-course-number', num)
+    world.css_fill('.new-course-run', run)
 
 
 def log_into_studio(
@@ -242,7 +254,7 @@ def save_button_disabled(step):
 @step('I confirm the prompt')
 def confirm_the_prompt(step):
     prompt_css = 'a.button.action-primary'
-    world.css_click(prompt_css)
+    world.css_click(prompt_css, success_condition=lambda: not world.css_visible(prompt_css))
 
 
 @step(u'I am shown a (.*)$')
@@ -252,6 +264,7 @@ def i_am_shown_a_notification(step, notification_type):
 
 def type_in_codemirror(index, text):
     world.css_click(".CodeMirror", index=index)
+    world.browser.execute_script("$('div.CodeMirror.CodeMirror-focused > div').css('overflow', '')")
     g = world.css_find("div.CodeMirror.CodeMirror-focused > div > textarea")
     if world.is_mac():
         g._element.send_keys(Keys.COMMAND + 'a')

cms/djangoapps/contentstore/features/component_settings_editor_helpers.py

@@ -12,11 +12,20 @@ def create_component_instance(step, component_button_css, category,
                               has_multiple_templates=True):
 
     click_new_component_button(step, component_button_css)
+    if category in ('problem', 'html'):
+        def animation_done(_driver):
+            return world.browser.evaluate_script("$('div.new-component').css('display')") == 'none'
+        world.wait_for(animation_done)
 
     if has_multiple_templates:
         click_component_from_menu(category, boilerplate, expected_css)
 
-    assert_equal(1, len(world.css_find(expected_css)))
+    assert_equal(
+        1,
+        len(world.css_find(expected_css)),
+        "Component instance with css {css} was not created successfully".format(css=expected_css))
+
+
 
 @world.absorb
 def click_new_component_button(step, component_button_css):
@@ -39,11 +48,13 @@ def click_component_from_menu(category, boilerplate, expected_css):
         elem_css = "a[data-category='{}']:not([data-boilerplate])".format(category)
     elements = world.css_find(elem_css)
     assert_equal(len(elements), 1)
-    world.css_click(elem_css)
+    world.wait_for(lambda _driver: world.css_visible(elem_css))
+    world.css_click(elem_css, success_condition=lambda: 1 == len(world.css_find(expected_css)))
 
 
 @world.absorb
 def edit_component_and_select_settings():
+    world.wait_for(lambda _driver: world.css_visible('a.edit-button'))
     world.css_click('a.edit-button')
     world.css_click('#settings-mode')
 

cms/djangoapps/contentstore/features/course-overview.feature

@@ -63,3 +63,10 @@ Feature: Course Overview
         When I navigate to the course overview page
         And I change an assignment's grading status
         Then I am shown a notification
+
+   Scenario: Notification is shown on subsection reorder
+        Given I have opened a new course section in Studio
+        And I have added a new subsection
+        And I have added a new subsection
+        When I reorder subsections
+        Then I am shown a notification

cms/djangoapps/contentstore/features/course-overview.py

@@ -124,3 +124,14 @@ def all_sections_are_collapsed(step):
 def change_grading_status(step):
     world.css_find('a.menu-toggle').click()
     world.css_find('.menu li').first.click()
+
+
+@step(u'I reorder subsections')
+def reorder_subsections(_step):
+    draggable_css = 'a.drag-handle'
+    ele = world.css_find(draggable_css).first
+    ele.action_chains.drag_and_drop_by_offset(
+        ele._element,
+        30,
+        0
+    ).perform()

cms/djangoapps/contentstore/features/course-team.feature

@@ -1,7 +1,7 @@
 Feature: Course Team
     As a course author, I want to be able to add others to my team
 
-    Scenario: Users can add other users
+    Scenario: Admins can add other users
         Given I have opened a new course in Studio
         And the user "alice" exists
         And I am viewing the course team settings
@@ -9,7 +9,7 @@ Feature: Course Team
         And "alice" logs in
         Then she does see the course on her page
 
-    Scenario: Added users cannot delete or add other users
+    Scenario: Added admins cannot delete or add other users
         Given I have opened a new course in Studio
         And the user "bob" exists
         And I am viewing the course team settings
@@ -18,7 +18,7 @@ Feature: Course Team
         Then he cannot delete users
         And he cannot add users
 
-    Scenario: Users can delete other users
+    Scenario: Admins can delete other users
         Given I have opened a new course in Studio
         And the user "carol" exists
         And I am viewing the course team settings
@@ -27,8 +27,33 @@ Feature: Course Team
         And "carol" logs in
         Then she does not see the course on her page
 
-    Scenario: Users cannot add users that do not exist
+    Scenario: Admins cannot add users that do not exist
         Given I have opened a new course in Studio
         And I am viewing the course team settings
         When I add "dennis" to the course team
         Then I should see "Could not find user by email address" somewhere on the page
+
+    Scenario: Admins should be able to make other people into admins
+        Given I have opened a new course in Studio
+        And the user "emily" exists
+        And I am viewing the course team settings
+        And I add "emily" to the course team
+        When I make "emily" a course team admin
+        And "emily" logs in
+        And she selects the new course
+        And she views the course team settings
+        Then "emily" should be marked as an admin
+        And she can add users
+        And she can delete users
+
+    Scenario: Admins should be able to remove other admins
+        Given I have opened a new course in Studio
+        And the user "frank" exists as a course admin
+        And I am viewing the course team settings
+        When I remove admin rights from "frank"
+        And "frank" logs in
+        And he selects the new course
+        And he views the course team settings
+        Then "frank" should not be marked as an admin
+        And he cannot add users
+        And he cannot delete users

cms/djangoapps/contentstore/features/course-team.py

@@ -3,65 +3,105 @@
 
 from lettuce import world, step
 from common import create_studio_user, log_into_studio
+from django.contrib.auth.models import Group
+from auth.authz import get_course_groupname_for_role
 
 PASSWORD = 'test'
 EMAIL_EXTENSION = '@edx.org'
 
 
-@step(u'I am viewing the course team settings')
-def view_grading_settings(_step):
+@step(u'(I am viewing|s?he views) the course team settings')
+def view_grading_settings(_step, whom):
     world.click_course_settings()
     link_css = 'li.nav-course-settings-team a'
     world.css_click(link_css)
 
 
-@step(u'the user "([^"]*)" exists$')
-def create_other_user(_step, name):
-    create_studio_user(uname=name, password=PASSWORD, email=(name + EMAIL_EXTENSION))
+@step(u'the user "([^"]*)" exists( as a course admin)?$')
+def create_other_user(_step, name, course_admin):
+    user = create_studio_user(uname=name, password=PASSWORD, email=(name + EMAIL_EXTENSION))
+    if course_admin:
+        location = world.scenario_dict["COURSE"].location
+        for role in ("staff", "instructor"):
+            group, __ = Group.objects.get_or_create(name=get_course_groupname_for_role(location, role))
+            user.groups.add(group)
+        user.save()
 
 
 @step(u'I add "([^"]*)" to the course team')
 def add_other_user(_step, name):
-    new_user_css = 'a.new-user-button'
+    new_user_css = 'a.create-user-button'
     world.css_click(new_user_css)
+    world.wait(0.5)
 
-    email_css = 'input.email-input'
+    email_css = 'input#user-email-input'
     f = world.css_find(email_css)
     f._element.send_keys(name, EMAIL_EXTENSION)
 
-    confirm_css = '#add_user'
+    confirm_css = 'form.create-user button.action-primary'
     world.css_click(confirm_css)
 
 
 @step(u'I delete "([^"]*)" from the course team')
 def delete_other_user(_step, name):
-    to_delete_css = 'a.remove-user[data-id="{name}{extension}"]'.format(name=name, extension=EMAIL_EXTENSION)
+    to_delete_css = '.user-item .item-actions a.remove-user[data-id="{email}"]'.format(
+        email="{0}{1}".format(name, EMAIL_EXTENSION))
     world.css_click(to_delete_css)
 
 
+@step(u'I make "([^"]*)" a course team admin')
+def make_course_team_admin(_step, name):
+    admin_btn_css = '.user-item[data-email="{email}"] .user-actions .add-admin-role'.format(
+        email=name+EMAIL_EXTENSION)
+    world.css_click(admin_btn_css)
+
+
+@step(u'I remove admin rights from "([^"]*)"')
+def remove_course_team_admin(_step, name):
+    admin_btn_css = '.user-item[data-email="{email}"] .user-actions .remove-admin-role'.format(
+        email=name+EMAIL_EXTENSION)
+    world.css_click(admin_btn_css)
+
+
 @step(u'"([^"]*)" logs in$')
 def other_user_login(_step, name):
     log_into_studio(uname=name, password=PASSWORD, email=name + EMAIL_EXTENSION)
 
 
 @step(u's?he does( not)? see the course on (his|her) page')
-def see_course(_step, doesnt_see_course, gender):
+def see_course(_step, inverted, gender):
     class_css = 'span.class-name'
     all_courses = world.css_find(class_css, wait_time=1)
     all_names = [item.html for item in all_courses]
-    if doesnt_see_course:
+    if inverted:
         assert not world.scenario_dict['COURSE'].display_name in all_names
     else:
         assert world.scenario_dict['COURSE'].display_name in all_names
 
 
-@step(u's?he cannot delete users')
-def cannot_delete(_step):
+@step(u'"([^"]*)" should( not)? be marked as an admin')
+def marked_as_admin(_step, name, inverted):
+    flag_css = '.user-item[data-email="{email}"] .flag-role.flag-role-admin'.format(
+        email=name+EMAIL_EXTENSION)
+    if inverted:
+        assert world.is_css_not_present(flag_css)
+    else:
+        assert world.is_css_present(flag_css)
+
+
+@step(u's?he can(not)? delete users')
+def can_delete_users(_step, inverted):
     to_delete_css = 'a.remove-user'
-    assert world.is_css_not_present(to_delete_css)
+    if inverted:
+        assert world.is_css_not_present(to_delete_css)
+    else:
+        assert world.is_css_present(to_delete_css)
 
 
-@step(u's?he cannot add users')
-def cannot_add(_step):
-    add_css = 'a.new-user'
-    assert world.is_css_not_present(add_css)
+@step(u's?he can(not)? add users')
+def can_add_users(_step, inverted):
+    add_css = 'a.create-user-button'
+    if inverted:
+        assert world.is_css_not_present(add_css)
+    else:
+        assert world.is_css_present(add_css)

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

@@ -155,6 +155,10 @@ def cancel_does_not_save_changes(step):
 @step('I have created a LaTeX Problem')
 def create_latex_problem(step):
     world.click_new_component_button(step, '.large-problem-icon')
+
+    def animation_done(_driver):
+        return world.browser.evaluate_script("$('div.new-component').css('display')") == 'none'
+    world.wait_for(animation_done)
     # Go to advanced tab.
     world.css_click('#ui-id-2')
     world.click_component_from_menu("problem", "latex_problem.yaml", '.xmodule_CapaModule')

cms/djangoapps/contentstore/features/signup.feature

@@ -8,8 +8,7 @@ Feature: Sign in
     When I click the link with the text "Sign Up"
     And I fill in the registration form
     And I press the Create My Account button on the registration form
-    Then I should see be on the studio home page
-    And I should see the message "complete your sign up we need you to verify your email address"
+    Then I should see an email verification prompt
 
   Scenario: Login with a valid redirect
     Given I have opened a new course in Studio

cms/djangoapps/contentstore/features/signup.py

@@ -22,14 +22,10 @@ def i_press_the_button_on_the_registration_form(step):
     world.css_click(submit_css)
 
 
-@step('I should see be on the studio home page$')
-def i_should_see_be_on_the_studio_home_page(step):
-    step.given('I should see the message "My Courses"')
-
-
-@step(u'I should see the message "([^"]*)"$')
-def i_should_see_the_message(step, msg):
-    assert world.browser.is_text_present(msg, 5)
+@step('I should see an email verification prompt')
+def i_should_see_an_email_verification_prompt(step):
+    world.css_has_text('h1.page-header', u'My Courses')
+    world.css_has_text('div.msg h3.title', u'We need to verify your email address')
 
 
 @step(u'I fill in and submit the signin form$')

cms/djangoapps/contentstore/features/upload.py

@@ -58,7 +58,7 @@ def delete_file(_step, file_name):
     world.css_click(delete_css, index=index)
 
     prompt_confirm_css = 'li.nav-item > a.action-primary'
-    world.css_click(prompt_confirm_css)
+    world.css_click(prompt_confirm_css, success_condition=lambda: not world.css_visible(prompt_confirm_css))
 
 
 @step(u'I should see only one "([^"]*)"$')

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

@@ -19,5 +19,6 @@ def i_see_the_correct_settings_and_values(step):
 @step('I have set "show captions" to (.*)')
 def set_show_captions(step, setting):
     world.css_click('a.edit-button')
+    world.wait_for(lambda _driver: world.css_visible('a.save-button'))
     world.browser.select('Show Captions', setting)
     world.css_click('a.save-button')

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

@@ -1,6 +1,6 @@
-###
-### Script for cloning a course
-###
+"""
+Script for cloning a course
+"""
 from django.core.management.base import BaseCommand, CommandError
 from xmodule.modulestore.store_utilities import clone_course
 from xmodule.modulestore.django import modulestore
@@ -15,23 +15,25 @@ from auth.authz import _copy_course_group
 
 
 class Command(BaseCommand):
+    """Clone a MongoDB-backed course to another location"""
     help = 'Clone a MongoDB backed course to another location'
 
     def handle(self, *args, **options):
+        "Execute the command"
         if len(args) != 2:
             raise CommandError("clone requires two arguments: <source-location> <dest-location>")
 
         source_location_str = args[0]
         dest_location_str = args[1]
 
-        ms = modulestore('direct')
-        cs = contentstore()
+        mstore = modulestore('direct')
+        cstore = contentstore()
 
-        print "Cloning course {0} to {1}".format(source_location_str, dest_location_str)
+        print("Cloning course {0} to {1}".format(source_location_str, dest_location_str))
 
         source_location = CourseDescriptor.id_to_location(source_location_str)
         dest_location = CourseDescriptor.id_to_location(dest_location_str)
 
-        if clone_course(ms, cs, source_location, dest_location):
-            print "copying User permissions..."
+        if clone_course(mstore, cstore, source_location, dest_location):
+            print("copying User permissions...")
             _copy_course_group(source_location, dest_location)

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

@@ -1,3 +1,6 @@
+"""
+Script for dumping course dumping the course structure
+"""
 from django.core.management.base import BaseCommand, CommandError
 from xmodule.course_module import CourseDescriptor
 from xmodule.modulestore.django import modulestore
@@ -9,10 +12,14 @@ filter_list = ['xml_attributes', 'checklists']
 
 
 class Command(BaseCommand):
+    """
+    The Django command for dumping course structure
+    """
     help = '''Write out to stdout a structural and metadata information about a course in a flat dictionary serialized
               in a JSON format. This can be used for analytics.'''
 
     def handle(self, *args, **options):
+        "Execute the command"
         if len(args) < 2 or len(args) > 3:
             raise CommandError("dump_course_structure requires two or more arguments: <location> <outfile> |<db>|")
 
@@ -32,7 +39,7 @@ class Command(BaseCommand):
         try:
             course = store.get_item(loc, depth=4)
         except:
-            print 'Could not find course at {0}'.format(course_id)
+            print('Could not find course at {0}'.format(course_id))
             return
 
         info = {}

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

@@ -1,6 +1,6 @@
-###
-### Script for exporting courseware from Mongo to a tar.gz file
-###
+"""
+Script for exporting courseware from Mongo to a tar.gz file
+"""
 import os
 
 from django.core.management.base import BaseCommand, CommandError
@@ -10,20 +10,21 @@ from xmodule.contentstore.django import contentstore
 from xmodule.course_module import CourseDescriptor
 
 
-unnamed_modules = 0
-
-
 class Command(BaseCommand):
+    """
+    Export the specified data directory into the default ModuleStore
+    """
     help = 'Export the specified data directory into the default ModuleStore'
 
     def handle(self, *args, **options):
+        "Execute the command"
         if len(args) != 2:
             raise CommandError("export requires two arguments: <course location> <output path>")
 
         course_id = args[0]
         output_path = args[1]
 
-        print "Exporting course id = {0} to {1}".format(course_id, output_path)
+        print("Exporting course id = {0} to {1}".format(course_id, output_path))
 
         location = CourseDescriptor.id_to_location(course_id)
 

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

@@ -1,8 +1,6 @@
-###
-### Script for exporting all courseware from Mongo to a directory
-###
-import os
-
+"""
+Script for exporting all courseware from Mongo to a directory
+"""
 from django.core.management.base import BaseCommand, CommandError
 from xmodule.modulestore.xml_exporter import export_to_xml
 from xmodule.modulestore.django import modulestore
@@ -10,13 +8,12 @@ from xmodule.contentstore.django import contentstore
 from xmodule.course_module import CourseDescriptor
 
 
-unnamed_modules = 0
-
-
 class Command(BaseCommand):
+    """Export all courses from mongo to the specified data directory"""
     help = 'Export all courses from mongo to the specified data directory'
 
     def handle(self, *args, **options):
+        "Execute the command"
         if len(args) != 1:
             raise CommandError("export requires one argument: <output path>")
 
@@ -27,14 +24,14 @@ class Command(BaseCommand):
         root_dir = output_path
         courses = ms.get_courses()
 
-        print "%d courses to export:" % len(courses)
+        print("%d courses to export:" % len(courses))
         cids = [x.id for x in courses]
-        print cids
+        print(cids)
 
         for course_id in cids:
 
-            print "-"*77
-            print "Exporting course id = {0} to {1}".format(course_id, output_path)
+            print("-"*77)
+            print("Exporting course id = {0} to {1}".format(course_id, output_path))
 
             if 1:
                 try:
@@ -42,6 +39,6 @@ class Command(BaseCommand):
                     course_dir = course_id.replace('/', '...')
                     export_to_xml(ms, cs, location, root_dir, course_dir, modulestore())
                 except Exception as err:
-                    print "="*30 + "> Oops, failed to export %s" % course_id
-                    print "Error:"
-                    print err
+                    print("="*30 + "> Oops, failed to export %s" % course_id)
+                    print("Error:")
+                    print(err)

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

@@ -1,6 +1,6 @@
-###
-### Script for importing courseware from XML format
-###
+"""
+Script for importing courseware from XML format
+"""
 
 from django.core.management.base import BaseCommand, CommandError
 from xmodule.modulestore.xml_importer import import_from_xml
@@ -8,13 +8,14 @@ from xmodule.modulestore.django import modulestore
 from xmodule.contentstore.django import contentstore
 
 
-unnamed_modules = 0
-
-
 class Command(BaseCommand):
+    """
+    Import the specified data directory into the default ModuleStore
+    """
     help = 'Import the specified data directory into the default ModuleStore'
 
     def handle(self, *args, **options):
+        "Execute the command"
         if len(args) == 0:
             raise CommandError("import requires at least one argument: <data directory> [<course dir>...]")
 
@@ -23,8 +24,8 @@ class Command(BaseCommand):
             course_dirs = args[1:]
         else:
             course_dirs = None
-        print "Importing.  Data_dir={data}, course_dirs={courses}".format(
+        print("Importing.  Data_dir={data}, course_dirs={courses}".format(
             data=data_dir,
-            courses=course_dirs)
+            courses=course_dirs))
         import_from_xml(modulestore('direct'), data_dir, course_dirs, load_error_modules=False,
                         static_content_store=contentstore(), verbose=True)

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

@@ -1,18 +1,17 @@
+"""
+Verify the structure of courseware as to it's suitability for import
+To run test: rake cms:xlint DATA_DIR=../data [COURSE_DIR=content-edx-101 (optional parameter)]
+"""
 from django.core.management.base import BaseCommand, CommandError
 from xmodule.modulestore.xml_importer import perform_xlint
 
 
-unnamed_modules = 0
-
-
 class Command(BaseCommand):
-    help = \
-        '''
-        Verify the structure of courseware as to it's suitability for import
-        To run test: rake cms:xlint DATA_DIR=../data [COURSE_DIR=content-edx-101 (optional parameter)]
-        '''
+    """Verify the structure of courseware as to it's suitability for import"""
+    help = "Verify the structure of courseware as to it's suitability for import"
 
     def handle(self, *args, **options):
+        "Execute the command"
         if len(args) == 0:
             raise CommandError("import requires at least one argument: <data directory> [<course dir>...]")
 
@@ -21,7 +20,7 @@ class Command(BaseCommand):
             course_dirs = args[1:]
         else:
             course_dirs = None
-        print "Importing.  Data_dir={data}, course_dirs={courses}".format(
+        print("Importing.  Data_dir={data}, course_dirs={courses}".format(
             data=data_dir,
-            courses=course_dirs)
+            courses=course_dirs))
         perform_xlint(data_dir, course_dirs, load_error_modules=False)

cms/djangoapps/contentstore/tests/test_assets.py

@@ -50,9 +50,9 @@ class UploadTestCase(CourseTestCase):
 
     @skip("CorruptGridFile error on continuous integration server")
     def test_happy_path(self):
-        file = BytesIO("sample content")
-        file.name = "sample.txt"
-        resp = self.client.post(self.url, {"name": "my-name", "file": file})
+        f = BytesIO("sample content")
+        f.name = "sample.txt"
+        resp = self.client.post(self.url, {"name": "my-name", "file": f})
         self.assert2XX(resp.status_code)
 
     def test_no_file(self):

cms/djangoapps/contentstore/tests/test_checklists.py

@@ -1,5 +1,5 @@
 """ Unit tests for checklist methods in views.py. """
-from contentstore.utils import get_modulestore, get_url_reverse
+from contentstore.utils import get_modulestore
 from xmodule.modulestore.inheritance import own_metadata
 from xmodule.modulestore.tests.factories import CourseFactory
 from django.core.urlresolvers import reverse
@@ -27,6 +27,7 @@ class ChecklistTestCase(CourseTestCase):
         """
         self.assertEqual(persisted['short_description'], request['short_description'])
         compare_urls = (persisted.get('action_urls_expanded') == request.get('action_urls_expanded'))
+        pers, req = None, None
         for pers, req in zip(persisted['items'], request['items']):
             self.assertEqual(pers['short_description'], req['short_description'])
         self.assertEqual(pers['long_description'], req['long_description'])
@@ -38,7 +39,11 @@ class ChecklistTestCase(CourseTestCase):
 
     def test_get_checklists(self):
         """ Tests the get checklists method. """
-        checklists_url = get_url_reverse('Checklists', self.course)
+        checklists_url = reverse("checklists", kwargs={
+            'org': self.course.location.org,
+            'course': self.course.location.course,
+            'name': self.course.location.name,
+        })
         response = self.client.get(checklists_url)
         self.assertContains(response, "Getting Started With Studio")
         payload = response.content

cms/djangoapps/contentstore/tests/test_contentstore.py

@@ -95,8 +95,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         self.client.login(username=uname, password=password)
 
     def tearDown(self):
-        mongo = MongoClient()
-        mongo.drop_database(TEST_DATA_CONTENTSTORE['OPTIONS']['db'])
+        MongoClient().drop_database(TEST_DATA_CONTENTSTORE['OPTIONS']['db'])
         _CONTENTSTORE.clear()
 
     def check_components_on_page(self, component_types, expected_types):
@@ -604,6 +603,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
             'org': 'MITx',
             'number': '999',
             'display_name': 'Robot Super Course',
+            'run': '2013_Spring'
         }
 
         module_store = modulestore('direct')
@@ -612,12 +612,12 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
         resp = self.client.post(reverse('create_new_course'), course_data)
         self.assertEqual(resp.status_code, 200)
         data = parse_json(resp)
-        self.assertEqual(data['id'], 'i4x://MITx/999/course/Robot_Super_Course')
+        self.assertEqual(data['id'], 'i4x://MITx/999/course/2013_Spring')
 
         content_store = contentstore()
 
         source_location = CourseDescriptor.id_to_location('edX/toy/2012_Fall')
-        dest_location = CourseDescriptor.id_to_location('MITx/999/Robot_Super_Course')
+        dest_location = CourseDescriptor.id_to_location('MITx/999/2013_Spring')
 
         clone_course(module_store, content_store, source_location, dest_location)
 
@@ -855,6 +855,68 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase):
 
         shutil.rmtree(root_dir)
 
+    def test_export_course_with_metadata_only_word_cloud(self):
+        """
+        Similar to `test_export_course_with_metadata_only_video`.
+        """
+        module_store = modulestore('direct')
+        draft_store = modulestore('draft')
+        content_store = contentstore()
+
+        import_from_xml(module_store, 'common/test/data/', ['word_cloud'])
+        location = CourseDescriptor.id_to_location('HarvardX/ER22x/2013_Spring')
+
+        verticals = module_store.get_items(['i4x', 'HarvardX', 'ER22x', 'vertical', None, None])
+
+        self.assertGreater(len(verticals), 0)
+
+        parent = verticals[0]
+
+        ItemFactory.create(parent_location=parent.location, category="word_cloud", display_name="untitled")
+
+        root_dir = path(mkdtemp_clean())
+
+        print 'Exporting to tempdir = {0}'.format(root_dir)
+
+        # export out to a tempdir
+        export_to_xml(module_store, content_store, location, root_dir, 'test_export', draft_modulestore=draft_store)
+
+        shutil.rmtree(root_dir)
+
+    def test_empty_data_roundtrip(self):
+        """
+        Test that an empty `data` field is preserved through
+        export/import.
+        """
+        module_store = modulestore('direct')
+        draft_store = modulestore('draft')
+        content_store = contentstore()
+
+        import_from_xml(module_store, 'common/test/data/', ['toy'])
+        location = CourseDescriptor.id_to_location('edX/toy/2012_Fall')
+
+        verticals = module_store.get_items(['i4x', 'edX', 'toy', 'vertical', None, None])
+
+        self.assertGreater(len(verticals), 0)
+
+        parent = verticals[0]
+
+        # Create a module, and ensure that its `data` field is empty
+        word_cloud = ItemFactory.create(parent_location=parent.location, category="word_cloud", display_name="untitled")
+        del word_cloud.data
+        self.assertEquals(word_cloud.data, '')
+
+        # Export the course
+        root_dir = path(mkdtemp_clean())
+        export_to_xml(module_store, content_store, location, root_dir, 'test_roundtrip', draft_modulestore=draft_store)
+
+        # Reimport and get the video back
+        import_from_xml(module_store, root_dir)
+        imported_word_cloud = module_store.get_item(Location(['i4x', 'edX', 'toy', 'word_cloud', 'untitled', None]))
+
+        # It should now contain empty data
+        self.assertEquals(imported_word_cloud.data, '')
+
     def test_course_handouts_rewrites(self):
         module_store = modulestore('direct')
 
@@ -954,6 +1016,7 @@ class ContentStoreTest(ModuleStoreTestCase):
             'org': 'MITx',
             'number': '999',
             'display_name': 'Robot Super Course',
+            'run': '2013_Spring'
         }
 
     def tearDown(self):
@@ -965,24 +1028,30 @@ class ContentStoreTest(ModuleStoreTestCase):
         """Test new course creation - happy path"""
         self.assert_created_course()
 
-    def assert_created_course(self):
+    def assert_created_course(self, number_suffix=None):
         """
         Checks that the course was created properly.
         """
-        resp = self.client.post(reverse('create_new_course'), self.course_data)
+        test_course_data = {}
+        test_course_data.update(self.course_data)
+        if number_suffix:
+            test_course_data['number'] = '{0}_{1}'.format(test_course_data['number'], number_suffix)
+        resp = self.client.post(reverse('create_new_course'), test_course_data)
         self.assertEqual(resp.status_code, 200)
         data = parse_json(resp)
-        self.assertEqual(data['id'], 'i4x://MITx/999/course/Robot_Super_Course')
+        self.assertNotIn('ErrMsg', data)
+        self.assertEqual(data['id'], 'i4x://MITx/{0}/course/2013_Spring'.format(test_course_data['number']))
+        return test_course_data
 
     def test_create_course_check_forum_seeding(self):
         """Test new course creation and verify forum seeding """
-        self.assert_created_course()
-        self.assertTrue(are_permissions_roles_seeded('MITx/999/Robot_Super_Course'))
+        test_course_data = self.assert_created_course(number_suffix=uuid4().hex)
+        self.assertTrue(are_permissions_roles_seeded('MITx/{0}/2013_Spring'.format(test_course_data['number'])))
 
     def test_create_course_duplicate_course(self):
         """Test new course creation - error path"""
         self.client.post(reverse('create_new_course'), self.course_data)
-        self.assert_course_creation_failed('There is already a course defined with this name.')
+        self.assert_course_creation_failed('There is already a course defined with the same organization, course number, and course run. Please change either organization or course number to be unique.')
 
     def assert_course_creation_failed(self, error_message):
         """
@@ -997,8 +1066,9 @@ class ContentStoreTest(ModuleStoreTestCase):
         """Test new course creation - error path"""
         self.client.post(reverse('create_new_course'), self.course_data)
         self.course_data['display_name'] = 'Robot Super Course Two'
+        self.course_data['run'] = '2013_Summer'
 
-        self.assert_course_creation_failed('There is already a course defined with the same organization and course number.')
+        self.assert_course_creation_failed('There is already a course defined with the same organization and course number. Please change at least one field to be unique.')
 
     def test_create_course_with_bad_organization(self):
         """Test new course creation - error path for bad organization name"""
@@ -1167,7 +1237,9 @@ class ContentStoreTest(ModuleStoreTestCase):
 
         # manage users
         resp = self.client.get(reverse('manage_users',
-                                       kwargs={'location': loc.url()}))
+                                       kwargs={'org': loc.org,
+                                               'course': loc.course,
+                                               'name': loc.name}))
         self.assertEqual(200, resp.status_code)
 
         # course info

cms/djangoapps/contentstore/tests/test_course_settings.py

@@ -18,8 +18,6 @@ from xmodule.modulestore.tests.factories import CourseFactory
 
 
 from models.settings.course_metadata import CourseMetadata
-from xmodule.modulestore.xml_importer import import_from_xml
-from xmodule.modulestore.django import modulestore
 from xmodule.fields import Date
 
 from .utils import CourseTestCase
@@ -167,8 +165,8 @@ class CourseDetailsViewTest(CourseTestCase):
         self.compare_details_with_encoding(json.loads(resp.content), details.__dict__, field + str(val))
 
     @staticmethod
-    def convert_datetime_to_iso(dt):
-        return Date().to_json(dt)
+    def convert_datetime_to_iso(datetime_obj):
+        return Date().to_json(datetime_obj)
 
     def test_update_and_fetch(self):
         loc = self.course.location

cms/djangoapps/contentstore/tests/test_course_updates.py

@@ -2,6 +2,7 @@
 from contentstore.tests.test_course_settings import CourseTestCase
 from django.core.urlresolvers import reverse
 import json
+from xmodule.modulestore.django import modulestore
 
 
 class CourseUpdateTest(CourseTestCase):
@@ -145,3 +146,36 @@ class CourseUpdateTest(CourseTestCase):
         resp = self.client.delete(url)
         payload = json.loads(resp.content)
         self.assertTrue(len(payload) == before_delete - 1)
+
+    def test_no_ol_course_update(self):
+        '''Test trying to add to a saved course_update which is not an ol.'''
+        # get the updates and set to something wrong
+        location = self.course.location.replace(category='course_info', name='updates')
+        modulestore('direct').create_and_save_xmodule(location)
+        course_updates = modulestore('direct').get_item(location)
+        course_updates.data = 'bad news'
+        modulestore('direct').update_item(location, course_updates.data)
+
+        init_content = '<iframe width="560" height="315" src="http://www.youtube.com/embed/RocY-Jd93XU" frameborder="0">'
+        content = init_content + '</iframe>'
+        payload = {'content': content,
+                   'date': 'January 8, 2013'}
+        url = reverse('course_info_json',
+                      kwargs={'org': self.course.location.org,
+                              'course': self.course.location.course,
+                              'provided_id': ''})
+
+        resp = self.client.post(url, json.dumps(payload), "application/json")
+
+        payload = json.loads(resp.content)
+
+        self.assertHTMLEqual(payload['content'], content)
+
+        # now confirm that the bad news and the iframe make up 2 updates
+        url = reverse('course_info_json',
+                      kwargs={'org': self.course.location.org,
+                              'course': self.course.location.course,
+                              'provided_id': ''})
+        resp = self.client.get(url)
+        payload = json.loads(resp.content)
+        self.assertTrue(len(payload) == 2)

cms/djangoapps/contentstore/tests/test_crud.py

@@ -127,7 +127,7 @@ class TemplateTests(unittest.TestCase):
         persistent_factories.ItemFactory.create(display_name='chapter 1',
             parent_location=test_course.location)
 
-        id_locator = CourseLocator(course_id=test_course.location.course_id, revision='draft')
+        id_locator = CourseLocator(course_id=test_course.location.course_id, branch='draft')
         guid_locator = CourseLocator(version_guid=test_course.location.version_guid)
         # verify it can be retireved by id
         self.assertIsInstance(modulestore('split').get_course(id_locator), CourseDescriptor)

cms/djangoapps/contentstore/tests/test_i18n.py

@@ -85,9 +85,11 @@ class InternationalizationTest(ModuleStoreTestCase):
                                HTTP_ACCEPT_LANGUAGE='fr'
                                )
 
-        TEST_STRING = u'<h1 class="title-1">' \
-                      + u'My \xc7\xf6\xfcrs\xe9s L#' \
-                      + u'</h1>'
+        TEST_STRING = (
+            u'<h1 class="title-1">'
+            u'My \xc7\xf6\xfcrs\xe9s L#'
+            u'</h1>'
+        )
 
         self.assertContains(resp,
                             TEST_STRING,

cms/djangoapps/contentstore/tests/test_item.py

@@ -14,19 +14,26 @@ class DeleteItem(CourseTestCase):
         super(DeleteItem, self).setUp()
         self.course = CourseFactory.create(org='mitX', number='333', display_name='Dummy Course')
 
-    def testDeleteStaticPage(self):
+    def test_delete_static_page(self):
         # Add static tab
         data = json.dumps({
             'parent_location': 'i4x://mitX/333/course/Dummy_Course',
             'category': 'static_tab'
         })
 
-        resp = self.client.post(reverse('create_item'), data,
-            content_type="application/json")
+        resp = self.client.post(
+            reverse('create_item'),
+            data,
+            content_type="application/json"
+        )
         self.assertEqual(resp.status_code, 200)
 
         # Now delete it. There was a bug that the delete was failing (static tabs do not exist in draft modulestore).
-        resp = self.client.post(reverse('delete_item'), resp.content, "application/json")
+        resp = self.client.post(
+            reverse('delete_item'),
+            resp.content,
+            "application/json"
+        )
         self.assertEqual(resp.status_code, 200)
 
 
@@ -122,6 +129,7 @@ class TestCreateItem(CourseTestCase):
         )
         self.assertEqual(resp.status_code, 200)
 
+
 class TestEditItem(CourseTestCase):
     """
     Test contentstore.views.item.save_item
@@ -151,10 +159,10 @@ class TestEditItem(CourseTestCase):
         chap_location = self.response_id(resp)
         resp = self.client.post(
             reverse('create_item'),
-            json.dumps(
-                {'parent_location': chap_location,
-                 'category': 'sequential'
-                }),
+            json.dumps({
+                'parent_location': chap_location,
+                'category': 'sequential',
+            }),
             content_type="application/json"
         )
         self.seq_location = self.response_id(resp)
@@ -162,9 +170,10 @@ class TestEditItem(CourseTestCase):
         template_id = 'multiplechoice.yaml'
         resp = self.client.post(
             reverse('create_item'),
-            json.dumps({'parent_location': self.seq_location,
-             'category': 'problem',
-             'boilerplate': template_id
+            json.dumps({
+                'parent_location': self.seq_location,
+                'category': 'problem',
+                'boilerplate': template_id,
             }),
             content_type="application/json"
         )
@@ -195,7 +204,6 @@ class TestEditItem(CourseTestCase):
         problem = modulestore('draft').get_item(self.problems[0])
         self.assertEqual(problem.rerandomize, 'never')
 
-
     def test_null_field(self):
         """
         Sending null in for a field 'deletes' it
@@ -240,4 +248,3 @@ class TestEditItem(CourseTestCase):
         sequential = modulestore().get_item(self.seq_location)
         self.assertEqual(sequential.lms.due, datetime.datetime(2010, 11, 22, 4, 0, tzinfo=UTC))
         self.assertEqual(sequential.lms.start, datetime.datetime(2010, 9, 12, 14, 0, tzinfo=UTC))
-

cms/djangoapps/contentstore/tests/test_users.py

@@ -1,195 +1,319 @@
-"""
-Tests for user.py.
-"""
 import json
-import mock
 from .utils import CourseTestCase
+from django.contrib.auth.models import User, Group
 from django.core.urlresolvers import reverse
-from contentstore.views.user import _get_course_creator_status
-from course_creators.views import add_user_with_status_granted
-from course_creators.admin import CourseCreatorAdmin
-from course_creators.models import CourseCreator
-
-from django.http import HttpRequest
-from django.contrib.auth.models import User
-from django.contrib.admin.sites import AdminSite
+from auth.authz import get_course_groupname_for_role
 
 
 class UsersTestCase(CourseTestCase):
     def setUp(self):
         super(UsersTestCase, self).setUp()
-        self.url = reverse("add_user", kwargs={"location": ""})
+        self.ext_user = User.objects.create_user(
+            "joe", "joe@comedycentral.com", "haha")
+        self.ext_user.is_active = True
+        self.ext_user.is_staff = False
+        self.ext_user.save()
+        self.inactive_user = User.objects.create_user(
+            "carl", "carl@comedycentral.com", "haha")
+        self.inactive_user.is_active = False
+        self.inactive_user.is_staff = False
+        self.inactive_user.save()
 
-    def test_empty(self):
-        resp = self.client.post(self.url)
+        self.index_url = reverse("manage_users", kwargs={
+            "org": self.course.location.org,
+            "course": self.course.location.course,
+            "name": self.course.location.name,
+        })
+        self.detail_url = reverse("course_team_user", kwargs={
+            "org": self.course.location.org,
+            "course": self.course.location.course,
+            "name": self.course.location.name,
+            "email": self.ext_user.email,
+        })
+        self.inactive_detail_url = reverse("course_team_user", kwargs={
+            "org": self.course.location.org,
+            "course": self.course.location.course,
+            "name": self.course.location.name,
+            "email": self.inactive_user.email,
+        })
+        self.invalid_detail_url = reverse("course_team_user", kwargs={
+            "org": self.course.location.org,
+            "course": self.course.location.course,
+            "name": self.course.location.name,
+            "email": "nonexistent@user.com",
+        })
+        self.staff_groupname = get_course_groupname_for_role(self.course.location, "staff")
+        self.inst_groupname = get_course_groupname_for_role(self.course.location, "instructor")
+
+    def test_index(self):
+        resp = self.client.get(self.index_url)
+        # ext_user is not currently a member of the course team, and so should
+        # not show up on the page.
+        self.assertNotContains(resp, self.ext_user.email)
+
+    def test_index_member(self):
+        group, _ = Group.objects.get_or_create(name=self.staff_groupname)
+        self.ext_user.groups.add(group)
+        self.ext_user.save()
+
+        resp = self.client.get(self.index_url)
+        self.assertContains(resp, self.ext_user.email)
+
+    def test_detail(self):
+        resp = self.client.get(self.detail_url)
+        self.assertEqual(resp.status_code, 200)
+        result = json.loads(resp.content)
+        self.assertEqual(result["role"], None)
+        self.assertTrue(result["active"])
+
+    def test_detail_inactive(self):
+        resp = self.client.get(self.inactive_detail_url)
+        self.assert2XX(resp.status_code)
+        result = json.loads(resp.content)
+        self.assertFalse(result["active"])
+
+    def test_detail_invalid(self):
+        resp = self.client.get(self.invalid_detail_url)
+        self.assert4XX(resp.status_code)
+        result = json.loads(resp.content)
+        self.assertIn("error", result)
+
+    def test_detail_post(self):
+        resp = self.client.post(
+            self.detail_url,
+            data={"role": None},
+        )
+        self.assert2XX(resp.status_code)
+        # reload user from DB
+        ext_user = User.objects.get(email=self.ext_user.email)
+        groups = [g.name for g in ext_user.groups.all()]
+        # no content: should not be in any roles
+        self.assertNotIn(self.staff_groupname, groups)
+        self.assertNotIn(self.inst_groupname, groups)
+
+    def test_detail_post_staff(self):
+        resp = self.client.post(
+            self.detail_url,
+            data=json.dumps({"role": "staff"}),
+            content_type="application/json",
+            HTTP_ACCEPT="application/json",
+        )
+        self.assert2XX(resp.status_code)
+        # reload user from DB
+        ext_user = User.objects.get(email=self.ext_user.email)
+        groups = [g.name for g in ext_user.groups.all()]
+        self.assertIn(self.staff_groupname, groups)
+        self.assertNotIn(self.inst_groupname, groups)
+
+    def test_detail_post_staff_other_inst(self):
+        inst_group, _ = Group.objects.get_or_create(name=self.inst_groupname)
+        self.user.groups.add(inst_group)
+        self.user.save()
+
+        resp = self.client.post(
+            self.detail_url,
+            data=json.dumps({"role": "staff"}),
+            content_type="application/json",
+            HTTP_ACCEPT="application/json",
+        )
+        self.assert2XX(resp.status_code)
+        # reload user from DB
+        ext_user = User.objects.get(email=self.ext_user.email)
+        groups = [g.name for g in ext_user.groups.all()]
+        self.assertIn(self.staff_groupname, groups)
+        self.assertNotIn(self.inst_groupname, groups)
+        # check that other user is unchanged
+        user = User.objects.get(email=self.user.email)
+        groups = [g.name for g in user.groups.all()]
+        self.assertNotIn(self.staff_groupname, groups)
+        self.assertIn(self.inst_groupname, groups)
+
+    def test_detail_post_instructor(self):
+        resp = self.client.post(
+            self.detail_url,
+            data=json.dumps({"role": "instructor"}),
+            content_type="application/json",
+            HTTP_ACCEPT="application/json",
+        )
+        self.assert2XX(resp.status_code)
+        # reload user from DB
+        ext_user = User.objects.get(email=self.ext_user.email)
+        groups = [g.name for g in ext_user.groups.all()]
+        self.assertNotIn(self.staff_groupname, groups)
+        self.assertIn(self.inst_groupname, groups)
+
+    def test_detail_post_missing_role(self):
+        resp = self.client.post(
+            self.detail_url,
+            data=json.dumps({"toys": "fun"}),
+            content_type="application/json",
+            HTTP_ACCEPT="application/json",
+        )
+        self.assert4XX(resp.status_code)
+        result = json.loads(resp.content)
+        self.assertIn("error", result)
+
+    def test_detail_post_bad_json(self):
+        resp = self.client.post(
+            self.detail_url,
+            data="{foo}",
+            content_type="application/json",
+            HTTP_ACCEPT="application/json",
+        )
+        self.assert4XX(resp.status_code)
+        result = json.loads(resp.content)
+        self.assertIn("error", result)
+
+    def test_detail_post_no_json(self):
+        resp = self.client.post(
+            self.detail_url,
+            data={"role": "staff"},
+            HTTP_ACCEPT="application/json",
+        )
+        self.assert2XX(resp.status_code)
+        # reload user from DB
+        ext_user = User.objects.get(email=self.ext_user.email)
+        groups = [g.name for g in ext_user.groups.all()]
+        self.assertIn(self.staff_groupname, groups)
+        self.assertNotIn(self.inst_groupname, groups)
+
+    def test_detail_delete_staff(self):
+        group, _ = Group.objects.get_or_create(name=self.staff_groupname)
+        self.ext_user.groups.add(group)
+        self.ext_user.save()
+
+        resp = self.client.delete(
+            self.detail_url,
+            HTTP_ACCEPT="application/json",
+        )
+        self.assert2XX(resp.status_code)
+        # reload user from DB
+        ext_user = User.objects.get(email=self.ext_user.email)
+        groups = [g.name for g in ext_user.groups.all()]
+        self.assertNotIn(self.staff_groupname, groups)
+
+    def test_detail_delete_instructor(self):
+        group, _ = Group.objects.get_or_create(name=self.inst_groupname)
+        self.user.groups.add(group)
+        self.ext_user.groups.add(group)
+        self.user.save()
+        self.ext_user.save()
+
+        resp = self.client.delete(
+            self.detail_url,
+            HTTP_ACCEPT="application/json",
+        )
+        self.assert2XX(resp.status_code)
+        # reload user from DB
+        ext_user = User.objects.get(email=self.ext_user.email)
+        groups = [g.name for g in ext_user.groups.all()]
+        self.assertNotIn(self.inst_groupname, groups)
+
+    def test_delete_last_instructor(self):
+        group, _ = Group.objects.get_or_create(name=self.inst_groupname)
+        self.ext_user.groups.add(group)
+        self.ext_user.save()
+
+        resp = self.client.delete(
+            self.detail_url,
+            HTTP_ACCEPT="application/json",
+        )
         self.assertEqual(resp.status_code, 400)
-        content = json.loads(resp.content)
-        self.assertEqual(content["Status"], "Failed")
+        result = json.loads(resp.content)
+        self.assertIn("error", result)
+        # reload user from DB
+        ext_user = User.objects.get(email=self.ext_user.email)
+        groups = [g.name for g in ext_user.groups.all()]
+        self.assertIn(self.inst_groupname, groups)
 
+    def test_post_last_instructor(self):
+        group, _ = Group.objects.get_or_create(name=self.inst_groupname)
+        self.ext_user.groups.add(group)
+        self.ext_user.save()
 
-class IndexCourseCreatorTests(CourseTestCase):
-    """
-    Tests the various permutations of course creator status.
-    """
-    def setUp(self):
-        super(IndexCourseCreatorTests, self).setUp()
+        resp = self.client.post(
+            self.detail_url,
+            data={"role": "staff"},
+            HTTP_ACCEPT="application/json",
+        )
+        self.assertEqual(resp.status_code, 400)
+        result = json.loads(resp.content)
+        self.assertIn("error", result)
+        # reload user from DB
+        ext_user = User.objects.get(email=self.ext_user.email)
+        groups = [g.name for g in ext_user.groups.all()]
+        self.assertIn(self.inst_groupname, groups)
 
-        self.index_url = reverse("index")
-        self.request_access_url = reverse("request_course_creator")
-
-        # Disable course creation takes precedence over enable creator group. I have enabled the
-        # latter to make this clear.
-        self.disable_course_creation = {
-            "DISABLE_COURSE_CREATION": True,
-            "ENABLE_CREATOR_GROUP": True,
-            'STUDIO_REQUEST_EMAIL': 'mark@marky.mark',
-        }
-
-        self.enable_creator_group = {"ENABLE_CREATOR_GROUP": True}
-
-        self.admin = User.objects.create_user('Mark', 'mark+courses@edx.org', 'foo')
-        self.admin.is_staff = True
-
-    def test_get_course_creator_status_disable_creation(self):
-        # DISABLE_COURSE_CREATION is True (this is the case on edx, where we have a marketing site).
-        # Only edx staff can create courses.
-        with mock.patch.dict('django.conf.settings.MITX_FEATURES', self.disable_course_creation):
-            self.assertTrue(self.user.is_staff)
-            self.assertEquals('granted', _get_course_creator_status(self.user))
-            self._set_user_non_staff()
-            self.assertFalse(self.user.is_staff)
-            self.assertEquals('disallowed_for_this_site', _get_course_creator_status(self.user))
-
-    def test_get_course_creator_status_default_cause(self):
-        # Neither ENABLE_CREATOR_GROUP nor DISABLE_COURSE_CREATION are enabled. Anyone can create a course.
-        self.assertEquals('granted', _get_course_creator_status(self.user))
-        self._set_user_non_staff()
-        self.assertEquals('granted', _get_course_creator_status(self.user))
-
-    def test_get_course_creator_status_creator_group(self):
-        # ENABLE_CREATOR_GROUP is True. This is the case on edge.
-        # Only staff members and users who have been granted access can create courses.
-        with mock.patch.dict('django.conf.settings.MITX_FEATURES', self.enable_creator_group):
-            # Staff members can always create courses.
-            self.assertEquals('granted', _get_course_creator_status(self.user))
-            # Non-staff must request access.
-            self._set_user_non_staff()
-            self.assertEquals('unrequested', _get_course_creator_status(self.user))
-            # Staff user requests access.
-            self.client.post(self.request_access_url)
-            self.assertEquals('pending', _get_course_creator_status(self.user))
-
-    def test_get_course_creator_status_creator_group_granted(self):
-        # ENABLE_CREATOR_GROUP is True. This is the case on edge.
-        # Check return value for a non-staff user who has been granted access.
-        with mock.patch.dict('django.conf.settings.MITX_FEATURES', self.enable_creator_group):
-            self._set_user_non_staff()
-            add_user_with_status_granted(self.admin, self.user)
-            self.assertEquals('granted', _get_course_creator_status(self.user))
-
-    def test_get_course_creator_status_creator_group_denied(self):
-        # ENABLE_CREATOR_GROUP is True. This is the case on edge.
-        # Check return value for a non-staff user who has been denied access.
-        with mock.patch.dict('django.conf.settings.MITX_FEATURES', self.enable_creator_group):
-            self._set_user_non_staff()
-            self._set_user_denied()
-            self.assertEquals('denied', _get_course_creator_status(self.user))
-
-    def test_disable_course_creation_enabled_non_staff(self):
-        # Test index page content when DISABLE_COURSE_CREATION is True, non-staff member.
-        with mock.patch.dict('django.conf.settings.MITX_FEATURES', self.disable_course_creation):
-            self._set_user_non_staff()
-            self._assert_cannot_create()
-
-    def test_disable_course_creation_enabled_staff(self):
-        # Test index page content when DISABLE_COURSE_CREATION is True, staff member.
-        with mock.patch.dict('django.conf.settings.MITX_FEATURES', self.disable_course_creation):
-            resp = self._assert_can_create()
-            self.assertFalse('Email staff to create course' in resp.content)
-
-    def test_can_create_by_default(self):
-        # Test index page content with neither ENABLE_CREATOR_GROUP nor DISABLE_COURSE_CREATION enabled.
-        # Anyone can create a course.
-        self._assert_can_create()
-        self._set_user_non_staff()
-        self._assert_can_create()
-
-    def test_course_creator_group_enabled(self):
-        # Test index page content with ENABLE_CREATOR_GROUP True.
-        # Staff can always create a course, others must request access.
-        with mock.patch.dict('django.conf.settings.MITX_FEATURES', self.enable_creator_group):
-            # Staff members can always create courses.
-            self._assert_can_create()
-
-            # Non-staff case.
-            self._set_user_non_staff()
-            resp = self._assert_cannot_create()
-            self.assertTrue(self.request_access_url in resp.content)
-
-            # Now request access.
-            self.client.post(self.request_access_url)
-
-            # Still cannot create a course, but the "request access button" is no longer there.
-            resp = self._assert_cannot_create()
-            self.assertFalse(self.request_access_url in resp.content)
-            self.assertTrue('has-status is-pending' in resp.content)
-
-    def test_course_creator_group_granted(self):
-        # Test index page content with ENABLE_CREATOR_GROUP True, non-staff member with access granted.
-        with mock.patch.dict('django.conf.settings.MITX_FEATURES', self.enable_creator_group):
-            self._set_user_non_staff()
-            add_user_with_status_granted(self.admin, self.user)
-            self._assert_can_create()
-
-    def test_course_creator_group_denied(self):
-        # Test index page content with ENABLE_CREATOR_GROUP True, non-staff member with access denied.
-        with mock.patch.dict('django.conf.settings.MITX_FEATURES', self.enable_creator_group):
-            self._set_user_non_staff()
-            self._set_user_denied()
-            resp = self._assert_cannot_create()
-            self.assertFalse(self.request_access_url in resp.content)
-            self.assertTrue('has-status is-denied' in resp.content)
-
-    def _assert_can_create(self):
-        """
-        Helper method that posts to the index page and checks that the user can create a course.
-
-        Returns the response from the post.
-        """
-        resp = self.client.post(self.index_url)
-        self.assertTrue('new-course-button' in resp.content)
-        self.assertFalse(self.request_access_url in resp.content)
-        self.assertFalse('Email staff to create course' in resp.content)
-        return resp
-
-    def _assert_cannot_create(self):
-        """
-        Helper method that posts to the index page and checks that the user cannot create a course.
-
-        Returns the response from the post.
-        """
-        resp = self.client.post(self.index_url)
-        self.assertFalse('new-course-button' in resp.content)
-        return resp
-
-    def _set_user_non_staff(self):
-        """
-        Sets user as non-staff.
-        """
+    def test_permission_denied_self(self):
+        group, _ = Group.objects.get_or_create(name=self.staff_groupname)
+        self.user.groups.add(group)
         self.user.is_staff = False
         self.user.save()
 
-    def _set_user_denied(self):
-        """
-        Sets course creator status to denied in admin table.
-        """
-        self.table_entry = CourseCreator(user=self.user)
-        self.table_entry.save()
+        self_url = reverse("course_team_user", kwargs={
+            "org": self.course.location.org,
+            "course": self.course.location.course,
+            "name": self.course.location.name,
+            "email": self.user.email,
+        })
 
-        self.deny_request = HttpRequest()
-        self.deny_request.user = self.admin
+        resp = self.client.post(
+            self_url,
+            data={"role": "instructor"},
+            HTTP_ACCEPT="application/json",
+        )
+        self.assert4XX(resp.status_code)
+        result = json.loads(resp.content)
+        self.assertIn("error", result)
 
-        self.creator_admin = CourseCreatorAdmin(self.table_entry, AdminSite())
+    def test_permission_denied_other(self):
+        group, _ = Group.objects.get_or_create(name=self.staff_groupname)
+        self.user.groups.add(group)
+        self.user.is_staff = False
+        self.user.save()
 
-        self.table_entry.state = CourseCreator.DENIED
-        self.creator_admin.save_model(self.deny_request, self.table_entry, None, True)
+        resp = self.client.post(
+            self.detail_url,
+            data={"role": "instructor"},
+            HTTP_ACCEPT="application/json",
+        )
+        self.assert4XX(resp.status_code)
+        result = json.loads(resp.content)
+        self.assertIn("error", result)
+
+    def test_staff_can_delete_self(self):
+        group, _ = Group.objects.get_or_create(name=self.staff_groupname)
+        self.user.groups.add(group)
+        self.user.is_staff = False
+        self.user.save()
+
+        self_url = reverse("course_team_user", kwargs={
+            "org": self.course.location.org,
+            "course": self.course.location.course,
+            "name": self.course.location.name,
+            "email": self.user.email,
+        })
+
+        resp = self.client.delete(self_url)
+        self.assert2XX(resp.status_code)
+        # reload user from DB
+        user = User.objects.get(email=self.user.email)
+        groups = [g.name for g in user.groups.all()]
+        self.assertNotIn(self.staff_groupname, groups)
+
+    def test_staff_cannot_delete_other(self):
+        group, _ = Group.objects.get_or_create(name=self.staff_groupname)
+        self.user.groups.add(group)
+        self.user.is_staff = False
+        self.user.save()
+        self.ext_user.groups.add(group)
+        self.ext_user.save()
+
+        resp = self.client.delete(self.detail_url)
+        self.assert4XX(resp.status_code)
+        result = json.loads(resp.content)
+        self.assertIn("error", result)
+        # reload user from DB
+        ext_user = User.objects.get(email=self.ext_user.email)
+        groups = [g.name for g in ext_user.groups.all()]
+        self.assertIn(self.staff_groupname, groups)

cms/djangoapps/contentstore/tests/test_utils.py

@@ -72,50 +72,6 @@ class LMSLinksTestCase(TestCase):
         )
 
 
-class UrlReverseTestCase(ModuleStoreTestCase):
-    """ Tests for get_url_reverse """
-    def test_course_page_names(self):
-        """ Test the defined course pages. """
-        course = CourseFactory.create(org='mitX', number='666', display_name='URL Reverse Course')
-
-        self.assertEquals(
-            '/manage_users/i4x://mitX/666/course/URL_Reverse_Course',
-            utils.get_url_reverse('ManageUsers', course)
-        )
-
-        self.assertEquals(
-            '/mitX/666/settings-details/URL_Reverse_Course',
-            utils.get_url_reverse('SettingsDetails', course)
-        )
-
-        self.assertEquals(
-            '/mitX/666/settings-grading/URL_Reverse_Course',
-            utils.get_url_reverse('SettingsGrading', course)
-        )
-
-        self.assertEquals(
-            '/mitX/666/course/URL_Reverse_Course',
-            utils.get_url_reverse('CourseOutline', course)
-        )
-
-        self.assertEquals(
-            '/mitX/666/checklists/URL_Reverse_Course',
-            utils.get_url_reverse('Checklists', course)
-        )
-
-    def test_unknown_passes_through(self):
-        """ Test that unknown values pass through. """
-        course = CourseFactory.create(org='mitX', number='666', display_name='URL Reverse Course')
-        self.assertEquals(
-            'foobar',
-            utils.get_url_reverse('foobar', course)
-        )
-        self.assertEquals(
-            'https://edge.edx.org/courses/edX/edX101/How_to_Create_an_edX_Course/about',
-            utils.get_url_reverse('https://edge.edx.org/courses/edX/edX101/How_to_Create_an_edX_Course/about', course)
-        )
-
-
 class ExtraPanelTabTestCase(TestCase):
     """ Tests adding and removing extra course tabs. """
 

cms/djangoapps/contentstore/tests/tests.py

@@ -15,14 +15,16 @@ class ContentStoreTestCase(ModuleStoreTestCase):
         Login.  View should always return 200.  The success/fail is in the
         returned json
         """
-        resp = self.client.post(reverse('login_post'),
-                                {'email': email, 'password': password})
+        resp = self.client.post(
+            reverse('login_post'),
+            {'email': email, 'password': password}
+        )
         self.assertEqual(resp.status_code, 200)
         return resp
 
-    def login(self, email, pw):
+    def login(self, email, password):
         """Login, check that it worked."""
-        resp = self._login(email, pw)
+        resp = self._login(email, password)
         data = parse_json(resp)
         self.assertTrue(data['success'])
         return resp
@@ -178,11 +180,15 @@ class ForumTestCase(CourseTestCase):
 
     def test_blackouts(self):
         now = datetime.datetime.now(UTC)
-        self.course.discussion_blackouts = [(t.isoformat(), t2.isoformat()) for t, t2 in
-            [(now - datetime.timedelta(days=14), now - datetime.timedelta(days=11)),
-                (now + datetime.timedelta(days=24), now + datetime.timedelta(days=30))]]
+        times1 = [
+            (now - datetime.timedelta(days=14), now - datetime.timedelta(days=11)),
+            (now + datetime.timedelta(days=24), now + datetime.timedelta(days=30))
+        ]
+        self.course.discussion_blackouts = [(t.isoformat(), t2.isoformat()) for t, t2 in times1]
         self.assertTrue(self.course.forum_posts_allowed)
-        self.course.discussion_blackouts = [(t.isoformat(), t2.isoformat()) for t, t2 in
-            [(now - datetime.timedelta(days=14), now + datetime.timedelta(days=2)),
-                (now + datetime.timedelta(days=24), now + datetime.timedelta(days=30))]]
+        times2 = [
+            (now - datetime.timedelta(days=14), now + datetime.timedelta(days=2)),
+            (now + datetime.timedelta(days=24), now + datetime.timedelta(days=30))
+        ]
+        self.course.discussion_blackouts = [(t.isoformat(), t2.isoformat()) for t, t2 in times2]
         self.assertFalse(self.course.forum_posts_allowed)

cms/djangoapps/contentstore/utils.py

@@ -188,38 +188,6 @@ def update_item(location, value):
         get_modulestore(location).update_item(location, value)
 
 
-def get_url_reverse(course_page_name, course_module):
-    """
-    Returns the course URL link to the specified location. This value is suitable to use as an href link.
-
-    course_page_name should correspond to an attribute in CoursePageNames (for example, 'ManageUsers'
-    or 'SettingsDetails'), or else it will simply be returned. This method passes back unknown values of
-    course_page_names so that it can also be used for absolute (known) URLs.
-
-    course_module is used to obtain the location, org, course, and name properties for a course, if
-    course_page_name corresponds to an attribute in CoursePageNames.
-    """
-    url_name = getattr(CoursePageNames, course_page_name, None)
-    ctx_loc = course_module.location
-
-    if CoursePageNames.ManageUsers == url_name:
-        return reverse(url_name, kwargs={"location": ctx_loc})
-    elif url_name in [CoursePageNames.SettingsDetails, CoursePageNames.SettingsGrading,
-                      CoursePageNames.CourseOutline, CoursePageNames.Checklists]:
-        return reverse(url_name, kwargs={'org': ctx_loc.org, 'course': ctx_loc.course, 'name': ctx_loc.name})
-    else:
-        return course_page_name
-
-
-class CoursePageNames:
-    """ Constants for pages that are recognized by get_url_reverse method. """
-    ManageUsers = "manage_users"
-    SettingsDetails = "settings_details"
-    SettingsGrading = "settings_grading"
-    CourseOutline = "course_index"
-    Checklists = "checklists"
-
-
 def add_extra_panel_tab(tab_type, course):
     """
     Used to add the panel tab to a course if it does not exist.

cms/djangoapps/contentstore/views/assets.py

@@ -29,7 +29,6 @@ from xmodule.util.date_utils import get_default_time_display
 from xmodule.modulestore import InvalidLocationError
 from xmodule.exceptions import NotFoundError
 
-from ..utils import get_url_reverse
 from .access import get_location_and_verify_access
 from util.json_request import JsonResponse
 
@@ -284,7 +283,7 @@ def import_course(request, org, course, name):
         tar_file.extractall(course_dir + '/')
 
         # find the 'course.xml' file
-
+        dirpath = None
         for dirpath, _dirnames, filenames in os.walk(course_dir):
             for filename in filenames:
                 if filename == 'course.xml':
@@ -320,7 +319,11 @@ def import_course(request, org, course, name):
 
         return render_to_response('import.html', {
             'context_course': course_module,
-            'successful_import_redirect_url': get_url_reverse('CourseOutline', course_module)
+            'successful_import_redirect_url': reverse('course_index', kwargs={
+                'org': location.org,
+                'course': location.course,
+                'name': location.name,
+            })
         })
 
 

cms/djangoapps/contentstore/views/checklist.py

@@ -4,12 +4,13 @@ from util.json_request import JsonResponse
 from django.http import HttpResponseBadRequest
 from django.contrib.auth.decorators import login_required
 from django.views.decorators.http import require_http_methods
+from django.core.urlresolvers import reverse
 from django_future.csrf import ensure_csrf_cookie
 from mitxmako.shortcuts import render_to_response
 
 from xmodule.modulestore.inheritance import own_metadata
 
-from ..utils import get_modulestore, get_url_reverse
+from ..utils import get_modulestore
 from .access import get_location_and_verify_access
 from xmodule.course_module import CourseDescriptor
 
@@ -96,10 +97,25 @@ def expand_checklist_action_urls(course_module):
     """
     checklists = course_module.checklists
     modified = False
+    urlconf_map = {
+        "ManageUsers": "manage_users",
+        "SettingsDetails": "settings_details",
+        "SettingsGrading": "settings_grading",
+        "CourseOutline": "course_index",
+        "Checklists": "checklists",
+    }
     for checklist in checklists:
         if not checklist.get('action_urls_expanded', False):
             for item in checklist.get('items'):
-                item['action_url'] = get_url_reverse(item.get('action_url'), course_module)
+                action_url = item.get('action_url')
+                if action_url not in urlconf_map:
+                    continue
+                urlconf_name = urlconf_map[action_url]
+                item['action_url'] = reverse(urlconf_name, kwargs={
+                    'org': course_module.location.org,
+                    'course': course_module.location.course,
+                    'name': course_module.location.name,
+                })
             checklist['action_urls_expanded'] = True
             modified = True
 

cms/djangoapps/contentstore/views/component.py

@@ -46,13 +46,19 @@ COMPONENT_TYPES = ['discussion', 'html', 'problem', 'video']
 
 OPEN_ENDED_COMPONENT_TYPES = ["combinedopenended", "peergrading"]
 NOTE_COMPONENT_TYPES = ['notes']
-ADVANCED_COMPONENT_TYPES = ['annotatable', 'word_cloud', 'videoalpha'] + OPEN_ENDED_COMPONENT_TYPES + NOTE_COMPONENT_TYPES
+ADVANCED_COMPONENT_TYPES = [
+    'annotatable',
+    'word_cloud',
+    'videoalpha',
+    'graphical_slider_tool'
+] + OPEN_ENDED_COMPONENT_TYPES + NOTE_COMPONENT_TYPES
 ADVANCED_COMPONENT_CATEGORY = 'advanced'
 ADVANCED_COMPONENT_POLICY_KEY = 'advanced_modules'
 
 
 @login_required
 def edit_subsection(request, location):
+    "Edit the subsection of a course"
     # check that we have permissions to edit this item
     try:
         course = get_course_for_item(location)
@@ -264,6 +270,7 @@ def assignment_type_update(request, org, course, category, name):
 @login_required
 @expect_json
 def create_draft(request):
+    "Create a draft"
     location = request.POST['id']
 
     # check permissions for this user within this course
@@ -280,6 +287,7 @@ def create_draft(request):
 @login_required
 @expect_json
 def publish_draft(request):
+    "Publish a draft"
     location = request.POST['id']
 
     # check permissions for this user within this course
@@ -295,6 +303,7 @@ def publish_draft(request):
 @login_required
 @expect_json
 def unpublish_unit(request):
+    "Unpublish a unit"
     location = request.POST['id']
 
     # check permissions for this user within this course
@@ -312,6 +321,7 @@ def unpublish_unit(request):
 @login_required
 @ensure_csrf_cookie
 def module_info(request, module_location):
+    "Get or set information for a module in the modulestore"
     location = Location(module_location)
 
     # check that logged in user has permissions to this item

cms/djangoapps/contentstore/views/course.py

@@ -3,6 +3,7 @@ Views related to operations on course objects
 """
 import json
 import random
+from django.utils.translation import ugettext as _
 import string  # pylint: disable=W0402
 
 from django.contrib.auth.decorators import login_required
@@ -101,12 +102,13 @@ def create_new_course(request):
     org = request.POST.get('org')
     number = request.POST.get('number')
     display_name = request.POST.get('display_name')
+    run = request.POST.get('run')
 
     try:
-        dest_location = Location('i4x', org, number, 'course', Location.clean(display_name))
+        dest_location = Location('i4x', org, number, 'course', run)
     except InvalidLocationError as error:
         return JsonResponse({
-            "ErrMsg": "Unable to create course '{name}'.\n\n{err}".format(
+            "ErrMsg": _("Unable to create course '{name}'.\n\n{err}").format(
                 name=display_name, err=error.message)})
 
     # see if the course already exists
@@ -116,12 +118,24 @@ def create_new_course(request):
     except ItemNotFoundError:
         pass
     if existing_course is not None:
-        return JsonResponse({'ErrMsg': 'There is already a course defined with this name.'})
+        return JsonResponse(
+            {
+                'ErrMsg': _('There is already a course defined with the same organization, course number, and course run. Please change either organization or course number to be unique.'),
+                'OrgErrMsg': _('Please change either the organization or course number so that it is unique.'),
+                'CourseErrMsg': _('Please change either the organization or course number so that it is unique.'),
+            }
+        )
 
     course_search_location = ['i4x', dest_location.org, dest_location.course, 'course', None]
     courses = modulestore().get_items(course_search_location)
     if len(courses) > 0:
-        return JsonResponse({'ErrMsg': 'There is already a course defined with the same organization and course number.'})
+        return JsonResponse(
+            {
+                'ErrMsg': _('There is already a course defined with the same organization and course number. Please change at least one field to be unique.'),
+                'OrgErrMsg': _('Please change either the organization or course number so that it is unique.'),
+                'CourseErrMsg': _('Please change either the organization or course number so that it is unique.'),
+            }
+        )
 
     # instantiate the CourseDescriptor and then persist it
     # note: no system to pass

cms/djangoapps/contentstore/views/preview.py

@@ -68,6 +68,7 @@ def preview_dispatch(request, preview_id, location, dispatch=None):
 
 @login_required
 def preview_component(request, location):
+    "Return the HTML preview of a component"
     # TODO (vshnayder): change name from id to location in coffee+html as well.
     if not has_access(request.user, location):
         return HttpResponseForbidden()
@@ -91,6 +92,7 @@ def preview_module_system(request, preview_id, descriptor):
     """
 
     def preview_model_data(descriptor):
+        "Helper method to create a DbModel from a descriptor"
         return DbModel(
             SessionKeyValueStore(request, descriptor._model_data),
             descriptor.module_class,
@@ -105,7 +107,7 @@ def preview_module_system(request, preview_id, descriptor):
         # TODO (cpennington): Do we want to track how instructors are using the preview problems?
         track_function=lambda event_type, event: None,
         filestore=descriptor.system.resources_fs,
-        get_module=partial(get_preview_module, request, preview_id),
+        get_module=partial(load_preview_module, request, preview_id),
         render_template=render_from_lms,
         debug=True,
         replace_urls=partial(static_replace.replace_static_urls, data_directory=None, course_namespace=descriptor.location),
@@ -115,28 +117,13 @@ def preview_module_system(request, preview_id, descriptor):
     )
 
 
-def get_preview_module(request, preview_id, descriptor):
-    """
-    Returns a preview XModule at the specified location. The preview_data is chosen arbitrarily
-    from the set of preview data for the descriptor specified by Location
-
-    request: The active django request
-    preview_id (str): An identifier specifying which preview this module is used for
-    location: A Location
-    """
-
-    return load_preview_module(request, preview_id, descriptor)
-
-
 def load_preview_module(request, preview_id, descriptor):
     """
-    Return a preview XModule instantiated from the supplied descriptor, instance_state, and shared_state
+    Return a preview XModule instantiated from the supplied descriptor.
 
     request: The active django request
     preview_id (str): An identifier specifying which preview this module is used for
     descriptor: An XModuleDescriptor
-    instance_state: An instance state string
-    shared_state: A shared state string
     """
     system = preview_module_system(request, preview_id, descriptor)
     try:

cms/djangoapps/contentstore/views/public.py

@@ -1,3 +1,6 @@
+"""
+Public views
+"""
 from django_future.csrf import ensure_csrf_cookie
 from django.core.context_processors import csrf
 from django.shortcuts import redirect
@@ -10,10 +13,6 @@ from .user import index
 
 __all__ = ['signup', 'old_login_redirect', 'login_page', 'howitworks']
 
-"""
-Public views
-"""
-
 
 @ensure_csrf_cookie
 def signup(request):
@@ -45,6 +44,7 @@ def login_page(request):
 
 
 def howitworks(request):
+    "Proxy view"
     if request.user.is_authenticated():
         return index(request)
     else:

cms/djangoapps/contentstore/views/requests.py

@@ -1,4 +1,5 @@
 from django.http import HttpResponse
+from django.shortcuts import redirect
 from mitxmako.shortcuts import render_to_string, render_to_response
 
 __all__ = ['edge', 'event', 'landing']
@@ -11,7 +12,7 @@ def landing(request, org, course, coursename):
 
 # points to the temporary edge page
 def edge(request):
-    return render_to_response('university_profiles/edge.html', {})
+    return redirect('/')
 
 
 def event(request):

cms/djangoapps/contentstore/views/tabs.py

@@ -1,3 +1,6 @@
+"""
+Views related to course tabs
+"""
 from access import has_access
 from util.json_request import expect_json
 
@@ -39,6 +42,7 @@ def initialize_course_tabs(course):
 @login_required
 @expect_json
 def reorder_static_tabs(request):
+    "Order the static tabs in the requested order"
     tabs = request.POST['tabs']
     course = get_course_for_item(tabs[0])
 
@@ -86,6 +90,7 @@ def reorder_static_tabs(request):
 @login_required
 @ensure_csrf_cookie
 def edit_tabs(request, org, course, coursename):
+    "Edit tabs"
     location = ['i4x', org, course, 'course', coursename]
     store = get_modulestore(location)
     course_item = store.get_item(location)
@@ -122,6 +127,7 @@ def edit_tabs(request, org, course, coursename):
 @login_required
 @ensure_csrf_cookie
 def static_pages(request, org, course, coursename):
+    "Static pages view"
 
     location = get_location_and_verify_access(request, org, course, coursename)
 

cms/djangoapps/contentstore/views/user.py

@@ -1,7 +1,10 @@
+import json
 from django.conf import settings
 from django.core.exceptions import PermissionDenied
 from django.core.urlresolvers import reverse
+from django.contrib.auth.models import User, Group
 from django.contrib.auth.decorators import login_required
+from django.views.decorators.http import require_http_methods
 from django.utils.translation import ugettext as _
 from django.views.decorators.http import require_POST
 from django_future.csrf import ensure_csrf_cookie
@@ -9,10 +12,13 @@ from mitxmako.shortcuts import render_to_response
 from django.core.context_processors import csrf
 
 from xmodule.modulestore.django import modulestore
-from contentstore.utils import get_url_reverse, get_lms_link_for_item
-from util.json_request import expect_json, JsonResponse
-from auth.authz import STAFF_ROLE_NAME, INSTRUCTOR_ROLE_NAME, 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 xmodule.modulestore import Location
+from contentstore.utils import get_lms_link_for_item
+from util.json_request import JsonResponse
+from auth.authz import (
+    STAFF_ROLE_NAME, INSTRUCTOR_ROLE_NAME,
+    add_user_to_course_group, remove_user_from_course_group,
+    get_course_groupname_for_role)
 from course_creators.views import get_course_creator_status, add_user_with_status_unrequested, user_requested_access
 
 from .access import has_access
@@ -36,11 +42,22 @@ def index(request):
                 and course.location.name != '')
     courses = filter(course_filter, courses)
 
+    def format_course_for_view(course):
+        return (
+            course.display_name,
+            reverse("course_index", kwargs={
+                'org': course.location.org,
+                'course': course.location.course,
+                'name': course.location.name,
+            }),
+            get_lms_link_for_item(
+                course.location,
+                course_id=course.location.course_id,
+            ),
+        )
+
     return render_to_response('index.html', {
-        'courses': [(course.display_name,
-                    get_url_reverse('CourseOutline', course),
-                    get_lms_link_for_item(course.location, course_id=course.location.course_id))
-                    for course in courses],
+        'courses': [format_course_for_view(c) for c in courses],
         'user': request.user,
         'request_course_creator_url': reverse('request_course_creator'),
         'course_creator_status': _get_course_creator_status(request.user),
@@ -60,104 +77,141 @@ def request_course_creator(request):
 
 @login_required
 @ensure_csrf_cookie
-def manage_users(request, location):
+def manage_users(request, org, course, name):
     '''
     This view will return all CMS users who are editors for the specified course
     '''
+    location = Location('i4x', org, course, 'course', name)
     # check that logged in user has permissions to this item
     if not has_access(request.user, location, role=INSTRUCTOR_ROLE_NAME) and not has_access(request.user, location, role=STAFF_ROLE_NAME):
         raise PermissionDenied()
 
     course_module = modulestore().get_item(location)
 
+    staff_groupname = get_course_groupname_for_role(location, "staff")
+    staff_group, __ = Group.objects.get_or_create(name=staff_groupname)
+    inst_groupname = get_course_groupname_for_role(location, "instructor")
+    inst_group, __ = Group.objects.get_or_create(name=inst_groupname)
+
     return render_to_response('manage_users.html', {
         'context_course': course_module,
-        'staff': get_users_in_course_group_by_role(location, STAFF_ROLE_NAME),
-        'add_user_postback_url': reverse('add_user', args=[location]).rstrip('/'),
-        'remove_user_postback_url': reverse('remove_user', args=[location]).rstrip('/'),
+        'staff': staff_group.user_set.all(),
+        'instructors': inst_group.user_set.all(),
         'allow_actions': has_access(request.user, location, role=INSTRUCTOR_ROLE_NAME),
-        'request_user_id': request.user.id
     })
 
 
-@expect_json
 @login_required
 @ensure_csrf_cookie
-def add_user(request, location):
-    '''
-    This POST-back view will add a user - specified by email - to the list of editors for
-    the specified course
-    '''
-    email = request.POST.get("email")
-
-    if not email:
+@require_http_methods(("GET", "POST", "PUT", "DELETE"))
+def course_team_user(request, org, course, name, email):
+    location = Location('i4x', org, course, 'course', name)
+    # check that logged in user has permissions to this item
+    if has_access(request.user, location, role=INSTRUCTOR_ROLE_NAME):
+        # instructors have full permissions
+        pass
+    elif has_access(request.user, location, role=STAFF_ROLE_NAME) and email == request.user.email:
+        # staff can only affect themselves
+        pass
+    else:
         msg = {
-            'Status': 'Failed',
-            'ErrMsg': _('Please specify an email address.'),
+            "error": _("Insufficient permissions")
         }
         return JsonResponse(msg, 400)
 
-    # remove leading/trailing whitespace if necessary
-    email = email.strip()
-
-    # check that logged in user has admin permissions to this course
-    if not has_access(request.user, location, role=INSTRUCTOR_ROLE_NAME):
-        raise PermissionDenied()
-
-    user = get_user_by_email(email)
-
-    # user doesn't exist?!? Return error.
-    if user is None:
+    try:
+        user = User.objects.get(email=email)
+    except:
         msg = {
-            'Status': 'Failed',
-            'ErrMsg': _("Could not find user by email address '{email}'.").format(email=email),
+            "error": _("Could not find user by email address '{email}'.").format(email=email),
         }
         return JsonResponse(msg, 404)
 
-    # user exists, but hasn't activated account?!?
+    # role hierarchy: "instructor" has more permissions than "staff" (in a course)
+    roles = ["instructor", "staff"]
+
+    if request.method == "GET":
+        # just return info about the user
+        msg = {
+            "email": user.email,
+            "active": user.is_active,
+            "role": None,
+        }
+        # what's the highest role that this user has?
+        groupnames = set(g.name for g in user.groups.all())
+        for role in roles:
+            role_groupname = get_course_groupname_for_role(location, role)
+            if role_groupname in groupnames:
+                msg["role"] = role
+                break
+        return JsonResponse(msg)
+
+    # can't modify an inactive user
     if not user.is_active:
         msg = {
-            'Status': 'Failed',
-            'ErrMsg': _('User {email} has registered but has not yet activated his/her account.').format(email=email),
+            "error": _('User {email} has registered but has not yet activated his/her account.').format(email=email),
         }
         return JsonResponse(msg, 400)
 
-    # ok, we're cool to add to the course group
-    add_user_to_course_group(request.user, user, location, STAFF_ROLE_NAME)
+    # make sure that the role groups exist
+    staff_groupname = get_course_groupname_for_role(location, "staff")
+    staff_group, __ = Group.objects.get_or_create(name=staff_groupname)
+    inst_groupname = get_course_groupname_for_role(location, "instructor")
+    inst_group, __ = Group.objects.get_or_create(name=inst_groupname)
 
-    return JsonResponse({"Status": "OK"})
+    if request.method == "DELETE":
+        # remove all roles in this course from this user: but fail if the user
+        # is the last instructor in the course team
+        instructors = set(inst_group.user_set.all())
+        staff = set(staff_group.user_set.all())
+        if user in instructors and len(instructors) == 1:
+            msg = {
+                "error": _("You may not remove the last instructor from a course")
+            }
+            return JsonResponse(msg, 400)
 
+        if user in instructors:
+            user.groups.remove(inst_group)
+        if user in staff:
+            user.groups.remove(staff_group)
+        user.save()
+        return JsonResponse()
 
-@expect_json
-@login_required
-@ensure_csrf_cookie
-def remove_user(request, location):
-    '''
-    This POST-back view will remove a user - specified by email - from the list of editors for
-    the specified course
-    '''
+    # all other operations require the requesting user to specify a role
+    if request.META.get("CONTENT_TYPE", "") == "application/json" and request.body:
+        try:
+            payload = json.loads(request.body)
+        except:
+            return JsonResponse({"error": _("malformed JSON")}, 400)
+        try:
+            role = payload["role"]
+        except KeyError:
+            return JsonResponse({"error": _("`role` is required")}, 400)
+    else:
+        if not "role" in request.POST:
+            return JsonResponse({"error": _("`role` is required")}, 400)
+        role = request.POST["role"]
 
-    email = request.POST["email"]
-
-    # check that logged in user has admin permissions on this course
-    if not has_access(request.user, location, role=INSTRUCTOR_ROLE_NAME):
-        raise PermissionDenied()
-
-    user = get_user_by_email(email)
-    if user is None:
-        msg = {
-            'Status': 'Failed',
-            'ErrMsg': _("Could not find user by email address '{email}'.").format(email=email),
-        }
-        return JsonResponse(msg, 404)
-
-    # make sure we're not removing ourselves
-    if user.id == request.user.id:
-        raise PermissionDenied()
-
-    remove_user_from_course_group(request.user, user, location, STAFF_ROLE_NAME)
-
-    return JsonResponse({"Status": "OK"})
+    if role == "instructor":
+        if not has_access(request.user, location, role=INSTRUCTOR_ROLE_NAME):
+            msg = {
+                "error": _("Only instructors may create other instructors")
+            }
+            return JsonResponse(msg, 400)
+        add_user_to_course_group(request.user, user, location, role)
+    elif role == "staff":
+        # if we're trying to downgrade a user from "instructor" to "staff",
+        # make sure we have at least one other instructor in the course team.
+        instructors = set(inst_group.user_set.all())
+        if user in instructors:
+            if len(instructors) == 1:
+                msg = {
+                    "error": _("You may not remove the last instructor from a course")
+                }
+                return JsonResponse(msg, 400)
+            remove_user_from_course_group(request.user, user, location, "instructor")
+        add_user_to_course_group(request.user, user, location, role)
+    return JsonResponse()
 
 
 def _get_course_creator_status(user):

cms/envs/aws_migrate.py

@@ -0,0 +1,24 @@
+"""
+A Django settings file for use on AWS while running
+database migrations, since we don't want to normally run the
+LMS with enough privileges to modify the database schema.
+"""
+
+# We intentionally define lots of variables that aren't used, and
+# want to import all variables from base settings files
+# pylint: disable=W0401, W0614
+
+# Import everything from .aws so that our settings are based on those.
+from .aws import *
+import os
+from django.core.exceptions import ImproperlyConfigured
+
+USER = os.environ.get('DB_MIGRATION_USER', 'root')
+PASSWORD = os.environ.get('DB_MIGRATION_PASS', None)
+
+if not PASSWORD:
+    raise ImproperlyConfigured("No database password was provided for running "
+                               "migrations.  This is fatal.")
+
+DATABASES['default']['USER'] = USER
+DATABASES['default']['PASSWORD'] = PASSWORD

cms/static/js/base.js

@@ -597,11 +597,9 @@ function cancelNewSection(e) {
 
 function addNewCourse(e) {
     e.preventDefault();
-    $('.new-course-button').addClass('disabled');
-    $(e.target).addClass('disabled');
-    var $newCourse = $($('#new-course-template').html());
+    $('.new-course-button').addClass('is-disabled');
+    var $newCourse = $('.wrapper-create-course').addClass('is-shown');
     var $cancelButton = $newCourse.find('.new-course-cancel');
-    $('.courses').prepend($newCourse);
     $newCourse.find('.new-course-name').focus().select();
     $newCourse.find('form').bind('submit', saveNewCourse);
     $cancelButton.bind('click', cancelNewCourse);
@@ -613,41 +611,97 @@ function addNewCourse(e) {
 function saveNewCourse(e) {
     e.preventDefault();
 
-    var $newCourse = $(this).closest('.new-course');
-    var org = $newCourse.find('.new-course-org').val();
-    var number = $newCourse.find('.new-course-number').val();
-    var display_name = $newCourse.find('.new-course-name').val();
+    var $newCourseForm = $(this).closest('#create-course-form');
+    var display_name = $newCourseForm.find('.new-course-name').val();
+    var org = $newCourseForm.find('.new-course-org').val();
+    var number = $newCourseForm.find('.new-course-number').val();
+    var run = $newCourseForm.find('.new-course-run').val();
 
-    if (org == '' || number == '' || display_name == '') {
-        alert(gettext('You must specify all fields in order to create a new course.'));
-        return;
+    var required_field_text = gettext('Required field');
+
+    var display_name_errMsg = (display_name === '') ? required_field_text : null;
+    var org_errMsg = (org === '') ? required_field_text : null;
+    var number_errMsg = (number === '') ? required_field_text : null;
+    var run_errMsg = (run === '') ? required_field_text : null;
+
+    var bInErr = (display_name_errMsg || org_errMsg || number_errMsg || run_errMsg);
+
+    // check for suitable encoding
+    if (!bInErr) {
+        var encoding_errMsg = gettext('Please do not use any spaces or special characters in this field.');
+
+        if (encodeURIComponent(org) != org)
+            org_errMsg = encoding_errMsg;
+        if (encodeURIComponent(number) != number)
+            number_errMsg = encoding_errMsg;
+        if (encodeURIComponent(run) != run)
+            run_errMsg = encoding_errMsg;
+
+        bInErr = (org_errMsg || number_errMsg || run_errMsg);
     }
 
+    var header_err_msg = (bInErr) ? gettext('Please correct the fields below.') : null;
+
+    var setNewCourseErrMsgs = function(header_err_msg, display_name_errMsg, org_errMsg, number_errMsg, run_errMsg) {
+        if (header_err_msg) {
+            $('.wrapper-create-course').addClass('has-errors');
+            $('.wrap-error').addClass('is-shown');
+            $('#course_creation_error').html('<p>' + header_err_msg + '</p>');
+        } else {
+            $('.wrap-error').removeClass('is-shown');
+            $('#course_creation_error').html('');
+        }
+
+        var setNewCourseFieldInErr = function(el, msg) {
+            el.children('.tip-error').remove();
+            if (msg !== null && msg !== '') {
+                el.addClass('error');
+                el.append('<span class="tip tip-error">' + msg + '</span>');
+            } else {
+                el.removeClass('error');
+            }
+        };
+
+        setNewCourseFieldInErr($('#field-course-name'), display_name_errMsg);
+        setNewCourseFieldInErr($('#field-organization'), org_errMsg);
+        setNewCourseFieldInErr($('#field-course-number'), number_errMsg);
+        setNewCourseFieldInErr($('#field-course-run'), run_errMsg);
+    };
+
+    setNewCourseErrMsgs(header_err_msg, display_name_errMsg, org_errMsg, number_errMsg, run_errMsg);
+
+    if (bInErr)
+        return;
+
     analytics.track('Created a Course', {
         'org': org,
         'number': number,
-        'display_name': display_name
+        'display_name': display_name,
+        'run': run
     });
 
     $.post('/create_new_course', {
-        'org': org,
-        'number': number,
-        'display_name': display_name
-    },
-
-    function(data) {
-        if (data.id != undefined) {
-            window.location = '/' + data.id.replace(/.*:\/\//, '');
-        } else if (data.ErrMsg != undefined) {
-            alert(data.ErrMsg);
+            'org': org,
+            'number': number,
+            'display_name': display_name,
+            'run': run
+        },
+        function(data) {
+            if (data.id !== undefined) {
+                window.location = '/' + data.id.replace(/.*:\/\//, '');
+            } else if (data.ErrMsg !== undefined) {
+                var orgErrMsg = (data.OrgErrMsg !== undefined) ? data.OrgErrMsg : null;
+                var courseErrMsg = (data.CourseErrMsg !== undefined) ? data.CourseErrMsg : null;
+                setNewCourseErrMsgs(data.ErrMsg, null, orgErrMsg, courseErrMsg, null);
+            }
         }
-    });
+    );
 }
 
 function cancelNewCourse(e) {
     e.preventDefault();
-    $('.new-course-button').removeClass('disabled');
-    $(this).parents('section.new-course').remove();
+    $('.new-course-button').removeClass('is-disabled');
+    $('.wrapper-create-course').removeClass('is-shown');
 }
 
 function addNewSubsection(e) {

cms/static/js/views/course_info_edit.js

@@ -34,16 +34,8 @@ CMS.Views.ClassInfoUpdateView = Backbone.View.extend({
     },
 
     initialize: function() {
-    	var self = this;
-        // instantiates an editor template for each update in the collection
-        window.templateLoader.loadRemoteTemplate("course_info_update",
-        	// TODO Where should the template reside? how to use the static.url to create the path?
-            "/static/client_templates/course_info_update.html",
-            function (raw_template) {
-        		self.template = _.template(raw_template);
-        		self.render();
-            }
-        );
+        this.template = _.template($("#course_info_update-tpl").text());
+        this.render();
         // when the client refetches the updates as a whole, re-render them
         this.listenTo(this.collection, 'reset', this.render);
     },
@@ -241,16 +233,11 @@ CMS.Views.ClassInfoHandoutsView = Backbone.View.extend({
     },
 
     initialize: function() {
+        this.template = _.template($("#course_info_handouts-tpl").text());
         var self = this;
         this.model.fetch({
             complete: function() {
-                window.templateLoader.loadRemoteTemplate("course_info_handouts",
-                    "/static/client_templates/course_info_handouts.html",
-                    function (raw_template) {
-                        self.template = _.template(raw_template);
-                        self.render();
-                    }
-                );
+                self.render();
             },
             reset: true
         });

cms/static/js/views/overview.js

@@ -225,12 +225,19 @@ function _handleReorder(event, ui, parentIdField, childrenSelector) {
                 ui.draggable.attr("style", "position:relative;"); // STYLE hack too
                 children.push(ui.draggable.data('id'));
         }
+        var saving = new CMS.Views.Notification.Mini({
+            title: gettext('Saving') + '…'
+        });
+        saving.show();
         $.ajax({
                 url: "/save_item",
                 type: "POST",
                 dataType: "json",
                 contentType: "application/json",
-                data:JSON.stringify({ 'id' : subsection_id, 'children' : children})
+                data:JSON.stringify({ 'id' : subsection_id, 'children' : children}),
+                success: function() {
+                    saving.hide();
+                }
         });
 
 }

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

@@ -11,16 +11,9 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({
         // TODO enable/disable save based on validation (currently enabled whenever there are changes)
     },
     initialize : function() {
-        var self = this;
-        // instantiates an editor template for each update in the collection
-        window.templateLoader.loadRemoteTemplate("advanced_entry",
-            "/static/client_templates/advanced_entry.html",
-            function (raw_template) {
-                self.template = _.template(raw_template);
-                self.render();
-            }
-        );
+        this.template = _.template($("#advanced_entry-tpl").text());
         this.listenTo(this.model, 'invalid', this.handleValidationError);
+        this.render();
     },
     render: function() {
         // catch potential outside call before template loaded
@@ -56,7 +49,7 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({
         CodeMirror.fromTextArea(textarea, {
             mode: "application/json", lineNumbers: false, lineWrapping: false,
             onChange: function(instance, changeobj) {
-                instance.save()
+                instance.save();
                 // this event's being called even when there's no change :-(
                 if (instance.getValue() !== oldValue) {
                     var message = gettext("Your changes will not take effect until you save your progress. Take care with key and value formatting, as validation is not implemented.");
@@ -105,8 +98,7 @@ CMS.Views.Settings.Advanced = CMS.Views.ValidatingView.extend({
         //    call validateKey on each to ensure proper format
         //    check for dupes
         var self = this;
-        this.model.save({},
-            {
+        this.model.save({}, {
             success : function() {
                 self.render();
                 var title = gettext("Your policy changes have been saved.");

cms/static/sass/_shame.scss

@@ -23,6 +23,13 @@ body.dashboard {
 }
 
 
+// yes we have no boldness today - need to fix the resets
+body strong,
+body b {
+  font-weight: 700;
+}
+
+// known things to do (paint the fence, sand the floor, wax on/off)
 // ====================
 
 

cms/static/sass/elements/_forms.scss

@@ -93,6 +93,227 @@ form {
   }
 }
 
+// ELEM: form wrapper
+.wrapper-create-element {
+  height: 0;
+  margin-bottom: $baseline;
+  opacity: 0.0;
+  pointer-events: none;
+  overflow: hidden;
+
+  &.animate {
+    @include transition(opacity $tmg-f1 ease-in-out 0s, height $tmg-f1 ease-in-out 0s);
+  }
+
+  &.is-shown {
+    height: auto; // define a specific height for the animating version of this UI to work properly
+    opacity: 1.0;
+    pointer-events: auto;
+  }
+}
+
+// ELEM: form
+// form styling for creating a new content item (course, user, textbook)
+form[class^="create-"] {
+  @extend .ui-window;
+
+  .title {
+    @extend .t-title4;
+    font-weight: 600;
+    padding: $baseline ($baseline*1.5) 0 ($baseline*1.5);
+  }
+
+  fieldset {
+    padding: $baseline ($baseline*1.5);
+  }
+
+
+  .list-input {
+    @extend .cont-no-list;
+
+    .field {
+      margin: 0 0 ($baseline*0.75) 0;
+
+      &:last-child {
+        margin-bottom: 0;
+      }
+
+      &.required {
+
+        label {
+          font-weight: 600;
+        }
+
+        label:after {
+          margin-left: ($baseline/4);
+          content: "*";
+        }
+      }
+
+      label, input, textarea {
+        display: block;
+      }
+
+      label {
+        @extend  .t-copy-sub1;
+        @include transition(color $tmg-f3 ease-in-out 0s);
+        margin: 0 0 ($baseline/4) 0;
+
+        &.is-focused {
+          color: $blue;
+        }
+      }
+
+
+      input, textarea {
+        @extend .t-copy-base;
+        @include transition(all $tmg-f2 ease-in-out 0s);
+        height: 100%;
+        width: 100%;
+        padding: ($baseline/2);
+
+        &.long {
+          width: 100%;
+        }
+
+        &.short {
+          width: 25%;
+        }
+
+        /*@include placeholder {
+          color: $gray-l3;
+        }*/
+
+        &:focus {
+
+          + .tip {
+            color: $gray;
+          }
+        }
+      }
+
+      textarea.long {
+        height: ($baseline*5);
+      }
+
+      input[type="checkbox"] {
+        display: inline-block;
+        margin-right: ($baseline/4);
+        width: auto;
+        height: auto;
+
+        & + label {
+          display: inline-block;
+        }
+      }
+
+      .tip {
+        @extend .t-copy-sub2;
+        @include transition(color, 0.15s, ease-in-out);
+        display: block;
+        margin-top: ($baseline/4);
+        color: $gray-l3;
+      }
+
+      .tip-error {
+        display: none;
+        float: none;
+      }
+
+      &.error {
+        label {
+          color: $red;
+        }
+
+        .tip-error {
+          @extend .anim-fadeIn;
+          display: block;
+          color: $red;
+        }
+
+        input {
+          border-color: $red;
+        }
+      }
+    }
+
+    .field-inline {
+
+      input, textarea, select {
+        width: 62%;
+        display: inline-block;
+      }
+
+      .tip-stacked {
+        display: inline-block;
+        float: right;
+        width: 35%;
+        margin-top: 0;
+      }
+
+      &.error {
+        .tip-error {
+        }
+      }
+
+    }
+
+    .field-group {
+      @include clearfix();
+      margin: 0 0 ($baseline/2) 0;
+
+      .field {
+        display: block;
+        width: 47%;
+        border-bottom: none;
+        margin: 0 ($baseline*0.75) 0 0;
+        padding: ($baseline/4) 0 0 0;
+        float: left;
+        position: relative;
+
+        &:nth-child(odd) {
+          float: left;
+        }
+
+        &:nth-child(even) {
+          float: right;
+          margin-right: 0;
+        }
+
+        input, textarea {
+          width: 100%;
+        }
+      }
+    }
+  }
+
+  .actions {
+    box-shadow: inset 0 1px 2px $shadow;
+    margin-top: ($baseline*0.75);
+    border-top: 1px solid $gray-l1;
+    padding: ($baseline*0.75) ($baseline*1.5);
+    background: $gray-l6;
+
+    .action {
+      @include transition(all $tmg-f2 linear 0s);
+      display: inline-block;
+      padding: ($baseline/5) $baseline;
+      font-weight: 600;
+      text-transform: uppercase;
+    }
+
+    .action-primary {
+      @include blue-button;
+      @extend .t-action2;
+    }
+
+    .action-secondary {
+      @include grey-button;
+      @extend .t-action2;
+    }
+  }
+}
+
 // ====================
 
 // forms - grandfathered

cms/static/sass/elements/_icons.scss

@@ -1,4 +1,4 @@
-// studio - elements - icons
+// studio - elements - icons & badges
 // ====================
 
 .icon {
@@ -14,3 +14,45 @@
   vertical-align: middle;
   margin-right: ($baseline/4);
 }
+
+// ui - badges
+.wrapper-ui-badge {
+  position: absolute;
+  top: -1px;
+  left: ($baseline*1.5);
+  width: 100%;
+}
+
+.ui-badge {
+  @extend .t-title9;
+  position: relative;
+  border-bottom-right-radius: ($baseline/10);
+  border-bottom-left-radius: ($baseline/10);
+  padding: ($baseline/4) ($baseline/2) ($baseline/4) ($baseline/2);
+  font-weight: 600;
+  text-transform: uppercase;
+
+  * [class^="icon-"] {
+    margin-right: ($baseline/5);
+  }
+
+  // OPTION: add this class for a visual hanging display
+  &.is-hanging {
+    @include box-sizing(border-box);
+    @extend .ui-depth2;
+    top: -($baseline/4);
+
+    &:after {
+      position: absolute;
+      top: 0;
+      right: -($baseline/4);
+      display: block;
+      height: 0;
+      width: 0;
+      border-bottom: ($baseline/4) solid $black-t3;
+      border-right: ($baseline/4) solid transparent;
+      content: "";
+      opacity: 0.5;
+    }
+  }
+}

cms/static/sass/elements/_navigation.scss

@@ -64,12 +64,14 @@ nav {
     opacity: 0.0;
     pointer-events: none;
     width: ($baseline*8);
+    overflow: hidden;
 
 
     // dropped down state
     &.is-shown {
       opacity: 1.0;
       pointer-events: auto;
+      overflow: visible;
     }
   }
 

cms/static/sass/elements/_system-help.scss

@@ -55,8 +55,8 @@
   margin-bottom: $baseline;
 
   .title {
-    @extend .t-title7;
-    margin-bottom: ($baseline/4);
+    @extend .t-title6;
+    margin-bottom: ($baseline/2);
     font-weight: 700;
   }
 
@@ -167,6 +167,34 @@
   }
 }
 
+// particular notice - create
+.notice-create {
+  background-color: $gray-l4;
+
+  .title {
+    color: $gray-d2;
+  }
+
+  .copy {
+    color: $gray-d2;
+  }
+
+  &.has-actions {
+
+    .list-actions {
+
+      .action-item {
+
+      }
+
+      .action-primary {
+        @extend .btn-primary-green;
+        @extend .t-action3;
+      }
+    }
+  }
+}
+
 // particular notice - confirmation
 .notice-confirmation {
   background-color: $green-l5;

cms/static/sass/views/_dashboard.scss

@@ -358,22 +358,30 @@ body.dashboard {
 		}
 	}
 
-	.new-course {
-		@include clearfix();
-		padding: ($baseline*0.75) ($baseline*1.25);
-		margin-top: $baseline;
-		border-radius: 3px;
-		border: 1px solid $gray;
-		background: $white;
-		box-shadow: 0 1px 2px rgba(0, 0, 0, .1);
 
-		.title {
-			@extend .t-title4;
-			font-weight: 600;
-			margin-bottom: ($baseline/2);
-			border-bottom: 1px solid $gray-l3;
-			padding-bottom: ($baseline/2);
-		}
+  // ELEM: new user form
+  .wrapper-create-course {
+
+    // CASE: when form is animating
+    &.animate {
+
+      // STATE: shown
+      &.is-shown {
+        height: ($baseline*26);
+
+        // STATE: errors
+        &.has-errors {
+          height: ($baseline*33);
+        }
+      }
+    }
+  }
+
+  // ====================
+
+  // course listings
+
+	.create-course {
 
 		.row {
 			@include clearfix();
@@ -389,10 +397,6 @@ body.dashboard {
 			margin-right: 4%;
 		}
 
-		.course-info {
-			width: 600px;
-		}
-
 		label {
 			@extend .t-title7;
 			display: block;
@@ -401,7 +405,8 @@ body.dashboard {
 
 		.new-course-org,
 		.new-course-number,
-		.new-course-name {
+		.new-course-name,
+		.new-course-run {
 			width: 100%;
 		}
 
@@ -421,5 +426,25 @@ body.dashboard {
 		.item-details {
 			padding-bottom: 0;
 		}
+
+	    .wrap-error {
+	      @include transition(all $tmg-f2 ease 0s);
+	      height: 0;
+	      overflow: hidden;
+	      opacity: 0;
+	    }
+
+	    .wrap-error.is-shown {
+	      height: 65px;
+	      opacity: 1;
+	    }
+
+	    .message-status {
+	      display: block;
+	      margin-bottom: 0;
+	      padding: ($baseline*.5) ($baseline*1.5) 8px ($baseline*1.5);
+	      font-weight: bold;
+	    }
+
 	}
 }

cms/static/sass/views/_textbooks.scss

@@ -30,7 +30,7 @@ body.course.textbooks {
     }
 
     .textbook {
-      @extend .window;
+      @extend .ui-window;
       position: relative;
 
       .view-textbook {

cms/static/sass/views/_users.scss

@@ -3,80 +3,227 @@
 
 body.course.users {
 
-  .new-user-form {
-    display: none;
-    padding: 15px 20px;
-    background-color: $lightBluishGrey2;
+  // LAYOUT: page
+  .content-primary, .content-supplementary {
+    @include box-sizing(border-box);
+    float: left;
+  }
 
-    #result {
-      display: none;
-      float: left;
-      margin-bottom: 15px;
-      padding: 3px 15px;
-      border-radius: 3px;
-      background: $error-red;
-      font-size: 14px;
-      color: #fff;
-    }
+  .content-primary {
+    width: flex-grid(9, 12);
+    margin-right: flex-gutter();
+  }
 
-    .form-elements {
-      clear: both;
-    }
+  .content-supplementary {
+    width: flex-grid(3, 12);
+  }
 
-    label {
-      display: inline-block;
-      margin-right: 10px;
-    }
+  // ELEM: content
+  .content {
 
-    .email-input {
-      width: 350px;
-      padding: 8px 8px 10px;
-      border-color: $darkGrey;
+    .introduction {
+      @extend .t-copy-sub1;
+      margin: 0 0 ($baseline*2) 0;
     }
+  }
 
-    .add-button {
-      @include blue-button;
-      padding: 5px 20px 9px;
-    }
+  // ELEM: no users notice
+  .content .notice-create {
+    width: flexgrid(9, 9);
+    margin-top: $baseline;
 
-    .cancel-button {
-      @include white-button;
-      padding: 5px 20px 9px;
+    // CASE: notice has actions {
+    &.has-actions {
+
+      .msg, .list-actions {
+        display: inline-block;
+        vertical-align: middle;
+      }
+
+      .msg {
+        width: flex-grid(6, 9);
+        margin-right: flex-gutter();
+      }
+
+      .list-actions {
+        width: flex-grid(3, 9);
+        text-align: right;
+        margin-top: 0;
+
+        .action-item {
+
+        }
+
+        .action-primary {
+          @include green-button(); // overwriting for the sake of syncing older green button styles for now
+          @extend .t-action3;
+          padding: ($baseline/2) $baseline;
+        }
+      }
     }
   }
 
+
+  // ELEM: new user form
+  .wrapper-create-user {
+
+    &.is-shown {
+      height: ($baseline*15);
+    }
+  }
+
+  // ELEM: listing of users
+  .user-list, .user-item, .item-metadata, .item-actions {
+    @include box-sizing(border-box);
+  }
+
   .user-list {
-    border: 1px solid $mediumGrey;
-    background: #fff;
 
-    li {
+    .user-item {
+      @extend .ui-window;
+      @include clearfix();
       position: relative;
-      padding: 20px;
-      border-bottom: 1px solid $mediumGrey;
+      width: flex-grid(9, 9);
+      margin: 0 0 ($baseline/2) 0;
+      padding: ($baseline*1.25) ($baseline*1.5) $baseline ($baseline*1.5);
 
       &:last-child {
-        border-bottom: none;
+        margin-bottom: 0;
       }
 
-      span {
+      .item-metadata, .item-actions {
         display: inline-block;
+        vertical-align: middle;
       }
 
-      .user-name {
-        margin-right: 10px;
-        font-size: 24px;
-        font-weight: 300;
+      // ELEM: item - flag
+      .flag-role {
+        @extend .ui-badge;
+        color: $white;
+
+        .msg-you {
+          margin-left: ($baseline/5);
+          text-transform: none;
+          font-weight: 500;
+          color: $pink-l3;
+        }
+
+        &:after {
+          border-bottom-color: $pink-d4;
+        }
+
+        &.flag-role-staff {
+          background: $pink-u3;
+        }
+
+        &.flag-role-admin {
+          background: $pink;
+        }
       }
 
-      .user-email {
-        font-size: 14px;
-        font-style: italic;
-        color: $mediumGrey;
+      // ELEM: item - metadata
+      .item-metadata {
+        width: flex-grid(5, 9);
+        margin-right: flex-gutter();
+
+        .user-username, .user-email {
+          display: inline-block;
+          vertical-align: middle;
+        }
+
+        .user-username {
+          @extend .t-title4;
+          @include transition(color $tmg-f2 ease-in-out 0s);
+          margin: 0 ($baseline/2) ($baseline/10) 0;
+          color: $gray-d4;
+          font-weight: 600;
+        }
+
+        .user-email {
+          @extend .t-title6;
+        }
       }
 
+      // ELEM: item - actions
       .item-actions {
-        top: 24px;
+        width: flex-grid(4, 9);
+        position: static; // nasty reset needed due to base.scss
+        text-align: right;
+
+        .action {
+          display: inline-block;
+          vertical-align: middle;
+        }
+
+        .action-role {
+          width: flex-grid(3, 4);
+          margin-right: flex-gutter();
+        }
+
+        .action-delete {
+          width: flex-grid(1, 4);
+
+          // STATE: disabled
+          &.is-disabled {
+            opacity: 0.0;
+            visibility: hidden;
+            pointer-events: none;
+          }
+        }
+
+        .delete {
+          @extend .ui-btn-non;
+        }
+
+        // HACK: nasty reset needed due to base.scss
+        .delete-button {
+          margin-right: 0;
+          float: none;
+          color: inherit;
+        }
+
+        // ELEM: admin role controls
+        .toggle-admin-role {
+
+          &.add-admin-role {
+            @include blue-button;
+            @extend .t-action2;
+            @include transition(all .15s);
+            display: inline-block;
+            padding: ($baseline/5) $baseline;
+            font-weight: 600;
+          }
+
+          &.remove-admin-role {
+            @include grey-button;
+            @extend .t-action2;
+            @include transition(all .15s);
+            display: inline-block;
+            padding: ($baseline/5) $baseline;
+            font-weight: 600;
+          }
+        }
+
+        .notoggleforyou {
+          @extend .t-copy-sub1;
+          color: $gray-l2;
+        }
+      }
+
+      // STATE: hover
+      &:hover {
+
+        .user-username {
+        }
+
+        .user-email {
+
+        }
+
+        .item-actions {
+
+        }
       }
     }
   }
-}
+}

cms/templates/course_info.html

@@ -6,9 +6,15 @@
 <%block name="title">${_("Course Updates")}</%block>
 <%block name="bodyclass">is-signedin course course-info updates</%block>
 
+<%block name="header_extras">
+% for template_name in ["course_info_update", "course_info_handouts"]:
+  <script type="text/template" id="${template_name}-tpl">
+    <%static:include path="js/${template_name}.underscore" />
+  </script>
+% endfor
+</%block>
 
 <%block name="jsextra">
-  <script type="text/javascript" src="${static.url('js/template_loader.js')}"></script>
   <script type="text/javascript" src="${static.url('js/models/course_info.js')}"></script>
   <script type="text/javascript" src="${static.url('js/models/module_info.js')}"></script>
   <script type="text/javascript" src="${static.url('js/views/course_info_edit.js')}"></script>

cms/templates/index.html

@@ -36,36 +36,6 @@
 </script>
 </%block>
 
-<%block name="header_extras">
-  <script type="text/template" id="new-course-template">
-    <section class="new-course">
-      <h3 class="title">${_("Create a New Course:")}</h3>
-      <div class="item-details">
-        <form class="course-info">
-          <div class="row">
-	    <label>${_("Course Name")}</label>
-            <input type="text" class="new-course-name" />
-          </div>
-          <div class="row">
-            <div class="column">
-	      <label>${_("Organization")}</label>
-              <input type="text" class="new-course-org" />
-            </div>
-            <div class="column">
-	      <label>${_("Course Number")}</label>
-              <input type="text" class="new-course-number" />
-            </div>
-          </div>
-          <div class="row">
-            <input type="submit" value="${_('Save')}" class="new-course-save"/>
-            <input type="button" value="${_('Cancel')}" class="new-course-cancel" />
-          </div>
-        </form>
-      </div>
-    </section>
-  </script>
-</%block>
-
 <%block name="content">
 <div class="wrapper-mast wrapper">
   <header class="mast has-actions">
@@ -109,6 +79,57 @@
         %endif
       </div>
 
+      % if course_creator_status=='granted':
+      <div class="wrapper-create-element wrapper-create-course">
+        <form class="create-course course-info" id="create-course-form" name="create-course-form">
+          <div class="wrap-error">
+            <div id="course_creation_error" name="course_creation_error" class="message message-status message-status error" role="alert">
+            <p>${_("Please correct the highlighted fields below.")}</p>
+            </div>
+          </div>
+
+          <div class="wrapper-form">
+            <h3 class="title">${_("Create a New Course")}</h3>
+
+            <fieldset>
+              <legend class="sr">${_("Required Information to Create a New Course")}</legend>
+
+              <ol class="list-input">
+                <li class="field field-inline text required" id="field-course-name">
+                  <label for="new-course-name">${_("Course Name")}</label>
+                  <input class="new-course-name" id="new-course-name" type="text" name="new-course-name" aria-required="true" placeholder="${_('e.g. Introduction to Computer Science')}" />
+                  <span class="tip tip-stacked">${_("The public display name for your course.")}</span>
+                </li>
+                <li class="field field-inline text required" id="field-organization">
+                  <label for="new-course-org">${_("Organization")}</label>
+                  <input class="new-course-org" id="new-course-org" type="text" name="new-course-org" aria-required="true" placeholder="${_('e.g. MITX or IMF')}" />
+                  <span class="tip tip-stacked">${_("The name of the organization sponsoring the course")}  - <strong>${_("Note: No spaces or special characters are allowed. This cannot be changed.")}</strong></span>
+                </li>
+
+                <li class="field field-inline text required" id="field-course-number">
+                  <label for="new-course-number">${_("Course Number")}</label>
+                  <input class="new-course-number" id="new-course-number" type="text" name="new-course-number" aria-required="true" placeholder="${_('e.g. CS101')}" />
+                  <span class="tip tip-stacked">${_("The unique number that identifies your course within your organization")} - <strong>${_("Note: No spaces or special characters are allowed. This cannot be changed.")}</strong></span>
+                </li>
+
+                <li class="field field-inline text required" id="field-course-run">
+                  <label for="new-course-run">${_("Course Run")}</label>
+                  <input class="new-course-run" id="new-course-run" type="text" name="new-course-run" aria-required="true"placeholder="${_('e.g. 2013_Spring')}" />
+                  <span class="tip tip-stacked">${_("The term in which your course will run")} - <strong>${_("Note: No spaces or special characters are allowed. This cannot be changed.")}</strong></span>
+                </li>
+              </ol>
+
+            </fieldset>
+          </div>
+
+          <div class="actions">
+            <input type="submit" value="${_('Save')}" class="action action-primary new-course-save" />
+            <input type="button" value="${_('Cancel')}" class="action action-secondary action-cancel new-course-cancel" />
+          </div>
+        </form>
+      </div>
+      % endif
+
       %if len(courses) > 0:
       <div class="courses">
         <ul class="list-courses">

cms/templates/js/advanced_entry.underscore

@@ -0,0 +1,11 @@
+<li class="field-group course-advanced-policy-list-item">
+	<div class="field is-not-editable text key" id="<%= key %>">
+		<label for="<%= keyUniqueId %>">Policy Key:</label>
+		<input readonly title="This field is disabled: policy keys cannot be edited." type="text" class="short policy-key" id="<%= keyUniqueId %>" value="<%= key %>" />
+  </div>
+
+  <div class="field text value">
+    <label for="<%= valueUniqueId %>">Policy Value:</label>
+    <textarea class="json text" id="<%= valueUniqueId %>"><%= value %></textarea>
+  </div>
+</li>

cms/templates/manage_users.html

@@ -1,14 +1,16 @@
 <%! from django.utils.translation import ugettext as _ %>
+<%! from django.core.urlresolvers import reverse %>
+<%! from auth.authz import is_user_in_course_group_role %>
 <%inherit file="base.html" />
 <%block name="title">${_("Course Team Settings")}</%block>
-<%block name="bodyclass">is-signedin course users settings team</%block>
+<%block name="bodyclass">is-signedin course users team</%block>
 
 
 <%block name="content">
 <div class="wrapper-mast wrapper">
   <header class="mast has-actions has-subtitle">
     <h1 class="page-header">
-      <small class="subtitle">${_("Course Settings")}</small>
+      <small class="subtitle">${_("Settings")}</small>
       <span class="sr">&gt; </span>${_("Course Team")}
     </h1>
 
@@ -17,7 +19,7 @@
       <ul>
         %if allow_actions:
         <li class="nav-item">
-          <a href="#" class="button new-button new-user-button"><i class="icon-plus"></i> ${_("New User")}</a>
+          <a href="#" class="button new-button create-user-button"><i class="icon-plus"></i> ${_("New Team Member")}</a>
         </li>
         %endif
       </ul>
@@ -25,111 +27,291 @@
   </header>
 </div>
 
-<div class="main-wrapper">
-  <div class="inner-wrapper">
-
-    <div class="details">
-      <p>${_("The following list of users have been designated as course staff. This means that these users will have permissions to modify course content. You may add additional course staff below, if you are the course instructor. Please note that they must have already registered and verified their account.")}</p>
-    </div>
-
-    <article class="user-overview">
+<div class="wrapper-content wrapper">
+  <section class="content">
+    <article class="content-primary" role="main">
       %if allow_actions:
-      <form class="new-user-form">
-        <div id="result"></div>
-        <div class="form-elements">
-          <label>email: </label><input type="text" id="email" class="email-input" autocomplete="off" placeholder="email@example.com">
-          <input type="submit" value="Add User" id="add_user" class="add-button" />
-          <input type="button" value="Cancel" class="cancel-button" />
-        </div>
-      </form>
-      %endif
-      <div>
-        <ol class="user-list">
-          % for user in staff:
-          <li>
-            <span class="user-name">${user.username}</span>
-            <span class="user-email">${user.email}</span>
-            %if allow_actions :
-            <div class="item-actions">
-              %if request_user_id != user.id:
-              <a href="#" class="delete-button remove-user" data-id="${user.email}"><span class="delete-icon"></span></a>
-              %endif
-            </div>
-            %endif
-          </li>
-          % endfor
-        </ol>
+      <div class="wrapper-create-element animate wrapper-create-user">
+        <form class="create-user" id="create-user-form" name="create-user-form">
+          <div class="wrapper-form">
+            <h3 class="title">${_("Add a User to Your Course's Team")}</h3>
+
+            <fieldset class="form-fields">
+              <legend class="sr">${_("New Team Member Information")}</legend>
+
+                <ol class="list-input">
+                  <li class="field text required create-user-email">
+                    <label for="user-email-input">${_("User's Email Address")}</label>
+                    <input id="user-email-input" name="user-email" type="text" placeholder="${_('e.g. jane.doe@gmail.com')}" value="">
+                    <span class="tip tip-stacked">${_("Please provide the email address of the course staff member you'd like to add")}</span>
+                  </li>
+                </ol>
+            </fieldset>
+          </div>
+
+          <div class="actions">
+            <button class="action action-primary" type="submit">${_("Add User")}</button>
+            <button class="action action-secondary action-cancel">${_("Cancel")}</button>
+          </div>
+        </form>
       </div>
+      %endif
+
+      <ol class="user-list">
+        % for user in staff:
+          <% api_url = reverse('course_team_user', kwargs=dict(
+               org=context_course.location.org,
+               course=context_course.location.course,
+               name=context_course.location.name,
+               email=user.email,
+             ))
+          %>
+        <li class="user-item" data-email="${user.email}" data-url="${api_url}">
+
+        <% is_instuctor = is_user_in_course_group_role(user, context_course.location, 'instructor', check_staff=False) %>
+          % if is_instuctor:
+          <span class="wrapper-ui-badge">
+            <span class="flag flag-role flag-role-admin is-hanging">
+              <span class="label sr">${_("Current Role:")}</span>
+              <span class="value">
+                ${_("Admin")}
+                % if request.user.id == user.id:
+                    <span class="msg-you">${_("You!")}</span>
+                % endif
+              </span>
+            </span>
+          </span>
+          % else:
+          <span class="wrapper-ui-badge">
+            <span class="flag flag-role flag-role-staff is-hanging">
+              <span class="label sr">${_("Current Role:")}</span>
+              <span class="value">
+                ${_("Staff")}
+                % if request.user.id == user.id:
+                    <span class="msg-you">${_("You!")}</span>
+                % endif
+              </span>
+            </span>
+          </span>
+          % endif
+
+          <div class="item-metadata">
+            <h3 class="user-name">
+              <span class="user-username">${user.username}</span>
+              <span class="user-email">
+                <a class="action action-email" href="mailto:${user.email}" title="${_("send an email message to {email}").format(email=user.email)}">${user.email}</a>
+              </span>
+            </h3>
+          </div>
+
+          % if allow_actions:
+          <ul  class="item-actions user-actions">
+            <li class="action action-role">
+              % if is_instuctor and len(instructors) == 1:
+                <span class="admin-role notoggleforyou">${_("Promote another member to Admin to remove your admin rights")}</span>
+              % else:
+                <a href="#" class="admin-role toggle-admin-role ${'remove' if is_instuctor else 'add'}-admin-role">${_("Remove Admin Access") if is_instuctor else _("Add Admin Access")}</a>
+              % endif
+            </li>
+            <li class="action action-delete ${"is-disabled" if request.user.id == user.id else ""}">
+                <a href="#" class="delete remove-user action-icon" data-id="${user.email}"><i class="icon-trash"></i><span class="sr">${_("Delete the user, {username}").format(username=user.username)}</span></a>
+            </li>
+          </ul>
+          % endif
+
+        </li>
+        % endfor
+      </ol>
+
+      <% user_is_instuctor = is_user_in_course_group_role(request.user, context_course.location, 'instructor', check_staff=False) %>
+      % if user_is_instuctor and len(staff) == 1:
+      <div class="notice notice-incontext notice-create has-actions">
+        <div class="msg">
+          <h3 class="title">${_('Add Team Members to This Course')}</h3>
+          <div class="copy">
+            <p>${_('Adding team members makes course authoring collaborative. Users must be signed up for Studio and have an active account. ')}</p>
+          </div>
+        </div>
+
+        <ul class="list-actions">
+          <li class="action-item">
+            <a href="#" class="action action-primary button new-button create-user-button"><i class="icon-plus icon-inline"></i> ${_('Add a New Team Member')}</a>
+          </li>
+        </ul>
+      </div>
+      %endif
     </article>
-  </div>
+
+    <aside class="content-supplementary" role="complimentary">
+      <div class="bit">
+        <h3 class="title-3">${_("About Roles within Your Course Team")}</h3>
+        <p>${_("Course team members are co-authors (staff). They have full access to all the content in the course and all the same editing privileges. Admins have the unique ability to add and remove course team members.")}</p>
+      </div>
+
+      % if user_is_instuctor and len(instructors) == 1:
+      <div class="bit">
+        <h3 class="title-3">${_("Tranferring Ownership")}</h3>
+        <p>${_("There must always be an Admin assigned to every course. To transfer your ownership of the course, add Admin access to another user and request they remove you from the Course Team list.")}</p>
+      </div>
+      % endif
+    </aside>
+  </section>
 </div>
 </%block>
 
 <%block name="jsextra">
 <script type="text/javascript">
-  var $newUserForm;
-  var addUserPostbackUrl = "${add_user_postback_url}";
-  var removeUserPostbackUrl = "${remove_user_postback_url}";
-
-  function showNewUserForm(e) {
-    e.preventDefault();
-    $newUserForm.slideDown(150);
-    $newUserForm.find('.email-input').focus();
-  }
-
-  function hideNewUserForm(e) {
-    e.preventDefault();
-    $newUserForm.slideUp(150);
-    $('#result').hide();
-    $('#email').val('');
-  }
-
-  function checkForCancel(e) {
-    if(e.which == 27) {
-      e.data.$cancelButton.click();
-    }
-  }
-
-  function addUser(e) {
-    e.preventDefault();
-
-    $.ajax({
-      url: addUserPostbackUrl,
-      type: 'POST',
-      dataType: 'json',
-      contentType: 'application/json',
-      data: JSON.stringify({ 'email': $('#email').val()}),
-      success: function(data) {
-        location.reload();
-      },
-      notifyOnError: false,
-      error: function(jqXHR, textStatus, errorThrown) {
-        data = JSON.parse(jqXHR.responseText);
-        $('#result').show().empty().append(data.ErrMsg);
-      }
-    });
-  }
+  var tplUserURL = "${reverse('course_team_user', kwargs=dict(
+      org=context_course.location.org,
+      course=context_course.location.course,
+      name=context_course.location.name,
+      email="@@EMAIL@@",
+    ))}"
 
   $(document).ready(function() {
-    $newUserForm = $('.new-user-form');
-    var $cancelButton = $newUserForm.find('.cancel-button');
-    $newUserForm.bind('submit', addUser);
-    $cancelButton.bind('click', hideNewUserForm);
-
-    $('.new-user-button').bind('click', showNewUserForm);
-    $('body').bind('keyup', { $cancelButton: $cancelButton }, checkForCancel);
-
-    $('.remove-user').click(function() {
+    var $createUserForm = $('#create-user-form');
+    var $createUserFormWrapper = $createUserForm.closest('.wrapper-create-user');
+    $createUserForm.bind('submit', function(e) {
+      e.preventDefault();
+      var url = tplUserURL.replace("@@EMAIL@@", $('#user-email-input').val().trim())
       $.ajax({
-        url: removeUserPostbackUrl,
+        url: url,
         type: 'POST',
         dataType: 'json',
         contentType: 'application/json',
-        data:JSON.stringify({ 'email': $(this).data('id')}),
-        }).done(function() {
+        data: JSON.stringify({
+          role: 'staff',
+        }),
+        success: function(data) {
           location.reload();
-        })
+        },
+        notifyOnError: false,
+        error: function(jqXHR, textStatus, errorThrown) {
+          var message;
+          try {
+            message = JSON.parse(jqXHR.responseText).error || "Unknown";
+          } catch (e) {
+            message = "Unknown";
+          }
+          var prompt = new CMS.Views.Prompt.Error({
+            title: gettext("Error adding user"),
+            message: message,
+            actions: {
+              primary: {
+                text: gettext("OK"),
+                click: function(view) {
+                  view.hide();
+                  $("#user-email-input").focus()
+                }
+              }
+            }
+          })
+          prompt.show();
+        }
+      });
     });
+
+    var $cancelButton = $createUserForm.find('.action-cancel');
+    $cancelButton.bind('click', function(e) {
+      e.preventDefault();
+      $('.create-user-button').toggleClass('is-disabled');
+      $createUserFormWrapper.toggleClass('is-shown');
+      $('#user-email-input').val('');
+    });
+
+    $('.create-user-button').bind('click', function(e) {
+      e.preventDefault();
+      $('.create-user-button').toggleClass('is-disabled');
+      $createUserFormWrapper.toggleClass('is-shown');
+      $createUserForm.find('#user-email-input').focus();
+    });
+
+    $('body').bind('keyup', function(e) {
+      if(e.which == 27) {
+        $cancelButton.click();
+      }
+    });
+
+    $('.remove-user').click(function() {
+      var url = tplUserURL.replace("@@EMAIL@@", $(this).data('id'))
+      $.ajax({
+        url: url,
+        type: 'DELETE',
+        dataType: 'json',
+        contentType: 'application/json',
+        success: function(data) {
+          location.reload();
+        },
+        notifyOnError: false,
+        error: function(jqXHR, textStatus, errorThrown) {
+          var message;
+          try {
+            message = JSON.parse(jqXHR.responseText).error || "Unknown";
+          } catch (e) {
+            message = "Unknown";
+          }
+          var prompt = new CMS.Views.Prompt.Error({
+            title: gettext("Error removing user"),
+            message: message,
+            actions: {
+              primary: {
+                text: gettext("OK"),
+                click: function(view) {
+                  view.hide();
+                }
+              }
+            }
+          })
+          prompt.show();
+        }
+      });
+    });
+
+    $(".toggle-admin-role").click(function(e) {
+      e.preventDefault()
+      var type;
+      if($(this).hasClass("add-admin-role")) {
+        role = 'instructor';
+      } else {
+        role = 'staff';
+      }
+      var url = $(this).closest("li[data-url]").data("url");
+      $.ajax({
+        url: url,
+        type: 'POST',
+        dataType: 'json',
+        contentType: 'application/json',
+        data: JSON.stringify({
+          role: role
+        }),
+        success: function(data) {
+          location.reload();
+        },
+        notifyOnError: false,
+        error: function(jqXHR, textStatus, errorThrown) {
+          var message;
+          try {
+            message = JSON.parse(jqXHR.responseText).error || "Unknown";
+          } catch (e) {
+            message = "Unknown";
+          }
+          var prompt = new CMS.Views.Prompt.Error({
+            title: gettext("Error changing user"),
+            message: message,
+            actions: {
+              primary: {
+                text: gettext("OK"),
+                click: function(view) {
+                  view.hide();
+                }
+              }
+            }
+          })
+          prompt.show();
+        }
+      })
+    })
+
   });
 </script>
 </%block>

cms/templates/settings.html

@@ -262,7 +262,7 @@ from contentstore import utils
         <nav class="nav-related">
           <ul>
             <li class="nav-item"><a href="${reverse('contentstore.views.course_config_graders_page', kwargs={'org' : ctx_loc.org, 'course' : ctx_loc.course, 'name': ctx_loc.name})}">${_("Grading")}</a></li>
-            <li class="nav-item"><a href="${reverse('manage_users', kwargs=dict(location=ctx_loc))}">${_("Course Team")}</a></li>
+            <li class="nav-item"><a href="${reverse('manage_users', kwargs=dict(org=ctx_loc.org, course=ctx_loc.course, name=ctx_loc.name))}">${_("Course Team")}</a></li>
             <li class="nav-item"><a href="${reverse('course_advanced_settings', kwargs={'org' : ctx_loc.org, 'course' : ctx_loc.course, 'name': ctx_loc.name})}">${_("Advanced Settings")}</a></li>
           </ul>
         </nav>

cms/templates/settings_advanced.html

@@ -1,15 +1,17 @@
-<%! from django.utils.translation import ugettext as _ %>
 <%inherit file="base.html" />
+<%namespace name='static' file='static_content.html'/>
 <%! from django.core.urlresolvers import reverse %>
+<%! from django.utils.translation import ugettext as _ %>
+<%! from contentstore import utils %>
 <%block name="title">${_("Advanced Settings")}</%block>
 <%block name="bodyclass">is-signedin course advanced settings</%block>
 
-<%namespace name='static' file='static_content.html'/>
-<%!
-from contentstore import utils
-%>
-
 <%block name="jsextra">
+% for template_name in ["advanced_entry"]:
+  <script type="text/template" id="${template_name}-tpl">
+    <%static:include path="js/${template_name}.underscore" />
+  </script>
+% endfor
 
 <script type="text/javascript" src="${static.url('js/template_loader.js')}"></script>
 <script type="text/javascript" src="${static.url('js/views/validating_view.js')}"></script>
@@ -96,7 +98,7 @@ editor.render();
           <ul>
             <li class="nav-item"><a href="${reverse('contentstore.views.get_course_settings', kwargs=dict(org=ctx_loc.org, course=ctx_loc.course, name=ctx_loc.name))}">${_("Details &amp; Schedule")}</a></li>
             <li class="nav-item"><a href="${reverse('contentstore.views.course_config_graders_page', kwargs={'org' : ctx_loc.org, 'course' : ctx_loc.course, 'name': ctx_loc.name})}">${_("Grading")}</a></li>
-            <li class="nav-item"><a href="${reverse('manage_users', kwargs=dict(location=ctx_loc))}">${_("Course Team")}</a></li>
+            <li class="nav-item"><a href="${reverse('manage_users', kwargs={'org' : ctx_loc.org, 'course' : ctx_loc.course, 'name': ctx_loc.name})}">${_("Course Team")}</a></li>
           </ul>
         </nav>
       % endif

cms/templates/settings_graders.html

@@ -140,7 +140,7 @@ from contentstore import utils
         <nav class="nav-related">
           <ul>
             <li class="nav-item"><a href="${reverse('contentstore.views.get_course_settings', kwargs=dict(org=ctx_loc.org, course=ctx_loc.course, name=ctx_loc.name))}">${_("Details &amp; Schedule")}</a></li>
-            <li class="nav-item"><a href="${reverse('manage_users', kwargs=dict(location=ctx_loc))}">${_("Course Team")}</a></li>
+            <li class="nav-item"><a href="${reverse('manage_users', kwargs={'org' : ctx_loc.org, 'course' : ctx_loc.course, 'name': ctx_loc.name})}">${_("Course Team")}</a></li>
             <li class="nav-item"><a href="${reverse('course_advanced_settings', kwargs={'org' : ctx_loc.org, 'course' : ctx_loc.course, 'name': ctx_loc.name})}">${_("Advanced Settings")}</a></li>
           </ul>
         </nav>

cms/templates/widgets/header.html

@@ -60,7 +60,7 @@
                     <a href="${reverse('contentstore.views.course_config_graders_page', kwargs={'org' : ctx_loc.org, 'course' : ctx_loc.course, 'name': ctx_loc.name})}">${_("Grading")}</a>
                   </li>
                   <li class="nav-item nav-course-settings-team">
-                    <a href="${reverse('manage_users', kwargs=dict(location=ctx_loc))}">${_("Course Team")}</a>
+                    <a href="${reverse('manage_users', kwargs=dict(org=ctx_loc.org, course=ctx_loc.course, name=ctx_loc.name))}">${_("Course Team")}</a>
                   </li>
                   <li class="nav-item nav-course-settings-advanced">
                     <a href="${reverse('course_advanced_settings', kwargs={'org' : ctx_loc.org, 'course' : ctx_loc.course, 'name': ctx_loc.name})}">${_("Advanced Settings")}</a>

cms/urls.py

@@ -40,14 +40,12 @@ urlpatterns = ('',  # nopep8
     url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/course/(?P<coursename>[^/]+)/upload_asset$',
         'contentstore.views.upload_asset', name='upload_asset'),
 
+    url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/team/(?P<name>[^/]+)$',
+        'contentstore.views.manage_users', name='manage_users'),
+    url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/team/(?P<name>[^/]+)/(?P<email>[^/]+)$',
+        'contentstore.views.course_team_user', name='course_team_user'),
+
 
-    url(r'^manage_users/(?P<location>.*?)$', 'contentstore.views.manage_users', name='manage_users'),
-    url(r'^add_user/(?P<location>.*?)$',
-        'contentstore.views.add_user', name='add_user'),
-    url(r'^remove_user/(?P<location>.*?)$',
-        'contentstore.views.remove_user', name='remove_user'),
-    url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/course/(?P<name>[^/]+)/remove_user$',
-        'contentstore.views.remove_user', name='remove_user'),
     url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/info/(?P<name>[^/]+)$',
         'contentstore.views.course_info', name='course_info'),
     url(r'^(?P<org>[^/]+)/(?P<course>[^/]+)/course_info/updates/(?P<provided_id>.*)$',

common/djangoapps/mitxmako/shortcuts.py

@@ -92,9 +92,10 @@ def render_to_string(template_name, dictionary, context=None, namespace='main'):
     return template.render_unicode(**context_dictionary)
 
 
-def render_to_response(template_name, dictionary, context_instance=None, namespace='main', **kwargs):
+def render_to_response(template_name, dictionary=None, context_instance=None, namespace='main', **kwargs):
     """
     Returns a HttpResponse whose content is filled with the result of calling
     lookup.get_template(args[0]).render with the passed arguments.
     """
+    dictionary = dictionary or {}
     return HttpResponse(render_to_string(template_name, dictionary, context_instance, namespace), **kwargs)

common/djangoapps/student/views.py

@@ -90,10 +90,7 @@ def index(request, extra_context={}, user=None):
     courses = get_courses(None, domain=domain)
     courses = sort_by_announcement(courses)
 
-    # Get the 3 most recent news
-    top_news = _get_news(top=3)
-
-    context = {'courses': courses, 'news': top_news}
+    context = {'courses': courses}
     context.update(extra_context)
     return render_to_response('index.html', context)
 
@@ -285,9 +282,6 @@ def dashboard(request):
 
     exam_registrations = {course.id: exam_registration_info(request.user, course) for course in courses}
 
-    # Get the 3 most recent news
-    top_news = _get_news(top=3) if not settings.MITX_FEATURES.get('ENABLE_MKTG_SITE', False) else None
-
     # get info w.r.t ExternalAuthMap
     external_auth_map = None
     try:
@@ -302,7 +296,6 @@ def dashboard(request):
                'errored_courses': errored_courses,
                'show_courseware_links_for': show_courseware_links_for,
                'cert_statuses': cert_statuses,
-               'news': top_news,
                'exam_registrations': exam_registrations,
                }
 
@@ -1242,28 +1235,3 @@ def accept_name_change(request):
         raise Http404
 
     return accept_name_change_by_id(int(request.POST['id']))
-
-
-def _get_news(top=None):
-    "Return the n top news items on settings.RSS_URL"
-
-    # Don't return anything if we're in a themed site
-    if settings.MITX_FEATURES["USE_CUSTOM_THEME"]:
-        return None
-
-    feed_data = cache.get("students_index_rss_feed_data")
-    if feed_data is None:
-        if hasattr(settings, 'RSS_URL'):
-            feed_data = urllib.urlopen(settings.RSS_URL).read()
-        else:
-            feed_data = render_to_string("feed.rss", None)
-        cache.set("students_index_rss_feed_data", feed_data, settings.RSS_TIMEOUT)
-
-    feed = feedparser.parse(feed_data)
-    entries = feed['entries'][0:top]  # all entries if top is None
-    for entry in entries:
-        soup = BeautifulSoup(entry.description)
-        entry.image = soup.img['src'] if soup.img else None
-        entry.summary = soup.getText()
-
-    return entries

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

@@ -0,0 +1,44 @@
+// In the LMS sliders use built-in styles from  jquery-ui-1.8.22.custom.css.
+// CMS uses its own sliders styles.
+// These styles we use only to sure, that slider in GST module
+// will be render correctly (just like a duplication some from jquery-ui-1.8.22.custom.css).
+// Cause, for example, CMS overwrites many jquery-ui-1.8.22.custom.css styles,
+// and we must overwrite them again.
+
+.ui-widget-content {
+	border: 1px solid #dddddd;
+	color: #333333;
+}
+
+.ui-widget {
+	font-family: Trebuchet MS, Tahoma, Verdana, Arial, sans-serif;
+	font-size: 1.1em;
+}
+
+.ui-corner-all, .ui-corner-top, .ui-corner-left, .ui-corner-tl {
+	-moz-border-radius-topleft: 4px;
+	-webkit-border-top-left-radius: 4px;
+	-khtml-border-top-left-radius: 4px;
+	border-top-left-radius: 4px;
+}
+
+.ui-corner-all, .ui-corner-top, .ui-corner-right, .ui-corner-tr {
+	-moz-border-radius-topright: 4px;
+	-webkit-border-top-right-radius: 4px;
+	-khtml-border-top-right-radius: 4px;
+	border-top-right-radius: 4px;
+}
+
+.ui-corner-all, .ui-corner-bottom, .ui-corner-left, .ui-corner-bl {
+	-moz-border-radius-bottomleft: 4px;
+	-webkit-border-bottom-left-radius: 4px;
+	-khtml-border-bottom-left-radius: 4px;
+	border-bottom-left-radius: 4px;
+}
+
+.ui-corner-all, .ui-corner-bottom, .ui-corner-right, .ui-corner-br {
+	-moz-border-radius-bottomright: 4px;
+	-webkit-border-bottom-right-radius: 4px;
+	-khtml-border-bottom-right-radius: 4px;
+	border-bottom-right-radius: 4px;
+}

common/lib/xmodule/xmodule/gst_module.py

@@ -9,17 +9,16 @@ from lxml import etree
 from lxml import html
 import xmltodict
 
-from xmodule.mako_module import MakoModuleDescriptor
+from xmodule.editing_module import XMLEditingDescriptor
 from xmodule.xml_module import XmlDescriptor
 from xmodule.x_module import XModule
 from xmodule.stringify import stringify_children
 from pkg_resources import resource_string
 from xblock.core import String, Scope
 
-
 log = logging.getLogger(__name__)
 
-DEFAULT_RENDER="""
+DEFAULT_RENDER = """
     <h2>Graphic slider tool: Dynamic range and implicit functions.</h2>
 
     <p>You can make the range of the x axis (but not ticks of x axis) of
@@ -33,13 +32,19 @@ DEFAULT_RENDER="""
    </div>
     <plot style="margin-top:15px;margin-bottom:15px;"/>
 """
-DEFAULT_CONFIGURATION="""
+
+DEFAULT_CONFIGURATION = """
     <parameters>
         <param var="r" min="5" max="25" step="0.5" initial="12.5" />
     </parameters>
     <functions>
       <function color="red">Math.sqrt(r * r - x * x)</function>
       <function color="red">-Math.sqrt(r * r - x * x)</function>
+      <function color="red">Math.sqrt(r * r / 20 - Math.pow(x-r/2.5, 2)) + r/8</function>
+      <function color="red">-Math.sqrt(r * r / 20 - Math.pow(x-r/2.5, 2)) + r/5.5</function>
+      <function color="red">Math.sqrt(r * r / 20 - Math.pow(x+r/2.5, 2)) + r/8</function>
+      <function color="red">-Math.sqrt(r * r / 20 - Math.pow(x+r/2.5, 2)) + r/5.5</function>
+      <function color="red">-Math.sqrt(r * r / 5 - x * x) - r/5.5</function>
     </functions>
     <plot>
       <xrange>
@@ -54,10 +59,13 @@ DEFAULT_CONFIGURATION="""
 """
 
 
-
 class GraphicalSliderToolFields(object):
-    render = String(scope=Scope.content, default=DEFAULT_RENDER)
-    configuration = String(scope=Scope.content, default=DEFAULT_CONFIGURATION)
+    data = String(
+        help="Html contents to display for this module",
+        default='<render>{}</render><configuration>{}</configuration>'.format(
+            DEFAULT_RENDER, DEFAULT_CONFIGURATION),
+        scope=Scope.content
+    )
 
 
 class GraphicalSliderToolModule(GraphicalSliderToolFields, XModule):
@@ -65,40 +73,54 @@ class GraphicalSliderToolModule(GraphicalSliderToolFields, XModule):
     '''
 
     js = {
-      'coffee': [resource_string(__name__, 'js/src/javascript_loader.coffee')],
-      'js': [
-        # 3rd party libraries used by graphic slider tool.
-        # TODO - where to store them - outside xmodule?
-        resource_string(__name__, 'js/src/graphical_slider_tool/gst_main.js'),
-        resource_string(__name__, 'js/src/graphical_slider_tool/state.js'),
-        resource_string(__name__, 'js/src/graphical_slider_tool/logme.js'),
-        resource_string(__name__, 'js/src/graphical_slider_tool/general_methods.js'),
-        resource_string(__name__, 'js/src/graphical_slider_tool/sliders.js'),
-        resource_string(__name__, 'js/src/graphical_slider_tool/inputs.js'),
-        resource_string(__name__, 'js/src/graphical_slider_tool/graph.js'),
-        resource_string(__name__, 'js/src/graphical_slider_tool/el_output.js'),
-        resource_string(__name__, 'js/src/graphical_slider_tool/g_label_el_output.js'),
-        resource_string(__name__, 'js/src/graphical_slider_tool/gst.js')
-
-      ]
+        'coffee': [resource_string(__name__, 'js/src/javascript_loader.coffee')],
+        'js': [
+            # 3rd party libraries used by graphic slider tool.
+            # TODO - where to store them - outside xmodule?
+            resource_string(__name__, 'js/src/graphical_slider_tool/gst_main.js'),
+            resource_string(__name__, 'js/src/graphical_slider_tool/state.js'),
+            resource_string(__name__, 'js/src/graphical_slider_tool/logme.js'),
+            resource_string(__name__, 'js/src/graphical_slider_tool/general_methods.js'),
+            resource_string(__name__, 'js/src/graphical_slider_tool/sliders.js'),
+            resource_string(__name__, 'js/src/graphical_slider_tool/inputs.js'),
+            resource_string(__name__, 'js/src/graphical_slider_tool/graph.js'),
+            resource_string(__name__, 'js/src/graphical_slider_tool/el_output.js'),
+            resource_string(__name__, 'js/src/graphical_slider_tool/g_label_el_output.js'),
+            resource_string(__name__, 'js/src/graphical_slider_tool/gst.js')
+        ]
     }
+    css = {'scss': [resource_string(__name__, 'css/gst/display.scss')]}
     js_module_name = "GraphicalSliderTool"
 
+    @property
+    def configuration(self):
+        return stringify_children(
+            html.fromstring(self.data).xpath('configuration')[0]
+        )
+
+    @property
+    def render(self):
+        return stringify_children(
+            html.fromstring(self.data).xpath('render')[0]
+        )
+
     def get_html(self):
         """ Renders parameters to template. """
 
         # these 3 will be used in class methods
         self.html_id = self.location.html_id()
         self.html_class = self.location.category
+
         self.configuration_json = self.build_configuration_json()
         params = {
-                  'gst_html': self.substitute_controls(self.render),
-                  'element_id': self.html_id,
-                  'element_class': self.html_class,
-                  'configuration_json': self.configuration_json
-                  }
+            'gst_html': self.substitute_controls(self.render),
+            'element_id': self.html_id,
+            'element_class': self.html_class,
+            'configuration_json': self.configuration_json
+        }
         content = self.system.render_template(
-                        'graphical_slider_tool.html', params)
+            'graphical_slider_tool.html', params
+        )
         return content
 
     def substitute_controls(self, html_string):
@@ -126,9 +148,10 @@ class GraphicalSliderToolModule(GraphicalSliderToolFields, XModule):
         if plot_el:
             plot_el = plot_el[0]
             plot_el.getparent().replace(plot_el, html.fromstring(
-                                plot_div.format(element_class=self.html_class,
-                                               element_id=self.html_id,
-                                               style=plot_el.get('style', ""))))
+                plot_div.format(
+                    element_class=self.html_class,
+                    element_id=self.html_id,
+                    style=plot_el.get('style', ""))))
 
         # substitute sliders
         slider_div = '<div class="{element_class}_slider" \
@@ -139,10 +162,11 @@ class GraphicalSliderToolModule(GraphicalSliderToolFields, XModule):
         slider_els = xml.xpath('//slider')
         for slider_el in slider_els:
             slider_el.getparent().replace(slider_el, html.fromstring(
-                                slider_div.format(element_class=self.html_class,
-                                    element_id=self.html_id,
-                                    var=slider_el.get('var', ""),
-                                    style=slider_el.get('style', ""))))
+                slider_div.format(
+                    element_class=self.html_class,
+                    element_id=self.html_id,
+                    var=slider_el.get('var', ""),
+                    style=slider_el.get('style', ""))))
 
         # substitute inputs aka textboxes
         input_div = '<input class="{element_class}_input" \
@@ -151,11 +175,12 @@ class GraphicalSliderToolModule(GraphicalSliderToolFields, XModule):
         input_els = xml.xpath('//textbox')
         for input_index, input_el in enumerate(input_els):
             input_el.getparent().replace(input_el, html.fromstring(
-                                input_div.format(element_class=self.html_class,
-                                        element_id=self.html_id,
-                                        var=input_el.get('var', ""),
-                                        style=input_el.get('style', ""),
-                                        input_index=input_index)))
+                input_div.format(
+                    element_class=self.html_class,
+                    element_id=self.html_id,
+                    var=input_el.get('var', ""),
+                    style=input_el.get('style', ""),
+                    input_index=input_index)))
 
         return html.tostring(xml)
 
@@ -170,11 +195,13 @@ class GraphicalSliderToolModule(GraphicalSliderToolFields, XModule):
         """
         # <root> added for interface compatibility with xmltodict.parse
         # class added for javascript's part purposes
-        return json.dumps(xmltodict.parse('<root class="' + self.html_class +
-                '">' + self.configuration + '</root>'))
+        root = '<root class="{}">{}</root>'.format(
+            self.html_class,
+            self.configuration)
+        return json.dumps(xmltodict.parse(root))
 
 
-class GraphicalSliderToolDescriptor(GraphicalSliderToolFields, MakoModuleDescriptor, XmlDescriptor):
+class GraphicalSliderToolDescriptor(GraphicalSliderToolFields, XMLEditingDescriptor, XmlDescriptor):
     module_class = GraphicalSliderToolModule
 
     @classmethod
@@ -202,24 +229,14 @@ class GraphicalSliderToolDescriptor(GraphicalSliderToolFields, MakoModuleDescrip
                     exactly one '{0}' tag".format(child))
         # finished
 
-        def parse(k):
-            """Assumes that xml_object has child k"""
-            return stringify_children(xml_object.xpath(k)[0])
         return {
-                    'render': parse('render'),
-                    'configuration': parse('configuration')
-                }, []
+            'data': stringify_children(xml_object)
+        }, []
 
     def definition_to_xml(self, resource_fs):
         '''Return an xml element representing this definition.'''
-        xml_object = etree.Element('graphical_slider_tool')
-
-        def add_child(k):
-            child_str = '<{tag}>{body}</{tag}>'.format(tag=k, body=getattr(self, k))
-            child_node = etree.fromstring(child_str)
-            xml_object.append(child_node)
-
-        for child in ['render', 'configuration']:
-            add_child(child)
-
+        data = '<{tag}>{body}</{tag}>'.format(
+            tag='graphical_slider_tool',
+            body=self.data)
+        xml_object = etree.fromstring(data)
         return xml_object

common/lib/xmodule/xmodule/modulestore/locator.py

@@ -45,7 +45,7 @@ class Locator(object):
 
     def __repr__(self):
         '''
-        repr(self) returns something like this: CourseLocator("edu.mit.eecs.6002x")
+        repr(self) returns something like this: CourseLocator("mit.eecs.6002x")
         '''
         classname = self.__class__.__name__
         if classname.find('.') != -1:
@@ -54,13 +54,13 @@ class Locator(object):
 
     def __str__(self):
         '''
-        str(self) returns something like this: "edu.mit.eecs.6002x"
+        str(self) returns something like this: "mit.eecs.6002x"
         '''
         return unicode(self).encode('utf8')
 
     def __unicode__(self):
         '''
-        unicode(self) returns something like this: "edu.mit.eecs.6002x"
+        unicode(self) returns something like this: "mit.eecs.6002x"
         '''
         return self.url()
 
@@ -89,15 +89,15 @@ class CourseLocator(Locator):
     """
     Examples of valid CourseLocator specifications:
      CourseLocator(version_guid=ObjectId('519665f6223ebd6980884f2b'))
-     CourseLocator(course_id='edu.mit.eecs.6002x')
-     CourseLocator(course_id='edu.mit.eecs.6002x;published')
-     CourseLocator(course_id='edu.mit.eecs.6002x', revision='published')
+     CourseLocator(course_id='mit.eecs.6002x')
+     CourseLocator(course_id='mit.eecs.6002x;published')
+     CourseLocator(course_id='mit.eecs.6002x', branch='published')
      CourseLocator(url='edx://@519665f6223ebd6980884f2b')
-     CourseLocator(url='edx://edu.mit.eecs.6002x')
-     CourseLocator(url='edx://edu.mit.eecs.6002x;published')
+     CourseLocator(url='edx://mit.eecs.6002x')
+     CourseLocator(url='edx://mit.eecs.6002x;published')
 
     Should have at lease a specific course_id (id for the course as if it were a project w/
-    versions) with optional 'revision' (must be 'draft', 'published', or None),
+    versions) with optional 'branch',
     or version_guid (which points to a specific version). Can contain both in which case
     the persistence layer may raise exceptions if the given version != the current such version
     of the course.
@@ -106,7 +106,7 @@ class CourseLocator(Locator):
     # Default values
     version_guid = None
     course_id = None
-    revision = None
+    branch = None
 
     def __unicode__(self):
         """
@@ -114,8 +114,8 @@ class CourseLocator(Locator):
         """
         if self.course_id:
             result = self.course_id
-            if self.revision:
-                result += ';' + self.revision
+            if self.branch:
+                result += ';' + self.branch
             return result
         elif self.version_guid:
             return '@' + str(self.version_guid)
@@ -131,7 +131,7 @@ class CourseLocator(Locator):
 
     # -- unused args which are used via inspect
     # pylint: disable= W0613
-    def validate_args(self, url, version_guid, course_id, revision):
+    def validate_args(self, url, version_guid, course_id, branch):
         """
         Validate provided arguments.
         """
@@ -144,12 +144,12 @@ class CourseLocator(Locator):
 
     def is_fully_specified(self):
         """
-        Returns True if either version_guid is specified, or course_id+revision
+        Returns True if either version_guid is specified, or course_id+branch
         are specified.
         This should always return True, since this should be validated in the constructor.
         """
         return self.version_guid is not None \
-            or (self.course_id is not None and self.revision is not None)
+            or (self.course_id is not None and self.branch is not None)
 
     def set_course_id(self, new):
         """
@@ -158,12 +158,12 @@ class CourseLocator(Locator):
         """
         self.set_property('course_id', new)
 
-    def set_revision(self, new):
+    def set_branch(self, new):
         """
-        Initialize revision to new value.
-        If revision has already been initialized to a different value, raise an exception.
+        Initialize branch to new value.
+        If branch has already been initialized to a different value, raise an exception.
         """
-        self.set_property('revision', new)
+        self.set_property('branch', new)
 
     def set_version_guid(self, new):
         """
@@ -181,29 +181,29 @@ class CourseLocator(Locator):
         """
         return CourseLocator(course_id=self.course_id,
                              version_guid=self.version_guid,
-                             revision=self.revision)
+                             branch=self.branch)
 
-    def __init__(self, url=None, version_guid=None, course_id=None, revision=None):
+    def __init__(self, url=None, version_guid=None, course_id=None, branch=None):
         """
         Construct a CourseLocator
         Caller may provide url (but no other parameters).
         Caller may provide version_guid (but no other parameters).
-        Caller may provide course_id (optionally provide revision).
+        Caller may provide course_id (optionally provide branch).
 
         Resulting CourseLocator will have either a version_guid property
-        or a course_id (with optional revision) property, or both.
+        or a course_id (with optional branch) property, or both.
 
         version_guid must be an instance of bson.objectid.ObjectId or None
-        url, course_id, and revision must be strings or None
+        url, course_id, and branch must be strings or None
 
         """
-        self.validate_args(url, version_guid, course_id, revision)
+        self.validate_args(url, version_guid, course_id, branch)
         if url:
             self.init_from_url(url)
         if version_guid:
             self.init_from_version_guid(version_guid)
-        if course_id or revision:
-            self.init_from_course_id(course_id, revision)
+        if course_id or branch:
+            self.init_from_course_id(course_id, branch)
         assert self.version_guid or self.course_id, \
             "Either version_guid or course_id should be set."
 
@@ -223,7 +223,7 @@ class CourseLocator(Locator):
     def init_from_url(self, url):
         """
         url must be a string beginning with 'edx://' and containing
-        either a valid version_guid or course_id (with optional revision)
+        either a valid version_guid or course_id (with optional branch)
         If a block ('#HW3') is present, it is ignored.
         """
         if isinstance(url, Locator):
@@ -237,7 +237,7 @@ class CourseLocator(Locator):
             self.set_version_guid(self.as_object_id(new_guid))
         else:
             self.set_course_id(parse['id'])
-            self.set_revision(parse['revision'])
+            self.set_branch(parse['branch'])
 
     def init_from_version_guid(self, version_guid):
         """
@@ -251,14 +251,14 @@ class CourseLocator(Locator):
             '%s is not an instance of ObjectId' % version_guid
         self.set_version_guid(version_guid)
 
-    def init_from_course_id(self, course_id, explicit_revision=None):
+    def init_from_course_id(self, course_id, explicit_branch=None):
         """
-        Course_id is a string like 'edu.mit.eecs.6002x' or 'edu.mit.eecs.6002x;published'.
+        Course_id is a string like 'mit.eecs.6002x' or 'mit.eecs.6002x;published'.
 
         Revision (optional) is a string like 'published'.
-        It may be provided explicitly (explicit_revision) or embedded into course_id.
-        If revision is part of course_id ("...;published"), parse it out separately.
-        If revision is provided both ways, that's ok as long as they are the same value.
+        It may be provided explicitly (explicit_branch) or embedded into course_id.
+        If branch is part of course_id ("...;published"), parse it out separately.
+        If branch is provided both ways, that's ok as long as they are the same value.
 
         If a block ('#HW3') is a part of course_id, it is ignored.
 
@@ -272,11 +272,11 @@ class CourseLocator(Locator):
             parse = parse_course_id(course_id)
             assert parse, 'Could not parse "%s" as a course_id' % course_id
             self.set_course_id(parse['id'])
-            rev = parse['revision']
+            rev = parse['branch']
             if rev:
-                self.set_revision(rev)
-        if explicit_revision:
-            self.set_revision(explicit_revision)
+                self.set_branch(rev)
+        if explicit_branch:
+            self.set_branch(explicit_branch)
 
     def version(self):
         """
@@ -305,37 +305,37 @@ class BlockUsageLocator(CourseLocator):
     the defined element in the course. Courses can be a version of an offering, the
     current draft head, or the current production version.
 
-    Locators can contain both a version and a course_id w/ revision. The split mongo functions
-    may raise errors if these conflict w/ the current db state (i.e., the course's revision !=
+    Locators can contain both a version and a course_id w/ branch. The split mongo functions
+    may raise errors if these conflict w/ the current db state (i.e., the course's branch !=
     the version_guid)
 
     Locations can express as urls as well as dictionaries. They consist of
         course_identifier: course_guid | version_guid
         block : guid
-        revision : 'draft' | 'published' (optional)
+        branch : string
     """
 
     # Default value
     usage_id = None
 
     def __init__(self, url=None, version_guid=None, course_id=None,
-                 revision=None, usage_id=None):
+                 branch=None, usage_id=None):
         """
         Construct a BlockUsageLocator
-        Caller may provide url, version_guid, or course_id, and optionally provide revision.
+        Caller may provide url, version_guid, or course_id, and optionally provide branch.
 
         The usage_id may be specified, either explictly or as part of
         the url or course_id. If omitted, the locator is created but it
         has not yet been initialized.
 
         Resulting BlockUsageLocator will have a usage_id property.
-        It will have either a version_guid property or a course_id (with optional revision) property, or both.
+        It will have either a version_guid property or a course_id (with optional branch) property, or both.
 
         version_guid must be an instance of bson.objectid.ObjectId or None
-        url, course_id, revision, and usage_id must be strings or None
+        url, course_id, branch, and usage_id must be strings or None
 
         """
-        self.validate_args(url, version_guid, course_id, revision)
+        self.validate_args(url, version_guid, course_id, branch)
         if url:
             self.init_block_ref_from_url(url)
         if course_id:
@@ -346,7 +346,7 @@ class BlockUsageLocator(CourseLocator):
                                url=url,
                                version_guid=version_guid,
                                course_id=course_id,
-                               revision=revision)
+                               branch=branch)
 
     def is_initialized(self):
         """
@@ -366,11 +366,11 @@ class BlockUsageLocator(CourseLocator):
         """
         if self.course_id and self.version_guid:
             return BlockUsageLocator(version_guid=self.version_guid,
-                                     revision=self.revision,
+                                     branch=self.branch,
                                      usage_id=self.usage_id)
         else:
             return BlockUsageLocator(course_id=self.course_id,
-                                     revision=self.revision,
+                                     branch=self.branch,
                                      usage_id=self.usage_id)
 
     def set_usage_id(self, new):

common/lib/xmodule/xmodule/modulestore/parsers.py

@@ -20,7 +20,7 @@ def parse_url(string):
     with key 'version_guid' and the value,
 
     If it can be parsed as a course_id, returns a dict
-    with keys 'id' and 'revision' (value of 'revision' may be None),
+    with keys 'id' and 'branch' (value of 'branch' may be None),
 
     """
     match = URL_RE.match(string)
@@ -69,14 +69,14 @@ def parse_guid(string):
         return None
 
 
-COURSE_ID_RE = re.compile(r'^(?P<id>(\w+)(\.\w+\w*)*)(;(?P<revision>\w+))?(#(?P<block>\w+))?$', re.IGNORECASE)
+COURSE_ID_RE = re.compile(r'^(?P<id>(\w+)(\.\w+\w*)*)(;(?P<branch>\w+))?(#(?P<block>\w+))?$', re.IGNORECASE)
 
 
 def parse_course_id(string):
     r"""
 
     A course_id has a main id component.
-    There may also be an optional revision (;published or ;draft).
+    There may also be an optional branch (;published or ;draft).
     There may also be an optional block (#HW3 or #Quiz2).
 
     Examples of valid course_ids:
@@ -89,11 +89,11 @@ def parse_course_id(string):
 
     Syntax:
 
-      course_id = main_id [; revision] [# block]
+      course_id = main_id [; branch] [# block]
 
       main_id = name [. name]*
 
-      revision = name
+      branch = name
 
       block = name
 
@@ -104,8 +104,8 @@ def parse_course_id(string):
     and the underscore. (see definition of \w in python regular expressions,
     at http://docs.python.org/dev/library/re.html)
 
-    If string is a course_id, returns a dict with keys 'id', 'revision', and 'block'.
-    Revision is optional: if missing returned_dict['revision'] is None.
+    If string is a course_id, returns a dict with keys 'id', 'branch', and 'block'.
+    Revision is optional: if missing returned_dict['branch'] is None.
     Block is optional: if missing returned_dict['block'] is None.
     Else returns None.
     """

common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py

@@ -81,7 +81,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
             version_guid=course_entry_override['_id'],
             usage_id=usage_id,
             course_id=course_entry_override.get('course_id'),
-            revision=course_entry_override.get('revision')
+            branch=course_entry_override.get('branch')
         )
 
         kvs = SplitMongoKVS(

common/lib/xmodule/xmodule/modulestore/split_mongo/split.py

@@ -186,8 +186,8 @@ class SplitMongoModuleStore(ModuleStoreBase):
         return the CourseDescriptor! It returns the actual db json from
         structures.
 
-        Semantics: if course_id and revision given, then it will get that revision. If
-        also give a version_guid, it will see if the current head of that revision == that guid. If not
+        Semantics: if course_id and branch given, then it will get that branch. If
+        also give a version_guid, it will see if the current head of that branch == that guid. If not
         it raises VersionConflictError (the version now differs from what it was when you got your
         reference)
 
@@ -198,19 +198,19 @@ class SplitMongoModuleStore(ModuleStoreBase):
         if not course_locator.is_fully_specified():
             raise InsufficientSpecificationError('Not fully specified: %s' % course_locator)
 
-        if course_locator.course_id is not None and course_locator.revision is not None:
+        if course_locator.course_id is not None and course_locator.branch is not None:
             # use the course_id
             index = self.course_index.find_one({'_id': course_locator.course_id})
             if index is None:
                 raise ItemNotFoundError(course_locator)
-            if course_locator.revision not in index['versions']:
+            if course_locator.branch not in index['versions']:
                 raise ItemNotFoundError(course_locator)
-            version_guid = index['versions'][course_locator.revision]
+            version_guid = index['versions'][course_locator.branch]
             if course_locator.version_guid is not None and version_guid != course_locator.version_guid:
                 # This may be a bit too touchy but it's hard to infer intent
                 raise VersionConflictError(course_locator, CourseLocator(course_locator, version_guid=version_guid))
         else:
-            # TODO should this raise an exception if revision was provided?
+            # TODO should this raise an exception if branch was provided?
             version_guid = course_locator.version_guid
 
         # cast string to ObjectId if necessary
@@ -223,29 +223,29 @@ class SplitMongoModuleStore(ModuleStoreBase):
 
         if course_locator.course_id:
             entry['course_id'] = course_locator.course_id
-            entry['revision'] = course_locator.revision
+            entry['branch'] = course_locator.branch
         return entry
 
-    def get_courses(self, revision, qualifiers=None):
+    def get_courses(self, branch, qualifiers=None):
         '''
         Returns a list of course descriptors matching any given qualifiers.
 
         qualifiers should be a dict of keywords matching the db fields or any
         legal query for mongo to use against the active_versions collection.
 
-        Note, this is to find the current head of the named revision type
+        Note, this is to find the current head of the named branch type
         (e.g., 'draft'). To get specific versions via guid use get_course.
         '''
         if qualifiers is None:
             qualifiers = {}
-        qualifiers.update({"versions.{}".format(revision): {"$exists": True}})
+        qualifiers.update({"versions.{}".format(branch): {"$exists": True}})
         matching = self.course_index.find(qualifiers)
 
         # collect ids and then query for those
         version_guids = []
         id_version_map = {}
         for course_entry in matching:
-            version_guid = course_entry['versions'][revision]
+            version_guid = course_entry['versions'][branch]
             version_guids.append(version_guid)
             id_version_map[version_guid] = course_entry['_id']
 
@@ -667,7 +667,7 @@ class SplitMongoModuleStore(ModuleStoreBase):
 
         # update the index entry if appropriate
         if index_entry is not None:
-            self._update_head(index_entry, course_or_parent_locator.revision, new_id)
+            self._update_head(index_entry, course_or_parent_locator.branch, new_id)
             course_parent = course_or_parent_locator.as_course_locator()
         else:
             course_parent = None
@@ -786,7 +786,7 @@ class SplitMongoModuleStore(ModuleStoreBase):
             'edited_on': datetime.datetime.utcnow(),
             'versions': versions_dict}
         new_id = self.course_index.insert(index_entry)
-        return self.get_course(CourseLocator(course_id=new_id, revision=master_version))
+        return self.get_course(CourseLocator(course_id=new_id, branch=master_version))
 
     def update_item(self, descriptor, user_id, force=False):
         """
@@ -835,7 +835,7 @@ class SplitMongoModuleStore(ModuleStoreBase):
 
             # update the index entry if appropriate
             if index_entry is not None:
-                self._update_head(index_entry, descriptor.location.revision, new_id)
+                self._update_head(index_entry, descriptor.location.branch, new_id)
 
             # fetch and return the new item--fetching is unnecessary but a good qc step
             return self.get_item(BlockUsageLocator(descriptor.location, version_guid=new_id))
@@ -876,7 +876,7 @@ class SplitMongoModuleStore(ModuleStoreBase):
 
             # update the index entry if appropriate
             if index_entry is not None:
-                self._update_head(index_entry, xblock.location.revision, new_id)
+                self._update_head(index_entry, xblock.location.branch, new_id)
 
             # fetch and return the new item--fetching is unnecessary but a good qc step
             return self.get_item(BlockUsageLocator(xblock.location, version_guid=new_id))
@@ -1028,9 +1028,9 @@ class SplitMongoModuleStore(ModuleStoreBase):
 
         # update the index entry if appropriate
         if index_entry is not None:
-            self._update_head(index_entry, usage_locator.revision, new_id)
+            self._update_head(index_entry, usage_locator.branch, new_id)
             result.course_id = usage_locator.course_id
-            result.revision = usage_locator.revision
+            result.branch = usage_locator.branch
 
         return result
 
@@ -1186,19 +1186,19 @@ class SplitMongoModuleStore(ModuleStoreBase):
 
         :param locator:
         """
-        if locator.course_id is None or locator.revision is None:
+        if locator.course_id is None or locator.branch is None:
             return None
         else:
             index_entry = self.course_index.find_one({'_id': locator.course_id})
             if (locator.version_guid is not None
-                and index_entry['versions'][locator.revision] != locator.version_guid
+                and index_entry['versions'][locator.branch] != locator.version_guid
                 and not force):
                 raise VersionConflictError(
                     locator,
                     CourseLocator(
                         course_id=index_entry['_id'],
-                        version_guid=index_entry['versions'][locator.revision],
-                        revision=locator.revision))
+                        version_guid=index_entry['versions'][locator.branch],
+                        branch=locator.branch))
             else:
                 return index_entry
 
@@ -1227,9 +1227,9 @@ class SplitMongoModuleStore(ModuleStoreBase):
         return False
 
 
-    def _update_head(self, index_entry, revision, new_id):
+    def _update_head(self, index_entry, branch, new_id):
         """
-        Update the active index for the given course's revision to point to new_id
+        Update the active index for the given course's branch to point to new_id
 
         :param index_entry:
         :param course_locator:
@@ -1237,4 +1237,4 @@ class SplitMongoModuleStore(ModuleStoreBase):
         """
         self.course_index.update(
             {"_id": index_entry["_id"]},
-            {"$set": {"versions.{}".format(revision): new_id}})
+            {"$set": {"versions.{}".format(branch): new_id}})

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

@@ -4,12 +4,10 @@ Created on Mar 14, 2013
 @author: dmitchell
 '''
 from unittest import TestCase
-from nose.plugins.skip import SkipTest
 
 from bson.objectid import ObjectId
 from xmodule.modulestore.locator import Locator, CourseLocator, BlockUsageLocator
-from xmodule.modulestore.exceptions import InvalidLocationError, \
-    InsufficientSpecificationError, OverSpecificationError
+from xmodule.modulestore.exceptions import InsufficientSpecificationError, OverSpecificationError
 
 
 class LocatorTest(TestCase):
@@ -21,30 +19,30 @@ class LocatorTest(TestCase):
         self.assertRaises(
             OverSpecificationError,
             CourseLocator,
-            url='edx://edu.mit.eecs.6002x',
-            course_id='edu.harvard.history',
-            revision='published',
+            url='edx://mit.eecs.6002x',
+            course_id='harvard.history',
+            branch='published',
             version_guid=ObjectId())
         self.assertRaises(
             OverSpecificationError,
             CourseLocator,
-            url='edx://edu.mit.eecs.6002x',
-            course_id='edu.harvard.history',
+            url='edx://mit.eecs.6002x',
+            course_id='harvard.history',
             version_guid=ObjectId())
         self.assertRaises(
             OverSpecificationError,
             CourseLocator,
-            url='edx://edu.mit.eecs.6002x;published',
-            revision='draft')
+            url='edx://mit.eecs.6002x;published',
+            branch='draft')
         self.assertRaises(
             OverSpecificationError,
             CourseLocator,
-            course_id='edu.mit.eecs.6002x;published',
-            revision='draft')
+            course_id='mit.eecs.6002x;published',
+            branch='draft')
 
     def test_course_constructor_underspecified(self):
         self.assertRaises(InsufficientSpecificationError, CourseLocator)
-        self.assertRaises(InsufficientSpecificationError, CourseLocator, revision='published')
+        self.assertRaises(InsufficientSpecificationError, CourseLocator, branch='published')
 
     def test_course_constructor_bad_version_guid(self):
         self.assertRaises(ValueError, CourseLocator, version_guid="012345")
@@ -73,467 +71,128 @@ class LocatorTest(TestCase):
         """
         Test all sorts of badly-formed course_ids (and urls with those course_ids)
         """
-        for bad_id in ('edu.mit.',
-                       ' edu.mit.eecs',
-                       'edu.mit.eecs ',
-                       '@edu.mit.eecs',
-                       '#edu.mit.eecs',
-                       'edu.mit.ee cs',
-                       'edu.mit.ee,cs',
-                       'edu.mit.ee/cs',
-                       'edu.mit.ee$cs',
-                       'edu.mit.ee&cs',
-                       'edu.mit.ee()cs',
+        for bad_id in ('mit.',
+                       ' mit.eecs',
+                       'mit.eecs ',
+                       '@mit.eecs',
+                       '#mit.eecs',
+                       'mit.ee cs',
+                       'mit.ee,cs',
+                       'mit.ee/cs',
+                       'mit.ee$cs',
+                       'mit.ee&cs',
+                       'mit.ee()cs',
                        ';this',
-                       'edu.mit.eecs;',
-                       'edu.mit.eecs;this;that',
-                       'edu.mit.eecs;this;',
-                       'edu.mit.eecs;this ',
-                       'edu.mit.eecs;th%is ',
+                       'mit.eecs;',
+                       'mit.eecs;this;that',
+                       'mit.eecs;this;',
+                       'mit.eecs;this ',
+                       'mit.eecs;th%is ',
                        ):
             self.assertRaises(AssertionError, CourseLocator, course_id=bad_id)
             self.assertRaises(AssertionError, CourseLocator, url='edx://' + bad_id)
 
     def test_course_constructor_bad_url(self):
         for bad_url in ('edx://',
-                        'edx:/edu.mit.eecs',
-                        'http://edu.mit.eecs',
-                        'edu.mit.eecs',
-                        'edx//edu.mit.eecs'):
+                        'edx:/mit.eecs',
+                        'http://mit.eecs',
+                        'mit.eecs',
+                        'edx//mit.eecs'):
             self.assertRaises(AssertionError, CourseLocator, url=bad_url)
 
     def test_course_constructor_redundant_001(self):
-        testurn = 'edu.mit.eecs.6002x'
+        testurn = 'mit.eecs.6002x'
         testobj = CourseLocator(course_id=testurn, url='edx://' + testurn)
         self.check_course_locn_fields(testobj, 'course_id', course_id=testurn)
 
     def test_course_constructor_redundant_002(self):
-        testurn = 'edu.mit.eecs.6002x;published'
-        expected_urn = 'edu.mit.eecs.6002x'
+        testurn = 'mit.eecs.6002x;published'
+        expected_urn = 'mit.eecs.6002x'
         expected_rev = 'published'
         testobj = CourseLocator(course_id=testurn, url='edx://' + testurn)
         self.check_course_locn_fields(testobj, 'course_id',
                                       course_id=expected_urn,
-                                      revision=expected_rev)
+                                      branch=expected_rev)
 
-    def test_course_constructor_course_id_no_revision(self):
-        testurn = 'edu.mit.eecs.6002x'
+    def test_course_constructor_course_id_no_branch(self):
+        testurn = 'mit.eecs.6002x'
         testobj = CourseLocator(course_id=testurn)
         self.check_course_locn_fields(testobj, 'course_id', course_id=testurn)
         self.assertEqual(testobj.course_id, testurn)
         self.assertEqual(str(testobj), testurn)
         self.assertEqual(testobj.url(), 'edx://' + testurn)
 
-    def test_course_constructor_course_id_with_revision(self):
-        testurn = 'edu.mit.eecs.6002x;published'
-        expected_id = 'edu.mit.eecs.6002x'
-        expected_revision = 'published'
+    def test_course_constructor_course_id_with_branch(self):
+        testurn = 'mit.eecs.6002x;published'
+        expected_id = 'mit.eecs.6002x'
+        expected_branch = 'published'
         testobj = CourseLocator(course_id=testurn)
-        self.check_course_locn_fields(testobj, 'course_id with revision',
+        self.check_course_locn_fields(testobj, 'course_id with branch',
                                       course_id=expected_id,
-                                      revision=expected_revision,
+                                      branch=expected_branch,
                                       )
         self.assertEqual(testobj.course_id, expected_id)
-        self.assertEqual(testobj.revision, expected_revision)
+        self.assertEqual(testobj.branch, expected_branch)
         self.assertEqual(str(testobj), testurn)
         self.assertEqual(testobj.url(), 'edx://' + testurn)
 
-    def test_course_constructor_course_id_separate_revision(self):
-        test_id = 'edu.mit.eecs.6002x'
-        test_revision = 'published'
-        expected_urn = 'edu.mit.eecs.6002x;published'
-        testobj = CourseLocator(course_id=test_id, revision=test_revision)
-        self.check_course_locn_fields(testobj, 'course_id with separate revision',
+    def test_course_constructor_course_id_separate_branch(self):
+        test_id = 'mit.eecs.6002x'
+        test_branch = 'published'
+        expected_urn = 'mit.eecs.6002x;published'
+        testobj = CourseLocator(course_id=test_id, branch=test_branch)
+        self.check_course_locn_fields(testobj, 'course_id with separate branch',
                                       course_id=test_id,
-                                      revision=test_revision,
+                                      branch=test_branch,
                                       )
         self.assertEqual(testobj.course_id, test_id)
-        self.assertEqual(testobj.revision, test_revision)
+        self.assertEqual(testobj.branch, test_branch)
         self.assertEqual(str(testobj), expected_urn)
         self.assertEqual(testobj.url(), 'edx://' + expected_urn)
 
-    def test_course_constructor_course_id_repeated_revision(self):
+    def test_course_constructor_course_id_repeated_branch(self):
         """
-        The same revision appears in the course_id and the revision field.
+        The same branch appears in the course_id and the branch field.
         """
-        test_id = 'edu.mit.eecs.6002x;published'
-        test_revision = 'published'
-        expected_id = 'edu.mit.eecs.6002x'
-        expected_urn = 'edu.mit.eecs.6002x;published'
-        testobj = CourseLocator(course_id=test_id, revision=test_revision)
-        self.check_course_locn_fields(testobj, 'course_id with repeated revision',
+        test_id = 'mit.eecs.6002x;published'
+        test_branch = 'published'
+        expected_id = 'mit.eecs.6002x'
+        expected_urn = 'mit.eecs.6002x;published'
+        testobj = CourseLocator(course_id=test_id, branch=test_branch)
+        self.check_course_locn_fields(testobj, 'course_id with repeated branch',
                                       course_id=expected_id,
-                                      revision=test_revision,
+                                      branch=test_branch,
                                       )
         self.assertEqual(testobj.course_id, expected_id)
-        self.assertEqual(testobj.revision, test_revision)
+        self.assertEqual(testobj.branch, test_branch)
         self.assertEqual(str(testobj), expected_urn)
         self.assertEqual(testobj.url(), 'edx://' + expected_urn)
 
     def test_block_constructor(self):
-        testurn = 'edu.mit.eecs.6002x;published#HW3'
-        expected_id = 'edu.mit.eecs.6002x'
-        expected_revision = 'published'
+        testurn = 'mit.eecs.6002x;published#HW3'
+        expected_id = 'mit.eecs.6002x'
+        expected_branch = 'published'
         expected_block_ref = 'HW3'
         testobj = BlockUsageLocator(course_id=testurn)
         self.check_block_locn_fields(testobj, 'test_block constructor',
                                      course_id=expected_id,
-                                     revision=expected_revision,
+                                     branch=expected_branch,
                                      block=expected_block_ref)
         self.assertEqual(str(testobj), testurn)
         self.assertEqual(testobj.url(), 'edx://' + testurn)
 
-    # ------------------------------------------------------------
-    # Disabled tests
-
-    def test_course_urls(self):
-        '''
-        Test constructor and property accessors.
-        '''
-        raise SkipTest()
-        self.assertRaises(TypeError, CourseLocator, 'empty constructor')
-
-        # url inits
-        testurn = 'edx://org/course/category/name'
-        self.assertRaises(InvalidLocationError, CourseLocator, url=testurn)
-        testurn = 'unknown/versionid/blockid'
-        self.assertRaises(InvalidLocationError, CourseLocator, url=testurn)
-
-        testurn = 'cvx/versionid'
-        testobj = CourseLocator(testurn)
-        self.check_course_locn_fields(testobj, testurn, 'versionid')
-        self.assertEqual(testobj, CourseLocator(testobj),
-                         'initialization from another instance')
-
-        testurn = 'cvx/versionid/'
-        testobj = CourseLocator(testurn)
-        self.check_course_locn_fields(testobj, testurn, 'versionid')
-
-        testurn = 'cvx/versionid/blockid'
-        testobj = CourseLocator(testurn)
-        self.check_course_locn_fields(testobj, testurn, 'versionid')
-
-        testurn = 'cvx/versionid/blockid/extraneousstuff?including=args'
-        testobj = CourseLocator(testurn)
-        self.check_course_locn_fields(testobj, testurn, 'versionid')
-
-        testurn = 'cvx://versionid/blockid'
-        testobj = CourseLocator(testurn)
-        self.check_course_locn_fields(testobj, testurn, 'versionid')
-
-        testurn = 'crx/courseid/blockid'
-        testobj = CourseLocator(testurn)
-        self.check_course_locn_fields(testobj, testurn, course_id='courseid')
-
-        testurn = 'crx/courseid@revision/blockid'
-        testobj = CourseLocator(testurn)
-        self.check_course_locn_fields(testobj, testurn, course_id='courseid',
-                                      revision='revision')
-        self.assertEqual(testobj, CourseLocator(testobj),
-                         'run initialization from another instance')
-
-    def test_course_keyword_setters(self):
-        raise SkipTest()
-        # arg list inits
-        testobj = CourseLocator(version_guid='versionid')
-        self.check_course_locn_fields(testobj, 'versionid arg', 'versionid')
-
-        testobj = CourseLocator(course_id='courseid')
-        self.check_course_locn_fields(testobj, 'courseid arg',
-                                      course_id='courseid')
-
-        testobj = CourseLocator(course_id='courseid', revision='rev')
-        self.check_course_locn_fields(testobj, 'rev arg',
-                                      course_id='courseid',
-                                      revision='rev')
-        # ignores garbage
-        testobj = CourseLocator(course_id='courseid', revision='rev',
-                                potato='spud')
-        self.check_course_locn_fields(testobj, 'extra keyword arg',
-                                      course_id='courseid',
-                                      revision='rev')
-
-        # url w/ keyword override
-        testurn = 'crx/courseid@revision/blockid'
-        testobj = CourseLocator(testurn, revision='rev')
-        self.check_course_locn_fields(testobj, 'rev override',
-                                      course_id='courseid',
-                                      revision='rev')
-
-    def test_course_dict(self):
-        raise SkipTest()
-        # dict init w/ keyword overwrites
-        testobj = CourseLocator({"version_guid": 'versionid'})
-        self.check_course_locn_fields(testobj, 'versionid dict', 'versionid')
-
-        testobj = CourseLocator({"course_id": 'courseid'})
-        self.check_course_locn_fields(testobj, 'courseid dict',
-                                      course_id='courseid')
-
-        testobj = CourseLocator({"course_id": 'courseid', "revision": 'rev'})
-        self.check_course_locn_fields(testobj, 'rev dict',
-                                      course_id='courseid',
-                                      revision='rev')
-        # ignores garbage
-        testobj = CourseLocator({"course_id": 'courseid', "revision": 'rev',
-                                 "potato": 'spud'})
-        self.check_course_locn_fields(testobj, 'extra keyword dict',
-                                      course_id='courseid',
-                                      revision='rev')
-        testobj = CourseLocator({"course_id": 'courseid', "revision": 'rev'},
-                                revision='alt')
-        self.check_course_locn_fields(testobj, 'rev dict',
-                                      course_id='courseid',
-                                      revision='alt')
-
-        # urn init w/ dict & keyword overwrites
-        testobj = CourseLocator('crx/notcourse@notthis',
-                                {"course_id": 'courseid'},
-                                revision='alt')
-        self.check_course_locn_fields(testobj, 'rev dict',
-                                      course_id='courseid',
-                                      revision='alt')
-
-    def test_url(self):
-        '''
-        Ensure CourseLocator generates expected urls.
-        '''
-        raise SkipTest()
-
-        testobj = CourseLocator(version_guid='versionid')
-        self.assertEqual(testobj.url(), 'cvx/versionid', 'versionid')
-        self.assertEqual(testobj, CourseLocator(testobj.url()),
-                         'versionid conversion through url')
-
-        testobj = CourseLocator(course_id='courseid')
-        self.assertEqual(testobj.url(), 'crx/courseid', 'courseid')
-        self.assertEqual(testobj, CourseLocator(testobj.url()),
-                         'courseid conversion through url')
-
-        testobj = CourseLocator(course_id='courseid', revision='rev')
-        self.assertEqual(testobj.url(), 'crx/courseid@rev', 'rev')
-        self.assertEqual(testobj, CourseLocator(testobj.url()),
-                         'rev conversion through url')
-
-    def test_html(self):
-        '''
-        Ensure CourseLocator generates expected urls.
-        '''
-        raise SkipTest()
-        testobj = CourseLocator(version_guid='versionid')
-        self.assertEqual(testobj.html_id(), 'cvx/versionid', 'versionid')
-        self.assertEqual(testobj, CourseLocator(testobj.html_id()),
-                         'versionid conversion through html_id')
-
-        testobj = CourseLocator(course_id='courseid')
-        self.assertEqual(testobj.html_id(), 'crx/courseid', 'courseid')
-        self.assertEqual(testobj, CourseLocator(testobj.html_id()),
-                         'courseid conversion through html_id')
-
-        testobj = CourseLocator(course_id='courseid', revision='rev')
-        self.assertEqual(testobj.html_id(), 'crx/courseid%40rev', 'rev')
-        self.assertEqual(testobj, CourseLocator(testobj.html_id()),
-                         'rev conversion through html_id')
-
-    def test_block_locator(self):
-        '''
-        Test constructor and property accessors.
-        '''
-        raise SkipTest()
-        self.assertIsInstance(BlockUsageLocator(), BlockUsageLocator,
-                              'empty constructor')
-
-        # url inits
-        testurn = 'edx://org/course/category/name'
-        self.assertRaises(InvalidLocationError, BlockUsageLocator, testurn)
-        testurn = 'unknown/versionid/blockid'
-        self.assertRaises(InvalidLocationError, BlockUsageLocator, testurn)
-
-        testurn = 'cvx/versionid'
-        testobj = BlockUsageLocator(testurn)
-        self.check_block_locn_fields(testobj, testurn, 'versionid')
-        self.assertEqual(testobj, BlockUsageLocator(testobj),
-                         'initialization from another instance')
-
-        testurn = 'cvx/versionid/'
-        testobj = BlockUsageLocator(testurn)
-        self.check_block_locn_fields(testobj, testurn, 'versionid')
-
-        testurn = 'cvx/versionid/blockid'
-        testobj = BlockUsageLocator(testurn)
-        self.check_block_locn_fields(testobj, testurn, 'versionid',
-                                     block='blockid')
-
-        testurn = 'cvx/versionid/blockid/extraneousstuff?including=args'
-        testobj = BlockUsageLocator(testurn)
-        self.check_block_locn_fields(testobj, testurn, 'versionid',
-                                     block='blockid')
-
-        testurn = 'cvx://versionid/blockid'
-        testobj = BlockUsageLocator(testurn)
-        self.check_block_locn_fields(testobj, testurn, 'versionid',
-                                     block='blockid')
-
-        testurn = 'crx/courseid/blockid'
-        testobj = BlockUsageLocator(testurn)
-        self.check_block_locn_fields(testobj, testurn, course_id='courseid',
-                                     block='blockid')
-
-        testurn = 'crx/courseid@revision/blockid'
-        testobj = BlockUsageLocator(testurn)
-        self.check_block_locn_fields(testobj, testurn, course_id='courseid',
-                                     revision='revision', block='blockid')
-        self.assertEqual(testobj, BlockUsageLocator(testobj),
-                         'run initialization from another instance')
-
-    def test_block_keyword_init(self):
-        # arg list inits
-        raise SkipTest()
-        testobj = BlockUsageLocator(version_guid='versionid')
-        self.check_block_locn_fields(testobj, 'versionid arg', 'versionid')
-
-        testobj = BlockUsageLocator(version_guid='versionid', usage_id='myblock')
-        self.check_block_locn_fields(testobj, 'versionid arg', 'versionid',
-                                     block='myblock')
-
-        testobj = BlockUsageLocator(course_id='courseid')
-        self.check_block_locn_fields(testobj, 'courseid arg',
-                                     course_id='courseid')
-
-        testobj = BlockUsageLocator(course_id='courseid', revision='rev')
-        self.check_block_locn_fields(testobj, 'rev arg',
-                                     course_id='courseid',
-                                     revision='rev')
-        # ignores garbage
-        testobj = BlockUsageLocator(course_id='courseid', revision='rev',
-                                    usage_id='this_block', potato='spud')
-        self.check_block_locn_fields(testobj, 'extra keyword arg',
-                                     course_id='courseid', block='this_block', revision='rev')
-
-        # url w/ keyword override
-        testurn = 'crx/courseid@revision/blockid'
-        testobj = BlockUsageLocator(testurn, revision='rev')
-        self.check_block_locn_fields(testobj, 'rev override',
-                                     course_id='courseid', block='blockid',
-                                     revision='rev')
-
-    def test_block_keywords(self):
-        # dict init w/ keyword overwrites
-        raise SkipTest()
-        testobj = BlockUsageLocator({"version_guid": 'versionid',
-                                     'usage_id': 'dictblock'})
-        self.check_block_locn_fields(testobj, 'versionid dict', 'versionid',
-                                     block='dictblock')
-
-        testobj = BlockUsageLocator({"course_id": 'courseid',
-                                     'usage_id': 'dictblock'})
-        self.check_block_locn_fields(testobj, 'courseid dict',
-                                     block='dictblock', course_id='courseid')
-
-        testobj = BlockUsageLocator({"course_id": 'courseid', "revision": 'rev',
-                                     'usage_id': 'dictblock'})
-        self.check_block_locn_fields(testobj, 'rev dict',
-                                     course_id='courseid', block='dictblock',
-                                     revision='rev')
-        # ignores garbage
-        testobj = BlockUsageLocator({"course_id": 'courseid', "revision": 'rev',
-                                     'usage_id': 'dictblock', "potato": 'spud'})
-        self.check_block_locn_fields(testobj, 'extra keyword dict',
-                                     course_id='courseid', block='dictblock',
-                                     revision='rev')
-        testobj = BlockUsageLocator({"course_id": 'courseid', "revision": 'rev',
-                                     'usage_id': 'dictblock'}, revision='alt', usage_id='anotherblock')
-        self.check_block_locn_fields(testobj, 'rev dict',
-                                     course_id='courseid', block='anotherblock',
-                                     revision='alt')
-
-        # urn init w/ dict & keyword overwrites
-        testobj = BlockUsageLocator('crx/notcourse@notthis/northis',
-                                    {"course_id": 'courseid'}, revision='alt', usage_id='anotherblock')
-        self.check_block_locn_fields(testobj, 'rev dict',
-                                     course_id='courseid', block='anotherblock',
-                                     revision='alt')
-
-    def test_ensure_fully_specd(self):
-        '''
-        Test constructor and property accessors.
-        '''
-        raise SkipTest()
-        self.assertRaises(InsufficientSpecificationError,
-                          BlockUsageLocator.ensure_fully_specified, BlockUsageLocator())
-
-        # url inits
-        testurn = 'edx://org/course/category/name'
-        self.assertRaises(InvalidLocationError,
-                          BlockUsageLocator.ensure_fully_specified, testurn)
-        testurn = 'unknown/versionid/blockid'
-        self.assertRaises(InvalidLocationError,
-                          BlockUsageLocator.ensure_fully_specified, testurn)
-
-        testurn = 'cvx/versionid'
-        self.assertRaises(InsufficientSpecificationError,
-                          BlockUsageLocator.ensure_fully_specified, testurn)
-
-        testurn = 'cvx/versionid/'
-        self.assertRaises(InsufficientSpecificationError,
-                          BlockUsageLocator.ensure_fully_specified, testurn)
-
-        testurn = 'cvx/versionid/blockid'
-        self.assertIsInstance(BlockUsageLocator.ensure_fully_specified(testurn),
-                              BlockUsageLocator, testurn)
-
-        testurn = 'cvx/versionid/blockid/extraneousstuff?including=args'
-        self.assertIsInstance(BlockUsageLocator.ensure_fully_specified(testurn),
-                              BlockUsageLocator, testurn)
-
-        testurn = 'cvx://versionid/blockid'
-        self.assertIsInstance(BlockUsageLocator.ensure_fully_specified(testurn),
-                              BlockUsageLocator, testurn)
-
-        testurn = 'crx/courseid/blockid'
-        self.assertIsInstance(BlockUsageLocator.ensure_fully_specified(testurn),
-                              BlockUsageLocator, testurn)
-
-        testurn = 'crx/courseid@revision/blockid'
-        self.assertIsInstance(BlockUsageLocator.ensure_fully_specified(testurn),
-                              BlockUsageLocator, testurn)
-
-    def test_ensure_fully_via_keyword(self):
-        # arg list inits
-        raise SkipTest()
-        testobj = BlockUsageLocator(version_guid='versionid')
-        self.assertRaises(InsufficientSpecificationError,
-                          BlockUsageLocator.ensure_fully_specified, testobj)
-
-        testurn = 'crx/courseid@revision/blockid'
-        testobj = BlockUsageLocator(version_guid='versionid', usage_id='myblock')
-        self.assertIsInstance(BlockUsageLocator.ensure_fully_specified(testurn),
-                              BlockUsageLocator, testurn)
-
-        testobj = BlockUsageLocator(course_id='courseid')
-        self.assertRaises(InsufficientSpecificationError,
-                          BlockUsageLocator.ensure_fully_specified, testobj)
-
-        testobj = BlockUsageLocator(course_id='courseid', revision='rev')
-        self.assertRaises(InsufficientSpecificationError,
-                          BlockUsageLocator.ensure_fully_specified, testobj)
-
-        testobj = BlockUsageLocator(course_id='courseid', revision='rev',
-                                    usage_id='this_block')
-        self.assertIsInstance(BlockUsageLocator.ensure_fully_specified(testurn),
-                              BlockUsageLocator, testurn)
 
     # ------------------------------------------------------------------
     # Utilities
 
     def check_course_locn_fields(self, testobj, msg, version_guid=None,
-                                 course_id=None, revision=None):
+                                 course_id=None, branch=None):
         self.assertEqual(testobj.version_guid, version_guid, msg)
         self.assertEqual(testobj.course_id, course_id, msg)
-        self.assertEqual(testobj.revision, revision, msg)
+        self.assertEqual(testobj.branch, branch, msg)
 
     def check_block_locn_fields(self, testobj, msg, version_guid=None,
-                                course_id=None, revision=None, block=None):
+                                course_id=None, branch=None, block=None):
         self.check_course_locn_fields(testobj, msg, version_guid, course_id,
-                                      revision)
+                                      branch)
         self.assertEqual(testobj.usage_id, block)

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

@@ -69,10 +69,17 @@ class SplitModuleTest(unittest.TestCase):
                 collection_prefix + collection, '--jsonArray',
                 '--file',
                 SplitModuleTest.COMMON_ROOT + '/test/data/splitmongo_json/' + collection + '.json'
-            ])
+                ],
+                stdout=subprocess.PIPE,
+                stderr=subprocess.PIPE,
+            )
             for collection in ('active_versions', 'structures', 'definitions')]
         for p in processes:
-            if p.wait() != 0:
+            stdout, stderr = p.communicate()
+            if p.returncode != 0:
+                print "Couldn't run mongoimport:"
+                print stdout
+                print stderr
                 raise Exception("DB did not init correctly")
 
     @classmethod
@@ -129,8 +136,8 @@ class SplitModuleCourseTests(SplitModuleTest):
         self.assertEqual(str(course.previous_version), self.GUID_D1)
         self.assertDictEqual(course.grade_cutoffs, {"Pass": 0.45})
 
-    def test_revision_requests(self):
-        # query w/ revision qualifier (both draft and published)
+    def test_branch_requests(self):
+        # query w/ branch qualifier (both draft and published)
         courses_published = modulestore().get_courses('published')
         self.assertEqual(len(courses_published), 1, len(courses_published))
         course = self.findByIdInResult(courses_published, "head23456")
@@ -182,7 +189,7 @@ class SplitModuleCourseTests(SplitModuleTest):
         self.assertEqual(course.edited_by, "testassist@edx.org")
         self.assertDictEqual(course.grade_cutoffs, {"Pass": 0.55})
 
-        locator = CourseLocator(course_id='GreekHero', revision='draft')
+        locator = CourseLocator(course_id='GreekHero', branch='draft')
         course = modulestore().get_course(locator)
         self.assertEqual(course.location.course_id, "GreekHero")
         self.assertEqual(str(course.location.version_guid), self.GUID_D0)
@@ -195,12 +202,12 @@ class SplitModuleCourseTests(SplitModuleTest):
         self.assertEqual(course.edited_by, "testassist@edx.org")
         self.assertDictEqual(course.grade_cutoffs, {"Pass": 0.45})
 
-        locator = CourseLocator(course_id='wonderful', revision='published')
+        locator = CourseLocator(course_id='wonderful', branch='published')
         course = modulestore().get_course(locator)
         self.assertEqual(course.location.course_id, "wonderful")
         self.assertEqual(str(course.location.version_guid), self.GUID_P)
 
-        locator = CourseLocator(course_id='wonderful', revision='draft')
+        locator = CourseLocator(course_id='wonderful', branch='draft')
         course = modulestore().get_course(locator)
         self.assertEqual(str(course.location.version_guid), self.GUID_D2)
 
@@ -209,10 +216,10 @@ class SplitModuleCourseTests(SplitModuleTest):
         self.assertRaises(InsufficientSpecificationError,
                           modulestore().get_course, CourseLocator(course_id='edu.meh.blah'))
         self.assertRaises(ItemNotFoundError,
-                          modulestore().get_course, CourseLocator(course_id='nosuchthing', revision='draft'))
+                          modulestore().get_course, CourseLocator(course_id='nosuchthing', branch='draft'))
         self.assertRaises(ItemNotFoundError,
                           modulestore().get_course,
-                          CourseLocator(course_id='GreekHero', revision='published'))
+                          CourseLocator(course_id='GreekHero', branch='published'))
 
     def test_course_successors(self):
         """
@@ -250,7 +257,7 @@ class SplitModuleItemTests(SplitModuleTest):
         self.assertTrue(modulestore().has_item(locator),
                         "couldn't find in %s" % self.GUID_D1)
 
-        locator = BlockUsageLocator(course_id='GreekHero', usage_id='head12345', revision='draft')
+        locator = BlockUsageLocator(course_id='GreekHero', usage_id='head12345', branch='draft')
         self.assertTrue(
             modulestore().has_item(locator),
             "couldn't find in 12345"
@@ -258,7 +265,7 @@ class SplitModuleItemTests(SplitModuleTest):
         self.assertTrue(
             modulestore().has_item(BlockUsageLocator(
                 course_id=locator.course_id,
-                revision='draft',
+                branch='draft',
                 usage_id=locator.usage_id
             )),
             "couldn't find in draft 12345"
@@ -266,38 +273,38 @@ class SplitModuleItemTests(SplitModuleTest):
         self.assertFalse(
             modulestore().has_item(BlockUsageLocator(
                 course_id=locator.course_id,
-                revision='published',
+                branch='published',
                 usage_id=locator.usage_id)),
             "found in published 12345"
         )
-        locator.revision = 'draft'
+        locator.branch = 'draft'
         self.assertTrue(
             modulestore().has_item(locator),
             "not found in draft 12345"
         )
 
         # not a course obj
-        locator = BlockUsageLocator(course_id='GreekHero', usage_id='chapter1', revision='draft')
+        locator = BlockUsageLocator(course_id='GreekHero', usage_id='chapter1', branch='draft')
         self.assertTrue(
             modulestore().has_item(locator),
             "couldn't find chapter1"
         )
 
         # in published course
-        locator = BlockUsageLocator(course_id="wonderful", usage_id="head23456", revision='draft')
+        locator = BlockUsageLocator(course_id="wonderful", usage_id="head23456", branch='draft')
         self.assertTrue(modulestore().has_item(BlockUsageLocator(course_id=locator.course_id,
                                                                  usage_id=locator.usage_id,
-                                                                 revision='published')),
+                                                                 branch='published')),
                         "couldn't find in 23456")
-        locator.revision = 'published'
+        locator.branch = 'published'
         self.assertTrue(modulestore().has_item(locator), "couldn't find in 23456")
 
     def test_negative_has_item(self):
         # negative tests--not found
         # no such course or block
-        locator = BlockUsageLocator(course_id="doesnotexist", usage_id="head23456", revision='draft')
+        locator = BlockUsageLocator(course_id="doesnotexist", usage_id="head23456", branch='draft')
         self.assertFalse(modulestore().has_item(locator))
-        locator = BlockUsageLocator(course_id="wonderful", usage_id="doesnotexist", revision='draft')
+        locator = BlockUsageLocator(course_id="wonderful", usage_id="doesnotexist", branch='draft')
         self.assertFalse(modulestore().has_item(locator))
 
         # negative tests--insufficient specification
@@ -316,7 +323,7 @@ class SplitModuleItemTests(SplitModuleTest):
         block = modulestore().get_item(locator)
         self.assertIsInstance(block, CourseDescriptor)
 
-        locator = BlockUsageLocator(course_id='GreekHero', usage_id='head12345', revision='draft')
+        locator = BlockUsageLocator(course_id='GreekHero', usage_id='head12345', branch='draft')
         block = modulestore().get_item(locator)
         self.assertEqual(block.location.course_id, "GreekHero")
         # look at this one in detail
@@ -331,13 +338,13 @@ class SplitModuleItemTests(SplitModuleTest):
             block.grade_cutoffs, {"Pass": 0.45},
         )
 
-        # try to look up other revisions
+        # try to look up other branches
         self.assertRaises(ItemNotFoundError,
                           modulestore().get_item,
                           BlockUsageLocator(course_id=locator.as_course_locator(),
                                             usage_id=locator.usage_id,
-                                            revision='published'))
-        locator.revision = 'draft'
+                                            branch='published'))
+        locator.branch = 'draft'
         self.assertIsInstance(
             modulestore().get_item(locator),
             CourseDescriptor
@@ -345,7 +352,7 @@ class SplitModuleItemTests(SplitModuleTest):
 
     def test_get_non_root(self):
         # not a course obj
-        locator = BlockUsageLocator(course_id='GreekHero', usage_id='chapter1', revision='draft')
+        locator = BlockUsageLocator(course_id='GreekHero', usage_id='chapter1', branch='draft')
         block = modulestore().get_item(locator)
         self.assertEqual(block.location.course_id, "GreekHero")
         self.assertEqual(block.category, 'chapter')
@@ -354,7 +361,7 @@ class SplitModuleItemTests(SplitModuleTest):
         self.assertEqual(block.edited_by, "testassist@edx.org")
 
         # in published course
-        locator = BlockUsageLocator(course_id="wonderful", usage_id="head23456", revision='published')
+        locator = BlockUsageLocator(course_id="wonderful", usage_id="head23456", branch='published')
         self.assertIsInstance(
             modulestore().get_item(locator),
             CourseDescriptor
@@ -362,10 +369,10 @@ class SplitModuleItemTests(SplitModuleTest):
 
         # negative tests--not found
         # no such course or block
-        locator = BlockUsageLocator(course_id="doesnotexist", usage_id="head23456", revision='draft')
+        locator = BlockUsageLocator(course_id="doesnotexist", usage_id="head23456", branch='draft')
         with self.assertRaises(ItemNotFoundError):
             modulestore().get_item(locator)
-        locator = BlockUsageLocator(course_id="wonderful", usage_id="doesnotexist", revision='draft')
+        locator = BlockUsageLocator(course_id="wonderful", usage_id="doesnotexist", branch='draft')
         with self.assertRaises(ItemNotFoundError):
             modulestore().get_item(locator)
 
@@ -373,7 +380,7 @@ class SplitModuleItemTests(SplitModuleTest):
         with self.assertRaises(InsufficientSpecificationError):
             modulestore().get_item(BlockUsageLocator(version_guid=self.GUID_D1))
         with self.assertRaises(InsufficientSpecificationError):
-            modulestore().get_item(BlockUsageLocator(course_id='GreekHero', revision='draft'))
+            modulestore().get_item(BlockUsageLocator(course_id='GreekHero', branch='draft'))
 
     # pylint: disable=W0212
     def test_matching(self):
@@ -404,7 +411,7 @@ class SplitModuleItemTests(SplitModuleTest):
 
     def test_get_items(self):
         '''
-        get_items(locator, qualifiers, [revision])
+        get_items(locator, qualifiers, [branch])
         '''
         locator = CourseLocator(version_guid=self.GUID_D0)
         # get all modules
@@ -429,9 +436,9 @@ class SplitModuleItemTests(SplitModuleTest):
 
     def test_get_parents(self):
         '''
-        get_parent_locations(locator, [usage_id], [revision]): [BlockUsageLocator]
+        get_parent_locations(locator, [usage_id], [branch]): [BlockUsageLocator]
         '''
-        locator = CourseLocator(course_id="GreekHero", revision='draft')
+        locator = CourseLocator(course_id="GreekHero", branch='draft')
         parents = modulestore().get_parent_locations(locator, usage_id='chapter1')
         self.assertEqual(len(parents), 1)
         self.assertEqual(parents[0].usage_id, 'head12345')
@@ -447,7 +454,7 @@ class SplitModuleItemTests(SplitModuleTest):
         """
         Test the existing get_children method on xdescriptors
         """
-        locator = BlockUsageLocator(course_id="GreekHero", usage_id="head12345", revision='draft')
+        locator = BlockUsageLocator(course_id="GreekHero", usage_id="head12345", branch='draft')
         block = modulestore().get_item(locator)
         children = block.get_children()
         expected_ids = [
@@ -490,7 +497,7 @@ class TestItemCrud(SplitModuleTest):
         metadata=None): new_desciptor
         """
         # grab link to course to ensure new versioning works
-        locator = CourseLocator(course_id="GreekHero", revision='draft')
+        locator = CourseLocator(course_id="GreekHero", branch='draft')
         premod_course = modulestore().get_course(locator)
         premod_time = datetime.datetime.now(UTC) - datetime.timedelta(seconds=1)
         # add minimal one w/o a parent
@@ -527,7 +534,7 @@ class TestItemCrud(SplitModuleTest):
         """
         Test create_item w/ specifying the parent of the new item
         """
-        locator = BlockUsageLocator(course_id="wonderful", usage_id="head23456", revision='draft')
+        locator = BlockUsageLocator(course_id="wonderful", usage_id="head23456", branch='draft')
         premod_course = modulestore().get_course(locator)
         category = 'chapter'
         new_module = modulestore().create_item(
@@ -547,7 +554,7 @@ class TestItemCrud(SplitModuleTest):
         a definition id and new def data that it branches the definition in the db.
         Actually, this tries to test all create_item features not tested above.
         """
-        locator = BlockUsageLocator(course_id="contender", usage_id="head345679", revision='draft')
+        locator = BlockUsageLocator(course_id="contender", usage_id="head345679", branch='draft')
         category = 'problem'
         premod_time = datetime.datetime.now(UTC) - datetime.timedelta(seconds=1)
         new_payload = "<problem>empty</problem>"
@@ -585,7 +592,7 @@ class TestItemCrud(SplitModuleTest):
         """
         test updating an items metadata ensuring the definition doesn't version but the course does if it should
         """
-        locator = BlockUsageLocator(course_id="GreekHero", usage_id="problem3_2", revision='draft')
+        locator = BlockUsageLocator(course_id="GreekHero", usage_id="problem3_2", branch='draft')
         problem = modulestore().get_item(locator)
         pre_def_id = problem.definition_locator.definition_id
         pre_version_guid = problem.location.version_guid
@@ -622,7 +629,7 @@ class TestItemCrud(SplitModuleTest):
         """
         test updating an item's children ensuring the definition doesn't version but the course does if it should
         """
-        locator = BlockUsageLocator(course_id="GreekHero", usage_id="chapter3", revision='draft')
+        locator = BlockUsageLocator(course_id="GreekHero", usage_id="chapter3", branch='draft')
         block = modulestore().get_item(locator)
         pre_def_id = block.definition_locator.definition_id
         pre_version_guid = block.location.version_guid
@@ -646,7 +653,7 @@ class TestItemCrud(SplitModuleTest):
         """
         test updating an item's definition: ensure it gets versioned as well as the course getting versioned
         """
-        locator = BlockUsageLocator(course_id="GreekHero", usage_id="head12345", revision='draft')
+        locator = BlockUsageLocator(course_id="GreekHero", usage_id="head12345", branch='draft')
         block = modulestore().get_item(locator)
         pre_def_id = block.definition_locator.definition_id
         pre_version_guid = block.location.version_guid
@@ -663,7 +670,7 @@ class TestItemCrud(SplitModuleTest):
         Test updating metadata, children, and definition in a single call ensuring all the versioning occurs
         """
         # first add 2 children to the course for the update to manipulate
-        locator = BlockUsageLocator(course_id="contender", usage_id="head345679", revision='draft')
+        locator = BlockUsageLocator(course_id="contender", usage_id="head345679", branch='draft')
         category = 'problem'
         new_payload = "<problem>empty</problem>"
         modulestore().create_item(
@@ -707,14 +714,14 @@ class TestItemCrud(SplitModuleTest):
         reusable_location = BlockUsageLocator(
             course_id=course.location.course_id,
             usage_id=course.location.usage_id,
-            revision='draft')
+            branch='draft')
 
         # delete a leaf
         problems = modulestore().get_items(reusable_location, {'category': 'problem'})
         locn_to_del = problems[0].location
         new_course_loc = modulestore().delete_item(locn_to_del, 'deleting_user')
         deleted = BlockUsageLocator(course_id=reusable_location.course_id,
-                                    revision=reusable_location.revision,
+                                    branch=reusable_location.branch,
                                     usage_id=locn_to_del.usage_id)
         self.assertFalse(modulestore().has_item(deleted))
         self.assertRaises(VersionConflictError, modulestore().has_item, locn_to_del)
@@ -736,7 +743,7 @@ class TestItemCrud(SplitModuleTest):
                 self.assertFalse(modulestore().has_item(
                     BlockUsageLocator(
                         course_id=node_loc.course_id,
-                        revision=node_loc.revision,
+                        branch=node_loc.branch,
                         usage_id=node.location.usage_id)))
                 locator = BlockUsageLocator(
                     version_guid=node.location.version_guid,
@@ -752,7 +759,7 @@ class TestItemCrud(SplitModuleTest):
         root = BlockUsageLocator(
             course_id=course.location.course_id,
             usage_id=course.location.usage_id,
-            revision='draft')
+            branch='draft')
         for _ in range(4):
             self.create_subtree_for_deletion(root, ['chapter', 'vertical', 'problem'])
         return modulestore().get_item(root)
@@ -807,7 +814,7 @@ class TestCourseCreation(SplitModuleTest):
         Test making a course which points to an existing draft and published but not making any changes to either.
         """
         pre_time = datetime.datetime.now(UTC)
-        original_locator = CourseLocator(course_id="wonderful", revision='draft')
+        original_locator = CourseLocator(course_id="wonderful", branch='draft')
         original_index = modulestore().get_course_index_info(original_locator)
         new_draft = modulestore().create_course(
             'leech', 'best_course', 'leech_master', id_root='best',
@@ -824,7 +831,7 @@ class TestCourseCreation(SplitModuleTest):
         self.assertLessEqual(new_index["edited_on"], datetime.datetime.now(UTC))
         self.assertEqual(new_index['edited_by'], 'leech_master')
 
-        new_published_locator = CourseLocator(course_id=new_draft_locator.course_id, revision='published')
+        new_published_locator = CourseLocator(course_id=new_draft_locator.course_id, branch='published')
         new_published = modulestore().get_course(new_published_locator)
         self.assertEqual(new_published.edited_by, 'test@edx.org')
         self.assertLess(new_published.edited_on, pre_time)
@@ -863,7 +870,7 @@ class TestCourseCreation(SplitModuleTest):
         Create a new course which overrides metadata and course_data
         """
         pre_time = datetime.datetime.now(UTC)
-        original_locator = CourseLocator(course_id="contender", revision='draft')
+        original_locator = CourseLocator(course_id="contender", branch='draft')
         original = modulestore().get_course(original_locator)
         original_index = modulestore().get_course_index_info(original_locator)
         data_payload = {}
@@ -902,7 +909,7 @@ class TestCourseCreation(SplitModuleTest):
         """
         Test changing the org, pretty id, etc of a course. Test that it doesn't allow changing the id, etc.
         """
-        locator = CourseLocator(course_id="GreekHero", revision='draft')
+        locator = CourseLocator(course_id="GreekHero", branch='draft')
         modulestore().update_course_index(locator, {'org': 'funkyU'})
         course_info = modulestore().get_course_index_info(locator)
         self.assertEqual(course_info['org'], 'funkyU')
@@ -938,7 +945,7 @@ class TestCourseCreation(SplitModuleTest):
         # an allowed but not recommended way to publish a course
         versions['published'] = self.GUID_D1
         modulestore().update_course_index(locator, {'versions': versions}, update_versions=True)
-        course = modulestore().get_course(CourseLocator(course_id=locator.course_id, revision="published"))
+        course = modulestore().get_course(CourseLocator(course_id=locator.course_id, branch="published"))
         self.assertEqual(str(course.location.version_guid), self.GUID_D1)
 
 
@@ -952,11 +959,11 @@ class TestInheritance(SplitModuleTest):
         """
         # Note, not testing value where defined (course) b/c there's no
         # defined accessor for it on CourseDescriptor.
-        locator = BlockUsageLocator(course_id="GreekHero", usage_id="problem3_2", revision='draft')
+        locator = BlockUsageLocator(course_id="GreekHero", usage_id="problem3_2", branch='draft')
         node = modulestore().get_item(locator)
         # inherited
         self.assertEqual(node.graceperiod, datetime.timedelta(hours=2))
-        locator = BlockUsageLocator(course_id="GreekHero", usage_id="problem1", revision='draft')
+        locator = BlockUsageLocator(course_id="GreekHero", usage_id="problem1", branch='draft')
         node = modulestore().get_item(locator)
         # overridden
         self.assertEqual(node.graceperiod, datetime.timedelta(hours=4))

common/lib/xmodule/xmodule/raw_module.py

@@ -32,3 +32,22 @@ class RawDescriptor(XmlDescriptor, XMLEditingDescriptor):
                    context=lines[line - 1][offset - 40:offset + 40],
                    loc=self.location))
             raise Exception, msg, sys.exc_info()[2]
+
+
+class EmptyDataRawDescriptor(XmlDescriptor, XMLEditingDescriptor):
+    """
+    Version of RawDescriptor for modules which may have no XML data,
+    but use XMLEditingDescriptor for import/export handling.
+    """
+    data = String(default='', scope=Scope.content)
+
+    @classmethod
+    def definition_from_xml(cls, xml_object, system):
+        if len(xml_object) == 0 and len(xml_object.items()) == 0:
+            return {'data': ''}, []
+        return {'data': etree.tostring(xml_object, pretty_print=True, encoding='unicode')}, []
+
+    def definition_to_xml(self, resource_fs):
+        if self.data:
+            return etree.fromstring(self.data)
+        return etree.Element(self.category)

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

@@ -445,7 +445,7 @@ class ImportTestCase(BaseCourseTestCase):
         render_string_from_sample_gst_xml = """
         <slider var="a" style="width:400px;float:left;"/>\
 <plot style="margin-top:15px;margin-bottom:15px;"/>""".strip()
-        self.assertEqual(gst_sample.render, render_string_from_sample_gst_xml)
+        self.assertIn(render_string_from_sample_gst_xml, gst_sample.data)
 
     def test_word_cloud_import(self):
         modulestore = XMLModuleStore(DATA_DIR, course_dirs=['word_cloud'])

common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py

@@ -66,7 +66,7 @@ class TestXBlockWrapper(object):
     @property
     def leaf_module_runtime(self):
         runtime = Mock()
-        runtime.render_template = lambda *args, **kwargs: unicode((args, kwargs))
+        runtime.render_template = lambda *args, **kwargs: u'{!r}, {!r}'.format(args, kwargs)
         runtime.anonymous_student_id = 'dummy_anonymous_student_id'
         runtime.open_ended_grading_interface = {}
         runtime.seed = 5
@@ -78,7 +78,7 @@ class TestXBlockWrapper(object):
     @property
     def leaf_descriptor_runtime(self):
         runtime = Mock()
-        runtime.render_template = lambda *args, **kwargs: unicode((args, kwargs))
+        runtime.render_template = lambda *args, **kwargs: u'{!r}, {!r}'.format(args, kwargs)
         return runtime
 
     def leaf_descriptor(self, descriptor_cls):
@@ -102,7 +102,7 @@ class TestXBlockWrapper(object):
     @property
     def container_descriptor_runtime(self):
         runtime = Mock()
-        runtime.render_template = lambda *args, **kwargs: unicode((args, kwargs))
+        runtime.render_template = lambda *args, **kwargs: u'{!r}, {!r}'.format(args, kwargs)
         return runtime
 
     def container_descriptor(self, descriptor_cls):

common/lib/xmodule/xmodule/video_module.py

@@ -12,7 +12,7 @@ import time
 from django.http import Http404
 
 from xmodule.x_module import XModule
-from xmodule.raw_module import RawDescriptor
+from xmodule.raw_module import EmptyDataRawDescriptor
 from xmodule.editing_module import MetadataOnlyEditingDescriptor
 from xblock.core import Integer, Scope, String, Float, Boolean
 
@@ -97,7 +97,7 @@ class VideoModule(VideoFields, XModule):
 
 class VideoDescriptor(VideoFields,
                       MetadataOnlyEditingDescriptor,
-                      RawDescriptor):
+                      EmptyDataRawDescriptor):
     module_class = VideoModule
 
     def __init__(self, *args, **kwargs):
@@ -130,19 +130,15 @@ class VideoDescriptor(VideoFields,
         _parse_video_xml(video, video.data)
         return video
 
-    def definition_to_xml(self, resource_fs):
-        """
-        Override the base implementation. We don't actually have anything in the 'data' field
-        (it's an empty string), so we just return a simple XML element
-        """
-        return etree.Element('video')
-
 
 def _parse_video_xml(video, xml_data):
     """
     Parse video fields out of xml_data. The fields are set if they are
     present in the XML.
     """
+    if not xml_data:
+        return
+
     xml = etree.fromstring(xml_data)
 
     display_name = xml.get('display_name')

common/lib/xmodule/xmodule/word_cloud_module.py

@@ -10,7 +10,7 @@ import json
 import logging
 
 from pkg_resources import resource_string
-from xmodule.raw_module import RawDescriptor
+from xmodule.raw_module import EmptyDataRawDescriptor
 from xmodule.editing_module import MetadataOnlyEditingDescriptor
 from xmodule.x_module import XModule
 
@@ -240,7 +240,7 @@ class WordCloudModule(WordCloudFields, XModule):
         return self.content
 
 
-class WordCloudDescriptor(WordCloudFields, MetadataOnlyEditingDescriptor, RawDescriptor):
+class WordCloudDescriptor(WordCloudFields, MetadataOnlyEditingDescriptor, EmptyDataRawDescriptor):
     """Descriptor for WordCloud Xmodule."""
     module_class = WordCloudModule
     template_dir_name = 'word_cloud'

common/lib/xmodule/xmodule/x_module.py

@@ -12,6 +12,7 @@ from xmodule.modulestore.exceptions import ItemNotFoundError, InsufficientSpecif
 
 from xblock.core import XBlock, Scope, String, Integer, Float, ModelType
 from xblock.fragment import Fragment
+from xblock.runtime import Runtime
 from xmodule.modulestore.locator import BlockUsageLocator
 
 log = logging.getLogger(__name__)
@@ -870,7 +871,7 @@ class XMLParsingSystem(DescriptorSystem):
         self.policy = policy
 
 
-class ModuleSystem(object):
+class ModuleSystem(Runtime):
     '''
     This is an abstraction such that x_modules can function independent
     of the courseware (e.g. import into other types of courseware, LMS,

common/static/sass/_mixins.scss

@@ -205,7 +205,7 @@
 // extends - UI archetypes - well
 .ui-well {
   box-shadow: inset 0 1px 2px 1px $shadow;
-  padding: ($baseline*0.75);
+  padding: ($baseline*0.75) $baseline;
 }
 
 // ====================

common/templates/courseware_vendor_js.html

@@ -8,6 +8,7 @@
 <script type="text/javascript" src="${static.url('js/vendor/jquery.cookie.js')}"></script>
 <script type="text/javascript" src="${static.url('js/vendor/jquery.qtip.min.js')}"></script>
 <script type="text/javascript" src="${static.url('js/vendor/jquery.scrollTo-1.4.2-min.js')}"></script>
+<script type="text/javascript" src="${static.url('js/vendor/flot/jquery.flot.js')}"></script>
 
 ## codemirror
 <link rel="stylesheet" href="${static.url('js/vendor/CodeMirror/codemirror.css')}" type="text/css" media="all" />

conf/locale/babel.cfg

@@ -17,3 +17,5 @@ input_encoding = utf-8
 input_encoding = utf-8
 [mako: common/templates/**.html]
 input_encoding = utf-8
+[mako: cms/templates/emails/**.txt]
+input_encoding = utf-8

conf/locale/config

@@ -1,4 +1,4 @@
 {
- "locales" : ["en", "es"],
+ "locales" : ["en", "zh_CN"],
  "dummy-locale" : "fr"
 }

docs/internal/testing.md

@@ -186,8 +186,8 @@ uses [Selenium](http://docs.seleniumhq.org/) to control the Chrome browser.
 
 **Prerequisite**: You must have [ChromeDriver](https://code.google.com/p/selenium/wiki/ChromeDriver)
 installed to run the tests in Chrome.  The tests are confirmed to run
-with Chrome (not Chromium) version 26.0.0.1410.63 with ChromeDriver
-version r195636.
+with Chrome (not Chromium) version 28.0.1500.71 with ChromeDriver
+version 2.1.210398.
 
 To run all the acceptance tests:
 

lms/djangoapps/branding/views.py

@@ -24,13 +24,13 @@ def index(request):
         from external_auth.views import ssl_login
         return ssl_login(request)
     if settings.MITX_FEATURES.get('ENABLE_MKTG_SITE'):
-         return redirect(settings.MKTG_URLS.get('ROOT'))
+        return redirect(settings.MKTG_URLS.get('ROOT'))
 
     university = branding.get_university(request.META.get('HTTP_HOST'))
     if university is None:
         return student.views.index(request, user=request.user)
 
-    return courseware.views.university_profile(request, university)
+    return redirect('/')
 
 
 @ensure_csrf_cookie
@@ -48,4 +48,4 @@ def courses(request):
     if university is None:
         return courseware.views.courses(request)
 
-    return courseware.views.university_profile(request, university)
+    return redirect('/')

lms/djangoapps/course_wiki/tests/tests.py

@@ -90,8 +90,8 @@ class WikiRedirectTestCase(LoginEnrollmentTestCase):
         """
         Ensure that the response has the course navigator.
         """
-        self.assertTrue("course info" in resp.content.lower())
-        self.assertTrue("courseware" in resp.content.lower())
+        self.assertContains(resp, "Course Info")
+        self.assertContains(resp, "courseware")
 
     def test_course_navigator(self):
         """"

lms/djangoapps/courseware/courses.py

@@ -163,7 +163,7 @@ def get_course_about_section(course, section_key):
             html = ''
 
             if about_module is not None:
-                html = about_module.get_html()
+                html = about_module.runtime.render(about_module, None, 'student_view').content
 
             return html
 
@@ -211,7 +211,7 @@ def get_course_info_section(request, course, section_key):
     html = ''
 
     if info_module is not None:
-        html = info_module.get_html()
+        html = info_module.runtime.render(info_module, None, 'student_view').content
 
     return html
 

lms/djangoapps/courseware/features/homepage.py

@@ -8,7 +8,7 @@ from nose.tools import assert_in, assert_equals
 @step(u'I should see the following Partners in the Partners section')
 def i_should_see_partner(step):
     partners = world.browser.find_by_css(".partner .name span")
-    names = set(span.text for span in partners)
+    names = set(span.html for span in partners)
     for partner in step.hashes:
         assert_in(partner['Partner'], names)
 

lms/djangoapps/courseware/tabs.py

@@ -91,6 +91,13 @@ def _discussion(tab, user, course, active_page):
     return []
 
 
+def _external_discussion(tab, user, course, active_page):
+    """
+    This returns a tab that links to an external discussion service
+    """
+    return [CourseTab('Discussion', tab['link'], active_page == 'discussion')]
+
+
 def _external_link(tab, user, course, active_page):
     # external links are never active
     return [CourseTab(tab['name'], tab['link'], False)]
@@ -150,6 +157,12 @@ def _staff_grading(tab, user, course, active_page):
     return []
 
 
+def _syllabus(tab, user, course, active_page):
+    """Display the syllabus tab"""
+    link = reverse('syllabus', args=[course.id])
+    return [CourseTab('Syllabus', link, active_page == 'syllabus')]
+
+
 def _peer_grading(tab, user, course, active_page):
 
     if user.is_authenticated():
@@ -216,6 +229,7 @@ VALID_TAB_TYPES = {
     'course_info': TabImpl(need_name, _course_info),
     'wiki': TabImpl(need_name, _wiki),
     'discussion': TabImpl(need_name, _discussion),
+    'external_discussion': TabImpl(key_checker(['link']), _external_discussion),
     'external_link': TabImpl(key_checker(['name', 'link']), _external_link),
     'textbooks': TabImpl(null_validator, _textbooks),
     'pdf_textbooks': TabImpl(null_validator, _pdf_textbooks),
@@ -225,7 +239,8 @@ VALID_TAB_TYPES = {
     'peer_grading': TabImpl(null_validator, _peer_grading),
     'staff_grading': TabImpl(null_validator, _staff_grading),
     'open_ended': TabImpl(null_validator, _combined_open_ended_grading),
-    'notes': TabImpl(null_validator, _notes_tab)
+    'notes': TabImpl(null_validator, _notes_tab),
+    'syllabus': TabImpl(null_validator, _syllabus)
     }
 
 
@@ -371,6 +386,6 @@ def get_static_tab_contents(request, course, tab):
     html = ''
 
     if tab_module is not None:
-        html = tab_module.get_html()
+        html = tab_module.runtime.render(tab_module, None, 'student_view').content
 
     return html

lms/djangoapps/courseware/tests/__init__.py

@@ -77,13 +77,15 @@ class BaseTestXmodule(ModuleStoreTestCase):
             data=self.DATA
         )
 
-        system = get_test_system()
-        system.render_template = lambda template, context: context
+        self.runtime = get_test_system()
+        # Allow us to assert that the template was called in the same way from
+        # different code paths while maintaining the type returned by render_template
+        self.runtime.render_template = lambda template, context: u'{!r}, {!r}'.format(template, sorted(context.items()))
         model_data = {'location': self.item_descriptor.location}
         model_data.update(self.MODEL_DATA)
 
         self.item_module = self.item_descriptor.module_class(
-            system, self.item_descriptor, model_data
+            self.runtime, self.item_descriptor, model_data
         )
         self.item_url = Location(self.item_module.location).url()
 

lms/djangoapps/courseware/tests/test_module_render.py

@@ -1,7 +1,7 @@
 """
 Test for lms courseware app, module render unit
 """
-from mock import MagicMock, patch
+from mock import MagicMock, patch, Mock
 import json
 
 from django.http import Http404, HttpResponse
@@ -12,8 +12,10 @@ from django.test.client import RequestFactory
 from django.test.utils import override_settings
 
 from xmodule.modulestore.django import modulestore
+from xmodule.modulestore.tests.factories import ItemFactory, CourseFactory
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 import courseware.module_render as render
-from courseware.tests.tests import LoginEnrollmentTestCase
+from courseware.tests.tests import LoginEnrollmentTestCase, TEST_DATA_MONGO_MODULESTORE
 from courseware.model_data import ModelDataCache
 from modulestore_config import TEST_DATA_XML_MODULESTORE
 
@@ -49,8 +51,10 @@ class ModuleRenderTestCase(LoginEnrollmentTestCase):
                                                                    dispatch=self.dispatch))
 
     def test_get_module(self):
-        self.assertIsNone(render.get_module('dummyuser', None,
-                                            'invalid location', None, None))
+        self.assertEqual(
+            None,
+            render.get_module('dummyuser', None, 'invalid location', None, None)
+        )
 
     def test_module_render_with_jump_to_id(self):
         """
@@ -230,7 +234,8 @@ class TestTOC(TestCase):
                       'url_name': 'secret:magic', 'display_name': 'secret:magic'}])
 
         actual = render.toc_for_course(self.portal_user, request, self.toy_course, chapter, None, model_data_cache)
-        assert reduce(lambda x, y: x and (y in actual), expected, True)
+        for toc_section in expected:
+            self.assertIn(toc_section, actual)
 
     def test_toc_toy_from_section(self):
         chapter = 'Overview'
@@ -257,4 +262,109 @@ class TestTOC(TestCase):
                       'url_name': 'secret:magic', 'display_name': 'secret:magic'}])
 
         actual = render.toc_for_course(self.portal_user, request, self.toy_course, chapter, section, model_data_cache)
-        assert reduce(lambda x, y: x and (y in actual), expected, True)
+        for toc_section in expected:
+            self.assertIn(toc_section, actual)
+
+
+@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE)
+class TestHtmlModifiers(ModuleStoreTestCase):
+    """
+    Tests to verify that standard modifications to the output of XModule/XBlock
+    student_view are taking place
+    """
+    def setUp(self):
+        self.user = UserFactory.create()
+        self.request = RequestFactory().get('/')
+        self.request.user = self.user
+        self.request.session = {}
+        self.course = CourseFactory.create()
+        self.content_string = '<p>This is the content<p>'
+        self.rewrite_link = '<a href="/static/foo/content">Test rewrite</a>'
+        self.course_link = '<a href="/course/bar/content">Test course rewrite</a>'
+        self.descriptor = ItemFactory.create(
+            category='html',
+            data=self.content_string + self.rewrite_link + self.course_link
+        )
+        self.location = self.descriptor.location
+        self.model_data_cache = ModelDataCache.cache_for_descriptor_descendents(
+            self.course.id,
+            self.user,
+            self.descriptor
+        )
+
+    def test_xmodule_display_wrapper_enabled(self):
+        module = render.get_module(
+            self.user,
+            self.request,
+            self.location,
+            self.model_data_cache,
+            self.course.id,
+            wrap_xmodule_display=True,
+        )
+        result_fragment = module.runtime.render(module, None, 'student_view')
+
+        self.assertIn('section class="xmodule_display xmodule_HtmlModule"', result_fragment.content)
+
+    def test_xmodule_display_wrapper_disabled(self):
+        module = render.get_module(
+            self.user,
+            self.request,
+            self.location,
+            self.model_data_cache,
+            self.course.id,
+            wrap_xmodule_display=False,
+        )
+        result_fragment = module.runtime.render(module, None, 'student_view')
+
+        self.assertNotIn('section class="xmodule_display xmodule_HtmlModule"', result_fragment.content)
+
+    def test_static_link_rewrite(self):
+        module = render.get_module(
+            self.user,
+            self.request,
+            self.location,
+            self.model_data_cache,
+            self.course.id,
+        )
+        result_fragment = module.runtime.render(module, None, 'student_view')
+
+        self.assertIn(
+            '/c4x/{org}/{course}/asset/foo_content'.format(
+                org=self.course.location.org,
+                course=self.course.location.course,
+            ),
+            result_fragment.content
+        )
+
+    def test_course_link_rewrite(self):
+        module = render.get_module(
+            self.user,
+            self.request,
+            self.location,
+            self.model_data_cache,
+            self.course.id,
+        )
+        result_fragment = module.runtime.render(module, None, 'student_view')
+
+        self.assertIn(
+            '/courses/{course_id}/bar/content'.format(
+                course_id=self.course.id
+            ),
+            result_fragment.content
+        )
+
+    @patch('courseware.module_render.has_access', Mock(return_value=True))
+    def test_histogram(self):
+        module = render.get_module(
+            self.user,
+            self.request,
+            self.location,
+            self.model_data_cache,
+            self.course.id,
+        )
+        result_fragment = module.runtime.render(module, None, 'student_view')
+
+        self.assertIn(
+            'Staff Debug',
+            result_fragment.content
+        )

lms/djangoapps/courseware/tests/test_videoalpha_mongo.py

@@ -34,9 +34,7 @@ class TestVideo(BaseTestXmodule):
     def test_videoalpha_constructor(self):
         """Make sure that all parameters extracted correclty from xml"""
 
-        # `get_html` return only context, cause we
-        # overwrite `system.render_template`
-        context = self.item_module.get_html()
+        fragment = self.runtime.render(self.item_module, None, 'student_view')
         expected_context = {
             'data_dir': getattr(self, 'data_dir', None),
             'caption_asset_path': '/c4x/MITx/999/asset/subs_',
@@ -51,7 +49,7 @@ class TestVideo(BaseTestXmodule):
             'youtube_streams': self.item_module.youtube_streams,
             'autoplay': settings.MITX_FEATURES.get('AUTOPLAY_VIDEOS', True)
         }
-        self.assertDictEqual(context, expected_context)
+        self.assertEqual(fragment.content, self.runtime.render_template('videoalpha.html', expected_context))
 
 
 class TestVideoNonYouTube(TestVideo):
@@ -78,9 +76,7 @@ class TestVideoNonYouTube(TestVideo):
             the template generates an empty string for the YouTube streams.
         """
 
-        # `get_html` return only context, cause we
-        # overwrite `system.render_template`
-        context = self.item_module.get_html()
+        fragment = self.runtime.render(self.item_module, None, 'student_view')
         expected_context = {
             'data_dir': getattr(self, 'data_dir', None),
             'caption_asset_path': '/c4x/MITx/999/asset/subs_',
@@ -95,4 +91,4 @@ class TestVideoNonYouTube(TestVideo):
             'youtube_streams': '',
             'autoplay': settings.MITX_FEATURES.get('AUTOPLAY_VIDEOS', True)
         }
-        self.assertDictEqual(context, expected_context)
+        self.assertEqual(fragment.content, self.runtime.render_template('videoalpha.html', expected_context))

lms/djangoapps/courseware/tests/test_videoalpha_xml.py

@@ -104,10 +104,9 @@ class VideoAlphaModuleUnitTest(unittest.TestCase):
     def test_videoalpha_constructor(self):
         """Make sure that all parameters extracted correclty from xml"""
         module = VideoAlphaFactory.create()
+        module.runtime.render_template = lambda template, context: u'{!r}, {!r}'.format(template, sorted(context.items()))
 
-        # `get_html` return only context, cause we
-        # overwrite `system.render_template`
-        context = module.get_html()
+        fragment = module.runtime.render(module, None, 'student_view')
         expected_context = {
             'caption_asset_path': '/static/subs/',
             'sub': module.sub,
@@ -122,7 +121,7 @@ class VideoAlphaModuleUnitTest(unittest.TestCase):
             'track': module.track,
             'autoplay': settings.MITX_FEATURES.get('AUTOPLAY_VIDEOS', True)
         }
-        self.assertDictEqual(context, expected_context)
+        self.assertEqual(fragment.content, module.runtime.render_template('videoalpha.html', expected_context))
 
         self.assertDictEqual(
             json.loads(module.get_instance_state()),

lms/djangoapps/courseware/tests/test_word_cloud.py

@@ -242,9 +242,7 @@ class TestWordCloud(BaseTestXmodule):
 
     def test_word_cloud_constructor(self):
         """Make sure that all parameters extracted correclty from xml"""
-        # `get_html` return only context, cause we
-        # overwrite `system.render_template`
-        context = self.item_module.get_html()
+        fragment = self.runtime.render(self.item_module, None, 'student_view')
 
         expected_context = {
             'ajax_url': self.item_module.system.ajax_url,
@@ -253,4 +251,4 @@ class TestWordCloud(BaseTestXmodule):
             'num_inputs': 5,  # default value
             'submitted': False  # default value
         }
-        self.assertDictEqual(context, expected_context)
+        self.assertEqual(fragment.content, self.runtime.render_template('word_cloud.html', expected_context))

lms/djangoapps/courseware/tests/tests.py

@@ -120,9 +120,8 @@ class PageLoaderTestCase(LoginEnrollmentTestCase):
             self.assertEqual(response.redirect_chain[0][1], 302)
 
         if check_content:
-            unavailable_msg = "this module is temporarily unavailable"
-            self.assertEqual(response.content.find(unavailable_msg), -1)
-            self.assertFalse(isinstance(descriptor, ErrorDescriptor))
+            self.assertNotContains(response, "this module is temporarily unavailable")
+            self.assertNotIsInstance(descriptor, ErrorDescriptor)
 
 
 @override_settings(MODULESTORE=TEST_DATA_XML_MODULESTORE)

lms/djangoapps/courseware/views.py

@@ -400,7 +400,7 @@ def index(request, course_id, chapter=None, section=None,
                 # add in the appropriate timer information to the rendering context:
                 context.update(check_for_active_timelimit_module(request, course_id, course))
 
-            context['content'] = section_module.get_html()
+            context['content'] = section_module.runtime.render(section_module, None, 'student_view').content
         else:
             # section is none, so display a message
             prev_section = get_current_child(chapter_module)
@@ -632,57 +632,6 @@ def mktg_course_about(request, course_id):
                                'show_courseware_link': show_courseware_link})
 
 
-
-@ensure_csrf_cookie
-@cache_if_anonymous
-def static_university_profile(request, org_id):
-    """
-    Return the profile for the particular org_id that does not have any courses.
-    """
-    # Redirect to the properly capitalized org_id
-    last_path = request.path.split('/')[-1]
-    if last_path != org_id:
-        return redirect('static_university_profile', org_id=org_id)
-
-    # Render template
-    template_file = "university_profile/{0}.html".format(org_id).lower()
-    context = dict(courses=[], org_id=org_id)
-    return render_to_response(template_file, context)
-
-
-@ensure_csrf_cookie
-@cache_if_anonymous
-def university_profile(request, org_id):
-    """
-    Return the profile for the particular org_id.  404 if it's not valid.
-    """
-    virtual_orgs_ids = settings.VIRTUAL_UNIVERSITIES
-    meta_orgs = getattr(settings, 'META_UNIVERSITIES', {})
-
-    # Get all the ids associated with this organization
-    all_courses = modulestore().get_courses()
-    valid_orgs_ids = set(c.org for c in all_courses)
-    valid_orgs_ids.update(virtual_orgs_ids + meta_orgs.keys())
-
-    if org_id not in valid_orgs_ids:
-        raise Http404("University Profile not found for {0}".format(org_id))
-
-    # Grab all courses for this organization(s)
-    org_ids = set([org_id] + meta_orgs.get(org_id, []))
-    org_courses = []
-    domain = request.META.get('HTTP_HOST')
-    for key in org_ids:
-        cs = get_courses_by_university(request.user, domain=domain)[key]
-        org_courses.extend(cs)
-
-    org_courses = sort_by_announcement(org_courses)
-
-    context = dict(courses=org_courses, org_id=org_id)
-    template_file = "university_profile/{0}.html".format(org_id).lower()
-
-    return render_to_response(template_file, context)
-
-
 def render_notifications(request, course, notifications):
     context = {
         'notifications': notifications,
@@ -779,12 +728,16 @@ def submission_history(request, course_id, student_username, location):
     except StudentModule.DoesNotExist:
         return HttpResponse(escape("{0} has never accessed problem {1}".format(student_username, location)))
 
-    history_entries = StudentModuleHistory.objects.filter(student_module=student_module).order_by('-id')
+    history_entries = StudentModuleHistory.objects.filter(
+        student_module=student_module
+    ).order_by('-id')
 
     # If no history records exist, let's force a save to get history started.
     if not history_entries:
         student_module.save()
-        history_entries = StudentModuleHistory.objects.filter(student_module=student_module).order_by('-id')
+        history_entries = StudentModuleHistory.objects.filter(
+            student_module=student_module
+        ).order_by('-id')
 
     context = {
         'history_entries': history_entries,

lms/djangoapps/instructor/views.py

@@ -21,6 +21,7 @@ from django_future.csrf import ensure_csrf_cookie
 from django.views.decorators.cache import cache_control
 from django.core.urlresolvers import reverse
 from django.core.mail import send_mail
+from django.utils import timezone
 
 import xmodule.graders as xmgraders
 from xmodule.modulestore.django import modulestore
@@ -93,6 +94,7 @@ def instructor_dashboard(request, course_id):
                      'title': 'Course Statistics At A Glance',
                      }
         data = [['# Enrolled', CourseEnrollment.objects.filter(course_id=course_id).count()]]
+        data += [['Date', timezone.now().isoformat()]]
         data += compute_course_stats(course).items()
         if request.user.is_staff:
             for field in course.fields:

lms/djangoapps/user_api/tests/test_views.py

@@ -1,3 +1,5 @@
+import base64
+
 from django.contrib.auth.models import User
 from django.test import TestCase
 from django.test.utils import override_settings
@@ -31,6 +33,9 @@ class UserApiTestCase(TestCase):
             UserPreferenceFactory.create(user=self.users[1], key="key0")
         ]
 
+    def basic_auth(self, username, password):
+        return {'HTTP_AUTHORIZATION': 'Basic ' + base64.b64encode('%s:%s' % (username, password))}
+
     def request_with_auth(self, method, *args, **kwargs):
         """Issue a get request to the given URI with the API key header"""
         return getattr(self.client, method)(*args, HTTP_X_EDX_API_KEY=TEST_API_KEY, **kwargs)
@@ -127,6 +132,15 @@ class UserViewSetTest(UserApiTestCase):
     def test_debug_auth(self):
         self.assertHttpOK(self.client.get(self.LIST_URI))
 
+    @override_settings(DEBUG=False)
+    @override_settings(EDX_API_KEY=TEST_API_KEY)
+    def test_basic_auth(self):
+        # ensure that having basic auth headers in the mix does not break anything
+        self.assertHttpOK(
+                self.request_with_auth("get", self.LIST_URI, **self.basic_auth('someuser', 'somepass')))
+        self.assertHttpForbidden(
+                self.client.get(self.LIST_URI, **self.basic_auth('someuser', 'somepass')))
+
     def test_get_list_empty(self):
         User.objects.all().delete()
         result = self.get_json(self.LIST_URI)