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 9733f9878210..a65df36fe65d 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 @@ -128,7 +128,7 @@ def _create_block_from_upstream( "library_content_key": upstream_key, }, expect_response=expect_response) - def _update_course_block_fields(self, usage_key: str, fields: dict[str, Any] = None): + def _update_course_block_fields(self, usage_key: str, fields: dict[str, Any] | None = None): """ Update fields of an XBlock """ return self._api('patch', f"/xblock/{usage_key}", { "metadata": fields, @@ -158,7 +158,7 @@ def _get_downstream_links( data["use_top_level_parents"] = str(use_top_level_parents) return self.client.get("/api/contentstore/v2/downstreams/", data=data) - def assertXmlEqual(self, xml_str_a: str, xml_str_b: str) -> bool: + def assertXmlEqual(self, xml_str_a: str, xml_str_b: str) -> None: """ Assert that the given XML strings are equal, ignoring attribute order and some whitespace variations. """ self.assertEqual( ElementTree.canonicalize(xml_str_a, strip_text=True), 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 4a3b13f9c4ab..f82859f74bd3 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 @@ -11,6 +11,7 @@ from freezegun import freeze_time from opaque_keys.edx.keys import ContainerKey, UsageKey from opaque_keys.edx.locator import LibraryLocatorV2, LibraryUsageLocatorV2 +from openedx_content import models_api as content_models from organizations.models import Organization from cms.djangoapps.contentstore.helpers import StaticFileNotices @@ -221,6 +222,11 @@ def setUp(self): self._publish_library_block(self.video_lib_id) self._publish_library_block(self.html_lib_id) + def tearDown(self): + # If we're working with Containers in test cases, we need this line: + content_models.Container.reset_cache() + return super().tearDown() + def _api(self, method, url, data, expect_response): """ Call a REST API diff --git a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py index 6a51610ac9f2..93a9941f4f1a 100644 --- a/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py +++ b/cms/djangoapps/contentstore/rest_api/v2/views/tests/test_home.py @@ -3,10 +3,9 @@ """ from collections import OrderedDict -from datetime import datetime, timedelta +from datetime import datetime, timedelta, timezone import ddt -import pytz from django.conf import settings from django.urls import reverse from rest_framework import status @@ -36,7 +35,7 @@ def setUp(self): display_name="Demo Course (Sample)", id=archived_course_key, org=archived_course_key.org, - end=(datetime.now() - timedelta(days=365)).replace(tzinfo=pytz.UTC), + end=(datetime.now() - timedelta(days=365)).replace(tzinfo=timezone.utc), ) self.non_staff_client, _ = self.create_non_staff_authed_user_client() @@ -256,7 +255,7 @@ def test_filter_and_ordering_courses( display_name="Course (Demo)", id=archived_course_key, org=archived_course_key.org, - end=(datetime.now() - timedelta(days=365)).replace(tzinfo=pytz.UTC), + end=(datetime.now() - timedelta(days=365)).replace(tzinfo=timezone.utc), ) active_course_key = self.store.make_course_key("foo-org", "foo-number", "foo-run") CourseOverviewFactory.create( diff --git a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py index 419f941efbb7..9a30807b60a3 100644 --- a/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py +++ b/cms/djangoapps/contentstore/xblock_storage_handlers/view_handlers.py @@ -9,7 +9,7 @@ Along with it, we moved the business logic of the other views in that file, since that is related. """ import logging -from datetime import datetime +from datetime import datetime, timezone from uuid import uuid4 from attrs import asdict @@ -20,7 +20,9 @@ from django.http import HttpResponse, HttpResponseBadRequest from django.utils.translation import gettext as _ from edx_django_utils.plugins import pluggable_override -from openedx.core.djangoapps.content_libraries.api import ContainerMetadata, ContainerType, LibraryXBlockMetadata +from openedx_content import api as content_api +from openedx_content import models_api as content_models +from openedx.core.djangoapps.content_libraries.api import ContainerMetadata, LibraryXBlockMetadata from openedx.core.djangoapps.content_tagging.api import get_object_tag_counts from openedx.core import toggles as core_toggles from openedx_authz import api as authz_api @@ -40,7 +42,6 @@ from edx_proctoring.exceptions import ProctoredExamNotFoundException from help_tokens.core import HelpUrlExpert from opaque_keys.edx.locator import LibraryUsageLocator, LibraryUsageLocatorV2 -from pytz import UTC from xblock.core import XBlock from xblock.fields import Scope from .xblock_helpers import get_block_key_string @@ -659,7 +660,7 @@ def sync_library_content( with store.bulk_operations(downstream.usage_key.context_key): upstream_children = sync_from_upstream_container(downstream=downstream, user=request.user) downstream_children = downstream.get_children() - downstream_children_keys = [child.upstream for child in downstream_children] + downstream_children_key_strings: list[str] = [child.upstream for child in downstream_children] # Sync the children: notices = [] # Store final children keys to update order of items in containers @@ -669,22 +670,19 @@ def sync_library_content( for i, upstream_child in enumerate(upstream_children): if isinstance(upstream_child, LibraryXBlockMetadata): - upstream_key = str(upstream_child.usage_key) + upstream_child_key_string = str(upstream_child.usage_key) block_type = upstream_child.usage_key.block_type elif isinstance(upstream_child, ContainerMetadata): - upstream_key = str(upstream_child.container_key) - match upstream_child.container_type: - case ContainerType.Unit: - block_type = "vertical" - case ContainerType.Subsection: - block_type = "sequential" - case _: - # We don't support other container types for now. - log.error( - "Unexpected upstream child container type: %s", - upstream_child.container_type, - ) - continue + upstream_child_key_string = str(upstream_child.container_key) + if upstream_child.container_type_code not in ( + content_models.Unit.type_code, + content_models.Subsection.type_code, + ): + # We don't support other container types for now. + log.error("Unexpected upstream child container type: %s", upstream_child.container_type_code) + continue + # convert "unit" -> "vertical", "subsection" -> "sequential" + block_type = content_api.get_container_subclass(upstream_child.container_type_code).olx_tag_name else: log.error( "Unexpected type of upstream child: %s", @@ -692,7 +690,7 @@ def sync_library_content( ) continue - if upstream_key not in downstream_children_keys: + if upstream_child_key_string not in downstream_children_key_strings: # This upstream_child is new, create it. downstream_child = store.create_child( parent_usage_key=downstream.usage_key, @@ -702,14 +700,14 @@ def sync_library_content( # 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": upstream_key, + "upstream": upstream_child_key_string, "top_level_downstream_parent_key": get_block_key_string( top_level_downstream_parent.usage_key, ), }, ) else: - downstream_child_old_index = downstream_children_keys.index(upstream_key) + downstream_child_old_index = downstream_children_key_strings.index(upstream_child_key_string) downstream_child = downstream_children[downstream_child_old_index] children.append(downstream_child.usage_key) @@ -1313,7 +1311,7 @@ def create_xblock_info( # lint-amnesty, pylint: disable=too-many-statements "studio_url": xblock_studio_url(xblock, parent_xblock), "lms_url": xblock_lms_url(xblock), "embed_lms_url": xblock_embed_lms_url(xblock), - "released_to_students": datetime.now(UTC) > xblock.start, + "released_to_students": datetime.now(timezone.utc) > xblock.start, "release_date": release_date, "visibility_state": visibility_state, "has_explicit_staff_lock": xblock.fields[ @@ -1652,7 +1650,7 @@ def _compute_visibility_state( return VisibilityState.needs_attention is_unscheduled = xblock.start == DEFAULT_START_DATE - is_live = is_course_self_paced or datetime.now(UTC) > xblock.start + is_live = is_course_self_paced or datetime.now(timezone.utc) > xblock.start if child_info and child_info.get("children", []): # pylint: disable=too-many-nested-blocks all_staff_only = True all_unscheduled = True diff --git a/cms/djangoapps/modulestore_migrator/api/read_api.py b/cms/djangoapps/modulestore_migrator/api/read_api.py index 3e48adc0dc1a..c418223b4db7 100644 --- a/cms/djangoapps/modulestore_migrator/api/read_api.py +++ b/cms/djangoapps/modulestore_migrator/api/read_api.py @@ -60,9 +60,7 @@ def get_forwarding_for_blocks(source_keys: t.Iterable[UsageKey]) -> dict[UsageKe # For building component key "forwarded__target__component__component_type", # For building container key - "forwarded__target__container__section", - "forwarded__target__container__subsection", - "forwarded__target__container__unit", + "forwarded__target__container__container_type", # For determining title and version "forwarded__change_log_record__new_version", ) @@ -166,11 +164,7 @@ def get_migration_blocks(migration_pk: int) -> dict[UsageKey, ModulestoreBlockMi # For building component key "target__component__component_type", # For building container key. - # (Hard-coding these exact 3 container types here is not a good pattern, but it's what is needed - # here in order to avoid additional SELECTs while determining the container type). - "target__container__section", - "target__container__subsection", - "target__container__unit", + "target__container__container_type", # For determining title and version "change_log_record__new_version", ) diff --git a/cms/djangoapps/modulestore_migrator/data.py b/cms/djangoapps/modulestore_migrator/data.py index f2a9d725656c..ef6c1f2bb708 100644 --- a/cms/djangoapps/modulestore_migrator/data.py +++ b/cms/djangoapps/modulestore_migrator/data.py @@ -18,8 +18,7 @@ LibraryLocatorV2, LibraryUsageLocatorV2, ) - -from openedx.core.djangoapps.content_libraries.api import ContainerType +from openedx_content import models_api as content_models class CompositionLevel(Enum): @@ -32,9 +31,9 @@ class CompositionLevel(Enum): Component = 'component' # Container types currently supported by Content Libraries - Unit = ContainerType.Unit.value - Subsection = ContainerType.Subsection.value - Section = ContainerType.Section.value + Unit = content_models.Unit.type_code # "unit" + Subsection = content_models.Subsection.type_code # "subsection" + Section = content_models.Section.type_code # "section" @property def is_container(self) -> bool: diff --git a/cms/djangoapps/modulestore_migrator/tasks.py b/cms/djangoapps/modulestore_migrator/tasks.py index cbcbc6db52c8..6e821775440f 100644 --- a/cms/djangoapps/modulestore_migrator/tasks.py +++ b/cms/djangoapps/modulestore_migrator/tasks.py @@ -46,7 +46,7 @@ from common.djangoapps.split_modulestore_django.models import SplitModulestoreCourseIndex from common.djangoapps.util.date_utils import DEFAULT_DATE_TIME_FORMAT, strftime_localized from openedx.core.djangoapps.content_libraries import api as libraries_api -from openedx.core.djangoapps.content_libraries.api import ContainerType, get_library +from openedx.core.djangoapps.content_libraries.api import get_library from openedx.core.djangoapps.content_staging import api as staging_api from xmodule.modulestore import exceptions as modulestore_exceptions from xmodule.modulestore.django import modulestore @@ -766,13 +766,13 @@ def _migrate_node( # do not support in libraries as of Ulmo. should_migrate_node: bool should_migrate_children: bool - container_type: ContainerType | None # if None, it's a Component + container_cls: content_api.ContainerSubclass | None # if None, it's a Component if source_node.tag == "wiki": return _MigratedNode(None, []) try: - container_type = ContainerType.from_source_olx_tag(source_node.tag) + container_cls = libraries_api.container_subclass_for_olx_tag(source_node.tag) except ValueError: - container_type = None + container_cls = None if source_node.tag in {"course", "library"}: should_migrate_node = False should_migrate_children = True @@ -780,7 +780,7 @@ def _migrate_node( should_migrate_node = True should_migrate_children = False else: - node_level = CompositionLevel(container_type.value) + node_level = CompositionLevel(container_cls.type_code) should_migrate_node = not node_level.is_higher_than(context.composition_level) should_migrate_children = True migrated_children: list[_MigratedNode] = [] @@ -802,7 +802,7 @@ def _migrate_node( _migrate_container( context=context, source_key=source_key, - container_type=container_type, + container_cls=container_cls, title=title, children=[ migrated_child.source_to_target[1] @@ -810,7 +810,7 @@ def _migrate_node( migrated_child.source_to_target and migrated_child.source_to_target[1] ], ) - if container_type else + if container_cls else _migrate_component( context=context, source_key=source_key, @@ -818,7 +818,7 @@ def _migrate_node( title=title, ) ) - if container_type is None and target_entity_version is None and reason is not None: + if container_cls is None and target_entity_version is None and reason is not None: # Currently, components with children are not supported children_length = len(source_node.getchildren()) if children_length: @@ -842,7 +842,7 @@ def _migrate_container( *, context: _MigrationContext, source_key: UsageKey, - container_type: ContainerType, + container_cls: content_api.ContainerSubclass, title: str, children: list[PublishableEntityVersion], ) -> tuple[PublishableEntityVersion, str | None]: @@ -857,7 +857,7 @@ def _migrate_container( target_key = _get_distinct_target_container_key( context, source_key, - container_type, + container_cls, title, ) try: @@ -874,7 +874,7 @@ def _migrate_container( else: container = libraries_api.create_container( library_key=context.target_library_key, - container_type=container_type, + container_cls=container_cls, slug=target_key.container_id, title=title, created=context.created_at, @@ -889,13 +889,9 @@ def _migrate_container( container_publishable_entity_version = content_api.create_next_container_version( container.container_pk, title=title, - entity_rows=[ - content_api.ContainerEntityRow(entity_pk=child.entity_id, version_pk=None) - for child in children - ], + entities=[child.entity for child in children], created=context.created_at, created_by=context.created_by, - container_version_cls=container_type.container_model_classes[1], ).publishable_entity_version # Publish the container @@ -990,7 +986,7 @@ def _migrate_component( def _get_distinct_target_container_key( context: _MigrationContext, source_key: UsageKey, - container_type: ContainerType, + container_cls: content_api.ContainerSubclass, title: str, ) -> LibraryContainerLocator: """ @@ -1012,14 +1008,14 @@ def _get_distinct_target_container_key( # Use base base slug if available if base_slug not in context.used_container_slugs: return LibraryContainerLocator( - context.target_library_key, container_type.value, base_slug + context.target_library_key, container_cls.type_code, base_slug ) # Try numbered variations until we find one that doesn't exist for i in range(1, _MAX_UNIQUE_SLUG_ATTEMPTS + 1): candidate_slug = f"{base_slug}_{i}" if candidate_slug not in context.used_container_slugs: return LibraryContainerLocator( - context.target_library_key, container_type.value, candidate_slug + context.target_library_key, container_cls.type_code, candidate_slug ) # It would be extremely unlikely for us to run out of attempts raise RuntimeError( diff --git a/cms/djangoapps/modulestore_migrator/tests/test_api.py b/cms/djangoapps/modulestore_migrator/tests/test_api.py index 068a0308a78e..ba7135ad87e1 100644 --- a/cms/djangoapps/modulestore_migrator/tests/test_api.py +++ b/cms/djangoapps/modulestore_migrator/tests/test_api.py @@ -6,6 +6,7 @@ from unittest.mock import patch from opaque_keys.edx.locator import LibraryLocator, LibraryLocatorV2, CourseLocator from openedx_content import api as content_api +from openedx_content import models_api as content_models from organizations.tests.factories import OrganizationFactory from cms.djangoapps.modulestore_migrator import api @@ -148,6 +149,11 @@ def setUp(self): user_id=self.user.id, publish_item=False, ) + def tearDown(self): + # If we're working with Containers in test cases, we need this line: + content_models.Container.reset_cache() + return super().tearDown() + def test_start_migration_to_library(self): """ Test that the API can start a migration to a library. diff --git a/cms/djangoapps/modulestore_migrator/tests/test_tasks.py b/cms/djangoapps/modulestore_migrator/tests/test_tasks.py index 32d9e426f933..1f95f9d4024d 100644 --- a/cms/djangoapps/modulestore_migrator/tests/test_tasks.py +++ b/cms/djangoapps/modulestore_migrator/tests/test_tasks.py @@ -10,7 +10,7 @@ from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.locator import LibraryLocator, LibraryLocatorV2 from openedx_content import api as content_api -from openedx_content.models_api import Collection, PublishableEntityVersion +from openedx_content.models_api import Collection, Container, PublishableEntityVersion, Section, Subsection, Unit from organizations.tests.factories import OrganizationFactory from user_tasks.models import UserTaskArtifact from user_tasks.tasks import UserTaskStatus @@ -101,6 +101,11 @@ def setUp(self): title="Test Collection 2", ) + def tearDown(self): + # If we're working with Containers in test cases, we need this line: + Container.reset_cache() + return super().tearDown() + def _make_migration_context(self, **kwargs) -> _MigrationContext: """ Builds a _MigrationContext object with default values, overridable with kwargs @@ -752,7 +757,7 @@ def test_migrate_container_creates_new_container(self): result, reason = _migrate_container( context=context, source_key=source_key, - container_type=lib_api.ContainerType.Unit, + container_cls=Unit, title="Test Vertical", children=children, ) @@ -775,14 +780,14 @@ def test_migrate_container_different_container_types(self): Test _migrate_container works with different container types """ container_types = [ - (lib_api.ContainerType.Unit, "vertical"), - (lib_api.ContainerType.Subsection, "sequential"), - (lib_api.ContainerType.Section, "chapter"), + (Unit, "vertical"), + (Subsection, "sequential"), + (Section, "chapter"), ] context = self._make_migration_context(repeat_handling_strategy=RepeatHandlingStrategy.Skip) - for container_type, block_type in container_types: - with self.subTest(container_type=container_type, block_type=block_type): + for container_cls, block_type in container_types: + with self.subTest(container_cls=container_cls, block_type=block_type): source_key = self.course.id.make_usage_key( block_type, f"test_{block_type}" ) @@ -790,7 +795,7 @@ def test_migrate_container_different_container_types(self): result, reason = _migrate_container( context=context, source_key=source_key, - container_type=container_type, + container_cls=container_cls, title=f"Test {block_type.title()}", children=[], ) @@ -861,7 +866,7 @@ def test_migrate_container_with_library_source_key(self): result, _ = _migrate_container( context=context, source_key=source_key, - container_type=lib_api.ContainerType.Unit, + container_cls=Unit, title="Library Vertical", children=[], ) @@ -880,7 +885,7 @@ def test_migrate_container_empty_children_list(self): result, reason = _migrate_container( context=context, source_key=source_key, - container_type=lib_api.ContainerType.Unit, + container_cls=Unit, title="Empty Vertical", children=[], ) @@ -919,7 +924,7 @@ def test_migrate_container_preserves_child_order(self): result, _ = _migrate_container( context=context, source_key=source_key, - container_type=lib_api.ContainerType.Unit, + container_cls=Unit, title="Ordered Vertical", children=children, ) @@ -996,7 +1001,7 @@ def test_migrate_container_with_mixed_child_types(self): result, _ = _migrate_container( context=context, source_key=source_key, - container_type=lib_api.ContainerType.Unit, + container_cls=Unit, title="Mixed Content Vertical", children=children, ) diff --git a/cms/lib/xblock/tagging/test.py b/cms/lib/xblock/tagging/test.py index 173523452d54..cee2bf5e000c 100644 --- a/cms/lib/xblock/tagging/test.py +++ b/cms/lib/xblock/tagging/test.py @@ -4,14 +4,13 @@ import json -from datetime import datetime +from datetime import datetime, timezone from io import StringIO import ddt from django.test.client import RequestFactory from lxml import etree from opaque_keys.edx.asides import AsideUsageKeyV1, AsideUsageKeyV2 -from pytz import UTC from xblock.fields import ScopeIds from xblock.runtime import DictKeyValueStore, KvsFieldData from xblock.test.tools import TestRuntime @@ -56,21 +55,21 @@ def setUp(self): category='chapter', display_name="Week 1", publish_item=True, - start=datetime(2015, 3, 1, tzinfo=UTC), + start=datetime(2015, 3, 1, tzinfo=timezone.utc), ) self.sequential = BlockFactory.create( parent_location=self.chapter.location, category='sequential', display_name="Lesson 1", publish_item=True, - start=datetime(2015, 3, 1, tzinfo=UTC), + start=datetime(2015, 3, 1, tzinfo=timezone.utc), ) self.vertical = BlockFactory.create( parent_location=self.sequential.location, category='vertical', display_name='Subsection 1', publish_item=True, - start=datetime(2015, 4, 1, tzinfo=UTC), + start=datetime(2015, 4, 1, tzinfo=timezone.utc), ) self.problem = BlockFactory.create( category="problem", diff --git a/cms/lib/xblock/test/test_upstream_sync.py b/cms/lib/xblock/test/test_upstream_sync.py index 94ee69e5f1f3..1af3914658ce 100644 --- a/cms/lib/xblock/test/test_upstream_sync.py +++ b/cms/lib/xblock/test/test_upstream_sync.py @@ -6,7 +6,6 @@ import ddt from organizations.api import ensure_organization from organizations.models import Organization -from pytz import utc from cms.lib.xblock.upstream_sync import ( BadDownstream, @@ -292,7 +291,7 @@ def test_sync_updates_to_downstream_only_fields(self): # Modifing downstream-only fields are "safe" customizations downstream.display_name = "Downstream Title Override" downstream.attempts_before_showanswer_button = 2 - downstream.due = datetime.datetime(2025, 2, 2, tzinfo=utc) + downstream.due = datetime.datetime(2025, 2, 2, tzinfo=datetime.timezone.utc) downstream.force_save_button = True downstream.graceperiod = '2d' downstream.grading_method = 'last_score' @@ -320,7 +319,7 @@ def test_sync_updates_to_downstream_only_fields(self): # but "safe" customizations survive assert downstream.display_name == "Downstream Title Override" assert downstream.attempts_before_showanswer_button == 2 - assert downstream.due == datetime.datetime(2025, 2, 2, tzinfo=utc) + assert downstream.due == datetime.datetime(2025, 2, 2, tzinfo=datetime.timezone.utc) assert downstream.force_save_button assert downstream.graceperiod == '2d' assert downstream.grading_method == 'last_score' diff --git a/cms/lib/xblock/upstream_sync.py b/cms/lib/xblock/upstream_sync.py index 729d18bf5bc7..21f4201cdfde 100644 --- a/cms/lib/xblock/upstream_sync.py +++ b/cms/lib/xblock/upstream_sync.py @@ -252,8 +252,9 @@ def get_for_block(cls, downstream: XBlock) -> t.Self: If link exists, is supported, and is followable, returns UpstreamLink. Otherwise, raises an UpstreamLinkException. """ - # We import this here b/c UpstreamSyncMixin is used by cms/envs, which loads before the djangoapps are ready. + # We import these here b/c UpstreamSyncMixin is used by cms/envs, which loads before the djangoapps are ready. from openedx.core.djangoapps.content_libraries import api as lib_api + from openedx_content import api as content_api if not isinstance(downstream, UpstreamSyncMixin): raise BadDownstream(_("Downstream is not an XBlock or is missing required UpstreamSyncMixin")) @@ -290,7 +291,8 @@ def get_for_block(cls, downstream: XBlock) -> t.Self: container_meta = lib_api.get_container(upstream_key) except lib_api.ContentLibraryContainerNotFound as exc: raise BadUpstream(_("Linked upstream library container was not found in the system")) from exc - expected_downstream_block_type = container_meta.container_type.olx_tag + container_cls = content_api.get_container_subclass(container_meta.container_type_code) + expected_downstream_block_type = container_cls.olx_tag_name version_available = container_meta.published_version_num else: raise BadUpstream(_("Linked `upstream_key` is not a valid key")) diff --git a/mypy.ini b/mypy.ini index 6cb44c9300a2..f45a46d14acb 100644 --- a/mypy.ini +++ b/mypy.ini @@ -6,9 +6,9 @@ plugins = mypy_django_plugin.main, mypy_drf_plugin.main files = - cms/lib/xblock/upstream_sync.py, - cms/lib/xblock/upstream_sync_container.py, - cms/djangoapps/contentstore/rest_api/v2/views/downstreams.py, + cms/lib/xblock, + cms/djangoapps/contentstore/rest_api/v2/views, + cms/djangoapps/contentstore/xblock_storage_handlers, cms/djangoapps/modulestore_migrator, openedx/core/djangoapps/content/learning_sequences, # FIXME: need to solve type issues and add 'search' app here: diff --git a/openedx/core/djangoapps/content/search/api.py b/openedx/core/djangoapps/content/search/api.py index b36beeba4710..d997b28ed1cb 100644 --- a/openedx/core/djangoapps/content/search/api.py +++ b/openedx/core/djangoapps/content/search/api.py @@ -26,6 +26,7 @@ LibraryLocatorV2, ) from openedx_content import api as content_api +from openedx_content import models_api as content_models from rest_framework.request import Request from common.djangoapps.student.role_helpers import get_course_roles @@ -530,11 +531,11 @@ def index_container_batch(batch, num_done, library_key) -> int: doc = searchable_doc_for_container(container_key) doc.update(searchable_doc_tags(container_key)) doc.update(searchable_doc_collections(container_key)) - container_type = lib_api.ContainerType(container_key.container_type) - match container_type: - case lib_api.ContainerType.Unit: + container_type_code = container_key.container_type + match container_type_code: + case content_models.Unit.type_code: doc.update(searchable_doc_containers(container_key, "subsections")) - case lib_api.ContainerType.Subsection: + case content_models.Subsection.type_code: doc.update(searchable_doc_containers(container_key, "sections")) docs.append(doc) except Exception as err: # pylint: disable=broad-except @@ -827,19 +828,19 @@ def update_library_containers_collections( """ library_key = collection_key.lib_key library = lib_api.get_library(library_key) - containers = content_api.get_collection_containers( + container_entities = content_api.get_collection_entities( library.learning_package_id, collection_key.collection_id, - ) + ).exclude(container=None).select_related("container") - paginator = Paginator(containers, batch_size) + paginator = Paginator(container_entities, batch_size) for page in paginator.page_range: docs = [] - for container in paginator.page(page).object_list: + for container_entity in paginator.page(page).object_list: container_key = lib_api.library_container_locator( library_key, - container, + container_entity.container, ) doc = searchable_doc_for_key(container_key) doc.update(searchable_doc_collections(container_key)) diff --git a/openedx/core/djangoapps/content/search/documents.py b/openedx/core/djangoapps/content/search/documents.py index 4f10070b02b4..11408f2e6f43 100644 --- a/openedx/core/djangoapps/content/search/documents.py +++ b/openedx/core/djangoapps/content/search/documents.py @@ -11,7 +11,7 @@ from opaque_keys.edx.keys import ContainerKey, LearningContextKey, UsageKey, OpaqueKey from opaque_keys.edx.locator import LibraryCollectionLocator, LibraryContainerLocator from openedx_content import api as content_api -from openedx_content.models_api import Collection +from openedx_content.models_api import Collection, Section, Subsection, Unit from rest_framework.exceptions import NotFound from openedx.core.djangoapps.content.search.models import SearchAccess @@ -624,16 +624,16 @@ def searchable_doc_for_container( elif container.has_unpublished_changes: publish_status = PublishStatus.modified - container_type = lib_api.ContainerType(container_key.container_type) + container_type_code = container_key.container_type def get_child_keys(children) -> list[str]: - match container_type: - case lib_api.ContainerType.Unit: + match container_type_code: + case Unit.type_code: return [ str(child.usage_key) for child in children ] - case lib_api.ContainerType.Subsection | lib_api.ContainerType.Section: + case Subsection.type_code | Section.type_code: return [ str(child.container_key) for child in children diff --git a/openedx/core/djangoapps/content/search/tests/test_api.py b/openedx/core/djangoapps/content/search/tests/test_api.py index 4ac9abe3008d..27a8201330f7 100644 --- a/openedx/core/djangoapps/content/search/tests/test_api.py +++ b/openedx/core/djangoapps/content/search/tests/test_api.py @@ -9,6 +9,7 @@ from unittest.mock import MagicMock, Mock, call, patch from opaque_keys.edx.keys import UsageKey from opaque_keys.edx.locator import LibraryCollectionLocator, LibraryContainerLocator +from openedx_content import models_api as content_models import ddt import pytest @@ -224,7 +225,7 @@ def setUp(self) -> None: with freeze_time(self.created_date): self.unit = library_api.create_container( library_key=self.library.key, - container_type=library_api.ContainerType.Unit, + container_cls=content_models.Unit, slug="unit-1", title="Unit 1", user_id=None, @@ -232,7 +233,7 @@ def setUp(self) -> None: self.unit_key = "lct:org1:lib:unit:unit-1" self.subsection = library_api.create_container( self.library.key, - container_type=library_api.ContainerType.Subsection, + container_cls=content_models.Subsection, slug="subsection-1", title="Subsection 1", user_id=None, @@ -245,7 +246,7 @@ def setUp(self) -> None: self.subsection_key = "lct:org1:lib:subsection:subsection-1" self.section = library_api.create_container( self.library.key, - container_type=library_api.ContainerType.Section, + container_cls=content_models.Section, slug="section-1", title="Section 1", user_id=None, @@ -327,6 +328,10 @@ def setUp(self) -> None: # "published" is not set since we haven't published it yet } + def tearDown(self): + content_models.Container.reset_cache() + return super().tearDown() + @override_settings(MEILISEARCH_ENABLED=False) def test_reindex_meilisearch_disabled(self, mock_meilisearch) -> None: with self.assertRaises(RuntimeError): diff --git a/openedx/core/djangoapps/content/search/tests/test_documents.py b/openedx/core/djangoapps/content/search/tests/test_documents.py index a2bc09052b50..bd155a3b3b8f 100644 --- a/openedx/core/djangoapps/content/search/tests/test_documents.py +++ b/openedx/core/djangoapps/content/search/tests/test_documents.py @@ -8,6 +8,7 @@ from freezegun import freeze_time from opaque_keys.edx.locator import LibraryCollectionLocator, LibraryContainerLocator from openedx_content import api as content_api +from openedx_content import models_api as content_models from organizations.models import Organization from openedx.core.djangoapps.content_libraries import api as library_api @@ -92,7 +93,7 @@ def setUpClass(cls): ) cls.container = library_api.create_container( cls.library.key, - container_type=library_api.ContainerType.Unit, + container_cls=content_models.Unit, slug="unit1", title="A Unit in the Search Index", user_id=None, @@ -102,7 +103,7 @@ def setUpClass(cls): ) cls.subsection = library_api.create_container( cls.library.key, - container_type=library_api.ContainerType.Subsection, + container_cls=content_models.Subsection, slug="subsection1", title="A Subsection in the Search Index", user_id=None, @@ -112,7 +113,7 @@ def setUpClass(cls): ) cls.section = library_api.create_container( cls.library.key, - container_type=library_api.ContainerType.Section, + container_cls=content_models.Section, slug="section1", title="A Section in the Search Index", user_id=None, @@ -153,6 +154,11 @@ def setUpClass(cls): tagging_api.tag_object(str(cls.subsection_key), cls.difficulty_tags, tags=["Normal"]) tagging_api.tag_object(str(cls.section_key), cls.difficulty_tags, tags=["Normal"]) + def tearDown(self): + # If we're working with Containers in test cases, we need this line: + content_models.Container.reset_cache() + return super().tearDown() + @property def toy_course_access_id(self): """ diff --git a/openedx/core/djangoapps/content_libraries/api/blocks.py b/openedx/core/djangoapps/content_libraries/api/blocks.py index dc0913d0fdc7..3bae77f3c1cc 100644 --- a/openedx/core/djangoapps/content_libraries/api/blocks.py +++ b/openedx/core/djangoapps/content_libraries/api/blocks.py @@ -65,8 +65,8 @@ get_containers_contains_item, update_container_children, ContainerMetadata, - ContainerType, ) +from .container_metadata import container_subclass_for_olx_tag from .collections import library_collection_locator from .libraries import PublishableItem from .. import tasks @@ -547,7 +547,7 @@ def _import_staged_block_as_container( container = create_container( library_key=library_key, - container_type=ContainerType.from_source_olx_tag(olx_node.tag), + container_cls=container_subclass_for_olx_tag(olx_node.tag), slug=None, # auto-generate slug from title title=title, user_id=user.id, diff --git a/openedx/core/djangoapps/content_libraries/api/container_metadata.py b/openedx/core/djangoapps/content_libraries/api/container_metadata.py index 0e6ed1ea6173..b0b84a43108a 100644 --- a/openedx/core/djangoapps/content_libraries/api/container_metadata.py +++ b/openedx/core/djangoapps/content_libraries/api/container_metadata.py @@ -1,6 +1,7 @@ """ Content libraries data classes related to Containers. """ + from __future__ import annotations from dataclasses import dataclass, field as dataclass_field @@ -12,94 +13,53 @@ from openedx_content.models_api import ( Component, Container, - ContainerVersion, Unit, - UnitVersion, Subsection, - SubsectionVersion, Section, - SectionVersion, PublishableEntity, + PublishableEntityMixin, ) from openedx.core.djangoapps.content_tagging.api import get_object_tag_counts from openedx.core.djangoapps.xblock.api import get_component_from_usage_key from ..models import ContentLibrary -from .exceptions import ContentLibraryContainerNotFound +from .exceptions import ContentLibraryBlockNotFound, ContentLibraryContainerNotFound from .libraries import PublishableItem, library_component_usage_key # The public API is only the following symbols: __all__ = [ # Models "ContainerMetadata", - "ContainerType", # Methods + "container_subclass_for_olx_tag", "library_container_locator", ] +# For now, we only allow the following types of containers in content libraries, and their hierarchy is hard-coded. +LIBRARY_ALLOWED_CONTAINER_TYPES = [ + Unit.type_code, + Subsection.type_code, + Section.type_code, +] -class ContainerType(Enum): - """ - The container types supported by content_libraries, and logic to map them to OLX. - """ - Unit = "unit" - Subsection = "subsection" - Section = "section" - - @property - def container_model_classes(self) -> tuple[type[Container], type[ContainerVersion]]: - """ - Get the container, containerversion subclasses associated with this type. - @@TODO Is this what we want, a hard mapping between container_types and Container classes? - * If so, then expand on this pattern, so that all ContainerType logic is contained within - this class, and get rid of the match-case statements that are all over the content_libraries - app. - * If not, then figure out what to do instead. - """ - match self: - case self.Unit: - return (Unit, UnitVersion) - case self.Subsection: - return (Subsection, SubsectionVersion) - case self.Section: - return (Section, SectionVersion) - raise TypeError(f"unexpected ContainerType: {self!r}") - - @property - def olx_tag(self) -> str: - """ - Canonical XML tag to use when representing this container as OLX. - - For example, Units are encoded as .... - These tag names are historical. We keep them around for the backwards compatibility of OLX - and for easier interaction with legacy modulestore-powered structural XBlocks - (e.g., copy-paste of Units between courses and V2 libraries). - """ - match self: - case self.Unit: - return "vertical" - case self.Subsection: - return "sequential" - case self.Section: - return "chapter" - raise TypeError(f"unexpected ContainerType: {self!r}") +def container_subclass_for_olx_tag(olx_tag: str) -> content_api.ContainerSubclass: + """ + Given an OLX tag code (e.g. `"vertical"` for ``), get the + corresponding `Container` subclass, e.g. `Unit`. - @classmethod - def from_source_olx_tag(cls, olx_tag: str) -> 'ContainerType': - """ - Get the ContainerType that this OLX tag maps to. - """ - if olx_tag == "unit": - # There is an alternative implementation to VerticalBlock called UnitBlock whose - # OLX tag is . When converting from OLX, we want to handle both - # and as Unit containers, although the canonical serialization is still . - return cls.Unit - try: - return next(ct for ct in cls if olx_tag == ct.olx_tag) - except StopIteration: - raise ValueError(f"no container_type for XML tag: <{olx_tag}>") from None + This method is specific to content libraries. + """ + try: + subclass = next(ct for ct in content_api.get_all_container_subclasses() if olx_tag == ct.olx_tag_name) + except StopIteration: + raise ValueError(f"Content libraries does not support containers with XML tag: <{olx_tag}>") from None + if subclass.type_code not in LIBRARY_ALLOWED_CONTAINER_TYPES: + raise ValueError( + f'Content libraries does not support "{subclass.type_code}" containers (with XML tag <{olx_tag}>)' + ) from None + return subclass @dataclass(frozen=True, kw_only=True) @@ -107,8 +67,9 @@ class ContainerMetadata(PublishableItem): """ Class that represents the metadata about a Container (e.g. Unit) in a content library. """ + container_key: LibraryContainerLocator - container_type: ContainerType + container_type_code: str container_pk: int @classmethod @@ -121,7 +82,6 @@ def from_container(cls, library_key, container: Container, associated_collection library_key, container=container, ) - container_type = ContainerType(container_key.container_type) published_by = None if last_publish_log and last_publish_log.published_by: published_by = last_publish_log.published_by.username @@ -137,7 +97,7 @@ def from_container(cls, library_key, container: Container, associated_collection return cls( container_key=container_key, - container_type=container_type, + container_type_code=container_key.container_type, container_pk=container.pk, display_name=draft.title, created=container.created, @@ -164,6 +124,7 @@ class ContainerHierarchy: database queries in the future. More details being discussed in https://github.com/openedx/edx-platform/pull/36813#issuecomment-3136631767 """ + sections: list[ContainerHierarchyMember] = dataclass_field(default_factory=list) subsections: list[ContainerHierarchyMember] = dataclass_field(default_factory=list) units: list[ContainerHierarchyMember] = dataclass_field(default_factory=list) @@ -174,6 +135,7 @@ class Level(Enum): """ Enumeratable levels contained by the ContainerHierarchy. """ + none = 0 components = 1 units = 2 @@ -295,10 +257,12 @@ def _get_containers_with_entities( """ qs = Container.objects.none() for member in members: - qs = qs.union(content_api.get_containers_with_entity( - member.entity.pk, - ignore_pinned=ignore_pinned, - )) + qs = qs.union( + content_api.get_containers_with_entity( + member.entity.pk, + ignore_pinned=ignore_pinned, + ) + ) return qs @@ -338,6 +302,7 @@ class ContainerHierarchyMember: """ Represents an individual member of ContainerHierarchy which is ready to be serialized. """ + id: LibraryContainerLocator | LibraryUsageLocatorV2 display_name: str has_unpublished_changes: bool @@ -396,23 +361,11 @@ def library_container_locator( """ Returns a LibraryContainerLocator for the given library + container. """ - container_type = None - if hasattr(container, 'unit'): - container_type = ContainerType.Unit - elif hasattr(container, 'subsection'): - container_type = ContainerType.Subsection - elif hasattr(container, 'section'): - container_type = ContainerType.Section - else: - # This should never happen, but we assert to ensure that we handle all cases. - # If this fails, it means that a new Container type was added without updating this code. - raise ValueError(f"Unexpected container type: {container!r}") + container_type_code = content_api.get_container_type_code_of(container) + if container_type_code not in LIBRARY_ALLOWED_CONTAINER_TYPES: + raise ValueError(f"Unsupported container type for content libraries: {container!r}") - return LibraryContainerLocator( - library_key, - container_type=container_type.value, - container_id=container.publishable_entity.key, - ) + return LibraryContainerLocator(library_key, container_type=container_type_code, container_id=container.key) def get_container_from_key(container_key: LibraryContainerLocator, include_deleted=False) -> Container: @@ -425,13 +378,27 @@ def get_container_from_key(container_key: LibraryContainerLocator, include_delet content_library = ContentLibrary.objects.get_by_key(container_key.lib_key) learning_package = content_library.learning_package assert learning_package is not None - container = content_api.get_container_by_key( - learning_package.id, - key=container_key.container_id, - ) + container = content_api.get_container_by_key(learning_package.id, key=container_key.container_id) + assert content_api.get_container_type_code_of(container) in LIBRARY_ALLOWED_CONTAINER_TYPES # We only return the container if it exists and either: # 1. the container has a draft version (which means it is not soft-deleted) OR # 2. the container was soft-deleted but the `include_deleted` flag is set to True if container and (include_deleted or container.versioning.draft): return container raise ContentLibraryContainerNotFound + + +def get_entity_from_key( + key: LibraryContainerLocator | LibraryUsageLocatorV2, /, *, include_deleted=False +) -> PublishableEntityMixin: + """ + Given a key for an item in a library, load it as a `Component` or a `Container` subclass. + """ + if isinstance(key, LibraryContainerLocator): + return get_container_from_key(key, include_deleted=False) + else: + assert isinstance(key, LibraryUsageLocatorV2) + component = get_component_from_usage_key(key) + if not include_deleted and not component.versioning.draft: + raise ContentLibraryBlockNotFound("Component has been deleted.") + return component diff --git a/openedx/core/djangoapps/content_libraries/api/containers.py b/openedx/core/djangoapps/content_libraries/api/containers.py index 54740d130321..907f614f7466 100644 --- a/openedx/core/djangoapps/content_libraries/api/containers.py +++ b/openedx/core/djangoapps/content_libraries/api/containers.py @@ -1,6 +1,7 @@ """ API for containers (Sections, Subsections, Units) in Content Libraries """ + from __future__ import annotations from datetime import datetime, timezone @@ -23,20 +24,19 @@ LIBRARY_CONTAINER_UPDATED, ) from openedx_content import api as content_api -from openedx_content.models_api import Container, ContainerVersion, Component +from openedx_content.models_api import Container, Unit from openedx.core.djangoapps.content_libraries.api.collections import library_collection_locator -from openedx.core.djangoapps.xblock.api import get_component_from_usage_key - from .. import tasks from ..models import ContentLibrary from .block_metadata import LibraryXBlockMetadata from .container_metadata import ( ContainerHierarchy, ContainerMetadata, - ContainerType, library_container_locator, get_container_from_key, + get_entity_from_key, + LIBRARY_ALLOWED_CONTAINER_TYPES, ) from .serializers import ContainerSerializer @@ -79,7 +79,7 @@ def get_container( associated_collections = content_api.get_entity_collections( container.publishable_entity.learning_package_id, container_key.container_id, - ).values('key', 'title') + ).values("key", "title") else: associated_collections = None container_meta = ContainerMetadata.from_container( @@ -87,13 +87,13 @@ def get_container( container, associated_collections=associated_collections, ) - assert container_meta.container_type.value == container_key.container_type + assert container_meta.container_type_code == container_key.container_type return container_meta def create_container( library_key: LibraryLocatorV2, - container_type: ContainerType, + container_cls: content_api.ContainerSubclass, slug: str | None, title: str, user_id: int | None, @@ -104,61 +104,37 @@ def create_container( It will initially be empty. """ + assert container_cls.type_code in LIBRARY_ALLOWED_CONTAINER_TYPES assert isinstance(library_key, LibraryLocatorV2) content_library = ContentLibrary.objects.get_by_key(library_key) assert content_library.learning_package_id # Should never happen but we made this a nullable field so need to check if slug is None: # Automatically generate a slug. Append a random suffix so it should be unique. - slug = slugify(title, allow_unicode=True) + '-' + uuid4().hex[-6:] + slug = slugify(title, allow_unicode=True) + "-" + uuid4().hex[-6:] # Make sure the slug is valid by first creating a key for the new container: container_key = LibraryContainerLocator( library_key, - container_type=container_type.value, + container_type=container_cls.type_code, container_id=slug, ) if not created: created = datetime.now(tz=timezone.utc) - container: Container - _initial_version: ContainerVersion - # Then try creating the actual container: - match container_type: - case ContainerType.Unit: - container, _initial_version = content_api.create_unit_and_version( - content_library.learning_package_id, - key=slug, - title=title, - created=created, - created_by=user_id, - ) - case ContainerType.Subsection: - container, _initial_version = content_api.create_subsection_and_version( - content_library.learning_package_id, - key=slug, - title=title, - created=created, - created_by=user_id, - ) - case ContainerType.Section: - container, _initial_version = content_api.create_section_and_version( - content_library.learning_package_id, - key=slug, - title=title, - created=created, - created_by=user_id, - ) - case _: - raise NotImplementedError(f"Library does not support {container_type} yet") + container, _initial_version = content_api.create_container_and_version( + content_library.learning_package_id, + key=slug, + title=title, + container_cls=container_cls, + entities=[], + created=created, + created_by=user_id, + ) # .. event_implemented_name: LIBRARY_CONTAINER_CREATED # .. event_type: org.openedx.content_authoring.content_library.container.created.v1 - LIBRARY_CONTAINER_CREATED.send_event( - library_container=LibraryContainerData( - container_key=container_key, - ) - ) + LIBRARY_CONTAINER_CREATED.send_event(library_container=LibraryContainerData(container_key=container_key)) return ContainerMetadata.from_container(library_key, container) @@ -175,78 +151,41 @@ def update_container( library_key = container_key.lib_key created = datetime.now(tz=timezone.utc) - container_type = ContainerType(container_key.container_type) - - version: ContainerVersion - affected_containers: list[ContainerMetadata] = [] # Get children containers or components to update their index data - children = get_container_children( - container_key, - published=False, - ) - child_key_name = 'container_key' - - match container_type: - case ContainerType.Unit: - version = content_api.create_next_unit_version( - container.unit, - title=display_name, - created=created, - created_by=user_id, - ) - affected_containers = get_containers_contains_item(container_key) - # Components have usage_key instead of container_key - child_key_name = 'usage_key' - case ContainerType.Subsection: - version = content_api.create_next_subsection_version( - container.subsection, - title=display_name, - created=created, - created_by=user_id, - ) - affected_containers = get_containers_contains_item(container_key) - case ContainerType.Section: - version = content_api.create_next_section_version( - container.section, - title=display_name, - created=created, - created_by=user_id, - ) + children = get_container_children(container_key, published=False) - # The `affected_containers` are not obtained, because the sections are - # not contained in any container. - case _: - raise NotImplementedError(f"Library does not support {container_type} yet") + version = content_api.create_next_container_version( + container, + title=display_name, + created=created, + created_by=user_id, + ) # Send event related to the updated container # .. event_implemented_name: LIBRARY_CONTAINER_UPDATED # .. event_type: org.openedx.content_authoring.content_library.container.updated.v1 - LIBRARY_CONTAINER_UPDATED.send_event( - library_container=LibraryContainerData( - container_key=container_key, - ) - ) + LIBRARY_CONTAINER_UPDATED.send_event(library_container=LibraryContainerData(container_key=container_key)) # Send events related to the containers that contains the updated container. # This is to update the children display names used in the section/subsection previews. + affected_containers = get_containers_contains_item(container_key) for affected_container in affected_containers: # .. event_implemented_name: LIBRARY_CONTAINER_UPDATED # .. event_type: org.openedx.content_authoring.content_library.container.updated.v1 LIBRARY_CONTAINER_UPDATED.send_event( - library_container=LibraryContainerData( - container_key=affected_container.container_key, - ) + library_container=LibraryContainerData(container_key=affected_container.container_key) ) # Update children components and containers index data, for example, # All subsections under a section have section key in index that needs to be updated. # So if parent section name has been changed, it needs to be reflected in sections key of children + is_unit = container_key.container_type == Unit.type_code for child in children: # .. event_implemented_name: CONTENT_OBJECT_ASSOCIATIONS_CHANGED # .. event_type: org.openedx.content_authoring.content.object.associations.changed.v1 CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( content_object=ContentObjectChangedData( - object_id=str(getattr(child, child_key_name)), - changes=[container_key.container_type + "s"], + object_id=str(child.usage_key if is_unit else child.container_key), # type: ignore[union-attr] + changes=[container_key.container_type + "s"], # e.g. "units" ), ) @@ -311,11 +250,10 @@ def delete_container( container_key=affected_container.container_key, ) ) - container_type = ContainerType(container_key.container_type) - key_name = 'container_key' - if container_type == ContainerType.Unit: + key_name = "container_key" + if isinstance(container, Unit): # Components have usage_key instead of container_key - key_name = 'usage_key' + key_name = "usage_key" # Update children components and containers index data, for example, # All subsections under a section have section key in index that needs to be updated. # So if parent section is deleted, it needs to be removed from sections key of children @@ -346,10 +284,7 @@ def restore_container(container_key: LibraryContainerLocator) -> None: # Fetch related containers after restore affected_containers = get_containers_contains_item(container_key) # Get children containers or components to update their index data - children = get_container_children( - container_key, - published=False, - ) + children = get_container_children(container_key, published=False) # .. event_implemented_name: LIBRARY_CONTAINER_CREATED # .. event_type: org.openedx.content_authoring.content_library.container.created.v1 @@ -362,7 +297,7 @@ def restore_container(container_key: LibraryContainerLocator) -> None: content_changes = ["collections", "tags"] if affected_containers and len(affected_containers) > 0: # Update parent key data in index. Eg. `sections` key in index for subsection - content_changes.append(str(affected_containers[0].container_type.value) + "s") + content_changes.append(str(affected_containers[0].container_type_code) + "s") # Add tags, collections and parent data back to index # .. event_implemented_name: CONTENT_OBJECT_ASSOCIATIONS_CHANGED # .. event_type: org.openedx.content_authoring.content.object.associations.changed.v1 @@ -398,10 +333,8 @@ def restore_container(container_key: LibraryContainerLocator) -> None: container_key=affected_container.container_key, ) ) - container_type = ContainerType(container_key.container_type) - key_name = 'container_key' - if container_type == ContainerType.Unit: - key_name = 'usage_key' + + is_unit = container_key.container_type == Unit.type_code # Update children components and containers index data, for example, # All subsections under a section have section key in index that needs to be updated. # Should restore removed parent section in sections key of children subsections @@ -410,7 +343,7 @@ def restore_container(container_key: LibraryContainerLocator) -> None: # .. event_type: org.openedx.content_authoring.content.object.associations.changed.v1 CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( content_object=ContentObjectChangedData( - object_id=str(getattr(child, key_name)), + object_id=str(child.usage_key if is_unit else child.container_key), # type: ignore[union-attr] changes=[container_key.container_type + "s"], ), ) @@ -426,33 +359,16 @@ def get_container_children( (e.g. the components/xblocks in a unit, units in a subsection, subsections in a section) """ container = get_container_from_key(container_key) - container_type = ContainerType(container_key.container_type) - - match container_type: - case ContainerType.Unit: - child_components = content_api.get_components_in_unit(container.unit, published=published) - return [LibraryXBlockMetadata.from_component( - container_key.lib_key, - entry.component - ) for entry in child_components] - case ContainerType.Subsection: - child_units = content_api.get_units_in_subsection(container.subsection, published=published) - return [ContainerMetadata.from_container( - container_key.lib_key, - entry.unit - ) for entry in child_units] - case ContainerType.Section: - child_subsections = content_api.get_subsections_in_section(container.section, published=published) - return [ContainerMetadata.from_container( - container_key.lib_key, - entry.subsection, - ) for entry in child_subsections] - case _: - child_entities = content_api.get_entities_in_container(container, published=published) - return [ContainerMetadata.from_container( - container_key.lib_key, - entry.entity - ) for entry in child_entities] + + child_entities = content_api.get_entities_in_container(container, published=published) + result: list[LibraryXBlockMetadata | ContainerMetadata] = [] + for entry in child_entities: + if hasattr(entry.entity, "component"): # the child is a Component + result.append(LibraryXBlockMetadata.from_component(container_key.lib_key, entry.entity.component)) + else: + assert isinstance(entry.entity.container, Container) + result.append(ContainerMetadata.from_container(container_key.lib_key, entry.entity.container)) + return result def get_container_children_count( @@ -468,78 +384,32 @@ def get_container_children_count( def update_container_children( container_key: LibraryContainerLocator, - children_ids: list[LibraryUsageLocatorV2] | list[LibraryContainerLocator], + children_keys: list[LibraryUsageLocatorV2] | list[LibraryContainerLocator], user_id: int | None, entities_action: content_api.ChildrenEntitiesAction = content_api.ChildrenEntitiesAction.REPLACE, ): """ [ 🛑 UNSTABLE ] Adds children components or containers to given container. """ - library_key = container_key.lib_key - container_type = ContainerType(container_key.container_type) container = get_container_from_key(container_key) created = datetime.now(tz=timezone.utc) - new_version: ContainerVersion - match container_type: - case ContainerType.Unit: - components = [get_component_from_usage_key(key) for key in children_ids] # type: ignore[arg-type] - new_version = content_api.create_next_unit_version( - container.unit, - components=components, # type: ignore[arg-type] - created=created, - created_by=user_id, - entities_action=entities_action, - ) - - for key in children_ids: - # .. event_implemented_name: CONTENT_OBJECT_ASSOCIATIONS_CHANGED - # .. event_type: org.openedx.content_authoring.content.object.associations.changed.v1 - CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( - content_object=ContentObjectChangedData( - object_id=str(key), - changes=["units"], - ), - ) - case ContainerType.Subsection: - units = [get_container_from_key(key).unit for key in children_ids] # type: ignore[arg-type] - new_version = content_api.create_next_subsection_version( - container.subsection, - units=units, # type: ignore[arg-type] - created=created, - created_by=user_id, - entities_action=entities_action, - ) - for key in children_ids: - # .. event_implemented_name: CONTENT_OBJECT_ASSOCIATIONS_CHANGED - # .. event_type: org.openedx.content_authoring.content.object.associations.changed.v1 - CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( - content_object=ContentObjectChangedData( - object_id=str(key), - changes=["subsections"], - ), - ) - case ContainerType.Section: - subsections = [get_container_from_key(key).subsection for key in children_ids] # type: ignore[arg-type] - new_version = content_api.create_next_section_version( - container.section, - subsections=subsections, # type: ignore[arg-type] - created=created, - created_by=user_id, - entities_action=entities_action, - ) - - for key in children_ids: - # .. event_implemented_name: CONTENT_OBJECT_ASSOCIATIONS_CHANGED - # .. event_type: org.openedx.content_authoring.content.object.associations.changed.v1 - CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( - content_object=ContentObjectChangedData( - object_id=str(key), - changes=["sections"], - ), - ) - case _: - raise ValueError(f"Invalid container type: {container_type}") + new_version = content_api.create_next_container_version( + container, + created=created, + created_by=user_id, + entities=[get_entity_from_key(key) for key in children_keys], + entities_action=entities_action, + ) + for key in children_keys: + # .. event_implemented_name: CONTENT_OBJECT_ASSOCIATIONS_CHANGED + # .. event_type: org.openedx.content_authoring.content.object.associations.changed.v1 + CONTENT_OBJECT_ASSOCIATIONS_CHANGED.send_event( + content_object=ContentObjectChangedData( + object_id=str(key), + changes=[f"{container_key.container_type}s"], # "units", "subsections", "sections" + ), + ) # .. event_implemented_name: LIBRARY_CONTAINER_UPDATED # .. event_type: org.openedx.content_authoring.content_library.container.updated.v1 @@ -549,30 +419,16 @@ def update_container_children( ) ) - return ContainerMetadata.from_container(library_key, new_version.container) + return ContainerMetadata.from_container(container_key.lib_key, new_version.container) -def get_containers_contains_item( - key: LibraryUsageLocatorV2 | LibraryContainerLocator -) -> list[ContainerMetadata]: +def get_containers_contains_item(key: LibraryUsageLocatorV2 | LibraryContainerLocator) -> list[ContainerMetadata]: """ [ 🛑 UNSTABLE ] Get containers that contains the item, that can be a component or another container. """ - item: Component | Container - - if isinstance(key, LibraryUsageLocatorV2): - item = get_component_from_usage_key(key) - - elif isinstance(key, LibraryContainerLocator): - item = get_container_from_key(key) - - containers = content_api.get_containers_with_entity( - item.publishable_entity.pk, - ) - return [ - ContainerMetadata.from_container(key.lib_key, container) - for container in containers - ] + entity = get_entity_from_key(key) + containers = content_api.get_containers_with_entity(entity.pk).select_related("container_type") + return [ContainerMetadata.from_container(key.lib_key, container) for container in containers] def publish_container_changes( @@ -611,7 +467,7 @@ def copy_container(container_key: LibraryContainerLocator, user_id: int) -> User """ container_metadata = get_container(container_key) container_serializer = ContainerSerializer(container_metadata) - block_type = ContainerType(container_key.container_type).olx_tag + block_type = content_api.get_container_subclass(container_key.container_type).olx_tag_name from openedx.core.djangoapps.content_staging import api as content_staging_api diff --git a/openedx/core/djangoapps/content_libraries/api/serializers.py b/openedx/core/djangoapps/content_libraries/api/serializers.py index 7590222d4521..35fe13c26f3c 100644 --- a/openedx/core/djangoapps/content_libraries/api/serializers.py +++ b/openedx/core/djangoapps/content_libraries/api/serializers.py @@ -3,6 +3,8 @@ """ from lxml import etree +from openedx_content import api as content_api + from openedx.core.djangoapps.xblock import api as xblock_api from openedx.core.lib.xblock_serializer.api import StaticFile, XBlockSerializer from openedx.core.djangoapps.content_tagging.api import TagValuesByObjectIdDict, get_all_object_tags @@ -30,10 +32,10 @@ def _serialize_container(self, container_metadata: container_api.ContainerMetada Serialize the given container to OLX. """ # Create an XML node to hold the exported data - container_type = container_api.ContainerType(container_metadata.container_key.container_type) + container_cls = content_api.get_container_subclass(container_metadata.container_type_code) container_key = container_metadata.container_key - olx = etree.Element(container_type.olx_tag) + olx = etree.Element(container_cls.olx_tag_name) olx.attrib["copied_from_block"] = str(container_key) olx.attrib["copied_from_version"] = str(container_metadata.draft_version_num) diff --git a/openedx/core/djangoapps/content_libraries/rest_api/containers.py b/openedx/core/djangoapps/content_libraries/rest_api/containers.py index 4b368e8eb9b2..991af9c39ca4 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/containers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/containers.py @@ -14,6 +14,7 @@ from opaque_keys.edx.locator import LibraryLocatorV2, LibraryContainerLocator from openedx_authz.constants import permissions as authz_permissions from openedx_content import api as content_api +from openedx_content import models_api as content_models from rest_framework.generics import GenericAPIView from rest_framework.response import Response from rest_framework.status import HTTP_204_NO_CONTENT, HTTP_200_OK @@ -51,10 +52,10 @@ def post(self, request, lib_key_str): serializer = serializers.LibraryContainerMetadataSerializer(data=request.data) serializer.is_valid(raise_exception=True) - container_type = serializer.validated_data['container_type'] + container_type_code = serializer.validated_data['container_type_code'] container = api.create_container( library_key, - container_type, + container_cls=content_api.get_container_subclass(container_type_code), title=serializer.validated_data['display_name'], slug=serializer.validated_data.get('slug'), user_id=request.user.id, @@ -190,7 +191,7 @@ def get(self, request, container_key: LibraryContainerLocator): permissions.CAN_VIEW_THIS_CONTENT_LIBRARY, ) child_entities = api.get_container_children(container_key, published=published) - if container_key.container_type == api.ContainerType.Unit.value: + if container_key.container_type == content_models.Unit.type_code: data = serializers.LibraryXBlockMetadataSerializer(child_entities, many=True).data else: data = serializers.LibraryContainerMetadataSerializer(child_entities, many=True).data @@ -215,7 +216,7 @@ def _update_component_children( container = api.update_container_children( container_key, - children_ids=serializer.validated_data["usage_keys"], + children_keys=serializer.validated_data["usage_keys"], user_id=request.user.id, entities_action=action, ) diff --git a/openedx/core/djangoapps/content_libraries/rest_api/serializers.py b/openedx/core/djangoapps/content_libraries/rest_api/serializers.py index 87a5dd3e3b6f..b7c7124fde08 100644 --- a/openedx/core/djangoapps/content_libraries/rest_api/serializers.py +++ b/openedx/core/djangoapps/content_libraries/rest_api/serializers.py @@ -15,7 +15,6 @@ from openedx.core.djangoapps.content_libraries.tasks import LibraryRestoreTask from openedx.core.djangoapps.content_libraries import api -from openedx.core.djangoapps.content_libraries.api.containers import ContainerType from openedx.core.djangoapps.content_libraries.constants import ALL_RIGHTS_RESERVED, LICENSE_OPTIONS from openedx.core.djangoapps.content_libraries.models import ( ContentLibrary, @@ -252,21 +251,28 @@ class LibraryContainerMetadataSerializer(PublishableItemSerializer): Converts from ContainerMetadata to JSON-compatible data """ - # Use 'source' to get this as a string, not an enum value instance which the container_type field has. - container_type = serializers.CharField(source="container_key.container_type") + container_type_code = serializers.CharField(source="container_key.container_type") + # Deprecated (ambiguously named). Identical to container_type_code. + container_type = serializers.CharField(source="container_key.container_type", required=False) # When creating a new container in a library, the slug becomes the ID part of # the definition key and usage key: slug = serializers.CharField(write_only=True, required=False) - def to_internal_value(self, data): + def to_internal_value(self, data: dict): """ Convert JSON-ish data back to native python types. Returns a dictionary, not a ContainerMetadata instance. """ - result = super().to_internal_value(data) - result["container_type"] = ContainerType(data["container_type"]) - return result + if "container_type_code" not in data: + data["container_type_code"] = data.get("container_type") # Support deprecated field name + validated_data = super().to_internal_value(data) + # If creating a new container, the above function results in: + # {'display_name': '...', 'container_key': {'container_type': 'unit'}} + # Fix that: + validated_data["container_type_code"] = validated_data["container_key"]["container_type"] + del validated_data["container_key"] + return validated_data class LibraryContainerUpdateSerializer(serializers.Serializer): diff --git a/openedx/core/djangoapps/content_libraries/tasks.py b/openedx/core/djangoapps/content_libraries/tasks.py index dfe1237b5613..a28bf1e861dd 100644 --- a/openedx/core/djangoapps/content_libraries/tasks.py +++ b/openedx/core/djangoapps/content_libraries/tasks.py @@ -104,7 +104,9 @@ def send_events_after_publish(publish_log_pk: int, library_key_str: str) -> None """ publish_log = PublishLog.objects.get(pk=publish_log_pk) library_key = LibraryLocatorV2.from_string(library_key_str) - affected_entities = publish_log.records.select_related("entity", "entity__container", "entity__component").all() + affected_entities = publish_log.records.select_related( + "entity", "entity__container", "entity__container__container_type", "entity__component", + ).all() affected_containers: set[LibraryContainerLocator] = set() # Update anything that needs to be updated (e.g. search index): @@ -122,9 +124,13 @@ def send_events_after_publish(publish_log_pk: int, library_key_str: str) -> None # Publishing a container will auto-publish its children, but publishing a single component or all changes # in the library will NOT usually include any parent containers. But we do need to notify listeners that the # parent container(s) have changed, e.g. so the search index can update the "has_unpublished_changes" - for parent_container in api.get_containers_contains_item(usage_key): - affected_containers.add(parent_container.container_key) - # TODO: should this be a CONTAINER_CHILD_PUBLISHED event instead of CONTAINER_PUBLISHED ? + try: + for parent_container in api.get_containers_contains_item(usage_key): + affected_containers.add(parent_container.container_key) + # TODO: should this be a CONTAINER_CHILD_PUBLISHED event instead of CONTAINER_PUBLISHED ? + except api.ContentLibraryBlockNotFound: + # The component has been deleted. + pass elif hasattr(record.entity, "container"): container_key = api.library_container_locator(library_key, record.entity.container) affected_containers.add(container_key) @@ -224,8 +230,12 @@ def send_events_after_revert(draft_change_log_id: int, library_key_str: str) -> # If any containers contain this component, their child list / component count may need to be updated # e.g. if this was a newly created component in the container and is now deleted, or this was deleted and # is now restored. - for parent_container in api.get_containers_contains_item(usage_key): - updated_container_keys.add(parent_container.container_key) + # TODO: we should be able to rewrite this to use the "side effects" functionality of the publishing API. + try: + for parent_container in api.get_containers_contains_item(usage_key): + updated_container_keys.add(parent_container.container_key) + except api.ContentLibraryBlockNotFound: + pass # The item 'usage_key' has been deleted. But shouldn't we still handle that? # TODO: do we also need to send CONTENT_OBJECT_ASSOCIATIONS_CHANGED for this component, or is # LIBRARY_BLOCK_UPDATED sufficient? diff --git a/openedx/core/djangoapps/content_libraries/tests/base.py b/openedx/core/djangoapps/content_libraries/tests/base.py index 04a5386c3127..1ca5c4ed0aea 100644 --- a/openedx/core/djangoapps/content_libraries/tests/base.py +++ b/openedx/core/djangoapps/content_libraries/tests/base.py @@ -10,6 +10,7 @@ from rest_framework.test import APITransactionTestCase, APIClient from opaque_keys.edx.keys import ContainerKey, UsageKey from opaque_keys.edx.locator import LibraryLocatorV2, LibraryCollectionLocator +from openedx_content import models_api as content_models from common.djangoapps.student.tests.factories import UserFactory from common.djangoapps.util.json_request import JsonResponse as SpecialJsonResponse @@ -95,6 +96,11 @@ def setUp(self): self.clients_by_user = {} self.client.login(username=self.user.username, password="edx") + def tearDown(self): + # If we're working with Containers in test cases, we need this line: + content_models.Container.reset_cache() + return super().tearDown() + # Assertions def assertDictContainsEntries(self, big_dict, subset_dict): @@ -408,9 +414,15 @@ def _set_library_block_fields(self, block_key, new_fields, expect_response=200): """ Set the fields of a specific block in the library. This API is only used by the MFE editors. """ return self._api('post', URL_BLOCK_FIELDS_URL.format(block_key=block_key), new_fields, expect_response) - def _create_container(self, lib_key, container_type, slug: str | None, display_name: str, expect_response=200): + def _create_container(self, + lib_key, + container_type_code: str, + slug: str | None, + display_name: str, + expect_response=200, + ): """ Create a container (unit etc.) """ - data = {"container_type": container_type, "display_name": display_name} + data = {"container_type_code": container_type_code, "display_name": display_name} if slug: data["slug"] = slug return self._api('post', URL_LIB_CONTAINERS.format(lib_key=lib_key), data, expect_response) diff --git a/openedx/core/djangoapps/content_libraries/tests/test_api.py b/openedx/core/djangoapps/content_libraries/tests/test_api.py index 69fae56b2d2c..1fe394b21b7a 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_api.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_api.py @@ -30,6 +30,7 @@ ) from openedx_authz.api.users import get_user_role_assignments_in_scope from openedx_content import api as content_api +from openedx_content import models_api as content_models from common.djangoapps.student.tests.factories import UserFactory from .. import api @@ -320,7 +321,7 @@ def setUp(self) -> None: # Create a subsection container self.subsection1 = api.create_container( self.lib1.library_key, - api.ContainerType.Subsection, + content_models.Subsection, 'subsection-1', 'Subsection 1', None, @@ -784,21 +785,21 @@ def setUp(self) -> None: self.lib1 = ContentLibrary.objects.get(slug="test-lib-cont-1") # Create Units - self.unit1 = api.create_container(self.lib1.library_key, api.ContainerType.Unit, 'unit-1', 'Unit 1', None) - self.unit2 = api.create_container(self.lib1.library_key, api.ContainerType.Unit, 'unit-2', 'Unit 2', None) - self.unit3 = api.create_container(self.lib1.library_key, api.ContainerType.Unit, 'unit-3', 'Unit 3', None) + self.unit1 = api.create_container(self.lib1.library_key, content_models.Unit, 'unit-1', 'Unit 1', None) + self.unit2 = api.create_container(self.lib1.library_key, content_models.Unit, 'unit-2', 'Unit 2', None) + self.unit3 = api.create_container(self.lib1.library_key, content_models.Unit, 'unit-3', 'Unit 3', None) # Create Subsections self.subsection1 = api.create_container( self.lib1.library_key, - api.ContainerType.Subsection, + content_models.Subsection, 'subsection-1', 'Subsection 1', None, ) self.subsection2 = api.create_container( self.lib1.library_key, - api.ContainerType.Subsection, + content_models.Subsection, 'subsection-2', 'Subsection 2', None, @@ -807,14 +808,14 @@ def setUp(self) -> None: # Create Sections self.section1 = api.create_container( self.lib1.library_key, - api.ContainerType.Section, + content_models.Section, 'section-1', 'Section 1', None, ) self.section2 = api.create_container( self.lib1.library_key, - api.ContainerType.Section, + content_models.Section, 'section-2', 'Section 2', None, @@ -1085,7 +1086,7 @@ def test_call_object_changed_signal_when_remove_component(self) -> None: ) def test_call_object_changed_signal_when_remove_unit(self) -> None: - unit4 = api.create_container(self.lib1.library_key, api.ContainerType.Unit, 'unit-4', 'Unit 4', None) + unit4 = api.create_container(self.lib1.library_key, content_models.Unit, 'unit-4', 'Unit 4', None) api.update_container_children( self.subsection2.container_key, @@ -1119,7 +1120,7 @@ def test_call_object_changed_signal_when_remove_unit(self) -> None: def test_call_object_changed_signal_when_remove_subsection(self) -> None: subsection3 = api.create_container( self.lib1.library_key, - api.ContainerType.Subsection, + content_models.Subsection, 'subsection-3', 'Subsection 3', None, @@ -1202,8 +1203,8 @@ def test_call_object_changed_signal_when_add_unit(self) -> None: event_reciver = mock.Mock() CONTENT_OBJECT_ASSOCIATIONS_CHANGED.connect(event_reciver) - unit4 = api.create_container(self.lib1.library_key, api.ContainerType.Unit, 'unit-4', 'Unit 4', None) - unit5 = api.create_container(self.lib1.library_key, api.ContainerType.Unit, 'unit-5', 'Unit 5', None) + unit4 = api.create_container(self.lib1.library_key, content_models.Unit, 'unit-4', 'Unit 4', None) + unit5 = api.create_container(self.lib1.library_key, content_models.Unit, 'unit-5', 'Unit 5', None) api.update_container_children( self.subsection2.container_key, @@ -1241,14 +1242,14 @@ def test_call_object_changed_signal_when_add_subsection(self) -> None: subsection3 = api.create_container( self.lib1.library_key, - api.ContainerType.Subsection, + content_models.Subsection, 'subsection-3', 'Subsection 3', None, ) subsection4 = api.create_container( self.lib1.library_key, - api.ContainerType.Subsection, + content_models.Subsection, 'subsection-4', 'Subsection 4', None, @@ -1323,7 +1324,7 @@ def test_copy_and_paste_container_same_library(self) -> None: ) # Verify that the container is copied - assert new_container.container_type == self.section1.container_type + assert new_container.container_type_code == self.section1.container_type_code assert new_container.display_name == self.section1.display_name # Verify that the children are linked @@ -1346,7 +1347,7 @@ def test_copy_and_paste_container_another_library(self) -> None: ) # Verify that the container is copied - assert new_container.container_type == self.section1.container_type + assert new_container.container_type_code == self.section1.container_type_code assert new_container.display_name == self.section1.display_name # Verify that the children are copied diff --git a/openedx/core/djangoapps/content_libraries/tests/test_containers.py b/openedx/core/djangoapps/content_libraries/tests/test_containers.py index 0d72d0dfcc51..b5f6326660b3 100644 --- a/openedx/core/djangoapps/content_libraries/tests/test_containers.py +++ b/openedx/core/djangoapps/content_libraries/tests/test_containers.py @@ -604,7 +604,7 @@ def test_unit_collections(self) -> None: assert unit_as_read['collections'] == [{"title": col1.title, "key": col1.key}] def test_section_hierarchy(self): - with self.assertNumQueries(133): + with self.assertNumQueries(126): hierarchy = self._get_container_hierarchy(self.section_with_subsections["id"]) assert hierarchy["object_key"] == self.section_with_subsections["id"] assert hierarchy["components"] == [ @@ -630,7 +630,7 @@ def test_section_hierarchy(self): ] def test_subsection_hierarchy(self): - with self.assertNumQueries(95): + with self.assertNumQueries(91): hierarchy = self._get_container_hierarchy(self.subsection_with_units["id"]) assert hierarchy["object_key"] == self.subsection_with_units["id"] assert hierarchy["components"] == [ @@ -653,7 +653,7 @@ def test_subsection_hierarchy(self): ] def test_units_hierarchy(self): - with self.assertNumQueries(60): + with self.assertNumQueries(56): hierarchy = self._get_container_hierarchy(self.unit_with_components["id"]) assert hierarchy["object_key"] == self.unit_with_components["id"] assert hierarchy["components"] == [ @@ -679,7 +679,7 @@ def test_container_hierarchy_not_found(self): ) def test_block_hierarchy(self): - with self.assertNumQueries(27): + with self.assertNumQueries(24): hierarchy = self._get_block_hierarchy(self.problem_block["id"]) assert hierarchy["object_key"] == self.problem_block["id"] assert hierarchy["components"] == [ diff --git a/openedx/core/lib/xblock_serializer/test_api.py b/openedx/core/lib/xblock_serializer/test_api.py index e11e278bf985..935700cead41 100644 --- a/openedx/core/lib/xblock_serializer/test_api.py +++ b/openedx/core/lib/xblock_serializer/test_api.py @@ -98,7 +98,7 @@ def setUpClass(cls): root1 = Tag.objects.create(taxonomy=cls.taxonomy1, value="ROOT1") root2 = Tag.objects.create(taxonomy=cls.taxonomy2, value="ROOT2") Tag.objects.create(taxonomy=cls.taxonomy1, value="normal tag", parent=root1) - Tag.objects.create(taxonomy=cls.taxonomy1, value=" tag", parent=root1) + Tag.objects.create(taxonomy=cls.taxonomy1, value=" tag", parent=root1) Tag.objects.create(taxonomy=cls.taxonomy1, value="anotherTag", parent=root1) Tag.objects.create(taxonomy=cls.taxonomy2, value="tag", parent=root2) Tag.objects.create(taxonomy=cls.taxonomy2, value="other tag", parent=root2) @@ -322,7 +322,7 @@ def test_tagged_units(self): tagging_api.tag_object( object_id=str(unit.location), taxonomy=self.taxonomy1, - tags=["normal tag", " tag", "anotherTag"] + tags=["normal tag", " tag", "anotherTag"] ) tagging_api.tag_object( object_id=str(unit.location), @@ -345,7 +345,7 @@ def test_tagged_units(self): self.assertEqual( serialized.tags, { str(unit.location): { - self.taxonomy1.id: ["normal tag", " tag", "anotherTag"], + self.taxonomy1.id: ["normal tag", " tag", "anotherTag"], self.taxonomy2.id: ["tag", "other tag"], } } @@ -371,7 +371,7 @@ def test_tagged_html_block(self): tagging_api.tag_object( object_id=str(html_block.location), taxonomy=self.taxonomy1, - tags=["normal tag", " tag", "anotherTag"] + tags=["normal tag", " tag", "anotherTag"] ) tagging_api.tag_object( object_id=str(html_block.location), @@ -398,7 +398,7 @@ def test_tagged_html_block(self): self.assertEqual( serialized.tags, { str(html_block.location): { - self.taxonomy1.id: ["normal tag", " tag", "anotherTag"], + self.taxonomy1.id: ["normal tag", " tag", "anotherTag"], self.taxonomy2.id: ["tag", "other tag"], } } @@ -436,7 +436,7 @@ def test_tagged_problem_blocks(self): tagging_api.tag_object( object_id=str(regular_problem.location), taxonomy=self.taxonomy1, - tags=["normal tag", " tag", "anotherTag"] + tags=["normal tag", " tag", "anotherTag"] ) tagging_api.tag_object( object_id=str(regular_problem.location), @@ -446,7 +446,7 @@ def test_tagged_problem_blocks(self): tagging_api.tag_object( object_id=str(python_problem.location), taxonomy=self.taxonomy1, - tags=["normal tag", " tag", "anotherTag"] + tags=["normal tag", " tag", "anotherTag"] ) tagging_api.tag_object( object_id=str(python_problem.location), @@ -472,7 +472,7 @@ def test_tagged_problem_blocks(self): self.assertEqual( serialized.tags, { str(regular_problem.location): { - self.taxonomy1.id: ["normal tag", " tag", "anotherTag"], + self.taxonomy1.id: ["normal tag", " tag", "anotherTag"], self.taxonomy2.id: ["tag", "other tag"], } } @@ -494,7 +494,7 @@ def test_tagged_problem_blocks(self): self.assertEqual( serialized.tags, { str(python_problem.location): { - self.taxonomy1.id: ["normal tag", " tag", "anotherTag"], + self.taxonomy1.id: ["normal tag", " tag", "anotherTag"], self.taxonomy2.id: ["tag", "other tag"], } } @@ -518,7 +518,7 @@ def test_tagged_library_content_blocks(self): tagging_api.tag_object( object_id=str(lc_block.location), taxonomy=self.taxonomy1, - tags=["normal tag", " tag", "anotherTag"] + tags=["normal tag", " tag", "anotherTag"] ) # Check that the tags data is serialized, omitted from the OLX, and properly escaped @@ -537,7 +537,7 @@ def test_tagged_library_content_blocks(self): ) self.assertEqual(serialized.tags, { str(lc_block.location): { - self.taxonomy1.id: ["normal tag", " tag", "anotherTag"], + self.taxonomy1.id: ["normal tag", " tag", "anotherTag"], } }) @@ -556,7 +556,7 @@ def test_tagged_video_block(self): tagging_api.tag_object( object_id=str(video_block.location), taxonomy=self.taxonomy1, - tags=["normal tag", " tag", "anotherTag"] + tags=["normal tag", " tag", "anotherTag"] ) # Check that the tags data is serialized and omitted from the OLX. @@ -574,7 +574,7 @@ def test_tagged_video_block(self): ) self.assertEqual(serialized.tags, { str(video_block.location): { - self.taxonomy1.id: ["normal tag", " tag", "anotherTag"], + self.taxonomy1.id: ["normal tag", " tag", "anotherTag"], } }) @@ -593,19 +593,19 @@ def test_tagged_openassessment_block(self): tagging_api.tag_object( object_id=str(openassessment_block.location), taxonomy=self.taxonomy1, - tags=["normal tag", " tag", "anotherTag"] + tags=["normal tag", " tag", "anotherTag"] ) # Check that the tags data is serialized and omitted from the OLX serialized = api.serialize_xblock_to_olx(openassessment_block) self.assertNotIn("normal tag", serialized.olx_str) - self.assertNotIn(" tag", serialized.olx_str) + self.assertNotIn(" tag", serialized.olx_str) self.assertNotIn("anotherTag", serialized.olx_str) self.assertEqual(serialized.tags, { str(openassessment_block.location): { - self.taxonomy1.id: ["normal tag", " tag", "anotherTag"], + self.taxonomy1.id: ["normal tag", " tag", "anotherTag"], } }) diff --git a/requirements/constraints.txt b/requirements/constraints.txt index deec7e9d954c..d14765d0dc81 100644 --- a/requirements/constraints.txt +++ b/requirements/constraints.txt @@ -65,7 +65,7 @@ numpy<2.0.0 # breaking changes which openedx-core devs want to roll out manually. New patch versions # are OK to accept automatically. # Issue for unpinning: https://github.com/openedx/edx-platform/issues/35269 -openedx-core<0.37 +openedx-core<0.39 # Date: 2023-11-29 # Open AI version 1.0.0 dropped support for openai.ChatCompletion which is currently in use in enterprise. diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index 749a694feac5..e89bbb77d151 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -829,7 +829,7 @@ openedx-calc==5.0.0 # via # -r requirements/edx/kernel.in # xblocks-contrib -openedx-core==0.36.0 +openedx-core==0.38.0 # via # -c requirements/constraints.txt # -r requirements/edx/kernel.in diff --git a/requirements/edx/development.txt b/requirements/edx/development.txt index 262980994b6e..4a9a2d55f155 100644 --- a/requirements/edx/development.txt +++ b/requirements/edx/development.txt @@ -1382,7 +1382,7 @@ openedx-calc==5.0.0 # -r requirements/edx/doc.txt # -r requirements/edx/testing.txt # xblocks-contrib -openedx-core==0.36.0 +openedx-core==0.38.0 # via # -c requirements/constraints.txt # -r requirements/edx/doc.txt diff --git a/requirements/edx/doc.txt b/requirements/edx/doc.txt index 5eea18d61031..659b862df2da 100644 --- a/requirements/edx/doc.txt +++ b/requirements/edx/doc.txt @@ -1007,7 +1007,7 @@ openedx-calc==5.0.0 # via # -r requirements/edx/base.txt # xblocks-contrib -openedx-core==0.36.0 +openedx-core==0.38.0 # via # -c requirements/constraints.txt # -r requirements/edx/base.txt diff --git a/requirements/edx/testing.txt b/requirements/edx/testing.txt index 734b9d6036ae..5fa000d059f7 100644 --- a/requirements/edx/testing.txt +++ b/requirements/edx/testing.txt @@ -1056,7 +1056,7 @@ openedx-calc==5.0.0 # via # -r requirements/edx/base.txt # xblocks-contrib -openedx-core==0.36.0 +openedx-core==0.38.0 # via # -c requirements/constraints.txt # -r requirements/edx/base.txt