diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index 1f1a80e2b4..877c730539 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -11,6 +11,8 @@ from django.http import HttpResponse from django.utils import simplejson from django_comment_client.models import Role from django_comment_client.permissions import check_permissions_by_view +from xmodule.modulestore.exceptions import NoPathToItem + from mitxmako import middleware import pystache_custom as pystache @@ -164,6 +166,7 @@ def initialize_discussion_info(course): # get all discussion models within this course_id all_modules = modulestore().get_items(['i4x', course.location.org, course.location.course, 'discussion', None], course_id=course_id) + path_to_locations = {} for module in all_modules: skip_module = False for key in ('id', 'discussion_category', 'for'): @@ -171,6 +174,14 @@ def initialize_discussion_info(course): log.warning("Required key '%s' not in discussion %s, leaving out of category map" % (key, module.location)) skip_module = True + # cdodge: pre-compute the path_to_location. Note this can throw an exception for any + # dangling discussion modules + try: + path_to_locations[module.location] = path_to_location(modulestore(), course.id, module.location) + except NoPathToItem: + log.warning("Could not compute path_to_location for {0}. Perhaps this is an orphaned discussion module?!? Skipping...".format(module.location)) + skip_module = True + if skip_module: continue @@ -235,6 +246,7 @@ def initialize_discussion_info(course): _DISCUSSIONINFO[course.id]['id_map'] = discussion_id_map _DISCUSSIONINFO[course.id]['category_map'] = category_map _DISCUSSIONINFO[course.id]['timestamp'] = datetime.now() + _DISCUSSIONINFO[course.id]['path_to_location'] = path_to_locations class JsonResponse(HttpResponse): @@ -390,11 +402,23 @@ def get_courseware_context(content, course): if id in id_map: location = id_map[id]["location"].url() title = id_map[id]["title"] - (course_id, chapter, section, position) = path_to_location(modulestore(), course.id, location) - url = reverse('courseware_position', kwargs={"course_id": course_id, - "chapter": chapter, - "section": section, - "position": position}) + + # cdodge: did we pre-compute, if so, then let's use that rather than recomputing + if 'path_to_location' in _DISCUSSIONINFO[course.id] and location in _DISCUSSIONINFO[course.id]['path_to_location']: + (course_id, chapter, section, position) = _DISCUSSIONINFO[course.id]['path_to_location'][location] + else: + try: + (course_id, chapter, section, position) = path_to_location(modulestore(), course.id, location) + except NoPathToItem: + # Object is not in the graph any longer, let's just get path to the base of the course + # so that we can at least return something to the caller + (course_id, chapter, section, position) = path_to_location(modulestore(), course.id, course.location) + + url = reverse('courseware_position', kwargs={"course_id":course_id, + "chapter":chapter, + "section":section, + "position":position}) + content_info = {"courseware_url": url, "courseware_title": title} return content_info