feat: add ContainerTypes to openedx-core (v0.38.0) (#495)

- Add a ContainerType concept and corresponding database table (ContainerTypeRecord). Now every container must have a corresponding type; plain Container instances cannot be saved on their own.
- The test cases for Containers are now in the publishing app and don't depend on Unit, Subsection, etc. This consolidates the core container logic, and should also make it easier to extract to a separate containers app.
- The Unit, Subsection, and Section classes, apps, and test cases are substantially simplified.
Because the container APIs are now type-aware you can use the generic APIs even with specific subclasses like Unit.
- The `create_next_container_version` API now accepts a `Container` object. Previously it only accepted a container PK, but passing the object is more convenient

* feat: add new ContainerType model, refactor Containers implementation
* feat: move containers into a new applet
* feat: store `olx_tag_name` in openedx_content's core container type models
* perf: slightly reduce query count when computing publishing side effects
* feat: improved container admin views
* fix: prevent creation of plain Containers

Author: bradenmacdonald

.importlinter

@@ -48,7 +48,9 @@ layers=
     # Problems, Videos, and blocks of HTML text. This is also the type we would
     # associate with a single "leaf" XBlock–one that is not a container type and
     # has no child elements.
-    openedx_content.applets.components
+    # The "containers" app is built on top of publishing, and is a peer to
+    # "components" but they do not depend on each other.
+    openedx_content.applets.components | openedx_content.applets.containers
 
     # The "media" applet stores the simplest pieces of binary and text data,
     # without versioning information. These belong to a single Learning Package.

src/openedx_content/admin.py

@@ -6,8 +6,6 @@
 from .applets.backup_restore.admin import *
 from .applets.collections.admin import *
 from .applets.components.admin import *
+from .applets.containers.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 *

src/openedx_content/api.py

@@ -13,6 +13,7 @@
 from .applets.backup_restore.api import *
 from .applets.collections.api import *
 from .applets.components.api import *
+from .applets.containers.api import *
 from .applets.media.api import *
 from .applets.publishing.api import *
 from .applets.sections.api import *

src/openedx_content/applets/backup_restore/toml.py

@@ -9,7 +9,7 @@
 from django.contrib.auth.models import User as UserType  # pylint: disable=imported-auth-user
 
 from ..collections.models import Collection
-from ..publishing import api as publishing_api
+from ..containers import api as containers_api
 from ..publishing.models import PublishableEntity, PublishableEntityVersion
 from ..publishing.models.learning_package import LearningPackage
 
@@ -191,7 +191,7 @@ def toml_publishable_entity_version(version: PublishableEntityVersion) -> tomlki
     if hasattr(version, 'containerversion'):
         # If the version has a container version, add its children
         container_table = tomlkit.table()
-        children = publishing_api.get_container_children_entities_keys(version.containerversion)
+        children = containers_api.get_container_children_entities_keys(version.containerversion)
         container_table.add("children", children)
         version_table.add("container", container_table)
     return version_table

src/openedx_content/applets/backup_restore/zipper.py

@@ -31,11 +31,12 @@
 
 from ..collections import api as collections_api
 from ..components import api as components_api
+from ..containers import api as containers_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 ..sections.models import Section
+from ..subsections.models import Subsection
+from ..units.models import Unit
 from .serializers import (
     CollectionSerializer,
     ComponentSerializer,
@@ -804,70 +805,70 @@ def _save_components(self, learning_package, components, component_static_files)
                 **valid_published
             )
 
-    def _save_units(self, learning_package, containers):
-        """Save units and published unit versions."""
-        for valid_unit in containers.get("unit", []):
-            entity_key = valid_unit.get("key")
-            unit = units_api.create_unit(learning_package.id, created_by=self.user_id, **valid_unit)
-            self.units_map_by_key[entity_key] = unit
+    def _save_container(
+        self,
+        learning_package,
+        containers,
+        *,
+        container_cls: containers_api.ContainerSubclass,
+        container_map: dict,
+        children_map: dict,
+    ):
+        """Internal logic for _save_units, _save_subsections, and _save_sections"""
+        type_code = container_cls.type_code  # e.g. "unit"
+        for data in containers.get(type_code, []):
+            entity_key = data.get("key")
+            container = containers_api.create_container(
+                learning_package.id,
+                **data,  # should this be allowed to override any of the following fields?
+                created_by=self.user_id,
+                container_cls=container_cls,
+            )
+            container_map[entity_key] = container  # e.g. `self.units_map_by_key[entity_key] = unit`
 
-        for valid_published in containers.get("unit_published", []):
+        for valid_published in containers.get(f"{type_code}_published", []):
             entity_key = valid_published.pop("entity_key")
-            children = self._resolve_children(valid_published, self.components_map_by_key)
+            children = self._resolve_children(valid_published, children_map)
             self.all_published_entities_versions.add(
                 (entity_key, valid_published.get('version_num'))
             )  # Track published version
-            units_api.create_next_unit_version(
-                self.units_map_by_key[entity_key],
+            containers_api.create_next_container_version(
+                container_map[entity_key],
+                **valid_published,  # should this be allowed to override any of the following fields?
                 force_version_num=valid_published.pop("version_num", None),
-                components=children,
+                entities=children,
                 created_by=self.user_id,
-                **valid_published
             )
 
+    def _save_units(self, learning_package, containers):
+        """Save units and published unit versions."""
+        self._save_container(
+            learning_package,
+            containers,
+            container_cls=Unit,
+            container_map=self.units_map_by_key,
+            children_map=self.components_map_by_key,
+        )
+
     def _save_subsections(self, learning_package, containers):
         """Save subsections and published subsection versions."""
-        for valid_subsection in containers.get("subsection", []):
-            entity_key = valid_subsection.get("key")
-            subsection = subsections_api.create_subsection(
-                learning_package.id, created_by=self.user_id, **valid_subsection
-            )
-            self.subsections_map_by_key[entity_key] = subsection
-
-        for valid_published in containers.get("subsection_published", []):
-            entity_key = valid_published.pop("entity_key")
-            children = self._resolve_children(valid_published, self.units_map_by_key)
-            self.all_published_entities_versions.add(
-                (entity_key, valid_published.get('version_num'))
-            )  # Track published version
-            subsections_api.create_next_subsection_version(
-                self.subsections_map_by_key[entity_key],
-                units=children,
-                force_version_num=valid_published.pop("version_num", None),
-                created_by=self.user_id,
-                **valid_published
-            )
+        self._save_container(
+            learning_package,
+            containers,
+            container_cls=Subsection,
+            container_map=self.subsections_map_by_key,
+            children_map=self.units_map_by_key,
+        )
 
     def _save_sections(self, learning_package, containers):
         """Save sections and published section versions."""
-        for valid_section in containers.get("section", []):
-            entity_key = valid_section.get("key")
-            section = sections_api.create_section(learning_package.id, created_by=self.user_id, **valid_section)
-            self.sections_map_by_key[entity_key] = section
-
-        for valid_published in containers.get("section_published", []):
-            entity_key = valid_published.pop("entity_key")
-            children = self._resolve_children(valid_published, self.subsections_map_by_key)
-            self.all_published_entities_versions.add(
-                (entity_key, valid_published.get('version_num'))
-            )  # Track published version
-            sections_api.create_next_section_version(
-                self.sections_map_by_key[entity_key],
-                subsections=children,
-                force_version_num=valid_published.pop("version_num", None),
-                created_by=self.user_id,
-                **valid_published
-            )
+        self._save_container(
+            learning_package,
+            containers,
+            container_cls=Section,
+            container_map=self.sections_map_by_key,
+            children_map=self.subsections_map_by_key,
+        )
 
     def _save_draft_versions(self, components, containers, component_static_files):
         """Save draft versions for all entity types."""
@@ -888,47 +889,29 @@ def _save_draft_versions(self, components, containers, component_static_files):
                 **valid_draft
             )
 
-        for valid_draft in containers.get("unit_drafts", []):
-            entity_key = valid_draft.pop("entity_key")
-            version_num = valid_draft["version_num"]  # Should exist, validated earlier
-            if self._is_version_already_exists(entity_key, version_num):
-                continue
-            children = self._resolve_children(valid_draft, self.components_map_by_key)
-            units_api.create_next_unit_version(
-                self.units_map_by_key[entity_key],
-                components=children,
-                force_version_num=valid_draft.pop("version_num", None),
-                created_by=self.user_id,
-                **valid_draft
-            )
-
-        for valid_draft in containers.get("subsection_drafts", []):
-            entity_key = valid_draft.pop("entity_key")
-            version_num = valid_draft["version_num"]  # Should exist, validated earlier
-            if self._is_version_already_exists(entity_key, version_num):
-                continue
-            children = self._resolve_children(valid_draft, self.units_map_by_key)
-            subsections_api.create_next_subsection_version(
-                self.subsections_map_by_key[entity_key],
-                units=children,
-                force_version_num=valid_draft.pop("version_num", None),
-                created_by=self.user_id,
-                **valid_draft
-            )
+        def _process_draft_containers(
+            container_cls: containers_api.ContainerSubclass,
+            container_map: dict,
+            children_map: dict,
+        ):
+            for valid_draft in containers.get(f"{container_cls.type_code}_drafts", []):
+                entity_key = valid_draft.pop("entity_key")
+                version_num = valid_draft["version_num"]  # Should exist, validated earlier
+                if self._is_version_already_exists(entity_key, version_num):
+                    continue
+                children = self._resolve_children(valid_draft, children_map)
+                del valid_draft["version_num"]
+                containers_api.create_next_container_version(
+                    container_map[entity_key],
+                    **valid_draft,  # should this be allowed to override any of the following fields?
+                    entities=children,
+                    force_version_num=version_num,
+                    created_by=self.user_id,
+                )
 
-        for valid_draft in containers.get("section_drafts", []):
-            entity_key = valid_draft.pop("entity_key")
-            version_num = valid_draft["version_num"]  # Should exist, validated earlier
-            if self._is_version_already_exists(entity_key, version_num):
-                continue
-            children = self._resolve_children(valid_draft, self.subsections_map_by_key)
-            sections_api.create_next_section_version(
-                self.sections_map_by_key[entity_key],
-                subsections=children,
-                force_version_num=valid_draft.pop("version_num", None),
-                created_by=self.user_id,
-                **valid_draft
-            )
+        _process_draft_containers(Unit, self.units_map_by_key, children_map=self.components_map_by_key)
+        _process_draft_containers(Subsection, self.subsections_map_by_key, children_map=self.units_map_by_key)
+        _process_draft_containers(Section, self.sections_map_by_key, children_map=self.subsections_map_by_key)
 
     # --------------------------
     # Utilities

src/openedx_content/applets/collections/api.py

@@ -24,6 +24,7 @@
     "get_collection",
     "get_collections",
     "get_entity_collections",
+    "get_collection_entities",
     "remove_from_collection",
     "restore_collection",
     "update_collection",
@@ -195,6 +196,18 @@ def get_entity_collections(learning_package_id: int, entity_key: str) -> QuerySe
     return entity.collections.filter(enabled=True).order_by("pk")
 
 
+def get_collection_entities(learning_package_id: int, collection_key: str) -> QuerySet[PublishableEntity]:
+    """
+    Returns a QuerySet of PublishableEntities in a Collection.
+
+    This is the same as `collection.entities.all()`
+    """
+    return PublishableEntity.objects.filter(
+        learning_package_id=learning_package_id,
+        collections__key=collection_key,
+    ).order_by("pk")
+
+
 def get_collections(learning_package_id: int, enabled: bool | None = True) -> QuerySet[Collection]:
     """
     Get all collections for a given learning package

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}"