refactor: make create functions more DRY, remove redundant 'entity' arg

Author: bradenmacdonald

openedx_learning/apps/authoring/containers/api.py

@@ -110,35 +110,62 @@ def create_entity_list_with_rows(
     return entity_list
 
 
-def get_entity_list_with_pinned_versions(
-    rows: QuerySet[EntityListRow],
-) -> EntityList:
+def _create_container_version(
+    container_pk: int,
+    *,
+    title: str,
+    publishable_entities_pks: list[int],
+    entity_version_pks: list[int | None],
+    created: datetime,
+    created_by: int | None,
+    force_version_num: int | None = None,
+) -> ContainerEntityVersion:
     """
-    [ 🛑 UNSTABLE ]
-    Copy rows from an existing entity list to a new entity list.
+    Helper method used by create_container_version() and
+    create_next_container_version().
 
-    Args:
-        entity_list: The entity list to copy the rows to.
-        rows: The rows to copy to the new entity list.
+    Almost always, either force_version_num = 1 to create the initial version,
+    or force_version_num is omitted to create the "next" version in sequence.
 
-    Returns:
-        The newly created entity list.
+    However, all our APIs do allow creating arbitrary version numbers, perhaps
+    for import/export purposes.
     """
-    entity_list = create_entity_list()
     with atomic():
-        _ = EntityListRow.objects.bulk_create(
-            [
-                EntityListRow(
-                    entity_list=entity_list,
-                    entity_id=row.entity.id,
-                    order_num=row.order_num,
-                    entity_version_id=None,  # For simplicity, we are not copying the pinned versions
-                )
-                for row in rows
-            ]
+        container = ContainerEntity.objects.select_related("publishable_entity").get(pk=container_pk)
+        entity = container.publishable_entity
+
+        # Do a quick check that the given entities are in the right learning package:
+        if PublishableEntity.objects.filter(
+            pk__in=publishable_entities_pks,
+        ).exclude(
+            learning_package_id=entity.learning_package_id,
+        ).exists():
+            raise ValidationError("Container entities must be from the same learning package.")
+
+        if force_version_num is not None:
+            next_version_num = force_version_num
+        else:
+            last_version = container.versioning.latest
+            assert last_version is not None
+            next_version_num = last_version.version_num + 1
+        publishable_entity_version = publishing_api.create_publishable_entity_version(
+            entity.pk,
+            version_num=next_version_num,
+            title=title,
+            created=created,
+            created_by=created_by,
+        )
+        next_entity_list = create_entity_list_with_rows(
+            entity_pks=publishable_entities_pks,
+            entity_version_pks=entity_version_pks,
+        )
+        next_container_version = ContainerEntityVersion.objects.create(
+            publishable_entity_version=publishable_entity_version,
+            container_id=container_pk,
+            entity_list=next_entity_list,
         )
 
-    return entity_list
+    return next_container_version
 
 
 def create_container_version(
@@ -148,7 +175,6 @@ def create_container_version(
     title: str,
     publishable_entities_pks: list[int],
     entity_version_pks: list[int | None],
-    entity: PublishableEntity,
     created: datetime,
     created_by: int | None,
 ) -> ContainerEntityVersion:
@@ -161,31 +187,21 @@ def create_container_version(
         version_num: The version number of the container.
         title: The title of the container.
         publishable_entities_pks: The IDs of the members of the container.
-        entity: The entity that the container belongs to.
         created: The date and time the container version was created.
         created_by: The ID of the user who created the container version.
 
     Returns:
         The newly created container version.
     """
-    with atomic():
-        publishable_entity_version = publishing_api.create_publishable_entity_version(
-            entity.pk,
-            version_num=version_num,
-            title=title,
-            created=created,
-            created_by=created_by,
-        )
-        entity_list = create_entity_list_with_rows(
-            entity_pks=publishable_entities_pks,
-            entity_version_pks=entity_version_pks,
-        )
-        container_version = ContainerEntityVersion.objects.create(
-            publishable_entity_version=publishable_entity_version,
-            container_id=container_pk,
-            entity_list=entity_list,
-        )
-    return container_version
+    return _create_container_version(
+        container_pk,
+        title=title,
+        publishable_entities_pks=publishable_entities_pks,
+        entity_version_pks=entity_version_pks,
+        created=created,
+        created_by=created_by,
+        force_version_num=version_num,
+    )
 
 
 def create_next_container_version(
@@ -194,7 +210,6 @@ def create_next_container_version(
     title: str,
     publishable_entities_pks: list[int],
     entity_version_pks: list[int | None],
-    entity: PublishableEntity,
     created: datetime,
     created_by: int | None,
 ) -> ContainerEntityVersion:
@@ -213,42 +228,20 @@ def create_next_container_version(
         container_pk: The ID of the container to create the next version of.
         title: The title of the container.
         publishable_entities_pks: The IDs of the members current members of the container.
-        entity: The entity that the container belongs to.
         created: The date and time the container version was created.
         created_by: The ID of the user who created the container version.
 
     Returns:
         The newly created container version.
     """
-    # Do a quick check that the given entities are in the right learning package:
-    if PublishableEntity.objects.filter(
-        pk__in=publishable_entities_pks,
-    ).exclude(
-        learning_package_id=entity.learning_package_id,
-    ).exists():
-        raise ValidationError("Container entities must be from the same learning package.")
-    with atomic():
-        container = ContainerEntity.objects.get(pk=container_pk)
-        last_version = container.versioning.latest
-        next_version_num = last_version.version_num + 1
-        publishable_entity_version = publishing_api.create_publishable_entity_version(
-            entity.pk,
-            version_num=next_version_num,
-            title=title,
-            created=created,
-            created_by=created_by,
-        )
-        next_entity_list = create_entity_list_with_rows(
-            entity_pks=publishable_entities_pks,
-            entity_version_pks=entity_version_pks,
-        )
-        next_container_version = ContainerEntityVersion.objects.create(
-            publishable_entity_version=publishable_entity_version,
-            container_id=container_pk,
-            entity_list=next_entity_list,
-        )
-
-    return next_container_version
+    return _create_container_version(
+        container_pk,
+        title=title,
+        publishable_entities_pks=publishable_entities_pks,
+        entity_version_pks=entity_version_pks,
+        created=created,
+        created_by=created_by,
+    )
 
 
 def create_container_and_version(
@@ -280,12 +273,11 @@ def create_container_and_version(
     with atomic():
         container = create_container(learning_package_id, key, created, created_by)
         container_version = create_container_version(
-            container_pk=container.publishable_entity.pk,
-            version_num=1,
+            container.publishable_entity.pk,
+            1,
             title=title,
             publishable_entities_pks=publishable_entities_pks,
             entity_version_pks=entity_version_pks,
-            entity=container.publishable_entity,
             created=created,
             created_by=created_by,
         )

openedx_learning/apps/authoring/units/api.py

@@ -82,7 +82,6 @@ def create_unit_version(
             title=title,
             publishable_entities_pks=publishable_entities_pks,
             entity_version_pks=entity_version_pks,
-            entity=unit.container_entity.publishable_entity,
             created=created,
             created_by=created_by,
         )
@@ -131,7 +130,6 @@ def create_next_unit_version(
             title=title,
             publishable_entities_pks=publishable_entities_pks,
             entity_version_pks=entity_version_pks,
-            entity=unit.container_entity.publishable_entity,
             created=created,
             created_by=created_by,
         )