diff --git a/openedx_learning/__init__.py b/openedx_learning/__init__.py index 387eb5606..5f85ec83b 100644 --- a/openedx_learning/__init__.py +++ b/openedx_learning/__init__.py @@ -2,4 +2,4 @@ Open edX Learning ("Learning Core"). """ -__version__ = "0.19.1" +__version__ = "0.19.2" diff --git a/openedx_learning/apps/authoring/publishing/api.py b/openedx_learning/apps/authoring/publishing/api.py index 1f24ca672..e9641dc14 100644 --- a/openedx_learning/apps/authoring/publishing/api.py +++ b/openedx_learning/apps/authoring/publishing/api.py @@ -8,6 +8,7 @@ from dataclasses import dataclass from datetime import datetime, timezone +from enum import Enum from typing import TypeVar from django.core.exceptions import ObjectDoesNotExist, ValidationError @@ -73,10 +74,12 @@ "get_container", "get_container_by_key", "get_containers", + "ChildrenEntitiesAction", "ContainerEntityListEntry", "get_entities_in_container", "contains_unpublished_changes", "get_containers_with_entity", + "get_container_children_count", ] @@ -771,6 +774,70 @@ def create_container_version( return container_version +class ChildrenEntitiesAction(Enum): + """Possible actions for children entities""" + + APPEND = "append" + REMOVE = "remove" + REPLACE = "replace" + + +def create_next_entity_list( + learning_package_id: int, + last_version: ContainerVersion, + publishable_entities_pks: list[int], + entity_version_pks: list[int | None] | None, + entities_action: ChildrenEntitiesAction = ChildrenEntitiesAction.REPLACE, +) -> EntityList: + """ + Creates next entity list based on the given entities_action. + + Args: + learning_package_id: Learning package ID + last_version: Last version of container. + publishable_entities_pks: The IDs of the members current members of the container. + entity_version_pks: The IDs of the versions to pin to, if pinning is desired. + entities_action: APPEND, REMOVE or REPLACE given entities from/to the container + + Returns: + The newly created entity list. + """ + if entity_version_pks is None: + entity_version_pks: list[int | None] = [None] * len(publishable_entities_pks) # type: ignore[no-redef] + if entities_action == ChildrenEntitiesAction.APPEND: + # get previous entity list rows + last_entities = last_version.entity_list.entitylistrow_set.only( + "entity_id", + "entity_version_id" + ).order_by("order_num") + # append given publishable_entities_pks and entity_version_pks + publishable_entities_pks = [entity.entity_id for entity in last_entities] + publishable_entities_pks + entity_version_pks = [ # type: ignore[operator, assignment] + entity.entity_version_id + for entity in last_entities + ] + entity_version_pks + elif entities_action == ChildrenEntitiesAction.REMOVE: + # get previous entity list rows + last_entities = last_version.entity_list.entitylistrow_set.only( + "entity_id", + "entity_version_id" + ).order_by("order_num") + # Remove entities that are in publishable_entities_pks + new_entities = [ + entity + for entity in last_entities + if entity.entity_id not in publishable_entities_pks + ] + publishable_entities_pks = [entity.entity_id for entity in new_entities] + entity_version_pks = [entity.entity_version_id for entity in new_entities] + next_entity_list = create_entity_list_with_rows( + entity_pks=publishable_entities_pks, + entity_version_pks=entity_version_pks, # type: ignore[arg-type] + learning_package_id=learning_package_id, + ) + return next_entity_list + + def create_next_container_version( container_pk: int, *, @@ -780,6 +847,7 @@ def create_next_container_version( created: datetime, created_by: int | None, container_version_cls: type[ContainerVersionModel] = ContainerVersion, # type: ignore[assignment] + entities_action: ChildrenEntitiesAction = ChildrenEntitiesAction.REPLACE, ) -> ContainerVersionModel: """ [ 🛑 UNSTABLE ] @@ -815,13 +883,14 @@ def create_next_container_version( # We're only changing metadata. Keep the same entity list. next_entity_list = last_version.entity_list else: - if entity_version_pks is None: - entity_version_pks = [None] * len(publishable_entities_pks) - next_entity_list = create_entity_list_with_rows( - entity_pks=publishable_entities_pks, - entity_version_pks=entity_version_pks, - learning_package_id=entity.learning_package_id, + next_entity_list = create_next_entity_list( + entity.learning_package_id, + last_version, + publishable_entities_pks, + entity_version_pks, + entities_action ) + next_container_version = _create_container_version( container, next_version_num, @@ -1018,3 +1087,29 @@ def get_containers_with_entity( # publishable_entity__draft__version__containerversion__entity_list__in=lists # ) return qs + + +def get_container_children_count( + container: Container, + *, + published: bool, +): + """ + [ 🛑 UNSTABLE ] + Get the count of entities in the current draft or published version of the given container. + + Args: + container: The Container, e.g. returned by `get_container()` + published: `True` if we want the published version of the container, or + `False` for the draft version. + """ + assert isinstance(container, Container) + container_version = container.versioning.published if published else container.versioning.draft + if container_version is None: + raise ContainerVersion.DoesNotExist # This container has not been published yet, or has been deleted. + assert isinstance(container_version, ContainerVersion) + if published: + filter_deleted = {"entity__published__version__isnull": False} + else: + filter_deleted = {"entity__draft__version__isnull": False} + return container_version.entity_list.entitylistrow_set.filter(**filter_deleted).count() diff --git a/openedx_learning/apps/authoring/publishing/models/entity_list.py b/openedx_learning/apps/authoring/publishing/models/entity_list.py index af7d0eca2..19dab09ae 100644 --- a/openedx_learning/apps/authoring/publishing/models/entity_list.py +++ b/openedx_learning/apps/authoring/publishing/models/entity_list.py @@ -60,6 +60,7 @@ class EntityListRow(models.Model): ) class Meta: + ordering = ["order_num"] constraints = [ # If (entity_list, order_num) is not unique, it likely indicates a race condition - so force uniqueness. models.UniqueConstraint( diff --git a/openedx_learning/apps/authoring/units/api.py b/openedx_learning/apps/authoring/units/api.py index 8c6a37d92..6d19306d1 100644 --- a/openedx_learning/apps/authoring/units/api.py +++ b/openedx_learning/apps/authoring/units/api.py @@ -130,6 +130,7 @@ def create_next_unit_version( components: list[Component | ComponentVersion] | None = None, created: datetime, created_by: int | None = None, + entities_action: publishing_api.ChildrenEntitiesAction = publishing_api.ChildrenEntitiesAction.REPLACE, ) -> UnitVersion: """ [ 🛑 UNSTABLE ] Create the next unit version. @@ -151,6 +152,7 @@ def create_next_unit_version( created=created, created_by=created_by, container_version_cls=UnitVersion, + entities_action=entities_action, ) return unit_version diff --git a/tests/openedx_learning/apps/authoring/units/test_api.py b/tests/openedx_learning/apps/authoring/units/test_api.py index 35b59d81b..a07024db9 100644 --- a/tests/openedx_learning/apps/authoring/units/test_api.py +++ b/tests/openedx_learning/apps/authoring/units/test_api.py @@ -412,6 +412,7 @@ def test_add_component_after_publish(self): components=[self.component_1], created=self.now, created_by=None, + entities_action=authoring_api.ChildrenEntitiesAction.APPEND, ) # Now the unit should have unpublished changes: unit.refresh_from_db() # Reloading the unit is necessary @@ -697,8 +698,9 @@ def test_removing_component(self): authoring_api.create_next_unit_version( unit=unit, title="Revised with component 2 deleted", - components=[self.component_1], # component 2 is gone + components=[self.component_2], created=self.now, + entities_action=authoring_api.ChildrenEntitiesAction.REMOVE, ) # Now it should not be listed in the unit: @@ -768,8 +770,9 @@ def test_soft_deleting_and_removing_component(self): authoring_api.create_next_unit_version( unit=unit, title="Revised with component 2 deleted", - components=[self.component_1], + components=[self.component_2], created=self.now, + entities_action=authoring_api.ChildrenEntitiesAction.REMOVE, ) # Now it should not be listed in the unit: @@ -965,6 +968,94 @@ def test_units_containing(self): ] assert result2 == [unit4_unpinned] + def test_add_remove_container_children(self): + """ + Test adding and removing children components from containers. + """ + unit, unit_version = authoring_api.create_unit_and_version( + learning_package_id=self.learning_package.id, + key="unit:key", + title="Unit", + components=[self.component_1], + created=self.now, + created_by=None, + ) + assert authoring_api.get_components_in_unit(unit, published=False) == [ + Entry(self.component_1.versioning.draft), + ] + component_3, _ = self.create_component( + key="Query Counting (3)", + title="Querying Counting Problem (3)", + ) + # Add component_2 and component_3 + unit_version_v2 = authoring_api.create_next_unit_version( + unit=unit, + title=unit_version.title, + components=[self.component_2, component_3], + created=self.now, + created_by=None, + entities_action=authoring_api.ChildrenEntitiesAction.APPEND, + ) + unit.refresh_from_db() + assert unit_version_v2.version_num == 2 + assert unit_version_v2 in unit.versioning.versions.all() + # Verify that component_2 and component_3 is added to end + assert authoring_api.get_components_in_unit(unit, published=False) == [ + Entry(self.component_1.versioning.draft), + Entry(self.component_2.versioning.draft), + Entry(component_3.versioning.draft), + ] + + # Remove component_1 + authoring_api.create_next_unit_version( + unit=unit, + title=unit_version.title, + components=[self.component_1], + created=self.now, + created_by=None, + entities_action=authoring_api.ChildrenEntitiesAction.REMOVE, + ) + unit.refresh_from_db() + # Verify that component_1 is removed + assert authoring_api.get_components_in_unit(unit, published=False) == [ + Entry(self.component_2.versioning.draft), + Entry(component_3.versioning.draft), + ] + + def test_get_container_children_count(self): + """ + Test get_container_children_count() + """ + unit = self.create_unit_with_components([self.component_1]) + assert authoring_api.get_container_children_count(unit.container, published=False) == 1 + # publish + authoring_api.publish_all_drafts(self.learning_package.id) + unit_version = unit.versioning.draft + authoring_api.create_next_unit_version( + unit=unit, + title=unit_version.title, + components=[self.component_2], + created=self.now, + created_by=None, + entities_action=authoring_api.ChildrenEntitiesAction.APPEND, + ) + unit.refresh_from_db() + # Should have two components in draft version and 1 in published version + assert authoring_api.get_container_children_count(unit.container, published=False) == 2 + assert authoring_api.get_container_children_count(unit.container, published=True) == 1 + # publish + authoring_api.publish_all_drafts(self.learning_package.id) + unit.refresh_from_db() + assert authoring_api.get_container_children_count(unit.container, published=True) == 2 + # Soft delete component_1 + authoring_api.soft_delete_draft(self.component_1.pk) + unit.refresh_from_db() + # Should contain only 1 child + assert authoring_api.get_container_children_count(unit.container, published=False) == 1 + authoring_api.publish_all_drafts(self.learning_package.id) + unit.refresh_from_db() + assert authoring_api.get_container_children_count(unit.container, published=True) == 1 + # Tests TODO: # Test that I can get a [PublishLog] history of a given unit and all its children, including children that aren't # currently in the unit and excluding children that are only in other units.