diff --git a/src/openedx_content/admin.py b/src/openedx_content/admin.py index c6ec40633..02dad8063 100644 --- a/src/openedx_content/admin.py +++ b/src/openedx_content/admin.py @@ -8,6 +8,3 @@ from .applets.components.admin import * from .applets.media.admin import * from .applets.publishing.admin import * -from .applets.sections.admin import * -from .applets.subsections.admin import * -from .applets.units.admin import * diff --git a/src/openedx_content/api.py b/src/openedx_content/api.py index 9aaa9b7b9..7daeda408 100644 --- a/src/openedx_content/api.py +++ b/src/openedx_content/api.py @@ -15,6 +15,6 @@ from .applets.components.api import * from .applets.media.api import * from .applets.publishing.api import * -from .applets.sections.api import * -from .applets.subsections.api import * -from .applets.units.api import * +from .applets.sections.models import Section # Note this isn't a model. Should we move it? +from .applets.subsections.models import Subsection # Note this isn't a model. Should we move it? +from .applets.units.models import Unit # Note this isn't a model. Should we move it? diff --git a/src/openedx_content/applets/backup_restore/zipper.py b/src/openedx_content/applets/backup_restore/zipper.py index 1bb8bbba6..4c096386a 100644 --- a/src/openedx_content/applets/backup_restore/zipper.py +++ b/src/openedx_content/applets/backup_restore/zipper.py @@ -33,9 +33,6 @@ from ..components import api as components_api from ..media import api as media_api from ..publishing import api as publishing_api -from ..sections import api as sections_api -from ..subsections import api as subsections_api -from ..units import api as units_api from .serializers import ( CollectionSerializer, ComponentSerializer, diff --git a/src/openedx_content/applets/components/models.py b/src/openedx_content/applets/components/models.py index e34a39776..3ec181fa2 100644 --- a/src/openedx_content/applets/components/models.py +++ b/src/openedx_content/applets/components/models.py @@ -64,16 +64,16 @@ class ComponentType(models.Model): # the UsageKey. name = case_sensitive_char_field(max_length=100, blank=True) - # TODO: this needs to go into a class Meta - constraints = [ - models.UniqueConstraint( - fields=[ - "namespace", - "name", - ], - name="oel_component_type_uniq_ns_n", - ), - ] + class Meta: + constraints = [ + models.UniqueConstraint( + fields=[ + "namespace", + "name", + ], + name="oel_component_type_uniq_ns_n", + ), + ] def __str__(self) -> str: return f"{self.namespace}:{self.name}" diff --git a/src/openedx_content/applets/publishing/api.py b/src/openedx_content/applets/publishing/api.py index 64b190a09..4f0341c92 100644 --- a/src/openedx_content/applets/publishing/api.py +++ b/src/openedx_content/applets/publishing/api.py @@ -4,13 +4,14 @@ Please look at the models.py file for more information about the kinds of data are stored in this app. """ + from __future__ import annotations from contextlib import nullcontext from dataclasses import dataclass from datetime import datetime, timezone from enum import Enum -from typing import ContextManager, Optional, TypeVar +from typing import final, ContextManager, Optional from django.core.exceptions import ObjectDoesNotExist, ValidationError from django.db.models import F, Prefetch, Q, QuerySet @@ -21,6 +22,7 @@ from .contextmanagers import DraftChangeLogContext from .models import ( Container, + ContainerTypeRecord, ContainerVersion, Draft, DraftChangeLog, @@ -41,12 +43,6 @@ ) from .models.publish_log import Published -# A few of the APIs in this file are generic and can be used for Containers in -# general, or e.g. Units (subclass of Container) in particular. These type -# variables are used to provide correct typing for those generic API methods. -ContainerModel = TypeVar('ContainerModel', bound=Container) -ContainerVersionModel = TypeVar('ContainerVersionModel', bound=ContainerVersion) - # The public API that will be re-exported by openedx_content.api # is listed in the __all__ entries below. Internal helper functions that are # private to this module should start with an underscore. If a function does not @@ -78,17 +74,23 @@ "filter_publishable_entities", # 🛑 UNSTABLE: All APIs related to containers are unstable until we've figured # out our approach to dynamic content (randomized, A/B tests, etc.) + "ContainerTypeRecord", + "ContainerType", "create_container", "create_container_version", + "create_container_and_version", "create_next_container_version", "get_container", + "get_container_version", "get_container_by_key", + "get_container_type_code", + "get_container_type", "get_containers", "get_collection_containers", "ChildrenEntitiesAction", "ContainerEntityListEntry", - "ContainerEntityRow", "get_entities_in_container", + "get_entities_in_container_as_of", "contains_unpublished_changes", "get_containers_with_entity", "get_container_children_count", @@ -1344,13 +1346,134 @@ def get_published_version_as_of(entity_id: int, publish_log_id: int) -> Publisha This is a semi-private function, only available to other apps in the authoring package. """ - record = PublishLogRecord.objects.filter( - entity_id=entity_id, - publish_log_id__lte=publish_log_id, - ).order_by('-publish_log_id').first() + record = ( + PublishLogRecord.objects.filter( + entity_id=entity_id, + publish_log_id__lte=publish_log_id, + ) + .order_by("-publish_log_id") + .first() + ) return record.new_version if record else None +######################################################################################################################## + + +_registered_container_types: dict[str, ContainerTypeImplementation] = {} + + +class ContainerTypeImplementation: + """ + Abstract base class for container type implementations (Unit, Subsection, etc.) + """ + + type_code: str # e.g. "unit" + + @classmethod + def validate_entity(cls, entity: PublishableEntity) -> None: + """Check if the given entity is allowed as a child of this Container type""" + + @final + def __init__(self): + raise TypeError("ContainerType and its subclasses should not be initialized") + + @final + @classmethod + def get_type_record(cls) -> ContainerTypeRecord: + """ + Get the ContainerTypeRecord for this type of container, auto-creating it + if need be. + """ + if cls is ContainerTypeImplementation: + raise TypeError('Manipulating "naked" Containers is not allowed. Use a specific ContainerType like Unit.') + assert cls.type_code, f"ContainerTypeImplementation subclasses like {cls.__name__} must override type_code" + if not hasattr(cls, "_type_instance"): + cls._type_instance, _ = ContainerTypeRecord.objects.get_or_create(type_code=cls.type_code) + return cls._type_instance + + @staticmethod + def register(cti: type[ContainerTypeImplementation]): + assert cti.type_code, "ContainerTypeImplementation subclasses must override type_code" + assert cti.type_code not in _registered_container_types, f"{cti.type_code} already registered" + _registered_container_types[cti.type_code] = cti + + @staticmethod + def for_code(type_code: str) -> type[ContainerTypeImplementation]: + """ + Get the subclass for the specified container type_code. + """ + try: + return _registered_container_types[type_code] + except KeyError as exc: + raise ValueError( + 'An implementation for "{type_code}" containers is not currently installed. ' + "Such containers can be read but not modified." + ) from exc + + +@dataclass(frozen=True) +class ContainerEntityListEntry: + """ + [ 🛑 UNSTABLE ] + Data about a single entity in a container, e.g. a component in a unit. + """ + + entity_version: PublishableEntityVersion + pinned: bool + + @property + def entity(self): + return self.entity_version.entity + + +EntityListInput = list[ + PublishableEntity | PublishableEntityMixin | PublishableEntityVersion | PublishableEntityVersionMixin +] +ContainerType = type[ContainerTypeImplementation] + + +@dataclass(frozen=True, kw_only=True, slots=True) +class ParsedEntityReference: + """ + Internal format to represent an entity, and/or a specific version of an + entity. Used to construct entity lists. + + The public API contains `ContainerEntityListEntry` which plays a similar + role, but is only used when reading data out, not mutating containers. + """ + + entity_pk: int + version_pk: int | None = None + + @staticmethod + def parse(entities: EntityListInput) -> list[ParsedEntityReference]: + """ + Helper method to create a list of entities in the correct format. If you + pass `*Version` objects, they will be "frozen" at that version, whereas + if you pass `*Entity` objects, they'll use the latest version. + """ + new_list: list[ParsedEntityReference] = [] + for obj in entities: + if isinstance(obj, PublishableEntityMixin): + try: + obj = obj.publishable_entity + except obj.__class__.publishable_entity.RelatedObjectDoesNotExist as exc: + # If this happens, since it's a 1:1 relationship, likely both 'obj' (e.g. "Component") and + # 'obj.publishable_entity' have been deleted, so give a clearer error. + raise obj.DoesNotExist from exc + elif isinstance(obj, PublishableEntityVersionMixin): + obj = obj.publishable_entity_version + + if isinstance(obj, PublishableEntity): + new_list.append(ParsedEntityReference(entity_pk=obj.pk)) + elif isinstance(obj, PublishableEntityVersion): + new_list.append(ParsedEntityReference(entity_pk=obj.entity_id, version_pk=obj.pk)) + else: + raise TypeError(f"Unexpected entitity in list: {obj}") + return new_list + + def create_container( learning_package_id: int, key: str, @@ -1358,9 +1481,8 @@ def create_container( created_by: int | None, *, can_stand_alone: bool = True, - # The types on the following line are correct, but mypy will complain - https://github.com/python/mypy/issues/3737 - container_cls: type[ContainerModel] = Container, # type: ignore[assignment] -) -> ContainerModel: + container_type: ContainerType, +) -> Container: """ [ 🛑 UNSTABLE ] Create a new container. @@ -1371,12 +1493,12 @@ def create_container( created: The date and time the container was created. created_by: The ID of the user who created the container can_stand_alone: Set to False when created as part of containers - container_cls: The subclass of Container to use, if applicable + container_type: The type of container to create (e.g. Unit) Returns: The newly created container. """ - assert issubclass(container_cls, Container) + assert issubclass(container_type, ContainerTypeImplementation) with atomic(): publishable_entity = create_publishable_entity( learning_package_id, @@ -1385,8 +1507,9 @@ def create_container( created_by, can_stand_alone=can_stand_alone, ) - container = container_cls.objects.create( + container = Container.objects.create( publishable_entity=publishable_entity, + container_type_record=container_type.get_type_record(), ) return container @@ -1404,7 +1527,7 @@ def create_entity_list() -> EntityList: def create_entity_list_with_rows( - entity_rows: list[ContainerEntityRow], + parsed_entities: list[ParsedEntityReference], *, learning_package_id: int | None, ) -> EntityList: @@ -1413,7 +1536,11 @@ def create_entity_list_with_rows( Create new entity list rows for an entity list. Args: - entity_rows: List of ContainerEntityRows specifying the publishable entity ID and version ID (if pinned). + entities: List of the entities that will comprise the entity list, in + order. Pass `PublishableEntityVersion` or objects that use + `PublishableEntityVersionMixin` to pin to a specific version. Pass + `PublishableEntity` or objects that use `PublishableEntityMixin` for + unpinned. learning_package_id: Optional. Verify that all the entities are from the specified learning package. @@ -1422,26 +1549,17 @@ def create_entity_list_with_rows( """ # Do a quick check that the given entities are in the right learning package: if learning_package_id: - if PublishableEntity.objects.filter( - pk__in=[entity.entity_pk for entity in entity_rows], - ).exclude( - learning_package_id=learning_package_id, - ).exists(): + if ( + PublishableEntity.objects.filter( + pk__in=[entity.entity_pk for entity in parsed_entities], + ) + .exclude( + learning_package_id=learning_package_id, + ) + .exists() + ): raise ValidationError("Container entities must be from the same learning package.") - # Ensure that any pinned entity versions are linked to the correct entity - pinned_entities = { - entity.version_pk: entity.entity_pk - for entity in entity_rows if entity.pinned - } - if pinned_entities: - entity_versions = PublishableEntityVersion.objects.filter( - pk__in=pinned_entities.keys(), - ).only('pk', 'entity_id') - for entity_version in entity_versions: - if pinned_entities[entity_version.pk] != entity_version.entity_id: - raise ValidationError("Container entity versions must belong to the specified entity.") - with atomic(savepoint=False): entity_list = create_entity_list() EntityListRow.objects.bulk_create( @@ -1452,7 +1570,7 @@ def create_entity_list_with_rows( order_num=order_num, entity_version_id=entity.version_pk, ) - for order_num, entity in enumerate(entity_rows) + for order_num, entity in enumerate(parsed_entities) ] ) return entity_list @@ -1466,13 +1584,23 @@ def _create_container_version( entity_list: EntityList, created: datetime, created_by: int | None, - container_version_cls: type[ContainerVersionModel] = ContainerVersion, # type: ignore[assignment] -) -> ContainerVersionModel: +) -> ContainerVersion: """ Private internal method for logic shared by create_container_version() and create_next_container_version(). """ - assert issubclass(container_version_cls, ContainerVersion) + # validate entity_list using the type implementation: + container_type = ContainerTypeImplementation.for_code(container.container_type_record.type_code) + for entity_row in entity_list.rows: + try: + container_type.validate_entity(entity_row.entity) + except Exception as exc: + # This exception is carefully worded. The validation may have failed because the entity is of the wrong + # type, but it _could_ be a of the correct type but otherwise invalid/corrupt, e.g. partially deleted. + raise ValidationError( + f'The entity "{entity_row.entity}" cannot be added to a "{container_type.type_code}" container.' + ) from exc + with atomic(savepoint=False): # Make sure this will happen atomically but we don't need to create a new savepoint. publishable_entity_version = create_publishable_entity_version( container.publishable_entity_id, @@ -1480,13 +1608,9 @@ def _create_container_version( title=title, created=created, created_by=created_by, - dependencies=[ - entity_row.entity_id - for entity_row in entity_list.rows - if entity_row.is_unpinned() - ] + dependencies=[entity_row.entity_id for entity_row in entity_list.rows if entity_row.is_unpinned()], ) - container_version = container_version_cls.objects.create( + container_version = ContainerVersion.objects.create( publishable_entity_version=publishable_entity_version, container_id=container.pk, entity_list=entity_list, @@ -1500,11 +1624,10 @@ def create_container_version( version_num: int, *, title: str, - entity_rows: list[ContainerEntityRow], + entities: EntityListInput, created: datetime, created_by: int | None, - container_version_cls: type[ContainerVersionModel] = ContainerVersion, # type: ignore[assignment] -) -> ContainerVersionModel: +) -> ContainerVersion: """ [ 🛑 UNSTABLE ] Create a new container version. @@ -1513,24 +1636,25 @@ def create_container_version( container_id: The ID of the container that the version belongs to. version_num: The version number of the container. title: The title of the container. - entity_rows: List of ContainerEntityRows specifying the publishable entity ID and version ID (if pinned). + entities: List of the entities that will comprise the entity list, in + order. Pass `PublishableEntityVersion` or objects that use + `PublishableEntityVersionMixin` to pin to a specific version. Pass + `PublishableEntity` or objects that use `PublishableEntityMixin` for + unpinned. created: The date and time the container version was created. created_by: The ID of the user who created the container version. - container_version_cls: The subclass of ContainerVersion to use, if applicable. Returns: The newly created container version. """ assert title is not None - assert entity_rows is not None + assert entities is not None with atomic(savepoint=False): container = Container.objects.select_related("publishable_entity").get(pk=container_id) entity = container.publishable_entity - entity_list = create_entity_list_with_rows( - entity_rows=entity_rows, - learning_package_id=entity.learning_package_id, - ) + parsed_entities = ParsedEntityReference.parse(entities) + entity_list = create_entity_list_with_rows(parsed_entities, learning_package_id=entity.learning_package_id) container_version = _create_container_version( container, version_num, @@ -1538,12 +1662,59 @@ def create_container_version( entity_list=entity_list, created=created, created_by=created_by, - container_version_cls=container_version_cls, ) return container_version +def create_container_and_version( + learning_package_id: int, + key: str, + *, + title: str, + container_type: ContainerType, + entities: EntityListInput | None = None, + created: datetime, + created_by: int | None = None, + can_stand_alone: bool = True, +) -> tuple[Container, ContainerVersion]: + """ + [ 🛑 UNSTABLE ] Create a new unit and its version. + + Args: + learning_package_id: The learning package ID. + key: The key. + title: The title of the new container. + container_type: The type of container to create (e.g. Unit) + entities: List of the entities that will comprise the entity list, in + order. Pass `PublishableEntityVersion` or objects that use + `PublishableEntityVersionMixin` to pin to a specific version. Pass + `PublishableEntity` or objects that use `PublishableEntityMixin` for + unpinned. Pass `None` for "no change". + created: The creation date. + created_by: The user who created the unit. + can_stand_alone: Set to False when created as part of containers + """ + with atomic(savepoint=False): + container = create_container( + learning_package_id, + key, + created, + created_by, + can_stand_alone=can_stand_alone, + container_type=container_type, + ) + container_version = create_container_version( + container, + 1, + title=title, + entities=entities or [], + created=created, + created_by=created_by, + ) + return container, container_version + + class ChildrenEntitiesAction(Enum): """Possible actions for children entities""" @@ -1555,7 +1726,7 @@ class ChildrenEntitiesAction(Enum): def create_next_entity_list( learning_package_id: int, last_version: ContainerVersion, - entity_rows: list[ContainerEntityRow], + entities: EntityListInput, entities_action: ChildrenEntitiesAction = ChildrenEntitiesAction.REPLACE, ) -> EntityList: """ @@ -1564,59 +1735,65 @@ def create_next_entity_list( Args: learning_package_id: Learning package ID last_version: Last version of container. - entity_rows: List of ContainerEntityRows specifying the publishable entity ID and version ID (if pinned). + entities: List of the entities that will comprise the entity list, in + order. Pass `PublishableEntityVersion` or objects that use + `PublishableEntityVersionMixin` to pin to a specific version. Pass + `PublishableEntity` or objects that use `PublishableEntityMixin` for + unpinned. entities_action: APPEND, REMOVE or REPLACE given entities from/to the container Returns: The newly created entity list. """ + parsed_entities = ParsedEntityReference.parse(entities) + # Do a quick check that the given entities are in the right learning package: + if learning_package_id: + if ( + PublishableEntity.objects.filter( + pk__in=[entity.entity_pk for entity in parsed_entities], + ) + .exclude( + learning_package_id=learning_package_id, + ) + .exists() + ): + raise ValidationError("Container entities must be from the same learning package.") + if entities_action == ChildrenEntitiesAction.APPEND: # get previous entity list rows - last_entities = last_version.entity_list.entitylistrow_set.only( - "entity_id", - "entity_version_id" - ).order_by("order_num") + last_entities = last_version.entity_list.entitylistrow_set.only("entity_id", "entity_version_id").order_by( + "order_num" + ) # append given entity_rows to the existing children - entity_rows = [ - ContainerEntityRow( - entity_pk=entity.entity_id, - version_pk=entity.entity_version_id, - ) + parsed_entities = [ + ParsedEntityReference(entity_pk=entity.entity_id, version_pk=entity.entity_version_id) for entity in last_entities - ] + entity_rows + ] + parsed_entities elif entities_action == ChildrenEntitiesAction.REMOVE: # get previous entity list, excluding the entities in entity_rows - last_entities = last_version.entity_list.entitylistrow_set.only( - "entity_id", - "entity_version_id" - ).exclude( - entity_id__in=[entity.entity_pk for entity in entity_rows] - ).order_by("order_num") - entity_rows = [ - ContainerEntityRow( - entity_pk=entity.entity_id, - version_pk=entity.entity_version_id, - ) + last_entities = ( + last_version.entity_list.entitylistrow_set.only("entity_id", "entity_version_id") + .exclude(entity_id__in=[entity.entity_pk for entity in parsed_entities]) + .order_by("order_num") + ) + parsed_entities = [ + ParsedEntityReference(entity_pk=entity.entity_id, version_pk=entity.entity_version_id) for entity in last_entities.all() ] - return create_entity_list_with_rows( - entity_rows=entity_rows, - learning_package_id=learning_package_id, - ) + return create_entity_list_with_rows(parsed_entities, learning_package_id=learning_package_id) def create_next_container_version( container_pk: int, *, title: str | None, - entity_rows: list[ContainerEntityRow] | None, + entities: EntityListInput | None = None, created: datetime, created_by: int | None, - container_version_cls: type[ContainerVersionModel] = ContainerVersion, # type: ignore[assignment] entities_action: ChildrenEntitiesAction = ChildrenEntitiesAction.REPLACE, force_version_num: int | None = None, -) -> ContainerVersionModel: +) -> ContainerVersion: """ [ 🛑 UNSTABLE ] Create the next version of a container. A new version of the container is created @@ -1631,8 +1808,11 @@ def create_next_container_version( Args: container_pk: The ID of the container to create the next version of. title: The title of the container. None to keep the current title. - entity_rows: List of ContainerEntityRows specifying the publishable entity ID and version ID (if pinned). - Or None for no change. + entities: List of the entities that will comprise the entity list, in + order. Pass `PublishableEntityVersion` or objects that use + `PublishableEntityVersionMixin` to pin to a specific version. Pass + `PublishableEntity` or objects that use `PublishableEntityMixin` for + unpinned. Pass `None` for "no change". created: The date and time the container version was created. created_by: The ID of the user who created the container version. container_version_cls: The subclass of ContainerVersion to use, if applicable. @@ -1649,7 +1829,6 @@ def create_next_container_version( importing legacy data, or synchronizing with another system), use force_version_num to override the default behavior. """ - assert issubclass(container_version_cls, ContainerVersion) with atomic(): container = Container.objects.select_related("publishable_entity").get(pk=container_pk) entity = container.publishable_entity @@ -1662,15 +1841,12 @@ def create_next_container_version( if force_version_num is not None: next_version_num = force_version_num - if entity_rows is None and last_version is not None: + if entities is None and last_version is not None: # We're only changing metadata. Keep the same entity list. next_entity_list = last_version.entity_list else: next_entity_list = create_next_entity_list( - entity.learning_package_id, - last_version, - entity_rows if entity_rows is not None else [], - entities_action + entity.learning_package_id, last_version, entities if entities is not None else [], entities_action ) next_container_version = _create_container_version( @@ -1680,7 +1856,6 @@ def create_next_container_version( entity_list=next_entity_list, created=created, created_by=created_by, - container_version_cls=container_version_cls, ) return next_container_version @@ -1700,6 +1875,20 @@ def get_container(pk: int) -> Container: return Container.objects.get(pk=pk) +def get_container_version(container_version_pk: int) -> ContainerVersion: + """ + [ 🛑 UNSTABLE ] + Get a container version by its primary key. + + Args: + pk: The primary key of the container version. + + Returns: + The container version with the given primary key. + """ + return ContainerVersion.objects.get(pk=container_version_pk) + + def get_container_by_key(learning_package_id: int, /, key: str) -> Container: """ [ 🛑 UNSTABLE ] @@ -1718,11 +1907,27 @@ def get_container_by_key(learning_package_id: int, /, key: str) -> Container: ) +def get_container_type_code(container: Container | int, /) -> str: + """Get the type of a container, as a string - e.g. "unit".""" + if not isinstance(container, Container): + container = get_container(container) + return container.container_type_record.type_code + + +def get_container_type(container: Container | int, /) -> ContainerType: + """ + Get the type of a container. + + Will raise a ValueError if the type is not currently installed. + """ + type_code = get_container_type_code(container) + return ContainerTypeImplementation.for_code(type_code) + + def get_containers( learning_package_id: int, - container_cls: type[ContainerModel] = Container, # type: ignore[assignment] include_deleted: bool | None = False, -) -> QuerySet[ContainerModel]: +) -> QuerySet[Container]: """ [ 🛑 UNSTABLE ] Get all containers in the given learning package. @@ -1735,12 +1940,11 @@ def get_containers( Returns: A queryset containing the container associated with the given learning package. """ - assert issubclass(container_cls, Container) - container_qset = container_cls.objects.filter(publishable_entity__learning_package=learning_package_id) + container_qset = Container.objects.filter(publishable_entity__learning_package=learning_package_id) if not include_deleted: container_qset = container_qset.filter(publishable_entity__draft__version__isnull=False) - return container_qset.order_by('pk') + return container_qset.order_by("pk") def get_collection_containers( @@ -1755,38 +1959,7 @@ def get_collection_containers( return Container.objects.filter( publishable_entity__learning_package_id=learning_package_id, publishable_entity__collections__key=collection_key, - ).order_by('pk') - - -@dataclass(frozen=True) -class ContainerEntityListEntry: - """ - [ 🛑 UNSTABLE ] - Data about a single entity in a container, e.g. a component in a unit. - """ - entity_version: PublishableEntityVersion - pinned: bool - - @property - def entity(self): - return self.entity_version.entity - - -@dataclass(frozen=True, kw_only=True, slots=True) -class ContainerEntityRow: - """ - [ 🛑 UNSTABLE ] - Used to specify the primary key of PublishableEntity and optional PublishableEntityVersion. - - If version_pk is None (default), then the entity is considered "unpinned", - meaning that the latest version of the entity will be used. - """ - entity_pk: int - version_pk: int | None = None - - @property - def pinned(self): - return self.entity_pk and self.version_pk is not None + ).order_by("pk") def get_entities_in_container( @@ -1812,7 +1985,8 @@ def get_entities_in_container( if published: # Very minor optimization: reload the container with related 1:1 entities container = Container.objects.select_related( - "publishable_entity__published__version__containerversion__entity_list").get(pk=container.pk) + "publishable_entity__published__version__containerversion__entity_list" + ).get(pk=container.pk) container_version = container.versioning.published select_related = ["entity__published__version"] if select_related_version: @@ -1820,7 +1994,8 @@ def get_entities_in_container( else: # Very minor optimization: reload the container with related 1:1 entities container = Container.objects.select_related( - "publishable_entity__draft__version__containerversion__entity_list").get(pk=container.pk) + "publishable_entity__draft__version__containerversion__entity_list" + ).get(pk=container.pk) container_version = container.versioning.draft select_related = ["entity__draft__version"] if select_related_version: @@ -1837,15 +2012,54 @@ def get_entities_in_container( if not entity_version: # If this entity is "unpinned", use the latest published/draft version: entity_version = row.entity.published.version if published else row.entity.draft.version if entity_version is not None: # As long as this hasn't been soft-deleted: - entity_list.append(ContainerEntityListEntry( - entity_version=entity_version, - pinned=row.entity_version is not None, - )) + entity_list.append( + ContainerEntityListEntry( + entity_version=entity_version, + pinned=row.entity_version is not None, + ) + ) # else we could indicate somehow a deleted item was here, e.g. by returning a ContainerEntityListEntry with # deleted=True, but we don't have a use case for that yet. return entity_list +def get_entities_in_container_as_of( + container: Container, + publish_log_id: int, +) -> tuple[ContainerVersion | None, list[ContainerEntityListEntry]]: + """ + [ 🛑 UNSTABLE ] + Get the list of entities and their versions in the published version of the + given container as of the given PublishLog version (which is essentially a + version for the entire learning package). + + Also returns the ContainerVersion so you can see the container title, + settings?, and any other metadata from that point in time. + + TODO: optimize, perhaps by having the publishlog store a record of all + ancestors of every modified PublishableEntity in the publish. + """ + assert isinstance(container, Container) + pub_entity_version = get_published_version_as_of(container.publishable_entity_id, publish_log_id) + if pub_entity_version is None: + return None, [] # This container was not published as of the given PublishLog ID. + container_version = pub_entity_version.containerversion + + entity_list: list[ContainerEntityListEntry] = [] + rows = container_version.entity_list.entitylistrow_set.order_by("order_num") + for row in rows: + if row.entity_version is not None: + # Pinned child entity: + entity_list.append(ContainerEntityListEntry(entity_version=row.entity_version, pinned=True)) + else: + # Unpinned entity - figure out what its latest published version was. + # This is not optimized. It could be done in one query per unit rather than one query per component. + pub_entity_version = get_published_version_as_of(row.entity_id, publish_log_id) + if pub_entity_version: + entity_list.append(ContainerEntityListEntry(entity_version=pub_entity_version, pinned=False)) + return container_version, entity_list + + def contains_unpublished_changes(container_id: int) -> bool: """ [ 🛑 UNSTABLE ] @@ -1865,10 +2079,9 @@ def contains_unpublished_changes(container_id: int) -> bool: in either case. """ container = ( - Container.objects - .select_related('publishable_entity__draft__draft_log_record') - .select_related('publishable_entity__published__publish_log_record') - .get(pk=container_id) + Container.objects.select_related("publishable_entity__draft__draft_log_record") + .select_related("publishable_entity__published__publish_log_record") + .get(pk=container_id) ) if container.versioning.has_unpublished_changes: return True @@ -1913,8 +2126,7 @@ def get_containers_with_entity( # Note: these two conditions must be in the same filter() call, # or the query won't be correct. ( - f"publishable_entity__{branch}__version__" - "containerversion__entity_list__entitylistrow__entity_id" + f"publishable_entity__{branch}__version__containerversion__entity_list__entitylistrow__entity_id" ): publishable_entity_pk, ( f"publishable_entity__{branch}__version__" @@ -1925,8 +2137,7 @@ def get_containers_with_entity( else: filter_dict = { ( - f"publishable_entity__{branch}__version__" - "containerversion__entity_list__entitylistrow__entity_id" + f"publishable_entity__{branch}__version__containerversion__entity_list__entitylistrow__entity_id" ): publishable_entity_pk } qs = Container.objects.filter(**filter_dict) @@ -1970,9 +2181,7 @@ def get_container_children_entities_keys(container_version: ContainerVersion) -> A list of entity keys for all entities in the container version, ordered by entity key. """ return list( - container_version.entity_list.entitylistrow_set - .values_list("entity__key", flat=True) - .order_by("order_num") + container_version.entity_list.entitylistrow_set.values_list("entity__key", flat=True).order_by("order_num") ) diff --git a/src/openedx_content/applets/publishing/models/__init__.py b/src/openedx_content/applets/publishing/models/__init__.py index 32a73b214..6abb16dd5 100644 --- a/src/openedx_content/applets/publishing/models/__init__.py +++ b/src/openedx_content/applets/publishing/models/__init__.py @@ -13,7 +13,7 @@ * Storing and querying publish history. """ -from .container import Container, ContainerVersion +from .container import Container, ContainerTypeRecord, ContainerVersion from .draft_log import Draft, DraftChangeLog, DraftChangeLogRecord, DraftSideEffect from .entity_list import EntityList, EntityListRow from .learning_package import LearningPackage diff --git a/src/openedx_content/applets/publishing/models/container.py b/src/openedx_content/applets/publishing/models/container.py index e34bb6a7e..b32ac097b 100644 --- a/src/openedx_content/applets/publishing/models/container.py +++ b/src/openedx_content/applets/publishing/models/container.py @@ -1,13 +1,50 @@ """ Container and ContainerVersion models """ + +from __future__ import annotations + from django.core.exceptions import ValidationError from django.db import models +from openedx_django_lib.fields import case_sensitive_char_field + from .entity_list import EntityList from .publishable_entity import PublishableEntityMixin, PublishableEntityVersionMixin +class ContainerTypeRecord(models.Model): + """ + Normalized representation of the type of Container. + + Typical container types are "unit", "subsection", and "section", but there + may be others in the future. + """ + + id = models.AutoField(primary_key=True) + + # type_code uniquely identifies the type of container, e.g. "unit", "subsection", etc. + # Plugins/apps that add their own ContainerTypes should prefix it, e.g. + # "myapp_custom_unit" instead of "custom_unit", to avoid collisions. + type_code = case_sensitive_char_field( + max_length=100, + blank=False, + unique=True, + ) + + class Meta: + constraints = [ + models.CheckConstraint( + # No whitespace, uppercase, or special characters allowed in "type_code". + condition=models.lookups.Regex(models.F("type_code"), r"^[a-z0-9\-_\.]+$"), + name="oex_publishing_containertyperecord_type_code_rx", + ), + ] + + def __str__(self) -> str: + return self.type_code + + class Container(PublishableEntityMixin): """ A Container is a type of PublishableEntity that holds other @@ -18,12 +55,16 @@ class Container(PublishableEntityMixin): containers/components/enities they hold. As we complete the Containers API, we will also add support for dynamic containers which may contain different entities for different learners or at different times. - - NOTE: We're going to want to eventually have some association between the - PublishLog and Containers that were affected in a publish because their - child elements were published. """ + # The type of the container. Cannot be changed once the container is created. + container_type_record = models.ForeignKey( + ContainerTypeRecord, + null=False, + on_delete=models.RESTRICT, + editable=False, + ) + class ContainerVersion(PublishableEntityVersionMixin): """ @@ -46,6 +87,8 @@ class ContainerVersion(PublishableEntityVersionMixin): changes for a given ContainerVersion. """ + container_class: type[Container] # Subclasses need to specify this + container = models.ForeignKey( Container, on_delete=models.CASCADE, diff --git a/src/openedx_content/applets/sections/admin.py b/src/openedx_content/applets/sections/admin.py deleted file mode 100644 index e0c081059..000000000 --- a/src/openedx_content/applets/sections/admin.py +++ /dev/null @@ -1,48 +0,0 @@ -""" -Django admin for sections models -""" -from django.contrib import admin -from django.utils.safestring import SafeText - -from openedx_django_lib.admin_utils import ReadOnlyModelAdmin, model_detail_link - -from ..publishing.models import ContainerVersion -from .models import Section, SectionVersion - - -class SectionVersionInline(admin.TabularInline): - """ - Minimal table for section versions in a section. - - (Generally, this information is useless, because each SectionVersion should have a - matching ContainerVersion, shown in much more detail on the Container detail page. - But we've hit at least one bug where ContainerVersions were being created without - their connected SectionVersions, so we'll leave this table here for debugging - at least until we've made the APIs more robust against that sort of data corruption.) - """ - model = SectionVersion - fields = ["pk"] - readonly_fields = ["pk"] - ordering = ["-pk"] # Newest first - - def pk(self, obj: ContainerVersion) -> SafeText: - return obj.pk - - -@admin.register(Section) -class SectionAdmin(ReadOnlyModelAdmin): - """ - Very minimal interface... just direct the admin user's attention towards the related Container model admin. - """ - inlines = [SectionVersionInline] - list_display = ["pk", "key"] - fields = ["key"] - readonly_fields = ["key"] - - def key(self, obj: Section) -> SafeText: - return model_detail_link(obj.container, obj.key) - - def get_form(self, request, obj=None, change=False, **kwargs): - help_texts = {'key': f'For more details of this {self.model.__name__}, click above to see its Container view'} - kwargs.update({'help_texts': help_texts}) - return super().get_form(request, obj, **kwargs) diff --git a/src/openedx_content/applets/sections/api.py b/src/openedx_content/applets/sections/api.py deleted file mode 100644 index 05e0d6141..000000000 --- a/src/openedx_content/applets/sections/api.py +++ /dev/null @@ -1,330 +0,0 @@ -"""Sections API. - -This module provides functions to manage sections. -""" -from dataclasses import dataclass -from datetime import datetime - -from django.db.transaction import atomic - -from ..publishing import api as publishing_api -from ..subsections.models import Subsection, SubsectionVersion -from .models import Section, SectionVersion - -# 🛑 UNSTABLE: All APIs related to containers are unstable until we've figured -# out our approach to dynamic content (randomized, A/B tests, etc.) -__all__ = [ - "create_section", - "create_section_version", - "create_next_section_version", - "create_section_and_version", - "get_section", - "get_section_version", - "get_latest_section_version", - "SectionListEntry", - "get_subsections_in_section", - "get_subsections_in_published_section_as_of", -] - - -def create_section( - learning_package_id: int, - key: str, - created: datetime, - created_by: int | None, - *, - can_stand_alone: bool = True, -) -> Section: - """ - [ 🛑 UNSTABLE ] Create a new section. - - Args: - learning_package_id: The learning package ID. - key: The key. - created: The creation date. - created_by: The user who created the section. - can_stand_alone: Set to False when created as part of containers - """ - return publishing_api.create_container( - learning_package_id, - key, - created, - created_by, - can_stand_alone=can_stand_alone, - container_cls=Section, - ) - - -def create_section_version( - section: Section, - version_num: int, - *, - title: str, - entity_rows: list[publishing_api.ContainerEntityRow], - created: datetime, - created_by: int | None = None, -) -> SectionVersion: - """ - [ 🛑 UNSTABLE ] Create a new section version. - - This is a very low-level API, likely only needed for import/export. In - general, you will use `create_section_and_version()` and - `create_next_section_version()` instead. - - Args: - section_pk: The section ID. - version_num: The version number. - title: The title. - entity_rows: child entities/versions - created: The creation date. - created_by: The user who created the section. - """ - return publishing_api.create_container_version( - section.pk, - version_num, - title=title, - entity_rows=entity_rows, - created=created, - created_by=created_by, - container_version_cls=SectionVersion, - ) - - -def _pub_entities_for_subsections( - subsections: list[Subsection | SubsectionVersion] | None, -) -> list[publishing_api.ContainerEntityRow] | None: - """ - Helper method: given a list of Subsection | SubsectionVersion, return the - lists of publishable_entities_pks and entity_version_pks needed for the - base container APIs. - - SubsectionVersion is passed when we want to pin a specific version, otherwise - Subsection is used for unpinned. - """ - if subsections is None: - # When these are None, that means don't change the entities in the list. - return None - for u in subsections: - if not isinstance(u, (Subsection, SubsectionVersion)): - raise TypeError("Section subsections must be either Subsection or SubsectionVersion.") - return [ - ( - publishing_api.ContainerEntityRow( - entity_pk=s.container.publishable_entity_id, - version_pk=None, - ) if isinstance(s, Subsection) - else publishing_api.ContainerEntityRow( - entity_pk=s.subsection.container.publishable_entity_id, - version_pk=s.container_version.publishable_entity_version_id, - ) - ) - for s in subsections - ] - - -def create_next_section_version( - section: Section, - *, - title: str | None = None, - subsections: list[Subsection | SubsectionVersion] | None = None, - created: datetime, - created_by: int | None = None, - entities_action: publishing_api.ChildrenEntitiesAction = publishing_api.ChildrenEntitiesAction.REPLACE, - force_version_num: int | None = None, -) -> SectionVersion: - """ - [ 🛑 UNSTABLE ] Create the next section version. - - Args: - section_pk: The section ID. - title: The title. Leave as None to keep the current title. - subsections: The subsections, as a list of Subsections (unpinned) and/or SubsectionVersions (pinned). - Passing None will leave the existing subsections unchanged. - created: The creation date. - created_by: The user who created the section. - force_version_num (int, optional): If provided, overrides the automatic version number increment and sets - this version's number explicitly. Use this if you need to restore or import a version with a specific - version number, such as during data migration or when synchronizing with external systems. - - Returns: - The newly created SectionVersion. - - Why use force_version_num? - Normally, the version number is incremented automatically from the latest version. - If you need to set a specific version number (for example, when restoring from backup, - importing legacy data, or synchronizing with another system), - use force_version_num to override the default behavior. - - Why not use create_component_version? - The main reason is that we want to reuse the logic for adding entities to this container. - """ - entity_rows = _pub_entities_for_subsections(subsections) - section_version = publishing_api.create_next_container_version( - section.pk, - title=title, - entity_rows=entity_rows, - created=created, - created_by=created_by, - container_version_cls=SectionVersion, - entities_action=entities_action, - force_version_num=force_version_num, - ) - return section_version - - -def create_section_and_version( - learning_package_id: int, - key: str, - *, - title: str, - subsections: list[Subsection | SubsectionVersion] | None = None, - created: datetime, - created_by: int | None = None, - can_stand_alone: bool = True, -) -> tuple[Section, SectionVersion]: - """ - [ 🛑 UNSTABLE ] Create a new section and its version. - - Args: - learning_package_id: The learning package ID. - key: The key. - created: The creation date. - created_by: The user who created the section. - can_stand_alone: Set to False when created as part of containers - """ - entity_rows = _pub_entities_for_subsections(subsections) - with atomic(): - section = create_section( - learning_package_id, - key, - created, - created_by, - can_stand_alone=can_stand_alone, - ) - section_version = create_section_version( - section, - 1, - title=title, - entity_rows=entity_rows or [], - created=created, - created_by=created_by, - ) - return section, section_version - - -def get_section(section_pk: int) -> Section: - """ - [ 🛑 UNSTABLE ] Get a section. - - Args: - section_pk: The section ID. - """ - return Section.objects.get(pk=section_pk) - - -def get_section_version(section_version_pk: int) -> SectionVersion: - """ - [ 🛑 UNSTABLE ] Get a section version. - - Args: - section_version_pk: The section version ID. - """ - return SectionVersion.objects.get(pk=section_version_pk) - - -def get_latest_section_version(section_pk: int) -> SectionVersion: - """ - [ 🛑 UNSTABLE ] Get the latest section version. - - Args: - section_pk: The section ID. - """ - return Section.objects.get(pk=section_pk).versioning.latest - - -@dataclass(frozen=True) -class SectionListEntry: - """ - [ 🛑 UNSTABLE ] - Data about a single entity in a container, e.g. a subsection in a section. - """ - subsection_version: SubsectionVersion - pinned: bool = False - - @property - def subsection(self): - return self.subsection_version.subsection - - -def get_subsections_in_section( - section: Section, - *, - published: bool, -) -> list[SectionListEntry]: - """ - [ 🛑 UNSTABLE ] - Get the list of entities and their versions in the draft or published - version of the given Section. - - Args: - section: The Section, e.g. returned by `get_section()` - published: `True` if we want the published version of the section, or - `False` for the draft version. - """ - assert isinstance(section, Section) - subsections = [] - entries = publishing_api.get_entities_in_container( - section, - published=published, - select_related_version="containerversion__subsectionversion", - ) - for entry in entries: - # Convert from generic PublishableEntityVersion to SubsectionVersion: - subsection_version = entry.entity_version.containerversion.subsectionversion - assert isinstance(subsection_version, SubsectionVersion) - subsections.append(SectionListEntry(subsection_version=subsection_version, pinned=entry.pinned)) - return subsections - - -def get_subsections_in_published_section_as_of( - section: Section, - publish_log_id: int, -) -> list[SectionListEntry] | None: - """ - [ 🛑 UNSTABLE ] - Get the list of entities and their versions in the published version of the - given container as of the given PublishLog version (which is essentially a - version for the entire learning package). - - TODO: This API should be updated to also return the SectionVersion so we can - see the section title and any other metadata from that point in time. - TODO: accept a publish log UUID, not just int ID? - TODO: move the implementation to be a generic 'containers' implementation - that this sections function merely wraps. - TODO: optimize, perhaps by having the publishlog store a record of all - ancestors of every modified PublishableEntity in the publish. - """ - assert isinstance(section, Section) - section_pub_entity_version = publishing_api.get_published_version_as_of( - section.publishable_entity_id, publish_log_id - ) - if section_pub_entity_version is None: - return None # This section was not published as of the given PublishLog ID. - container_version = section_pub_entity_version.containerversion - - entity_list = [] - rows = container_version.entity_list.entitylistrow_set.order_by("order_num") - for row in rows: - if row.entity_version is not None: - subsection_version = row.entity_version.containerversion.subsectionversion - assert isinstance(subsection_version, SubsectionVersion) - entity_list.append(SectionListEntry(subsection_version=subsection_version, pinned=True)) - else: - # Unpinned subsection - figure out what its latest published version was. - # This is not optimized. It could be done in one query per section rather than one query per subsection. - pub_entity_version = publishing_api.get_published_version_as_of(row.entity_id, publish_log_id) - if pub_entity_version: - entity_list.append(SectionListEntry( - subsection_version=pub_entity_version.containerversion.subsectionversion, pinned=False - )) - return entity_list diff --git a/src/openedx_content/applets/sections/models.py b/src/openedx_content/applets/sections/models.py index afcb0ae0c..f4ee706ea 100644 --- a/src/openedx_content/applets/sections/models.py +++ b/src/openedx_content/applets/sections/models.py @@ -1,50 +1,40 @@ """ Models that implement sections """ -from django.db import models -from ..publishing.models import Container, ContainerVersion +from typing import override + +from django.core.exceptions import ValidationError + +from ..publishing.api import ContainerTypeImplementation, get_container_type +from ..publishing.models import PublishableEntity +from ..subsections.models import Subsection __all__ = [ "Section", - "SectionVersion", ] -class Section(Container): +class Section(ContainerTypeImplementation): """ - A Section is type of Container that holds Units. + A Section is type of Container that holds Subsections. Via Container and its PublishableEntityMixin, Sections are also publishable entities and can be added to other containers. """ - container = models.OneToOneField( - Container, - on_delete=models.CASCADE, - parent_link=True, - primary_key=True, - ) + type_code = "section" -class SectionVersion(ContainerVersion): - """ - A SectionVersion is a specific version of a Section. + @override + @classmethod + def validate_entity(cls, entity: PublishableEntity) -> None: + """Check if the given entity is allowed as a child of a Section""" + # Sections only allow Subsections as children, so the entity must be 1:1 with Container: + container = entity.container # Could raise PublishableEntity.container.RelatedObjectDoesNotExist + if get_container_type(container) is not Subsection: + raise ValidationError("Only Subsection can be added as children of a Section") - Via ContainerVersion and its EntityList, it defines the list of Units - in this version of the Section. - """ - container_version = models.OneToOneField( - ContainerVersion, - on_delete=models.CASCADE, - parent_link=True, - primary_key=True, - ) - - @property - def section(self): - """ Convenience accessor to the Section this version is associated with """ - return self.container_version.container.section # pylint: disable=no-member - - # Note: the 'publishable_entity_version' field is inherited and will appear on this model, but does not exist - # in the underlying database table. It only exists in the ContainerVersion table. - # You can verify this by running 'python manage.py sqlmigrate oel_sections 0001_initial' + # validate settings + + +ContainerTypeImplementation.register(Section) diff --git a/src/openedx_content/applets/subsections/admin.py b/src/openedx_content/applets/subsections/admin.py deleted file mode 100644 index d9d197b3c..000000000 --- a/src/openedx_content/applets/subsections/admin.py +++ /dev/null @@ -1,48 +0,0 @@ -""" -Django admin for subsection models -""" -from django.contrib import admin -from django.utils.safestring import SafeText - -from openedx_django_lib.admin_utils import ReadOnlyModelAdmin, model_detail_link - -from ..publishing.models import ContainerVersion -from .models import Subsection, SubsectionVersion - - -class SubsectionVersionInline(admin.TabularInline): - """ - Minimal table for subsection versions in a subsection. - - (Generally, this information is useless, because each SubsectionVersion should have a - matching ContainerVersion, shown in much more detail on the Container detail page. - But we've hit at least one bug where ContainerVersions were being created without - their connected SubsectionVersions, so we'll leave this table here for debugging - at least until we've made the APIs more robust against that sort of data corruption.) - """ - model = SubsectionVersion - fields = ["pk"] - readonly_fields = ["pk"] - ordering = ["-pk"] # Newest first - - def pk(self, obj: ContainerVersion) -> SafeText: - return obj.pk - - -@admin.register(Subsection) -class SubsectionAdmin(ReadOnlyModelAdmin): - """ - Very minimal interface... just direct the admin user's attention towards the related Container model admin. - """ - inlines = [SubsectionVersionInline] - list_display = ["pk", "key"] - fields = ["key"] - readonly_fields = ["key"] - - def key(self, obj: Subsection) -> SafeText: - return model_detail_link(obj.container, obj.key) - - def get_form(self, request, obj=None, change=False, **kwargs): - help_texts = {'key': f'For more details of this {self.model.__name__}, click above to see its Container view'} - kwargs.update({'help_texts': help_texts}) - return super().get_form(request, obj, **kwargs) diff --git a/src/openedx_content/applets/subsections/api.py b/src/openedx_content/applets/subsections/api.py deleted file mode 100644 index d39c5700a..000000000 --- a/src/openedx_content/applets/subsections/api.py +++ /dev/null @@ -1,329 +0,0 @@ -"""Subsections API. - -This module provides functions to manage subsections. -""" -from dataclasses import dataclass -from datetime import datetime - -from django.db.transaction import atomic - -from ..publishing import api as publishing_api -from ..units.models import Unit, UnitVersion -from .models import Subsection, SubsectionVersion - -# 🛑 UNSTABLE: All APIs related to containers are unstable until we've figured -# out our approach to dynamic content (randomized, A/B tests, etc.) -__all__ = [ - "create_subsection", - "create_subsection_version", - "create_next_subsection_version", - "create_subsection_and_version", - "get_subsection", - "get_subsection_version", - "get_latest_subsection_version", - "SubsectionListEntry", - "get_units_in_subsection", - "get_units_in_published_subsection_as_of", -] - - -def create_subsection( - learning_package_id: int, - key: str, - created: datetime, - created_by: int | None, - *, - can_stand_alone: bool = True, -) -> Subsection: - """ - [ 🛑 UNSTABLE ] Create a new subsection. - - Args: - learning_package_id: The learning package ID. - key: The key. - created: The creation date. - created_by: The user who created the subsection. - can_stand_alone: Set to False when created as part of containers - """ - return publishing_api.create_container( - learning_package_id, - key, - created, - created_by, - can_stand_alone=can_stand_alone, - container_cls=Subsection, - ) - - -def create_subsection_version( - subsection: Subsection, - version_num: int, - *, - title: str, - entity_rows: list[publishing_api.ContainerEntityRow], - created: datetime, - created_by: int | None = None, -) -> SubsectionVersion: - """ - [ 🛑 UNSTABLE ] Create a new subsection version. - - This is a very low-level API, likely only needed for import/export. In - general, you will use `create_subsection_and_version()` and - `create_next_subsection_version()` instead. - - Args: - subsection_pk: The subsection ID. - version_num: The version number. - title: The title. - entity_rows: child entities/versions - created: The creation date. - created_by: The user who created the subsection. - """ - return publishing_api.create_container_version( - subsection.pk, - version_num, - title=title, - entity_rows=entity_rows, - created=created, - created_by=created_by, - container_version_cls=SubsectionVersion, - ) - - -def _pub_entities_for_units( - units: list[Unit | UnitVersion] | None, -) -> list[publishing_api.ContainerEntityRow] | None: - """ - Helper method: given a list of Unit | UnitVersion, return the - list of ContainerEntityRows needed for the base container APIs. - - UnitVersion is passed when we want to pin a specific version, otherwise - Unit is used for unpinned. - """ - if units is None: - # When these are None, that means don't change the entities in the list. - return None - for u in units: - if not isinstance(u, (Unit, UnitVersion)): - raise TypeError("Subsection units must be either Unit or UnitVersion.") - return [ - ( - publishing_api.ContainerEntityRow( - entity_pk=u.container.publishable_entity_id, - version_pk=None, - ) if isinstance(u, Unit) - else publishing_api.ContainerEntityRow( - entity_pk=u.unit.container.publishable_entity_id, - version_pk=u.container_version.publishable_entity_version_id, - ) - ) - for u in units - ] - - -def create_next_subsection_version( - subsection: Subsection, - *, - title: str | None = None, - units: list[Unit | UnitVersion] | None = None, - created: datetime, - created_by: int | None = None, - entities_action: publishing_api.ChildrenEntitiesAction = publishing_api.ChildrenEntitiesAction.REPLACE, - force_version_num: int | None = None, -) -> SubsectionVersion: - """ - [ 🛑 UNSTABLE ] Create the next subsection version. - - Args: - subsection_pk: The subsection ID. - title: The title. Leave as None to keep the current title. - units: The units, as a list of Units (unpinned) and/or UnitVersions (pinned). Passing None - will leave the existing units unchanged. - created: The creation date. - created_by: The user who created the subsection. - force_version_num (int, optional): If provided, overrides the automatic version number increment and sets - this version's number explicitly. Use this if you need to restore or import a version with a specific - version number, such as during data migration or when synchronizing with external systems. - - Returns: - The newly created subsection version. - - Why use force_version_num? - Normally, the version number is incremented automatically from the latest version. - If you need to set a specific version number (for example, when restoring from backup, - importing legacy data, or synchronizing with another system), - use force_version_num to override the default behavior. - - Why not use create_component_version? - The main reason is that we want to reuse the logic for adding entities to this container. - """ - entity_rows = _pub_entities_for_units(units) - subsection_version = publishing_api.create_next_container_version( - subsection.pk, - title=title, - entity_rows=entity_rows, - created=created, - created_by=created_by, - container_version_cls=SubsectionVersion, - entities_action=entities_action, - force_version_num=force_version_num, - ) - return subsection_version - - -def create_subsection_and_version( - learning_package_id: int, - key: str, - *, - title: str, - units: list[Unit | UnitVersion] | None = None, - created: datetime, - created_by: int | None = None, - can_stand_alone: bool = True, -) -> tuple[Subsection, SubsectionVersion]: - """ - [ 🛑 UNSTABLE ] Create a new subsection and its version. - - Args: - learning_package_id: The learning package ID. - key: The key. - created: The creation date. - created_by: The user who created the subsection. - can_stand_alone: Set to False when created as part of containers - """ - entity_rows = _pub_entities_for_units(units) - with atomic(): - subsection = create_subsection( - learning_package_id, - key, - created, - created_by, - can_stand_alone=can_stand_alone, - ) - subsection_version = create_subsection_version( - subsection, - 1, - title=title, - entity_rows=entity_rows or [], - created=created, - created_by=created_by, - ) - return subsection, subsection_version - - -def get_subsection(subsection_pk: int) -> Subsection: - """ - [ 🛑 UNSTABLE ] Get a subsection. - - Args: - subsection_pk: The subsection ID. - """ - return Subsection.objects.get(pk=subsection_pk) - - -def get_subsection_version(subsection_version_pk: int) -> SubsectionVersion: - """ - [ 🛑 UNSTABLE ] Get a subsection version. - - Args: - subsection_version_pk: The subsection version ID. - """ - return SubsectionVersion.objects.get(pk=subsection_version_pk) - - -def get_latest_subsection_version(subsection_pk: int) -> SubsectionVersion: - """ - [ 🛑 UNSTABLE ] Get the latest subsection version. - - Args: - subsection_pk: The subsection ID. - """ - return Subsection.objects.get(pk=subsection_pk).versioning.latest - - -@dataclass(frozen=True) -class SubsectionListEntry: - """ - [ 🛑 UNSTABLE ] - Data about a single entity in a container, e.g. a unit in a subsection. - """ - unit_version: UnitVersion - pinned: bool = False - - @property - def unit(self): - return self.unit_version.unit - - -def get_units_in_subsection( - subsection: Subsection, - *, - published: bool, -) -> list[SubsectionListEntry]: - """ - [ 🛑 UNSTABLE ] - Get the list of entities and their versions in the draft or published - version of the given Subsection. - - Args: - subsection: The Subsection, e.g. returned by `get_subsection()` - published: `True` if we want the published version of the subsection, or - `False` for the draft version. - """ - assert isinstance(subsection, Subsection) - units = [] - entries = publishing_api.get_entities_in_container( - subsection, - published=published, - select_related_version="containerversion__unitversion", - ) - for entry in entries: - # Convert from generic PublishableEntityVersion to UnitVersion: - unit_version = entry.entity_version.containerversion.unitversion - assert isinstance(unit_version, UnitVersion) - units.append(SubsectionListEntry(unit_version=unit_version, pinned=entry.pinned)) - return units - - -def get_units_in_published_subsection_as_of( - subsection: Subsection, - publish_log_id: int, -) -> list[SubsectionListEntry] | None: - """ - [ 🛑 UNSTABLE ] - Get the list of entities and their versions in the published version of the - given container as of the given PublishLog version (which is essentially a - version for the entire learning package). - - TODO: This API should be updated to also return the SubsectionVersion so we can - see the subsection title and any other metadata from that point in time. - TODO: accept a publish log UUID, not just int ID? - TODO: move the implementation to be a generic 'containers' implementation - that this subsections function merely wraps. - TODO: optimize, perhaps by having the publishlog store a record of all - ancestors of every modified PublishableEntity in the publish. - """ - assert isinstance(subsection, Subsection) - subsection_pub_entity_version = publishing_api.get_published_version_as_of( - subsection.publishable_entity_id, publish_log_id - ) - if subsection_pub_entity_version is None: - return None # This subsection was not published as of the given PublishLog ID. - container_version = subsection_pub_entity_version.containerversion - - entity_list = [] - rows = container_version.entity_list.entitylistrow_set.order_by("order_num") - for row in rows: - if row.entity_version is not None: - unit_version = row.entity_version.containerversion.unitversion - assert isinstance(unit_version, UnitVersion) - entity_list.append(SubsectionListEntry(unit_version=unit_version, pinned=True)) - else: - # Unpinned unit - figure out what its latest published version was. - # This is not optimized. It could be done in one query per subsection rather than one query per unit. - pub_entity_version = publishing_api.get_published_version_as_of(row.entity_id, publish_log_id) - if pub_entity_version: - entity_list.append( - SubsectionListEntry(unit_version=pub_entity_version.containerversion.unitversion, pinned=False) - ) - return entity_list diff --git a/src/openedx_content/applets/subsections/models.py b/src/openedx_content/applets/subsections/models.py index 8d662ed4e..292796747 100644 --- a/src/openedx_content/applets/subsections/models.py +++ b/src/openedx_content/applets/subsections/models.py @@ -1,50 +1,40 @@ """ Models that implement subsections """ -from django.db import models -from ..publishing.models import Container, ContainerVersion +from typing import override + +from django.core.exceptions import ValidationError + +from ..publishing.api import ContainerTypeImplementation, get_container_type +from ..publishing.models import PublishableEntity +from ..units.models import Unit __all__ = [ "Subsection", - "SubsectionVersion", ] -class Subsection(Container): +class Subsection(ContainerTypeImplementation): """ A Subsection is type of Container that holds Units. Via Container and its PublishableEntityMixin, Subsections are also publishable entities and can be added to other containers. """ - container = models.OneToOneField( - Container, - on_delete=models.CASCADE, - parent_link=True, - primary_key=True, - ) + type_code = "subsection" -class SubsectionVersion(ContainerVersion): - """ - A SubsectionVersion is a specific version of a Subsection. + @override + @classmethod + def validate_entity(cls, entity: PublishableEntity) -> None: + """Check if the given entity is allowed as a child of a Subsection""" + # Subsections only allow Units as children, so the entity must be 1:1 with Container: + container = entity.container # Could raise PublishableEntity.container.RelatedObjectDoesNotExist + if get_container_type(container) is not Unit: + raise ValidationError("Only Units can be added as children of a Subsection") - Via ContainerVersion and its EntityList, it defines the list of Units - in this version of the Subsection. - """ - container_version = models.OneToOneField( - ContainerVersion, - on_delete=models.CASCADE, - parent_link=True, - primary_key=True, - ) - - @property - def subsection(self): - """ Convenience accessor to the Subsection this version is associated with """ - return self.container_version.container.subsection # pylint: disable=no-member - - # Note: the 'publishable_entity_version' field is inherited and will appear on this model, but does not exist - # in the underlying database table. It only exists in the ContainerVersion table. - # You can verify this by running 'python manage.py sqlmigrate oel_subsections 0001_initial' + # validate settings + + +ContainerTypeImplementation.register(Subsection) diff --git a/src/openedx_content/applets/units/admin.py b/src/openedx_content/applets/units/admin.py deleted file mode 100644 index d079875f8..000000000 --- a/src/openedx_content/applets/units/admin.py +++ /dev/null @@ -1,48 +0,0 @@ -""" -Django admin for units models -""" -from django.contrib import admin -from django.utils.safestring import SafeText - -from openedx_django_lib.admin_utils import ReadOnlyModelAdmin, model_detail_link - -from ..publishing.models import ContainerVersion -from .models import Unit, UnitVersion - - -class UnitVersionInline(admin.TabularInline): - """ - Minimal table for unit versions in a unit - - (Generally, this information is useless, because each UnitVersion should have a - matching ContainerVersion, shown in much more detail on the Container detail page. - But we've hit at least one bug where ContainerVersions were being created without - their connected UnitVersions, so we'll leave this table here for debugging - at least until we've made the APIs more robust against that sort of data corruption.) - """ - model = UnitVersion - fields = ["pk"] - readonly_fields = ["pk"] - ordering = ["-pk"] # Newest first - - def pk(self, obj: ContainerVersion) -> SafeText: - return obj.pk - - -@admin.register(Unit) -class UnitAdmin(ReadOnlyModelAdmin): - """ - Very minimal interface... just direct the admin user's attention towards the related Container model admin. - """ - inlines = [UnitVersionInline] - list_display = ["pk", "key"] - fields = ["key"] - readonly_fields = ["key"] - - def key(self, obj: Unit) -> SafeText: - return model_detail_link(obj.container, obj.key) - - def get_form(self, request, obj=None, change=False, **kwargs): - help_texts = {'key': f'For more details of this {self.model.__name__}, click above to see its Container view'} - kwargs.update({'help_texts': help_texts}) - return super().get_form(request, obj, **kwargs) diff --git a/src/openedx_content/applets/units/api.py b/src/openedx_content/applets/units/api.py deleted file mode 100644 index 779b5b3d0..000000000 --- a/src/openedx_content/applets/units/api.py +++ /dev/null @@ -1,326 +0,0 @@ -"""Units API. - -This module provides functions to manage units. -""" -from dataclasses import dataclass -from datetime import datetime - -from django.db.transaction import atomic - -from ..components.models import Component, ComponentVersion -from ..publishing import api as publishing_api -from .models import Unit, UnitVersion - -# 🛑 UNSTABLE: All APIs related to containers are unstable until we've figured -# out our approach to dynamic content (randomized, A/B tests, etc.) -__all__ = [ - "create_unit", - "create_unit_version", - "create_next_unit_version", - "create_unit_and_version", - "get_unit", - "get_unit_version", - "get_latest_unit_version", - "UnitListEntry", - "get_components_in_unit", - "get_components_in_published_unit_as_of", -] - - -def create_unit( - learning_package_id: int, - key: str, - created: datetime, - created_by: int | None, - *, - can_stand_alone: bool = True, -) -> Unit: - """ - [ 🛑 UNSTABLE ] Create a new unit. - - Args: - learning_package_id: The learning package ID. - key: The key. - created: The creation date. - created_by: The user who created the unit. - can_stand_alone: Set to False when created as part of containers - """ - return publishing_api.create_container( - learning_package_id, - key, - created, - created_by, - can_stand_alone=can_stand_alone, - container_cls=Unit, - ) - - -def create_unit_version( - unit: Unit, - version_num: int, - *, - title: str, - entity_rows: list[publishing_api.ContainerEntityRow], - created: datetime, - created_by: int | None = None, -) -> UnitVersion: - """ - [ 🛑 UNSTABLE ] Create a new unit version. - - This is a very low-level API, likely only needed for import/export. In - general, you will use `create_unit_and_version()` and - `create_next_unit_version()` instead. - - Args: - unit: The unit object. - version_num: The version number. - title: The title. - entity_rows: child entities/versions - created: The creation date. - created_by: The user who created the unit. - force_version_num (int, optional): If provided, overrides the automatic version number increment and sets - this version's number explicitly. Use this if you need to restore or import a version with a specific - version number, such as during data migration or when synchronizing with external systems. - - Returns: - UnitVersion: The newly created UnitVersion instance. - - Why use force_version_num? - Normally, the version number is incremented automatically from the latest version. - If you need to set a specific version number (for example, when restoring from backup, - importing legacy data, or synchronizing with another system), - use force_version_num to override the default behavior. - - Why not use create_component_version? - The main reason is that we want to reuse the logic for adding entities to this container. - """ - return publishing_api.create_container_version( - unit.pk, - version_num, - title=title, - entity_rows=entity_rows, - created=created, - created_by=created_by, - container_version_cls=UnitVersion, - ) - - -def _pub_entities_for_components( - components: list[Component | ComponentVersion] | None, -) -> list[publishing_api.ContainerEntityRow] | None: - """ - Helper method: given a list of Component | ComponentVersion, return the - list of ContainerEntityRows needed for the base container APIs. - - ComponentVersion is passed when we want to pin a specific version, otherwise - Component is used for unpinned. - """ - if components is None: - # When these are None, that means don't change the entities in the list. - return None - for c in components: - if not isinstance(c, (Component, ComponentVersion)): - raise TypeError("Unit components must be either Component or ComponentVersion.") - return [ - ( - publishing_api.ContainerEntityRow( - entity_pk=c.publishable_entity_id, - version_pk=None, - ) if isinstance(c, Component) - else # isinstance(c, ComponentVersion) - publishing_api.ContainerEntityRow( - entity_pk=c.component.publishable_entity_id, - version_pk=c.pk, - ) - ) - for c in components - ] - - -def create_next_unit_version( - unit: Unit, - *, - title: str | None = None, - components: list[Component | ComponentVersion] | None = None, - created: datetime, - created_by: int | None = None, - entities_action: publishing_api.ChildrenEntitiesAction = publishing_api.ChildrenEntitiesAction.REPLACE, - force_version_num: int | None = None, -) -> UnitVersion: - """ - [ 🛑 UNSTABLE ] Create the next unit version. - - Args: - unit_pk: The unit ID. - title: The title. Leave as None to keep the current title. - components: The components, as a list of Components (unpinned) and/or ComponentVersions (pinned). Passing None - will leave the existing components unchanged. - created: The creation date. - created_by: The user who created the unit. - """ - entity_rows = _pub_entities_for_components(components) - unit_version = publishing_api.create_next_container_version( - unit.pk, - title=title, - entity_rows=entity_rows, - created=created, - created_by=created_by, - container_version_cls=UnitVersion, - entities_action=entities_action, - force_version_num=force_version_num, - ) - return unit_version - - -def create_unit_and_version( - learning_package_id: int, - key: str, - *, - title: str, - components: list[Component | ComponentVersion] | None = None, - created: datetime, - created_by: int | None = None, - can_stand_alone: bool = True, -) -> tuple[Unit, UnitVersion]: - """ - [ 🛑 UNSTABLE ] Create a new unit and its version. - - Args: - learning_package_id: The learning package ID. - key: The key. - created: The creation date. - created_by: The user who created the unit. - can_stand_alone: Set to False when created as part of containers - """ - entity_rows = _pub_entities_for_components(components) - with atomic(savepoint=False): - unit = create_unit( - learning_package_id, - key, - created, - created_by, - can_stand_alone=can_stand_alone, - ) - unit_version = create_unit_version( - unit, - 1, - title=title, - entity_rows=entity_rows or [], - created=created, - created_by=created_by, - ) - return unit, unit_version - - -def get_unit(unit_pk: int) -> Unit: - """ - [ 🛑 UNSTABLE ] Get a unit. - - Args: - unit_pk: The unit ID. - """ - return Unit.objects.get(pk=unit_pk) - - -def get_unit_version(unit_version_pk: int) -> UnitVersion: - """ - [ 🛑 UNSTABLE ] Get a unit version. - - Args: - unit_version_pk: The unit version ID. - """ - return UnitVersion.objects.get(pk=unit_version_pk) - - -def get_latest_unit_version(unit_pk: int) -> UnitVersion: - """ - [ 🛑 UNSTABLE ] Get the latest unit version. - - Args: - unit_pk: The unit ID. - """ - return Unit.objects.get(pk=unit_pk).versioning.latest - - -@dataclass(frozen=True) -class UnitListEntry: - """ - [ 🛑 UNSTABLE ] - Data about a single entity in a container, e.g. a component in a unit. - """ - component_version: ComponentVersion - pinned: bool = False - - @property - def component(self): - return self.component_version.component - - -def get_components_in_unit( - unit: Unit, - *, - published: bool, -) -> list[UnitListEntry]: - """ - [ 🛑 UNSTABLE ] - Get the list of entities and their versions in the draft or published - version of the given Unit. - - Args: - unit: The Unit, e.g. returned by `get_unit()` - published: `True` if we want the published version of the unit, or - `False` for the draft version. - """ - assert isinstance(unit, Unit) - components = [] - entries = publishing_api.get_entities_in_container( - unit, - published=published, - select_related_version="componentversion", - ) - for entry in entries: - # Convert from generic PublishableEntityVersion to ComponentVersion: - component_version = entry.entity_version.componentversion - assert isinstance(component_version, ComponentVersion) - components.append(UnitListEntry(component_version=component_version, pinned=entry.pinned)) - return components - - -def get_components_in_published_unit_as_of( - unit: Unit, - publish_log_id: int, -) -> list[UnitListEntry] | None: - """ - [ 🛑 UNSTABLE ] - Get the list of entities and their versions in the published version of the - given container as of the given PublishLog version (which is essentially a - version for the entire learning package). - - TODO: This API should be updated to also return the UnitVersion so we can - see the unit title and any other metadata from that point in time. - TODO: accept a publish log UUID, not just int ID? - TODO: move the implementation to be a generic 'containers' implementation - that this units function merely wraps. - TODO: optimize, perhaps by having the publishlog store a record of all - ancestors of every modified PublishableEntity in the publish. - """ - assert isinstance(unit, Unit) - unit_pub_entity_version = publishing_api.get_published_version_as_of(unit.publishable_entity_id, publish_log_id) - if unit_pub_entity_version is None: - return None # This unit was not published as of the given PublishLog ID. - container_version = unit_pub_entity_version.containerversion - - entity_list = [] - rows = container_version.entity_list.entitylistrow_set.order_by("order_num") - for row in rows: - if row.entity_version is not None: - component_version = row.entity_version.componentversion - assert isinstance(component_version, ComponentVersion) - entity_list.append(UnitListEntry(component_version=component_version, pinned=True)) - else: - # Unpinned component - figure out what its latest published version was. - # This is not optimized. It could be done in one query per unit rather than one query per component. - pub_entity_version = publishing_api.get_published_version_as_of(row.entity_id, publish_log_id) - if pub_entity_version: - entity_list.append(UnitListEntry(component_version=pub_entity_version.componentversion, pinned=False)) - return entity_list diff --git a/src/openedx_content/applets/units/models.py b/src/openedx_content/applets/units/models.py index 0c5255846..686a58421 100644 --- a/src/openedx_content/applets/units/models.py +++ b/src/openedx_content/applets/units/models.py @@ -1,50 +1,35 @@ """ Models that implement units """ -from django.db import models -from ..publishing.models import Container, ContainerVersion +from typing import override + +from ..publishing.api import ContainerTypeImplementation +from ..publishing.models import PublishableEntity __all__ = [ "Unit", - "UnitVersion", ] -class Unit(Container): +class Unit(ContainerTypeImplementation): """ A Unit is type of Container that holds Components. Via Container and its PublishableEntityMixin, Units are also publishable entities and can be added to other containers. """ - container = models.OneToOneField( - Container, - on_delete=models.CASCADE, - parent_link=True, - primary_key=True, - ) + type_code = "unit" -class UnitVersion(ContainerVersion): - """ - A UnitVersion is a specific version of a Unit. + @override + @classmethod + def validate_entity(cls, entity: PublishableEntity) -> None: + """Check if the given entity is allowed as a child of a Unit""" + # Units only allow Components as children, so the entity must be 1:1 with Component: + getattr(entity, "component") # Could raise PublishableEntity.component.RelatedObjectDoesNotExist - Via ContainerVersion and its EntityList, it defines the list of Components - in this version of the Unit. - """ - container_version = models.OneToOneField( - ContainerVersion, - on_delete=models.CASCADE, - parent_link=True, - primary_key=True, - ) - - @property - def unit(self): - """ Convenience accessor to the Unit this version is associated with """ - return self.container_version.container.unit # pylint: disable=no-member - - # Note: the 'publishable_entity_version' field is inherited and will appear on this model, but does not exist - # in the underlying database table. It only exists in the ContainerVersion table. - # You can verify this by running 'python manage.py sqlmigrate oel_units 0001_initial' + # validate settings + + +ContainerTypeImplementation.register(Unit) diff --git a/src/openedx_content/apps.py b/src/openedx_content/apps.py index 563d5d03a..3f259277a 100644 --- a/src/openedx_content/apps.py +++ b/src/openedx_content/apps.py @@ -30,18 +30,9 @@ def register_publishable_models(self): ComponentVersion, Container, ContainerVersion, - Section, - SectionVersion, - Subsection, - SubsectionVersion, - Unit, - UnitVersion, ) register_publishable_models(Component, ComponentVersion) register_publishable_models(Container, ContainerVersion) - register_publishable_models(Section, SectionVersion) - register_publishable_models(Subsection, SubsectionVersion) - register_publishable_models(Unit, UnitVersion) def ready(self): """ diff --git a/src/openedx_content/migrations/0004_componenttype_constraint.py b/src/openedx_content/migrations/0004_componenttype_constraint.py new file mode 100644 index 000000000..f556d14b8 --- /dev/null +++ b/src/openedx_content/migrations/0004_componenttype_constraint.py @@ -0,0 +1,16 @@ +# Generated by Django 5.2.11 on 2026-03-02 23:37 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [ + ("openedx_content", "0003_rename_content_to_media"), + ] + + operations = [ + migrations.AddConstraint( + model_name="componenttype", + constraint=models.UniqueConstraint(fields=("namespace", "name"), name="oel_component_type_uniq_ns_n"), + ), + ] diff --git a/src/openedx_content/migrations/0005_containertypes.py b/src/openedx_content/migrations/0005_containertypes.py new file mode 100644 index 000000000..5afab35d6 --- /dev/null +++ b/src/openedx_content/migrations/0005_containertypes.py @@ -0,0 +1,82 @@ +# Generated by Django 5.2.11 on 2026-03-03 00:54 + +import django.db.models.deletion +import django.db.models.lookups +import openedx_django_lib.fields +from django.db import migrations, models + + +def backfill_container_types(apps, schema_editor): + """ + Fill in the new, mandatory "container_type" foreign key field on all + existing containers. + """ + Container = apps.get_model("openedx_content", "Container") + ContainerTypeRecord = apps.get_model("openedx_content", "ContainerTypeRecord") + section_type, _ = ContainerTypeRecord.objects.get_or_create(type_code="section") + subsection_type, _ = ContainerTypeRecord.objects.get_or_create(type_code="subsection") + unit_type, _ = ContainerTypeRecord.objects.get_or_create(type_code="unit") + + containers_to_update = Container.objects.filter(container_type_record=None) + + containers_to_update.exclude(section=None).update(container_type_record=section_type) + containers_to_update.exclude(subsection=None).update(container_type_record=subsection_type) + containers_to_update.exclude(unit=None).update(container_type_record=unit_type) + + unknown_containers = containers_to_update.all() + if unknown_containers: + raise ValueError(f"container {unknown_containers[0]} is of unknown container type. Cannot apply migration.") + + +class Migration(migrations.Migration): + dependencies = [ + ("openedx_content", "0004_componenttype_constraint"), + ] + + operations = [ + # 1. Create the new ContainerTypeRecord model + migrations.CreateModel( + name="ContainerTypeRecord", + fields=[ + ("id", models.AutoField(primary_key=True, serialize=False)), + ( + "type_code", + openedx_django_lib.fields.MultiCollationCharField( + db_collations={"mysql": "utf8mb4_bin", "sqlite": "BINARY"}, max_length=100, unique=True + ), + ), + ], + options={ + "constraints": [ + models.CheckConstraint( + condition=django.db.models.lookups.Regex(models.F("type_code"), "^[a-z0-9\\-_\\.]+$"), + name="oex_publishing_containertyperecord_type_code_rx", + ) + ], + }, + ), + # 2. Define the ForeignKey from Container to ContainerType + migrations.AddField( + model_name="container", + name="container_type_record", + field=models.ForeignKey( + editable=False, + null=True, + on_delete=django.db.models.deletion.RESTRICT, + to="openedx_content.containertyperecord", + ), + ), + # 3. Populate the container_type column, which is currently NULL for all existing containers + migrations.RunPython(backfill_container_types), + # 4. disallow NULL values from now on + migrations.AlterField( + model_name="container", + name="container_type_record", + field=models.ForeignKey( + editable=False, + null=False, + on_delete=django.db.models.deletion.RESTRICT, + to="openedx_content.containertyperecord", + ), + ), + ] diff --git a/src/openedx_content/migrations/0006_remove_empty_container_models.py b/src/openedx_content/migrations/0006_remove_empty_container_models.py new file mode 100644 index 000000000..7bdc91ebf --- /dev/null +++ b/src/openedx_content/migrations/0006_remove_empty_container_models.py @@ -0,0 +1,51 @@ +# Generated by Django 5.2.11 on 2026-03-10 19:51 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('openedx_content', '0005_containertypes'), + ] + + operations = [ + migrations.RemoveField( + model_name='sectionversion', + name='container_version', + ), + migrations.RemoveField( + model_name='subsection', + name='container', + ), + migrations.RemoveField( + model_name='subsectionversion', + name='container_version', + ), + migrations.RemoveField( + model_name='unit', + name='container', + ), + migrations.RemoveField( + model_name='unitversion', + name='container_version', + ), + migrations.DeleteModel( + name='Section', + ), + migrations.DeleteModel( + name='SectionVersion', + ), + migrations.DeleteModel( + name='Subsection', + ), + migrations.DeleteModel( + name='SubsectionVersion', + ), + migrations.DeleteModel( + name='Unit', + ), + migrations.DeleteModel( + name='UnitVersion', + ), + ] diff --git a/src/openedx_content/models.py b/src/openedx_content/models.py index 91696b5f3..6e009e399 100644 --- a/src/openedx_content/models.py +++ b/src/openedx_content/models.py @@ -12,6 +12,3 @@ from .applets.components.models import * from .applets.media.models import * from .applets.publishing.models import * -from .applets.sections.models import * -from .applets.subsections.models import * -from .applets.units.models import * diff --git a/src/openedx_content/models_api.py b/src/openedx_content/models_api.py index 1e035b43f..c46dd2d02 100644 --- a/src/openedx_content/models_api.py +++ b/src/openedx_content/models_api.py @@ -11,6 +11,4 @@ from .applets.components.models import * from .applets.media.models import * from .applets.publishing.models import * -from .applets.sections.models import * -from .applets.subsections.models import * -from .applets.units.models import * + diff --git a/test_settings.py b/test_settings.py index 371903ad3..196353293 100644 --- a/test_settings.py +++ b/test_settings.py @@ -97,3 +97,27 @@ def root(*args): } STATIC_URL = 'static/' + +# Required for Django admin which is required because it's referenced by projects.urls (ROOT_URLCONF) +TEMPLATES = [ + { + 'NAME': 'django', + 'BACKEND': 'django.template.backends.django.DjangoTemplates', + # Don't look for template source files inside installed applications. + # 'APP_DIRS': False, + # Instead, look for template source files in these dirs. + # 'DIRS': [], + 'OPTIONS': { + 'context_processors': [ + 'django.template.context_processors.request', + 'django.contrib.messages.context_processors.messages', + 'django.contrib.auth.context_processors.auth', + ], + } + }, +] +MIDDLEWARE = [ + "django.contrib.sessions.middleware.SessionMiddleware", + "django.contrib.auth.middleware.AuthenticationMiddleware", + "django.contrib.messages.middleware.MessageMiddleware", +] \ No newline at end of file diff --git a/tests/openedx_content/applets/publishing/container_test_case.py b/tests/openedx_content/applets/publishing/container_test_case.py new file mode 100644 index 000000000..7dded9f9c --- /dev/null +++ b/tests/openedx_content/applets/publishing/container_test_case.py @@ -0,0 +1,136 @@ +""" +Basic tests for the units API. +""" + +from functools import partial + +import openedx_content.api as content_api +from openedx_content import models_api as authoring_models + +from ..components.test_api import ComponentTestCase + + +def Entry( + component_version: authoring_models.PublishableEntityVersionMixin, + pinned: bool = False, +) -> content_api.ContainerEntityListEntry: + """Helper for quickly constructing ContainerEntityListEntry entries""" + return content_api.ContainerEntityListEntry(component_version.publishable_entity_version, pinned=pinned) + + +class ContainerTestCase(ComponentTestCase): + """Base class with useful functions for testing containers. Has no tests on its own.""" + + def setUp(self) -> None: + super().setUp() + self.create_unit = partial(self.create_container, container_type=content_api.Unit) + self.create_unit_and_version = partial(self.create_container_and_version, container_type=content_api.Unit) + self.create_subsection = partial(self.create_container, container_type=content_api.Subsection) + self.create_subsection_and_version = partial( + self.create_container_and_version, container_type=content_api.Subsection + ) + self.create_section = partial(self.create_container, container_type=content_api.Section) + self.create_section_and_version = partial(self.create_container_and_version, container_type=content_api.Section) + + def create_component( + self, *, title: str = "Test Component", key: str = "component:1" + ) -> tuple[authoring_models.Component, authoring_models.ComponentVersion]: + """Helper method to quickly create a component""" + return content_api.create_component_and_version( + self.learning_package.id, + component_type=self.problem_type, + local_key=key, + title=title, + created=self.now, + created_by=None, + ) + + def create_container_and_version( + self, + *, + entities: list[ + authoring_models.Component + | authoring_models.ComponentVersion + | authoring_models.Container + | authoring_models.ContainerVersion + ] + | None = None, + container_type: content_api.ContainerType, + title: str | None = None, + key: str | None = None, + ) -> authoring_models.Container: + """Helper method to quickly create a container with some child entities""" + container, version = content_api.create_container_and_version( + learning_package_id=self.learning_package.id, + key=key or f"{container_type.type_code}:key", + title=title or f"Test {container_type.type_code}", + entities=entities, + created=self.now, + created_by=None, + container_type=container_type, + ) + return container, version + + def create_container( + self, + *, + entities: list[ + authoring_models.Component + | authoring_models.ComponentVersion + | authoring_models.Container + | authoring_models.ContainerVersion + ] + | None = None, + container_type: content_api.ContainerType, + title: str | None = None, + key: str | None = None, + ) -> authoring_models.Container: + """Helper method to quickly create a container with some components""" + container, _version = self.create_container_and_version( + entities=entities, container_type=container_type, title=title, key=key + ) + return container + + def modify_component( + self, + component: authoring_models.Component, + *, + title="Modified Component", + timestamp=None, + ) -> authoring_models.ComponentVersion: + """ + Helper method to modify a component for the purposes of testing units/drafts/pinning/publishing/etc. + """ + return content_api.create_next_component_version( + component.pk, + media_to_replace={}, + title=title, + created=timestamp or self.now, + created_by=None, + ) + + def modify_container( + self, + container: authoring_models.Container, + *, + title="", + timestamp=None, + ) -> authoring_models.ContainerVersion: + """ + Helper method to modify a unit for the purposes of testing units/drafts/pinning/publishing/etc. + """ + return content_api.create_next_container_version( + container.pk, + title=title or f"Modified {content_api.get_container_type_code(container)}", + created=timestamp or self.now, + created_by=None, + ) + + def publish_container(self, container: authoring_models.Container): + """ + Helper method to publish a single container. + """ + content_api.publish_from_drafts( + self.learning_package.pk, + draft_qset=content_api.get_all_drafts(self.learning_package.pk).filter(entity=container.publishable_entity), + ) diff --git a/tests/openedx_content/applets/publishing/test_api.py b/tests/openedx_content/applets/publishing/test_api.py index c0f113787..a32bf5d47 100644 --- a/tests/openedx_content/applets/publishing/test_api.py +++ b/tests/openedx_content/applets/publishing/test_api.py @@ -27,6 +27,16 @@ User = get_user_model() +class TestContainer(Container): + """ + A fake subclass of Container used for test purposes. + """ + CONTAINER_TYPE = "fake_test" + + class Meta: + app_label = "openedx_content" + + class LearningPackageTestCase(TestCase): """ Test creating a LearningPackage @@ -1068,6 +1078,7 @@ def test_bulk_parent_child_side_effects(self) -> None: "my_container", created=self.now, created_by=None, + container_cls=TestContainer, ) container_v1: ContainerVersion = publishing_api.create_container_version( container.pk, diff --git a/tests/openedx_content/applets/sections/test_api.py b/tests/openedx_content/applets/sections/test_api.py index f31771131..507eceb33 100644 --- a/tests/openedx_content/applets/sections/test_api.py +++ b/tests/openedx_content/applets/sections/test_api.py @@ -1,387 +1,104 @@ """ Basic tests for the subsections API. """ + import ddt # type: ignore[import] -import pytest from django.core.exceptions import ValidationError +import pytest import openedx_content.api as content_api from openedx_content import models_api as authoring_models -from ..subsections.test_api import SubSectionTestCase +from ..publishing.container_test_case import ContainerTestCase, Entry -Entry = content_api.SectionListEntry - -# TODO: Turn SubSectionTestCase into SubSectionTestMixin and remove the -# test-inherits-tests pylint warning below. -# https://github.com/openedx/openedx-core/issues/308 @ddt.ddt -class SectionTestCase(SubSectionTestCase): # pylint: disable=test-inherits-tests - """ Test cases for Sections (containers of subsections) """ +class SubSectionTestCase(ContainerTestCase): + """Test cases for Sections (containers of subsections)""" def setUp(self) -> None: super().setUp() - self.subsection_1, self.subsection_1_v1 = self.create_subsection( + self.subsection_1, self.subsection_1_v1 = self.create_subsection_and_version( key="Subsection (1)", title="Subsection (1)", ) - self.subsection_2, self.subsection_2_v1 = self.create_subsection( + self.subsection_2, self.subsection_2_v1 = self.create_subsection_and_version( key="Subsection (2)", title="Subsection (2)", ) - def create_subsection(self, *, title: str = "Test Subsection", key: str = "subsection:1") -> tuple[ - authoring_models.Subsection, authoring_models.SubsectionVersion - ]: - """ Helper method to quickly create a subsection """ - return content_api.create_subsection_and_version( - self.learning_package.id, - key=key, - title=title, - created=self.now, - created_by=None, - ) - - def create_section_with_subsections( - self, - subsections: list[authoring_models.Subsection | authoring_models.SubsectionVersion], - *, - title="Subsection", - key="subsection:key", - ) -> authoring_models.Section: - """ Helper method to quickly create a section with some subsections """ - section, _section_v1 = content_api.create_section_and_version( - learning_package_id=self.learning_package.id, - key=key, - title=title, - subsections=subsections, - created=self.now, - created_by=None, - ) - return section - - def modify_subsection( - self, - subsection: authoring_models.Subsection, - *, - title="Modified Subsection", - timestamp=None, - ) -> authoring_models.SubsectionVersion: - """ - Helper method to modify a subsection for the purposes of testing subsections/drafts/pinning/publishing/etc. - """ - return content_api.create_next_subsection_version( - subsection, - title=title, - created=timestamp or self.now, - created_by=None, - ) - - def publish_subsection(self, subsection: authoring_models.Subsection): - """ - Helper method to publish a single subsection. - """ - content_api.publish_from_drafts( - self.learning_package.pk, - draft_qset=content_api.get_all_drafts(self.learning_package.pk).filter( - entity=subsection.publishable_entity, - ), - ) - - def test_get_section(self): - """ - Test get_section() - """ - section = self.create_section_with_subsections([self.subsection_1, self.subsection_2]) - with self.assertNumQueries(1): - result = content_api.get_section(section.pk) - assert result == section - # Versioning data should be pre-loaded via select_related() - with self.assertNumQueries(0): - assert result.versioning.has_unpublished_changes - - def test_get_section_version(self): - """ - Test get_section_version() - """ - section = self.create_section_with_subsections([]) - draft = section.versioning.draft - with self.assertNumQueries(1): - result = content_api.get_section_version(draft.pk) - assert result == draft - - def test_get_latest_section_version(self): - """ - Test test_get_latest_section_version() - """ - section = self.create_section_with_subsections([]) - draft = section.versioning.draft - with self.assertNumQueries(2): - result = content_api.get_latest_section_version(section.pk) - assert result == draft - - def test_get_containers(self): - """ - Test get_containers() - """ - section = self.create_section_with_subsections([]) - with self.assertNumQueries(1): - result = list(content_api.get_containers(self.learning_package.id)) - self.assertCountEqual(result, [ - self.unit_1.container, - self.unit_2.container, - self.subsection_1.container, - self.subsection_2.container, - section.container, - ]) - # Versioning data should be pre-loaded via select_related() - with self.assertNumQueries(0): - assert result[0].versioning.has_unpublished_changes - - def test_get_containers_deleted(self): - """ - Test that get_containers() does not return soft-deleted sections. - """ - section = self.create_section_with_subsections([]) - content_api.soft_delete_draft(section.pk) - - with self.assertNumQueries(1): - result = list(content_api.get_containers(self.learning_package.id, include_deleted=True)) - - assert result == [ - self.unit_1.container, - self.unit_2.container, - self.subsection_1.container, - self.subsection_2.container, - section.container, - ] - - with self.assertNumQueries(1): - result = list(content_api.get_containers(self.learning_package.id)) - - assert result == [ - self.unit_1.container, - self.unit_2.container, - self.subsection_1.container, - self.subsection_2.container, - ] - - def test_get_container(self): - """ - Test get_container() - """ - section = self.create_section_with_subsections([self.subsection_1, self.subsection_2]) - with self.assertNumQueries(1): - result = content_api.get_container(section.pk) - assert result == section.container - # Versioning data should be pre-loaded via select_related() - with self.assertNumQueries(0): - assert result.versioning.has_unpublished_changes - - def test_get_container_by_key(self): - """ - Test get_container_by_key() - """ - section = self.create_section_with_subsections([]) - with self.assertNumQueries(1): - result = content_api.get_container_by_key( - self.learning_package.id, - key=section.publishable_entity.key, - ) - assert result == section.container - # Versioning data should be pre-loaded via select_related() - with self.assertNumQueries(0): - assert result.versioning.has_unpublished_changes - - def test_section_container_versioning(self): - """ - Test that the .versioning helper of a Sebsection returns a SectionVersion, and - same for the generic Container equivalent. - """ - section = self.create_section_with_subsections([self.subsection_1, self.subsection_2]) - container = section.container - container_version = container.versioning.draft - assert isinstance(container_version, authoring_models.ContainerVersion) - section_version = section.versioning.draft - assert isinstance(section_version, authoring_models.SectionVersion) - assert section_version.container_version == container_version - assert section_version.container_version.container == container - assert section_version.section == section - - def test_create_section_queries(self): - """ - Test how many database queries are required to create a section - """ - # The exact numbers here aren't too important - this is just to alert us if anything significant changes. - with self.assertNumQueries(28): - _empty_section = self.create_section_with_subsections([]) - with self.assertNumQueries(35): - # And try with a non-empty section: - self.create_section_with_subsections([self.subsection_1, self.subsection_2_v1], key="u2") - def test_create_section_with_invalid_children(self): """ Verify that only subsections can be added to sections, and a specific exception is raised. """ # Create two sections: - section, section_version = content_api.create_section_and_version( - learning_package_id=self.learning_package.id, + section, section_version = self.create_section_and_version( key="section:key", title="Section", - created=self.now, - created_by=None, ) assert section.versioning.draft == section_version - section2, _s2v1 = content_api.create_section_and_version( - learning_package_id=self.learning_package.id, + section2, _s2v1 = self.create_section_and_version( key="section:key2", title="Section 2", - created=self.now, - created_by=None, ) # Try adding a Section to a Section - with pytest.raises(TypeError, match="Section subsections must be either Subsection or SubsectionVersion."): - content_api.create_next_section_version( - section=section, + with pytest.raises( + ValidationError, match='The entity "section:key2" cannot be added to a "section" container.' + ) as exc: + content_api.create_next_container_version( + section.pk, title="Section Containing a Section", - subsections=[section2], + entities=[section2], created=self.now, created_by=None, ) + assert "Only Subsection can be added as children of a Section" in exc.value.__cause__ # Check that a new version was not created: section.refresh_from_db() - assert content_api.get_section(section.pk).versioning.draft == section_version + assert content_api.get_container(section.pk).versioning.draft == section_version assert section.versioning.draft == section_version - def test_adding_external_subsections(self): - """ - Test that subsections from another learning package cannot be added to a - section. - """ - learning_package2 = content_api.create_learning_package(key="other-package", title="Other Package") - section, _section_version = content_api.create_section_and_version( - learning_package_id=learning_package2.pk, - key="section:key", - title="Section", - created=self.now, - created_by=None, - ) - assert self.subsection_1.container.publishable_entity.learning_package != learning_package2 - # Try adding a a subsection from LP 1 (self.learning_package) to a section from LP 2 - with pytest.raises(ValidationError, match="Container entities must be from the same learning package."): - content_api.create_next_section_version( - section=section, - title="Section Containing an External Subsection", - subsections=[self.subsection_1], - created=self.now, - created_by=None, - ) - - def test_create_empty_section_and_version(self): - """Test creating a section with no subsections. - - Expected results: - 1. A section and section version are created. - 2. The section version number is 1. - 3. The section is a draft with unpublished changes. - 4. There is no published version of the section. - """ - section, section_version = content_api.create_section_and_version( - learning_package_id=self.learning_package.id, - key="section:key", - title="Section", - created=self.now, - created_by=None, - ) - assert section, section_version - assert section_version.version_num == 1 - assert section_version in section.versioning.versions.all() - assert section.versioning.has_unpublished_changes - assert section.versioning.draft == section_version - assert section.versioning.published is None - assert section.publishable_entity.can_stand_alone - - def test_create_next_section_version_with_two_unpinned_subsections(self): - """Test creating a section version with two unpinned subsections. - - Expected results: - 1. A new section version is created. - 2. The section version number is 2. - 3. The section version is in the section's versions. - 4. The subsections are in the draft section version's subsection list and are unpinned. - """ - section, _section_version = content_api.create_section_and_version( - learning_package_id=self.learning_package.id, - key="section:key", - title="Section", - created=self.now, - created_by=None, - ) - section_version_v2 = content_api.create_next_section_version( - section=section, - title="Section", - subsections=[self.subsection_1, self.subsection_2], - created=self.now, - created_by=None, - ) - assert section_version_v2.version_num == 2 - assert section_version_v2 in section.versioning.versions.all() - assert content_api.get_subsections_in_section(section, published=False) == [ - Entry(self.subsection_1.versioning.draft), - Entry(self.subsection_2.versioning.draft), - ] - with pytest.raises(authoring_models.ContainerVersion.DoesNotExist): - # There is no published version of the section: - content_api.get_subsections_in_section(section, published=True) - def test_create_next_section_version_with_unpinned_and_pinned_subsections(self): """ Test creating a section version with one unpinned and one pinned 📌 subsection. """ - section, _section_version = content_api.create_section_and_version( - learning_package_id=self.learning_package.id, + section, _section_version = self.create_section_and_version( key="section:key", title="Section", - created=self.now, - created_by=None, ) - section_version_v2 = content_api.create_next_section_version( - section=section, + section_version_v2 = content_api.create_next_container_version( + section.pk, title="Section", - subsections=[ + entities=[ self.subsection_1, - self.subsection_2_v1 + self.subsection_2_v1, ], # Note the "v1" pinning 📌 the second one to version 1 created=self.now, created_by=None, ) assert section_version_v2.version_num == 2 assert section_version_v2 in section.versioning.versions.all() - assert content_api.get_subsections_in_section(section, published=False) == [ + assert content_api.get_entities_in_container(section, published=False) == [ Entry(self.subsection_1_v1), Entry(self.subsection_2_v1, pinned=True), # Pinned 📌 to v1 ] with pytest.raises(authoring_models.ContainerVersion.DoesNotExist): # There is no published version of the section: - content_api.get_subsections_in_section(section, published=True) + content_api.get_entities_in_container(section, published=True) def test_create_next_section_version_forcing_version_num(self): """ Test creating a section version while forcing the next version number. """ - section, _section_version = content_api.create_section_and_version( - learning_package_id=self.learning_package.id, + section, _section_version = self.create_section_and_version( key="section:key", title="Section", - created=self.now, - created_by=None, ) - section_version_v2 = content_api.create_next_section_version( - section=section, + section_version_v2 = content_api.create_next_container_version( + section.pk, title="Section", - subsections=[self.subsection_1, self.subsection_2], + entities=[self.subsection_1, self.subsection_2], created=self.now, created_by=None, force_version_num=5, # Forcing the next version number to be 5 (instead of the usual 2) @@ -393,11 +110,9 @@ def test_auto_publish_children(self): Test that publishing a section publishes its child subsections automatically. """ # Create a draft section with two draft subsections - section = self.create_section_with_subsections([self.subsection_1, self.subsection_2]) + section = self.create_section(entities=[self.subsection_1, self.subsection_2]) # Also create another subsection that's not in the section at all: - other_subsection, _os_v1 = self.create_subsection( - title="A draft subsection not in the section", key="subsection:3" - ) + other_subsection = self.create_subsection(title="A draft subsection not in the section", key="subsection:3") assert content_api.contains_unpublished_changes(section.pk) assert self.subsection_1.versioning.published is None @@ -426,10 +141,10 @@ def test_no_publish_parent(self): Test that publishing a subsection does NOT publish changes to its parent section """ # Create a draft section with two draft subsections - section = self.create_section_with_subsections([self.subsection_1, self.subsection_2]) + section = self.create_section(entities=[self.subsection_1, self.subsection_2]) assert section.versioning.has_unpublished_changes # Publish ONLY one of its child subsections - self.publish_subsection(self.subsection_1) + self.publish_container(self.subsection_1) self.subsection_1.refresh_from_db() # Clear cache on '.versioning' assert self.subsection_1.versioning.has_unpublished_changes is False @@ -439,19 +154,16 @@ def test_no_publish_parent(self): assert section.versioning.published is None with pytest.raises(authoring_models.ContainerVersion.DoesNotExist): # There is no published version of the section: - content_api.get_subsections_in_section(section, published=True) + content_api.get_entities_in_container(section, published=True) def test_add_subsection_after_publish(self): """ Adding a subsection to a published section will create a new version and show that the section has unpublished changes. """ - section, section_version = content_api.create_section_and_version( - learning_package_id=self.learning_package.id, + section, section_version = self.create_section_and_version( key="section:key", title="Section", - created=self.now, - created_by=None, ) assert section.versioning.draft == section_version assert section.versioning.published is None @@ -464,10 +176,10 @@ def test_add_subsection_after_publish(self): # Add a published subsection (unpinned): assert self.subsection_1.versioning.has_unpublished_changes is False - section_version_v2 = content_api.create_next_section_version( - section=section, + section_version_v2 = content_api.create_next_container_version( + section.pk, title=section_version.title, - subsections=[self.subsection_1], + entities=[self.subsection_1], created=self.now, created_by=None, entities_action=content_api.ChildrenEntitiesAction.APPEND, @@ -488,7 +200,7 @@ def test_modify_unpinned_subsection_after_publish(self): """ # Create a section with one unpinned draft subsection: assert self.subsection_1.versioning.has_unpublished_changes - section = self.create_section_with_subsections([self.subsection_1]) + section = self.create_section(entities=[self.subsection_1]) assert section.versioning.has_unpublished_changes # Publish the section and the subsection: @@ -500,7 +212,7 @@ def test_modify_unpinned_subsection_after_publish(self): assert self.subsection_1.versioning.has_unpublished_changes is False # Now modify the subsection by changing its title (it remains a draft): - subsection_1_v2 = self.modify_subsection(self.subsection_1, title="Modified Counting Problem with new title") + subsection_1_v2 = self.modify_container(self.subsection_1, title="Modified Counting Problem with new title") # The subsection now has unpublished changes; the section doesn't directly but does contain section.refresh_from_db() # Reloading the section is necessary, or 'section.versioning' will be outdated @@ -510,19 +222,19 @@ def test_modify_unpinned_subsection_after_publish(self): assert self.subsection_1.versioning.has_unpublished_changes # Since the subsection changes haven't been published, they should only appear in the draft section - assert content_api.get_subsections_in_section(section, published=False) == [ + assert content_api.get_entities_in_container(section, published=False) == [ Entry(subsection_1_v2), # new version ] - assert content_api.get_subsections_in_section(section, published=True) == [ + assert content_api.get_entities_in_container(section, published=True) == [ Entry(self.subsection_1_v1), # old version ] # But if we publish the subsection, the changes will appear in the published version of the section. - self.publish_subsection(self.subsection_1) - assert content_api.get_subsections_in_section(section, published=False) == [ + self.publish_container(self.subsection_1) + assert content_api.get_entities_in_container(section, published=False) == [ Entry(subsection_1_v2), # new version ] - assert content_api.get_subsections_in_section(section, published=True) == [ + assert content_api.get_entities_in_container(section, published=True) == [ Entry(subsection_1_v2), # new version ] assert content_api.contains_unpublished_changes(section.pk) is False # No longer contains unpublished changes @@ -534,17 +246,17 @@ def test_modify_pinned_subsection(self): which will continue to use the pinned version. """ # Create a section with one subsection (pinned 📌 to v1): - section = self.create_section_with_subsections([self.subsection_1_v1]) + section = self.create_section(entities=[self.subsection_1_v1]) # Publish the section and the subsection: content_api.publish_all_drafts(self.learning_package.id) expected_section_contents = [ Entry(self.subsection_1_v1, pinned=True), # pinned 📌 to v1 ] - assert content_api.get_subsections_in_section(section, published=True) == expected_section_contents + assert content_api.get_entities_in_container(section, published=True) == expected_section_contents # Now modify the subsection by changing its title (it remains a draft): - self.modify_subsection(self.subsection_1, title="Modified Counting Problem with new title") + self.modify_container(self.subsection_1, title="Modified Counting Problem with new title") # The subsection now has unpublished changes; the section is entirely unaffected section.refresh_from_db() # Reloading the section is necessary, or 'section.versioning' will be outdated @@ -554,12 +266,12 @@ def test_modify_pinned_subsection(self): assert self.subsection_1.versioning.has_unpublished_changes is True # Neither the draft nor the published version of the section is affected - assert content_api.get_subsections_in_section(section, published=False) == expected_section_contents - assert content_api.get_subsections_in_section(section, published=True) == expected_section_contents + assert content_api.get_entities_in_container(section, published=False) == expected_section_contents + assert content_api.get_entities_in_container(section, published=True) == expected_section_contents # Even if we publish the subsection, the section stays pinned to the specified version: - self.publish_subsection(self.subsection_1) - assert content_api.get_subsections_in_section(section, published=False) == expected_section_contents - assert content_api.get_subsections_in_section(section, published=True) == expected_section_contents + self.publish_container(self.subsection_1) + assert content_api.get_entities_in_container(section, published=False) == expected_section_contents + assert content_api.get_entities_in_container(section, published=True) == expected_section_contents def test_create_two_sections_with_same_subsections(self): """ @@ -567,48 +279,54 @@ def test_create_two_sections_with_same_subsections(self): subsections in each section. """ # Create a section with subsection 2 unpinned, subsection 2 pinned 📌, and subsection 1: - section1 = self.create_section_with_subsections( - [self.subsection_2, self.subsection_2_v1, self.subsection_1], key="u1" - ) + section1 = self.create_section(entities=[self.subsection_2, self.subsection_2_v1, self.subsection_1], key="u1") # Create a second section with subsection 1 pinned 📌, subsection 2, and subsection 1 unpinned: - section2 = self.create_section_with_subsections( - [self.subsection_1_v1, self.subsection_2, self.subsection_1], key="u2" - ) + section2 = self.create_section(entities=[self.subsection_1_v1, self.subsection_2, self.subsection_1], key="u2") # Check that the contents are as expected: assert [ - row.subsection_version for row in content_api.get_subsections_in_section(section1, published=False) - ] == [self.subsection_2_v1, self.subsection_2_v1, self.subsection_1_v1,] + row.entity_version.containerversion + for row in content_api.get_entities_in_container(section1, published=False) + ] == [ + self.subsection_2_v1, + self.subsection_2_v1, + self.subsection_1_v1, + ] assert [ - row.subsection_version for row in content_api.get_subsections_in_section(section2, published=False) - ] == [self.subsection_1_v1, self.subsection_2_v1, self.subsection_1_v1,] + row.entity_version.containerversion + for row in content_api.get_entities_in_container(section2, published=False) + ] == [ + self.subsection_1_v1, + self.subsection_2_v1, + self.subsection_1_v1, + ] # Modify subsection 1 - subsection_1_v2 = self.modify_subsection(self.subsection_1, title="subsection 1 v2") + subsection_1_v2 = self.modify_container(self.subsection_1, title="subsection 1 v2") # Publish changes content_api.publish_all_drafts(self.learning_package.id) # Modify subsection 2 - only in the draft - subsection_2_v2 = self.modify_subsection(self.subsection_2, title="subsection 2 DRAFT") + subsection_2_v2 = self.modify_container(self.subsection_2, title="subsection 2 DRAFT") # Check that the draft contents are as expected: - assert content_api.get_subsections_in_section(section1, published=False) == [ + assert content_api.get_entities_in_container(section1, published=False) == [ Entry(subsection_2_v2), # v2 in the draft version Entry(self.subsection_2_v1, pinned=True), # pinned 📌 to v1 Entry(subsection_1_v2), # v2 ] - assert content_api.get_subsections_in_section(section2, published=False) == [ + assert content_api.get_entities_in_container(section2, published=False) == [ Entry(self.subsection_1_v1, pinned=True), # pinned 📌 to v1 Entry(subsection_2_v2), # v2 in the draft version Entry(subsection_1_v2), # v2 ] # Check that the published contents are as expected: - assert content_api.get_subsections_in_section(section1, published=True) == [ + assert content_api.get_entities_in_container(section1, published=True) == [ Entry(self.subsection_2_v1), # v1 in the published version Entry(self.subsection_2_v1, pinned=True), # pinned 📌 to v1 Entry(subsection_1_v2), # v2 ] - assert content_api.get_subsections_in_section(section2, published=True) == [ + assert content_api.get_entities_in_container(section2, published=True) == [ Entry(self.subsection_1_v1, pinned=True), # pinned 📌 to v1 Entry(self.subsection_2_v1), # v1 in the published version Entry(subsection_1_v2), # v2 @@ -625,16 +343,16 @@ def test_publishing_shared_subsection(self): """ # 1️⃣ Create the sections and publish them: (s1, s1_v1), (s2, _s2_v1), (s3, s3_v1), (s4, s4_v1), (s5, s5_v1) = [ - self.create_subsection(key=f"C{i}", title=f"Subsection {i}") for i in range(1, 6) + self.create_subsection_and_version(key=f"C{i}", title=f"Subsection {i}") for i in range(1, 6) ] - section1 = self.create_section_with_subsections([s1, s2, s3], title="Section 1", key="section:1") - section2 = self.create_section_with_subsections([s2, s4, s5], title="Section 2", key="section:2") + section1 = self.create_section(entities=[s1, s2, s3], title="Section 1", key="section:1") + section2 = self.create_section(entities=[s2, s4, s5], title="Section 2", key="section:2") content_api.publish_all_drafts(self.learning_package.id) assert content_api.contains_unpublished_changes(section1.pk) is False assert content_api.contains_unpublished_changes(section2.pk) is False # 2️⃣ Then the author edits S2 inside of Section 1 making S2v2. - s2_v2 = self.modify_subsection(s2, title="U2 version 2") + s2_v2 = self.modify_container(s2, title="U2 version 2") # This makes S1, S2 both show up as Sections that CONTAIN unpublished changes, because they share the subsection assert content_api.contains_unpublished_changes(section1.pk) assert content_api.contains_unpublished_changes(section2.pk) @@ -645,7 +363,7 @@ def test_publishing_shared_subsection(self): assert section2.versioning.has_unpublished_changes is False # 3️⃣ In addition to this, the author also modifies another subsection in Section 2 (U5) - s5_v2 = self.modify_subsection(s5, title="S5 version 2") + s5_v2 = self.modify_container(s5, title="S5 version 2") # 4️⃣ The author then publishes Section 1, and therefore everything in it. content_api.publish_from_drafts( @@ -657,7 +375,7 @@ def test_publishing_shared_subsection(self): ) # Result: Section 1 will show the newly published version of U2: - assert content_api.get_subsections_in_section(section1, published=True) == [ + assert content_api.get_entities_in_container(section1, published=True) == [ Entry(s1_v1), Entry(s2_v2), # new published version of U2 Entry(s3_v1), @@ -667,7 +385,7 @@ def test_publishing_shared_subsection(self): # because publishing it anywhere publishes it everywhere. # But publishing U2 and Section 1 does not affect the other subsections in Section 2. # (Publish propagates downward, not upward) - assert content_api.get_subsections_in_section(section2, published=True) == [ + assert content_api.get_entities_in_container(section2, published=True) == [ Entry(s2_v2), # new published version of U2 Entry(s4_v1), # still original version of U4 (it was never modified) Entry(s5_v1), # still original version of U5 (it hasn't been published) @@ -679,9 +397,9 @@ def test_publishing_shared_subsection(self): assert content_api.contains_unpublished_changes(section2.pk) # 5️⃣ Publish subsection U5, which should be the only thing unpublished in the learning package - self.publish_subsection(s5) + self.publish_container(s5) # Result: Section 2 shows the new version of C5 and no longer contains unpublished changes: - assert content_api.get_subsections_in_section(section2, published=True) == [ + assert content_api.get_entities_in_container(section2, published=True) == [ Entry(s2_v2), # new published version of U2 Entry(s4_v1), # still original version of U4 (it was never modified) Entry(s5_v2), # new published version of U5 @@ -697,19 +415,19 @@ def test_query_count_of_contains_unpublished_changes(self): subsection_count = 2 subsections = [] for i in range(subsection_count): - subsection, _version = self.create_subsection( + subsection = self.create_subsection( key=f"Subsection {i}", title=f"Subsection {i}", ) subsections.append(subsection) - section = self.create_section_with_subsections(subsections) + section = self.create_section(entities=subsections) content_api.publish_all_drafts(self.learning_package.id) section.refresh_from_db() with self.assertNumQueries(1): assert content_api.contains_unpublished_changes(section.pk) is False # Modify the most recently created subsection: - self.modify_subsection(subsection, title="Modified Subsection") + self.modify_container(subsection, title="Modified Subsection") with self.assertNumQueries(1): assert content_api.contains_unpublished_changes(section.pk) is True @@ -719,12 +437,12 @@ def test_metadata_change_doesnt_create_entity_list(self): version, but can re-use the same EntityList. API consumers generally shouldn't depend on this behavior; it's an optimization. """ - section = self.create_section_with_subsections([self.subsection_1, self.subsection_2_v1]) + section = self.create_section(entities=[self.subsection_1, self.subsection_2_v1]) orig_version_num = section.versioning.draft.version_num orig_entity_list_id = section.versioning.draft.entity_list.pk - content_api.create_next_section_version(section, title="New Title", created=self.now) + content_api.create_next_container_version(section.pk, title="New Title", created=self.now, created_by=None) section.refresh_from_db() new_version_num = section.versioning.draft.version_num @@ -734,28 +452,29 @@ def test_metadata_change_doesnt_create_entity_list(self): assert new_entity_list_id == orig_entity_list_id def test_removing_subsection(self): - """ Test removing a subsection from a section (but not deleting it) """ - section = self.create_section_with_subsections([self.subsection_1, self.subsection_2]) + """Test removing a subsection from a section (but not deleting it)""" + section = self.create_section(entities=[self.subsection_1, self.subsection_2]) content_api.publish_all_drafts(self.learning_package.id) # Now remove subsection 2 - content_api.create_next_section_version( - section=section, + content_api.create_next_container_version( + section.pk, title="Revised with subsection 2 deleted", - subsections=[self.subsection_2], + entities=[self.subsection_2], created=self.now, + created_by=None, entities_action=content_api.ChildrenEntitiesAction.REMOVE, ) # Now it should not be listed in the section: - assert content_api.get_subsections_in_section(section, published=False) == [ + assert content_api.get_entities_in_container(section, published=False) == [ Entry(self.subsection_1_v1), ] section.refresh_from_db() assert section.versioning.has_unpublished_changes # The section itself and its subsection list have change assert content_api.contains_unpublished_changes(section.pk) # The published version of the section is not yet affected: - assert content_api.get_subsections_in_section(section, published=True) == [ + assert content_api.get_entities_in_container(section, published=True) == [ Entry(self.subsection_1_v1), Entry(self.subsection_2_v1), ] @@ -768,20 +487,20 @@ def test_removing_subsection(self): # but that would involve additional database lookup(s). section.refresh_from_db() assert content_api.contains_unpublished_changes(section.pk) is False - assert content_api.get_subsections_in_section(section, published=True) == [ + assert content_api.get_entities_in_container(section, published=True) == [ Entry(self.subsection_1_v1), ] def test_soft_deleting_subsection(self): - """ Test soft deleting a subsection that's in a section (but not removing it) """ - section = self.create_section_with_subsections([self.subsection_1, self.subsection_2]) + """Test soft deleting a subsection that's in a section (but not removing it)""" + section = self.create_section(entities=[self.subsection_1, self.subsection_2]) content_api.publish_all_drafts(self.learning_package.id) # Now soft delete subsection 2 content_api.soft_delete_draft(self.subsection_2.pk) # Now it should not be listed in the section: - assert content_api.get_subsections_in_section(section, published=False) == [ + assert content_api.get_entities_in_container(section, published=False) == [ Entry(self.subsection_1_v1), # subsection 2 is soft deleted from the draft. # TODO: should we return some kind of placeholder here, to indicate that a subsection is still listed in the @@ -791,7 +510,7 @@ def test_soft_deleting_subsection(self): assert section.versioning.has_unpublished_changes is False # The section and its subsection list is not changed assert content_api.contains_unpublished_changes(section.pk) # But it CONTAINS unpublished change (deletion) # The published version of the section is not yet affected: - assert content_api.get_subsections_in_section(section, published=True) == [ + assert content_api.get_entities_in_container(section, published=True) == [ Entry(self.subsection_1_v1), Entry(self.subsection_2_v1), ] @@ -799,34 +518,35 @@ def test_soft_deleting_subsection(self): # But when we publish the deletion, the published version is affected: content_api.publish_all_drafts(self.learning_package.id) assert content_api.contains_unpublished_changes(section.pk) is False - assert content_api.get_subsections_in_section(section, published=True) == [ + assert content_api.get_entities_in_container(section, published=True) == [ Entry(self.subsection_1_v1), ] def test_soft_deleting_and_removing_subsection(self): - """ Test soft deleting a subsection that's in a section AND removing it """ - section = self.create_section_with_subsections([self.subsection_1, self.subsection_2]) + """Test soft deleting a subsection that's in a section AND removing it""" + section = self.create_section(entities=[self.subsection_1, self.subsection_2]) content_api.publish_all_drafts(self.learning_package.id) # Now soft delete subsection 2 content_api.soft_delete_draft(self.subsection_2.pk) # And remove it from the section: - content_api.create_next_section_version( - section=section, + content_api.create_next_container_version( + section.pk, title="Revised with subsection 2 deleted", - subsections=[self.subsection_2], + entities=[self.subsection_2], created=self.now, entities_action=content_api.ChildrenEntitiesAction.REMOVE, + created_by=None, ) # Now it should not be listed in the section: - assert content_api.get_subsections_in_section(section, published=False) == [ + assert content_api.get_entities_in_container(section, published=False) == [ Entry(self.subsection_1_v1), ] assert section.versioning.has_unpublished_changes is True assert content_api.contains_unpublished_changes(section.pk) # The published version of the section is not yet affected: - assert content_api.get_subsections_in_section(section, published=True) == [ + assert content_api.get_entities_in_container(section, published=True) == [ Entry(self.subsection_1_v1), Entry(self.subsection_2_v1), ] @@ -834,27 +554,27 @@ def test_soft_deleting_and_removing_subsection(self): # But when we publish the deletion, the published version is affected: content_api.publish_all_drafts(self.learning_package.id) assert content_api.contains_unpublished_changes(section.pk) is False - assert content_api.get_subsections_in_section(section, published=True) == [ + assert content_api.get_entities_in_container(section, published=True) == [ Entry(self.subsection_1_v1), ] def test_soft_deleting_pinned_subsection(self): - """ Test soft deleting a pinned 📌 subsection that's in a section """ - section = self.create_section_with_subsections([self.subsection_1_v1, self.subsection_2_v1]) + """Test soft deleting a pinned 📌 subsection that's in a section""" + section = self.create_section(entities=[self.subsection_1_v1, self.subsection_2_v1]) content_api.publish_all_drafts(self.learning_package.id) # Now soft delete subsection 2 content_api.soft_delete_draft(self.subsection_2.pk) # Now it should still be listed in the section: - assert content_api.get_subsections_in_section(section, published=False) == [ + assert content_api.get_entities_in_container(section, published=False) == [ Entry(self.subsection_1_v1, pinned=True), Entry(self.subsection_2_v1, pinned=True), ] assert section.versioning.has_unpublished_changes is False # The section and its subsection list is not changed assert content_api.contains_unpublished_changes(section.pk) is False # nor does it contain changes # The published version of the section is also not affected: - assert content_api.get_subsections_in_section(section, published=True) == [ + assert content_api.get_entities_in_container(section, published=True) == [ Entry(self.subsection_1_v1, pinned=True), Entry(self.subsection_2_v1, pinned=True), ] @@ -866,8 +586,8 @@ def test_soft_delete_section(self): See https://github.com/openedx/frontend-app-authoring/issues/1693 """ # Create two sections, one of which we will soon delete: - section_to_delete = self.create_section_with_subsections([self.subsection_1, self.subsection_2]) - other_section = self.create_section_with_subsections([self.subsection_1], key="other") + section_to_delete = self.create_section(entities=[self.subsection_1, self.subsection_2]) + other_section = self.create_section(entities=[self.subsection_1], key="other") # Publish everything: content_api.publish_all_drafts(self.learning_package.id) @@ -879,7 +599,7 @@ def test_soft_delete_section(self): assert section_to_delete.versioning.published is not None self.subsection_1.refresh_from_db() assert self.subsection_1.versioning.draft is not None - assert content_api.get_subsections_in_section(other_section, published=False) == [Entry(self.subsection_1_v1)] + assert content_api.get_entities_in_container(other_section, published=False) == [Entry(self.subsection_1_v1)] # Publish everything: content_api.publish_all_drafts(self.learning_package.id) @@ -890,8 +610,8 @@ def test_soft_delete_section(self): self.subsection_1.refresh_from_db() assert self.subsection_1.versioning.draft is not None assert self.subsection_1.versioning.published is not None - assert content_api.get_subsections_in_section(other_section, published=False) == [Entry(self.subsection_1_v1)] - assert content_api.get_subsections_in_section(other_section, published=True) == [Entry(self.subsection_1_v1)] + assert content_api.get_entities_in_container(other_section, published=False) == [Entry(self.subsection_1_v1)] + assert content_api.get_entities_in_container(other_section, published=True) == [Entry(self.subsection_1_v1)] def test_snapshots_of_published_section(self): """ @@ -899,10 +619,10 @@ def test_snapshots_of_published_section(self): sections and their contents. """ # At first the section has one subsection (unpinned): - section = self.create_section_with_subsections([self.subsection_1]) - self.modify_subsection(self.subsection_1, title="Subsection 1 as of checkpoint 1") - before_publish = content_api.get_subsections_in_published_section_as_of(section, 0) - assert before_publish is None + section = self.create_section(entities=[self.subsection_1]) + self.modify_container(self.subsection_1, title="Subsection 1 as of checkpoint 1") + _, before_publish = content_api.get_entities_in_container_as_of(section, 0) + assert before_publish == [] # Publish everything, creating Checkpoint 1 checkpoint_1 = content_api.publish_all_drafts(self.learning_package.id, message="checkpoint 1") @@ -910,19 +630,20 @@ def test_snapshots_of_published_section(self): ######################################################################## # Now we update the title of the subsection. - self.modify_subsection(self.subsection_1, title="Subsection 1 as of checkpoint 2") + self.modify_container(self.subsection_1, title="Subsection 1 as of checkpoint 2") # Publish everything, creating Checkpoint 2 checkpoint_2 = content_api.publish_all_drafts(self.learning_package.id, message="checkpoint 2") ######################################################################## # Now add a second subsection to the section: - self.modify_subsection(self.subsection_1, title="Subsection 1 as of checkpoint 3") - self.modify_subsection(self.subsection_2, title="Subsection 2 as of checkpoint 3") - content_api.create_next_section_version( - section=section, + self.modify_container(self.subsection_1, title="Subsection 1 as of checkpoint 3") + self.modify_container(self.subsection_2, title="Subsection 2 as of checkpoint 3") + content_api.create_next_container_version( + section.pk, title="Section title in checkpoint 3", - subsections=[self.subsection_1, self.subsection_2], + entities=[self.subsection_1, self.subsection_2], created=self.now, + created_by=None, ) # Publish everything, creating Checkpoint 3 checkpoint_3 = content_api.publish_all_drafts(self.learning_package.id, message="checkpoint 3") @@ -930,36 +651,37 @@ def test_snapshots_of_published_section(self): # Now add a third subsection to the section, a pinned 📌 version of subsection 1. # This will test pinned versions and also test adding at the beginning rather than the end of the section. - content_api.create_next_section_version( - section=section, + content_api.create_next_container_version( + section.pk, title="Section title in checkpoint 4", - subsections=[self.subsection_1_v1, self.subsection_1, self.subsection_2], + entities=[self.subsection_1_v1, self.subsection_1, self.subsection_2], created=self.now, + created_by=None, ) # Publish everything, creating Checkpoint 4 checkpoint_4 = content_api.publish_all_drafts(self.learning_package.id, message="checkpoint 4") ######################################################################## # Modify the drafts, but don't publish: - self.modify_subsection(self.subsection_1, title="Subsection 1 draft") - self.modify_subsection(self.subsection_2, title="Subsection 2 draft") + self.modify_container(self.subsection_1, title="Subsection 1 draft") + self.modify_container(self.subsection_2, title="Subsection 2 draft") # Now fetch the snapshots: - as_of_checkpoint_1 = content_api.get_subsections_in_published_section_as_of(section, checkpoint_1.pk) - assert [cv.subsection_version.title for cv in as_of_checkpoint_1] == [ + _, as_of_checkpoint_1 = content_api.get_entities_in_container_as_of(section, checkpoint_1.pk) + assert [ev.entity_version.title for ev in as_of_checkpoint_1] == [ "Subsection 1 as of checkpoint 1", ] - as_of_checkpoint_2 = content_api.get_subsections_in_published_section_as_of(section, checkpoint_2.pk) - assert [cv.subsection_version.title for cv in as_of_checkpoint_2] == [ + _, as_of_checkpoint_2 = content_api.get_entities_in_container_as_of(section, checkpoint_2.pk) + assert [ev.entity_version.title for ev in as_of_checkpoint_2] == [ "Subsection 1 as of checkpoint 2", ] - as_of_checkpoint_3 = content_api.get_subsections_in_published_section_as_of(section, checkpoint_3.pk) - assert [cv.subsection_version.title for cv in as_of_checkpoint_3] == [ + _, as_of_checkpoint_3 = content_api.get_entities_in_container_as_of(section, checkpoint_3.pk) + assert [ev.entity_version.title for ev in as_of_checkpoint_3] == [ "Subsection 1 as of checkpoint 3", "Subsection 2 as of checkpoint 3", ] - as_of_checkpoint_4 = content_api.get_subsections_in_published_section_as_of(section, checkpoint_4.pk) - assert [cv.subsection_version.title for cv in as_of_checkpoint_4] == [ + _, as_of_checkpoint_4 = content_api.get_entities_in_container_as_of(section, checkpoint_4.pk) + assert [ev.entity_version.title for ev in as_of_checkpoint_4] == [ "Subsection (1)", # Pinned. This title is self.subsection_1_v1.title (original v1 title) "Subsection 1 as of checkpoint 3", # we didn't modify these subsections so they're same as in snapshot 3 "Subsection 2 as of checkpoint 3", # we didn't modify these subsections so they're same as in snapshot 3 @@ -970,32 +692,34 @@ def test_sections_containing(self): Test that we can efficiently get a list of all the [draft] sections containing a given subsection. """ - subsection_1_v2 = self.modify_subsection(self.subsection_1, title="modified subsection 1") + subsection_1_v2 = self.modify_container(self.subsection_1, title="modified subsection 1") # Create a few sections, some of which contain subsection 1 and others which don't: # Note: it is important that some of these sections contain other subsections, to ensure complex JOINs required # for this query are working correctly, especially in the case of ignore_pinned=True. # Section 1 ✅ has subsection 1, pinned 📌 to V1 - section1_1pinned = self.create_section_with_subsections([self.subsection_1_v1, self.subsection_2], key="s1") + section1_1pinned = self.create_section(entities=[self.subsection_1_v1, self.subsection_2], key="s1") # Section 2 ✅ has subsection 1, pinned 📌 to V2 - section2_1pinned_v2 = self.create_section_with_subsections([subsection_1_v2, self.subsection_2_v1], key="s2") + section2_1pinned_v2 = self.create_section(entities=[subsection_1_v2, self.subsection_2_v1], key="s2") # Section 3 doesn't contain it - _section3_no = self.create_section_with_subsections([self.subsection_2], key="s3") + _section3_no = self.create_section(entities=[self.subsection_2], key="s3") # Section 4 ✅ has subsection 1, unpinned - section4_unpinned = self.create_section_with_subsections([ - self.subsection_1, self.subsection_2, self.subsection_2_v1, - ], key="s4") + section4_unpinned = self.create_section( + entities=[ + self.subsection_1, + self.subsection_2, + self.subsection_2_v1, + ], + key="s4", + ) # Sections 5/6 don't contain it - _section5_no = self.create_section_with_subsections([self.subsection_2_v1, self.subsection_2], key="s5") - _section6_no = self.create_section_with_subsections([], key="s6") + _section5_no = self.create_section(entities=[self.subsection_2_v1, self.subsection_2], key="s5") + _section6_no = self.create_section(entities=[], key="s6") # No need to publish anything as the get_containers_with_entity() API only considers drafts (for now). with self.assertNumQueries(1): - result = [ - c.section for c in - content_api.get_containers_with_entity(self.subsection_1.pk).select_related("section") - ] + result = list(content_api.get_containers_with_entity(self.subsection_1.pk)) assert result == [ section1_1pinned, section2_1pinned_v2, @@ -1006,34 +730,31 @@ def test_sections_containing(self): # about pinned uses anyways (they would be unaffected by a delete). with self.assertNumQueries(1): - result2 = [ - c.section for c in - content_api.get_containers_with_entity( - self.subsection_1.pk, ignore_pinned=True - ).select_related("section") - ] + result2 = list(content_api.get_containers_with_entity(self.subsection_1.pk, ignore_pinned=True)) assert result2 == [section4_unpinned] - def test_get_subsections_in_section_queries(self): + def test_get_entities_in_container_queries(self): """ - Test the query count of get_subsections_in_section() + Test the query count of get_entities_in_container() This also tests the generic method get_entities_in_container() """ - section = self.create_section_with_subsections([ - self.subsection_1, - self.subsection_2, - self.subsection_2_v1, - ]) - with self.assertNumQueries(4): - result = content_api.get_subsections_in_section(section, published=False) + section = self.create_section( + entities=[ + self.subsection_1, + self.subsection_2, + self.subsection_2_v1, + ] + ) + with self.assertNumQueries(2): + result = content_api.get_entities_in_container(section, published=False) assert result == [ Entry(self.subsection_1.versioning.draft), Entry(self.subsection_2.versioning.draft), Entry(self.subsection_2.versioning.draft, pinned=True), ] content_api.publish_all_drafts(self.learning_package.id) - with self.assertNumQueries(4): - result = content_api.get_subsections_in_section(section, published=True) + with self.assertNumQueries(2): + result = content_api.get_entities_in_container(section, published=True) assert result == [ Entry(self.subsection_1.versioning.draft), Entry(self.subsection_2.versioning.draft), @@ -1044,26 +765,23 @@ def test_add_remove_container_children(self): """ Test adding and removing children subsections from sections. """ - section, section_version = content_api.create_section_and_version( - learning_package_id=self.learning_package.id, + section, section_version = self.create_section_and_version( key="section:key", title="Section", - subsections=[self.subsection_1], - created=self.now, - created_by=None, + entities=[self.subsection_1], ) - assert content_api.get_subsections_in_section(section, published=False) == [ + assert content_api.get_entities_in_container(section, published=False) == [ Entry(self.subsection_1.versioning.draft), ] - subsection_3, _ = self.create_subsection( + subsection_3 = self.create_subsection( key="Subsection (3)", title="Subsection (3)", ) # Add subsection_2 and subsection_3 - section_version_v2 = content_api.create_next_section_version( - section=section, + section_version_v2 = content_api.create_next_container_version( + section.pk, title=section_version.title, - subsections=[self.subsection_2, subsection_3], + entities=[self.subsection_2, subsection_3], created=self.now, created_by=None, entities_action=content_api.ChildrenEntitiesAction.APPEND, @@ -1072,24 +790,24 @@ def test_add_remove_container_children(self): assert section_version_v2.version_num == 2 assert section_version_v2 in section.versioning.versions.all() # Verify that subsection_2 and subsection_3 is added to end - assert content_api.get_subsections_in_section(section, published=False) == [ + assert content_api.get_entities_in_container(section, published=False) == [ Entry(self.subsection_1.versioning.draft), Entry(self.subsection_2.versioning.draft), Entry(subsection_3.versioning.draft), ] # Remove subsection_1 - content_api.create_next_section_version( - section=section, + content_api.create_next_container_version( + section.pk, title=section_version.title, - subsections=[self.subsection_1], + entities=[self.subsection_1], created=self.now, created_by=None, entities_action=content_api.ChildrenEntitiesAction.REMOVE, ) section.refresh_from_db() # Verify that subsection_1 is removed - assert content_api.get_subsections_in_section(section, published=False) == [ + assert content_api.get_entities_in_container(section, published=False) == [ Entry(self.subsection_2.versioning.draft), Entry(subsection_3.versioning.draft), ] @@ -1098,35 +816,35 @@ def test_get_container_children_count(self): """ Test get_container_children_count() """ - section = self.create_section_with_subsections([self.subsection_1]) - assert content_api.get_container_children_count(section.container, published=False) == 1 + section = self.create_section(entities=[self.subsection_1]) + assert content_api.get_container_children_count(section, published=False) == 1 # publish content_api.publish_all_drafts(self.learning_package.id) section_version = section.versioning.draft - content_api.create_next_section_version( - section=section, + content_api.create_next_container_version( + section.pk, title=section_version.title, - subsections=[self.subsection_2], + entities=[self.subsection_2], created=self.now, created_by=None, entities_action=content_api.ChildrenEntitiesAction.APPEND, ) section.refresh_from_db() # Should have two subsections in draft version and 1 in published version - assert content_api.get_container_children_count(section.container, published=False) == 2 - assert content_api.get_container_children_count(section.container, published=True) == 1 + assert content_api.get_container_children_count(section, published=False) == 2 + assert content_api.get_container_children_count(section, published=True) == 1 # publish content_api.publish_all_drafts(self.learning_package.id) section.refresh_from_db() - assert content_api.get_container_children_count(section.container, published=True) == 2 + assert content_api.get_container_children_count(section, published=True) == 2 # Soft delete subsection_1 content_api.soft_delete_draft(self.subsection_1.pk) section.refresh_from_db() # Should contain only 1 child - assert content_api.get_container_children_count(section.container, published=False) == 1 + assert content_api.get_container_children_count(section, published=False) == 1 content_api.publish_all_drafts(self.learning_package.id) section.refresh_from_db() - assert content_api.get_container_children_count(section.container, published=True) == 1 + assert content_api.get_container_children_count(section, published=True) == 1 # Tests TODO: # Test that I can get a [PublishLog] history of a given section and all its children, including children that aren't diff --git a/tests/openedx_content/applets/subsections/test_api.py b/tests/openedx_content/applets/subsections/test_api.py index 8d93e1085..2e6fa2588 100644 --- a/tests/openedx_content/applets/subsections/test_api.py +++ b/tests/openedx_content/applets/subsections/test_api.py @@ -1,137 +1,34 @@ """ Basic tests for the subsections API. """ -from unittest.mock import patch import ddt # type: ignore[import] import pytest from django.core.exceptions import ValidationError -from django.db import IntegrityError import openedx_content.api as content_api from openedx_content import models_api as authoring_models -from ..units.test_api import UnitTestCase +from ..publishing.container_test_case import ContainerTestCase, Entry -Entry = content_api.SubsectionListEntry - -# TODO: Turn UnitTestCase into UnitTestMixin and remove the -# test-inherits-tests pylint warning below. -# https://github.com/openedx/openedx-core/issues/308 @ddt.ddt -class SubSectionTestCase(UnitTestCase): # pylint: disable=test-inherits-tests - """ Test cases for Subsections (containers of units) """ +class SubSectionTestCase(ContainerTestCase): + """Test cases for Subsections (containers of units)""" def setUp(self) -> None: super().setUp() - self.unit_1, self.unit_1_v1 = self.create_unit( - key="Unit (1)", - title="Unit (1)", - ) - self.unit_2, self.unit_2_v1 = self.create_unit( - key="Unit (2)", - title="Unit (2)", - ) - - def create_unit(self, *, title: str = "Test Unit", key: str = "unit:1") -> tuple[ - authoring_models.Unit, authoring_models.UnitVersion - ]: - """ Helper method to quickly create a unit """ - return content_api.create_unit_and_version( - self.learning_package.id, - key=key, - title=title, - created=self.now, - created_by=None, - ) - - def create_subsection_with_units( - self, - units: list[authoring_models.Unit | authoring_models.UnitVersion], - *, - title="Unit", - key="unit:key", - ) -> authoring_models.Subsection: - """ Helper method to quickly create a subsection with some units """ - subsection, _subsection_v1 = content_api.create_subsection_and_version( - learning_package_id=self.learning_package.id, - key=key, - title=title, - units=units, - created=self.now, - created_by=None, - ) - return subsection - - def modify_unit( - self, - unit: authoring_models.Unit, - *, - title="Modified Unit", - timestamp=None, - ) -> authoring_models.UnitVersion: - """ - Helper method to modify a unit for the purposes of testing units/drafts/pinning/publishing/etc. - """ - return content_api.create_next_unit_version( - unit, - title=title, - created=timestamp or self.now, - created_by=None, - ) - - def publish_unit(self, unit: authoring_models.Unit): - """ - Helper method to publish a single unit. - """ - content_api.publish_from_drafts( - self.learning_package.pk, - draft_qset=content_api.get_all_drafts(self.learning_package.pk).filter( - entity=unit.publishable_entity, - ), - ) - - def test_get_subsection(self): - """ - Test get_subsection() - """ - subsection = self.create_subsection_with_units([self.unit_1, self.unit_2]) - with self.assertNumQueries(1): - result = content_api.get_subsection(subsection.pk) - assert result == subsection - # Versioning data should be pre-loaded via select_related() - with self.assertNumQueries(0): - assert result.versioning.has_unpublished_changes - - def test_get_subsection_version(self): - """ - Test get_subsection_version() - """ - subsection = self.create_subsection_with_units([]) - draft = subsection.versioning.draft - with self.assertNumQueries(1): - result = content_api.get_subsection_version(draft.pk) - assert result == draft - - def test_get_latest_subsection_version(self): - """ - Test test_get_latest_subsection_version() - """ - subsection = self.create_subsection_with_units([]) - draft = subsection.versioning.draft - with self.assertNumQueries(2): - result = content_api.get_latest_subsection_version(subsection.pk) - assert result == draft + self.unit_1, self.unit_1_v1 = self.create_unit_and_version(key="Unit (1)", title="Unit (1)") + self.unit_2, self.unit_2_v1 = self.create_unit_and_version(key="Unit (2)", title="Unit (2)") def test_get_containers(self): """ Test get_containers() """ - subsection = self.create_subsection_with_units([]) + subsection = self.create_subsection(entities=[]) with self.assertNumQueries(1): result = list(content_api.get_containers(self.learning_package.id)) - assert result == [self.unit_1.container, self.unit_2.container, subsection.container] + assert result == [self.unit_1, self.unit_2, subsection] # Versioning data should be pre-loaded via select_related() with self.assertNumQueries(0): assert result[0].versioning.has_unpublished_changes @@ -140,24 +37,23 @@ def test_get_containers_deleted(self): """ Test that get_containers() does not return soft-deleted sections. """ - subsection = self.create_subsection_with_units([]) + subsection = self.create_subsection(entities=[]) content_api.soft_delete_draft(subsection.pk) with self.assertNumQueries(1): result = list(content_api.get_containers(self.learning_package.id, include_deleted=True)) - assert result == [self.unit_1.container, self.unit_2.container, subsection.container] + assert result == [self.unit_1, self.unit_2, subsection] with self.assertNumQueries(1): result = list(content_api.get_containers(self.learning_package.id)) - assert result == [self.unit_1.container, self.unit_2.container] + assert result == [self.unit_1, self.unit_2] def test_get_container(self): """ Test get_container() """ - subsection = self.create_subsection_with_units([self.unit_1, self.unit_2]) + subsection = self.create_subsection(entities=[self.unit_1, self.unit_2]) with self.assertNumQueries(1): result = content_api.get_container(subsection.pk) - assert result == subsection.container # Versioning data should be pre-loaded via select_related() with self.assertNumQueries(0): assert result.versioning.has_unpublished_changes @@ -166,42 +62,26 @@ def test_get_container_by_key(self): """ Test get_container_by_key() """ - subsection = self.create_subsection_with_units([]) + subsection = self.create_subsection(entities=[]) with self.assertNumQueries(1): result = content_api.get_container_by_key( self.learning_package.id, key=subsection.publishable_entity.key, ) - assert result == subsection.container # Versioning data should be pre-loaded via select_related() with self.assertNumQueries(0): assert result.versioning.has_unpublished_changes - def test_subsection_container_versioning(self): - """ - Test that the .versioning helper of a Sebsection returns a SubsectionVersion, and - same for the generic Container equivalent. - """ - subsection = self.create_subsection_with_units([self.unit_1, self.unit_2]) - container = subsection.container - container_version = container.versioning.draft - assert isinstance(container_version, authoring_models.ContainerVersion) - subsection_version = subsection.versioning.draft - assert isinstance(subsection_version, authoring_models.SubsectionVersion) - assert subsection_version.container_version == container_version - assert subsection_version.container_version.container == container - assert subsection_version.subsection == subsection - def test_create_subsection_queries(self): """ Test how many database queries are required to create a subsection """ # The exact numbers here aren't too important - this is just to alert us if anything significant changes. - with self.assertNumQueries(28): - _empty_subsection = self.create_subsection_with_units([]) - with self.assertNumQueries(35): + with self.assertNumQueries(23): + _empty_subsection = self.create_subsection(entities=[]) + with self.assertNumQueries(33): # And try with a non-empty subsection: - self.create_subsection_with_units([self.unit_1, self.unit_2_v1], key="u2") + self.create_subsection(entities=[self.unit_1, self.unit_2_v1], key="u2") def test_create_subsection_with_invalid_children(self): """ @@ -209,33 +89,38 @@ def test_create_subsection_with_invalid_children(self): exception is raised. """ # Create two subsections: - subsection, subsection_version = content_api.create_subsection_and_version( + subsection, subsection_version = content_api.create_container_and_version( learning_package_id=self.learning_package.id, key="subsection:key", title="Subsection", created=self.now, created_by=None, + container_type=content_api.Subsection, ) assert subsection.versioning.draft == subsection_version - subsection2, _s2v1 = content_api.create_subsection_and_version( + subsection2, _s2v1 = content_api.create_container_and_version( learning_package_id=self.learning_package.id, key="subsection:key2", title="Subsection 2", created=self.now, created_by=None, + container_type=content_api.Subsection, ) # Try adding a Subsection to a Subsection - with pytest.raises(TypeError, match="Subsection units must be either Unit or UnitVersion."): - content_api.create_next_subsection_version( - subsection=subsection, + with pytest.raises( + ValidationError, match='The entity "subsection:key2" cannot be added to a "subsection" container.' + ) as exc: + content_api.create_next_container_version( + subsection.pk, title="Subsection Containing a Subsection", - units=[subsection2], + entities=[subsection2], created=self.now, created_by=None, ) + assert "Only Units can be added as children of a Subsection" in str(exc.value.__cause__) # Check that a new version was not created: subsection.refresh_from_db() - assert content_api.get_subsection(subsection.pk).versioning.draft == subsection_version + assert content_api.get_container(subsection.pk).versioning.draft == subsection_version assert subsection.versioning.draft == subsection_version def test_adding_external_units(self): @@ -244,60 +129,48 @@ def test_adding_external_units(self): subsection. """ learning_package2 = content_api.create_learning_package(key="other-package", title="Other Package") - subsection, _subsection_version = content_api.create_subsection_and_version( + subsection, _subsection_version = content_api.create_container_and_version( learning_package_id=learning_package2.pk, key="subsection:key", title="Subsection", created=self.now, created_by=None, + container_type=content_api.Subsection, ) - assert self.unit_1.container.publishable_entity.learning_package != learning_package2 + assert self.unit_1.publishable_entity.learning_package != learning_package2 # Try adding a a unit from LP 1 (self.learning_package) to a subsection from LP 2 with pytest.raises(ValidationError, match="Container entities must be from the same learning package."): - content_api.create_next_subsection_version( - subsection=subsection, + content_api.create_next_container_version( + subsection.pk, title="Subsection Containing an External Unit", - units=[self.unit_1], + entities=[self.unit_1], created=self.now, created_by=None, ) - @patch('openedx_content.applets.subsections.api._pub_entities_for_units') - def test_adding_mismatched_versions(self, mock_entities_for_units): # pylint: disable=arguments-renamed + def test_cannot_add_deleted_container(self): """ - Test that versioned units must match their entities. + Test that non-existent units cannot be added to subsections """ - mock_entities_for_units.return_value = [ - content_api.ContainerEntityRow( - entity_pk=self.unit_1.pk, - version_pk=self.unit_2_v1.pk, - ), - ] - # Try adding a a unit from LP 1 (self.learning_package) to a subsection from LP 2 - with pytest.raises(ValidationError, match="Container entity versions must belong to the specified entity"): - content_api.create_subsection_and_version( - learning_package_id=self.unit_1.container.publishable_entity.learning_package.pk, - key="subsection:key", - title="Subsection", - units=[self.unit_1], - created=self.now, - created_by=None, - ) + self.unit_1.delete() + with pytest.raises(authoring_models.Container.DoesNotExist): + self.create_subsection(entities=[self.unit_1]) - @ddt.data(True, False) - @pytest.mark.skip(reason="FIXME: publishable_entity is not deleted from the database with the unit.") - # FIXME: Also, exception is Container.DoesNotExist, not Unit.DoesNotExist - def test_cannot_add_invalid_ids(self, pin_version): + def test_cannot_add_corrupted_unit(self): """ Test that non-existent units cannot be added to subsections """ self.unit_1.delete() - if pin_version: - units = [self.unit_1_v1] - else: - units = [self.unit_1] - with pytest.raises((IntegrityError, authoring_models.Unit.DoesNotExist)): - self.create_subsection_with_units(units) + # Note the PublishableEntity and PublishableEntityVersion still exist, so this is a weird corrupt state: + self.unit_1_v1.publishable_entity_version.refresh_from_db() # No error + self.unit_1_v1.publishable_entity_version.entity.refresh_from_db() # No error + # Now add this corrupted unit, pinned to v1: + with pytest.raises( + ValidationError, match=r'The entity "Unit \(1\)" cannot be added to a "subsection" container.' + ) as exc: + self.create_subsection(entities=[self.unit_1_v1]) + # And the exception should be chained from a more specific exception: + assert isinstance(exc.value.__cause__, authoring_models.Container.DoesNotExist) def test_create_empty_subsection_and_version(self): """Test creating a subsection with no units. @@ -308,12 +181,13 @@ def test_create_empty_subsection_and_version(self): 3. The subsection is a draft with unpublished changes. 4. There is no published version of the subsection. """ - subsection, subsection_version = content_api.create_subsection_and_version( + subsection, subsection_version = content_api.create_container_and_version( learning_package_id=self.learning_package.id, key="subsection:key", title="Subsection", created=self.now, created_by=None, + container_type=content_api.Subsection, ) assert subsection, subsection_version assert subsection_version.version_num == 1 @@ -332,48 +206,50 @@ def test_create_next_subsection_version_with_two_unpinned_units(self): 3. The subsection version is in the subsection's versions. 4. The units are in the draft subsection version's unit list and are unpinned. """ - subsection, _subsection_version = content_api.create_subsection_and_version( + subsection, _subsection_version = content_api.create_container_and_version( learning_package_id=self.learning_package.id, key="subsection:key", title="Subsection", created=self.now, created_by=None, + container_type=content_api.Subsection, ) - subsection_version_v2 = content_api.create_next_subsection_version( - subsection=subsection, + subsection_version_v2 = content_api.create_next_container_version( + subsection.pk, title="Subsection", - units=[self.unit_1, self.unit_2], + entities=[self.unit_1, self.unit_2], created=self.now, created_by=None, ) assert subsection_version_v2.version_num == 2 assert subsection_version_v2 in subsection.versioning.versions.all() - assert content_api.get_units_in_subsection(subsection, published=False) == [ + assert content_api.get_entities_in_container(subsection, published=False) == [ Entry(self.unit_1.versioning.draft), Entry(self.unit_2.versioning.draft), ] with pytest.raises(authoring_models.ContainerVersion.DoesNotExist): # There is no published version of the subsection: - content_api.get_units_in_subsection(subsection, published=True) + content_api.get_entities_in_container(subsection, published=True) def test_create_next_subsection_version_forcing_version_num(self): """ Test creating a subsection version while forcing the next version number. """ - subsection, _subsection_version = content_api.create_subsection_and_version( + subsection, _subsection_version = content_api.create_container_and_version( learning_package_id=self.learning_package.id, key="subsection:key", title="Subsection", created=self.now, created_by=None, + container_type=content_api.Subsection, ) - subsection_version_v2 = content_api.create_next_subsection_version( - subsection=subsection, + subsection_version_v2 = content_api.create_next_container_version( + subsection.pk, title="Subsection", - units=[self.unit_1, self.unit_2], + entities=[self.unit_1, self.unit_2], created=self.now, created_by=None, - force_version_num=4 + force_version_num=4, ) assert subsection_version_v2.version_num == 4 @@ -381,38 +257,39 @@ def test_create_next_subsection_version_with_unpinned_and_pinned_units(self): """ Test creating a subsection version with one unpinned and one pinned 📌 unit. """ - subsection, _subsection_version = content_api.create_subsection_and_version( + subsection, _subsection_version = content_api.create_container_and_version( learning_package_id=self.learning_package.id, key="subsection:key", title="Subsection", created=self.now, created_by=None, + container_type=content_api.Subsection, ) - subsection_version_v2 = content_api.create_next_subsection_version( - subsection=subsection, + subsection_version_v2 = content_api.create_next_container_version( + subsection.pk, title="Subsection", - units=[self.unit_1, self.unit_2_v1], # Note the "v1" pinning 📌 the second one to version 1 + entities=[self.unit_1, self.unit_2_v1], # Note the "v1" pinning 📌 the second one to version 1 created=self.now, created_by=None, ) assert subsection_version_v2.version_num == 2 assert subsection_version_v2 in subsection.versioning.versions.all() - assert content_api.get_units_in_subsection(subsection, published=False) == [ + assert content_api.get_entities_in_container(subsection, published=False) == [ Entry(self.unit_1_v1), Entry(self.unit_2_v1, pinned=True), # Pinned 📌 to v1 ] with pytest.raises(authoring_models.ContainerVersion.DoesNotExist): # There is no published version of the subsection: - content_api.get_units_in_subsection(subsection, published=True) + content_api.get_entities_in_container(subsection, published=True) def test_auto_publish_children(self): """ Test that publishing a subsection publishes its child units automatically. """ # Create a draft subsection with two draft units - subsection = self.create_subsection_with_units([self.unit_1, self.unit_2]) + subsection = self.create_subsection(entities=[self.unit_1, self.unit_2]) # Also create another unit that's not in the subsection at all: - other_unit, _ou_v1 = self.create_unit(title="A draft unit not in the subsection", key="unit:3") + other_unit = self.create_unit(title="A draft unit not in the subsection", key="unit:3") assert content_api.contains_unpublished_changes(subsection.pk) assert self.unit_1.versioning.published is None @@ -443,10 +320,10 @@ def test_no_publish_parent(self): Test that publishing a unit does NOT publish changes to its parent subsection """ # Create a draft subsection with two draft units - subsection = self.create_subsection_with_units([self.unit_1, self.unit_2]) + subsection = self.create_subsection(entities=[self.unit_1, self.unit_2]) assert subsection.versioning.has_unpublished_changes # Publish ONLY one of its child units - self.publish_unit(self.unit_1) + self.publish_container(self.unit_1) self.unit_1.refresh_from_db() # Clear cache on '.versioning' assert self.unit_1.versioning.has_unpublished_changes is False @@ -456,19 +333,20 @@ def test_no_publish_parent(self): assert subsection.versioning.published is None with pytest.raises(authoring_models.ContainerVersion.DoesNotExist): # There is no published version of the subsection: - content_api.get_units_in_subsection(subsection, published=True) + content_api.get_entities_in_container(subsection, published=True) def test_add_unit_after_publish(self): """ Adding a unit to a published subsection will create a new version and show that the subsection has unpublished changes. """ - subsection, subsection_version = content_api.create_subsection_and_version( + subsection, subsection_version = content_api.create_container_and_version( learning_package_id=self.learning_package.id, key="subsection:key", title="Subsection", created=self.now, created_by=None, + container_type=content_api.Subsection, ) assert subsection.versioning.draft == subsection_version assert subsection.versioning.published is None @@ -481,10 +359,10 @@ def test_add_unit_after_publish(self): # Add a published unit (unpinned): assert self.unit_1.versioning.has_unpublished_changes is False - subsection_version_v2 = content_api.create_next_subsection_version( - subsection=subsection, + subsection_version_v2 = content_api.create_next_container_version( + subsection.pk, title=subsection_version.title, - units=[self.unit_1], + entities=[self.unit_1], created=self.now, created_by=None, entities_action=content_api.ChildrenEntitiesAction.APPEND, @@ -505,7 +383,7 @@ def test_modify_unpinned_unit_after_publish(self): """ # Create a subsection with one unpinned draft unit: assert self.unit_1.versioning.has_unpublished_changes - subsection = self.create_subsection_with_units([self.unit_1]) + subsection = self.create_subsection(entities=[self.unit_1]) assert subsection.versioning.has_unpublished_changes # Publish the subsection and the unit: @@ -517,7 +395,7 @@ def test_modify_unpinned_unit_after_publish(self): assert self.unit_1.versioning.has_unpublished_changes is False # Now modify the unit by changing its title (it remains a draft): - unit_1_v2 = self.modify_unit(self.unit_1, title="Modified Counting Problem with new title") + unit_1_v2 = self.modify_container(self.unit_1, title="Modified Counting Problem with new title") # The unit now has unpublished changes; the subsection doesn't directly but does contain subsection.refresh_from_db() # Refresh to avoid stale 'versioning' cache @@ -527,19 +405,19 @@ def test_modify_unpinned_unit_after_publish(self): assert self.unit_1.versioning.has_unpublished_changes # Since the unit changes haven't been published, they should only appear in the draft subsection - assert content_api.get_units_in_subsection(subsection, published=False) == [ + assert content_api.get_entities_in_container(subsection, published=False) == [ Entry(unit_1_v2), # new version ] - assert content_api.get_units_in_subsection(subsection, published=True) == [ + assert content_api.get_entities_in_container(subsection, published=True) == [ Entry(self.unit_1_v1), # old version ] # But if we publish the unit, the changes will appear in the published version of the subsection. - self.publish_unit(self.unit_1) - assert content_api.get_units_in_subsection(subsection, published=False) == [ + self.publish_container(self.unit_1) + assert content_api.get_entities_in_container(subsection, published=False) == [ Entry(unit_1_v2), # new version ] - assert content_api.get_units_in_subsection(subsection, published=True) == [ + assert content_api.get_entities_in_container(subsection, published=True) == [ Entry(unit_1_v2), # new version ] assert content_api.contains_unpublished_changes(subsection.pk) is False # No more unpublished changes @@ -551,17 +429,17 @@ def test_modify_pinned_unit(self): which will continue to use the pinned version. """ # Create a subsection with one unit (pinned 📌 to v1): - subsection = self.create_subsection_with_units([self.unit_1_v1]) + subsection = self.create_subsection(entities=[self.unit_1_v1]) # Publish the subsection and the unit: content_api.publish_all_drafts(self.learning_package.id) expected_subsection_contents = [ Entry(self.unit_1_v1, pinned=True), # pinned 📌 to v1 ] - assert content_api.get_units_in_subsection(subsection, published=True) == expected_subsection_contents + assert content_api.get_entities_in_container(subsection, published=True) == expected_subsection_contents # Now modify the unit by changing its title (it remains a draft): - self.modify_unit(self.unit_1, title="Modified Counting Problem with new title") + self.modify_container(self.unit_1, title="Modified Counting Problem with new title") # The unit now has unpublished changes; the subsection is entirely unaffected subsection.refresh_from_db() # Refresh to avoid stale 'versioning' cache @@ -571,12 +449,12 @@ def test_modify_pinned_unit(self): assert self.unit_1.versioning.has_unpublished_changes is True # Neither the draft nor the published version of the subsection is affected - assert content_api.get_units_in_subsection(subsection, published=False) == expected_subsection_contents - assert content_api.get_units_in_subsection(subsection, published=True) == expected_subsection_contents + assert content_api.get_entities_in_container(subsection, published=False) == expected_subsection_contents + assert content_api.get_entities_in_container(subsection, published=True) == expected_subsection_contents # Even if we publish the unit, the subsection stays pinned to the specified version: - self.publish_unit(self.unit_1) - assert content_api.get_units_in_subsection(subsection, published=False) == expected_subsection_contents - assert content_api.get_units_in_subsection(subsection, published=True) == expected_subsection_contents + self.publish_container(self.unit_1) + assert content_api.get_entities_in_container(subsection, published=False) == expected_subsection_contents + assert content_api.get_entities_in_container(subsection, published=True) == expected_subsection_contents def test_create_two_subsections_with_same_units(self): """ @@ -584,44 +462,54 @@ def test_create_two_subsections_with_same_units(self): units in each subsection. """ # Create a subsection with unit 2 unpinned, unit 2 pinned 📌, and unit 1: - subsection1 = self.create_subsection_with_units([self.unit_2, self.unit_2_v1, self.unit_1], key="u1") + subsection1 = self.create_subsection(entities=[self.unit_2, self.unit_2_v1, self.unit_1], key="u1") # Create a second subsection with unit 1 pinned 📌, unit 2, and unit 1 unpinned: - subsection2 = self.create_subsection_with_units([self.unit_1_v1, self.unit_2, self.unit_1], key="u2") + subsection2 = self.create_subsection(entities=[self.unit_1_v1, self.unit_2, self.unit_1], key="u2") # Check that the contents are as expected: - assert [row.unit_version for row in content_api.get_units_in_subsection(subsection1, published=False)] == [ - self.unit_2_v1, self.unit_2_v1, self.unit_1_v1, + assert [ + row.entity_version.containerversion + for row in content_api.get_entities_in_container(subsection1, published=False) + ] == [ + self.unit_2_v1, + self.unit_2_v1, + self.unit_1_v1, ] - assert [row.unit_version for row in content_api.get_units_in_subsection(subsection2, published=False)] == [ - self.unit_1_v1, self.unit_2_v1, self.unit_1_v1, + assert [ + row.entity_version.containerversion + for row in content_api.get_entities_in_container(subsection2, published=False) + ] == [ + self.unit_1_v1, + self.unit_2_v1, + self.unit_1_v1, ] # Modify unit 1 - unit_1_v2 = self.modify_unit(self.unit_1, title="unit 1 v2") + unit_1_v2 = self.modify_container(self.unit_1, title="unit 1 v2") # Publish changes content_api.publish_all_drafts(self.learning_package.id) # Modify unit 2 - only in the draft - unit_2_v2 = self.modify_unit(self.unit_2, title="unit 2 DRAFT") + unit_2_v2 = self.modify_container(self.unit_2, title="unit 2 DRAFT") # Check that the draft contents are as expected: - assert content_api.get_units_in_subsection(subsection1, published=False) == [ + assert content_api.get_entities_in_container(subsection1, published=False) == [ Entry(unit_2_v2), # v2 in the draft version Entry(self.unit_2_v1, pinned=True), # pinned 📌 to v1 Entry(unit_1_v2), # v2 ] - assert content_api.get_units_in_subsection(subsection2, published=False) == [ + assert content_api.get_entities_in_container(subsection2, published=False) == [ Entry(self.unit_1_v1, pinned=True), # pinned 📌 to v1 Entry(unit_2_v2), # v2 in the draft version Entry(unit_1_v2), # v2 ] # Check that the published contents are as expected: - assert content_api.get_units_in_subsection(subsection1, published=True) == [ + assert content_api.get_entities_in_container(subsection1, published=True) == [ Entry(self.unit_2_v1), # v1 in the published version Entry(self.unit_2_v1, pinned=True), # pinned 📌 to v1 Entry(unit_1_v2), # v2 ] - assert content_api.get_units_in_subsection(subsection2, published=True) == [ + assert content_api.get_entities_in_container(subsection2, published=True) == [ Entry(self.unit_1_v1, pinned=True), # pinned 📌 to v1 Entry(self.unit_2_v1), # v1 in the published version Entry(unit_1_v2), # v2 @@ -638,16 +526,16 @@ def test_publishing_shared_unit(self): """ # 1️⃣ Create the subsections and publish them: (u1, u1_v1), (u2, _u2_v1), (u3, u3_v1), (u4, u4_v1), (u5, u5_v1) = [ - self.create_unit(key=f"C{i}", title=f"Unit {i}") for i in range(1, 6) + self.create_unit_and_version(key=f"C{i}", title=f"Unit {i}") for i in range(1, 6) ] - subsection1 = self.create_subsection_with_units([u1, u2, u3], title="Subsection 1", key="subsection:1") - subsection2 = self.create_subsection_with_units([u2, u4, u5], title="Subsection 2", key="subsection:2") + subsection1 = self.create_subsection(entities=[u1, u2, u3], title="Subsection 1", key="subsection:1") + subsection2 = self.create_subsection(entities=[u2, u4, u5], title="Subsection 2", key="subsection:2") content_api.publish_all_drafts(self.learning_package.id) assert content_api.contains_unpublished_changes(subsection1.pk) is False assert content_api.contains_unpublished_changes(subsection2.pk) is False # 2️⃣ Then the author edits U2 inside of Subsection 1 making U2v2. - u2_v2 = self.modify_unit(u2, title="U2 version 2") + u2_v2 = self.modify_container(u2, title="U2 version 2") # Both S1 and S2 now contain unpublished changes since they share the unit. assert content_api.contains_unpublished_changes(subsection1.pk) assert content_api.contains_unpublished_changes(subsection2.pk) @@ -658,7 +546,7 @@ def test_publishing_shared_unit(self): assert subsection2.versioning.has_unpublished_changes is False # 3️⃣ In addition to this, the author also modifies another unit in Subsection 2 (U5) - u5_v2 = self.modify_unit(u5, title="U5 version 2") + u5_v2 = self.modify_container(u5, title="U5 version 2") # 4️⃣ The author then publishes Subsection 1, and therefore everything in it. content_api.publish_from_drafts( @@ -670,7 +558,7 @@ def test_publishing_shared_unit(self): ) # Result: Subsection 1 will show the newly published version of U2: - assert content_api.get_units_in_subsection(subsection1, published=True) == [ + assert content_api.get_entities_in_container(subsection1, published=True) == [ Entry(u1_v1), Entry(u2_v2), # new published version of U2 Entry(u3_v1), @@ -679,7 +567,7 @@ def test_publishing_shared_unit(self): # Result: someone looking at Subsection 2 should see the newly published unit 2, because publishing it anywhere # publishes it everywhere. But publishing U2 and Subsection 1 does not affect the other units in Subsection 2. # (Publish propagates downward, not upward) - assert content_api.get_units_in_subsection(subsection2, published=True) == [ + assert content_api.get_entities_in_container(subsection2, published=True) == [ Entry(u2_v2), # new published version of U2 Entry(u4_v1), # still original version of U4 (it was never modified) Entry(u5_v1), # still original version of U5 (it hasn't been published) @@ -690,9 +578,9 @@ def test_publishing_shared_unit(self): assert content_api.contains_unpublished_changes(subsection2.pk) # 5️⃣ Publish unit U5, which should be the only thing unpublished in the learning package - self.publish_unit(u5) + self.publish_container(u5) # Result: Subsection 2 shows the new version of C5 and no longer contains unpublished changes: - assert content_api.get_units_in_subsection(subsection2, published=True) == [ + assert content_api.get_entities_in_container(subsection2, published=True) == [ Entry(u2_v2), # new published version of U2 Entry(u4_v1), # still original version of U4 (it was never modified) Entry(u5_v2), # new published version of U5 @@ -708,19 +596,19 @@ def test_query_count_of_contains_unpublished_changes(self): unit_count = 2 units = [] for i in range(unit_count): - unit, _version = self.create_unit( + unit, _version = self.create_unit_and_version( key=f"Unit {i}", title=f"Unit {i}", ) units.append(unit) - subsection = self.create_subsection_with_units(units) + subsection = self.create_subsection(entities=units) content_api.publish_all_drafts(self.learning_package.id) subsection.refresh_from_db() with self.assertNumQueries(1): assert content_api.contains_unpublished_changes(subsection.pk) is False # Modify the most recently created unit: - self.modify_unit(unit, title="Modified Unit") + self.modify_container(unit, title="Modified Unit") with self.assertNumQueries(1): assert content_api.contains_unpublished_changes(subsection.pk) is True @@ -730,12 +618,12 @@ def test_metadata_change_doesnt_create_entity_list(self): version, but can re-use the same EntityList. API consumers generally shouldn't depend on this behavior; it's an optimization. """ - subsection = self.create_subsection_with_units([self.unit_1, self.unit_2_v1]) + subsection = self.create_subsection(entities=[self.unit_1, self.unit_2_v1]) orig_version_num = subsection.versioning.draft.version_num orig_entity_list_id = subsection.versioning.draft.entity_list.pk - content_api.create_next_subsection_version(subsection, title="New Title", created=self.now) + content_api.create_next_container_version(subsection, title="New Title", created=self.now, created_by=None) subsection.refresh_from_db() new_version_num = subsection.versioning.draft.version_num @@ -762,31 +650,32 @@ def test_cannot_add_soft_deleted_unit(self, publish_first): content_api.soft_delete_draft(unit.pk) # Now try adding that unit to a subsection: with pytest.raises(ValidationError, match="unit is deleted"): - self.create_subsection_with_units([unit]) + self.create_subsection(entities=[unit]) def test_removing_unit(self): - """ Test removing a unit from a subsection (but not deleting it) """ - subsection = self.create_subsection_with_units([self.unit_1, self.unit_2]) + """Test removing a unit from a subsection (but not deleting it)""" + subsection = self.create_subsection(entities=[self.unit_1, self.unit_2]) content_api.publish_all_drafts(self.learning_package.id) # Now remove unit 2 - content_api.create_next_subsection_version( - subsection=subsection, + content_api.create_next_container_version( + subsection.pk, title="Revised with unit 2 deleted", - units=[self.unit_2], + entities=[self.unit_2], created=self.now, + created_by=None, entities_action=content_api.ChildrenEntitiesAction.REMOVE, ) # Now it should not be listed in the subsection: - assert content_api.get_units_in_subsection(subsection, published=False) == [ + assert content_api.get_entities_in_container(subsection, published=False) == [ Entry(self.unit_1_v1), ] subsection.refresh_from_db() assert subsection.versioning.has_unpublished_changes # The subsection itself and its unit list have change assert content_api.contains_unpublished_changes(subsection.pk) # The published version of the subsection is not yet affected: - assert content_api.get_units_in_subsection(subsection, published=True) == [ + assert content_api.get_entities_in_container(subsection, published=True) == [ Entry(self.unit_1_v1), Entry(self.unit_2_v1), ] @@ -799,20 +688,20 @@ def test_removing_unit(self): # but that would involve additional database lookup(s). subsection.refresh_from_db() assert content_api.contains_unpublished_changes(subsection.pk) is False - assert content_api.get_units_in_subsection(subsection, published=True) == [ + assert content_api.get_entities_in_container(subsection, published=True) == [ Entry(self.unit_1_v1), ] def test_soft_deleting_unit(self): - """ Test soft deleting a unit that's in a subsection (but not removing it) """ - subsection = self.create_subsection_with_units([self.unit_1, self.unit_2]) + """Test soft deleting a unit that's in a subsection (but not removing it)""" + subsection = self.create_subsection(entities=[self.unit_1, self.unit_2]) content_api.publish_all_drafts(self.learning_package.id) # Now soft delete unit 2 content_api.soft_delete_draft(self.unit_2.pk) # Now it should not be listed in the subsection: - assert content_api.get_units_in_subsection(subsection, published=False) == [ + assert content_api.get_entities_in_container(subsection, published=False) == [ Entry(self.unit_1_v1), # unit 2 is soft deleted from the draft. # TODO: should we return some kind of placeholder here, to indicate that a unit is still listed in the @@ -822,7 +711,7 @@ def test_soft_deleting_unit(self): assert subsection.versioning.has_unpublished_changes is False # Subsection and unit list unchanged assert content_api.contains_unpublished_changes(subsection.pk) # It still contains an unpublished deletion # The published version of the subsection is not yet affected: - assert content_api.get_units_in_subsection(subsection, published=True) == [ + assert content_api.get_entities_in_container(subsection, published=True) == [ Entry(self.unit_1_v1), Entry(self.unit_2_v1), ] @@ -830,34 +719,35 @@ def test_soft_deleting_unit(self): # But when we publish the deletion, the published version is affected: content_api.publish_all_drafts(self.learning_package.id) assert content_api.contains_unpublished_changes(subsection.pk) is False - assert content_api.get_units_in_subsection(subsection, published=True) == [ + assert content_api.get_entities_in_container(subsection, published=True) == [ Entry(self.unit_1_v1), ] def test_soft_deleting_and_removing_unit(self): - """ Test soft deleting a unit that's in a subsection AND removing it """ - subsection = self.create_subsection_with_units([self.unit_1, self.unit_2]) + """Test soft deleting a unit that's in a subsection AND removing it""" + subsection = self.create_subsection(entities=[self.unit_1, self.unit_2]) content_api.publish_all_drafts(self.learning_package.id) # Now soft delete unit 2 content_api.soft_delete_draft(self.unit_2.pk) # And remove it from the subsection: - content_api.create_next_subsection_version( - subsection=subsection, + content_api.create_next_container_version( + subsection.pk, title="Revised with unit 2 deleted", - units=[self.unit_2], + entities=[self.unit_2], created=self.now, + created_by=None, entities_action=content_api.ChildrenEntitiesAction.REMOVE, ) # Now it should not be listed in the subsection: - assert content_api.get_units_in_subsection(subsection, published=False) == [ + assert content_api.get_entities_in_container(subsection, published=False) == [ Entry(self.unit_1_v1), ] assert subsection.versioning.has_unpublished_changes is True assert content_api.contains_unpublished_changes(subsection.pk) # The published version of the subsection is not yet affected: - assert content_api.get_units_in_subsection(subsection, published=True) == [ + assert content_api.get_entities_in_container(subsection, published=True) == [ Entry(self.unit_1_v1), Entry(self.unit_2_v1), ] @@ -865,27 +755,27 @@ def test_soft_deleting_and_removing_unit(self): # But when we publish the deletion, the published version is affected: content_api.publish_all_drafts(self.learning_package.id) assert content_api.contains_unpublished_changes(subsection.pk) is False - assert content_api.get_units_in_subsection(subsection, published=True) == [ + assert content_api.get_entities_in_container(subsection, published=True) == [ Entry(self.unit_1_v1), ] def test_soft_deleting_pinned_unit(self): - """ Test soft deleting a pinned 📌 unit that's in a subsection """ - subsection = self.create_subsection_with_units([self.unit_1_v1, self.unit_2_v1]) + """Test soft deleting a pinned 📌 unit that's in a subsection""" + subsection = self.create_subsection(entities=[self.unit_1_v1, self.unit_2_v1]) content_api.publish_all_drafts(self.learning_package.id) # Now soft delete unit 2 content_api.soft_delete_draft(self.unit_2.pk) # Now it should still be listed in the subsection: - assert content_api.get_units_in_subsection(subsection, published=False) == [ + assert content_api.get_entities_in_container(subsection, published=False) == [ Entry(self.unit_1_v1, pinned=True), Entry(self.unit_2_v1, pinned=True), ] assert subsection.versioning.has_unpublished_changes is False # Subsection and unit list unchanged assert content_api.contains_unpublished_changes(subsection.pk) is False # nor does it contain changes # The published version of the subsection is also not affected: - assert content_api.get_units_in_subsection(subsection, published=True) == [ + assert content_api.get_entities_in_container(subsection, published=True) == [ Entry(self.unit_1_v1, pinned=True), Entry(self.unit_2_v1, pinned=True), ] @@ -897,8 +787,8 @@ def test_soft_delete_subsection(self): See https://github.com/openedx/frontend-app-authoring/issues/1693 """ # Create two subsections, one of which we will soon delete: - subsection_to_delete = self.create_subsection_with_units([self.unit_1, self.unit_2]) - other_subsection = self.create_subsection_with_units([self.unit_1], key="other") + subsection_to_delete = self.create_subsection(entities=[self.unit_1, self.unit_2]) + other_subsection = self.create_subsection(entities=[self.unit_1], key="other") # Publish everything: content_api.publish_all_drafts(self.learning_package.id) @@ -910,7 +800,7 @@ def test_soft_delete_subsection(self): assert subsection_to_delete.versioning.published is not None self.unit_1.refresh_from_db() assert self.unit_1.versioning.draft is not None - assert content_api.get_units_in_subsection(other_subsection, published=False) == [Entry(self.unit_1_v1)] + assert content_api.get_entities_in_container(other_subsection, published=False) == [Entry(self.unit_1_v1)] # Publish everything: content_api.publish_all_drafts(self.learning_package.id) @@ -921,8 +811,8 @@ def test_soft_delete_subsection(self): self.unit_1.refresh_from_db() assert self.unit_1.versioning.draft is not None assert self.unit_1.versioning.published is not None - assert content_api.get_units_in_subsection(other_subsection, published=False) == [Entry(self.unit_1_v1)] - assert content_api.get_units_in_subsection(other_subsection, published=True) == [Entry(self.unit_1_v1)] + assert content_api.get_entities_in_container(other_subsection, published=False) == [Entry(self.unit_1_v1)] + assert content_api.get_entities_in_container(other_subsection, published=True) == [Entry(self.unit_1_v1)] def test_snapshots_of_published_subsection(self): """ @@ -930,9 +820,9 @@ def test_snapshots_of_published_subsection(self): subsections and their contents. """ # At first the subsection has one unit (unpinned): - subsection = self.create_subsection_with_units([self.unit_1]) - self.modify_unit(self.unit_1, title="Unit 1 as of checkpoint 1") - before_publish = content_api.get_units_in_published_subsection_as_of(subsection, 0) + subsection = self.create_subsection(entities=[self.unit_1]) + self.modify_container(self.unit_1, title="Unit 1 as of checkpoint 1") + before_publish, _ = content_api.get_entities_in_container_as_of(subsection, 0) assert before_publish is None # Publish everything, creating Checkpoint 1 @@ -941,19 +831,20 @@ def test_snapshots_of_published_subsection(self): ######################################################################## # Now we update the title of the unit. - self.modify_unit(self.unit_1, title="Unit 1 as of checkpoint 2") + self.modify_container(self.unit_1, title="Unit 1 as of checkpoint 2") # Publish everything, creating Checkpoint 2 checkpoint_2 = content_api.publish_all_drafts(self.learning_package.id, message="checkpoint 2") ######################################################################## # Now add a second unit to the subsection: - self.modify_unit(self.unit_1, title="Unit 1 as of checkpoint 3") - self.modify_unit(self.unit_2, title="Unit 2 as of checkpoint 3") - content_api.create_next_subsection_version( - subsection=subsection, + self.modify_container(self.unit_1, title="Unit 1 as of checkpoint 3") + self.modify_container(self.unit_2, title="Unit 2 as of checkpoint 3") + content_api.create_next_container_version( + subsection.pk, title="Subsection title in checkpoint 3", - units=[self.unit_1, self.unit_2], + entities=[self.unit_1, self.unit_2], created=self.now, + created_by=None, ) # Publish everything, creating Checkpoint 3 checkpoint_3 = content_api.publish_all_drafts(self.learning_package.id, message="checkpoint 3") @@ -961,36 +852,37 @@ def test_snapshots_of_published_subsection(self): # Now add a third unit to the subsection, a pinned 📌 version of unit 1. # This will test pinned versions and also test adding at the beginning rather than the end of the subsection. - content_api.create_next_subsection_version( - subsection=subsection, + content_api.create_next_container_version( + subsection.pk, title="Subsection title in checkpoint 4", - units=[self.unit_1_v1, self.unit_1, self.unit_2], + entities=[self.unit_1_v1, self.unit_1, self.unit_2], created=self.now, + created_by=None, ) # Publish everything, creating Checkpoint 4 checkpoint_4 = content_api.publish_all_drafts(self.learning_package.id, message="checkpoint 4") ######################################################################## # Modify the drafts, but don't publish: - self.modify_unit(self.unit_1, title="Unit 1 draft") - self.modify_unit(self.unit_2, title="Unit 2 draft") + self.modify_container(self.unit_1, title="Unit 1 draft") + self.modify_container(self.unit_2, title="Unit 2 draft") # Now fetch the snapshots: - as_of_checkpoint_1 = content_api.get_units_in_published_subsection_as_of(subsection, checkpoint_1.pk) - assert [cv.unit_version.title for cv in as_of_checkpoint_1] == [ + _, as_of_checkpoint_1 = content_api.get_entities_in_container_as_of(subsection, checkpoint_1.pk) + assert [ev.entity_version.title for ev in as_of_checkpoint_1] == [ "Unit 1 as of checkpoint 1", ] - as_of_checkpoint_2 = content_api.get_units_in_published_subsection_as_of(subsection, checkpoint_2.pk) - assert [cv.unit_version.title for cv in as_of_checkpoint_2] == [ + _, as_of_checkpoint_2 = content_api.get_entities_in_container_as_of(subsection, checkpoint_2.pk) + assert [ev.entity_version.title for ev in as_of_checkpoint_2] == [ "Unit 1 as of checkpoint 2", ] - as_of_checkpoint_3 = content_api.get_units_in_published_subsection_as_of(subsection, checkpoint_3.pk) - assert [cv.unit_version.title for cv in as_of_checkpoint_3] == [ + _, as_of_checkpoint_3 = content_api.get_entities_in_container_as_of(subsection, checkpoint_3.pk) + assert [ev.entity_version.title for ev in as_of_checkpoint_3] == [ "Unit 1 as of checkpoint 3", "Unit 2 as of checkpoint 3", ] - as_of_checkpoint_4 = content_api.get_units_in_published_subsection_as_of(subsection, checkpoint_4.pk) - assert [cv.unit_version.title for cv in as_of_checkpoint_4] == [ + _, as_of_checkpoint_4 = content_api.get_entities_in_container_as_of(subsection, checkpoint_4.pk) + assert [ev.entity_version.title for ev in as_of_checkpoint_4] == [ "Unit (1)", # Pinned. This title is self.unit_1_v1.title (original v1 title) "Unit 1 as of checkpoint 3", # we didn't modify these units so they're same as in snapshot 3 "Unit 2 as of checkpoint 3", # we didn't modify these units so they're same as in snapshot 3 @@ -1001,32 +893,34 @@ def test_subsections_containing(self): Test that we can efficiently get a list of all the [draft] subsections containing a given unit. """ - unit_1_v2 = self.modify_unit(self.unit_1, title="modified unit 1") + unit_1_v2 = self.modify_container(self.unit_1, title="modified unit 1") # Create a few subsections, some of which contain unit 1 and others which don't: # Note: it is important that some of these subsections contain other units, to ensure the complex JOINs required # for this query are working correctly, especially in the case of ignore_pinned=True. # Subsection 1 ✅ has unit 1, pinned 📌 to V1 - subsection1_1pinned = self.create_subsection_with_units([self.unit_1_v1, self.unit_2], key="u1") + subsection1_1pinned = self.create_subsection(entities=[self.unit_1_v1, self.unit_2], key="u1") # Subsection 2 ✅ has unit 1, pinned 📌 to V2 - subsection2_1pinned_v2 = self.create_subsection_with_units([unit_1_v2, self.unit_2_v1], key="u2") + subsection2_1pinned_v2 = self.create_subsection(entities=[unit_1_v2, self.unit_2_v1], key="u2") # Subsection 3 doesn't contain it - _subsection3_no = self.create_subsection_with_units([self.unit_2], key="u3") + _subsection3_no = self.create_subsection(entities=[self.unit_2], key="u3") # Subsection 4 ✅ has unit 1, unpinned - subsection4_unpinned = self.create_subsection_with_units([ - self.unit_1, self.unit_2, self.unit_2_v1, - ], key="u4") + subsection4_unpinned = self.create_subsection(entities= + [ + self.unit_1, + self.unit_2, + self.unit_2_v1, + ], + key="u4", + ) # Subsections 5/6 don't contain it - _subsection5_no = self.create_subsection_with_units([self.unit_2_v1, self.unit_2], key="u5") - _subsection6_no = self.create_subsection_with_units([], key="u6") + _subsection5_no = self.create_subsection(entities=[self.unit_2_v1, self.unit_2], key="u5") + _subsection6_no = self.create_subsection(entities=[], key="u6") # No need to publish anything as the get_containers_with_entity() API only considers drafts (for now). with self.assertNumQueries(1): - result = [ - c.subsection for c in - content_api.get_containers_with_entity(self.unit_1.pk).select_related("subsection") - ] + result = list(content_api.get_containers_with_entity(self.unit_1.pk)) assert result == [ subsection1_1pinned, subsection2_1pinned_v2, @@ -1037,34 +931,30 @@ def test_subsections_containing(self): # about pinned uses anyways (they would be unaffected by a delete). with self.assertNumQueries(1): - result2 = [ - c.subsection for c in - content_api.get_containers_with_entity( - self.unit_1.pk, ignore_pinned=True - ).select_related("subsection") - ] + result2 = list(content_api.get_containers_with_entity(self.unit_1.pk, ignore_pinned=True)) assert result2 == [subsection4_unpinned] - def test_get_units_in_subsection_queries(self): + def test_get_entities_in_container_queries(self): """ - Test the query count of get_units_in_subsection() - This also tests the generic method get_entities_in_container() + Test the query count of get_entities_in_container() """ - subsection = self.create_subsection_with_units([ - self.unit_1, - self.unit_2, - self.unit_2_v1, - ]) - with self.assertNumQueries(4): - result = content_api.get_units_in_subsection(subsection, published=False) + subsection = self.create_subsection(entities= + [ + self.unit_1, + self.unit_2, + self.unit_2_v1, + ] + ) + with self.assertNumQueries(2): + result = content_api.get_entities_in_container(subsection, published=False) assert result == [ Entry(self.unit_1.versioning.draft), Entry(self.unit_2.versioning.draft), Entry(self.unit_2.versioning.draft, pinned=True), ] content_api.publish_all_drafts(self.learning_package.id) - with self.assertNumQueries(4): - result = content_api.get_units_in_subsection(subsection, published=True) + with self.assertNumQueries(2): + result = content_api.get_entities_in_container(subsection, published=True) assert result == [ Entry(self.unit_1.versioning.draft), Entry(self.unit_2.versioning.draft), @@ -1075,26 +965,24 @@ def test_add_remove_container_children(self): """ Test adding and removing children units from subsections. """ - subsection, subsection_version = content_api.create_subsection_and_version( + subsection, subsection_version = content_api.create_container_and_version( learning_package_id=self.learning_package.id, key="subsection:key", title="Subsection", - units=[self.unit_1], + entities=[self.unit_1], created=self.now, created_by=None, + container_type=content_api.Subsection, ) - assert content_api.get_units_in_subsection(subsection, published=False) == [ + assert content_api.get_entities_in_container(subsection, published=False) == [ Entry(self.unit_1.versioning.draft), ] - unit_3, _ = self.create_unit( - key="Unit (3)", - title="Unit (3)", - ) + unit_3 = self.create_unit(key="Unit (3)", title="Unit (3)") # Add unit_2 and unit_3 - subsection_version_v2 = content_api.create_next_subsection_version( - subsection=subsection, + subsection_version_v2 = content_api.create_next_container_version( + subsection.pk, title=subsection_version.title, - units=[self.unit_2, unit_3], + entities=[self.unit_2, unit_3], created=self.now, created_by=None, entities_action=content_api.ChildrenEntitiesAction.APPEND, @@ -1103,24 +991,24 @@ def test_add_remove_container_children(self): assert subsection_version_v2.version_num == 2 assert subsection_version_v2 in subsection.versioning.versions.all() # Verify that unit_2 and unit_3 is added to end - assert content_api.get_units_in_subsection(subsection, published=False) == [ + assert content_api.get_entities_in_container(subsection, published=False) == [ Entry(self.unit_1.versioning.draft), Entry(self.unit_2.versioning.draft), Entry(unit_3.versioning.draft), ] # Remove unit_1 - content_api.create_next_subsection_version( - subsection=subsection, + content_api.create_next_container_version( + subsection.pk, title=subsection_version.title, - units=[self.unit_1], + entities=[self.unit_1], created=self.now, created_by=None, entities_action=content_api.ChildrenEntitiesAction.REMOVE, ) subsection.refresh_from_db() # Verify that unit_1 is removed - assert content_api.get_units_in_subsection(subsection, published=False) == [ + assert content_api.get_entities_in_container(subsection, published=False) == [ Entry(self.unit_2.versioning.draft), Entry(unit_3.versioning.draft), ] @@ -1129,35 +1017,35 @@ def test_get_container_children_count(self): """ Test get_container_children_count() """ - subsection = self.create_subsection_with_units([self.unit_1]) - assert content_api.get_container_children_count(subsection.container, published=False) == 1 + subsection = self.create_subsection(entities=[self.unit_1]) + assert content_api.get_container_children_count(subsection, published=False) == 1 # publish content_api.publish_all_drafts(self.learning_package.id) subsection_version = subsection.versioning.draft - content_api.create_next_subsection_version( - subsection=subsection, + content_api.create_next_container_version( + subsection.pk, title=subsection_version.title, - units=[self.unit_2], + entities=[self.unit_2], created=self.now, created_by=None, entities_action=content_api.ChildrenEntitiesAction.APPEND, ) subsection.refresh_from_db() # Should have two units in draft version and 1 in published version - assert content_api.get_container_children_count(subsection.container, published=False) == 2 - assert content_api.get_container_children_count(subsection.container, published=True) == 1 + assert content_api.get_container_children_count(subsection, published=False) == 2 + assert content_api.get_container_children_count(subsection, published=True) == 1 # publish content_api.publish_all_drafts(self.learning_package.id) subsection.refresh_from_db() - assert content_api.get_container_children_count(subsection.container, published=True) == 2 + assert content_api.get_container_children_count(subsection, published=True) == 2 # Soft delete unit_1 content_api.soft_delete_draft(self.unit_1.pk) subsection.refresh_from_db() # Should contain only 1 child - assert content_api.get_container_children_count(subsection.container, published=False) == 1 + assert content_api.get_container_children_count(subsection, published=False) == 1 content_api.publish_all_drafts(self.learning_package.id) subsection.refresh_from_db() - assert content_api.get_container_children_count(subsection.container, published=True) == 1 + assert content_api.get_container_children_count(subsection, published=True) == 1 # Tests TODO: # Test that I can get a [PublishLog] history of a given subsection and all its children, including children diff --git a/tests/openedx_content/applets/units/test_api.py b/tests/openedx_content/applets/units/test_api.py index 588d9d839..02815a6b7 100644 --- a/tests/openedx_content/applets/units/test_api.py +++ b/tests/openedx_content/applets/units/test_api.py @@ -1,24 +1,20 @@ """ Basic tests for the units API. """ -from unittest.mock import patch import ddt # type: ignore[import] import pytest from django.core.exceptions import ValidationError -from django.db import IntegrityError import openedx_content.api as content_api from openedx_content import models_api as authoring_models -from ..components.test_api import ComponentTestCase - -Entry = content_api.UnitListEntry +from ..publishing.container_test_case import ContainerTestCase, Entry @ddt.ddt -class UnitTestCase(ComponentTestCase): - """ Test cases for Units (containers of components) """ +class SubSectionTestCase(ContainerTestCase): + """Test cases for Units (containers of components)""" def setUp(self) -> None: super().setUp() @@ -31,95 +27,36 @@ def setUp(self) -> None: title="Querying Counting Problem (2)", ) - def create_component(self, *, title: str = "Test Component", key: str = "component:1") -> tuple[ - authoring_models.Component, authoring_models.ComponentVersion - ]: - """ Helper method to quickly create a component """ - return content_api.create_component_and_version( - self.learning_package.id, - component_type=self.problem_type, - local_key=key, - title=title, - created=self.now, - created_by=None, - ) - - def create_unit_with_components( - self, - components: list[authoring_models.Component | authoring_models.ComponentVersion], - *, - title="Unit", - key="unit:key", - ) -> authoring_models.Unit: - """ Helper method to quickly create a unit with some components """ - unit, _unit_v1 = content_api.create_unit_and_version( - learning_package_id=self.learning_package.id, - key=key, - title=title, - components=components, - created=self.now, - created_by=None, - ) - return unit - - def modify_component( - self, - component: authoring_models.Component, - *, - title="Modified Component", - timestamp=None, - ) -> authoring_models.ComponentVersion: - """ - Helper method to modify a component for the purposes of testing units/drafts/pinning/publishing/etc. - """ - return content_api.create_next_component_version( - component.pk, - media_to_replace={}, - title=title, - created=timestamp or self.now, - created_by=None, - ) - - def test_get_unit(self): + def test_get_container(self): """ - Test get_unit() + Test get_container() """ - unit = self.create_unit_with_components([self.component_1, self.component_2]) + unit = self.create_unit(entities=[self.component_1, self.component_2]) with self.assertNumQueries(1): - result = content_api.get_unit(unit.pk) + result = content_api.get_container(unit.pk) assert result == unit # Versioning data should be pre-loaded via select_related() with self.assertNumQueries(0): assert result.versioning.has_unpublished_changes - def test_get_unit_version(self): + def test_get_container_version(self): """ - Test get_unit_version() + Test get_container_version() """ - unit = self.create_unit_with_components([]) + unit = self.create_unit(entities=[]) draft = unit.versioning.draft with self.assertNumQueries(1): - result = content_api.get_unit_version(draft.pk) - assert result == draft - - def test_get_latest_unit_version(self): - """ - Test test_get_latest_unit_version() - """ - unit = self.create_unit_with_components([]) - draft = unit.versioning.draft - with self.assertNumQueries(2): - result = content_api.get_latest_unit_version(unit.pk) + result = content_api.get_container_version(draft.pk) assert result == draft def test_get_containers(self): """ Test get_containers() """ - unit = self.create_unit_with_components([]) + unit = self.create_unit(entities=[]) with self.assertNumQueries(1): result = list(content_api.get_containers(self.learning_package.id)) - assert result == [unit.container] + assert result == [unit] # Versioning data should be pre-loaded via select_related() with self.assertNumQueries(0): assert result[0].versioning.has_unpublished_changes @@ -128,68 +65,41 @@ def test_get_containers_deleted(self): """ Test that get_containers() does not return soft-deleted units. """ - unit = self.create_unit_with_components([]) + unit = self.create_unit(entities=[]) content_api.soft_delete_draft(unit.pk) with self.assertNumQueries(1): result = list(content_api.get_containers(self.learning_package.id, include_deleted=True)) - assert result == [unit.container] + assert result == [unit] with self.assertNumQueries(1): result = list(content_api.get_containers(self.learning_package.id)) assert not result - def test_get_container(self): - """ - Test get_container() - """ - unit = self.create_unit_with_components([self.component_1, self.component_2]) - with self.assertNumQueries(1): - result = content_api.get_container(unit.pk) - assert result == unit.container - # Versioning data should be pre-loaded via select_related() - with self.assertNumQueries(0): - assert result.versioning.has_unpublished_changes - def test_get_container_by_key(self): """ Test get_container_by_key() """ - unit = self.create_unit_with_components([]) + unit = self.create_unit(entities=[]) with self.assertNumQueries(1): result = content_api.get_container_by_key( self.learning_package.id, key=unit.publishable_entity.key, ) - assert result == unit.container + assert result == unit # Versioning data should be pre-loaded via select_related() with self.assertNumQueries(0): assert result.versioning.has_unpublished_changes - def test_unit_container_versioning(self): - """ - Test that the .versioning helper of a Unit returns a UnitVersion, and - same for the generic Container equivalent. - """ - unit = self.create_unit_with_components([self.component_1, self.component_2]) - container = unit.container - container_version = container.versioning.draft - assert isinstance(container_version, authoring_models.ContainerVersion) - unit_version = unit.versioning.draft - assert isinstance(unit_version, authoring_models.UnitVersion) - assert unit_version.container_version == container_version - assert unit_version.container_version.container == container - assert unit_version.unit == unit - def test_create_unit_queries(self): """ Test how many database queries are required to create a unit """ # The exact numbers here aren't too important - this is just to alert us if anything significant changes. - with self.assertNumQueries(26): - _empty_unit = self.create_unit_with_components([]) - with self.assertNumQueries(32): + with self.assertNumQueries(23): + _empty_unit = self.create_unit(entities=[]) + with self.assertNumQueries(31): # And try with a non-empty unit: - self.create_unit_with_components([self.component_1, self.component_2_v1], key="u2") + self.create_unit(entities=[self.component_1, self.component_2_v1], key="u2") def test_create_unit_with_invalid_children(self): """ @@ -197,33 +107,27 @@ def test_create_unit_with_invalid_children(self): exception is raised. """ # Create two units: - unit, unit_version = content_api.create_unit_and_version( - learning_package_id=self.learning_package.id, + unit, unit_version = self.create_unit_and_version( key="unit:key", title="Unit", - created=self.now, - created_by=None, ) assert unit.versioning.draft == unit_version - unit2, _u2v1 = content_api.create_unit_and_version( - learning_package_id=self.learning_package.id, + unit2 = self.create_unit( key="unit:key2", title="Unit 2", - created=self.now, - created_by=None, ) # Try adding a Unit to a Unit - with pytest.raises(TypeError, match="Unit components must be either Component or ComponentVersion."): - content_api.create_next_unit_version( - unit=unit, + with pytest.raises(ValidationError, match='The entity "unit:key2" cannot be added to a "unit" container.'): + content_api.create_next_container_version( + container_pk=unit.pk, title="Unit Containing a Unit", - components=[unit2], + entities=[unit2], created=self.now, created_by=None, ) # Check that a new version was not created: unit.refresh_from_db() - assert content_api.get_unit(unit.pk).versioning.draft == unit_version + assert content_api.get_container(unit.pk).versioning.draft == unit_version assert unit.versioning.draft == unit_version def test_adding_external_components(self): @@ -232,58 +136,49 @@ def test_adding_external_components(self): unit. """ learning_package2 = content_api.create_learning_package(key="other-package", title="Other Package") - unit, _unit_version = content_api.create_unit_and_version( + unit, _version = content_api.create_container_and_version( learning_package_id=learning_package2.pk, key="unit:key", title="Unit", created=self.now, created_by=None, + container_type=content_api.Unit, ) assert self.component_1.learning_package != learning_package2 # Try adding a a component from LP 1 (self.learning_package) to a unit from LP 2 with pytest.raises(ValidationError, match="Container entities must be from the same learning package."): - content_api.create_next_unit_version( - unit=unit, + content_api.create_next_container_version( + unit.pk, title="Unit Containing an External Component", - components=[self.component_1], + entities=[self.component_1], created=self.now, created_by=None, ) - @patch('openedx_content.applets.units.api._pub_entities_for_components') - def test_adding_mismatched_versions(self, mock_entities_for_components): + def test_add_deleted_component(self): """ - Test that versioned components must match their entities. + Test adding a deleted component. + Mostly this checks that the exception thrown is reasonable. """ - mock_entities_for_components.return_value = [ - content_api.ContainerEntityRow( - entity_pk=self.component_1.pk, - version_pk=self.component_2_v1.pk, - ), - ] - # Try adding a a component from LP 1 (self.learning_package) to a unit from LP 2 - with pytest.raises(ValidationError, match="Container entity versions must belong to the specified entity"): - content_api.create_unit_and_version( - learning_package_id=self.component_1.learning_package.id, - key="unit:key", - title="Unit", - components=[self.component_1], - created=self.now, - created_by=None, - ) + self.component_1.delete() + with pytest.raises(authoring_models.Component.DoesNotExist): + self.create_unit(entities=[self.component_1]) - @ddt.data(True, False) - def test_cannot_add_invalid_ids(self, pin_version): + def test_add_corrupted_component(self): """ - Test that non-existent components cannot be added to units + Test adding a corrupted component (partially deleted) + Mostly this checks that the exception thrown is reasonable. """ self.component_1.delete() - if pin_version: - components = [self.component_1_v1] - else: - components = [self.component_1] - with pytest.raises((IntegrityError, authoring_models.Component.DoesNotExist)): - self.create_unit_with_components(components) + # Note the PublishableEntity and PublishableEntityVersion still exist, so this is a weird state: + self.component_1_v1.publishable_entity_version.refresh_from_db() # No error + self.component_1_v1.publishable_entity_version.entity.refresh_from_db() # No error + # Now add this corrupted component, pinned to v1: + with pytest.raises( + ValidationError, + match='The entity "xblock.v1:problem:Query Counting" cannot be added to a "unit" container.', + ): + self.create_unit(entities=[self.component_1_v1]) def test_create_empty_unit_and_version(self): """Test creating a unit with no components. @@ -294,12 +189,9 @@ def test_create_empty_unit_and_version(self): 3. The unit is a draft with unpublished changes. 4. There is no published version of the unit. """ - unit, unit_version = content_api.create_unit_and_version( - learning_package_id=self.learning_package.id, + unit, unit_version = self.create_unit_and_version( key="unit:key", title="Unit", - created=self.now, - created_by=None, ) assert unit, unit_version assert unit_version.version_num == 1 @@ -318,73 +210,64 @@ def test_create_next_unit_version_with_two_unpinned_components(self): 3. The unit version is in the unit's versions. 4. The components are in the draft unit version's component list and are unpinned. """ - unit, _unit_version = content_api.create_unit_and_version( - learning_package_id=self.learning_package.id, + unit = self.create_unit( key="unit:key", title="Unit", - created=self.now, - created_by=None, ) - unit_version_v2 = content_api.create_next_unit_version( - unit=unit, + unit_version_v2 = content_api.create_next_container_version( + unit.pk, title="Unit", - components=[self.component_1, self.component_2], + entities=[self.component_1, self.component_2], created=self.now, created_by=None, ) assert unit_version_v2.version_num == 2 assert unit_version_v2 in unit.versioning.versions.all() - assert content_api.get_components_in_unit(unit, published=False) == [ + assert content_api.get_entities_in_container(unit, published=False) == [ Entry(self.component_1.versioning.draft), Entry(self.component_2.versioning.draft), ] with pytest.raises(authoring_models.ContainerVersion.DoesNotExist): # There is no published version of the unit: - content_api.get_components_in_unit(unit, published=True) + content_api.get_entities_in_container(unit, published=True) def test_create_next_unit_version_with_unpinned_and_pinned_components(self): """ Test creating a unit version with one unpinned and one pinned 📌 component. """ - unit, _unit_version = content_api.create_unit_and_version( - learning_package_id=self.learning_package.id, + unit, _unit_version = self.create_unit_and_version( key="unit:key", title="Unit", - created=self.now, - created_by=None, ) - unit_version_v2 = content_api.create_next_unit_version( - unit=unit, + unit_version_v2 = content_api.create_next_container_version( + unit.pk, title="Unit", - components=[self.component_1, self.component_2_v1], # Note the "v1" pinning 📌 the second one to version 1 + entities=[self.component_1, self.component_2_v1], # Note the "v1" pinning 📌 the second one to version 1 created=self.now, created_by=None, ) assert unit_version_v2.version_num == 2 assert unit_version_v2 in unit.versioning.versions.all() - assert content_api.get_components_in_unit(unit, published=False) == [ + assert content_api.get_entities_in_container(unit, published=False) == [ Entry(self.component_1_v1), Entry(self.component_2_v1, pinned=True), # Pinned 📌 to v1 ] with pytest.raises(authoring_models.ContainerVersion.DoesNotExist): # There is no published version of the unit: - content_api.get_components_in_unit(unit, published=True) + content_api.get_entities_in_container(unit, published=True) def test_create_next_unit_version_forcing_version_num(self): """ Test creating a unit version with forcing the version number. """ - unit, _unit_version = content_api.create_unit_and_version( - learning_package_id=self.learning_package.id, + unit, _unit_version = self.create_unit_and_version( key="unit:key", title="Unit", - created=self.now, - created_by=None, ) - unit_version_v2 = content_api.create_next_unit_version( - unit=unit, + unit_version_v2 = content_api.create_next_container_version( + unit.pk, title="Unit", - components=[self.component_1, self.component_2_v1], # Note the "v1" pinning 📌 the second one to version 1 + entities=[self.component_1, self.component_2_v1], # Note the "v1" pinning 📌 the second one to version 1 created=self.now, created_by=None, force_version_num=5, @@ -396,7 +279,7 @@ def test_auto_publish_children(self): Test that publishing a unit publishes its child components automatically. """ # Create a draft unit with two draft components - unit = self.create_unit_with_components([self.component_1, self.component_2]) + unit = self.create_unit(entities=[self.component_1, self.component_2]) # Also create another component that's not in the unit at all: other_component, _oc_v1 = self.create_component(title="A draft component not in the unit", key="component:3") @@ -427,7 +310,7 @@ def test_no_publish_parent(self): Test that publishing a component does NOT publish changes to its parent unit """ # Create a draft unit with two draft components - unit = self.create_unit_with_components([self.component_1, self.component_2]) + unit = self.create_unit(entities=[self.component_1, self.component_2]) assert unit.versioning.has_unpublished_changes # Publish ONLY one of its child components self.publish_component(self.component_1) @@ -440,19 +323,16 @@ def test_no_publish_parent(self): assert unit.versioning.published is None with pytest.raises(authoring_models.ContainerVersion.DoesNotExist): # There is no published version of the unit: - content_api.get_components_in_unit(unit, published=True) + content_api.get_entities_in_container(unit, published=True) def test_add_component_after_publish(self): """ Adding a component to a published unit will create a new version and show that the unit has unpublished changes. """ - unit, unit_version = content_api.create_unit_and_version( - learning_package_id=self.learning_package.id, + unit, unit_version = self.create_unit_and_version( key="unit:key", title="Unit", - created=self.now, - created_by=None, ) assert unit.versioning.draft == unit_version assert unit.versioning.published is None @@ -465,10 +345,10 @@ def test_add_component_after_publish(self): # Add a published component (unpinned): assert self.component_1.versioning.has_unpublished_changes is False - unit_version_v2 = content_api.create_next_unit_version( - unit=unit, + unit_version_v2 = content_api.create_next_container_version( + unit.pk, title=unit_version.title, - components=[self.component_1], + entities=[self.component_1], created=self.now, created_by=None, entities_action=content_api.ChildrenEntitiesAction.APPEND, @@ -489,7 +369,7 @@ def test_modify_unpinned_component_after_publish(self): """ # Create a unit with one unpinned draft component: assert self.component_1.versioning.has_unpublished_changes - unit = self.create_unit_with_components([self.component_1]) + unit = self.create_unit(entities=[self.component_1]) assert unit.versioning.has_unpublished_changes # Publish the unit and the component: @@ -511,19 +391,19 @@ def test_modify_unpinned_component_after_publish(self): assert self.component_1.versioning.has_unpublished_changes # Since the component changes haven't been published, they should only appear in the draft unit - assert content_api.get_components_in_unit(unit, published=False) == [ + assert content_api.get_entities_in_container(unit, published=False) == [ Entry(component_1_v2), # new version ] - assert content_api.get_components_in_unit(unit, published=True) == [ + assert content_api.get_entities_in_container(unit, published=True) == [ Entry(self.component_1_v1), # old version ] # But if we publish the component, the changes will appear in the published version of the unit. self.publish_component(self.component_1) - assert content_api.get_components_in_unit(unit, published=False) == [ + assert content_api.get_entities_in_container(unit, published=False) == [ Entry(component_1_v2), # new version ] - assert content_api.get_components_in_unit(unit, published=True) == [ + assert content_api.get_entities_in_container(unit, published=True) == [ Entry(component_1_v2), # new version ] assert content_api.contains_unpublished_changes(unit.pk) is False # No longer contains unpublished changes @@ -535,14 +415,14 @@ def test_modify_pinned_component(self): which will continue to use the pinned version. """ # Create a unit with one component (pinned 📌 to v1): - unit = self.create_unit_with_components([self.component_1_v1]) + unit = self.create_unit(entities=[self.component_1_v1]) # Publish the unit and the component: content_api.publish_all_drafts(self.learning_package.id) expected_unit_contents = [ Entry(self.component_1_v1, pinned=True), # pinned 📌 to v1 ] - assert content_api.get_components_in_unit(unit, published=True) == expected_unit_contents + assert content_api.get_entities_in_container(unit, published=True) == expected_unit_contents # Now modify the component by changing its title (it remains a draft): self.modify_component(self.component_1, title="Modified Counting Problem with new title") @@ -555,12 +435,12 @@ def test_modify_pinned_component(self): assert self.component_1.versioning.has_unpublished_changes is True # Neither the draft nor the published version of the unit is affected - assert content_api.get_components_in_unit(unit, published=False) == expected_unit_contents - assert content_api.get_components_in_unit(unit, published=True) == expected_unit_contents + assert content_api.get_entities_in_container(unit, published=False) == expected_unit_contents + assert content_api.get_entities_in_container(unit, published=True) == expected_unit_contents # Even if we publish the component, the unit stays pinned to the specified version: self.publish_component(self.component_1) - assert content_api.get_components_in_unit(unit, published=False) == expected_unit_contents - assert content_api.get_components_in_unit(unit, published=True) == expected_unit_contents + assert content_api.get_entities_in_container(unit, published=False) == expected_unit_contents + assert content_api.get_entities_in_container(unit, published=True) == expected_unit_contents def test_create_two_units_with_same_components(self): """ @@ -568,16 +448,24 @@ def test_create_two_units_with_same_components(self): components in each unit. """ # Create a unit with component 2 unpinned, component 2 pinned 📌, and component 1: - unit1 = self.create_unit_with_components([self.component_2, self.component_2_v1, self.component_1], key="u1") + unit1 = self.create_unit(entities=[self.component_2, self.component_2_v1, self.component_1], key="u1") # Create a second unit with component 1 pinned 📌, component 2, and component 1 unpinned: - unit2 = self.create_unit_with_components([self.component_1_v1, self.component_2, self.component_1], key="u2") + unit2 = self.create_unit(entities=[self.component_1_v1, self.component_2, self.component_1], key="u2") # Check that the contents are as expected: - assert [row.component_version for row in content_api.get_components_in_unit(unit1, published=False)] == [ - self.component_2_v1, self.component_2_v1, self.component_1_v1, + assert [ + row.entity_version.componentversion for row in content_api.get_entities_in_container(unit1, published=False) + ] == [ + self.component_2_v1, + self.component_2_v1, + self.component_1_v1, ] - assert [row.component_version for row in content_api.get_components_in_unit(unit2, published=False)] == [ - self.component_1_v1, self.component_2_v1, self.component_1_v1, + assert [ + row.entity_version.componentversion for row in content_api.get_entities_in_container(unit2, published=False) + ] == [ + self.component_1_v1, + self.component_2_v1, + self.component_1_v1, ] # Modify component 1 @@ -588,24 +476,24 @@ def test_create_two_units_with_same_components(self): component_2_v2 = self.modify_component(self.component_2, title="component 2 DRAFT") # Check that the draft contents are as expected: - assert content_api.get_components_in_unit(unit1, published=False) == [ + assert content_api.get_entities_in_container(unit1, published=False) == [ Entry(component_2_v2), # v2 in the draft version Entry(self.component_2_v1, pinned=True), # pinned 📌 to v1 Entry(component_1_v2), # v2 ] - assert content_api.get_components_in_unit(unit2, published=False) == [ + assert content_api.get_entities_in_container(unit2, published=False) == [ Entry(self.component_1_v1, pinned=True), # pinned 📌 to v1 Entry(component_2_v2), # v2 in the draft version Entry(component_1_v2), # v2 ] # Check that the published contents are as expected: - assert content_api.get_components_in_unit(unit1, published=True) == [ + assert content_api.get_entities_in_container(unit1, published=True) == [ Entry(self.component_2_v1), # v1 in the published version Entry(self.component_2_v1, pinned=True), # pinned 📌 to v1 Entry(component_1_v2), # v2 ] - assert content_api.get_components_in_unit(unit2, published=True) == [ + assert content_api.get_entities_in_container(unit2, published=True) == [ Entry(self.component_1_v1, pinned=True), # pinned 📌 to v1 Entry(self.component_2_v1), # v1 in the published version Entry(component_1_v2), # v2 @@ -624,8 +512,8 @@ def test_publishing_shared_component(self): (c1, c1_v1), (c2, _c2_v1), (c3, c3_v1), (c4, c4_v1), (c5, c5_v1) = [ self.create_component(key=f"C{i}", title=f"Component {i}") for i in range(1, 6) ] - unit1 = self.create_unit_with_components([c1, c2, c3], title="Unit 1", key="unit:1") - unit2 = self.create_unit_with_components([c2, c4, c5], title="Unit 2", key="unit:2") + unit1 = self.create_unit(entities=[c1, c2, c3], title="Unit 1", key="unit:1") + unit2 = self.create_unit(entities=[c2, c4, c5], title="Unit 2", key="unit:2") content_api.publish_all_drafts(self.learning_package.id) assert content_api.contains_unpublished_changes(unit1.pk) is False assert content_api.contains_unpublished_changes(unit2.pk) is False @@ -654,7 +542,7 @@ def test_publishing_shared_component(self): ) # Result: Unit 1 will show the newly published version of C2: - assert content_api.get_components_in_unit(unit1, published=True) == [ + assert content_api.get_entities_in_container(unit1, published=True) == [ Entry(c1_v1), Entry(c2_v2), # new published version of C2 Entry(c3_v1), @@ -663,7 +551,7 @@ def test_publishing_shared_component(self): # Result: someone looking at Unit 2 should see the newly published component 2, because publishing it anywhere # publishes it everywhere. But publishing C2 and Unit 1 does not affect the other components in Unit 2. # (Publish propagates downward, not upward) - assert content_api.get_components_in_unit(unit2, published=True) == [ + assert content_api.get_entities_in_container(unit2, published=True) == [ Entry(c2_v2), # new published version of C2 Entry(c4_v1), # still original version of C4 (it was never modified) Entry(c5_v1), # still original version of C5 (it hasn't been published) @@ -676,7 +564,7 @@ def test_publishing_shared_component(self): # 5️⃣ Publish component C5, which should be the only thing unpublished in the learning package self.publish_component(c5) # Result: Unit 2 shows the new version of C5 and no longer contains unpublished changes: - assert content_api.get_components_in_unit(unit2, published=True) == [ + assert content_api.get_entities_in_container(unit2, published=True) == [ Entry(c2_v2), # new published version of C2 Entry(c4_v1), # still original version of C4 (it was never modified) Entry(c5_v2), # new published version of C5 @@ -697,7 +585,7 @@ def test_query_count_of_contains_unpublished_changes(self): title=f"Querying Counting Problem {i}", ) components.append(component) - unit = self.create_unit_with_components(components) + unit = self.create_unit(entities=components) content_api.publish_all_drafts(self.learning_package.id) unit.refresh_from_db() with self.assertNumQueries(1): @@ -714,12 +602,12 @@ def test_metadata_change_doesnt_create_entity_list(self): version, but can re-use the same EntityList. API consumers generally shouldn't depend on this behavior; it's an optimization. """ - unit = self.create_unit_with_components([self.component_1, self.component_2_v1]) + unit = self.create_unit(entities=[self.component_1, self.component_2_v1]) orig_version_num = unit.versioning.draft.version_num orig_entity_list_id = unit.versioning.draft.entity_list.pk - content_api.create_next_unit_version(unit, title="New Title", created=self.now) + content_api.create_next_container_version(unit.pk, title="New Title", created=self.now, created_by=None) unit.refresh_from_db() new_version_num = unit.versioning.draft.version_num @@ -746,31 +634,32 @@ def test_cannot_add_soft_deleted_component(self, publish_first): content_api.soft_delete_draft(component.pk) # Now try adding that component to a unit: with pytest.raises(ValidationError, match="component is deleted"): - self.create_unit_with_components([component]) + self.create_unit(entities=[component]) def test_removing_component(self): - """ Test removing a component from a unit (but not deleting it) """ - unit = self.create_unit_with_components([self.component_1, self.component_2]) + """Test removing a component from a unit (but not deleting it)""" + unit = self.create_unit(entities=[self.component_1, self.component_2]) content_api.publish_all_drafts(self.learning_package.id) # Now remove component 2 - content_api.create_next_unit_version( - unit=unit, + content_api.create_next_container_version( + unit.pk, title="Revised with component 2 deleted", - components=[self.component_2], + entities=[self.component_2], created=self.now, + created_by=None, entities_action=content_api.ChildrenEntitiesAction.REMOVE, ) # Now it should not be listed in the unit: - assert content_api.get_components_in_unit(unit, published=False) == [ + assert content_api.get_entities_in_container(unit, published=False) == [ Entry(self.component_1_v1), ] unit.refresh_from_db() assert unit.versioning.has_unpublished_changes # The unit itself and its component list have change assert content_api.contains_unpublished_changes(unit.pk) # The published version of the unit is not yet affected: - assert content_api.get_components_in_unit(unit, published=True) == [ + assert content_api.get_entities_in_container(unit, published=True) == [ Entry(self.component_1_v1), Entry(self.component_2_v1), ] @@ -783,20 +672,20 @@ def test_removing_component(self): # but that would involve additional database lookup(s). unit.refresh_from_db() assert content_api.contains_unpublished_changes(unit.pk) is False - assert content_api.get_components_in_unit(unit, published=True) == [ + assert content_api.get_entities_in_container(unit, published=True) == [ Entry(self.component_1_v1), ] def test_soft_deleting_component(self): - """ Test soft deleting a component that's in a unit (but not removing it) """ - unit = self.create_unit_with_components([self.component_1, self.component_2]) + """Test soft deleting a component that's in a unit (but not removing it)""" + unit = self.create_unit(entities=[self.component_1, self.component_2]) content_api.publish_all_drafts(self.learning_package.id) # Now soft delete component 2 content_api.soft_delete_draft(self.component_2.pk) # Now it should not be listed in the unit: - assert content_api.get_components_in_unit(unit, published=False) == [ + assert content_api.get_entities_in_container(unit, published=False) == [ Entry(self.component_1_v1), # component 2 is soft deleted from the draft. # TODO: should we return some kind of placeholder here, to indicate that a component is still listed in the @@ -806,7 +695,7 @@ def test_soft_deleting_component(self): assert unit.versioning.has_unpublished_changes is False # The unit itself and its component list is not changed assert content_api.contains_unpublished_changes(unit.pk) # But it CONTAINS an unpublished change (a deletion) # The published version of the unit is not yet affected: - assert content_api.get_components_in_unit(unit, published=True) == [ + assert content_api.get_entities_in_container(unit, published=True) == [ Entry(self.component_1_v1), Entry(self.component_2_v1), ] @@ -814,34 +703,35 @@ def test_soft_deleting_component(self): # But when we publish the deletion, the published version is affected: content_api.publish_all_drafts(self.learning_package.id) assert content_api.contains_unpublished_changes(unit.pk) is False - assert content_api.get_components_in_unit(unit, published=True) == [ + assert content_api.get_entities_in_container(unit, published=True) == [ Entry(self.component_1_v1), ] def test_soft_deleting_and_removing_component(self): - """ Test soft deleting a component that's in a unit AND removing it """ - unit = self.create_unit_with_components([self.component_1, self.component_2]) + """Test soft deleting a component that's in a unit AND removing it""" + unit = self.create_unit(entities=[self.component_1, self.component_2]) content_api.publish_all_drafts(self.learning_package.id) # Now soft delete component 2 content_api.soft_delete_draft(self.component_2.pk) # And remove it from the unit: - content_api.create_next_unit_version( - unit=unit, + content_api.create_next_container_version( + unit.pk, title="Revised with component 2 deleted", - components=[self.component_2], + entities=[self.component_2], created=self.now, + created_by=None, entities_action=content_api.ChildrenEntitiesAction.REMOVE, ) # Now it should not be listed in the unit: - assert content_api.get_components_in_unit(unit, published=False) == [ + assert content_api.get_entities_in_container(unit, published=False) == [ Entry(self.component_1_v1), ] assert unit.versioning.has_unpublished_changes is True assert content_api.contains_unpublished_changes(unit.pk) # The published version of the unit is not yet affected: - assert content_api.get_components_in_unit(unit, published=True) == [ + assert content_api.get_entities_in_container(unit, published=True) == [ Entry(self.component_1_v1), Entry(self.component_2_v1), ] @@ -849,27 +739,27 @@ def test_soft_deleting_and_removing_component(self): # But when we publish the deletion, the published version is affected: content_api.publish_all_drafts(self.learning_package.id) assert content_api.contains_unpublished_changes(unit.pk) is False - assert content_api.get_components_in_unit(unit, published=True) == [ + assert content_api.get_entities_in_container(unit, published=True) == [ Entry(self.component_1_v1), ] def test_soft_deleting_pinned_component(self): - """ Test soft deleting a pinned 📌 component that's in a unit """ - unit = self.create_unit_with_components([self.component_1_v1, self.component_2_v1]) + """Test soft deleting a pinned 📌 component that's in a unit""" + unit = self.create_unit(entities=[self.component_1_v1, self.component_2_v1]) content_api.publish_all_drafts(self.learning_package.id) # Now soft delete component 2 content_api.soft_delete_draft(self.component_2.pk) # Now it should still be listed in the unit: - assert content_api.get_components_in_unit(unit, published=False) == [ + assert content_api.get_entities_in_container(unit, published=False) == [ Entry(self.component_1_v1, pinned=True), Entry(self.component_2_v1, pinned=True), ] assert unit.versioning.has_unpublished_changes is False # The unit itself and its component list is not changed assert content_api.contains_unpublished_changes(unit.pk) is False # nor does it contain changes # The published version of the unit is also not affected: - assert content_api.get_components_in_unit(unit, published=True) == [ + assert content_api.get_entities_in_container(unit, published=True) == [ Entry(self.component_1_v1, pinned=True), Entry(self.component_2_v1, pinned=True), ] @@ -881,8 +771,8 @@ def test_soft_delete_unit(self): See https://github.com/openedx/frontend-app-authoring/issues/1693 """ # Create two units, one of which we will soon delete: - unit_to_delete = self.create_unit_with_components([self.component_1, self.component_2]) - other_unit = self.create_unit_with_components([self.component_1], key="other") + unit_to_delete = self.create_unit(entities=[self.component_1, self.component_2]) + other_unit = self.create_unit(entities=[self.component_1], key="other") # Publish everything: content_api.publish_all_drafts(self.learning_package.id) @@ -894,7 +784,7 @@ def test_soft_delete_unit(self): assert unit_to_delete.versioning.published is not None self.component_1.refresh_from_db() assert self.component_1.versioning.draft is not None - assert content_api.get_components_in_unit(other_unit, published=False) == [Entry(self.component_1_v1)] + assert content_api.get_entities_in_container(other_unit, published=False) == [Entry(self.component_1_v1)] # Publish everything: content_api.publish_all_drafts(self.learning_package.id) @@ -905,8 +795,8 @@ def test_soft_delete_unit(self): self.component_1.refresh_from_db() assert self.component_1.versioning.draft is not None assert self.component_1.versioning.published is not None - assert content_api.get_components_in_unit(other_unit, published=False) == [Entry(self.component_1_v1)] - assert content_api.get_components_in_unit(other_unit, published=True) == [Entry(self.component_1_v1)] + assert content_api.get_entities_in_container(other_unit, published=False) == [Entry(self.component_1_v1)] + assert content_api.get_entities_in_container(other_unit, published=True) == [Entry(self.component_1_v1)] def test_snapshots_of_published_unit(self): """ @@ -914,10 +804,10 @@ def test_snapshots_of_published_unit(self): units and their contents. """ # At first the unit has one component (unpinned): - unit = self.create_unit_with_components([self.component_1]) + unit = self.create_unit(entities=[self.component_1]) self.modify_component(self.component_1, title="Component 1 as of checkpoint 1") - before_publish = content_api.get_components_in_published_unit_as_of(unit, 0) - assert before_publish is None + _, before_publish = content_api.get_entities_in_container_as_of(unit, 0) + assert before_publish == [] # Empty list # Publish everything, creating Checkpoint 1 checkpoint_1 = content_api.publish_all_drafts(self.learning_package.id, message="checkpoint 1") @@ -933,11 +823,12 @@ def test_snapshots_of_published_unit(self): # Now add a second component to the unit: self.modify_component(self.component_1, title="Component 1 as of checkpoint 3") self.modify_component(self.component_2, title="Component 2 as of checkpoint 3") - content_api.create_next_unit_version( - unit=unit, + content_api.create_next_container_version( + unit.pk, title="Unit title in checkpoint 3", - components=[self.component_1, self.component_2], + entities=[self.component_1, self.component_2], created=self.now, + created_by=None, ) # Publish everything, creating Checkpoint 3 checkpoint_3 = content_api.publish_all_drafts(self.learning_package.id, message="checkpoint 3") @@ -945,11 +836,12 @@ def test_snapshots_of_published_unit(self): # Now add a third component to the unit, a pinned 📌 version of component 1. # This will test pinned versions and also test adding at the beginning rather than the end of the unit. - content_api.create_next_unit_version( - unit=unit, + content_api.create_next_container_version( + unit.pk, title="Unit title in checkpoint 4", - components=[self.component_1_v1, self.component_1, self.component_2], + entities=[self.component_1_v1, self.component_1, self.component_2], created=self.now, + created_by=None, ) # Publish everything, creating Checkpoint 4 checkpoint_4 = content_api.publish_all_drafts(self.learning_package.id, message="checkpoint 4") @@ -960,21 +852,21 @@ def test_snapshots_of_published_unit(self): self.modify_component(self.component_2, title="Component 2 draft") # Now fetch the snapshots: - as_of_checkpoint_1 = content_api.get_components_in_published_unit_as_of(unit, checkpoint_1.pk) - assert [cv.component_version.title for cv in as_of_checkpoint_1] == [ + _, as_of_checkpoint_1 = content_api.get_entities_in_container_as_of(unit, checkpoint_1.pk) + assert [ev.entity_version.componentversion.title for ev in as_of_checkpoint_1] == [ "Component 1 as of checkpoint 1", ] - as_of_checkpoint_2 = content_api.get_components_in_published_unit_as_of(unit, checkpoint_2.pk) - assert [cv.component_version.title for cv in as_of_checkpoint_2] == [ + _, as_of_checkpoint_2 = content_api.get_entities_in_container_as_of(unit, checkpoint_2.pk) + assert [ev.entity_version.componentversion.title for ev in as_of_checkpoint_2] == [ "Component 1 as of checkpoint 2", ] - as_of_checkpoint_3 = content_api.get_components_in_published_unit_as_of(unit, checkpoint_3.pk) - assert [cv.component_version.title for cv in as_of_checkpoint_3] == [ + _, as_of_checkpoint_3 = content_api.get_entities_in_container_as_of(unit, checkpoint_3.pk) + assert [ev.entity_version.componentversion.title for ev in as_of_checkpoint_3] == [ "Component 1 as of checkpoint 3", "Component 2 as of checkpoint 3", ] - as_of_checkpoint_4 = content_api.get_components_in_published_unit_as_of(unit, checkpoint_4.pk) - assert [cv.component_version.title for cv in as_of_checkpoint_4] == [ + _, as_of_checkpoint_4 = content_api.get_entities_in_container_as_of(unit, checkpoint_4.pk) + assert [ev.entity_version.componentversion.title for ev in as_of_checkpoint_4] == [ "Querying Counting Problem", # Pinned. This title is self.component_1_v1.title (original v1 title) "Component 1 as of checkpoint 3", # we didn't modify these components so they're same as in snapshot 3 "Component 2 as of checkpoint 3", # we didn't modify these components so they're same as in snapshot 3 @@ -991,31 +883,39 @@ def test_units_containing(self): # Note: it is important that some of these units contain other components, to ensure the complex JOINs required # for this query are working correctly, especially in the case of ignore_pinned=True. # Unit 1 ✅ has component 1, pinned 📌 to V1 - unit1_1pinned = self.create_unit_with_components([self.component_1_v1, self.component_2], key="u1") + unit1_1pinned = self.create_unit(entities=[self.component_1_v1, self.component_2], key="u1") # Unit 2 ✅ has component 1, pinned 📌 to V2 - unit2_1pinned_v2 = self.create_unit_with_components([component_1_v2, self.component_2_v1], key="u2") + unit2_1pinned_v2 = self.create_unit(entities=[component_1_v2, self.component_2_v1], key="u2") # Unit 3 doesn't contain it - _unit3_no = self.create_unit_with_components([self.component_2], key="u3") + _unit3_no = self.create_unit(entities=[self.component_2], key="u3") # Unit 4 ✅ has component 1, unpinned - unit4_unpinned = self.create_unit_with_components([ - self.component_1, self.component_2, self.component_2_v1, - ], key="u4") + unit4_unpinned = self.create_unit(entities= + [ + self.component_1, + self.component_2, + self.component_2_v1, + ], + key="u4", + ) # Units 5/6 don't contain it - _unit5_no = self.create_unit_with_components([self.component_2_v1, self.component_2], key="u5") - _unit6_no = self.create_unit_with_components([], key="u6") + _unit5_no = self.create_unit(entities=[self.component_2_v1, self.component_2], key="u5") + _unit6_no = self.create_unit(entities=[], key="u6") # To test unique results, unit 7 ✅ contains several copies of component 1. Also tests matching against # components that aren't in the first position. - unit7_several = self.create_unit_with_components([ - self.component_2, self.component_1, self.component_1_v1, self.component_1, - ], key="u7") + unit7_several = self.create_unit(entities= + [ + self.component_2, + self.component_1, + self.component_1_v1, + self.component_1, + ], + key="u7", + ) # No need to publish anything as the get_containers_with_entity() API only considers drafts (for now). with self.assertNumQueries(1): - result = [ - c.unit for c in - content_api.get_containers_with_entity(self.component_1.pk).select_related("unit") - ] + result = list(content_api.get_containers_with_entity(self.component_1.pk)) assert result == [ unit1_1pinned, unit2_1pinned_v2, @@ -1027,32 +927,31 @@ def test_units_containing(self): # about pinned uses anyways (they would be unaffected by a delete). with self.assertNumQueries(1): - result2 = [ - c.unit for c in - content_api.get_containers_with_entity(self.component_1.pk, ignore_pinned=True).select_related("unit") - ] + result2 = list(content_api.get_containers_with_entity(self.component_1.pk, ignore_pinned=True)) assert result2 == [unit4_unpinned, unit7_several] - def test_get_components_in_unit_queries(self): + def test_get_entities_in_container_queries(self): """ - Test the query count of get_components_in_unit() + Test the query count of get_entities_in_container() This also tests the generic method get_entities_in_container() """ - unit = self.create_unit_with_components([ - self.component_1, - self.component_2, - self.component_2_v1, - ]) - with self.assertNumQueries(3): - result = content_api.get_components_in_unit(unit, published=False) + unit = self.create_unit(entities= + [ + self.component_1, + self.component_2, + self.component_2_v1, + ] + ) + with self.assertNumQueries(2): + result = content_api.get_entities_in_container(unit, published=False) assert result == [ Entry(self.component_1.versioning.draft), Entry(self.component_2.versioning.draft), Entry(self.component_2.versioning.draft, pinned=True), ] content_api.publish_all_drafts(self.learning_package.id) - with self.assertNumQueries(3): - result = content_api.get_components_in_unit(unit, published=True) + with self.assertNumQueries(2): + result = content_api.get_entities_in_container(unit, published=True) assert result == [ Entry(self.component_1.versioning.draft), Entry(self.component_2.versioning.draft), @@ -1063,15 +962,12 @@ def test_add_remove_container_children(self): """ Test adding and removing children components from containers. """ - unit, unit_version = content_api.create_unit_and_version( - learning_package_id=self.learning_package.id, + unit, unit_version = self.create_unit_and_version( key="unit:key", title="Unit", - components=[self.component_1], - created=self.now, - created_by=None, + entities=[self.component_1], ) - assert content_api.get_components_in_unit(unit, published=False) == [ + assert content_api.get_entities_in_container(unit, published=False) == [ Entry(self.component_1.versioning.draft), ] component_3, _ = self.create_component( @@ -1079,10 +975,13 @@ def test_add_remove_container_children(self): title="Querying Counting Problem (3)", ) # Add component_2 and component_3 - unit_version_v2 = content_api.create_next_unit_version( - unit=unit, + unit_version_v2 = content_api.create_next_container_version( + unit.pk, title=unit_version.title, - components=[self.component_2, component_3], + entities=[ + self.component_2, + component_3, + ], created=self.now, created_by=None, entities_action=content_api.ChildrenEntitiesAction.APPEND, @@ -1091,24 +990,24 @@ def test_add_remove_container_children(self): assert unit_version_v2.version_num == 2 assert unit_version_v2 in unit.versioning.versions.all() # Verify that component_2 and component_3 is added to end - assert content_api.get_components_in_unit(unit, published=False) == [ + assert content_api.get_entities_in_container(unit, published=False) == [ Entry(self.component_1.versioning.draft), Entry(self.component_2.versioning.draft), Entry(component_3.versioning.draft), ] # Remove component_1 - content_api.create_next_unit_version( - unit=unit, + content_api.create_next_container_version( + unit.pk, title=unit_version.title, - components=[self.component_1], + entities=[self.component_1], created=self.now, created_by=None, entities_action=content_api.ChildrenEntitiesAction.REMOVE, ) unit.refresh_from_db() # Verify that component_1 is removed - assert content_api.get_components_in_unit(unit, published=False) == [ + assert content_api.get_entities_in_container(unit, published=False) == [ Entry(self.component_2.versioning.draft), Entry(component_3.versioning.draft), ] @@ -1117,35 +1016,35 @@ def test_get_container_children_count(self): """ Test get_container_children_count() """ - unit = self.create_unit_with_components([self.component_1]) - assert content_api.get_container_children_count(unit.container, published=False) == 1 + unit = self.create_unit(entities=[self.component_1]) + assert content_api.get_container_children_count(unit, published=False) == 1 # publish content_api.publish_all_drafts(self.learning_package.id) unit_version = unit.versioning.draft - content_api.create_next_unit_version( - unit=unit, + content_api.create_next_container_version( + unit.pk, title=unit_version.title, - components=[self.component_2], + entities=[self.component_2], created=self.now, created_by=None, entities_action=content_api.ChildrenEntitiesAction.APPEND, ) unit.refresh_from_db() # Should have two components in draft version and 1 in published version - assert content_api.get_container_children_count(unit.container, published=False) == 2 - assert content_api.get_container_children_count(unit.container, published=True) == 1 + assert content_api.get_container_children_count(unit, published=False) == 2 + assert content_api.get_container_children_count(unit, published=True) == 1 # publish content_api.publish_all_drafts(self.learning_package.id) unit.refresh_from_db() - assert content_api.get_container_children_count(unit.container, published=True) == 2 + assert content_api.get_container_children_count(unit, published=True) == 2 # Soft delete component_1 content_api.soft_delete_draft(self.component_1.pk) unit.refresh_from_db() # Should contain only 1 child - assert content_api.get_container_children_count(unit.container, published=False) == 1 + assert content_api.get_container_children_count(unit, published=False) == 1 content_api.publish_all_drafts(self.learning_package.id) unit.refresh_from_db() - assert content_api.get_container_children_count(unit.container, published=True) == 1 + assert content_api.get_container_children_count(unit, published=True) == 1 # Tests TODO: # Test that I can get a [PublishLog] history of a given unit and all its children, including children that aren't