From b403f342e74a5ea2a5857e672d25988df592c13c Mon Sep 17 00:00:00 2001 From: "Kyle D. McCormick" Date: Thu, 26 Sep 2024 15:51:19 -0400 Subject: [PATCH] feat: ItemBankBlock --- mypy.ini | 3 +- setup.py | 1 + xmodule/item_bank_block.py | 461 ++++++++++++++++++++++++++ xmodule/library_content_block.py | 431 +++--------------------- xmodule/library_tools.py | 38 --- xmodule/tests/test_library_content.py | 4 + 6 files changed, 511 insertions(+), 427 deletions(-) create mode 100644 xmodule/item_bank_block.py diff --git a/mypy.ini b/mypy.ini index 5027c3bb5595..d1ffdb927605 100644 --- a/mypy.ini +++ b/mypy.ini @@ -12,7 +12,8 @@ files = openedx/core/djangoapps/xblock, openedx/core/types, openedx/core/djangoapps/content_tagging, - xmodule/util/keys.py + xmodule/util/keys.py, + xmodule/item_bank_block.py [mypy.plugins.django-stubs] # content_staging only works with CMS; others work with either, so we run mypy with CMS settings. diff --git a/setup.py b/setup.py index 21c8e537c97e..3b8f8c59498d 100644 --- a/setup.py +++ b/setup.py @@ -19,6 +19,7 @@ "error = xmodule.error_block:ErrorBlock", "hidden = xmodule.hidden_block:HiddenBlock", "html = xmodule.html_block:HtmlBlock", + "itembank = xmodule.item_bank_block:ItemBankBlock", "image = xmodule.template_block:TranslateCustomTagBlock", "library = xmodule.library_root_xblock:LibraryRoot", "library_content = xmodule.library_content_block:LegacyLibraryContentBlock", diff --git a/xmodule/item_bank_block.py b/xmodule/item_bank_block.py new file mode 100644 index 000000000000..ac4411b60bde --- /dev/null +++ b/xmodule/item_bank_block.py @@ -0,0 +1,461 @@ +""" +@@TODO +""" +from __future__ import annotations + +import json +import logging +import random +from copy import copy +from gettext import ngettext, gettext +from django.conf import settings +from django.utils.functional import classproperty +from lxml import etree +from lxml.etree import XMLSyntaxError +from rest_framework import status +from web_fragments.fragment import Fragment +from webob import Response +from xblock.completable import XBlockCompletionMode +from xblock.core import XBlock +from xblock.fields import Boolean, Integer, List, Scope, String + +from xmodule.validation import StudioValidation, StudioValidationMessage +from xmodule.x_module import STUDENT_VIEW + + +_ = lambda text: text + +logger = logging.getLogger(__name__) + + +@XBlock.needs('mako') +@XBlock.wants('user') +class ItemBankBlock(XBlock): # @@TODO: Does this need StudioEditableBlock and/or MakoTemplateBase? + """ + @@TODO + """ + # pylint: disable=abstract-method + has_children = True + has_author_view = True + + resources_dir = 'assets/library_content' + + mako_template = 'widgets/metadata-edit.html' + studio_js_module_name = "VerticalDescriptor" + + show_in_read_only_mode = True + + @classproperty + def completion_mode(cls): # pylint: disable=no-self-argument + """ + Allow overriding the completion mode with a feature flag. + + This is a property, so it can be dynamically overridden in tests, as it is not evaluated at runtime. + """ + if settings.FEATURES.get('MARK_LIBRARY_CONTENT_BLOCK_COMPLETE_ON_VIEW', False): + return XBlockCompletionMode.COMPLETABLE + + return XBlockCompletionMode.AGGREGATOR + + display_name = String( + display_name=_("Display Name"), + help=_("The display name for this component."), + default="Randomized Content Block", + scope=Scope.settings, + ) + max_count = Integer( + display_name=_("Count"), + help=_("Enter the number of components to display to each student. Set it to -1 to display all components."), + default=1, + scope=Scope.settings, + ) + selected = List( + # This is a list of (block_type, block_id) tuples used to record + # which random/first set of matching blocks was selected per user + default=[], + scope=Scope.user_state, + ) + # This cannot be called `show_reset_button`, because children blocks inherit this as a default value. + allow_resetting_children = Boolean( + display_name=_("Show Reset Button"), + help=_("Determines whether a 'Reset Problems' button is shown, so users may reset their answers and reshuffle " + "selected items."), + scope=Scope.settings, + default=False + ) + + @classmethod + def make_selection(cls, selected, children, max_count): + """ + Dynamically selects block_ids indicating which of the possible children are displayed to the current user. + + Arguments: + selected - list of (block_type, block_id) tuples assigned to this student + children - children of this block + max_count - number of components to display to each student + mode - how content is drawn from the library + + Returns: + A dict containing the following keys: + + 'selected' (set) of (block_type, block_id) tuples assigned to this student + 'invalid' (set) of dropped (block_type, block_id) tuples that are no longer valid + 'overlimit' (set) of dropped (block_type, block_id) tuples that were previously selected + 'added' (set) of newly added (block_type, block_id) tuples + """ + rand = random.Random() + + selected_keys = {tuple(k) for k in selected} # set of (block_type, block_id) tuples assigned to this student + + # Determine which of our children we will show: + valid_block_keys = {(c.block_type, c.block_id) for c in children} + + # Remove any selected blocks that are no longer valid: + invalid_block_keys = (selected_keys - valid_block_keys) + if invalid_block_keys: + selected_keys -= invalid_block_keys + + # If max_count has been decreased, we may have to drop some previously selected blocks: + overlimit_block_keys = set() + if len(selected_keys) > max_count: + num_to_remove = len(selected_keys) - max_count + overlimit_block_keys = set(rand.sample(list(selected_keys), num_to_remove)) + selected_keys -= overlimit_block_keys + + # Do we have enough blocks now? + num_to_add = max_count - len(selected_keys) + + added_block_keys = None + if num_to_add > 0: + # We need to select [more] blocks to display to this user: + pool = valid_block_keys - selected_keys + num_to_add = min(len(pool), num_to_add) + added_block_keys = set(rand.sample(list(pool), num_to_add)) + selected_keys |= added_block_keys + + if any((invalid_block_keys, overlimit_block_keys, added_block_keys)): + selected = list(selected_keys) + random.shuffle(selected) + + return { + 'selected': selected, + 'invalid': invalid_block_keys, + 'overlimit': overlimit_block_keys, + 'added': added_block_keys, + } + + def _publish_event(self, event_name, result, **kwargs): + """ + Helper method to publish an event for analytics purposes + """ + event_data = { + "location": str(self.location), + "result": result, + "previous_count": getattr(self, "_last_event_result_count", len(self.selected)), + "max_count": self.max_count, + } + event_data.update(kwargs) + self.runtime.publish(self, f"edx.librarycontentblock.content.{event_name}", event_data) + self._last_event_result_count = len(result) # pylint: disable=attribute-defined-outside-init + + @classmethod + def publish_selected_children_events(cls, block_keys, format_block_keys, publish_event): + """ + Helper method for publishing events when children blocks are + selected/updated for a user. This helper is also used by + the ContentLibraryTransformer. + + Arguments: + + block_keys - + A dict describing which events to publish (add or + remove), see `make_selection` above for format details. + + format_block_keys - + A function to convert block keys to the format expected + by publish_event. Must have the signature: + + [(block_type, block_id)] -> T + + Where T is a collection of block keys as accepted by + `publish_event`. + + publish_event - + Function that handles the actual publishing. Must have + the signature: + + <'removed'|'assigned'> -> result:T -> removed:T -> reason:str -> None + + Where T is a collection of block_keys as returned by + `format_block_keys`. + """ + if block_keys['invalid']: + # reason "invalid" means deleted from library or a different library is now being used. + publish_event( + "removed", + result=format_block_keys(block_keys['selected']), + removed=format_block_keys(block_keys['invalid']), + reason="invalid" + ) + + if block_keys['overlimit']: + publish_event( + "removed", + result=format_block_keys(block_keys['selected']), + removed=format_block_keys(block_keys['overlimit']), + reason="overlimit" + ) + + if block_keys['added']: + publish_event( + "assigned", + result=format_block_keys(block_keys['selected']), + added=format_block_keys(block_keys['added']) + ) + + def selected_children(self): + """ + Returns a [] of block_ids indicating which of the possible children + have been selected to display to the current user. + + This reads and updates the "selected" field, which has user_state scope. + + Note: the return value (self.selected) contains block_ids. To get + actual BlockUsageLocators, it is necessary to use self.children, + because the block_ids alone do not specify the block type. + """ + max_count = self.max_count + if max_count < 0: + max_count = len(self.children) + + block_keys = self.make_selection(self.selected, self.children, max_count) # pylint: disable=no-member + + self.publish_selected_children_events( + block_keys, + self.format_block_keys_for_analytics, + self._publish_event, + ) + + if any(block_keys[changed] for changed in ('invalid', 'overlimit', 'added')): + # Save our selections to the user state, to ensure consistency: + selected = block_keys['selected'] + self.selected = selected # TODO: this doesn't save from the LMS "Progress" page. + + return self.selected + + def format_block_keys_for_analytics(self, block_keys: list[tuple[str, str]]) -> dict: + """ + @@TODO + """ + return [{"usage_key": str(self.context_key.make_usage_key(*block_key))} for block_key in block_keys] + + @XBlock.handler + def reset_selected_children(self, _, __): + """ + Resets the XBlock's state for a user. + + This resets the state of all `selected` children and then clears the `selected` field + so that the new blocks are randomly chosen for this user. + """ + if not self.allow_resetting_children: + return Response('"Resetting selected children" is not allowed for this XBlock', + status=status.HTTP_400_BAD_REQUEST) + + for block_type, block_id in self.selected_children(): + block = self.runtime.get_block(self.location.course_key.make_usage_key(block_type, block_id)) + if hasattr(block, 'reset_problem'): + block.reset_problem(None) + block.save() + + self.selected = [] + return Response(json.dumps(self.student_view({}).content)) + + def _get_selected_child_blocks(self): + """ + Generator returning XBlock instances of the children selected for the + current user. + """ + for block_type, block_id in self.selected_children(): + yield self.runtime.get_block(self.location.course_key.make_usage_key(block_type, block_id)) + + def student_view(self, context): # lint-amnesty, pylint: disable=missing-function-docstring + fragment = Fragment() + contents = [] + child_context = {} if not context else copy(context) + + for child in self._get_selected_child_blocks(): + if child is None: + # TODO: Fix the underlying issue in TNL-7424 + # This shouldn't be happening, but does for an as-of-now + # unknown reason. Until we address the underlying issue, + # let's at least log the error explicitly, ignore the + # exception, and prevent the page from resulting in a + # 500-response. + logger.error('Skipping display for child block that is None') + continue + + rendered_child = child.render(STUDENT_VIEW, child_context) + fragment.add_fragment_resources(rendered_child) + contents.append({ + 'id': str(child.location), + 'content': rendered_child.content, + }) + + fragment.add_content(self.runtime.service(self, 'mako').render_lms_template('vert_module.html', { + 'items': contents, + 'xblock_context': context, + 'show_bookmark_button': False, + 'watched_completable_blocks': set(), + 'completion_delay_ms': None, + 'reset_button': self.allow_resetting_children, + })) + + fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/library_content_reset.js')) + fragment.initialize_js('LibraryContentReset') + return fragment + + def author_view(self, context): + """ + Renders the Studio views. + Normal studio view: If block is properly configured, displays library status summary + Studio container view: displays a preview of all possible children. + """ + fragment = Fragment() + root_xblock = context.get('root_xblock') + is_root = root_xblock and root_xblock.location == self.location + # User has clicked the "View" link. Show a preview of all possible children: + if self.children: # pylint: disable=no-member + max_count = self.max_count + if max_count < 0: + max_count = len(self.children) + context['can_edit_visibility'] = False + context['can_move'] = False + context['can_collapse'] = True + self.render_children(context, fragment, can_reorder=False, can_add=False) + context['is_loading'] = False + + fragment.initialize_js('LibraryContentAuthorView') + return fragment + + def get_child_blocks(self): + """ + Return only the subset of our children relevant to the current student. + """ + return list(self._get_selected_child_blocks()) + + def get_user_id(self): + """ + Get the ID of the current user. + """ + user_service = self.runtime.service(self, 'user') + if user_service: + user_id = user_service.get_current_user().opt_attrs.get('edx-platform.user_id', None) + else: + user_id = None + return user_id + + def _set_validation_error_if_empty(self, validation, summary): + """ Helper method to only set validation summary if it's empty """ + if validation.empty: + validation.set_summary(summary) + + def validate(self): + """ + Validates the state of this Pool Block Instance. This + is the override of the general XBlock method, and it will also ask + its superclass to validate. + """ + validation = super().validate() + if not isinstance(validation, StudioValidation): + validation = StudioValidation.copy(validation) + matching_children_count = len(self.children) # pylint: disable=no-member + if matching_children_count == 0: + self._set_validation_error_if_empty( + validation, + StudioValidationMessage( + StudioValidationMessage.WARNING, + (gettext('There are no problems in the specified library of type {capa_type}.')) + .format(capa_type=self.capa_type), + action_class='edit-button', + action_label=_("Select another problem type.") + ) + ) + + if matching_children_count < self.max_count: + self._set_validation_error_if_empty( + validation, + StudioValidationMessage( + StudioValidationMessage.WARNING, + ( + ngettext( + 'The specified library is configured to fetch {count} problem, ', + 'The specified library is configured to fetch {count} problems, ', + self.max_count + ) + + ngettext( + 'but there is only {actual} matching problem.', + 'but there are only {actual} matching problems.', + matching_children_count + ) + ).format(count=self.max_count, actual=matching_children_count), + action_class='edit-button', + action_label=_("Edit the library configuration.") + ) + ) + + return validation + + def has_dynamic_children(self): + """ + Inform the runtime that our children vary per-user. + See get_child_blocks() above + """ + return True + + def get_content_titles(self): + """ + Returns list of friendly titles for our selected children only; without + thi, all possible children's titles would be seen in the sequence bar in + the LMS. + + This overwrites the get_content_titles method included in x_module by default. + """ + titles = [] + for child in self.get_child_blocks(): + titles.extend(child.get_content_titles()) + return titles + + @classmethod + def definition_from_xml(cls, xml_object, system): + """ + @@TODO + """ + children = [] + + for child in xml_object.getchildren(): + try: + children.append(system.process_xml(etree.tostring(child)).scope_ids.usage_id) + except (XMLSyntaxError, AttributeError): + msg = ( + "Unable to load child when parsing {self.usage_key.block_type} Block. " + "This can happen when a comment is manually added to the course export." + ) + logger.error(msg) + if system.error_tracker is not None: + system.error_tracker(msg) + + definition = dict(xml_object.attrib.items()) + return definition, children + + def definition_to_xml(self, resource_fs): + """ Exports PoolBlock to XML """ + xml_object = etree.Element(self.usage_key.block_type) + for child in self.get_children(): + self.runtime.add_block_as_child_node(child, xml_object) + # Set node attributes based on our fields. + for field_name, field in self.fields.items(): # pylint: disable=no-member + if field_name in ('children', 'parent', 'content'): + continue + if field.is_set_on(self): + xml_object.set(field_name, str(field.read_from(self))) + return xml_object diff --git a/xmodule/library_content_block.py b/xmodule/library_content_block.py index adb07101d04c..2a35c624ac05 100644 --- a/xmodule/library_content_block.py +++ b/xmodule/library_content_block.py @@ -12,40 +12,31 @@ import json import logging -import random -from copy import copy -from gettext import ngettext, gettext import nh3 -from django.conf import settings from django.core.exceptions import ObjectDoesNotExist, PermissionDenied -from django.utils.functional import classproperty -from lxml import etree -from lxml.etree import XMLSyntaxError from opaque_keys.edx.locator import LibraryLocator -from rest_framework import status from web_fragments.fragment import Fragment from webob import Response -from xblock.completable import XBlockCompletionMode from xblock.core import XBlock -from xblock.fields import Boolean, Integer, List, Scope, String +from xblock.fields import Integer, Scope, String from xmodule.capa.responsetypes import registry from xmodule.mako_block import MakoTemplateBlockBase +from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.studio_editable import StudioEditableBlock +from xmodule.item_bank_block import ItemBankBlock from xmodule.util.builtin_assets import add_webpack_js_to_fragment from xmodule.validation import StudioValidation, StudioValidationMessage from xmodule.xml_block import XmlMixin from xmodule.x_module import ( - STUDENT_VIEW, ResourceTemplates, XModuleMixin, XModuleToXBlockMixin, shim_xmodule_js, ) -# Make '_' a no-op so we can scrape strings. Using lambda instead of -# `django.utils.translation.ugettext_noop` because Django cannot be imported in this file + _ = lambda text: text logger = logging.getLogger(__name__) @@ -80,11 +71,10 @@ def __init__(self): super().__init__("Needed 'library_tools' features which were not available in the current runtime") -@XBlock.wants('library_tools') # TODO: Split this service into its LMS and CMS parts. +@XBlock.wants('library_tools') @XBlock.wants('studio_user_permissions') # Only available in CMS. -@XBlock.wants('user') -@XBlock.needs('mako') class LegacyLibraryContentBlock( + ItemBankBlock, MakoTemplateBlockBase, XmlMixin, XModuleToXBlockMixin, @@ -100,36 +90,12 @@ class LegacyLibraryContentBlock( as children of this block, but only a subset of those children are shown to any particular student. """ - # pylint: disable=abstract-method - has_children = True - has_author_view = True - resources_dir = 'assets/library_content' mako_template = 'widgets/metadata-edit.html' - studio_js_module_name = "VerticalDescriptor" show_in_read_only_mode = True - # noinspection PyMethodParameters - @classproperty - def completion_mode(cls): # pylint: disable=no-self-argument - """ - Allow overriding the completion mode with a feature flag. - - This is a property, so it can be dynamically overridden in tests, as it is not evaluated at runtime. - """ - if settings.FEATURES.get('MARK_LIBRARY_CONTENT_BLOCK_COMPLETE_ON_VIEW', False): - return XBlockCompletionMode.COMPLETABLE - - return XBlockCompletionMode.AGGREGATOR - - display_name = String( - display_name=_("Display Name"), - help=_("The display name for this component."), - default="Randomized Content Block", - scope=Scope.settings, - ) source_library_id = String( display_name=_("Library"), help=_("Select the library from which you want to draw content."), @@ -154,257 +120,14 @@ def completion_mode(cls): # pylint: disable=no-self-argument values=_get_capa_types(), scope=Scope.settings, ) - selected = List( - # This is a list of (block_type, block_id) tuples used to record - # which random/first set of matching blocks was selected per user - default=[], - scope=Scope.user_state, - ) - # This cannot be called `show_reset_button`, because children blocks inherit this as a default value. - allow_resetting_children = Boolean( - display_name=_("Show Reset Button"), - help=_("Determines whether a 'Reset Problems' button is shown, so users may reset their answers and reshuffle " - "selected items."), - scope=Scope.settings, - default=False - ) @property def source_library_key(self): """ Convenience method to get the library ID as a LibraryLocator and not just a string. - - Supports only v1 libraries. """ return LibraryLocator.from_string(self.source_library_id) - @classmethod - def make_selection(cls, selected, children, max_count): - """ - Dynamically selects block_ids indicating which of the possible children are displayed to the current user. - - Arguments: - selected - list of (block_type, block_id) tuples assigned to this student - children - children of this block - max_count - number of components to display to each student - - Returns: - A dict containing the following keys: - - 'selected' (set) of (block_type, block_id) tuples assigned to this student - 'invalid' (set) of dropped (block_type, block_id) tuples that are no longer valid - 'overlimit' (set) of dropped (block_type, block_id) tuples that were previously selected - 'added' (set) of newly added (block_type, block_id) tuples - """ - rand = random.Random() - - selected_keys = {tuple(k) for k in selected} # set of (block_type, block_id) tuples assigned to this student - - # Determine which of our children we will show: - valid_block_keys = {(c.block_type, c.block_id) for c in children} - - # Remove any selected blocks that are no longer valid: - invalid_block_keys = (selected_keys - valid_block_keys) - if invalid_block_keys: - selected_keys -= invalid_block_keys - - # If max_count has been decreased, we may have to drop some previously selected blocks: - overlimit_block_keys = set() - if len(selected_keys) > max_count: - num_to_remove = len(selected_keys) - max_count - overlimit_block_keys = set(rand.sample(list(selected_keys), num_to_remove)) - selected_keys -= overlimit_block_keys - - # Do we have enough blocks now? - num_to_add = max_count - len(selected_keys) - - added_block_keys = None - if num_to_add > 0: - # We need to select [more] blocks to display to this user: - pool = valid_block_keys - selected_keys - num_to_add = min(len(pool), num_to_add) - added_block_keys = set(rand.sample(list(pool), num_to_add)) - # We now have the correct n random children to show for this user. - selected_keys |= added_block_keys - - if any((invalid_block_keys, overlimit_block_keys, added_block_keys)): - selected = list(selected_keys) - random.shuffle(selected) - - return { - 'selected': selected, - 'invalid': invalid_block_keys, - 'overlimit': overlimit_block_keys, - 'added': added_block_keys, - } - - def _publish_event(self, event_name, result, **kwargs): - """ - Helper method to publish an event for analytics purposes - """ - event_data = { - "location": str(self.location), - "result": result, - "previous_count": getattr(self, "_last_event_result_count", len(self.selected)), - "max_count": self.max_count, - } - event_data.update(kwargs) - self.runtime.publish(self, f"edx.librarycontentblock.content.{event_name}", event_data) - self._last_event_result_count = len(result) # pylint: disable=attribute-defined-outside-init - - @classmethod - def publish_selected_children_events(cls, block_keys, format_block_keys, publish_event): - """ - Helper method for publishing events when children blocks are - selected/updated for a user. This helper is also used by - the ContentLibraryTransformer. - - Arguments: - - block_keys - - A dict describing which events to publish (add or - remove), see `make_selection` above for format details. - - format_block_keys - - A function to convert block keys to the format expected - by publish_event. Must have the signature: - - [(block_type, block_id)] -> T - - Where T is a collection of block keys as accepted by - `publish_event`. - - publish_event - - Function that handles the actual publishing. Must have - the signature: - - <'removed'|'assigned'> -> result:T -> removed:T -> reason:str -> None - - Where T is a collection of block_keys as returned by - `format_block_keys`. - """ - if block_keys['invalid']: - # reason "invalid" means deleted from library or a different library is now being used. - publish_event( - "removed", - result=format_block_keys(block_keys['selected']), - removed=format_block_keys(block_keys['invalid']), - reason="invalid" - ) - - if block_keys['overlimit']: - publish_event( - "removed", - result=format_block_keys(block_keys['selected']), - removed=format_block_keys(block_keys['overlimit']), - reason="overlimit" - ) - - if block_keys['added']: - publish_event( - "assigned", - result=format_block_keys(block_keys['selected']), - added=format_block_keys(block_keys['added']) - ) - - def selected_children(self): - """ - Returns a [] of block_ids indicating which of the possible children - have been selected to display to the current user. - - This reads and updates the "selected" field, which has user_state scope. - - Note: the return value (self.selected) contains block_ids. To get - actual BlockUsageLocators, it is necessary to use self.children, - because the block_ids alone do not specify the block type. - """ - max_count = self.max_count - if max_count < 0: - max_count = len(self.children) - - block_keys = self.make_selection(self.selected, self.children, max_count) # pylint: disable=no-member - - # Publish events for analytics purposes: - lib_tools = self.get_tools() - format_block_keys = lambda keys: lib_tools.create_block_analytics_summary(self.location.course_key, keys) - self.publish_selected_children_events( - block_keys, - format_block_keys, - self._publish_event, - ) - - if any(block_keys[changed] for changed in ('invalid', 'overlimit', 'added')): - # Save our selections to the user state, to ensure consistency: - selected = block_keys['selected'] - self.selected = selected # TODO: this doesn't save from the LMS "Progress" page. - - return self.selected - - @XBlock.handler - def reset_selected_children(self, _, __): - """ - Resets the XBlock's state for a user. - - This resets the state of all `selected` children and then clears the `selected` field - so that the new blocks are randomly chosen for this user. - """ - if not self.allow_resetting_children: - return Response('"Resetting selected children" is not allowed for this XBlock', - status=status.HTTP_400_BAD_REQUEST) - - for block_type, block_id in self.selected_children(): - block = self.runtime.get_block(self.location.course_key.make_usage_key(block_type, block_id)) - if hasattr(block, 'reset_problem'): - block.reset_problem(None) - block.save() - - self.selected = [] - return Response(json.dumps(self.student_view({}).content)) - - def _get_selected_child_blocks(self): - """ - Generator returning XBlock instances of the children selected for the - current user. - """ - for block_type, block_id in self.selected_children(): - yield self.runtime.get_block(self.location.course_key.make_usage_key(block_type, block_id)) - - def student_view(self, context): # lint-amnesty, pylint: disable=missing-function-docstring - fragment = Fragment() - contents = [] - child_context = {} if not context else copy(context) - - for child in self._get_selected_child_blocks(): - if child is None: - # TODO: Fix the underlying issue in TNL-7424 - # This shouldn't be happening, but does for an as-of-now - # unknown reason. Until we address the underlying issue, - # let's at least log the error explicitly, ignore the - # exception, and prevent the page from resulting in a - # 500-response. - logger.error('Skipping display for child block that is None') - continue - - rendered_child = child.render(STUDENT_VIEW, child_context) - fragment.add_fragment_resources(rendered_child) - contents.append({ - 'id': str(child.location), - 'content': rendered_child.content, - }) - - fragment.add_content(self.runtime.service(self, 'mako').render_lms_template('vert_module.html', { - 'items': contents, - 'xblock_context': context, - 'show_bookmark_button': False, - 'watched_completable_blocks': set(), - 'completion_delay_ms': None, - 'reset_button': self.allow_resetting_children, - })) - - fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/library_content_reset.js')) - fragment.initialize_js('LibraryContentReset') - return fragment - def author_view(self, context): """ Renders the Studio views. @@ -459,12 +182,6 @@ def studio_view(self, _context): shim_xmodule_js(fragment, self.studio_js_module_name) return fragment - def get_child_blocks(self): - """ - Return only the subset of our children relevant to the current student. - """ - return list(self._get_selected_child_blocks()) - @property def non_editable_metadata_fields(self): non_editable_fields = super().non_editable_metadata_fields @@ -482,17 +199,6 @@ def get_tools(self, to_read_library_content: bool = False) -> 'LegacyLibraryTool return tools raise LibraryToolsUnavailable() - def get_user_id(self): - """ - Get the ID of the current user. - """ - user_service = self.runtime.service(self, 'user') - if user_service: - user_id = user_service.get_current_user().opt_attrs.get('edx-platform.user_id', None) - else: - user_id = None - return user_id - def _validate_sync_permissions(self): """ Raises PermissionDenied() if we can't confirm that user has write on this block and read on source library. @@ -624,6 +330,8 @@ def validate(self): is the override of the general XBlock method, and it will also ask its superclass to validate. """ + # Note: we assume children have been synced + # since the last time fields like source_library_id or capa_types were changed. validation = super().validate() if not isinstance(validation, StudioValidation): validation = StudioValidation.copy(validation) @@ -651,44 +359,6 @@ def validate(self): ) return validation self._validate_library_version(validation, lib_tools, self.source_library_version, self.source_library_key) - - # Note: we assume children have been synced - # since the last time fields like source_library_id or capa_types were changed. - matching_children_count = len(self.children) # pylint: disable=no-member - if matching_children_count == 0: - self._set_validation_error_if_empty( - validation, - StudioValidationMessage( - StudioValidationMessage.WARNING, - (gettext('There are no problems in the specified library of type {capa_type}.')) - .format(capa_type=self.capa_type), - action_class='edit-button', - action_label=_("Select another problem type.") - ) - ) - - if matching_children_count < self.max_count: - self._set_validation_error_if_empty( - validation, - StudioValidationMessage( - StudioValidationMessage.WARNING, - ( - ngettext( - 'The specified library is configured to fetch {count} problem, ', - 'The specified library is configured to fetch {count} problems, ', - self.max_count - ) + - ngettext( - 'but there is only {actual} matching problem.', - 'but there are only {actual} matching problems.', - matching_children_count - ) - ).format(count=self.max_count, actual=matching_children_count), - action_class='edit-button', - action_label=_("Edit the library configuration.") - ) - ) - return validation def source_library_values(self): @@ -711,8 +381,6 @@ def source_library_values(self): def post_editor_saved(self, user, old_metadata, old_content): # pylint: disable=unused-argument """ If source library or capa_type have been edited, upgrade library & sync automatically. - - TODO: capa_type doesn't really need to trigger an upgrade once we've migrated to V2. """ source_lib_changed = (self.source_library_id != old_metadata.get("source_library_id", "")) capa_filter_changed = (self.capa_type != old_metadata.get("capa_type", ANY_CAPA_TYPE_VALUE)) @@ -723,57 +391,44 @@ def post_editor_saved(self, user, old_metadata, old_content): # pylint: disable # The validation area will display an error message, no need to do anything now. pass - def has_dynamic_children(self): - """ - Inform the runtime that our children vary per-user. - See get_child_blocks() above - """ - return True - - def get_content_titles(self): + def format_block_keys_for_analytics(self, block_keys: list[tuple[str, str]]) -> dict: """ - Returns list of friendly titles for our selected children only; without - thi, all possible children's titles would be seen in the sequence bar in - the LMS. + Given a list of (block_type, block_id) pairs, + prepare the JSON-ready metadata needed for analytics logging. - This overwrites the get_content_titles method included in x_module by default. - """ - titles = [] - for child in self.get_child_blocks(): - titles.extend(child.get_content_titles()) - return titles - - @classmethod - def definition_from_xml(cls, xml_object, system): - children = [] - - for child in xml_object.getchildren(): + This is [ + {"usage_key": x, "original_usage_key": y, "original_usage_version": z, "descendants": [...]} + ] + where the main list contains all top-level blocks, and descendants contains a *flat* list of all + descendants of the top level blocks, if any. + """ + def summarize_block(usage_key): + """ Basic information about the given block """ + orig_key, orig_version = self.runtime.modulestore.get_block_original_usage(usage_key) + return { + "usage_key": str(usage_key), + "original_usage_key": str(orig_key) if orig_key else None, + "original_usage_version": str(orig_version) if orig_version else None, + } + + result_json = [] + for block_key in block_keys: + key = self.context_key.make_usage_key(*block_key) + info = summarize_block(key) + info['descendants'] = [] try: - children.append(system.process_xml(etree.tostring(child)).scope_ids.usage_id) - except (XMLSyntaxError, AttributeError): - msg = ( - "Unable to load child when parsing Library Content Block. " - "This can happen when a comment is manually added to the course export." - ) - logger.error(msg) - if system.error_tracker is not None: - system.error_tracker(msg) - - definition = dict(xml_object.attrib.items()) - return definition, children - - def definition_to_xml(self, resource_fs): - """ Exports Library Content Block to XML """ - xml_object = etree.Element('library_content') - for child in self.get_children(): - self.runtime.add_block_as_child_node(child, xml_object) - # Set node attributes based on our fields. - for field_name, field in self.fields.items(): # pylint: disable=no-member - if field_name in ('children', 'parent', 'content'): - continue - if field.is_set_on(self): - xml_object.set(field_name, str(field.read_from(self))) - return xml_object + block = self.runtime.modulestore.get_item(key, depth=None) # Load the item and all descendants + children = list(getattr(block, "children", [])) + while children: + # @@TODO why is there branch info here now for us to clear?? + child_key = children.pop().replace(version=None, branch=None) + child = self.runtime.modulestore.get_item(child_key) + info['descendants'].append(summarize_block(child_key)) + children.extend(getattr(child, "children", [])) + except ItemNotFoundError: + pass # The block has been deleted + result_json.append(info) + return result_json class LegacyLibrarySummary: diff --git a/xmodule/library_tools.py b/xmodule/library_tools.py index 7f9e83a9373d..52a31c0608ea 100644 --- a/xmodule/library_tools.py +++ b/xmodule/library_tools.py @@ -58,44 +58,6 @@ def get_latest_library_version(self, library_id: str | LibraryLocator) -> str | assert library.location.library_key.version_guid is not None return str(library.location.library_key.version_guid) - def create_block_analytics_summary(self, course_key, block_keys): - """ - Given a CourseKey and a list of (block_type, block_id) pairs, - prepare the JSON-ready metadata needed for analytics logging. - - This is [ - {"usage_key": x, "original_usage_key": y, "original_usage_version": z, "descendants": [...]} - ] - where the main list contains all top-level blocks, and descendants contains a *flat* list of all - descendants of the top level blocks, if any. - """ - def summarize_block(usage_key): - """ Basic information about the given block """ - orig_key, orig_version = self.store.get_block_original_usage(usage_key) - return { - "usage_key": str(usage_key), - "original_usage_key": str(orig_key) if orig_key else None, - "original_usage_version": str(orig_version) if orig_version else None, - } - - result_json = [] - for block_key in block_keys: - key = course_key.make_usage_key(*block_key) - info = summarize_block(key) - info['descendants'] = [] - try: - block = self.store.get_item(key, depth=None) # Load the item and all descendants - children = list(getattr(block, "children", [])) - while children: - child_key = children.pop() - child = self.store.get_item(child_key) - info['descendants'].append(summarize_block(child_key)) - children.extend(getattr(child, "children", [])) - except ItemNotFoundError: - pass # The block has been deleted - result_json.append(info) - return result_json - def can_use_library_content(self, block): """ Determines whether a modulestore holding a course_id supports libraries. diff --git a/xmodule/tests/test_library_content.py b/xmodule/tests/test_library_content.py index 9914ef2d9098..fb3ce1f62cba 100644 --- a/xmodule/tests/test_library_content.py +++ b/xmodule/tests/test_library_content.py @@ -414,7 +414,11 @@ def test_non_editable_settings(self): Test the settings that are marked as "non-editable". """ non_editable_metadata_fields = self.lc_block.non_editable_metadata_fields +<<<<<<< HEAD assert LegacyLibraryContentBlock.display_name not in non_editable_metadata_fields +======= + assert LibraryContentBlock.display_name not in non_editable_metadata_fields +>>>>>>> 49f4a0865b (feat: ItemBankBlock) def test_overlimit_blocks_chosen_randomly(self): """