fix: prevent soft-deletes from being re-published every time

openedx_learning/apps/authoring/publishing/api.py

@@ -9,7 +9,7 @@
 from datetime import datetime, timezone
 
 from django.core.exceptions import ObjectDoesNotExist
-from django.db.models import F, QuerySet
+from django.db.models import F, Q, QuerySet
 from django.db.transaction import atomic
 
 from .model_mixins import PublishableContentModelRegistry, PublishableEntityMixin, PublishableEntityVersionMixin
@@ -245,10 +245,23 @@ def publish_all_drafts(
     """
     Publish everything that is a Draft and is not already published.
     """
-    draft_qset = Draft.objects \
-                      .select_related("entity__published") \
-                      .filter(entity__learning_package_id=learning_package_id) \
-                      .exclude(entity__published__version_id=F("version_id"))
+    draft_qset = (
+        Draft.objects.select_related("entity__published")
+        .filter(entity__learning_package_id=learning_package_id)
+
+        # Exclude entities where the Published version already matches the
+        # Draft version.
+        .exclude(entity__published__version_id=F("version_id"))
+
+        # Account for soft-deletes:
+        # NULL != NULL in SQL, so simply excluding entities where the Draft
+        # and Published versions match will not catch the case where a
+        # soft-delete has been published (i.e. both the Draft and Published
+        # versions are NULL). We need to explicitly check for that case
+        # instead, or else we will re-publish the same soft-deletes over
+        # and over again.
+        .exclude(Q(version__isnull=True) & Q(entity__published__version__isnull=True))
+    )
     return publish_from_drafts(
         learning_package_id, draft_qset, message, published_at, published_by
     )

tests/openedx_learning/apps/authoring/publishing/test_api.py

@@ -152,6 +152,49 @@ def test_draft_lifecycle(self) -> None:
         deleted_entity_version = publishing_api.get_draft_version(entity.id)
         assert deleted_entity_version is None
 
+    def test_soft_deletes(self) -> None:
+        """Test the publishing behavior of soft deletes."""
+        entity = publishing_api.create_publishable_entity(
+            self.learning_package.id,
+            "my_entity",
+            created=self.now,
+            created_by=None,
+        )
+        entity_version = publishing_api.create_publishable_entity_version(
+            entity.id,
+            version_num=1,
+            title="An Entity 🌴",
+            created=self.now,
+            created_by=None,
+        )
+
+        # Initial publish
+        publish_log = publishing_api.publish_all_drafts(self.learning_package.id)
+        log_records = list(publish_log.records.all())
+        assert len(log_records) == 1
+        record = log_records[0]
+        assert record.entity_id == entity.id
+        assert record.old_version is None
+        assert record.new_version_id == entity_version.id
+
+        # Publishing the soft-delete
+        publishing_api.soft_delete_draft(entity.id)
+        publish_log = publishing_api.publish_all_drafts(self.learning_package.id)
+        log_records = list(publish_log.records.all())
+        assert len(log_records) == 1
+        record = log_records[0]
+        assert record.entity_id == entity.id
+        assert record.old_version_id == entity_version.id
+        assert record.new_version is None
+
+        # Verify that we do not re-publish soft-deleted records. We initially
+        # had a bug here because NULL != NULL in SQL, so the check to "publish
+        # all the Drafts that have different versions than their Published
+        # counterparts" would mistakenly pull in records that were NULL in both
+        # places.
+        publish_log = publishing_api.publish_all_drafts(self.learning_package.id)
+        assert publish_log.records.count() == 0
+
     def test_reset_drafts_to_published(self) -> None:
         """
         Test throwing out Draft data and resetting to the Published versions.