From 010905eb996bc4e2f6391bf115e74d4cbaf6a00a Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Thu, 3 Oct 2013 12:06:26 -0400 Subject: [PATCH] RESTful api for getting course listing and opening course in studio. Pattern for how to do refactoring from locations to locators and from old style urls to restful ones. --- cms/djangoapps/auth/authz.py | 29 ++-- cms/djangoapps/contentstore/views/access.py | 8 +- cms/djangoapps/contentstore/views/course.py | 80 ++++++++-- cms/djangoapps/contentstore/views/user.py | 12 +- cms/templates/widgets/header.html | 13 +- cms/urls.py | 30 ++-- .../xmodule/modulestore/loc_mapper_store.py | 2 +- .../xmodule/xmodule/modulestore/locator.py | 144 +++++++++++++++--- .../xmodule/xmodule/modulestore/parsers.py | 13 +- .../tests/test_split_modulestore.py | 18 +-- .../data/splitmongo_json/definitions.json | 50 +++--- .../test/data/splitmongo_json/structures.json | 24 +-- 12 files changed, 310 insertions(+), 113 deletions(-) diff --git a/cms/djangoapps/auth/authz.py b/cms/djangoapps/auth/authz.py index 4923851445..7ecddae274 100644 --- a/cms/djangoapps/auth/authz.py +++ b/cms/djangoapps/auth/authz.py @@ -1,13 +1,16 @@ +#======================================================================================================================= +# +# This code is somewhat duplicative of access.py in the LMS. We will unify the code as a separate story +# but this implementation should be data compatible with the LMS implementation +# +#======================================================================================================================= from django.contrib.auth.models import User, Group from django.core.exceptions import PermissionDenied from django.conf import settings from xmodule.modulestore import Location +from xmodule.modulestore.locator import CourseLocator, Locator -''' -This code is somewhat duplicative of access.py in the LMS. We will unify the code as a separate story -but this implementation should be data compatible with the LMS implementation -''' # define a couple of simple roles, we just need ADMIN and EDITOR now for our purposes INSTRUCTOR_ROLE_NAME = 'instructor' @@ -22,16 +25,22 @@ COURSE_CREATOR_GROUP_NAME = "course_creator_group" def get_course_groupname_for_role(location, role): - loc = Location(location) + location = Locator.to_locator_or_location(location) + # hack: check for existence of a group name in the legacy LMS format _ # if it exists, then use that one, otherwise use a _ which contains # more information - groupname = '{0}_{1}'.format(role, loc.course) + groupnames = [] + groupnames.append('{0}_{1}'.format(role, location.course_id)) + if isinstance(location, Location): + groupnames.append('{0}_{1}'.format(role, location.course)) + elif isinstance(location, CourseLocator): + groupnames.append('{0}_{1}'.format(role, location.as_old_location_course_id)) - if len(Group.objects.filter(name=groupname)) == 0: - groupname = '{0}_{1}'.format(role, loc.course_id) - - return groupname + for groupname in groupnames: + if Group.objects.filter(name=groupname).exists(): + return groupname + return groupnames[0] def get_users_in_course_group_by_role(location, role): diff --git a/cms/djangoapps/contentstore/views/access.py b/cms/djangoapps/contentstore/views/access.py index 5cb6b8c6f4..3c75215bd1 100644 --- a/cms/djangoapps/contentstore/views/access.py +++ b/cms/djangoapps/contentstore/views/access.py @@ -3,6 +3,7 @@ from auth.authz import is_user_in_course_group_role from django.core.exceptions import PermissionDenied from ..utils import get_course_location_for_item from xmodule.modulestore import Location +from xmodule.modulestore.locator import CourseLocator def get_location_and_verify_access(request, org, course, name): @@ -29,13 +30,14 @@ def has_access(user, location, role=STAFF_ROLE_NAME): will not be in both INSTRUCTOR and STAFF groups, so we have to cascade our queries here as INSTRUCTOR has all the rights that STAFF do ''' - course_location = get_course_location_for_item(location) - _has_access = is_user_in_course_group_role(user, course_location, role) + if not isinstance(location, CourseLocator): + location = get_course_location_for_item(location) + _has_access = is_user_in_course_group_role(user, location, role) # if we're not in STAFF, perhaps we're in INSTRUCTOR groups if not _has_access and role == STAFF_ROLE_NAME: _has_access = is_user_in_course_group_role( user, - course_location, + location, INSTRUCTOR_ROLE_NAME ) return _has_access diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 772dfd2778..f043c09d73 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -12,11 +12,11 @@ from django.conf import settings from django.views.decorators.http import require_http_methods, require_POST from django.core.exceptions import PermissionDenied from django.core.urlresolvers import reverse -from django.http import HttpResponseBadRequest +from django.http import HttpResponseBadRequest, HttpResponseNotFound from util.json_request import JsonResponse from mitxmako.shortcuts import render_to_response -from xmodule.modulestore.django import modulestore +from xmodule.modulestore.django import modulestore, loc_mapper from xmodule.modulestore.inheritance import own_metadata from xmodule.contentstore.content import StaticContent @@ -48,7 +48,8 @@ from django_comment_common.utils import seed_permissions_roles from student.models import CourseEnrollment from xmodule.html_module import AboutDescriptor -__all__ = ['course_index', 'create_new_course', 'course_info', +from xmodule.modulestore.locator import BlockUsageLocator +__all__ = ['create_new_course', 'course_info', 'course_handler', 'course_info_updates', 'get_course_settings', 'course_config_graders_page', 'course_config_advanced_page', @@ -58,25 +59,84 @@ __all__ = ['course_index', 'create_new_course', 'course_info', 'create_textbook'] +@login_required +def course_handler(request, course_url): + """ + The restful handler for course specific requests. + It provides the course tree with the necessary information for identifying and labeling the parts. The root + will typically be a 'course' object but may not be especially as we support modules. + + GET + html: return html page overview for the given course + json: return json representing the course branch's index entry as well as dag w/ all of the children + replaced w/ json docs where each doc has {'_id': , 'display_name': , 'children': } + POST + json: create (or update?) this course or branch in this course for this user, return resulting json + descriptor (same as in GET course/...). Leaving off /branch/draft would imply create the course w/ default + branches. Cannot change the structure contents ('_id', 'display_name', 'children') but can change the + index entry. + PUT + json: update this course (index entry not xblock) such as repointing head, changing display name, org, + course_id, prettyid. Return same json as above. + DELETE + json: delete this branch from this course (leaving off /branch/draft would imply delete the course) + """ + if 'application/json' in request.META.get('HTTP_ACCEPT', 'application/json'): + if request.method == 'GET': + raise NotImplementedError('coming soon') + elif not has_access(request.user, BlockUsageLocator(course_url)): + raise PermissionDenied() + elif request.method == 'POST': + raise NotImplementedError() + elif request.method == 'PUT': + raise NotImplementedError() + elif request.method == 'DELETE': + raise NotImplementedError() + else: + return HttpResponseBadRequest() + elif request.method == 'GET': # assume html + return course_index(request, course_url) + else: + return HttpResponseNotFound() + + @login_required @ensure_csrf_cookie -def course_index(request, org, course, name): +def old_course_index_shim(request, org, course, name): + """ + A shim for any unconverted uses of course_index + """ + old_location = Location(['i4x', org, course, 'course', name]) + locator = loc_mapper().translate_location(old_location.course_id, old_location, False, True) + return course_index(request, locator) + + +@login_required +@ensure_csrf_cookie +def course_index(request, course_url): """ Display an editable course overview. org, course, name: Attributes of the Location for the item to edit """ - location = get_location_and_verify_access(request, org, course, name) + location = BlockUsageLocator(course_url) + # TODO: when converting to split backend, if location does not have a usage_id, + # we'll need to get the course's root block_id + if not has_access(request.user, location): + raise PermissionDenied() - lms_link = get_lms_link_for_item(location) + + old_location = loc_mapper().translate_locator_to_location(location) + + lms_link = get_lms_link_for_item(old_location) upload_asset_callback_url = reverse('upload_asset', kwargs={ - 'org': org, - 'course': course, - 'coursename': name + 'org': location.as_old_location_org, + 'course': location.as_old_location_course, + 'coursename': location.as_old_location_run }) - course = modulestore().get_item(location, depth=3) + course = modulestore().get_item(old_location, depth=3) sections = course.get_children() return render_to_response('overview.html', { diff --git a/cms/djangoapps/contentstore/views/user.py b/cms/djangoapps/contentstore/views/user.py index 9414bfb9e8..d307e1295e 100644 --- a/cms/djangoapps/contentstore/views/user.py +++ b/cms/djangoapps/contentstore/views/user.py @@ -11,7 +11,7 @@ from django_future.csrf import ensure_csrf_cookie from mitxmako.shortcuts import render_to_response from django.core.context_processors import csrf -from xmodule.modulestore.django import modulestore +from xmodule.modulestore.django import modulestore, loc_mapper from xmodule.modulestore import Location from xmodule.error_module import ErrorDescriptor from contentstore.utils import get_lms_link_for_item @@ -46,13 +46,13 @@ def index(request): courses = filter(course_filter, courses) def format_course_for_view(course): + # published = false b/c studio manipulates draft versions not b/c the course isn't pub'd + course_url = loc_mapper().translate_location( + course.location.course_id, course.location, published=False, add_entry_if_missing=True + ) return ( course.display_name, - reverse("course_index", kwargs={ - 'org': course.location.org, - 'course': course.location.course, - 'name': course.location.name, - }), + reverse("contentstore.views.course_handler", kwargs={'course_url': course_url}), get_lms_link_for_item( course.location ), diff --git a/cms/templates/widgets/header.html b/cms/templates/widgets/header.html index f5829e4e1f..3be0b3723a 100644 --- a/cms/templates/widgets/header.html +++ b/cms/templates/widgets/header.html @@ -2,6 +2,7 @@ <%! from django.core.urlresolvers import reverse from django.utils.translation import ugettext as _ + from xmodule.modulestore.django import loc_mapper %>
@@ -12,10 +13,16 @@

edX Studio

% if context_course: - <% ctx_loc = context_course.location %> + <% + ctx_loc = context_course.location + index_url = reverse( + 'contentstore.views.course_handler', + kwargs={'course_url': loc_mapper().translate_location(ctx_loc.course_id, ctx_loc, False, True)} + ) + %>

${_("Current Course:")} - + ${context_course.display_org_with_default | h}${context_course.display_number_with_default | h} ${context_course.display_name_with_default} @@ -31,7 +38,7 @@
  • - ${_("Outline")} + ${_("Outline")}
  • ${_("Updates")} diff --git a/cms/urls.py b/cms/urls.py index b465b07f32..3a1856fa7a 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -9,7 +9,7 @@ startup.run() from ratelimitbackend import admin admin.autodiscover() -urlpatterns = ('', # nopep8 +urlpatterns = patterns('', # nopep8 url(r'^$', 'contentstore.views.howitworks', name='homepage'), url(r'^listing', 'contentstore.views.index', name='index'), url(r'^request_course_creator$', 'contentstore.views.request_course_creator', name='request_course_creator'), @@ -25,8 +25,6 @@ urlpatterns = ('', # nopep8 url(r'^create_new_course', 'contentstore.views.create_new_course', name='create_new_course'), url(r'^reorder_static_tabs', 'contentstore.views.reorder_static_tabs', name='reorder_static_tabs'), - url(r'^(?P[^/]+)/(?P[^/]+)/course/(?P[^/]+)$', - 'contentstore.views.course_index', name='course_index'), url(r'^(?P[^/]+)/(?P[^/]+)/import/(?P[^/]+)$', 'contentstore.views.import_course', name='import_course'), url(r'^(?P[^/]+)/(?P[^/]+)/import_status/(?P[^/]+)$', @@ -106,7 +104,8 @@ urlpatterns = ('', # nopep8 ) # User creation and updating views -urlpatterns += ( +urlpatterns += patterns( + '', url(r'^(?P[^/]+)/(?P[^/]+)/checklists/(?P[^/]+)$', 'contentstore.views.get_checklists', name='checklists'), url(r'^(?P[^/]+)/(?P[^/]+)/checklists/(?P[^/]+)/update(/)?(?P.+)?.*$', 'contentstore.views.update_checklist', name='checklists_updates'), @@ -125,22 +124,37 @@ urlpatterns += ( url(r'^logout$', 'student.views.logout_user', name='logout'), ) +# restful api +urlpatterns += patterns( + 'contentstore.views', + # index page, course outline page, and course structure json access + # replaces url(r'^listing', 'contentstore.views.index', name='index'), + # ? url(r'^create_new_course', 'contentstore.views.create_new_course', name='create_new_course') + # TODO remove shim and this pattern once import_export and test_contentstore no longer use + url(r'^(?P[^/]+)/(?P[^/]+)/course/(?P[^/]+)$', + 'course.old_course_index_shim', name='course_index' + ), + + url(r'^course$', 'index'), + url(r'^course/(?P.*)$', 'course_handler'), +) + js_info_dict = { 'domain': 'djangojs', 'packages': ('cms',), } -urlpatterns += ( +urlpatterns += patterns('', # Serve catalog of localized strings to be rendered by Javascript url(r'^i18n.js$', 'django.views.i18n.javascript_catalog', js_info_dict), ) if settings.MITX_FEATURES.get('ENABLE_SERVICE_STATUS'): - urlpatterns += ( + urlpatterns += patterns('', url(r'^status/', include('service_status.urls')), ) -urlpatterns += (url(r'^admin/', include(admin.site.urls)),) +urlpatterns += patterns('', url(r'^admin/', include(admin.site.urls)),) # enable automatic login if settings.MITX_FEATURES.get('AUTOMATIC_AUTH_FOR_TESTING'): @@ -155,8 +169,6 @@ if settings.DEBUG: except ImportError: pass -urlpatterns = patterns(*urlpatterns) - # Custom error pages #pylint: disable=C0103 handler404 = 'contentstore.views.render_404' diff --git a/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py b/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py index 289e4ad102..c15d84e771 100644 --- a/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py +++ b/common/lib/xmodule/xmodule/modulestore/loc_mapper_store.py @@ -56,7 +56,7 @@ class LocMapperStore(object): """ Add a new entry to map this course_location to the new style CourseLocator.course_id. If course_id is not provided, it creates the default map of using org.course.name from the location (just like course_id) if - the location.cateogry = 'course'; otherwise, it uses org.course. + the location.category = 'course'; otherwise, it uses org.course. You can create more than one mapping to the same course_id target. In that case, the reverse translate will be arbitrary (no guarantee of which wins). diff --git a/common/lib/xmodule/xmodule/modulestore/locator.py b/common/lib/xmodule/xmodule/modulestore/locator.py index 47753a4941..749e2556ab 100644 --- a/common/lib/xmodule/xmodule/modulestore/locator.py +++ b/common/lib/xmodule/xmodule/modulestore/locator.py @@ -14,6 +14,8 @@ from xmodule.modulestore.exceptions import InsufficientSpecificationError, OverS from .parsers import parse_url, parse_course_id, parse_block_ref from .parsers import BRANCH_PREFIX, BLOCK_PREFIX, URL_VERSION_PREFIX +import re +from xmodule.modulestore import Location log = logging.getLogger(__name__) @@ -89,6 +91,59 @@ class Locator(object): (property_name, current, new)) setattr(self, property_name, new) + @staticmethod + def to_locator_or_location(location): + """ + Convert the given locator like thing to the appropriate type of object, or, if already + that type, just return it. Returns an old Location, BlockUsageLocator, + or DefinitionLocator. + + :param location: can be a Location, Locator, string, tuple, list, or dict. + """ + if isinstance(location, (Location, Locator)): + return location + if isinstance(location, basestring): + return Locator.parse_url(location) + if isinstance(location, (list, tuple)): + return Location(location) + if isinstance(location, dict) and 'name' in location: + return Location(location) + if isinstance(location, dict): + return BlockUsageLocator(**location) + raise ValueError(location) + + URL_TAG_RE = re.compile(r'^(\w+)://') + @staticmethod + def parse_url(url): + """ + Parse the url into one of the Locator types (must have a tag indicating type) + Return the new instance. Supports i4x, cvx, edx, defx + + :param url: the url to parse + """ + parsed = Locator.URL_TAG_RE.match(url) + if parsed is None: + raise ValueError(parsed) + parsed = parsed.group(1) + if parsed in ['i4x', 'c4x']: + return Location(url) + elif parsed == 'edx': + return BlockUsageLocator(url) + elif parsed == 'defx': + return DefinitionLocator(url) + return None + + @classmethod + def as_object_id(cls, value): + """ + Attempts to cast value as a bson.objectid.ObjectId. + If cast fails, raises ValueError + """ + try: + return ObjectId(value) + except InvalidId: + raise ValueError('"%s" is not a valid version_guid' % value) + class CourseLocator(Locator): """ @@ -208,18 +263,55 @@ class CourseLocator(Locator): version_guid=self.version_guid, branch=self.branch) - @classmethod - def as_object_id(cls, value): + OLD_COURSE_ID_RE = re.compile(r'^(?P[^.]+)\.?(?P.+)?\.(?P[^.]+)$') + @property + def as_old_location_course_id(self): """ - Attempts to cast value as a bson.objectid.ObjectId. - If cast fails, raises ValueError + The original Location type presented its course id as org/course/run. This function + assumes the course_id starts w/ org, has an arbitrarily long 'course' identifier, and then + ends w/ run all separated by periods. + + If this object does not have a course_id, this function returns None. """ - if isinstance(value, ObjectId): - return value - try: - return ObjectId(value) - except InvalidId: - raise ValueError('"%s" is not a valid version_guid' % value) + if self.course_id is None: + return None + parsed = self.OLD_COURSE_ID_RE.match(self.course_id) + # check whether there are 2 or > 2 'fields' + if parsed.group('old_course_id'): + return '/'.join(parsed.groups()) + else: + return parsed.group('org') + '/' + parsed.group('run') + + def _old_location_field_helper(self, field): + """ + Parse course_id to get the old location field named field out + """ + if self.course_id is None: + return None + parsed = self.OLD_COURSE_ID_RE.match(self.course_id) + return parsed.group(field) + + @property + def as_old_location_org(self): + """ + Presume the first part of the course_id is the org and return it. + """ + return self._old_location_field_helper('org') + + @property + def as_old_location_course(self): + """ + Presume the middle part, if any, of the course_id is the old location scheme's + course id and return it. + """ + return self._old_location_field_helper('old_course_id') + + @property + def as_old_location_run(self): + """ + Presume the last part of the course_id is the old location scheme's run and return it. + """ + return self._old_location_field_helper('run') def init_from_url(self, url): """ @@ -230,7 +322,7 @@ class CourseLocator(Locator): url = url.url() if not isinstance(url, basestring): raise TypeError('%s is not an instance of basestring' % url) - parse = parse_url(url) + parse = parse_url(url, tag_optional=True) if not parse: raise ValueError('Could not parse "%s" as a url' % url) self._set_value( @@ -349,7 +441,7 @@ class BlockUsageLocator(CourseLocator): """ self._validate_args(url, version_guid, course_id) if url: - self.init_block_ref_from_url(url) + self.init_block_ref_from_str(url) if course_id: self.init_block_ref_from_course_id(course_id) if usage_id: @@ -401,11 +493,18 @@ class BlockUsageLocator(CourseLocator): raise ValueError('Could not parse "%s" as a block_ref' % block_ref) self.set_usage_id(parse['block']) - def init_block_ref_from_url(self, url): - if isinstance(url, Locator): - url = url.url() - parse = parse_url(url) - assert parse, 'Could not parse "%s" as a url' % url + 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) + 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)) def init_block_ref_from_course_id(self, course_id): @@ -429,8 +528,13 @@ class DefinitionLocator(Locator): Container for how to locate a description (the course-independent content). """ + URL_RE = re.compile(r'^defx://' + URL_VERSION_PREFIX + '([^/]+)$', re.IGNORECASE) def __init__(self, definition_id): - self.definition_id = definition_id + if isinstance(definition_id, basestring): + regex_match = self.URL_RE.match(definition_id) + if regex_match is not None: + definition_id = self.as_object_id(regex_match.group(1)) + self.definition_id = self.as_object_id(definition_id) def __unicode__(self): ''' @@ -442,9 +546,9 @@ class DefinitionLocator(Locator): def url(self): """ Return a string containing the URL for this location. - url(self) returns something like this: 'edx://version/519665f6223ebd6980884f2b' + url(self) returns something like this: 'defx://version/519665f6223ebd6980884f2b' """ - return 'edx://' + unicode(self) + return 'defx://' + unicode(self) def version(self): """ diff --git a/common/lib/xmodule/xmodule/modulestore/parsers.py b/common/lib/xmodule/xmodule/modulestore/parsers.py index 4996cc4e8a..aeb84b5a12 100644 --- a/common/lib/xmodule/xmodule/modulestore/parsers.py +++ b/common/lib/xmodule/xmodule/modulestore/parsers.py @@ -9,13 +9,14 @@ VERSION_PREFIX = "/version/" # Prefix for version when it begins the URL (no course ID). URL_VERSION_PREFIX = 'version/' -URL_RE = re.compile(r'^edx://(.+)$', re.IGNORECASE) +URL_RE = re.compile(r'^(edx://)?(.+)$', re.IGNORECASE) -def parse_url(string): +def parse_url(string, tag_optional=False): """ - A url must begin with 'edx://' (case-insensitive match), - followed by either a version_guid or a course_id. + A url usually begins with 'edx://' (case-insensitive match), + followed by either a version_guid or a course_id. If tag_optional, then + the url does not have to start with the tag and edx will be assumed. Examples: 'edx://version/0123FFFF' @@ -36,7 +37,9 @@ def parse_url(string): match = URL_RE.match(string) if not match: return None - path = match.group(1) + if match.group(1) is None and not tag_optional: + return None + path = match.group(2) if path.startswith(URL_VERSION_PREFIX): return parse_guid(path[len(URL_VERSION_PREFIX):]) return parse_course_id(path) 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 2373c409de..78affa9aca 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -135,7 +135,7 @@ class SplitModuleCourseTests(SplitModuleTest): self.assertEqual( len(course.children), 3, "children") - self.assertEqual(course.definition_locator.definition_id, "head12345_12") + self.assertEqual(str(course.definition_locator.definition_id), "ad00000000000000dddd0000") # check dates and graders--forces loading of descriptor self.assertEqual(course.edited_by, "testassist@edx.org") self.assertEqual(str(course.previous_version), self.GUID_D1) @@ -195,7 +195,7 @@ class SplitModuleCourseTests(SplitModuleTest): self.assertEqual(course.graceperiod, datetime.timedelta(hours=2)) self.assertIsNone(course.advertised_start) self.assertEqual(len(course.children), 0) - self.assertEqual(course.definition_locator.definition_id, "head12345_11") + self.assertEqual(str(course.definition_locator.definition_id), "ad00000000000000dddd0001") # check dates and graders--forces loading of descriptor self.assertEqual(course.edited_by, "testassist@edx.org") self.assertDictEqual(course.grade_cutoffs, {"Pass": 0.55}) @@ -345,7 +345,7 @@ class SplitModuleItemTests(SplitModuleTest): self.assertEqual(block.display_name, "The Ancient Greek Hero") self.assertEqual(block.advertised_start, "Fall 2013") self.assertEqual(len(block.children), 3) - self.assertEqual(block.definition_locator.definition_id, "head12345_12") + self.assertEqual(str(block.definition_locator.definition_id), "ad00000000000000dddd0000") # check dates and graders--forces loading of descriptor self.assertEqual(block.edited_by, "testassist@edx.org") self.assertDictEqual( @@ -375,7 +375,7 @@ class SplitModuleItemTests(SplitModuleTest): block = modulestore().get_item(locator) self.assertEqual(block.location.course_id, "GreekHero") self.assertEqual(block.category, 'chapter') - self.assertEqual(block.definition_locator.definition_id, "chapter12345_1") + self.assertEqual(str(block.definition_locator.definition_id), "cd00000000000000dddd0020") self.assertEqual(block.display_name, "Hercules") self.assertEqual(block.edited_by, "testassist@edx.org") @@ -562,13 +562,13 @@ class TestItemCrud(SplitModuleTest): new_module = modulestore().create_item( locator, category, 'user123', fields={'display_name': 'new chapter'}, - definition_locator=DefinitionLocator("chapter12345_2") + definition_locator=DefinitionLocator("cd00000000000000dddd0022") ) # 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.assertEqual(new_module.definition_locator.definition_id, "chapter12345_2") + self.assertEqual(str(new_module.definition_locator.definition_id), "cd00000000000000dddd0022") def test_unique_naming(self): """ @@ -588,7 +588,7 @@ class TestItemCrud(SplitModuleTest): another_module = modulestore().create_item( locator, category, 'anotheruser', fields={'display_name': 'problem 2', 'data': another_payload}, - definition_locator=DefinitionLocator("problem12345_3_1"), + definition_locator=DefinitionLocator("0d00000040000000dddd0031"), ) # check that course version changed and course's previous is the other one parent = modulestore().get_item(locator) @@ -605,7 +605,7 @@ class TestItemCrud(SplitModuleTest): self.assertLessEqual(new_history['edited_on'], datetime.datetime.now(UTC)) self.assertGreaterEqual(new_history['edited_on'], premod_time) another_history = modulestore().get_definition_history_info(another_module.definition_locator) - self.assertEqual(another_history['previous_version'], 'problem12345_3_1') + self.assertEqual(str(another_history['previous_version']), '0d00000040000000dddd0031') def test_create_continue_version(self): """ @@ -789,7 +789,7 @@ class TestItemCrud(SplitModuleTest): modulestore().create_item( locator, category, 'test_update_manifold', fields={'display_name': 'problem 2', 'data': another_payload}, - definition_locator=DefinitionLocator("problem12345_3_1"), + definition_locator=DefinitionLocator("0d00000040000000dddd0031"), ) # pylint: disable=W0212 modulestore()._clear_cache() diff --git a/common/test/data/splitmongo_json/definitions.json b/common/test/data/splitmongo_json/definitions.json index 433afbe8ac..81b84fe951 100644 --- a/common/test/data/splitmongo_json/definitions.json +++ b/common/test/data/splitmongo_json/definitions.json @@ -1,6 +1,6 @@ [ { - "_id":"head12345_12", + "_id": { "$oid" : "ad00000000000000dddd0000"}, "category":"course", "fields":{ "textbooks":[ @@ -46,12 +46,12 @@ "edit_info": { "edited_by":"testassist@edx.org", "edited_on":{"$date" : 1364481713238}, - "previous_version":"head12345_11", - "original_version":"head12345_10" + "previous_version":{ "$oid" : "ad00000000000000dddd0001"}, + "original_version":{ "$oid" : "ad00000000000000dddd0010"} } }, { - "_id":"head12345_11", + "_id":{ "$oid" : "ad00000000000000dddd0001"}, "category":"course", "fields":{ "textbooks":[ @@ -97,12 +97,12 @@ "edit_info": { "edited_by":"testassist@edx.org", "edited_on":{"$date" : 1364481713238}, - "previous_version":"head12345_10", - "original_version":"head12345_10" + "previous_version":{ "$oid" : "ad00000000000000dddd0010"}, + "original_version":{ "$oid" : "ad00000000000000dddd0010"} } }, { - "_id":"head12345_10", + "_id":{ "$oid" : "ad00000000000000dddd0010"}, "category":"course", "fields":{ "textbooks":[ @@ -149,11 +149,11 @@ "edited_by":"test@edx.org", "edited_on":{"$date": 1364473713238}, "previous_version":null, - "original_version":"head12345_10" + "original_version":{ "$oid" : "ad00000000000000dddd0010"} } }, { - "_id":"head23456_1", + "_id":{ "$oid" : "ad00000000000000dddd0020"}, "category":"course", "fields":{ "textbooks":[ @@ -199,12 +199,12 @@ "edit_info": { "edited_by":"test@edx.org", "edited_on":{"$date": 1364481313238}, - "previous_version":"head23456_0", - "original_version":"head23456_0" + "previous_version":{ "$oid" : "2d00000000000000dddd0020"}, + "original_version":{ "$oid" : "2d00000000000000dddd0020"} } }, { - "_id":"head23456_0", + "_id":{ "$oid" : "2d00000000000000dddd0020"}, "category":"course", "fields":{ "textbooks":[ @@ -251,11 +251,11 @@ "edited_by":"test@edx.org", "edited_on":{"$date" : 1364481313238}, "previous_version":null, - "original_version":"head23456_0" + "original_version":{ "$oid" : "2d00000000000000dddd0020"} } }, { - "_id":"head345679_1", + "_id":{ "$oid" : "3d00000000000000dddd0020"}, "category":"course", "fields":{ "textbooks":[ @@ -295,62 +295,62 @@ "edited_by":"test@edx.org", "edited_on":{"$date" : 1364481313238}, "previous_version":null, - "original_version":"head23456_0" + "original_version":{ "$oid" : "2d00000000000000dddd0020"} } }, { - "_id":"chapter12345_1", + "_id":{ "$oid" : "cd00000000000000dddd0020"}, "category":"chapter", "fields":{}, "edit_info": { "edited_by":"testassist@edx.org", "edited_on":{"$date" : 1364483713238}, "previous_version":null, - "original_version":"chapter12345_1" + "original_version":{ "$oid" : "cd00000000000000dddd0020"} } }, { - "_id":"chapter12345_2", + "_id":{ "$oid" : "cd00000000000000dddd0022"}, "category":"chapter", "fields":{}, "edit_info": { "edited_by":"testassist@edx.org", "edited_on":{"$date" : 1364483713238}, "previous_version":null, - "original_version":"chapter12345_2" + "original_version":{ "$oid" : "cd00000000000000dddd0022"} } }, { - "_id":"chapter12345_3", + "_id":{ "$oid" : "cd00000000000000dddd0032"}, "category":"chapter", "fields":{}, "edit_info": { "edited_by":"testassist@edx.org", "edited_on":{"$date" : 1364483713238}, "previous_version":null, - "original_version":"chapter12345_3" + "original_version":{ "$oid" : "cd00000000000000dddd0032"} } }, { - "_id":"problem12345_3_1", + "_id":{ "$oid" : "0d00000040000000dddd0031"}, "category":"problem", "fields": {"data": ""}, "edit_info": { "edited_by":"testassist@edx.org", "edited_on":{"$date" : 1364483713238}, "previous_version":null, - "original_version":"problem12345_3_1" + "original_version":{ "$oid" : "0d00000040000000dddd0031"} } }, { - "_id":"problem12345_3_2", + "_id":{ "$oid" : "0d00000040000000dddd0032"}, "category":"problem", "fields": {"data": ""}, "edit_info": { "edited_by":"testassist@edx.org", "edited_on":{"$date" : 1364483713238}, "previous_version":null, - "original_version":"problem12345_3_2" + "original_version":{ "$oid" : "0d00000040000000dddd0032"} } } ] \ No newline at end of file diff --git a/common/test/data/splitmongo_json/structures.json b/common/test/data/splitmongo_json/structures.json index b72c0fd7a5..00d5c7892b 100644 --- a/common/test/data/splitmongo_json/structures.json +++ b/common/test/data/splitmongo_json/structures.json @@ -11,7 +11,7 @@ "blocks":{ "head12345":{ "category":"course", - "definition":"head12345_12", + "definition":{ "$oid" : "ad00000000000000dddd0000"}, "fields":{ "children":[ "chapter1", @@ -65,7 +65,7 @@ }, "chapter1":{ "category":"chapter", - "definition":"chapter12345_1", + "definition":{ "$oid" : "cd00000000000000dddd0020"}, "fields":{ "children":[ @@ -83,7 +83,7 @@ }, "chapter2":{ "category":"chapter", - "definition":"chapter12345_2", + "definition":{ "$oid" : "cd00000000000000dddd0022"}, "fields":{ "children":[ @@ -101,7 +101,7 @@ }, "chapter3":{ "category":"chapter", - "definition":"chapter12345_3", + "definition":{ "$oid" : "cd00000000000000dddd0032"}, "fields":{ "children":[ "problem1", @@ -120,7 +120,7 @@ }, "problem1":{ "category":"problem", - "definition":"problem12345_3_1", + "definition":{ "$oid" : "0d00000040000000dddd0031"}, "fields":{ "children":[ @@ -139,7 +139,7 @@ }, "problem3_2":{ "category":"problem", - "definition":"problem12345_3_2", + "definition":{ "$oid" : "0d00000040000000dddd0032"}, "fields":{ "children":[ @@ -169,7 +169,7 @@ "blocks":{ "head12345":{ "category":"course", - "definition":"head12345_11", + "definition":{ "$oid" : "ad00000000000000dddd0001"}, "fields":{ "children":[ @@ -233,7 +233,7 @@ "blocks":{ "head12345":{ "category":"course", - "definition":"head12345_10", + "definition":{ "$oid" : "ad00000000000000dddd0010"}, "fields":{ "children":[ @@ -287,7 +287,7 @@ "blocks":{ "head23456":{ "category":"course", - "definition":"head23456_1", + "definition":{ "$oid" : "ad00000000000000dddd0020"}, "fields":{ "children":[ @@ -342,7 +342,7 @@ "blocks":{ "head23456":{ "category":"course", - "definition":"head23456_0", + "definition":{ "$oid" : "2d00000000000000dddd0020"}, "fields":{ "children":[ @@ -396,7 +396,7 @@ "blocks":{ "head23456":{ "category":"course", - "definition":"head23456_1", + "definition":{ "$oid" : "ad00000000000000dddd0020"}, "fields":{ "children":[ @@ -450,7 +450,7 @@ "blocks":{ "head345679":{ "category":"course", - "definition":"head345679_1", + "definition":{ "$oid" : "3d00000000000000dddd0020"}, "fields":{ "children":[