Skip to content

Commit 93f361e

Browse files
authored
fix: mark container as ready to sync if any child block is deleted (#37606)
Backport of #37603
1 parent a221c03 commit 93f361e

3 files changed

Lines changed: 134 additions & 7 deletions

File tree

cms/djangoapps/contentstore/models.py

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@
77

88
from config_models.models import ConfigurationModel
99
from django.db import models
10-
from django.db.models import QuerySet, OuterRef, Case, When, Exists, Value, ExpressionWrapper
11-
from django.db.models.fields import IntegerField, TextField, BooleanField
10+
from django.db.models import Case, Exists, ExpressionWrapper, OuterRef, Q, QuerySet, Value, When
11+
from django.db.models.fields import BooleanField, IntegerField, TextField
1212
from django.db.models.functions import Coalesce
1313
from django.db.models.lookups import GreaterThan
1414
from django.utils.translation import gettext_lazy as _
15-
from opaque_keys.edx.django.models import CourseKeyField, ContainerKeyField, UsageKeyField
15+
from opaque_keys.edx.django.models import ContainerKeyField, CourseKeyField, UsageKeyField
1616
from opaque_keys.edx.keys import CourseKey, UsageKey
1717
from opaque_keys.edx.locator import LibraryContainerLocator
1818
from openedx_learning.api.authoring import get_published_version
@@ -23,7 +23,6 @@
2323
manual_date_time_field,
2424
)
2525

26-
2726
logger = logging.getLogger(__name__)
2827

2928

@@ -391,7 +390,7 @@ def filter_links(
391390
cls.objects.filter(**link_filter).select_related(*RELATED_FIELDS),
392391
)
393392
if ready_to_sync is not None:
394-
result = result.filter(ready_to_sync=ready_to_sync)
393+
result = result.filter(Q(ready_to_sync=ready_to_sync) | Q(ready_to_sync_from_children=ready_to_sync))
395394

396395
# Handle top-level parents logic
397396
if use_top_level_parents:
@@ -436,6 +435,11 @@ def _annotate_query_with_ready_to_sync(cls, query_set: QuerySet["EntityLinkBase"
436435
),
437436
then=1
438437
),
438+
# If upstream block was deleted, set ready_to_sync = True
439+
When(
440+
Q(upstream_container__publishable_entity__published__version__version_num__isnull=True),
441+
then=1
442+
),
439443
default=0,
440444
output_field=models.IntegerField()
441445
)
@@ -457,6 +461,11 @@ def _annotate_query_with_ready_to_sync(cls, query_set: QuerySet["EntityLinkBase"
457461
),
458462
then=1
459463
),
464+
# If upstream block was deleted, set ready_to_sync = True
465+
When(
466+
Q(upstream_block__publishable_entity__published__version__version_num__isnull=True),
467+
then=1
468+
),
460469
default=0,
461470
output_field=models.IntegerField()
462471
)

cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py

Lines changed: 112 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
from common.djangoapps.student.tests.factories import UserFactory
2424
from openedx.core.djangoapps.content_libraries import api as lib_api
2525
from xmodule.modulestore.django import modulestore
26-
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase, ImmediateOnCommitMixin
26+
from xmodule.modulestore.tests.django_utils import ImmediateOnCommitMixin, SharedModuleStoreTestCase
2727
from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory
2828

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

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

281286
class SharedErrorTestCases(_BaseDownstreamViewTestMixin):
282287
"""
@@ -1503,3 +1508,109 @@ def test_200_summary(self):
15031508
'last_published_at': self.now.strftime('%Y-%m-%dT%H:%M:%S.%fZ'),
15041509
}]
15051510
self.assertListEqual(data, expected)
1511+
1512+
1513+
class GetDownstreamDeletedUpstream(
1514+
_BaseDownstreamViewTestMixin,
1515+
ImmediateOnCommitMixin,
1516+
SharedModuleStoreTestCase,
1517+
):
1518+
"""
1519+
Test that parent container is marked ready_to_sync when even when the only change is a deleted component under it
1520+
"""
1521+
def call_api(
1522+
self,
1523+
course_id: str | None = None,
1524+
ready_to_sync: bool | None = None,
1525+
upstream_key: str | None = None,
1526+
item_type: str | None = None,
1527+
use_top_level_parents: bool | None = None,
1528+
):
1529+
data = {}
1530+
if course_id is not None:
1531+
data["course_id"] = str(course_id)
1532+
if ready_to_sync is not None:
1533+
data["ready_to_sync"] = str(ready_to_sync)
1534+
if upstream_key is not None:
1535+
data["upstream_key"] = str(upstream_key)
1536+
if item_type is not None:
1537+
data["item_type"] = str(item_type)
1538+
if use_top_level_parents is not None:
1539+
data["use_top_level_parents"] = str(use_top_level_parents)
1540+
return self.client.get("/api/contentstore/v2/downstreams/", data=data)
1541+
1542+
def test_delete_component_should_be_ready_to_sync(self):
1543+
"""
1544+
Test deleting a component from library should mark the entire section container ready to sync
1545+
"""
1546+
# Create blocks
1547+
section_id = self._create_container(self.library_id, "section", "section-12", "Section 12")["id"]
1548+
subsection_id = self._create_container(self.library_id, "subsection", "subsection-12", "Subsection 12")["id"]
1549+
unit_id = self._create_container(self.library_id, "unit", "unit-12", "Unit 12")["id"]
1550+
video_id = self._add_block_to_library(self.library_id, "video", "video-bar-13")["id"]
1551+
section_key = ContainerKey.from_string(section_id)
1552+
subsection_key = ContainerKey.from_string(subsection_id)
1553+
unit_key = ContainerKey.from_string(unit_id)
1554+
video_key = LibraryUsageLocatorV2.from_string(video_id)
1555+
1556+
# Set children
1557+
lib_api.update_container_children(section_key, [subsection_key], None)
1558+
lib_api.update_container_children(subsection_key, [unit_key], None)
1559+
lib_api.update_container_children(unit_key, [video_key], None)
1560+
self._publish_container(unit_id)
1561+
self._publish_container(subsection_id)
1562+
self._publish_container(section_id)
1563+
self._publish_library_block(video_id)
1564+
course = CourseFactory.create(display_name="Course New")
1565+
add_users(self.superuser, CourseStaffRole(course.id), self.course_user)
1566+
chapter = BlockFactory.create(
1567+
category='chapter', parent=course, upstream=section_id, upstream_version=2,
1568+
)
1569+
sequential = BlockFactory.create(
1570+
category='sequential',
1571+
parent=chapter,
1572+
upstream=subsection_id,
1573+
upstream_version=2,
1574+
top_level_downstream_parent_key=get_block_key_string(chapter.usage_key),
1575+
)
1576+
vertical = BlockFactory.create(
1577+
category='vertical',
1578+
parent=sequential,
1579+
upstream=unit_id,
1580+
upstream_version=2,
1581+
top_level_downstream_parent_key=get_block_key_string(chapter.usage_key),
1582+
)
1583+
BlockFactory.create(
1584+
category='video',
1585+
parent=vertical,
1586+
upstream=video_id,
1587+
upstream_version=1,
1588+
top_level_downstream_parent_key=get_block_key_string(chapter.usage_key),
1589+
)
1590+
self._delete_component(video_id)
1591+
self._publish_container(unit_id)
1592+
response = self.call_api(course_id=course.id, ready_to_sync=True, use_top_level_parents=True)
1593+
assert response.status_code == 200
1594+
data = response.json()['results']
1595+
assert len(data) == 1
1596+
date_format = self.now.isoformat().split("+")[0] + 'Z'
1597+
expected_results = {
1598+
'created': date_format,
1599+
'downstream_context_key': str(course.id),
1600+
'downstream_usage_key': str(chapter.usage_key),
1601+
'downstream_customized': [],
1602+
'id': 8,
1603+
'ready_to_sync': False,
1604+
'ready_to_sync_from_children': True,
1605+
'top_level_parent_usage_key': None,
1606+
'updated': date_format,
1607+
'upstream_context_key': self.library_id,
1608+
'upstream_context_title': self.library_title,
1609+
'upstream_key': section_id,
1610+
'upstream_type': 'container',
1611+
'upstream_version': 2,
1612+
'version_declined': None,
1613+
'version_synced': 2,
1614+
}
1615+
1616+
self.assertDictEqual(data[0], expected_results)

cms/lib/xblock/upstream_sync.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,14 +87,21 @@ class UpstreamLink:
8787
downstream_customized: list[str] | None # List of fields modified in downstream
8888
has_top_level_parent: bool # True if this Upstream link has a top-level parent
8989

90+
@property
91+
def is_upstream_deleted(self) -> bool:
92+
return bool(
93+
self.upstream_ref and
94+
self.version_available is None
95+
)
96+
9097
@property
9198
def is_ready_to_sync_individually(self) -> bool:
9299
return bool(
93100
self.upstream_ref and
94101
self.version_available and
95102
self.version_available > (self.version_synced or 0) and
96103
self.version_available > (self.version_declined or 0)
97-
)
104+
) or self.is_upstream_deleted
98105

99106
def _check_children_ready_to_sync(self, xblock_downstream: XBlock, return_fast: bool) -> list[dict[str, str]]:
100107
"""

0 commit comments

Comments
 (0)