diff --git a/cms/djangoapps/contentstore/models.py b/cms/djangoapps/contentstore/models.py index f5ee218f3e11..a4f2ce3c6119 100644 --- a/cms/djangoapps/contentstore/models.py +++ b/cms/djangoapps/contentstore/models.py @@ -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 @@ -23,7 +23,6 @@ manual_date_time_field, ) - logger = logging.getLogger(__name__) @@ -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: @@ -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() ) @@ -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() ) diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py index ad10e373cfc8..b33d980732fa 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstreams.py @@ -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 @@ -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 @@ -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): """ @@ -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) diff --git a/cms/lib/xblock/upstream_sync.py b/cms/lib/xblock/upstream_sync.py index 8a089aeda75c..b56e0d95684d 100644 --- a/cms/lib/xblock/upstream_sync.py +++ b/cms/lib/xblock/upstream_sync.py @@ -87,6 +87,13 @@ 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( @@ -94,7 +101,7 @@ def is_ready_to_sync_individually(self) -> bool: 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]]: """