refactor: soft delete changes, comments, test names

openedx_learning/core/publishing/api.py

@@ -12,7 +12,11 @@
 from django.db.models import F, QuerySet
 from django.db.transaction import atomic
 
-from .model_mixins import PublishableContentModelRegistry, PublishableEntityMixin, PublishableEntityVersionMixin
+from .model_mixins import (
+    PublishableContentModelRegistry,
+    PublishableEntityMixin,
+    PublishableEntityVersionMixin,
+)
 from .models import (
     Draft,
     LearningPackage,
@@ -296,36 +300,60 @@ def get_draft_version(publishable_entity_id: int) -> PublishableEntityVersion |
     return draft.version
 
 
-def set_draft_version(publishable_entity_id: int, publishable_entity_version_pk: int) -> None:
+def set_draft_version(publishable_entity_id: int, publishable_entity_version_pk: int | None) -> None:
     """
     Modify the Draft of a PublishableEntity to be a PublishableEntityVersion.
 
     This would most commonly be used to set the Draft to point to a newly
     created PublishableEntityVersion that was created in Studio (because someone
     edited some content). Setting a Draft's version to None is like deleting it
-    from Studio's editing point of view. We don't actually delete the Draft row
-    because we'll need that for publishing purposes (i.e. to delete content from
-    the published branch).
+    from Studio's editing point of view (see ``soft_delete_draft`` for more
+    details).
     """
     draft = Draft.objects.get(entity_id=publishable_entity_id)
     draft.version_id = publishable_entity_version_pk
     draft.save()
 
 
 def soft_delete_draft(publishable_entity_id: int) -> None:
-    draft = Draft.objects.get(entity_id=publishable_entity_id)
-    draft.version_id = None
-    draft.save()
+    """
+    Sets the Draft version to None.
+
+    This "deletes" the ``PublishableEntity`` from the point of an authoring
+    environment like Studio, but doesn't immediately remove the ``Published``
+    version. No version data is actually deleted, so restoring is just a matter
+    of pointing the Draft back to the most recent ``PublishableEntityVersion``
+    for a given ``PublishableEntity``.
+    """
+    return set_draft_version(publishable_entity_id, None)
 
 
 def reset_drafts_to_published(learning_package_id: int) -> None:
     """
     Reset all Drafts to point to the most recently Published versions.
 
     This is a way to say "discard my unpublished changes" at the level of an
-    entire LearningPackage.
+    entire LearningPackage. Note that the ``PublishableEntityVersions`` that
+    were created in the mean time are not deleted.
+
+    Let's look at the example of a PublishableEntity where Draft and Published
+    both point to version 4.
+
+    * The PublishableEntity is then edited multiple times, creating a new
+      version with every save. Each new save also updates the the Draft to point
+      to it. After three such saves, Draft points at version 7.
+    * No new publishes have happened, so Published still points to version 4.
+    * ``reset_drafts_to_published`` is called. Draft now points to version 4 to
+      match Published.
+    * The PublishableEntity is edited again. This creates version 8, and Draft
+      now points to this new version.
+
+    So in the above example, versions 5-7 aren't discarded from the history, and
+    it's important that the code creating the "next" version_num looks at the
+    latest version created for a PublishableEntity (its ``latest`` attribute),
+    rather than basing it off of the version that Draft points to.
     """
-    # These are all the drafts that are different from the publisehd versions.
+    # These are all the drafts that are different from the published versions.
     draft_qset = (
         Draft
             .objects

tests/openedx_learning/core/components/test_api.py

@@ -14,7 +14,7 @@
 from openedx_learning.lib.test_utils import TestCase
 
 
-class TestPerformance(TestCase):
+class PerformanceTestCase(TestCase):
     """
     Performance related tests for Components.
 
@@ -66,7 +66,7 @@ def test_component_num_queries(self) -> None:
             published = component.versioning.published
             assert draft.title == published.title
 
-class TestGetComponents(TestCase):
+class GetComponentsTestCase(TestCase):
     """
     Test grabbing a queryset of Components.
     """
@@ -274,7 +274,7 @@ def test_published_title_filter(self):
         ]
 
 
-class TestComponentGetAndExists(TestCase):
+class ComponentGetAndExistsTestCase(TestCase):
     """
     Test getting a Component by primary key or key string.
     """
@@ -349,7 +349,7 @@ def test_exists_by_key(self):
         )
 
 
-class TestCreateNewVersions(TestCase):
+class CreateNewVersionsTestCase(TestCase):
     """
     Create new ComponentVersions in various ways.
     """

tests/openedx_learning/core/publishing/test_api.py

@@ -8,6 +8,7 @@
 from django.core.exceptions import ValidationError
 
 from openedx_learning.core.publishing import api as publishing_api
+from openedx_learning.core.publishing.models import LearningPackage
 from openedx_learning.lib.test_utils import TestCase
 
 
@@ -94,21 +95,26 @@ class DraftTestCase(TestCase):
     """
     Test basic operations with Drafts.
     """
+    now: datetime
+    learning_package: LearningPackage
+
+    @classmethod
+    def setUpTestData(cls) -> None:
+        cls.now = datetime(2024, 1, 28, 16, 45, 30, tzinfo=timezone.utc)
+        cls.learning_package = publishing_api.create_learning_package(
+            "my_package_key",
+            "Draft Testing LearningPackage 🔥",
+            created=cls.now,
+        )
 
     def test_draft_lifecycle(self):
         """
         Test basic lifecycle of a Draft.
         """
-        created = datetime(2023, 4, 2, 15, 9, 0, tzinfo=timezone.utc)
-        package = publishing_api.create_learning_package(
-            "my_package_key",
-            "Draft Testing LearningPackage 🔥",
-            created=created,
-        )
         entity = publishing_api.create_publishable_entity(
-            package.id,
+            self.learning_package.id,
             "my_entity",
-            created,
+            created=self.now,
             created_by=None,
         )
         # Drafts are NOT created when a PublishableEntity is created, only when
@@ -119,7 +125,7 @@ def test_draft_lifecycle(self):
             entity_id=entity.id,
             version_num=1,
             title="An Entity 🌴",
-            created=created,
+            created=self.now,
             created_by=None,
         )
         assert entity_version == publishing_api.get_draft_version(entity.id)
@@ -129,3 +135,12 @@ def test_draft_lifecycle(self):
         publishing_api.set_draft_version(entity.id, None)
         entity_version = publishing_api.get_draft_version(entity.id)
         assert entity_version is None
+
+    def test_reset_drafts_to_published(self):
+        """
+        Test throwing out Draft data and resetting to the Published versions.
+
+        One place this might turn up is if we've imported an older version of
+        the library and it causes a bunch of new versions to be created.
+        """
+