Merge pull request #2660 from edx/dbarch/parse_course_id

Dbarch/parse course
This commit is contained in:
Don Mitchell
2014-02-24 13:24:00 -05:00
22 changed files with 128 additions and 95 deletions

View File

@@ -50,7 +50,7 @@ from dark_lang.models import DarkLangConfig
from xmodule.course_module import CourseDescriptor
from xmodule.modulestore.exceptions import ItemNotFoundError
from xmodule.modulestore.django import modulestore
from xmodule.modulestore import XML_MODULESTORE_TYPE
from xmodule.modulestore import XML_MODULESTORE_TYPE, Location
from collections import namedtuple
@@ -593,12 +593,12 @@ def change_enrollment(request):
current_mode = available_modes[0]
org, course_num, run = course_id.split("/")
course_id_dict = Location.parse_course_id(course_id)
dog_stats_api.increment(
"common.student.enrollment",
tags=[u"org:{0}".format(org),
u"course:{0}".format(course_num),
u"run:{0}".format(run)]
tags=[u"org:{org}".format(**course_id_dict),
u"course:{course}".format(**course_id_dict),
u"run:{name}".format(**course_id_dict)]
)
CourseEnrollment.enroll(user, course.id, mode=current_mode.slug)
@@ -622,12 +622,12 @@ def change_enrollment(request):
if not CourseEnrollment.is_enrolled(user, course_id):
return HttpResponseBadRequest(_("You are not enrolled in this course"))
CourseEnrollment.unenroll(user, course_id)
org, course_num, run = course_id.split("/")
course_id_dict = Location.parse_course_id(course_id)
dog_stats_api.increment(
"common.student.unenrollment",
tags=["org:{0}".format(org),
"course:{0}".format(course_num),
"run:{0}".format(run)]
tags=[u"org:{org}".format(**course_id_dict),
u"course:{course}".format(**course_id_dict),
u"run:{name}".format(**course_id_dict)]
)
return HttpResponse()
else:

View File

@@ -117,11 +117,11 @@ class StaticContent(object):
Returns a path to a piece of static content when we are provided with a filepath and
a course_id
"""
org, course_num, __ = course_id.split("/")
# Generate url of urlparse.path component
scheme, netloc, orig_path, params, query, fragment = urlparse(path)
loc = StaticContent.compute_location(org, course_num, orig_path)
course_id_dict = Location.parse_course_id(course_id)
loc = StaticContent.compute_location(course_id_dict['org'], course_id_dict['course'], orig_path)
loc_url = StaticContent.get_url_path_from_location(loc)
# Reconstruct with new path

View File

@@ -802,8 +802,10 @@ class CourseDescriptor(CourseFields, SequenceDescriptor):
'''Convert the given course_id (org/course/name) to a location object.
Throws ValueError if course_id is of the wrong format.
'''
org, course, name = course_id.split('/')
return Location('i4x', org, course, 'course', name)
course_id_dict = Location.parse_course_id(course_id)
course_id_dict['tag'] = 'i4x'
course_id_dict['category'] = 'course'
return Location(course_id_dict)
@staticmethod
def location_to_id(location):

View File

@@ -261,6 +261,24 @@ class Location(_LocationBase):
return "/".join([self.org, self.course, self.name])
COURSE_ID_RE = re.compile("""
(?P<org>[^/]+)/
(?P<course>[^/]+)/
(?P<name>.*)
""", re.VERBOSE)
@staticmethod
def parse_course_id(course_id):
"""
Given a org/course/name course_id, return a dict of {"org": org, "course": course, "name": name}
If the course_id is not of the right format, raise ValueError
"""
match = Location.COURSE_ID_RE.match(course_id)
if match is None:
raise ValueError("{} is not of form ORG/COURSE/NAME".format(course_id))
return match.groupdict()
def _replace(self, **kwargs):
"""
Return a new :class:`Location` with values replaced

View File

@@ -565,9 +565,11 @@ class MongoModuleStore(ModuleStoreWriteBase):
"""
Get the course with the given courseid (org/course/run)
"""
id_components = course_id.split('/')
id_components = Location.parse_course_id(course_id)
id_components['tag'] = 'i4x'
id_components['category'] = 'course'
try:
return self.get_item(Location('i4x', id_components[0], id_components[1], 'course', id_components[2]))
return self.get_item(Location(id_components))
except ItemNotFoundError:
return None

View File

@@ -46,8 +46,9 @@ def rewrite_nonportable_content_links(source_course_id, dest_course_id, text):
"""
org, course, run = source_course_id.split("/")
dest_org, dest_course, dest_run = dest_course_id.split("/")
course_id_dict = Location.parse_course_id(source_course_id)
course_id_dict['tag'] = 'i4x'
course_id_dict['category'] = 'course'
def portable_asset_link_subtitution(match):
quote = match.group('quote')
@@ -60,14 +61,12 @@ def rewrite_nonportable_content_links(source_course_id, dest_course_id, text):
return quote + '/jump_to_id/' + rest + quote
def generic_courseware_link_substitution(match):
quote = match.group('quote')
rest = match.group('rest')
dest_generic_courseware_lik_base = '/courses/{org}/{course}/{run}/'.format(
org=dest_org, course=dest_course, run=dest_run
)
return quote + dest_generic_courseware_lik_base + rest + quote
parts = Location.parse_course_id(dest_course_id)
parts['quote'] = match.group('quote')
parts['rest'] = match.group('rest')
return u'{quote}/courses/{org}/{course}/{name}/{rest}{quote}'.format(**parts)
course_location = Location(['i4x', org, course, 'course', run])
course_location = Location(course_id_dict)
# NOTE: ultimately link updating is not a hard requirement, so if something blows up with
# the regex subsitution, log the error and continue
@@ -78,24 +77,20 @@ def rewrite_nonportable_content_links(source_course_id, dest_course_id, text):
logging.warning("Error going regex subtituion %r on text = %r.\n\nError msg = %s", c4x_link_base, text, str(e))
try:
jump_to_link_base = u'/courses/{org}/{course}/{run}/jump_to/i4x://{org}/{course}/'.format(
org=org, course=course, run=run
)
jump_to_link_base = u'/courses/{org}/{course}/{name}/jump_to/i4x://{org}/{course}/'.format(**course_id_dict)
text = re.sub(_prefix_and_category_url_replace_regex(jump_to_link_base), portable_jump_to_link_substitution, text)
except Exception, e:
logging.warning("Error going regex subtituion %r on text = %r.\n\nError msg = %s", jump_to_link_base, text, str(e))
# Also, there commonly is a set of link URL's used in the format:
# /courses/<org>/<course>/<run> which will be broken if migrated to a different course_id
# /courses/<org>/<course>/<name> which will be broken if migrated to a different course_id
# so let's rewrite those, but the target will also be non-portable,
#
# Note: we only need to do this if we are changing course-id's
#
if source_course_id != dest_course_id:
try:
generic_courseware_link_base = u'/courses/{org}/{course}/{run}/'.format(
org=org, course=course, run=run
)
generic_courseware_link_base = u'/courses/{org}/{course}/{name}/'.format(**course_id_dict)
text = re.sub(_prefix_only_url_replace_regex(generic_courseware_link_base), portable_asset_link_subtitution, text)
except Exception, e:
logging.warning("Error going regex subtituion %r on text = %r.\n\nError msg = %s", generic_courseware_link_base, text, str(e))

View File

@@ -191,3 +191,15 @@ class TestLocations(TestCase):
loc = Location('t://o/c/c/n@r')
with self.assertRaises(AttributeError):
setattr(loc, attr, attr)
def test_parse_course_id(self):
"""
Test the parse_course_id class method
"""
source_string = "myorg/mycourse/myrun"
parsed = Location.parse_course_id(source_string)
self.assertEqual(parsed['org'], 'myorg')
self.assertEqual(parsed['course'], 'mycourse')
self.assertEqual(parsed['name'], 'myrun')
with self.assertRaises(ValueError):
Location.parse_course_id('notlegit.id/foo')

View File

@@ -78,8 +78,11 @@ class TestMixedModuleStore(object):
tz_aware=True,
)
cls.connection.drop_database(DB)
cls.fake_location = Location(['i4x', 'foo', 'bar', 'vertical', 'baz'])
cls.import_org, cls.import_course, cls.import_run = IMPORT_COURSEID.split('/')
cls.fake_location = Location('i4x', 'foo', 'bar', 'vertical', 'baz')
import_course_dict = Location.parse_course_id(IMPORT_COURSEID)
cls.import_org = import_course_dict['org']
cls.import_course = import_course_dict['course']
cls.import_run = import_course_dict['name']
# NOTE: Creating a single db for all the tests to save time. This
# is ok only as long as none of the tests modify the db.
# If (when!) that changes, need to either reload the db, or load

View File

@@ -58,7 +58,10 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem):
"""
self.unnamed = defaultdict(int) # category -> num of new url_names for that category
self.used_names = defaultdict(set) # category -> set of used url_names
self.org, self.course, self.url_name = course_id.split('/')
course_id_dict = Location.parse_course_id(course_id)
self.org = course_id_dict['org']
self.course = course_id_dict['course']
self.url_name = course_id_dict['name']
if id_reader is None:
id_reader = LocationReader()
id_generator = CourseLocationGenerator(self.org, self.course)

View File

@@ -149,14 +149,10 @@ def import_from_xml(
for course_id in xml_module_store.modules.keys():
if target_location_namespace is not None:
pseudo_course_id = '/'.join(
[target_location_namespace.org, target_location_namespace.course]
)
pseudo_course_id = u'{0.org}/{0.course}'.format(target_location_namespace)
else:
course_id_components = course_id.split('/')
pseudo_course_id = '/'.join(
[course_id_components[0], course_id_components[1]]
)
course_id_components = Location.parse_course_id(course_id)
pseudo_course_id = u'{org}/{course}'.format(**course_id_components)
try:
# turn off all write signalling while importing as this
@@ -761,11 +757,11 @@ def perform_xlint(
)
# check for a presence of a course marketing video
location_elements = course_id.split('/')
loc = Location([
'i4x', location_elements[0], location_elements[1],
'about', 'video', None
])
location_elements = Location.parse_course_id(course_id)
location_elements['tag'] = 'i4x'
location_elements['category'] = 'about'
location_elements['name'] = 'video'
loc = Location(location_elements)
if loc not in module_store.modules[course_id]:
print(
"WARN: Missing course marketing video. It is recommended "