diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 07ce908ba9..43968d6716 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -9,6 +9,14 @@ Blades: When start time and end time are specified for a video, a visual range will be shown on the time slider to highlight the place in the video that will be played. +Studio: added restful interface for finding orphans in courses. +An orphan is an xblock to which no children relation points and whose type is not +in the set contentstore.views.item.DETACHED_CATEGORIES nor 'course'. + GET http://host/orphan/org.course returns json array of ids. + Requires course author access. + DELETE http://orphan/org.course deletes all the orphans in that course. + Requires is_staff access + Studio: Bug fix for text loss in Course Updates when the text exists before the first tag. diff --git a/cms/djangoapps/contentstore/tests/test_orphan.py b/cms/djangoapps/contentstore/tests/test_orphan.py new file mode 100644 index 0000000000..c6f16fd47b --- /dev/null +++ b/cms/djangoapps/contentstore/tests/test_orphan.py @@ -0,0 +1,58 @@ +""" +Test finding orphans via the view and django config +""" +import json +from contentstore.tests.utils import CourseTestCase +from xmodule.modulestore.django import editable_modulestore +from django.core.urlresolvers import reverse + +class TestOrphan(CourseTestCase): + """ + Test finding orphans via view and django config + """ + def setUp(self): + super(TestOrphan, self).setUp() + + runtime = self.course.runtime + + self._create_item('chapter', 'Chapter1', {}, {'display_name': 'Chapter 1'}, 'course', self.course.location.name, runtime) + self._create_item('chapter', 'Chapter2', {}, {'display_name': 'Chapter 2'}, 'course', self.course.location.name, runtime) + self._create_item('chapter', 'OrphanChapter', {}, {'display_name': 'Orphan Chapter'}, None, None, runtime) + self._create_item('vertical', 'Vert1', {}, {'display_name': 'Vertical 1'}, 'chapter', 'Chapter1', runtime) + self._create_item('vertical', 'OrphanVert', {}, {'display_name': 'Orphan Vertical'}, None, None, runtime) + self._create_item('html', 'Html1', "

Goodbye

", {'display_name': 'Parented Html'}, 'vertical', 'Vert1', runtime) + self._create_item('html', 'OrphanHtml', "

Hello

", {'display_name': 'Orphan html'}, None, None, runtime) + self._create_item('static_tab', 'staticuno', "

tab

", {'display_name': 'Tab uno'}, None, None, runtime) + self._create_item('about', 'overview', "

overview

", {}, None, None, runtime) + self._create_item('course_info', 'updates', "
  1. Sep 22

    test

", {}, None, None, runtime) + + def _create_item(self, category, name, data, metadata, parent_category, parent_name, runtime): + location = self.course.location.replace(category=category, name=name) + editable_modulestore('direct').create_and_save_xmodule(location, data, metadata, runtime) + if parent_name: + # add child to parent in mongo + parent_location = self.course.location.replace(category=parent_category, name=parent_name) + parent = editable_modulestore('direct').get_item(parent_location) + parent.children.append(location.url()) + editable_modulestore('direct').update_children(parent_location, parent.children) + + def test_mongo_orphan(self): + """ + Test that old mongo finds the orphans + """ + orphans = json.loads( + self.client.get( + reverse( + 'orphan', + kwargs={'course_id': '{}.{}'.format(self.course.location.org, self.course.location.course)} + ), + HTTP_ACCEPT='application/json' + ).content + ) + self.assertEqual(len(orphans), 3, "Wrong # {}".format(orphans)) + location = self.course.location.replace(category='chapter', name='OrphanChapter') + self.assertIn(location.url(), orphans) + location = self.course.location.replace(category='vertical', name='OrphanVert') + self.assertIn(location.url(), orphans) + location = self.course.location.replace(category='html', name='OrphanHtml') + self.assertIn(location.url(), orphans) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 1ebdc8fb4f..816b05c16b 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -21,7 +21,7 @@ from .access import has_access from .helpers import _xmodule_recurse from xmodule.x_module import XModuleDescriptor -__all__ = ['save_item', 'create_item', 'delete_item'] +__all__ = ['save_item', 'create_item', 'delete_item', 'orphan'] log = logging.getLogger(__name__) @@ -200,3 +200,20 @@ def delete_item(request): modulestore('direct').update_children(parent.location, parent.children) return JsonResponse() + + +@login_required +def orphan(request, course_id): + """ + View for handling orphan related requests. A get gets all of the current orphans. + DELETE, PUT and POST are meaningless for now. + + An orphan is a block whose category is not in the DETACHED_CATEGORY list, is not the root, and is not reachable + from the root via children + + :param request: + :param course_id: Locator syntax course_id + """ + # dhm: I'd add DELETE but I'm not sure what type of authentication/authorization we'd need + if request.method == 'GET': + return JsonResponse(modulestore().get_orphans(course_id, DETACHED_CATEGORIES, 'draft')) diff --git a/cms/urls.py b/cms/urls.py index 9bbf1664f8..c22b2c0cf5 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -130,6 +130,8 @@ urlpatterns += patterns( url(r'^login_post$', 'student.views.login_user', name='login_post'), url(r'^logout$', 'student.views.logout_user', name='logout'), + + url(r'^(?P[^/]+)/orphan', 'contentstore.views.orphan', name='orphan') ) # restful api diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 352ebce61a..b4488e6497 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -34,6 +34,7 @@ from xblock.fields import Scope, ScopeIds from xmodule.modulestore import ModuleStoreBase, Location, MONGO_MODULESTORE_TYPE from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.inheritance import own_metadata, InheritanceMixin, inherit_metadata, InheritanceKeyValueStore +import re log = logging.getLogger(__name__) @@ -697,7 +698,7 @@ class MongoModuleStore(ModuleStoreBase): course.tabs = existing_tabs # Save any changes to the course to the MongoKeyValueStore course.save() - self.update_metadata(course.location, course.xblock_kvs._metadata) + self.update_metadata(course.location, course.get_explicitly_set_fields_by_scope(Scope.settings)) def fire_updated_modulestore_signal(self, course_id, location): """ @@ -854,6 +855,38 @@ class MongoModuleStore(ModuleStoreBase): """ return MONGO_MODULESTORE_TYPE + COURSE_ID_RE = re.compile(r'(?P[^.]+)\.(?P.+)') + def parse_course_id(self, course_id): + """ + Parse a Locator style course_id into a dict w/ the org and course_id + :param course_id: a string looking like 'org.course.id.part' + """ + match = self.COURSE_ID_RE.match(course_id) + if match is None: + raise ValueError(course_id) + return match.groupdict() + + def get_orphans(self, course_id, detached_categories, _branch): + """ + Return a dict of all of the orphans in the course. + + :param course_id: + """ + locator_dict = self.parse_course_id(course_id) + all_items = self.collection.find({ + '_id.org': locator_dict['org'], + '_id.course': locator_dict['course_id'], + '_id.category': {'$nin': detached_categories} + }) + all_reachable = set() + item_locs = set() + for item in all_items: + if item['_id']['category'] != 'course': + item_locs.add(Location(item['_id']).replace(revision=None).url()) + all_reachable = all_reachable.union(item.get('definition', {}).get('children', [])) + item_locs -= all_reachable + return list(item_locs) + def _create_new_field_data(self, category, location, definition_data, metadata): """ To instantiate a new xmodule which will be saved latter, set up the dbModel and kvs diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index f310586c66..68aa883441 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -421,6 +421,24 @@ class SplitMongoModuleStore(ModuleStoreBase): items.append(BlockUsageLocator(url=locator.as_course_locator(), usage_id=parent_id)) return items + def get_orphans(self, course_id, detached_categories, branch): + """ + Return a dict of all of the orphans in the course. + + :param course_id: + """ + course = self._lookup_course(CourseLocator(course_id=course_id, branch=branch)) + items = set(course['structure']['blocks'].keys()) + items.remove(course['structure']['root']) + for block_id, block_data in course['structure']['blocks'].iteritems(): + items.difference_update(block_data.get('fields', {}).get('children', [])) + if block_data['category'] in detached_categories: + items.discard(block_id) + return [ + BlockUsageLocator(course_id=course_id, branch=branch, usage_id=block_id) + for block_id in items + ] + def get_course_index_info(self, course_locator): """ The index records the initial creation of the indexed course and tracks the current version diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py b/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py new file mode 100644 index 0000000000..f8f7e73218 --- /dev/null +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py @@ -0,0 +1,147 @@ +import uuid +import mock +import unittest +import random +import datetime + +from xmodule.modulestore.inheritance import InheritanceMixin +from xmodule.modulestore.mongo import MongoModuleStore +from xmodule.modulestore.split_mongo import SplitMongoModuleStore +from xmodule.modulestore import Location +from xmodule.fields import Date +from xmodule.modulestore.locator import BlockUsageLocator, CourseLocator + + +class TestOrphan(unittest.TestCase): + """ + Test the orphan finding code + """ + + # Snippet of what would be in the django settings envs file + db_config = { + 'host': 'localhost', + 'db': 'test_xmodule', + } + + modulestore_options = dict({ + 'default_class': 'xmodule.raw_module.RawDescriptor', + 'fs_root': '', + 'render_template': mock.Mock(return_value=""), + 'xblock_mixins': (InheritanceMixin,) + }, **db_config) + + split_course_id = 'test_org.test_course.runid' + + def setUp(self): + self.modulestore_options['collection'] = 'modulestore{0}'.format(uuid.uuid4().hex) + + self.userid = random.getrandbits(32) + super(TestOrphan, self).setUp() + self.split_mongo = SplitMongoModuleStore( + **self.modulestore_options + ) + self.addCleanup(self.tearDownSplit) + self.old_mongo = MongoModuleStore(**self.modulestore_options) + self.addCleanup(self.tearDownMongo) + self.course_location = None + self._create_course() + + def tearDownSplit(self): + split_db = self.split_mongo.db + split_db.drop_collection(split_db.course_index) + split_db.drop_collection(split_db.structures) + split_db.drop_collection(split_db.definitions) + split_db.connection.close() + + def tearDownMongo(self): + split_db = self.split_mongo.db + # old_mongo doesn't give a db attr, but all of the dbs are the same + split_db.drop_collection(self.old_mongo.collection) + + def _create_item(self, category, name, data, metadata, parent_category, parent_name, runtime): + location = Location('i4x', 'test_org', 'test_course', category, name) + self.old_mongo.create_and_save_xmodule(location, data, metadata, runtime) + if isinstance(data, basestring): + fields = {'data': data} + else: + fields = data.copy() + fields.update(metadata) + if parent_name: + # add child to parent in mongo + parent_location = Location('i4x', 'test_org', 'test_course', parent_category, parent_name) + parent = self.old_mongo.get_item(parent_location) + parent.children.append(location.url()) + self.old_mongo.update_children(parent_location, parent.children) + # create pointer for split + course_or_parent_locator = BlockUsageLocator( + course_id=self.split_course_id, + branch='draft', + usage_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) + + def _create_course(self): + """ + * some detached items + * some attached children + * some orphans + """ + date_proxy = Date() + metadata = { + 'start': date_proxy.to_json(datetime.datetime(2000, 3, 13, 4)), + 'display_name': 'Migration test course', + } + data = { + 'wiki_slug': 'test_course_slug' + } + fields = metadata.copy() + 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' + ) + self.course_location = Location('i4x', 'test_org', 'test_course', 'course', 'runid') + self.old_mongo.create_and_save_xmodule(self.course_location, data, metadata) + runtime = self.old_mongo.get_item(self.course_location).runtime + + self._create_item('chapter', 'Chapter1', {}, {'display_name': 'Chapter 1'}, 'course', 'runid', runtime) + self._create_item('chapter', 'Chapter2', {}, {'display_name': 'Chapter 2'}, 'course', 'runid', runtime) + self._create_item('chapter', 'OrphanChapter', {}, {'display_name': 'Orphan Chapter'}, None, None, runtime) + self._create_item('vertical', 'Vert1', {}, {'display_name': 'Vertical 1'}, 'chapter', 'Chapter1', runtime) + self._create_item('vertical', 'OrphanVert', {}, {'display_name': 'Orphan Vertical'}, None, None, runtime) + self._create_item('html', 'Html1', "

Goodbye

", {'display_name': 'Parented Html'}, 'vertical', 'Vert1', runtime) + self._create_item('html', 'OrphanHtml', "

Hello

", {'display_name': 'Orphan html'}, None, None, runtime) + self._create_item('static_tab', 'staticuno', "

tab

", {'display_name': 'Tab uno'}, None, None, runtime) + self._create_item('about', 'overview', "

overview

", {}, None, None, runtime) + self._create_item('course_info', 'updates', "
  1. Sep 22

    test

", {}, None, None, runtime) + + def test_mongo_orphan(self): + """ + Test that old mongo finds the orphans + """ + orphans = self.old_mongo.get_orphans('test_org.test_course', ['static_tab', 'about', 'course_info'], None) + self.assertEqual(len(orphans), 3, "Wrong # {}".format(orphans)) + location = self.course_location.replace(category='chapter', name='OrphanChapter') + self.assertIn(location.url(), orphans) + location = self.course_location.replace(category='vertical', name='OrphanVert') + self.assertIn(location.url(), orphans) + location = self.course_location.replace(category='html', name='OrphanHtml') + self.assertIn(location.url(), orphans) + + def test_split_orphan(self): + """ + Test that old mongo finds the orphans + """ + 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') + self.assertIn(location, orphans) + location = BlockUsageLocator(course_id=self.split_course_id, branch='draft', usage_id='OrphanVert') + self.assertIn(location, orphans) + location = BlockUsageLocator(course_id=self.split_course_id, branch='draft', usage_id='OrphanHtml') + self.assertIn(location, orphans)