diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 508236a1e9..18afd331d0 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -33,31 +33,6 @@ def get_course_location_for_item(location): return location -def get_course_for_item(location): - ''' - cdodge: for a given Xmodule, return the course that it belongs to - NOTE: This makes a lot of assumptions about the format of the course location - Also we have to assert that this module maps to only one course item - it'll throw an - assert if not - ''' - item_loc = Location(location) - - # @hack! We need to find the course location however, we don't - # know the 'name' parameter in this context, so we have - # to assume there's only one item in this query even though we are not specifying a name - course_search_location = ['i4x', item_loc.org, item_loc.course, 'course', None] - courses = modulestore().get_items(course_search_location) - - # make sure we found exactly one match on this above course search - found_cnt = len(courses) - if found_cnt == 0: - raise BaseException('Could not find course at {0}'.format(course_search_location)) - - if found_cnt > 1: - raise BaseException('Found more than one course at {0}. There should only be one!!! Dump = {1}'.format(course_search_location, courses)) - - return courses[0] - def get_lms_link_for_item(location, preview=False): location = Location(location) diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 6c02215be1..f680dd7262 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -55,7 +55,7 @@ from cache_toolbox.core import set_cached_content, get_cached_content, del_cache from auth.authz import is_user_in_course_group_role, get_users_in_course_group_by_role from auth.authz import get_user_by_email, add_user_to_course_group, remove_user_from_course_group from auth.authz import INSTRUCTOR_ROLE_NAME, STAFF_ROLE_NAME, create_all_course_groups -from .utils import get_course_location_for_item, get_lms_link_for_item, compute_unit_state, get_date_display, UnitState, get_course_for_item +from .utils import get_course_location_for_item, get_lms_link_for_item, compute_unit_state, get_date_display, UnitState from xmodule.templates import all_templates from xmodule.modulestore.xml_importer import import_from_xml @@ -622,17 +622,6 @@ def save_item(request): # commit to datastore store.update_metadata(item_location, existing_item.metadata) - # cdodge: special case logic for updating static_tabs - # unfortunately tabs are enumerated in the course policy data structure, so if we change the display name of - # the tab, we need to update the course policy (which has a nice .tabs property on it) - item_loc = Location(item_location) - if item_loc.category == 'static_tab': - # VS[compat] Rework when we can stop having to support tabs lists in the policy - tag_module = store.get_item(item_location) - course = get_course_for_item(item_location) - course.update_tab_reference(tag_module) - modulestore('direct').update_metadata(course.location, course.metadata) - return HttpResponse() @@ -709,13 +698,6 @@ def clone_item(request): if new_item.location.category not in DETACHED_CATEGORIES: _modulestore(parent.location).update_children(parent_location, parent.definition.get('children', []) + [new_item.location.url()]) - elif new_item.location.category == 'static_tab': - # static tabs - in our data model - are described in the course policy - # VS[compat]: Rework when we can stop having to support tabs lists in the policy - if parent.location.category != 'course': - raise BaseException('adding a new static_tab must be on a course object') - parent.add_tab_reference(new_item) - _modulestore(parent.location).update_metadata(parent.location.url(), parent.metadata) return HttpResponse(json.dumps({'id': dest_location.url()})) diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index fe2c6ca87a..512247a429 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -270,29 +270,6 @@ class CourseDescriptor(SequenceDescriptor): def tabs(self, value): self.metadata['tabs'] = value - def add_tab_reference(self, tab_module): - ''' - Since in CMS we can add static tabs via data, the current data model expects that - the list of tabs be encapsulated in the course. - VS[compat] we can rework this to avoid pointers to static tab modules once we fully deprecate filesystem support - ''' - existing_tabs = self.tabs or [] - existing_tabs.append({'type':'static_tab', 'name' : tab_module.metadata.get('display_name'), 'url_slug' : tab_module.location.name}) - self.tabs = existing_tabs - - - def update_tab_reference(self, tab_module): - ''' - Since in CMS we can add static tabs via data, the current data model expects that - the list of tabs be encapsulated in the course. - VS[compat] we can rework this to avoid pointers to static tab modules once we fully deprecate filesystem support - ''' - existing_tabs = self.tabs - for tab in existing_tabs: - if tab.get('url_slug') == tab_module.location.name: - tab['name'] = tab_module.metadata.get('display_name') - self.tabs = existing_tabs - @property def show_calculator(self): return self.metadata.get("show_calculator", None) == "Yes" diff --git a/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py index 550e6570ac..19f506906c 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo.py @@ -276,10 +276,49 @@ class MongoModuleStore(ModuleStoreBase): source_item = self.collection.find_one(location_to_query(source)) source_item['_id'] = Location(location).dict() self.collection.insert(source_item) - return self._load_items([source_item])[0] + item = self._load_items([source_item])[0] + + # VS[compat] cdodge: This is a hack because static_tabs also have references from the course module, so + # if we add one then we need to also add it to the policy information (i.e. metadata) + # we should remove this once we can break this reference from the course to static tabs + if location.category == 'static_tab': + course = self.get_course_for_item(item.location) + existing_tabs = course.tabs or [] + existing_tabs.append({'type':'static_tab', 'name' : item.metadata.get('display_name'), 'url_slug' : item.location.name}) + course.tabs = existing_tabs + self.update_metadata(course.location, course.metadata) + + return item except pymongo.errors.DuplicateKeyError: raise DuplicateItemError(location) + + def get_course_for_item(self, location): + ''' + VS[compat] + cdodge: for a given Xmodule, return the course that it belongs to + NOTE: This makes a lot of assumptions about the format of the course location + Also we have to assert that this module maps to only one course item - it'll throw an + assert if not + This is only used to support static_tabs as we need to be course module aware + ''' + + # @hack! We need to find the course location however, we don't + # know the 'name' parameter in this context, so we have + # to assume there's only one item in this query even though we are not specifying a name + course_search_location = ['i4x', location.org, location.course, 'course', None] + courses = self.get_items(course_search_location) + + # make sure we found exactly one match on this above course search + found_cnt = len(courses) + if found_cnt == 0: + raise BaseException('Could not find course at {0}'.format(course_search_location)) + + if found_cnt > 1: + raise BaseException('Found more than one course at {0}. There should only be one!!! Dump = {1}'.format(course_search_location, courses)) + + return courses[0] + def _update_single_item(self, location, update): """ Set update on the specified item, and raises ItemNotFoundError @@ -327,6 +366,19 @@ class MongoModuleStore(ModuleStoreBase): location: Something that can be passed to Location metadata: A nested dictionary of module metadata """ + # VS[compat] cdodge: This is a hack because static_tabs also have references from the course module, so + # if we add one then we need to also add it to the policy information (i.e. metadata) + # we should remove this once we can break this reference from the course to static tabs + loc = Location(location) + if loc.category == 'static_tab': + course = self.get_course_for_item(loc) + existing_tabs = course.tabs or [] + for tab in existing_tabs: + if tab.get('url_slug') == loc.name: + tab['name'] = metadata.get('display_name') + break + course.tabs = existing_tabs + self.update_metadata(course.location, course.metadata) self._update_single_item(location, {'metadata': metadata}) @@ -336,6 +388,16 @@ class MongoModuleStore(ModuleStoreBase): location: Something that can be passed to Location """ + # VS[compat] cdodge: This is a hack because static_tabs also have references from the course module, so + # if we add one then we need to also add it to the policy information (i.e. metadata) + # we should remove this once we can break this reference from the course to static tabs + if location.category == 'static_tab': + item = self.get_item(location) + course = self.get_course_for_item(item.location) + existing_tabs = course.tabs or [] + course.tabs = [tab for tab in existing_tabs if tab.get('url_slug') != location.name] + self.update_metadata(course.location, course.metadata) + self.collection.remove({'_id': Location(location).dict()}) def get_parent_locations(self, location): diff --git a/common/lib/xmodule/xmodule/templates/about/empty.yaml b/common/lib/xmodule/xmodule/templates/about/empty.yaml index 71bcef8914..fa3ed606bd 100644 --- a/common/lib/xmodule/xmodule/templates/about/empty.yaml +++ b/common/lib/xmodule/xmodule/templates/about/empty.yaml @@ -1,5 +1,5 @@ --- metadata: display_name: Empty -data: "This is where you can add additional information about your course." +data: "
This is where you can add additional information about your course.
" children: [] \ No newline at end of file diff --git a/common/lib/xmodule/xmodule/templates/courseinfo/empty.yaml b/common/lib/xmodule/xmodule/templates/courseinfo/empty.yaml index 71bcef8914..fa3ed606bd 100644 --- a/common/lib/xmodule/xmodule/templates/courseinfo/empty.yaml +++ b/common/lib/xmodule/xmodule/templates/courseinfo/empty.yaml @@ -1,5 +1,5 @@ --- metadata: display_name: Empty -data: "This is where you can add additional information about your course." +data: "This is where you can add additional information about your course.
" children: [] \ No newline at end of file diff --git a/common/lib/xmodule/xmodule/templates/statictab/empty.yaml b/common/lib/xmodule/xmodule/templates/statictab/empty.yaml index 31306e2ce6..410e1496c2 100644 --- a/common/lib/xmodule/xmodule/templates/statictab/empty.yaml +++ b/common/lib/xmodule/xmodule/templates/statictab/empty.yaml @@ -1,5 +1,5 @@ --- metadata: display_name: Empty -data: "This is where you can add additional pages to your courseware. Click the 'edit' button to begin editing." +data: "This is where you can add additional pages to your courseware. Click the 'edit' button to begin editing.
" children: [] \ No newline at end of file