From 5f0823165468dfdae0c6f3eb00d0fcc65cc884cb Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Fri, 14 Feb 2025 15:37:32 -0800 Subject: [PATCH] refactor: Simplify create_next_unit_version() API --- openedx_learning/apps/authoring/units/api.py | 17 ++++++++--- .../apps/authoring/units/test_api.py | 29 +++++-------------- 2 files changed, 21 insertions(+), 25 deletions(-) diff --git a/openedx_learning/apps/authoring/units/api.py b/openedx_learning/apps/authoring/units/api.py index ae1d8ab3..5c854e98 100644 --- a/openedx_learning/apps/authoring/units/api.py +++ b/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. diff --git a/tests/openedx_learning/apps/authoring/units/test_api.py b/tests/openedx_learning/apps/authoring/units/test_api.py index 472bfd54..e7d2e057 100644 --- a/tests/openedx_learning/apps/authoring/units/test_api.py +++ b/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)