diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 746374b3946f..ecda7b4f9dd5 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -47,6 +47,7 @@ from models.settings.course_grading import CourseGradingModel from cms.lib.xblock.runtime import handler_url, local_resource_url from opaque_keys.edx.keys import UsageKey, CourseKey +from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator __all__ = ['orphan_handler', 'xblock_handler', 'xblock_view_handler', 'xblock_outline_handler'] @@ -84,6 +85,21 @@ def usage_key_with_run(usage_key_string): return usage_key +def _should_remove_branch(key): + """ + Determine whether or not we want the modulestore to be stripping branch + information from any keys it returns to us. + """ + branch=None + if isinstance(key, CourseLocator): + branch = key.branch + elif isinstance(key, BlockUsageLocator): + branch = key.course_key.branch + if branch is None or branch == ModuleStoreEnum.BranchName.draft: + return True # This is the default branch, so strip out branch from any locators + return False # This is not the default branch - we must leave branch information in + + # pylint: disable=unused-argument @require_http_methods(("DELETE", "GET", "PUT", "POST", "PATCH")) @login_required @@ -203,7 +219,7 @@ def xblock_view_handler(request, usage_key_string, view_name): if 'application/json' in accept_header: store = modulestore() - xblock = store.get_item(usage_key, remove_branch=False) + xblock = store.get_item(usage_key, remove_branch=_should_remove_branch(usage_key)) container_views = ['container_preview', 'reorderable_container_child_preview'] # wrap the generated fragment in the xmodule_editor div so that the javascript diff --git a/common/lib/xmodule/xmodule/library_content_module.py b/common/lib/xmodule/xmodule/library_content_module.py index b2be1277bee6..43ea13a14761 100644 --- a/common/lib/xmodule/xmodule/library_content_module.py +++ b/common/lib/xmodule/xmodule/library_content_module.py @@ -2,6 +2,7 @@ from collections import namedtuple from copy import copy import hashlib +from .mako_module import MakoModuleDescriptor from opaque_keys.edx.locator import CourseLocator import random from webob import Response @@ -10,8 +11,8 @@ from xblock.fragment import Fragment from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.x_module import XModule, STUDENT_VIEW -from xmodule.seq_module import SequenceDescriptor from xmodule.studio_editable import StudioEditableModule, StudioEditableDescriptor +from .xml_module import XmlDescriptor from pkg_resources import resource_string # Make '_' a no-op so we can scrape strings @@ -102,14 +103,14 @@ class LibraryContentFields(object): ) filters = String(default="") # TBD has_score = Boolean( - display_name=_("Graded"), - help=_("Is this a graded assignment"), + display_name=_("Scored"), + help=_("Set this true if this is meant to be either a graded assignment or a practice problem."), default=False, scope=Scope.settings, ) weight = Integer( display_name=_("Weight"), - help=_("If this is a graded assignment, this determines the total point value available."), + help=_("If this is scored, the total possible score will be scaled to this weight."), default=1, scope=Scope.settings, ) @@ -130,8 +131,20 @@ class LibraryContentModule(LibraryContentFields, XModule, StudioEditableModule): as children of this block, but only a subset of those children are shown to any particular student. """ + def selected_children(self): + """ + Returns a set() of block_ids indicating which of the possible children + have been selected to display to the current user. - def student_view(self, context): + This reads and updates the "selected" field, which has user_state scope. + + Note: self.selected and the return value contain block_ids. To get + actual BlockUsageLocators, it is necessary to use self.children, + because the block_ids alone do not specify the block type. + """ + if hasattr(self, "_selected_set"): + # Already done: + return self._selected_set # Determine which of our children we will show: selected = set(self.selected) if self.selected else set() # set of block_ids valid_block_ids = set([c.block_id for c in self.children]) @@ -159,15 +172,26 @@ def student_view(self, context): raise NotImplementedError("Unsupported mode.") # Save our selections to the user state, to ensure consistency: self.selected = list(selected) + # Cache the results + self._selected_set = selected + return selected + def _get_selected_child_blocks(self): + """ + Generator returning XBlock instances of the children selected for the + current user. + """ + selected = self.selected_children() + for child_loc in self.children: + if child_loc.block_id in selected: + yield self.runtime.get_block(child_loc) + + def student_view(self, context): fragment = Fragment() contents = [] child_context = {} if not context else copy(context) - for child_loc in self.children: - if child_loc.block_id not in selected: - continue - child = self.runtime.get_block(child_loc) + for child in self._get_selected_child_blocks(): for displayable in child.displayable_items(): rendered_child = displayable.render(STUDENT_VIEW, child_context) fragment.add_frag_resources(rendered_child) @@ -267,13 +291,61 @@ def _get_library(self, library_key): # Note library version is also possibly available at library.runtime.course_entry.course_key.version return library + def get_child_descriptors(self): + """ + We override normal handling of children. + In this case, our goal is to prevent the LMS from expecting any grades + from child XBlocks. So we tell the system that we have no children. + This way, we can report a single consolidated grade. + + Without this, the progress tab in the LMS would show an expected grade + from each possible child, rather than the n chosen child[ren]. + """ + return [] + + def get_score(self): + """ + Return the current user's total score and max possible score. + """ + if not self.has_score: + return None + total = 0 + correct = 0 + for child in self._get_selected_child_blocks(): + info = child.get_score() + if info: + total += info["total"] + correct += info["score"] + correct = correct * self.weight / total + total = self.weight + return {"score": correct, "total": total} + + def max_score(self): + """ + Return the current user's max possible score. + """ + return self.weight + # If we were able to intercept 'grade' events from our children, we could + # then tell the LMS runtime to update the grade and max_grade in this XBlock's + # StudentModule (currently it is always NULL). In that case, we could return + # the unweighted max score here, and the LMS runtime would do weighting for us. + # However, for now we have set always_recalculate_grades=True, so this XBlock's + # StudentModule grade/max_grade is ignored, and we must also do our own weighting. + # + #return sum(child.max_score() for child in self._get_selected_child_blocks()) + @XBlock.wants('user') -class LibraryContentDescriptor(LibraryContentFields, SequenceDescriptor, StudioEditableDescriptor): +class LibraryContentDescriptor(LibraryContentFields, MakoModuleDescriptor, XmlDescriptor, StudioEditableDescriptor): """ Descriptor class for LibraryContentModule XBlock. """ + mako_template = 'widgets/metadata-edit.html' module_class = LibraryContentModule + always_recalculate_grades = True # when children publish 'grade' events, the LMS runtime updates the + # grade/max_grade of the child's StudentModule, but this block's StudentModule + # isn't updated. This forces the grading system to ignore the cached grades in + # this block's StudentModule and instead call get_score() and max_score(). @XBlock.handler def refresh_children(self, request, _): @@ -332,6 +404,7 @@ def refresh_children(self, request, _): def has_dynamic_children(self): """ Inform the runtime that our children vary per-user. + See get_child_descriptors() above """ return True diff --git a/lms/djangoapps/courseware/grades.py b/lms/djangoapps/courseware/grades.py index 72bac150f700..8ef1bcc872de 100644 --- a/lms/djangoapps/courseware/grades.py +++ b/lms/djangoapps/courseware/grades.py @@ -225,7 +225,7 @@ def create_module(descriptor): # TODO: We need the request to pass into here. If we could forego that, our arguments # would be simpler with manual_transaction(): - field_data_cache = FieldDataCache([descriptor], course.id, student) + field_data_cache = FieldDataCache.cache_for_descriptor_descendents(course.id, student, descriptor, depth=None) return get_module_for_descriptor(student, request, descriptor, field_data_cache, course.id) for module_descriptor in yield_dynamic_descriptor_descendents(section_descriptor, create_module): diff --git a/lms/djangoapps/courseware/models.py b/lms/djangoapps/courseware/models.py index cb471dd79c75..dfb8207f4bbb 100644 --- a/lms/djangoapps/courseware/models.py +++ b/lms/djangoapps/courseware/models.py @@ -32,6 +32,10 @@ class StudentModule(models.Model): MODULE_TYPES = (('problem', 'problem'), ('video', 'video'), ('html', 'html'), + ('course', 'course'), + ('chapter', 'Section'), + ('sequential', 'Subsection'), + ('library_content', 'Library Content'), ) ## These three are the key for the object module_type = models.CharField(max_length=32, choices=MODULE_TYPES, default='problem', db_index=True)