Skip to content

fix: mark container as ready to sync if any child block is deleted (backport) #37606

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ChrisChV merged 1 commit into from
Nov 5, 2025
Merged

fix: mark container as ready to sync if any child block is deleted (backport) #37606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions cms/djangoapps/contentstore/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@

from config_models.models import ConfigurationModel
from django.db import models
from django.db.models import QuerySet, OuterRef, Case, When, Exists, Value, ExpressionWrapper
from django.db.models.fields import IntegerField, TextField, BooleanField
from django.db.models import Case, Exists, ExpressionWrapper, OuterRef, Q, QuerySet, Value, When
from django.db.models.fields import BooleanField, IntegerField, TextField
from django.db.models.functions import Coalesce
from django.db.models.lookups import GreaterThan
from django.utils.translation import gettext_lazy as _
from opaque_keys.edx.django.models import CourseKeyField, ContainerKeyField, UsageKeyField
from opaque_keys.edx.django.models import ContainerKeyField, CourseKeyField, UsageKeyField
from opaque_keys.edx.keys import CourseKey, UsageKey
from opaque_keys.edx.locator import LibraryContainerLocator
from openedx_learning.api.authoring import get_published_version
Expand All @@ -23,7 +23,6 @@
manual_date_time_field,
)


logger = logging.getLogger(__name__)


Expand Down Expand Up @@ -391,7 +390,7 @@ def filter_links(
cls.objects.filter(**link_filter).select_related(*RELATED_FIELDS),
)
if ready_to_sync is not None:
result = result.filter(ready_to_sync=ready_to_sync)
result = result.filter(Q(ready_to_sync=ready_to_sync) | Q(ready_to_sync_from_children=ready_to_sync))

# Handle top-level parents logic
if use_top_level_parents:
Expand Down Expand Up @@ -436,6 +435,11 @@ def _annotate_query_with_ready_to_sync(cls, query_set: QuerySet["EntityLinkBase"
),
then=1
),
# If upstream block was deleted, set ready_to_sync = True
When(
Q(upstream_container__publishable_entity__published__version__version_num__isnull=True),
then=1
),
default=0,
output_field=models.IntegerField()
)
Expand All @@ -457,6 +461,11 @@ def _annotate_query_with_ready_to_sync(cls, query_set: QuerySet["EntityLinkBase"
),
then=1
),
# If upstream block was deleted, set ready_to_sync = True
When(
Q(upstream_block__publishable_entity__published__version__version_num__isnull=True),
then=1
),
default=0,
output_field=models.IntegerField()
)
Expand Down
113 changes: 112 additions & 1 deletion cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
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, ImmediateOnCommitMixin
from xmodule.modulestore.tests.django_utils import ImmediateOnCommitMixin, SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory

from .. import downstreams as downstreams_views
Expand All @@ -32,6 +32,7 @@
URL_PREFIX = '/api/libraries/v2/'
URL_LIB_CREATE = URL_PREFIX
URL_LIB_BLOCKS = URL_PREFIX + '{lib_key}/blocks/'
URL_LIB_BLOCK = URL_PREFIX + 'blocks/{block_key}/'
URL_LIB_BLOCK_PUBLISH = URL_PREFIX + 'blocks/{block_key}/publish/'
URL_LIB_BLOCK_OLX = URL_PREFIX + 'blocks/{block_key}/olx/'
URL_LIB_CONTAINER = URL_PREFIX + 'containers/{container_key}/' # Get a container in this library
Expand Down Expand Up @@ -277,6 +278,10 @@ def _create_container(self, lib_key, container_type, slug: str | None, display_n
data["slug"] = slug
return self._api('post', URL_LIB_CONTAINERS.format(lib_key=lib_key), data, expect_response)

def _delete_component(self, block_key, expect_response=200):
""" Publish all changes in the specified container + children """
return self._api('delete', URL_LIB_BLOCK.format(block_key=block_key), None, expect_response)


class SharedErrorTestCases(_BaseDownstreamViewTestMixin):
"""
Expand Down Expand Up @@ -1503,3 +1508,109 @@ def test_200_summary(self):
'last_published_at': self.now.strftime('%Y-%m-%dT%H:%M:%S.%fZ'),
}]
self.assertListEqual(data, expected)


class GetDownstreamDeletedUpstream(
_BaseDownstreamViewTestMixin,
ImmediateOnCommitMixin,
SharedModuleStoreTestCase,
):
"""
Test that parent container is marked ready_to_sync when even when the only change is a deleted component under it
"""
def call_api(
self,
course_id: str | None = None,
ready_to_sync: bool | None = None,
upstream_key: str | None = None,
item_type: str | None = None,
use_top_level_parents: bool | None = None,
):
data = {}
if course_id is not None:
data["course_id"] = str(course_id)
if ready_to_sync is not None:
data["ready_to_sync"] = str(ready_to_sync)
if upstream_key is not None:
data["upstream_key"] = str(upstream_key)
if item_type is not None:
data["item_type"] = str(item_type)
if use_top_level_parents is not None:
data["use_top_level_parents"] = str(use_top_level_parents)
return self.client.get("/api/contentstore/v2/downstreams/", data=data)

def test_delete_component_should_be_ready_to_sync(self):
"""
Test deleting a component from library should mark the entire section container ready to sync
"""
# Create blocks
section_id = self._create_container(self.library_id, "section", "section-12", "Section 12")["id"]
subsection_id = self._create_container(self.library_id, "subsection", "subsection-12", "Subsection 12")["id"]
unit_id = self._create_container(self.library_id, "unit", "unit-12", "Unit 12")["id"]
video_id = self._add_block_to_library(self.library_id, "video", "video-bar-13")["id"]
section_key = ContainerKey.from_string(section_id)
subsection_key = ContainerKey.from_string(subsection_id)
unit_key = ContainerKey.from_string(unit_id)
video_key = LibraryUsageLocatorV2.from_string(video_id)

# Set children
lib_api.update_container_children(section_key, [subsection_key], None)
lib_api.update_container_children(subsection_key, [unit_key], None)
lib_api.update_container_children(unit_key, [video_key], None)
self._publish_container(unit_id)
self._publish_container(subsection_id)
self._publish_container(section_id)
self._publish_library_block(video_id)
course = CourseFactory.create(display_name="Course New")
add_users(self.superuser, CourseStaffRole(course.id), self.course_user)
chapter = BlockFactory.create(
category='chapter', parent=course, upstream=section_id, upstream_version=2,
)
sequential = BlockFactory.create(
category='sequential',
parent=chapter,
upstream=subsection_id,
upstream_version=2,
top_level_downstream_parent_key=get_block_key_string(chapter.usage_key),
)
vertical = BlockFactory.create(
category='vertical',
parent=sequential,
upstream=unit_id,
upstream_version=2,
top_level_downstream_parent_key=get_block_key_string(chapter.usage_key),
)
BlockFactory.create(
category='video',
parent=vertical,
upstream=video_id,
upstream_version=1,
top_level_downstream_parent_key=get_block_key_string(chapter.usage_key),
)
self._delete_component(video_id)
self._publish_container(unit_id)
response = self.call_api(course_id=course.id, ready_to_sync=True, use_top_level_parents=True)
assert response.status_code == 200
data = response.json()['results']
assert len(data) == 1
date_format = self.now.isoformat().split("+")[0] + 'Z'
expected_results = {
'created': date_format,
'downstream_context_key': str(course.id),
'downstream_usage_key': str(chapter.usage_key),
'downstream_customized': [],
'id': 8,
'ready_to_sync': False,
'ready_to_sync_from_children': True,
'top_level_parent_usage_key': None,
'updated': date_format,
'upstream_context_key': self.library_id,
'upstream_context_title': self.library_title,
'upstream_key': section_id,
'upstream_type': 'container',
'upstream_version': 2,
'version_declined': None,
'version_synced': 2,
}

self.assertDictEqual(data[0], expected_results)
9 changes: 8 additions & 1 deletion cms/lib/xblock/upstream_sync.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,14 +87,21 @@ class UpstreamLink:
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

@property
def is_upstream_deleted(self) -> bool:
return bool(
self.upstream_ref and
self.version_available is None
)

@property
def is_ready_to_sync_individually(self) -> bool:
return bool(
self.upstream_ref and
self.version_available and
self.version_available > (self.version_synced or 0) and
self.version_available > (self.version_declined or 0)
)
) or self.is_upstream_deleted

def _check_children_ready_to_sync(self, xblock_downstream: XBlock, return_fast: bool) -> list[dict[str, str]]:
"""
Expand Down
Loading