diff --git a/CHANGELOG.rst b/CHANGELOG.rst index cb8eec738f..4fea30a5c5 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -5,6 +5,15 @@ 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: Users are no longer auto-activated if they click "reset password" +This is now done when they click on the link in the reset password +email they receive (along with usual path through activation email). + +LMS: Problem rescoring. Added options on the Grades tab of the +Instructor Dashboard to allow a particular student's submission for a +particular problem to be rescored. Provides an option to see a +history of background tasks for a given problem and student. + Blades: Small UX fix on capa multiple-choice problems. Make labels only as wide as the text to reduce accidental choice selections. diff --git a/cms/djangoapps/auth/authz.py b/cms/djangoapps/auth/authz.py index 58b63abd23..a544906875 100644 --- a/cms/djangoapps/auth/authz.py +++ b/cms/djangoapps/auth/authz.py @@ -1,5 +1,6 @@ from django.contrib.auth.models import User, Group from django.core.exceptions import PermissionDenied +from django.conf import settings from xmodule.modulestore import Location @@ -12,6 +13,9 @@ but this implementation should be data compatible with the LMS implementation INSTRUCTOR_ROLE_NAME = 'instructor' STAFF_ROLE_NAME = 'staff' +# This is the group of people who have permission to create new courses on edge or edx. +COURSE_CREATOR_GROUP_NAME = "course_creator_group" + # we're just making a Django group for each location/role combo # to do this we're just creating a Group name which is a formatted string # of those two variables @@ -36,10 +40,10 @@ def get_users_in_course_group_by_role(location, role): return group.user_set.all() -''' -Create all permission groups for a new course and subscribe the caller into those roles -''' def create_all_course_groups(creator, location): + """ + Create all permission groups for a new course and subscribe the caller into those roles + """ create_new_course_group(creator, location, INSTRUCTOR_ROLE_NAME) create_new_course_group(creator, location, STAFF_ROLE_NAME) @@ -56,10 +60,10 @@ def create_new_course_group(creator, location, role): return def _delete_course_group(location): - ''' + """ This is to be called only by either a command line code path or through a app which has already asserted permissions - ''' + """ # remove all memberships instructors = Group.objects.get(name=get_course_groupname_for_role(location, INSTRUCTOR_ROLE_NAME)) for user in instructors.user_set.all(): @@ -72,10 +76,10 @@ def _delete_course_group(location): user.save() def _copy_course_group(source, dest): - ''' + """ This is to be called only by either a command line code path or through an app which has already asserted permissions to do this action - ''' + """ instructors = Group.objects.get(name=get_course_groupname_for_role(source, INSTRUCTOR_ROLE_NAME)) new_instructors_group = Group.objects.get(name=get_course_groupname_for_role(dest, INSTRUCTOR_ROLE_NAME)) for user in instructors.user_set.all(): @@ -94,10 +98,34 @@ def add_user_to_course_group(caller, user, location, role): if not is_user_in_course_group_role(caller, location, INSTRUCTOR_ROLE_NAME): raise PermissionDenied - if user.is_active and user.is_authenticated: - groupname = get_course_groupname_for_role(location, role) + group = Group.objects.get(name=get_course_groupname_for_role(location, role)) + return _add_user_to_group(user, group) - group = Group.objects.get(name=groupname) + +def add_user_to_creator_group(caller, user): + """ + Adds the user to the group of course creators. + + The caller must have staff access to perform this operation. + + Note that on the edX site, we currently limit course creators to edX staff, and this + method is a no-op in that environment. + """ + if not caller.is_active or not caller.is_authenticated or not caller.is_staff: + raise PermissionDenied + + (group, created) = Group.objects.get_or_create(name=COURSE_CREATOR_GROUP_NAME) + if created: + group.save() + return _add_user_to_group(user, group) + + +def _add_user_to_group(user, group): + """ + This is to be called only by either a command line code path or through an app which has already + asserted permissions to do this action + """ + if user.is_active and user.is_authenticated: user.groups.add(group) user.save() return True @@ -123,11 +151,29 @@ def remove_user_from_course_group(caller, user, location, role): # see if the user is actually in that role, if not then we don't have to do anything if is_user_in_course_group_role(user, location, role): - groupname = get_course_groupname_for_role(location, role) + _remove_user_from_group(user, get_course_groupname_for_role(location, role)) - group = Group.objects.get(name=groupname) - user.groups.remove(group) - user.save() + +def remove_user_from_creator_group(caller, user): + """ + Removes user from the course creator group. + + The caller must have staff access to perform this operation. + """ + if not caller.is_active or not caller.is_authenticated or not caller.is_staff: + raise PermissionDenied + + _remove_user_from_group(user, COURSE_CREATOR_GROUP_NAME) + + +def _remove_user_from_group(user, group_name): + """ + This is to be called only by either a command line code path or through an app which has already + asserted permissions to do this action + """ + group = Group.objects.get(name=group_name) + user.groups.remove(group) + user.save() def is_user_in_course_group_role(user, location, role): @@ -136,3 +182,26 @@ def is_user_in_course_group_role(user, location, role): return user.is_staff or user.groups.filter(name=get_course_groupname_for_role(location, role)).count() > 0 return False + + +def is_user_in_creator_group(user): + """ + Returns true if the user has permissions to create a course. + + Will always return True if user.is_staff is True. + + Note that on the edX site, we currently limit course creators to edX staff. On + other sites, this method checks that the user is in the course creator group. + """ + if user.is_staff: + return True + + # On edx, we only allow edX staff to create courses. This may be relaxed in the future. + if settings.MITX_FEATURES.get('DISABLE_COURSE_CREATION', False): + return False + + # Feature flag for using the creator group setting. Will be removed once the feature is complete. + if settings.MITX_FEATURES.get('ENABLE_CREATOR_GROUP', False): + return user.groups.filter(name=COURSE_CREATOR_GROUP_NAME).count() > 0 + + return True diff --git a/cms/djangoapps/auth/tests/test_authz.py b/cms/djangoapps/auth/tests/test_authz.py new file mode 100644 index 0000000000..173155df4c --- /dev/null +++ b/cms/djangoapps/auth/tests/test_authz.py @@ -0,0 +1,176 @@ +""" +Tests authz.py +""" +import mock + +from django.test import TestCase +from django.contrib.auth.models import User +from django.core.exceptions import PermissionDenied + +from auth.authz import add_user_to_creator_group, remove_user_from_creator_group, is_user_in_creator_group,\ + create_all_course_groups, add_user_to_course_group, STAFF_ROLE_NAME, INSTRUCTOR_ROLE_NAME,\ + is_user_in_course_group_role, remove_user_from_course_group + + +class CreatorGroupTest(TestCase): + """ + Tests for the course creator group. + """ + + def setUp(self): + """ Test case setup """ + self.user = User.objects.create_user('testuser', 'test+courses@edx.org', 'foo') + self.admin = User.objects.create_user('Mark', 'admin+courses@edx.org', 'foo') + self.admin.is_staff = True + + def test_creator_group_not_enabled(self): + """ + Tests that is_user_in_creator_group always returns True if ENABLE_CREATOR_GROUP + and DISABLE_COURSE_CREATION are both not turned on. + """ + self.assertTrue(is_user_in_creator_group(self.user)) + + def test_creator_group_enabled_but_empty(self): + """ Tests creator group feature on, but group empty. """ + with mock.patch.dict('django.conf.settings.MITX_FEATURES', {"ENABLE_CREATOR_GROUP": True}): + self.assertFalse(is_user_in_creator_group(self.user)) + + # Make user staff. This will cause is_user_in_creator_group to return True. + self.user.is_staff = True + self.assertTrue(is_user_in_creator_group(self.user)) + + def test_creator_group_enabled_nonempty(self): + """ Tests creator group feature on, user added. """ + with mock.patch.dict('django.conf.settings.MITX_FEATURES', {"ENABLE_CREATOR_GROUP": True}): + self.assertTrue(add_user_to_creator_group(self.admin, self.user)) + self.assertTrue(is_user_in_creator_group(self.user)) + + # check that a user who has not been added to the group still returns false + user_not_added = User.objects.create_user('testuser2', 'test+courses2@edx.org', 'foo2') + self.assertFalse(is_user_in_creator_group(user_not_added)) + + # remove first user from the group and verify that is_user_in_creator_group now returns false + remove_user_from_creator_group(self.admin, self.user) + self.assertFalse(is_user_in_creator_group(self.user)) + + def test_add_user_not_authenticated(self): + """ + Tests that adding to creator group fails if user is not authenticated + """ + self.user.is_authenticated = False + self.assertFalse(add_user_to_creator_group(self.admin, self.user)) + + def test_add_user_not_active(self): + """ + Tests that adding to creator group fails if user is not active + """ + self.user.is_active = False + self.assertFalse(add_user_to_creator_group(self.admin, self.user)) + + def test_course_creation_disabled(self): + """ Tests that the COURSE_CREATION_DISABLED flag overrides course creator group settings. """ + with mock.patch.dict('django.conf.settings.MITX_FEATURES', + {'DISABLE_COURSE_CREATION': True, "ENABLE_CREATOR_GROUP": True}): + # Add user to creator group. + self.assertTrue(add_user_to_creator_group(self.admin, self.user)) + + # DISABLE_COURSE_CREATION overrides (user is not marked as staff). + self.assertFalse(is_user_in_creator_group(self.user)) + + # Mark as staff. Now is_user_in_creator_group returns true. + self.user.is_staff = True + self.assertTrue(is_user_in_creator_group(self.user)) + + # Remove user from creator group. is_user_in_creator_group still returns true because is_staff=True + remove_user_from_creator_group(self.admin, self.user) + self.assertTrue(is_user_in_creator_group(self.user)) + + def test_add_user_to_group_requires_staff_access(self): + with self.assertRaises(PermissionDenied): + self.admin.is_staff = False + add_user_to_creator_group(self.admin, self.user) + + with self.assertRaises(PermissionDenied): + add_user_to_creator_group(self.user, self.user) + + def test_add_user_to_group_requires_active(self): + with self.assertRaises(PermissionDenied): + self.admin.is_active = False + add_user_to_creator_group(self.admin, self.user) + + def test_add_user_to_group_requires_authenticated(self): + with self.assertRaises(PermissionDenied): + self.admin.is_authenticated = False + add_user_to_creator_group(self.admin, self.user) + + def test_remove_user_from_group_requires_staff_access(self): + with self.assertRaises(PermissionDenied): + self.admin.is_staff = False + remove_user_from_creator_group(self.admin, self.user) + + def test_remove_user_from_group_requires_active(self): + with self.assertRaises(PermissionDenied): + self.admin.is_active = False + remove_user_from_creator_group(self.admin, self.user) + + def test_remove_user_from_group_requires_authenticated(self): + with self.assertRaises(PermissionDenied): + self.admin.is_authenticated = False + remove_user_from_creator_group(self.admin, self.user) + + +class CourseGroupTest(TestCase): + """ + Tests for instructor and staff groups for a particular course. + """ + + def setUp(self): + """ Test case setup """ + self.creator = User.objects.create_user('testcreator', 'testcreator+courses@edx.org', 'foo') + self.staff = User.objects.create_user('teststaff', 'teststaff+courses@edx.org', 'foo') + self.location = 'i4x', 'mitX', '101', 'course', 'test' + + def test_add_user_to_course_group(self): + """ + Tests adding user to course group (happy path). + """ + # Create groups for a new course (and assign instructor role to the creator). + self.assertFalse(is_user_in_course_group_role(self.creator, self.location, INSTRUCTOR_ROLE_NAME)) + create_all_course_groups(self.creator, self.location) + self.assertTrue(is_user_in_course_group_role(self.creator, self.location, INSTRUCTOR_ROLE_NAME)) + + # Add another user to the staff role. + self.assertFalse(is_user_in_course_group_role(self.staff, self.location, STAFF_ROLE_NAME)) + self.assertTrue(add_user_to_course_group(self.creator, self.staff, self.location, STAFF_ROLE_NAME)) + self.assertTrue(is_user_in_course_group_role(self.staff, self.location, STAFF_ROLE_NAME)) + + def test_add_user_to_course_group_permission_denied(self): + """ + Verifies PermissionDenied if caller of add_user_to_course_group is not instructor role. + """ + create_all_course_groups(self.creator, self.location) + with self.assertRaises(PermissionDenied): + add_user_to_course_group(self.staff, self.staff, self.location, STAFF_ROLE_NAME) + + def test_remove_user_from_course_group(self): + """ + Tests removing user from course group (happy path). + """ + create_all_course_groups(self.creator, self.location) + + self.assertTrue(add_user_to_course_group(self.creator, self.staff, self.location, STAFF_ROLE_NAME)) + self.assertTrue(is_user_in_course_group_role(self.staff, self.location, STAFF_ROLE_NAME)) + + remove_user_from_course_group(self.creator, self.staff, self.location, STAFF_ROLE_NAME) + self.assertFalse(is_user_in_course_group_role(self.staff, self.location, STAFF_ROLE_NAME)) + + remove_user_from_course_group(self.creator, self.creator, self.location, INSTRUCTOR_ROLE_NAME) + self.assertFalse(is_user_in_course_group_role(self.creator, self.location, INSTRUCTOR_ROLE_NAME)) + + def test_remove_user_from_course_group_permission_denied(self): + """ + Verifies PermissionDenied if caller of remove_user_from_course_group is not instructor role. + """ + create_all_course_groups(self.creator, self.location) + with self.assertRaises(PermissionDenied): + remove_user_from_course_group(self.staff, self.staff, self.location, STAFF_ROLE_NAME) diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index d24deacecf..b946aac6bb 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -1,5 +1,6 @@ import json import shutil +import mock from django.test.client import Client from django.test.utils import override_settings from django.conf import settings @@ -16,6 +17,8 @@ from django.dispatch import Signal from contentstore.utils import get_modulestore from contentstore.tests.utils import parse_json +from auth.authz import add_user_to_creator_group + from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory @@ -23,7 +26,7 @@ from xmodule.modulestore import Location from xmodule.modulestore.store_utilities import clone_course from xmodule.modulestore.store_utilities import delete_course from xmodule.modulestore.django import modulestore -from xmodule.contentstore.django import contentstore +from xmodule.contentstore.django import contentstore, _CONTENTSTORE from xmodule.templates import update_templates from xmodule.modulestore.xml_exporter import export_to_xml from xmodule.modulestore.xml_importer import import_from_xml, perform_xlint @@ -43,10 +46,12 @@ from django_comment_common.utils import are_permissions_roles_seeded from xmodule.exceptions import InvalidVersionError import datetime from pytz import UTC +from uuid import uuid4 +from pymongo import MongoClient -TEST_DATA_MODULESTORE = copy.deepcopy(settings.MODULESTORE) -TEST_DATA_MODULESTORE['default']['OPTIONS']['fs_root'] = path('common/test/data') -TEST_DATA_MODULESTORE['direct']['OPTIONS']['fs_root'] = path('common/test/data') + +TEST_DATA_CONTENTSTORE = copy.deepcopy(settings.CONTENTSTORE) +TEST_DATA_CONTENTSTORE['OPTIONS']['db'] = 'test_xcontent_%s' % uuid4().hex class MongoCollectionFindWrapper(object): @@ -59,13 +64,16 @@ class MongoCollectionFindWrapper(object): return self.original(query, *args, **kwargs) -@override_settings(MODULESTORE=TEST_DATA_MODULESTORE) +@override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE) class ContentStoreToyCourseTest(ModuleStoreTestCase): """ Tests that rely on the toy courses. TODO: refactor using CourseFactory so they do not. """ def setUp(self): + + settings.MODULESTORE['default']['OPTIONS']['fs_root'] = path('common/test/data') + settings.MODULESTORE['direct']['OPTIONS']['fs_root'] = path('common/test/data') uname = 'testuser' email = 'test+courses@edx.org' password = 'foo' @@ -83,6 +91,11 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): self.client = Client() self.client.login(username=uname, password=password) + def tearDown(self): + mongo = MongoClient() + mongo.drop_database(TEST_DATA_CONTENTSTORE['OPTIONS']['db']) + _CONTENTSTORE.clear() + def check_components_on_page(self, component_types, expected_types): """ Ensure that the right types end up on the page. @@ -403,7 +416,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): self.assertGreater(len(all_assets), 0) # make sure we have some thumbnails in our contentstore - all_thumbnails = content_store.get_all_content_thumbnails_for_course(course_location) + content_store.get_all_content_thumbnails_for_course(course_location) # # cdodge: temporarily comment out assertion on thumbnails because many environments @@ -442,7 +455,6 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): content_store = contentstore() trash_store = contentstore('trashcan') module_store = modulestore('direct') - import_from_xml(module_store, 'common/test/data/', ['full'], static_content_store=content_store) # look up original (and thumbnail) in content store, should be there after import @@ -519,7 +531,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): self.assertGreater(len(all_assets), 0) # make sure we have some thumbnails in our trashcan - all_thumbnails = trash_store.get_all_content_thumbnails_for_course(course_location) + _all_thumbnails = trash_store.get_all_content_thumbnails_for_course(course_location) # # cdodge: temporarily comment out assertion on thumbnails because many environments # will not have the jpeg converter installed and this test will fail @@ -533,7 +545,6 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): all_assets = trash_store.get_all_content_for_course(course_location) self.assertEqual(len(all_assets), 0) - all_thumbnails = trash_store.get_all_content_thumbnails_for_course(course_location) self.assertEqual(len(all_thumbnails), 0) @@ -581,24 +592,21 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): location = Location('i4x://MITx/999/chapter/neuvo') self.assertRaises(InvalidVersionError, draft_store.clone_item, 'i4x://edx/templates/chapter/Empty', - location) + location) direct_store.clone_item('i4x://edx/templates/chapter/Empty', location) - self.assertRaises(InvalidVersionError, draft_store.clone_item, location, - location) + self.assertRaises(InvalidVersionError, draft_store.clone_item, location, location) - self.assertRaises(InvalidVersionError, draft_store.update_item, location, - 'chapter data') + self.assertRaises(InvalidVersionError, draft_store.update_item, location, 'chapter data') # taking advantage of update_children and other functions never checking that the ids are valid self.assertRaises(InvalidVersionError, draft_store.update_children, location, - ['i4x://MITx/999/problem/doesntexist']) + ['i4x://MITx/999/problem/doesntexist']) self.assertRaises(InvalidVersionError, draft_store.update_metadata, location, - {'due': datetime.datetime.now(UTC)}) + {'due': datetime.datetime.now(UTC)}) self.assertRaises(InvalidVersionError, draft_store.unpublish, location) - def test_bad_contentstore_request(self): resp = self.client.get('http://localhost:8001/c4x/CDX/123123/asset/&images_circuits_Lab7Solution2.png') self.assertEqual(resp.status_code, 400) @@ -809,6 +817,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): export_to_xml(module_store, content_store, location, root_dir, 'test_export') +@override_settings(CONTENTSTORE=TEST_DATA_CONTENTSTORE) class ContentStoreTest(ModuleStoreTestCase): """ Tests for the CMS ContentStore application. @@ -845,8 +854,19 @@ class ContentStoreTest(ModuleStoreTestCase): 'display_name': 'Robot Super Course', } + def tearDown(self): + mongo = MongoClient() + mongo.drop_database(TEST_DATA_CONTENTSTORE['OPTIONS']['db']) + _CONTENTSTORE.clear() + def test_create_course(self): """Test new course creation - happy path""" + self.assert_created_course() + + def assert_created_course(self): + """ + Checks that the course was created properly. + """ resp = self.client.post(reverse('create_new_course'), self.course_data) self.assertEqual(resp.status_code, 200) data = parse_json(resp) @@ -854,41 +874,72 @@ class ContentStoreTest(ModuleStoreTestCase): def test_create_course_check_forum_seeding(self): """Test new course creation and verify forum seeding """ - resp = self.client.post(reverse('create_new_course'), self.course_data) - self.assertEqual(resp.status_code, 200) - data = parse_json(resp) - self.assertEqual(data['id'], 'i4x://MITx/999/course/Robot_Super_Course') + self.assert_created_course() self.assertTrue(are_permissions_roles_seeded('MITx/999/Robot_Super_Course')) def test_create_course_duplicate_course(self): """Test new course creation - error path""" self.client.post(reverse('create_new_course'), self.course_data) + self.assert_course_creation_failed('There is already a course defined with this name.') + + def assert_course_creation_failed(self, error_message): + """ + Checks that the course did not get created + """ resp = self.client.post(reverse('create_new_course'), self.course_data) - data = parse_json(resp) self.assertEqual(resp.status_code, 200) - self.assertEqual(data['ErrMsg'], 'There is already a course defined with this name.') + data = parse_json(resp) + self.assertEqual(data['ErrMsg'], error_message) def test_create_course_duplicate_number(self): """Test new course creation - error path""" self.client.post(reverse('create_new_course'), self.course_data) self.course_data['display_name'] = 'Robot Super Course Two' - resp = self.client.post(reverse('create_new_course'), self.course_data) - data = parse_json(resp) - - self.assertEqual(resp.status_code, 200) - self.assertEqual(data['ErrMsg'], - 'There is already a course defined with the same organization and course number.') + self.assert_course_creation_failed('There is already a course defined with the same organization and course number.') def test_create_course_with_bad_organization(self): """Test new course creation - error path for bad organization name""" self.course_data['org'] = 'University of California, Berkeley' - resp = self.client.post(reverse('create_new_course'), self.course_data) - data = parse_json(resp) + self.assert_course_creation_failed( + "Unable to create course 'Robot Super Course'.\n\nInvalid characters in 'University of California, Berkeley'.") - self.assertEqual(resp.status_code, 200) - self.assertEqual(data['ErrMsg'], - "Unable to create course 'Robot Super Course'.\n\nInvalid characters in 'University of California, Berkeley'.") + def test_create_course_with_course_creation_disabled_staff(self): + """Test new course creation -- course creation disabled, but staff access.""" + with mock.patch.dict('django.conf.settings.MITX_FEATURES', {'DISABLE_COURSE_CREATION': True}): + self.assert_created_course() + + def test_create_course_with_course_creation_disabled_not_staff(self): + """Test new course creation -- error path for course creation disabled, not staff access.""" + with mock.patch.dict('django.conf.settings.MITX_FEATURES', {'DISABLE_COURSE_CREATION': True}): + self.user.is_staff = False + self.user.save() + self.assert_course_permission_denied() + + def test_create_course_no_course_creators_staff(self): + """Test new course creation -- course creation group enabled, staff, group is empty.""" + with mock.patch.dict('django.conf.settings.MITX_FEATURES', {'ENABLE_CREATOR_GROUP': True}): + self.assert_created_course() + + def test_create_course_no_course_creators_not_staff(self): + """Test new course creation -- error path for course creator group enabled, not staff, group is empty.""" + with mock.patch.dict('django.conf.settings.MITX_FEATURES', {"ENABLE_CREATOR_GROUP": True}): + self.user.is_staff = False + self.user.save() + self.assert_course_permission_denied() + + def test_create_course_with_course_creator(self): + """Test new course creation -- use course creator group""" + with mock.patch.dict('django.conf.settings.MITX_FEATURES', {"ENABLE_CREATOR_GROUP": True}): + add_user_to_creator_group(self.user, self.user) + self.assert_created_course() + + def assert_course_permission_denied(self): + """ + Checks that the course did not get created due to a PermissionError. + """ + resp = self.client.post(reverse('create_new_course'), self.course_data) + self.assertEqual(resp.status_code, 403) def test_course_index_view_with_no_courses(self): """Test viewing the index page with no courses""" diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index dd7573bad5..8862115c45 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -21,7 +21,7 @@ from contentstore.utils import get_lms_link_for_item, add_extra_panel_tab, remov from models.settings.course_details import CourseDetails, CourseSettingsEncoder from models.settings.course_grading import CourseGradingModel from models.settings.course_metadata import CourseMetadata -from auth.authz import create_all_course_groups +from auth.authz import create_all_course_groups, is_user_in_creator_group from util.json_request import expect_json from .access import has_access, get_location_and_verify_access @@ -81,7 +81,7 @@ def course_index(request, org, course, name): @expect_json def create_new_course(request): - if settings.MITX_FEATURES.get('DISABLE_COURSE_CREATION', False) and not request.user.is_staff: + if not is_user_in_creator_group(request.user): raise PermissionDenied() # This logic is repeated in xmodule/modulestore/tests/factories.py diff --git a/cms/envs/acceptance.py b/cms/envs/acceptance.py index 6293219f43..871b744282 100644 --- a/cms/envs/acceptance.py +++ b/cms/envs/acceptance.py @@ -40,6 +40,21 @@ MODULESTORE = { 'OPTIONS': MODULESTORE_OPTIONS } } + +CONTENTSTORE = { + 'ENGINE': 'xmodule.contentstore.mongo.MongoContentStore', + 'OPTIONS': { + 'host': 'localhost', + 'db': 'acceptance_xcontent', + }, + # allow for additional options that can be keyed on a name, e.g. 'trashcan' + 'ADDITIONAL_OPTIONS': { + 'trashcan': { + 'bucket': 'trash_fs' + } + } +} + # Set this up so that rake lms[acceptance] and running the # harvest command both use the same (test) database # which they can flush without messing up your dev db diff --git a/cms/envs/test.py b/cms/envs/test.py index 954a553e10..89813dd937 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -70,7 +70,7 @@ CONTENTSTORE = { 'ENGINE': 'xmodule.contentstore.mongo.MongoContentStore', 'OPTIONS': { 'host': 'localhost', - 'db': 'test_xmodule', + 'db': 'test_xcontent', }, # allow for additional options that can be keyed on a name, e.g. 'trashcan' 'ADDITIONAL_OPTIONS': { diff --git a/cms/static/coffee/spec/views/feedback_spec.coffee b/cms/static/coffee/spec/views/feedback_spec.coffee index e5916c5ed3..adec11e2a7 100644 --- a/cms/static/coffee/spec/views/feedback_spec.coffee +++ b/cms/static/coffee/spec/views/feedback_spec.coffee @@ -17,6 +17,16 @@ beforeEach -> return text.test(trimmedText) else return trimmedText.indexOf(text) != -1; + toHaveBeenPrevented: -> + # remove this when we upgrade jasmine-jquery + eventName = @actual.eventName + selector = @actual.selector + @message = -> + [ + "Expected event #{eventName} to have been prevented on #{selector}", + "Expected event #{eventName} not to have been prevented on #{selector}" + ] + return jasmine.JQuery.events.wasPrevented(selector, eventName) describe "CMS.Views.SystemFeedback", -> beforeEach -> @@ -123,6 +133,35 @@ describe "CMS.Views.SystemFeedback click events", -> it "should apply class to secondary action", -> expect(@view.$(".action-secondary")).toHaveClass("cancel-button") + it "should preventDefault on primary action", -> + spyOnEvent(".action-primary", "click") + @view.$(".action-primary").click() + expect("click").toHaveBeenPreventedOn(".action-primary") + + it "should preventDefault on secondary action", -> + spyOnEvent(".action-secondary", "click") + @view.$(".action-secondary").click() + expect("click").toHaveBeenPreventedOn(".action-secondary") + +describe "CMS.Views.SystemFeedback not preventing events", -> + beforeEach -> + @clickSpy = jasmine.createSpy('clickSpy') + @view = new CMS.Views.Alert.Confirmation( + title: "It's all good" + message: "No reason for this alert" + actions: + primary: + text: "Whatever" + click: @clickSpy + preventDefault: false + ) + @view.show() + + it "should not preventDefault", -> + spyOnEvent(".action-primary", "click") + @view.$(".action-primary").click() + expect("click").not.toHaveBeenPreventedOn(".action-primary") + expect(@clickSpy).toHaveBeenCalled() describe "CMS.Views.SystemFeedback multiple secondary actions", -> beforeEach -> diff --git a/cms/static/js/views/feedback.js b/cms/static/js/views/feedback.js index 3f161d5b1f..3bfeeb5af2 100644 --- a/cms/static/js/views/feedback.js +++ b/cms/static/js/views/feedback.js @@ -10,8 +10,12 @@ CMS.Views.SystemFeedback = Backbone.View.extend({ minShown: 0, // length of time after this view has been shown before it can be hidden (milliseconds) maxShown: Infinity // length of time after this view has been shown before it will be automatically hidden (milliseconds) - /* could also have an "actions" hash: here is an example demonstrating - the expected structure + /* Could also have an "actions" hash: here is an example demonstrating + the expected structure. For each action, by default the framework + will call preventDefault on the click event before the function is + run; to make it not do that, just pass `preventDefault: false` in + the action object. + actions: { primary: { "text": "Save", @@ -106,6 +110,9 @@ CMS.Views.SystemFeedback = Backbone.View.extend({ if(!actions) { return; } var primary = actions.primary; if(!primary) { return; } + if(primary.preventDefault !== false) { + event.preventDefault(); + } if(primary.click) { primary.click.call(event.target, this, event); } @@ -121,6 +128,9 @@ CMS.Views.SystemFeedback = Backbone.View.extend({ i = _.indexOf(this.$(".action-secondary"), event.target); } var secondary = secondaryList[i]; + if(secondary.preventDefault !== false) { + event.preventDefault(); + } if(secondary.click) { secondary.click.call(event.target, this, event); } diff --git a/cms/templates/emails/activation_email_subject.txt b/cms/templates/emails/activation_email_subject.txt index 0b0fb2ffe9..f4ffdccb14 100644 --- a/cms/templates/emails/activation_email_subject.txt +++ b/cms/templates/emails/activation_email_subject.txt @@ -1 +1 @@ -Your account for edX edge +Your account for edX Studio diff --git a/common/djangoapps/student/forms.py b/common/djangoapps/student/forms.py new file mode 100644 index 0000000000..1096092117 --- /dev/null +++ b/common/djangoapps/student/forms.py @@ -0,0 +1,21 @@ +from django import forms +from django.contrib.auth.models import User +from django.contrib.auth.forms import PasswordResetForm +from django.contrib.auth.hashers import UNUSABLE_PASSWORD + +class PasswordResetFormNoActive(PasswordResetForm): + def clean_email(self): + """ + This is a literal copy from Django 1.4.5's django.contrib.auth.forms.PasswordResetForm + Except removing the requirement of active users + Validates that a user exists with the given email address. + """ + email = self.cleaned_data["email"] + #The line below contains the only change, removing is_active=True + self.users_cache = User.objects.filter(email__iexact=email) + if not len(self.users_cache): + raise forms.ValidationError(self.error_messages['unknown']) + if any((user.password == UNUSABLE_PASSWORD) + for user in self.users_cache): + raise forms.ValidationError(self.error_messages['unusable']) + return email diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index 4638da44b2..844ddb536e 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -5,18 +5,127 @@ when you run "manage.py test". Replace this with more appropriate tests for your application. """ import logging +import json +import re +import unittest +from django import forms +from django.conf import settings from django.test import TestCase -from mock import Mock +from django.test.client import RequestFactory +from django.contrib.auth.models import User +from django.contrib.auth.hashers import UNUSABLE_PASSWORD +from django.contrib.auth.tokens import default_token_generator +from django.template.loader import render_to_string, get_template, TemplateDoesNotExist +from django.core.urlresolvers import is_valid_path +from django.utils.http import int_to_base36 + + +from mock import Mock, patch +from textwrap import dedent from student.models import unique_id_for_user -from student.views import process_survey_link, _cert_info - +from student.views import process_survey_link, _cert_info, password_reset, password_reset_confirm_wrapper +from student.tests.factories import UserFactory +from student.tests.test_email import mock_render_to_string COURSE_1 = 'edX/toy/2012_Fall' COURSE_2 = 'edx/full/6.002_Spring_2012' log = logging.getLogger(__name__) +try: + get_template('registration/password_reset_email.html') + project_uses_password_reset = True +except TemplateDoesNotExist: + project_uses_password_reset = False + + +class ResetPasswordTests(TestCase): + """ Tests that clicking reset password sends email, and doesn't activate the user + """ + request_factory = RequestFactory() + + def setUp(self): + self.user = UserFactory.create() + self.user.is_active = False + self.user.save() + self.token = default_token_generator.make_token(self.user) + self.uidb36 = int_to_base36(self.user.id) + + self.user_bad_passwd = UserFactory.create() + self.user_bad_passwd.is_active = False + self.user_bad_passwd.password = UNUSABLE_PASSWORD + self.user_bad_passwd.save() + + def test_user_bad_password_reset(self): + """Tests password reset behavior for user with password marked UNUSABLE_PASSWORD""" + + bad_pwd_req = self.request_factory.post('/password_reset/', {'email': self.user_bad_passwd.email}) + bad_pwd_resp = password_reset(bad_pwd_req) + self.assertEquals(bad_pwd_resp.status_code, 200) + self.assertEquals(bad_pwd_resp.content, json.dumps({'success': False, + 'error': 'Invalid e-mail or user'})) + + def test_nonexist_email_password_reset(self): + """Now test the exception cases with of reset_password called with invalid email.""" + + bad_email_req = self.request_factory.post('/password_reset/', {'email': self.user.email+"makeItFail"}) + bad_email_resp = password_reset(bad_email_req) + self.assertEquals(bad_email_resp.status_code, 200) + self.assertEquals(bad_email_resp.content, json.dumps({'success': False, + 'error': 'Invalid e-mail or user'})) + + @unittest.skipUnless(project_uses_password_reset, + dedent("""Skipping Test because CMS has not provided necessary templates for password reset. + If LMS tests print this message, that needs to be fixed.""")) + @patch('django.core.mail.send_mail') + @patch('student.views.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True)) + def test_reset_password_email(self, send_email): + """Tests contents of reset password email, and that user is not active""" + + good_req = self.request_factory.post('/password_reset/', {'email': self.user.email}) + good_resp = password_reset(good_req) + self.assertEquals(good_resp.status_code, 200) + self.assertEquals(good_resp.content, + json.dumps({'success': True, + 'value': "('registration/password_reset_done.html', [])"})) + + ((subject, msg, from_addr, to_addrs), sm_kwargs) = send_email.call_args + self.assertIn("Password reset", subject) + self.assertIn("You're receiving this e-mail because you requested a password reset", msg) + self.assertEquals(from_addr, settings.DEFAULT_FROM_EMAIL) + self.assertEquals(len(to_addrs), 1) + self.assertIn(self.user.email, to_addrs) + + #test that the user is not active + self.user = User.objects.get(pk=self.user.pk) + self.assertFalse(self.user.is_active) + reset_match = re.search(r'password_reset_confirm/(?P[0-9A-Za-z]+)-(?P.+)/', msg).groupdict() + + @patch('student.views.password_reset_confirm') + def test_reset_password_bad_token(self, reset_confirm): + """Tests bad token and uidb36 in password reset""" + + bad_reset_req = self.request_factory.get('/password_reset_confirm/NO-OP/') + password_reset_confirm_wrapper(bad_reset_req, 'NO', 'OP') + (confirm_args, confirm_kwargs) = reset_confirm.call_args + self.assertEquals(confirm_kwargs['uidb36'], 'NO') + self.assertEquals(confirm_kwargs['token'], 'OP') + self.user = User.objects.get(pk=self.user.pk) + self.assertFalse(self.user.is_active) + + @patch('student.views.password_reset_confirm') + def test_reset_password_good_token(self, reset_confirm): + """Tests good token and uidb36 in password reset""" + + good_reset_req = self.request_factory.get('/password_reset_confirm/{0}-{1}/'.format(self.uidb36, self.token)) + password_reset_confirm_wrapper(good_reset_req, self.uidb36, self.token) + (confirm_args, confirm_kwargs) = reset_confirm.call_args + self.assertEquals(confirm_kwargs['uidb36'], self.uidb36) + self.assertEquals(confirm_kwargs['token'], self.token) + self.user = User.objects.get(pk=self.user.pk) + self.assertTrue(self.user.is_active) + class CourseEndingTest(TestCase): """Test things related to course endings: certificates, surveys, etc""" diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 6b9c9104c5..f0eab39781 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -11,9 +11,9 @@ import time from django.conf import settings from django.contrib.auth import logout, authenticate, login -from django.contrib.auth.forms import PasswordResetForm from django.contrib.auth.models import User from django.contrib.auth.decorators import login_required +from django.contrib.auth.views import password_reset_confirm from django.core.cache import cache from django.core.context_processors import csrf from django.core.mail import send_mail @@ -24,6 +24,7 @@ from django.http import HttpResponse, HttpResponseBadRequest, HttpResponseForbid from django.shortcuts import redirect from django_future.csrf import ensure_csrf_cookie from django.utils.http import cookie_date +from django.utils.http import base36_to_int from mitxmako.shortcuts import render_to_response, render_to_string from bs4 import BeautifulSoup @@ -34,6 +35,8 @@ from student.models import (Registration, UserProfile, TestCenterUser, TestCente CourseEnrollment, unique_id_for_user, get_testcenter_registration, CourseEnrollmentAllowed) +from student.forms import PasswordResetFormNoActive + from certificates.models import CertificateStatuses, certificate_status_for_student from xmodule.course_module import CourseDescriptor @@ -962,17 +965,7 @@ def password_reset(request): if request.method != "POST": raise Http404 - # By default, Django doesn't allow Users with is_active = False to reset their passwords, - # but this bites people who signed up a long time ago, never activated, and forgot their - # password. So for their sake, we'll auto-activate a user for whom password_reset is called. - try: - user = User.objects.get(email=request.POST['email']) - user.is_active = True - user.save() - except: - log.exception("Tried to auto-activate user to enable password reset, but failed.") - - form = PasswordResetForm(request.POST) + form = PasswordResetFormNoActive(request.POST) if form.is_valid(): form.save(use_https=request.is_secure(), from_email=settings.DEFAULT_FROM_EMAIL, @@ -982,7 +975,21 @@ def password_reset(request): 'value': render_to_string('registration/password_reset_done.html', {})})) else: return HttpResponse(json.dumps({'success': False, - 'error': 'Invalid e-mail'})) + 'error': 'Invalid e-mail or user'})) + +def password_reset_confirm_wrapper(request, uidb36=None, token=None): + ''' A wrapper around django.contrib.auth.views.password_reset_confirm. + Needed because we want to set the user as active at this step. + ''' + #cribbed from django.contrib.auth.views.password_reset_confirm + try: + uid_int = base36_to_int(uidb36) + user = User.objects.get(id=uid_int) + user.is_active = True + user.save() + except (ValueError, User.DoesNotExist): + pass + return password_reset_confirm(request, uidb36=uidb36, token=token) def reactivation_email_for_user(user): diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index d620bac60a..2c813f49d5 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -373,7 +373,7 @@ class LoncapaProblem(object): html = contextualize_text(etree.tostring(self._extract_html(self.tree)), self.context) return html - def handle_input_ajax(self, get): + def handle_input_ajax(self, data): ''' InputTypes can support specialized AJAX calls. Find the correct input and pass along the correct data @@ -381,10 +381,10 @@ class LoncapaProblem(object): ''' # pull out the id - input_id = get['input_id'] + input_id = data['input_id'] if self.inputs[input_id]: - dispatch = get['dispatch'] - return self.inputs[input_id].handle_ajax(dispatch, get) + dispatch = data['dispatch'] + return self.inputs[input_id].handle_ajax(dispatch, data) else: log.warning("Could not find matching input for id: %s" % input_id) return {} diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py index f026568da1..4c40a2cd3e 100644 --- a/common/lib/capa/capa/inputtypes.py +++ b/common/lib/capa/capa/inputtypes.py @@ -223,13 +223,13 @@ class InputTypeBase(object): """ pass - def handle_ajax(self, dispatch, get): + def handle_ajax(self, dispatch, data): """ InputTypes that need to handle specialized AJAX should override this. Input: dispatch: a string that can be used to determine how to handle the data passed in - get: a dictionary containing the data that was sent with the ajax call + data: a dictionary containing the data that was sent with the ajax call Output: a dictionary object that can be serialized into JSON. This will be sent back to the Javascript. @@ -677,20 +677,20 @@ class MatlabInput(CodeInput): self.queue_len = 1 self.msg = self.plot_submitted_msg - def handle_ajax(self, dispatch, get): + def handle_ajax(self, dispatch, data): ''' Handle AJAX calls directed to this input Args: - dispatch (str) - indicates how we want this ajax call to be handled - - get (dict) - dictionary of key-value pairs that contain useful data + - data (dict) - dictionary of key-value pairs that contain useful data Returns: dict - 'success' - whether or not we successfully queued this submission - 'message' - message to be rendered in case of error ''' if dispatch == 'plot': - return self._plot_data(get) + return self._plot_data(data) return {} def ungraded_response(self, queue_msg, queuekey): @@ -751,7 +751,7 @@ class MatlabInput(CodeInput): msg = result['msg'] return msg - def _plot_data(self, get): + def _plot_data(self, data): ''' AJAX handler for the plot button Args: @@ -765,7 +765,7 @@ class MatlabInput(CodeInput): return {'success': False, 'message': 'Cannot connect to the queue'} # pull relevant info out of get - response = get['submission'] + response = data['submission'] # construct xqueue headers qinterface = self.system.xqueue['interface'] @@ -951,16 +951,16 @@ class ChemicalEquationInput(InputTypeBase): """ return {'previewer': '/static/js/capa/chemical_equation_preview.js', } - def handle_ajax(self, dispatch, get): + def handle_ajax(self, dispatch, data): ''' Since we only have chemcalc preview this input, check to see if it matches the corresponding dispatch and send it through if it does ''' if dispatch == 'preview_chemcalc': - return self.preview_chemcalc(get) + return self.preview_chemcalc(data) return {} - def preview_chemcalc(self, get): + def preview_chemcalc(self, data): """ Render an html preview of a chemical formula or equation. get should contain a key 'formula' and value 'some formula string'. @@ -974,7 +974,7 @@ class ChemicalEquationInput(InputTypeBase): result = {'preview': '', 'error': ''} - formula = get['formula'] + formula = data['formula'] if formula is None: result['error'] = "No formula specified." return result diff --git a/common/lib/capa/capa/safe_exec/safe_exec.py b/common/lib/capa/capa/safe_exec/safe_exec.py index 3ab8f0bf9e..be33bcaa5b 100644 --- a/common/lib/capa/capa/safe_exec/safe_exec.py +++ b/common/lib/capa/capa/safe_exec/safe_exec.py @@ -18,7 +18,6 @@ import random as random_module import sys random = random_module.Random(%r) random.Random = random_module.Random -del random_module sys.modules['random'] = random """ diff --git a/common/lib/capa/capa/tests/test_inputtypes.py b/common/lib/capa/capa/tests/test_inputtypes.py index 313eb28249..1b52d41890 100644 --- a/common/lib/capa/capa/tests/test_inputtypes.py +++ b/common/lib/capa/capa/tests/test_inputtypes.py @@ -467,8 +467,8 @@ class MatlabTest(unittest.TestCase): self.assertEqual(context, expected) def test_plot_data(self): - get = {'submission': 'x = 1234;'} - response = self.the_input.handle_ajax("plot", get) + data = {'submission': 'x = 1234;'} + response = self.the_input.handle_ajax("plot", data) test_system().xqueue['interface'].send_to_queue.assert_called_with(header=ANY, body=ANY) @@ -477,10 +477,10 @@ class MatlabTest(unittest.TestCase): self.assertEqual(self.the_input.input_state['queuestate'], 'queued') def test_plot_data_failure(self): - get = {'submission': 'x = 1234;'} + data = {'submission': 'x = 1234;'} error_message = 'Error message!' test_system().xqueue['interface'].send_to_queue.return_value = (1, error_message) - response = self.the_input.handle_ajax("plot", get) + response = self.the_input.handle_ajax("plot", data) self.assertFalse(response['success']) self.assertEqual(response['message'], error_message) self.assertTrue('queuekey' not in self.the_input.input_state) diff --git a/common/lib/capa/capa/tests/test_responsetypes.py b/common/lib/capa/capa/tests/test_responsetypes.py index 68be54b6af..594e2ca629 100644 --- a/common/lib/capa/capa/tests/test_responsetypes.py +++ b/common/lib/capa/capa/tests/test_responsetypes.py @@ -1266,6 +1266,24 @@ class CustomResponseTest(ResponseTest): msg = correct_map.get_msg('1_2_1') self.assertEqual(msg, self._get_random_number_result(problem.seed)) + def test_random_isnt_none(self): + # Bug LMS-500 says random.seed(10) fails with: + # File "", line 61, in + # File "/usr/lib/python2.7/random.py", line 116, in seed + # super(Random, self).seed(a) + # TypeError: must be type, not None + + r = random.Random() + r.seed(10) + num = r.randint(0, 1e9) + + script = textwrap.dedent(""" + random.seed(10) + num = random.randint(0, 1e9) + """) + problem = self.build_problem(script=script) + self.assertEqual(problem.context['num'], num) + def test_module_imports_inline(self): ''' Check that the correct modules are available to custom diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index bb06912f7a..eeb8f19439 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -519,11 +519,11 @@ class CapaModule(CapaFields, XModule): # now do the substitutions which are filesystem based, e.g. '/static/' prefixes return self.system.replace_urls(html) - def handle_ajax(self, dispatch, get): + def handle_ajax(self, dispatch, data): """ This is called by courseware.module_render, to handle an AJAX call. - `get` is request.POST. + `data` is request.POST. Returns a json dictionary: { 'progress_changed' : True/False, @@ -547,18 +547,19 @@ class CapaModule(CapaFields, XModule): before = self.get_progress() try: - d = handlers[dispatch](get) - + result = handlers[dispatch](data) except Exception as err: _, _, traceback_obj = sys.exc_info() - raise ProcessingError, err.message, traceback_obj + raise ProcessingError(err.message, traceback_obj) after = self.get_progress() - d.update({ + + result.update({ 'progress_changed': after != before, 'progress_status': Progress.to_js_status_str(after), }) - return json.dumps(d, cls=ComplexEncoder) + + return json.dumps(result, cls=ComplexEncoder) def is_past_due(self): """ @@ -633,32 +634,32 @@ class CapaModule(CapaFields, XModule): return False - def update_score(self, get): + def update_score(self, data): """ Delivers grading response (e.g. from asynchronous code checking) to the capa problem, so its score can be updated - `get` must have a field `response` which is a string that contains the + 'data' must have a key 'response' which is a string that contains the grader's response No ajax return is needed. Return empty dict. """ - queuekey = get['queuekey'] - score_msg = get['xqueue_body'] + queuekey = data['queuekey'] + score_msg = data['xqueue_body'] self.lcp.update_score(score_msg, queuekey) self.set_state_from_lcp() self.publish_grade() return dict() # No AJAX return is needed - def handle_ungraded_response(self, get): + def handle_ungraded_response(self, data): """ Delivers a response from the XQueue to the capa problem The score of the problem will not be updated Args: - - get (dict) must contain keys: + - data (dict) must contain keys: queuekey - a key specific to this response xqueue_body - the body of the response Returns: @@ -666,28 +667,30 @@ class CapaModule(CapaFields, XModule): No ajax return is needed, so an empty dict is returned """ - queuekey = get['queuekey'] - score_msg = get['xqueue_body'] + queuekey = data['queuekey'] + score_msg = data['xqueue_body'] + # pass along the xqueue message to the problem self.lcp.ungraded_response(score_msg, queuekey) self.set_state_from_lcp() return dict() - def handle_input_ajax(self, get): + def handle_input_ajax(self, data): """ Handle ajax calls meant for a particular input in the problem Args: - - get (dict) - data that should be passed to the input + - data (dict) - data that should be passed to the input Returns: - dict containing the response from the input """ - response = self.lcp.handle_input_ajax(get) + response = self.lcp.handle_input_ajax(data) + # save any state changes that may occur self.set_state_from_lcp() return response - def get_answer(self, get): + def get_answer(self, data): """ For the "show answer" button. @@ -717,10 +720,9 @@ class CapaModule(CapaFields, XModule): return {'answers': new_answers} # Figure out if we should move these to capa_problem? - def get_problem(self, get): + def get_problem(self, _data): """ Return results of get_problem_html, as a simple dict for json-ing. - { 'html': } Used if we want to reconfirm we have the right thing e.g. after @@ -729,27 +731,27 @@ class CapaModule(CapaFields, XModule): return {'html': self.get_problem_html(encapsulate=False)} @staticmethod - def make_dict_of_responses(get): + def make_dict_of_responses(data): """ Make dictionary of student responses (aka "answers") - `get` is POST dictionary (Django QueryDict). + `data` is POST dictionary (Django QueryDict). - The `get` dict has keys of the form 'x_y', which are mapped + The `data` dict has keys of the form 'x_y', which are mapped to key 'y' in the returned dict. For example, 'input_1_2_3' would be mapped to '1_2_3' in the returned dict. Some inputs always expect a list in the returned dict (e.g. checkbox inputs). The convention is that - keys in the `get` dict that end with '[]' will always + keys in the `data` dict that end with '[]' will always have list values in the returned dict. - For example, if the `get` dict contains {'input_1[]': 'test' } + For example, if the `data` dict contains {'input_1[]': 'test' } then the output dict would contain {'1': ['test'] } (the value is a list). Raises an exception if: - -A key in the `get` dictionary does not contain at least one underscore + -A key in the `data` dictionary does not contain at least one underscore (e.g. "input" is invalid, but "input_1" is valid) -Two keys end up with the same name in the returned dict. @@ -758,7 +760,7 @@ class CapaModule(CapaFields, XModule): """ answers = dict() - for key in get: + for key in data: # e.g. input_resistor_1 ==> resistor_1 _, _, name = key.partition('_') @@ -777,9 +779,9 @@ class CapaModule(CapaFields, XModule): name = name[:-2] if is_list_key else name if is_list_key: - val = get.getlist(key) + val = data.getlist(key) else: - val = get[key] + val = data[key] # If the name already exists, then we don't want # to override it. Raise an error instead @@ -801,7 +803,7 @@ class CapaModule(CapaFields, XModule): 'max_value': score['total'], }) - def check_problem(self, get): + def check_problem(self, data): """ Checks whether answers to a problem are correct @@ -813,8 +815,9 @@ class CapaModule(CapaFields, XModule): event_info['state'] = self.lcp.get_state() event_info['problem_id'] = self.location.url() - answers = self.make_dict_of_responses(get) + answers = self.make_dict_of_responses(data) event_info['answers'] = convert_files_to_filenames(answers) + # Too late. Cannot submit if self.closed(): event_info['failure'] = 'closed' @@ -972,7 +975,7 @@ class CapaModule(CapaFields, XModule): return {'success': success} - def save_problem(self, get): + def save_problem(self, data): """ Save the passed in answers. Returns a dict { 'success' : bool, 'msg' : message } @@ -982,7 +985,7 @@ class CapaModule(CapaFields, XModule): event_info['state'] = self.lcp.get_state() event_info['problem_id'] = self.location.url() - answers = self.make_dict_of_responses(get) + answers = self.make_dict_of_responses(data) event_info['answers'] = answers # Too late. Cannot submit @@ -1011,7 +1014,7 @@ class CapaModule(CapaFields, XModule): return {'success': True, 'msg': msg} - def reset_problem(self, get): + def reset_problem(self, _data): """ Changes problem state to unfinished -- removes student answers, and causes problem to rerender itself. diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index 68285cae0d..52d98f032e 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -204,9 +204,9 @@ class CombinedOpenEndedModule(CombinedOpenEndedFields, XModule): return_value = self.child_module.get_html() return return_value - def handle_ajax(self, dispatch, get): + def handle_ajax(self, dispatch, data): self.save_instance_data() - return_value = self.child_module.handle_ajax(dispatch, get) + return_value = self.child_module.handle_ajax(dispatch, data) self.save_instance_data() return return_value @@ -266,4 +266,3 @@ class CombinedOpenEndedDescriptor(CombinedOpenEndedFields, RawDescriptor): non_editable_fields.extend([CombinedOpenEndedDescriptor.due, CombinedOpenEndedDescriptor.graceperiod, CombinedOpenEndedDescriptor.markdown, CombinedOpenEndedDescriptor.version]) return non_editable_fields - diff --git a/common/lib/xmodule/xmodule/conditional_module.py b/common/lib/xmodule/xmodule/conditional_module.py index 6dc86880ae..5bdc8e7797 100644 --- a/common/lib/xmodule/xmodule/conditional_module.py +++ b/common/lib/xmodule/xmodule/conditional_module.py @@ -135,7 +135,7 @@ class ConditionalModule(ConditionalFields, XModule): 'depends': ';'.join(self.required_html_ids) }) - def handle_ajax(self, dispatch, post): + def handle_ajax(self, _dispatch, _data): """This is called by courseware.moduleodule_render, to handle an AJAX call. """ diff --git a/common/lib/xmodule/xmodule/js/src/capa/display.coffee b/common/lib/xmodule/xmodule/js/src/capa/display.coffee index f773fc81c4..1f3be9e5e9 100644 --- a/common/lib/xmodule/xmodule/js/src/capa/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/capa/display.coffee @@ -138,7 +138,8 @@ class @Problem # maybe preferable to consolidate all dispatches to use FormData ### check_fd: => - Logger.log 'problem_check', @answers + # Calling check from check_fd will result in firing the 'problem_check' event twice, since it is also called in the check function. + #Logger.log 'problem_check', @answers # If there are no file inputs in the problem, we can fall back on @check if $('input:file').length == 0 diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py index 04e79ce521..e0e5c1a46f 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py @@ -27,6 +27,7 @@ class ModuleStoreTestCase(TestCase): # Remove everything except templates modulestore.collection.remove(query) + modulestore.collection.drop() @staticmethod def load_templates_if_necessary(): diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index c5ef0d751a..44e69fb0ed 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -13,11 +13,12 @@ from xmodule.templates import update_templates from .test_modulestore import check_path_to_location from . import DATA_DIR +from uuid import uuid4 HOST = 'localhost' PORT = 27017 -DB = 'test' +DB = 'test_mongo_%s' % uuid4().hex COLLECTION = 'modulestore' FS_ROOT = DATA_DIR # TODO (vshnayder): will need a real fs_root for testing load_item DEFAULT_CLASS = 'xmodule.raw_module.RawDescriptor' @@ -39,7 +40,8 @@ class TestMongoModuleStore(object): @classmethod def teardownClass(cls): - pass + cls.connection = pymongo.connection.Connection(HOST, PORT) + cls.connection.drop_database(DB) @staticmethod def initdb(): diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py index 9fc438d4c0..538901890c 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py @@ -500,10 +500,10 @@ class CombinedOpenEndedV1Module(): pass return return_html - def get_rubric(self, get): + def get_rubric(self, _data): """ Gets the results of a given grader via ajax. - Input: AJAX get dictionary + Input: AJAX data dictionary Output: Dictionary to be rendered via ajax that contains the result html. """ all_responses = [] @@ -532,10 +532,10 @@ class CombinedOpenEndedV1Module(): html = self.system.render_template('{0}/combined_open_ended_results.html'.format(self.TEMPLATE_DIR), context) return {'html': html, 'success': True} - def get_legend(self, get): + def get_legend(self, _data): """ Gets the results of a given grader via ajax. - Input: AJAX get dictionary + Input: AJAX data dictionary Output: Dictionary to be rendered via ajax that contains the result html. """ context = { @@ -544,10 +544,10 @@ class CombinedOpenEndedV1Module(): html = self.system.render_template('{0}/combined_open_ended_legend.html'.format(self.TEMPLATE_DIR), context) return {'html': html, 'success': True} - def get_results(self, get): + def get_results(self, _data): """ Gets the results of a given grader via ajax. - Input: AJAX get dictionary + Input: AJAX data dictionary Output: Dictionary to be rendered via ajax that contains the result html. """ self.update_task_states() @@ -588,19 +588,19 @@ class CombinedOpenEndedV1Module(): html = self.system.render_template('{0}/combined_open_ended_results.html'.format(self.TEMPLATE_DIR), context) return {'html': html, 'success': True} - def get_status_ajax(self, get): + def get_status_ajax(self, _data): """ Gets the results of a given grader via ajax. - Input: AJAX get dictionary + Input: AJAX data dictionary Output: Dictionary to be rendered via ajax that contains the result html. """ html = self.get_status(True) return {'html': html, 'success': True} - def handle_ajax(self, dispatch, get): + def handle_ajax(self, dispatch, data): """ This is called by courseware.module_render, to handle an AJAX call. - "get" is request.POST. + "data" is request.POST. Returns a json dictionary: { 'progress_changed' : True/False, @@ -618,30 +618,30 @@ class CombinedOpenEndedV1Module(): } if dispatch not in handlers: - return_html = self.current_task.handle_ajax(dispatch, get, self.system) + return_html = self.current_task.handle_ajax(dispatch, data, self.system) return self.update_task_states_ajax(return_html) - d = handlers[dispatch](get) + d = handlers[dispatch](data) return json.dumps(d, cls=ComplexEncoder) - def next_problem(self, get): + def next_problem(self, _data): """ Called via ajax to advance to the next problem. - Input: AJAX get request. + Input: AJAX data request. Output: Dictionary to be rendered """ self.update_task_states() return {'success': True, 'html': self.get_html_nonsystem(), 'allow_reset': self.ready_to_reset} - def reset(self, get): + def reset(self, data): """ If resetting is allowed, reset the state of the combined open ended module. - Input: AJAX get dictionary + Input: AJAX data dictionary Output: AJAX dictionary to tbe rendered """ if self.state != self.DONE: if not self.ready_to_reset: - return self.out_of_sync_error(get) + return self.out_of_sync_error(data) if self.student_attempts > self.attempts: return { @@ -789,13 +789,13 @@ class CombinedOpenEndedV1Module(): return progress_object - def out_of_sync_error(self, get, msg=''): + def out_of_sync_error(self, data, msg=''): """ return dict out-of-sync error message, and also log. """ #This is a dev_facing_error - log.warning("Combined module state out sync. state: %r, get: %r. %s", - self.state, get, msg) + log.warning("Combined module state out sync. state: %r, data: %r. %s", + self.state, data, msg) #This is a student_facing_error return {'success': False, 'error': 'The problem state got out-of-sync. Please try reloading the page.'} 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 2ac55a8318..0f0851fbf7 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 @@ -122,17 +122,17 @@ class OpenEndedModule(openendedchild.OpenEndedChild): self.payload = {'grader_payload': updated_grader_payload} - def skip_post_assessment(self, get, system): + def skip_post_assessment(self, _data, system): """ Ajax function that allows one to skip the post assessment phase - @param get: AJAX dictionary + @param data: AJAX dictionary @param system: ModuleSystem @return: Success indicator """ self.child_state = self.DONE return {'success': True} - def message_post(self, get, system): + def message_post(self, data, system): """ Handles a student message post (a reaction to the grade they received from an open ended grader type) Returns a boolean success/fail and an error message @@ -141,7 +141,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild): event_info = dict() event_info['problem_id'] = self.location_string event_info['student_id'] = system.anonymous_student_id - event_info['survey_responses'] = get + event_info['survey_responses'] = data survey_responses = event_info['survey_responses'] for tag in ['feedback', 'submission_id', 'grader_id', 'score']: @@ -587,10 +587,10 @@ class OpenEndedModule(openendedchild.OpenEndedChild): html = system.render_template('{0}/open_ended_evaluation.html'.format(self.TEMPLATE_DIR), context) return html - def handle_ajax(self, dispatch, get, system): + def handle_ajax(self, dispatch, data, system): ''' This is called by courseware.module_render, to handle an AJAX call. - "get" is request.POST. + "data" is request.POST. Returns a json dictionary: { 'progress_changed' : True/False, @@ -612,7 +612,7 @@ class OpenEndedModule(openendedchild.OpenEndedChild): return json.dumps({'error': 'Error handling action. Please try again.', 'success': False}) before = self.get_progress() - d = handlers[dispatch](get, system) + d = handlers[dispatch](data, system) after = self.get_progress() d.update({ 'progress_changed': after != before, @@ -620,20 +620,20 @@ class OpenEndedModule(openendedchild.OpenEndedChild): }) return json.dumps(d, cls=ComplexEncoder) - def check_for_score(self, get, system): + def check_for_score(self, _data, system): """ Checks to see if a score has been received yet. - @param get: AJAX get dictionary + @param data: AJAX dictionary @param system: Modulesystem (needed to align with other ajax functions) @return: Returns the current state """ state = self.child_state return {'state': state} - def save_answer(self, get, system): + def save_answer(self, data, system): """ Saves a student answer - @param get: AJAX get dictionary + @param data: AJAX dictionary @param system: modulesystem @return: Success indicator """ @@ -644,17 +644,17 @@ class OpenEndedModule(openendedchild.OpenEndedChild): return msg if self.child_state != self.INITIAL: - return self.out_of_sync_error(get) + return self.out_of_sync_error(data) # add new history element with answer and empty score and hint. - success, get = self.append_image_to_student_answer(get) + success, data = self.append_image_to_student_answer(data) error_message = "" if success: success, allowed_to_submit, error_message = self.check_if_student_can_submit() if allowed_to_submit: - get['student_answer'] = OpenEndedModule.sanitize_html(get['student_answer']) - self.new_history_entry(get['student_answer']) - self.send_to_grader(get['student_answer'], system) + 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: # Error message already defined @@ -666,17 +666,17 @@ class OpenEndedModule(openendedchild.OpenEndedChild): return { 'success': success, 'error': error_message, - 'student_response': get['student_answer'] + 'student_response': data['student_answer'] } - def update_score(self, get, system): + def update_score(self, data, system): """ Updates the current score via ajax. Called by xqueue. - Input: AJAX get dictionary, modulesystem + Input: AJAX data dictionary, modulesystem Output: None """ - queuekey = get['queuekey'] - score_msg = get['xqueue_body'] + queuekey = data['queuekey'] + score_msg = data['xqueue_body'] # TODO: Remove need for cmap self._update_score(score_msg, queuekey, system) 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 4f524d2cd7..047ab0244c 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/openendedchild.py @@ -272,13 +272,13 @@ class OpenEndedChild(object): return None return None - def out_of_sync_error(self, get, msg=''): + def out_of_sync_error(self, data, msg=''): """ return dict out-of-sync error message, and also log. """ # This is a dev_facing_error - log.warning("Open ended child state out sync. state: %r, get: %r. %s", - self.child_state, get, msg) + log.warning("Open ended child state out sync. state: %r, data: %r. %s", + self.child_state, data, msg) # This is a student_facing_error return {'success': False, 'error': 'The problem state got out-of-sync. Please try reloading the page.'} @@ -345,24 +345,24 @@ class OpenEndedChild(object): return success, image_ok, s3_public_url - def check_for_image_and_upload(self, get_data): + def check_for_image_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 get_data: AJAX get data - @return: Success, whether or not a file was in the get dictionary, + @param data: AJAX data + @return: Success, whether or not a file was in the data dictionary, and the html corresponding to the uploaded image """ has_file_to_upload = False uploaded_to_s3 = False image_tag = "" image_ok = False - if 'can_upload_files' in get_data: - if get_data['can_upload_files'] in ['true', '1']: + if 'can_upload_files' in data: + if data['can_upload_files'] in ['true', '1']: has_file_to_upload = True - file = get_data['student_file'][0] - uploaded_to_s3, image_ok, s3_public_url = self.upload_image_to_s3(file) + 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, file.name) + 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 @@ -371,27 +371,27 @@ class OpenEndedChild(object): 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 get data + @return: Boolean success, updated AJAX data """ image_template = """ {1} """.format(s3_public_url, image_name) return image_template - def append_image_to_student_answer(self, get_data): + def append_image_to_student_answer(self, data): """ Adds an image to a student answer after uploading it to S3 - @param get_data: AJAx get data - @return: Boolean success, updated AJAX get data + @param data: AJAx data + @return: Boolean success, updated AJAX data """ overall_success = False if not self.accept_file_upload: # If the question does not accept file uploads, do not do anything - return True, get_data + return True, data - has_file_to_upload, uploaded_to_s3, image_ok, image_tag = self.check_for_image_and_upload(get_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: - get_data['student_answer'] += image_tag + data['student_answer'] += image_tag overall_success = True elif has_file_to_upload and not uploaded_to_s3 and image_ok: # In this case, an image was submitted by the student, but the image could not be uploaded to S3. Likely @@ -403,12 +403,12 @@ class OpenEndedChild(object): 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, get_data['student_answer'] = self.check_for_url_in_text(get_data['student_answer']) + success, data['student_answer'] = self.check_for_url_in_text(data['student_answer']) overall_success = success # log.debug("Has file: {0} Uploaded: {1} Image Ok: {2}".format(has_file_to_upload, uploaded_to_s3, image_ok)) - return overall_success, get_data + return overall_success, data def check_for_url_in_text(self, string): """ 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 7beca7a72f..a5498289e2 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 @@ -75,10 +75,10 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild): html = system.render_template('{0}/self_assessment_prompt.html'.format(self.TEMPLATE_DIR), context) return html - def handle_ajax(self, dispatch, get, system): + def handle_ajax(self, dispatch, data, system): """ This is called by courseware.module_render, to handle an AJAX call. - "get" is request.POST. + "data" is request.POST. Returns a json dictionary: { 'progress_changed' : True/False, @@ -99,7 +99,7 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild): return json.dumps({'error': 'Error handling action. Please try again.', 'success': False}) before = self.get_progress() - d = handlers[dispatch](get, system) + d = handlers[dispatch](data, system) after = self.get_progress() d.update({ 'progress_changed': after != before, @@ -160,12 +160,12 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild): return system.render_template('{0}/self_assessment_hint.html'.format(self.TEMPLATE_DIR), context) - def save_answer(self, get, system): + def save_answer(self, data, system): """ After the answer is submitted, show the rubric. Args: - get: the GET dictionary passed to the ajax request. Should contain + data: the request dictionary passed to the ajax request. Should contain a key 'student_answer' Returns: @@ -178,16 +178,16 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild): return msg if self.child_state != self.INITIAL: - return self.out_of_sync_error(get) + return self.out_of_sync_error(data) error_message = "" # add new history element with answer and empty score and hint. - success, get = self.append_image_to_student_answer(get) + success, data = self.append_image_to_student_answer(data) if success: success, allowed_to_submit, error_message = self.check_if_student_can_submit() if allowed_to_submit: - get['student_answer'] = SelfAssessmentModule.sanitize_html(get['student_answer']) - self.new_history_entry(get['student_answer']) + data['student_answer'] = SelfAssessmentModule.sanitize_html(data['student_answer']) + self.new_history_entry(data['student_answer']) self.change_state(self.ASSESSING) else: # Error message already defined @@ -200,10 +200,10 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild): 'success': success, 'rubric_html': self.get_rubric_html(system), 'error': error_message, - 'student_response': get['student_answer'], + 'student_response': data['student_answer'], } - def save_assessment(self, get, system): + def save_assessment(self, data, _system): """ Save the assessment. If the student said they're right, don't ask for a hint, and go straight to the done state. Otherwise, do ask for a hint. @@ -219,11 +219,11 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild): """ if self.child_state != self.ASSESSING: - return self.out_of_sync_error(get) + return self.out_of_sync_error(data) try: - score = int(get['assessment']) - score_list = get.getlist('score_list[]') + score = int(data['assessment']) + score_list = data.getlist('score_list[]') for i in xrange(0, len(score_list)): score_list[i] = int(score_list[i]) except ValueError: @@ -244,7 +244,7 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild): d['state'] = self.child_state return d - def save_hint(self, get, system): + def save_hint(self, data, _system): ''' Not used currently, as hints have been removed from the system. Save the hint. @@ -258,9 +258,9 @@ class SelfAssessmentModule(openendedchild.OpenEndedChild): if self.child_state != self.POST_ASSESSMENT: # Note: because we only ask for hints on wrong answers, may not have # the same number of hints and answers. - return self.out_of_sync_error(get) + return self.out_of_sync_error(data) - self.record_latest_post_assessment(get['hint']) + self.record_latest_post_assessment(data['hint']) self.change_state(self.DONE) return {'success': True, diff --git a/common/lib/xmodule/xmodule/peer_grading_module.py b/common/lib/xmodule/xmodule/peer_grading_module.py index a13fef8e40..7df444a892 100644 --- a/common/lib/xmodule/xmodule/peer_grading_module.py +++ b/common/lib/xmodule/xmodule/peer_grading_module.py @@ -133,8 +133,8 @@ class PeerGradingModule(PeerGradingFields, XModule): """ return {'success': False, 'error': msg} - def _check_required(self, get, required): - actual = set(get.keys()) + def _check_required(self, data, required): + actual = set(data.keys()) missing = required - actual if len(missing) > 0: return False, "Missing required keys: {0}".format(', '.join(missing)) @@ -153,7 +153,7 @@ class PeerGradingModule(PeerGradingFields, XModule): else: return self.peer_grading_problem({'location': self.link_to_location})['html'] - def handle_ajax(self, dispatch, get): + def handle_ajax(self, dispatch, data): """ Needs to be implemented by child modules. Handles AJAX events. @return: @@ -173,7 +173,7 @@ class PeerGradingModule(PeerGradingFields, XModule): # This is a dev_facing_error return json.dumps({'error': 'Error handling action. Please try again.', 'success': False}) - d = handlers[dispatch](get) + d = handlers[dispatch](data) return json.dumps(d, cls=ComplexEncoder) @@ -244,7 +244,7 @@ class PeerGradingModule(PeerGradingFields, XModule): max_grade = self.max_grade return max_grade - def get_next_submission(self, get): + def get_next_submission(self, data): """ Makes a call to the grading controller for the next essay that should be graded Returns a json dict with the following keys: @@ -263,11 +263,11 @@ class PeerGradingModule(PeerGradingFields, XModule): 'error': if success is False, will have an error message with more info. """ required = set(['location']) - success, message = self._check_required(get, required) + success, message = self._check_required(data, required) if not success: return self._err_response(message) grader_id = self.system.anonymous_student_id - location = get['location'] + location = data['location'] try: response = self.peer_gs.get_next_submission(location, grader_id) @@ -280,7 +280,7 @@ class PeerGradingModule(PeerGradingFields, XModule): return {'success': False, 'error': EXTERNAL_GRADER_NO_CONTACT_ERROR} - def save_grade(self, get): + def save_grade(self, data): """ Saves the grade of a given submission. Input: @@ -298,18 +298,18 @@ class PeerGradingModule(PeerGradingFields, XModule): required = set(['location', 'submission_id', 'submission_key', 'score', 'feedback', 'rubric_scores[]', 'submission_flagged']) - success, message = self._check_required(get, required) + success, message = self._check_required(data, required) if not success: return self._err_response(message) grader_id = self.system.anonymous_student_id - location = get.get('location') - submission_id = get.get('submission_id') - score = get.get('score') - feedback = get.get('feedback') - submission_key = get.get('submission_key') - rubric_scores = get.getlist('rubric_scores[]') - submission_flagged = get.get('submission_flagged') + location = data.get('location') + submission_id = data.get('submission_id') + score = data.get('score') + feedback = data.get('feedback') + submission_key = data.get('submission_key') + rubric_scores = data.getlist('rubric_scores[]') + submission_flagged = data.get('submission_flagged') try: response = self.peer_gs.save_grade(location, grader_id, submission_id, @@ -328,7 +328,7 @@ class PeerGradingModule(PeerGradingFields, XModule): 'error': EXTERNAL_GRADER_NO_CONTACT_ERROR } - def is_student_calibrated(self, get): + def is_student_calibrated(self, data): """ Calls the grading controller to see if the given student is calibrated on the given problem @@ -347,12 +347,12 @@ class PeerGradingModule(PeerGradingFields, XModule): """ required = set(['location']) - success, message = self._check_required(get, required) + success, message = self._check_required(data, required) if not success: return self._err_response(message) grader_id = self.system.anonymous_student_id - location = get['location'] + location = data['location'] try: response = self.peer_gs.is_student_calibrated(location, grader_id) @@ -367,7 +367,7 @@ class PeerGradingModule(PeerGradingFields, XModule): 'error': EXTERNAL_GRADER_NO_CONTACT_ERROR } - def show_calibration_essay(self, get): + def show_calibration_essay(self, data): """ Fetch the next calibration essay from the grading controller and return it Inputs: @@ -392,13 +392,13 @@ class PeerGradingModule(PeerGradingFields, XModule): """ required = set(['location']) - success, message = self._check_required(get, required) + success, message = self._check_required(data, required) if not success: return self._err_response(message) grader_id = self.system.anonymous_student_id - location = get['location'] + location = data['location'] try: response = self.peer_gs.show_calibration_essay(location, grader_id) return response @@ -417,8 +417,7 @@ class PeerGradingModule(PeerGradingFields, XModule): return {'success': False, 'error': 'Error displaying submission. Please notify course staff.'} - - def save_calibration_essay(self, get): + def save_calibration_essay(self, data): """ Saves the grader's grade of a given calibration. Input: @@ -437,17 +436,17 @@ class PeerGradingModule(PeerGradingFields, XModule): """ required = set(['location', 'submission_id', 'submission_key', 'score', 'feedback', 'rubric_scores[]']) - success, message = self._check_required(get, required) + success, message = self._check_required(data, required) if not success: return self._err_response(message) grader_id = self.system.anonymous_student_id - location = get.get('location') - calibration_essay_id = get.get('submission_id') - submission_key = get.get('submission_key') - score = get.get('score') - feedback = get.get('feedback') - rubric_scores = get.getlist('rubric_scores[]') + location = data.get('location') + calibration_essay_id = data.get('submission_id') + submission_key = data.get('submission_key') + score = data.get('score') + feedback = data.get('feedback') + rubric_scores = data.getlist('rubric_scores[]') try: response = self.peer_gs.save_calibration_essay(location, grader_id, calibration_essay_id, @@ -473,8 +472,7 @@ class PeerGradingModule(PeerGradingFields, XModule): }) return html - - def peer_grading(self, get=None): + def peer_grading(self, _data=None): ''' Show a peer grading interface ''' @@ -553,11 +551,11 @@ class PeerGradingModule(PeerGradingFields, XModule): return html - def peer_grading_problem(self, get=None): + def peer_grading_problem(self, data=None): ''' Show individual problem interface ''' - if get is None or get.get('location') is None: + if data is None or data.get('location') is None: if not self.use_for_single_location: # This is an error case, because it must be set to use a single location to be called without get parameters # This is a dev_facing_error @@ -566,8 +564,8 @@ class PeerGradingModule(PeerGradingFields, XModule): return {'html': "", 'success': False} problem_location = self.link_to_location - elif get.get('location') is not None: - problem_location = get.get('location') + elif data.get('location') is not None: + problem_location = data.get('location') ajax_url = self.ajax_url html = self.system.render_template('peer_grading/peer_grading_problem.html', { @@ -617,4 +615,3 @@ class PeerGradingDescriptor(PeerGradingFields, RawDescriptor): non_editable_fields.extend([PeerGradingFields.due_date, PeerGradingFields.grace_period_string, PeerGradingFields.max_grade]) return non_editable_fields - diff --git a/common/lib/xmodule/xmodule/poll_module.py b/common/lib/xmodule/xmodule/poll_module.py index 9f2359865a..ca12f239ab 100644 --- a/common/lib/xmodule/xmodule/poll_module.py +++ b/common/lib/xmodule/xmodule/poll_module.py @@ -47,12 +47,12 @@ class PollModule(PollFields, XModule): css = {'scss': [resource_string(__name__, 'css/poll/display.scss')]} js_module_name = "Poll" - def handle_ajax(self, dispatch, get): + def handle_ajax(self, dispatch, data): """Ajax handler. Args: dispatch: string request slug - get: dict request get parameters + data: dict request data parameters Returns: json string diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 580f51f6dd..088967ebc0 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -62,10 +62,10 @@ class SequenceModule(SequenceFields, XModule): progress = reduce(Progress.add_counts, progresses) return progress - def handle_ajax(self, dispatch, get): # TODO: bounds checking + def handle_ajax(self, dispatch, data): # TODO: bounds checking ''' get = request.POST instance ''' if dispatch == 'goto_position': - self.position = int(get['position']) + self.position = int(data['position']) return json.dumps({'success': True}) raise NotFoundError('Unexpected dispatch type') diff --git a/common/lib/xmodule/xmodule/tests/test_logic.py b/common/lib/xmodule/xmodule/tests/test_logic.py index e62b9a1cee..9be533885c 100644 --- a/common/lib/xmodule/xmodule/tests/test_logic.py +++ b/common/lib/xmodule/xmodule/tests/test_logic.py @@ -40,9 +40,9 @@ class LogicTest(unittest.TestCase): self.raw_model_data ) - def ajax_request(self, dispatch, get): + def ajax_request(self, dispatch, data): """Call Xmodule.handle_ajax.""" - return json.loads(self.xmodule.handle_ajax(dispatch, get)) + return json.loads(self.xmodule.handle_ajax(dispatch, data)) class PollModuleTest(LogicTest): diff --git a/common/lib/xmodule/xmodule/tests/test_video_module.py b/common/lib/xmodule/xmodule/tests/test_video_module.py index f516e1a179..e11686176a 100644 --- a/common/lib/xmodule/xmodule/tests/test_video_module.py +++ b/common/lib/xmodule/xmodule/tests/test_video_module.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- import unittest +from xmodule.modulestore import Location from xmodule.video_module import VideoDescriptor from .test_import import DummySystem @@ -10,6 +11,33 @@ class VideoDescriptorImportTestCase(unittest.TestCase): Make sure that VideoDescriptor can import an old XML-based video correctly. """ + def test_constructor(self): + sample_xml = ''' + + ''' + location = Location(["i4x", "edX", "video", "default", + "SampleProblem1"]) + model_data = {'data': sample_xml, + 'location': location} + system = DummySystem(load_error_modules=True) + descriptor = VideoDescriptor(system, model_data) + self.assertEquals(descriptor.youtube_id_0_75, 'izygArpw-Qo') + self.assertEquals(descriptor.youtube_id_1_0, 'p2Q6BrNhdh8') + self.assertEquals(descriptor.youtube_id_1_25, '1EeWXzPdhSA') + self.assertEquals(descriptor.youtube_id_1_5, 'rABDYkeK0x8') + self.assertEquals(descriptor.show_captions, False) + self.assertEquals(descriptor.start_time, 1.0) + self.assertEquals(descriptor.end_time, 60) + self.assertEquals(descriptor.track, 'http://www.example.com/track') + self.assertEquals(descriptor.source, 'http://www.example.com/source.mp4') + def test_from_xml(self): module_system = DummySystem(load_error_modules=True) xml_data = ''' diff --git a/common/lib/xmodule/xmodule/timelimit_module.py b/common/lib/xmodule/xmodule/timelimit_module.py index 9446176f01..3f52ae0baa 100644 --- a/common/lib/xmodule/xmodule/timelimit_module.py +++ b/common/lib/xmodule/xmodule/timelimit_module.py @@ -98,7 +98,7 @@ class TimeLimitModule(TimeLimitFields, XModule): progress = reduce(Progress.add_counts, progresses) return progress - def handle_ajax(self, dispatch, get): + def handle_ajax(self, _dispatch, _data): raise NotFoundError('Unexpected dispatch type') def render(self): @@ -141,4 +141,3 @@ class TimeLimitDescriptor(TimeLimitFields, XMLEditingDescriptor, XmlDescriptor): xml_object.append( etree.fromstring(child.export_to_xml(resource_fs))) return xml_object - diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py index 6344da7994..3c6203107d 100644 --- a/common/lib/xmodule/xmodule/video_module.py +++ b/common/lib/xmodule/xmodule/video_module.py @@ -54,9 +54,9 @@ class VideoModule(VideoFields, XModule): def __init__(self, *args, **kwargs): XModule.__init__(self, *args, **kwargs) - def handle_ajax(self, dispatch, get): + def handle_ajax(self, dispatch, data): """This is not being called right now and we raise 404 error.""" - log.debug(u"GET {0}".format(get)) + log.debug(u"GET {0}".format(data)) log.debug(u"DISPATCH {0}".format(dispatch)) raise Http404() @@ -88,6 +88,13 @@ class VideoDescriptor(VideoFields, module_class = VideoModule template_dir_name = "video" + def __init__(self, *args, **kwargs): + super(VideoDescriptor, self).__init__(*args, **kwargs) + # If we don't have a `youtube_id_1_0`, this is an XML course + # and we parse out the fields. + if self.data and 'youtube_id_1_0' not in self._model_data: + _parse_video_xml(self, self.data) + @property def non_editable_metadata_fields(self): non_editable_fields = super(MetadataOnlyEditingDescriptor, self).non_editable_metadata_fields @@ -108,47 +115,54 @@ class VideoDescriptor(VideoFields, url identifiers """ video = super(VideoDescriptor, cls).from_xml(xml_data, system, org, course) - xml = etree.fromstring(xml_data) - - display_name = xml.get('display_name') - if display_name: - video.display_name = display_name - - youtube = xml.get('youtube') - if youtube: - speeds = _parse_youtube(youtube) - if speeds['0.75']: - video.youtube_id_0_75 = speeds['0.75'] - if speeds['1.00']: - video.youtube_id_1_0 = speeds['1.00'] - if speeds['1.25']: - video.youtube_id_1_25 = speeds['1.25'] - if speeds['1.50']: - video.youtube_id_1_5 = speeds['1.50'] - - show_captions = xml.get('show_captions') - if show_captions: - video.show_captions = json.loads(show_captions) - - source = _get_first_external(xml, 'source') - if source: - video.source = source - - track = _get_first_external(xml, 'track') - if track: - video.track = track - - start_time = _parse_time(xml.get('from')) - if start_time: - video.start_time = start_time - - end_time = _parse_time(xml.get('to')) - if end_time: - video.end_time = end_time - + _parse_video_xml(video, xml_data) return video +def _parse_video_xml(video, xml_data): + """ + Parse video fields out of xml_data. The fields are set if they are + present in the XML. + """ + xml = etree.fromstring(xml_data) + + display_name = xml.get('display_name') + if display_name: + video.display_name = display_name + + youtube = xml.get('youtube') + if youtube: + speeds = _parse_youtube(youtube) + if speeds['0.75']: + video.youtube_id_0_75 = speeds['0.75'] + if speeds['1.00']: + video.youtube_id_1_0 = speeds['1.00'] + if speeds['1.25']: + video.youtube_id_1_25 = speeds['1.25'] + if speeds['1.50']: + video.youtube_id_1_5 = speeds['1.50'] + + show_captions = xml.get('show_captions') + if show_captions: + video.show_captions = json.loads(show_captions) + + source = _get_first_external(xml, 'source') + if source: + video.source = source + + track = _get_first_external(xml, 'track') + if track: + video.track = track + + start_time = _parse_time(xml.get('from')) + if start_time: + video.start_time = start_time + + end_time = _parse_time(xml.get('to')) + if end_time: + video.end_time = end_time + + def _get_first_external(xmltree, tag): """ Returns the src attribute of the nested `tag` in `xmltree`, if it diff --git a/common/lib/xmodule/xmodule/videoalpha_module.py b/common/lib/xmodule/xmodule/videoalpha_module.py index a64e094a58..6b27bcda2b 100644 --- a/common/lib/xmodule/xmodule/videoalpha_module.py +++ b/common/lib/xmodule/xmodule/videoalpha_module.py @@ -125,9 +125,9 @@ class VideoAlphaModule(VideoAlphaFields, XModule): return parse_time(xmltree.get('start_time')), parse_time(xmltree.get('end_time')) - def handle_ajax(self, dispatch, get): + def handle_ajax(self, dispatch, data): """This is not being called right now and we raise 404 error.""" - log.debug(u"GET {0}".format(get)) + log.debug(u"GET {0}".format(data)) log.debug(u"DISPATCH {0}".format(dispatch)) raise Http404() diff --git a/common/lib/xmodule/xmodule/word_cloud_module.py b/common/lib/xmodule/xmodule/word_cloud_module.py index ac5b3051de..a7f3f92795 100644 --- a/common/lib/xmodule/xmodule/word_cloud_module.py +++ b/common/lib/xmodule/xmodule/word_cloud_module.py @@ -168,12 +168,12 @@ class WordCloudModule(WordCloudFields, XModule): )[:amount] ) - def handle_ajax(self, dispatch, post): + def handle_ajax(self, dispatch, data): """Ajax handler. Args: dispatch: string request slug - post: dict request get parameters + data: dict request get parameters Returns: json string @@ -187,7 +187,7 @@ class WordCloudModule(WordCloudFields, XModule): # Student words from client. # FIXME: we must use raw JSON, not a post data (multipart/form-data) - raw_student_words = post.getlist('student_words[]') + raw_student_words = data.getlist('student_words[]') student_words = filter(None, map(self.good_word, raw_student_words)) self.student_words = student_words diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index f5705bf662..0f5bbf4f2e 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -272,9 +272,9 @@ class XModule(XModuleFields, HTMLSnippet, XBlock): ''' return None - def handle_ajax(self, _dispatch, _get): + def handle_ajax(self, _dispatch, _data): ''' dispatch is last part of the URL. - get is a dictionary-like object ''' + data is a dictionary-like object with the content of the request''' return "" diff --git a/common/static/coffee/spec/logger_spec.coffee b/common/static/coffee/spec/logger_spec.coffee index 8866daa570..7fe734d8b5 100644 --- a/common/static/coffee/spec/logger_spec.coffee +++ b/common/static/coffee/spec/logger_spec.coffee @@ -3,10 +3,15 @@ describe 'Logger', -> expect(window.log_event).toBe Logger.log describe 'log', -> - it 'sends an event to Segment.io, if the event is whitelisted', -> + it 'sends an event to Segment.io, if the event is whitelisted and the data is not a dictionary', -> spyOn(analytics, 'track') Logger.log 'seq_goto', 'data' - expect(analytics.track).toHaveBeenCalledWith 'seq_goto', 'data' + expect(analytics.track).toHaveBeenCalledWith 'seq_goto', value: 'data' + + it 'sends an event to Segment.io, if the event is whitelisted and the data is a dictionary', -> + spyOn(analytics, 'track') + Logger.log 'seq_goto', value: 'data' + expect(analytics.track).toHaveBeenCalledWith 'seq_goto', value: 'data' it 'send a request to log event', -> spyOn $, 'getWithPrefix' diff --git a/common/static/coffee/src/logger.coffee b/common/static/coffee/src/logger.coffee index 6da4929fb0..f2dfef5132 100644 --- a/common/static/coffee/src/logger.coffee +++ b/common/static/coffee/src/logger.coffee @@ -1,11 +1,15 @@ class @Logger # events we want sent to Segment.io for tracking - SEGMENT_IO_WHITELIST = ["seq_goto", "seq_next", "seq_prev"] + SEGMENT_IO_WHITELIST = ["seq_goto", "seq_next", "seq_prev", "problem_check", "problem_reset", "problem_show", "problem_save"] @log: (event_type, data) -> + # Segment.io event tracking if event_type in SEGMENT_IO_WHITELIST - # Segment.io event tracking - analytics.track event_type, data + # to avoid changing the format of data sent to our servers, we only massage it here + if typeof data isnt 'object' or data is null + analytics.track event_type, value: data + else + analytics.track event_type, data $.getWithPrefix '/event', event_type: event_type diff --git a/jenkins/test.sh b/jenkins/test.sh index c7728ab367..05f978e32f 100755 --- a/jenkins/test.sh +++ b/jenkins/test.sh @@ -60,9 +60,6 @@ fi export PIP_DOWNLOAD_CACHE=/mnt/pip-cache -# Allow django liveserver tests to use a range of ports -export DJANGO_LIVE_TEST_SERVER_ADDRESS=${DJANGO_LIVE_TEST_SERVER_ADDRESS-localhost:8000-9000} - source /mnt/virtualenvs/"$JOB_NAME"/bin/activate bundle install diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 3ffb1d1b1d..d17efa3697 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -2,8 +2,6 @@ import json import logging import re import sys -import static_replace - from functools import partial from django.conf import settings @@ -15,27 +13,31 @@ from django.http import Http404 from django.http import HttpResponse, HttpResponseBadRequest from django.views.decorators.csrf import csrf_exempt +import pyparsing from requests.auth import HTTPBasicAuth +from statsd import statsd from capa.xqueue_interface import XQueueInterface -from courseware.masquerade import setup_masquerade -from courseware.access import has_access from mitxmako.shortcuts import render_to_string -from .models import StudentModule -from psychometrics.psychoanalyze import make_psychometrics_data_update_handler -from student.models import unique_id_for_user +from xblock.runtime import DbModel +from xmodule.error_module import ErrorDescriptor, NonStaffErrorDescriptor from xmodule.errortracker import exc_info_to_str from xmodule.exceptions import NotFoundError, ProcessingError from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore -from xmodule.x_module import ModuleSystem -from xmodule.error_module import ErrorDescriptor, NonStaffErrorDescriptor -from xblock.runtime import DbModel -from xmodule_modifiers import replace_course_urls, replace_static_urls, add_histogram, wrap_xmodule -from .model_data import LmsKeyValueStore, LmsUsage, ModelDataCache - from xmodule.modulestore.exceptions import ItemNotFoundError -from statsd import statsd +from xmodule.x_module import ModuleSystem +from xmodule_modifiers import replace_course_urls, replace_static_urls, add_histogram, wrap_xmodule + +import static_replace +from psychometrics.psychoanalyze import make_psychometrics_data_update_handler +from student.models import unique_id_for_user + +from courseware.access import has_access +from courseware.masquerade import setup_masquerade +from courseware.model_data import LmsKeyValueStore, LmsUsage, ModelDataCache +from courseware.models import StudentModule + log = logging.getLogger(__name__) @@ -221,7 +223,7 @@ def get_module_for_descriptor_internal(user, descriptor, model_data_cache, cours relative_xqueue_callback_url = reverse('xqueue_callback', kwargs=dict(course_id=course_id, userid=str(user.id), - id=descriptor.location.url(), + mod_id=descriptor.location.url(), dispatch=dispatch), ) return xqueue_callback_url_prefix + relative_xqueue_callback_url @@ -397,40 +399,47 @@ def get_module_for_descriptor_internal(user, descriptor, model_data_cache, cours @csrf_exempt -def xqueue_callback(request, course_id, userid, id, dispatch): +def xqueue_callback(request, course_id, userid, mod_id, dispatch): ''' Entry point for graded results from the queueing system. ''' + data = request.POST.copy() + # Test xqueue package, which we expect to be: # xpackage = {'xqueue_header': json.dumps({'lms_key':'secretkey',...}), # 'xqueue_body' : 'Message from grader'} - get = request.POST.copy() for key in ['xqueue_header', 'xqueue_body']: - if not get.has_key(key): + if key not in data: raise Http404 - header = json.loads(get['xqueue_header']) - if not isinstance(header, dict) or not header.has_key('lms_key'): + + header = json.loads(data['xqueue_header']) + if not isinstance(header, dict) or 'lms_key' not in header: raise Http404 # Retrieve target StudentModule user = User.objects.get(id=userid) - - model_data_cache = ModelDataCache.cache_for_descriptor_descendents(course_id, - user, modulestore().get_instance(course_id, id), depth=0, select_for_update=True) - instance = get_module(user, request, id, model_data_cache, course_id, grade_bucket_type='xqueue') + model_data_cache = ModelDataCache.cache_for_descriptor_descendents( + course_id, + user, + modulestore().get_instance(course_id, mod_id), + depth=0, + select_for_update=True + ) + instance = get_module(user, request, mod_id, model_data_cache, course_id, grade_bucket_type='xqueue') if instance is None: - log.debug("No module {0} for user {1}--access denied?".format(id, user)) + msg = "No module {0} for user {1}--access denied?".format(mod_id, user) + log.debug(msg) raise Http404 - # Transfer 'queuekey' from xqueue response header to 'get'. This is required to - # use the interface defined by 'handle_ajax' - get.update({'queuekey': header['lms_key']}) + # Transfer 'queuekey' from xqueue response header to the data. + # This is required to use the interface defined by 'handle_ajax' + data.update({'queuekey': header['lms_key']}) # We go through the "AJAX" path - # So far, the only dispatch from xqueue will be 'score_update' + # So far, the only dispatch from xqueue will be 'score_update' try: # Can ignore the return value--not used for xqueue_callback - instance.handle_ajax(dispatch, get) + instance.handle_ajax(dispatch, data) except: log.exception("error processing ajax call") raise @@ -464,23 +473,15 @@ def modx_dispatch(request, dispatch, location, course_id): if not request.user.is_authenticated(): raise PermissionDenied - # Check for submitted files and basic file size checks - p = request.POST.copy() - if request.FILES: - for fileinput_id in request.FILES.keys(): - inputfiles = request.FILES.getlist(fileinput_id) + # Get the submitted data + data = request.POST.copy() - if len(inputfiles) > settings.MAX_FILEUPLOADS_PER_INPUT: - too_many_files_msg = 'Submission aborted! Maximum %d files may be submitted at once' % \ - settings.MAX_FILEUPLOADS_PER_INPUT - return HttpResponse(json.dumps({'success': too_many_files_msg})) - - for inputfile in inputfiles: - if inputfile.size > settings.STUDENT_FILEUPLOAD_MAX_SIZE: # Bytes - file_too_big_msg = 'Submission aborted! Your file "%s" is too large (max size: %d MB)' % \ - (inputfile.name, settings.STUDENT_FILEUPLOAD_MAX_SIZE / (1000 ** 2)) - return HttpResponse(json.dumps({'success': file_too_big_msg})) - p[fileinput_id] = inputfiles + # Get and check submitted files + files = request.FILES or {} + error_msg = _check_files_limits(files) + if error_msg: + return HttpResponse(json.dumps({'success': error_msg})) + data.update(files) # Merge files into data dictionary try: descriptor = modulestore().get_instance(course_id, location) @@ -493,8 +494,11 @@ def modx_dispatch(request, dispatch, location, course_id): ) raise Http404 - model_data_cache = ModelDataCache.cache_for_descriptor_descendents(course_id, - request.user, descriptor) + model_data_cache = ModelDataCache.cache_for_descriptor_descendents( + course_id, + request.user, + descriptor + ) instance = get_module(request.user, request, location, model_data_cache, course_id, grade_bucket_type='ajax') if instance is None: @@ -505,7 +509,7 @@ def modx_dispatch(request, dispatch, location, course_id): # Let the module handle the AJAX try: - ajax_return = instance.handle_ajax(dispatch, p) + ajax_return = instance.handle_ajax(dispatch, data) # If we can't find the module, respond with a 404 except NotFoundError: @@ -527,7 +531,6 @@ def modx_dispatch(request, dispatch, location, course_id): return HttpResponse(ajax_return) - def get_score_bucket(grade, max_grade): """ Function to split arbitrary score ranges into 3 buckets. @@ -540,3 +543,30 @@ def get_score_bucket(grade, max_grade): score_bucket = "correct" return score_bucket + + +def _check_files_limits(files): + """ + Check if the files in a request are under the limits defined by + `settings.MAX_FILEUPLOADS_PER_INPUT` and + `settings.STUDENT_FILEUPLOAD_MAX_SIZE`. + + Returns None if files are correct or an error messages otherwise. + """ + for fileinput_id in files.keys(): + inputfiles = files.getlist(fileinput_id) + + # Check number of files submitted + if len(inputfiles) > settings.MAX_FILEUPLOADS_PER_INPUT: + msg = 'Submission aborted! Maximum %d files may be submitted at once' %\ + settings.MAX_FILEUPLOADS_PER_INPUT + return msg + + # Check file sizes + for inputfile in inputfiles: + if inputfile.size > settings.STUDENT_FILEUPLOAD_MAX_SIZE: # Bytes + msg = 'Submission aborted! Your file "%s" is too large (max size: %d MB)' %\ + (inputfile.name, settings.STUDENT_FILEUPLOAD_MAX_SIZE / (1000 ** 2)) + return msg + + return None diff --git a/lms/templates/registration/password_reset_email.html b/lms/templates/registration/password_reset_email.html index bf6c3e0891..68073d9ddd 100644 --- a/lms/templates/registration/password_reset_email.html +++ b/lms/templates/registration/password_reset_email.html @@ -3,7 +3,7 @@ {% trans "Please go to the following page and choose a new password:" %} {% block reset_link %} -https://{{domain}}{% url 'django.contrib.auth.views.password_reset_confirm' uidb36=uid token=token %} +https://{{domain}}{% url 'student.views.password_reset_confirm_wrapper' uidb36=uid token=token %} {% endblock %} If you didn't request this change, you can disregard this email - we have not yet reset your password. diff --git a/lms/urls.py b/lms/urls.py index 52ce539f73..52a7d99aaf 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -51,7 +51,7 @@ urlpatterns = ('', # nopep8 url(r'^password_change_done/$', django.contrib.auth.views.password_change_done, name='auth_password_change_done'), url(r'^password_reset_confirm/(?P[0-9A-Za-z]+)-(?P.+)/$', - django.contrib.auth.views.password_reset_confirm, + 'student.views.password_reset_confirm_wrapper', name='auth_password_reset_confirm'), url(r'^password_reset_complete/$', django.contrib.auth.views.password_reset_complete, name='auth_password_reset_complete'), @@ -188,7 +188,7 @@ if settings.COURSEWARE_ENABLED: # into the database. url(r'^software-licenses$', 'licenses.views.user_software_license', name="user_software_license"), - url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/xqueue/(?P[^/]*)/(?P.*?)/(?P[^/]*)$', + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/xqueue/(?P[^/]*)/(?P.*?)/(?P[^/]*)$', 'courseware.module_render.xqueue_callback', name='xqueue_callback'), url(r'^change_setting$', 'student.views.change_setting', @@ -438,5 +438,3 @@ if settings.DEBUG: #Custom error pages handler404 = 'static_template_view.views.render_404' handler500 = 'static_template_view.views.render_500' - - diff --git a/rakelib/tests.rake b/rakelib/tests.rake index 3cb5e8f4e5..0ca7c5c1e9 100644 --- a/rakelib/tests.rake +++ b/rakelib/tests.rake @@ -16,7 +16,7 @@ def run_tests(system, report_dir, test_id=nil, stop_on_failure=true) ENV['NOSE_XUNIT_FILE'] = File.join(report_dir, "nosetests.xml") 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', test_id) + cmd = django_admin(system, :test, 'test', '--logging-clear-handlers', '--liveserver=localhost:8000-9000', test_id) test_sh(run_under_coverage(cmd, system)) end diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 5ce748e7b5..f64568dc10 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -10,4 +10,4 @@ # Our libraries: -e git+https://github.com/edx/XBlock.git@4d8735e883#egg=XBlock -e git+https://github.com/edx/codejail.git@0a1b468#egg=codejail --e git+https://github.com/edx/diff-cover.git@v0.1.2#egg=diff_cover +-e git+https://github.com/edx/diff-cover.git@v0.1.3#egg=diff_cover diff --git a/scripts/create-dev-env.sh b/scripts/create-dev-env.sh index edb0bcdcae..0816b72d21 100755 --- a/scripts/create-dev-env.sh +++ b/scripts/create-dev-env.sh @@ -98,19 +98,23 @@ clone_repos() { set_base_default() { # if PROJECT_HOME not set # 2 possibilities: this is from cloned repo, or not - # this script is in "./scripts" if a git clone - this_repo=$(cd "${BASH_SOURCE%/*}/.." && pwd) - if [[ "${this_repo##*/}" = "edx-platform" && -d "$this_repo/.git" ]]; then - # set BASE one-up from this_repo; - echo "${this_repo%/*}" + + # See if remote's url is named edx-platform (this works for forks too, but + # not if the name was changed). + cd "$( dirname "${BASH_SOURCE[0]}" )" + this_repo=$(basename $(git ls-remote --get-url 2>/dev/null) 2>/dev/null) || + echo -n "" + + if [[ "x$this_repo" = "xedx-platform.git" ]]; then + # We are in the edx repo and already have git installed. Let git do the + # work of finding base dir: + echo "$(dirname $(git rev-parse --show-toplevel))" else echo "$HOME/edx_all" fi } - - ### START PROG=${0##*/}