Commit 81a92e4b by Don Mitchell

Consolidate course_id triple parsing

parent 2057f518
......@@ -7,6 +7,7 @@ from xmodule.modulestore.django import modulestore
from xmodule.contentstore.django import contentstore
from xmodule.course_module import CourseDescriptor
from student.roles import CourseInstructorRole, CourseStaffRole
from xmodule.modulestore import Location
#
......@@ -27,8 +28,8 @@ class Command(BaseCommand):
mstore = modulestore('direct')
cstore = contentstore()
org, course_num, _ = dest_course_id.split("/")
mstore.ignore_write_events_on_courses.append('{0}/{1}'.format(org, course_num))
course_id_dict = Location.parse_course_id(dest_course_id)
mstore.ignore_write_events_on_courses.append('{org}/{course}'.format(**course_id_dict))
print("Cloning course {0} to {1}".format(source_course_id, dest_course_id))
......
......@@ -35,8 +35,8 @@ def delete_course_and_groups(course_id, commit=False):
module_store = modulestore('direct')
content_store = contentstore()
org, course_num, _ = course_id.split("/")
module_store.ignore_write_events_on_courses.append('{0}/{1}'.format(org, course_num))
course_id_dict = Location.parse_course_id(course_id)
module_store.ignore_write_events_on_courses.append('{org}/{course}'.format(**course_id_dict))
loc = CourseDescriptor.id_to_location(course_id)
if delete_course(module_store, content_store, loc, commit):
......
......@@ -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:
......
......@@ -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
......
......@@ -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):
......
......@@ -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
......
......@@ -557,9 +557,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
......
......@@ -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))
......
......@@ -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')
......@@ -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
......
......@@ -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)
......
......@@ -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 "
......
......@@ -45,6 +45,7 @@ from instructor_task.subtasks import (
check_subtask_is_valid,
update_subtask_status,
)
from xmodule.modulestore import Location
log = get_task_logger(__name__)
......@@ -372,7 +373,7 @@ def _get_source_address(course_id, course_title):
# so pull out the course_num. Then make sure that it can be used
# in an email address, by substituting a '_' anywhere a non-(ascii, period, or dash)
# character appears.
course_num = course_id.split('/')[1]
course_num = Location.parse_course_id(course_id)['course']
invalid_chars = re.compile(r"[^\w.-]")
course_num = invalid_chars.sub('_', course_num)
......
......@@ -11,7 +11,7 @@ from courseware.tests.tests import TEST_DATA_MONGO_MODULESTORE
from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
from xmodule.modulestore.django import modulestore
from xmodule.modulestore import XML_MODULESTORE_TYPE
from xmodule.modulestore import XML_MODULESTORE_TYPE, Location
from mock import patch
......@@ -95,17 +95,16 @@ class CourseAuthorizationFormTest(ModuleStoreTestCase):
@patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True})
def test_course_name_only(self):
# Munge course id - common
bad_id = self.course.id.split('/')[-1]
bad_id = Location.parse_course_id(self.course.id)['name']
form_data = {'course_id': bad_id, 'email_enabled': True}
form = CourseAuthorizationAdminForm(data=form_data)
# Validation shouldn't work
self.assertFalse(form.is_valid())
msg = u'Error encountered (Need more than 1 value to unpack)'
msg += u' --- Entered course id was: "{0}". '.format(bad_id)
msg += 'Please recheck that you have supplied a course id in the format: ORG/COURSE/RUN'
self.assertEquals(msg, form._errors['course_id'][0]) # pylint: disable=protected-access
error_msg = form._errors['course_id'][0]
self.assertIn(u'--- Entered course id was: "{0}". '.format(bad_id), error_msg)
self.assertIn(u'Please recheck that you have supplied a course id in the format: ORG/COURSE/RUN', error_msg)
with self.assertRaisesRegexp(ValueError, "The CourseAuthorization could not be created because the data didn't validate."):
form.save()
......
......@@ -15,6 +15,7 @@ from verify_student.models import SoftwareSecurePhotoVerification
import json
import random
import logging
from xmodule.modulestore import Location
logger = logging.getLogger(__name__)
......@@ -179,23 +180,18 @@ class XQueueCertInterface(object):
mode_is_verified = (enrollment_mode == GeneratedCertificate.MODES.verified)
user_is_verified = SoftwareSecurePhotoVerification.user_is_verified(student)
user_is_reverified = SoftwareSecurePhotoVerification.user_is_reverified_for_all(course_id, student)
org = course_id.split('/')[0]
course_num = course_id.split('/')[1]
course_id_dict = Location.parse_course_id(course_id)
cert_mode = enrollment_mode
if (mode_is_verified and user_is_verified and user_is_reverified):
template_pdf = "certificate-template-{0}-{1}-verified.pdf".format(
org, course_num)
template_pdf = "certificate-template-{org}-{course}-verified.pdf".format(**course_id_dict)
elif (mode_is_verified and not (user_is_verified and user_is_reverified)):
template_pdf = "certificate-template-{0}-{1}.pdf".format(
org, course_num)
template_pdf = "certificate-template-{org}-{course}.pdf".format(**course_id_dict)
cert_mode = GeneratedCertificate.MODES.honor
else:
# honor code and audit students
template_pdf = "certificate-template-{0}-{1}.pdf".format(
org, course_num)
template_pdf = "certificate-template-{org}-{course}.pdf".format(**course_id_dict)
cert, created = GeneratedCertificate.objects.get_or_create(
user=student, course_id=course_id)
cert, __ = GeneratedCertificate.objects.get_or_create(user=student, course_id=course_id)
cert.mode = cert_mode
cert.user = student
......
......@@ -320,12 +320,12 @@ def get_module_for_descriptor_internal(user, descriptor, field_data_cache, cours
# Bin score into range and increment stats
score_bucket = get_score_bucket(student_module.grade, student_module.max_grade)
org, course_num, run = course_id.split("/")
course_id_dict = Location.parse_course_id(course_id)
tags = [
u"org:{0}".format(org),
u"course:{0}".format(course_num),
u"run:{0}".format(run),
u"org:{org}".format(**course_id_dict),
u"course:{course}".format(**course_id_dict),
u"run:{name}".format(**course_id_dict),
u"score_bucket:{0}".format(score_bucket)
]
......
......@@ -17,6 +17,7 @@ from django.utils.translation import ugettext as _
import mongoengine
from dashboard.models import CourseImportLog
from xmodule.modulestore import Location
log = logging.getLogger(__name__)
......@@ -158,15 +159,14 @@ def add_repo(repo, rdir_in):
# extract course ID from output of import-command-run and make symlink
# this is needed in order for custom course scripts to work
match = re.search('(?ms)===> IMPORTING course to location ([^ \n]+)',
match = re.search('(?ms)===> IMPORTING course to location (\S+)',
ret_import)
if match:
location = match.group(1).strip()
location = Location(match.group(1))
log.debug('location = {0}'.format(location))
course_id = location.replace('i4x://', '').replace(
'/course/', '/').split('\n')[0].strip()
course_id = location.course_id
cdir = '{0}/{1}'.format(GIT_REPO_DIR, course_id.split('/')[1])
cdir = '{0}/{1}'.format(GIT_REPO_DIR, location.course)
log.debug('Studio course dir = {0}'.format(cdir))
if os.path.exists(cdir) and not os.path.islink(cdir):
......@@ -200,7 +200,7 @@ def add_repo(repo, rdir_in):
'check MONGODB_LOG settings')
cil = CourseImportLog(
course_id=course_id,
location=location,
location=unicode(location),
repo_dir=rdir,
created=timezone.now(),
import_log=ret_import,
......
......@@ -89,7 +89,8 @@ def get_hints(request, course_id, field):
# The course_id is of the form school/number/classname.
# We want to use the course_id to find all matching usage_id's.
# To do this, just take the school/number part - leave off the classname.
chopped_id = '/'.join(course_id.split('/')[:-1])
course_id_dict = Location.parse_course_id(course_id)
chopped_id = u'{org}/{course}'.format(**course_id_dict)
chopped_id = re.escape(chopped_id)
all_hints = XModuleUserStateSummaryField.objects.filter(field_name=field, usage_id__regex=chopped_id)
# big_out_dict[problem id] = [[answer, {pk: [hint, votes]}], sorted by answer]
......
......@@ -53,6 +53,7 @@ from .tools import (
set_due_date_extension,
strip_if_string,
)
from xmodule.modulestore import Location
log = logging.getLogger(__name__)
......@@ -1115,6 +1116,7 @@ def _msk_from_problem_urlname(course_id, urlname):
if "combinedopenended" not in urlname:
urlname = "problem/" + urlname
(org, course_name, __) = course_id.split("/")
module_state_key = "i4x://" + org + "/" + course_name + "/" + urlname
parts = Location.parse_course_id(course_id)
parts['urlname'] = urlname
module_state_key = u"i4x://{org}/{course}/{urlname}".format(**parts)
return module_state_key
......@@ -13,7 +13,7 @@ from django.conf import settings
from xmodule_modifiers import wrap_xblock
from xmodule.html_module import HtmlDescriptor
from xmodule.modulestore import XML_MODULESTORE_TYPE
from xmodule.modulestore import XML_MODULESTORE_TYPE, Location
from xmodule.modulestore.django import modulestore
from xblock.field_data import DictFieldData
from xblock.fields import ScopeIds
......@@ -102,16 +102,16 @@ def _section_course_info(course_id, access):
""" Provide data for the corresponding dashboard section """
course = get_course_by_id(course_id, depth=None)
course_org, course_num, course_name = course_id.split('/')
course_id_dict = Location.parse_course_id(course_id)
section_data = {
'section_key': 'course_info',
'section_display_name': _('Course Info'),
'access': access,
'course_id': course_id,
'course_org': course_org,
'course_num': course_num,
'course_name': course_name,
'course_org': course_id_dict['org'],
'course_num': course_id_dict['course'],
'course_name': course_id_dict['name'],
'course_display_name': course.display_name,
'enrollment_count': CourseEnrollment.num_enrolled_in(course_id),
'has_started': course.has_started(),
......
......@@ -24,7 +24,7 @@ from django.utils import timezone
from xmodule_modifiers import wrap_xblock
import xmodule.graders as xmgraders
from xmodule.modulestore import XML_MODULESTORE_TYPE
from xmodule.modulestore import XML_MODULESTORE_TYPE, Location
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import ItemNotFoundError
from xmodule.html_module import HtmlDescriptor
......@@ -170,8 +170,9 @@ def instructor_dashboard(request, course_id):
urlname = "problem/" + urlname
# complete the url using information about the current course:
(org, course_name, _) = course_id.split("/")
return u"i4x://{org}/{name}/{url}".format(org=org, name=course_name, url=urlname)
parts = Location.parse_course_id(course_id)
parts['url'] = urlname
return u"i4x://{org}/{name}/{url}".format(**parts)
def get_student_from_identifier(unique_student_identifier):
"""Gets a student object using either an email address or username"""
......@@ -571,10 +572,12 @@ def instructor_dashboard(request, course_id):
if problem_to_dump[-4:] == ".xml":
problem_to_dump = problem_to_dump[:-4]
try:
(org, course_name, _) = course_id.split("/")
module_state_key = "i4x://" + org + "/" + course_name + "/problem/" + problem_to_dump
smdat = StudentModule.objects.filter(course_id=course_id,
module_state_key=module_state_key)
course_id_dict = Location.parse_course_id(course_id)
module_state_key = u"i4x://{org}/{course}/problem/{0}".format(problem_to_dump, **course_id_dict)
smdat = StudentModule.objects.filter(
course_id=course_id,
module_state_key=module_state_key
)
smdat = smdat.order_by('student')
msg += _u("Found {num} records to dump.").format(num=smdat)
except Exception as err:
......
......@@ -91,10 +91,9 @@ def find_peer_grading_module(course):
problem_url = ""
# Get the course id and split it.
course_id_parts = course.id.split("/")
peer_grading_query = course.location.replace(category='peergrading', name=None)
# Get the peer grading modules currently in the course. Explicitly specify the course id to avoid issues with different runs.
items = modulestore().get_items(Location('i4x', course_id_parts[0], course_id_parts[1], 'peergrading', None),
course_id=course.id)
items = modulestore().get_items(peer_grading_query, course_id=course.id)
#See if any of the modules are centralized modules (ie display info from multiple problems)
items = [i for i in items if not getattr(i, "use_for_single_location", True)]
# Loop through all potential peer grading modules, and find the first one that has a path to it.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment