diff --git a/cms/djangoapps/contentstore/migrations/0012_componentlink_top_level_parent_and_more.py b/cms/djangoapps/contentstore/migrations/0012_componentlink_top_level_parent_and_more.py new file mode 100644 index 000000000000..02f94f4b6593 --- /dev/null +++ b/cms/djangoapps/contentstore/migrations/0012_componentlink_top_level_parent_and_more.py @@ -0,0 +1,24 @@ +# Generated by Django 4.2.23 on 2025-08-04 18:56 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('contentstore', '0011_enable_markdown_editor_flag_by_default'), + ] + + operations = [ + migrations.AddField( + model_name='componentlink', + name='top_level_parent', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='contentstore.containerlink'), + ), + migrations.AddField( + model_name='containerlink', + name='top_level_parent', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, to='contentstore.containerlink'), + ), + ] diff --git a/cms/djangoapps/contentstore/models.py b/cms/djangoapps/contentstore/models.py index 65f83c23b15f..f641fbee1f7f 100644 --- a/cms/djangoapps/contentstore/models.py +++ b/cms/djangoapps/contentstore/models.py @@ -1,9 +1,9 @@ """ Models for contentstore """ - - +import logging from datetime import datetime, timezone +from itertools import chain from config_models.models import ConfigurationModel from django.db import models @@ -24,6 +24,9 @@ ) +logger = logging.getLogger(__name__) + + class VideoUploadConfig(ConfigurationModel): """ Configuration for the video upload feature. @@ -98,6 +101,11 @@ class EntityLinkBase(models.Model): downstream_usage_key = UsageKeyField(max_length=255, unique=True) # Search by course/downstream key downstream_context_key = CourseKeyField(max_length=255, db_index=True) + # This is present if the creation of this link is a consequence of + # importing a container that has one or more levels of children. + # This represents the parent (container) in the top level + # at the moment of the import. + top_level_parent = models.ForeignKey("ContainerLink", on_delete=models.SET_NULL, null=True, blank=True) version_synced = models.IntegerField() version_declined = models.IntegerField(null=True, blank=True) created = manual_date_time_field() @@ -152,17 +160,27 @@ def upstream_context_title(self) -> str: @classmethod def filter_links( cls, + *, + use_top_level_parents=False, **link_filter, - ) -> QuerySet["EntityLinkBase"]: + ) -> QuerySet["EntityLinkBase"] | list["EntityLinkBase"]: """ Get all links along with sync flag, upstream context title and version, with optional filtering. + + `use_top_level_parents` is an special filter, replace any result with the top-level parent if exists. + Example: We have linkA and linkB with top-level parent as linkC, and linkD without top-level parent. + After all other filters: + Case 1: `use_top_level_parents` is False, the result is [linkA, linkB, linkC, linkD] + Case 2: `use_top_level_parents` is True, the result is [linkC, linkD] """ - ready_to_sync = link_filter.pop('ready_to_sync', None) - result = cls.objects.filter(**link_filter).select_related( + RELATED_FIELDS = [ "upstream_block__publishable_entity__published__version", "upstream_block__publishable_entity__learning_package", "upstream_block__publishable_entity__published__publish_log_record__publish_log", - ).annotate( + ] + + ready_to_sync = link_filter.pop('ready_to_sync', None) + result = cls.objects.filter(**link_filter).select_related(*RELATED_FIELDS).annotate( ready_to_sync=( GreaterThan( Coalesce("upstream_block__publishable_entity__published__version__version_num", 0), @@ -175,6 +193,27 @@ def filter_links( ) if ready_to_sync is not None: result = result.filter(ready_to_sync=ready_to_sync) + + # Handle top-level parents logic + if use_top_level_parents: + # Get objects without top_level_parent + objects_without_top_level = result.filter(top_level_parent__isnull=True) + + # Get the top-level parent keys + top_level_keys = result.filter(top_level_parent__isnull=False).values_list( + 'top_level_parent', flat=True, + ) + + # Get the top-level parents + # Any top-level parent is a container + top_level_objects = ContainerLink.filter_links(**{ + "id__in": top_level_keys + }) + + # Returns a list of `EntityLinkBase` as can be a combination of `ComponentLink`` + # and `ContainerLink`` + return list(chain(top_level_objects, objects_without_top_level)) + return result @classmethod @@ -221,6 +260,7 @@ def update_or_create( downstream_usage_key: UsageKey, downstream_context_key: CourseKey, version_synced: int, + top_level_parent_usage_key: UsageKey | None = None, version_declined: int | None = None, created: datetime | None = None, ) -> "ComponentLink": @@ -229,6 +269,15 @@ def update_or_create( """ if not created: created = datetime.now(tz=timezone.utc) + top_level_parent = None + if top_level_parent_usage_key is not None: + try: + top_level_parent = ContainerLink.get_by_downstream_usage_key( + top_level_parent_usage_key, + ) + except ContainerLink.DoesNotExist: + logger.info(f"Unable to find the link for the container with the link: {top_level_parent_usage_key}") + new_values = { 'upstream_usage_key': upstream_usage_key, 'upstream_context_key': upstream_context_key, @@ -236,6 +285,7 @@ def update_or_create( 'downstream_context_key': downstream_context_key, 'version_synced': version_synced, 'version_declined': version_declined, + 'top_level_parent': top_level_parent, } if upstream_block: new_values['upstream_block'] = upstream_block @@ -304,17 +354,55 @@ def upstream_context_title(self) -> str: @classmethod def filter_links( cls, + *, + use_top_level_parents=False, **link_filter, ) -> QuerySet["EntityLinkBase"]: """ Get all links along with sync flag, upstream context title and version, with optional filtering. + + `use_top_level_parents` is an special filter, replace any result with the top-level parent if exists. + Example: We have linkA and linkB with top-level parent as linkC and linkD without top-level parent. + After all other filters: + Case 1: `use_top_level_parents` is False, the result is [linkA, linkB, linkC, linkD] + Case 2: `use_top_level_parents` is True, the result is [linkC, linkD] """ - ready_to_sync = link_filter.pop('ready_to_sync', None) - result = cls.objects.filter(**link_filter).select_related( + RELATED_FIELDS = [ "upstream_container__publishable_entity__published__version", "upstream_container__publishable_entity__learning_package", "upstream_container__publishable_entity__published__publish_log_record__publish_log", - ).annotate( + ] + + ready_to_sync = link_filter.pop('ready_to_sync', None) + result = cls._annotate_query_with_ready_to_sync( + 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) + + # Handle top-level parents logic + if use_top_level_parents: + # Get objects without top_level_parent + objects_without_top_level = result.filter(top_level_parent__isnull=True) + + # Get the top-level parent keys + top_level_keys = result.filter(top_level_parent__isnull=False).values_list( + 'top_level_parent', flat=True, + ) + + # Get the top-level parents + # Any top-level parent is a container + top_level_objects = cls._annotate_query_with_ready_to_sync(cls.objects.filter( + id__in=top_level_keys, + ).select_related(*RELATED_FIELDS)) + + result = top_level_objects.union(objects_without_top_level) + + return result + + @classmethod + def _annotate_query_with_ready_to_sync(cls, query_set: QuerySet["EntityLinkBase"]) -> QuerySet["EntityLinkBase"]: + return query_set.annotate( ready_to_sync=( GreaterThan( Coalesce("upstream_container__publishable_entity__published__version__version_num", 0), @@ -325,9 +413,6 @@ def filter_links( ) ) ) - if ready_to_sync is not None: - result = result.filter(ready_to_sync=ready_to_sync) - return result @classmethod def summarize_by_downstream_context(cls, downstream_context_key: CourseKey) -> QuerySet: @@ -373,6 +458,7 @@ def update_or_create( downstream_usage_key: UsageKey, downstream_context_key: CourseKey, version_synced: int, + top_level_parent_usage_key: UsageKey | None = None, version_declined: int | None = None, created: datetime | None = None, ) -> "ContainerLink": @@ -381,6 +467,15 @@ def update_or_create( """ if not created: created = datetime.now(tz=timezone.utc) + top_level_parent = None + if top_level_parent_usage_key is not None: + try: + top_level_parent = ContainerLink.get_by_downstream_usage_key( + top_level_parent_usage_key, + ) + except ContainerLink.DoesNotExist: + logger.info(f"Unable to find the link for the container with the link: {top_level_parent_usage_key}") + new_values = { 'upstream_container_key': upstream_container_key, 'upstream_context_key': upstream_context_key, @@ -388,6 +483,7 @@ def update_or_create( 'downstream_context_key': downstream_context_key, 'version_synced': version_synced, 'version_declined': version_declined, + 'top_level_parent': top_level_parent, } if upstream_container_id: new_values['upstream_container_id'] = upstream_container_id @@ -409,6 +505,10 @@ def update_or_create( 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): """ diff --git a/cms/djangoapps/contentstore/rest_api/v2/serializers/downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/serializers/downstreams.py index 4024cad8ff0e..d7143a4f14bc 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/serializers/downstreams.py +++ b/cms/djangoapps/contentstore/rest_api/v2/serializers/downstreams.py @@ -14,10 +14,15 @@ class ComponentLinksSerializer(serializers.ModelSerializer): upstream_context_title = serializers.CharField(read_only=True) upstream_version = serializers.IntegerField(read_only=True, source="upstream_version_num") ready_to_sync = serializers.BooleanField() + top_level_parent_usage_key = serializers.CharField( + source='top_level_parent.downstream_usage_key', + read_only=True, + allow_null=True + ) class Meta: model = ComponentLink - exclude = ['upstream_block', 'uuid'] + exclude = ['upstream_block', 'uuid', 'top_level_parent'] class PublishableEntityLinksSummarySerializer(serializers.Serializer): @@ -38,10 +43,15 @@ class ContainerLinksSerializer(serializers.ModelSerializer): upstream_context_title = serializers.CharField(read_only=True) upstream_version = serializers.IntegerField(read_only=True, source="upstream_version_num") ready_to_sync = serializers.BooleanField() + top_level_parent_usage_key = serializers.CharField( + source='top_level_parent.downstream_usage_key', + read_only=True, + allow_null=True + ) class Meta: model = ContainerLink - exclude = ['upstream_container', 'uuid'] + exclude = ['upstream_container', 'uuid', 'top_level_parent'] class PublishableEntityLinkSerializer(serializers.Serializer): diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py index f85b68bd2654..f4ddbc7abe7f 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py @@ -168,6 +168,13 @@ class DownstreamListView(DeveloperErrorViewMixin, APIView): """ [ 🛑 UNSTABLE ] List all items (components and containers) wich are linked to an upstream context, with optional filtering. + + * `course_key_string`: Get the links of a specific course. + * `upstream_key`: Get the dowstream links of a spscific upstream component or container. + * `ready_to_sync`: Boolean to filter links that are ready to sync. + * `use_top_level_parents`: Set to True to return the top-level parents instead of downstream child, + if this parent exists. + * `item_type`: Filter the links by `components` or `containers`. """ def get(self, request: _AuthenticatedRequest): @@ -175,9 +182,11 @@ def get(self, request: _AuthenticatedRequest): Fetches publishable entity links for given course key """ course_key_string = request.GET.get('course_id') - ready_to_sync = request.GET.get('ready_to_sync') upstream_key = request.GET.get('upstream_key') + ready_to_sync = request.GET.get('ready_to_sync') + use_top_level_parents = request.GET.get('use_top_level_parents') item_type = request.GET.get('item_type') + link_filter: dict[str, CourseKey | UsageKey | LibraryContainerLocator | bool] = {} paginator = DownstreamListPaginator() @@ -197,6 +206,8 @@ def get(self, request: _AuthenticatedRequest): raise PermissionDenied if ready_to_sync is not None: link_filter["ready_to_sync"] = BooleanField().to_internal_value(ready_to_sync) + if use_top_level_parents is not None: + link_filter["use_top_level_parents"] = BooleanField().to_internal_value(use_top_level_parents) if upstream_key: try: upstream_usage_key = UsageKey.from_string(upstream_key) @@ -232,6 +243,14 @@ def get(self, request: _AuthenticatedRequest): ComponentLink.filter_links(**link_filter), ContainerLink.filter_links(**link_filter) )) + + if use_top_level_parents is not None: + # Delete duplicates. From `ComponentLink` and `ContainerLink` + # repeated containers may come in this case: + # If we have a `Unit A` and a `Component B`, if you update and publish + # both, form `ComponentLink` and `ContainerLink` you get the same `Unit A`. + links = self._remove_duplicates(links) + elif item_type == 'components': links = ComponentLink.filter_links(**link_filter) elif item_type == 'containers': @@ -240,6 +259,20 @@ def get(self, request: _AuthenticatedRequest): serializer = PublishableEntityLinkSerializer(paginated_links, many=True) return paginator.get_paginated_response(serializer.data, self.request) + def _remove_duplicates(self, links: list[EntityLinkBase]) -> list[EntityLinkBase]: + """ + Remove duplicates based on `EntityLinkBase.downstream_usage_key` + """ + seen_keys = set() + unique_links = [] + + for link in links: + if link.downstream_usage_key not in seen_keys: + seen_keys.add(link.downstream_usage_key) + unique_links.append(link) + + return unique_links + @view_auth_classes() class DownstreamComponentsListView(DeveloperErrorViewMixin, APIView): diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstream_sync_integration.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstream_sync_integration.py index b7890f821dcc..64d8b4ab5cdd 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstream_sync_integration.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_downstream_sync_integration.py @@ -2,13 +2,16 @@ Unit and integration tests to ensure that syncing content from libraries to courses is working. """ +from datetime import datetime, timezone from typing import Any from xml.etree import ElementTree import ddt from opaque_keys.edx.keys import UsageKey +from freezegun import freeze_time from openedx.core.djangoapps.content_libraries.tests import ContentLibrariesRestApiTest +from cms.djangoapps.contentstore.xblock_storage_handlers.xblock_helpers import get_block_key_dict from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import BlockFactory, CourseFactory @@ -23,33 +26,38 @@ class CourseToLibraryTestCase(ContentLibrariesRestApiTest, ModuleStoreTestCase): def setUp(self): super().setUp() + self.now = datetime.now(timezone.utc) + freezer = freeze_time(self.now) + self.addCleanup(freezer.stop) + freezer.start() # self.user is set up by ContentLibrariesRestApiTest # The source library (contains the upstreams): - self.library = self._create_library(slug="testlib", title="Upstream Library") - lib_id = self.library["id"] # the library ID as a string - self.upstream_problem1 = self._add_block_to_library(lib_id, "problem", "prob1", can_stand_alone=True) + self.library_title = "Upstream Library" + self.library = self._create_library(slug="testlib", title=self.library_title) + self.library_id = self.library["id"] # the library ID as a string + self.upstream_problem1 = self._add_block_to_library(self.library_id, "problem", "prob1", can_stand_alone=True) self._set_library_block_olx( self.upstream_problem1["id"], 'multiple choice...' ) - self.upstream_problem2 = self._add_block_to_library(lib_id, "problem", "prob2", can_stand_alone=True) + self.upstream_problem2 = self._add_block_to_library(self.library_id, "problem", "prob2", can_stand_alone=True) self._set_library_block_olx( self.upstream_problem2["id"], 'multi select...' ) - self.upstream_html1 = self._add_block_to_library(lib_id, "html", "html1", can_stand_alone=False) + self.upstream_html1 = self._add_block_to_library(self.library_id, "html", "html1", can_stand_alone=False) self._set_library_block_olx( self.upstream_html1["id"], 'This is the HTML.' ) - self.upstream_unit = self._create_container(lib_id, "unit", slug="u1", display_name="Unit 1 Title") + self.upstream_unit = self._create_container(self.library_id, "unit", slug="u1", display_name="Unit 1 Title") self._add_container_children(self.upstream_unit["id"], [ self.upstream_html1["id"], self.upstream_problem1["id"], self.upstream_problem2["id"], ]) - self._commit_library_changes(lib_id) # publish everything + self._commit_library_changes(self.library_id) # publish everything # The destination course: self.course = CourseFactory.create() @@ -100,6 +108,30 @@ def _update_course_block_fields(self, usage_key: str, fields: dict[str, Any] = N "metadata": fields, }, expect_response=200) + def _get_downstream_links( + 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, + ): + """ + Call the API to get the downstreams links + """ + 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-all/", data=data) + def assertXmlEqual(self, xml_str_a: str, xml_str_b: str) -> bool: """ Assert that the given XML strings are equal, ignoring attribute order and some whitespace variations. """ self.assertEqual( @@ -221,7 +253,10 @@ def test_unit_sync(self): """ Test that we can sync a unit from the library into the course """ + # pylint: disable=too-many-statements + # 1️⃣ Create a "vertical" block in the course based on a "unit" container: + date_format = self.now.isoformat().split("+")[0] + 'Z' downstream_unit = self._create_block_from_upstream( # The API consumer needs to specify "vertical" here, even though upstream is "unit". # In the future we could create a nicer REST API endpoint for this that's not part of @@ -230,6 +265,9 @@ def test_unit_sync(self): parent_usage_key=str(self.course_subsection.usage_key), upstream_key=self.upstream_unit["id"], ) + downstream_unit_block_key = get_block_key_dict( + UsageKey.from_string(downstream_unit["locator"]), + ) status = self._get_sync_status(downstream_unit["locator"]) self.assertDictContainsEntries(status, { 'upstream_ref': self.upstream_unit["id"], # e.g. 'lct:CL-TEST:testlib:unit:u1' @@ -247,6 +285,7 @@ def test_unit_sync(self): # Note that: # (1) Every XBlock has an "upstream" field # (2) some "downstream only" fields like weight and max_attempts are omitted. + # (3) The "top_level_downstream_parent" is the container created self.assertXmlEqual(self._get_course_block_olx(downstream_unit["locator"]), f""" This is the HTML. multiple choice... multi select... """) - # 2️⃣ Now, lets modify the upstream problem 1: + children_downstream_keys = self._get_course_block_children(downstream_unit["locator"]) + downstream_html1 = children_downstream_keys[0] + assert "type@html" in downstream_html1 + downstream_problem1 = children_downstream_keys[1] + assert "type@problem" in downstream_problem1 + downstream_problem2 = children_downstream_keys[2] + assert "type@problem" in downstream_problem2 + # Check that: The downstream links are created as expected for each component and the container + downstreams = self._get_downstream_links( + course_id=str(self.course.id) + ) + expected_downstreams = [ + { + 'id': 1, + 'upstream_context_title': self.library_title, + 'upstream_version': 2, + 'ready_to_sync': False, + 'upstream_context_key': self.library_id, + 'downstream_usage_key': downstream_html1, + 'downstream_context_key': str(self.course.id), + 'top_level_parent_usage_key': downstream_unit["locator"], + 'version_synced': 2, + 'version_declined': None, + 'created': date_format, + 'updated': date_format, + 'upstream_key': self.upstream_html1["id"], + 'upstream_type': 'component', + }, + { + 'id': 2, + 'upstream_context_title': self.library_title, + 'upstream_version': 2, + 'ready_to_sync': False, + 'upstream_context_key': self.library_id, + 'downstream_usage_key': downstream_problem1, + 'downstream_context_key': str(self.course.id), + 'top_level_parent_usage_key': downstream_unit["locator"], + 'version_synced': 2, + 'version_declined': None, + 'created': date_format, + 'updated': date_format, + 'upstream_key': self.upstream_problem1["id"], + 'upstream_type': 'component' + }, + { + 'id': 3, + 'upstream_context_title': self.library_title, + 'upstream_version': 2, + 'ready_to_sync': False, + 'upstream_context_key': self.library_id, + 'downstream_usage_key': downstream_problem2, + 'downstream_context_key': str(self.course.id), + 'top_level_parent_usage_key': downstream_unit["locator"], + 'version_synced': 2, + 'version_declined': None, + 'created': date_format, + 'updated': date_format, + 'upstream_key': self.upstream_problem2["id"], + 'upstream_type': 'component' + }, + { + 'id': 1, + 'upstream_context_title': self.library_title, + 'upstream_version': 2, + 'ready_to_sync': False, + 'upstream_context_key': self.library_id, + 'downstream_usage_key': downstream_unit["locator"], + 'downstream_context_key': str(self.course.id), + 'top_level_parent_usage_key': None, + 'version_synced': 2, + 'version_declined': None, + 'created': date_format, + 'updated': date_format, + 'upstream_key': self.upstream_unit["id"], + 'upstream_type': 'container' + } + ] + data = downstreams.json() + self.assertEqual(data["count"], 4) + self.assertListEqual(data["results"], expected_downstreams) + + # 2️⃣ Now, lets modify the upstream problem 1: self._set_library_block_olx( self.upstream_problem1["id"], 'multiple choice v2...' @@ -302,8 +425,6 @@ def test_unit_sync(self): # Check the upstream/downstream status of [one of] the children - downstream_problem1 = self._get_course_block_children(downstream_unit["locator"])[1] - assert "type@problem" in downstream_problem1 self.assertDictContainsEntries(self._get_sync_status(downstream_problem1), { 'upstream_ref': self.upstream_problem1["id"], 'version_available': 3, # <--- updated since we modified the problem @@ -313,8 +434,7 @@ def test_unit_sync(self): 'error_message': None, }) - # 3️⃣ Now, sync and check the resulting OLX of the downstream - + # Sync and check the resulting OLX of the downstream self._sync_downstream(downstream_unit["locator"]) self.assertXmlEqual(self._get_course_block_olx(downstream_unit["locator"]), f""" @@ -330,6 +450,7 @@ def test_unit_sync(self): editor="visual" upstream="{self.upstream_html1['id']}" upstream_version="2" + top_level_downstream_parent_key="{downstream_unit_block_key}" >This is the HTML. multiple choice v2... multi select... """) - # Now, add and delete a component + # Check that: The downstream link of the problem is updated and no more links are created + downstreams = self._get_downstream_links( + course_id=str(self.course.id) + ) + expected_downstreams = [ + { + 'id': 1, + 'upstream_context_title': self.library_title, + 'upstream_version': 2, + 'ready_to_sync': False, + 'upstream_context_key': self.library_id, + 'downstream_usage_key': downstream_html1, + 'downstream_context_key': str(self.course.id), + 'top_level_parent_usage_key': downstream_unit["locator"], + 'version_synced': 2, + 'version_declined': None, + 'created': date_format, + 'updated': date_format, + 'upstream_key': self.upstream_html1["id"], + 'upstream_type': 'component', + }, + { + 'id': 2, + 'upstream_context_title': self.library_title, + 'upstream_version': 3, # <--- updated + 'ready_to_sync': False, + 'upstream_context_key': self.library_id, + 'downstream_usage_key': downstream_problem1, + 'downstream_context_key': str(self.course.id), + 'top_level_parent_usage_key': downstream_unit["locator"], + 'version_synced': 3, # <--- updated + 'version_declined': None, + 'created': date_format, + 'updated': date_format, + 'upstream_key': self.upstream_problem1["id"], + 'upstream_type': 'component' + }, + { + 'id': 3, + 'upstream_context_title': self.library_title, + 'upstream_version': 2, + 'ready_to_sync': False, + 'upstream_context_key': self.library_id, + 'downstream_usage_key': downstream_problem2, + 'downstream_context_key': str(self.course.id), + 'top_level_parent_usage_key': downstream_unit["locator"], + 'version_synced': 2, + 'version_declined': None, + 'created': date_format, + 'updated': date_format, + 'upstream_key': self.upstream_problem2["id"], + 'upstream_type': 'component' + }, + { + 'id': 1, + 'upstream_context_title': self.library_title, + 'upstream_version': 2, # <--- not updated since we didn't directly modify the unit + 'ready_to_sync': False, + 'upstream_context_key': self.library_id, + 'downstream_usage_key': downstream_unit["locator"], + 'downstream_context_key': str(self.course.id), + 'top_level_parent_usage_key': None, + 'version_synced': 2, + 'version_declined': None, + 'created': date_format, + 'updated': date_format, + 'upstream_key': self.upstream_unit["id"], + 'upstream_type': 'container' + } + ] + data = downstreams.json() + self.assertEqual(data["count"], 4) + self.assertListEqual(data["results"], expected_downstreams) + + # 3️⃣ Now, add and delete a component upstream_problem3 = self._add_block_to_library( self.library["id"], "problem", @@ -376,7 +573,7 @@ def test_unit_sync(self): 'error_message': None, }) - # 3️⃣ Now, sync and check the resulting OLX of the downstream + # Sync and check the resulting OLX of the downstream self._sync_downstream(downstream_unit["locator"]) self.assertXmlEqual(self._get_course_block_olx(downstream_unit["locator"]), f""" @@ -392,6 +589,7 @@ def test_unit_sync(self): editor="visual" upstream="{self.upstream_html1['id']}" upstream_version="2" + top_level_downstream_parent_key="{downstream_unit_block_key}" >This is the HTML. multiple choice v2... @@ -410,11 +609,88 @@ def test_unit_sync(self): {self.standard_capa_attributes} upstream="{upstream_problem3['id']}" upstream_version="2" + top_level_downstream_parent_key="{downstream_unit_block_key}" >single select... """) - # Now, reorder components + # Check that: The downstream links are created deleted as expected + downstream_problem3 = self._get_course_block_children(downstream_unit["locator"])[2] + assert "type@problem" in downstream_problem3 + downstreams = self._get_downstream_links( + course_id=str(self.course.id) + ) + expected_downstreams = [ + { + 'id': 1, + 'upstream_context_title': self.library_title, + 'upstream_version': 2, + 'ready_to_sync': False, + 'upstream_context_key': self.library_id, + 'downstream_usage_key': downstream_html1, + 'downstream_context_key': str(self.course.id), + 'top_level_parent_usage_key': downstream_unit["locator"], + 'version_synced': 2, + 'version_declined': None, + 'created': date_format, + 'updated': date_format, + 'upstream_key': self.upstream_html1["id"], + 'upstream_type': 'component', + }, + { + 'id': 2, + 'upstream_context_title': self.library_title, + 'upstream_version': 3, + 'ready_to_sync': False, + 'upstream_context_key': self.library_id, + 'downstream_usage_key': downstream_problem1, + 'downstream_context_key': str(self.course.id), + 'top_level_parent_usage_key': downstream_unit["locator"], + 'version_synced': 3, + 'version_declined': None, + 'created': date_format, + 'updated': date_format, + 'upstream_key': self.upstream_problem1["id"], + 'upstream_type': 'component' + }, + { + 'id': 4, + 'upstream_context_title': self.library_title, + 'upstream_version': 2, + 'ready_to_sync': False, + 'upstream_context_key': self.library_id, + 'downstream_usage_key': downstream_problem3, + 'downstream_context_key': str(self.course.id), + 'top_level_parent_usage_key': downstream_unit["locator"], + 'version_synced': 2, + 'version_declined': None, + 'created': date_format, + 'updated': date_format, + 'upstream_key': upstream_problem3["id"], + 'upstream_type': 'component' + }, + { + 'id': 1, + 'upstream_context_title': self.library_title, + 'upstream_version': 4, # <--- updated + 'ready_to_sync': False, + 'upstream_context_key': self.library_id, + 'downstream_usage_key': downstream_unit["locator"], + 'downstream_context_key': str(self.course.id), + 'top_level_parent_usage_key': None, + 'version_synced': 4, # <--- updated + 'version_declined': None, + 'created': date_format, + 'updated': date_format, + 'upstream_key': self.upstream_unit["id"], + 'upstream_type': 'container' + } + ] + data = downstreams.json() + self.assertEqual(data["count"], 4) + self.assertListEqual(data["results"], expected_downstreams) + + # 4️⃣ Now, reorder components self._patch_container_components(self.upstream_unit["id"], [ upstream_problem3["id"], self.upstream_problem1["id"], @@ -422,7 +698,7 @@ def test_unit_sync(self): ]) self._publish_container(self.upstream_unit["id"]) - # 3️⃣ Now, sync and check the resulting OLX of the downstream + # Sync and check the resulting OLX of the downstream self._sync_downstream(downstream_unit["locator"]) self.assertXmlEqual(self._get_course_block_olx(downstream_unit["locator"]), f""" @@ -440,6 +716,7 @@ def test_unit_sync(self): {self.standard_capa_attributes} upstream="{upstream_problem3['id']}" upstream_version="2" + top_level_downstream_parent_key="{downstream_unit_block_key}" >single select... multiple choice v2... This is the HTML. """) + + # Check that: The downstream link of the unit is updated and no more links are created + downstreams = self._get_downstream_links( + course_id=str(self.course.id) + ) + expected_downstreams = [ + { + 'id': 1, + 'upstream_context_title': self.library_title, + 'upstream_version': 2, + 'ready_to_sync': False, + 'upstream_context_key': self.library_id, + 'downstream_usage_key': downstream_html1, + 'downstream_context_key': str(self.course.id), + 'top_level_parent_usage_key': downstream_unit["locator"], + 'version_synced': 2, + 'version_declined': None, + 'created': date_format, + 'updated': date_format, + 'upstream_key': self.upstream_html1["id"], + 'upstream_type': 'component', + }, + { + 'id': 2, + 'upstream_context_title': self.library_title, + 'upstream_version': 3, + 'ready_to_sync': False, + 'upstream_context_key': self.library_id, + 'downstream_usage_key': downstream_problem1, + 'downstream_context_key': str(self.course.id), + 'top_level_parent_usage_key': downstream_unit["locator"], + 'version_synced': 3, + 'version_declined': None, + 'created': date_format, + 'updated': date_format, + 'upstream_key': self.upstream_problem1["id"], + 'upstream_type': 'component' + }, + { + 'id': 4, + 'upstream_context_title': self.library_title, + 'upstream_version': 2, + 'ready_to_sync': False, + 'upstream_context_key': self.library_id, + 'downstream_usage_key': downstream_problem3, + 'downstream_context_key': str(self.course.id), + 'top_level_parent_usage_key': downstream_unit["locator"], + 'version_synced': 2, + 'version_declined': None, + 'created': date_format, + 'updated': date_format, + 'upstream_key': upstream_problem3["id"], + 'upstream_type': 'component' + }, + { + 'id': 1, + 'upstream_context_title': self.library_title, + 'upstream_version': 5, # <--- updated + 'ready_to_sync': False, + 'upstream_context_key': self.library_id, + 'downstream_usage_key': downstream_unit["locator"], + 'downstream_context_key': str(self.course.id), + 'top_level_parent_usage_key': None, + 'version_synced': 5, # <--- updated + 'version_declined': None, + 'created': date_format, + 'updated': date_format, + 'upstream_key': self.upstream_unit["id"], + 'upstream_type': 'container' + } + ] + data = downstreams.json() + self.assertEqual(data["count"], 4) + self.assertListEqual(data["results"], expected_downstreams) 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 0b62b5ba1adb..5cd953aedd43 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 @@ -15,6 +15,7 @@ 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 @@ -92,11 +93,29 @@ def setUp(self): self.unit_id = self._create_container(self.library_id, "unit", "unit-1", "Unit 1")["id"] self.subsection_id = self._create_container(self.library_id, "subsection", "subsection-1", "Subsection 1")["id"] self.section_id = self._create_container(self.library_id, "section", "section-1", "Section 1")["id"] + + # Creating container to test the top-level parent + self.top_level_unit_id = self._create_container(self.library_id, "unit", "unit-2", "Unit 2")["id"] + self.top_level_subsection_id = self._create_container( + self.library_id, + "subsection", + "subsection-2", + "Subsection 2", + )["id"] + self.top_level_section_id = self._create_container(self.library_id, "section", "section-2", "Section 2")["id"] + self.html_lib_id_2 = self._add_block_to_library(self.library_id, "html", "html-baz-2")["id"] + self.video_lib_id_2 = self._add_block_to_library(self.library_id, "video", "video-baz-2")["id"] + self._publish_library_block(self.html_lib_id) self._publish_library_block(self.video_lib_id) + self._publish_library_block(self.html_lib_id_2) + self._publish_library_block(self.video_lib_id_2) self._publish_container(self.unit_id) self._publish_container(self.subsection_id) self._publish_container(self.section_id) + self._publish_container(self.top_level_unit_id) + self._publish_container(self.top_level_subsection_id) + self._publish_container(self.top_level_section_id) self.mock_upstream_link = f"{settings.COURSE_AUTHORING_MICROFRONTEND_URL}/library/{self.library_id}/components?usageKey={self.video_lib_id}" # pylint: disable=line-too-long # noqa: E501 self.course = CourseFactory.create() add_users(self.superuser, CourseStaffRole(self.course.id), self.course_user) @@ -120,6 +139,47 @@ def setUp(self): category='vertical', parent=sequential, upstream=self.unit_id, upstream_version=1, ).usage_key + # Creating Blocks with top-level-parents + self.top_level_downstream_chapter = BlockFactory.create( + category='chapter', parent=self.course, upstream=self.top_level_section_id, upstream_version=1, + ) + self.top_level_downstream_sequential = BlockFactory.create( + category='sequential', + parent=self.top_level_downstream_chapter, + upstream=self.top_level_subsection_id, + upstream_version=1, + top_level_downstream_parent_key=get_block_key_dict( + self.top_level_downstream_chapter.usage_key, + ), + ) + self.top_level_downstream_unit = BlockFactory.create( + category='vertical', + parent=self.top_level_downstream_sequential, + upstream=self.top_level_unit_id, + upstream_version=1, + top_level_downstream_parent_key=get_block_key_dict( + self.top_level_downstream_sequential.usage_key, + ) + ) + self.top_level_downstream_html_key = BlockFactory.create( + category='html', + parent=self.top_level_downstream_unit, + upstream=self.html_lib_id_2, + upstream_version=1, + top_level_downstream_parent_key=get_block_key_dict( + self.top_level_downstream_unit.usage_key, + ) + ).usage_key + self.top_level_downstream_video_key = BlockFactory.create( + category='video', + parent=self.top_level_downstream_unit, + upstream=self.video_lib_id_2, + upstream_version=1, + top_level_downstream_parent_key=get_block_key_dict( + self.top_level_downstream_chapter.usage_key, + ) + ).usage_key + self.another_course = CourseFactory.create(display_name="Another Course") another_chapter = BlockFactory.create(category="chapter", parent=self.another_course) another_sequential = BlockFactory.create(category="sequential", parent=another_chapter) @@ -498,6 +558,7 @@ def call_api( 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: @@ -508,6 +569,8 @@ def call_api( 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-all/", data=data) def test_200_all_downstreams_for_a_course(self): @@ -533,7 +596,8 @@ def test_200_all_downstreams_for_a_course(self): 'upstream_type': 'component', 'upstream_version': 1, 'version_declined': None, - 'version_synced': 1 + 'version_synced': 1, + 'top_level_parent_usage_key': None, }, { 'created': date_format, @@ -549,6 +613,39 @@ def test_200_all_downstreams_for_a_course(self): 'upstream_version': 2, 'version_declined': None, 'version_synced': 1, + 'top_level_parent_usage_key': None, + }, + { + 'created': date_format, + 'downstream_context_key': str(self.course.id), + 'downstream_usage_key': str(self.top_level_downstream_html_key), + 'id': 3, + 'ready_to_sync': False, + 'updated': date_format, + 'upstream_context_key': self.library_id, + 'upstream_context_title': self.library_title, + 'upstream_key': self.html_lib_id_2, + 'upstream_type': 'component', + 'upstream_version': 1, + 'version_declined': None, + 'version_synced': 1, + 'top_level_parent_usage_key': str(self.top_level_downstream_unit.usage_key), + }, + { + 'created': date_format, + 'downstream_context_key': str(self.course.id), + 'downstream_usage_key': str(self.top_level_downstream_video_key), + 'id': 4, + 'ready_to_sync': False, + 'updated': date_format, + 'upstream_context_key': self.library_id, + 'upstream_context_title': self.library_title, + 'upstream_key': self.video_lib_id_2, + 'upstream_type': 'component', + 'upstream_version': 1, + 'version_declined': None, + 'version_synced': 1, + 'top_level_parent_usage_key': str(self.top_level_downstream_chapter.usage_key), }, { 'created': date_format, @@ -564,6 +661,7 @@ def test_200_all_downstreams_for_a_course(self): 'upstream_version': 1, 'version_declined': None, 'version_synced': 1, + 'top_level_parent_usage_key': None, }, { 'created': date_format, @@ -579,6 +677,7 @@ def test_200_all_downstreams_for_a_course(self): 'upstream_version': 1, 'version_declined': None, 'version_synced': 1, + 'top_level_parent_usage_key': None, }, { 'created': date_format, @@ -593,11 +692,60 @@ def test_200_all_downstreams_for_a_course(self): 'upstream_type': 'container', 'upstream_version': 2, 'version_declined': None, - 'version_synced': 1 + 'version_synced': 1, + 'top_level_parent_usage_key': None, + }, + { + 'created': date_format, + 'downstream_context_key': str(self.course.id), + 'downstream_usage_key': str(self.top_level_downstream_chapter.usage_key), + 'id': 4, + 'ready_to_sync': False, + 'updated': date_format, + 'upstream_context_key': self.library_id, + 'upstream_context_title': self.library_title, + 'upstream_key': self.top_level_section_id, + 'upstream_type': 'container', + 'upstream_version': 1, + 'version_declined': None, + 'version_synced': 1, + 'top_level_parent_usage_key': None, + }, + { + 'created': date_format, + 'downstream_context_key': str(self.course.id), + 'downstream_usage_key': str(self.top_level_downstream_sequential.usage_key), + 'id': 5, + 'ready_to_sync': False, + 'updated': date_format, + 'upstream_context_key': self.library_id, + 'upstream_context_title': self.library_title, + 'upstream_key': self.top_level_subsection_id, + 'upstream_type': 'container', + 'upstream_version': 1, + 'version_declined': None, + 'version_synced': 1, + 'top_level_parent_usage_key': str(self.top_level_downstream_chapter.usage_key), + }, + { + 'created': date_format, + 'downstream_context_key': str(self.course.id), + 'downstream_usage_key': str(self.top_level_downstream_unit.usage_key), + 'id': 6, + 'ready_to_sync': False, + 'updated': date_format, + 'upstream_context_key': self.library_id, + 'upstream_context_title': self.library_title, + 'upstream_key': self.top_level_unit_id, + 'upstream_type': 'container', + 'upstream_version': 1, + 'version_declined': None, + 'version_synced': 1, + 'top_level_parent_usage_key': str(self.top_level_downstream_sequential.usage_key), }, ] self.assertListEqual(data["results"], expected) - self.assertEqual(data["count"], 5) + self.assertEqual(data["count"], 10) def test_permission_denied_with_course_filter(self): self.client.login(username="simple_user", password="password") @@ -630,7 +778,8 @@ def test_200_component_downstreams_for_a_course(self): 'upstream_type': 'component', 'upstream_version': 1, 'version_declined': None, - 'version_synced': 1 + 'version_synced': 1, + 'top_level_parent_usage_key': None, }, { 'created': date_format, @@ -646,10 +795,43 @@ def test_200_component_downstreams_for_a_course(self): 'upstream_version': 2, 'version_declined': None, 'version_synced': 1, + 'top_level_parent_usage_key': None, + }, + { + 'created': date_format, + 'downstream_context_key': str(self.course.id), + 'downstream_usage_key': str(self.top_level_downstream_html_key), + 'id': 3, + 'ready_to_sync': False, + 'updated': date_format, + 'upstream_context_key': self.library_id, + 'upstream_context_title': self.library_title, + 'upstream_key': self.html_lib_id_2, + 'upstream_type': 'component', + 'upstream_version': 1, + 'version_declined': None, + 'version_synced': 1, + 'top_level_parent_usage_key': str(self.top_level_downstream_unit.usage_key), + }, + { + 'created': date_format, + 'downstream_context_key': str(self.course.id), + 'downstream_usage_key': str(self.top_level_downstream_video_key), + 'id': 4, + 'ready_to_sync': False, + 'updated': date_format, + 'upstream_context_key': self.library_id, + 'upstream_context_title': self.library_title, + 'upstream_key': self.video_lib_id_2, + 'upstream_type': 'component', + 'upstream_version': 1, + 'version_declined': None, + 'version_synced': 1, + 'top_level_parent_usage_key': str(self.top_level_downstream_chapter.usage_key), }, ] self.assertListEqual(data["results"], expected) - self.assertEqual(data["count"], 2) + self.assertEqual(data["count"], 4) def test_200_container_downstreams_for_a_course(self): """ @@ -678,6 +860,7 @@ def test_200_container_downstreams_for_a_course(self): 'upstream_version': 1, 'version_declined': None, 'version_synced': 1, + 'top_level_parent_usage_key': None, }, { 'created': date_format, @@ -693,6 +876,7 @@ def test_200_container_downstreams_for_a_course(self): 'upstream_version': 1, 'version_declined': None, 'version_synced': 1, + 'top_level_parent_usage_key': None, }, { 'created': date_format, @@ -707,11 +891,60 @@ def test_200_container_downstreams_for_a_course(self): 'upstream_type': 'container', 'upstream_version': 2, 'version_declined': None, - 'version_synced': 1 + 'version_synced': 1, + 'top_level_parent_usage_key': None, + }, + { + 'created': date_format, + 'downstream_context_key': str(self.course.id), + 'downstream_usage_key': str(self.top_level_downstream_chapter.usage_key), + 'id': 4, + 'ready_to_sync': False, + 'updated': date_format, + 'upstream_context_key': self.library_id, + 'upstream_context_title': self.library_title, + 'upstream_key': self.top_level_section_id, + 'upstream_type': 'container', + 'upstream_version': 1, + 'version_declined': None, + 'version_synced': 1, + 'top_level_parent_usage_key': None, + }, + { + 'created': date_format, + 'downstream_context_key': str(self.course.id), + 'downstream_usage_key': str(self.top_level_downstream_sequential.usage_key), + 'id': 5, + 'ready_to_sync': False, + 'updated': date_format, + 'upstream_context_key': self.library_id, + 'upstream_context_title': self.library_title, + 'upstream_key': self.top_level_subsection_id, + 'upstream_type': 'container', + 'upstream_version': 1, + 'version_declined': None, + 'version_synced': 1, + 'top_level_parent_usage_key': str(self.top_level_downstream_chapter.usage_key), + }, + { + 'created': date_format, + 'downstream_context_key': str(self.course.id), + 'downstream_usage_key': str(self.top_level_downstream_unit.usage_key), + 'id': 6, + 'ready_to_sync': False, + 'updated': date_format, + 'upstream_context_key': self.library_id, + 'upstream_context_title': self.library_title, + 'upstream_key': self.top_level_unit_id, + 'upstream_type': 'container', + 'upstream_version': 1, + 'version_declined': None, + 'version_synced': 1, + 'top_level_parent_usage_key': str(self.top_level_downstream_sequential.usage_key), }, ] self.assertListEqual(data["results"], expected) - self.assertEqual(data["count"], 3) + self.assertEqual(data["count"], 6) @ddt.data( ('all', 2), @@ -764,52 +997,64 @@ def test_200_container_downstream_context_list(self): self.assertListEqual(got, expected) self.assertEqual(data["count"], 1) - -class GetComponentUpstreamViewTest( - _BaseDownstreamViewTestMixin, - SharedModuleStoreTestCase, -): - """ - Test that `GET /api/v2/contentstore/downstreams?...` returns list of component links based on the provided filter. - """ - def call_api( - self, - course_id: str | None = None, - ready_to_sync: bool | None = None, - upstream_usage_key: str | 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_usage_key is not None: - data["upstream_usage_key"] = str(upstream_usage_key) - return self.client.get("/api/contentstore/v2/downstreams/", data=data) - - def test_200_all_component_downstreams_for_a_course(self): + def test_200_get_ready_to_sync_top_level_parents_with_components(self): """ - Returns all component links for given course + Returns all links that are syncable using the top-level parents of components """ self.client.login(username="superuser", password="password") - response = self.call_api(course_id=self.course.id) + + # Publish components + self._set_library_block_olx(self.html_lib_id_2, "Hello world!") + self._publish_library_block(self.html_lib_id_2) + self._set_library_block_olx(self.video_lib_id_2, "") + self._publish_library_block(self.video_lib_id_2) + + response = self.call_api( + ready_to_sync=True, + item_type="all", + use_top_level_parents=True, + ) assert response.status_code == 200 data = response.json() + self.assertEqual(data["count"], 4) date_format = self.now.isoformat().split("+")[0] + 'Z' + + # The expected results are + # * The section that is the top-level parent of `video_lib_id_2` + # * The unit that is the top-level parent of `html_lib_id_2` + # * 2 links without top-level parents expected = [ { 'created': date_format, 'downstream_context_key': str(self.course.id), - 'downstream_usage_key': str(self.downstream_video_key), - 'id': 1, + 'downstream_usage_key': str(self.top_level_downstream_chapter.usage_key), + 'id': 4, 'ready_to_sync': False, 'updated': date_format, 'upstream_context_key': self.library_id, 'upstream_context_title': self.library_title, - 'upstream_usage_key': self.video_lib_id, + 'upstream_key': self.top_level_section_id, + 'upstream_type': 'container', 'upstream_version': 1, 'version_declined': None, - 'version_synced': 1 + 'version_synced': 1, + 'top_level_parent_usage_key': None, + }, + { + 'created': date_format, + 'downstream_context_key': str(self.course.id), + 'downstream_usage_key': str(self.top_level_downstream_unit.usage_key), + 'id': 6, + 'ready_to_sync': False, + 'updated': date_format, + 'upstream_context_key': self.library_id, + 'upstream_context_title': self.library_title, + 'upstream_key': self.top_level_unit_id, + 'upstream_type': 'container', + 'upstream_version': 1, + 'version_declined': None, + 'version_synced': 1, + 'top_level_parent_usage_key': str(self.top_level_downstream_sequential.usage_key), }, { 'created': date_format, @@ -820,139 +1065,171 @@ def test_200_all_component_downstreams_for_a_course(self): 'updated': date_format, 'upstream_context_key': self.library_id, 'upstream_context_title': self.library_title, - 'upstream_usage_key': self.html_lib_id, + 'upstream_key': self.html_lib_id, + 'upstream_type': 'component', + 'upstream_version': 2, + 'version_declined': None, + 'version_synced': 1, + 'top_level_parent_usage_key': None, + }, + { + 'created': date_format, + 'downstream_context_key': str(self.course.id), + 'downstream_usage_key': str(self.downstream_unit_key), + 'id': 3, + 'ready_to_sync': True, + 'updated': date_format, + 'upstream_context_key': self.library_id, + 'upstream_context_title': self.library_title, + 'upstream_key': self.unit_id, + 'upstream_type': 'container', 'upstream_version': 2, 'version_declined': None, 'version_synced': 1, + 'top_level_parent_usage_key': None, }, ] self.assertListEqual(data["results"], expected) - self.assertEqual(data["count"], 2) - def test_200_all_component_downstreams_ready_to_sync(self): + def test_200_get_ready_to_sync_top_level_parents_with_containers(self): """ - Returns all component links that are syncable + Returns all links that are syncable using the top-level parents of containers """ self.client.login(username="superuser", password="password") - response = self.call_api(ready_to_sync=True) - assert response.status_code == 200 - data = response.json() - self.assertTrue(all(o["ready_to_sync"] for o in data["results"])) - self.assertEqual(data["count"], 1) - def test_200_component_downstream_context_list(self): - """ - Returns all component downstream courses for given library block - """ - self.client.login(username="superuser", password="password") - response = self.call_api(upstream_usage_key=self.video_lib_id) + # Publish Subsection + self._update_container(self.top_level_subsection_id, display_name="Subsection 3") + self._publish_container(self.top_level_subsection_id) + + response = self.call_api( + ready_to_sync=True, + item_type="all", + use_top_level_parents=True, + ) assert response.status_code == 200 data = response.json() - expected = [str(self.downstream_video_key)] + [str(key) for key in self.another_video_keys] - got = [str(o["downstream_usage_key"]) for o in data["results"]] - self.assertListEqual(got, expected) - self.assertEqual(data["count"], 4) - + self.assertEqual(data["count"], 3) + date_format = self.now.isoformat().split("+")[0] + 'Z' -class GetDownstreamSummaryViewTest( - _BaseDownstreamViewTestMixin, - SharedModuleStoreTestCase, -): - """ - Test that `GET /api/v2/contentstore/downstreams//summary` returns summary of links in course. - """ - def call_api(self, course_id): - return self.client.get(f"/api/contentstore/v2/downstreams/{course_id}/summary") + # The expected results are + # * 2 links without top-level parents + # * The section that is the top-level parent of `top_level_subsection_id` + expected = [ + { + 'created': date_format, + 'downstream_context_key': str(self.course.id), + 'downstream_usage_key': str(self.downstream_html_key), + 'id': 2, + 'ready_to_sync': True, + 'updated': date_format, + 'upstream_context_key': self.library_id, + 'upstream_context_title': self.library_title, + 'upstream_key': self.html_lib_id, + 'upstream_type': 'component', + 'upstream_version': 2, + 'version_declined': None, + 'version_synced': 1, + 'top_level_parent_usage_key': None, + }, + { + 'created': date_format, + 'downstream_context_key': str(self.course.id), + 'downstream_usage_key': str(self.downstream_unit_key), + 'id': 3, + 'ready_to_sync': True, + 'updated': date_format, + 'upstream_context_key': self.library_id, + 'upstream_context_title': self.library_title, + 'upstream_key': self.unit_id, + 'upstream_type': 'container', + 'upstream_version': 2, + 'version_declined': None, + 'version_synced': 1, + 'top_level_parent_usage_key': None, + }, + { + 'created': date_format, + 'downstream_context_key': str(self.course.id), + 'downstream_usage_key': str(self.top_level_downstream_chapter.usage_key), + 'id': 4, + 'ready_to_sync': False, + 'updated': date_format, + 'upstream_context_key': self.library_id, + 'upstream_context_title': self.library_title, + 'upstream_key': self.top_level_section_id, + 'upstream_type': 'container', + 'upstream_version': 1, + 'version_declined': None, + 'version_synced': 1, + 'top_level_parent_usage_key': None, + }, + ] + self.assertListEqual(data["results"], expected) - @patch.object(UpstreamLink, "get_for_block", _get_upstream_link_good_and_syncable) - def test_200_summary(self): + def test_200_get_ready_to_sync_duplicated_top_level_parents(self): """ - Does the happy path work? + Returns all links that are syncable using the same top-level parents + + According to the requirements, only the top-level parents should be displayed. + Even if all containers and components within a section are updated, only the top-level parent, + which is the section, should be displayed. + This test checks that only the top-level parent is displayed and is not duplicated in the result. """ self.client.login(username="superuser", password="password") - response = self.call_api(str(self.another_course.id)) - assert response.status_code == 200 - data = response.json() - expected = [{ - 'upstream_context_title': 'Test Library 1', - 'upstream_context_key': self.library_id, - 'ready_to_sync_count': 0, - 'total_count': 3, - 'last_published_at': self.now.strftime('%Y-%m-%dT%H:%M:%S.%fZ'), - }] - self.assertListEqual(data, expected) - response = self.call_api(str(self.course.id)) - assert response.status_code == 200 - data = response.json() - expected = [{ - 'upstream_context_title': 'Test Library 1', - 'upstream_context_key': self.library_id, - 'ready_to_sync_count': 2, - 'total_count': 5, - 'last_published_at': self.now.strftime('%Y-%m-%dT%H:%M:%S.%fZ'), - }] - self.assertListEqual(data, expected) + # Publish Section and component/subsection that has the same section as top-level parent + self._update_container(self.top_level_section_id, display_name="Section 3") + self._publish_container(self.top_level_section_id) + self._set_library_block_olx(self.video_lib_id_2, "") + self._publish_library_block(self.video_lib_id_2) + self._update_container(self.top_level_subsection_id, display_name="Subsection 3") + self._publish_container(self.top_level_subsection_id) -class GetContainerUpstreamViewTest( - _BaseDownstreamViewTestMixin, - SharedModuleStoreTestCase, -): - """ - Test that `GET /api/v2/contentstore/downstream-containers?...` returns list of links based on the provided filter. - """ - def call_api( - self, - course_id: str | None = None, - ready_to_sync: bool | None = None, - upstream_container_key: str | 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_container_key is not None: - data["upstream_container_key"] = str(upstream_container_key) - return self.client.get("/api/contentstore/v2/downstream-containers/", data=data) - - def test_200_all_container_downstreams_for_a_course(self): - """ - Returns all container links for given course - """ - self.client.login(username="superuser", password="password") - response = self.call_api(course_id=self.course.id) + response = self.call_api( + ready_to_sync=True, + item_type="all", + use_top_level_parents=True, + ) assert response.status_code == 200 data = response.json() + self.assertEqual(data["count"], 3) date_format = self.now.isoformat().split("+")[0] + 'Z' + + # The expected results are + # * The section that is the top-level parent of `video_lib_id_2` and `top_level_subsection_id` + # * 2 links without top-level parents expected = [ { 'created': date_format, 'downstream_context_key': str(self.course.id), - 'downstream_usage_key': str(self.downstream_chapter_key), - 'id': 1, - 'ready_to_sync': False, + 'downstream_usage_key': str(self.top_level_downstream_chapter.usage_key), + 'id': 4, + 'ready_to_sync': True, 'updated': date_format, 'upstream_context_key': self.library_id, 'upstream_context_title': self.library_title, - 'upstream_container_key': self.section_id, - 'upstream_version': 1, + 'upstream_key': self.top_level_section_id, + 'upstream_type': 'container', + 'upstream_version': 2, 'version_declined': None, 'version_synced': 1, + 'top_level_parent_usage_key': None, }, { 'created': date_format, 'downstream_context_key': str(self.course.id), - 'downstream_usage_key': str(self.downstream_sequential_key), + 'downstream_usage_key': str(self.downstream_html_key), 'id': 2, - 'ready_to_sync': False, + 'ready_to_sync': True, 'updated': date_format, 'upstream_context_key': self.library_id, 'upstream_context_title': self.library_title, - 'upstream_container_key': self.subsection_id, - 'upstream_version': 1, + 'upstream_key': self.html_lib_id, + 'upstream_type': 'component', + 'upstream_version': 2, 'version_declined': None, 'version_synced': 1, + 'top_level_parent_usage_key': None, }, { 'created': date_format, @@ -963,22 +1240,52 @@ def test_200_all_container_downstreams_for_a_course(self): 'updated': date_format, 'upstream_context_key': self.library_id, 'upstream_context_title': self.library_title, - 'upstream_container_key': self.unit_id, + 'upstream_key': self.unit_id, + 'upstream_type': 'container', 'upstream_version': 2, 'version_declined': None, - 'version_synced': 1 + 'version_synced': 1, + 'top_level_parent_usage_key': None, }, ] self.assertListEqual(data["results"], expected) - self.assertEqual(data["count"], 3) - def test_200_all_downstreams_ready_to_sync(self): + +class GetDownstreamSummaryViewTest( + _BaseDownstreamViewTestMixin, + SharedModuleStoreTestCase, +): + """ + Test that `GET /api/v2/contentstore/downstreams//summary` returns summary of links in course. + """ + def call_api(self, course_id): + return self.client.get(f"/api/contentstore/v2/downstreams/{course_id}/summary") + + @patch.object(UpstreamLink, "get_for_block", _get_upstream_link_good_and_syncable) + def test_200_summary(self): """ - Returns all links that are syncable + Does the happy path work? """ self.client.login(username="superuser", password="password") - response = self.call_api(ready_to_sync=True) + response = self.call_api(str(self.another_course.id)) assert response.status_code == 200 data = response.json() - self.assertTrue(all(o["ready_to_sync"] for o in data["results"])) - self.assertEqual(data["count"], 1) + expected = [{ + 'upstream_context_title': 'Test Library 1', + 'upstream_context_key': self.library_id, + 'ready_to_sync_count': 0, + 'total_count': 3, + 'last_published_at': self.now.strftime('%Y-%m-%dT%H:%M:%S.%fZ'), + }] + self.assertListEqual(data, expected) + response = self.call_api(str(self.course.id)) + assert response.status_code == 200 + data = response.json() + expected = [{ + 'upstream_context_title': 'Test Library 1', + 'upstream_context_key': self.library_id, + 'ready_to_sync_count': 2, + 'total_count': 10, + 'last_published_at': self.now.strftime('%Y-%m-%dT%H:%M:%S.%fZ'), + }] + self.assertListEqual(data, expected) diff --git a/cms/djangoapps/contentstore/signals/handlers.py b/cms/djangoapps/contentstore/signals/handlers.py index 263bf632f585..0dd9eaa9c519 100644 --- a/cms/djangoapps/contentstore/signals/handlers.py +++ b/cms/djangoapps/contentstore/signals/handlers.py @@ -49,7 +49,8 @@ from ..models import ComponentLink, ContainerLink from ..tasks import ( create_or_update_upstream_links, - handle_create_or_update_xblock_upstream_link, + handle_create_xblock_upstream_link, + handle_update_xblock_upstream_link, handle_unlink_upstream_block, handle_unlink_upstream_container, ) @@ -260,17 +261,29 @@ def handle_grading_policy_changed(sender, **kwargs): @receiver(XBLOCK_CREATED) +def create_upstream_downstream_link_handler(**kwargs): + """ + Automatically create upstream->downstream link in database. + """ + xblock_info = kwargs.get("xblock_info", None) + if not xblock_info or not isinstance(xblock_info, XBlockData): + log.error("Received null or incorrect data for event") + return + + handle_create_xblock_upstream_link.delay(str(xblock_info.usage_key)) + + @receiver(XBLOCK_UPDATED) -def create_or_update_upstream_downstream_link_handler(**kwargs): +def update_upstream_downstream_link_handler(**kwargs): """ - Automatically create or update upstream->downstream link in database. + Automatically update upstream->downstream link in database. """ xblock_info = kwargs.get("xblock_info", None) if not xblock_info or not isinstance(xblock_info, XBlockData): log.error("Received null or incorrect data for event") return - handle_create_or_update_xblock_upstream_link.delay(str(xblock_info.usage_key)) + handle_update_xblock_upstream_link.delay(str(xblock_info.usage_key)) @receiver(XBLOCK_DELETED) diff --git a/cms/djangoapps/contentstore/tasks.py b/cms/djangoapps/contentstore/tasks.py index 1b5c1061e78a..69df4cfd43cf 100644 --- a/cms/djangoapps/contentstore/tasks.py +++ b/cms/djangoapps/contentstore/tasks.py @@ -34,7 +34,7 @@ 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 +from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocator, BlockUsageLocator from organizations.api import add_organization_course, ensure_organization from organizations.exceptions import InvalidOrganizationException from organizations.models import Organization @@ -1598,11 +1598,42 @@ def _write_broken_links_to_file(broken_or_locked_urls, broken_links_file): @shared_task @set_code_owner_attribute -def handle_create_or_update_xblock_upstream_link(usage_key): +def handle_create_xblock_upstream_link(usage_key): """ - Create or update upstream link for a single xblock. + Create upstream link for a single xblock. + If the xblock has top-level parent, verify if the link for the parent is created, + if not, create it before any subsequent operation. """ - ensure_cms("handle_create_or_update_xblock_upstream_link may only be executed in a CMS context") + ensure_cms("handle_create_xblock_upstream_link may only be executed in a CMS context") + try: + xblock = modulestore().get_item(UsageKey.from_string(usage_key)) + except (ItemNotFoundError, InvalidKeyError): + LOGGER.exception(f'Could not find item for given usage_key: {usage_key}') + return + if not xblock.upstream or not xblock.upstream_version: + return + if xblock.top_level_downstream_parent_key is not None: + top_level_parent_usage_key = BlockUsageLocator( + xblock.course_id, + xblock.top_level_downstream_parent_key.get('type'), + xblock.top_level_downstream_parent_key.get('id'), + ) + try: + ContainerLink.get_by_downstream_usage_key(top_level_parent_usage_key) + except ContainerLink.DoesNotExist: + # 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) + + +@shared_task +@set_code_owner_attribute +def handle_update_xblock_upstream_link(usage_key): + """ + Update upstream link for a single xblock. + """ + ensure_cms("handle_update_xblock_upstream_link may only be executed in a CMS context") try: xblock = modulestore().get_item(UsageKey.from_string(usage_key)) except (ItemNotFoundError, InvalidKeyError): diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 0562886acd3d..6204f020d990 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -26,7 +26,7 @@ from milestones import api as milestones_api from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey, UsageKey, UsageKeyV2 -from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocator +from opaque_keys.edx.locator import LibraryContainerLocator, LibraryLocator, BlockUsageLocator from openedx_events.content_authoring.data import DuplicatedXBlockData from openedx_events.content_authoring.signals import XBLOCK_DUPLICATED from openedx_events.learning.data import CourseNotificationData @@ -2395,12 +2395,22 @@ def _create_or_update_component_link(course_key: CourseKey, created: datetime | except ObjectDoesNotExist: log.error(f"Library component not found for {upstream_usage_key}") lib_component = None + + 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.top_level_downstream_parent_key.get('type'), + xblock.top_level_downstream_parent_key.get('id'), + ) + ComponentLink.update_or_create( lib_component, upstream_usage_key=upstream_usage_key, upstream_context_key=str(upstream_usage_key.context_key), downstream_context_key=course_key, downstream_usage_key=xblock.usage_key, + top_level_parent_usage_key=top_level_parent_usage_key, version_synced=xblock.upstream_version, version_declined=xblock.upstream_version_declined, created=created, @@ -2417,6 +2427,15 @@ def _create_or_update_container_link(course_key: CourseKey, created: datetime | except ObjectDoesNotExist: log.error(f"Library component not found for {upstream_container_key}") lib_component = None + + 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.top_level_downstream_parent_key.get('type'), + xblock.top_level_downstream_parent_key.get('id'), + ) + ContainerLink.update_or_create( lib_component, upstream_container_key=upstream_container_key, @@ -2424,6 +2443,7 @@ def _create_or_update_container_link(course_key: CourseKey, created: datetime | downstream_context_key=course_key, downstream_usage_key=xblock.usage_key, version_synced=xblock.upstream_version, + top_level_parent_usage_key=top_level_parent_usage_key, version_declined=xblock.upstream_version_declined, created=created, ) diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index a7ae0e518e84..afaf193e6038 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -33,6 +33,7 @@ from pytz import UTC from xblock.core import XBlock from xblock.fields import Scope +from .xblock_helpers import get_block_key_dict from cms.djangoapps.contentstore.config.waffle import SHOW_REVIEW_RULES_FLAG from cms.djangoapps.contentstore.helpers import StaticFileNotices @@ -528,12 +529,16 @@ def create_item(request): return _create_block(request) -def sync_library_content(downstream: XBlock, request, store) -> StaticFileNotices: +def sync_library_content( + downstream: XBlock, + request, + store, + top_level_parent: XBlock | None = None +) -> StaticFileNotices: """ Handle syncing library content for given xblock depending on its upstream type. It can sync unit containers and lower level xblocks. """ - # CHECK: Sync library content for given xblock depending on its upstream type. link = UpstreamLink.get_for_block(downstream) upstream_key = link.upstream_key if isinstance(upstream_key, LibraryUsageLocatorV2): @@ -547,15 +552,17 @@ def sync_library_content(downstream: XBlock, request, store) -> StaticFileNotice downstream_children_keys = [child.upstream for child in downstream_children] # Sync the children: notices = [] - # Store final children keys to update order of components in unit + # Store final children keys to update order of items in containers children = [] + top_level_downstream_parent = top_level_parent or downstream + for i, upstream_child in enumerate(upstream_children): if isinstance(upstream_child, LibraryXBlockMetadata): - upstream_key = upstream_child.usage_key + upstream_key = str(upstream_child.usage_key) block_type = upstream_child.usage_key.block_type elif isinstance(upstream_child, ContainerMetadata): - upstream_key = upstream_child.container_key + upstream_key = str(upstream_child.container_key) match upstream_child.container_type: case ContainerType.Unit: block_type = "vertical" @@ -585,7 +592,10 @@ def sync_library_content(downstream: XBlock, request, store) -> StaticFileNotice # TODO: Can we generate a unique but friendly block_id, perhaps using upstream block_id block_id=f"{block_type}{uuid4().hex[:8]}", fields={ - "upstream": str(upstream_key), + "upstream": upstream_key, + "top_level_downstream_parent_key": get_block_key_dict( + top_level_downstream_parent.usage_key, + ), }, ) else: @@ -594,7 +604,12 @@ def sync_library_content(downstream: XBlock, request, store) -> StaticFileNotice children.append(downstream_child.usage_key) - result = sync_library_content(downstream=downstream_child, request=request, store=store) + result = sync_library_content( + downstream=downstream_child, + request=request, + store=store, + top_level_parent=top_level_downstream_parent, + ) notices.append(result) for child in downstream_children: @@ -661,7 +676,6 @@ def _create_block(request): status=400, ) - # CHECK: Add container to course created_block = create_xblock( parent_locator=parent_locator, user=request.user, diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/xblock_helpers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/xblock_helpers.py index 3afe9be813de..322bf530ab84 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/xblock_helpers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/xblock_helpers.py @@ -3,11 +3,13 @@ """ from opaque_keys.edx.keys import UsageKey +from xblock.core import XBlock from xmodule.modulestore.django import modulestore +from xmodule.util.keys import BlockKey from openedx.core.djangoapps.content_tagging.api import get_object_tag_counts -def usage_key_with_run(usage_key_string): +def usage_key_with_run(usage_key_string: str) -> UsageKey: """ Converts usage_key_string to a UsageKey, adding a course run if necessary """ @@ -16,7 +18,14 @@ def usage_key_with_run(usage_key_string): return usage_key -def get_tags_count(xblock, include_children=False): +def get_block_key_dict(usage_key: UsageKey) -> dict: + """ + Converts the usage_key in a dict with the form: `{"type": block_type, "id": block_id}` + """ + return BlockKey.from_usage_key(usage_key)._asdict() + + +def get_tags_count(xblock: XBlock, include_children=False) -> dict[str, int]: """ Returns a map with tag count of the `xblock` diff --git a/cms/lib/xblock/upstream_sync.py b/cms/lib/xblock/upstream_sync.py index 805019f5850d..bc1148b2cebe 100644 --- a/cms/lib/xblock/upstream_sync.py +++ b/cms/lib/xblock/upstream_sync.py @@ -25,7 +25,7 @@ from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryContainerLocator, LibraryUsageLocatorV2 from xblock.exceptions import XBlockNotFoundError -from xblock.fields import Scope, String, Integer +from xblock.fields import Scope, String, Integer, Dict from xblock.core import XBlockMixin, XBlock if t.TYPE_CHECKING: @@ -327,6 +327,17 @@ class UpstreamSyncMixin(XBlockMixin): default=None, scope=Scope.settings, hidden=True, enforce_type=True, ) + top_level_downstream_parent_key = Dict( + help=( + "The block key ('block_type@block_id') of the downstream block that is the top-level parent of " + "this block. This is present if the creation of this block is a consequence of " + "importing a container that has one or more levels of children. " + "This represents the parent (container) in the top level " + "at the moment of the import." + ), + default=None, scope=Scope.settings, hidden=True, enforce_type=True, + ) + @classmethod def get_customizable_fields(cls) -> dict[str, str | None]: """ diff --git a/openedx/core/lib/xblock_serializer/block_serializer.py b/openedx/core/lib/xblock_serializer/block_serializer.py index 353f399bac73..4fb7301ced6b 100644 --- a/openedx/core/lib/xblock_serializer/block_serializer.py +++ b/openedx/core/lib/xblock_serializer/block_serializer.py @@ -138,6 +138,10 @@ def _serialize_normal_block(self, block) -> etree.Element: if block.has_children: self._serialize_children(block, olx_node) + if "top_level_downstream_parent_key" in block.fields \ + and block.fields["top_level_downstream_parent_key"].is_set_on(block): + olx_node.attrib["top_level_downstream_parent_key"] = str(block.top_level_downstream_parent_key) + return olx_node def _serialize_children(self, block, parent_olx_node) -> None: @@ -162,7 +166,8 @@ def _serialize_html_block(self, block) -> etree.Element: if block.use_latex_compiler: olx_node.attrib["use_latex_compiler"] = "true" for field_name in block.fields: - if field_name.startswith("upstream") and block.fields[field_name].is_set_on(block): + if (field_name.startswith("upstream") or field_name == "top_level_downstream_parent_key") \ + and block.fields[field_name].is_set_on(block): olx_node.attrib[field_name] = str(getattr(block, field_name)) # Escape any CDATA special chars diff --git a/xmodule/modulestore/tests/test_mixed_modulestore.py b/xmodule/modulestore/tests/test_mixed_modulestore.py index c7770beb0f3c..1f4bcfac7b0c 100644 --- a/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -167,11 +167,16 @@ def setUp(self): # mock and ignore publishable link entity related tasks to avoid unnecessary # errors as it is tested separately if settings.ROOT_URLCONF == 'cms.urls': - create_or_update_xblock_upstream_link_patch = patch( - 'cms.djangoapps.contentstore.signals.handlers.handle_create_or_update_xblock_upstream_link' + create_xblock_upstream_link_patch = patch( + 'cms.djangoapps.contentstore.signals.handlers.handle_create_xblock_upstream_link' ) - create_or_update_xblock_upstream_link_patch.start() - self.addCleanup(create_or_update_xblock_upstream_link_patch.stop) + create_xblock_upstream_link_patch.start() + self.addCleanup(create_xblock_upstream_link_patch.stop) + update_xblock_upstream_link_patch = patch( + 'cms.djangoapps.contentstore.signals.handlers.handle_update_xblock_upstream_link' + ) + update_xblock_upstream_link_patch.start() + self.addCleanup(update_xblock_upstream_link_patch.stop) component_link_patch = patch( 'cms.djangoapps.contentstore.signals.handlers.ComponentLink' )