From 4f60a30afc06caae6f986a9d33543aef6610053b Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Mon, 21 Oct 2013 17:17:20 -0400 Subject: [PATCH] Remove as_old_location methods They were incorrect if there were periods in the org or name. --- cms/djangoapps/auth/authz.py | 4 +- cms/djangoapps/contentstore/views/course.py | 6 +-- .../xmodule/xmodule/modulestore/locator.py | 50 ------------------- .../modulestore/tests/test_locators.py | 15 ------ 4 files changed, 4 insertions(+), 71 deletions(-) diff --git a/cms/djangoapps/auth/authz.py b/cms/djangoapps/auth/authz.py index 42b427605d..446d7be982 100644 --- a/cms/djangoapps/auth/authz.py +++ b/cms/djangoapps/auth/authz.py @@ -37,9 +37,7 @@ def get_course_groupname_for_role(location, role): groupnames.append('{0}_{1}'.format(role, location.course)) elif isinstance(location, CourseLocator): old_location = loc_mapper().translate_locator_to_location(location) - if old_location is None: - groupnames.append('{0}_{1}'.format(role, location.as_old_location_course_id)) - else: + if old_location: groupnames.append('{0}_{1}'.format(role, old_location.course_id)) for groupname in groupnames: diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index f043c09d73..cf6ef7f626 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -131,9 +131,9 @@ def course_index(request, course_url): lms_link = get_lms_link_for_item(old_location) upload_asset_callback_url = reverse('upload_asset', kwargs={ - 'org': location.as_old_location_org, - 'course': location.as_old_location_course, - 'coursename': location.as_old_location_run + 'org': old_location.org, + 'course': old_location.course, + 'coursename': old_location.name }) course = modulestore().get_item(old_location, depth=3) diff --git a/common/lib/xmodule/xmodule/modulestore/locator.py b/common/lib/xmodule/xmodule/modulestore/locator.py index 749e2556ab..dffd182764 100644 --- a/common/lib/xmodule/xmodule/modulestore/locator.py +++ b/common/lib/xmodule/xmodule/modulestore/locator.py @@ -263,56 +263,6 @@ class CourseLocator(Locator): version_guid=self.version_guid, branch=self.branch) - OLD_COURSE_ID_RE = re.compile(r'^(?P[^.]+)\.?(?P.+)?\.(?P[^.]+)$') - @property - def as_old_location_course_id(self): - """ - 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 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): """ url must be a string beginning with 'edx://' and containing diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py b/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py index be00f341b8..2db00f279d 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_locators.py @@ -283,21 +283,6 @@ class LocatorTest(TestCase): Locator.to_locator_or_location("hello.world.not.a.url") self.assertIsNone(Locator.parse_url("unknown://foo.bar/baz")) - def test_as_old(self): - """ - Test the as_old_location_xxx accessors - """ - locator = CourseLocator(course_id='org.course.id.run', branch='mybranch') - self.assertEqual('org', locator.as_old_location_org) - self.assertEqual('course.id', locator.as_old_location_course) - self.assertEqual('run', locator.as_old_location_run) - self.assertEqual('org/course.id/run', locator.as_old_location_course_id) - locator = CourseLocator(course_id='org.course', branch='mybranch') - self.assertEqual('org', locator.as_old_location_org) - self.assertIsNone(locator.as_old_location_course) - self.assertEqual('course', locator.as_old_location_run) - self.assertEqual('org/course', locator.as_old_location_course_id) - def test_description_locator_url(self): object_id = '{:024x}'.format(random.randrange(16 ** 24)) definition_locator = DefinitionLocator(object_id)