refactor: Simplify EntityList: merge draft_version+published_version -> entity_version

Author: bradenmacdonald

openedx_learning/apps/authoring/containers/api.py

@@ -81,8 +81,7 @@ def create_next_defined_list(
     previous_entity_list: EntityList | None,
     new_entity_list: EntityList,
     entity_pks: list[int],
-    draft_version_pks: list[int | None],
-    published_version_pks: list[int | None],
+    entity_version_pks: list[int | None],
 ) -> EntityListRow:
     """
     Create new entity list rows for an entity list.
@@ -91,8 +90,9 @@ def create_next_defined_list(
         previous_entity_list: The previous entity list that the new entity list is based on.
         new_entity_list: The entity list to create the rows for.
         entity_pks: The IDs of the publishable entities that the entity list rows reference.
-        draft_version_pks: The IDs of the draft versions of the entities (PublishableEntityVersion) that the entity list rows reference.
-        published_version_pks: The IDs of the published versions of the entities (PublishableEntityVersion) that the entity list rows reference.
+        entity_version_pks: The IDs of the draft versions of the entities
+            (PublishableEntityVersion) that the entity list rows reference, or
+            Nones for "unpinned" (default).
 
     Returns:
         The newly created entity list rows.
@@ -109,8 +109,8 @@ def create_next_defined_list(
         current_rows = previous_entity_list.entitylistrow_set.all()
         publishable_entities_in_rows = {row.entity.pk: row for row in current_rows}
         new_rows = []
-        for order_num, entity_pk, draft_version_pk, published_version_pk in zip(
-            order_nums, entity_pks, draft_version_pks, published_version_pks
+        for order_num, entity_pk, entity_version_pk in zip(
+            order_nums, entity_pks, entity_version_pks
         ):
             row = publishable_entities_in_rows.get(entity_pk)
             if row and row.order_num == order_num:
@@ -121,26 +121,25 @@ def create_next_defined_list(
                     entity_list=new_entity_list,
                     entity_id=entity_pk,
                     order_num=order_num,
-                    draft_version_id=draft_version_pk,
-                    published_version_id=published_version_pk,
+                    entity_version_id=entity_version_pk,
                 )
             )
         EntityListRow.objects.bulk_create(new_rows)
     return new_entity_list
 
 def create_defined_list_with_rows(
     entity_pks: list[int],
-    draft_version_pks: list[int | None],
-    published_version_pks: list[int | None],
+    entity_version_pks: list[int | None],
 ) -> EntityList:
     """
     Create new entity list rows for an entity list.
 
     Args:
         entity_list: The entity list to create the rows for.
         entity_pks: The IDs of the publishable entities that the entity list rows reference.
-        draft_version_pks: The IDs of the draft versions of the entities (PublishableEntityVersion) that the entity list rows reference.
-        published_version_pks: The IDs of the published versions of the entities (PublishableEntityVersion) that the entity list rows reference.
+        entity_version_pks: The IDs of the versions of the entities
+            (PublishableEntityVersion) that the entity list rows reference, or
+            Nones for "unpinned" (default).
 
     Returns:
         The newly created entity list.
@@ -154,11 +153,10 @@ def create_defined_list_with_rows(
                     entity_list=entity_list,
                     entity_id=entity_pk,
                     order_num=order_num,
-                    draft_version_id=draft_version_pk,
-                    published_version_id=published_version_pk,
+                    entity_version_id=entity_version_pk,
                 )
-                for order_num, entity_pk, draft_version_pk, published_version_pk in zip(
-                    order_nums, entity_pks, draft_version_pks, published_version_pks
+                for order_num, entity_pk, entity_version_pk in zip(
+                    order_nums, entity_pks, entity_version_pks
                 )
             ]
         )
@@ -186,8 +184,7 @@ def get_entity_list_with_pinned_versions(
                     entity_list=entity_list,
                     entity_id=row.entity.id,
                     order_num=row.order_num,
-                    draft_version_id=None,
-                    published_version_id=None, # For simplicity, we are not copying the pinned versions
+                    entity_version_id=None,  # For simplicity, we are not copying the pinned versions
                 )
                 for row in rows
             ]
@@ -210,7 +207,7 @@ def check_unpinned_versions_in_defined_list(
     """
     # Is there a way to short-circuit this?
     return any(
-        row.draft_version is None or row.published_version is None
+        row.entity_version is None
         for row in defined_list.entitylistrow_set.all()
     )
 
@@ -239,8 +236,7 @@ def create_container_version(
     version_num: int,
     title: str,
     publishable_entities_pk: list[int],
-    draft_version_pks: list[int | None],
-    published_version_pks: list[int | None],
+    entity_version_pks: list[int | None],
     entity: PublishableEntity,
     created: datetime,
     created_by: int | None,
@@ -270,8 +266,7 @@ def create_container_version(
         )
         defined_list = create_defined_list_with_rows(
             entity_pks=publishable_entities_pk,
-            draft_version_pks=draft_version_pks,
-            published_version_pks=published_version_pks,
+            entity_version_pks=entity_version_pks,
         )
         container_version = ContainerEntityVersion.objects.create(
             publishable_entity_version=publishable_entity_version,
@@ -290,8 +285,7 @@ def create_next_container_version(
     container_pk: int,
     title: str,
     publishable_entities_pk: list[int],
-    draft_version_pks: list[int | None],
-    published_version_pks: list[int | None],
+    entity_version_pks: list[int | None],
     entity: PublishableEntity,
     created: datetime,
     created_by: int | None,
@@ -349,8 +343,7 @@ def create_next_container_version(
                 previous_entity_list=last_version.defined_list,
                 new_entity_list=create_entity_list(),
                 entity_pks=publishable_entities_pk,
-                draft_version_pks=draft_version_pks,
-                published_version_pks=published_version_pks,
+                entity_version_pks=entity_version_pks,
             )
             next_initial_list = get_entity_list_with_pinned_versions(
                 rows=next_defined_list.entitylistrow_set.all()
@@ -385,8 +378,7 @@ def create_container_and_version(
     created_by: int | None,
     title: str,
     publishable_entities_pk: list[int],
-    draft_version_pks: list[int | None],
-    published_version_pks: list[int | None],
+    entity_version_pks: list[int | None],
 ) -> ContainerEntityVersion:
     """
     Create a new container and its first version.
@@ -410,8 +402,7 @@ def create_container_and_version(
             version_num=1,
             title=title,
             publishable_entities_pk=publishable_entities_pk,
-            draft_version_pks=draft_version_pks,
-            published_version_pks=published_version_pks,
+            entity_version_pks=entity_version_pks,
             entity=container.publishable_entity,
             created=created,
             created_by=created_by,
@@ -507,8 +498,8 @@ def get_entities_in_draft_container(
     defined_list = container.versioning.draft.defined_list
     for row in defined_list.entitylistrow_set.order_by("order_num"):
         entity_list.append(ContainerEntityListEntry(
-            entity_version=row.draft_version or row.entity.draft.version,
-            pinned=row.draft_version is not None,
+            entity_version=row.entity_version or row.entity.draft.version,
+            pinned=row.entity_version is not None,
         ))
     return entity_list
 
@@ -535,8 +526,8 @@ def get_entities_in_published_container(
     entity_list = []
     for row in cev.defined_list.entitylistrow_set.order_by("order_num"):
         entity_list.append(ContainerEntityListEntry(
-            entity_version=row.published_version or row.entity.published.version,
-            pinned=row.published_version is not None,
+            entity_version=row.entity_version or row.entity.published.version,
+            pinned=row.entity_version is not None,
         ))
     return entity_list
 
@@ -572,7 +563,7 @@ def contains_unpublished_changes(
     # TODO: This is a naive inefficient implementation but hopefully correct.
     # Once we know it's correct and have a good test suite, then we can optimize.
     # We will likely change to a tracking-based approach rather than a "scan for changes" based approach.
-    for row in defined_list.entitylistrow_set.filter(draft_version=None).select_related(
+    for row in defined_list.entitylistrow_set.filter(entity_version=None).select_related(
         "entity__containerentity",
         "entity__draft__version",
         "entity__published__version",

openedx_learning/apps/authoring/containers/migrations/0001_initial.py

@@ -1,4 +1,4 @@
-# Generated by Django 4.2.16 on 2024-10-29 12:57
+# Generated by Django 4.2.19 on 2025-02-14 23:04
 
 from django.db import migrations, models
 import django.db.models.deletion
@@ -33,10 +33,9 @@ class Migration(migrations.Migration):
             fields=[
                 ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
                 ('order_num', models.PositiveIntegerField()),
-                ('draft_version', models.ForeignKey(null=True, on_delete=django.db.models.deletion.RESTRICT, related_name='draft_version', to='oel_publishing.publishableentityversion')),
                 ('entity', models.ForeignKey(on_delete=django.db.models.deletion.RESTRICT, to='oel_publishing.publishableentity')),
                 ('entity_list', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='oel_containers.entitylist')),
-                ('published_version', models.ForeignKey(null=True, on_delete=django.db.models.deletion.RESTRICT, related_name='published_version', to='oel_publishing.publishableentityversion')),
+                ('entity_version', models.ForeignKey(null=True, on_delete=django.db.models.deletion.RESTRICT, related_name='+', to='oel_publishing.publishableentityversion')),
             ],
         ),
         migrations.CreateModel(

openedx_learning/apps/authoring/containers/models.py

@@ -58,17 +58,11 @@ class EntityListRow(models.Model):
     # So our approach to this is to use a value of None (null) to represent an
     # unpinned reference to a PublishableEntity. It's shorthand for "just use
     # the latest draft or published version of this, as appropriate".
-    draft_version = models.ForeignKey(
+    entity_version = models.ForeignKey(
         PublishableEntityVersion,
         on_delete=models.RESTRICT,
         null=True,
-        related_name="draft_version",
-    )
-    published_version = models.ForeignKey(
-        PublishableEntityVersion,
-        on_delete=models.RESTRICT,
-        null=True,
-        related_name="published_version",
+        related_name="+",  # Do we need the reverse relation?
     )
 
 

openedx_learning/apps/authoring/units/api.py

@@ -60,8 +60,7 @@ def create_unit_version(
     version_num: int,
     title: str,
     publishable_entities_pks: list[int],
-    draft_version_pks: list[int | None],
-    published_version_pks: list[int | None],
+    entity_version_pks: list[int | None],
     created: datetime,
     created_by: int | None = None,
 ) -> Unit:
@@ -82,8 +81,7 @@ def create_unit_version(
             version_num,
             title,
             publishable_entities_pks,
-            draft_version_pks,
-            published_version_pks,
+            entity_version_pks,
             unit.container_entity.publishable_entity,
             created,
             created_by,
@@ -100,8 +98,7 @@ def create_next_unit_version(
     unit: Unit,
     title: str,
     publishable_entities_pks: list[int],
-    draft_version_pks: list[int | None],
-    published_version_pks: list[int | None],
+    entity_version_pks: list[int | None],
     created: datetime,
     created_by: int | None = None,
 ) -> Unit:
@@ -122,8 +119,7 @@ def create_next_unit_version(
             unit.container_entity.pk,
             title,
             publishable_entities_pks,
-            draft_version_pks,
-            published_version_pks,
+            entity_version_pks,
             unit.container_entity.publishable_entity,
             created,
             created_by,
@@ -157,11 +153,10 @@ def create_unit_and_version(
             unit,
             1,
             title,
-            [],
-            [],
-            [],
-            created,
-            created_by,
+            publishable_entities_pks=[],
+            entity_version_pks=[],
+            created=created,
+            created_by=created_by,
         )
     return unit, unit_version
 

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

@@ -81,8 +81,7 @@ def test_create_next_unit_version_with_two_components(self):
                 self.component_1.publishable_entity.id,
                 self.component_2.publishable_entity.id,
             ],
-            draft_version_pks=[None, None],
-            published_version_pks=[None, None],  # FIXME: why do we specify this?
+            entity_version_pks=[None, None],
             created=self.now,
             created_by=None,
         )
@@ -122,8 +121,7 @@ def test_add_component_after_publish(self):
             publishable_entities_pks=[
                 self.component_1.publishable_entity.id,
             ],
-            draft_version_pks=[None],
-            published_version_pks=[None],  # FIXME: why do we specify this?
+            entity_version_pks=[None],
             created=self.now,
             created_by=None,
         )
@@ -157,8 +155,7 @@ def test_modify_component_after_publish(self):
             publishable_entities_pks=[
                 self.component_1.publishable_entity.id,
             ],
-            draft_version_pks=[None],
-            published_version_pks=[None],  # FIXME: why do we specify this?
+            entity_version_pks=[None],
             created=self.now,
             created_by=None,
         )
@@ -233,8 +230,7 @@ def test_query_count_of_contains_unpublished_changes(self):
             unit=unit,
             title=unit_version.title,
             publishable_entities_pks=publishable_entities_pks,
-            draft_version_pks=[None] * component_count,
-            published_version_pks=[None] * component_count,  # FIXME: why do we specify this?
+            entity_version_pks=[None] * component_count,
             created=self.now,
         )
         authoring_api.publish_all_drafts(self.learning_package.id)