diff --git a/cms/djangoapps/contentstore/tests/test_crud.py b/cms/djangoapps/contentstore/tests/test_crud.py index f5168ad34f..9bce00d650 100644 --- a/cms/djangoapps/contentstore/tests/test_crud.py +++ b/cms/djangoapps/contentstore/tests/test_crud.py @@ -64,7 +64,7 @@ class TemplateTests(unittest.TestCase): self.assertIsInstance(test_chapter, SequenceDescriptor) # refetch parent which should now point to child test_course = modulestore('split').get_course(test_chapter.location) - self.assertIn(test_chapter.location.usage_id, test_course.children) + self.assertIn(test_chapter.location.block_id, test_course.children) def test_temporary_xblocks(self): """ @@ -95,15 +95,20 @@ class TemplateTests(unittest.TestCase): """ try saving temporary xblocks """ - test_course = persistent_factories.PersistentCourseFactory.create(org='testx', prettyid='tempcourse', + test_course = persistent_factories.PersistentCourseFactory.create( + org='testx', prettyid='tempcourse', display_name='fun test course', user_id='testbot') test_chapter = self.load_from_json({'category': 'chapter', 'fields': {'display_name': 'chapter n'}}, test_course.system, parent_xblock=test_course) test_def_content = 'boo' # create child - _ = self.load_from_json({'category': 'problem', - 'fields': {'data': test_def_content}}, + self.load_from_json({ + 'category': 'problem', + 'fields': { + 'data': test_def_content, + 'display_name': 'problem' + }}, test_course.system, parent_xblock=test_chapter) # better to pass in persisted parent over the subdag so # subdag gets the parent pointer (otherwise 2 ops, persist dag, update parent children, @@ -117,6 +122,10 @@ class TemplateTests(unittest.TestCase): persisted_problem = persisted_chapter.get_children()[0] self.assertEqual(persisted_problem.category, 'problem') self.assertEqual(persisted_problem.data, test_def_content) + # update it + persisted_problem.display_name = 'altered problem' + persisted_problem = modulestore('split').persist_xblock_dag(persisted_problem, 'testbot') + self.assertEqual(persisted_problem.display_name, 'altered problem') def test_delete_course(self): test_course = persistent_factories.PersistentCourseFactory.create( @@ -166,7 +175,7 @@ class TemplateTests(unittest.TestCase): second_problem = persistent_factories.ItemFactory.create( display_name='problem 2', - parent_location=BlockUsageLocator(updated_loc, usage_id=sub.location.usage_id), + parent_location=BlockUsageLocator(updated_loc, block_id=sub.location.block_id), user_id='testbot', category='problem', data="" ) diff --git a/cms/djangoapps/contentstore/views/assets.py b/cms/djangoapps/contentstore/views/assets.py index ed3588d895..6df38f3751 100644 --- a/cms/djangoapps/contentstore/views/assets.py +++ b/cms/djangoapps/contentstore/views/assets.py @@ -51,7 +51,7 @@ def assets_handler(request, tag=None, course_id=None, branch=None, version_guid= DELETE json: delete an asset """ - location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) + location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block) if not has_access(request.user, location): raise PermissionDenied() diff --git a/cms/djangoapps/contentstore/views/checklist.py b/cms/djangoapps/contentstore/views/checklist.py index 7326c3d16b..f9156703f4 100644 --- a/cms/djangoapps/contentstore/views/checklist.py +++ b/cms/djangoapps/contentstore/views/checklist.py @@ -36,7 +36,7 @@ def checklists_handler(request, tag=None, course_id=None, branch=None, version_g POST or PUT json: updates the checked state for items within a particular checklist. checklist_index is required. """ - location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) + location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block) if not has_access(request.user, location): raise PermissionDenied() diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index deda47f5fa..62c020dd39 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -62,7 +62,7 @@ def subsection_handler(request, tag=None, course_id=None, branch=None, version_g json: not currently supported """ if 'text/html' in request.META.get('HTTP_ACCEPT', 'text/html'): - locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) + locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block) try: old_location, course, item, lms_link = _get_item_in_course(request, locator) except ItemNotFoundError: @@ -151,7 +151,7 @@ def unit_handler(request, tag=None, course_id=None, branch=None, version_guid=No json: not currently supported """ if 'text/html' in request.META.get('HTTP_ACCEPT', 'text/html'): - locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) + locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block) try: old_location, course, item, lms_link = _get_item_in_course(request, locator) except ItemNotFoundError: diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 261370b7ff..ecbb0890b2 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -11,7 +11,7 @@ from django.utils.translation import ugettext as _ from django.contrib.auth.decorators import login_required from django_future.csrf import ensure_csrf_cookie from django.conf import settings -from django.views.decorators.http import require_http_methods, require_POST +from django.views.decorators.http import require_http_methods from django.core.exceptions import PermissionDenied from django.core.urlresolvers import reverse from django.http import HttpResponseBadRequest, HttpResponseNotFound @@ -60,12 +60,12 @@ __all__ = ['course_info_handler', 'course_handler', 'course_info_update_handler' 'textbooks_list_handler', 'textbooks_detail_handler'] -def _get_locator_and_course(course_id, branch, version_guid, usage_id, user, depth=0): +def _get_locator_and_course(course_id, branch, version_guid, block_id, user, depth=0): """ Internal method used to calculate and return the locator and course module for the view functions in this file. """ - locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=usage_id) + locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block_id) if not has_access(user, locator): raise PermissionDenied() course_location = loc_mapper().translate_locator_to_location(locator) @@ -104,7 +104,7 @@ def course_handler(request, tag=None, course_id=None, branch=None, version_guid= return create_new_course(request) elif not has_access( request.user, - BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) + BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block) ): raise PermissionDenied() elif request.method == 'PUT': @@ -366,7 +366,7 @@ def course_info_update_handler(request, tag=None, course_id=None, branch=None, v """ if 'application/json' not in request.META.get('HTTP_ACCEPT', 'application/json'): return HttpResponseBadRequest("Only supports json requests") - updates_locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) + updates_locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block) updates_location = loc_mapper().translate_locator_to_location(updates_locator) if provided_id == '': provided_id = None diff --git a/cms/djangoapps/contentstore/views/import_export.py b/cms/djangoapps/contentstore/views/import_export.py index c36d30a34d..d555748fb4 100644 --- a/cms/djangoapps/contentstore/views/import_export.py +++ b/cms/djangoapps/contentstore/views/import_export.py @@ -60,7 +60,7 @@ def import_handler(request, tag=None, course_id=None, branch=None, version_guid= POST or PUT json: import a course via the .tar.gz file specified in request.FILES """ - location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) + location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block) if not has_access(request.user, location): raise PermissionDenied() @@ -271,7 +271,7 @@ def import_status_handler(request, tag=None, course_id=None, branch=None, versio 3 : Importing to mongo """ - location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) + location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block) if not has_access(request.user, location): raise PermissionDenied() @@ -302,7 +302,7 @@ def export_handler(request, tag=None, course_id=None, branch=None, version_guid= If the tar.gz file has been requested but the export operation fails, an HTML page will be returned which describes the error. """ - location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) + location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block) if not has_access(request.user, location): raise PermissionDenied() diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 8ab7cea4e1..bf059390df 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -79,7 +79,7 @@ def xblock_handler(request, tag=None, course_id=None, branch=None, version_guid= The locator (and old-style id) for the created xblock (minus children) is returned. """ if course_id is not None: - locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) + locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block) if not has_access(request.user, locator): raise PermissionDenied() old_location = loc_mapper().translate_locator_to_location(locator) @@ -330,7 +330,7 @@ def orphan_handler(request, tag=None, course_id=None, branch=None, version_guid= :param request: :param course_id: Locator syntax course_id """ - location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) + location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block) # DHM: when split becomes back-end, move or conditionalize this conversion old_location = loc_mapper().translate_locator_to_location(location) if request.method == 'GET': diff --git a/cms/djangoapps/contentstore/views/tabs.py b/cms/djangoapps/contentstore/views/tabs.py index 0cbfb0ac96..b9bda31291 100644 --- a/cms/djangoapps/contentstore/views/tabs.py +++ b/cms/djangoapps/contentstore/views/tabs.py @@ -62,7 +62,7 @@ def tabs_handler(request, tag=None, course_id=None, branch=None, version_guid=No Creating a tab, deleting a tab, or changing its contents is not supported through this method. Instead use the general xblock URL (see item.xblock_handler). """ - locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) + locator = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block) if not has_access(request.user, locator): raise PermissionDenied() diff --git a/cms/djangoapps/contentstore/views/user.py b/cms/djangoapps/contentstore/views/user.py index 624754558b..ac6ca3bc4c 100644 --- a/cms/djangoapps/contentstore/views/user.py +++ b/cms/djangoapps/contentstore/views/user.py @@ -52,7 +52,7 @@ def course_team_handler(request, tag=None, course_id=None, branch=None, version_ DELETE: json: remove a particular course team member from the course team (email is required). """ - location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, usage_id=block) + location = BlockUsageLocator(course_id=course_id, branch=branch, version_guid=version_guid, block_id=block) if not has_access(request.user, location): raise PermissionDenied() diff --git a/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py b/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py index 15b2c1141c..fdfb757267 100644 --- a/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py +++ b/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py @@ -6,7 +6,7 @@ import re import pymongo import bson.son -from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError, DuplicateItemError +from xmodule.modulestore.exceptions import InvalidLocationError, ItemNotFoundError from xmodule.modulestore.locator import BlockUsageLocator from xmodule.modulestore import Location import urllib @@ -156,27 +156,27 @@ class LocMapperStore(object): entry = item break - usage_id = entry['block_map'].get(self._encode_for_mongo(location.name)) - if usage_id is None: + block_id = entry['block_map'].get(self._encode_for_mongo(location.name)) + if block_id is None: if add_entry_if_missing: - usage_id = self._add_to_block_map(location, location_id, entry['block_map']) + block_id = self._add_to_block_map(location, location_id, entry['block_map']) else: raise ItemNotFoundError(location) - elif isinstance(usage_id, dict): + elif isinstance(block_id, dict): # name is not unique, look through for the right category - if location.category in usage_id: - usage_id = usage_id[location.category] + if location.category in block_id: + block_id = block_id[location.category] elif add_entry_if_missing: - usage_id = self._add_to_block_map(location, location_id, entry['block_map']) + block_id = self._add_to_block_map(location, location_id, entry['block_map']) else: raise ItemNotFoundError() else: raise InvalidLocationError() published_usage = BlockUsageLocator( - course_id=entry['course_id'], branch=entry['prod_branch'], usage_id=usage_id) + course_id=entry['course_id'], branch=entry['prod_branch'], block_id=block_id) draft_usage = BlockUsageLocator( - course_id=entry['course_id'], branch=entry['draft_branch'], usage_id=usage_id) + course_id=entry['course_id'], branch=entry['draft_branch'], block_id=block_id) if published: result = published_usage else: @@ -191,7 +191,7 @@ class LocMapperStore(object): Returns an old style Location for the given Locator if there's an appropriate entry in the mapping collection. Note, it requires that the course was previously mapped (a side effect of translate_location or explicitly via create_map_entry) and - the block's usage_id was previously stored in the + the block's block_id was previously stored in the map (a side effect of translate_location or via add|update_block_location). If get_course, then rather than finding the map for this locator, it finds the 'course' root @@ -200,7 +200,7 @@ class LocMapperStore(object): If there are no matches, it returns None. If there's more than one location to locator mapping to the same course_id, it looks for the first - one with a mapping for the block usage_id and picks that arbitrary course location. + one with a mapping for the block block_id and picks that arbitrary course location. :param locator: a BlockUsageLocator """ @@ -214,14 +214,14 @@ class LocMapperStore(object): # This does not require that the course exist in any modulestore # only that it has a mapping entry. maps = self.location_map.find({'course_id': locator.course_id}) - # look for one which maps to this block usage_id + # look for one which maps to this block block_id if maps.count() == 0: return None result = None for candidate in maps: old_course_id = self._generate_location_course_id(candidate['_id']) for old_name, cat_to_usage in candidate['block_map'].iteritems(): - for category, usage_id in cat_to_usage.iteritems(): + for category, block_id in cat_to_usage.iteritems(): # cache all entries and then figure out if we have the one we want # Always return revision=None because the # old draft module store wraps locations as draft before @@ -234,16 +234,16 @@ class LocMapperStore(object): self._decode_from_mongo(old_name), None) published_locator = BlockUsageLocator( - candidate['course_id'], branch=candidate['prod_branch'], usage_id=usage_id + candidate['course_id'], branch=candidate['prod_branch'], block_id=block_id ) draft_locator = BlockUsageLocator( - candidate['course_id'], branch=candidate['draft_branch'], usage_id=usage_id + candidate['course_id'], branch=candidate['draft_branch'], block_id=block_id ) self._cache_location_map_entry(old_course_id, location, published_locator, draft_locator) if get_course and category == 'course': result = location - elif not get_course and usage_id == locator.usage_id: + elif not get_course and block_id == locator.block_id: result = location if result is not None: return result @@ -256,13 +256,15 @@ class LocMapperStore(object): # The downside is that if there's more than one course mapped to from the same org/course root # the block ids will likely be out of sync and collide from an id perspective. HOWEVER, # if there are few == org/course roots or their content is unrelated, this will work well. - usage_id = self._verify_uniqueness(location.category + location.name[:3], block_map) + block_id = self._verify_uniqueness(location.category + location.name[:3], block_map) else: - usage_id = location.name + # if 2 different category locations had same name, then they'll collide. Make the later + # mapped ones unique + block_id = self._verify_uniqueness(location.name, block_map) encoded_location_name = self._encode_for_mongo(location.name) - block_map.setdefault(encoded_location_name, {})[location.category] = usage_id + block_map.setdefault(encoded_location_name, {})[location.category] = block_id self.location_map.update(location_id, {'$set': {'block_map': block_map}}) - return usage_id + return block_id def _interpret_location_course_id(self, course_id, location): """ diff --git a/common/lib/xmodule/xmodule/modulestore/locator.py b/common/lib/xmodule/xmodule/modulestore/locator.py index a7d1d8435a..7e2854b042 100644 --- a/common/lib/xmodule/xmodule/modulestore/locator.py +++ b/common/lib/xmodule/xmodule/modulestore/locator.py @@ -257,7 +257,7 @@ class CourseLocator(Locator): Returns a copy of itself (downcasting) as a CourseLocator. The copy has the same CourseLocator fields as the original. The copy does not include subclass information, such as - a usage_id (a property of BlockUsageLocator). + a block_id (a property of BlockUsageLocator). """ return CourseLocator(course_id=self.course_id, version_guid=self.version_guid, @@ -298,8 +298,8 @@ class CourseLocator(Locator): self._set_value( parse, 'version_guid', lambda (new_guid): self.set_version_guid(self.as_object_id(new_guid)) ) - self._set_value(parse, 'course_id', lambda (new_id): self.set_course_id(new_id)) - self._set_value(parse, 'branch', lambda (new_branch): self.set_branch(new_branch)) + self._set_value(parse, 'course_id', self.set_course_id) + self._set_value(parse, 'branch', self.set_branch) def init_from_version_guid(self, version_guid): """ @@ -390,23 +390,23 @@ class BlockUsageLocator(CourseLocator): """ # Default value - usage_id = None + block_id = None def __init__(self, url=None, version_guid=None, course_id=None, - branch=None, usage_id=None): + branch=None, block_id=None): """ Construct a BlockUsageLocator Caller may provide url, version_guid, or course_id, and optionally provide branch. - The usage_id may be specified, either explictly or as part of + The block_id may be specified, either explictly or as part of the url or course_id. If omitted, the locator is created but it has not yet been initialized. - Resulting BlockUsageLocator will have a usage_id property. + Resulting BlockUsageLocator will have a block_id property. It will have either a version_guid property or a course_id (with optional branch) property, or both. version_guid must be an instance of bson.objectid.ObjectId or None - url, course_id, branch, and usage_id must be strings or None + url, course_id, branch, and block_id must be strings or None """ self._validate_args(url, version_guid, course_id) @@ -414,8 +414,8 @@ class BlockUsageLocator(CourseLocator): self.init_block_ref_from_str(url) if course_id: self.init_block_ref_from_course_id(course_id) - if usage_id: - self.init_block_ref(usage_id) + if block_id: + self.init_block_ref(block_id) super(BlockUsageLocator, self).__init__( url=url, version_guid=version_guid, @@ -425,9 +425,9 @@ class BlockUsageLocator(CourseLocator): def is_initialized(self): """ - Returns True if usage_id has been initialized, else returns False + Returns True if block_id has been initialized, else returns False """ - return self.usage_id is not None + return self.block_id is not None def version_agnostic(self): """ @@ -442,58 +442,57 @@ class BlockUsageLocator(CourseLocator): if self.version_guid: return BlockUsageLocator(version_guid=self.version_guid, branch=self.branch, - usage_id=self.usage_id) + block_id=self.block_id) else: return BlockUsageLocator(course_id=self.course_id, branch=self.branch, - usage_id=self.usage_id) + block_id=self.block_id) - def set_usage_id(self, new): + def set_block_id(self, new): """ - Initialize usage_id to new value. - If usage_id has already been initialized to a different value, raise an exception. + Initialize block_id to new value. + If block_id has already been initialized to a different value, raise an exception. """ - self.set_property('usage_id', new) + self.set_property('block_id', new) def init_block_ref(self, block_ref): if isinstance(block_ref, LocalId): - self.set_usage_id(block_ref) + self.set_block_id(block_ref) else: parse = parse_block_ref(block_ref) if not parse: raise ValueError('Could not parse "%s" as a block_ref' % block_ref) - self.set_usage_id(parse['block']) + self.set_block_id(parse['block']) def init_block_ref_from_str(self, value): """ Create a block locator from the given string which may be a url or just the repr (no tag) """ - if hasattr(value, 'usage_id'): - self.init_block_ref(value.usage_id) + if hasattr(value, 'block_id'): + self.init_block_ref(value.block_id) return if not isinstance(value, basestring): return None parse = parse_url(value, tag_optional=True) if parse is None: raise ValueError('Could not parse "%s" as a url' % value) - self._set_value(parse, 'block', lambda(new_block): self.set_usage_id(new_block)) + self._set_value(parse, 'block', self.set_block_id) def init_block_ref_from_course_id(self, course_id): if isinstance(course_id, CourseLocator): - # FIXME the parsed course_id should never contain a block ref course_id = course_id.course_id assert course_id, "%s does not have a valid course_id" parse = parse_course_id(course_id) if parse is None: raise ValueError('Could not parse "%s" as a course_id' % course_id) - self._set_value(parse, 'block', lambda(new_block): self.set_usage_id(new_block)) + self._set_value(parse, 'block', self.set_block_id) def __unicode__(self): """ Return a string representing this location. """ rep = super(BlockUsageLocator, self).__unicode__() - return rep + '/' + BLOCK_PREFIX + unicode(self.usage_id) + return rep + '/' + BLOCK_PREFIX + unicode(self.block_id) class DefinitionLocator(Locator): diff --git a/common/lib/xmodule/xmodule/modulestore/split_migrator.py b/common/lib/xmodule/xmodule/modulestore/split_migrator.py index 355f5bfa62..93a42b5f12 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_migrator.py +++ b/common/lib/xmodule/xmodule/modulestore/split_migrator.py @@ -50,7 +50,7 @@ class SplitMigrator(object): course_location.org, original_course.display_name, user_id, id_root=new_course_id, fields=self._get_json_fields_translate_children(original_course, old_course_id, True), - root_usage_id=new_course_root_locator.usage_id, + root_block_id=new_course_root_locator.block_id, master_branch=new_course_root_locator.branch ) @@ -74,13 +74,13 @@ class SplitMigrator(object): # don't copy the course again. No drafts should get here but check if module.location != old_course_loc and not getattr(module, 'is_draft', False): # create split_xblock using split.create_item - # where usage_id is computed by translate_location_to_locator + # where block_id is computed by translate_location_to_locator new_locator = self.loc_mapper.translate_location( old_course_id, module.location, True, add_entry_if_missing=True ) _new_module = self.split_modulestore.create_item( course_version_locator, module.category, user_id, - usage_id=new_locator.usage_id, + block_id=new_locator.block_id, fields=self._get_json_fields_translate_children(module, old_course_id, True), continue_version=True ) @@ -130,18 +130,18 @@ class SplitMigrator(object): # create a new course version just in case the current head is also the prod head _new_module = self.split_modulestore.create_item( new_draft_course_loc, module.category, user_id, - usage_id=new_locator.usage_id, + block_id=new_locator.block_id, fields=self._get_json_fields_translate_children(module, old_course_id, True) ) - awaiting_adoption[module.location] = new_locator.usage_id - for draft_location, new_usage_id in awaiting_adoption.iteritems(): + awaiting_adoption[module.location] = new_locator.block_id + for draft_location, new_block_id in awaiting_adoption.iteritems(): for parent_loc in self.draft_modulestore.get_parent_locations(draft_location, old_course_id): old_parent = self.draft_modulestore.get_item(parent_loc) new_parent = self.split_modulestore.get_item( self.loc_mapper.translate_location(old_course_id, old_parent.location, False) ) # this only occurs if the parent was also awaiting adoption - if new_usage_id in new_parent.children: + if new_block_id in new_parent.children: break # find index for module: new_parent may be missing quite a few of old_parent's children new_parent_cursor = 0 @@ -152,15 +152,15 @@ class SplitMigrator(object): sibling_loc = self.loc_mapper.translate_location(old_course_id, Location(old_child_loc), False) # sibling may move cursor for idx in range(new_parent_cursor, len(new_parent.children)): - if new_parent.children[idx] == sibling_loc.usage_id: + if new_parent.children[idx] == sibling_loc.block_id: new_parent_cursor = idx + 1 break - new_parent.children.insert(new_parent_cursor, new_usage_id) + new_parent.children.insert(new_parent_cursor, new_block_id) new_parent = self.split_modulestore.update_item(new_parent, user_id) def _get_json_fields_translate_children(self, xblock, old_course_id, published): """ - Return the json repr for explicitly set fields but convert all children to their usage_id's + Return the json repr for explicitly set fields but convert all children to their block_id's """ fields = self.get_json_fields_explicitly_set(xblock) # this will too generously copy the children even for ones that don't exist in the published b/c the old mongo @@ -169,7 +169,7 @@ class SplitMigrator(object): fields['children'] = [ self.loc_mapper.translate_location( old_course_id, Location(child), published, add_entry_if_missing=True - ).usage_id + ).block_id for child in fields['children']] return fields diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py index a0549074c3..b7381b57df 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py @@ -47,26 +47,26 @@ class CachingDescriptorSystem(MakoDescriptorSystem): self.default_class = default_class self.local_modules = {} - def _load_item(self, usage_id, course_entry_override=None): - if isinstance(usage_id, BlockUsageLocator) and isinstance(usage_id.usage_id, LocalId): + def _load_item(self, block_id, course_entry_override=None): + if isinstance(block_id, BlockUsageLocator) and isinstance(block_id.block_id, LocalId): try: - return self.local_modules[usage_id] + return self.local_modules[block_id] except KeyError: raise ItemNotFoundError - json_data = self.module_data.get(usage_id) + json_data = self.module_data.get(block_id) if json_data is None: # deeper than initial descendant fetch or doesn't exist - self.modulestore.cache_items(self, [usage_id], lazy=self.lazy) - json_data = self.module_data.get(usage_id) + self.modulestore.cache_items(self, [block_id], lazy=self.lazy) + json_data = self.module_data.get(block_id) if json_data is None: - raise ItemNotFoundError(usage_id) + raise ItemNotFoundError(block_id) class_ = XModuleDescriptor.load_class( json_data.get('category'), self.default_class ) - return self.xblock_from_json(class_, usage_id, json_data, course_entry_override) + return self.xblock_from_json(class_, block_id, json_data, course_entry_override) # xblock's runtime does not always pass enough contextual information to figure out # which named container (course x branch) or which parent is requesting an item. Because split allows @@ -79,7 +79,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem): # low; thus, the course_entry is most likely correct. If the thread is looking at > 1 named container # pointing to the same structure, the access is likely to be chunky enough that the last known container # is the intended one when not given a course_entry_override; thus, the caching of the last branch/course_id. - def xblock_from_json(self, class_, usage_id, json_data, course_entry_override=None): + def xblock_from_json(self, class_, block_id, json_data, course_entry_override=None): if course_entry_override is None: course_entry_override = self.course_entry else: @@ -91,12 +91,12 @@ class CachingDescriptorSystem(MakoDescriptorSystem): definition_id = self.modulestore.definition_locator(definition) # If no usage id is provided, generate an in-memory id - if usage_id is None: - usage_id = LocalId() + if block_id is None: + block_id = LocalId() block_locator = BlockUsageLocator( version_guid=course_entry_override['structure']['_id'], - usage_id=usage_id, + block_id=block_id, course_id=course_entry_override.get('course_id'), branch=course_entry_override.get('branch') ) @@ -121,7 +121,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem): self, BlockUsageLocator( version_guid=course_entry_override['structure']['_id'], - usage_id=usage_id + block_id=block_id ), error_msg=exc_info_to_str(sys.exc_info()) ) @@ -136,7 +136,7 @@ class CachingDescriptorSystem(MakoDescriptorSystem): module.save() # If this is an in-memory block, store it in this system - if isinstance(block_locator.usage_id, LocalId): + if isinstance(block_locator.block_id, LocalId): self.local_modules[block_locator] = module return module diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index ac515b1013..ff61571126 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -11,7 +11,7 @@ Representation: ** 'versions': versions_dict: {branch_id: structure_id, ...} * structure: ** '_id': an ObjectId (guid), - ** 'root': root_usage_id (string of key in 'blocks' for the root of this structure, + ** 'root': root_block_id (string of key in 'blocks' for the root of this structure, ** 'previous_version': the structure from which this one was derived. For published courses, this points to the previously published version of the structure not the draft published to this. ** 'original_version': the original structure id in the previous_version relation. Is a pseudo object @@ -130,20 +130,20 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): # the split mongo store is used for item creation as well as item persistence self.mixologist = Mixologist(self.xblock_mixins) - def cache_items(self, system, base_usage_ids, depth=0, lazy=True): + def cache_items(self, system, base_block_ids, depth=0, lazy=True): ''' Handles caching of items once inheritance and any other one time per course per fetch operations are done. :param system: a CachingDescriptorSystem - :param base_usage_ids: list of usage_ids to fetch + :param base_block_ids: list of block_ids to fetch :param depth: how deep below these to prefetch :param lazy: whether to fetch definitions or use placeholders ''' new_module_data = {} - for usage_id in base_usage_ids: + for block_id in base_block_ids: new_module_data = self.descendants( system.course_entry['structure']['blocks'], - usage_id, + block_id, depth, new_module_data ) @@ -167,7 +167,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): system.module_data.update(new_module_data) return system.module_data - def _load_items(self, course_entry, usage_ids, depth=0, lazy=True): + def _load_items(self, course_entry, block_ids, depth=0, lazy=True): ''' Load & cache the given blocks from the course. Prefetch down to the given depth. Load the definitions into each block if lazy is False; @@ -187,8 +187,8 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): mixins=self.xblock_mixins ) self._add_cache(course_entry['structure']['_id'], system) - self.cache_items(system, usage_ids, depth, lazy) - return [system.load_item(usage_id, course_entry) for usage_id in usage_ids] + self.cache_items(system, block_ids, depth, lazy) + return [system.load_item(block_id, course_entry) for block_id in block_ids] def _get_cache(self, course_version_guid): """ @@ -333,7 +333,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): the course or the block w/in the course do not exist for the given version. raises InsufficientSpecificationError if the locator does not id a block """ - if block_location.usage_id is None: + if block_location.block_id is None: raise InsufficientSpecificationError(block_location) try: course_structure = self._lookup_course(block_location)['structure'] @@ -341,7 +341,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): # this error only occurs if the course does not exist return False - return course_structure['blocks'].get(block_location.usage_id) is not None + return course_structure['blocks'].get(block_location.block_id) is not None def get_item(self, location, depth=0): """ @@ -365,7 +365,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): if not location.is_initialized(): raise InsufficientSpecificationError("Not yet initialized: %s" % location) course = self._lookup_course(location) - items = self._load_items(course, [location.usage_id], depth, lazy=True) + items = self._load_items(course, [location.block_id], depth, lazy=True) if len(items) == 0: raise ItemNotFoundError(location) return items[0] @@ -397,9 +397,9 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): qualifiers = {} course = self._lookup_course(locator) items = [] - for usage_id, value in course['structure']['blocks'].iteritems(): + for block_id, value in course['structure']['blocks'].iteritems(): if self._block_matches(value, qualifiers): - items.append(usage_id) + items.append(block_id) if len(items) > 0: return self._load_items(course, items, 0, lazy=True) @@ -421,16 +421,16 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): def get_parent_locations(self, locator, course_id=None): ''' - Return the locations (Locators w/ usage_ids) for the parents of this location in this - course. Could use get_items(location, {'children': usage_id}) but this is slightly faster. - NOTE: the locator must contain the usage_id, and this code does not actually ensure usage_id exists + Return the locations (Locators w/ block_ids) for the parents of this location in this + course. Could use get_items(location, {'children': block_id}) but this is slightly faster. + NOTE: the locator must contain the block_id, and this code does not actually ensure block_id exists :param locator: BlockUsageLocator restricting search scope - :param usage_id: ignored. Only included for API compatibility. Specify the usage_id within the locator. + :param course_id: ignored. Only included for API compatibility. Specify the course_id within the locator. ''' course = self._lookup_course(locator) - items = self._get_parents_from_structure(locator.usage_id, course['structure']) - return [BlockUsageLocator(url=locator.as_course_locator(), usage_id=parent_id) + items = self._get_parents_from_structure(locator.block_id, course['structure']) + return [BlockUsageLocator(url=locator.as_course_locator(), block_id=parent_id) for parent_id in items] def get_orphans(self, course_id, detached_categories, branch): @@ -447,7 +447,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): if block_data['category'] in detached_categories: items.discard(block_id) return [ - BlockUsageLocator(course_id=course_id, branch=branch, usage_id=block_id) + BlockUsageLocator(course_id=course_id, branch=branch, block_id=block_id) for block_id in items ] @@ -547,25 +547,25 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): ''' # version_agnostic means we don't care if the head and version don't align, trust the version course_struct = self._lookup_course(block_locator.version_agnostic())['structure'] - usage_id = block_locator.usage_id - update_version_field = 'blocks.{}.edit_info.update_version'.format(usage_id) + block_id = block_locator.block_id + update_version_field = 'blocks.{}.edit_info.update_version'.format(block_id) all_versions_with_block = self.db_connection.find_matching_structures({'original_version': course_struct['original_version'], update_version_field: {'$exists': True}}) # find (all) root versions and build map previous: [successors] possible_roots = [] result = {} for version in all_versions_with_block: - if version['_id'] == version['blocks'][usage_id]['edit_info']['update_version']: - if version['blocks'][usage_id]['edit_info'].get('previous_version') is None: - possible_roots.append(version['blocks'][usage_id]['edit_info']['update_version']) + if version['_id'] == version['blocks'][block_id]['edit_info']['update_version']: + if version['blocks'][block_id]['edit_info'].get('previous_version') is None: + possible_roots.append(version['blocks'][block_id]['edit_info']['update_version']) else: - result.setdefault(version['blocks'][usage_id]['edit_info']['previous_version'], set()).add( - version['blocks'][usage_id]['edit_info']['update_version']) + result.setdefault(version['blocks'][block_id]['edit_info']['previous_version'], set()).add( + version['blocks'][block_id]['edit_info']['update_version']) # more than one possible_root means usage was added and deleted > 1x. if len(possible_roots) > 1: # find the history segment including block_locator's version - element_to_find = course_struct['blocks'][usage_id]['edit_info']['update_version'] + element_to_find = course_struct['blocks'][block_id]['edit_info']['update_version'] if element_to_find in possible_roots: possible_roots = [element_to_find] for possibility in possible_roots: @@ -576,9 +576,9 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): return None # convert the results value sets to locators for k, versions in result.iteritems(): - result[k] = [BlockUsageLocator(version_guid=version, usage_id=usage_id) + result[k] = [BlockUsageLocator(version_guid=version, block_id=block_id) for version in versions] - return VersionTree(BlockUsageLocator(version_guid=possible_roots[0], usage_id=usage_id), result) + return VersionTree(BlockUsageLocator(version_guid=possible_roots[0], block_id=block_id), result) def get_definition_successors(self, definition_locator, version_history_depth=1): ''' @@ -646,7 +646,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): else: return definition_locator, False - def _generate_usage_id(self, course_blocks, category): + def _generate_block_id(self, course_blocks, category): """ Generate a somewhat readable block id unique w/in this course using the category :param course_blocks: the current list of blocks. @@ -687,7 +687,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): # validation behavior a responsibility of the model layer rather than the persistence layer. def create_item( self, course_or_parent_locator, category, user_id, - usage_id=None, definition_locator=None, fields=None, + block_id=None, definition_locator=None, fields=None, force=False, continue_version=False ): """ @@ -712,7 +712,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): to the existing definition. If fields contains Scope.content and definition_locator is not None, then the Scope.content fields are assumed to be a new payload for definition_locator. - :param usage_id: if provided, must not already exist in the structure. Provides the block id for the + :param block_id: if provided, must not already exist in the structure. Provides the block id for the new item in this structure. Otherwise, one is computed using the category appended w/ a few digits. :param continue_version: continue changing the current structure at the head of the course. Very dangerous @@ -755,18 +755,18 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): new_id = new_structure['_id'] # generate usage id - if usage_id is not None: - if usage_id in new_structure['blocks']: - raise DuplicateItemError(usage_id, self, 'structures') + if block_id is not None: + if block_id in new_structure['blocks']: + raise DuplicateItemError(block_id, self, 'structures') else: - new_usage_id = usage_id + new_block_id = block_id else: - new_usage_id = self._generate_usage_id(new_structure['blocks'], category) + new_block_id = self._generate_block_id(new_structure['blocks'], category) block_fields = partitioned_fields.get(Scope.settings, {}) if Scope.children in partitioned_fields: block_fields.update(partitioned_fields[Scope.children]) - new_structure['blocks'][new_usage_id] = { + new_structure['blocks'][new_block_id] = { "category": category, "definition": definition_locator.definition_id, "fields": block_fields, @@ -780,9 +780,9 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): # if given parent, add new block as child and update parent's version parent = None - if isinstance(course_or_parent_locator, BlockUsageLocator) and course_or_parent_locator.usage_id is not None: - parent = new_structure['blocks'][course_or_parent_locator.usage_id] - parent['fields'].setdefault('children', []).append(new_usage_id) + if isinstance(course_or_parent_locator, BlockUsageLocator) and course_or_parent_locator.block_id is not None: + parent = new_structure['blocks'][course_or_parent_locator.block_id] + parent['fields'].setdefault('children', []).append(new_block_id) if not continue_version or parent['edit_info']['update_version'] != structure['_id']: parent['edit_info']['edited_on'] = datetime.datetime.now(UTC) parent['edit_info']['edited_by'] = user_id @@ -806,13 +806,13 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): # reconstruct the new_item from the cache return self.get_item(BlockUsageLocator(course_id=course_parent, - usage_id=new_usage_id, + block_id=new_block_id, version_guid=new_id)) def create_course( self, org, prettyid, user_id, id_root=None, fields=None, master_branch='draft', versions_dict=None, root_category='course', - root_usage_id='course' + root_block_id='course' ): """ Create a new entry in the active courses index which points to an existing or new structure. Returns @@ -870,7 +870,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): self.db_connection.insert_definition(definition_entry) draft_structure = self._new_structure( - user_id, root_usage_id, root_category, block_fields, definition_id + user_id, root_block_id, root_category, block_fields, definition_id ) new_id = draft_structure['_id'] @@ -944,10 +944,10 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): descriptor.definition_locator, is_updated = self.update_definition_from_data( descriptor.definition_locator, descriptor.get_explicitly_set_fields_by_scope(Scope.content), user_id) # check children - original_entry = original_structure['blocks'][descriptor.location.usage_id] - if (not is_updated and descriptor.has_children - and not self._xblock_lists_equal(original_entry['fields']['children'], descriptor.children)): - is_updated = True + original_entry = original_structure['blocks'][descriptor.location.block_id] + is_updated = is_updated or ( + descriptor.has_children and original_entry['fields']['children'] != descriptor.children + ) # check metadata if not is_updated: is_updated = self._compare_settings( @@ -958,12 +958,12 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): # if updated, rev the structure if is_updated: new_structure = self._version_structure(original_structure, user_id) - block_data = new_structure['blocks'][descriptor.location.usage_id] + block_data = new_structure['blocks'][descriptor.location.block_id] block_data["definition"] = descriptor.definition_locator.definition_id block_data["fields"] = descriptor.get_explicitly_set_fields_by_scope(Scope.settings) if descriptor.has_children: - block_data['fields']["children"] = [self._usage_id(child) for child in descriptor.children] + block_data['fields']["children"] = descriptor.children new_id = new_structure['_id'] block_data['edit_info'] = { @@ -990,7 +990,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): create or update the xblock and all of its children. The xblock's location must specify a course. If it doesn't specify a usage_id, then it's presumed to be new and need creation. This function descends the children performing the same operation for any that are xblocks. Any children which - are usage_ids just update the children pointer. + are block_ids just update the children pointer. All updates go into the same course version (bulk updater). @@ -1020,7 +1020,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): return self.get_item( BlockUsageLocator( course_id=xblock.location.course_id, - usage_id=xblock.location.usage_id, + block_id=xblock.location.block_id, branch=xblock.location.branch, version_guid=new_id ) @@ -1039,38 +1039,38 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): xblock.definition_locator, is_updated = self.update_definition_from_data( xblock.definition_locator, new_def_data, user_id) - if isinstance(xblock.scope_ids.usage_id.usage_id, LocalId): + if isinstance(xblock.scope_ids.usage_id.block_id, LocalId): # generate an id is_new = True is_updated = True - usage_id = self._generate_usage_id(structure_blocks, xblock.category) - xblock.scope_ids.usage_id.usage_id = usage_id + block_id = self._generate_block_id(structure_blocks, xblock.category) + xblock.scope_ids.usage_id.block_id = block_id else: is_new = False - usage_id = xblock.location.usage_id - if (not is_updated and xblock.has_children - and not self._xblock_lists_equal(structure_blocks[usage_id]['fields']['children'], xblock.children)): - is_updated = True + block_id = xblock.location.block_id + is_updated = is_updated or ( + xblock.has_children and structure_blocks[block_id]['fields']['children'] != xblock.children + ) children = [] if xblock.has_children: for child in xblock.children: - if isinstance(child.usage_id, LocalId): + if isinstance(child.block_id, LocalId): child_block = xblock.system.get_block(child) is_updated = self._persist_subdag(child_block, user_id, structure_blocks, new_id) or is_updated - children.append(child_block.location.usage_id) + children.append(child_block.location.block_id) else: children.append(child) block_fields = xblock.get_explicitly_set_fields_by_scope(Scope.settings) if not is_new and not is_updated: - is_updated = self._compare_settings(block_fields, structure_blocks[usage_id]['fields']) + is_updated = self._compare_settings(block_fields, structure_blocks[block_id]['fields']) if children: block_fields['children'] = children if is_updated: - previous_version = None if is_new else structure_blocks[usage_id]['edit_info'].get('update_version') - structure_blocks[usage_id] = { + previous_version = None if is_new else structure_blocks[block_id]['edit_info'].get('update_version') + structure_blocks[block_id] = { "category": xblock.category, "definition": xblock.definition_locator.definition_id, "fields": block_fields, @@ -1214,7 +1214,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): """ assert isinstance(usage_locator, BlockUsageLocator) and usage_locator.is_initialized() original_structure = self._lookup_course(usage_locator)['structure'] - if original_structure['root'] == usage_locator.usage_id: + if original_structure['root'] == usage_locator.block_id: raise ValueError("Cannot delete the root of a course") index_entry = self._get_index_if_valid(usage_locator, force) new_structure = self._version_structure(original_structure, user_id) @@ -1222,22 +1222,24 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): new_id = new_structure['_id'] parents = self.get_parent_locations(usage_locator) for parent in parents: - parent_block = new_blocks[parent.usage_id] - parent_block['fields']['children'].remove(usage_locator.usage_id) + parent_block = new_blocks[parent.block_id] + parent_block['fields']['children'].remove(usage_locator.block_id) parent_block['edit_info']['edited_on'] = datetime.datetime.now(UTC) parent_block['edit_info']['edited_by'] = user_id parent_block['edit_info']['previous_version'] = parent_block['edit_info']['update_version'] parent_block['edit_info']['update_version'] = new_id - # remove subtree - def remove_subtree(usage_id): - for child in new_blocks[usage_id]['fields'].get('children', []): + def remove_subtree(block_id): + """ + Remove the subtree rooted at block_id + """ + for child in new_blocks[block_id]['fields'].get('children', []): remove_subtree(child) - del new_blocks[usage_id] + del new_blocks[block_id] if delete_children: - remove_subtree(usage_locator.usage_id) + remove_subtree(usage_locator.block_id) else: - del new_blocks[usage_locator.usage_id] + del new_blocks[usage_locator.block_id] # update index if appropriate and structures self.db_connection.insert_structure(new_structure) @@ -1307,22 +1309,22 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): # migration where the old mongo published had pointers to privates pass - def descendants(self, block_map, usage_id, depth, descendent_map): + def descendants(self, block_map, block_id, depth, descendent_map): """ adds block and its descendants out to depth to descendent_map Depth specifies the number of levels of descendants to return (0 => this usage only, 1 => this usage and its children, etc...) A depth of None returns all descendants """ - if usage_id not in block_map: + if block_id not in block_map: return descendent_map - if usage_id not in descendent_map: - descendent_map[usage_id] = block_map[usage_id] + if block_id not in descendent_map: + descendent_map[block_id] = block_map[block_id] if depth is None or depth > 0: depth = depth - 1 if depth is not None else None - for child in block_map[usage_id]['fields'].get('children', []): + for child in block_map[block_id]['fields'].get('children', []): descendent_map = self.descendants(block_map, child, depth, descendent_map) @@ -1343,7 +1345,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): def internal_clean_children(self, course_locator): """ Only intended for rather low level methods to use. Goes through the children attrs of - each block removing any whose usage_id is not a member of the course. Does not generate + each block removing any whose block_id is not a member of the course. Does not generate a new version of the course but overwrites the existing one. :param course_locator: the course to clean @@ -1352,7 +1354,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): for block in original_structure['blocks'].itervalues(): if 'fields' in block and 'children' in block['fields']: block['fields']["children"] = [ - usage_id for usage_id in block['fields']["children"] if usage_id in original_structure['blocks'] + block_id for block_id in block['fields']["children"] if block_id in original_structure['blocks'] ] self.db_connection.update_structure(original_structure) # clear cache again b/c inheritance may be wrong over orphans @@ -1390,32 +1392,6 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): else: return criteria == target - def _xblock_lists_equal(self, lista, listb): - """ - Do the 2 lists refer to the same xblocks in the same order (presumes they're from the - same course) - - :param lista: - :param listb: - """ - if len(lista) != len(listb): - return False - for ele_a, ele_b in zip(lista, listb): - if ele_a != ele_b: - if self._usage_id(ele_a) != self._usage_id(ele_b): - return False - return True - - def _usage_id(self, xblock_or_id): - """ - arg is either an xblock or an id. If an xblock, get the usage_id from its location. Otherwise, return itself. - :param xblock_or_id: - """ - if isinstance(xblock_or_id, XModuleDescriptor): - return xblock_or_id.location.usage_id - else: - return xblock_or_id - def _get_index_if_valid(self, locator, force=False, continue_version=False): """ If the locator identifies a course and points to its draft (or plausibly its draft), @@ -1523,7 +1499,7 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): """ return SPLIT_MONGO_MODULESTORE_TYPE - def _new_structure(self, user_id, root_usage_id, + def _new_structure(self, user_id, root_block_id, root_category=None, block_fields=None, definition_id=None): """ Internal function: create a structure element with no previous version. Must provide the root id @@ -1533,13 +1509,13 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): new_id = ObjectId() if root_category is not None: blocks = { - root_usage_id: self._new_block(user_id, root_category, block_fields, definition_id, new_id) + root_block_id: self._new_block(user_id, root_category, block_fields, definition_id, new_id) } else: blocks = {} return { '_id': new_id, - 'root': root_usage_id, + 'root': root_block_id, 'previous_version': None, 'original_version': new_id, 'edited_by': user_id, @@ -1547,14 +1523,14 @@ class SplitMongoModuleStore(ModuleStoreWriteBase): 'blocks': blocks } - def _get_parents_from_structure(self, usage_id, structure): + def _get_parents_from_structure(self, block_id, structure): """ - Given a structure, find all of usage_id's parents in that structure + Given a structure, find all of block_id's parents in that structure """ items = [] for parent_id, value in structure['blocks'].iteritems(): for child_id in value['fields'].get('children', []): - if usage_id == child_id: + if block_id == child_id: items.append(parent_id) return items diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py b/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py index 18745bd856..7783681262 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_location_mapper.py @@ -75,10 +75,9 @@ class TestLocationMapper(unittest.TestCase): self.assertEqual(entry['prod_branch'], 'live') self.assertEqual(entry['block_map'], block_map) - - def translate_n_check(self, location, old_style_course_id, new_style_course_id, usage_id, branch, add_entry=False): + def translate_n_check(self, location, old_style_course_id, new_style_course_id, block_id, branch, add_entry=False): """ - Request translation, check course_id, usage_id, and branch + Request translation, check course_id, block_id, and branch """ prob_locator = loc_mapper().translate_location( old_style_course_id, @@ -87,7 +86,7 @@ class TestLocationMapper(unittest.TestCase): add_entry_if_missing=add_entry ) self.assertEqual(prob_locator.course_id, new_style_course_id) - self.assertEqual(prob_locator.usage_id, usage_id) + self.assertEqual(prob_locator.block_id, block_id) self.assertEqual(prob_locator.branch, branch) def test_translate_location_read_only(self): @@ -243,7 +242,7 @@ class TestLocationMapper(unittest.TestCase): new_style_course_id = '{}.geek_dept.{}.baz_run'.format(org, course) prob_locator = BlockUsageLocator( course_id=new_style_course_id, - usage_id='problem2', + block_id='problem2', branch='published' ) prob_location = loc_mapper().translate_locator_to_location(prob_locator) @@ -267,21 +266,21 @@ class TestLocationMapper(unittest.TestCase): self.assertEqual(prob_location, Location('i4x', org, course, 'course', 'baz_run', None)) # explicit branch prob_locator = BlockUsageLocator( - course_id=prob_locator.course_id, branch='draft', usage_id=prob_locator.usage_id + course_id=prob_locator.course_id, branch='draft', block_id=prob_locator.block_id ) prob_location = loc_mapper().translate_locator_to_location(prob_locator) # Even though the problem was set as draft, we always return revision=None to work # with old mongo/draft modulestores. self.assertEqual(prob_location, Location('i4x', org, course, 'problem', 'abc123', None)) prob_locator = BlockUsageLocator( - course_id=new_style_course_id, usage_id='problem2', branch='production' + course_id=new_style_course_id, block_id='problem2', branch='production' ) prob_location = loc_mapper().translate_locator_to_location(prob_locator) self.assertEqual(prob_location, Location('i4x', org, course, 'problem', 'abc123', None)) # same for chapter except chapter cannot be draft in old system chap_locator = BlockUsageLocator( course_id=new_style_course_id, - usage_id='chapter48f', + block_id='chapter48f', branch='production' ) chap_location = loc_mapper().translate_locator_to_location(chap_locator) @@ -291,7 +290,7 @@ class TestLocationMapper(unittest.TestCase): chap_location = loc_mapper().translate_locator_to_location(chap_locator) self.assertEqual(chap_location, Location('i4x', org, course, 'chapter', '48f23a10395384929234')) chap_locator = BlockUsageLocator( - course_id=new_style_course_id, usage_id='chapter48f', branch='production' + course_id=new_style_course_id, block_id='chapter48f', branch='production' ) chap_location = loc_mapper().translate_locator_to_location(chap_locator) self.assertEqual(chap_location, Location('i4x', org, course, 'chapter', '48f23a10395384929234')) @@ -300,7 +299,7 @@ class TestLocationMapper(unittest.TestCase): prob_locator2 = BlockUsageLocator( course_id=new_style_course_id, branch='draft', - usage_id='problem3' + block_id='problem3' ) prob_location = loc_mapper().translate_locator_to_location(prob_locator2) self.assertIsNone(prob_location, 'Found non-existent problem') @@ -325,7 +324,7 @@ class TestLocationMapper(unittest.TestCase): prob_locator = BlockUsageLocator( course_id=new_style_course_id, branch='production', - usage_id='problem3' + block_id='problem3' ) prob_location = loc_mapper().translate_locator_to_location(prob_locator) self.assertEqual(prob_location, Location('i4x', org, course, 'problem', 'abc123')) @@ -348,6 +347,25 @@ class TestLocationMapper(unittest.TestCase): reverted_location = loc_mapper().translate_locator_to_location(prob_locator) self.assertEqual(location, reverted_location) + def test_name_collision(self): + """ + Test dwim translation when the old name was not unique + """ + org = "myorg" + course = "another_course" + name = "running_again" + course_location = Location('i4x', org, course, 'course', name) + course_xlate = loc_mapper().translate_location(None, course_location, add_entry_if_missing=True) + self.assertEqual(course_location, loc_mapper().translate_locator_to_location(course_xlate)) + eponymous_block = course_location.replace(category='chapter') + chapter_xlate = loc_mapper().translate_location(None, eponymous_block, add_entry_if_missing=True) + self.assertEqual(course_location, loc_mapper().translate_locator_to_location(course_xlate)) + self.assertEqual(eponymous_block, loc_mapper().translate_locator_to_location(chapter_xlate)) + # and a non-existent one w/o add + eponymous_block = course_location.replace(category='problem') + with self.assertRaises(ItemNotFoundError): + chapter_xlate = loc_mapper().translate_location(None, eponymous_block, add_entry_if_missing=False) + #================================== # functions to mock existing services diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py b/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py index e9fc4ecf73..64a6ab2b5b 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py @@ -255,9 +255,9 @@ class LocatorTest(TestCase): """ course_id = 'mit.eecs-1' branch = 'foo' - usage_id = 'problem:with-colon~2' - testobj = BlockUsageLocator(course_id=course_id, branch=branch, usage_id=usage_id) - self.check_block_locn_fields(testobj, 'Cannot handle colon', course_id=course_id, branch=branch, block=usage_id) + block_id = 'problem:with-colon~2' + testobj = BlockUsageLocator(course_id=course_id, branch=branch, block_id=block_id) + self.check_block_locn_fields(testobj, 'Cannot handle colon', course_id=course_id, branch=branch, block=block_id) def test_repr(self): testurn = 'mit.eecs.6002x/' + BRANCH_PREFIX + 'published/' + BLOCK_PREFIX + 'HW3' @@ -275,7 +275,7 @@ class LocatorTest(TestCase): self.assertEqual(location, Locator.to_locator_or_location(list(location_tuple))) self.assertEqual(location, Locator.to_locator_or_location(location.dict())) - locator = BlockUsageLocator(course_id='foo.bar', branch='alpha', usage_id='deep') + locator = BlockUsageLocator(course_id='foo.bar', branch='alpha', block_id='deep') self.assertEqual(locator, Locator.to_locator_or_location(locator)) self.assertEqual(locator.as_course_locator(), Locator.to_locator_or_location(locator.as_course_locator())) self.assertEqual(location, Locator.to_locator_or_location(location.url())) @@ -347,4 +347,4 @@ class LocatorTest(TestCase): """ self.check_course_locn_fields(testobj, msg, version_guid, course_id, branch) - self.assertEqual(testobj.usage_id, block) + self.assertEqual(testobj.block_id, block) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py b/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py index 145aa77c23..877c7bcb36 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py @@ -87,14 +87,14 @@ class TestOrphan(unittest.TestCase): course_or_parent_locator = BlockUsageLocator( course_id=self.split_course_id, branch='draft', - usage_id=parent_name + block_id=parent_name ) else: course_or_parent_locator = CourseLocator( course_id='test_org.test_course.runid', branch='draft', ) - self.split_mongo.create_item(course_or_parent_locator, category, self.userid, usage_id=name, fields=fields) + self.split_mongo.create_item(course_or_parent_locator, category, self.userid, block_id=name, fields=fields) def _create_course(self): """ @@ -114,7 +114,7 @@ class TestOrphan(unittest.TestCase): fields.update(data) # split requires the course to be created separately from creating items self.split_mongo.create_course( - 'test_org', 'my course', self.userid, self.split_course_id, fields=fields, root_usage_id='runid' + 'test_org', 'my course', self.userid, self.split_course_id, fields=fields, root_block_id='runid' ) self.course_location = Location('i4x', 'test_org', 'test_course', 'course', 'runid') self.old_mongo.create_and_save_xmodule(self.course_location, data, metadata) @@ -150,9 +150,9 @@ class TestOrphan(unittest.TestCase): """ orphans = self.split_mongo.get_orphans(self.split_course_id, ['static_tab', 'about', 'course_info'], 'draft') self.assertEqual(len(orphans), 3, "Wrong # {}".format(orphans)) - location = BlockUsageLocator(course_id=self.split_course_id, branch='draft', usage_id='OrphanChapter') + location = BlockUsageLocator(course_id=self.split_course_id, branch='draft', block_id='OrphanChapter') self.assertIn(location, orphans) - location = BlockUsageLocator(course_id=self.split_course_id, branch='draft', usage_id='OrphanVert') + location = BlockUsageLocator(course_id=self.split_course_id, branch='draft', block_id='OrphanVert') self.assertIn(location, orphans) - location = BlockUsageLocator(course_id=self.split_course_id, branch='draft', usage_id='OrphanHtml') + location = BlockUsageLocator(course_id=self.split_course_id, branch='draft', block_id='OrphanHtml') self.assertIn(location, orphans) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py index 7cb98b26e7..e3fb461fbc 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -105,7 +105,7 @@ class SplitModuleTest(unittest.TestCase): matches the _id. """ for element in collection: - if element.location.usage_id == _id: + if element.location.block_id == _id: return element @@ -235,6 +235,16 @@ class SplitModuleCourseTests(SplitModuleTest): modulestore().get_course, CourseLocator(course_id='GreekHero', branch='published')) + def test_cache(self): + """ + Test that the mechanics of caching work. + """ + locator = CourseLocator(version_guid=self.GUID_D0) + course = modulestore().get_course(locator) + block_map = modulestore().cache_items(course.system, course.children, depth=3) + self.assertIn('chapter1', block_map) + self.assertIn('problem3_2', block_map) + def test_course_successors(self): """ get_course_successors(course_locator, version_history_depth=1) @@ -267,11 +277,11 @@ class SplitModuleItemTests(SplitModuleTest): ''' course_id = 'GreekHero' # positive tests of various forms - locator = BlockUsageLocator(version_guid=self.GUID_D1, usage_id='head12345') + locator = BlockUsageLocator(version_guid=self.GUID_D1, block_id='head12345') self.assertTrue(modulestore().has_item(course_id, locator), "couldn't find in %s" % self.GUID_D1) - locator = BlockUsageLocator(course_id='GreekHero', usage_id='head12345', branch='draft') + locator = BlockUsageLocator(course_id='GreekHero', block_id='head12345', branch='draft') self.assertTrue( modulestore().has_item(locator.course_id, locator), "couldn't find in 12345" @@ -280,7 +290,7 @@ class SplitModuleItemTests(SplitModuleTest): modulestore().has_item(locator.course_id, BlockUsageLocator( course_id=locator.course_id, branch='draft', - usage_id=locator.usage_id + block_id=locator.block_id )), "couldn't find in draft 12345" ) @@ -288,7 +298,7 @@ class SplitModuleItemTests(SplitModuleTest): modulestore().has_item(locator.course_id, BlockUsageLocator( course_id=locator.course_id, branch='published', - usage_id=locator.usage_id)), + block_id=locator.block_id)), "found in published 12345" ) locator.branch = 'draft' @@ -298,18 +308,18 @@ class SplitModuleItemTests(SplitModuleTest): ) # not a course obj - locator = BlockUsageLocator(course_id='GreekHero', usage_id='chapter1', branch='draft') + locator = BlockUsageLocator(course_id='GreekHero', block_id='chapter1', branch='draft') self.assertTrue( modulestore().has_item(locator.course_id, locator), "couldn't find chapter1" ) # in published course - locator = BlockUsageLocator(course_id="wonderful", usage_id="head23456", branch='draft') + locator = BlockUsageLocator(course_id="wonderful", block_id="head23456", branch='draft') self.assertTrue( modulestore().has_item( locator.course_id, - BlockUsageLocator(course_id=locator.course_id, usage_id=locator.usage_id, branch='published') + BlockUsageLocator(course_id=locator.course_id, block_id=locator.block_id, branch='published') ), "couldn't find in 23456" ) locator.branch = 'published' @@ -319,9 +329,9 @@ class SplitModuleItemTests(SplitModuleTest): # negative tests--not found # no such course or block course_id = 'GreekHero' - locator = BlockUsageLocator(course_id="doesnotexist", usage_id="head23456", branch='draft') + locator = BlockUsageLocator(course_id="doesnotexist", block_id="head23456", branch='draft') self.assertFalse(modulestore().has_item(course_id, locator)) - locator = BlockUsageLocator(course_id="wonderful", usage_id="doesnotexist", branch='draft') + locator = BlockUsageLocator(course_id="wonderful", block_id="doesnotexist", branch='draft') self.assertFalse(modulestore().has_item(course_id, locator)) # negative tests--insufficient specification @@ -336,7 +346,7 @@ class SplitModuleItemTests(SplitModuleTest): get_item(blocklocator) ''' # positive tests of various forms - locator = BlockUsageLocator(version_guid=self.GUID_D1, usage_id='head12345') + locator = BlockUsageLocator(version_guid=self.GUID_D1, block_id='head12345') block = modulestore().get_item(locator) self.assertIsInstance(block, CourseDescriptor) # get_instance just redirects to get_item, ignores course_id @@ -355,7 +365,7 @@ class SplitModuleItemTests(SplitModuleTest): block.grade_cutoffs, {"Pass": 0.45}, ) - locator = BlockUsageLocator(course_id='GreekHero', usage_id='head12345', branch='draft') + locator = BlockUsageLocator(course_id='GreekHero', block_id='head12345', branch='draft') verify_greek_hero(modulestore().get_item(locator)) # get_instance just redirects to get_item, ignores course_id verify_greek_hero(modulestore().get_instance("course_id", locator)) @@ -364,7 +374,7 @@ class SplitModuleItemTests(SplitModuleTest): self.assertRaises(ItemNotFoundError, modulestore().get_item, BlockUsageLocator(course_id=locator.as_course_locator(), - usage_id=locator.usage_id, + block_id=locator.block_id, branch='published')) locator.branch = 'draft' self.assertIsInstance( @@ -374,7 +384,7 @@ class SplitModuleItemTests(SplitModuleTest): def test_get_non_root(self): # not a course obj - locator = BlockUsageLocator(course_id='GreekHero', usage_id='chapter1', branch='draft') + locator = BlockUsageLocator(course_id='GreekHero', block_id='chapter1', branch='draft') block = modulestore().get_item(locator) self.assertEqual(block.location.course_id, "GreekHero") self.assertEqual(block.category, 'chapter') @@ -383,7 +393,7 @@ class SplitModuleItemTests(SplitModuleTest): self.assertEqual(block.edited_by, "testassist@edx.org") # in published course - locator = BlockUsageLocator(course_id="wonderful", usage_id="head23456", branch='published') + locator = BlockUsageLocator(course_id="wonderful", block_id="head23456", branch='published') self.assertIsInstance( modulestore().get_item(locator), CourseDescriptor @@ -391,10 +401,10 @@ class SplitModuleItemTests(SplitModuleTest): # negative tests--not found # no such course or block - locator = BlockUsageLocator(course_id="doesnotexist", usage_id="head23456", branch='draft') + locator = BlockUsageLocator(course_id="doesnotexist", block_id="head23456", branch='draft') with self.assertRaises(ItemNotFoundError): modulestore().get_item(locator) - locator = BlockUsageLocator(course_id="wonderful", usage_id="doesnotexist", branch='draft') + locator = BlockUsageLocator(course_id="wonderful", block_id="doesnotexist", branch='draft') with self.assertRaises(ItemNotFoundError): modulestore().get_item(locator) @@ -457,22 +467,22 @@ class SplitModuleItemTests(SplitModuleTest): matches = modulestore().get_items(locator, qualifiers={'fields': {'children': 'chapter2'}}) self.assertEqual(len(matches), 1) - self.assertEqual(matches[0].location.usage_id, 'head12345') + self.assertEqual(matches[0].location.block_id, 'head12345') def test_get_parents(self): ''' - get_parent_locations(locator, [usage_id], [branch]): [BlockUsageLocator] + get_parent_locations(locator, [block_id], [branch]): [BlockUsageLocator] ''' - locator = BlockUsageLocator(course_id="GreekHero", branch='draft', usage_id='chapter1') + locator = BlockUsageLocator(course_id="GreekHero", branch='draft', block_id='chapter1') parents = modulestore().get_parent_locations(locator) self.assertEqual(len(parents), 1) - self.assertEqual(parents[0].usage_id, 'head12345') + self.assertEqual(parents[0].block_id, 'head12345') self.assertEqual(parents[0].course_id, "GreekHero") - locator.usage_id = 'chapter2' + locator.block_id = 'chapter2' parents = modulestore().get_parent_locations(locator) self.assertEqual(len(parents), 1) - self.assertEqual(parents[0].usage_id, 'head12345') - locator.usage_id = 'nosuchblock' + self.assertEqual(parents[0].block_id, 'head12345') + locator.block_id = 'nosuchblock' parents = modulestore().get_parent_locations(locator) self.assertEqual(len(parents), 0) @@ -480,7 +490,7 @@ class SplitModuleItemTests(SplitModuleTest): """ Test the existing get_children method on xdescriptors """ - locator = BlockUsageLocator(course_id="GreekHero", usage_id="head12345", branch='draft') + locator = BlockUsageLocator(course_id="GreekHero", block_id="head12345", branch='draft') block = modulestore().get_item(locator) children = block.get_children() expected_ids = [ @@ -488,8 +498,8 @@ class SplitModuleItemTests(SplitModuleTest): ] for child in children: self.assertEqual(child.category, "chapter") - self.assertIn(child.location.usage_id, expected_ids) - expected_ids.remove(child.location.usage_id) + self.assertIn(child.location.block_id, expected_ids) + expected_ids.remove(child.location.block_id) self.assertEqual(len(expected_ids), 0) @@ -551,7 +561,7 @@ class TestItemCrud(SplitModuleTest): # check that block does not exist in previous version locator = BlockUsageLocator( version_guid=premod_course.location.version_guid, - usage_id=new_module.location.usage_id + block_id=new_module.location.block_id ) self.assertRaises(ItemNotFoundError, modulestore().get_item, locator) @@ -559,7 +569,7 @@ class TestItemCrud(SplitModuleTest): """ Test create_item w/ specifying the parent of the new item """ - locator = BlockUsageLocator(course_id="wonderful", usage_id="head23456", branch='draft') + locator = BlockUsageLocator(course_id="wonderful", block_id="head23456", branch='draft') premod_course = modulestore().get_course(locator) category = 'chapter' new_module = modulestore().create_item( @@ -570,16 +580,16 @@ class TestItemCrud(SplitModuleTest): # check that course version changed and course's previous is the other one self.assertNotEqual(new_module.location.version_guid, premod_course.location.version_guid) parent = modulestore().get_item(locator) - self.assertIn(new_module.location.usage_id, parent.children) + self.assertIn(new_module.location.block_id, parent.children) self.assertEqual(str(new_module.definition_locator.definition_id), "cd00000000000000dddd0022") def test_unique_naming(self): """ - Check that 2 modules of same type get unique usage_ids. Also check that if creation provides + Check that 2 modules of same type get unique block_ids. Also check that if creation provides a definition id and new def data that it branches the definition in the db. Actually, this tries to test all create_item features not tested above. """ - locator = BlockUsageLocator(course_id="contender", usage_id="head345679", branch='draft') + locator = BlockUsageLocator(course_id="contender", block_id="head345679", branch='draft') category = 'problem' premod_time = datetime.datetime.now(UTC) - datetime.timedelta(seconds=1) new_payload = "empty" @@ -595,9 +605,9 @@ class TestItemCrud(SplitModuleTest): ) # check that course version changed and course's previous is the other one parent = modulestore().get_item(locator) - self.assertNotEqual(new_module.location.usage_id, another_module.location.usage_id) - self.assertIn(new_module.location.usage_id, parent.children) - self.assertIn(another_module.location.usage_id, parent.children) + self.assertNotEqual(new_module.location.block_id, another_module.location.block_id) + self.assertIn(new_module.location.block_id, parent.children) + self.assertIn(another_module.location.block_id, parent.children) self.assertEqual(new_module.data, new_payload) self.assertEqual(another_module.data, another_payload) # check definition histories @@ -641,13 +651,13 @@ class TestItemCrud(SplitModuleTest): self.assertEqual(refetch_course.update_version, course_block_update_version) refetch_index_history_info = modulestore().get_course_history_info(refetch_course.location) self.assertEqual(refetch_index_history_info, index_history_info) - self.assertIn(new_ele.location.usage_id, refetch_course.children) + self.assertIn(new_ele.location.block_id, refetch_course.children) # try to create existing item with self.assertRaises(DuplicateItemError): _fail = modulestore().create_item( new_course.location, 'chapter', user, - usage_id=new_ele.location.usage_id, + block_id=new_ele.location.block_id, fields={'display_name': 'chapter 2'}, continue_version=True ) @@ -678,7 +688,7 @@ class TestItemCrud(SplitModuleTest): # add new child to old parent in continued (leave off version_guid) course_module_locator = BlockUsageLocator( course_id=new_course.location.course_id, - usage_id=new_course.location.usage_id, + block_id=new_course.location.block_id, branch=new_course.location.branch ) new_ele = modulestore().create_item( @@ -691,7 +701,7 @@ class TestItemCrud(SplitModuleTest): # check children, previous_version refetch_course = modulestore().get_course(versionless_course_locator) - self.assertIn(new_ele.location.usage_id, refetch_course.children) + self.assertIn(new_ele.location.block_id, refetch_course.children) self.assertEqual(refetch_course.previous_version, course_block_update_version) self.assertEqual(refetch_course.update_version, transaction_guid) @@ -699,7 +709,7 @@ class TestItemCrud(SplitModuleTest): """ test updating an items metadata ensuring the definition doesn't version but the course does if it should """ - locator = BlockUsageLocator(course_id="GreekHero", usage_id="problem3_2", branch='draft') + locator = BlockUsageLocator(course_id="GreekHero", block_id="problem3_2", branch='draft') problem = modulestore().get_item(locator) pre_def_id = problem.definition_locator.definition_id pre_version_guid = problem.location.version_guid @@ -718,7 +728,7 @@ class TestItemCrud(SplitModuleTest): # refetch to ensure original didn't change original_location = BlockUsageLocator( version_guid=pre_version_guid, - usage_id=problem.location.usage_id + block_id=problem.location.block_id ) problem = modulestore().get_item(original_location) self.assertNotEqual(problem.max_attempts, 4, "original changed") @@ -737,7 +747,7 @@ class TestItemCrud(SplitModuleTest): """ test updating an item's children ensuring the definition doesn't version but the course does if it should """ - locator = BlockUsageLocator(course_id="GreekHero", usage_id="chapter3", branch='draft') + locator = BlockUsageLocator(course_id="GreekHero", block_id="chapter3", branch='draft') block = modulestore().get_item(locator) pre_def_id = block.definition_locator.definition_id pre_version_guid = block.location.version_guid @@ -752,7 +762,7 @@ class TestItemCrud(SplitModuleTest): self.assertNotEqual(updated_problem.location.version_guid, pre_version_guid) self.assertEqual(updated_problem.children, block.children) self.assertNotIn(moved_child, updated_problem.children) - locator.usage_id = "chapter1" + locator.block_id = "chapter1" other_block = modulestore().get_item(locator) other_block.children.append(moved_child) other_block.save() # decache model changes @@ -763,7 +773,7 @@ class TestItemCrud(SplitModuleTest): """ test updating an item's definition: ensure it gets versioned as well as the course getting versioned """ - locator = BlockUsageLocator(course_id="GreekHero", usage_id="head12345", branch='draft') + locator = BlockUsageLocator(course_id="GreekHero", block_id="head12345", branch='draft') block = modulestore().get_item(locator) pre_def_id = block.definition_locator.definition_id pre_version_guid = block.location.version_guid @@ -781,7 +791,7 @@ class TestItemCrud(SplitModuleTest): Test updating metadata, children, and definition in a single call ensuring all the versioning occurs """ # first add 2 children to the course for the update to manipulate - locator = BlockUsageLocator(course_id="contender", usage_id="head345679", branch='draft') + locator = BlockUsageLocator(course_id="contender", block_id="head345679", branch='draft') category = 'problem' new_payload = "empty" modulestore().create_item( @@ -823,7 +833,7 @@ class TestItemCrud(SplitModuleTest): 'deleting_user') reusable_location = BlockUsageLocator( course_id=course.location.course_id, - usage_id=course.location.usage_id, + block_id=course.location.block_id, branch='draft') # delete a leaf @@ -832,12 +842,12 @@ class TestItemCrud(SplitModuleTest): new_course_loc = modulestore().delete_item(locn_to_del, 'deleting_user', delete_children=False) deleted = BlockUsageLocator(course_id=reusable_location.course_id, branch=reusable_location.branch, - usage_id=locn_to_del.usage_id) + block_id=locn_to_del.block_id) self.assertFalse(modulestore().has_item(reusable_location.course_id, deleted)) self.assertRaises(VersionConflictError, modulestore().has_item, reusable_location.course_id, locn_to_del) locator = BlockUsageLocator( version_guid=locn_to_del.version_guid, - usage_id=locn_to_del.usage_id + block_id=locn_to_del.block_id ) self.assertTrue(modulestore().has_item(reusable_location.course_id, locator)) self.assertNotEqual(new_course_loc.version_guid, course.location.version_guid) @@ -854,10 +864,10 @@ class TestItemCrud(SplitModuleTest): BlockUsageLocator( course_id=node_loc.course_id, branch=node_loc.branch, - usage_id=node.location.usage_id))) + block_id=node.location.block_id))) locator = BlockUsageLocator( version_guid=node.location.version_guid, - usage_id=node.location.usage_id) + block_id=node.location.block_id) self.assertTrue(modulestore().has_item(reusable_location.course_id, locator)) if node.has_children: for sub in node.get_children(): @@ -868,7 +878,7 @@ class TestItemCrud(SplitModuleTest): course = modulestore().create_course('nihilx', 'deletion', 'deleting_user') root = BlockUsageLocator( course_id=course.location.course_id, - usage_id=course.location.usage_id, + block_id=course.location.block_id, branch='draft') for _ in range(4): self.create_subtree_for_deletion(root, ['chapter', 'vertical', 'problem']) @@ -878,7 +888,7 @@ class TestItemCrud(SplitModuleTest): if not category_queue: return node = modulestore().create_item(parent, category_queue[0], 'deleting_user') - node_loc = BlockUsageLocator(parent.as_course_locator(), usage_id=node.location.usage_id) + node_loc = BlockUsageLocator(parent.as_course_locator(), block_id=node.location.block_id) for _ in range(4): self.create_subtree_for_deletion(node_loc, category_queue[1:]) @@ -971,7 +981,7 @@ class TestCourseCreation(SplitModuleTest): self.assertFalse( modulestore().has_item(new_draft_locator.course_id, BlockUsageLocator( original_locator, - usage_id=new_item.location.usage_id + block_id=new_item.location.block_id )) ) @@ -1051,9 +1061,9 @@ class TestCourseCreation(SplitModuleTest): user = random.getrandbits(32) new_course = modulestore().create_course( 'test_org', 'test_transaction', user, - root_usage_id='top', root_category='chapter' + root_block_id='top', root_category='chapter' ) - self.assertEqual(new_course.location.usage_id, 'top') + self.assertEqual(new_course.location.block_id, 'top') self.assertEqual(new_course.category, 'chapter') # look at db to verify db_structure = modulestore().db_connection.get_structure( @@ -1076,11 +1086,11 @@ class TestInheritance(SplitModuleTest): """ # Note, not testing value where defined (course) b/c there's no # defined accessor for it on CourseDescriptor. - locator = BlockUsageLocator(course_id="GreekHero", usage_id="problem3_2", branch='draft') + locator = BlockUsageLocator(course_id="GreekHero", block_id="problem3_2", branch='draft') node = modulestore().get_item(locator) # inherited self.assertEqual(node.graceperiod, datetime.timedelta(hours=2)) - locator = BlockUsageLocator(course_id="GreekHero", usage_id="problem1", branch='draft') + locator = BlockUsageLocator(course_id="GreekHero", block_id="problem1", branch='draft') node = modulestore().get_item(locator) # overridden self.assertEqual(node.graceperiod, datetime.timedelta(hours=4)) @@ -1117,24 +1127,24 @@ class TestPublish(SplitModuleTest): expected.remove("chapter1") # check that it's not in published course with self.assertRaises(ItemNotFoundError): - modulestore().get_item(self._usage(dest_course, new_module.location.usage_id)) + modulestore().get_item(self._usage(dest_course, new_module.location.block_id)) # publish it - modulestore().xblock_publish(self.user, source_course, dest_course, [new_module.location.usage_id], None) - expected.append(new_module.location.usage_id) + modulestore().xblock_publish(self.user, source_course, dest_course, [new_module.location.block_id], None) + expected.append(new_module.location.block_id) # check that it is in the published course and that its parent is the chapter - pub_module = modulestore().get_item(self._usage(dest_course, new_module.location.usage_id)) + pub_module = modulestore().get_item(self._usage(dest_course, new_module.location.block_id)) self.assertEqual( - modulestore().get_parent_locations(pub_module.location)[0].usage_id, "chapter1" + modulestore().get_parent_locations(pub_module.location)[0].block_id, "chapter1" ) # ensure intentionally orphaned blocks work (e.g., course_info) new_module = modulestore().create_item( - source_course, "course_info", self.user, usage_id="handouts" + source_course, "course_info", self.user, block_id="handouts" ) # publish it - modulestore().xblock_publish(self.user, source_course, dest_course, [new_module.location.usage_id], None) - expected.append(new_module.location.usage_id) + modulestore().xblock_publish(self.user, source_course, dest_course, [new_module.location.block_id], None) + expected.append(new_module.location.block_id) # check that it is in the published course (no error means it worked) - pub_module = modulestore().get_item(self._usage(dest_course, new_module.location.usage_id)) + pub_module = modulestore().get_item(self._usage(dest_course, new_module.location.block_id)) self._check_course( source_course, dest_course, expected, ["chapter2", "chapter3", "problem1", "problem3_2"] ) @@ -1200,7 +1210,7 @@ class TestPublish(SplitModuleTest): """ Generate a BlockUsageLocator for the combo of the course location and block id """ - return BlockUsageLocator(course_id=course_loc.course_id, branch=course_loc.branch, usage_id=block_id) + return BlockUsageLocator(course_id=course_loc.course_id, branch=course_loc.branch, block_id=block_id) def _compare_children(self, source_children, dest_children, unexpected): """ diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 8f8674b615..6f5577b2b1 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -168,7 +168,7 @@ class XModuleMixin(XBlockMixin): if isinstance(self.location, Location): return self.location.name elif isinstance(self.location, BlockUsageLocator): - return self.location.usage_id + return self.location.block_id else: raise InsufficientSpecificationError()