Skip to content

Commit

Permalink
feat: Upstream Sync with Content Library Blocks
Browse files Browse the repository at this point in the history
Including:

* A new XBlockMixin: UpstreamSyncMixin.
* A new CMS Python API: cms.lib.xblock.upstream_sync
* A new CMS JSON API: /api/contentstore/v2/downstreams
* A temporary, very basic UI for syncing from Content Library blocks

Implements:
https://github.com/kdmccormick/edx-platform/blob/kdmccormick/upstream-proto/docs/decisions/0020-upstream-block.rst

Co-Authored-By: Braden MacDonald <[email protected]>
  • Loading branch information
kdmccormick and bradenmacdonald committed Oct 16, 2024
1 parent ecbedbc commit 4d20e57
Show file tree
Hide file tree
Showing 22 changed files with 1,562 additions and 23 deletions.
47 changes: 38 additions & 9 deletions cms/djangoapps/contentstore/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

from attrs import frozen, Factory
from django.conf import settings
from django.contrib.auth import get_user_model
from django.utils.translation import gettext as _
from opaque_keys.edx.keys import AssetKey, CourseKey, UsageKey
from opaque_keys.edx.locator import DefinitionLocator, LocalId
Expand All @@ -22,6 +23,7 @@
from xmodule.xml_block import XmlMixin

from cms.djangoapps.models.settings.course_grading import CourseGradingModel
from cms.lib.xblock.upstream_sync import UpstreamLink, BadUpstream, BadDownstream, fetch_customizable_fields
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
Expand All @@ -30,6 +32,10 @@

log = logging.getLogger(__name__)


User = get_user_model()


# Note: Grader types are used throughout the platform but most usages are simply in-line
# strings. In addition, new grader types can be defined on the fly anytime one is needed
# (because they're just strings). This dict is an attempt to constrain the sprawl in Studio.
Expand Down Expand Up @@ -282,9 +288,10 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request) ->
node,
parent_xblock,
store,
user_id=request.user.id,
user=request.user,
slug_hint=user_clipboard.source_usage_key.block_id,
copied_from_block=str(user_clipboard.source_usage_key),
copied_from_version_num=user_clipboard.content.version_num,
tags=user_clipboard.content.tags,
)
# Now handle static files that need to go into Files & Uploads:
Expand All @@ -293,7 +300,6 @@ def import_staged_content_from_user_clipboard(parent_key: UsageKey, request) ->
staged_content_id=user_clipboard.content.id,
static_files=static_files,
)

return new_xblock, notices


Expand All @@ -302,12 +308,15 @@ def _import_xml_node_to_parent(
parent_xblock: XBlock,
# The modulestore we're using
store,
# The ID of the user who is performing this operation
user_id: int,
# The user who is performing this operation
user: User,
# Hint to use as usage ID (block_id) for the new XBlock
slug_hint: str | None = None,
# UsageKey of the XBlock that this one is a copy of
copied_from_block: str | None = None,
# Positive int version of source block, if applicable (e.g., library block).
# Zero if not applicable (e.g., course block).
copied_from_version_num: int = 0,
# Content tags applied to the source XBlock(s)
tags: dict[str, str] | None = None,
) -> XBlock:
Expand Down Expand Up @@ -373,12 +382,32 @@ def _import_xml_node_to_parent(
raise NotImplementedError("We don't yet support pasting XBlocks with children")
temp_xblock.parent = parent_key
if copied_from_block:
# Store a reference to where this block was copied from, in the 'copied_from_block' field (AuthoringMixin)
temp_xblock.copied_from_block = copied_from_block
# Try to link the pasted block (downstream) to the copied block (upstream).
temp_xblock.upstream = copied_from_block
try:
UpstreamLink.get_for_block(temp_xblock)
except (BadDownstream, BadUpstream):
# Usually this will fail. For example, if the copied block is a modulestore course block, it can't be an
# upstream. That's fine! Instead, we store a reference to where this block was copied from, in the
# 'copied_from_block' field (from AuthoringMixin).
temp_xblock.upstream = None
temp_xblock.copied_from_block = copied_from_block
else:
# But if it doesn't fail, then populate the `upstream_version` field based on what was copied. Note that
# this could be the latest published version, or it could be an an even newer draft version.
temp_xblock.upstream_version = copied_from_version_num
# Also, fetch upstream values (`upstream_display_name`, etc.).
# Recall that the copied block could be a draft. So, rather than fetching from the published upstream (which
# could be older), fetch from the copied block itself. That way, if an author customizes a field, but then
# later wants to restore it, it will restore to the value that the field had when the block was pasted. Of
# course, if the author later syncs updates from a *future* published upstream version, then that will fetch
# new values from the published upstream content.
fetch_customizable_fields(upstream=temp_xblock, downstream=temp_xblock, user=user)

# 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)
new_xblock = store.update_item(temp_xblock, user.id, allow_not_found=True)
parent_xblock.children.append(new_xblock.location)
store.update_item(parent_xblock, user_id)
store.update_item(parent_xblock, user.id)

children_handled = False
if hasattr(new_xblock, 'studio_post_paste'):
Expand All @@ -394,7 +423,7 @@ def _import_xml_node_to_parent(
child_node,
new_xblock,
store,
user_id=user_id,
user=user,
copied_from_block=str(child_copied_from),
tags=tags,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,18 @@ def get_assets_url(self, obj):
return None


class UpstreamLinkSerializer(serializers.Serializer):
"""
Serializer holding info for syncing a block with its upstream (eg, a library block).
"""
upstream_ref = serializers.CharField()
version_synced = serializers.IntegerField()
version_available = serializers.IntegerField(allow_null=True)
version_declined = serializers.IntegerField(allow_null=True)
error_message = serializers.CharField(allow_null=True)
ready_to_sync = serializers.BooleanField()


class ChildVerticalContainerSerializer(serializers.Serializer):
"""
Serializer for representing a xblock child of vertical container.
Expand All @@ -113,6 +125,7 @@ class ChildVerticalContainerSerializer(serializers.Serializer):
block_type = serializers.CharField()
user_partition_info = serializers.DictField()
user_partitions = serializers.ListField()
upstream_link = UpstreamLinkSerializer(allow_null=True)
actions = serializers.SerializerMethodField()
validation_messages = MessageValidation(many=True)
render_error = serializers.CharField()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ def setup_xblock(self):
parent=self.vertical.location,
category="html",
display_name="Html Content 2",
upstream="lb:FakeOrg:FakeLib:html:FakeBlock",
upstream_version=5,
)

def create_block(self, parent, category, display_name, **kwargs):
Expand Down Expand Up @@ -193,6 +195,7 @@ def test_children_content(self):
"name": self.html_unit_first.display_name_with_default,
"block_id": str(self.html_unit_first.location),
"block_type": self.html_unit_first.location.block_type,
"upstream_link": None,
"user_partition_info": expected_user_partition_info,
"user_partitions": expected_user_partitions,
"actions": {
Expand All @@ -218,12 +221,21 @@ def test_children_content(self):
"can_delete": True,
"can_manage_tags": True,
},
"upstream_link": {
"upstream_ref": "lb:FakeOrg:FakeLib:html:FakeBlock",
"version_synced": 5,
"version_available": None,
"version_declined": None,
"error_message": "Linked library item was not found in the system",
"ready_to_sync": False,
},
"user_partition_info": expected_user_partition_info,
"user_partitions": expected_user_partitions,
"validation_messages": [],
"render_error": "",
},
]
self.maxDiff = None
self.assertEqual(response.data["children"], expected_response)

def test_not_valid_usage_key_string(self):
Expand Down
23 changes: 23 additions & 0 deletions cms/djangoapps/contentstore/rest_api/v1/views/vertical_block.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
ContainerHandlerSerializer,
VerticalContainerSerializer,
)
from cms.lib.xblock.upstream_sync import UpstreamLink
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
Expand Down Expand Up @@ -198,6 +199,7 @@ def get(self, request: Request, usage_key_string: str):
"block_type": "drag-and-drop-v2",
"user_partition_info": {},
"user_partitions": {}
"upstream_link": null,
"actions": {
"can_copy": true,
"can_duplicate": true,
Expand All @@ -215,6 +217,13 @@ def get(self, request: Request, usage_key_string: str):
"block_type": "video",
"user_partition_info": {},
"user_partitions": {}
"upstream_link": {
"upstream_ref": "lb:org:mylib:video:404",
"version_synced": 16
"version_available": null,
"error_message": "Linked library item not found: lb:org:mylib:video:404",
"ready_to_sync": false,
},
"actions": {
"can_copy": true,
"can_duplicate": true,
Expand All @@ -232,6 +241,13 @@ def get(self, request: Request, usage_key_string: str):
"block_type": "html",
"user_partition_info": {},
"user_partitions": {},
"upstream_link": {
"upstream_ref": "lb:org:mylib:html:abcd",
"version_synced": 43,
"version_available": 49,
"error_message": null,
"ready_to_sync": true,
},
"actions": {
"can_copy": true,
"can_duplicate": true,
Expand Down Expand Up @@ -267,6 +283,7 @@ def get(self, request: Request, usage_key_string: str):
child_info = modulestore().get_item(child)
user_partition_info = get_visibility_partition_info(child_info, course=course)
user_partitions = get_user_partition_info(child_info, course=course)
upstream_link = UpstreamLink.try_get_for_block(child_info)
validation_messages = get_xblock_validation_messages(child_info)
render_error = get_xblock_render_error(request, child_info)

Expand All @@ -277,6 +294,12 @@ 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_link": (
# If the block isn't linked to an upstream (which is by far the most common case) then just
# make this field null, which communicates the same info, but with less noise.
upstream_link.to_json() if upstream_link.upstream_ref
else None
),
"validation_messages": validation_messages,
"render_error": render_error,
})
Expand Down
24 changes: 20 additions & 4 deletions cms/djangoapps/contentstore/rest_api/v2/urls.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,31 @@
"""Contenstore API v2 URLs."""

from django.urls import path

from cms.djangoapps.contentstore.rest_api.v2.views import HomePageCoursesViewV2
from django.conf import settings
from django.urls import path, re_path

from cms.djangoapps.contentstore.rest_api.v2.views import home, downstreams
app_name = "v2"

urlpatterns = [
path(
"home/courses",
HomePageCoursesViewV2.as_view(),
home.HomePageCoursesViewV2.as_view(),
name="courses",
),
# TODO: Potential future path.
# re_path(
# fr'^downstreams/$',
# downstreams.DownstreamsListView.as_view(),
# name="downstreams_list",
# ),
re_path(
fr'^downstreams/{settings.USAGE_KEY_PATTERN}$',
downstreams.DownstreamView.as_view(),
name="downstream"
),
re_path(
fr'^downstreams/{settings.USAGE_KEY_PATTERN}/sync$',
downstreams.SyncFromUpstreamView.as_view(),
name="sync_from_upstream"
),
]
3 changes: 0 additions & 3 deletions cms/djangoapps/contentstore/rest_api/v2/views/__init__.py
Original file line number Diff line number Diff line change
@@ -1,3 +0,0 @@
"""Module for v2 views."""

from cms.djangoapps.contentstore.rest_api.v2.views.home import HomePageCoursesViewV2
Loading

0 comments on commit 4d20e57

Please sign in to comment.