From d550aabcfed6f7b968d0ce6a0b8c0d309f5019cb Mon Sep 17 00:00:00 2001 From: Jillian Date: Tue, 24 Sep 2024 13:07:03 +0930 Subject: [PATCH] Collections Django Admin + soft delete, hard delete, and restore (#229) * feat: adds delete_collection and restore_collection to api * feat: adds Django Admin for Collection model * feat: adds get_collection_components to the authoring api * chore: bumps version to 0.11.0 * chore: adds import-lint to quality make target --- Makefile | 1 + openedx_learning/__init__.py | 2 +- .../apps/authoring/collections/admin.py | 27 +++ .../apps/authoring/collections/api.py | 38 ++++ .../apps/authoring/components/api.py | 16 ++ .../apps/authoring/collections/test_api.py | 182 +++++++++++++----- 6 files changed, 221 insertions(+), 45 deletions(-) create mode 100644 openedx_learning/apps/authoring/collections/admin.py diff --git a/Makefile b/Makefile index 379aff15..7ae9b982 100644 --- a/Makefile +++ b/Makefile @@ -55,6 +55,7 @@ upgrade: ## update the pip requirements files to use the latest releases satisf quality: ## check coding style with pycodestyle and pylint tox -e quality + lint-imports pii_check: ## check for PII annotations on all Django models tox -e pii_check diff --git a/openedx_learning/__init__.py b/openedx_learning/__init__.py index 73623346..31d6d241 100644 --- a/openedx_learning/__init__.py +++ b/openedx_learning/__init__.py @@ -1,4 +1,4 @@ """ Open edX Learning ("Learning Core"). """ -__version__ = "0.12.0" +__version__ = "0.13.0" diff --git a/openedx_learning/apps/authoring/collections/admin.py b/openedx_learning/apps/authoring/collections/admin.py new file mode 100644 index 00000000..0d49732c --- /dev/null +++ b/openedx_learning/apps/authoring/collections/admin.py @@ -0,0 +1,27 @@ +""" +Django Admin pages for Collection models. +""" +from django.contrib import admin + +from .models import Collection + + +class CollectionAdmin(admin.ModelAdmin): + """ + The Collection model admin. + + Allows users to easily disable/enable (aka soft delete and restore) or bulk delete Collections. + """ + readonly_fields = ["key", "learning_package"] + list_filter = ["enabled"] + list_display = ["key", "title", "enabled", "modified"] + list_editable = ["enabled"] + + def has_add_permission(self, request, *args, **kwargs): + """ + Disallow adding new collections via Django Admin. + """ + return False # pragma: no cover + + +admin.site.register(Collection, CollectionAdmin) diff --git a/openedx_learning/apps/authoring/collections/api.py b/openedx_learning/apps/authoring/collections/api.py index 1b366b5a..07b7a2ea 100644 --- a/openedx_learning/apps/authoring/collections/api.py +++ b/openedx_learning/apps/authoring/collections/api.py @@ -20,10 +20,12 @@ __all__ = [ "add_to_collection", "create_collection", + "delete_collection", "get_collection", "get_collections", "get_entity_collections", "remove_from_collection", + "restore_collection", "update_collection", ] @@ -84,6 +86,42 @@ def update_collection( return collection +def delete_collection( + learning_package_id: int, + key: str, + *, + hard_delete=False, +) -> Collection: + """ + Disables or deletes a collection identified by the given learning_package + key. + + By default (hard_delete=False), the collection is "soft deleted", i.e disabled. + Soft-deleted collections can be re-enabled using restore_collection. + """ + collection = get_collection(learning_package_id, key) + + if hard_delete: + collection.delete() + else: + collection.enabled = False + collection.save() + return collection + + +def restore_collection( + learning_package_id: int, + key: str, +) -> Collection: + """ + Undo a "soft delete" by re-enabling a Collection. + """ + collection = get_collection(learning_package_id, key) + + collection.enabled = True + collection.save() + return collection + + def add_to_collection( learning_package_id: int, key: str, diff --git a/openedx_learning/apps/authoring/components/api.py b/openedx_learning/apps/authoring/components/api.py index 3fd37a56..a65eee80 100644 --- a/openedx_learning/apps/authoring/components/api.py +++ b/openedx_learning/apps/authoring/components/api.py @@ -42,6 +42,7 @@ "get_component_by_uuid", "get_component_version_by_uuid", "component_exists_by_key", + "get_collection_components", "get_components", "create_component_version_content", "look_up_component_version_content", @@ -339,6 +340,21 @@ def get_components( return qset +def get_collection_components( + learning_package_id: int, + collection_key: str, +) -> QuerySet[Component]: + """ + Returns a QuerySet of Components relating to the PublishableEntities in a Collection. + + Components have a one-to-one relationship with PublishableEntity, but the reverse may not always be true. + """ + return Component.objects.filter( + learning_package_id=learning_package_id, + publishable_entity__collections__key=collection_key, + ).order_by('pk') + + def look_up_component_version_content( learning_package_key: str, component_key: str, diff --git a/tests/openedx_learning/apps/authoring/collections/test_api.py b/tests/openedx_learning/apps/authoring/collections/test_api.py index 8407537c..179869a3 100644 --- a/tests/openedx_learning/apps/authoring/collections/test_api.py +++ b/tests/openedx_learning/apps/authoring/collections/test_api.py @@ -12,9 +12,10 @@ from openedx_learning.api.authoring_models import ( Collection, CollectionPublishableEntity, + Component, + ComponentType, LearningPackage, PublishableEntity, - PublishableEntityVersion, ) from openedx_learning.lib.test_utils import TestCase @@ -208,13 +209,13 @@ def test_create_collection_without_description(self): class CollectionEntitiesTestCase(CollectionsTestCase): """ - Test collections that contain publishable entitites. + Base class with collections that contain components. """ - published_entity: PublishableEntity - pe_version: PublishableEntityVersion - draft_entity: PublishableEntity - de_version: PublishableEntityVersion + published_component: Component + draft_component: Component user: User # type: ignore [valid-type] + html_type: ComponentType + problem_type: ComponentType @classmethod def setUpTestData(cls) -> None: @@ -228,17 +229,15 @@ def setUpTestData(cls) -> None: email="user@example.com", ) - # Make and Publish one PublishableEntity - cls.published_entity = api.create_publishable_entity( + cls.html_type = api.get_or_create_component_type("xblock.v1", "html") + cls.problem_type = api.get_or_create_component_type("xblock.v1", "problem") + + # Make and publish one Component + cls.published_component, _ = api.create_component_and_version( cls.learning_package.id, - key="my_entity_published_example", - created=cls.now, - created_by=cls.user.id, - ) - cls.pe_version = api.create_publishable_entity_version( - cls.published_entity.id, - version_num=1, - title="An Entity that we'll Publish 🌴", + cls.problem_type, + local_key="my_published_example", + title="My published problem", created=cls.now, created_by=cls.user.id, ) @@ -248,27 +247,22 @@ def setUpTestData(cls) -> None: published_at=cls.now, ) - # Create two Draft PublishableEntities, one in each learning package - cls.draft_entity = api.create_publishable_entity( + # Create a Draft component, one in each learning package + cls.draft_component, _ = api.create_component_and_version( cls.learning_package.id, - key="my_entity_draft_example", - created=cls.now, - created_by=cls.user.id, - ) - cls.de_version = api.create_publishable_entity_version( - cls.draft_entity.id, - version_num=1, - title="An Entity that we'll keep in Draft 🌴", + cls.html_type, + local_key="my_draft_example", + title="My draft html", created=cls.now, created_by=cls.user.id, ) - # Add some shared entities to the collections + # Add some shared components to the collections cls.collection1 = api.add_to_collection( cls.learning_package.id, key=cls.collection1.key, entities_qset=PublishableEntity.objects.filter(id__in=[ - cls.published_entity.id, + cls.published_component.pk, ]), created_by=cls.user.id, ) @@ -276,28 +270,34 @@ def setUpTestData(cls) -> None: cls.learning_package.id, key=cls.collection2.key, entities_qset=PublishableEntity.objects.filter(id__in=[ - cls.published_entity.id, - cls.draft_entity.id, + cls.published_component.pk, + cls.draft_component.pk, ]), ) cls.disabled_collection = api.add_to_collection( cls.learning_package.id, key=cls.disabled_collection.key, entities_qset=PublishableEntity.objects.filter(id__in=[ - cls.published_entity.id, + cls.published_component.pk, ]), ) + +class CollectionAddRemoveEntitiesTestCase(CollectionEntitiesTestCase): + """ + Test collections that contain publishable entitites. + """ + def test_create_collection_entities(self): """ Ensure the collections were pre-populated with the expected publishable entities. """ assert list(self.collection1.entities.all()) == [ - self.published_entity, + self.published_component.publishable_entity, ] assert list(self.collection2.entities.all()) == [ - self.published_entity, - self.draft_entity, + self.published_component.publishable_entity, + self.draft_component.publishable_entity, ] assert not list(self.collection3.entities.all()) @@ -311,14 +311,14 @@ def test_add_to_collection(self): self.learning_package.id, self.collection1.key, PublishableEntity.objects.filter(id__in=[ - self.draft_entity.id, + self.draft_component.pk, ]), created_by=self.user.id, ) assert list(self.collection1.entities.all()) == [ - self.published_entity, - self.draft_entity, + self.published_component.publishable_entity, + self.draft_component.publishable_entity, ] for collection_entity in CollectionPublishableEntity.objects.filter(collection=self.collection1): assert collection_entity.created_by == self.user @@ -334,13 +334,13 @@ def test_add_to_collection_again(self): self.learning_package.id, self.collection2.key, PublishableEntity.objects.filter(id__in=[ - self.published_entity.id, + self.published_component.pk, ]), ) assert list(self.collection2.entities.all()) == [ - self.published_entity, - self.draft_entity, + self.published_component.publishable_entity, + self.draft_component.publishable_entity, ] assert self.collection2.modified == modified_time @@ -353,7 +353,7 @@ def test_add_to_collection_wrong_learning_package(self): self.learning_package_2.id, self.collection3.key, PublishableEntity.objects.filter(id__in=[ - self.published_entity.id, + self.published_component.pk, ]), ) @@ -369,12 +369,12 @@ def test_remove_from_collection(self): self.learning_package.id, self.collection2.key, PublishableEntity.objects.filter(id__in=[ - self.published_entity.id, + self.published_component.pk, ]), ) assert list(self.collection2.entities.all()) == [ - self.draft_entity, + self.draft_component.publishable_entity, ] assert self.collection2.modified == modified_time @@ -384,13 +384,27 @@ def test_get_entity_collections(self): """ collections = api.get_entity_collections( self.learning_package.id, - self.published_entity.key, + self.published_component.publishable_entity.key, ) assert list(collections) == [ self.collection1, self.collection2, ] + def test_get_collection_components(self): + assert list(api.get_collection_components( + self.learning_package.id, + self.collection1.key, + )) == [self.published_component] + assert list(api.get_collection_components( + self.learning_package.id, + self.collection2.key, + )) == [self.published_component, self.draft_component] + assert not list(api.get_collection_components( + self.learning_package.id, + self.collection3.key, + )) + class UpdateCollectionTestCase(CollectionTestCase): """ @@ -478,3 +492,83 @@ def test_update_collection_not_found(self): key="12345", title="New Title", ) + + +class DeleteCollectionTestCase(CollectionEntitiesTestCase): + """ + Tests soft-deleting, restoring, and deleting collections. + """ + + def test_soft_delete(self): + """ + Collections are soft-deleted by default. + """ + modified_time = datetime(2024, 8, 8, tzinfo=timezone.utc) + with freeze_time(modified_time): + collection = api.delete_collection( + self.learning_package.id, + key=self.collection2.key, + ) + + # Collection was disabled and still exists in the database + assert not collection.enabled + assert collection.modified == modified_time + assert collection == api.get_collection(self.learning_package.id, collection.key) + # ...and the collection's entities remain intact. + assert list(collection.entities.all()) == [ + self.published_component.publishable_entity, + self.draft_component.publishable_entity, + ] + + def test_delete(self): + """ + Collections can be deleted completely. + """ + modified_time = datetime(2024, 8, 8, tzinfo=timezone.utc) + with freeze_time(modified_time): + collection = api.delete_collection( + self.learning_package.id, + key=self.collection2.key, + hard_delete=True, + ) + + # Collection was returned unchanged, but it's been deleted + assert collection.enabled + assert not collection.id + with self.assertRaises(ObjectDoesNotExist): + api.get_collection(self.learning_package.id, collection.key) + # ...and the entities have been removed from this collection + assert list(api.get_entity_collections( + self.learning_package.id, + self.published_component.publishable_entity.key, + )) == [self.collection1] + assert not list(api.get_entity_collections( + self.learning_package.id, + self.draft_component.publishable_entity.key, + )) + + def test_restore(self): + """ + Soft-deleted collections can be restored. + """ + collection = api.delete_collection( + self.learning_package.id, + key=self.collection2.key, + ) + + modified_time = datetime(2024, 8, 8, tzinfo=timezone.utc) + with freeze_time(modified_time): + collection = api.restore_collection( + self.learning_package.id, + key=self.collection2.key, + ) + + # Collection was enabled and still exists in the database + assert collection.enabled + assert collection.modified == modified_time + assert collection == api.get_collection(self.learning_package.id, collection.key) + # ...and the collection's entities remain intact. + assert list(collection.entities.all()) == [ + self.published_component.publishable_entity, + self.draft_component.publishable_entity, + ]