fix: squashme: changes after merging ADR (WIP)

cms/djangoapps/contentstore/helpers.py

@@ -23,7 +23,7 @@
 from xmodule.xml_block import XmlMixin
 
 from cms.djangoapps.models.settings.course_grading import CourseGradingModel
-from cms.lib.xblock.upstream_sync import validate_upstream_key, UnsupportedUpstreamKeyType
+from cms.lib.xblock.upstream_sync import BadUpstream
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
 import openedx.core.djangoapps.content_staging.api as content_staging_api
 import openedx.core.djangoapps.content_tagging.api as content_tagging_api
@@ -387,13 +387,16 @@ def _import_xml_node_to_parent(
         temp_xblock.copied_from_block = copied_from_block
     if copied_from_block and link_to_upstream:
         # If requested, link this block as a downstream of where it was copied from
+        temp_xblock.upstream = copied_from_block
         try:
-            validate_upstream_key(copied_from_block)
-        except UnsupportedUpstreamKeyType:
-            pass  # @@TODO - should we let this error bubble up?
-        else:
-            temp_xblock.upstream = copied_from_block
-            temp_xblock.sync_from_upstream(user=user, apply_updates=False)
+            temp_xblock.runtime.service(
+                temp_xblock, 'upstream_sync'
+            ).sync_from_upstream(temp_xblock, apply_updates=False)
+        except BadUpstream as exc:
+            log.exception(
+                "Pasting content with link_to_upstream=True, but copied content is not a valid upstream. Will not link."
+            )
+            temp_xblock.upstream = None
 
     # Save the XBlock into modulestore. We need to save the block and its parent for this to work:
     new_xblock = store.update_item(temp_xblock, user.id, allow_not_found=True)

cms/djangoapps/contentstore/rest_api/v1/serializers/vertical_block.py

@@ -107,12 +107,11 @@ class UpstreamInfoSerializer(serializers.Serializer):
     """
     Serializer holding info for syncing a block with its upstream (eg, a library block).
     """
-    usage_key = serializers.CharField()
+    upstream_ref = serializers.CharField()
     current_version = serializers.IntegerField(allow_null=True)
     latest_version = serializers.IntegerField(allow_null=True)
-    sync_url = serializers.CharField()
-    error = serializers.CharField(allow_null=True)
-    sync_available = serializers.BooleanField()
+    warning = serializers.CharField(allow_null=True)
+    can_sync = serializers.BooleanField()
 
 
 class ChildVerticalContainerSerializer(serializers.Serializer):

cms/djangoapps/contentstore/rest_api/v1/views/vertical_block.py

@@ -2,6 +2,7 @@
 
 import logging
 import edx_api_doc_tools as apidocs
+from dataclasses import asdict
 from django.http import HttpResponseBadRequest
 from rest_framework.request import Request
 from rest_framework.response import Response
@@ -20,6 +21,7 @@
     ContainerHandlerSerializer,
     VerticalContainerSerializer,
 )
+from cms.lib.xblock.upstream_sync import BadUpstream
 from openedx.core.lib.api.view_utils import view_auth_classes
 from xmodule.modulestore.django import modulestore  # lint-amnesty, pylint: disable=wrong-import-order
 from xmodule.modulestore.exceptions import ItemNotFoundError  # lint-amnesty, pylint: disable=wrong-import-order
@@ -217,11 +219,10 @@ def get(self, request: Request, usage_key_string: str):
                     "user_partition_info": {},
                     "user_partitions": {}
                     "upstream_info": {
-                        "usage_key": "lb:org:mylib:video:404",
+                        "upstream_ref": "lb:org:mylib:video:404",
                         "current_version": 16
                         "latest_version": null,
-                        "sync_url": "http://...",
-                        "error": "Linked library item not found: lb:org:mylib:video:404",
+                        "warning": "Linked library item not found: lb:org:mylib:video:404",
                         "can_sync": false,
                     },
                     "actions": {
@@ -242,11 +243,10 @@ def get(self, request: Request, usage_key_string: str):
                     "user_partition_info": {},
                     "user_partitions": {},
                     "upstream_info": {
-                        "usage_key": "lb:org:mylib:html:abcd",
+                        "upstream_ref": "lb:org:mylib:html:abcd",
                         "current_version": 43,
                         "latest_version": 49,
-                        "sync_url": "http://...",
-                        "error": "null",
+                        "warning": null,
                         "can_sync": true,
                     },
                     "actions": {
@@ -282,6 +282,27 @@ def get(self, request: Request, usage_key_string: str):
             if hasattr(current_xblock, "children"):
                 for child in current_xblock.children:
                     child_info = modulestore().get_item(child)
+                    try:
+                        upstream_info = child_info.runtime.service(
+                            child_info, "upstream_sync"
+                        ).inspect_upstream(child_info)
+                    except BadUpstream as exc:
+                        upstream_info_json = {
+                            "upstream_ref": child_info.upstream,
+                            "current_version": None,
+                            "latest_version": None,
+                            "can_sync": False,
+                            "warning": str(exc),
+                        }
+                    else:
+                        upstream_info_json = {
+                            **asdict(upstream_info),
+                            "can_sync": (
+                                upstream_info.upstream and
+                                upstream_info.latest_version > upstream_info.current_version
+                            ),
+                            "warning": None,
+                        }
                     user_partition_info = get_visibility_partition_info(child_info, course=course)
                     user_partitions = get_user_partition_info(child_info, course=course)
                     validation_messages = get_xblock_validation_messages(child_info)
@@ -294,7 +315,7 @@ def get(self, request: Request, usage_key_string: str):
                         "block_type": child_info.location.block_type,
                         "user_partition_info": user_partition_info,
                         "user_partitions": user_partitions,
-                        "upstream_info": child_info.get_upstream_info_json(),
+                        "upstream_info": upstream_info_json,
                         "validation_messages": validation_messages,
                         "render_error": render_error,
                     })

cms/djangoapps/contentstore/utils.py

@@ -98,6 +98,7 @@
 )
 from cms.djangoapps.models.settings.course_grading import CourseGradingModel
 from cms.djangoapps.models.settings.course_metadata import CourseMetadata
+from cms.lib.xblock.upstream_sync import UpstreamSyncService
 from xmodule.library_tools import LibraryToolsService
 from xmodule.course_block import DEFAULT_START_DATE  # lint-amnesty, pylint: disable=wrong-import-order
 from xmodule.data import CertificatesDisplayBehaviors
@@ -1265,7 +1266,8 @@ def load_services_for_studio(runtime, user):
         "settings": SettingsService(),
         "lti-configuration": ConfigurationService(CourseAllowPIISharingInLTIFlag),
         "teams_configuration": TeamsConfigurationService(),
-        "library_tools": LibraryToolsService(modulestore(), user.id)
+        "library_tools": LibraryToolsService(modulestore(), user.id),
+        "upstream_sync": UpstreamSyncService(user),
     }
 
     runtime._services.update(services)  # lint-amnesty, pylint: disable=protected-access

cms/lib/xblock/test/test_upstream_sync.py

@@ -1,26 +1,14 @@
 """
 Test CMS's upstream->downstream syncing system
 """
-
-from django.conf import settings
-from django.test.utils import override_settings
 from organizations.api import ensure_organization
 from organizations.models import Organization
-from xblock.core import XBlock
 
+from cms.lib.xblock.upstream_sync import UpstreamSyncService
 from openedx.core.djangoapps.content_libraries import api as libs
 from openedx.core.djangoapps.xblock import api as xblock
-from xmodule.modulestore.tests.django_utils import TEST_DATA_SPLIT_MODULESTORE, ModuleStoreTestCase
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory
-from xmodule.partitions.partitions import (
-    ENROLLMENT_TRACK_PARTITION_ID,
-    MINIMUM_UNUSED_PARTITION_ID,
-    Group,
-    UserPartition
-)
-from xmodule.tests.test_export import PureXBlock
-
-from common.djangoapps.course_modes.tests.factories import CourseModeFactory
 
 
 class UpstreamSyncTestCase(ModuleStoreTestCase):
@@ -50,7 +38,7 @@ def setUp(self):
             display_name='Test Vertical'
         )
         ensure_organization("TestX")
-        upstream_key = libs.create_library_block(
+        self.upstream_key = libs.create_library_block(
             libs.create_library(
                 org=Organization.objects.get(short_name="TestX"),
                 slug="TestLib",
@@ -59,53 +47,54 @@ def setUp(self):
             "html",
             "test-upstream",
         ).usage_key
-        upstream = xblock.load_block(upstream_key, self.user)
+        upstream = xblock.load_block(self.upstream_key, self.user)
         upstream.display_name = "original upstream title"
         upstream.data = "<p>original upstream content</p>"
         upstream.save()
-        downstream = BlockFactory.create(category='html', parent=vertical, upstream=str(upstream_key))
-        downstream.sync_from_upstream(user=self.user, apply_updates=True)
+        downstream = BlockFactory.create(category='html', parent=vertical, upstream=str(self.upstream_key))
+        self.sync_service = UpstreamSyncService(self.user)
+        self.sync_service.sync_from_upstream(downstream, apply_updates=True)
         downstream.save()
         self.store.update_item(downstream, self.user.id)
         self.downstream_key = downstream.usage_key
 
-    def test_sync_updates_content_and_settings(self):
+    def test_sync_to_unmodified_content(self):
         """
         Can we sync updates from a content library block to a linked out-of-date course block?
         """
         downstream = self.store.get_item(self.downstream_key)
-        assert downstream.upstream_settings["display_name"] == "original upstream title"
-        assert downstream.upstream_overridden == []
+        assert downstream.upstream_display_name == "original upstream title"
+        assert downstream.downstream_customized == set()
         assert downstream.display_name == "original upstream title"
-        assert downstream.data == "<p>original upstream content</p>"
+        assert downstream.data == "\n<p>original upstream content</p>\n"  # @@TODO newlines??
 
-        upstream = xblock.load_block(upstream_key, self.user)
+        upstream = xblock.load_block(self.upstream_key, self.user)
         upstream.display_name = "NEW upstream title"
         upstream.data = "<p>NEW upstream content</p>"
         upstream.save()
 
-        downstream.sync_from_upstream(user=self.user, apply_updates=True)
-        assert downstream.upstream_settings["display_name"] == "NEW upstream title"
-        assert downstream.upstream_overridden == []
+        self.sync_service.sync_from_upstream(downstream, apply_updates=True)
+        assert downstream.upstream_display_name == "NEW upstream title"
+        assert downstream.downstream_customized == set()
         assert downstream.display_name == "NEW upstream title"
-        assert downstream.data == "<p>NEW upstream content</p>"
+        assert downstream.data == "\n<p>NEW upstream content</p>\n"  # @@TODO newlines??
 
-    def test_sync_overwrites_content_but_preserves_settings(self):
+    def test_sync_to_modified_contenet(self):
         """
-        Can we sync updates from a content library block to a linked out-of-date course block?
+        If we sync to modified content, will it preserve customizable fields, but overwrite the rest?
         """
         downstream = self.store.get_item(self.downstream_key)
         downstream.display_name = "downstream OVERRIDE of the title"
         downstream.data = "<p>downstream OVERRIDE of the content</p>"
         downstream.save()
 
-        upstream = xblock.load_block(upstream_key, self.user)
+        upstream = xblock.load_block(self.upstream_key, self.user)
         upstream.display_name = "NEW upstream title"
         upstream.data = "<p>NEW upstream content</p>"
         upstream.save()
 
-        downstream.sync_from_upstream(user=self.user, apply_updates=True)
-        assert downstream.upstream_settings["display_name"] == "NEW upstream title"
-        assert downstream.upstream_overridden == ["display_name"]
+        self.sync_service.sync_from_upstream(downstream, apply_updates=True)
+        assert downstream.upstream_display_name == "NEW upstream title"
+        assert downstream.downstream_customized == {"display_name"}
         assert downstream.display_name == "downstream OVERRIDE of the title"
-        assert downstream.data == "<p>NEW upstream content</p>"
+        assert downstream.data == "\n<p>NEW upstream content</p>\n"  # @@TODO newlines??