From 18ee1018e63db73a8d19c3e9b86bd2515bbbd3d6 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Sun, 10 Mar 2013 11:11:17 -0400 Subject: [PATCH 1/2] optimize forum page rendering as we don't pre-compute the link urls to inline discussions. We can use jump_to urls and then figure out the link path if/when end-user clicks on it. This saves a lot of unnecessary round trips to the DB as path computation is expensive, especially when it being done for every discussion module in a course in a loop. --- lms/djangoapps/django_comment_client/utils.py | 28 ++----------------- lms/envs/dev.py | 3 +- 2 files changed, 4 insertions(+), 27 deletions(-) diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index dde219294c..7cc36c491b 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/lms/djangoapps/django_comment_client/utils.py @@ -18,7 +18,6 @@ import pystache_custom as pystache from xmodule.modulestore import Location from xmodule.modulestore.django import modulestore -from xmodule.modulestore.search import path_to_location log = logging.getLogger(__name__) @@ -166,7 +165,6 @@ 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'): @@ -174,14 +172,6 @@ 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 @@ -246,7 +236,6 @@ 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): @@ -403,21 +392,8 @@ def get_courseware_context(content, course): location = id_map[id]["location"].url() title = id_map[id]["title"] - # 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}) + url = reverse('jump_to', kwargs={"course_id":course.location.course_id, + "location": location}) content_info = {"courseware_url": url, "courseware_title": title} return content_info diff --git a/lms/envs/dev.py b/lms/envs/dev.py index 6ecbbb0f85..84cd80859c 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -104,7 +104,8 @@ SUBDOMAIN_BRANDING = { # have an actual course with that org set VIRTUAL_UNIVERSITIES = [] -COMMENTS_SERVICE_KEY = "PUT_YOUR_API_KEY_HERE" +COMMENTS_SERVICE_KEY = "***REMOVED***" +COMMENTS_SERVICE_URL = "https://comments-edge-stage.herokuapp.com" ############################## Course static files ########################## if os.path.isdir(DATA_DIR): From cbcda0fc65a11961e87b3d2ff6df741bad4c9472 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Sun, 10 Mar 2013 11:14:25 -0400 Subject: [PATCH 2/2] oops didn't mean to commit the test API key --- lms/envs/dev.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lms/envs/dev.py b/lms/envs/dev.py index 84cd80859c..6ecbbb0f85 100644 --- a/lms/envs/dev.py +++ b/lms/envs/dev.py @@ -104,8 +104,7 @@ SUBDOMAIN_BRANDING = { # have an actual course with that org set VIRTUAL_UNIVERSITIES = [] -COMMENTS_SERVICE_KEY = "***REMOVED***" -COMMENTS_SERVICE_URL = "https://comments-edge-stage.herokuapp.com" +COMMENTS_SERVICE_KEY = "PUT_YOUR_API_KEY_HERE" ############################## Course static files ########################## if os.path.isdir(DATA_DIR):