diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index 6c5d875fbb..9385b83c86 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -69,7 +69,7 @@ class CourseDescriptor(SequenceDescriptor): The grading context has two keys: graded_sections - This contains the sections that are graded, as - well as all possible children modules that can effect the + well as all possible children modules that can affect the grading. This allows some sections to be skipped if the student hasn't seen any part of it. @@ -101,10 +101,8 @@ class CourseDescriptor(SequenceDescriptor): for s in c.get_children(): if s.metadata.get('graded', False): - xmoduledescriptors = [] - for module in yield_descriptor_descendents(s): - # TODO: Only include modules that have a score here - xmoduledescriptors.append(module) + # TODO: Only include modules that have a score here + xmoduledescriptors = [s for s in yield_descriptor_descendents(s)] section_description = { 'section_descriptor' : s, 'xmoduledescriptors' : xmoduledescriptors} diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index 75e7fb35f3..aa7caea31b 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -27,7 +27,7 @@ def grade(student, request, course, student_module_cache=None): grading_context = course.grading_context if student_module_cache == None: - student_module_cache = StudentModuleCache(student, descriptors=grading_context['all_descriptors']) + student_module_cache = StudentModuleCache(student, grading_context['all_descriptors']) totaled_scores = {} # This next complicated loop is just to collect the totaled_scores, which is diff --git a/lms/djangoapps/courseware/management/commands/check_course.py b/lms/djangoapps/courseware/management/commands/check_course.py index bf54147513..80f4b57983 100644 --- a/lms/djangoapps/courseware/management/commands/check_course.py +++ b/lms/djangoapps/courseware/management/commands/check_course.py @@ -78,7 +78,7 @@ class Command(BaseCommand): # TODO (cpennington): Get coursename in a legitimate way course_location = 'i4x://edx/6002xs12/course/6.002_Spring_2012' - student_module_cache = StudentModuleCache(sample_user, modulestore().get_item(course_location)) + student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(sample_user, modulestore().get_item(course_location)) course = get_module(sample_user, None, course_location, student_module_cache) to_run = [ diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index 79617e428d..631a750643 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -67,30 +67,19 @@ class StudentModuleCache(object): """ A cache of StudentModules for a specific student """ - def __init__(self, user, descriptor=None, depth=None, descriptors=None, descriptor_filter=lambda descriptor: True): + def __init__(self, user, descriptors): ''' Find any StudentModule objects that are needed by any child modules of the supplied descriptor, or caches only the StudentModule objects specifically for every descriptor in descriptors. Avoids making multiple queries to the database. - There are two ways to init: - descriptor: An XModuleDescriptor - depth is the number of levels of descendent modules to load StudentModules for, in addition to - the supplied descriptor. If depth is None, load all descendent StudentModules - OR - descriptors: An array of XModuleDescriptors. - - descriptor_filter is a function that accepts a descriptor and return wether the StudentModule - should be cached + Arguments + user: The user for which to fetch maching StudentModules + descriptors: An array of XModuleDescriptors. ''' if user.is_authenticated(): - if not (descriptor == None) != (descriptors == None): #An xor on the descriptor and descriptors parameters. - raise ValueError("Either the descriptor or descriptors must be supplied to StudentModuleCache.") - - if descriptor != None: - descriptors = self._get_child_descriptors(descriptor, depth) - module_ids = self._get_module_state_keys(descriptors, descriptor_filter) + module_ids = self._get_module_state_keys(descriptors) # This works around a limitation in sqlite3 on the number of parameters # that can be put into a single query @@ -104,24 +93,38 @@ class StudentModuleCache(object): else: self.cache = [] - - def _get_child_descriptors(self, descriptor, depth): + + + @classmethod + def cache_for_descriptor_descendents(cls, user, descriptor, depth=None, descriptor_filter=lambda descriptor: True): """ descriptor: An XModuleDescriptor depth is the number of levels of descendent modules to load StudentModules for, in addition to the supplied descriptor. If depth is None, load all descendent StudentModules + descriptor_filter is a function that accepts a descriptor and return wether the StudentModule + should be cached """ - descriptors = [descriptor] - - if depth is None or depth > 0: - new_depth = depth - 1 if depth is not None else depth + + def get_child_descriptors(descriptor, depth, descriptor_filter): + if descriptor_filter(descriptor): + descriptors = [descriptor] + else: + descriptors = [] + + if depth is None or depth > 0: + new_depth = depth - 1 if depth is not None else depth - for child in descriptor.get_children(): - descriptors.extend(self._get_child_descriptors(child, new_depth)) + for child in descriptor.get_children(): + descriptors.extend(get_child_descriptors(child, new_depth, descriptor_filter)) - return descriptors + return descriptors + + + descriptors = get_child_descriptors(descriptor, depth, descriptor_filter) + + return StudentModuleCache(user, descriptors) - def _get_module_state_keys(self, descriptors, descriptor_filter): + def _get_module_state_keys(self, descriptors): ''' Get a list of the state_keys needed for StudentModules required for this module descriptor @@ -131,12 +134,11 @@ class StudentModuleCache(object): ''' keys = [] for descriptor in descriptors: - if descriptor_filter(descriptor): - keys.append(descriptor.location.url()) - - shared_state_key = getattr(descriptor, 'shared_state_key', None) - if shared_state_key is not None: - keys.append(shared_state_key) + keys.append(descriptor.location.url()) + + shared_state_key = getattr(descriptor, 'shared_state_key', None) + if shared_state_key is not None: + keys.append(shared_state_key) return keys diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index c84a61e526..118f53887a 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -48,7 +48,7 @@ def toc_for_course(user, request, course, active_chapter, active_section): chapters with name 'hidden' are skipped. ''' - student_module_cache = StudentModuleCache(user, course, depth=2) + student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(user, course, depth=2) course = get_module(user, request, course.location, student_module_cache) chapters = list() @@ -259,7 +259,7 @@ def xqueue_callback(request, userid, id, dispatch): # Retrieve target StudentModule user = User.objects.get(id=userid) - student_module_cache = StudentModuleCache(user, modulestore().get_item(id)) + student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(user, modulestore().get_item(id)) instance = get_module(request.user, request, id, student_module_cache) instance_module = get_instance_module(request.user, instance, student_module_cache) @@ -307,7 +307,7 @@ def modx_dispatch(request, dispatch=None, id=None): ''' # ''' (fix emacs broken parsing) - student_module_cache = StudentModuleCache(request.user, modulestore().get_item(id)) + student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(request.user, modulestore().get_item(id)) instance = get_module(request.user, request, id, student_module_cache) instance_module = get_instance_module(request.user, instance, student_module_cache) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index e60df34b95..4540f809d8 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -110,7 +110,7 @@ def profile(request, course_id, student_id=None): user_info = UserProfile.objects.get(user=student) - student_module_cache = StudentModuleCache(request.user, course) + student_module_cache = StudentModuleCache.cache_for_descriptor_descendents(request.user, course) course_module = get_module(request.user, request, course.location, student_module_cache) courseware_summary = grades.progress_summary(student, course_module, course.grader, student_module_cache) @@ -191,11 +191,12 @@ def index(request, course_id, chapter=None, section=None, if look_for_module: section_descriptor = get_section(course, chapter, section) if section_descriptor is not None: - student_module_cache = StudentModuleCache(request.user, + student_module_cache = StudentModuleCache.cache_for_descriptor_descendents( + request.user, section_descriptor) - module, _, _, _ = get_module(request.user, request, - section_descriptor.location, - student_module_cache) + module = get_module(request.user, request, + section_descriptor.location, + student_module_cache) context['content'] = module.get_html() else: log.warning("Couldn't find a section descriptor for course_id '{0}',"