From 2614be91a0bf7ebcdd4c51c61b3f3ed03d5c7407 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Tue, 28 Oct 2014 21:08:24 -0700 Subject: [PATCH 01/12] Use opaque_keys version with library-related locators --- requirements/edx/github.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 0064263d2102..c38c1d3429ba 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -30,7 +30,7 @@ -e git+https://github.com/edx-solutions/django-splash.git@7579d052afcf474ece1239153cffe1c89935bc4f#egg=django-splash -e git+https://github.com/edx/acid-block.git@df1a7f0cae46567c251d507b8c72168aed8ec042#egg=acid-xblock -e git+https://github.com/edx/edx-ora2.git@release-2014-10-27T19.33#egg=edx-ora2 --e git+https://github.com/edx/opaque-keys.git@0.1.2#egg=opaque-keys +-e git+https://github.com/edx/opaque-keys.git@b12401384921c075e5a4ed7aedc3bea57f56ec32#egg=opaque-keys -e git+https://github.com/edx/ease.git@97de68448e5495385ba043d3091f570a699d5b5f#egg=ease -e git+https://github.com/edx/i18n-tools.git@56f048af9b6868613c14aeae760548834c495011#egg=i18n-tools -e git+https://github.com/edx/edx-oauth2-provider.git@0.3.1#egg=oauth2-provider From 1c7220e0668a48a16031b4032a069d271f99d1db Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Tue, 28 Oct 2014 21:53:57 -0700 Subject: [PATCH 02/12] Split mongo support for libraries --- .../xmodule/xmodule/modulestore/__init__.py | 1 + .../lib/xmodule/xmodule/modulestore/mixed.py | 71 ++++++ .../xmodule/xmodule/modulestore/mongo/base.py | 4 +- .../split_mongo/caching_descriptor_system.py | 10 +- .../xmodule/modulestore/split_mongo/split.py | 210 ++++++++++++------ .../modulestore/split_mongo/split_draft.py | 30 ++- 6 files changed, 245 insertions(+), 81 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 84968eff6b74..a09d9f75d39b 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -82,6 +82,7 @@ class BranchName(object): """ draft = 'draft-branch' published = 'published-branch' + library = 'library' class UserID(object): """ diff --git a/common/lib/xmodule/xmodule/modulestore/mixed.py b/common/lib/xmodule/xmodule/modulestore/mixed.py index b284c6bddda8..6688e4c5b0c5 100644 --- a/common/lib/xmodule/xmodule/modulestore/mixed.py +++ b/common/lib/xmodule/xmodule/modulestore/mixed.py @@ -13,6 +13,7 @@ from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey, AssetKey +from opaque_keys.edx.locator import LibraryLocator from opaque_keys.edx.locations import SlashSeparatedCourseKey from xmodule.assetstore import AssetMetadata @@ -259,6 +260,23 @@ def get_courses(self, **kwargs): courses[course_id] = course return courses.values() + @strip_key + def get_libraries(self, **kwargs): + ''' + Returns a list containing the top level XModuleDescriptors of the libraries in this modulestore. + ''' + libraries = {} + for store in self.modulestores: + if not hasattr(store, 'get_libraries'): + continue + # filter out ones which were fetched from earlier stores but locations may not be == + for course in store.get_libraries(**kwargs): + course_id = self._clean_course_id_for_mapping(course.location) + if course_id not in libraries: + # course is indeed unique. save it in result + libraries[course_id] = course + return libraries.values() + def make_course_key(self, org, course, run): """ Return a valid :class:`~opaque_keys.edx.keys.CourseKey` for this modulestore @@ -290,6 +308,31 @@ def get_course(self, course_key, depth=0, **kwargs): except ItemNotFoundError: return None + @strip_key + def get_library(self, library_key, depth=0, **kwargs): + """ + returns the library block associated with the given key. If no such library exists, + it returns None + + :param library_key: must be a LibraryLocator + """ + assert(isinstance(library_key, LibraryLocator)) + store = self._get_modulestore_for_courseid(library_key) + if not hasattr(store, 'get_library'): + return None # This key seems invalid since its modulestore doesn't support libraries + try: + return store.get_library(library_key, depth=depth, **kwargs) + except ItemNotFoundError: + return None + + def get_courselike(self, key, depth=0, **kwargs): + """ + Fetch a course or a library + """ + if isinstance(key, LibraryLocator): + return self.get_library(key, depth, **kwargs) + return self.get_course(key, depth, **kwargs) + @strip_key def has_course(self, course_id, ignore_case=False, **kwargs): """ @@ -507,6 +550,34 @@ def create_course(self, org, course, run, user_id, **kwargs): return course + @strip_key + def create_library(self, org, library, user_id, fields, **kwargs): + """ + Creates and returns a new library. + + Args: + org (str): the organization that owns the course + library (str): the code/number/name of the library + user_id: id of the user creating the course + fields (dict): Fields to set on the course at initialization - e.g. display_name + kwargs: Any optional arguments understood by a subset of modulestores to customize instantiation + + Returns: a LibraryDescriptor + """ + # first make sure an existing course/lib doesn't already exist in the mapping + lib_key = LibraryLocator(org=org, library=library) + if lib_key in self.mappings: + raise DuplicateCourseError(lib_key, lib_key) + + # create the library + store = self._verify_modulestore_support(None, 'create_library') + library = store.create_library(org, library, user_id, fields, **kwargs) + + # add new library to the mapping + self.mappings[lib_key] = store + + return library + @strip_key def clone_course(self, source_course_id, dest_course_id, user_id, fields=None, **kwargs): """ diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index f1c66545b79c..4cded192f686 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -33,7 +33,7 @@ from opaque_keys.edx.keys import UsageKey, CourseKey, AssetKey from opaque_keys.edx.locations import Location from opaque_keys.edx.locations import SlashSeparatedCourseKey -from opaque_keys.edx.locator import CourseLocator +from opaque_keys.edx.locator import CourseLocator, LibraryLocator from xblock.core import XBlock from xblock.exceptions import InvalidScopeError @@ -871,6 +871,8 @@ def has_course(self, course_key, ignore_case=False, **kwargs): otherwise, do a case sensitive search """ assert(isinstance(course_key, CourseKey)) + if isinstance(course_key, LibraryLocator): + return None # Libraries require split mongo course_key = self.fill_in_run(course_key) location = course_key.make_usage_key('course', course_key.run) if ignore_case: 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 73159bf22165..7881be1c2f0d 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 @@ -4,7 +4,7 @@ from lazy import lazy from xblock.runtime import KvsFieldData from xblock.fields import ScopeIds -from opaque_keys.edx.locator import BlockUsageLocator, LocalId, CourseLocator, DefinitionLocator +from opaque_keys.edx.locator import BlockUsageLocator, LocalId, CourseLocator, LibraryLocator, DefinitionLocator from xmodule.mako_module import MakoDescriptorSystem from xmodule.error_module import ErrorDescriptor from xmodule.errortracker import exc_info_to_str @@ -19,6 +19,8 @@ log = logging.getLogger(__name__) new_contract('BlockUsageLocator', BlockUsageLocator) +new_contract('CourseLocator', CourseLocator) +new_contract('LibraryLocator', LibraryLocator) new_contract('BlockKey', BlockKey) new_contract('CourseEnvelope', CourseEnvelope) @@ -115,7 +117,7 @@ def _load_item(self, usage_key, course_entry_override=None, **kwargs): self.modulestore.cache_block(course_key, version_guid, block_key, block) return block - @contract(block_key=BlockKey, course_key=CourseLocator) + @contract(block_key=BlockKey, course_key="CourseLocator | LibraryLocator") def get_module_data(self, block_key, course_key): """ Get block from module_data adding it to module_data if it's not already there but is in the structure @@ -178,8 +180,8 @@ def xblock_from_json( if definition_id is None: definition_id = LocalId() - block_locator = BlockUsageLocator( - course_key, + # Construct the Block Usage Locator: + block_locator = course_key.make_usage_key( block_type=block_key.type, block_id=block_key.id, ) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index f6fa21271fc1..9ea559f2b6e7 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -65,8 +65,7 @@ from xblock.fields import Scope, Reference, ReferenceList, ReferenceValueDict from xmodule.errortracker import null_error_tracker from opaque_keys.edx.locator import ( - BlockUsageLocator, DefinitionLocator, CourseLocator, VersionTree, - LocalId, + BlockUsageLocator, DefinitionLocator, CourseLocator, LibraryLocator, VersionTree, LocalId, ) from xmodule.modulestore.exceptions import InsufficientSpecificationError, VersionConflictError, DuplicateItemError, \ DuplicateCourseError @@ -190,8 +189,8 @@ def _get_bulk_ops_record(self, course_key, ignore_case=False): if course_key is None: return self._bulk_ops_record_type() - if not isinstance(course_key, CourseLocator): - raise TypeError(u'{!r} is not a CourseLocator'.format(course_key)) + if not isinstance(course_key, (CourseLocator, LibraryLocator)): + raise TypeError(u'{!r} is not a CourseLocator or LibraryLocator'.format(course_key)) # handle version_guid based retrieval locally if course_key.org is None or course_key.course is None or course_key.run is None: return self._active_bulk_ops.records[ @@ -207,8 +206,8 @@ def _clear_bulk_ops_record(self, course_key): """ Clear the record for this course """ - if not isinstance(course_key, CourseLocator): - raise TypeError('{!r} is not a CourseLocator'.format(course_key)) + if not isinstance(course_key, (CourseLocator, LibraryLocator)): + raise TypeError('{!r} is not a CourseLocator or LibraryLocator'.format(course_key)) if course_key.org and course_key.course and course_key.run: del self._active_bulk_ops.records[course_key.replace(branch=None, version_guid=None)] @@ -776,19 +775,10 @@ def _lookup_course(self, course_key): # add it in the envelope for the structure. return CourseEnvelope(course_key.replace(version_guid=version_guid), entry) - @autoretry_read() - def get_courses(self, branch, **kwargs): - ''' - Returns a list of course descriptors matching any given qualifiers. - - qualifiers should be a dict of keywords matching the db fields or any - legal query for mongo to use against the active_versions collection. - - Note, this is to find the current head of the named branch type. - To get specific versions via guid use get_course. - - :param branch: the branch for which to return courses. - ''' + def _get_structures_for_branch(self, branch): + """ + Internal generator for fetching lists of courses, libraries, etc. + """ matching_indexes = self.find_matching_course_indexes(branch) # collect ids and then query for those @@ -800,14 +790,27 @@ def get_courses(self, branch, **kwargs): id_version_map[version_guid] = course_index if not version_guids: - return [] + return + + for entry in self.find_structures_by_id(version_guids): + yield entry, id_version_map[entry['_id']] + + @autoretry_read() + def get_courses(self, branch, **kwargs): + ''' + Returns a list of course descriptors matching any given qualifiers. + + qualifiers should be a dict of keywords matching the db fields or any + legal query for mongo to use against the active_versions collection. - matching_structures = self.find_structures_by_id(version_guids) + Note, this is to find the current head of the named branch type. + To get specific versions via guid use get_course. + :param branch: the branch for which to return courses. + ''' # get the blocks for each course index (s/b the root) result = [] - for entry in matching_structures: - course_info = id_version_map[entry['_id']] + for entry, course_info in self._get_structures_for_branch(branch): envelope = CourseEnvelope( CourseLocator( org=course_info['org'], @@ -823,6 +826,28 @@ def get_courses(self, branch, **kwargs): result.append(course_list[0]) return result + def get_libraries(self, branch="library", **kwargs): + ''' + Returns a list of "library" root blocks matching any given qualifiers. + + TODO: better way of identifying library index entry vs. course index entry. + ''' + result = [] + for entry, course_info in self._get_structures_for_branch(branch): + envelope = CourseEnvelope( + LibraryLocator( + org=course_info['org'], + library=course_info['course'], + branch=branch, + ), + entry + ) + root = entry['root'] + course_list = self._load_items(envelope, [root], 0, lazy=True, **kwargs) + if not isinstance(course_list[0], ErrorDescriptor): + result.append(course_list[0]) + return result + def make_course_key(self, org, course, run): """ Return a valid :class:`~opaque_keys.edx.keys.CourseKey` for this modulestore @@ -845,6 +870,28 @@ def get_course(self, course_id, depth=0, **kwargs): result = self._load_items(course_entry, [root], depth, lazy=True, **kwargs) return result[0] + def get_library(self, library_id, depth=0, **kwargs): + ''' + Gets the 'library' root block for the library identified by the locator + ''' + if not isinstance(library_id, LibraryLocator): + # The supplied CourseKey is of the wrong type, so it can't possibly be stored in this modulestore. + raise ItemNotFoundError(library_id) + + course_entry = self._lookup_course(library_id) + root = course_entry.structure['root'] + result = self._load_items(course_entry, [root], depth, lazy=True, **kwargs) + return result[0] + + def get_courselike(self, locator, depth=0, **kwargs): + """ + Gets a course or a library. + """ + if isinstance(locator, LibraryLocator): + return self.get_library(locator, depth, **kwargs) + else: + return self.get_course(locator, depth, **kwargs) + def has_course(self, course_id, ignore_case=False, **kwargs): ''' Does this course exist in this modulestore. This method does not verify that the branch &/or @@ -1227,20 +1274,28 @@ def needs_saved(): new_def_data = self._serialize_fields(old_definition['block_type'], new_def_data) if needs_saved(): - # Do a deep copy so that we don't corrupt the cached version of the definition - new_definition = copy.deepcopy(old_definition) - new_definition['_id'] = ObjectId() - new_definition['fields'] = new_def_data - new_definition['edit_info']['edited_by'] = user_id - new_definition['edit_info']['edited_on'] = datetime.datetime.now(UTC) - # previous version id - new_definition['edit_info']['previous_version'] = definition_locator.definition_id - new_definition['schema_version'] = self.SCHEMA_VERSION - self.update_definition(course_key, new_definition) - return DefinitionLocator(new_definition['block_type'], new_definition['_id']), True + definition_locator = self._update_definition_from_data(course_key, old_definition, new_def_data, user_id) + return definition_locator, True else: return definition_locator, False + def _update_definition_from_data(self, course_key, old_definition, new_def_data, user_id): + """ + Update the persisted version of the given definition and return the + locator of the new definition. Does not check if data differs from the + previous version. + """ + new_definition = copy.deepcopy(old_definition) + new_definition['_id'] = ObjectId() + new_definition['fields'] = new_def_data + new_definition['edit_info']['edited_by'] = user_id + new_definition['edit_info']['edited_on'] = datetime.datetime.now(UTC) + # previous version id + new_definition['edit_info']['previous_version'] = old_definition['_id'] + new_definition['schema_version'] = self.SCHEMA_VERSION + self.update_definition(course_key, new_definition) + return DefinitionLocator(new_definition['block_type'], new_definition['_id']) + def _generate_block_key(self, course_blocks, category): """ Generate a somewhat readable block id unique w/in this course using the category @@ -1325,7 +1380,7 @@ def create_item( # persist the definition if persisted != passed if (definition_locator is None or isinstance(definition_locator.definition_id, LocalId)): definition_locator = self.create_definition_from_data(course_key, new_def_data, block_type, user_id) - elif new_def_data is not None: + elif new_def_data: definition_locator, _ = self.update_definition_from_data(course_key, definition_locator, new_def_data, user_id) # copy the structure and modify the new one @@ -1494,6 +1549,19 @@ def create_course( assert master_branch is not None # check course and run's uniqueness locator = CourseLocator(org=org, course=course, run=run, branch=master_branch) + return self._create_courselike( + locator, user_id, master_branch, fields, versions_dict, + search_targets, root_category, root_block_id, **kwargs + ) + + def _create_courselike( + self, locator, user_id, master_branch, fields=None, + versions_dict=None, search_targets=None, root_category='course', + root_block_id=None, **kwargs + ): + """ + Internal code for creating a course or library + """ index = self.get_course_index(locator) if index is not None: raise DuplicateCourseError(locator, index) @@ -1508,20 +1576,7 @@ def create_course( # if building a wholly new structure if versions_dict is None or master_branch not in versions_dict: # create new definition and structure - definition_id = ObjectId() - definition_entry = { - '_id': definition_id, - 'block_type': root_category, - 'fields': definition_fields, - 'edit_info': { - 'edited_by': user_id, - 'edited_on': datetime.datetime.now(UTC), - 'previous_version': None, - 'original_version': definition_id, - }, - 'schema_version': self.SCHEMA_VERSION, - } - self.update_definition(locator, definition_entry) + definition_id = self.create_definition_from_data(locator, definition_fields, root_category, user_id).definition_id draft_structure = self._new_structure( user_id, @@ -1549,15 +1604,11 @@ def create_course( if block_fields is not None: root_block['fields'].update(self._serialize_fields(root_category, block_fields)) if definition_fields is not None: - definition = copy.deepcopy(self.get_definition(locator, root_block['definition'])) - definition['fields'].update(definition_fields) - definition['edit_info']['previous_version'] = definition['_id'] - definition['edit_info']['edited_by'] = user_id - definition['edit_info']['edited_on'] = datetime.datetime.now(UTC) - definition['_id'] = ObjectId() - definition['schema_version'] = self.SCHEMA_VERSION - self.update_definition(locator, definition) - root_block['definition'] = definition['_id'] + old_def = self.get_definition(locator, root_block['definition']) + new_fields = old_def['fields'] + new_fields.update(definition_fields) + definition_id = self._update_definition_from_data(locator, old_def, new_fields, user_id).definition_id + root_block['definition'] = definition_id root_block['edit_info']['edited_on'] = datetime.datetime.now(UTC) root_block['edit_info']['edited_by'] = user_id root_block['edit_info']['previous_version'] = root_block['edit_info'].get('update_version') @@ -1574,9 +1625,9 @@ def create_course( self.update_structure(locator, draft_structure) index_entry = { '_id': ObjectId(), - 'org': org, - 'course': course, - 'run': run, + 'org': locator.org, + 'course': locator.course, + 'run': locator.run, 'edited_by': user_id, 'edited_on': datetime.datetime.now(UTC), 'versions': versions_dict, @@ -1588,9 +1639,20 @@ def create_course( self.insert_course_index(locator, index_entry) # expensive hack to persist default field values set in __init__ method (e.g., wiki_slug) - course = self.get_course(locator, **kwargs) + course = self.get_courselike(locator, **kwargs) return self.update_item(course, user_id, **kwargs) + def create_library(self, org, library, user_id, fields, **kwargs): + """ + Create a new library. Arguments are similar to create_course(). + """ + kwargs["fields"] = fields + kwargs["master_branch"] = kwargs.get("master_branch", ModuleStoreEnum.BranchName.library) + kwargs["root_category"] = kwargs.get("root_category", "library") + kwargs["root_block_id"] = kwargs.get("root_block_id", "library") + locator = LibraryLocator(org=org, library=library, branch=kwargs["master_branch"]) + return self._create_courselike(locator, user_id, **kwargs) + def update_item(self, descriptor, user_id, allow_not_found=False, force=False, **kwargs): """ Save the descriptor's fields. it doesn't descend the course dag to save the children. @@ -1680,14 +1742,24 @@ def _update_item_from_fields( if index_entry is not None: self._update_search_targets(index_entry, definition_fields) self._update_search_targets(index_entry, settings) - course_key = CourseLocator( - org=index_entry['org'], - course=index_entry['course'], - run=index_entry['run'], - branch=course_key.branch, - version_guid=new_id - ) + if isinstance(course_key, LibraryLocator): + course_key = LibraryLocator( + org=index_entry['org'], + library=index_entry['course'], + branch=course_key.branch, + version_guid=new_id + ) + else: + course_key = CourseLocator( + org=index_entry['org'], + course=index_entry['course'], + run=index_entry['run'], + branch=course_key.branch, + version_guid=new_id + ) self._update_head(course_key, index_entry, course_key.branch, new_id) + elif isinstance(course_key, LibraryLocator): + course_key = LibraryLocator(version_guid=new_id) else: course_key = CourseLocator(version_guid=new_id) diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py index 6c277f561843..7c4b0368f2f5 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split_draft.py @@ -9,7 +9,7 @@ from xmodule.modulestore.draft_and_published import ( ModuleStoreDraftAndPublished, DIRECT_ONLY_CATEGORIES, UnsupportedRevisionError ) -from opaque_keys.edx.locator import CourseLocator +from opaque_keys.edx.locator import CourseLocator, LibraryLocator, LibraryUsageLocator from xmodule.modulestore.split_mongo import BlockKey from contracts import contract @@ -57,6 +57,10 @@ def get_course(self, course_id, depth=0, **kwargs): course_id = self._map_revision_to_branch(course_id) return super(DraftVersioningModuleStore, self).get_course(course_id, depth=depth, **kwargs) + def get_library(self, library_id, depth=0, **kwargs): + library_id = self._map_revision_to_branch(library_id) + return super(DraftVersioningModuleStore, self).get_library(library_id, depth=depth, **kwargs) + def clone_course(self, source_course_id, dest_course_id, user_id, fields=None, revision=None, **kwargs): """ See :py:meth: xmodule.modulestore.split_mongo.split.SplitMongoModuleStore.clone_course @@ -153,12 +157,17 @@ def delete_item(self, location, user_id, revision=None, **kwargs): Otherwise, raises a ValueError. """ with self.bulk_operations(location.course_key): - if revision == ModuleStoreEnum.RevisionOption.published_only: + if isinstance(location, LibraryUsageLocator): + branches_to_delete = [ModuleStoreEnum.BranchName.library] # Libraries don't yet have draft/publish support + elif revision == ModuleStoreEnum.RevisionOption.published_only: branches_to_delete = [ModuleStoreEnum.BranchName.published] elif revision == ModuleStoreEnum.RevisionOption.all: branches_to_delete = [ModuleStoreEnum.BranchName.published, ModuleStoreEnum.BranchName.draft] elif revision is None: - branches_to_delete = [ModuleStoreEnum.BranchName.draft] + if location.course_key.branch: + branches_to_delete = [location.course_key.branch] # Delete from whatever branch is explicitly requested + else: + branches_to_delete = [ModuleStoreEnum.BranchName.draft] # Default else: raise UnsupportedRevisionError( [ @@ -178,18 +187,25 @@ def _map_revision_to_branch(self, key, revision=None): """ Maps RevisionOptions to BranchNames, inserting them into the key """ + if isinstance(key, (LibraryLocator, LibraryUsageLocator)): + # Libraries don't yet have draft/publish support: + draft_branch = ModuleStoreEnum.BranchName.library + published_branch = ModuleStoreEnum.BranchName.library + else: + draft_branch = ModuleStoreEnum.BranchName.draft + published_branch = ModuleStoreEnum.BranchName.published if revision == ModuleStoreEnum.RevisionOption.published_only: - return key.for_branch(ModuleStoreEnum.BranchName.published) + return key.for_branch(published_branch) elif revision == ModuleStoreEnum.RevisionOption.draft_only: - return key.for_branch(ModuleStoreEnum.BranchName.draft) + return key.for_branch(draft_branch) elif revision is None: if key.branch is not None: return key elif self.get_branch_setting(key) == ModuleStoreEnum.Branch.draft_preferred: - return key.for_branch(ModuleStoreEnum.BranchName.draft) + return key.for_branch(draft_branch) else: - return key.for_branch(ModuleStoreEnum.BranchName.published) + return key.for_branch(published_branch) else: raise UnsupportedRevisionError() From d38aadc5f8098f45973fd74a47eb2d904b4851ed Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Tue, 28 Oct 2014 21:57:17 -0700 Subject: [PATCH 03/12] Library XBlock (stores library meta-information) --- common/lib/xmodule/setup.py | 1 + common/lib/xmodule/xmodule/library_module.py | 78 ++++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 common/lib/xmodule/xmodule/library_module.py diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py index 5182a8454c7d..018a5efd40a4 100644 --- a/common/lib/xmodule/setup.py +++ b/common/lib/xmodule/setup.py @@ -11,6 +11,7 @@ "discuss = xmodule.backcompat_module:TranslateCustomTagDescriptor", "html = xmodule.html_module:HtmlDescriptor", "image = xmodule.backcompat_module:TranslateCustomTagDescriptor", + "library = xmodule.library_module:LibraryDescriptor", "error = xmodule.error_module:ErrorDescriptor", "peergrading = xmodule.peer_grading_module:PeerGradingDescriptor", "poll_question = xmodule.poll_module:PollDescriptor", diff --git a/common/lib/xmodule/xmodule/library_module.py b/common/lib/xmodule/xmodule/library_module.py new file mode 100644 index 000000000000..fb1f4fdbf5d4 --- /dev/null +++ b/common/lib/xmodule/xmodule/library_module.py @@ -0,0 +1,78 @@ +""" +'library' XBlock/XModule + +The "library" XBlock/XModule is the root of every content library structure +tree. All content blocks in the library are its children. It is analagous to +the "course" XBlock/XModule used as the root of each normal course structure +tree. +""" +import logging + +from xmodule.vertical_module import VerticalDescriptor, VerticalModule + +from xblock.fields import Scope, String, List + +log = logging.getLogger(__name__) + +# Make '_' a no-op so we can scrape strings +_ = lambda text: text + + +class LibraryFields(object): + """ + Fields of the "library" XBlock - see below. + """ + display_name = String( + help=_("Enter the name of the library as it should appear in Studio."), + default="Library", + display_name=_("Library Display Name"), + scope=Scope.settings + ) + advanced_modules = List( + display_name=_("Advanced Module List"), + help=_("Enter the names of the advanced components to use in your library."), + scope=Scope.settings + ) + has_children = True + + +class LibraryDescriptor(LibraryFields, VerticalDescriptor): + """ + Descriptor for our library XBlock/XModule. + """ + module_class = VerticalModule + + def __init__(self, *args, **kwargs): + """ + Expects the same arguments as XModuleDescriptor.__init__ + """ + super(LibraryDescriptor, self).__init__(*args, **kwargs) + + def __unicode__(self): + return u"Library: {}".format(self.display_name) + + def __str__(self): + return "Library: {}".format(self.display_name) + + @property + def display_org_with_default(self): + """ + Return a display organization if it has been specified, otherwise return the 'org' that is in the location. + """ + return self.location.course_key.org + + @property + def display_number_with_default(self): + """ + Return a display course number if it has been specified, otherwise return the 'library' that is in the location + """ + return self.location.course_key.library + + @classmethod + def from_xml(cls, xml_data, system, id_generator): + """ XML support not yet implemented. """ + raise NotImplementedError + + def export_to_xml(self, resource_fs): + """ XML support not yet implemented. """ + raise NotImplementedError From 2622fcf5fd78e2e17cfc96d8e1d57f7611ef1e8d Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Tue, 28 Oct 2014 22:23:26 -0700 Subject: [PATCH 04/12] Studio support for editing libraries --- cms/djangoapps/contentstore/utils.py | 7 ++ cms/djangoapps/contentstore/views/__init__.py | 1 + cms/djangoapps/contentstore/views/course.py | 25 ++++ cms/djangoapps/contentstore/views/helpers.py | 5 +- cms/djangoapps/contentstore/views/item.py | 19 ++- cms/djangoapps/contentstore/views/library.py | 112 ++++++++++++++++++ cms/templates/base.html | 2 + cms/templates/index.html | 30 +++++ cms/templates/library.html | 87 ++++++++++++++ cms/templates/widgets/header.html | 37 ++++++ cms/urls.py | 6 + 11 files changed, 324 insertions(+), 7 deletions(-) create mode 100644 cms/djangoapps/contentstore/views/library.py create mode 100644 cms/templates/library.html diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 2f61345f9afa..38f1abd94b71 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -293,6 +293,13 @@ def reverse_course_url(handler_name, course_key, kwargs=None): return reverse_url(handler_name, 'course_key_string', course_key, kwargs) +def reverse_library_url(handler_name, library_key, kwargs=None): + """ + Creates the URL for handlers that use library_keys as URL parameters. + """ + return reverse_url(handler_name, 'library_key_string', library_key, kwargs) + + def reverse_usage_url(handler_name, usage_key, kwargs=None): """ Creates the URL for handlers that use usage_keys as URL parameters. diff --git a/cms/djangoapps/contentstore/views/__init__.py b/cms/djangoapps/contentstore/views/__init__.py index 5e644468fdb3..9e2e1e1828b9 100644 --- a/cms/djangoapps/contentstore/views/__init__.py +++ b/cms/djangoapps/contentstore/views/__init__.py @@ -12,6 +12,7 @@ from .helpers import * from .item import * from .import_export import * +from .library import * from .preview import * from .public import * from .export_git import * diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 5d5529aaf385..84d442286140 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -38,6 +38,7 @@ add_extra_panel_tab, remove_extra_panel_tab, reverse_course_url, + reverse_library_url, reverse_usage_url, reverse_url, remove_all_instructors, @@ -56,6 +57,7 @@ ADVANCED_COMPONENT_TYPES, ) from contentstore.tasks import rerun_course +from .library import LIBRARIES_ENABLED from .item import create_xblock_info from course_creators.views import get_course_creator_status, add_user_with_status_unrequested from contentstore import utils @@ -341,6 +343,14 @@ def _accessible_courses_list_from_groups(request): return courses_list.values(), in_process_course_actions +def _accessible_libraries_list(user): + """ + List all libraries available to the logged in user by iterating through all libraries + """ + # No need to worry about ErrorDescriptors - split's get_libraries() never returns them. + return [lib for lib in modulestore().get_libraries() if has_course_access(user, lib.location)] + + @login_required @ensure_csrf_cookie def course_listing(request): @@ -360,6 +370,8 @@ def course_listing(request): # so fallback to iterating through all courses courses, in_process_course_actions = _accessible_courses_list(request) + libraries = _accessible_libraries_list(request.user) if LIBRARIES_ENABLED else [] + def format_course_for_view(course): """ Return a dict of the data which the view requires for each course @@ -393,6 +405,18 @@ def format_in_process_course_view(uca): }) if uca.state == CourseRerunUIStateManager.State.FAILED else '' } + def format_library_for_view(library): + """ + Return a dict of the data which the view requires for each library + """ + return { + 'display_name': library.display_name, + 'library_key': unicode(library.location.library_key), + 'url': reverse_library_url('library_handler', unicode(library.location.library_key)), + 'org': library.display_org_with_default, + 'number': library.display_number_with_default, + } + # remove any courses in courses that are also in the in_process_course_actions list in_process_action_course_keys = [uca.course_key for uca in in_process_course_actions] courses = [ @@ -406,6 +430,7 @@ def format_in_process_course_view(uca): return render_to_response('index.html', { 'courses': courses, 'in_process_course_actions': in_process_course_actions, + 'libraries': [format_library_for_view(lib) for lib in libraries], 'user': request.user, 'request_course_creator_url': reverse('contentstore.views.request_course_creator'), 'course_creator_status': _get_course_creator_status(request.user), diff --git a/cms/djangoapps/contentstore/views/helpers.py b/cms/djangoapps/contentstore/views/helpers.py index 34ef869f170f..3769c81978fd 100644 --- a/cms/djangoapps/contentstore/views/helpers.py +++ b/cms/djangoapps/contentstore/views/helpers.py @@ -13,7 +13,7 @@ from edxmako.shortcuts import render_to_string, render_to_response from xblock.core import XBlock from xmodule.modulestore.django import modulestore -from contentstore.utils import reverse_course_url, reverse_usage_url +from contentstore.utils import reverse_course_url, reverse_library_url, reverse_usage_url __all__ = ['edge', 'event', 'landing'] @@ -106,6 +106,9 @@ def xblock_studio_url(xblock, parent_xblock=None): url=reverse_course_url('course_handler', xblock.location.course_key), usage_key=urllib.quote(unicode(xblock.location)) ) + elif category == 'library': + library_key = xblock.location.course_key + return reverse_library_url('library_handler', library_key) else: return reverse_usage_url('container_handler', xblock.location) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index e09db7a4e499..c42fb711fb48 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -47,6 +47,7 @@ from models.settings.course_grading import CourseGradingModel from cms.lib.xblock.runtime import handler_url, local_resource_url from opaque_keys.edx.keys import UsageKey, CourseKey +from opaque_keys.edx.locator import LibraryUsageLocator __all__ = ['orphan_handler', 'xblock_handler', 'xblock_view_handler', 'xblock_outline_handler'] @@ -649,7 +650,9 @@ def _get_module_info(xblock, rewrite_static_links=True): ) # Pre-cache has changes for the entire course because we'll need it for the ancestor info - modulestore().has_changes(modulestore().get_course(xblock.location.course_key, depth=None)) + # Except library blocks which don't use draft/publish + if not isinstance(xblock.location, LibraryUsageLocator): + modulestore().has_changes(modulestore().get_courselike(xblock.location.course_key, depth=None)) # Note that children aren't being returned until we have a use case. return create_xblock_info(xblock, data=data, metadata=own_metadata(xblock), include_ancestor_info=True) @@ -690,12 +693,16 @@ def safe_get_username(user_id): return None + is_library_block = isinstance(xblock.location, LibraryUsageLocator) is_xblock_unit = is_unit(xblock, parent_xblock) - # this should not be calculated for Sections and Subsections on Unit page - has_changes = modulestore().has_changes(xblock) if (is_xblock_unit or course_outline) else None + # this should not be calculated for Sections and Subsections on Unit page or for library blocks + has_changes = modulestore().has_changes(xblock) if (is_xblock_unit or course_outline) and not is_library_block else None if graders is None: - graders = CourseGradingModel.fetch(xblock.location.course_key).graders + if not is_library_block: + graders = CourseGradingModel.fetch(xblock.location.course_key).graders + else: + graders = [] # Compute the child info first so it can be included in aggregate information for the parent should_visit_children = include_child_info and (course_outline and not is_xblock_unit or not course_outline) @@ -715,7 +722,7 @@ def safe_get_username(user_id): visibility_state = _compute_visibility_state(xblock, child_info, is_xblock_unit and has_changes) else: visibility_state = None - published = modulestore().has_published_version(xblock) + published = modulestore().has_published_version(xblock) if not is_library_block else None xblock_info = { "id": unicode(xblock.location), @@ -723,7 +730,7 @@ def safe_get_username(user_id): "category": xblock.category, "edited_on": get_default_time_display(xblock.subtree_edited_on) if xblock.subtree_edited_on else None, "published": published, - "published_on": get_default_time_display(xblock.published_on) if xblock.published_on else None, + "published_on": get_default_time_display(xblock.published_on) if published and xblock.published_on else None, "studio_url": xblock_studio_url(xblock, parent_xblock), "released_to_students": datetime.now(UTC) > xblock.start, "release_date": release_date, diff --git a/cms/djangoapps/contentstore/views/library.py b/cms/djangoapps/contentstore/views/library.py new file mode 100644 index 000000000000..1e9de732c9ed --- /dev/null +++ b/cms/djangoapps/contentstore/views/library.py @@ -0,0 +1,112 @@ +""" +Views related to content libraries. +A content library is a structure containing XBlocks which can be re-used in the +multiple courses. +""" +from __future__ import absolute_import + +import json +import logging + +from contentstore.views.item import create_xblock_info +from django.http import HttpResponseNotAllowed, Http404 +from django.contrib.auth.decorators import login_required +from django.core.exceptions import PermissionDenied +from django.conf import settings +from django.utils.translation import ugettext as _ +from django_future.csrf import ensure_csrf_cookie +from edxmako.shortcuts import render_to_response +from opaque_keys.edx.keys import CourseKey +from opaque_keys.edx.locator import LibraryLocator, LibraryUsageLocator +from xmodule.modulestore.django import modulestore + +from .access import has_course_access +from .component import get_component_templates +from util.json_request import JsonResponse + +__all__ = ['library_handler'] + +log = logging.getLogger(__name__) + +LIBRARIES_ENABLED = settings.FEATURES.get('ENABLE_CONTENT_LIBRARIES', False) + + +@login_required +@ensure_csrf_cookie +def library_handler(request, library_key_string=None): + """ + RESTful interface to most content library related functionality. + """ + if not LIBRARIES_ENABLED: + raise Http404 # Should never happen because we test the feature in urls.py also + + response_format = 'html' + if request.REQUEST.get('format', 'html') == 'json' or 'application/json' in request.META.get('HTTP_ACCEPT', 'text/html'): + response_format = 'json' + + if library_key_string: + library_key = CourseKey.from_string(library_key_string) + if not isinstance(library_key, LibraryLocator): + raise Http404 # This is not a library + if not has_course_access(request.user, library_key): + raise PermissionDenied() + + library = modulestore().get_library(library_key) + if library is None: + raise Http404 + + if request.method == 'GET': + return library_blocks_view(library, response_format) + return HttpResponseNotAllowed(['GET']) + + elif request.method == 'GET': + # List all accessible libraries: + lib_info = [ + { + "display_name": lib.display_name, + "library_key": unicode(lib.location.library_key), + } + for lib in modulestore().get_libraries() + if has_course_access(request.user, lib.location.library_key) + ] + return JsonResponse(lib_info) + else: + return HttpResponseNotAllowed(['GET']) + + +def library_blocks_view(library, response_format): + """ + The main view of a course's content library. + Shows all the XBlocks in the library, and allows adding/editing/deleting + them. + Can be called with response_format="json" to get a JSON-formatted list of + the XBlocks in the library along with library metadata. + """ + children = library.children + if response_format == "json": + # The JSON response for this request is short and sweet: + prev_version = library.runtime.course_entry.structure['previous_version'] + return JsonResponse({ + "display_name": library.display_name, + "library_id": unicode(library.location.course_key), # library.course_id raises UndefinedContext - fix? + "version": unicode(library.runtime.course_entry.course_key.version), + "previous_version": unicode(prev_version) if prev_version else None, + "blocks": [unicode(x) for x in children], + }) + + xblock_info = create_xblock_info(library, include_ancestor_info=False, graders=[]) + + component_templates = get_component_templates(library) + + assert isinstance(library.location.library_key, LibraryLocator) + assert isinstance(library.location, LibraryUsageLocator) + + return render_to_response('library.html', { + 'context_library': library, + 'action': 'view', + 'xblock': library, + 'xblock_locator': library.location, + 'unit': None, + 'component_templates': json.dumps(component_templates), + 'xblock_info': xblock_info, + }) diff --git a/cms/templates/base.html b/cms/templates/base.html index 8a8a70c0ded2..0b31ebf4239c 100644 --- a/cms/templates/base.html +++ b/cms/templates/base.html @@ -21,6 +21,8 @@ % if context_course: <% ctx_loc = context_course.location %> ${context_course.display_name_with_default | h} | + % elif context_library: + ${context_library.display_name_with_default | h} | % endif edX Studio diff --git a/cms/templates/index.html b/cms/templates/index.html index 7a155f9ceca4..3831202b2b83 100644 --- a/cms/templates/index.html +++ b/cms/templates/index.html @@ -356,6 +356,36 @@

${_('Your Course Creator Request Status:')}

% endif + %if libraries: +
+

Content Libraries

+

${_("Warning: Content Libraries are currently a beta feature, and may be subject to backwards-incompatible changes.")}

+

${_("Here are all of the libraries you currently have access to in Studio:")}

+
+ +
+ +
+ %endif +