From 81a92e4bfc3cc30b13c32e81e4ae5ab84312fc95 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Thu, 20 Feb 2014 10:45:37 -0500 Subject: [PATCH] Consolidate course_id triple parsing --- .../management/commands/clone_course.py | 5 ++-- cms/djangoapps/contentstore/utils.py | 4 +-- common/djangoapps/student/views.py | 18 ++++++------- .../xmodule/xmodule/contentstore/content.py | 4 +-- common/lib/xmodule/xmodule/course_module.py | 6 +++-- .../xmodule/xmodule/modulestore/__init__.py | 18 +++++++++++++ .../xmodule/xmodule/modulestore/mongo/base.py | 6 +++-- .../xmodule/modulestore/store_utilities.py | 27 ++++++++----------- .../modulestore/tests/test_location.py | 12 +++++++++ .../tests/test_mixed_modulestore.py | 7 +++-- common/lib/xmodule/xmodule/modulestore/xml.py | 5 +++- .../xmodule/modulestore/xml_importer.py | 20 ++++++-------- lms/djangoapps/bulk_email/tasks.py | 3 ++- lms/djangoapps/bulk_email/tests/test_forms.py | 11 ++++---- lms/djangoapps/certificates/queue.py | 16 +++++------ lms/djangoapps/courseware/module_render.py | 8 +++--- lms/djangoapps/dashboard/git_import.py | 12 ++++----- lms/djangoapps/instructor/hint_manager.py | 3 ++- lms/djangoapps/instructor/views/api.py | 6 +++-- .../instructor/views/instructor_dashboard.py | 10 +++---- lms/djangoapps/instructor/views/legacy.py | 17 +++++++----- lms/djangoapps/open_ended_grading/views.py | 5 ++-- 22 files changed, 128 insertions(+), 95 deletions(-) diff --git a/cms/djangoapps/contentstore/management/commands/clone_course.py b/cms/djangoapps/contentstore/management/commands/clone_course.py index 1336b2f0d1..77daeaa975 100644 --- a/cms/djangoapps/contentstore/management/commands/clone_course.py +++ b/cms/djangoapps/contentstore/management/commands/clone_course.py @@ -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)) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 29874583c4..b152880732 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -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): diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 24808d7539..29d4ee06df 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -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: diff --git a/common/lib/xmodule/xmodule/contentstore/content.py b/common/lib/xmodule/xmodule/contentstore/content.py index 8d583124cc..536fe994e2 100644 --- a/common/lib/xmodule/xmodule/contentstore/content.py +++ b/common/lib/xmodule/xmodule/contentstore/content.py @@ -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 diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 66b3c0630f..a4bdaa757d 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -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): diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index 517295af06..8663d6d65b 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -261,6 +261,24 @@ class Location(_LocationBase): return "/".join([self.org, self.course, self.name]) + COURSE_ID_RE = re.compile(""" + (?P[^/]+)/ + (?P[^/]+)/ + (?P.*) + """, 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 diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 1b26b9f46e..773f0fcda9 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -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 diff --git a/common/lib/xmodule/xmodule/modulestore/store_utilities.py b/common/lib/xmodule/xmodule/modulestore/store_utilities.py index bcb51908a7..43ef3000d3 100644 --- a/common/lib/xmodule/xmodule/modulestore/store_utilities.py +++ b/common/lib/xmodule/xmodule/modulestore/store_utilities.py @@ -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/// which will be broken if migrated to a different course_id + # /courses/// 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)) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_location.py b/common/lib/xmodule/xmodule/modulestore/tests/test_location.py index f0c815f41c..c2ab1fb4b7 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_location.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_location.py @@ -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') diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index 253e209bc4..1b7cbb443b 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -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 diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 89cda4c399..8ddd542365 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -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) diff --git a/common/lib/xmodule/xmodule/modulestore/xml_importer.py b/common/lib/xmodule/xmodule/modulestore/xml_importer.py index ead07df520..a8a36a8667 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml_importer.py +++ b/common/lib/xmodule/xmodule/modulestore/xml_importer.py @@ -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 " diff --git a/lms/djangoapps/bulk_email/tasks.py b/lms/djangoapps/bulk_email/tasks.py index c7e89d9142..530b8e12c2 100644 --- a/lms/djangoapps/bulk_email/tasks.py +++ b/lms/djangoapps/bulk_email/tasks.py @@ -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) diff --git a/lms/djangoapps/bulk_email/tests/test_forms.py b/lms/djangoapps/bulk_email/tests/test_forms.py index e463f9e7df..cd823a1a41 100644 --- a/lms/djangoapps/bulk_email/tests/test_forms.py +++ b/lms/djangoapps/bulk_email/tests/test_forms.py @@ -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() diff --git a/lms/djangoapps/certificates/queue.py b/lms/djangoapps/certificates/queue.py index 04d1d0ba48..fe8148b25d 100644 --- a/lms/djangoapps/certificates/queue.py +++ b/lms/djangoapps/certificates/queue.py @@ -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 diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 382131f262..7df05a7138 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -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) ] diff --git a/lms/djangoapps/dashboard/git_import.py b/lms/djangoapps/dashboard/git_import.py index 336cee787e..812dc34796 100644 --- a/lms/djangoapps/dashboard/git_import.py +++ b/lms/djangoapps/dashboard/git_import.py @@ -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, diff --git a/lms/djangoapps/instructor/hint_manager.py b/lms/djangoapps/instructor/hint_manager.py index ab903e1909..c546b668c0 100644 --- a/lms/djangoapps/instructor/hint_manager.py +++ b/lms/djangoapps/instructor/hint_manager.py @@ -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] diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index ac09afaf7e..b30cc34920 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -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 diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index b368153bd6..a6caea63c7 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -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(), diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index 33c1851479..dccf1da79b 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -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: diff --git a/lms/djangoapps/open_ended_grading/views.py b/lms/djangoapps/open_ended_grading/views.py index 02fb287905..50f34f6395 100644 --- a/lms/djangoapps/open_ended_grading/views.py +++ b/lms/djangoapps/open_ended_grading/views.py @@ -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.