diff --git a/.gitignore b/.gitignore index 72de96e0c4..e92d49a0f2 100644 --- a/.gitignore +++ b/.gitignore @@ -1,49 +1,73 @@ -*.pyc -*~ -*.scssc -*.swp -*.orig -*.DS_Store -*.mo -:2e_* -:2e# -.AppleDouble -database.sqlite +# .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 -courseware/static/js/mathjax/* -flushdb.sh -build + +### Python artifacts +*.pyc + +### Editor and IDE artifacts +*~ +*.swp +*.orig +/nbproject +.idea/ +.redcar/ + +### OS X artifacts +*.DS_Store +.AppleDouble +:2e_* +:2e# + +### 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/ diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 443e787482..bd28c47a74 100644 --- a/CHANGELOG.rst +++ b/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. diff --git a/cms/djangoapps/contentstore/features/upload.feature b/cms/djangoapps/contentstore/features/upload.feature index e01bcf8fed..2e73c11c0c 100644 --- a/cms/djangoapps/contentstore/features/upload.feature +++ b/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 - # 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'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 + @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 diff --git a/cms/djangoapps/contentstore/features/upload.py b/cms/djangoapps/contentstore/features/upload.py index b94ccd114a..25e33f7e5e 100644 --- a/cms/djangoapps/contentstore/features/upload.py +++ b/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 + expected_text = 'Unauthorized' - -@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' - - 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$') diff --git a/cms/djangoapps/contentstore/management/commands/edit_course_tabs.py b/cms/djangoapps/contentstore/management/commands/edit_course_tabs.py new file mode 100644 index 0000000000..d9c73e42fa --- /dev/null +++ b/cms/djangoapps/contentstore/management/commands/edit_course_tabs.py @@ -0,0 +1,88 @@ +### +### 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 required, e.g. Stanford/CS99/2013_spring') + delete_option = make_option('--delete', + action='store_true', + dest='delete', + default=False, + help='--delete ') + insert_option = make_option('--insert', + action='store_true', + dest='insert', + default=False, + help='--insert , 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) diff --git a/cms/djangoapps/contentstore/tests/test_tabs.py b/cms/djangoapps/contentstore/tests/test_tabs.py new file mode 100644 index 0000000000..f1cf8ddfa5 --- /dev/null +++ b/cms/djangoapps/contentstore/tests/test_tabs.py @@ -0,0 +1,41 @@ +""" 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'}) diff --git a/cms/djangoapps/contentstore/views/tabs.py b/cms/djangoapps/contentstore/views/tabs.py index f38685edfc..f897fa1378 100644 --- a/cms/djangoapps/contentstore/views/tabs.py +++ b/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)) diff --git a/cms/envs/common.py b/cms/envs/common.py index 7f2559eece..13faf5520e 100644 --- a/cms/envs/common.py +++ b/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 ################################ diff --git a/cms/envs/test.py b/cms/envs/test.py index 364bee2441..f3eee45ea7 100644 --- a/cms/envs/test.py +++ b/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. diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py index 4d0b7eeb2b..5872955780 100644 --- a/common/djangoapps/external_auth/views.py +++ b/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): diff --git a/common/djangoapps/terrain/course_helpers.py b/common/djangoapps/terrain/course_helpers.py index fc01d25d66..22222d30a4 100644 --- a/common/djangoapps/terrain/course_helpers.py +++ b/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 diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py index 6a24bf8f27..c99234e385 100644 --- a/common/lib/xmodule/setup.py +++ b/common/lib/xmodule/setup.py @@ -1,5 +1,47 @@ 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', - ] - } + ], + }, ) diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index 5b1a57f20b..0bc79a4a1c 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/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, diff --git a/common/lib/xmodule/xmodule/css/video/display.scss b/common/lib/xmodule/xmodule/css/video/display.scss index 1a1ca14fbd..c087d18098 100644 --- a/common/lib/xmodule/xmodule/css/video/display.scss +++ b/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 { diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py index 5c13b90ccf..7fc3747f44 100644 --- a/common/lib/xmodule/xmodule/error_module.py +++ b/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): diff --git a/common/lib/xmodule/xmodule/js/fixtures/video.html b/common/lib/xmodule/xmodule/js/fixtures/video.html index 410b5869f0..f607430ba0 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video.html @@ -26,26 +26,26 @@
    -
    • +
  • 0:00 / 0:00
- +

Speed
    - +
    - Fill Browser - HD - Captions + Fill Browser + HD + Captions
    diff --git a/common/lib/xmodule/xmodule/js/fixtures/video_all.html b/common/lib/xmodule/xmodule/js/fixtures/video_all.html index f155905282..57052bf65d 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video_all.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video_all.html @@ -29,26 +29,26 @@
      -
      • +
    • 0:00 / 0:00
    - + >

    Speed
      - +
      - Fill Browser - HD - Captions + Fill Browser + HD + Captions
      diff --git a/common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html b/common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html index 834de10406..c6b40cdf16 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html @@ -26,26 +26,26 @@
        -
        • +
      • 0:00 / 0:00
      - +

      Speed
        - +
        - Fill Browser - HD - Captions + Fill Browser + HD + Captions
        diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js index a94a35e124..0f729da62d 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js +++ b/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); diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js index 7beb2957da..a7df088d67 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js +++ b/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() { diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_quality_control_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_quality_control_spec.js index 627438c736..eb2f19aa60 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_quality_control_spec.js +++ b/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(""); }); diff --git a/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee b/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee index d1006ae9a9..030d93e9b5 100644 --- a/common/lib/xmodule/xmodule/js/src/combinedopenended/display.coffee +++ b/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 diff --git a/common/lib/xmodule/xmodule/js/src/peergrading/track_changes.coffee b/common/lib/xmodule/xmodule/js/src/peergrading/track_changes.coffee index ff0b4111f2..72df455b0b 100644 --- a/common/lib/xmodule/xmodule/js/src/peergrading/track_changes.coffee +++ b/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 diff --git a/common/lib/xmodule/xmodule/js/src/video/04_video_control.js b/common/lib/xmodule/xmodule/js/src/video/04_video_control.js index 5cdb5c7536..796ba07060 100644 --- a/common/lib/xmodule/xmodule/js/src/video/04_video_control.js +++ b/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); diff --git a/common/lib/xmodule/xmodule/js/src/video/06_video_progress_slider.js b/common/lib/xmodule/xmodule/js/src/video/06_video_progress_slider.js index b45494ca34..18fa7ee3ad 100644 --- a/common/lib/xmodule/xmodule/js/src/video/06_video_progress_slider.js +++ b/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)); diff --git a/common/lib/xmodule/xmodule/js/src/video/07_video_volume_control.js b/common/lib/xmodule/xmodule/js/src/video/07_video_volume_control.js index d8398ab530..ea813f3912 100644 --- a/common/lib/xmodule/xmodule/js/src/video/07_video_volume_control.js +++ b/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,11 +196,41 @@ 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'; + } + }); }(RequireJS.requirejs, RequireJS.require, RequireJS.define)); diff --git a/common/lib/xmodule/xmodule/js/src/video/09_video_caption.js b/common/lib/xmodule/xmodule/js/src/video/09_video_caption.js index 3dc675b4c2..b97717e97c 100644 --- a/common/lib/xmodule/xmodule/js/src/video/09_video_caption.js +++ b/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(); } diff --git a/common/lib/xmodule/xmodule/modulestore/locator.py b/common/lib/xmodule/xmodule/modulestore/locator.py index dfeafa5e1d..47753a4941 100644 --- a/common/lib/xmodule/xmodule/modulestore/locator.py +++ b/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 = [] diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 44865ab58e..2843287055 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/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() diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py index ba81a5231d..a0549074c3 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py +++ b/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) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/definition_lazy_loader.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/definition_lazy_loader.py index 5ccaaa7ed3..9b2a652a95 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/definition_lazy_loader.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/definition_lazy_loader.py @@ -1,4 +1,4 @@ -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): """ diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index d85092b363..cdcb55d899 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/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): """ diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py b/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py index 05f4d25283..6ec9acfcd5 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py +++ b/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()) # ------------------------------------------------------------------ diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py index 0f1b65d0a3..2373c409de 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/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() diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 93ab0497f9..da86279b68 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/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 diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_image_submission.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_image_submission.py deleted file mode 100644 index ea5c3b3527..0000000000 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_image_submission.py +++ /dev/null @@ -1,270 +0,0 @@ -""" -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 diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py index d7fe8c0d26..2d973aa9ed 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/open_ended_module.py +++ b/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, diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py index d7555ce77e..67a058f478 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py @@ -2,8 +2,6 @@ import json import logging from lxml.html.clean import Cleaner, autolink_html import re - -import open_ended_image_submission from xmodule.progress import Progress import capa.xqueue_interface as xqueue_interface from capa.util import * @@ -12,6 +10,9 @@ import controller_query_service from datetime import datetime from pytz import UTC +import requests +from boto.s3.connection import S3Connection +from boto.s3.key import Key log = logging.getLogger("mitx.courseware") @@ -24,6 +25,50 @@ MAX_ATTEMPTS = 1 # Overriden by max_score specified in xml. MAX_SCORE = 1 +FILE_NOT_FOUND_IN_RESPONSE_MESSAGE = "We could not find a file in your submission. Please try choosing a file or pasting a link to your file into the answer box." +ERROR_SAVING_FILE_MESSAGE = "We are having trouble saving your file. Please try another file or paste a link to your file into the answer box." + +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 + ''' + + 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) + + k.set_acl("public-read") + public_url = k.generate_url(60 * 60 * 24 * 365) # URL timeout in seconds. + + return public_url + +class WhiteListCleaner(Cleaner): + """ + By default, lxml cleaner strips out all links that are not in a defined whitelist. + We want to allow all links, and rely on the peer grading flagging mechanic to catch + the "bad" ones. So, don't define a whitelist at all. + """ + def allow_embedded_url(self, el, url): + """ + Override the Cleaner allow_embedded_url method to remove the whitelist url requirement. + Ensure that any tags not in the whitelist are stripped beforehand. + """ + + # Tell cleaner to strip any element with a tag that isn't whitelisted. + if self.whitelist_tags is not None and el.tag not in self.whitelist_tags: + return False + + # Tell cleaner to allow all urls. + return True + class OpenEndedChild(object): """ @@ -70,6 +115,7 @@ class OpenEndedChild(object): except: log.error( "Could not load instance state for open ended. Setting it to nothing.: {0}".format(instance_state)) + instance_state = {} else: instance_state = {} @@ -176,11 +222,22 @@ class OpenEndedChild(object): @staticmethod def sanitize_html(answer): + """ + Take a student response and sanitize the HTML to prevent malicious script injection + or other unwanted content. + answer - any string + return - a cleaned version of the string + """ try: answer = autolink_html(answer) - cleaner = Cleaner(style=True, links=True, add_nofollow=False, page_structure=True, safe_attrs_only=True, - host_whitelist=open_ended_image_submission.TRUSTED_IMAGE_DOMAINS, - whitelist_tags=set(['embed', 'iframe', 'a', 'img', 'br'])) + cleaner = WhiteListCleaner( + style=True, + links=True, + add_nofollow=False, + page_structure=True, + safe_attrs_only=True, + whitelist_tags=('embed', 'iframe', 'a', 'img', 'br',) + ) clean_html = cleaner.clean_html(answer) clean_html = re.sub(r'

        $', '', re.sub(r'^

        ', '', clean_html)) clean_html = re.sub("\n","
        ", clean_html) @@ -351,119 +408,116 @@ class OpenEndedChild(object): correctness = 'correct' if self.is_submission_correct(score) else 'incorrect' return correctness - def upload_image_to_s3(self, image_data): + def upload_file_to_s3(self, file_data): """ - Uploads an image to S3 - Image_data: InMemoryUploadedFileObject that responds to read() and seek() - @return:Success and a URL corresponding to the uploaded object + Uploads a file to S3. + file_data: InMemoryUploadedFileObject that responds to read() and seek(). + @return: A URL corresponding to the uploaded object. """ - success = False - s3_public_url = "" - image_ok = False - try: - image_data.seek(0) - image_ok = open_ended_image_submission.run_image_tests(image_data) - except Exception: - log.exception("Could not create image and check it.") - if image_ok: - image_key = image_data.name + datetime.now(UTC).strftime( - xqueue_interface.dateformat - ) + file_key = file_data.name + datetime.now(UTC).strftime( + xqueue_interface.dateformat + ) - try: - image_data.seek(0) - success, s3_public_url = open_ended_image_submission.upload_to_s3( - image_data, image_key, self.s3_interface - ) - except Exception: - log.exception("Could not upload image to S3.") + file_data.seek(0) + s3_public_url = upload_to_s3( + file_data, file_key, self.s3_interface + ) - return success, image_ok, s3_public_url + return s3_public_url - def check_for_image_and_upload(self, data): + def check_for_file_and_upload(self, data): """ - Checks to see if an image was passed back in the AJAX query. If so, it will upload it to S3 - @param data: AJAX data - @return: Success, whether or not a file was in the data dictionary, - and the html corresponding to the uploaded image + Checks to see if a file was passed back by the student. If so, it will be uploaded to S3. + @param data: AJAX post dictionary containing keys student_file and valid_files_attached. + @return: has_file_to_upload, whether or not a file was in the data dictionary, + and image_tag, the html needed to create a link to the uploaded file. """ has_file_to_upload = False - uploaded_to_s3 = False image_tag = "" - image_ok = False - if 'can_upload_files' in data: - if data['can_upload_files'] in ['true', '1']: + + # Ensure that a valid file was uploaded. + if ('valid_files_attached' in data + and data['valid_files_attached'] in ['true', '1', True] + and data['student_file'] is not None + and len(data['student_file']) > 0): has_file_to_upload = True student_file = data['student_file'][0] - uploaded_to_s3, image_ok, s3_public_url = self.upload_image_to_s3(student_file) - if uploaded_to_s3: - image_tag = self.generate_image_tag_from_url(s3_public_url, student_file.name) - return has_file_to_upload, uploaded_to_s3, image_ok, image_tag + # Upload the file to S3 and generate html to embed a link. + s3_public_url = self.upload_file_to_s3(student_file) + image_tag = self.generate_file_link_html_from_url(s3_public_url, student_file.name) - def generate_image_tag_from_url(self, s3_public_url, image_name): + return has_file_to_upload, image_tag + + def generate_file_link_html_from_url(self, s3_public_url, file_name): """ - Makes an image tag from a given URL - @param s3_public_url: URL of the image - @param image_name: Name of the image - @return: Boolean success, updated AJAX data + Create an html link to a given URL. + @param s3_public_url: URL of the file. + @param file_name: Name of the file. + @return: Boolean success, updated AJAX data. """ - image_template = """ + image_link = """         {1} - """.format(s3_public_url, image_name) - return image_template + """.format(s3_public_url, file_name) + return image_link - def append_image_to_student_answer(self, data): + def append_file_link_to_student_answer(self, data): """ - Adds an image to a student answer after uploading it to S3 - @param data: AJAx data - @return: Boolean success, updated AJAX data + Adds a file to a student answer after uploading it to S3. + @param data: AJAX data containing keys student_answer, valid_files_attached, and student_file. + @return: Boolean success, and updated AJAX data dictionary. """ - overall_success = False + + error_message = "" + if not self.accept_file_upload: # If the question does not accept file uploads, do not do anything - return True, data + return True, error_message, data - has_file_to_upload, uploaded_to_s3, image_ok, image_tag = self.check_for_image_and_upload(data) - if uploaded_to_s3 and has_file_to_upload and image_ok: + try: + # Try to upload the file to S3. + has_file_to_upload, image_tag = self.check_for_file_and_upload(data) data['student_answer'] += image_tag - overall_success = True - elif has_file_to_upload and not uploaded_to_s3 and image_ok: + success = True + if not has_file_to_upload: + # If there is no file to upload, probably the student has embedded the link in the answer text + success, data['student_answer'] = self.check_for_url_in_text(data['student_answer']) + + # If success is False, we have not found a link, and no file was attached. + # Show error to student. + if success is False: + error_message = FILE_NOT_FOUND_IN_RESPONSE_MESSAGE + + except Exception: # In this case, an image was submitted by the student, but the image could not be uploaded to S3. Likely - # a config issue (development vs deployment). For now, just treat this as a "success" - log.exception("Student AJAX post to combined open ended xmodule indicated that it contained an image, " - "but the image was not able to be uploaded to S3. This could indicate a config" - "issue with this deployment, but it could also indicate a problem with S3 or with the" - "student image itself.") - overall_success = True - elif not has_file_to_upload: - # If there is no file to upload, probably the student has embedded the link in the answer text - success, data['student_answer'] = self.check_for_url_in_text(data['student_answer']) - overall_success = success + # a config issue (development vs deployment). + log.exception("Student AJAX post to combined open ended xmodule indicated that it contained a file, " + "but the image was not able to be uploaded to S3. This could indicate a configuration " + "issue with this deployment and the S3_INTERFACE setting.") + success = False + error_message = ERROR_SAVING_FILE_MESSAGE - # log.debug("Has file: {0} Uploaded: {1} Image Ok: {2}".format(has_file_to_upload, uploaded_to_s3, image_ok)) - - return overall_success, data + return success, error_message, data def check_for_url_in_text(self, string): """ - Checks for urls in a string - @param string: Arbitrary string - @return: Boolean success, the edited string + Checks for urls in a string. + @param string: Arbitrary string. + @return: Boolean success, and the edited string. """ - success = False - links = re.findall(r'(https?://\S+)', string) - if len(links) > 0: - for link in links: - success = open_ended_image_submission.run_url_tests(link) - if not success: - string = re.sub(link, '', string) - else: - string = re.sub(link, self.generate_image_tag_from_url(link, link), string) - success = True + has_link = False - return success, string + # Find all links in the string. + links = re.findall(r'(https?://\S+)', string) + if len(links)>0: + has_link = True + + # Autolink by wrapping links in anchor tags. + for link in links: + string = re.sub(link, self.generate_file_link_html_from_url(link, link), string) + + return has_link, string def get_eta(self): if self.controller_qs: diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py index cc830f88c8..6c0d1bbf08 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/self_assessment_module.py +++ b/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, diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index 088b2c32b6..99caec0840 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/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), ) diff --git a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py b/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py index 5d11a4924f..65fc2bb608 100644 --- a/common/lib/xmodule/xmodule/tests/test_combined_open_ended.py +++ b/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 = " nodes""" + tag = 'course' + + +class SequenceFactory(XmlImportFactory): + """Factory for nodes""" + tag = 'sequential' + + +class ProblemFactory(XmlImportFactory): + """Factory for nodes""" + tag = 'problem' + text = '

        Empty Problem!

        ' diff --git a/common/lib/xmodule/xmodule/tests/xml/test_inheritance.py b/common/lib/xmodule/xmodule/tests/xml/test_inheritance.py new file mode 100644 index 0000000000..dc27f0c792 --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/xml/test_inheritance.py @@ -0,0 +1,29 @@ +""" +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) diff --git a/common/lib/xmodule/xmodule/tests/xml/test_policy.py b/common/lib/xmodule/xmodule/tests/xml/test_policy.py new file mode 100644 index 0000000000..2e702cc82d --- /dev/null +++ b/common/lib/xmodule/xmodule/tests/xml/test_policy.py @@ -0,0 +1,30 @@ +""" +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) diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py index d97d5aa144..96d0a09498 100644 --- a/common/lib/xmodule/xmodule/video_module.py +++ b/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: diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 63a73c4530..ef37843371 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -1,5 +1,4 @@ import logging -import copy import yaml import os @@ -11,7 +10,7 @@ from xmodule.modulestore import Location from xmodule.modulestore.exceptions import ItemNotFoundError, InsufficientSpecificationError, InvalidLocationError from xblock.core import XBlock -from xblock.fields import Scope, String, Integer, Float, List +from xblock.fields import Scope, Integer, Float, List, XBlockMixin, String from xblock.fragment import Fragment from xblock.runtime import Runtime from xmodule.modulestore.locator import BlockUsageLocator @@ -83,7 +82,13 @@ class HTMLSnippet(object): .format(self.__class__)) -class XModuleFields(object): +class XModuleMixin(XBlockMixin): + """ + Fields and methods used by XModules internally. + + Adding this Mixin to an :class:`XBlock` allows it to cooperate with old-style :class:`XModules` + """ + display_name = String( display_name="Display Name", help="This name appears in the horizontal navigation at the top of the page.", @@ -93,41 +98,6 @@ class XModuleFields(object): default=None ) - -class XModule(XModuleFields, HTMLSnippet, XBlock): - ''' Implements a generic learning module. - - Subclasses must at a minimum provide a definition for get_html in order - to be displayed to users. - - See the HTML module for a simple example. - ''' - - # The default implementation of get_icon_class returns the icon_class - # attribute of the class - # - # This attribute can be overridden by subclasses, and - # the function can also be overridden if the icon class depends on the data - # in the module - icon_class = 'other' - - - def __init__(self, descriptor, *args, **kwargs): - ''' - Construct a new xmodule - - runtime: An XBlock runtime allowing access to external resources - - descriptor: the XModuleDescriptor that this module is an instance of. - - field_data: A dictionary-like object that maps field names to values - for those fields. - ''' - super(XModule, self).__init__(*args, **kwargs) - self.system = self.runtime - self.descriptor = descriptor - self._loaded_children = None - @property def id(self): return self.location.url() @@ -155,31 +125,133 @@ class XModule(XModuleFields, HTMLSnippet, XBlock): @property def url_name(self): - if self.descriptor: - return self.descriptor.url_name - elif isinstance(self.location, Location): + if isinstance(self.location, Location): return self.location.name elif isinstance(self.location, BlockUsageLocator): return self.location.usage_id else: raise InsufficientSpecificationError() - @property def display_name_with_default(self): - ''' + """ Return a display name for the module: use display_name if defined in metadata, otherwise convert the url name. - ''' + """ name = self.display_name if name is None: name = self.url_name.replace('_', ' ') return name + def get_explicitly_set_fields_by_scope(self, scope=Scope.content): + """ + Get a dictionary of the fields for the given scope which are set explicitly on this xblock. (Including + any set to None.) + """ + result = {} + for field in self.fields.values(): + if (field.scope == scope and field.is_set_on(self)): + result[field.name] = field.read_json(self) + return result + + @property + def xblock_kvs(self): + """ + Use w/ caution. Really intended for use by the persistence layer. + """ + # if caller wants kvs, caller's assuming it's up to date; so, decache it + self.save() + return self._field_data._kvs # pylint: disable=protected-access + def get_children(self): - ''' + """Returns a list of XBlock instances for the children of + this module""" + + if not self.has_children: + return [] + + if getattr(self, '_child_instances', None) is None: + self._child_instances = [] # pylint: disable=attribute-defined-outside-init + for child_loc in self.children: + try: + child = self.runtime.get_block(child_loc) + except ItemNotFoundError: + log.exception('Unable to load item {loc}, skipping'.format(loc=child_loc)) + continue + self._child_instances.append(child) + + return self._child_instances + + def get_required_module_descriptors(self): + """Returns a list of XModuleDescriptor instances upon which this module depends, but are + not children of this module""" + return [] + + def get_display_items(self): + """ + Returns a list of descendent module instances that will display + immediately inside this module. + """ + items = [] + for child in self.get_children(): + items.extend(child.displayable_items()) + + return items + + def displayable_items(self): + """ + Returns list of displayable modules contained by this module. If this + module is visible, should return [self]. + """ + return [self] + + def get_child_by(self, selector): + """ + Return a child XBlock that matches the specified selector + """ + for child in self.get_children(): + if selector(child): + return child + return None + + +class XModule(XModuleMixin, HTMLSnippet, XBlock): # pylint: disable=abstract-method + """ Implements a generic learning module. + + Subclasses must at a minimum provide a definition for get_html in order + to be displayed to users. + + See the HTML module for a simple example. + """ + + # The default implementation of get_icon_class returns the icon_class + # attribute of the class + # + # This attribute can be overridden by subclasses, and + # the function can also be overridden if the icon class depends on the data + # in the module + icon_class = 'other' + + def __init__(self, descriptor, *args, **kwargs): + """ + Construct a new xmodule + + runtime: An XBlock runtime allowing access to external resources + + descriptor: the XModuleDescriptor that this module is an instance of. + + field_data: A dictionary-like object that maps field names to values + for those fields. + """ + super(XModule, self).__init__(*args, **kwargs) + self.system = self.runtime + self.descriptor = descriptor + self._loaded_children = None + + def get_children(self): + """ Return module instances for all the children of this module. - ''' + """ if self._loaded_children is None: child_descriptors = self.get_child_descriptors() @@ -200,7 +272,7 @@ class XModule(XModuleFields, HTMLSnippet, XBlock): return ''.format(self.id) def get_child_descriptors(self): - ''' + """ Returns the descriptors of the child modules Overriding this changes the behavior of get_children and @@ -211,40 +283,13 @@ class XModule(XModuleFields, HTMLSnippet, XBlock): These children will be the same children returned by the descriptor unless descriptor.has_dynamic_children() is true. - ''' + """ return self.descriptor.get_children() - def get_child_by(self, selector): - """ - Return a child XModuleDescriptor with the specified url_name, if it exists, and None otherwise. - """ - for child in self.get_children(): - if selector(child): - return child - return None - - def get_display_items(self): - ''' - Returns a list of descendent module instances that will display - immediately inside this module. - ''' - items = [] - for child in self.get_children(): - items.extend(child.displayable_items()) - - return items - - def displayable_items(self): - ''' - Returns list of displayable modules contained by this module. If this - module is visible, should return [self]. - ''' - return [self] - def get_icon_class(self): - ''' + """ Return a css class identifying this module in the context of an icon - ''' + """ return self.icon_class # Functions used in the LMS @@ -267,7 +312,7 @@ class XModule(XModuleFields, HTMLSnippet, XBlock): return None def max_score(self): - ''' Maximum score. Two notes: + """ Maximum score. Two notes: * This is generic; in abstract, a problem could be 3/5 points on one randomization, and 5/7 on another @@ -275,22 +320,22 @@ class XModule(XModuleFields, HTMLSnippet, XBlock): * In practice, this is a Very Bad Idea, and (a) will break some code in place (although that code should get fixed), and (b) break some analytics we plan to put in place. - ''' + """ return None def get_progress(self): - ''' Return a progress.Progress object that represents how far the + """ Return a progress.Progress object that represents how far the student has gone in this module. Must be implemented to get correct progress tracking behavior in nesting modules like sequence and vertical. If this module has no notion of progress, return None. - ''' + """ return None def handle_ajax(self, _dispatch, _data): - ''' dispatch is last part of the URL. - data is a dictionary-like object with the content of the request''' + """ dispatch is last part of the URL. + data is a dictionary-like object with the content of the request""" return "" @@ -381,7 +426,7 @@ class ResourceTemplates(object): return None -class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): +class XModuleDescriptor(XModuleMixin, HTMLSnippet, ResourceTemplates, XBlock): """ An XModuleDescriptor is a specification for an element of a course. This could be a problem, an organizational element (a group of content), or a @@ -446,86 +491,6 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): self.edited_by = self.edited_on = self.previous_version = self.update_version = self.definition_locator = None self._child_instances = None - @property - def id(self): - return self.location.url() - - @property - def category(self): - return self.scope_ids.block_type - - @property - def location(self): - try: - return Location(self.scope_ids.usage_id) - except InvalidLocationError: - if isinstance(self.scope_ids.usage_id, BlockUsageLocator): - return self.scope_ids.usage_id - else: - return BlockUsageLocator(self.scope_ids.usage_id) - - @location.setter - def location(self, value): - self.scope_ids = self.scope_ids._replace( - def_id=value, - usage_id=value, - ) - - @property - def url_name(self): - if isinstance(self.location, Location): - return self.location.name - elif isinstance(self.location, BlockUsageLocator): - return self.location.usage_id - else: - raise InsufficientSpecificationError() - - - @property - def display_name_with_default(self): - ''' - Return a display name for the module: use display_name if defined in - metadata, otherwise convert the url name. - ''' - name = self.display_name - if name is None: - name = self.url_name.replace('_', ' ') - return name - - def get_required_module_descriptors(self): - """Returns a list of XModuleDescritpor instances upon which this module depends, but are - not children of this module""" - return [] - - def get_children(self): - """Returns a list of XModuleDescriptor instances for the children of - this module""" - if not self.has_children: - return [] - - if self._child_instances is None: - self._child_instances = [] - for child_loc in self.children: - if isinstance(child_loc, XModuleDescriptor): - child = child_loc - else: - try: - child = self.runtime.get_block(child_loc) - except ItemNotFoundError: - log.exception('Unable to load item {loc}, skipping'.format(loc=child_loc)) - continue - self._child_instances.append(child) - - return self._child_instances - - def get_child_by(self, selector): - """ - Return a child XModuleDescriptor with the specified url_name, if it exists, and None otherwise. - """ - for child in self.get_children(): - if selector(child): - return child - return None def xmodule(self, system): """ @@ -537,8 +502,8 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): module = system.construct_xblock_from_class( self.module_class, descriptor=self, - field_data=system.xmodule_field_data(self), scope_ids=self.scope_ids, + field_data=system.xmodule_field_data(self), ) module.save() return module @@ -559,38 +524,21 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): return cls.metadata_translations.get(key, key) # ================================= XML PARSING ============================ - @staticmethod - def load_from_xml(xml_data, - system, - org=None, - course=None, - default_class=None): + @classmethod + def parse_xml(cls, node, runtime, keys): """ - This method instantiates the correct subclass of XModuleDescriptor based - on the contents of xml_data. - - xml_data must be a string containing valid xml - - system is an XMLParsingSystem - - org and course are optional strings that will be used in the generated - module's url identifiers + Interpret the parsed XML in `node`, creating an XModuleDescriptor. """ - class_ = system.mixologist.mix(XModuleDescriptor.load_class( - etree.fromstring(xml_data).tag, - default_class - )) - # leave next line, commented out - useful for low-level debugging - # log.debug('[XModuleDescriptor.load_from_xml] tag=%s, class_=%s' % ( - # etree.fromstring(xml_data).tag,class_)) - - return class_.from_xml(xml_data, system, org, course) + xml = etree.tostring(node) + # TODO: change from_xml to not take org and course, it can use self.system. + block = cls.from_xml(xml, runtime, runtime.org, runtime.course) + return block @classmethod def from_xml(cls, xml_data, system, org=None, course=None): """ Creates an instance of this descriptor from the supplied xml_data. - This may be overridden by subclasses + This may be overridden by subclasses. xml_data: A string of xml that will be translated into data and children for this module @@ -600,13 +548,12 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): org and course are optional strings that will be used in the generated module's url identifiers """ - raise NotImplementedError( - 'Modules must implement from_xml to be parsable from xml') + raise NotImplementedError('Modules must implement from_xml to be parsable from xml') def export_to_xml(self, resource_fs): """ Returns an xml string representing this module, and all modules - underneath it. May also write required resources out to resource_fs + underneath it. May also write required resources out to resource_fs. Assumes that modules have single parentage (that no module appears twice in the same course), and that it is thus safe to nest modules as xml @@ -616,17 +563,7 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): XModuleDescriptor using the from_xml method with the same system, org, and course """ - raise NotImplementedError( - 'Modules must implement export_to_xml to enable xml export') - - @property - def xblock_kvs(self): - """ - Use w/ caution. Really intended for use by the persistence layer. - """ - # if caller wants kvs, caller's assuming it's up to date; so, decache it - self.save() - return self._field_data._kvs + raise NotImplementedError('Modules must implement export_to_xml to enable xml export') # =============================== BUILTIN METHODS ========================== def __eq__(self, other): @@ -655,17 +592,6 @@ class XModuleDescriptor(XModuleFields, HTMLSnippet, ResourceTemplates, XBlock): return [XBlock.tags, XBlock.name] - def get_explicitly_set_fields_by_scope(self, scope=Scope.content): - """ - Get a dictionary of the fields for the given scope which are set explicitly on this xblock. (Including - any set to None.) - """ - result = {} - for field in self.fields.values(): - if (field.scope == scope and field.is_set_on(self)): - result[field.name] = field.read_json(self) - return result - @property def editable_metadata_fields(self): """ @@ -770,7 +696,9 @@ class DescriptorSystem(Runtime): that you're about to re-raise---let the caller track them. """ - super(DescriptorSystem, self).__init__(**kwargs) + # Right now, usage_store is unused, and field_data is always supplanted + # with an explicit field_data during construct_xblock, so None's suffice. + super(DescriptorSystem, self).__init__(usage_store=None, field_data=None, **kwargs) self.load_item = load_item self.resources_fs = resources_fs @@ -811,8 +739,6 @@ class DescriptorSystem(Runtime): class XMLParsingSystem(DescriptorSystem): def __init__(self, process_xml, policy, **kwargs): """ - load_item, resources_fs, error_tracker: see DescriptorSystem - policy: a policy dictionary for overriding xml metadata process_xml: Takes an xml string, and returns a XModuleDescriptor @@ -825,7 +751,7 @@ class XMLParsingSystem(DescriptorSystem): class ModuleSystem(Runtime): - ''' + """ This is an abstraction such that x_modules can function independent of the courseware (e.g. import into other types of courseware, LMS, or if we want to have a sandbox server for user-contributed content) @@ -835,7 +761,7 @@ class ModuleSystem(Runtime): Note that these functions can be closures over e.g. a django request and user, or other environment-specific info. - ''' + """ def __init__( self, ajax_url, track_function, get_module, render_template, replace_urls, xmodule_field_data, user=None, filestore=None, @@ -844,7 +770,7 @@ class ModuleSystem(Runtime): open_ended_grading_interface=None, s3_interface=None, cache=None, can_execute_unsafe_code=None, replace_course_urls=None, replace_jump_to_id_urls=None, **kwargs): - ''' + """ Create a closure around the system environment. ajax_url - the url where ajax calls to the encapsulating module go. @@ -893,8 +819,11 @@ class ModuleSystem(Runtime): can_execute_unsafe_code - A function returning a boolean, whether or not to allow the execution of unsafe, unsandboxed code. - ''' - super(ModuleSystem, self).__init__(**kwargs) + """ + + # Right now, usage_store is unused, and field_data is always supplanted + # with an explicit field_data during construct_xblock, so None's suffice. + super(ModuleSystem, self).__init__(usage_store=None, field_data=None, **kwargs) self.ajax_url = ajax_url self.xqueue = xqueue @@ -926,11 +855,11 @@ class ModuleSystem(Runtime): self.replace_jump_to_id_urls = replace_jump_to_id_urls def get(self, attr): - ''' provide uniform access to attributes (like etree).''' + """ provide uniform access to attributes (like etree).""" return self.__dict__.get(attr) def set(self, attr, val): - '''provide uniform access to attributes (like etree)''' + """provide uniform access to attributes (like etree)""" self.__dict__[attr] = val def __repr__(self): diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 5fa9ff3260..b555d8f775 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/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): """ @@ -166,20 +149,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): """ @@ -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) + 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 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) + 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: diff --git a/common/static/js/pdfviewer.js b/common/static/js/pdfviewer.js old mode 100644 new mode 100755 index 258541e0ad..a76e2f397c --- a/common/static/js/pdfviewer.js +++ b/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 diff --git a/common/test/data/open_ended/combinedopenended/SampleQuestionImageUpload.xml b/common/test/data/open_ended/combinedopenended/SampleQuestionImageUpload.xml new file mode 100644 index 0000000000..f52820cb26 --- /dev/null +++ b/common/test/data/open_ended/combinedopenended/SampleQuestionImageUpload.xml @@ -0,0 +1,24 @@ + + + + + Writing Applications + + + + + Language Conventions + + + + + + +

        Censorship in the Libraries

        +

        "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

        +

        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.

        +         + + + +         \ No newline at end of file diff --git a/common/test/data/open_ended/course/2012_Fall.xml b/common/test/data/open_ended/course/2012_Fall.xml index 232de855cc..57bcc6ddb6 100644 --- a/common/test/data/open_ended/course/2012_Fall.xml +++ b/common/test/data/open_ended/course/2012_Fall.xml @@ -2,6 +2,7 @@ + diff --git a/common/test/data/uploads/.gitignore b/common/test/data/uploads/.gitignore new file mode 100644 index 0000000000..a85ef7b7f3 --- /dev/null +++ b/common/test/data/uploads/.gitignore @@ -0,0 +1,3 @@ +test +test2 +asset.html diff --git a/common/test/data/uploads/test b/common/test/data/uploads/test deleted file mode 100644 index 588e9fb125..0000000000 --- a/common/test/data/uploads/test +++ /dev/null @@ -1 +0,0 @@ -This is an arbitrary file for testing uploads \ No newline at end of file diff --git a/docs/internal/discussion.md b/docs/internal/discussion.md index 9885cfabf6..e2956bb165 100644 --- a/docs/internal/discussion.md +++ b/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: diff --git a/docs/internal/testing.md b/docs/internal/testing.md index 5d6c75bddb..14cc2879b3 100644 --- a/docs/internal/testing.md +++ b/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: diff --git a/lms/djangoapps/branding/views.py b/lms/djangoapps/branding/views.py index 6a8ef8056b..3ce8dbd401 100644 --- a/lms/djangoapps/branding/views.py +++ b/lms/djangoapps/branding/views.py @@ -1,5 +1,6 @@ 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) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index acdd4f0aa0..f86bef29ad 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/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' ) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 4488b510d0..c5489d3b30 100644 --- a/lms/djangoapps/courseware/views.py +++ b/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(): diff --git a/lms/djangoapps/debug/management/__init__.py b/lms/djangoapps/debug/management/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/debug/management/commands/__init__.py b/lms/djangoapps/debug/management/commands/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/djangoapps/debug/management/commands/dump_xml_courses.py b/lms/djangoapps/debug/management/commands/dump_xml_courses.py new file mode 100644 index 0000000000..f0834282b7 --- /dev/null +++ b/lms/djangoapps/debug/management/commands/dump_xml_courses.py @@ -0,0 +1,59 @@ +""" +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 = '' + + 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) diff --git a/lms/djangoapps/open_ended_grading/tests.py b/lms/djangoapps/open_ended_grading/tests.py index 5a0d9e69e5..5224386664 100644 --- a/lms/djangoapps/open_ended_grading/tests.py +++ b/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) \ No newline at end of file + self.assertEqual(found, False) + +@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") + + diff --git a/lms/djangoapps/open_ended_grading/utils.py b/lms/djangoapps/open_ended_grading/utils.py index 15aaf0246f..1f860b1a6d 100644 --- a/lms/djangoapps/open_ended_grading/utils.py +++ b/lms/djangoapps/open_ended_grading/utils.py @@ -1,10 +1,58 @@ +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 diff --git a/lms/djangoapps/open_ended_grading/views.py b/lms/djangoapps/open_ended_grading/views.py index dcc1e11730..e8002e0883 100644 --- a/lms/djangoapps/open_ended_grading/views.py +++ b/lms/djangoapps/open_ended_grading/views.py @@ -1,5 +1,3 @@ -# Grading Views - import logging from django.conf import settings @@ -7,13 +5,9 @@ from django.views.decorators.cache import cache_control from mitxmako.shortcuts import render_to_response from django.core.urlresolvers import reverse -from xblock.field_data import DictFieldData - from student.models import unique_id_for_user from courseware.courses import get_course_with_access -from xmodule.x_module import ModuleSystem -from xmodule.open_ended_grading_classes.controller_query_service import ControllerQueryService, convert_seconds_to_human_readable from xmodule.open_ended_grading_classes.grading_service_module import GradingServiceError import json from student.models import unique_id_for_user @@ -26,28 +20,21 @@ from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem from django.http import HttpResponse, Http404, HttpResponseRedirect from mitxmako.shortcuts import render_to_string +from django.utils.translation import ugettext as _ + +from open_ended_grading.utils import (STAFF_ERROR_MESSAGE, STUDENT_ERROR_MESSAGE, + StudentProblemList, generate_problem_url, create_controller_query_service) log = logging.getLogger(__name__) -system = ModuleSystem( - ajax_url=None, - track_function=None, - get_module=None, - render_template=render_to_string, - replace_urls=None, - xmodule_field_data=DictFieldData({}), -) - -controller_qs = ControllerQueryService(settings.OPEN_ENDED_GRADING_INTERFACE, system) - -""" -Reverses the URL from the name and the course id, and then adds a trailing slash if -it does not exist yet - -""" - - def _reverse_with_slash(url_name, course_id): + """ + Reverses the URL given the name and the course id, and then adds a trailing slash if + it does not exist yet. + @param url_name: The name of the url (eg 'staff_grading'). + @param course_id: The id of the course object (eg course.id). + @returns: The reversed url with a trailing slash. + """ ajax_url = _reverse_without_slash(url_name, course_id) if not ajax_url.endswith('/'): ajax_url += '/' @@ -60,22 +47,19 @@ def _reverse_without_slash(url_name, course_id): DESCRIPTION_DICT = { - 'Peer Grading': "View all problems that require peer assessment in this particular course.", - 'Staff Grading': "View ungraded submissions submitted by students for the open ended problems in the course.", - 'Problems you have submitted': "View open ended problems that you have previously submitted for grading.", - 'Flagged Submissions': "View submissions that have been flagged by students as inappropriate." + 'Peer Grading': _("View all problems that require peer assessment in this particular course."), + 'Staff Grading': _("View ungraded submissions submitted by students for the open ended problems in the course."), + 'Problems you have submitted': _("View open ended problems that you have previously submitted for grading."), + 'Flagged Submissions': _("View submissions that have been flagged by students as inappropriate."), } + ALERT_DICT = { - 'Peer Grading': "New submissions to grade", - 'Staff Grading': "New submissions to grade", - 'Problems you have submitted': "New grades have been returned", - 'Flagged Submissions': "Submissions have been flagged for review" + 'Peer Grading': _("New submissions to grade"), + 'Staff Grading': _("New submissions to grade"), + 'Problems you have submitted': _("New grades have been returned"), + 'Flagged Submissions': _("Submissions have been flagged for review"), } -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 the development team. If you do not have a point of contact, please email Vik at vik@edx.org" - - @cache_control(no_cache=True, no_store=True, must_revalidate=True) def staff_grading(request, course_id): """ @@ -107,8 +91,6 @@ def find_peer_grading_module(course): # Get the course id and split it. course_id_parts = course.id.split("/") - log.info("COURSE ID PARTS") - log.info(course_id_parts) # Get the peer grading modules currently in the course. Explicitly specify the course id to avoid issues with different runs. items = modulestore().get_items(['i4x', course_id_parts[0], course_id_parts[1], 'peergrading', None], course_id=course.id) @@ -123,7 +105,7 @@ def find_peer_grading_module(course): except NoPathToItem: # In the case of nopathtoitem, the peer grading module that was found is in an invalid state, and # can no longer be accessed. Log an informational message, but this will not impact normal behavior. - log.info("Invalid peer grading module location {0} in course {1}. This module may need to be removed.".format(item_location, course.id)) + log.info(u"Invalid peer grading module location {0} in course {1}. This module may need to be removed.".format(item_location, course.id)) continue problem_url = generate_problem_url(problem_url_parts, base_course_url) found_module = True @@ -143,121 +125,60 @@ def peer_grading(request, course_id): found_module, problem_url = find_peer_grading_module(course) if not found_module: - #This is a student_facing_error - error_message = """ + error_message = _(""" Error with initializing peer grading. There has not been a peer grading module created in the courseware that would allow you to grade others. Please check back later for this. - """ - #This is a dev_facing_error - log.exception(error_message + "Current course is: {0}".format(course_id)) + """) + log.exception(error_message + u"Current course is: {0}".format(course_id)) return HttpResponse(error_message) return HttpResponseRedirect(problem_url) - -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 z in xrange(0, len(problem_url_parts)): - part = problem_url_parts[z] - if part is not None: - if z == 1: - problem_url += "courseware/" - problem_url += part + "/" - return problem_url - - @cache_control(no_cache=True, no_store=True, must_revalidate=True) def student_problem_list(request, course_id): - ''' - Show a student problem list to a student. Fetch the list from the grading controller server, get some metadata, - and then show it to the student. - ''' + """ + Show a list of problems they have attempted to a student. + Fetch the list from the grading controller server and append some data. + @param request: The request object for this view. + @param course_id: The id of the course to get the problem list for. + @return: Renders an HTML problem list table. + """ + + # Load the course. Don't catch any errors here, as we want them to be loud. course = get_course_with_access(request.user, course_id, 'load') + + # The anonymous student id is needed for communication with ORA. student_id = unique_id_for_user(request.user) - - # call problem list service - success = False - error_text = "" - problem_list = [] base_course_url = reverse('courses') - list_to_remove = [] + error_text = "" - try: - #Get list of all open ended problems that the grading server knows about - problem_list_json = controller_qs.get_grading_status_list(course_id, unique_id_for_user(request.user)) - problem_list_dict = json.loads(problem_list_json) - success = problem_list_dict['success'] - if 'error' in problem_list_dict: - error_text = problem_list_dict['error'] - problem_list = [] - else: - problem_list = problem_list_dict['problem_list'] + student_problem_list = StudentProblemList(course_id, student_id) + # Get the problem list from ORA. + success = student_problem_list.fetch_from_grading_service() + # If we fetched the problem list properly, add in additional problem data. + if success: + # Add in links to problems. + valid_problems = student_problem_list.add_problem_data(base_course_url) + else: + # Get an error message to show to the student. + valid_problems = [] + error_text = student_problem_list.error_text - #A list of problems to remove (problems that can't be found in the course) - for i in xrange(0, len(problem_list)): - try: - #Try to load each problem in the courseware to get links to them - problem_url_parts = search.path_to_location(modulestore(), course.id, problem_list[i]['location']) - except ItemNotFoundError: - #If the problem cannot be found at the location received from the grading controller server, it has been deleted by the course author. - #Continue with the rest of the location to construct the list - error_message = "Could not find module for course {0} at location {1}".format(course.id, - problem_list[i][ - 'location']) - log.error(error_message) - #Mark the problem for removal from the list - list_to_remove.append(i) - continue - problem_url = generate_problem_url(problem_url_parts, base_course_url) - problem_list[i].update({'actual_url': problem_url}) - eta_available = problem_list[i]['eta_available'] - if isinstance(eta_available, basestring): - eta_available = (eta_available.lower() == "true") - - eta_string = "N/A" - if eta_available: - try: - eta_string = convert_seconds_to_human_readable(int(problem_list[i]['eta'])) - except: - #This is a student_facing_error - eta_string = "Error getting ETA." - problem_list[i].update({'eta_string': eta_string}) - - except GradingServiceError: - #This is a student_facing_error - error_text = STUDENT_ERROR_MESSAGE - #This is a dev facing error - log.error("Problem contacting open ended grading service.") - success = False - # catch error if if the json loads fails - except ValueError: - #This is a student facing error - error_text = STUDENT_ERROR_MESSAGE - #This is a dev_facing_error - log.error("Problem with results from external grading service for open ended.") - success = False - - #Remove problems that cannot be found in the courseware from the list - problem_list = [problem_list[i] for i in xrange(0, len(problem_list)) if i not in list_to_remove] ajax_url = _reverse_with_slash('open_ended_problems', course_id) - return render_to_response('open_ended_problems/open_ended_problems.html', { + context = { 'course': course, 'course_id': course_id, 'ajax_url': ajax_url, 'success': success, - 'problem_list': problem_list, + 'problem_list': valid_problems, 'error_text': error_text, # Checked above - 'staff_access': False, }) + 'staff_access': False, + } + return render_to_response('open_ended_problems/open_ended_problems.html', context) @cache_control(no_cache=True, no_store=True, must_revalidate=True) def flagged_problem_list(request, course_id): @@ -273,6 +194,8 @@ def flagged_problem_list(request, course_id): problem_list = [] base_course_url = reverse('courses') + # Make a service that can query edX ORA. + controller_qs = create_controller_query_service() try: problem_list_json = controller_qs.get_flagged_problem_list(course_id) problem_list_dict = json.loads(problem_list_json) @@ -384,11 +307,12 @@ def take_action_on_flags(request, course_id): required = ['submission_id', 'action_type', 'student_id'] for key in required: if key not in request.POST: - #This is a staff_facing_error - return HttpResponse(json.dumps({'success': False, - 'error': STAFF_ERROR_MESSAGE + 'Missing key {0} from submission. Please reload and try again.'.format( - key)}), - mimetype="application/json") + error_message = u'Missing key {0} from submission. Please reload and try again.'.format(key) + response = { + 'success': False, + 'error': STAFF_ERROR_MESSAGE + error_message + } + return HttpResponse(json.dumps(response), mimetype="application/json") p = request.POST submission_id = p['submission_id'] @@ -397,12 +321,20 @@ def take_action_on_flags(request, course_id): student_id = student_id.strip(' \t\n\r') submission_id = submission_id.strip(' \t\n\r') action_type = action_type.lower().strip(' \t\n\r') + + # Make a service that can query edX ORA. + controller_qs = create_controller_query_service() try: response = controller_qs.take_action_on_flags(course_id, student_id, submission_id, action_type) return HttpResponse(response, mimetype="application/json") except GradingServiceError: - #This is a dev_facing_error log.exception( - "Error taking action on flagged peer grading submissions, submission_id: {0}, action_type: {1}, grader_id: {2}".format( - submission_id, action_type, grader_id)) - return _err_response(STAFF_ERROR_MESSAGE) + u"Error taking action on flagged peer grading submissions, " + u"submission_id: {0}, action_type: {1}, grader_id: {2}".format( + submission_id, action_type, student_id) + ) + response = { + 'success': False, + 'error': STAFF_ERROR_MESSAGE + } + return HttpResponse(json.dumps(response),mimetype="application/json") diff --git a/lms/envs/common.py b/lms/envs/common.py index 941809b82f..96b304294d 100644 --- a/lms/envs/common.py +++ b/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 ############################################ diff --git a/lms/envs/test.py b/lms/envs/test.py index 30475e26a0..f79c8c4218 100644 --- a/lms/envs/test.py +++ b/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. diff --git a/lms/static/sass/course/_staff_grading.scss b/lms/static/sass/course/_staff_grading.scss index 6810fe1bff..fd48ac28dc 100644 --- a/lms/static/sass/course/_staff_grading.scss +++ b/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; } diff --git a/lms/static/sass/course/_textbook.scss b/lms/static/sass/course/_textbook.scss old mode 100644 new mode 100755 index 72d73bdb78..d3632be2c5 --- a/lms/static/sass/course/_textbook.scss +++ b/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; diff --git a/lms/templates/courseware/course_about.html b/lms/templates/courseware/course_about.html index e63003b3ee..382dbf4660 100644 --- a/lms/templates/courseware/course_about.html +++ b/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: diff --git a/lms/templates/dashboard.html b/lms/templates/dashboard.html index a0add2ef00..f7e92ccce2 100644 --- a/lms/templates/dashboard.html +++ b/lms/templates/dashboard.html @@ -338,10 +338,14 @@ % else:
        -

        ${_("Looks like you haven't registered for any courses yet.")}

        -         + % if settings.MITX_FEATURES.get('COURSES_ARE_BROWSABLE'): +

        ${_("Looks like you haven't registered for any courses yet.")}

        +         ${_("Find courses now!")} - + + % else: +

        ${_("Looks like you haven't been enrolled in any courses yet.")}

        + %endif
        % endif diff --git a/lms/templates/index.html b/lms/templates/index.html index 0fecd24e84..83024e01aa 100644 --- a/lms/templates/index.html +++ b/lms/templates/index.html @@ -165,15 +165,17 @@ % endif -
        -
          - %for course in courses: -
        • - <%include file="course.html" args="course=course" /> -
          • - %endfor -
        -
        + % if settings.MITX_FEATURES.get('COURSES_ARE_BROWSABLE'): +
        +
          + %for course in courses: +
        • + <%include file="course.html" args="course=course" /> +
          • + %endfor +
        +
        + % endif diff --git a/lms/templates/navigation.html b/lms/templates/navigation.html index 8f6da00e68..e2e1d9d664 100644 --- a/lms/templates/navigation.html +++ b/lms/templates/navigation.html @@ -59,9 +59,11 @@ site_status_msg = get_site_status_msg(course_id)
          <%block name="navigation_global_links_authenticated"> -
        1. - ${_('Find Courses')} -
          2. + % if settings.MITX_FEATURES.get('COURSES_ARE_BROWSABLE'): +
        2. + ${_('Find Courses')} +
          3. + % endif
          @@ -82,7 +84,7 @@ site_status_msg = get_site_status_msg(course_id)
        % 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):
        1. diff --git a/lms/templates/open_ended_problems/open_ended_problems.html b/lms/templates/open_ended_problems/open_ended_problems.html index aa053111d8..1c7e2657ed 100644 --- a/lms/templates/open_ended_problems/open_ended_problems.html +++ b/lms/templates/open_ended_problems/open_ended_problems.html @@ -19,36 +19,32 @@

          ${_("Instructions")}

          ${_("Here is a list of open ended problems for this course.")}

          % if success: - % if len(problem_list) == 0: -
          - ${_("You have not attempted any open ended problems yet.")} -
          - %else: - - - - - - - - %for problem in problem_list: - - - - - - - %endfor -
          ${_("Problem Name")}${_("Status")}${_("Grader Type")}${_("ETA")}
          - ${problem['problem_name']} - - ${problem['state']} - - ${problem['grader_type']} - - ${problem['eta_string']} -
          - %endif + % if len(problem_list) == 0: +
          + ${_("You have not attempted any open ended problems yet.")} +
          + %else: + + + + + + + %for problem in problem_list: + + + + + + %endfor +
          ${_("Problem Name")}${_("Status")}${_("Grader Type")}
          + ${problem['problem_name']} + + ${problem['state']} + + ${problem['grader_type_display_name']} +
          + %endif %endif diff --git a/lms/templates/peer_grading/peer_grading_problem.html b/lms/templates/peer_grading/peer_grading_problem.html index be357bf054..bc0bbdfa34 100644 --- a/lms/templates/peer_grading/peer_grading_problem.html +++ b/lms/templates/peer_grading/peer_grading_problem.html @@ -55,8 +55,8 @@ ${_("This is a deletion.")}  ${_("[This is a comment.]")}  + Undo Change   Reset Changes - Undo Change

          @@ -65,8 +65,9 @@ % endif
          +
          - ${_("This submission has explicit, offensive, or (I suspect) plagiarized content: ")} + ${_("This submission has explicit, offensive, or (I suspect) plagiarized content. ")}
          diff --git a/lms/templates/video.html b/lms/templates/video.html index 3d0b9bd936..caf0aaa06f 100644 --- a/lms/templates/video.html +++ b/lms/templates/video.html @@ -42,31 +42,31 @@
          -
          +
          diff --git a/pylintrc b/pylintrc index 9525f04362..a3c84c1555 100644 --- a/pylintrc +++ b/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] diff --git a/rakelib/tests.rake b/rakelib/tests.rake index fb6cf575bd..ba436f3680 100644 --- a/rakelib/tests.rake +++ b/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 diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index addef7b629..278c1f8aa7 100644 --- a/requirements/edx/base.txt +++ b/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 diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index add9bda1d1..3a95566b2b 100644 --- a/requirements/edx/github.txt +++ b/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 diff --git a/requirements/system/mac_os_x/brew-formulas.txt b/requirements/system/mac_os_x/brew-formulas.txt index 5bc35aeb5f..22558042c4 100644 --- a/requirements/system/mac_os_x/brew-formulas.txt +++ b/requirements/system/mac_os_x/brew-formulas.txt @@ -11,3 +11,5 @@ mysql geos mongodb lynx +libjpeg +libtiff diff --git a/requirements/system/ubuntu/apt-packages.txt b/requirements/system/ubuntu/apt-packages.txt index 0e9687b19a..6f130107da 100644 --- a/requirements/system/ubuntu/apt-packages.txt +++ b/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