Skip to content

Create explicit Locations. #1910

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cahrens merged 1 commit into from
Dec 13, 2013
Merged

Create explicit Locations. #1910

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Empty file added cms/djangoapps/auth/tests/__init__.py
View file
Open in desktop
Empty file.
7 changes: 4 additions & 3 deletions cms/djangoapps/auth/tests/test_authz.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

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,\
Expand Down Expand Up @@ -129,7 +130,7 @@ 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'
self.location = Location('i4x', 'mitX', '101', 'course', 'test')

def test_add_user_to_course_group(self):
"""
Expand Down Expand Up @@ -181,7 +182,7 @@ def test_get_staff(self):
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)
Expand All @@ -193,7 +194,7 @@ def test_get_instructor(self):
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)
Expand Down
13 changes: 11 additions & 2 deletions cms/djangoapps/contentstore/management/commands/check_course.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
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):
Expand Down Expand Up @@ -54,8 +55,16 @@ def _get_discussion_items(module):
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:
Expand Down
68 changes: 34 additions & 34 deletions cms/djangoapps/contentstore/tests/test_contentstore.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,20 +202,20 @@ def test_get_items(self):
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))

@dmitchell dmitchell Dec 13, 2013

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if it makes sense to gen these reused locns in one place and just reference them. Seems very repetitive. No biggie.


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))

Expand All @@ -229,17 +229,17 @@ def test_draft_metadata(self):
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))

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))
Expand All @@ -248,14 +248,14 @@ def test_draft_metadata(self):
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)

Expand All @@ -270,7 +270,7 @@ def test_draft_metadata(self):
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)
Expand All @@ -280,7 +280,7 @@ def test_draft_metadata(self):

# 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)
Expand All @@ -289,7 +289,7 @@ def test_get_depth_with_drafts(self):
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
)

Expand All @@ -298,21 +298,21 @@ def test_get_depth_with_drafts(self):
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
modulestore('draft').convert_to_draft(problem.location)

# 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
)

Expand All @@ -324,10 +324,10 @@ def test_no_static_link_rewrites_on_import(self):
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')
Expand All @@ -341,14 +341,14 @@ def test_import_textbook_as_content_element(self, mock_get):
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)

Expand All @@ -365,7 +365,7 @@ def test_default_tabs_on_create_course(self):
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")

Expand Down Expand Up @@ -455,7 +455,7 @@ def _create_static_tabs(self):
""" 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(
Expand All @@ -473,7 +473,7 @@ def test_import_polls(self):
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)
Expand All @@ -489,7 +489,7 @@ def test_module_preview_in_whitelist(self):
"""
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')
Expand All @@ -501,7 +501,7 @@ def test_video_module_caption_asset_path(self):
"""
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):
Expand All @@ -522,13 +522,13 @@ def _test_preview(self, location):
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)
Expand Down Expand Up @@ -1124,7 +1124,7 @@ def test_export_course_with_metadata_only_video(self):
# 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)

Expand Down Expand Up @@ -1152,7 +1152,7 @@ def test_export_course_with_metadata_only_word_cloud(self):
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)

Expand Down Expand Up @@ -1181,7 +1181,7 @@ def test_empty_data_roundtrip(self):
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)

Expand All @@ -1198,7 +1198,7 @@ def test_empty_data_roundtrip(self):

# 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, '')
Expand All @@ -1224,14 +1224,14 @@ def test_html_export_roundtrip(self):
# 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('<p style="font:italic bold 72px/30px Georgia, serif; color: red; ">', html_module.data)

# get the sample HTML with just a simple <img> 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('<img src="/static/foo_bar.jpg" />', html_module.data)

Expand Down Expand Up @@ -1790,7 +1790,7 @@ def test_metadata_inheritance(self):

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:
Expand Down
15 changes: 4 additions & 11 deletions cms/djangoapps/contentstore/tests/test_utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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):
Expand Down Expand Up @@ -64,12 +57,12 @@ def about_page_no_lms_base_test(self):

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")
Expand All @@ -80,7 +73,7 @@ def lms_link_test(self):

# 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,
Expand Down
Loading