refactor: Units are Containers

openedx_learning/apps/authoring/publishing/api.py

@@ -13,12 +13,7 @@
 from django.db.models import F, Q, QuerySet
 from django.db.transaction import atomic
 
-from .model_mixins import (
-    ContainerMixin,
-    PublishableContentModelRegistry,
-    PublishableEntityMixin,
-    PublishableEntityVersionMixin,
-)
+from .model_mixins import PublishableContentModelRegistry, PublishableEntityMixin, PublishableEntityVersionMixin
 from .models import (
     Container,
     ContainerVersion,
@@ -579,6 +574,7 @@ def get_published_version_as_of(entity_id: int, publish_log_id: int) -> Publisha
 def create_container(
     learning_package_id: int,
     key: str,
+    container_type: str,
     created: datetime,
     created_by: int | None,
 ) -> Container:
@@ -595,12 +591,14 @@ def create_container(
     Returns:
         The newly created container.
     """
+    assert container_type  # Shouldn't be empty/none
     with atomic():
         publishable_entity = create_publishable_entity(
             learning_package_id, key, created, created_by
         )
         container = Container.objects.create(
             publishable_entity=publishable_entity,
+            container_type=container_type,
         )
     return container
 
@@ -635,7 +633,7 @@ def create_entity_list_with_rows(
         The newly created entity list.
     """
     order_nums = range(len(entity_pks))
-    with atomic():
+    with atomic(savepoint=False):
         entity_list = create_entity_list()
         EntityListRow.objects.bulk_create(
             [
@@ -679,7 +677,7 @@ def create_container_version(
     Returns:
         The newly created container version.
     """
-    with atomic():
+    with atomic(savepoint=False):
         container = Container.objects.select_related("publishable_entity").get(pk=container_pk)
         entity = container.publishable_entity
 
@@ -789,6 +787,7 @@ def create_container_and_version(
     learning_package_id: int,
     key: str,
     *,
+    container_type: str,
     created: datetime,
     created_by: int | None,
     title: str,
@@ -812,7 +811,7 @@ def create_container_and_version(
         The newly created container version.
     """
     with atomic():
-        container = create_container(learning_package_id, key, created, created_by)
+        container = create_container(learning_package_id, key, container_type, created, created_by)
         container_version = create_container_version(
             container.publishable_entity.pk,
             1,
@@ -854,15 +853,13 @@ def entity(self):
 
 
 def get_entities_in_draft_container(
-    container: Container | ContainerMixin,
+    container: Container,
 ) -> list[ContainerEntityListEntry]:
     """
     [ 🛑 UNSTABLE ]
     Get the list of entities and their versions in the draft version of the
     given container.
     """
-    if isinstance(container, ContainerMixin):
-        container = container.container
     assert isinstance(container, Container)
     entity_list = []
     for row in container.versioning.draft.entity_list.entitylistrow_set.order_by("order_num"):
@@ -877,19 +874,15 @@ def get_entities_in_draft_container(
 
 
 def get_entities_in_published_container(
-    container: Container | ContainerMixin,
+    container: Container,
 ) -> list[ContainerEntityListEntry] | None:
     """
     [ 🛑 UNSTABLE ]
     Get the list of entities and their versions in the published version of the
     given container.
     """
-    if isinstance(container, ContainerMixin):
-        cv = container.container.versioning.published
-    elif isinstance(container, Container):
-        cv = container.versioning.published
-    else:
-        raise TypeError(f"Expected Container or ContainerMixin; got {type(container)}")
+    assert isinstance(container, Container)
+    cv = container.versioning.published
     if cv is None:
         return None  # There is no published version of this container. Should this be an exception?
     assert isinstance(cv, ContainerVersion)
@@ -905,9 +898,7 @@ def get_entities_in_published_container(
     return entity_list
 
 
-def contains_unpublished_changes(
-    container: Container | ContainerMixin,
-) -> bool:
+def contains_unpublished_changes(container_id: int) -> bool:
     """
     [ 🛑 UNSTABLE ]
     Check recursively if a container has any unpublished changes.
@@ -920,14 +911,10 @@ def contains_unpublished_changes(
     that's in the container, it will be `False`. This method will return `True`
     in either case.
     """
-    if isinstance(container, ContainerMixin):
-        # This is similar to 'get_container(container.container_id)' but pre-loads more data.
-        container = Container.objects.select_related(
-            "publishable_entity__draft__version__containerversion__entity_list",
-        ).get(pk=container.container_id)
-    else:
-        pass  # TODO: select_related if we're given a raw Container rather than a ContainerMixin like Unit?
-    assert isinstance(container, Container)
+    # This is similar to 'get_container(container.container_id)' but pre-loads more data.
+    container = Container.objects.select_related(
+        "publishable_entity__draft__version__containerversion__entity_list",
+    ).get(pk=container_id)
 
     if container.versioning.has_unpublished_changes:
         return True
@@ -949,7 +936,7 @@ def contains_unpublished_changes(
             child_container = None
         if child_container:
             # This is itself a container - check recursively:
-            if contains_unpublished_changes(child_container):
+            if contains_unpublished_changes(child_container.pk):
                 return True
         else:
             # This is not a container:

openedx_learning/apps/authoring/publishing/migrations/0003_containers.py

@@ -1,8 +1,10 @@
-# Generated by Django 4.2.19 on 2025-03-07 23:09
+# Generated by Django 4.2.19 on 2025-03-10 18:03
 
 import django.db.models.deletion
 from django.db import migrations, models
 
+import openedx_learning.lib.fields
+
 
 class Migration(migrations.Migration):
 
@@ -15,6 +17,7 @@ class Migration(migrations.Migration):
             name='Container',
             fields=[
                 ('publishable_entity', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, primary_key=True, serialize=False, to='oel_publishing.publishableentity')),
+                ('container_type', openedx_learning.lib.fields.MultiCollationCharField(db_collations={'mysql': 'utf8mb4_bin', 'sqlite': 'BINARY'}, max_length=500)),
             ],
             options={
                 'abstract': False,

openedx_learning/apps/authoring/publishing/model_mixins/__init__.py

@@ -1,5 +1,4 @@
 """
 Mixins provided by the publishing app
 """
-from .container import *
 from .publishable_entity import *

openedx_learning/apps/authoring/publishing/models/container.py

@@ -1,11 +1,54 @@
 """
 Container and ContainerVersion models
 """
+from typing import ClassVar, Self, TypeVar
+
+from django.core.exceptions import ValidationError
 from django.db import models
 
+from openedx_learning.lib.fields import case_sensitive_char_field
+from openedx_learning.lib.managers import WithRelationsManager
+
 from ..model_mixins.publishable_entity import PublishableEntityMixin, PublishableEntityVersionMixin
 from .entity_list import EntityList
 
+M = TypeVar('M', bound="Container")
+
+
+class ContainerManager(WithRelationsManager[M]):
+    """
+    A custom manager used for Container and its subclasses.
+    """
+    def __init__(self):
+        """
+        Initialize the manager for Container / a Container subclass
+        """
+        super().__init__(
+            # Select these related entities by default:
+            "publishable_entity",
+            "publishable_entity__published",
+            "publishable_entity__draft",
+        )
+
+    def get_queryset(self) -> models.QuerySet:
+        """
+        Apply filter() and select_related() to all querysets.
+        """
+        qs = super().get_queryset()
+        if self.model.CONTAINER_TYPE:
+            qs = qs.filter(container_type=self.model.CONTAINER_TYPE)
+        return qs
+
+    def create(self, **kwargs) -> M:
+        """
+        Apply the values from our filter when creating new instances.
+        """
+        if self.model.CONTAINER_TYPE:
+            # Don't allow creating via a subclass, like Unit.objects.create().
+            # Instead use create_container() which calls Container.objects.create(..., container_type=...)
+            raise ValidationError("Container instances should only be created via APIs like create_container()")
+        return super().create(**kwargs)
+
 
 class Container(PublishableEntityMixin):
     """
@@ -22,6 +65,40 @@ class Container(PublishableEntityMixin):
     PublishLog and Containers that were affected in a publish because their
     child elements were published.
     """
+    # Subclasses (django proxy classes) should override this
+    CONTAINER_TYPE = ""
+
+    objects: ClassVar[ContainerManager[Self]] = ContainerManager()  # type: ignore[assignment]
+
+    container_type = case_sensitive_char_field(max_length=500)
+
+    def save(self, *args, **kwargs):
+        if not self.container_type:
+            raise ValidationError("Container instances should only be created via APIs like create_container()")
+        return super().save(*args, **kwargs)
+
+    def clean(self):
+        """
+        Validate this container subclass
+        """
+        if self.container_type and self.CONTAINER_TYPE:
+            if self.container_type != self.CONTAINER_TYPE:
+                raise ValidationError("container type field mismatch with model.")
+        super().clean()
+
+    @classmethod
+    def cast_from(cls, instance: "Container") -> Self:
+        """
+        Create a new copy of a Container object, with a different subclass
+        """
+        assert instance.container_type == cls.CONTAINER_TYPE
+        new_instance = cls(
+            pk=instance.pk,
+            container_type=instance.container_type,
+        )
+        # Copy Django's internal cache of related objects
+        new_instance._state.fields_cache.update(instance._state.fields_cache)  # pylint: disable=protected-access
+        return new_instance
 
 
 class ContainerVersion(PublishableEntityVersionMixin):
@@ -58,3 +135,17 @@ class ContainerVersion(PublishableEntityVersionMixin):
         null=False,
         related_name="container_versions",
     )
+
+    @classmethod
+    def cast_from(cls, instance: "ContainerVersion") -> Self:
+        """
+        Create a new copy of a Container object, with a different subclass
+        """
+        new_instance = cls(
+            pk=instance.pk,
+            container_id=instance.container_id,
+            entity_list_id=instance.entity_list_id,
+        )
+        # Copy Django's internal cache of related objects
+        new_instance._state.fields_cache.update(instance._state.fields_cache)  # pylint: disable=protected-access
+        return new_instance