fix: remove the branch/version while building BS (#37866)

This commit updates the logic in the build_block_structure function to
ensure that block locations are consistently normalized by removing
branch and version information. This change addresses issues when
creating a BlockStructure from modulestore using the published_only
branch. Without this change, we end up comparing versioned keys to
unversioned ones later on, which always yields False.

Author: marslanabdulrauf

openedx/core/djangoapps/content/block_structure/factory.py

@@ -31,7 +31,8 @@ def create_from_modulestore(cls, root_block_usage_key, modulestore):
             xmodule.modulestore.exceptions.ItemNotFoundError if a block for
                 root_block_usage_key is not found in the modulestore.
         """
-        block_structure = BlockStructureModulestoreData(root_block_usage_key)
+        root_xblock = modulestore.get_item(root_block_usage_key, depth=None, lazy=False)
+        block_structure = BlockStructureModulestoreData(root_block_usage_key.for_branch(None))
         blocks_visited = set()
 
         def build_block_structure(xblock):
@@ -41,19 +42,26 @@ def build_block_structure(xblock):
             """
             # Check if the xblock was already visited (can happen in
             # DAGs).
-            if xblock.location in blocks_visited:
+            # Normalize location to remove branch/version information
+            # When create_from_modulestore is wrapped in published_only branch decorator,
+            # "xblock being changed" location contains branch and version info which causes
+            # mismatch when removing inaccessible blocks in
+            # CourseNavigationBlocksView.filter_inaccessible_blocks
+            # while fetching course navigation.
+            location = xblock.location.for_branch(None)
+            if location in blocks_visited:
                 return
 
             # Add the xBlock.
-            blocks_visited.add(xblock.location)
-            block_structure._add_xblock(xblock.location, xblock)  # pylint: disable=protected-access
+            blocks_visited.add(location)
+            block_structure._add_xblock(location, xblock)  # pylint: disable=protected-access
 
             # Add relations with its children and recurse.
             for child in xblock.get_children():
-                block_structure._add_relation(xblock.location, child.location)  # pylint: disable=protected-access
+                child_location = child.location.for_branch(None)
+                block_structure._add_relation(location, child_location)  # pylint: disable=protected-access
                 build_block_structure(child)
 
-        root_xblock = modulestore.get_item(root_block_usage_key, depth=None, lazy=False)
         build_block_structure(root_xblock)
         return block_structure
 

openedx/core/djangoapps/content/block_structure/tests/test_factory.py

@@ -6,12 +6,13 @@
 from django.test import TestCase
 
 from opaque_keys.edx.keys import CourseKey
+from opaque_keys.edx.locator import BlockUsageLocator, CourseLocator
 from xmodule.modulestore.exceptions import ItemNotFoundError
 
 from ..exceptions import BlockStructureNotFound
 from ..factory import BlockStructureFactory
 from ..store import BlockStructureStore
-from .helpers import ChildrenMapTestMixin, MockCache, MockModulestoreFactory
+from .helpers import ChildrenMapTestMixin, MockCache, MockModulestoreFactory, MockXBlock, MockModulestore
 
 
 class TestBlockStructureFactory(TestCase, ChildrenMapTestMixin):
@@ -77,3 +78,78 @@ def test_new(self):
             block_structure._block_data_map,  # pylint: disable=protected-access
         )
         self.assert_block_structure(new_structure, self.children_map)
+
+    def test_from_modulestore_normalizes_locations_with_branch_info(self):
+        """
+        Test that locations with branch/version information are normalized
+        when building block structures.
+
+        This test verifies the fix for PR #37866, which ensures that when
+        creating block structures within the published_only branch context,
+        locations are normalized by removing branch/version information.
+        This prevents comparison mismatches when filtering inaccessible blocks.
+
+        Without the fix, locations with branch info would be stored as-is,
+        causing issues when comparing with normalized locations later.
+        """
+        # Create a course key with branch information to simulate
+        # the published_only branch context
+        course_key_with_branch = CourseLocator('org', 'course', 'run', branch='published')
+        root_usage_key = BlockUsageLocator(
+            course_key=course_key_with_branch,
+            block_type='html',
+            block_id='0'
+        )
+
+        # Create a modulestore with xblocks that have locations containing branch info
+        modulestore = MockModulestore()
+        blocks = {}
+        children_map = self.SIMPLE_CHILDREN_MAP
+
+        # Create blocks with branch information in their locations
+        for block_id, children in enumerate(children_map):
+            # Create location with branch info
+            block_location = BlockUsageLocator(
+                course_key=course_key_with_branch,
+                block_type='html',
+                block_id=str(block_id)
+            )
+            # Create child locations with branch info
+            child_locations = [
+                BlockUsageLocator(
+                    course_key=course_key_with_branch,
+                    block_type='html',
+                    block_id=str(child_id)
+                )
+                for child_id in children
+            ]
+            blocks[block_location] = MockXBlock(
+                location=block_location,
+                children=child_locations,
+                modulestore=modulestore
+            )
+        modulestore.set_blocks(blocks)
+
+        # Build block structure from modulestore
+        block_structure = BlockStructureFactory.create_from_modulestore(
+            root_block_usage_key=root_usage_key,
+            modulestore=modulestore
+        )
+
+        # Verify that all stored block keys are normalized (without branch info)
+        # This is the key assertion: with the fix, all keys should be normalized
+        for block_key in block_structure:
+            # The block_key should equal its normalized version
+            normalized_key = block_key.for_branch(None)
+            self.assertEqual(
+                block_key,
+                normalized_key,
+                f"Block key {block_key} should be normalized (without branch info). "
+                f"Normalized version: {normalized_key}"
+            )
+            # Verify it doesn't have branch information in the course_key
+            if hasattr(block_key.course_key, 'branch'):
+                self.assertIsNone(
+                    block_key.course_key.branch,
+                    f"Block key {block_key} should not have branch information"
+                )