diff --git a/openedx_learning/core/publishing/api.py b/openedx_learning/core/publishing/api.py index a6f9e919..ec486cf3 100644 --- a/openedx_learning/core/publishing/api.py +++ b/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,16 +300,15 @@ 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 @@ -313,9 +316,16 @@ def set_draft_version(publishable_entity_id: int, publishable_entity_version_pk: 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: @@ -323,9 +333,27 @@ 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 diff --git a/tests/openedx_learning/core/components/test_api.py b/tests/openedx_learning/core/components/test_api.py index 539ba1f9..ff8a1fca 100644 --- a/tests/openedx_learning/core/components/test_api.py +++ b/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. """ diff --git a/tests/openedx_learning/core/publishing/test_api.py b/tests/openedx_learning/core/publishing/test_api.py index df4809d4..b04ccfed 100644 --- a/tests/openedx_learning/core/publishing/test_api.py +++ b/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. + """ +