Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

feat: Add unlinkable to xblock actions and update top_level_parent_key on unlink [FC-0097] (#37215)

Browse Source 
- Adds the `unlinkable` action to the XBlock object sent to the frontend
- Updates the `top_level_parent_key` reference when unlinking containers. If you unlink a Section with Subsections and Units, this updates the `top_level_parent_key` for the Subsections to `None` (they are the top level now), and the `top_level_parent_key` for the Units to the corresponding parent Subsection.
This commit is contained in:
Rômulo Penido
2025-08-28 13:30:17 -03:00
committed by GitHub
parent 052b930ef5
commit 8085bf6be4
9 changed files with 169 additions and 33 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
cms/djangoapps/contentstore/models.py
View File

@@ -128,6 +128,10 @@ class EntityLinkBase(models.Model):
class Meta:
abstract = True
@classmethod
def get_by_downstream_usage_key(cls, downstream_usage_key: UsageKey):
return cls.objects.get(downstream_usage_key=downstream_usage_key)
class ComponentLink(EntityLinkBase):
"""
@@ -523,10 +527,6 @@ class ContainerLink(EntityLinkBase):
link.save()
return link
@classmethod
def get_by_downstream_usage_key(cls, downstream_usage_key: UsageKey):
return cls.objects.get(downstream_usage_key=downstream_usage_key)
class LearningContextLinksStatusChoices(models.TextChoices):
"""

29
cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py
View File

@@ -455,16 +455,39 @@ class DownstreamView(DeveloperErrorViewMixin, APIView):
Sever an XBlock's link to upstream content.
"""
downstream = _load_accessible_block(request.user, usage_key_string, require_write_access=True)
affected_blocks: list[XBlock] = []
try:
sever_upstream_link(downstream)
# Try to get the upstream key before severing the link, so we can delete
# the corresponding ComponentLink or ContainerLink below.
try:
upstream_key = UpstreamLink.get_for_block(downstream).upstream_key
except NoUpstream:
# Even if we don't have an UpstreamLink, we still need to check
# if the block has the upstream key set, so we don't want to
# raise an exception here.
upstream_key = None
affected_blocks = sever_upstream_link(downstream)
# Remove the ComponentLink or ContainerLink, if it exists.
if upstream_key:
if isinstance(upstream_key, LibraryUsageLocatorV2):
ComponentLink.get_by_downstream_usage_key(downstream.usage_key).delete()
elif isinstance(upstream_key, LibraryContainerLocator):
ContainerLink.get_by_downstream_usage_key(downstream.usage_key).delete()
except NoUpstream:
logger.exception(
"Tried to DELETE upstream link of '%s', but it wasn't linked to anything in the first place. "
"Will do nothing. ",
usage_key_string,
)
else:
modulestore().update_item(downstream, request.user.id)
finally:
if affected_blocks:
# If we successfully severed the upstream link, then we need to update the affected blocks.
with modulestore().bulk_operations(downstream.usage_key.context_key):
for block in affected_blocks:
modulestore().update_item(block, request.user.id)
return Response(status=204)

57
cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py
View File

@@ -2,29 +2,29 @@
Unit tests for /api/contentstore/v2/downstreams/* JSON APIs.
"""
import json
import ddt
from datetime import datetime, timezone
from unittest.mock import patch, MagicMock
from unittest.mock import MagicMock, patch
import ddt
from django.conf import settings
from django.urls import reverse
from freezegun import freeze_time
from opaque_keys.edx.keys import ContainerKey, UsageKey
from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2
from organizations.models import Organization
from cms.djangoapps.contentstore.helpers import StaticFileNotices
from cms.lib.xblock.upstream_sync import BadUpstream, UpstreamLink
from cms.djangoapps.contentstore.tests.utils import CourseTestCase
from cms.djangoapps.contentstore.xblock_storage_handlers import view_handlers as xblock_view_handlers
from cms.djangoapps.contentstore.xblock_storage_handlers.xblock_helpers import get_block_key_dict
from opaque_keys.edx.keys import ContainerKey, UsageKey
from opaque_keys.edx.locator import LibraryLocatorV2
from common.djangoapps.student.tests.factories import UserFactory
from cms.lib.xblock.upstream_sync import BadUpstream, UpstreamLink
from common.djangoapps.student.auth import add_users
from common.djangoapps.student.roles import CourseStaffRole
from common.djangoapps.student.tests.factories import UserFactory
from openedx.core.djangoapps.content_libraries import api as lib_api
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory
from openedx.core.djangoapps.content_libraries import api as lib_api
from .. import downstreams as downstreams_views
@@ -42,7 +42,7 @@ URL_LIB_CONTAINER_PUBLISH = URL_LIB_CONTAINER + 'publish/' # Publish changes to
def _get_upstream_link_good_and_syncable(downstream):
return UpstreamLink(
upstream_ref=downstream.upstream,
upstream_key=UsageKey.from_string(downstream.upstream),
upstream_key=LibraryUsageLocatorV2.from_string(downstream.upstream),
downstream_key=str(downstream.usage_key),
version_synced=downstream.upstream_version,
version_available=(downstream.upstream_version or 0) + 1,
@@ -433,6 +433,45 @@ class DeleteDownstreamViewTest(SharedErrorTestCases, SharedModuleStoreTestCase):
assert response.status_code == 204
assert mock_sever.call_count == 1
def test_unlink_parent_should_update_children_top_level_parent(self):
"""
If we unlink a parent block, do all children get the new top-level parent?
"""
self.client.login(username="superuser", password="password")
all_downstreams = self.client.get(
"/api/contentstore/v2/downstreams/",
data={"course_id": str(self.course.id)},
)
assert all_downstreams.data["count"] == 11
response = self.call_api(self.top_level_downstream_chapter.usage_key)
assert response.status_code == 204
# Check that all children have their top_level_downstream_parent_key updated
subsection = modulestore().get_item(self.top_level_downstream_sequential.usage_key)
assert subsection.top_level_downstream_parent_key is None
unit = modulestore().get_item(self.top_level_downstream_unit_2.usage_key)
# The sequential is the top-level parent for the unit
assert unit.top_level_downstream_parent_key == {
"id": str(self.top_level_downstream_sequential.usage_key.block_id),
"type": str(self.top_level_downstream_sequential.usage_key.block_type),
}
video = modulestore().get_item(self.top_level_downstream_video_key)
# The sequential is the top-level parent for the video
assert video.top_level_downstream_parent_key == {
"id": str(self.top_level_downstream_sequential.usage_key.block_id),
"type": str(self.top_level_downstream_sequential.usage_key.block_type),
}
all_downstreams = self.client.get(
"/api/contentstore/v2/downstreams/",
data={"course_id": str(self.course.id)},
)
assert all_downstreams.data["count"] == 10
class _DownstreamSyncViewTestMixin(SharedErrorTestCases):
"""
@@ -562,7 +601,7 @@ class GetUpstreamViewTest(
SharedModuleStoreTestCase,
):
"""
Test that `GET /api/v2/contentstore/downstreams-all?...` returns list of links based on the provided filter.
Test that `GET /api/v2/contentstore/downstreams?...` returns list of links based on the provided filter.
"""
def call_api(

6
cms/djangoapps/contentstore/tasks.py
View File

@@ -1654,7 +1654,7 @@ def handle_create_xblock_upstream_link(usage_key):
# The top-level parent link does not exist yet,
# it is necessary to create it first.
handle_create_xblock_upstream_link(str(top_level_parent_usage_key))
create_or_update_xblock_upstream_link(xblock, xblock.course_id)
create_or_update_xblock_upstream_link(xblock)
@shared_task
@@ -1671,7 +1671,7 @@ def handle_update_xblock_upstream_link(usage_key):
return
if not xblock.upstream or not xblock.upstream_version:
return
create_or_update_xblock_upstream_link(xblock, xblock.course_id)
create_or_update_xblock_upstream_link(xblock)
@shared_task
@@ -1711,7 +1711,7 @@ def create_or_update_upstream_links(
course_status.update_status(LearningContextLinksStatusChoices.FAILED)
return
for xblock in xblocks:
create_or_update_xblock_upstream_link(xblock, course_key, created)
create_or_update_xblock_upstream_link(xblock, created)
course_status.update_status(LearningContextLinksStatusChoices.COMPLETED)

18
cms/djangoapps/contentstore/utils.py
View File

@@ -2385,7 +2385,7 @@ def get_xblock_render_error(request, xblock):
return ""
def _create_or_update_component_link(course_key: CourseKey, created: datetime | None, xblock):
def _create_or_update_component_link(created: datetime | None, xblock):
"""
Create or update upstream->downstream link for components in database for given xblock.
"""
@@ -2399,7 +2399,7 @@ def _create_or_update_component_link(course_key: CourseKey, created: datetime |
top_level_parent_usage_key = None
if xblock.top_level_downstream_parent_key is not None:
top_level_parent_usage_key = BlockUsageLocator(
course_key,
xblock.usage_key.course_key,
xblock.top_level_downstream_parent_key.get('type'),
xblock.top_level_downstream_parent_key.get('id'),
)
@@ -2408,7 +2408,7 @@ def _create_or_update_component_link(course_key: CourseKey, created: datetime |
lib_component,
upstream_usage_key=upstream_usage_key,
upstream_context_key=str(upstream_usage_key.context_key),
downstream_context_key=course_key,
downstream_context_key=xblock.usage_key.course_key,
downstream_usage_key=xblock.usage_key,
top_level_parent_usage_key=top_level_parent_usage_key,
version_synced=xblock.upstream_version,
@@ -2417,7 +2417,7 @@ def _create_or_update_component_link(course_key: CourseKey, created: datetime |
)
def _create_or_update_container_link(course_key: CourseKey, created: datetime | None, xblock):
def _create_or_update_container_link(created: datetime | None, xblock):
"""
Create or update upstream->downstream link for containers in database for given xblock.
"""
@@ -2431,7 +2431,7 @@ def _create_or_update_container_link(course_key: CourseKey, created: datetime |
top_level_parent_usage_key = None
if xblock.top_level_downstream_parent_key is not None:
top_level_parent_usage_key = BlockUsageLocator(
course_key,
xblock.usage_key.course_key,
xblock.top_level_downstream_parent_key.get('type'),
xblock.top_level_downstream_parent_key.get('id'),
)
@@ -2440,7 +2440,7 @@ def _create_or_update_container_link(course_key: CourseKey, created: datetime |
lib_component,
upstream_container_key=upstream_container_key,
upstream_context_key=str(upstream_container_key.context_key),
downstream_context_key=course_key,
downstream_context_key=xblock.usage_key.course_key,
downstream_usage_key=xblock.usage_key,
version_synced=xblock.upstream_version,
top_level_parent_usage_key=top_level_parent_usage_key,
@@ -2449,7 +2449,7 @@ def _create_or_update_container_link(course_key: CourseKey, created: datetime |
)
def create_or_update_xblock_upstream_link(xblock, course_key: CourseKey, created: datetime | None = None) -> None:
def create_or_update_xblock_upstream_link(xblock, created: datetime | None = None) -> None:
"""
Create or update upstream->downstream link in database for given xblock.
"""
@@ -2457,11 +2457,11 @@ def create_or_update_xblock_upstream_link(xblock, course_key: CourseKey, created
return None
try:
# Try to create component link
_create_or_update_component_link(course_key, created, xblock)
_create_or_update_component_link(created, xblock)
except InvalidKeyError:
# It is possible that the upstream is a container and UsageKeyV2 parse failed
# Create upstream container link and raise InvalidKeyError if xblock.upstream is a valid key.
_create_or_update_container_link(course_key, created, xblock)
_create_or_update_container_link(created, xblock)
def get_previous_run_course_key(course_key):

9
cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py
View File

@@ -1118,11 +1118,14 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements
# defining the default value 'True' for delete, duplicate, drag and add new child actions
# in xblock_actions for each xblock.
# The unlinkable action is set to None by default, which means the action is not applicable for
# any xblock unless explicitly set to True or False for a specific xblock condition.
xblock_actions = {
"deletable": True,
"draggable": True,
"childAddable": True,
"duplicable": True,
"unlinkable": None,
}
explanatory_message = None
@@ -1320,9 +1323,13 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements
# Also add upstream info
upstream_info = UpstreamLink.try_get_for_block(xblock, log_error=False).to_json()
xblock_info["upstream_info"] = upstream_info
# Disable adding or removing children component if xblock is imported from library
if upstream_info["upstream_ref"]:
# Disable adding or removing children component if xblock is imported from library
xblock_actions["childAddable"] = False
# Enable unlinking only for top level imported components
xblock_actions["unlinkable"] = not upstream_info["has_top_level_parent"]
if is_xblock_unit:
# if xblock is a Unit we add the discussion_enabled option
xblock_info["discussion_enabled"] = xblock.discussion_enabled

49
cms/lib/xblock/upstream_sync.py
View File

@@ -28,6 +28,7 @@ from opaque_keys.edx.keys import UsageKey
from xblock.exceptions import XBlockNotFoundError
from xblock.fields import Scope, String, Integer, Dict
from xblock.core import XBlockMixin, XBlock
from xmodule.util.keys import BlockKey
if t.TYPE_CHECKING:
from django.contrib.auth.models import User # pylint: disable=imported-auth-user
@@ -190,14 +191,14 @@ class UpstreamLink:
downstream.upstream,
)
return cls(
upstream_ref=getattr(downstream, "upstream", ""),
upstream_ref=getattr(downstream, "upstream", None),
upstream_key=None,
downstream_key=str(getattr(downstream, "usage_key", "")),
version_synced=getattr(downstream, "upstream_version", None),
version_available=None,
version_declined=None,
error_message=str(exc),
has_top_level_parent=False,
has_top_level_parent=getattr(downstream, "top_level_downstream_parent_key", None) is not None,
)
@classmethod
@@ -311,7 +312,37 @@ def decline_sync(downstream: XBlock, user_id=None) -> None:
store.update_item(downstream, user_id)
def sever_upstream_link(downstream: XBlock) -> None:
def _update_children_top_level_parent(
downstream: XBlock,
new_top_level_parent_key: dict[str, str] | None
) -> list[XBlock]:
"""
Given a new top-level parent block, update the `top_level_downstream_parent_key` field on the downstream block
and all of its children.
If `new_top_level_parent_key` is None, use the current downstream block's usage_key for its children.
Returns a list of all affected blocks.
"""
if not downstream.has_children:
return []
affected_blocks = []
for child in downstream.get_children():
child.top_level_downstream_parent_key = new_top_level_parent_key
affected_blocks.append(child)
# If the `new_top_level_parent_key` is None, the current level assume the top-level
# parent key for its children.
child_top_level_parent_key = new_top_level_parent_key if new_top_level_parent_key is not None else (
BlockKey.from_usage_key(child.usage_key)._asdict()
)
affected_blocks.extend(_update_children_top_level_parent(child, child_top_level_parent_key))
return affected_blocks
def sever_upstream_link(downstream: XBlock) -> list[XBlock]:
"""
Given an XBlock that is linked to upstream content, disconnect the link, such that authors are never again prompted
to sync upstream updates. Erase all `.upstream*` fields from the downtream block.
@@ -320,10 +351,12 @@ def sever_upstream_link(downstream: XBlock) -> None:
because once a downstream block has been de-linked from source (e.g., a Content Library block), it is no different
than if the block had just been copy-pasted in the first place.
Does not save `downstream` to the store. That is left up to the caller.
Does not save `downstream` (or its children) to the store. That is left up to the caller.
If `downstream` lacks a link, then this raises NoUpstream (though it is reasonable for callers to handle such
exception and ignore it, as the end result is the same: `downstream.upstream is None`).
Returns a list of affected blocks, which includes the `downstream` block itself and all of its children.
"""
if not downstream.upstream:
raise NoUpstream()
@@ -336,6 +369,14 @@ def sever_upstream_link(downstream: XBlock) -> None:
continue
setattr(downstream, fetched_upstream_field, None) # Null out upstream_display_name, et al.
# Set the top_level_dowwnstream_parent_key to None, and calls `_update_children_top_level_parent` to
# update all children with the new top_level_dowwnstream_parent_key for each of them.
downstream.top_level_downstream_parent_key = None
affected_blocks = _update_children_top_level_parent(downstream, None)
# Return the list of affected blocks, which includes the `downstream` block itself.
return [downstream, *affected_blocks]
def _get_library_xblock_url(usage_key: LibraryUsageLocatorV2):
"""

20
cms/static/js/views/pages/container.js
View File

@@ -30,6 +30,7 @@ function($, _, Backbone, gettext, BasePage,
'click .copy-button': 'copyXBlock',
'click .move-button': 'showMoveXBlockModal',
'click .delete-button': 'deleteXBlock',
'click .unlink-button': 'unlinkXBlock',
'click .library-sync-button': 'showXBlockLibraryChangesPreview',
'click .problem-bank-v2-add-button': 'showSelectV2LibraryContent',
'click .show-actions-menu-button': 'showXBlockActionsMenu',
@@ -922,6 +923,25 @@ function($, _, Backbone, gettext, BasePage,
this.deleteComponent(this.findXBlockElement(event.target));
},
unlinkXBlock: function(event) {
event.preventDefault();
const primaryHeader = $(event.target).closest('.xblock-header-primary, .nav-actions');
const usageId = encodeURI(primaryHeader.attr('data-usage-id'));
try {
if (this.options.isIframeEmbed) {
window.parent.postMessage(
{
type: 'unlinkXBlock',
message: 'Unlink the XBlock',
payload: { usageId }
}, document.referrer
);
}
} catch (e) {
console.error(e);
}
},
createComponent: function(template, target, iframeMessageData) {
// A placeholder element is created in the correct location for the new xblock
// and then onNewXBlock will replace it with a rendering of the xblock. Note that

6
cms/templates/studio_xblock_wrapper.html
View File

@@ -26,6 +26,7 @@ has_not_configured_message = messages.get('summary',{}).get('type', None) == 'no
block_is_unit = is_unit(xblock)
upstream_info = UpstreamLink.try_get_for_block(xblock, log_error=False)
can_unlink = upstream_info.upstream_ref and not upstream_info.has_top_level_parent
%>
<%namespace name='static' file='static_content.html'/>
@@ -202,6 +203,11 @@ upstream_info = UpstreamLink.try_get_for_block(xblock, log_error=False)
<a class="delete-button" href="#" role="button">${_("Delete")}</a>
</li>
% endif
% if can_unlink:
<li class="nav-item">
<a class="unlink-button" href="#" role="button">${_("Unlink from Library")}</a>
</li>
% endif
</ul>
</div>
</div>