Merge pull request #1204 from edx/jmclaus_bugfix_video_player_controls_a11y

Bug fix video player controls a11y

.gitignore

+# .gitignore for edx-platform.
+# There's a lot here, please try to keep it organized.
+
+### Files private to developers
+
+requirements/private.txt
+lms/envs/private.py
+cms/envs/private.py
+
+### Python artifacts
 *.pyc
+
+### Editor and IDE artifacts
 *~
-*.scssc
 *.swp
 *.orig
+/nbproject
+.idea/
+.redcar/
+
+### OS X artifacts
 *.DS_Store
-*.mo
+.AppleDouble
 :2e_*
 :2e#
-.AppleDouble
-database.sqlite
-requirements/private.txt
-lms/envs/private.py
-cms/envs/private.py
-courseware/static/js/mathjax/*
-flushdb.sh
-build
+
+### Internationalization artifacts
+*.mo
+conf/locale/en/LC_MESSAGES/*.po
+!messages.po
+
+### Testing artifacts
+.testids/
+.noseids
+nosetests.xml
 .coverage
 coverage.xml
 cover/
-log/
+cover_html/
 reports/
-/src/
-\#*\#
+
+### Installation artifacts
 *.egg-info
 Gemfile.lock
-.env/
-conf/locale/en/LC_MESSAGES/*.po
-!messages.po
+.pip_download_cache/
+.prereqs_cache
+.vagrant/
+node_modules
+
+### Static assets pipeline artifacts
+*.scssc
 lms/static/sass/*.css
 lms/static/sass/application.scss
 lms/static/sass/course.scss
 cms/static/sass/*.css
-lms/lib/comment_client/python
-nosetests.xml
-cover_html/
-.idea/
-.redcar/
+
+### Logging artifacts
+log/
+logs
 chromedriver.log
-/nbproject
 ghostdriver.log
-node_modules
-.pip_download_cache/
-.prereqs_cache
+
+### Unknown artifacts
+database.sqlite
+courseware/static/js/mathjax/*
+flushdb.sh
+build
+/src/
+\#*\#
+.env/
+lms/lib/comment_client/python
 autodeploy.properties
 .ws_migrations_complete
-.vagrant/
-logs
-.testids/

CHANGELOG.rst

@@ -5,8 +5,13 @@ These are notable changes in edx-platform.  This is a rolling list of changes,
 in roughly chronological order, most recent first.  Add your entries at or near
 the top.  Include a label indicating the component affected.
 
+LMS: Add PaidCourseRegistration mode, where payment is required before course registration.
+
 LMS: Add split testing functionality for internal use.
 
+CMS: Add edit_course_tabs management command, providing a primitive
+editing capability for a course's list of tabs.
+
 Studio and LMS: add ability to lock assets (cannot be viewed unless registered for class).
 
 LMS: Improved accessibility of parts of forum navigation sidebar.
@@ -311,3 +316,6 @@ Common: Updated CodeJail.
 Common: Allow setting of authentication session cookie name.
 
 LMS: Option to email students when enroll/un-enroll them.
+
+Blades: Added WAI-ARIA markup to the video player controls. These are now fully
+accessible by screen readers. 

cms/djangoapps/contentstore/features/upload.feature

@@ -5,17 +5,16 @@ Feature: CMS.Upload Files
     # Uploading isn't working on safari with sauce labs
     @skip_safari
     Scenario: Users can upload files
-        Given I have opened a new course in Studio
-        And I go to the files and uploads page
+        Given I am at the files and upload page of a Studio course
         When I upload the file "test"
         Then I should see the file "test" was uploaded
         And The url for the file "test" is valid
 
+    # Uploading isn't working on safari with sauce labs
     @skip_safari
     Scenario: Users can upload multiple files
-        Given I have opened a new course in studio
-        And I go to the files and uploads page
-        When I upload the files "test","test2"
+        Given I am at the files and upload page of a Studio course
+        When I upload the files "test,test2"
         Then I should see the file "test" was uploaded
         And I should see the file "test2" was uploaded
         And The url for the file "test2" is valid
@@ -24,8 +23,7 @@ Feature: CMS.Upload Files
     # Uploading isn't working on safari with sauce labs
     @skip_safari
     Scenario: Users can update files
-        Given I have opened a new course in studio
-        And I go to the files and uploads page
+        Given I am at the files and upload page of a Studio course
         When I upload the file "test"
         And I upload the file "test"
         Then I should see only one "test"
@@ -33,8 +31,7 @@ Feature: CMS.Upload Files
     # Uploading isn't working on safari with sauce labs
     @skip_safari
     Scenario: Users can delete uploaded files
-        Given I have opened a new course in studio
-        And I go to the files and uploads page
+        Given I am at the files and upload page of a Studio course
         When I upload the file "test"
         And I delete the file "test"
         Then I should not see the file "test" was uploaded
@@ -43,16 +40,14 @@ Feature: CMS.Upload Files
     # Uploading isn't working on safari with sauce labs
     @skip_safari
     Scenario: Users can download files
-        Given I have opened a new course in studio
-        And I go to the files and uploads page
+        Given I am at the files and upload page of a Studio course
         When I upload the file "test"
         Then I can download the correct "test" file
 
     # Uploading isn't working on safari with sauce labs
     @skip_safari
     Scenario: Users can download updated files
-        Given I have opened a new course in studio
-        And I go to the files and uploads page
+        Given I am at the files and upload page of a Studio course
         When I upload the file "test"
         And I modify "test"
         And I reload the page
@@ -62,57 +57,59 @@ Feature: CMS.Upload Files
     # Uploading isn't working on safari with sauce labs
     @skip_safari
     Scenario: Users can lock assets through asset index
-        Given I have opened a new course in studio
-        And I go to the files and uploads page
-        When I upload the file "test"
-        And I lock "test"
-        Then "test" is locked
+        Given I am at the files and upload page of a Studio course
+        When I upload an asset
+        And I lock the asset
+        Then the asset is locked
         And I see a "saving" notification
         And I reload the page
-        Then "test" is locked
+        Then the asset is locked
 
     # Uploading isn't working on safari with sauce labs
     @skip_safari
     Scenario: Users can unlock assets through asset index
-        Given I have opened a course with a locked asset "test"
-        And I unlock "test"
-        Then "test" is unlocked
+        Given I have created a course with a locked asset
+        When I unlock the asset
+        Then the asset is unlocked
         And I see a "saving" notification
         And I reload the page
-        Then "test" is unlocked
+        Then the asset is unlocked
+
+    # Uploading isn't working on safari with sauce labs
+    @skip_safari
+    Scenario: Locked assets can't be viewed if logged in as an unregistered user
+        Given I have created a course with a locked asset
+        And the user "bob" exists
+        When "bob" logs in
+        Then the asset is protected
 
     # Uploading isn't working on safari with sauce labs
-    # TODO: work with Jay
-#    @skip_safari
-#    Scenario: Locked assets can't be viewed if logged in as unregistered user
-#        Given I have opened a course with a locked asset "locked.html"
-#        Then the asset "locked.html" can be clicked from the asset index
-#        And the user "bob" exists
-#        And "bob" logs in
-#        Then the asset "locked.html" is protected
+    @skip_safari
+    Scenario: Locked assets can be viewed if logged in as a registered user
+        Given I have created a course with a locked asset
+        And the user "bob" exists
+        And the user "bob" is enrolled in the course
+        When "bob" logs in
+        Then the asset is viewable
 
     # Uploading isn't working on safari with sauce labs
     @skip_safari
     Scenario: Locked assets can't be viewed if logged out
-        Given I have opened a course with a locked asset "locked.html"
-        # Note that logging out doesn't really matter at the moment-
-        # the asset will be protected because the user sent to middleware is the anonymous user.
-        # Need to work with Jay.
-        And I log out
-        Then the asset "locked.html" is protected
+        Given I have created a course with a locked asset
+        When I log out
+        Then the asset is protected
 
     # Uploading isn't working on safari with sauce labs
     @skip_safari
     Scenario: Locked assets can be viewed with is_staff account
-        Given I have opened a course with a locked asset "locked.html"
+        Given I have created a course with a locked asset
         And the user "staff" exists as a course is_staff
-        And "staff" logs in
-        Then the asset "locked.html" can be clicked from the asset index
+        When "staff" logs in
+        Then the asset is viewable
 
     # Uploading isn't working on safari with sauce labs
     @skip_safari
     Scenario: Unlocked assets can be viewed by anyone
-        Given I have opened a course with a unlocked asset "unlocked.html"
-        Then the asset "unlocked.html" can be clicked from the asset index
-        And I log out
-        Then the asset "unlocked.html" is viewable
+        Given I have created a course with a unlocked asset
+        When I log out
+        Then the asset is viewable

cms/djangoapps/contentstore/features/upload.py

@@ -2,14 +2,17 @@
 #pylint: disable=W0621
 
 from lettuce import world, step
+from lettuce.django import django_url
 from django.conf import settings
 import requests
 import string
 import random
 import os
+from django.contrib.auth.models import User
+from student.models import CourseEnrollment
+from splinter.request_handler.status_code import HttpResponseError
 from nose.tools import assert_equal, assert_not_equal # pylint: disable=E0611
 
-
 TEST_ROOT = settings.COMMON_TEST_DATA_ROOT
 ASSET_NAMES_CSS = 'td.name-col > span.title > a.filename'
 
@@ -26,7 +29,10 @@ def go_to_uploads(_step):
 def upload_file(_step, file_name):
     upload_css = 'a.upload-button'
     world.css_click(upload_css)
-    #uploading the file itself
+
+    _write_test_file(file_name, "test file")
+
+    # uploading the file itself
     path = os.path.join(TEST_ROOT, 'uploads/', file_name)
     world.browser.execute_script("$('input.file-input').css('display', 'block')")
     world.browser.attach_file('file', os.path.abspath(path))
@@ -34,19 +40,20 @@ def upload_file(_step, file_name):
     world.css_click(close_css)
 
 
-@step(u'I upload the files (".*")$')
+@step(u'I upload the files "([^"]*)"$')
 def upload_files(_step, files_string):
-    # Turn files_string to a list of file names
+    # files_string should be comma separated with no spaces.
     files = files_string.split(",")
-    files = map(lambda x: string.strip(x, ' "\''), files)
-
     upload_css = 'a.upload-button'
     world.css_click(upload_css)
-    #uploading the files
-    for f in files:
-        path = os.path.join(TEST_ROOT, 'uploads/', f)
+
+    # uploading the files
+    for filename in files:
+        _write_test_file(filename, "test file")
+        path = os.path.join(TEST_ROOT, 'uploads/', filename)
         world.browser.execute_script("$('input.file-input').css('display', 'block')")
         world.browser.attach_file('file', os.path.abspath(path))
+
     close_css = 'a.close-button'
     world.css_click(close_css)
 
@@ -104,13 +111,13 @@ def check_download(_step, file_name):
         r = get_file(file_name)
         downloaded_text = r.text
         assert cur_text == downloaded_text
-    #resetting the file back to its original state
+    # resetting the file back to its original state
     _write_test_file(file_name, "This is an arbitrary file for testing uploads")
 
 
 def _write_test_file(file_name, text):
     path = os.path.join(TEST_ROOT, 'uploads/', file_name)
-    #resetting the file back to its original state
+    # resetting the file back to its original state
     with open(os.path.abspath(path), 'w') as cur_file:
         cur_file.write(text)
 
@@ -121,68 +128,68 @@ def modify_upload(_step, file_name):
     _write_test_file(file_name, new_text)
 
 
-@step(u'I (lock|unlock) "([^"]*)"$')
-def lock_unlock_file(_step, _lock_state, file_name):
-    index = get_index(file_name)
-    assert index != -1
+@step(u'I upload an asset$')
+def upload_an_asset(step):
+    step.given('I upload the file "asset.html"')
+
+
+@step(u'I (lock|unlock) the asset$')
+def lock_unlock_file(_step, _lock_state):
+    index = get_index('asset.html')
+    assert index != -1, 'Expected to find an asset but could not.'
+
+    # Warning: this is a misnomer, it really only toggles the
+    # lock state. TODO: fix it.
     lock_css = "input.lock-checkbox"
     world.css_find(lock_css)[index].click()
 
 
-@step(u'Then "([^"]*)" is (locked|unlocked)$')
-def verify_lock_unlock_file(_step, file_name, lock_state):
-    index = get_index(file_name)
-    assert index != -1
+@step(u'the user "([^"]*)" is enrolled in the course$')
+def user_foo_is_enrolled_in_the_course(step, name):
+    world.create_user(name, 'test')
+    user = User.objects.get(username=name)
+
+    course_id = world.scenario_dict['COURSE'].location.course_id
+    CourseEnrollment.enroll(user, course_id)
+
+
+@step(u'Then the asset is (locked|unlocked)$')
+def verify_lock_unlock_file(_step, lock_state):
+    index = get_index('asset.html')
+    assert index != -1, 'Expected to find an asset but could not.'
     lock_css = "input.lock-checkbox"
     checked = world.css_find(lock_css)[index]._element.get_attribute('checked')
     assert_equal(lock_state == "locked", bool(checked))
 
 
-@step(u'I have opened a course with a (locked|unlocked) asset "([^"]*)"$')
-def open_course_with_locked(step, lock_state, file_name):
+@step(u'I am at the files and upload page of a Studio course')
+def at_upload_page(step):
     step.given('I have opened a new course in studio')
     step.given('I go to the files and uploads page')
-    _write_test_file(file_name, "test file")
-    step.given('I upload the file "' + file_name + '"')
+
+
+@step(u'I have created a course with a (locked|unlocked) asset$')
+def open_course_with_locked(step, lock_state):
+    step.given('I am at the files and upload page of a Studio course')
+    step.given('I upload the file "asset.html"')
+
     if lock_state == "locked":
-        step.given('I lock "' + file_name + '"')
+        step.given('I lock the asset')
         step.given('I reload the page')
 
 
-@step(u'Then the asset "([^"]*)" is (viewable|protected)$')
-def view_asset(_step, file_name, status):
-    url = '/c4x/MITx/999/asset/' + file_name
+@step(u'Then the asset is (viewable|protected)$')
+def view_asset(_step, status):
+    url = django_url('/c4x/MITx/999/asset/asset.html')
     if status == 'viewable':
-        world.visit(url)
-        _verify_body_text()
+        expected_text = 'test file'
     else:
-        error_thrown = False
-        try:
-            world.visit(url)
-        except Exception as e:
-            assert e.status_code == 403
-            error_thrown = True
-        assert error_thrown
-
-
-@step(u'Then the asset "([^"]*)" can be clicked from the asset index$')
-def click_asset_from_index(step, file_name):
-    # This is not ideal, but I'm having trouble with the middleware not having
-    # the same user in the request when I hit the URL directly.
-    course_link_css = 'a.course-link'
-    world.css_click(course_link_css)
-    step.given("I go to the files and uploads page")
-    index = get_index(file_name)
-    assert index != -1
-    world.css_click('a.filename', index=index)
-    _verify_body_text()
-
-
-def _verify_body_text():
-    def verify_text(driver):
-        return world.css_text('body') == 'test file'
+        expected_text = 'Unauthorized'
 
-    world.wait_for(verify_text)
+    # Note that world.visit would trigger a 403 error instead of displaying "Unauthorized"
+    # Instead, we can drop back into the selenium driver get command.
+    world.browser.driver.get(url)
+    assert_equal(world.css_text('body'),expected_text)
 
 
 @step('I see a confirmation that the file was deleted$')

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

+###
+### Script for editing the course's tabs
+###
+
+#
+# Run it this way:
+#   ./manage.py cms --settings dev edit_course_tabs --course Stanford/CS99/2013_spring
+# Or via rake:
+#   rake django-admin[edit_course_tabs,cms,dev,"--course Stanford/CS99/2013_spring --delete 4"]
+#
+from optparse import make_option
+from django.core.management.base import BaseCommand, CommandError
+from .prompt import query_yes_no
+
+from courseware.courses import get_course_by_id
+
+from contentstore.views import tabs
+
+
+def print_course(course):
+    "Prints out the course id and a numbered list of tabs."
+    print course.id
+    for index, item in enumerate(course.tabs):
+        print index + 1, '"' + item.get('type') + '"', '"' + item.get('name', '') + '"'
+
+
+# course.tabs looks like this
+# [{u'type': u'courseware'}, {u'type': u'course_info', u'name': u'Course Info'}, {u'type': u'textbooks'},
+# {u'type': u'discussion', u'name': u'Discussion'}, {u'type': u'wiki', u'name': u'Wiki'},
+# {u'type': u'progress', u'name': u'Progress'}]
+
+
+class Command(BaseCommand):
+    help = """See and edit a course's tabs list.
+Only supports insertion and deletion. Move and
+rename etc. can be done with a delete
+followed by an insert.
+The tabs are numbered starting with 1.
+Tabs 1 and 2 cannot be changed, and tabs of type
+static_tab cannot be edited (use Studio for those).
+"""
+    # Making these option objects separately, so can refer to their .help below
+    course_option = make_option('--course',
+                                action='store',
+                                dest='course',
+                                default=False,
+                                help='--course <id> required, e.g. Stanford/CS99/2013_spring')
+    delete_option = make_option('--delete',
+                                action='store_true',
+                                dest='delete',
+                                default=False,
+                                help='--delete <tab-number>')
+    insert_option = make_option('--insert',
+                                action='store_true',
+                                dest='insert',
+                                default=False,
+                                help='--insert <tab-number> <type> <name>, e.g. 2 "course_info" "Course Info"')
+
+    option_list = BaseCommand.option_list + (course_option, delete_option, insert_option)
+
+    def handle(self, *args, **options):
+        if not options['course']:
+            raise CommandError(Command.course_option.help)
+
+        course = get_course_by_id(options['course'])
+
+        print 'Warning: this command directly edits the list of course tabs in mongo.'
+        print 'Tabs before any changes:'
+        print_course(course)
+
+        try:
+            if options['delete']:
+                if len(args) != 1:
+                    raise CommandError(Command.delete_option.help)
+                num = int(args[0])
+                if query_yes_no('Deleting tab {0} Confirm?'.format(num), default='no'):
+                    tabs.primitive_delete(course, num - 1)  # -1 for 0-based indexing
+            elif options['insert']:
+                if len(args) != 3:
+                    raise CommandError(Command.insert_option.help)
+                num = int(args[0])
+                tab_type = args[1]
+                name = args[2]
+                if query_yes_no('Inserting tab {0} "{1}" "{2}" Confirm?'.format(num, tab_type, name), default='no'):
+                    tabs.primitive_insert(course, num - 1, tab_type, name)  # -1 as above
+        except ValueError as e:
+            # Cute: translate to CommandError so the CLI error prints nicely.
+            raise CommandError(e)

cms/djangoapps/contentstore/tests/test_tabs.py

+""" Tests for tab functions (just primitive). """
+
+from contentstore.views import tabs
+from django.test import TestCase
+from xmodule.modulestore.tests.factories import CourseFactory
+from courseware.courses import get_course_by_id
+
+
+class PrimitiveTabEdit(TestCase):
+    """Tests for the primitive tab edit data manipulations"""
+
+    def test_delete(self):
+        """Test primitive tab deletion."""
+        course = CourseFactory.create(org='edX', course='999')
+        with self.assertRaises(ValueError):
+            tabs.primitive_delete(course, 0)
+        with self.assertRaises(ValueError):
+            tabs.primitive_delete(course, 1)
+        with self.assertRaises(IndexError):
+            tabs.primitive_delete(course, 6)
+        tabs.primitive_delete(course, 2)
+        self.assertFalse({u'type': u'textbooks'} in course.tabs)
+        # Check that discussion has shifted down
+        self.assertEquals(course.tabs[2], {'type': 'discussion', 'name': 'Discussion'})
+
+    def test_insert(self):
+        """Test primitive tab insertion."""
+        course = CourseFactory.create(org='edX', course='999')
+        tabs.primitive_insert(course, 2, 'atype', 'aname')
+        self.assertEquals(course.tabs[2], {'type': 'atype', 'name': 'aname'})
+        with self.assertRaises(ValueError):
+            tabs.primitive_insert(course, 0, 'atype', 'aname')
+        with self.assertRaises(ValueError):
+            tabs.primitive_insert(course, 3, 'static_tab', 'aname')
+
+    def test_save(self):
+        """Test course saving."""
+        course = CourseFactory.create(org='edX', course='999')
+        tabs.primitive_insert(course, 3, 'atype', 'aname')
+        course2 = get_course_by_id(course.id)
+        self.assertEquals(course2.tabs[3], {'type': 'atype', 'name': 'aname'})

cms/djangoapps/contentstore/views/tabs.py

@@ -9,13 +9,14 @@ from django.contrib.auth.decorators import login_required
 from django.core.exceptions import PermissionDenied
 from django_future.csrf import ensure_csrf_cookie
 from mitxmako.shortcuts import render_to_response
-
 from xmodule.modulestore import Location
 from xmodule.modulestore.inheritance import own_metadata
 from xmodule.modulestore.django import modulestore
+
 from ..utils import get_course_for_item, get_modulestore
 from .access import get_location_and_verify_access
 
+
 __all__ = ['edit_tabs', 'reorder_static_tabs', 'static_pages']
 
 
@@ -84,6 +85,7 @@ def reorder_static_tabs(request):
     # MongoKeyValueStore before we update the mongo datastore.
     course.save()
     modulestore('direct').update_metadata(course.location, own_metadata(course))
+    # TODO: above two lines are used for the primitive-save case. Maybe factor them out?
     return HttpResponse()
 
 
@@ -136,3 +138,43 @@ def static_pages(request, org, course, coursename):
     return render_to_response('static-pages.html', {
         'context_course': course,
     })
+
+
+# "primitive" tab edit functions driven by the command line.
+# These should be replaced/deleted by a more capable GUI someday.
+# Note that the command line UI identifies the tabs with 1-based
+# indexing, but this implementation code is standard 0-based.
+
+def validate_args(num, tab_type):
+    "Throws for the disallowed cases."
+    if num <= 1:
+        raise ValueError('Tabs 1 and 2 cannot be edited')
+    if tab_type == 'static_tab':
+        raise ValueError('Tabs of type static_tab cannot be edited here (use Studio)')
+
+
+def primitive_delete(course, num):
+    "Deletes the given tab number (0 based)."
+    tabs = course.tabs
+    validate_args(num, tabs[num].get('type', ''))
+    del tabs[num]
+    # Note for future implementations: if you delete a static_tab, then Chris Dodge
+    # points out that there's other stuff to delete beyond this element.
+    # This code happens to not delete static_tab so it doesn't come up.
+    primitive_save(course)
+
+
+def primitive_insert(course, num, tab_type, name):
+    "Inserts a new tab at the given number (0 based)."
+    validate_args(num, tab_type)
+    new_tab = {u'type': unicode(tab_type), u'name': unicode(name)}
+    tabs = course.tabs
+    tabs.insert(num, new_tab)
+    primitive_save(course)
+
+
+def primitive_save(course):
+    "Saves the course back to modulestore."
+    # This code copied from reorder_static_tabs above
+    course.save()
+    modulestore('direct').update_metadata(course.location, own_metadata(course))

cms/envs/common.py

@@ -31,6 +31,7 @@ from path import path
 from lms.xblock.mixin import LmsBlockMixin
 from cms.xmodule_namespace import CmsBlockMixin
 from xmodule.modulestore.inheritance import InheritanceMixin
+from xmodule.x_module import XModuleMixin
 
 ############################ FEATURE CONFIGURATION #############################
 
@@ -168,7 +169,7 @@ MIDDLEWARE_CLASSES = (
 
 # This should be moved into an XBlock Runtime/Application object
 # once the responsibility of XBlock creation is moved out of modulestore - cpennington
-XBLOCK_MIXINS = (LmsBlockMixin, CmsBlockMixin, InheritanceMixin)
+XBLOCK_MIXINS = (LmsBlockMixin, CmsBlockMixin, InheritanceMixin, XModuleMixin)
 
 
 ############################ SIGNAL HANDLERS ################################

cms/envs/test.py

@@ -20,6 +20,16 @@ from warnings import filterwarnings
 # Nose Test Runner
 TEST_RUNNER = 'django_nose.NoseTestSuiteRunner'
 
+_system = 'cms'
+_report_dir = REPO_ROOT / 'reports' / _system
+_report_dir.makedirs_p()
+
+NOSE_ARGS = [
+    '--tests', PROJECT_ROOT / 'djangoapps', COMMON_ROOT / 'djangoapps',
+    '--id-file', REPO_ROOT / '.testids' / _system / 'noseids',
+    '--xunit-file', _report_dir / 'nosetests.xml',
+]
+
 TEST_ROOT = path('test_root')
 
 # Want static files in the same dir for running on jenkins.

common/djangoapps/external_auth/views.py

@@ -12,7 +12,7 @@ from external_auth.models import ExternalAuthMap
 from external_auth.djangostore import DjangoOpenIDStore
 
 from django.conf import settings
-from django.contrib.auth import REDIRECT_FIELD_NAME, authenticate, login, logout
+from django.contrib.auth import REDIRECT_FIELD_NAME, authenticate, login
 from django.contrib.auth.models import User
 from django.core.urlresolvers import reverse
 from django.core.validators import validate_email
@@ -45,9 +45,6 @@ from openid.extensions import ax, sreg
 from ratelimitbackend.exceptions import RateLimitException
 
 import student.views
-# Required for Pearson
-from courseware.views import get_module_for_descriptor, jump_to
-from courseware.model_data import FieldDataCache
 from xmodule.modulestore.django import modulestore
 from xmodule.course_module import CourseDescriptor
 from xmodule.modulestore import Location
@@ -238,6 +235,7 @@ def _flatten_to_ascii(txt):
     else:
         return unicode(unicodedata.normalize('NFKD', txt).encode('ASCII', 'ignore'))
 
+
 @ensure_csrf_cookie
 def _signup(request, eamap):
     """
@@ -896,12 +894,17 @@ def test_center_login(request):
     ''' Log in students taking exams via Pearson
 
     Takes a POST request that contains the following keys:
-        - code - a security code provided by  Pearson
+        - code - a security code provided by Pearson
         - clientCandidateID
         - registrationID
         - exitURL - the url that we redirect to once we're done
         - vueExamSeriesCode - a code that indicates the exam that we're using
     '''
+    # Imports from lms/djangoapps/courseware -- these should not be
+    # in a common djangoapps.
+    from courseware.views import get_module_for_descriptor, jump_to
+    from courseware.model_data import FieldDataCache
+
     # errors are returned by navigating to the error_url, adding a query parameter named "code"
     # which contains the error code describing the exceptional condition.
     def makeErrorURL(error_url, error_code):

common/djangoapps/terrain/course_helpers.py

@@ -2,17 +2,10 @@
 # pylint: disable=W0621
 
 from lettuce import world
-from .factories import *
-from django.conf import settings
-from django.http import HttpRequest
 from django.contrib.auth.models import User
-from django.contrib.auth import authenticate, login
-from django.contrib.auth.middleware import AuthenticationMiddleware
-from django.contrib.sessions.middleware import SessionMiddleware
 from student.models import CourseEnrollment
 from xmodule.modulestore.django import editable_modulestore
 from xmodule.contentstore.django import contentstore
-from urllib import quote_plus
 
 
 @world.absorb
@@ -22,7 +15,7 @@ def create_user(uname, password):
     if len(User.objects.filter(username=uname)) > 0:
         return
 
-    portal_user = UserFactory.build(username=uname, email=uname + '@edx.org')
+    portal_user = world.UserFactory.build(username=uname, email=uname + '@edx.org')
     portal_user.set_password(password)
     portal_user.save()
 
@@ -30,7 +23,7 @@ def create_user(uname, password):
     registration.register(portal_user)
     registration.activate()
 
-    user_profile = world.UserProfileFactory(user=portal_user)
+    world.UserProfileFactory(user=portal_user)
 
 
 @world.absorb

common/lib/xmodule/setup.py

 from setuptools import setup, find_packages
 
+XMODULES = [
+    "abtest = xmodule.abtest_module:ABTestDescriptor",
+    "book = xmodule.backcompat_module:TranslateCustomTagDescriptor",
+    "chapter = xmodule.seq_module:SequenceDescriptor",
+    "combinedopenended = xmodule.combined_open_ended_module:CombinedOpenEndedDescriptor",
+    "conditional = xmodule.conditional_module:ConditionalDescriptor",
+    "course = xmodule.course_module:CourseDescriptor",
+    "customtag = xmodule.template_module:CustomTagDescriptor",
+    "discuss = xmodule.backcompat_module:TranslateCustomTagDescriptor",
+    "html = xmodule.html_module:HtmlDescriptor",
+    "image = xmodule.backcompat_module:TranslateCustomTagDescriptor",
+    "error = xmodule.error_module:ErrorDescriptor",
+    "peergrading = xmodule.peer_grading_module:PeerGradingDescriptor",
+    "poll_question = xmodule.poll_module:PollDescriptor",
+    "problem = xmodule.capa_module:CapaDescriptor",
+    "problemset = xmodule.seq_module:SequenceDescriptor",
+    "randomize = xmodule.randomize_module:RandomizeDescriptor",
+    "section = xmodule.backcompat_module:SemanticSectionDescriptor",
+    "sequential = xmodule.seq_module:SequenceDescriptor",
+    "slides = xmodule.backcompat_module:TranslateCustomTagDescriptor",
+    "timelimit = xmodule.timelimit_module:TimeLimitDescriptor",
+    "vertical = xmodule.vertical_module:VerticalDescriptor",
+    "video = xmodule.video_module:VideoDescriptor",
+    "videoalpha = xmodule.video_module:VideoDescriptor",
+    "videodev = xmodule.backcompat_module:TranslateCustomTagDescriptor",
+    "videosequence = xmodule.seq_module:SequenceDescriptor",
+    "discussion = xmodule.discussion_module:DiscussionDescriptor",
+    "course_info = xmodule.html_module:CourseInfoDescriptor",
+    "static_tab = xmodule.html_module:StaticTabDescriptor",
+    "custom_tag_template = xmodule.raw_module:RawDescriptor",
+    "about = xmodule.html_module:AboutDescriptor",
+    "wrapper = xmodule.wrapper_module:WrapperDescriptor",
+    "graphical_slider_tool = xmodule.gst_module:GraphicalSliderToolDescriptor",
+    "annotatable = xmodule.annotatable_module:AnnotatableDescriptor",
+    "foldit = xmodule.foldit_module:FolditDescriptor",
+    "word_cloud = xmodule.word_cloud_module:WordCloudDescriptor",
+    "hidden = xmodule.hidden_module:HiddenDescriptor",
+    "raw = xmodule.raw_module:RawDescriptor",
+    "crowdsource_hinter = xmodule.crowdsource_hinter:CrowdsourceHinterDescriptor",
+    "lti = xmodule.lti_module:LTIModuleDescriptor",
+]
+
 setup(
     name="XModule",
     version="0.1",
@@ -11,55 +53,16 @@ setup(
         'path.py',
     ],
     package_data={
-        'xmodule': ['js/module/*']
+        'xmodule': ['js/module/*'],
     },
 
     # See http://guide.python-distribute.org/creation.html#entry-points
     # for a description of entry_points
     entry_points={
-        'xmodule.v1': [
-            "abtest = xmodule.abtest_module:ABTestDescriptor",
-            "book = xmodule.backcompat_module:TranslateCustomTagDescriptor",
-            "chapter = xmodule.seq_module:SequenceDescriptor",
-            "combinedopenended = xmodule.combined_open_ended_module:CombinedOpenEndedDescriptor",
-            "conditional = xmodule.conditional_module:ConditionalDescriptor",
-            "course = xmodule.course_module:CourseDescriptor",
-            "customtag = xmodule.template_module:CustomTagDescriptor",
-            "discuss = xmodule.backcompat_module:TranslateCustomTagDescriptor",
-            "html = xmodule.html_module:HtmlDescriptor",
-            "image = xmodule.backcompat_module:TranslateCustomTagDescriptor",
-            "error = xmodule.error_module:ErrorDescriptor",
-            "peergrading = xmodule.peer_grading_module:PeerGradingDescriptor",
-            "poll_question = xmodule.poll_module:PollDescriptor",
-            "problem = xmodule.capa_module:CapaDescriptor",
-            "problemset = xmodule.seq_module:SequenceDescriptor",
-            "randomize = xmodule.randomize_module:RandomizeDescriptor",
-            "section = xmodule.backcompat_module:SemanticSectionDescriptor",
-            "sequential = xmodule.seq_module:SequenceDescriptor",
-            "slides = xmodule.backcompat_module:TranslateCustomTagDescriptor",
-            "timelimit = xmodule.timelimit_module:TimeLimitDescriptor",
-            "vertical = xmodule.vertical_module:VerticalDescriptor",
-            "video = xmodule.video_module:VideoDescriptor",
-            "videoalpha = xmodule.video_module:VideoDescriptor",
-            "videodev = xmodule.backcompat_module:TranslateCustomTagDescriptor",
-            "videosequence = xmodule.seq_module:SequenceDescriptor",
-            "discussion = xmodule.discussion_module:DiscussionDescriptor",
-            "course_info = xmodule.html_module:CourseInfoDescriptor",
-            "static_tab = xmodule.html_module:StaticTabDescriptor",
-            "custom_tag_template = xmodule.raw_module:RawDescriptor",
-            "about = xmodule.html_module:AboutDescriptor",
-            "wrapper = xmodule.wrapper_module:WrapperDescriptor",
-            "graphical_slider_tool = xmodule.gst_module:GraphicalSliderToolDescriptor",
-            "annotatable = xmodule.annotatable_module:AnnotatableDescriptor",
-            "foldit = xmodule.foldit_module:FolditDescriptor",
-            "word_cloud = xmodule.word_cloud_module:WordCloudDescriptor",
-            "hidden = xmodule.hidden_module:HiddenDescriptor",
-            "raw = xmodule.raw_module:RawDescriptor",
-            "crowdsource_hinter = xmodule.crowdsource_hinter:CrowdsourceHinterDescriptor",
-            "lti = xmodule.lti_module:LTIModuleDescriptor"
-        ],
+        'xblock.v1': XMODULES,
+        'xmodule.v1': XMODULES,
         'console_scripts': [
             'xmodule_assets = xmodule.static_content:main',
-        ]
-    }
+        ],
+    },
 )

common/lib/xmodule/xmodule/combined_open_ended_module.py

@@ -306,7 +306,7 @@ class CombinedOpenEndedFields(object):
     )
     peer_grade_finished_submissions_when_none_pending = Boolean(
         display_name='Allow "overgrading" of peer submissions',
-        help=("Allow students to peer grade submissions that already have the requisite number of graders, "
+        help=("EXPERIMENTAL FEATURE.  Allow students to peer grade submissions that already have the requisite number of graders, "
               "but ONLY WHEN all submissions they are eligible to grade already have enough graders.  "
               "This is intended for use when settings for `Required Peer Grading` > `Peer Graders per Response`"),
         default=False,

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

@@ -136,7 +136,7 @@ div.video {
 
           &:focus, &:hover {
             background-color: lighten($pink, 10%);
-            outline: none;
+            outline: 0;
           }
         }
       }
@@ -162,9 +162,16 @@ div.video {
             text-indent: -9999px;
             width: 14px;
             background: url('../images/vcr.png') 15px 15px no-repeat;
-            outline: 0;
 
             &:focus {
+              position: relative;
+              z-index: 10000;
+              outline: #fff dotted thin;
+              outline-offset: -2px;
+              background: #333;
+            }
+
+            &:hover {
               outline: 0;
             }
 
@@ -176,7 +183,7 @@ div.video {
             &.play {
               background-position: 17px -114px;
 
-              &:hover, &:focus {
+              &:hover {
                 background-color: #444;
               }
             }
@@ -184,7 +191,7 @@ div.video {
             &.pause {
               background-position: 16px -50px;
 
-              &:hover, &:focus {
+              &:hover {
                 background-color: #444;
               }
             }
@@ -203,6 +210,19 @@ div.video {
       div.secondary-controls {
         float: right;
 
+        div.speeds>a, div.volume>a, a.add-fullscreen, a.quality_control,
+        a.hide-subtitles {
+          // overflow is used to bypass Firefox CSS :focus outline bug
+          // http://johndoesdesign.com/blog/2012/css/firefox-and-its-css-focus-outline-bug/
+          &:focus {
+            position: relative;
+            z-index: 10000;
+            outline: #fff dotted thin;
+            outline-offset: -2px;
+            overflow: auto;
+          }
+        }
+
         div.speeds {
           float: left;
           position: relative;
@@ -250,10 +270,15 @@ div.video {
               }
             }
 
-            outline: 0;
-
-            &:focus {
+            &:hover {
               outline: 0;
+              opacity: 1.0;
+              background-color: #444;
+            }
+
+            &:active {
+              opacity: 1.0;
+              background-color: #444;
             }
 
             h3 {
@@ -280,11 +305,6 @@ div.video {
               line-height: 46px;
               color: #fff;
             }
-
-            &:hover, &:active, &:focus {
-              opacity: 1.0;
-              background-color: #444;
-            }
           }
 
           // fix for now
@@ -320,6 +340,7 @@ div.video {
                 &:hover {
                   background-color: #666;
                   color: #aaa;
+                  outline-offset: -4px;
                 }
               }
 
@@ -371,9 +392,12 @@ div.video {
             @include transition(none);
             -webkit-font-smoothing: antialiased;
             width: 30px;
-
-            &:hover, &:active, &:focus {
+          
+            &:hover, &:active {
               background-color: #444;
+              color: #fff;
+              text-decoration: none;
+              outline: 0;
             }
           }
 
@@ -433,14 +457,16 @@ div.video {
           text-indent: -9999px;
           @include transition(none);
           width: 30px;
-
-          &:hover, &:active, &:focus {
+          
+          &:hover, &:active {
             background-color: #444;
             color: #fff;
             text-decoration: none;
+            outline: 0;
           }
         }
 
+
         a.quality_control {
           background: url(../images/hd.png) center no-repeat;
           border-right: 1px solid #000;
@@ -455,16 +481,18 @@ div.video {
           @include transition(none);
           width: 30px;
 
-          &:hover, &:focus {
+          &:hover {
             background-color: #444;
             color: #fff;
             text-decoration: none;
+            outline: 0;
           }
 
           &.active {
             background-color: #F44;
             color: #0ff;
             text-decoration: none;
+            outline: 0;
           }
         }
 
@@ -483,10 +511,11 @@ div.video {
           -webkit-font-smoothing: antialiased;
           width: 30px;
 
-          &:hover, &:focus {
+          &:hover {
             background-color: #444;
             color: #fff;
             text-decoration: none;
+            outline: 0;
           }
 
           &.off {
@@ -530,8 +559,7 @@ div.video {
       margin-bottom: 8px;
       padding: 0;
       line-height: lh();
-      outline-width: 0px;
-      outline-style: none;
+      outline: 0;
 
       &.current {
         color: #333;
@@ -539,8 +567,8 @@ div.video {
       }
 
       &.focused {
-        outline-width: 1px;
-        outline-style: dotted;
+        outline: #000 dotted thin;
+        outline-offset: -1px;
       }
 
       &:hover {

common/lib/xmodule/xmodule/error_module.py

@@ -105,10 +105,10 @@ class ErrorDescriptor(ErrorFields, XModuleDescriptor):
         })
         return system.construct_xblock_from_class(
             cls,
-            field_data,
             # The error module doesn't use scoped data, and thus doesn't need
             # real scope keys
-            ScopeIds('error', None, location, location)
+            ScopeIds('error', None, location, location),
+            field_data,
         )
 
     def get_context(self):

common/lib/xmodule/xmodule/js/fixtures/video.html

@@ -26,26 +26,26 @@
               <div class="slider"></div>
               <div>
                 <ul class="vcr">
-                  <li><a class="video_control" href="#" title="Play"></a></li>
+                  <li><a class="video_control" href="#" title="Play" role="button" aria-disabled="false"></a></li>
                   <li><div class="vidtime">0:00 / 0:00</div></li>
                 </ul>
                 <div class="secondary-controls">
                   <div class="speeds">
-                    <a href="#">
+                    <a href="#" title="Speeds" role="button" aria-disabled="false">
                       <h3>Speed</h3>
                       <p class="active"></p>
                     </a>
                     <ol class="video_speeds"></ol>
                   </div>
                   <div class="volume">
-                    <a href="#"></a>
+                    <a href="#" title="Volume" role="button" aria-disabled="false"></a>
                     <div class="volume-slider-container">
                       <div class="volume-slider"></div>
                     </div>
                   </div>
-                  <a href="#" class="add-fullscreen" title="Fill browser">Fill Browser</a>
-                  <a href="#" class="quality_control" title="HD">HD</a>
-                  <a href="#" class="hide-subtitles" title="Turn off captions">Captions</a>
+                  <a href="#" class="add-fullscreen" title="Fill browser" role="button" aria-disabled="false">Fill Browser</a>
+                  <a href="#" class="quality_control" title="HD" role="button" aria-disabled="false">HD</a>
+                  <a href="#" class="hide-subtitles" title="Turn off captions" role="button" aria-disabled="false">Captions</a>
                 </div>
               </div>
             </section>

common/lib/xmodule/xmodule/js/fixtures/video_all.html

@@ -29,26 +29,26 @@
               <div class="slider"></div>
               <div>
                 <ul class="vcr">
-                  <li><a class="video_control" href="#" title="Play"></a></li>
+                  <li><a class="video_control" href="#" title="Play" role="button" aria-disabled="false"></a></li>
                   <li><div class="vidtime">0:00 / 0:00</div></li>
                 </ul>
                 <div class="secondary-controls">
                   <div class="speeds">
-                    <a href="#">
+                    <a href="#" title="Speeds" role="button" aria-disabled="false">>
                       <h3>Speed</h3>
                       <p class="active"></p>
                     </a>
                     <ol class="video_speeds"></ol>
                   </div>
                   <div class="volume">
-                    <a href="#"></a>
+                    <a href="#" title="Volume" role="button" aria-disabled="false"></a>
                     <div class="volume-slider-container">
                       <div class="volume-slider"></div>
                     </div>
                   </div>
-                  <a href="#" class="add-fullscreen" title="Fill browser">Fill Browser</a>
-                  <a href="#" class="quality_control" title="HD">HD</a>
-                  <a href="#" class="hide-subtitles" title="Turn off captions">Captions</a>
+                  <a href="#" class="add-fullscreen" title="Fill browser" role="button" aria-disabled="false">Fill Browser</a>
+                  <a href="#" class="quality_control" title="HD" role="button" aria-disabled="false">HD</a>
+                  <a href="#" class="hide-subtitles" title="Turn off captions" role="button" aria-disabled="false">Captions</a>
                 </div>
               </div>
             </section>

common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html

@@ -26,26 +26,26 @@
               <div class="slider"></div>
               <div>
                 <ul class="vcr">
-                  <li><a class="video_control" href="#" title="Play"></a></li>
+                  <li><a class="video_control" href="#" title="Play" role="button" aria-disabled="false"></a></li>
                   <li><div class="vidtime">0:00 / 0:00</div></li>
                 </ul>
                 <div class="secondary-controls">
                   <div class="speeds">
-                    <a href="#">
+                    <a href="#" title="Speeds" role="button" aria-disabled="false">
                       <h3>Speed</h3>
                       <p class="active"></p>
                     </a>
                     <ol class="video_speeds"></ol>
                   </div>
                   <div class="volume">
-                    <a href="#"></a>
+                    <a href="#" title="Volume" role="button" aria-disabled="false"></a>
                     <div class="volume-slider-container">
                       <div class="volume-slider"></div>
                     </div>
                   </div>
-                  <a href="#" class="add-fullscreen" title="Fill browser">Fill Browser</a>
-                  <a href="#" class="quality_control" title="HD">HD</a>
-                  <a href="#" class="hide-subtitles" title="Turn off captions">Captions</a>
+                  <a href="#" class="add-fullscreen" title="Fill browser" role="button" aria-disabled="false">Fill Browser</a>
+                  <a href="#" class="quality_control" title="HD" role="button" aria-disabled="false">HD</a>
+                  <a href="#" class="hide-subtitles" title="Turn off captions" role="button" aria-disabled="false">Captions</a>
                 </div>
               </div>
             </section>

common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js

@@ -261,7 +261,7 @@
 
     describe('search', function() {
       it('return a correct caption index', function() {
-        expect(videoCaption.search(0)).toEqual(0);
+        expect(videoCaption.search(0)).toEqual(-1);
         expect(videoCaption.search(3120)).toEqual(1);
         expect(videoCaption.search(6270)).toEqual(2);
         expect(videoCaption.search(8490)).toEqual(2);

common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js

@@ -547,7 +547,7 @@
         });
 
         it('replace the full screen button tooltip', function() {
-          expect($('.add-fullscreen')).toHaveAttr('title', 'Exit fullscreen');
+          expect($('.add-fullscreen')).toHaveAttr('title', 'Exit full browser');
         });
 
         it('add the video-fullscreen class', function() {
@@ -573,7 +573,7 @@
         });
 
         it('replace the full screen button tooltip', function() {
-          expect($('.add-fullscreen')).toHaveAttr('title', 'Fullscreen');
+          expect($('.add-fullscreen')).toHaveAttr('title', 'Fill browser');
         });
 
         it('remove the video-fullscreen class', function() {

common/lib/xmodule/xmodule/js/spec/video/video_quality_control_spec.js

@@ -24,7 +24,8 @@
         initialize();
       });
 
-      it('render the quality control', function() {
+      // Disabled when ARIA markup was added to the anchor
+      xit('render the quality control', function() {
         expect(videoControl.secondaryControlsEl.html()).toContain("<a href=\"#\" class=\"quality_control\" title=\"HD\">");
       });
 

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

@@ -353,7 +353,7 @@ class @CombinedOpenEnded
       @save_button.attr("disabled",true)
       $.postWithPrefix "#{@ajax_url}/store_answer", data, (response) =>
         if response.success
-          @gentle_alert("Answer saved.")
+          @gentle_alert("Answer saved, but not yet submitted.")
         else
           @errors_area.html(response.error)
         @save_button.attr("disabled",false)
@@ -372,7 +372,8 @@ class @CombinedOpenEnded
       answer_area_div = @$(@answer_area_div_sel)
       answer_area_div.html(response.student_response)
     else
-      @can_upload_files = pre_can_upload_files
+      @submit_button.show()
+      @submit_button.attr('disabled', false)
       @gentle_alert response.error
 
   confirm_save_answer: (event) =>
@@ -385,23 +386,27 @@ class @CombinedOpenEnded
     event.preventDefault()
     @answer_area.attr("disabled", true)
     max_filesize = 2*1000*1000 #2MB
-    pre_can_upload_files = @can_upload_files
     if @child_state == 'initial'
       files = ""
+      valid_files_attached = false
       if @can_upload_files == true
         files = @$(@file_upload_box_sel)[0].files[0]
         if files != undefined
+          valid_files_attached = true
           if files.size > max_filesize
-            @can_upload_files = false
             files = ""
-        else
-          @can_upload_files = false
+            # Don't submit the file in the case of it being too large, deal with the error locally.
+            @submit_button.show()
+            @submit_button.attr('disabled', false)
+            @gentle_alert "You are trying to upload a file that is too large for our system.  Please choose a file under 2MB or paste a link to it into the answer box."
+            return
 
       fd = new FormData()
       fd.append('student_answer', @answer_area.val())
       fd.append('student_file', files)
-      fd.append('can_upload_files', @can_upload_files)
+      fd.append('valid_files_attached', valid_files_attached)
 
+      that=this
       settings =
         type: "POST"
         data: fd

common/lib/xmodule/xmodule/js/src/peergrading/track_changes.coffee

@@ -56,7 +56,7 @@ class @TrackChanges
       key = parseInt(@attr('data-cid'))
       if key > keyOfLatestChange
         keyOfLatestChange = key
-    ICEtracker.rejectChange('[data-cid="'+ keyOfLatestChange + '"]')
+    @tracker.rejectChange('[data-cid="'+ keyOfLatestChange + '"]')
 
   stop_tracking_on_submit: () =>
     @tracker.stopTracking()
\ No newline at end of file

common/lib/xmodule/xmodule/js/src/video/04_video_control.js

@@ -63,6 +63,14 @@ function () {
             state.videoControl.el.addClass('html5');
             state.controlHideTimeout = setTimeout(state.videoControl.hideControls, state.videoControl.fadeOutTimeout);
         }
+
+        // ARIA
+        // Let screen readers know that this anchor, representing the slider
+        // handle, behaves as a slider named 'video slider'.
+        state.videoControl.sliderEl.find('.ui-slider-handle').attr({
+            'role': 'slider',
+            'title': gettext('video slider')
+        });
     }
 
     // function _bindHandlers(state)
@@ -168,12 +176,14 @@ function () {
             this.videoControl.fullScreenState = false;
             fullScreenClassNameEl.removeClass('video-fullscreen');
             this.isFullScreen = false;
-            this.videoControl.fullScreenEl.attr('title', gettext('Fullscreen'));
+            this.videoControl.fullScreenEl.attr('title', gettext('Fill browser'))
+                                          .text(gettext('Fill browser'));
         } else {
             this.videoControl.fullScreenState = true;
             fullScreenClassNameEl.addClass('video-fullscreen');
             this.isFullScreen = true;
-            this.videoControl.fullScreenEl.attr('title', gettext('Exit fullscreen'));
+            this.videoControl.fullScreenEl.attr('title', gettext('Exit full browser'))
+                                          .text(gettext('Exit full browser'));
         }
 
         this.trigger('videoCaption.resize', null);

common/lib/xmodule/xmodule/js/src/video/06_video_progress_slider.js

@@ -54,6 +54,18 @@ function () {
 
     function _buildHandle(state) {
         state.videoProgressSlider.handle = state.videoProgressSlider.el.find('.ui-slider-handle');
+        
+        // ARIA
+        // We just want the knob to be selectable with keyboard
+        state.videoProgressSlider.el.attr('tabindex', -1);
+        // Let screen readers know that this anchor, representing the slider
+        // handle, behaves as a slider named 'video position'.
+        state.videoProgressSlider.handle.attr({
+            'role': 'slider',
+            'title': 'video position',
+            'aria-disabled': false,
+            'aria-valuetext': getTimeDescription(state.videoProgressSlider.slider.slider('option', 'value'))
+        });
     }
 
     // ***************************************************************
@@ -74,6 +86,11 @@ function () {
         this.videoProgressSlider.frozen = true;
 
         this.trigger('videoPlayer.onSlideSeek', {'type': 'onSlideSeek', 'time': ui.value});
+
+        // ARIA
+        this.videoProgressSlider.handle.attr(
+            'aria-valuetext', getTimeDescription(this.videoPlayer.currentTime)
+        );
     }
 
     function onStop(event, ui) {
@@ -83,6 +100,11 @@ function () {
 
         this.trigger('videoPlayer.onSlideSeek', {'type': 'onSlideSeek', 'time': ui.value});
 
+        // ARIA
+        this.videoProgressSlider.handle.attr(
+            'aria-valuetext', getTimeDescription(this.videoPlayer.currentTime)
+        );
+
         setTimeout(function() {
             _this.videoProgressSlider.frozen = false;
         }, 200);
@@ -99,6 +121,48 @@ function () {
         }
     }
 
+    // Returns a string describing the current time of video in hh:mm:ss format.
+    function getTimeDescription(time) {
+        var seconds = Math.floor(time),
+            minutes = Math.floor(seconds / 60),
+            hours = Math.floor(minutes / 60),
+            hrStr, minStr, secStr;
+        seconds = seconds % 60;
+        minutes = minutes % 60;
+
+        hrStr = hours.toString(10);
+        minStr = minutes.toString(10);
+        secStr = seconds.toString(10);
+
+        if (hours) {
+            hrStr += (hours < 2 ? ' hour ' : ' hours ');
+            if (minutes) {  
+                minStr += (minutes < 2 ? ' minute ' : ' minutes ');
+            } else {
+                minStr += ' 0 minutes ';
+            }
+            if (seconds) {   
+                secStr += (seconds < 2 ? ' second ' : ' seconds ');
+            } else {
+                secStr += ' 0 seconds ';
+            }    
+            return hrStr + minStr + secStr;
+        } else if (minutes) {
+            minStr += (minutes < 2 ? ' minute ' : ' minutes ');
+            if (seconds) {   
+                secStr += (seconds < 2 ? ' second ' : ' seconds ');
+            } else {
+                secStr += ' 0 seconds ';
+            }
+            return minStr + secStr;
+        } else if (seconds) {
+            secStr += (seconds < 2 ? ' second ' : ' seconds ');
+            return secStr;
+        }
+        
+        return '0 seconds';
+    }
+
 });
 
 }(RequireJS.requirejs, RequireJS.require, RequireJS.define));

common/lib/xmodule/xmodule/js/src/video/07_video_volume_control.js

@@ -62,6 +62,35 @@ function () {
         });
 
         state.videoVolumeControl.el.toggleClass('muted', state.videoVolumeControl.currentVolume === 0);
+
+        // ARIA
+        // Let screen readers know that:
+
+        // This anchor behaves as a button named 'Volume'.
+        var buttonStr = gettext(
+                state.videoVolumeControl.currentVolume === 0
+                ? 'Volume muted'
+                : 'Volume'
+            );
+        // We add the aria-label attribute because the title attribute cannot be
+        // read. 
+        state.videoVolumeControl.buttonEl.attr('aria-label', buttonStr);
+
+        // Let screen readers know that this anchor, representing the slider
+        // handle, behaves as a slider named 'volume'.
+        var volumeSlider = state.videoVolumeControl.slider;
+        state.videoVolumeControl.volumeSliderHandleEl = state.videoVolumeControl
+                                                             .volumeSliderEl
+                                                             .find('.ui-slider-handle');
+        state.videoVolumeControl.volumeSliderHandleEl.attr({
+            'role': 'slider',
+            'title': 'volume',
+            'aria-disabled': false,
+            'aria-valuemin': volumeSlider.slider('option', 'min'),
+            'aria-valuemax': volumeSlider.slider('option', 'max'),
+            'aria-valuenow': volumeSlider.slider('option', 'value'),
+            'aria-valuetext': getVolumeDescription(volumeSlider.slider('option', 'value'))
+        });
     }
 
     /**
@@ -147,6 +176,18 @@ function () {
         });
 
         this.trigger('videoPlayer.onVolumeChange', ui.value);
+        
+        // ARIA
+        this.videoVolumeControl.volumeSliderHandleEl.attr({
+            'aria-valuenow': ui.value,
+            'aria-valuetext': getVolumeDescription(ui.value)
+        });
+       
+        this.videoVolumeControl.buttonEl.attr(
+            'aria-label', this.videoVolumeControl.currentVolume === 0
+                          ? gettext('Volume muted')
+                          : gettext('Volume')
+        );
     }
 
     function toggleMute(event) {
@@ -155,9 +196,39 @@ function () {
         if (this.videoVolumeControl.currentVolume > 0) {
             this.videoVolumeControl.previousVolume = this.videoVolumeControl.currentVolume;
             this.videoVolumeControl.slider.slider('option', 'value', 0);
+            // ARIA
+            this.videoVolumeControl.volumeSliderHandleEl.attr({
+                'aria-valuenow': 0,
+                'aria-valuetext': getVolumeDescription(0),
+            });
         } else {
             this.videoVolumeControl.slider.slider('option', 'value', this.videoVolumeControl.previousVolume);
+            // ARIA
+            this.videoVolumeControl.volumeSliderHandleEl.attr({
+                'aria-valuenow': this.videoVolumeControl.previousVolume,
+                'aria-valuetext': getVolumeDescription(this.videoVolumeControl.previousVolume)
+            });
+        }
+    }
+
+    // ARIA
+    // Returns a string describing the level of volume.
+    function getVolumeDescription(vol) {
+        if (vol === 0) {
+            return 'muted';
+        } else if (vol <= 20) {
+            return 'very low';
+        } else if (vol <= 40) {
+            return 'low';
+        } else if (vol <= 60) {
+            return 'average';
+        } else if (vol <= 80) {
+            return 'loud';
+        } else if (vol <= 99) {
+            return 'very loud';
         }
+        
+        return 'maximum';
     }
 
 });

common/lib/xmodule/xmodule/js/src/video/09_video_caption.js

@@ -345,8 +345,8 @@ function () {
         // Keeps track of where the focus is situated in the array of captions.
         // Used to implement the automatic scrolling behavior and decide if the
         // outline around a caption has to be hidden or shown on a mouseenter or
-        // mouseleave.
-        this.videoCaption.currentCaptionIndex = 0;
+        // mouseleave. Initially, no caption has the focus, set the index to -1.
+        this.videoCaption.currentCaptionIndex = -1;
         // Used to track if the focus is coming from a click or tabbing. This 
         // has to be known to decide if, when a caption gets the focus, an 
         // outline has to be drawn (tabbing) or not (mouse click).
@@ -453,6 +453,9 @@ function () {
             min = 0;
             max = this.videoCaption.start.length - 1;
 
+            if (time < this.videoCaption.start[min]) {
+                return -1;
+            }
             while (min < max) {
                 index = Math.ceil((max + min) / 2);
 
@@ -497,20 +500,21 @@ function () {
                 // Total play time changes with speed change. Also there is
                 // a 250 ms delay we have to take into account.
                 time = Math.round(
-                    Time.convert(time, this.speed, '1.0') * 1000 + 250
+                    Time.convert(time, this.speed, '1.0') * 1000 + 100
                 );
             } else {
                 // Total play time remains constant when speed changes.
-                time = Math.round(parseInt(time, 10) * 1000);
+                time = Math.round(time * 1000 + 100);
             }
 
             newIndex = this.videoCaption.search(time);
 
             if (
-                newIndex !== void 0 &&
+                typeof newIndex !== 'undefined' &&
+                newIndex !== -1 &&
                 this.videoCaption.currentIndex !== newIndex
             ) {
-                if (this.videoCaption.currentIndex) {
+                if (typeof this.videoCaption.currentIndex !== 'undefined') {
                     this.videoCaption.subtitlesEl
                         .find('li.current')
                         .removeClass('current');
@@ -585,11 +589,13 @@ function () {
             type = 'hide_transcript';
             this.captionsHidden = true;
             this.videoCaption.hideSubtitlesEl.attr('title', gettext('Turn on captions'));
+            this.videoCaption.hideSubtitlesEl.text(gettext('Turn on captions'));
             this.el.addClass('closed');
         } else {
             type = 'show_transcript';
             this.captionsHidden = false;
             this.videoCaption.hideSubtitlesEl.attr('title', gettext('Turn off captions'));
+            this.videoCaption.hideSubtitlesEl.text(gettext('Turn off captions'));
             this.el.removeClass('closed');
             this.videoCaption.scrollCaption();
         }

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

@@ -114,6 +114,30 @@ class CourseLocator(Locator):
     course_id = None
     branch = 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 branch).
+
+        Resulting CourseLocator 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, and branch must be strings or None
+
+        """
+        self._validate_args(url, version_guid, course_id)
+        if url:
+            self.init_from_url(url)
+        if version_guid:
+            self.init_from_version_guid(version_guid)
+        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."
+
     def __unicode__(self):
         """
         Return a string representing this location.
@@ -135,18 +159,13 @@ class CourseLocator(Locator):
         """
         return 'edx://' + unicode(self)
 
-    # -- unused args which are used via inspect
-    # pylint: disable= W0613
-    def validate_args(self, url, version_guid, course_id, branch):
+    def _validate_args(self, url, version_guid, course_id):
         """
-        Validate provided arguments.
+        Validate provided arguments. Internal use only which is why it checks for each
+        arg and doesn't use keyword
         """
-        need_oneof = set(('url', 'version_guid', 'course_id'))
-        args, _, _, values = inspect.getargvalues(inspect.currentframe())
-        provided_args = [a for a in args if a != 'self' and values[a] is not None]
-        if len(need_oneof.intersection(provided_args)) == 0:
-            raise InsufficientSpecificationError("Must provide one of these args: %s " %
-                                                 list(need_oneof))
+        if not any((url, version_guid, course_id)):
+            raise InsufficientSpecificationError("Must provide one of url, version_guid, course_id")
 
     def is_fully_specified(self):
         """
@@ -154,8 +173,8 @@ class CourseLocator(Locator):
         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.branch is not None)
+        return (self.version_guid is not None or
+            (self.course_id is not None and self.branch is not None))
 
     def set_course_id(self, new):
         """
@@ -189,30 +208,6 @@ class CourseLocator(Locator):
                              version_guid=self.version_guid,
                              branch=self.branch)
 
-    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 branch).
-
-        Resulting CourseLocator 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, and branch must be strings or None
-
-        """
-        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 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."
-
     @classmethod
     def as_object_id(cls, value):
         """
@@ -233,9 +228,11 @@ class CourseLocator(Locator):
         """
         if isinstance(url, Locator):
             url = url.url()
-        assert isinstance(url, basestring), '%s is not an instance of basestring' % url
+        if not isinstance(url, basestring):
+            raise TypeError('%s is not an instance of basestring' % url)
         parse = parse_url(url)
-        assert parse, 'Could not parse "%s" as a url' % url
+        if not parse:
+            raise ValueError('Could not parse "%s" as a url' % url)
         self._set_value(
             parse, 'version_guid', lambda (new_guid): self.set_version_guid(self.as_object_id(new_guid))
         )
@@ -250,13 +247,13 @@ class CourseLocator(Locator):
         """
         version_guid = self.as_object_id(version_guid)
 
-        assert isinstance(version_guid, ObjectId), \
-            '%s is not an instance of ObjectId' % version_guid
+        if not isinstance(version_guid, ObjectId):
+            raise TypeError('%s is not an instance of ObjectId' % version_guid)
         self.set_version_guid(version_guid)
 
     def init_from_course_id(self, course_id, explicit_branch=None):
         """
-        Course_id is a string like 'mit.eecs.6002x' or 'mit.eecs.6002x/branch/published'.
+        Course_id is a CourseLocator or a string like 'mit.eecs.6002x' or 'mit.eecs.6002x/branch/published'.
 
         Revision (optional) is a string like 'published'.
         It may be provided explicitly (explicit_branch) or embedded into course_id.
@@ -270,10 +267,12 @@ class CourseLocator(Locator):
         if course_id:
             if isinstance(course_id, CourseLocator):
                 course_id = course_id.course_id
-                assert course_id, "%s does not have a valid course_id"
+                if not course_id:
+                    raise ValueError("%s does not have a valid course_id" % course_id)
 
             parse = parse_course_id(course_id)
-            assert parse, 'Could not parse "%s" as a course_id' % course_id
+            if not parse:
+                raise ValueError('Could not parse "%s" as a course_id' % course_id)
             self.set_course_id(parse['id'])
             rev = parse['branch']
             if rev:
@@ -348,7 +347,7 @@ class BlockUsageLocator(CourseLocator):
         url, course_id, branch, and usage_id must be strings or None
 
         """
-        self.validate_args(url, version_guid, course_id, branch)
+        self._validate_args(url, version_guid, course_id)
         if url:
             self.init_block_ref_from_url(url)
         if course_id:
@@ -398,7 +397,8 @@ class BlockUsageLocator(CourseLocator):
             self.set_usage_id(block_ref)
         else:
             parse = parse_block_ref(block_ref)
-            assert parse, 'Could not parse "%s" as a block_ref' % block_ref
+            if not parse:
+                raise ValueError('Could not parse "%s" as a block_ref' % block_ref)
             self.set_usage_id(parse['block'])
 
     def init_block_ref_from_url(self, url):
@@ -424,7 +424,7 @@ class BlockUsageLocator(CourseLocator):
         return rep + BLOCK_PREFIX + unicode(self.usage_id)
 
 
-class DescriptionLocator(Locator):
+class DefinitionLocator(Locator):
     """
     Container for how to locate a description (the course-independent content).
     """
@@ -461,10 +461,10 @@ class VersionTree(object):
         """
         :param locator: must be version specific (Course has version_guid or definition had id)
         """
-        assert isinstance(locator, Locator) and not inspect.isabstract(locator), \
-            "locator must be a concrete subclass of Locator"
-        assert locator.version(), \
-            "locator must be version specific (Course has version_guid or definition had id)"
+        if not isinstance(locator, Locator) and not inspect.isabstract(locator):
+            raise TypeError("locator {} must be a concrete subclass of Locator".format(locator))
+        if not locator.version():
+            raise ValueError("locator must be version specific (Course has version_guid or definition had id)")
         self.locator = locator
         if tree_dict is None:
             self.children = []

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

@@ -193,7 +193,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
 
                 field_data = DbModel(kvs)
                 scope_ids = ScopeIds(None, category, location, location)
-                module = self.construct_xblock_from_class(class_, field_data, scope_ids)
+                module = self.construct_xblock_from_class(class_, scope_ids, field_data)
                 if self.cached_metadata is not None:
                     # parent container pointers don't differentiate between draft and non-draft
                     # so when we do the lookup, we should do so with a non-draft location
@@ -621,12 +621,11 @@ class MongoModuleStore(ModuleStoreBase):
         dbmodel = self._create_new_field_data(location.category, location, definition_data, metadata)
         xmodule = system.construct_xblock_from_class(
             xblock_class,
-            dbmodel,
-
             # We're loading a descriptor, so student_id is meaningless
             # We also don't have separate notions of definition and usage ids yet,
             # so we use the location for both.
-            ScopeIds(None, location.category, location, location)
+            ScopeIds(None, location.category, location, location),
+            dbmodel,
         )
         # decache any pending field settings from init
         xmodule.save()

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

@@ -111,8 +111,8 @@ class CachingDescriptorSystem(MakoDescriptorSystem):
         try:
             module = self.construct_xblock_from_class(
                 class_,
+                ScopeIds(None, json_data.get('category'), definition_id, block_locator),
                 field_data,
-                ScopeIds(None, json_data.get('category'), definition_id, block_locator)
             )
         except Exception:
             log.warning("Failed to load descriptor", exc_info=True)

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

-from xmodule.modulestore.locator import DescriptionLocator
+from xmodule.modulestore.locator import DefinitionLocator
 
 
 class DefinitionLazyLoader(object):
@@ -15,7 +15,7 @@ class DefinitionLazyLoader(object):
         :param definition_locator: the id of the record in the above to fetch
         """
         self.modulestore = modulestore
-        self.definition_locator = DescriptionLocator(definition_id)
+        self.definition_locator = DefinitionLocator(definition_id)
 
     def fetch(self):
         """

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

@@ -11,7 +11,7 @@ from pytz import UTC
 
 from xmodule.errortracker import null_error_tracker
 from xmodule.x_module import XModuleDescriptor
-from xmodule.modulestore.locator import BlockUsageLocator, DescriptionLocator, CourseLocator, VersionTree, LocalId
+from xmodule.modulestore.locator import BlockUsageLocator, DefinitionLocator, CourseLocator, VersionTree, LocalId
 from xmodule.modulestore.exceptions import InsufficientSpecificationError, VersionConflictError, DuplicateItemError
 from xmodule.modulestore import inheritance, ModuleStoreBase, Location
 
@@ -43,8 +43,6 @@ log = logging.getLogger(__name__)
 #==============================================================================
 
 
-
-
 class SplitMongoModuleStore(ModuleStoreBase):
     """
     A Mongodb backed ModuleStore supporting versions, inheritance,
@@ -565,7 +563,7 @@ class SplitMongoModuleStore(ModuleStoreBase):
                 }
             }
         new_id = self.definitions.insert(document)
-        definition_locator = DescriptionLocator(new_id)
+        definition_locator = DefinitionLocator(new_id)
         document['edit_info']['original_version'] = new_id
         self.definitions.update({'_id': new_id}, {'$set': {"edit_info.original_version": new_id}})
         return definition_locator
@@ -599,7 +597,7 @@ class SplitMongoModuleStore(ModuleStoreBase):
             old_definition['edit_info']['edited_on'] = datetime.datetime.now(UTC)
             old_definition['edit_info']['previous_version'] = definition_locator.definition_id
             new_id = self.definitions.insert(old_definition)
-            return DescriptionLocator(new_id), True
+            return DefinitionLocator(new_id), True
         else:
             return definition_locator, False
 
@@ -1254,7 +1252,7 @@ class SplitMongoModuleStore(ModuleStoreBase):
         elif '_id' not in definition:
             return None
         else:
-            return DescriptionLocator(definition['_id'])
+            return DefinitionLocator(definition['_id'])
 
     def internal_clean_children(self, course_locator):
         """

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

@@ -4,7 +4,7 @@ Tests for xmodule.modulestore.locator.
 from unittest import TestCase
 
 from bson.objectid import ObjectId
-from xmodule.modulestore.locator import Locator, CourseLocator, BlockUsageLocator, DescriptionLocator
+from xmodule.modulestore.locator import Locator, CourseLocator, BlockUsageLocator, DefinitionLocator
 from xmodule.modulestore.parsers import BRANCH_PREFIX, BLOCK_PREFIX, VERSION_PREFIX, URL_VERSION_PREFIX
 from xmodule.modulestore.exceptions import InsufficientSpecificationError, OverSpecificationError
 
@@ -91,8 +91,8 @@ class LocatorTest(TestCase):
                        'mit.eecs' + BRANCH_PREFIX + 'this ',
                        'mit.eecs' + BRANCH_PREFIX + 'th%is ',
                        ):
-            self.assertRaises(AssertionError, CourseLocator, course_id=bad_id)
-            self.assertRaises(AssertionError, CourseLocator, url='edx://' + bad_id)
+            self.assertRaises(ValueError, CourseLocator, course_id=bad_id)
+            self.assertRaises(ValueError, CourseLocator, url='edx://' + bad_id)
 
     def test_course_constructor_bad_url(self):
         for bad_url in ('edx://',
@@ -100,7 +100,7 @@ class LocatorTest(TestCase):
                         'http://mit.eecs',
                         'mit.eecs',
                         'edx//mit.eecs'):
-            self.assertRaises(AssertionError, CourseLocator, url=bad_url)
+            self.assertRaises(ValueError, CourseLocator, url=bad_url)
 
     def test_course_constructor_redundant_001(self):
         testurn = 'mit.eecs.6002x'
@@ -254,11 +254,11 @@ class LocatorTest(TestCase):
         self.assertEqual('BlockUsageLocator("mit.eecs.6002x/branch/published/block/HW3")', repr(testobj))
 
     def test_description_locator_url(self):
-        definition_locator = DescriptionLocator("chapter12345_2")
+        definition_locator = DefinitionLocator("chapter12345_2")
         self.assertEqual('edx://' + URL_VERSION_PREFIX + 'chapter12345_2', definition_locator.url())
 
     def test_description_locator_version(self):
-        definition_locator = DescriptionLocator("chapter12345_2")
+        definition_locator = DefinitionLocator("chapter12345_2")
         self.assertEqual("chapter12345_2", definition_locator.version())
 
     # ------------------------------------------------------------------

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

@@ -13,8 +13,9 @@ from xblock.fields import Scope
 from xmodule.course_module import CourseDescriptor
 from xmodule.modulestore.exceptions import InsufficientSpecificationError, ItemNotFoundError, VersionConflictError, \
     DuplicateItemError
-from xmodule.modulestore.locator import CourseLocator, BlockUsageLocator, VersionTree, DescriptionLocator
+from xmodule.modulestore.locator import CourseLocator, BlockUsageLocator, VersionTree, DefinitionLocator
 from xmodule.modulestore.inheritance import InheritanceMixin
+from xmodule.x_module import XModuleMixin
 from pytz import UTC
 from path import path
 import re
@@ -34,7 +35,7 @@ class SplitModuleTest(unittest.TestCase):
         'db': 'test_xmodule',
         'collection': 'modulestore{0}'.format(uuid.uuid4().hex),
         'fs_root': '',
-        'xblock_mixins': (InheritanceMixin,)
+        'xblock_mixins': (InheritanceMixin, XModuleMixin)
     }
 
     MODULESTORE = {
@@ -561,7 +562,7 @@ class TestItemCrud(SplitModuleTest):
         new_module = modulestore().create_item(
             locator, category, 'user123',
             fields={'display_name': 'new chapter'},
-            definition_locator=DescriptionLocator("chapter12345_2")
+            definition_locator=DefinitionLocator("chapter12345_2")
         )
         # check that course version changed and course's previous is the other one
         self.assertNotEqual(new_module.location.version_guid, premod_course.location.version_guid)
@@ -587,7 +588,7 @@ class TestItemCrud(SplitModuleTest):
         another_module = modulestore().create_item(
             locator, category, 'anotheruser',
             fields={'display_name': 'problem 2', 'data': another_payload},
-            definition_locator=DescriptionLocator("problem12345_3_1"),
+            definition_locator=DefinitionLocator("problem12345_3_1"),
         )
         # check that course version changed and course's previous is the other one
         parent = modulestore().get_item(locator)
@@ -788,7 +789,7 @@ class TestItemCrud(SplitModuleTest):
         modulestore().create_item(
             locator, category, 'test_update_manifold',
             fields={'display_name': 'problem 2', 'data': another_payload},
-            definition_locator=DescriptionLocator("problem12345_3_1"),
+            definition_locator=DefinitionLocator("problem12345_3_1"),
         )
         # pylint: disable=W0212
         modulestore()._clear_cache()

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

@@ -17,9 +17,10 @@ from xmodule.error_module import ErrorDescriptor
 from xmodule.errortracker import make_error_tracker, exc_info_to_str
 from xmodule.course_module import CourseDescriptor
 from xmodule.mako_module import MakoDescriptorSystem
-from xmodule.x_module import XModuleDescriptor, XMLParsingSystem
+from xmodule.x_module import XMLParsingSystem, XModuleDescriptor
 
 from xmodule.html_module import HtmlDescriptor
+from xblock.core import XBlock
 from xblock.fields import ScopeIds
 from xblock.field_data import DictFieldData
 
@@ -63,7 +64,7 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
         self.load_error_modules = load_error_modules
 
         def process_xml(xml):
-            """Takes an xml string, and returns a XModuleDescriptor created from
+            """Takes an xml string, and returns a XBlock created from
             that xml.
             """
 
@@ -163,7 +164,7 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
 
                 make_name_unique(xml_data)
 
-                descriptor = XModuleDescriptor.load_from_xml(
+                descriptor = create_block_from_xml(
                     etree.tostring(xml_data, encoding='unicode'), self, self.org,
                     self.course, xmlstore.default_class)
             except Exception as err:
@@ -219,6 +220,38 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
         )
 
 
+def create_block_from_xml(xml_data, system, org=None, course=None, default_class=None):
+    """
+    Create an XBlock instance from XML data.
+
+    `xml_data' is a string containing valid xml.
+
+    `system` is an XMLParsingSystem.
+
+    `org` and `course` are optional strings that will be used in the generated
+    block's url identifiers.
+
+    `default_class` is the class to instantiate of the XML indicates a class
+    that can't be loaded.
+
+    Returns the fully instantiated XBlock.
+
+    """
+    node = etree.fromstring(xml_data)
+    raw_class = XModuleDescriptor.load_class(node.tag, default_class)
+    xblock_class = system.mixologist.mix(raw_class)
+
+    # leave next line commented out - useful for low-level debugging
+    # log.debug('[create_block_from_xml] tag=%s, class=%s' % (node.tag, xblock_class))
+
+    url_name = node.get('url_name', node.get('slug'))
+    location = Location('i4x', org, course, node.tag, url_name)
+
+    scope_ids = ScopeIds(None, location.category, location, location)
+    xblock = xblock_class.parse_xml(node, system, scope_ids)
+    return xblock
+
+
 class ParentTracker(object):
     """A simple class to factor out the logic for tracking location parent pointers."""
     def __init__(self):
@@ -278,8 +311,8 @@ class XMLModuleStore(ModuleStoreBase):
         super(XMLModuleStore, self).__init__(**kwargs)
 
         self.data_dir = path(data_dir)
-        self.modules = defaultdict(dict)  # course_id -> dict(location -> XModuleDescriptor)
-        self.courses = {}  # course_dir -> XModuleDescriptor for the course
+        self.modules = defaultdict(dict)  # course_id -> dict(location -> XBlock)
+        self.courses = {}  # course_dir -> XBlock for the course
         self.errored_courses = {}  # course_dir -> errorlog, for dirs that failed to load
 
         self.load_error_modules = load_error_modules
@@ -477,11 +510,11 @@ class XMLModuleStore(ModuleStoreBase):
                     loc = Location('i4x', course_descriptor.location.org, course_descriptor.location.course, category, slug)
                     module = system.construct_xblock_from_class(
                         HtmlDescriptor,
-                        DictFieldData({'data': html, 'location': loc, 'category': category}),
                         # We're loading a descriptor, so student_id is meaningless
                         # We also don't have separate notions of definition and usage ids yet,
                         # so we use the location for both
                         ScopeIds(None, category, loc, loc),
+                        DictFieldData({'data': html, 'location': loc, 'category': category}),
                     )
                     # VS[compat]:
                     # Hack because we need to pull in the 'display_name' for static tabs (because we need to edit them)
@@ -500,7 +533,7 @@ class XMLModuleStore(ModuleStoreBase):
 
     def get_instance(self, course_id, location, depth=0):
         """
-        Returns an XModuleDescriptor instance for the item at
+        Returns an XBlock instance for the item at
         location, with the policy for course_id.  (In case two xml
         dirs have different content at the same location, return the
         one for this course_id.)
@@ -528,7 +561,7 @@ class XMLModuleStore(ModuleStoreBase):
 
     def get_item(self, location, depth=0):
         """
-        Returns an XModuleDescriptor instance for the item at location.
+        Returns an XBlock instance for the item at location.
 
         If any segment of the location is None except revision, raises
             xmodule.modulestore.exceptions.InsufficientSpecificationError

common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_image_submission.py

-"""
-This contains functions and classes used to evaluate if images are acceptable (do not show improper content, etc), and
-to send them to S3.
-"""
-
-try:
-    from PIL import Image
-
-    ENABLE_PIL = True
-except:
-    ENABLE_PIL = False
-
-from urlparse import urlparse
-import requests
-from boto.s3.connection import S3Connection
-from boto.s3.key import Key
-import logging
-
-log = logging.getLogger(__name__)
-
-#Domains where any image linked to can be trusted to have acceptable content.
-TRUSTED_IMAGE_DOMAINS = [
-    'wikipedia',
-    'edxuploads.s3.amazonaws.com',
-    'wikimedia',
-]
-
-#Suffixes that are allowed in image urls
-ALLOWABLE_IMAGE_SUFFIXES = [
-    'jpg',
-    'png',
-    'gif',
-    'jpeg'
-]
-
-#Maximum allowed dimensions (x and y) for an uploaded image
-MAX_ALLOWED_IMAGE_DIM = 2000
-
-#Dimensions to which image is resized before it is evaluated for color count, etc
-MAX_IMAGE_DIM = 150
-
-#Maximum number of colors that should be counted in ImageProperties
-MAX_COLORS_TO_COUNT = 16
-
-#Maximum number of colors allowed in an uploaded image
-MAX_COLORS = 400
-
-
-class ImageProperties(object):
-    """
-    Class to check properties of an image and to validate if they are allowed.
-    """
-
-    def __init__(self, image_data):
-        """
-        Initializes class variables
-        @param image: Image object (from PIL)
-        @return: None
-        """
-        self.image = Image.open(image_data)
-        image_size = self.image.size
-        self.image_too_large = False
-        if image_size[0] > MAX_ALLOWED_IMAGE_DIM or image_size[1] > MAX_ALLOWED_IMAGE_DIM:
-            self.image_too_large = True
-        if image_size[0] > MAX_IMAGE_DIM or image_size[1] > MAX_IMAGE_DIM:
-            self.image = self.image.resize((MAX_IMAGE_DIM, MAX_IMAGE_DIM))
-            self.image_size = self.image.size
-
-    def count_colors(self):
-        """
-        Counts the number of colors in an image, and matches them to the max allowed
-        @return: boolean true if color count is acceptable, false otherwise
-        """
-        colors = self.image.getcolors(MAX_COLORS_TO_COUNT)
-        if colors is None:
-            color_count = MAX_COLORS_TO_COUNT
-        else:
-            color_count = len(colors)
-
-        too_many_colors = (color_count <= MAX_COLORS)
-        return too_many_colors
-
-    def check_if_rgb_is_skin(self, rgb):
-        """
-        Checks if a given input rgb tuple/list is a skin tone
-        @param rgb: RGB tuple
-        @return: Boolean true false
-        """
-        colors_okay = False
-        try:
-            r = rgb[0]
-            g = rgb[1]
-            b = rgb[2]
-            check_r = (r > 60)
-            check_g = (r * 0.4) < g < (r * 0.85)
-            check_b = (r * 0.2) < b < (r * 0.7)
-            colors_okay = check_r and check_b and check_g
-        except:
-            pass
-
-        return colors_okay
-
-    def get_skin_ratio(self):
-        """
-        Gets the ratio of skin tone colors in an image
-        @return: True if the ratio is low enough to be acceptable, false otherwise
-        """
-        colors = self.image.getcolors(MAX_COLORS_TO_COUNT)
-        is_okay = True
-        if colors is not None:
-            skin = sum([count for count, rgb in colors if self.check_if_rgb_is_skin(rgb)])
-            total_colored_pixels = sum([count for count, rgb in colors])
-            bad_color_val = float(skin) / total_colored_pixels
-            if bad_color_val > .4:
-                is_okay = False
-
-        return is_okay
-
-    def run_tests(self):
-        """
-        Does all available checks on an image to ensure that it is okay (size, skin ratio, colors)
-        @return: Boolean indicating whether or not image passes all checks
-        """
-        image_is_okay = False
-        try:
-            #image_is_okay = self.count_colors() and self.get_skin_ratio() and not self.image_too_large
-            image_is_okay = not self.image_too_large
-        except:
-            log.exception("Could not run image tests.")
-
-        if not ENABLE_PIL:
-            image_is_okay = True
-
-        #log.debug("Image OK: {0}".format(image_is_okay))
-
-        return image_is_okay
-
-
-class URLProperties(object):
-    """
-    Checks to see if a URL points to acceptable content.  Added to check if students are submitting reasonable
-    links to the peer grading image functionality of the external grading service.
-    """
-
-    def __init__(self, url_string):
-        self.url_string = url_string
-
-    def check_if_parses(self):
-        """
-        Check to see if a URL parses properly
-        @return: success (True if parses, false if not)
-        """
-        success = False
-        try:
-            self.parsed_url = urlparse(self.url_string)
-            success = True
-        except:
-            pass
-
-        return success
-
-    def check_suffix(self):
-        """
-        Checks the suffix of a url to make sure that it is allowed
-        @return: True if suffix is okay, false if not
-        """
-        good_suffix = False
-        for suffix in ALLOWABLE_IMAGE_SUFFIXES:
-            if self.url_string.endswith(suffix):
-                good_suffix = True
-                break
-        return good_suffix
-
-    def run_tests(self):
-        """
-        Runs all available url tests
-        @return: True if URL passes tests, false if not.
-        """
-        url_is_okay = self.check_suffix() and self.check_if_parses()
-        return url_is_okay
-
-    def check_domain(self):
-        """
-        Checks to see if url is from a trusted domain
-        """
-        success = False
-        for domain in TRUSTED_IMAGE_DOMAINS:
-            if domain in self.url_string:
-                success = True
-                return success
-        return success
-
-
-def run_url_tests(url_string):
-    """
-    Creates a URLProperties object and runs all tests
-    @param url_string: A URL in string format
-    @return: Boolean indicating whether or not URL has passed all tests
-    """
-    url_properties = URLProperties(url_string)
-    return url_properties.run_tests()
-
-
-def run_image_tests(image):
-    """
-    Runs all available image tests
-    @param image: PIL Image object
-    @return: Boolean indicating whether or not all tests have been passed
-    """
-    success = False
-    try:
-        image_properties = ImageProperties(image)
-        success = image_properties.run_tests()
-    except:
-        log.exception("Cannot run image tests in combined open ended xmodule.  May be an issue with a particular image,"
-                      "or an issue with the deployment configuration of PIL/Pillow")
-    return success
-
-
-def upload_to_s3(file_to_upload, keyname, s3_interface):
-    '''
-    Upload file to S3 using provided keyname.
-
-    Returns:
-        public_url: URL to access uploaded file
-    '''
-
-    #This commented out code is kept here in case we change the uploading method and require images to be
-    #converted before they are sent to S3.
-    #TODO: determine if commented code is needed and remove
-    #im = Image.open(file_to_upload)
-    #out_im = cStringIO.StringIO()
-    #im.save(out_im, 'PNG')
-
-    try:
-        conn = S3Connection(s3_interface['access_key'], s3_interface['secret_access_key'])
-        bucketname = str(s3_interface['storage_bucket_name'])
-        bucket = conn.create_bucket(bucketname.lower())
-
-        k = Key(bucket)
-        k.key = keyname
-        k.set_metadata('filename', file_to_upload.name)
-        k.set_contents_from_file(file_to_upload)
-
-        #This commented out code is kept here in case we change the uploading method and require images to be
-        #converted before they are sent to S3.
-        #k.set_contents_from_string(out_im.getvalue())
-        #k.set_metadata("Content-Type", 'images/png')
-
-        k.set_acl("public-read")
-        public_url = k.generate_url(60 * 60 * 24 * 365)   # URL timeout in seconds.
-
-        return True, public_url
-    except:
-        #This is a dev_facing_error
-        error_message = "Could not connect to S3 to upload peer grading image.  Trying to utilize bucket: {0}".format(
-            bucketname.lower())
-        log.error(error_message)
-        return False, error_message
-
-
-def get_from_s3(s3_public_url):
-    """
-    Gets an image from a given S3 url
-    @param s3_public_url: The URL where an image is located
-    @return: The image data
-    """
-    r = requests.get(s3_public_url, timeout=2)
-    data = r.text
-    return data

common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py

@@ -651,15 +651,12 @@ class OpenEndedModule(openendedchild.OpenEndedChild):
             return self.out_of_sync_error(data)
 
         # add new history element with answer and empty score and hint.
-        success, data = self.append_image_to_student_answer(data)
+        success, error_message, data = self.append_file_link_to_student_answer(data)
         if success:
             data['student_answer'] = OpenEndedModule.sanitize_html(data['student_answer'])
             self.new_history_entry(data['student_answer'])
             self.send_to_grader(data['student_answer'], system)
             self.change_state(self.ASSESSING)
-        else:
-            # This is a student_facing_error
-            error_message = "There was a problem saving the image in your submission.  Please try a different image, or try pasting a link to an image into the answer box."
 
         return {
             'success': success,

common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py

@@ -179,14 +179,11 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild):
 
         error_message = ""
         # add new history element with answer and empty score and hint.
-        success, data = self.append_image_to_student_answer(data)
+        success, error_message, data = self.append_file_link_to_student_answer(data)
         if success:
             data['student_answer'] = SelfAssessmentModule.sanitize_html(data['student_answer'])
             self.new_history_entry(data['student_answer'])
             self.change_state(self.ASSESSING)
-        else:
-            # This is a student_facing_error
-            error_message = "There was a problem saving the image in your submission.  Please try a different image, or try pasting a link to an image into the answer box."
 
         return {
             'success': success,

common/lib/xmodule/xmodule/tests/__init__.py

@@ -11,15 +11,11 @@ import json
 import os
 import unittest
 
-import fs
-import fs.osfs
-import numpy
 from mock import Mock
 from path import path
 
-import calc
 from xblock.field_data import DictFieldData
-from xmodule.x_module import ModuleSystem, XModuleDescriptor, DescriptorSystem
+from xmodule.x_module import ModuleSystem, XModuleDescriptor, XModuleMixin
 from xmodule.modulestore.inheritance import InheritanceMixin
 from xmodule.mako_module import MakoDescriptorSystem
 
@@ -81,7 +77,7 @@ def get_test_descriptor_system():
         resources_fs=Mock(),
         error_tracker=Mock(),
         render_template=lambda template, context: repr(context),
-        mixins=(InheritanceMixin,),
+        mixins=(InheritanceMixin, XModuleMixin),
     )
 
 

common/lib/xmodule/xmodule/tests/test_combined_open_ended.py

@@ -12,7 +12,7 @@ import logging
 import unittest
 
 from lxml import etree
-from mock import Mock, MagicMock, ANY
+from mock import Mock, MagicMock, ANY, patch
 from pytz import UTC
 
 from xmodule.open_ended_grading_classes.openendedchild import OpenEndedChild
@@ -26,7 +26,7 @@ from xmodule.progress import Progress
 from xmodule.tests.test_util_open_ended import (
     MockQueryDict, DummyModulestore, TEST_STATE_SA_IN,
     MOCK_INSTANCE_STATE, TEST_STATE_SA, TEST_STATE_AI, TEST_STATE_AI2, TEST_STATE_AI2_INVALID,
-    TEST_STATE_SINGLE, TEST_STATE_PE_SINGLE
+    TEST_STATE_SINGLE, TEST_STATE_PE_SINGLE, MockUploadedFile
 )
 
 from xblock.field_data import DictFieldData
@@ -374,7 +374,7 @@ class OpenEndedModuleTest(unittest.TestCase):
         # Submit a student response to the question.
         test_module.handle_ajax(
             "save_answer",
-            {"student_answer": submitted_response, "can_upload_files": False, "student_file": None},
+            {"student_answer": submitted_response},
             get_test_system()
         )
         # Submitting an answer should clear the stored answer.
@@ -753,7 +753,7 @@ class OpenEndedModuleXmlTest(unittest.TestCase, DummyModulestore):
 
         #Simulate a student saving an answer
         html = module.handle_ajax("get_html", {})
-        module.handle_ajax("save_answer", {"student_answer": self.answer, "can_upload_files": False, "student_file": None})
+        module.handle_ajax("save_answer", {"student_answer": self.answer})
         html = module.handle_ajax("get_html", {})
 
         #Mock a student submitting an assessment
@@ -902,3 +902,78 @@ class OpenEndedModuleXmlAttemptTest(unittest.TestCase, DummyModulestore):
         #Try to reset, should fail because only 1 attempt is allowed
         reset_data = json.loads(module.handle_ajax("reset", {}))
         self.assertEqual(reset_data['success'], False)
+
+class OpenEndedModuleXmlImageUploadTest(unittest.TestCase, DummyModulestore):
+    """
+    Test if student is able to upload images properly.
+    """
+    problem_location = Location(["i4x", "edX", "open_ended", "combinedopenended", "SampleQuestionImageUpload"])
+    answer_text = "Hello, this is my amazing answer."
+    file_text = "Hello, this is my amazing file."
+    file_name = "Student file 1"
+    answer_link = "http://www.edx.org"
+    autolink_tag = "<a href="
+
+    def setUp(self):
+        self.test_system = get_test_system()
+        self.test_system.open_ended_grading_interface = None
+        self.test_system.s3_interface = test_util_open_ended.S3_INTERFACE
+        self.test_system.xqueue['interface'] = Mock(
+            send_to_queue=Mock(side_effect=[1, "queued"])
+        )
+        self.setup_modulestore(COURSE)
+
+    def test_file_upload_fail(self):
+        """
+        Test to see if a student submission without a file attached fails.
+        """
+        module = self.get_module_from_location(self.problem_location, COURSE)
+
+        #Simulate a student saving an answer
+        response = module.handle_ajax("save_answer", {"student_answer": self.answer_text})
+        response = json.loads(response)
+        self.assertFalse(response['success'])
+        self.assertIn('error', response)
+
+    @patch(
+        'xmodule.open_ended_grading_classes.openendedchild.S3Connection',
+        test_util_open_ended.MockS3Connection
+    )
+    @patch(
+        'xmodule.open_ended_grading_classes.openendedchild.Key',
+        test_util_open_ended.MockS3Key
+    )
+    def test_file_upload_success(self):
+        """
+        Test to see if a student submission with a file is handled properly.
+        """
+        module = self.get_module_from_location(self.problem_location, COURSE)
+
+        #Simulate a student saving an answer with a file
+        response = module.handle_ajax("save_answer", {
+            "student_answer": self.answer_text,
+            "valid_files_attached": True,
+            "student_file": [MockUploadedFile(self.file_name, self.file_text)],
+        })
+
+        response = json.loads(response)
+        self.assertTrue(response['success'])
+        self.assertIn(self.file_name, response['student_response'])
+        self.assertIn(self.autolink_tag, response['student_response'])
+
+    def test_link_submission_success(self):
+        """
+        Students can submit links instead of files.  Check that the link is properly handled.
+        """
+        module = self.get_module_from_location(self.problem_location, COURSE)
+
+        # Simulate a student saving an answer with a link.
+        response = module.handle_ajax("save_answer", {
+            "student_answer": "{0} {1}".format(self.answer_text, self.answer_link)
+            })
+
+        response = json.loads(response)
+
+        self.assertTrue(response['success'])
+        self.assertIn(self.answer_link, response['student_response'])
+        self.assertIn(self.autolink_tag, response['student_response'])

common/lib/xmodule/xmodule/tests/test_editing_module.py

@@ -46,8 +46,8 @@ class TabsEditingDescriptorTestCase(unittest.TestCase):
         TabsEditingDescriptor.tabs = self.tabs
         self.descriptor = system.construct_xblock_from_class(
             TabsEditingDescriptor,
-            field_data=DictFieldData({}),
             scope_ids=ScopeIds(None, None, None, None),
+            field_data=DictFieldData({}),
         )
 
     def test_get_css(self):

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

@@ -13,6 +13,7 @@ from xmodule.xml_module import is_pointer_tag
 from xmodule.modulestore import Location
 from xmodule.modulestore.xml import ImportSystem, XMLModuleStore
 from xmodule.modulestore.inheritance import compute_inherited_metadata
+from xmodule.x_module import XModuleMixin
 from xmodule.fields import Date
 from xmodule.tests import DATA_DIR
 from xmodule.modulestore.inheritance import InheritanceMixin
@@ -42,7 +43,7 @@ class DummySystem(ImportSystem):
             error_tracker=error_tracker,
             parent_tracker=parent_tracker,
             load_error_modules=load_error_modules,
-            mixins=(InheritanceMixin,)
+            mixins=(InheritanceMixin, XModuleMixin)
         )
 
     def render_template(self, _template, _context):
@@ -91,7 +92,6 @@ class ImportTestCase(BaseCourseTestCase):
 
         self.assertNotEqual(descriptor1.location, descriptor2.location)
 
-    @unittest.skip('Temporarily disabled')
     def test_reimport(self):
         '''Make sure an already-exported error xml tag loads properly'''
 

common/lib/xmodule/xmodule/tests/test_self_assessment.py

@@ -133,7 +133,7 @@ class SelfAssessmentTest(unittest.TestCase):
         self.assertEqual(test_module.get_display_answer(), saved_response)
 
         # Submit a student response to the question.
-        test_module.handle_ajax("save_answer", {"student_answer": submitted_response, "can_upload_files": False, "student_file": None}, get_test_system())
+        test_module.handle_ajax("save_answer", {"student_answer": submitted_response}, get_test_system())
         # Submitting an answer should clear the stored answer.
         self.assertEqual(test_module.stored_answer, None)
         # Confirm that the answer is stored properly.

common/lib/xmodule/xmodule/tests/test_util_open_ended.py

@@ -2,6 +2,8 @@ from xmodule.modulestore import Location
 from xmodule.modulestore.xml import XMLModuleStore
 from xmodule.tests import DATA_DIR, get_test_system
 
+from StringIO import StringIO
+
 OPEN_ENDED_GRADING_INTERFACE = {
     'url': 'blah/',
     'username': 'incorrect',
@@ -12,11 +14,61 @@ OPEN_ENDED_GRADING_INTERFACE = {
 }
 
 S3_INTERFACE = {
-    'aws_access_key': "",
-    'aws_secret_key': "",
-    "aws_bucket_name": "",
+    'access_key': "",
+    'secret_access_key': "",
+    "storage_bucket_name": "",
 }
 
+class MockS3Key(object):
+    """
+    Mock an S3 Key object from boto.  Used for file upload testing.
+    """
+    def __init__(self, bucket):
+        pass
+
+    def set_metadata(self, key, value):
+        setattr(self, key, value)
+
+    def set_contents_from_file(self, fileobject):
+        self.data = fileobject.read()
+
+    def set_acl(self, acl):
+        self.set_metadata("acl", acl)
+
+    def generate_url(self, timeout):
+        return "http://www.edx.org/sample_url"
+
+
+class MockS3Connection(object):
+    """
+    Mock boto S3Connection for testing image uploads.
+    """
+    def __init__(self, access_key, secret_key, **kwargs):
+        """
+        Mock the init call.  S3Connection has a lot of arguments, but we don't need them.
+        """
+        pass
+
+    def create_bucket(self, bucket_name, **kwargs):
+        return "edX Bucket"
+
+class MockUploadedFile(object):
+    """
+    Create a mock uploaded file for image submission tests.
+    value - String data to place into the mock file.
+    return - A StringIO object that behaves like a file.
+    """
+    def __init__(self, name, value):
+        self.mock_file = StringIO()
+        self.mock_file.write(value)
+        self.name = name
+
+    def seek(self, index):
+        return self.mock_file.seek(index)
+
+    def read(self):
+        return self.mock_file.read()
+
 
 class MockQueryDict(dict):
     """

common/lib/xmodule/xmodule/tests/test_video.py

@@ -133,8 +133,8 @@ class VideoDescriptorTest(unittest.TestCase):
         system = get_test_descriptor_system()
         self.descriptor = system.construct_xblock_from_class(
             VideoDescriptor,
-            field_data=DictFieldData({}),
             scope_ids=ScopeIds(None, None, None, None),
+            field_data=DictFieldData({}),
         )
 
     def test_get_context(self):

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

@@ -87,8 +87,8 @@ class TestXBlockWrapper(object):
         runtime.render_template = lambda *args, **kwargs: u'{!r}, {!r}'.format(args, kwargs)
         return runtime.construct_xblock_from_class(
             descriptor_cls,
+            ScopeIds(None, descriptor_cls.__name__, location, location),
             DictFieldData({}),
-            ScopeIds(None, descriptor_cls.__name__, location, location)
         )
 
     def leaf_module(self, descriptor_cls):
@@ -109,10 +109,10 @@ class TestXBlockWrapper(object):
         runtime.render_template = lambda *args, **kwargs: u'{!r}, {!r}'.format(args, kwargs)
         return runtime.construct_xblock_from_class(
             descriptor_cls,
+            ScopeIds(None, descriptor_cls.__name__, location, location),
             DictFieldData({
                 'children': range(3)
             }),
-            ScopeIds(None, descriptor_cls.__name__, location, location)
         )
 
     def container_module(self, descriptor_cls, depth):

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

 # disable missing docstring
 #pylint: disable=C0111
 
-from xmodule.x_module import XModuleFields
-from xblock.fields import Scope, String, Dict, Boolean, Integer, Float, Any, List
-from xblock.field_data import DictFieldData
-from xmodule.fields import Date, Timedelta
-from xmodule.xml_module import XmlDescriptor, serialize_field, deserialize_field
 import unittest
-from nose.tools import assert_equals  # pylint: disable=E0611
+
 from mock import Mock
-from xmodule.modulestore.inheritance import InheritanceKeyValueStore, InheritanceMixin
+from nose.tools import assert_equals, assert_not_equals, assert_true, assert_false, assert_in, assert_not_in  # pylint: disable=E0611
+
+from xblock.field_data import DictFieldData
+from xblock.fields import Scope, String, Dict, Boolean, Integer, Float, Any, List
 from xblock.runtime import DbModel
+
+from xmodule.fields import Date, Timedelta
+from xmodule.modulestore.inheritance import InheritanceKeyValueStore, InheritanceMixin
+from xmodule.xml_module import XmlDescriptor, serialize_field, deserialize_field
+from xmodule.course_module import CourseDescriptor
+from xmodule.seq_module import SequenceDescriptor
+from xmodule.x_module import XModuleMixin
+
 from xmodule.tests import get_test_descriptor_system
+from xmodule.tests.xml import XModuleXmlImportTest
+from xmodule.tests.xml.factories import CourseFactory, SequenceFactory, ProblemFactory
 
 
 class CrazyJsonString(String):
@@ -57,7 +65,7 @@ class EditableMetadataFieldsTest(unittest.TestCase):
         # Also tests that xml_attributes is filtered out of XmlDescriptor.
         self.assertEqual(1, len(editable_fields), editable_fields)
         self.assert_field_values(
-            editable_fields, 'display_name', XModuleFields.display_name,
+            editable_fields, 'display_name', XModuleMixin.display_name,
             explicitly_set=False, value=None, default_value=None
         )
 
@@ -65,7 +73,7 @@ class EditableMetadataFieldsTest(unittest.TestCase):
         # Tests that explicitly_set is correct when a value overrides the default (not inheritable).
         editable_fields = self.get_xml_editable_fields(DictFieldData({'display_name': 'foo'}))
         self.assert_field_values(
-            editable_fields, 'display_name', XModuleFields.display_name,
+            editable_fields, 'display_name', XModuleMixin.display_name,
             explicitly_set=True, value='foo', default_value=None
         )
 
@@ -158,8 +166,8 @@ class EditableMetadataFieldsTest(unittest.TestCase):
         runtime = get_test_descriptor_system()
         return runtime.construct_xblock_from_class(
             XmlDescriptor,
+            scope_ids=Mock(),
             field_data=field_data,
-            scope_ids=Mock()
         ).editable_metadata_fields
 
     def get_descriptor(self, field_data):
@@ -379,3 +387,78 @@ class TestDeserializeTimedelta(TestDeserialize):
         self.assertDeserializeEqual('1 day 12 hours 59 minutes 59 seconds',
             '"1 day 12 hours 59 minutes 59 seconds"')
         self.assertDeserializeNonString()
+
+
+class TestXmlAttributes(XModuleXmlImportTest):
+
+    def test_unknown_attribute(self):
+        assert_false(hasattr(CourseDescriptor, 'unknown_attr'))
+        course = self.process_xml(CourseFactory.build(unknown_attr='value'))
+        assert_false(hasattr(course, 'unknown_attr'))
+        assert_equals('value', course.xml_attributes['unknown_attr'])
+
+    def test_known_attribute(self):
+        assert_true(hasattr(CourseDescriptor, 'show_chat'))
+        course = self.process_xml(CourseFactory.build(show_chat='true'))
+        assert_true(course.show_chat)
+        assert_not_in('show_chat', course.xml_attributes)
+
+    def test_rerandomize_in_policy(self):
+        # Rerandomize isn't a basic attribute of Sequence
+        assert_false(hasattr(SequenceDescriptor, 'rerandomize'))
+
+        root = SequenceFactory.build(policy={'rerandomize': 'never'})
+        ProblemFactory.build(parent=root)
+
+        seq = self.process_xml(root)
+
+        # Rerandomize is added to the constructed sequence via the InheritanceMixin
+        assert_equals('never', seq.rerandomize)
+
+        # Rerandomize is a known value coming from policy, and shouldn't appear
+        # in xml_attributes
+        assert_not_in('rerandomize', seq.xml_attributes)
+
+    def test_attempts_in_policy(self):
+        # attempts isn't a basic attribute of Sequence
+        assert_false(hasattr(SequenceDescriptor, 'attempts'))
+
+        root = SequenceFactory.build(policy={'attempts': '1'})
+        ProblemFactory.build(parent=root)
+
+        seq = self.process_xml(root)
+
+        # attempts isn't added to the constructed sequence, because
+        # it's not in the InheritanceMixin
+        assert_false(hasattr(seq, 'attempts'))
+
+        # attempts is an unknown attribute, so we should include it
+        # in xml_attributes so that it gets written out (despite the misleading
+        # name)
+        assert_in('attempts', seq.xml_attributes)
+
+    def test_inheritable_attribute(self):
+        # days_early_for_beta isn't a basic attribute of Sequence
+        assert_false(hasattr(SequenceDescriptor, 'days_early_for_beta'))
+
+        # days_early_for_beta is added by InheritanceMixin
+        assert_true(hasattr(InheritanceMixin, 'days_early_for_beta'))
+
+        root = SequenceFactory.build(policy={'days_early_for_beta': '2'})
+        ProblemFactory.build(parent=root)
+
+        # InheritanceMixin will be used when processing the XML
+        assert_in(InheritanceMixin, root.xblock_mixins)
+
+        seq = self.process_xml(root)
+
+        assert_equals(seq.unmixed_class, SequenceDescriptor)
+        assert_not_equals(type(seq), SequenceDescriptor)
+
+        # days_early_for_beta is added to the constructed sequence, because
+        # it's in the InheritanceMixin
+        assert_equals(2, seq.days_early_for_beta)
+
+        # days_early_for_beta is a known attribute, so we shouldn't include it
+        # in xml_attributes
+        assert_not_in('days_early_for_beta', seq.xml_attributes)

common/lib/xmodule/xmodule/tests/xml/__init__.py

+"""
+Xml parsing tests for XModules
+"""
+import pprint
+from mock import Mock
+
+from xmodule.x_module import XMLParsingSystem
+from xmodule.mako_module import MakoDescriptorSystem
+from xmodule.modulestore.xml import create_block_from_xml
+
+
+class InMemorySystem(XMLParsingSystem, MakoDescriptorSystem):  # pylint: disable=abstract-method
+    """
+    The simplest possible XMLParsingSystem
+    """
+    def __init__(self, xml_import_data):
+        self.org = xml_import_data.org
+        self.course = xml_import_data.course
+        self.default_class = xml_import_data.default_class
+        self._descriptors = {}
+        super(InMemorySystem, self).__init__(
+            policy=xml_import_data.policy,
+            process_xml=self.process_xml,
+            load_item=self.load_item,
+            error_tracker=Mock(),
+            resources_fs=xml_import_data.filesystem,
+            mixins=xml_import_data.xblock_mixins,
+            render_template=lambda template, context: pprint.pformat((template, context))
+        )
+
+    def process_xml(self, xml):  # pylint: disable=method-hidden
+        """Parse `xml` as an XBlock, and add it to `self._descriptors`"""
+        descriptor = create_block_from_xml(xml, self, self.org, self.course, self.default_class)
+        self._descriptors[descriptor.location.url()] = descriptor
+        return descriptor
+
+    def load_item(self, location):  # pylint: disable=method-hidden
+        """Return the descriptor loaded for `location`"""
+        return self._descriptors[location]
+
+
+class XModuleXmlImportTest(object):
+    """Base class for tests that use basic XML parsing"""
+    def process_xml(self, xml_import_data):
+        """Use the `xml_import_data` to import an :class:`XBlock` from XML."""
+        system = InMemorySystem(xml_import_data)
+        return system.process_xml(xml_import_data.xml_string)

common/lib/xmodule/xmodule/tests/xml/factories.py

+"""
+Factories for generating edXML for testing XModule import
+"""
+
+import inspect
+
+from fs.memoryfs import MemoryFS
+from factory import Factory, lazy_attribute, post_generation, Sequence
+from lxml import etree
+
+from xmodule.modulestore.inheritance import InheritanceMixin
+
+
+class XmlImportData(object):
+    """
+    Class to capture all of the data needed to actually run an XML import,
+    so that the Factories have something to generate
+    """
+    def __init__(self, xml_node, xml=None, org=None, course=None,
+                 default_class=None, policy=None,
+                 filesystem=None, parent=None,
+                 xblock_mixins=()):
+
+        self._xml_node = xml_node
+        self._xml_string = xml
+        self.org = org
+        self.course = course
+        self.default_class = default_class
+        self.filesystem = filesystem
+        self.xblock_mixins = xblock_mixins
+        self.parent = parent
+
+        if policy is None:
+            self.policy = {}
+        else:
+            self.policy = policy
+
+    @property
+    def xml_string(self):
+        """Return the stringified version of the generated xml"""
+        if self._xml_string is not None:
+            return self._xml_string
+
+        return etree.tostring(self._xml_node)
+
+    def __repr__(self):
+        return u"XmlImportData{!r}".format((
+            self._xml_node, self._xml_string, self.org,
+            self.course, self.default_class, self.policy,
+            self.filesystem, self.parent, self.xblock_mixins
+        ))
+
+
+# Extract all argument names used to construct XmlImportData objects,
+# so that the factory doesn't treat them as XML attributes
+XML_IMPORT_ARGS = inspect.getargspec(XmlImportData.__init__).args
+
+
+class XmlImportFactory(Factory):
+    """
+    Factory for generating XmlImportData's, which can hold all the data needed
+    to run an XModule XML import
+    """
+    FACTORY_FOR = XmlImportData
+
+    filesystem = MemoryFS()
+    xblock_mixins = (InheritanceMixin,)
+    url_name = Sequence(str)
+    attribs = {}
+    policy = {}
+    tag = 'unknown'
+
+    @classmethod
+    def _adjust_kwargs(cls, **kwargs):
+        """
+        Adjust the kwargs to be passed to the generated class.
+
+        Any kwargs that match :fun:`XmlImportData.__init__` will be passed
+        through. Any other unknown `kwargs` will be treated as XML attributes
+
+        :param tag: xml tag for the generated :class:`Element` node
+        :param text: (Optional) specifies the text of the generated :class:`Element`.
+        :param policy: (Optional) specifies data for the policy json file for this node
+        :type policy: dict
+        :param attribs: (Optional) specify attributes for the XML node
+        :type attribs: dict
+        """
+        tag = kwargs.pop('tag', 'unknown')
+        kwargs['policy'] = {'{tag}/{url_name}'.format(tag=tag, url_name=kwargs['url_name']): kwargs['policy']}
+
+        kwargs['xml_node'].text = kwargs.pop('text', None)
+
+        kwargs['xml_node'].attrib.update(kwargs.pop('attribs', {}))
+        for key in kwargs.keys():
+            if key not in XML_IMPORT_ARGS:
+                kwargs['xml_node'].set(key, kwargs.pop(key))
+
+        return kwargs
+
+    @lazy_attribute
+    def xml_node(self):
+        """An :class:`xml.etree.Element`"""
+        return etree.Element(self.tag)
+
+    @post_generation
+    def parent(self, _create, extracted, **_):
+        """Hook to merge this xml into a parent xml node"""
+        if extracted is None:
+            return
+
+        extracted._xml_node.append(self._xml_node)  # pylint: disable=no-member, protected-access
+        extracted.policy.update(self.policy)
+
+
+class CourseFactory(XmlImportFactory):
+    """Factory for <course> nodes"""
+    tag = 'course'
+
+
+class SequenceFactory(XmlImportFactory):
+    """Factory for <sequential> nodes"""
+    tag = 'sequential'
+
+
+class ProblemFactory(XmlImportFactory):
+    """Factory for <problem> nodes"""
+    tag = 'problem'
+    text = '<h1>Empty Problem!</h1>'

common/lib/xmodule/xmodule/tests/xml/test_inheritance.py

+"""
+Test that inherited fields work correctly when parsing XML
+"""
+from nose.tools import assert_equals  # pylint: disable=no-name-in-module
+
+from xmodule.tests.xml import XModuleXmlImportTest
+from xmodule.tests.xml.factories import CourseFactory, SequenceFactory, ProblemFactory
+
+
+class TestInheritedFieldParsing(XModuleXmlImportTest):
+    """
+    Test that inherited fields work correctly when parsing XML
+
+    """
+    def test_null_string(self):
+        # Test that the string inherited fields are passed through 'deserialize_field',
+        # which converts the string "null" to the python value None
+        root = CourseFactory.build(days_early_for_beta="null")
+        sequence = SequenceFactory.build(parent=root)
+        ProblemFactory.build(parent=sequence)
+
+        course = self.process_xml(root)
+        assert_equals(None, course.days_early_for_beta)
+
+        sequence = course.get_children()[0]
+        assert_equals(None, sequence.days_early_for_beta)
+
+        problem = sequence.get_children()[0]
+        assert_equals(None, problem.days_early_for_beta)

common/lib/xmodule/xmodule/tests/xml/test_policy.py

+"""
+Tests that policy json files import correctly when loading XML
+"""
+
+from nose.tools import assert_equals, assert_raises  # pylint: disable=no-name-in-module
+
+from xmodule.tests.xml.factories import CourseFactory
+from xmodule.tests.xml import XModuleXmlImportTest
+
+
+class TestPolicy(XModuleXmlImportTest):
+    """
+    Tests that policy json files import correctly when loading xml
+    """
+    def test_no_attribute_mapping(self):
+        # Policy files are json, and thus the values aren't passed through 'deserialize_field'
+        # Therefor, the string 'null' is passed unchanged to the Float field, which will trigger
+        # a ValueError
+        with assert_raises(ValueError):
+            course = self.process_xml(CourseFactory.build(policy={'days_early_for_beta': 'null'}))
+
+            # Trigger the exception by looking at the imported data
+            course.days_early_for_beta  # pylint: disable=pointless-statement
+
+    def test_course_policy(self):
+        course = self.process_xml(CourseFactory.build(policy={'days_early_for_beta': None}))
+        assert_equals(None, course.days_early_for_beta)
+
+        course = self.process_xml(CourseFactory.build(policy={'days_early_for_beta': 9}))
+        assert_equals(9, course.days_early_for_beta)

common/lib/xmodule/xmodule/video_module.py

@@ -24,7 +24,7 @@ from django.conf import settings
 from xmodule.x_module import XModule
 from xmodule.editing_module import TabsEditingDescriptor
 from xmodule.raw_module import EmptyDataRawDescriptor
-from xmodule.xml_module import is_pointer_tag, name_to_pathname
+from xmodule.xml_module import is_pointer_tag, name_to_pathname, deserialize_field
 from xmodule.modulestore import Location
 from xblock.fields import Scope, String, Boolean, Float, List, Integer, ScopeIds
 
@@ -217,7 +217,7 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
         # For backwards compatibility -- if we've got XML data, parse
         # it out and set the metadata fields
         if self.data:
-            field_data = VideoDescriptor._parse_video_xml(self.data)
+            field_data = self._parse_video_xml(self.data)
             self._field_data.set_many(self, field_data)
             del self.data
 
@@ -241,18 +241,17 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
         if is_pointer_tag(xml_object):
             filepath = cls._format_filepath(xml_object.tag, name_to_pathname(url_name))
             xml_data = etree.tostring(cls.load_file(filepath, system.resources_fs, location))
-        field_data = VideoDescriptor._parse_video_xml(xml_data)
+        field_data = cls._parse_video_xml(xml_data)
         field_data['location'] = location
         kvs = InheritanceKeyValueStore(initial_values=field_data)
         field_data = DbModel(kvs)
         video = system.construct_xblock_from_class(
             cls,
-            field_data,
-
             # We're loading a descriptor, so student_id is meaningless
             # We also don't have separate notions of definition and usage ids yet,
             # so we use the location for both
-            ScopeIds(None, location.category, location, location)
+            ScopeIds(None, location.category, location, location),
+            field_data,
         )
         return video
 
@@ -292,8 +291,8 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
             xml.append(ele)
         return xml
 
-    @staticmethod
-    def _parse_youtube(data):
+    @classmethod
+    def _parse_youtube(cls, data):
         """
         Parses a string of Youtube IDs such as "1.0:AXdE34_U,1.5:VO3SxfeD"
         into a dictionary. Necessary for backwards compatibility with
@@ -310,14 +309,14 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
                 # Handle the fact that youtube IDs got double-quoted for a period of time.
                 # Note: we pass in "VideoFields.youtube_id_1_0" so we deserialize as a String--
                 # it doesn't matter what the actual speed is for the purposes of deserializing.
-                youtube_id = VideoDescriptor._deserialize(VideoFields.youtube_id_1_0.name, pieces[1])
+                youtube_id = deserialize_field(cls.youtube_id_1_0, pieces[1])
                 ret[speed] = youtube_id
             except (ValueError, IndexError):
                 log.warning('Invalid YouTube ID: %s' % video)
         return ret
 
-    @staticmethod
-    def _parse_video_xml(xml_data):
+    @classmethod
+    def _parse_video_xml(cls, xml_data):
         """
         Parse video fields out of xml_data. The fields are set if they are
         present in the XML.
@@ -326,8 +325,8 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
         field_data = {}
 
         conversions = {
-            'start_time': VideoDescriptor._parse_time,
-            'end_time': VideoDescriptor._parse_time
+            'start_time': cls._parse_time,
+            'end_time': cls._parse_time
         }
 
         # Convert between key names for certain attributes --
@@ -349,10 +348,10 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
         for attr, value in xml.items():
             if attr in compat_keys:
                 attr = compat_keys[attr]
-            if attr in VideoDescriptor.metadata_to_strip + ('url_name', 'name'):
+            if attr in cls.metadata_to_strip + ('url_name', 'name'):
                 continue
             if attr == 'youtube':
-                speeds = VideoDescriptor._parse_youtube(value)
+                speeds = cls._parse_youtube(value)
                 for speed, youtube_id in speeds.items():
                     # should have made these youtube_id_1_00 for
                     # cleanliness, but hindsight doesn't need glasses
@@ -367,20 +366,13 @@ class VideoDescriptor(VideoFields, TabsEditingDescriptor, EmptyDataRawDescriptor
                 else:
                 # We export values with json.dumps (well, except for Strings, but
                 # for about a month we did it for Strings also).
-                    value = VideoDescriptor._deserialize(attr, value)
+                    value = deserialize_field(cls.fields[attr], value)
                 field_data[attr] = value
 
         return field_data
 
     @classmethod
-    def _deserialize(cls, attr, value):
-        """
-        Handles deserializing values that may have been encoded with json.dumps.
-        """
-        return cls.get_map_for_field(attr).from_xml(value)
-
-    @staticmethod
-    def _parse_time(str_time):
+    def _parse_time(cls, str_time):
         """Converts s in '12:34:45' format to seconds. If s is
         None, returns empty string"""
         if not str_time:

common/lib/xmodule/xmodule/xml_module.py

@@ -62,23 +62,6 @@ def get_metadata_from_xml(xml_object, remove=True):
         xml_object.remove(meta)
     return dmdata
 
-_AttrMapBase = namedtuple('_AttrMap', 'from_xml to_xml')
-
-
-class AttrMap(_AttrMapBase):
-    """
-    A class that specifies two functions:
-
-        from_xml: convert value from the xml representation into
-            an internal python representation
-
-        to_xml: convert the internal python representation into
-            the value to store in the xml.
-    """
-    def __new__(_cls, from_xml=lambda x: x,
-                to_xml=lambda x: x):
-        return _AttrMapBase.__new__(_cls, from_xml, to_xml)
-
 
 def serialize_field(value):
     """
@@ -167,20 +150,6 @@ class XmlDescriptor(XModuleDescriptor):
     metadata_to_export_to_policy = ('discussion_topics', 'checklists')
 
     @classmethod
-    def get_map_for_field(cls, attr):
-        """
-        Returns a serialize/deserialize AttrMap for the given field of a class.
-
-        Searches through fields defined by cls to find one named attr.
-        """
-        if attr in cls.fields:
-            from_xml = lambda val: deserialize_field(cls.fields[attr], val)
-            to_xml = lambda val: serialize_field(val)
-            return AttrMap(from_xml, to_xml)
-        else:
-            return AttrMap()
-
-    @classmethod
     def definition_from_xml(cls, xml_object, system):
         """
         Return the definition to be passed to the newly created descriptor
@@ -248,8 +217,7 @@ class XmlDescriptor(XModuleDescriptor):
             # give the class a chance to fix it up. The file will be written out
             # again in the correct format.  This should go away once the CMS is
             # online and has imported all current (fall 2012) courses from xml
-            if not system.resources_fs.exists(filepath) and hasattr(
-                    cls, 'backcompat_paths'):
+            if not system.resources_fs.exists(filepath) and hasattr(cls, 'backcompat_paths'):
                 candidates = cls.backcompat_paths(filepath)
                 for candidate in candidates:
                     if system.resources_fs.exists(candidate):
@@ -274,19 +242,19 @@ class XmlDescriptor(XModuleDescriptor):
 
         Returns a dictionary {key: value}.
         """
-        metadata = {}
-        for attr in xml_object.attrib:
-            val = xml_object.get(attr)
-            if val is not None:
-                # VS[compat].  Remove after all key translations done
-                attr = cls._translate(attr)
-
-                if attr in cls.metadata_to_strip:
-                    # don't load these
-                    continue
-
-                attr_map = cls.get_map_for_field(attr)
-                metadata[attr] = attr_map.from_xml(val)
+        metadata = {'xml_attributes': {}}
+        for attr, val in xml_object.attrib.iteritems():
+            # VS[compat].  Remove after all key translations done
+            attr = cls._translate(attr)
+
+            if attr in cls.metadata_to_strip:
+                # don't load these
+                continue
+
+            if attr not in cls.fields:
+                metadata['xml_attributes'][attr] = val
+            else:
+                metadata[attr] = deserialize_field(cls.fields[attr], val)
         return metadata
 
     @classmethod
@@ -295,9 +263,14 @@ class XmlDescriptor(XModuleDescriptor):
         Add the keys in policy to metadata, after processing them
         through the attrmap.  Updates the metadata dict in place.
         """
-        for attr in policy:
-            attr_map = cls.get_map_for_field(attr)
-            metadata[cls._translate(attr)] = attr_map.from_xml(policy[attr])
+        for attr, value in policy.iteritems():
+            attr = cls._translate(attr)
+            if attr not in cls.fields:
+                # Store unknown attributes coming from policy.json
+                # in such a way that they will export to xml unchanged
+                metadata['xml_attributes'][attr] = value
+            else:
+                metadata[attr] = value
 
     @classmethod
     def from_xml(cls, xml_data, system, org=None, course=None):
@@ -357,11 +330,7 @@ class XmlDescriptor(XModuleDescriptor):
         field_data.update(definition)
         field_data['children'] = children
 
-        field_data['xml_attributes'] = {}
         field_data['xml_attributes']['filename'] = definition.get('filename', ['', None])  # for git link
-        for key, value in metadata.items():
-            if key not in cls.fields:
-                field_data['xml_attributes'][key] = value
         field_data['location'] = location
         field_data['category'] = xml_object.tag
         kvs = InheritanceKeyValueStore(initial_values=field_data)
@@ -369,12 +338,11 @@ class XmlDescriptor(XModuleDescriptor):
 
         return system.construct_xblock_from_class(
             cls,
-            field_data,
-
             # We're loading a descriptor, so student_id is meaningless
             # We also don't have separate notions of definition and usage ids yet,
             # so we use the location for both
-            ScopeIds(None, location.category, location, location)
+            ScopeIds(None, location.category, location, location),
+            field_data,
         )
 
     @classmethod
@@ -415,18 +383,11 @@ class XmlDescriptor(XModuleDescriptor):
         # Set the tag so we get the file path right
         xml_object.tag = self.category
 
-        def val_for_xml(attr):
-            """Get the value for this attribute that we want to store.
-            (Possible format conversion through an AttrMap).
-             """
-            attr_map = self.get_map_for_field(attr)
-            return attr_map.to_xml(self._field_data.get(self, attr))
-
         # Add the non-inherited metadata
         for attr in sorted(own_metadata(self)):
             # don't want e.g. data_dir
             if attr not in self.metadata_to_strip and attr not in self.metadata_to_export_to_policy:
-                val = val_for_xml(attr)
+                val = serialize_field(self._field_data.get(self, attr))
                 try:
                     xml_object.set(attr, val)
                 except Exception, e:

common/static/js/pdfviewer.js

@@ -200,6 +200,18 @@ PDFJS.disableWorker = true;
             document.getElementById('numPages').textContent = 'of ' + pdfDocument.numPages;
             $("#pageNumber").max = pdfDocument.numPages;
             $("#pageNumber").val(pageNum);
+
+            // Enable/disable the previous/next buttons
+            if (pageNum <= 1) {
+              $("#previous").addClass("is-disabled");
+            } else {
+              $("#previous").removeClass("is-disabled");
+            }
+            if (pageNum >= pdfDocument.numPages) {
+              $("#next").addClass("is-disabled");
+            } else {
+              $("#next").removeClass("is-disabled");
+            }
         }
 
         // Go to previous page

common/test/data/open_ended/combinedopenended/SampleQuestionImageUpload.xml

+<combinedopenended attempts="1" display_name = "Humanities Question -- Machine Assessed" accept_file_upload="True">
+    <rubric>
+        <rubric>
+            <category>
+                <description>Writing Applications</description>
+                <option> The essay loses focus, has little information or supporting details, and the organization makes it difficult to follow.</option>
+                <option> The essay presents a mostly unified theme, includes sufficient information to convey the theme, and is generally organized well.</option>
+            </category>
+            <category>
+                <description> Language Conventions </description>
+                <option> The essay demonstrates a reasonable command of proper spelling and grammar. </option>
+                <option> The essay demonstrates superior command of proper spelling and grammar.</option>
+            </category>
+        </rubric>
+    </rubric>
+    <prompt>
+        <h4>Censorship in the Libraries</h4>
+        <p>"All of us can think of a book that we hope none of our children or any other children have taken off the shelf. But if I have the right to remove that book from the shelf -- that work I abhor -- then you also have exactly the same right and so does everyone else. And then we have no books left on the shelf for any of us." --Katherine Paterson, Author</p>
+        <p>Write a persuasive essay to a newspaper reflecting your views on censorship in libraries. Do you believe that certain materials, such as books, music, movies, magazines, etc., should be removed from the shelves if they are found offensive? Support your position with convincing arguments from your own experience, observations, and/or reading.</p>
+    </prompt>
+    <task>
+        <selfassessment/>
+    </task>
+</combinedopenended>
\ No newline at end of file

common/test/data/open_ended/course/2012_Fall.xml

@@ -2,6 +2,7 @@
   <chapter url_name="Overview">
     <combinedopenended url_name="SampleQuestion"/>
     <combinedopenended url_name="SampleQuestion1Attempt"/>
+      <combinedopenended url_name="SampleQuestionImageUpload"/>
     <peergrading url_name="PeerGradingSample"/>
     <peergrading url_name="PeerGradingScored"/>
     <peergrading url_name="PeerGradingLinked"/>

common/test/data/uploads/.gitignore

+test
+test2
+asset.html

common/test/data/uploads/test

-This is an arbitrary file for testing uploads
\ No newline at end of file

docs/internal/discussion.md

@@ -22,11 +22,11 @@ For debugging, it's often more convenient to have elasticsearch running in a ter
 
 ## Setting up the discussion service
 
-First, make sure that you have access to the [github repository](https://github.com/rll/cs_comments_service). If this were not the case, send an email to dementrock@gmail.com.
+You can retrieve the source code from the [github repository](https://github.com/edx/cs_comments_service).
 
-First go into the mitx_all directory. Then type
+First go into the edx_all directory. Then type
 
-    git clone git@github.com:rll/cs_comments_service.git
+    git clone https://github.com/edx/cs_comments_service.git
     cd cs_comments_service/
 
 If you see a prompt asking "Do you wish to trust this .rvmrc file?", type "y"
@@ -52,6 +52,13 @@ It's done! Launch the app now:
 
     ruby app.rb
 
+## Integrating with the edx platform
+
+The API key must match on both sides. It is configured here:
+* edx-platform: COMMENTS_SERVICE_KEY in your dev.py file (dev environment) or ENV_TOKENS (prod environment)
+* cs_comments_service: api_key in the application.yml file (dev environment) or ENV variable (prod environment)
+
+
 ## Running the delayed job worker
 
 In the discussion service, notifications are handled asynchronously using a third party gem called delayed_job. If you want to test this functionality, run the following command in a separate tab:

docs/internal/testing.md

@@ -128,11 +128,11 @@ other module level tests include
 
 To run a single django test class:
 
-    rake test_lms[courseware.tests.tests:testViewAuth]
+    rake test_lms[lms/djangoapps/courseware/tests/tests.py:ActivateLoginTest]
 
 To run a single django test:
 
-    rake test_lms[courseware.tests.tests:TestViewAuth.test_dark_launch]
+    rake test_lms[lms/djangoapps/courseware/tests/tests.py:ActivateLoginTest.test_activate_login]
 
 To re-run all failing django tests from lms or cms:
 

lms/djangoapps/branding/views.py

 from django.conf import settings
 from django.core.urlresolvers import reverse
+from django.http import Http404
 from django.shortcuts import redirect
 from django_future.csrf import ensure_csrf_cookie
 from mitxmako.shortcuts import render_to_response
@@ -48,10 +49,9 @@ def courses(request):
     if settings.MITX_FEATURES.get('ENABLE_MKTG_SITE', False):
         return redirect(marketing_link('COURSES'), permanent=True)
 
-    university = branding.get_university(request.META.get('HTTP_HOST'))
-    if university == 'edge':
-        return render_to_response('university_profile/edge.html', {})
+    if not settings.MITX_FEATURES.get('COURSES_ARE_BROWSABLE'):        
+        raise Http404
 
     #  we do not expect this case to be reached in cases where
-    #  marketing and edge are enabled
+    #  marketing is enabled or the courses are not browsable
     return courseware.views.courses(request)

lms/djangoapps/courseware/module_render.py

@@ -306,7 +306,7 @@ def get_module_for_descriptor_internal(user, descriptor, field_data_cache, cours
         # Construct the key for the module
         key = KeyValueStore.Key(
             scope=Scope.user_state,
-            student_id=user.id,
+            user_id=user.id,
             block_scope_id=descriptor.location,
             field_name='grade'
         )

lms/djangoapps/courseware/views.py

@@ -609,7 +609,8 @@ def course_about(request, course_id):
     registration_price = 0
     in_cart = False
     reg_then_add_to_cart_link = ""
-    if settings.MITX_FEATURES.get('ENABLE_PAID_COURSE_REGISTRATION'):
+    if (settings.MITX_FEATURES.get('ENABLE_SHOPPING_CART') and
+        settings.MITX_FEATURES.get('ENABLE_PAID_COURSE_REGISTRATION')):
         registration_price = CourseMode.min_course_price_for_currency(course_id,
                                                                       settings.PAID_COURSE_REGISTRATION_CURRENCY[0])
         if request.user.is_authenticated():

lms/djangoapps/debug/management/commands/dump_xml_courses.py

+"""
+Export all xml courses in a diffable format.
+
+This command loads all of the xml courses in the configured DATA_DIR.
+For each of the courses, it loops through all of the modules, and dumps
+each as a separate output file containing the json representation
+of each of its fields (including those fields that are set as default values).
+"""
+
+from __future__ import print_function
+
+import json
+from path import path
+
+from django.core.management.base import BaseCommand, CommandError
+from django.conf import settings
+
+from xmodule.modulestore.xml import XMLModuleStore
+
+
+class Command(BaseCommand):
+    """
+    Django management command to export diffable representations of all xml courses
+    """
+    help = '''Dump the in-memory representation of all xml courses in a diff-able format'''
+    args = '<export path>'
+
+    def handle(self, *args, **options):
+        if len(args) != 1:
+            raise CommandError('Must called with arguments: {}'.format(self.args))
+
+        xml_module_store = XMLModuleStore(
+            data_dir=settings.DATA_DIR,
+            default_class='xmodule.hidden_module.HiddenDescriptor',
+            load_error_modules=True,
+            xblock_mixins=settings.XBLOCK_MIXINS,
+        )
+
+        export_dir = path(args[0])
+
+        for course_id, course_modules in xml_module_store.modules.iteritems():
+            course_path = course_id.replace('/', '_')
+            for location, descriptor in course_modules.iteritems():
+                location_path = location.url().replace('/', '_')
+                data = {}
+                for field_name, field in descriptor.fields.iteritems():
+                    try:
+                        data[field_name] = field.read_json(descriptor)
+                    except Exception as exc:  # pylint: disable=broad-except
+                        data[field_name] = {
+                            '$type': str(type(exc)),
+                            '$value': descriptor._field_data.get(descriptor, field_name)  # pylint: disable=protected-access
+                        }
+
+                outdir = export_dir / course_path
+                outdir.makedirs_p()
+                with open(outdir / location_path + '.json', 'w') as outfile:
+                    json.dump(data, outfile, sort_keys=True, indent=4)
+                    print('', file=outfile)

lms/djangoapps/open_ended_grading/tests.py

@@ -18,8 +18,9 @@ from xmodule.modulestore.django import modulestore
 from xmodule.x_module import ModuleSystem
 from xblock.fields import ScopeIds
 
-from open_ended_grading import staff_grading_service, views
+from open_ended_grading import staff_grading_service, views, utils
 from courseware.access import _course_staff_group_name
+from student.models import unique_id_for_user
 
 import logging
 
@@ -46,6 +47,57 @@ class EmptyStaffGradingService(object):
         """
         return json.dumps({'success': True, 'error': 'No problems found.'})
 
+def make_instructor(course, user_email):
+    """
+    Makes a given user an instructor in a course.
+    """
+    group_name = _course_staff_group_name(course.location)
+    group = Group.objects.create(name=group_name)
+    group.user_set.add(User.objects.get(email=user_email))
+
+class StudentProblemListMockQuery(object):
+    """
+    Mock controller query service for testing student problem list functionality.
+    """
+    def get_grading_status_list(self, *args, **kwargs):
+        """
+        Get a mock grading status list with locations from the open_ended test course.
+        @returns: json formatted grading status message.
+        """
+        grading_status_list = json.dumps(
+            {
+                "version": 1,
+                "problem_list": [
+                    {
+                        "problem_name": "Test1",
+                        "grader_type": "IN",
+                        "eta_available": True,
+                        "state": "Finished",
+                        "eta": 259200,
+                        "location": "i4x://edX/open_ended/combinedopenended/SampleQuestion1Attempt"
+                    },
+                    {
+                        "problem_name": "Test2",
+                        "grader_type": "NA",
+                        "eta_available": True,
+                        "state": "Waiting to be Graded",
+                        "eta": 259200,
+                        "location": "i4x://edX/open_ended/combinedopenended/SampleQuestion"
+                    },
+                    {
+                        "problem_name": "Test3",
+                        "grader_type": "PE",
+                        "eta_available": True,
+                        "state": "Waiting to be Graded",
+                        "eta": 259200,
+                        "location": "i4x://edX/open_ended/combinedopenended/SampleQuestion454"
+                    },
+                ],
+                "success": True
+            }
+        )
+        return grading_status_list
+
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
 class TestStaffGradingService(ModuleStoreTestCase, LoginEnrollmentTestCase):
     '''
@@ -67,12 +119,7 @@ class TestStaffGradingService(ModuleStoreTestCase, LoginEnrollmentTestCase):
         self.course_id = "edX/toy/2012_Fall"
         self.toy = modulestore().get_course(self.course_id)
 
-        def make_instructor(course):
-            group_name = _course_staff_group_name(course.location)
-            group = Group.objects.create(name=group_name)
-            group.user_set.add(User.objects.get(email=self.instructor))
-
-        make_instructor(self.toy)
+        make_instructor(self.toy, self.instructor)
 
         self.mock_service = staff_grading_service.staff_grading_service()
 
@@ -324,7 +371,7 @@ class TestPeerGradingService(ModuleStoreTestCase, LoginEnrollmentTestCase):
 
 
 @override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
-class TestPanel(ModuleStoreTestCase, LoginEnrollmentTestCase):
+class TestPanel(ModuleStoreTestCase):
     """
     Run tests on the open ended panel
     """
@@ -343,7 +390,15 @@ class TestPanel(ModuleStoreTestCase, LoginEnrollmentTestCase):
         found_module, peer_grading_module = views.find_peer_grading_module(self.course)
         self.assertTrue(found_module)
 
-    @patch('open_ended_grading.views.controller_qs', controller_query_service.MockControllerQueryService(settings.OPEN_ENDED_GRADING_INTERFACE, views.system))
+    @patch(
+        'open_ended_grading.utils.create_controller_query_service',
+        Mock(
+            return_value=controller_query_service.MockControllerQueryService(
+                settings.OPEN_ENDED_GRADING_INTERFACE,
+                utils.system
+            )
+        )
+    )
     def test_problem_list(self):
         """
         Ensure that the problem list from the grading controller server can be rendered properly locally
@@ -370,4 +425,44 @@ class TestPeerGradingFound(ModuleStoreTestCase):
         """
 
         found, url = views.find_peer_grading_module(self.course)
-        self.assertEqual(found, False)
+        self.assertEqual(found, False)
\ No newline at end of file
+
+@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
+class TestStudentProblemList(ModuleStoreTestCase):
+    """
+    Test if the student problem list correctly fetches and parses problems.
+    """
+
+    def setUp(self):
+        # Load an open ended course with several problems.
+        self.course_name = 'edX/open_ended/2012_Fall'
+        self.course = modulestore().get_course(self.course_name)
+        self.user = factories.UserFactory()
+        # Enroll our user in our course and make them an instructor.
+        make_instructor(self.course, self.user.email)
+
+    @patch(
+        'open_ended_grading.utils.create_controller_query_service',
+        Mock(return_value=StudentProblemListMockQuery())
+    )
+    def test_get_problem_list(self):
+        """
+        Test to see if the StudentProblemList class can get and parse a problem list from ORA.
+        Mock the get_grading_status_list function using StudentProblemListMockQuery.
+        """
+        # Initialize a StudentProblemList object.
+        student_problem_list = utils.StudentProblemList(self.course.id, unique_id_for_user(self.user))
+        # Get the initial problem list from ORA.
+        success = student_problem_list.fetch_from_grading_service()
+        # Should be successful, and we should have three problems.  See mock class for details.
+        self.assertTrue(success)
+        self.assertEqual(len(student_problem_list.problem_list), 3)
+
+        # See if the problem locations are valid.
+        valid_problems = student_problem_list.add_problem_data(reverse('courses'))
+        # One location is invalid, so we should now have two.
+        self.assertEqual(len(valid_problems), 2)
+        # Ensure that human names are being set properly.
+        self.assertEqual(valid_problems[0]['grader_type_display_name'], "Instructor Assessment")
+
+

lms/djangoapps/open_ended_grading/utils.py

+import json
+
 from xmodule.modulestore import search
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem
+from xmodule.x_module import ModuleSystem
+from xmodule.open_ended_grading_classes.controller_query_service import ControllerQueryService
+from xmodule.open_ended_grading_classes.grading_service_module import GradingServiceError
+
+from django.utils.translation import ugettext as _
+from django.conf import settings
+
+from mitxmako.shortcuts import render_to_string
+
+from xblock.field_data import DictFieldData
+
 import logging
 
 log = logging.getLogger(__name__)
 
+GRADER_DISPLAY_NAMES = {
+    'ML': _("AI Assessment"),
+    'PE': _("Peer Assessment"),
+    'NA': _("Not yet available"),
+    'BC': _("Automatic Checker"),
+    'IN': _("Instructor Assessment"),
+    }
+
+STUDENT_ERROR_MESSAGE = _("Error occurred while contacting the grading service.  Please notify course staff.")
+STAFF_ERROR_MESSAGE = _("Error occurred while contacting the grading service.  Please notify your edX point of contact.")
+
+system = ModuleSystem(
+    ajax_url=None,
+    track_function=None,
+    get_module=None,
+    render_template=render_to_string,
+    replace_urls=None,
+    xmodule_field_data=DictFieldData({}),
+    )
+
+def generate_problem_url(problem_url_parts, base_course_url):
+    """
+    From a list of problem url parts generated by search.path_to_location and a base course url, generates a url to a problem
+    @param problem_url_parts: Output of search.path_to_location
+    @param base_course_url: Base url of a given course
+    @return: A path to the problem
+    """
+    problem_url = base_course_url + "/"
+    for i, part in enumerate(problem_url_parts):
+        if part is not None:
+            if i == 1:
+                problem_url += "courseware/"
+            problem_url += part + "/"
+    return problem_url
+
 def does_location_exist(course_id, location):
     """
     Checks to see if a valid module exists at a given location (ie has not been deleted)
@@ -25,3 +73,102 @@ def does_location_exist(course_id, location):
             log.warn(("Got an unexpected NoPathToItem error in staff grading with a non-draft location {0}. "
                       "Ensure that the location is valid.").format(location))
         return False
+
+def create_controller_query_service():
+    """
+    Return an instance of a service that can query edX ORA.
+    """
+
+    return ControllerQueryService(settings.OPEN_ENDED_GRADING_INTERFACE, system)
+
+class StudentProblemList(object):
+    """
+    Get a list of problems that the student has attempted from ORA.
+    Add in metadata as needed.
+    """
+    def __init__(self, course_id, user_id):
+        """
+        @param course_id: The id of a course object.  Get using course.id.
+        @param user_id: The anonymous id of the user, from the unique_id_for_user function.
+        """
+        self.course_id = course_id
+        self.user_id = user_id
+
+        # We want to append this string to all of our error messages.
+        self.course_error_ending = _("for course {0} and student {1}.").format(self.course_id, user_id)
+
+        # This is our generic error message.
+        self.error_text = STUDENT_ERROR_MESSAGE
+        self.success = False
+
+        # Create a service to query edX ORA.
+        self.controller_qs = create_controller_query_service()
+
+    def fetch_from_grading_service(self):
+        """
+        Fetch a list of problems that the student has answered from ORA.
+        Handle various error conditions.
+        @return: A boolean success indicator.
+        """
+        # In the case of multiple calls, ensure that success is false initially.
+        self.success = False
+        try:
+            #Get list of all open ended problems that the grading server knows about
+            problem_list_json = self.controller_qs.get_grading_status_list(self.course_id, self.user_id)
+        except GradingServiceError:
+            log.error("Problem contacting open ended grading service " + self.course_error_ending)
+            return self.success
+        try:
+            problem_list_dict = json.loads(problem_list_json)
+        except ValueError:
+            log.error("Problem with results from external grading service for open ended" + self.course_error_ending)
+            return self.success
+
+        success = problem_list_dict['success']
+        if 'error' in problem_list_dict:
+            self.error_text = problem_list_dict['error']
+            return success
+        if 'problem_list' not in problem_list_dict:
+            log.error("Did not receive a problem list in ORA response" + self.course_error_ending)
+            return success
+
+        self.problem_list = problem_list_dict['problem_list']
+
+        self.success = True
+        return self.success
+
+    def add_problem_data(self, base_course_url):
+        """
+        Add metadata to problems.
+        @param base_course_url: the base url for any course.  Can get with reverse('course')
+        @return: A list of valid problems in the course and their appended data.
+        """
+        # Our list of valid problems.
+        valid_problems = []
+
+        if not self.success or not isinstance(self.problem_list, list):
+            log.error("Called add_problem_data without a valid problem list" + self.course_error_ending)
+            return valid_problems
+
+        # Iterate through all of our problems and add data.
+        for problem in self.problem_list:
+            try:
+                # Try to load the problem.
+                problem_url_parts = search.path_to_location(modulestore(), self.course_id, problem['location'])
+            except (ItemNotFoundError, NoPathToItem):
+                # If the problem cannot be found at the location received from the grading controller server,
+                # it has been deleted by the course author. We should not display it.
+                error_message = "Could not find module for course {0} at location {1}".format(self.course_id,
+                                                                                              problem['location'])
+                log.error(error_message)
+                continue
+
+            # Get the problem url in the courseware.
+            problem_url = generate_problem_url(problem_url_parts, base_course_url)
+
+            # Map the grader name from ORA to a human readable version.
+            grader_type_display_name = GRADER_DISPLAY_NAMES.get(problem['grader_type'], "edX Assessment")
+            problem['actual_url'] = problem_url
+            problem['grader_type_display_name'] = grader_type_display_name
+            valid_problems.append(problem)
+        return valid_problems

lms/envs/common.py

@@ -32,6 +32,7 @@ from .discussionsettings import *
 
 from lms.xblock.mixin import LmsBlockMixin
 from xmodule.modulestore.inheritance import InheritanceMixin
+from xmodule.x_module import XModuleMixin
 
 ################################### FEATURES ###################################
 # The display name of the platform to be used in templates/emails/etc.
@@ -104,6 +105,9 @@ MITX_FEATURES = {
     # with Shib.  Feature was requested by Stanford's office of general counsel
     'SHIB_DISABLE_TOS': False,
 
+    # Can be turned off if course lists need to be hidden. Effects views and templates.
+    'COURSES_ARE_BROWSABLE': True,
+
     # Enables ability to restrict enrollment in specific courses by the user account login method
     'RESTRICT_ENROLL_BY_REG_METHOD': False,
 
@@ -363,7 +367,7 @@ CONTENTSTORE = None
 
 # This should be moved into an XBlock Runtime/Application object
 # once the responsibility of XBlock creation is moved out of modulestore - cpennington
-XBLOCK_MIXINS = (LmsBlockMixin, InheritanceMixin)
+XBLOCK_MIXINS = (LmsBlockMixin, InheritanceMixin, XModuleMixin)
 
 #################### Python sandbox ############################################
 

lms/envs/test.py

@@ -17,6 +17,8 @@ import os
 from path import path
 from warnings import filterwarnings
 
+os.environ['DJANGO_LIVE_TEST_SERVER_ADDRESS'] = 'localhost:8000-9000'
+
 # can't test start dates with this True, but on the other hand,
 # can test everything else :)
 MITX_FEATURES['DISABLE_START_DATES'] = True
@@ -43,6 +45,17 @@ SOUTH_TESTS_MIGRATE = False   # To disable migrations and use syncdb instead
 # Nose Test Runner
 TEST_RUNNER = 'django_nose.NoseTestSuiteRunner'
 
+_system = 'lms'
+
+_report_dir = REPO_ROOT / 'reports' / _system
+_report_dir.makedirs_p()
+
+NOSE_ARGS = [
+    '--tests', PROJECT_ROOT / 'djangoapps', COMMON_ROOT / 'djangoapps',
+    '--id-file', REPO_ROOT / '.testids' / _system / 'noseids',
+    '--xunit-file', _report_dir / 'nosetests.xml',
+]
+
 # Local Directories
 TEST_ROOT = path("test_root")
 # Want static files in the same dir for running on jenkins.

lms/static/sass/course/_staff_grading.scss

@@ -18,7 +18,7 @@ div.peer-grading{
         overflow: auto;
     }
 
-    div.feedback-area.track-changes, p.legend {
+    div.feedback-area.track-changes, p.ice-legend {
         .ice-controls {
             float: right;
         }

lms/static/sass/course/_textbook.scss

@@ -163,10 +163,13 @@ div.book-wrapper {
 
             &:hover {
               opacity: 1.0;
-              filter: alpha(opacity=100);
             }
-          }
 
+            &.is-disabled {
+              display:none;
+            }
+          }
+          
           &.last {
             left: 0;
 

lms/templates/courseware/course_about.html

@@ -3,8 +3,12 @@
   from django.core.urlresolvers import reverse
   from courseware.courses import course_image_url, get_course_about_section
   from courseware.access import has_access
+  from django.conf import settings
 
-  cart_link = reverse('shoppingcart.views.show_cart')
+  if settings.MITX_FEATURES.get('ENABLE_SHOPPING_CART'):
+      cart_link = reverse('shoppingcart.views.show_cart')
+  else:
+      cart_link = ""
 %>
 <%namespace name='static' file='../static_content.html'/>
 
@@ -26,29 +30,31 @@
       $("#class_enroll_form").submit();
       event.preventDefault();
     });
-    add_course_complete_handler = function(jqXHR, textStatus) {
-      if (jqXHR.status == 200) {
-        location.href = "${cart_link}";
-      }
-      if (jqXHR.status == 400) {
-        $("#register_error")
-          .html(jqXHR.responseText ? jqXHR.responseText : "${_('An error occurred. Please try again later.')}")
-          .css("display", "block");
-      }
-      else if (jqXHR.status == 403) {
-          location.href = "${reg_then_add_to_cart_link}";
-      }
-    };
-    $("#add_to_cart_post").click(function(event){
-      $.ajax({
-        url: "${reverse('add_course_to_cart', args=[course.id])}",
-        type: "POST",
-        /* Rant: HAD TO USE COMPLETE B/C PROMISE.DONE FOR SOME REASON DOES NOT WORK ON THIS PAGE. */
-        complete: add_course_complete_handler
-      })
-      event.preventDefault();
-    });
 
+    % if settings.MITX_FEATURES.get('ENABLE_SHOPPING_CART') and settings.MITX_FEATURES.get('ENABLE_PAID_COURSE_REGISTRATION'):
+      add_course_complete_handler = function(jqXHR, textStatus) {
+        if (jqXHR.status == 200) {
+          location.href = "${cart_link}";
+        }
+        if (jqXHR.status == 400) {
+          $("#register_error")
+            .html(jqXHR.responseText ? jqXHR.responseText : "${_('An error occurred. Please try again later.')}")
+            .css("display", "block");
+        }
+        else if (jqXHR.status == 403) {
+            location.href = "${reg_then_add_to_cart_link}";
+        }
+      };
+      $("#add_to_cart_post").click(function(event){
+        $.ajax({
+          url: "${reverse('add_course_to_cart', args=[course.id])}",
+          type: "POST",
+          /* Rant: HAD TO USE COMPLETE B/C PROMISE.DONE FOR SOME REASON DOES NOT WORK ON THIS PAGE. */
+          complete: add_course_complete_handler
+        })
+        event.preventDefault();
+      });
+    % endif
 
     ## making the conditional around this entire JS block for sanity
     %if settings.MITX_FEATURES.get('RESTRICT_ENROLL_BY_REG_METHOD') and course.enrollment_domain:

lms/templates/dashboard.html

@@ -338,10 +338,14 @@
       </ul>
     % else:
       <section class="empty-dashboard-message">
-        <p>${_("Looks like you haven't registered for any courses yet.")}</p>
-        <a href="${marketing_link('COURSES')}">
+        % if settings.MITX_FEATURES.get('COURSES_ARE_BROWSABLE'):
+          <p>${_("Looks like you haven't registered for any courses yet.")}</p>
+          <a href="${marketing_link('COURSES')}">
             ${_("Find courses now!")}
-        </a>
+          </a>
+        % else:
+          <p>${_("Looks like you haven't been enrolled in any courses yet.")}</p>
+        %endif
       </section>
     % endif
 

lms/templates/index.html

@@ -165,15 +165,17 @@
         </section>
       % endif
 
-      <section class="courses">
-          <ul class="courses-listing">
-          %for course in courses:
-            <li class="courses-listing-item">
-              <%include file="course.html" args="course=course" />
-            </li>
-          %endfor
-          </ul>
-      </section>
+      % if settings.MITX_FEATURES.get('COURSES_ARE_BROWSABLE'):
+        <section class="courses">
+            <ul class="courses-listing">
+            %for course in courses:
+              <li class="courses-listing-item">
+                <%include file="course.html" args="course=course" />
+              </li>
+            %endfor
+            </ul>
+        </section>
+      % endif
     </section>
   </section>
 </section>

lms/templates/navigation.html

@@ -59,9 +59,11 @@ site_status_msg = get_site_status_msg(course_id)
 
     <ol class="left nav-global authenticated">
       <%block name="navigation_global_links_authenticated">
-        <li class="nav-global-01">
-          <a href="${marketing_link('COURSES')}">${_('Find Courses')}</a>
-        </li>
+        % if settings.MITX_FEATURES.get('COURSES_ARE_BROWSABLE'):
+          <li class="nav-global-01">
+            <a href="${marketing_link('COURSES')}">${_('Find Courses')}</a>
+          </li>
+        % endif  
       </%block>
     </ol>
     <ol class="user">
@@ -82,7 +84,7 @@ site_status_msg = get_site_status_msg(course_id)
       </li>
     </ol>
       % if settings.MITX_FEATURES.get('ENABLE_PAID_COURSE_REGISTRATION') and \
-           settings.MITX_FEATURES['ENABLE_SHOPPING_CART'] and \
+           settings.MITX_FEATURES.get('ENABLE_SHOPPING_CART') and \
            shoppingcart.models.Order.user_cart_has_items(user):
         <ol class="user">
           <li class="primary">

lms/templates/open_ended_problems/open_ended_problems.html

@@ -19,36 +19,32 @@
         <h2>${_("Instructions")}</h2>
         <p>${_("Here is a list of open ended problems for this course.")}</p>
         % if success:
-            % if len(problem_list) == 0:
-                <div class="message-container">
-                  ${_("You have not attempted any open ended problems yet.")}
-                </div>
-            %else:
-                <table class="problem-list">
-                    <tr>
-                        <th>${_("Problem Name")}</th>
-                        <th>${_("Status")}</th>
-                        <th>${_("Grader Type")}</th>
-                        <th>${_("ETA")}</th>
-                    </tr>
-                    %for problem in problem_list:
-                    <tr>
-                        <td>
-                            <a href="${problem['actual_url']}">${problem['problem_name']}</a>
-                        </td>
-                        <td>
-                            ${problem['state']}
-                        </td>
-                        <td>
-                            ${problem['grader_type']}
-                        </td>
-                        <td>
-                            ${problem['eta_string']}
-                        </td>
-                    </tr>
-                    %endfor
-                </table>
-            %endif
+        % if len(problem_list) == 0:
+        <div class="message-container">
+          ${_("You have not attempted any open ended problems yet.")}
+        </div>
+        %else:
+        <table class="problem-list">
+            <tr>
+                <th>${_("Problem Name")}</th>
+                <th>${_("Status")}</th>
+                <th>${_("Grader Type")}</th>
+            </tr>
+            %for problem in problem_list:
+            <tr>
+                <td>
+                    <a href="${problem['actual_url']}">${problem['problem_name']}</a>
+                </td>
+                <td>
+                    ${problem['state']}
+                </td>
+                <td>
+                    ${problem['grader_type_display_name']}
+                </td>
+            </tr>
+            %endfor
+        </table>
+        %endif
         %endif
     </div>
 </section>

lms/templates/peer_grading/peer_grading_problem.html

@@ -55,8 +55,8 @@
                                 <span class="del">${_("This is a deletion.")}</span>&nbsp;
                                 <span class="ins">${_("[This is a comment.]")}</span>&nbsp;
                                 <span class="ice-controls">
+                                    <a href="#" class="undo-change"><i class="icon-undo"></i> Undo Change</a>&nbsp;&nbsp;
                                     <a href="#" class="reset-changes"><i class="icon-refresh"></i> Reset Changes</a>
-                                    <a href="#" class="undo-change"><i class="icon-undo"></i> Undo Change</a>
                                 </span>
                             </p>
                             <div name="feedback" class="feedback-area track-changes" contenteditable="true"></div>
@@ -65,8 +65,9 @@
                             <textarea name="feedback" placeholder="Feedback for student" class="feedback-area" cols="70" ></textarea>
                         % endif
                         <div class="flag-student-container">
+                            <br />
                             <input type="checkbox" class="flag-checkbox" value="student_is_flagged"> 
-                            ${_("This submission has explicit, offensive, or (I suspect) plagiarized content: ")}
+                            ${_("This submission has explicit, offensive, or (I suspect) plagiarized content. ")}
                         </div>
                     </div>
                     <hr />

lms/templates/video.html

@@ -42,31 +42,31 @@
             <div class="video-player-post"></div>
 
             <section class="video-controls">
-                <div class="slider" tabindex="0" title="Video position"></div>
+                <div class="slider" title="Video position"></div>
 
                 <div>
                     <ul class="vcr">
-                        <li><a class="video_control" href="#" title="${_('Play')}"></a></li>
+                        <li><a class="video_control" href="#" title="${_('Play')}" role="button" aria-disabled="false"></a></li>
                         <li><div class="vidtime">0:00 / 0:00</div></li>
                     </ul>
                     <div class="secondary-controls">
                         <div class="speeds">
-                            <a href="#" title="Speeds">
+                            <a href="#" title="${_('Speeds')}" role="button" aria-disabled="false">
                                 <h3>${_('Speed')}</h3>
                                 <p class="active"></p>
                             </a>
                             <ol class="video_speeds"></ol>
                         </div>
                         <div class="volume">
-                            <a href="#" title="Volume"></a>
+                            <a href="#" title="${_('Volume')}" role="button" aria-disabled="false"></a>
                             <div class="volume-slider-container">
                                 <div class="volume-slider"></div>
                             </div>
                         </div>
-                        <a href="#" class="add-fullscreen" title="${_('Fill browser')}">${_('Fill browser')}</a>
-                        <a href="#" class="quality_control" title="${_('HD')}">${_('HD')}</a>
+                        <a href="#" class="add-fullscreen" title="${_('Fill browser')}" role="button" aria-disabled="false">${_('Fill browser')}</a>
+                        <a href="#" class="quality_control" title="${_('HD')}" role="button" aria-disabled="false">${_('HD')}</a>
 
-                        <a href="#" class="hide-subtitles" title="${_('Turn off captions')}">${_('Captions')}</a>
+                        <a href="#" class="hide-subtitles" title="${_('Turn off captions')}" role="button" aria-disabled="false">${_('Turn off captions')}</a>
                     </div>
                 </div>
             </section>

pylintrc

@@ -89,6 +89,9 @@ evaluation=10.0 - ((float(5 * error + warning + refactor + convention) / stateme
 # evaluation report (RP0004).
 comment=no
 
+# Display symbolic names of messages in reports
+symbols=yes
+
 
 [TYPECHECK]
 
@@ -120,7 +123,10 @@ generated-members=
     content,
     status_code,
 # For factory_boy factories
-    create
+    create,
+    build,
+# For xblocks
+    fields,
 
 
 [BASIC]

rakelib/tests.rake

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

requirements/edx/base.txt

@@ -29,7 +29,7 @@ django-storages==1.1.5
 django-threaded-multihost==1.4-1
 django-method-override==0.1.0
 djangorestframework==2.3.5
-django==1.4.5
+django==1.4.8
 feedparser==5.1.3
 fs==0.4.0
 GitPython==0.3.2.RC1
@@ -47,7 +47,7 @@ Pillow==1.7.8
 pip>=1.4
 polib==1.0.3
 pycrypto>=2.6
-pygments==1.5
+pygments==1.6
 pygraphviz==1.1
 pymongo==2.4.1
 pyparsing==1.5.6

requirements/edx/github.txt

@@ -14,8 +14,8 @@
 -e git+https://github.com/eventbrite/zendesk.git@d53fe0e81b623f084e91776bcf6369f8b7b63879#egg=zendesk
 
 # Our libraries:
--e git+https://github.com/edx/XBlock.git@aa0d60627#egg=XBlock
+-e git+https://github.com/edx/XBlock.git@8a66ca3#egg=XBlock
 -e git+https://github.com/edx/codejail.git@0a1b468#egg=codejail
--e git+https://github.com/edx/diff-cover.git@v0.2.3#egg=diff_cover
+-e git+https://github.com/edx/diff-cover.git@v0.2.4#egg=diff_cover
 -e git+https://github.com/edx/js-test-tool.git@v0.0.7#egg=js_test_tool
 -e git+https://github.com/edx/django-waffle.git@823a102e48#egg=django-waffle

requirements/system/mac_os_x/brew-formulas.txt

@@ -11,3 +11,5 @@ mysql
 geos
 mongodb
 lynx
+libjpeg
+libtiff

requirements/system/ubuntu/apt-packages.txt

@@ -16,6 +16,8 @@ gfortran
 libfreetype6-dev
 libpng12-dev
 libjpeg-dev
+libtiff4-dev
+zlib1g-dev
 libxml2-dev
 libxslt-dev
 yui-compressor