diff --git a/cms/djangoapps/contentstore/views/component.py b/cms/djangoapps/contentstore/views/component.py index 181fc223d6..0da0a25e82 100644 --- a/cms/djangoapps/contentstore/views/component.py +++ b/cms/djangoapps/contentstore/views/component.py @@ -176,7 +176,7 @@ def container_handler(request, usage_key_string): # Fetch the XBlock info for use by the container page. Note that it includes information # about the block's ancestors and siblings for use by the Unit Outline. - xblock_info = create_xblock_info(xblock, include_ancestor_info=is_unit_page, include_edited_by=True, include_published_by=True) + xblock_info = create_xblock_info(xblock, include_ancestor_info=is_unit_page) # Create the link for preview. preview_lms_base = settings.FEATURES.get('PREVIEW_LMS_BASE') diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 2adb4d3ff7..d1e204e191 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -239,6 +239,7 @@ def _course_outline_json(request, course_key): return create_xblock_info( course_module, include_child_info=True, + course_outline=True, include_children_predicate=lambda xblock: not xblock.category == 'vertical' ) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 1c1b0dd75a..3c2066c779 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -286,6 +286,7 @@ def xblock_outline_handler(request, usage_key_string): return JsonResponse(create_xblock_info( root_xblock, include_child_info=True, + course_outline=True, include_children_predicate=lambda xblock: not xblock.category == 'vertical' )) else: @@ -587,16 +588,18 @@ def _get_module_info(xblock, rewrite_static_links=True): def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=False, include_child_info=False, - include_edited_by=False, include_published_by=False, include_children_predicate=NEVER): + course_outline=False, include_children_predicate=NEVER): """ Creates the information needed for client-side XBlockInfo. If data or metadata are not specified, their information will not be added (regardless of whether or not the xblock actually has data or metadata). - There are two optional boolean parameters: + There are three optional boolean parameters: include_ancestor_info - if true, ancestor info is added to the response include_child_info - if true, direct child info is included in the response + course_outline - if true, the xblock is being rendered on behalf of the course outline. + There are certain expensive computations that do not need to be included in this case. In addition, an optional include_children_predicate argument can be provided to define whether or not a particular xblock should have its children included. @@ -622,7 +625,9 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F # Compute the child info first so it can be included in aggregate information for the parent if include_child_info and xblock.has_children: child_info = _create_xblock_child_info( - xblock, include_children_predicate=include_children_predicate + xblock, + course_outline, + include_children_predicate=include_children_predicate ) else: child_info = None @@ -630,7 +635,6 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F # Treat DEFAULT_START_DATE as a magic number that means the release date has not been set release_date = get_default_time_display(xblock.start) if xblock.start != DEFAULT_START_DATE else None published = modulestore().has_item(xblock.location, revision=ModuleStoreEnum.RevisionOption.published_only) - currently_visible_to_students = is_currently_visible_to_students(xblock) is_xblock_unit = is_unit(xblock) is_unit_with_changes = is_xblock_unit and modulestore().has_changes(xblock.location) @@ -645,8 +649,6 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F 'studio_url': xblock_studio_url(xblock), "released_to_students": datetime.now(UTC) > xblock.start, "release_date": release_date, - "release_date_from": _get_release_date_from(xblock) if release_date else None, - "currently_visible_to_students": currently_visible_to_students, "visibility_state": _compute_visibility_state(xblock, child_info, is_unit_with_changes) if not xblock.category == 'course' else None } if data is not None: @@ -654,19 +656,19 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F if metadata is not None: xblock_info["metadata"] = metadata if include_ancestor_info: - xblock_info['ancestor_info'] = _create_xblock_ancestor_info(xblock) + xblock_info['ancestor_info'] = _create_xblock_ancestor_info(xblock, course_outline) if child_info: xblock_info['child_info'] = child_info - # Currently, 'edited_by' and 'published_by' are only used by the container page. Only compute them when asked to do - # so, since safe_get_username() is expensive. - if include_edited_by: - xblock_info['edited_by'] = safe_get_username(xblock.subtree_edited_by) - if include_published_by: - xblock_info['published_by'] = safe_get_username(xblock.published_by) - # On the unit page only, add 'has_changes' to indicate when there are changes that can be discarded. - # We don't add it in general because it is an expensive operation. - if is_xblock_unit: + # Currently, 'edited_by', 'published_by', and 'release_date_from', and 'has_changes' are only used by the + # container page when rendering a unit. Since they are expensive to compute, only include them for units + # that are not being rendered on the course outline. + if is_xblock_unit and not course_outline: + xblock_info["edited_by"] = safe_get_username(xblock.subtree_edited_by) + xblock_info["published_by"] = safe_get_username(xblock.published_by) + xblock_info["currently_visible_to_students"] = is_currently_visible_to_students(xblock) xblock_info['has_changes'] = is_unit_with_changes + if release_date: + xblock_info["release_date_from"] = _get_release_date_from(xblock) return xblock_info @@ -740,7 +742,7 @@ def _compute_visibility_state(xblock, child_info, is_unit_with_changes): return VisibilityState.ready -def _create_xblock_ancestor_info(xblock): +def _create_xblock_ancestor_info(xblock, course_outline): """ Returns information about the ancestors of an xblock. Note that the direct parent will also return information about all of its children. @@ -756,6 +758,7 @@ def _create_xblock_ancestor_info(xblock): ancestors.append(create_xblock_info( ancestor, include_child_info=include_child_info, + course_outline=course_outline, include_children_predicate=direct_children_only )) collect_ancestor_info(get_parent_xblock(ancestor)) @@ -765,7 +768,7 @@ def _create_xblock_ancestor_info(xblock): } -def _create_xblock_child_info(xblock, include_children_predicate=NEVER): +def _create_xblock_child_info(xblock, course_outline, include_children_predicate=NEVER): """ Returns information about the children of an xblock, as well as about the primary category of xblock expected as children. @@ -780,7 +783,8 @@ def _create_xblock_child_info(xblock, include_children_predicate=NEVER): if xblock.has_children and include_children_predicate(xblock): child_info['children'] = [ create_xblock_info( - child, include_child_info=True, include_children_predicate=include_children_predicate + child, include_child_info=True, course_outline=course_outline, + include_children_predicate=include_children_predicate ) for child in xblock.get_children() ] return child_info diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index d33ffc498b..594cea06ca 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -1108,7 +1108,7 @@ class TestXBlockInfo(ItemTest): outline_url = reverse_usage_url('xblock_outline_handler', self.usage_key) resp = self.client.get(outline_url, HTTP_ACCEPT='application/json') json_response = json.loads(resp.content) - self.validate_course_xblock_info(json_response) + self.validate_course_xblock_info(json_response, course_outline=True) def test_chapter_xblock_info(self): chapter = modulestore().get_item(self.chapter.location) @@ -1133,7 +1133,6 @@ class TestXBlockInfo(ItemTest): xblock_info = create_xblock_info( vertical, include_child_info=True, - include_edited_by=True, include_children_predicate=ALWAYS, include_ancestor_info=True ) @@ -1148,7 +1147,7 @@ class TestXBlockInfo(ItemTest): ) self.validate_component_xblock_info(xblock_info) - def validate_course_xblock_info(self, xblock_info, has_child_info=True): + def validate_course_xblock_info(self, xblock_info, has_child_info=True, course_outline=False): """ Validate that the xblock info is correct for the test course. """ @@ -1158,7 +1157,7 @@ class TestXBlockInfo(ItemTest): self.assertTrue(xblock_info['published']) # Finally, validate the entire response for consistency - self.validate_xblock_info_consistency(xblock_info, has_child_info=has_child_info) + self.validate_xblock_info_consistency(xblock_info, has_child_info=has_child_info, course_outline=course_outline) def validate_chapter_xblock_info(self, xblock_info, has_child_info=True): """ @@ -1168,18 +1167,20 @@ class TestXBlockInfo(ItemTest): self.assertEqual(xblock_info['id'], 'i4x://MITx/999/chapter/Week_1') self.assertEqual(xblock_info['display_name'], 'Week 1') self.assertTrue(xblock_info['published']) + self.assertIsNone(xblock_info.get('edited_by', None)) # Finally, validate the entire response for consistency self.validate_xblock_info_consistency(xblock_info, has_child_info=has_child_info) def validate_sequential_xblock_info(self, xblock_info, has_child_info=True): """ - Validate that the xblock info is correct for the test chapter. + Validate that the xblock info is correct for the test sequential. """ self.assertEqual(xblock_info['category'], 'sequential') self.assertEqual(xblock_info['id'], 'i4x://MITx/999/sequential/Lesson_1') self.assertEqual(xblock_info['display_name'], 'Lesson 1') self.assertTrue(xblock_info['published']) + self.assertIsNone(xblock_info.get('edited_by', None)) # Finally, validate the entire response for consistency self.validate_xblock_info_consistency(xblock_info, has_child_info=has_child_info) @@ -1204,7 +1205,7 @@ class TestXBlockInfo(ItemTest): self.validate_course_xblock_info(ancestors[2], has_child_info=False) # Finally, validate the entire response for consistency - self.validate_xblock_info_consistency(xblock_info, has_child_info=True, has_ancestor_info=True, has_edited_by=True) + self.validate_xblock_info_consistency(xblock_info, has_child_info=True, has_ancestor_info=True) def validate_component_xblock_info(self, xblock_info): """ @@ -1214,11 +1215,13 @@ class TestXBlockInfo(ItemTest): self.assertEqual(xblock_info['id'], 'i4x://MITx/999/video/My_Video') self.assertEqual(xblock_info['display_name'], 'My Video') self.assertTrue(xblock_info['published']) + self.assertIsNone(xblock_info.get('edited_by', None)) # Finally, validate the entire response for consistency self.validate_xblock_info_consistency(xblock_info) - def validate_xblock_info_consistency(self, xblock_info, has_ancestor_info=False, has_child_info=False, has_edited_by=False): + def validate_xblock_info_consistency(self, xblock_info, has_ancestor_info=False, has_child_info=False, + course_outline=False): """ Validate that the xblock info is internally consistent. """ @@ -1232,7 +1235,8 @@ class TestXBlockInfo(ItemTest): for ancestor in xblock_info['ancestor_info']['ancestors']: self.validate_xblock_info_consistency( ancestor, - has_child_info=(ancestor == ancestors[0]) # Only the direct ancestor includes children + has_child_info=(ancestor == ancestors[0]), # Only the direct ancestor includes children + course_outline=course_outline ) else: self.assertIsNone(xblock_info.get('ancestor_info', None)) @@ -1242,11 +1246,12 @@ class TestXBlockInfo(ItemTest): for child_response in xblock_info['child_info']['children']: self.validate_xblock_info_consistency( child_response, - has_child_info=(not child_response.get('child_info', None) is None) + has_child_info=(not child_response.get('child_info', None) is None), + course_outline=course_outline ) else: self.assertIsNone(xblock_info.get('child_info', None)) - if has_edited_by: + if xblock_info['category'] == 'vertical' and not course_outline: self.assertEqual(xblock_info['edited_by'], 'testuser') else: self.assertIsNone(xblock_info.get('edited_by', None))