refactor: xblock api upstream info and course details api (#37971)
- Returns top parent key instead of boolean in upstream info api - Adds edited_on raw time in course outline api - Adds has_changes to course details api
This commit is contained in:
@@ -53,6 +53,7 @@ class CourseDetailsSerializer(serializers.Serializer):
|
||||
pre_requisite_courses = serializers.ListField(child=CourseKeyField())
|
||||
run = serializers.CharField()
|
||||
self_paced = serializers.BooleanField()
|
||||
has_changes = serializers.BooleanField()
|
||||
short_description = serializers.CharField(allow_blank=True)
|
||||
start_date = serializers.DateTimeField()
|
||||
subtitle = serializers.CharField(allow_blank=True)
|
||||
|
||||
@@ -125,7 +125,7 @@ class UpstreamLinkSerializer(serializers.Serializer):
|
||||
error_message = serializers.CharField(allow_null=True)
|
||||
ready_to_sync = serializers.BooleanField()
|
||||
downstream_customized = serializers.ListField(child=serializers.CharField(), allow_empty=True)
|
||||
has_top_level_parent = serializers.BooleanField()
|
||||
top_level_parent_key = serializers.CharField(allow_null=True)
|
||||
ready_to_sync_children = UpstreamChildrenInfoSerializer(many=True, required=False)
|
||||
|
||||
|
||||
|
||||
@@ -323,7 +323,7 @@ class ContainerVerticalViewTest(BaseXBlockContainer):
|
||||
"version_declined": None,
|
||||
"error_message": None,
|
||||
"ready_to_sync": True,
|
||||
"has_top_level_parent": False,
|
||||
"top_level_parent_key": None,
|
||||
"downstream_customized": [],
|
||||
},
|
||||
"user_partition_info": expected_user_partition_info,
|
||||
|
||||
@@ -50,7 +50,7 @@ def _get_upstream_link_good_and_syncable(downstream):
|
||||
version_declined=downstream.upstream_version_declined,
|
||||
error_message=None,
|
||||
downstream_customized=[],
|
||||
has_top_level_parent=False,
|
||||
top_level_parent_key=None,
|
||||
upstream_name=downstream.upstream_display_name,
|
||||
)
|
||||
|
||||
|
||||
@@ -35,7 +35,7 @@ from olxcleaner.exceptions import ErrorLevel
|
||||
from olxcleaner.reporting import report_error_summary, report_errors
|
||||
from opaque_keys import InvalidKeyError
|
||||
from opaque_keys.edx.keys import CourseKey, UsageKey
|
||||
from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocator, BlockUsageLocator
|
||||
from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocator
|
||||
from openedx_events.content_authoring.data import CourseData
|
||||
from openedx_events.content_authoring.signals import COURSE_RERUN_COMPLETED
|
||||
from organizations.api import add_organization_course, ensure_organization
|
||||
@@ -1641,11 +1641,7 @@ def handle_create_xblock_upstream_link(usage_key):
|
||||
return
|
||||
if xblock.top_level_downstream_parent_key is not None:
|
||||
block_key = BlockKey.from_string(xblock.top_level_downstream_parent_key)
|
||||
top_level_parent_usage_key = BlockUsageLocator(
|
||||
xblock.course_id,
|
||||
block_key.type,
|
||||
block_key.id,
|
||||
)
|
||||
top_level_parent_usage_key = block_key.to_usage_key(xblock.course_id)
|
||||
try:
|
||||
ContainerLink.get_by_downstream_usage_key(top_level_parent_usage_key)
|
||||
except ContainerLink.DoesNotExist:
|
||||
|
||||
@@ -1202,6 +1202,9 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements
|
||||
"edited_on": get_default_time_display(xblock.subtree_edited_on)
|
||||
if xblock.subtree_edited_on
|
||||
else None,
|
||||
"edited_on_raw": str(xblock.subtree_edited_on)
|
||||
if xblock.subtree_edited_on
|
||||
else None,
|
||||
"published": published,
|
||||
"published_on": published_on,
|
||||
"studio_url": xblock_studio_url(xblock, parent_xblock),
|
||||
@@ -1331,7 +1334,7 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements
|
||||
# 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"]
|
||||
xblock_actions["unlinkable"] = not upstream_info["top_level_parent_key"]
|
||||
|
||||
if is_xblock_unit:
|
||||
# if xblock is a Unit we add the discussion_enabled option
|
||||
|
||||
@@ -86,7 +86,7 @@ class UpstreamLink:
|
||||
version_declined: int | None # Latest version which the user has declined to sync with, if any.
|
||||
error_message: str | None # If link is valid, None. Otherwise, a localized, human-friendly error message.
|
||||
downstream_customized: list[str] | None # List of fields modified in downstream
|
||||
has_top_level_parent: bool # True if this Upstream link has a top-level parent
|
||||
top_level_parent_key: str | None # key of top-level parent if Upstream link has a one.
|
||||
|
||||
@property
|
||||
def is_upstream_deleted(self) -> bool:
|
||||
@@ -153,7 +153,7 @@ class UpstreamLink:
|
||||
from xmodule.modulestore.django import modulestore
|
||||
|
||||
# If this component/container has top-level parent, so we need to sync the parent
|
||||
if self.has_top_level_parent:
|
||||
if self.top_level_parent_key:
|
||||
return False
|
||||
|
||||
if isinstance(self.upstream_key, LibraryUsageLocatorV2):
|
||||
@@ -222,6 +222,10 @@ class UpstreamLink:
|
||||
downstream.usage_key,
|
||||
downstream.upstream,
|
||||
)
|
||||
if top_level_parent_key := getattr(downstream, "top_level_downstream_parent_key", None):
|
||||
top_level_parent_key = str(
|
||||
BlockKey.from_string(top_level_parent_key).to_usage_key(downstream.usage_key.context_key)
|
||||
)
|
||||
return cls(
|
||||
upstream_ref=getattr(downstream, "upstream", None),
|
||||
upstream_name=getattr(downstream, "upstream_display_name", None),
|
||||
@@ -232,7 +236,7 @@ class UpstreamLink:
|
||||
version_declined=None,
|
||||
error_message=str(exc),
|
||||
downstream_customized=getattr(downstream, "downstream_customized", []),
|
||||
has_top_level_parent=getattr(downstream, "top_level_downstream_parent_key", None) is not None,
|
||||
top_level_parent_key=top_level_parent_key,
|
||||
)
|
||||
|
||||
@classmethod
|
||||
@@ -306,6 +310,10 @@ class UpstreamLink:
|
||||
)
|
||||
)
|
||||
|
||||
if top_level_parent_key := getattr(downstream, "top_level_downstream_parent_key", None):
|
||||
top_level_parent_key = str(
|
||||
BlockKey.from_string(top_level_parent_key).to_usage_key(downstream.usage_key.context_key)
|
||||
)
|
||||
result = cls(
|
||||
upstream_ref=downstream.upstream,
|
||||
upstream_key=upstream_key,
|
||||
@@ -316,7 +324,7 @@ class UpstreamLink:
|
||||
version_declined=downstream.upstream_version_declined,
|
||||
error_message=None,
|
||||
downstream_customized=getattr(downstream, "downstream_customized", []),
|
||||
has_top_level_parent=downstream.top_level_downstream_parent_key is not None,
|
||||
top_level_parent_key=top_level_parent_key,
|
||||
)
|
||||
|
||||
return result
|
||||
|
||||
@@ -25,7 +25,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
|
||||
can_unlink = upstream_info.upstream_ref and not upstream_info.top_level_parent_key
|
||||
%>
|
||||
|
||||
<%namespace name='static' file='static_content.html'/>
|
||||
|
||||
@@ -77,6 +77,7 @@ class CourseDetails:
|
||||
self.self_paced = None
|
||||
self.learning_info = []
|
||||
self.instructor_info = []
|
||||
self.has_changes = None
|
||||
|
||||
@classmethod
|
||||
def fetch_about_attribute(cls, course_key, attribute):
|
||||
@@ -127,6 +128,7 @@ class CourseDetails:
|
||||
course_details.video_thumbnail_image_asset_path = course_image_url(block, 'video_thumbnail_image')
|
||||
course_details.language = block.language
|
||||
course_details.self_paced = block.self_paced
|
||||
course_details.has_changes = modulestore().has_changes(block)
|
||||
course_details.learning_info = block.learning_info
|
||||
course_details.instructor_info = block.instructor_info
|
||||
course_details.title = block.display_name
|
||||
|
||||
@@ -1,15 +1,15 @@
|
||||
"""
|
||||
Tests for xmodule/util/keys.py
|
||||
"""
|
||||
import ddt
|
||||
import pytest
|
||||
from unittest import TestCase
|
||||
from unittest.mock import Mock
|
||||
|
||||
from opaque_keys.edx.locator import BlockUsageLocator
|
||||
import ddt
|
||||
import pytest
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
from xmodule.util.keys import BlockKey, derive_key
|
||||
from opaque_keys.edx.locator import BlockUsageLocator
|
||||
|
||||
from xmodule.util.keys import BlockKey, derive_key
|
||||
|
||||
mock_block = Mock()
|
||||
mock_block.id = CourseKey.from_string('course-v1:Beeper+B33P+BOOP')
|
||||
@@ -70,3 +70,19 @@ class TestBlockKeyParsing(TestCase):
|
||||
@ddt.unpack
|
||||
def test_block_key_to_string(self, block_key, block_key_str):
|
||||
assert str(block_key) == block_key_str
|
||||
|
||||
@ddt.data(
|
||||
[BlockKey('chapter', 'some-id'), BlockUsageLocator(
|
||||
mock_block.id,
|
||||
'chapter',
|
||||
'some-id'
|
||||
)],
|
||||
[BlockKey('section', 'one-more-id'), BlockUsageLocator(
|
||||
mock_block.id,
|
||||
'section',
|
||||
'one-more-id'
|
||||
)]
|
||||
)
|
||||
@ddt.unpack
|
||||
def test_block_key_to_usage_key(self, block_key: BlockKey, block_key_str):
|
||||
assert block_key.to_usage_key(mock_block.id) == block_key_str
|
||||
|
||||
@@ -6,7 +6,7 @@ Consider moving these into opaque-keys if they generalize well.
|
||||
import hashlib
|
||||
from typing import NamedTuple, Self
|
||||
|
||||
from opaque_keys.edx.keys import UsageKey
|
||||
from opaque_keys.edx.keys import CourseKey, UsageKey
|
||||
|
||||
|
||||
class BlockKey(NamedTuple):
|
||||
@@ -40,6 +40,12 @@ class BlockKey(NamedTuple):
|
||||
raise ValueError(f"Invalid string format for BlockKey: {s}")
|
||||
return cls(parts[0], parts[1])
|
||||
|
||||
def to_usage_key(self, course_key: CourseKey) -> UsageKey:
|
||||
"""
|
||||
Converts this BlockKey into a UsageKey.
|
||||
"""
|
||||
return course_key.make_usage_key(self.type, self.id)
|
||||
|
||||
|
||||
def derive_key(source: UsageKey, dest_parent: BlockKey) -> BlockKey:
|
||||
"""
|
||||
|
||||
Reference in New Issue
Block a user