diff --git a/cms/djangoapps/auth/tests/__init__.py b/cms/djangoapps/auth/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/cms/djangoapps/auth/tests/test_authz.py b/cms/djangoapps/auth/tests/test_authz.py index 6bbb8d0a41..cf0337628e 100644 --- a/cms/djangoapps/auth/tests/test_authz.py +++ b/cms/djangoapps/auth/tests/test_authz.py @@ -5,6 +5,7 @@ import mock from django.test import TestCase from django.contrib.auth.models import User +from xmodule.modulestore import Location 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,\ @@ -129,7 +130,7 @@ class CourseGroupTest(TestCase): """ 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' + self.location = Location('i4x', 'mitX', '101', 'course', 'test') def test_add_user_to_course_group(self): """ @@ -181,7 +182,7 @@ class CourseGroupTest(TestCase): create_all_course_groups(self.creator, self.location) add_user_to_course_group(self.creator, self.staff, self.location, STAFF_ROLE_NAME) - location2 = 'i4x', 'mitX', '103', 'course', 'test2' + location2 = Location('i4x', 'mitX', '103', 'course', 'test2') staff2 = User.objects.create_user('teststaff2', 'teststaff2+courses@edx.org', 'foo') create_all_course_groups(self.creator, location2) add_user_to_course_group(self.creator, staff2, location2, STAFF_ROLE_NAME) @@ -193,7 +194,7 @@ class CourseGroupTest(TestCase): create_all_course_groups(self.creator, self.location) add_user_to_course_group(self.creator, self.staff, self.location, STAFF_ROLE_NAME) - location2 = 'i4x', 'mitX', '103', 'course', 'test2' + location2 = Location('i4x', 'mitX', '103', 'course', 'test2') creator2 = User.objects.create_user('testcreator2', 'testcreator2+courses@edx.org', 'foo') staff2 = User.objects.create_user('teststaff2', 'teststaff2+courses@edx.org', 'foo') create_all_course_groups(creator2, location2) diff --git a/cms/djangoapps/contentstore/management/commands/check_course.py b/cms/djangoapps/contentstore/management/commands/check_course.py index 541f5dee75..c3f1b97a5e 100644 --- a/cms/djangoapps/contentstore/management/commands/check_course.py +++ b/cms/djangoapps/contentstore/management/commands/check_course.py @@ -2,6 +2,7 @@ from django.core.management.base import BaseCommand, CommandError from xmodule.modulestore.django import modulestore from xmodule.modulestore.xml_importer import check_module_metadata_editability from xmodule.course_module import CourseDescriptor +from xmodule.modulestore import Location class Command(BaseCommand): @@ -54,8 +55,16 @@ class Command(BaseCommand): discussion_items = _get_discussion_items(course) # now query all discussion items via get_items() and compare with the tree-traversal - queried_discussion_items = store.get_items(['i4x', course.location.org, course.location.course, - 'discussion', None, None]) + queried_discussion_items = store.get_items( + Location( + 'i4x', + course.location.org, + course.location.course, + 'discussion', + None, + None + ) + ) for item in queried_discussion_items: if item.location.url() not in discussion_items: diff --git a/cms/djangoapps/contentstore/tests/test_contentstore.py b/cms/djangoapps/contentstore/tests/test_contentstore.py index f1b230bb05..563c05affb 100644 --- a/cms/djangoapps/contentstore/tests/test_contentstore.py +++ b/cms/djangoapps/contentstore/tests/test_contentstore.py @@ -202,20 +202,20 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): draft_store = modulestore('draft') import_from_xml(store, 'common/test/data/', ['simple']) - html_module = draft_store.get_item(['i4x', 'edX', 'simple', 'html', 'test_html', None]) + html_module = draft_store.get_item(Location('i4x', 'edX', 'simple', 'html', 'test_html', None)) draft_store.convert_to_draft(html_module.location) # now query get_items() to get this location with revision=None, this should just # return back a single item (not 2) - items = store.get_items(['i4x', 'edX', 'simple', 'html', 'test_html', None]) + items = store.get_items(Location('i4x', 'edX', 'simple', 'html', 'test_html', None)) self.assertEqual(len(items), 1) self.assertFalse(getattr(items[0], 'is_draft', False)) # now refetch from the draft store. Note that even though we pass # None in the revision field, the draft store will replace that with 'draft' - items = draft_store.get_items(['i4x', 'edX', 'simple', 'html', 'test_html', None]) + items = draft_store.get_items(Location('i4x', 'edX', 'simple', 'html', 'test_html', None)) self.assertEqual(len(items), 1) self.assertTrue(getattr(items[0], 'is_draft', False)) @@ -229,9 +229,9 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): draft_store = modulestore('draft') import_from_xml(store, 'common/test/data/', ['simple']) - course = draft_store.get_item(Location(['i4x', 'edX', 'simple', - 'course', '2012_Fall', None]), depth=None) - html_module = draft_store.get_item(['i4x', 'edX', 'simple', 'html', 'test_html', None]) + course = draft_store.get_item(Location('i4x', 'edX', 'simple', + 'course', '2012_Fall', None), depth=None) + html_module = draft_store.get_item(Location('i4x', 'edX', 'simple', 'html', 'test_html', None)) self.assertEqual(html_module.graceperiod, course.graceperiod) self.assertNotIn('graceperiod', own_metadata(html_module)) @@ -239,7 +239,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): draft_store.convert_to_draft(html_module.location) # refetch to check metadata - html_module = draft_store.get_item(['i4x', 'edX', 'simple', 'html', 'test_html', None]) + html_module = draft_store.get_item(Location('i4x', 'edX', 'simple', 'html', 'test_html', None)) self.assertEqual(html_module.graceperiod, course.graceperiod) self.assertNotIn('graceperiod', own_metadata(html_module)) @@ -248,14 +248,14 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): draft_store.publish(html_module.location, 0) # refetch to check metadata - html_module = draft_store.get_item(['i4x', 'edX', 'simple', 'html', 'test_html', None]) + html_module = draft_store.get_item(Location('i4x', 'edX', 'simple', 'html', 'test_html', None)) self.assertEqual(html_module.graceperiod, course.graceperiod) self.assertNotIn('graceperiod', own_metadata(html_module)) # put back in draft and change metadata and see if it's now marked as 'own_metadata' draft_store.convert_to_draft(html_module.location) - html_module = draft_store.get_item(['i4x', 'edX', 'simple', 'html', 'test_html', None]) + html_module = draft_store.get_item(Location('i4x', 'edX', 'simple', 'html', 'test_html', None)) new_graceperiod = timedelta(hours=1) @@ -270,7 +270,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): draft_store.update_metadata(html_module.location, own_metadata(html_module)) # read back to make sure it reads as 'own-metadata' - html_module = draft_store.get_item(['i4x', 'edX', 'simple', 'html', 'test_html', None]) + html_module = draft_store.get_item(Location('i4x', 'edX', 'simple', 'html', 'test_html', None)) self.assertIn('graceperiod', own_metadata(html_module)) self.assertEqual(html_module.graceperiod, new_graceperiod) @@ -280,7 +280,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): # and re-read and verify 'own-metadata' draft_store.convert_to_draft(html_module.location) - html_module = draft_store.get_item(['i4x', 'edX', 'simple', 'html', 'test_html', None]) + html_module = draft_store.get_item(Location('i4x', 'edX', 'simple', 'html', 'test_html', None)) self.assertIn('graceperiod', own_metadata(html_module)) self.assertEqual(html_module.graceperiod, new_graceperiod) @@ -289,7 +289,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): import_from_xml(modulestore('direct'), 'common/test/data/', ['simple']) course = modulestore('draft').get_item( - Location(['i4x', 'edX', 'simple', 'course', '2012_Fall', None]), + Location('i4x', 'edX', 'simple', 'course', '2012_Fall', None), depth=None ) @@ -298,7 +298,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): self.assertEqual(num_drafts, 0) problem = modulestore('draft').get_item( - Location(['i4x', 'edX', 'simple', 'problem', 'ps01-simple', None]) + Location('i4x', 'edX', 'simple', 'problem', 'ps01-simple', None) ) # put into draft @@ -306,13 +306,13 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): # make sure we can query that item and verify that it is a draft draft_problem = modulestore('draft').get_item( - Location(['i4x', 'edX', 'simple', 'problem', 'ps01-simple', None]) + Location('i4x', 'edX', 'simple', 'problem', 'ps01-simple', None) ) self.assertTrue(getattr(draft_problem, 'is_draft', False)) # now requery with depth course = modulestore('draft').get_item( - Location(['i4x', 'edX', 'simple', 'course', '2012_Fall', None]), + Location('i4x', 'edX', 'simple', 'course', '2012_Fall', None), depth=None ) @@ -324,10 +324,10 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): module_store = modulestore('direct') import_from_xml(module_store, 'common/test/data/', ['toy']) - handouts = module_store.get_item(Location(['i4x', 'edX', 'toy', 'course_info', 'handouts', None])) + handouts = module_store.get_item(Location('i4x', 'edX', 'toy', 'course_info', 'handouts', None)) self.assertIn('/static/', handouts.data) - handouts = module_store.get_item(Location(['i4x', 'edX', 'toy', 'html', 'toyhtml', None])) + handouts = module_store.get_item(Location('i4x', 'edX', 'toy', 'html', 'toyhtml', None)) self.assertIn('/static/', handouts.data) @mock.patch('xmodule.course_module.requests.get') @@ -341,14 +341,14 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): module_store = modulestore('direct') import_from_xml(module_store, 'common/test/data/', ['toy']) - course = module_store.get_item(Location(['i4x', 'edX', 'toy', 'course', '2012_Fall', None])) + course = module_store.get_item(Location('i4x', 'edX', 'toy', 'course', '2012_Fall', None)) self.assertGreater(len(course.textbooks), 0) def test_default_tabs_on_create_course(self): module_store = modulestore('direct') CourseFactory.create(org='edX', course='999', display_name='Robot Super Course') - course_location = Location(['i4x', 'edX', '999', 'course', 'Robot_Super_Course', None]) + course_location = Location('i4x', 'edX', '999', 'course', 'Robot_Super_Course', None) course = module_store.get_item(course_location) @@ -365,7 +365,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): def test_create_static_tab_and_rename(self): module_store = modulestore('direct') CourseFactory.create(org='edX', course='999', display_name='Robot Super Course') - course_location = Location(['i4x', 'edX', '999', 'course', 'Robot_Super_Course', None]) + course_location = Location('i4x', 'edX', '999', 'course', 'Robot_Super_Course', None) item = ItemFactory.create(parent_location=course_location, category='static_tab', display_name="My Tab") @@ -455,7 +455,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): """ Creates two static tabs in a dummy course. """ module_store = modulestore('direct') CourseFactory.create(org='edX', course='999', display_name='Robot Super Course') - course_location = Location(['i4x', 'edX', '999', 'course', 'Robot_Super_Course', None]) + course_location = Location('i4x', 'edX', '999', 'course', 'Robot_Super_Course', None) new_location = loc_mapper().translate_location(course_location.course_id, course_location, False, True) ItemFactory.create( @@ -473,7 +473,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): module_store = modulestore('direct') import_from_xml(module_store, 'common/test/data/', ['toy']) - items = module_store.get_items(['i4x', 'edX', 'toy', 'poll_question', None, None]) + items = module_store.get_items(Location('i4x', 'edX', 'toy', 'poll_question', None, None)) found = len(items) > 0 self.assertTrue(found) @@ -489,7 +489,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): """ Tests the ajax callback to render an XModule """ - resp = self._test_preview(Location(['i4x', 'edX', 'toy', 'vertical', 'vertical_test', None])) + resp = self._test_preview(Location('i4x', 'edX', 'toy', 'vertical', 'vertical_test', None)) # These are the data-ids of the xblocks contained in the vertical. # Ultimately, these must be converted to new locators. self.assertContains(resp, 'i4x://edX/toy/video/sample_video') @@ -501,7 +501,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): """ This verifies that a video caption url is as we expect it to be """ - resp = self._test_preview(Location(['i4x', 'edX', 'toy', 'video', 'sample_video', None])) + resp = self._test_preview(Location('i4x', 'edX', 'toy', 'video', 'sample_video', None)) self.assertContains(resp, 'data-caption-asset-path="/c4x/edX/toy/asset/subs_"') def _test_preview(self, location): @@ -522,13 +522,13 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): def test_delete(self): direct_store = modulestore('direct') CourseFactory.create(org='edX', course='999', display_name='Robot Super Course') - course_location = Location(['i4x', 'edX', '999', 'course', 'Robot_Super_Course', None]) + course_location = Location('i4x', 'edX', '999', 'course', 'Robot_Super_Course', None) chapterloc = ItemFactory.create(parent_location=course_location, display_name="Chapter").location ItemFactory.create(parent_location=chapterloc, category='sequential', display_name="Sequential") - sequential = direct_store.get_item(Location(['i4x', 'edX', '999', 'sequential', 'Sequential', None])) - chapter = direct_store.get_item(Location(['i4x', 'edX', '999', 'chapter', 'Chapter', None])) + sequential = direct_store.get_item(Location('i4x', 'edX', '999', 'sequential', 'Sequential', None)) + chapter = direct_store.get_item(Location('i4x', 'edX', '999', 'chapter', 'Chapter', None)) # make sure the parent points to the child object which is to be deleted self.assertTrue(sequential.location.url() in chapter.children) @@ -1124,7 +1124,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): # create a new video module and add it as a child to a vertical # this re-creates a bug whereby since the video template doesn't have # anything in 'data' field, the export was blowing up - verticals = module_store.get_items(['i4x', 'edX', 'toy', 'vertical', None, None]) + verticals = module_store.get_items(Location('i4x', 'edX', 'toy', 'vertical', None, None)) self.assertGreater(len(verticals), 0) @@ -1152,7 +1152,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): import_from_xml(module_store, 'common/test/data/', ['word_cloud']) location = CourseDescriptor.id_to_location('HarvardX/ER22x/2013_Spring') - verticals = module_store.get_items(['i4x', 'HarvardX', 'ER22x', 'vertical', None, None]) + verticals = module_store.get_items(Location('i4x', 'HarvardX', 'ER22x', 'vertical', None, None)) self.assertGreater(len(verticals), 0) @@ -1181,7 +1181,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): import_from_xml(module_store, 'common/test/data/', ['toy']) location = CourseDescriptor.id_to_location('edX/toy/2012_Fall') - verticals = module_store.get_items(['i4x', 'edX', 'toy', 'vertical', None, None]) + verticals = module_store.get_items(Location('i4x', 'edX', 'toy', 'vertical', None, None)) self.assertGreater(len(verticals), 0) @@ -1198,7 +1198,7 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): # Reimport and get the video back import_from_xml(module_store, root_dir) - imported_word_cloud = module_store.get_item(Location(['i4x', 'edX', 'toy', 'word_cloud', 'untitled', None])) + imported_word_cloud = module_store.get_item(Location('i4x', 'edX', 'toy', 'word_cloud', 'untitled', None)) # It should now contain empty data self.assertEquals(imported_word_cloud.data, '') @@ -1224,14 +1224,14 @@ class ContentStoreToyCourseTest(ModuleStoreTestCase): # get the sample HTML with styling information html_module = module_store.get_instance( 'edX/toy/2012_Fall', - Location(['i4x', 'edX', 'toy', 'html', 'with_styling']) + Location('i4x', 'edX', 'toy', 'html', 'with_styling') ) self.assertIn('

', html_module.data) # get the sample HTML with just a simple tag information html_module = module_store.get_instance( 'edX/toy/2012_Fall', - Location(['i4x', 'edX', 'toy', 'html', 'just_img']) + Location('i4x', 'edX', 'toy', 'html', 'just_img') ) self.assertIn('', html_module.data) @@ -1790,7 +1790,7 @@ class ContentStoreTest(ModuleStoreTestCase): course = module_store.get_item(Location(['i4x', 'edX', 'toy', 'course', '2012_Fall', None])) - verticals = module_store.get_items(['i4x', 'edX', 'toy', 'vertical', None, None]) + verticals = module_store.get_items(Location('i4x', 'edX', 'toy', 'vertical', None, None)) # let's assert on the metadata_inheritance on an existing vertical for vertical in verticals: diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index 514fd80278..41ac02c75d 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -1,21 +1,14 @@ """ Tests for utils. """ from contentstore import utils import mock -import unittest import collections import copy -import json -from uuid import uuid4 from django.test import TestCase from xmodule.modulestore.tests.factories import CourseFactory from django.test.utils import override_settings -from xmodule.modulestore.tests.factories import CourseFactory -from xmodule.contentstore.content import StaticContent -from xmodule.contentstore.django import contentstore -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.exceptions import NotFoundError +from xmodule.modulestore import Location class LMSLinksTestCase(TestCase): @@ -64,12 +57,12 @@ class LMSLinksTestCase(TestCase): def get_about_page_link(self): """ create mock course and return the about page link """ - location = 'i4x', 'mitX', '101', 'course', 'test' + location = Location('i4x', 'mitX', '101', 'course', 'test') return utils.get_lms_link_for_about_page(location) def lms_link_test(self): """ Tests get_lms_link_for_item. """ - location = 'i4x', 'mitX', '101', 'vertical', 'contacting_us' + location = Location('i4x', 'mitX', '101', 'vertical', 'contacting_us') link = utils.get_lms_link_for_item(location, False, "mitX/101/test") self.assertEquals(link, "//localhost:8000/courses/mitX/101/test/jump_to/i4x://mitX/101/vertical/contacting_us") link = utils.get_lms_link_for_item(location, True, "mitX/101/test") @@ -80,7 +73,7 @@ class LMSLinksTestCase(TestCase): # If no course_id is passed in, it is obtained from the location. This is the case for # Studio dashboard. - location = 'i4x', 'mitX', '101', 'course', 'test' + location = Location('i4x', 'mitX', '101', 'course', 'test') link = utils.get_lms_link_for_item(location) self.assertEquals( link, diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index cf4bffc503..b7d9cf9e77 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -79,7 +79,7 @@ def get_course_location_for_item(location): # @hack! We need to find the course location however, we don't # know the 'name' parameter in this context, so we have # to assume there's only one item in this query even though we are not specifying a name - course_search_location = ['i4x', item_loc.org, item_loc.course, 'course', None] + course_search_location = Location('i4x', item_loc.org, item_loc.course, 'course', None) courses = modulestore().get_items(course_search_location) # make sure we found exactly one match on this above course search @@ -107,7 +107,7 @@ def get_course_for_item(location): # @hack! We need to find the course location however, we don't # know the 'name' parameter in this context, so we have # to assume there's only one item in this query even though we are not specifying a name - course_search_location = ['i4x', item_loc.org, item_loc.course, 'course', None] + course_search_location = Location('i4x', item_loc.org, item_loc.course, 'course', None) courses = modulestore().get_items(course_search_location) # make sure we found exactly one match on this above course search diff --git a/cms/djangoapps/contentstore/views/access.py b/cms/djangoapps/contentstore/views/access.py index 3c75215bd1..df4fbed623 100644 --- a/cms/djangoapps/contentstore/views/access.py +++ b/cms/djangoapps/contentstore/views/access.py @@ -1,27 +1,11 @@ from auth.authz import STAFF_ROLE_NAME, INSTRUCTOR_ROLE_NAME from auth.authz import is_user_in_course_group_role -from django.core.exceptions import PermissionDenied from ..utils import get_course_location_for_item -from xmodule.modulestore import Location from xmodule.modulestore.locator import CourseLocator -def get_location_and_verify_access(request, org, course, name): - """ - Create the location, verify that the user has permissions - to view the location. Returns the location as a Location - """ - location = ['i4x', org, course, 'course', name] - - # check that logged in user has permissions to this item - if not has_access(request.user, location): - raise PermissionDenied() - - return Location(location) - - def has_access(user, location, role=STAFF_ROLE_NAME): - ''' + """ Return True if user allowed to access this piece of data Note that the CMS permissions model is with respect to courses There is a super-admin permissions if user.is_staff is set @@ -29,7 +13,7 @@ def has_access(user, location, role=STAFF_ROLE_NAME): I'm presuming that the course instructor (formally known as admin) will not be in both INSTRUCTOR and STAFF groups, so we have to cascade our queries here as INSTRUCTOR has all the rights that STAFF do - ''' + """ if not isinstance(location, CourseLocator): location = get_course_location_for_item(location) _has_access = is_user_in_course_group_role(user, location, role) diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 888bfcc061..32538a80c7 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -128,7 +128,7 @@ def course_listing(request): """ List all courses available to the logged in user """ - courses = modulestore('direct').get_items(['i4x', None, None, 'course', None]) + courses = modulestore('direct').get_items(Location('i4x', None, None, 'course', None)) # filter out courses that we don't have access too def course_filter(course):