refactor: update sync model helper function docs and minor cleanup

cms/djangoapps/contentstore/models.py

@@ -232,25 +232,15 @@ def update_or_create(
             'version_declined': version_declined,
         }
         if upstream_block:
-            new_values.update(
-                {
-                    'upstream_block': upstream_block,
-                }
-            )
+            new_values['upstream_block'] = upstream_block
         try:
             link = cls.objects.get(downstream_usage_key=downstream_usage_key)
-            # TODO: until we save modified datetime for course xblocks in index, the modified time for links are updated
-            # everytime a downstream/course block is updated. This allows us to order links[1] based on recently
-            # modified downstream version.
-            # pylint: disable=line-too-long
-            # 1. https://github.com/open-craft/frontend-app-course-authoring/blob/0443d88824095f6f65a3a64b77244af590d4edff/src/course-libraries/ReviewTabContent.tsx#L222-L233
-            has_changes = True  # change to false once above condition is met.
-            for key, value in new_values.items():
-                prev = getattr(link, key)
-                # None != None is True, so we need to check for it specially
-                if prev != value and ~(prev is None and value is None):
+            has_changes = False
+            for key, new_value in new_values.items():
+                prev_value = getattr(link, key)
+                if prev_value != new_value:
                     has_changes = True
-                    setattr(link, key, value)
+                    setattr(link, key, new_value)
             if has_changes:
                 link.updated = created
                 link.save()
@@ -317,25 +307,15 @@ def update_or_create(
             'version_declined': version_declined,
         }
         if upstream_container:
-            new_values.update(
-                {
-                    'upstream_container': upstream_container,
-                }
-            )
+            new_values['upstream_container'] = upstream_container
         try:
             link = cls.objects.get(downstream_usage_key=downstream_usage_key)
-            # TODO: until we save modified datetime for course xblocks in index, the modified time for links are updated
-            # everytime a downstream/course block is updated. This allows us to order links[1] based on recently
-            # modified downstream version.
-            # pylint: disable=line-too-long
-            # 1. https://github.com/open-craft/frontend-app-course-authoring/blob/0443d88824095f6f65a3a64b77244af590d4edff/src/course-libraries/ReviewTabContent.tsx#L222-L233
-            has_changes = True  # change to false once above condition is met.
-            for key, value in new_values.items():
-                prev = getattr(link, key)
-                # None != None is True, so we need to check for it specially
-                if prev != value and ~(prev is None and value is None):
+            has_changes = False
+            for key, new_value in new_values.items():
+                prev_value = getattr(link, key)
+                if prev_value != new_value:
                     has_changes = True
-                    setattr(link, key, value)
+                    setattr(link, key, new_value)
             if has_changes:
                 link.updated = created
                 link.save()

cms/djangoapps/contentstore/utils.py

@@ -2429,8 +2429,5 @@ def create_or_update_xblock_upstream_link(xblock, course_key: CourseKey, created
         _create_or_update_component_link(course_key, created, xblock)
     except InvalidKeyError:
         # It is possible that the upstream is a container and UsageKeyV2 parse failed
-        # Create upstream container link
-        try:
-            _create_or_update_container_link(course_key, created, xblock)
-        except InvalidKeyError:
-            log.error(f"Invalid key: {xblock.upstream}")
+        # Create upstream container link and raise InvalidKeyError if xblock.upstream is a valid key.
+        _create_or_update_container_link(course_key, created, xblock)

cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py

@@ -548,8 +548,7 @@ def sync_library_content(downstream: XBlock, request, store) -> StaticFileNotice
             notices = []
             # Store final children keys to update order of components in unit
             children = []
-            for i in range(len(upstream_children)):
-                upstream_child = upstream_children[i]
+            for i, upstream_child in enumerate(upstream_children):
                 assert isinstance(upstream_child, LibraryXBlockMetadata)  # for now we only support units
                 if upstream_child.usage_key not in downstream_children_keys:
                     # This upstream_child is new, create it.
@@ -574,6 +573,7 @@ def sync_library_content(downstream: XBlock, request, store) -> StaticFileNotice
             for child in downstream_children:
                 if child.usage_key not in children:
                     # This downstream block was added, or deleted from upstream block.
+                    # NOTE: This will also delete any local additions to a unit in the next upstream sync.
                     store.delete_item(child.usage_key, user_id=request.user.id)
             downstream.children = children
             store.update_item(downstream, request.user.id)

openedx/core/djangoapps/content/search/documents.py

@@ -212,6 +212,8 @@ class implementation returns only:
         Fields.access_id: _meili_access_id_from_context_key(block.usage_key.context_key),
         Fields.breadcrumbs: [],
     }
+    if hasattr(block, "edited_on"):
+        block_data[Fields.modified] = block.edited_on.timestamp()
     # Get the breadcrumbs (course, section, subsection, etc.):
     if block.usage_key.context_key.is_course:  # Getting parent is not yet implemented in Learning Core (for libraries).
         cur_block = block

openedx/core/djangoapps/content/search/tests/test_api.py

@@ -61,19 +61,27 @@ def setUp(self):
         # Clear the Meilisearch client to avoid side effects from other tests
         api.clear_meilisearch_client()
 
+        modified_date = datetime(2024, 5, 6, 7, 8, 9, tzinfo=timezone.utc)
         # Create course
-        self.course = self.store.create_course(
-            "org1",
-            "test_course",
-            "test_run",
-            self.user_id,
-            fields={"display_name": "Test Course"},
-        )
-        course_access, _ = SearchAccess.objects.get_or_create(context_key=self.course.id)
-        self.course_block_key = "block-v1:org1+test_course+test_run+type@course+block@course"
-
-        # Create XBlocks
-        self.sequential = self.store.create_child(self.user_id, self.course.location, "sequential", "test_sequential")
+        with freeze_time(modified_date):
+            self.course = self.store.create_course(
+                "org1",
+                "test_course",
+                "test_run",
+                self.user_id,
+                fields={"display_name": "Test Course"},
+            )
+            course_access, _ = SearchAccess.objects.get_or_create(context_key=self.course.id)
+            self.course_block_key = "block-v1:org1+test_course+test_run+type@course+block@course"
+
+            # Create XBlocks
+            self.sequential = self.store.create_child(
+                self.user_id,
+                self.course.location,
+                "sequential",
+                "test_sequential"
+            )
+            self.store.create_child(self.user_id, self.sequential.location, "vertical", "test_vertical")
         self.doc_sequential = {
             "id": "block-v1org1test_coursetest_runtypesequentialblocktest_sequential-f702c144",
             "type": "course_block",
@@ -90,8 +98,8 @@ def setUp(self):
             ],
             "content": {},
             "access_id": course_access.id,
+            "modified": modified_date.timestamp(),
         }
-        self.store.create_child(self.user_id, self.sequential.location, "vertical", "test_vertical")
         self.doc_vertical = {
             "id": "block-v1org1test_coursetest_runtypeverticalblocktest_vertical-e76a10a4",
             "type": "course_block",
@@ -112,6 +120,7 @@ def setUp(self):
             ],
             "content": {},
             "access_id": course_access.id,
+            "modified": modified_date.timestamp(),
         }
         # Make sure the CourseOverview for the course is created:
         CourseOverview.get_from_id(self.course.id)
@@ -130,7 +139,6 @@ def setUp(self):
             self.problem1 = library_api.create_library_block(self.library.key, "problem", "p1")
             self.problem2 = library_api.create_library_block(self.library.key, "problem", "p2")
         # Update problem1, freezing the date so we can verify modified date serializes correctly.
-        modified_date = datetime(2024, 5, 6, 7, 8, 9, tzinfo=timezone.utc)
         with freeze_time(modified_date):
             library_api.set_library_block_olx(self.problem1.usage_key, "<problem />")
 

openedx/core/djangoapps/content/search/tests/test_documents.py

@@ -52,23 +52,23 @@ class StudioDocumentsTest(SharedModuleStoreTestCase):
     def setUpClass(cls):
         super().setUpClass()
         cls.store = modulestore()
-        cls.org = Organization.objects.create(name="edX", short_name="edX")
-        cls.toy_course = ToyCourseFactory.create()  # See xmodule/modulestore/tests/sample_courses.py
-        cls.toy_course_key = cls.toy_course.id
-
-        # Get references to some blocks in the toy course
-        cls.html_block_key = cls.toy_course_key.make_usage_key("html", "toyjumpto")
-        # Create a problem in course
-        cls.problem_block = BlockFactory.create(
-            category="problem",
-            parent_location=cls.toy_course_key.make_usage_key("vertical", "vertical_test"),
-            display_name='Test Problem',
-            data="<problem>What is a test?<multiplechoiceresponse></multiplechoiceresponse></problem>",
-        )
-
         # Create a library and collection with a block
-        created_date = datetime(2023, 4, 5, 6, 7, 8, tzinfo=timezone.utc)
-        with freeze_time(created_date):
+        cls.created_date = datetime(2023, 4, 5, 6, 7, 8, tzinfo=timezone.utc)
+        with freeze_time(cls.created_date):
+            # Get references to some blocks in the toy course
+            cls.org = Organization.objects.create(name="edX", short_name="edX")
+            cls.toy_course = ToyCourseFactory.create()  # See xmodule/modulestore/tests/sample_courses.py
+            cls.toy_course_key = cls.toy_course.id
+
+            cls.html_block_key = cls.toy_course_key.make_usage_key("html", "toyjumpto")
+            # Create a problem in course
+            cls.problem_block = BlockFactory.create(
+                category="problem",
+                parent_location=cls.toy_course_key.make_usage_key("vertical", "vertical_test"),
+                display_name='Test Problem',
+                data="<problem>What is a test?<multiplechoiceresponse></multiplechoiceresponse></problem>",
+            )
+
             cls.library = library_api.create_library(
                 org=cls.org,
                 slug="2012_Fall",
@@ -190,6 +190,7 @@ def test_problem_block(self):
                     'usage_key': 'block-v1:edX+toy+2012_Fall+type@vertical+block@vertical_test',
                 },
             ],
+            "modified": self.created_date.timestamp(),
             "content": {
                 "capa_content": "What is a test?",
                 "problem_types": ["multiplechoiceresponse"],
@@ -223,6 +224,7 @@ def test_html_block(self):
             "display_name": "Text",
             "description": "This is a link to another page and some Chinese 四節比分和七年前 Some "
                          "more Chinese 四節比分和七年前 ",
+            "modified": self.created_date.timestamp(),
             "breadcrumbs": [
                 {
                     'display_name': 'Toy Course',
@@ -276,6 +278,7 @@ def test_video_block_untagged(self):
                 },
             ],
             "content": {},
+            "modified": self.created_date.timestamp(),
             # This video has no tags.
         }
 

openedx/core/djangoapps/content/search/tests/test_handlers.py

@@ -59,7 +59,9 @@ def test_create_delete_xblock(self, meilisearch_client):
         course_access, _ = SearchAccess.objects.get_or_create(context_key=course.id)
 
         # Create XBlocks
-        sequential = self.store.create_child(self.user_id, course.location, "sequential", "test_sequential")
+        created_date = datetime(2023, 4, 5, 6, 7, 8, tzinfo=timezone.utc)
+        with freeze_time(created_date):
+            sequential = self.store.create_child(self.user_id, course.location, "sequential", "test_sequential")
         doc_sequential = {
             "id": "block-v1orgatest_coursetest_runtypesequentialblocktest_sequential-0cdb9395",
             "type": "course_block",
@@ -76,10 +78,11 @@ def test_create_delete_xblock(self, meilisearch_client):
             ],
             "content": {},
             "access_id": course_access.id,
-
+            "modified": created_date.timestamp(),
         }
         meilisearch_client.return_value.index.return_value.update_documents.assert_called_with([doc_sequential])
-        vertical = self.store.create_child(self.user_id, sequential.location, "vertical", "test_vertical")
+        with freeze_time(created_date):
+            vertical = self.store.create_child(self.user_id, sequential.location, "vertical", "test_vertical")
         doc_vertical = {
             "id": "block-v1orgatest_coursetest_runtypeverticalblocktest_vertical-011f143b",
             "type": "course_block",
@@ -100,18 +103,22 @@ def test_create_delete_xblock(self, meilisearch_client):
             ],
             "content": {},
             "access_id": course_access.id,
+            "modified": created_date.timestamp(),
         }
 
         meilisearch_client.return_value.index.return_value.update_documents.assert_called_with([doc_vertical])
 
         # Update the XBlock
         sequential = self.store.get_item(sequential.location, self.user_id)  # Refresh the XBlock
         sequential.display_name = "Updated Sequential"
-        self.store.update_item(sequential, self.user_id)
+        modified_date = datetime(2024, 5, 6, 7, 8, 9, tzinfo=timezone.utc)
+        with freeze_time(modified_date):
+            self.store.update_item(sequential, self.user_id)
 
         # The display name and the child's breadcrumbs should be updated
         doc_sequential["display_name"] = "Updated Sequential"
         doc_vertical["breadcrumbs"][1]["display_name"] = "Updated Sequential"
+        doc_sequential["modified"] = modified_date.timestamp()
         meilisearch_client.return_value.index.return_value.update_documents.assert_called_with([
             doc_sequential,
             doc_vertical,