diff --git a/lms/djangoapps/courseware/block_render.py b/lms/djangoapps/courseware/block_render.py index 8ef637b82bf0..4eecb605c974 100644 --- a/lms/djangoapps/courseware/block_render.py +++ b/lms/djangoapps/courseware/block_render.py @@ -33,6 +33,7 @@ from eventtracking import tracker from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey, UsageKey +from openedx_filters.learning.filters import VerticalBlockChildRenderStarted from rest_framework.decorators import api_view from rest_framework.exceptions import APIException from web_fragments.fragment import Fragment @@ -61,6 +62,7 @@ ) from lms.djangoapps.courseware.model_data import DjangoKeyValueStore, FieldDataCache from lms.djangoapps.courseware.services import UserStateService +from lms.djangoapps.courseware.toggles import incremental_assessment_load_is_enabled from lms.djangoapps.grades.api import GradesUtilService from lms.djangoapps.lms_xblock.field_data import LmsFieldData from lms.djangoapps.lms_xblock.runtime import UserTagsService, lms_applicable_aside_types, lms_wrappers_aside @@ -79,6 +81,7 @@ from openedx.core.lib.cache_utils import CacheService from openedx.core.lib.gating.services import GatingService from openedx.core.lib.license import wrap_with_license +from openedx.core.lib.mobile_utils import is_request_from_mobile_app from openedx.core.lib.url_utils import quote_slashes, unquote_slashes from openedx.core.lib.xblock_services.call_to_action import CallToActionService from openedx.core.lib.xblock_utils import ( @@ -91,6 +94,7 @@ from openedx.core.lib.xblock_utils import request_token as xblock_request_token from openedx.features.content_type_gating.services import ContentTypeGatingService from openedx.features.course_duration_limits.access import course_expiration_wrapper +from openedx.features.course_experience.url_helpers import is_request_from_learning_mfe from openedx.features.discounts.utils import offer_banner_wrapper from xmodule.contentstore.django import contentstore from xmodule.exceptions import NotFoundError as XModuleNotFoundError @@ -106,6 +110,7 @@ XQueueService, ) from xmodule.util.sandboxing import SandboxService +from xmodule.x_module import STUDENT_VIEW if TYPE_CHECKING: from rest_framework.request import Request @@ -861,10 +866,21 @@ def _get_block_by_usage_key(usage_key): def get_block_by_usage_id(request, course_id, usage_id, disable_staff_debug_info=False, course=None, - will_recheck_access=False): + will_recheck_access=False, field_data_cache_depth=None, field_data_cache=None): """ Gets a block instance based on its `usage_id` in a course, for a given request/user + Arguments: + field_data_cache_depth: depth passed through to + FieldDataCache.cache_for_block_descendents. None (the default) prefetches every + descendant, matching historical behavior. Used by the incremental assessment load + (see render_xblock_children) so a batch of children only prefetches state for + themselves, not their own descendants. + field_data_cache: optional FieldDataCache to populate and bind the block with, + instead of building a new one. The runtime binding the block is shared with + every block loaded through it later in the request, so a caller that adds more + state to this cache afterwards makes that state visible to those blocks too. + Returns (instance, tracking_context) """ course_key = CourseKey.from_string(course_id) @@ -872,12 +888,16 @@ def get_block_by_usage_id(request, course_id, usage_id, disable_staff_debug_info block, tracking_context = _get_block_by_usage_key(usage_key) _, user = setup_masquerade(request, course_key, has_access(request.user, 'staff', block, course_key)) - field_data_cache = FieldDataCache.cache_for_block_descendents( - course_key, - user, - block, - read_only=CrawlersConfig.is_crawler(request), - ) + if field_data_cache is None: + field_data_cache = FieldDataCache.cache_for_block_descendents( + course_key, + user, + block, + depth=field_data_cache_depth, + read_only=CrawlersConfig.is_crawler(request), + ) + else: + field_data_cache.add_block_descendents(block, depth=field_data_cache_depth) instance = get_block_for_descriptor( user, request, @@ -1081,3 +1101,344 @@ def append_data_to_webob_response(response, data): response_data.update(data) response.body = json.dumps(response_data).encode('utf-8') return response + + +# Leaf block types counted by count_renderable_descendants. Only types whose render cost +# is significant are counted -- a unit with fifty short HTML blocks renders quickly and +# gains nothing from being split up. +INCREMENTAL_LOAD_COUNTED_BLOCK_TYPES = frozenset(['problem']) + + +def count_renderable_descendants(block, depth=4): + """ + Estimate how many leaf blocks a learner will actually be shown under `block`. + + Used only to decide whether a unit is large enough to be worth loading incrementally. + For randomized banks (blocks whose has_dynamic_children() is True, such as + library_content / item_bank) this counts what the learner will see -- `max_count`, or + the whole pool when it is unset or -1 -- rather than the full candidate pool. + """ + if block is None or depth < 0: + return 0 + + has_dynamic_children = getattr(block, 'has_dynamic_children', None) + if callable(has_dynamic_children) and has_dynamic_children(): + max_count = getattr(block, 'max_count', None) + pool_size = len(getattr(block, 'children', None) or []) + if isinstance(max_count, int) and max_count > 0: + return min(max_count, pool_size) if pool_size else max_count + # max_count of -1 (or unset) means "show all of them". + return pool_size + + block_type = getattr(getattr(block, 'location', None), 'block_type', None) + get_children = getattr(block, 'get_children', None) + if not callable(get_children): + return 1 if block_type in INCREMENTAL_LOAD_COUNTED_BLOCK_TYPES else 0 + + children = get_children() + if not children: + return 1 if block_type in INCREMENTAL_LOAD_COUNTED_BLOCK_TYPES else 0 + + return sum(count_renderable_descendants(child, depth=depth - 1) for child in children) + + +def should_use_incremental_load(request, block): + """ + Return True when `block` should be rendered as a shell -- the first + settings.INCREMENTAL_LOAD_EAGER_COUNT children rendered inline and the rest left as + placeholders for XBlockChildren to fill in batches -- instead of rendering every + child in one response. + + All of the following must hold: + 1. courseware.incremental_assessment_load is enabled for the course (kill switch, + off by default). + 2. `block` is a vertical -- the only block the courseware iframe renders as a unit, + and the only place a shell makes sense. + 3. The unit is big enough that a single render risks the upstream request timeout. + 4. The request is not from a mobile app -- mobile webviews have not been validated + against the incremental-load bootstrap. + """ + block_type = getattr(getattr(block, 'location', None), 'block_type', None) + if block_type not in ('vertical', 'unit'): + return False + + if is_request_from_mobile_app(request): + return False + + course_key = block.location.course_key + if not incremental_assessment_load_is_enabled(course_key): + return False + + return count_renderable_descendants(block) > settings.INCREMENTAL_LOAD_PROBLEM_THRESHOLD + + +def _allowed_child_usage_keys(parent): + """ + Return the set of usage keys the current learner is allowed to load under `parent`. + + This is the batch endpoint's security boundary: without it, a learner could ask for + any usage key under a randomized bank, including problems it never assigned them. + Dynamic blocks (has_dynamic_children() is True, such as library_content / item_bank) + expose only the learner's persisted selection via get_child_blocks(); everything else + exposes its ordinary children. + """ + has_dynamic_children = getattr(parent, 'has_dynamic_children', None) + get_child_blocks = getattr(parent, 'get_child_blocks', None) + if callable(has_dynamic_children) and has_dynamic_children() and callable(get_child_blocks): + children = get_child_blocks() + return {child.location for child in children if child is not None} + + # ``children`` is a content-scoped field containing UsageKeys. Reading it does not + # instantiate/bind the child blocks. Calling ``parent.get_children()`` here would + # create child instances through the parent's shallow field-data cache; those + # instances must not later be rendered with a different learner-state cache. + child_usage_keys = getattr(parent, 'children', None) + if isinstance(child_usage_keys, (list, tuple)): + return {child_usage_key for child_usage_key in child_usage_keys if child_usage_key is not None} + + # Keep compatibility with XBlocks that do not expose their children as a normal + # UsageKey list (and with older test doubles). This fallback is only an access check; + # the endpoint still resolves the objects it renders from the modulestore below. + children = parent.get_children() + return {child.location for child in children if child is not None} + + +def _load_unbound_child_descriptors(child_usage_keys, parent=None): + """ + Resolve child usage keys to fresh, unbound XBlock descriptors. + + The parent is intentionally not used for this lookup. A parent loaded with a shallow + FieldDataCache can already have child instances bound to that cache. Reusing one of + those instances and then passing a second cache to ``get_block_for_descriptor`` is + unsafe because ``bind_for_student`` skips rebinding when the user id is unchanged. + + ``parent`` is optional and is used only to carry over the runtime's export filesystem + setting, which ``XModuleMixin.get_child()`` normally copies to child instances. + """ + descriptors = {} + missing = [] + store = modulestore() + for child_usage_key in child_usage_keys: + try: + descriptor = store.get_item(child_usage_key) + except ItemNotFoundError: + missing.append(child_usage_key) + continue + if descriptor is None: + missing.append(child_usage_key) + continue + if parent is not None: + descriptor.runtime.export_fs = parent.runtime.export_fs + descriptors[child_usage_key] = descriptor + return descriptors, missing + + +def _lazy_child_student_view_context(request): + """ + Build the child render context used by the incremental load endpoint. + + Keep this aligned with render_xblock's student_view_context and + VerticalBlock._student_or_public_view's child context so lazily-rendered + children behave like children rendered inline by the unit. + """ + context = request.GET.dict() + context['show_bookmark_button'] = request.GET.get('show_bookmark_button', '0') == '1' + context['show_title'] = request.GET.get('show_title', '1') == '1' + context['hide_access_error_blocks'] = ( + is_request_from_learning_mfe(request) and request.GET.get('recheck_access') == '1' + ) + context['is_mobile_app'] = is_request_from_mobile_app(request) + context['child_of_vertical'] = True + return context + + +def render_xblock_children(request, parent_usage_key, child_usage_keys, course=None): + """ + Render a bounded batch of `parent`'s children and return their HTML. + + Backs the XBlockChildren API that the incremental assessment load bootstrap calls + repeatedly to fill in the placeholders VerticalBlock leaves for children past its + eager-render window. One FieldDataCache is built for the whole batch, and the + masquerade user is resolved once and reused for every child in it -- building a + separate cache, or re-resolving masquerade, per child would multiply exactly the + per-child render cost this feature exists to bound. + + A child that fails to render is reported in `errors` rather than failing the whole + batch, so one bad problem doesn't cost the learner the rest of the batch. + + Returns: + dict with `parent_usage_key`, `results` (list of `{usage_key, html}`) and + `errors` (list of `{usage_key, error, message}`). + + Raises: + ValueError: the request is malformed or the batch is too large. Callers should + map this to a 400. + """ + if not child_usage_keys: + raise ValueError('child_usage_keys is required') + + max_batch = settings.XBLOCK_CHILDREN_BATCH_MAX + if len(child_usage_keys) > max_batch: + raise ValueError(f'At most {max_batch} child_usage_keys are allowed per request') + + course_key = parent_usage_key.course_key + staff_access = bool(has_access(request.user, 'staff', course_key)) + _, user = setup_masquerade(request, course_key, staff_access) + + # One cache for the parent and the whole batch. Binding the parent prepares the shared + # runtime around this cache, and blocks a child loads itself through that runtime -- + # e.g. the problem an item_bank / library_content child selected -- are bound with it + # too, so they only see the learner's state if it is added to this same cache. + field_data_cache = FieldDataCache([], course_key, user, read_only=CrawlersConfig.is_crawler(request)) + + with modulestore().bulk_operations(course_key): + parent, _ = get_block_by_usage_id( + request, + str(course_key), + str(parent_usage_key), + course=course, + field_data_cache_depth=0, + will_recheck_access=True, + field_data_cache=field_data_cache, + ) + + requested = [] + errors = [] + allowed_keys = _allowed_child_usage_keys(parent) + for child_key in child_usage_keys: + if child_key in allowed_keys: + requested.append(child_key) + else: + errors.append({ + 'usage_key': str(child_key), + 'error': 'forbidden', + 'message': "Block is not among this learner's children for that parent.", + }) + + results = [] + if not requested: + return {'parent_usage_key': str(parent_usage_key), 'results': results, 'errors': errors} + + # Resolve from the modulestore rather than reusing parent.get_children() instances. + # The latter may already be bound to the parent's depth-0 cache for this same user. + # Those instances would then skip rebinding when get_block_for_descriptor() is + # called with the batch cache, causing a valid StudentModule row to look absent. + children_by_key, missing_keys = _load_unbound_child_descriptors(requested, parent=parent) + for child_key in missing_keys: + errors.append({ + 'usage_key': str(child_key), + 'error': 'not_found', + 'message': 'Block could not be loaded.', + }) + + renderable_children = [ + children_by_key[child_key] for child_key in requested if child_key in children_by_key + ] + + # One state lookup for the whole batch, not one per child. Depth=1 also covers the + # problems inside each library_content/item_bank child, without recursing further. + field_data_cache.add_block_descendents_batch(renderable_children, depth=1) + + # Bind each fresh descriptor exactly once, after the cache containing the learner's + # state has been populated. Besides fixing state hydration, doing this before the + # completion/access pass keeps those checks aligned with the eventual render. + instances_by_key = {} + for child_key in requested: + child = children_by_key.get(child_key) + if child is None: + continue + try: + instance = get_block_for_descriptor( + user, + request, + child, + field_data_cache, + course_key, + course=course, + will_recheck_access=True, + ) + except Exception as exc: # pylint: disable=broad-except + log.exception('Incremental load: child %s under %s failed to bind', child_key, parent_usage_key) + errors.append({ + 'usage_key': str(child_key), + 'error': 'render_failed', + 'message': str(exc), + }) + continue + if instance is None: + errors.append({ + 'usage_key': str(child_key), + 'error': 'forbidden', + 'message': 'Learner does not have access to this block.', + }) + continue + log.debug( + 'Incremental load: bound child %s for parent %s with the batch state cache; ' + 'user_id before=%s after=%s', + child_key, + parent_usage_key, + getattr(child.scope_ids, 'user_id', None), + getattr(instance.scope_ids, 'user_id', None), + ) + instances_by_key[child_key] = instance + + # Both computed once for the whole batch, not per child, matching how + # VerticalBlock computes them once for its own eager-rendered children. + block_has_access_error = getattr(parent, 'block_has_access_error', None) + completion_service = parent.runtime.service(parent, 'completion') + child_blocks_to_complete_on_view = set() + complete_on_view_delay = None + if completion_service and completion_service.completion_tracking_enabled(): + requested_children = [ + instances_by_key[child_key] for child_key in requested if child_key in instances_by_key + ] + child_blocks_to_complete_on_view = completion_service.blocks_to_mark_complete_on_view( + requested_children + ) + complete_on_view_delay = completion_service.get_complete_on_view_delay_ms() + + for child_key in requested: + instance = instances_by_key.get(child_key) + if instance is None: + continue + + child_context = _lazy_child_student_view_context(request) + if ( + child_context.get('hide_access_error_blocks') + and callable(block_has_access_error) + and block_has_access_error(instance) + ): + # Matches VerticalBlock's own eager-render skip (block_has_access_error): + # a block gated for this learner must stay hidden whether it renders + # inline or gets lazy-loaded later. + results.append({'usage_key': str(child_key), 'html': ''}) + continue + if instance in child_blocks_to_complete_on_view: + child_context['wrap_xblock_data'] = { + 'mark-completed-on-view-after-delay': complete_on_view_delay + } + + try: + instance, child_context = VerticalBlockChildRenderStarted.run_filter( + block=instance, + context=child_context + ) + fragment = instance.render(STUDENT_VIEW, context=child_context) + results.append({'usage_key': str(child_key), 'html': fragment.content}) + except VerticalBlockChildRenderStarted.PreventChildBlockRender as exc: + log.info('Incremental load: skipping child %s. Reason: %s', child_key, exc.message) + errors.append({ + 'usage_key': str(child_key), + 'error': 'forbidden', + 'message': exc.message, + }) + except Exception as exc: # pylint: disable=broad-except + # One problem failing to render must not cost the learner the rest of the batch. + log.exception('Incremental load: child %s under %s failed to render', child_key, parent_usage_key) + errors.append({ + 'usage_key': str(child_key), + 'error': 'render_failed', + 'message': str(exc), + }) + + return {'parent_usage_key': str(parent_usage_key), 'results': results, 'errors': errors} diff --git a/lms/djangoapps/courseware/model_data.py b/lms/djangoapps/courseware/model_data.py index 6218b0e9f3e7..ec6c138d35e7 100644 --- a/lms/djangoapps/courseware/model_data.py +++ b/lms/djangoapps/courseware/model_data.py @@ -733,6 +733,24 @@ def add_block_descendents(self, block, depth=None, block_filter=lambda block: Tr should be cached """ + self.add_block_descendents_batch([block], depth=depth, block_filter=block_filter) + + def add_block_descendents_batch(self, blocks, depth=None, block_filter=lambda block: True): + """ + Add descendants of several blocks in one state-cache population operation. + + This is the batch equivalent of :meth:`add_block_descendents`. It is useful for + endpoints that render several sibling blocks independently: walking each sibling + separately and calling ``add_blocks_to_cache`` each time causes one user-state + lookup per sibling even though all of the data belongs to the same request. + + The supplied blocks must belong to the same course as this cache. Duplicate usage + keys are removed before the fields are read so shared required descriptors do not + cause duplicate work. + """ + if not blocks: + return + def get_child_blocks(block, depth, block_filter): """ Return a list of all child blocks down to the specified depth @@ -756,10 +774,16 @@ def get_child_blocks(block, depth, block_filter): return blocks - with modulestore().bulk_operations(block.location.course_key): - blocks = get_child_blocks(block, depth, block_filter) + blocks_to_cache = [] + seen_locations = set() + with modulestore().bulk_operations(blocks[0].location.course_key): + for block in blocks: + for descendant in get_child_blocks(block, depth, block_filter): + if descendant.location not in seen_locations: + seen_locations.add(descendant.location) + blocks_to_cache.append(descendant) - self.add_blocks_to_cache(blocks) + self.add_blocks_to_cache(blocks_to_cache) @classmethod def cache_for_block_descendents(cls, course_id, user, block, depth=None, diff --git a/lms/djangoapps/courseware/tests/test_incremental_render.py b/lms/djangoapps/courseware/tests/test_incremental_render.py new file mode 100644 index 000000000000..b9919838913c --- /dev/null +++ b/lms/djangoapps/courseware/tests/test_incremental_render.py @@ -0,0 +1,311 @@ +""" +Tests for the incremental assessment load: descendant counting, shell eligibility, the +batch endpoint's access boundary, and VerticalBlock's eager-window rendering. +""" +from unittest.mock import Mock, patch + +from django.test import RequestFactory, TestCase, override_settings +from opaque_keys.edx.locator import CourseLocator + +from lms.djangoapps.courseware.block_render import ( + _allowed_child_usage_keys, + _lazy_child_student_view_context, + _load_unbound_child_descriptors, + count_renderable_descendants, + should_use_incremental_load, +) +from xmodule.modulestore.exceptions import ItemNotFoundError + +COURSE_KEY = CourseLocator('org', 'course', 'run') + + +def _block(block_type, children=None, dynamic=False, max_count=None, pool=0): + """ + Build a stand-in XBlock with just the attributes the helpers under test read. + """ + block = Mock() + block.location.block_type = block_type + block.location.course_key = COURSE_KEY + block.has_dynamic_children.return_value = dynamic + block.get_children.return_value = children or [] + if dynamic: + block.max_count = max_count + block.children = [Mock() for _ in range(pool)] + return block + + +class CountRenderableDescendantsTests(TestCase): + """ + Tests for count_renderable_descendants. + """ + + def test_counts_problems_in_a_flat_unit(self): + vertical = _block('vertical', children=[_block('problem'), _block('problem')]) + assert count_renderable_descendants(vertical) == 2 + + def test_ignores_cheap_leaf_types(self): + # A unit full of short HTML renders quickly and should not be split up. + vertical = _block('vertical', children=[_block('html'), _block('video'), _block('problem')]) + assert count_renderable_descendants(vertical) == 1 + + def test_randomized_bank_counts_what_the_learner_sees(self): + # This is the shape that caused the incident: a large pool, one problem shown. + bank = _block('library_content', dynamic=True, max_count=1, pool=40) + vertical = _block('vertical', children=[bank]) + assert count_renderable_descendants(vertical) == 1 + + def test_many_single_selection_banks_add_up(self): + banks = [_block('library_content', dynamic=True, max_count=1, pool=4) for _ in range(90)] + vertical = _block('vertical', children=banks) + assert count_renderable_descendants(vertical) == 90 + + def test_max_count_of_minus_one_counts_whole_pool(self): + # -1 means "show all of them", the most expensive configuration there is. + bank = _block('library_content', dynamic=True, max_count=-1, pool=30) + assert count_renderable_descendants(bank) == 30 + + def test_max_count_above_pool_is_capped_by_pool(self): + bank = _block('library_content', dynamic=True, max_count=50, pool=6) + assert count_renderable_descendants(bank) == 6 + + def test_depth_limit_stops_runaway_recursion(self): + deep = _block('problem') + for _unused in range(10): + deep = _block('vertical', children=[deep]) + assert count_renderable_descendants(deep) >= 0 + + +class ShouldUseIncrementalLoadTests(TestCase): + """ + Tests for should_use_incremental_load. All conditions must hold. + """ + + def setUp(self): + super().setUp() + self.request = Mock() + self.large_vertical = _block('vertical', children=[_block('problem') for _ in range(30)]) + + def _check(self, block, flag_enabled=True, mobile=False): + """ + Call should_use_incremental_load with the flag and mobile-app checks mocked. + """ + with patch( + 'lms.djangoapps.courseware.block_render.incremental_assessment_load_is_enabled', + return_value=flag_enabled, + ), patch( + 'lms.djangoapps.courseware.block_render.is_request_from_mobile_app', + return_value=mobile, + ): + return should_use_incremental_load(self.request, block) + + @override_settings(INCREMENTAL_LOAD_PROBLEM_THRESHOLD=20) + def test_large_vertical_with_flag_on(self): + assert self._check(self.large_vertical) is True + + @override_settings(INCREMENTAL_LOAD_PROBLEM_THRESHOLD=20) + def test_flag_off_is_the_kill_switch(self): + assert self._check(self.large_vertical, flag_enabled=False) is False + + @override_settings(INCREMENTAL_LOAD_PROBLEM_THRESHOLD=20) + def test_small_vertical_is_untouched_even_when_flagged(self): + small = _block('vertical', children=[_block('problem') for _ in range(5)]) + assert self._check(small) is False + + @override_settings(INCREMENTAL_LOAD_PROBLEM_THRESHOLD=20) + def test_non_vertical_blocks_never_shell(self): + assert self._check(_block('sequential', children=[_block('problem')] * 30)) is False + assert self._check(_block('problem')) is False + + @override_settings(INCREMENTAL_LOAD_PROBLEM_THRESHOLD=20) + def test_mobile_apps_are_excluded(self): + assert self._check(self.large_vertical, mobile=True) is False + + @override_settings(INCREMENTAL_LOAD_PROBLEM_THRESHOLD=100) + def test_threshold_is_respected(self): + assert self._check(self.large_vertical) is False + + +class AllowedChildUsageKeysTests(TestCase): + """ + Tests for the batch endpoint's access boundary. + """ + + def test_static_parent_exposes_its_children(self): + children = [_block('problem'), _block('problem')] + parent = _block('vertical', children=children) + assert _allowed_child_usage_keys(parent) == {child.location for child in children} + + def test_static_parent_uses_content_keys_without_instantiating_children(self): + children = [_block('problem'), _block('problem')] + parent = _block('vertical') + parent.children = [child.location for child in children] + parent.get_children.side_effect = AssertionError('static children must not be instantiated') + + assert _allowed_child_usage_keys(parent) == {child.location for child in children} + + def test_dynamic_parent_exposes_only_the_learner_selection(self): + # A learner must not be able to pull problems the bank did not assign them. + selected = _block('problem') + unselected = _block('problem') + parent = _block('library_content', children=[selected, unselected], dynamic=True, max_count=1, pool=2) + parent.get_child_blocks.return_value = [selected] + + allowed = _allowed_child_usage_keys(parent) + assert allowed == {selected.location} + assert unselected.location not in allowed + + def test_none_children_are_skipped(self): + good = _block('problem') + parent = _block('vertical', children=[good, None]) + assert _allowed_child_usage_keys(parent) == {good.location} + + +class UnboundChildDescriptorTests(TestCase): + """Tests for resolving lazy children independently of a parent instance cache.""" + + def test_resolves_requested_keys_without_using_parent_children(self): + first_key = COURSE_KEY.make_usage_key('problem', 'first') + second_key = COURSE_KEY.make_usage_key('problem', 'second') + first_descriptor = Mock() + second_descriptor = Mock() + store = Mock() + store.get_item.side_effect = [first_descriptor, second_descriptor] + + with patch('lms.djangoapps.courseware.block_render.modulestore', return_value=store): + descriptors, missing = _load_unbound_child_descriptors([first_key, second_key]) + + assert list(descriptors) == [first_key, second_key] + assert descriptors[first_key] is first_descriptor + assert descriptors[second_key] is second_descriptor + assert not missing + + def test_reports_missing_descriptor_without_aborting_the_batch(self): + existing_key = COURSE_KEY.make_usage_key('problem', 'existing') + missing_key = COURSE_KEY.make_usage_key('problem', 'missing') + existing_descriptor = Mock() + store = Mock() + store.get_item.side_effect = [existing_descriptor, ItemNotFoundError()] + + with patch('lms.djangoapps.courseware.block_render.modulestore', return_value=store): + descriptors, missing = _load_unbound_child_descriptors([existing_key, missing_key]) + + assert descriptors == {existing_key: existing_descriptor} + assert missing == [missing_key] + + +class LazyChildStudentViewContextTests(TestCase): + """ + Tests for the child render context used by incremental batch rendering. + """ + + def test_context_matches_unit_child_render_flags(self): + request = RequestFactory().get( + '/', + { + 'recheck_access': '1', + 'show_bookmark_button': '0', + 'show_title': '0', + 'view': 'student_view', + } + ) + + context = _lazy_child_student_view_context(request) + + assert context['child_of_vertical'] is True + assert context['show_bookmark_button'] is False + assert context['show_title'] is False + assert context['is_mobile_app'] is False + assert context['view'] == 'student_view' + + +class VerticalEagerWindowRenderTests(TestCase): + """ + Tests that VerticalBlock renders only the eager window and reports the rest as + placeholders. + """ + + def _render_context(self, eager_count, child_count=8): + """ + Run VerticalBlock._student_or_public_view with stubbed children and return the + fragment context it handed to the template. + """ + from xmodule.vertical_block import VerticalBlock + + children = [] + for index in range(child_count): + child = Mock() + child.location = f'block-v1:org+course+run+type@problem+block@p{index}' + rendered = Mock() + rendered.content = f'
problem {index}
' + rendered.resources = [] + child.render.return_value = rendered + children.append(child) + + # Not Mock(spec=VerticalBlock): `runtime` is set dynamically per-instance by the + # XBlock runtime binding, not declared on the class, so spec= would reject it. + block = Mock() + block.get_children.return_value = children + block.is_block_complete_for_assignments.return_value = None + block.due = None + block.location = 'block-v1:org+course+run+type@vertical+block@v1' + block.display_name_with_default = 'Unit' + + captured = {} + + def capture_render(_template, fragment_context): + captured.update(fragment_context) + return '' + + mako = Mock() + mako.render_lms_template.side_effect = capture_render + completion = Mock() + completion.completion_tracking_enabled.return_value = False + user_service = Mock() + user_service.get_current_user.return_value.opt_attrs = {'edx-platform.username': 'learner'} + + def service(_self, name): + return { + 'mako': mako, + 'completion': completion, + 'bookmarks': Mock(is_bookmarked=Mock(return_value=False)), + 'user': user_service, + 'call_to_action': None, + }.get(name) + + block.runtime.service.side_effect = service + + context = {'incremental_load_eager_count': eager_count} if eager_count is not None else {} + with patch('xmodule.vertical_block.add_webpack_js_to_fragment'): + VerticalBlock._student_or_public_view( # pylint: disable=protected-access + block, context, 'student_view' + ) + return captured, children + + def test_renders_only_the_eager_window(self): + captured, children = self._render_context(eager_count=5, child_count=8) + + assert len(captured['items']) == 5 + assert len(captured['lazy_children']) == 3 + for child in children[:5]: + child.render.assert_called_once() + for child in children[5:]: + child.render.assert_not_called() + + def test_placeholders_are_reported_in_order(self): + captured, children = self._render_context(eager_count=2, child_count=6) + assert captured['lazy_children'] == [str(child.location) for child in children[2:]] + assert captured['parent_usage_key'] is not None + + def test_without_eager_count_every_child_renders(self): + # The default path: flag off, no shell, behavior identical to before this feature. + captured, children = self._render_context(eager_count=None, child_count=6) + assert len(captured['items']) == 6 + assert captured['lazy_children'] == [] + assert captured['parent_usage_key'] is None + for child in children: + child.render.assert_called_once() + + def test_eager_count_at_or_above_child_count_renders_everything(self): + captured, children = self._render_context(eager_count=10, child_count=6) + assert len(captured['items']) == 6 + assert captured['lazy_children'] == [] diff --git a/lms/djangoapps/courseware/tests/test_model_data.py b/lms/djangoapps/courseware/tests/test_model_data.py index c7f9d02597ea..1c8255a2fcf9 100644 --- a/lms/djangoapps/courseware/tests/test_model_data.py +++ b/lms/djangoapps/courseware/tests/test_model_data.py @@ -452,3 +452,35 @@ class TestStudentInfoStorage(OtherUserFailureTestMixin, StorageTestBase, TestCas storage_class = XModuleStudentInfoField other_key_factory = partial(DjangoKeyValueStore.Key, Scope.user_info, 2, 'mock_problem') # user_id=2, not 1 existing_field_name = "existing_field" + + +class TestFieldDataCacheBatchDescendents(TestCase): + """ + Tests for FieldDataCache.add_block_descendents_batch. + """ + + def setUp(self): + super().setUp() + self.user = UserFactory() + + def test_batch_descendent_prefetch_populates_the_cache_once(self): + """ + Sibling lazy children share one user-state lookup instead of triggering one + cache population call per child. + """ + first = mock_block() + first.location = COURSE_KEY.make_usage_key('problem', 'first') + first.get_children.return_value = [] + first.get_required_block_descriptors.return_value = [] + + second = mock_block() + second.location = COURSE_KEY.make_usage_key('problem', 'second') + second.get_children.return_value = [] + second.get_required_block_descriptors.return_value = [] + + cache = FieldDataCache([], COURSE_KEY, self.user) + with patch.object(cache, 'add_blocks_to_cache') as add_blocks_to_cache: + cache.add_block_descendents_batch([first, second], depth=1) + + add_blocks_to_cache.assert_called_once() + assert add_blocks_to_cache.call_args.args[0] == [first, second] diff --git a/lms/djangoapps/courseware/toggles.py b/lms/djangoapps/courseware/toggles.py index 3a51ff1f6d91..4c74a9f0cca7 100644 --- a/lms/djangoapps/courseware/toggles.py +++ b/lms/djangoapps/courseware/toggles.py @@ -111,6 +111,28 @@ f'{WAFFLE_FLAG_NAMESPACE}.optimized_render_xblock', __name__ ) +# .. toggle_name: courseware.incremental_assessment_load +# .. toggle_implementation: CourseWaffleFlag +# .. toggle_default: False +# .. toggle_description: Render large assessment units as a shell -- the first few problems +# plus placeholders -- and load the remaining problems in batches from +# /api/courseware/xblock_children/. Units at or below +# settings.INCREMENTAL_LOAD_PROBLEM_THRESHOLD are unaffected even when this is on. Intended +# for assessments large enough that rendering every problem in one response risks exceeding +# the upstream request timeout. +# .. toggle_use_cases: opt_in +# .. toggle_creation_date: 2026-08-27 +COURSEWARE_INCREMENTAL_ASSESSMENT_LOAD = CourseWaffleFlag( + f'{WAFFLE_FLAG_NAMESPACE}.incremental_assessment_load', __name__ +) + + +def incremental_assessment_load_is_enabled(course_key=None): + """ + Return whether large assessment units in this course should load incrementally. + """ + return COURSEWARE_INCREMENTAL_ASSESSMENT_LOAD.is_enabled(course_key) + # .. toggle_name: COURSES_INVITE_ONLY # .. toggle_implementation: SettingToggle # .. toggle_type: feature_flag diff --git a/lms/djangoapps/courseware/views/views.py b/lms/djangoapps/courseware/views/views.py index 4dcb50579251..c56a3241cefb 100644 --- a/lms/djangoapps/courseware/views/views.py +++ b/lms/djangoapps/courseware/views/views.py @@ -150,7 +150,7 @@ from xmodule.tabs import CourseTabList from xmodule.x_module import STUDENT_VIEW -from ..block_render import get_block, get_block_by_usage_id, get_block_for_descriptor +from ..block_render import get_block, get_block_by_usage_id, get_block_for_descriptor, should_use_incremental_load from ..tabs import _get_dynamic_tabs from ..toggles import COURSEWARE_OPTIMIZED_RENDER_XBLOCK, ENABLE_COURSE_DISCOVERY_DEFAULT_LANGUAGE_FILTER @@ -1642,6 +1642,14 @@ def render_xblock(request, usage_key_string, check_if_enrolled=True, disable_sta student_view_context['show_bookmark_button'] = request.GET.get('show_bookmark_button', '0') == '1' student_view_context['show_title'] = request.GET.get('show_title', '1') == '1' + # Large assessment units render the first few problems and leave placeholders + # for the rest, which the page then loads in batches from XBlockChildren. Off + # unless the course opts in, so every other unit renders in a single response + # exactly as it does today. + if should_use_incremental_load(request, block): + student_view_context['incremental_load_eager_count'] = settings.INCREMENTAL_LOAD_EAGER_COUNT + set_custom_attribute('incremental_assessment_load', True) + is_learning_mfe = is_request_from_learning_mfe(request) # Right now, we only care about this in regards to the Learning MFE because it results # in a bad UX if we display blocks with access errors (repeated upgrade messaging). diff --git a/lms/envs/common.py b/lms/envs/common.py index fb60afe0070a..dbbd1886f927 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -918,6 +918,30 @@ SEARCH_COURSEWARE_CONTENT_LOG_PARAMS = False +############################# Incremental assessment load ##################### +# Only consulted when the courseware.incremental_assessment_load waffle flag is on for +# the course. See lms/djangoapps/courseware/block_render.py. + +# .. setting_name: INCREMENTAL_LOAD_PROBLEM_THRESHOLD +# .. setting_default: 20 +# .. setting_description: Units with more estimated problems than this are rendered as a shell +# (the first INCREMENTAL_LOAD_EAGER_COUNT children inline, the rest as placeholders) instead of +# rendering every child in one response. Units at or below it are unaffected. +INCREMENTAL_LOAD_PROBLEM_THRESHOLD = 20 + +# .. setting_name: INCREMENTAL_LOAD_EAGER_COUNT +# .. setting_default: 5 +# .. setting_description: How many children a shell response renders inline before it starts +# emitting placeholders. These arrive with the page, so the learner can begin immediately, and +# rendering them also pulls in the CSS/JS the batch-loaded children will need. +INCREMENTAL_LOAD_EAGER_COUNT = 5 + +# .. setting_name: XBLOCK_CHILDREN_BATCH_MAX +# .. setting_default: 10 +# .. setting_description: Maximum children expected and accepted by a single request to +# /api/courseware/xblock_children/. Unexpected requests above this are rejected with a 400. +XBLOCK_CHILDREN_BATCH_MAX = 10 + # .. setting_name: ELASTIC_SEARCH_INDEX_PREFIX # .. setting_default: '' # .. setting_description: Specifies the prefix used when naming elasticsearch indexes related to edx-search. diff --git a/lms/templates/vert_module.html b/lms/templates/vert_module.html index 909e7db829c1..9ea90614b2b5 100644 --- a/lms/templates/vert_module.html +++ b/lms/templates/vert_module.html @@ -59,7 +59,11 @@

${unit_title}

% endfor % endif -
+
% for idx, item in enumerate(items): % if item['content']:
@@ -67,8 +71,24 @@

${unit_title}

%endif % endfor +% if lazy_children: + ## Deliberately not rendered server-side -- vert_module_lazy_bootstrap.html fetches + ## these in batches from XBlockChildren and fills each placeholder in turn. + % for offset, usage_key in enumerate(lazy_children): +
+
+ ${_("Loading question {number}").format(number=len(items) + offset + 1)} +
+
+ % endfor +% endif
+% if lazy_children: + <%include file='vert_module_lazy_bootstrap.html' args="parent_usage_key=parent_usage_key, lazy_children=lazy_children"/> +% endif + % if reset_button:
diff --git a/lms/templates/vert_module_lazy_bootstrap.html b/lms/templates/vert_module_lazy_bootstrap.html new file mode 100644 index 000000000000..2eaa315da551 --- /dev/null +++ b/lms/templates/vert_module_lazy_bootstrap.html @@ -0,0 +1,263 @@ +<%page expression_filter="h"/> + +<%! +from django.conf import settings +from django.utils.translation import gettext as _ + +from openedx.core.djangolib.js_utils import dump_js_escaped_json, js_escaped_string +%> + +
+ + diff --git a/openedx/core/djangoapps/courseware_api/tests/test_views.py b/openedx/core/djangoapps/courseware_api/tests/test_views.py index 3f4f5fae0ebc..13f9ab53d644 100644 --- a/openedx/core/djangoapps/courseware_api/tests/test_views.py +++ b/openedx/core/djangoapps/courseware_api/tests/test_views.py @@ -13,8 +13,10 @@ from django.contrib.auth import get_user_model from django.test import override_settings from django.test.client import RequestFactory +from django.urls import reverse from edx_django_utils.cache import TieredCache from edx_toggles.toggles.testutils import override_waffle_flag, override_waffle_switch +from xblocks_contrib.problem.capa.testing.response_xml_factory import OptionResponseXMLFactory from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.course_modes.tests.factories import CourseModeFactory @@ -38,6 +40,7 @@ from lms.djangoapps.verify_student.services import IDVerificationService from openedx.core.djangoapps.agreements.api import create_integrity_signature from openedx.core.djangolib.testing.utils import skip_unless_lms +from openedx.core.lib.url_utils import quote_slashes from openedx.features.course_experience.waffle import ENABLE_COURSE_ABOUT_SIDEBAR_HTML from xmodule.data import CertificatesDisplayBehaviors from xmodule.modulestore.django import modulestore @@ -501,6 +504,175 @@ def test_hidden_after_due(self, is_past_due, masquerade_config, expected_hidden, assert bool(response.data['banner_text']) == expected_banner +class XBlockChildrenApiTestViews(BaseCoursewareTests): + """ + Tests for the xblock_children batch REST API (incremental assessment load) + """ + @classmethod + def setUpClass(cls): + super().setUpClass() + CourseEnrollment.enroll(cls.user, cls.course.id, 'audit') + cls.children = [ + BlockFactory(parent=cls.unit, category='html', display_name=f'child-{i}', data=f'

child {i}

') + for i in range(3) + ] + cls.url = f'/api/courseware/xblock_children/{cls.unit.location}' + + @staticmethod + def _child_keys_param(children): + return ','.join(str(child.location) for child in children) + + def test_batch_renders_requested_children(self): + response = self.client.get(self.url, {'child_usage_keys': self._child_keys_param(self.children)}) + assert response.status_code == 200 + data = response.json() + assert data['parent_usage_key'] == str(self.unit.location) + assert not data['errors'] + returned_keys = {result['usage_key'] for result in data['results']} + assert returned_keys == {str(child.location) for child in self.children} + for result in data['results']: + assert result['html'] + + def _create_problem(self, parent_location): + """ + Create a one-input option-response problem under `parent_location`. + """ + problem_xml = OptionResponseXMLFactory().build_xml( + question_text='The correct answer is Correct', + num_inputs=1, + weight=1, + options=['Correct', 'Incorrect'], + correct_option='Correct', + ) + return BlockFactory.create( + parent_location=parent_location, + category='problem', + data=problem_xml, + display_name='persisted-problem', + ) + + def _submit_correct_answer(self, problem): + """ + Submit `problem` through the normal problem_check handler; return its answer input key. + """ + problem_check_url = reverse( + 'xblock_handler', + kwargs={ + 'course_id': str(self.course.id), + 'usage_id': quote_slashes(str(problem.location)), + 'handler': 'xmodule_handler', + 'suffix': 'problem_check', + }, + ) + answer_key = f'input_{problem.location.html_id()}_2_1' + submit_response = self.client.post(problem_check_url, {answer_key: 'Correct'}) + assert submit_response.status_code == 200 + return answer_key + + def _render_single_child(self, child): + """ + Fetch one child through the batch endpoint and return its rendered HTML. + """ + response = self.client.get(self.url, {'child_usage_keys': str(child.location)}) + assert response.status_code == 200 + data = response.json() + assert not data['errors'] + return data['results'][0]['html'] + + def test_batch_renders_persisted_problem_state(self): + """ + A problem rendered after the eager window must use the same StudentModule state + as the normal problem handler, rather than appearing as a fresh attempt. + """ + problem = self._create_problem(self.unit.location) + answer_key = self._submit_correct_answer(problem) + + rendered_html = self._render_single_child(problem) + assert 'data-attempts-used="1"' in rendered_html + assert answer_key in rendered_html + + def test_batch_renders_persisted_item_bank_problem_state(self): + """ + A problem the learner was assigned from a randomized bank (item_bank / + library_content) is loaded by the bank through the shared runtime, not by this + endpoint, so it must still see the learner's saved state. Otherwise it renders as + unanswered, and resubmitting fails with "The state of this problem has changed". + """ + bank = BlockFactory.create(parent_location=self.unit.location, category='itembank', max_count=1) + problem = self._create_problem(bank.location) + + # First visit assigns and persists the learner's selection, as it would in courseware. + assert 'data-attempts-used="0"' in self._render_single_child(bank) + self._submit_correct_answer(problem) + + rendered_html = self._render_single_child(bank) + assert 'data-attempts-used="1"' in rendered_html + + def test_batch_requires_authentication(self): + self.client.logout() + response = self.client.get(self.url, {'child_usage_keys': self._child_keys_param(self.children)}) + assert response.status_code in (401, 403) + + def test_missing_child_usage_keys_is_a_400(self): + response = self.client.get(self.url) + assert response.status_code == 400 + + def test_invalid_child_usage_key_is_a_400(self): + response = self.client.get(self.url, {'child_usage_keys': 'not-a-usage-key'}) + assert response.status_code == 400 + + def test_oversized_batch_is_a_400(self): + with override_settings(XBLOCK_CHILDREN_BATCH_MAX=2): + response = self.client.get(self.url, {'child_usage_keys': self._child_keys_param(self.children)}) + assert response.status_code == 400 + + def test_child_not_under_this_parent_is_forbidden(self): + # A learner must not be able to pull HTML for a block that isn't among this + # parent's own children -- this is the batch endpoint's security boundary. + # Use parent_location=... (not parent=...) for these scaffolding blocks: passing + # the live object would mutate self.chapter's in-memory .children, leaking into + # other tests -- see SequenceApiTestViews.test_hidden_after_due for the same note. + other_sequence = BlockFactory(parent_location=self.chapter.location, category='sequential') + other_unit = BlockFactory(parent_location=other_sequence.location, category='vertical') + stray_child = BlockFactory(parent_location=other_unit.location, category='html', display_name='stray') + + response = self.client.get(self.url, {'child_usage_keys': str(stray_child.location)}) + assert response.status_code == 200 + data = response.json() + assert not data['results'] + assert data['errors'][0]['usage_key'] == str(stray_child.location) + assert data['errors'][0]['error'] == 'forbidden' + + def test_partial_failure_still_returns_the_rest(self): + # One bad key in the batch shouldn't cost the learner the other, valid ones. + # parent_location=... (not parent=...) for the same reason as + # test_child_not_under_this_parent_is_forbidden above. + other_sequence = BlockFactory(parent_location=self.chapter.location, category='sequential') + other_unit = BlockFactory(parent_location=other_sequence.location, category='vertical') + stray_child = BlockFactory(parent_location=other_unit.location, category='html') + response = self.client.get( + self.url, + {'child_usage_keys': self._child_keys_param(self.children) + ',' + str(stray_child.location)}, + ) + assert response.status_code == 200 + data = response.json() + assert len(data['results']) == len(self.children) + assert len(data['errors']) == 1 + assert data['errors'][0]['usage_key'] == str(stray_child.location) + + def test_parent_with_no_children_forbids_every_requested_key(self): + # A leaf block has no get_children() of its own, so nothing is in its allowed set. + response = self.client.get( + f'/api/courseware/xblock_children/{self.children[0].location}', + {'child_usage_keys': self._child_keys_param(self.children[1:])}, + ) + assert response.status_code == 200 + data = response.json() + assert not data['results'] + assert len(data['errors']) == len(self.children) - 1 + assert all(error['error'] == 'forbidden' for error in data['errors']) + + class ResumeApiTestViews(BaseCoursewareTests, CompletionWaffleTestMixin): """ Tests for the resume API diff --git a/openedx/core/djangoapps/courseware_api/urls.py b/openedx/core/djangoapps/courseware_api/urls.py index 12fdf8cff2b4..a5060dd729f2 100644 --- a/openedx/core/djangoapps/courseware_api/urls.py +++ b/openedx/core/djangoapps/courseware_api/urls.py @@ -15,6 +15,9 @@ re_path(fr'^sequence/{settings.USAGE_KEY_PATTERN}', views.SequenceMetadata.as_view(), name="sequence-api"), + re_path(fr'^xblock_children/{settings.USAGE_KEY_PATTERN}', + views.XBlockChildren.as_view(), + name="xblock-children-api"), re_path(fr'^resume/{settings.COURSE_KEY_PATTERN}', views.Resume.as_view(), name="resume-api"), diff --git a/openedx/core/djangoapps/courseware_api/views.py b/openedx/core/djangoapps/courseware_api/views.py index 1b0bd0c50f5f..db22483bd056 100644 --- a/openedx/core/djangoapps/courseware_api/views.py +++ b/openedx/core/djangoapps/courseware_api/views.py @@ -13,7 +13,7 @@ from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey, UsageKey from rest_framework import status -from rest_framework.exceptions import NotFound +from rest_framework.exceptions import NotFound, ValidationError from rest_framework.generics import RetrieveAPIView from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response @@ -34,7 +34,7 @@ from lms.djangoapps.course_goals.models import UserActivity from lms.djangoapps.courseware.access import has_access from lms.djangoapps.courseware.access_utils import check_public_access -from lms.djangoapps.courseware.block_render import get_block_by_usage_id +from lms.djangoapps.courseware.block_render import get_block_by_usage_id, render_xblock_children from lms.djangoapps.courseware.context_processor import user_timezone_locale_prefs from lms.djangoapps.courseware.courses import ( get_course_about_section, @@ -42,6 +42,7 @@ get_permission_for_course_about, ) from lms.djangoapps.courseware.entrance_exams import course_has_entrance_exam, user_has_passed_entrance_exam +from lms.djangoapps.courseware.exceptions import CourseAccessRedirect from lms.djangoapps.courseware.masquerade import ( is_masquerading_as_non_audit_enrollment, is_masquerading_as_specific_student, @@ -821,6 +822,75 @@ def get(self, request, usage_key_string, *args, **kwargs): # pylint: disable=un return Response(sequence.get_metadata(view=view, context=context)) +class XBlockChildren(DeveloperErrorViewMixin, APIView): + """ + **Use Cases** + + Request a bounded batch of a unit's not-yet-rendered children, rendered as HTML. + Backs the incremental assessment load: VerticalBlock leaves placeholders for + children past its eager-render window, and the page's own JS calls this endpoint + repeatedly, in sequential batches, to fill them in. + + **Example Requests** + + GET /api/courseware/xblock_children/{usage_key}?child_usage_keys=key1,key2 + + **Response Values** + + Body consists of the following fields: + * parent_usage_key: the usage key passed in + * results: list of {usage_key, html} for children that rendered + * errors: list of {usage_key, error, message} for children that didn't + + **Returns** + + * 200 on success, including partial success (see `errors`). + * 400 if child_usage_keys is missing, malformed, or larger than + settings.XBLOCK_CHILDREN_BATCH_MAX. + * 404 if the parent is not available or not visible to this learner. + """ + + authentication_classes = ( + JwtAuthentication, + SessionAuthenticationAllowInactiveUser, + ) + permission_classes = (IsAuthenticated,) + + def get(self, request, usage_key_string, *args, **kwargs): # lint-amnesty, pylint: disable=unused-argument + """ + Return response to a GET request. + """ + try: + parent_usage_key = UsageKey.from_string(usage_key_string) + except InvalidKeyError as exc: + raise NotFound(f"Invalid usage key: '{usage_key_string}'.") from exc + + child_usage_keys_param = request.GET.get('child_usage_keys', '') + try: + child_usage_keys = [ + UsageKey.from_string(key) for key in child_usage_keys_param.split(',') if key + ] + except InvalidKeyError as exc: + raise ValidationError(f"Invalid child usage key in '{child_usage_keys_param}'.") from exc + + try: + course = get_course_with_access( + request.user, + 'load', + parent_usage_key.course_key, + check_if_enrolled=True, + ) + except CourseAccessRedirect as exc: + raise NotFound() from exc + + try: + result = render_xblock_children(request, parent_usage_key, child_usage_keys, course=course) + except ValueError as exc: + raise ValidationError(str(exc)) from exc + + return Response(result) + + class Resume(DeveloperErrorViewMixin, APIView): """ **Use Cases** diff --git a/xmodule/vertical_block.py b/xmodule/vertical_block.py index 5ca26088f3a6..8517f8f8741e 100644 --- a/xmodule/vertical_block.py +++ b/xmodule/vertical_block.py @@ -99,17 +99,29 @@ def _student_or_public_view(self, context, view): # pylint: disable=too-many-st child_blocks = self.get_children() # pylint: disable=no-member + # Large assessment units render only the first `incremental_load_eager_count` + # children here; the rest are left as placeholders (`lazy_children`) for the page's + # own JS to fill in via batched calls to XBlockChildren. Absent that context key, + # every child renders inline, exactly as before. + eager_count = (context or {}).get('incremental_load_eager_count') + if eager_count is not None and eager_count < len(child_blocks): + eager_blocks = child_blocks[:eager_count] + lazy_blocks = child_blocks[eager_count:] + else: + eager_blocks = child_blocks + lazy_blocks = [] + child_blocks_to_complete_on_view = set() completion_service = self.runtime.service(self, 'completion') if completion_service and completion_service.completion_tracking_enabled(): - child_blocks_to_complete_on_view = completion_service.blocks_to_mark_complete_on_view(child_blocks) + child_blocks_to_complete_on_view = completion_service.blocks_to_mark_complete_on_view(eager_blocks) complete_on_view_delay = completion_service.get_complete_on_view_delay_ms() child_context['child_of_vertical'] = True is_child_of_vertical = context.get('child_of_vertical', False) # pylint: disable=no-member - for child in child_blocks: + for child in eager_blocks: child_has_access_error = self.block_has_access_error(child) if context.get('hide_access_error_blocks') and child_has_access_error: continue @@ -141,6 +153,8 @@ def _student_or_public_view(self, context, view): # pylint: disable=too-many-st cta_service = self.runtime.service(self, 'call_to_action') vertical_banner_ctas = cta_service.get_ctas(self, 'vertical_banner', completed) if cta_service else [] + lazy_children = [str(child.location) for child in lazy_blocks] + fragment_context = { 'items': contents, 'xblock_context': context, @@ -151,6 +165,8 @@ def _student_or_public_view(self, context, view): # pylint: disable=too-many-st 'has_assignments': completed is not None, 'subsection_format': context.get('format', ''), 'vertical_banner_ctas': vertical_banner_ctas, + 'lazy_children': lazy_children, + 'parent_usage_key': str(self.location) if lazy_children else None, } if view == STUDENT_VIEW: