refactor: pin versions in frozen list each time a new version is created

Author: mariajgrimaldi

openedx_learning/apps/authoring/containers/api.py

@@ -70,34 +70,42 @@ def create_entity_list() -> EntityList:
     return EntityList.objects.create()
 
 
-def create_entity_list_row(
+def create_entity_list_rows(
     entity_list: EntityList,
-    entity_pk: int,
-    order_num: int,
-    draft_version_pk: int | None,
-    published_version_pk: int | None,
+    entity_pks: list[int],
+    draft_version_pks: list[int | None],
+    published_version_pks: list[int | None],
 ) -> EntityListRow:
     """
-    Create a new entity list row. This is a row in an entity list that references
-    publishable entities.
+    Create new entity list rows for an entity list.
 
     Args:
-        entity_list: The entity list that the entity list row belongs to.
-        entity: The ID of the publishable entity that the entity list row references.
-        order_num: The order_num of the entity list row in the entity list.
-        draft_version_pk: The ID of the draft version of the entity (PublishableEntityVersion) that the entity list row references.
-        published_version_pk: The ID of the published version of the entity (PublishableEntityVersion) that the entity list row references
+        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.
 
     Returns:
-        The newly created entity list row.
+        The newly created entity list rows.
     """
-    return EntityListRow.objects.create(
-        entity_list=entity_list,
-        entity_id=entity_pk,
-        order_num=order_num,
-        draft_version_id=draft_version_pk,
-        published_version_id=published_version_pk,
-    )
+    order_nums = range(len(entity_pks))
+    with atomic():
+        entity_list_rows = EntityListRow.objects.bulk_create(
+            [
+                EntityListRow(
+                    entity_list=entity_list,
+                    entity_id=entity_pk,
+                    order_num=order_num,
+                    draft_version_id=draft_version_pk,
+                    published_version_id=published_version_pk,
+                )
+                for entity_pk, order_num, draft_version_pk, published_version_pk in zip(
+                    entity_pks, order_nums, draft_version_pks, published_version_pks
+                )
+            ]
+        )
+
+    return entity_list_rows
 
 
 def create_entity_list_with_rows(
@@ -109,26 +117,55 @@ def create_entity_list_with_rows(
     Create a new entity list with rows.
 
     Args:
-        entity_pks: The IDs of the publishable entities that the entity list rows reference.
+        entity_pks: The IDs of the publishable entities that the entity list rows
+        reference.
         order_nums: The order_nums of the entity list rows in the entity list.
-        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.
+        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.
 
     Returns:
         The newly created entity list.
     """
     entity_list = create_entity_list()
-    order_nums = range(len(entity_pks))
-    for entity_pk, order_num, draft_version_pk, published_version_pk in zip(
-        entity_pks, order_nums, draft_version_pks, published_version_pks
-    ):
-        create_entity_list_row(
-            entity_list=entity_list,
-            entity_pk=entity_pk,
-            order_num=order_num,
-            draft_version_pk=draft_version_pk,
-            published_version_pk=published_version_pk,
+    create_entity_list_rows(
+        entity_list=entity_list,
+        entity_pks=entity_pks,
+        draft_version_pks=draft_version_pks,
+        published_version_pks=published_version_pks,
+    )
+    return entity_list
+
+
+def copy_rows_to_new_entity_list(
+    rows: QuerySet[EntityListRow],
+) -> EntityList:
+    """
+    Copy rows from an existing entity list to a new entity list.
+
+    Args:
+        entity_list: The entity list to copy the rows to.
+        rows: The rows to copy to the new entity list.
+
+    Returns:
+        The newly created entity list.
+    """
+    entity_list = create_entity_list()
+    with atomic():
+        entity_list_rows = EntityListRow.objects.bulk_create(
+            [
+                EntityListRow(
+                    entity_list=entity_list,
+                    entity_id=row.entity.id,
+                    order_num=row.order_num,
+                    draft_version_id=row.entity.draft.version.pk,
+                    published_version_id=row.entity.published.version.pk,
+                )
+                for row in rows
+            ]
         )
+
     return entity_list
 
 
@@ -137,6 +174,8 @@ 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: PublishableEntity,
     created: datetime,
     created_by: int | None,
@@ -164,20 +203,17 @@ def create_container_version(
             created=created,
             created_by=created_by,
         )
-        # This implementation assumes:
-        # 1. We are creating the first version of the container, so the defined list is the same as the initial list.
-        # 2. The frozen list is empty because this is the first version.
-        # 3. Published and draft versions are always the latest for all members.
         entity_list = create_entity_list_with_rows(
             entity_pks=publishable_entities_pk,
-            draft_version_pks=[None] * len(publishable_entities_pk),
-            published_version_pks=[None] * len(publishable_entities_pk),
+            draft_version_pks=draft_version_pks,
+            published_version_pks=published_version_pks,
         )
         container_version = ContainerEntityVersion.objects.create(
             publishable_entity_version=publishable_entity_version,
             container_id=container_pk,
             defined_list=entity_list,
             initial_list=entity_list,
+            # Would frozen_list be always None the 1st time a container is created?
             frozen_list=None,
         )
     return container_version
@@ -187,6 +223,8 @@ 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: PublishableEntity,
     created: datetime,
     created_by: int | None,
@@ -216,23 +254,26 @@ def create_next_container_version(
             created=created,
             created_by=created_by,
         )
-        # This implementation assumes:
-        # 1. The changes provoking a new version are the addition, removal of members or reordering.
+        # 1. The changes provoking a new version are always the addition, removal of members or reordering. How can we detect changes only in the metadata?
         # 2. Published and draft versions are always the latest for all members.
         # 3. When creating a new version, a new user-defined entity list is created to preserve the latest state as the previous user-defined list.
-        # TODO: instead consider copying the previous user-defined list as the frozen list, and add/remove to the previous user-defined list.
-        # If it's a reordering, the previous user-defined list is copied as the frozen and a new user-defined list is created with the new order.
+        # 4. When creating a new version, a new frozen entity list is created copying the state of the user-defined list of the previous version.
+        # 5. While copying the versions into the new frozen list, versions are pinned in new rows for published/draft versions.
         new_user_defined_list = create_entity_list_with_rows(
             entity_pks=publishable_entities_pk,
-            draft_version_pks=[None] * len(publishable_entities_pk),
-            published_version_pks=[None] * len(publishable_entities_pk),
+            draft_version_pks=draft_version_pks,
+            published_version_pks=published_version_pks,
         )
+        new_frozen_list = copy_rows_to_new_entity_list(
+            rows=get_defined_list_rows_for_container_version(last_version)
+        )
+
         container_version = ContainerEntityVersion.objects.create(
             publishable_entity_version=publishable_entity_version,
             container_id=container_pk,
             defined_list=new_user_defined_list,
             initial_list=last_version.initial_list,
-            frozen_list=last_version.defined_list,
+            frozen_list=new_frozen_list,
         )
     return container_version
 
@@ -244,6 +285,8 @@ 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],
 ) -> ContainerEntityVersion:
     """
     Create a new container and its first version.
@@ -267,6 +310,8 @@ 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=container.publishable_entity,
             created=created,
             created_by=created_by,
@@ -283,13 +328,12 @@ def get_container(pk: int) -> ContainerEntity:
 
     Returns:
         The container with the given primary key.
-
-    TODO: should this use with_publishing_relations as in components?
     """
+    # TODO: should this use with_publishing_relations as in components?
     return ContainerEntity.objects.get(pk=pk)
 
 
-def get_defined_list_for_container_version(
+def get_defined_list_rows_for_container_version(
     container_version: ContainerEntityVersion,
 ) -> QuerySet[EntityListRow]:
     """
@@ -304,7 +348,7 @@ def get_defined_list_for_container_version(
     return container_version.defined_list.entitylistrow_set.all()
 
 
-def get_initial_list_for_container_version(
+def get_initial_list_rows_for_container_version(
     container_version: ContainerEntityVersion,
 ) -> QuerySet[EntityListRow]:
     """
@@ -319,7 +363,7 @@ def get_initial_list_for_container_version(
     return container_version.initial_list.entitylistrow_set.all()
 
 
-def get_frozen_list_for_container_version(
+def get_frozen_list_rows_for_container_version(
     container_version: ContainerEntityVersion,
 ) -> QuerySet[EntityListRow]:
     """
@@ -331,4 +375,6 @@ def get_frozen_list_for_container_version(
     Returns:
         The frozen members of the container version.
     """
+    if container_version.frozen_list is None:
+        return QuerySet[EntityListRow]()
     return container_version.frozen_list.entitylistrow_set.all()

openedx_learning/apps/authoring/units/api.py

@@ -54,7 +54,9 @@ def create_unit_version(
     unit: Unit,
     version_num: int,
     title: str,
-    publishable_entities_pk: list[int],
+    publishable_entities_pks: list[int],
+    draft_version_pks: list[int | None],
+    published_version_pks: list[int | None],
     created: datetime,
     created_by: int | None,
 ) -> Unit:
@@ -74,7 +76,9 @@ def create_unit_version(
             unit.container_entity.pk,
             version_num,
             title,
-            publishable_entities_pk,
+            publishable_entities_pks,
+            draft_version_pks,
+            published_version_pks,
             unit.container_entity.publishable_entity,
             created,
             created_by,
@@ -90,7 +94,9 @@ def create_unit_version(
 def create_next_unit_version(
     unit: Unit,
     title: str,
-    publishable_entities_pk: list[int],
+    publishable_entities_pks: list[int],
+    draft_version_pks: list[int | None],
+    published_version_pks: list[int | None],
     created: datetime,
     created_by: int | None,
 ) -> Unit:
@@ -110,7 +116,9 @@ def create_next_unit_version(
         container_entity_version = container_api.create_next_container_version(
             unit.container_entity.pk,
             title,
-            publishable_entities_pk,
+            publishable_entities_pks,
+            draft_version_pks,
+            published_version_pks,
             unit.container_entity.publishable_entity,
             created,
             created_by,