refactor: Simplify create_next_unit_version() API

Author: bradenmacdonald

openedx_learning/apps/authoring/units/api.py

@@ -6,7 +6,7 @@
 
 from django.db.transaction import atomic
 
-from openedx_learning.apps.authoring.components.models import ComponentVersion
+from openedx_learning.apps.authoring.components.models import Component, ComponentVersion
 from openedx_learning.apps.authoring.containers.models import EntityListRow
 from ..publishing import api as publishing_api
 from ..containers import api as container_api
@@ -97,8 +97,7 @@ def create_unit_version(
 def create_next_unit_version(
     unit: Unit,
     title: str,
-    publishable_entities_pks: list[int],
-    entity_version_pks: list[int | None],
+    components: list[Component | ComponentVersion],
     created: datetime,
     created_by: int | None = None,
 ) -> Unit:
@@ -107,11 +106,21 @@ def create_next_unit_version(
     Args:
         unit_pk: The unit ID.
         title: The title.
-        publishable_entities_pk: The components.
+        components: The components, as a list of Components (unpinned) and/or ComponentVersions (pinned)
         entity: The entity.
         created: The creation date.
         created_by: The user who created the unit.
     """
+    for c in components:
+        assert isinstance(c, (Component, ComponentVersion))
+    publishable_entities_pks = [
+        (c.publishable_entity_id if isinstance(c, Component) else c.component.publishable_entity_id)
+        for c in components
+    ]
+    entity_version_pks = [
+        (cv.pk if isinstance(cv, ComponentVersion) else None)
+        for cv in components
+    ]
     with atomic():
         # TODO: how can we enforce that publishable entities must be components?
         # This currently allows for any publishable entity to be added to a unit.

tests/openedx_learning/apps/authoring/units/test_api.py

@@ -57,15 +57,13 @@ def test_create_empty_unit_and_version(self):
         assert unit.versioning.published is None
 
     def test_create_next_unit_version_with_two_components(self):
-        """Test creating a unit version with two components.
+        """Test creating a unit version with two unpinned components.
 
         Expected results:
         1. A new unit version is created.
         2. The unit version number is 2.
         3. The unit version is in the unit's versions.
-        4. The components are in the unit version's user defined list.
-        5. Initial list contains the pinned versions of the defined list.
-        6. Frozen list is empty.
+        4. The components are in the draft unit version's component list and are unpinned.
         """
         unit, unit_version = authoring_api.create_unit_and_version(
             learning_package_id=self.learning_package.id,
@@ -77,11 +75,7 @@ def test_create_next_unit_version_with_two_components(self):
         unit_version_v2 = authoring_api.create_next_unit_version(
             unit=unit,
             title="Unit",
-            publishable_entities_pks=[
-                self.component_1.publishable_entity.id,
-                self.component_2.publishable_entity.id,
-            ],
-            entity_version_pks=[None, None],
+            components=[self.component_1, self.component_2],
             created=self.now,
             created_by=None,
         )
@@ -118,10 +112,7 @@ def test_add_component_after_publish(self):
         unit_version_v2 = authoring_api.create_next_unit_version(
             unit=unit,
             title=unit_version.title,
-            publishable_entities_pks=[
-                self.component_1.publishable_entity.id,
-            ],
-            entity_version_pks=[None],
+            components=[self.component_1],
             created=self.now,
             created_by=None,
         )
@@ -152,10 +143,7 @@ def test_modify_component_after_publish(self):
         unit_version_v2 = authoring_api.create_next_unit_version(
             unit=unit,
             title=unit_version.title,
-            publishable_entities_pks=[
-                self.component_1.publishable_entity.id,
-            ],
-            entity_version_pks=[None],
+            components=[self.component_1],
             created=self.now,
             created_by=None,
         )
@@ -216,7 +204,7 @@ def test_query_count_of_contains_unpublished_changes(self):
         )
         # Add 100 components (unpinned)
         component_count = 100
-        publishable_entities_pks = []
+        components = []
         for i in range(0, component_count):
             component, _version = authoring_api.create_component_and_version(
                 self.learning_package.id,
@@ -225,12 +213,11 @@ def test_query_count_of_contains_unpublished_changes(self):
                 title=f"Querying Counting Problem {i}",
                 created=self.now,
             )
-            publishable_entities_pks.append(component.publishable_entity_id)
+            components.append(component)
         authoring_api.create_next_unit_version(
             unit=unit,
             title=unit_version.title,
-            publishable_entities_pks=publishable_entities_pks,
-            entity_version_pks=[None] * component_count,
+            components=components,
             created=self.now,
         )
         authoring_api.publish_all_drafts(self.learning_package.id)