From 31ae3245b4d4a00e9c303f1e528d155688a2e071 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Fri, 1 May 2026 16:43:37 -0700 Subject: [PATCH 1/2] fix: don't allow publishing within a draft change log context --- src/openedx_content/applets/publishing/api.py | 2 ++ .../applets/publishing/test_api.py | 36 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/src/openedx_content/applets/publishing/api.py b/src/openedx_content/applets/publishing/api.py index 9494bfcf1..d2cc6fc85 100644 --- a/src/openedx_content/applets/publishing/api.py +++ b/src/openedx_content/applets/publishing/api.py @@ -474,6 +474,8 @@ def publish_from_drafts( By default, this will also publish all dependencies (e.g. unpinned children) of the Drafts that are passed in. """ + if DraftChangeLogContext.get_active_draft_change_log(learning_package_id) is not None: + raise ValidationError("Cannot publish while in bulk_draft_changes_for().") if published_at is None: published_at = datetime.now(tz=timezone.utc) diff --git a/tests/openedx_content/applets/publishing/test_api.py b/tests/openedx_content/applets/publishing/test_api.py index 157c091a7..9600b1d0a 100644 --- a/tests/openedx_content/applets/publishing/test_api.py +++ b/tests/openedx_content/applets/publishing/test_api.py @@ -2533,3 +2533,39 @@ def test_create_version_rejects_cross_package_dependencies(self) -> None: created_by=None, dependencies=[entity_in_lp2.id], ) + + def test_publish_functions_rejected_inside_bulk_draft_changes_for(self) -> None: + """ + publish_all_drafts() and publish_from_drafts() must not be callable + from within a bulk_draft_changes_for() context. + + bulk_draft_changes_for() opens a DraftChangeLog for accumulating draft + edits; running a publish inside it mixes draft-change bookkeeping with + publish bookkeeping in the same atomic block, which corrupts the + ordering of DraftChangeLog vs. PublishLog records and can leave Drafts + and Published rows out of sync if the outer context later raises. + """ + entity = publishing_api.create_publishable_entity( + self.learning_package_1.id, + "entity_for_bulk_publish_check", + created=self.now, + created_by=None, + ) + publishing_api.create_publishable_entity_version( + entity.id, + version_num=1, + title="Entity v1", + created=self.now, + created_by=None, + ) + + with pytest.raises(ValidationError, match="Cannot publish while in bulk_draft_changes_for()."): + with publishing_api.bulk_draft_changes_for(self.learning_package_1.id): + publishing_api.publish_all_drafts(self.learning_package_1.id) + + with pytest.raises(ValidationError, match="Cannot publish while in bulk_draft_changes_for()."): + with publishing_api.bulk_draft_changes_for(self.learning_package_1.id): + publishing_api.publish_from_drafts( + self.learning_package_1.id, + Draft.objects.filter(entity__learning_package_id=self.learning_package_1.id), + ) From 9168232f02c7c3db5c25a429b38f511886f4b06d Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Wed, 3 Jun 2026 16:21:01 +0800 Subject: [PATCH 2/2] fix: address review comments --- src/openedx_content/applets/publishing/api.py | 4 ++- .../applets/publishing/test_api.py | 26 ++++++++++++------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/openedx_content/applets/publishing/api.py b/src/openedx_content/applets/publishing/api.py index d2cc6fc85..0a51ea05f 100644 --- a/src/openedx_content/applets/publishing/api.py +++ b/src/openedx_content/applets/publishing/api.py @@ -475,7 +475,9 @@ def publish_from_drafts( of the Drafts that are passed in. """ if DraftChangeLogContext.get_active_draft_change_log(learning_package_id) is not None: - raise ValidationError("Cannot publish while in bulk_draft_changes_for().") + raise ValidationError( + f"Cannot publish learning package {learning_package_id} while in bulk_draft_changes_for()." + ) if published_at is None: published_at = datetime.now(tz=timezone.utc) diff --git a/tests/openedx_content/applets/publishing/test_api.py b/tests/openedx_content/applets/publishing/test_api.py index 9600b1d0a..5c82edd1a 100644 --- a/tests/openedx_content/applets/publishing/test_api.py +++ b/tests/openedx_content/applets/publishing/test_api.py @@ -2545,6 +2545,7 @@ def test_publish_functions_rejected_inside_bulk_draft_changes_for(self) -> None: ordering of DraftChangeLog vs. PublishLog records and can leave Drafts and Published rows out of sync if the outer context later raises. """ + lp1_id = self.learning_package_1.id entity = publishing_api.create_publishable_entity( self.learning_package_1.id, "entity_for_bulk_publish_check", @@ -2559,13 +2560,20 @@ def test_publish_functions_rejected_inside_bulk_draft_changes_for(self) -> None: created_by=None, ) - with pytest.raises(ValidationError, match="Cannot publish while in bulk_draft_changes_for()."): - with publishing_api.bulk_draft_changes_for(self.learning_package_1.id): - publishing_api.publish_all_drafts(self.learning_package_1.id) + with pytest.raises( + ValidationError, + match=f"Cannot publish learning package {lp1_id} while in bulk_draft_changes_for()." + ): + with publishing_api.bulk_draft_changes_for(lp1_id): + publishing_api.publish_all_drafts(lp1_id) - with pytest.raises(ValidationError, match="Cannot publish while in bulk_draft_changes_for()."): - with publishing_api.bulk_draft_changes_for(self.learning_package_1.id): - publishing_api.publish_from_drafts( - self.learning_package_1.id, - Draft.objects.filter(entity__learning_package_id=self.learning_package_1.id), - ) + with pytest.raises( + ValidationError, + match=f"Cannot publish learning package {lp1_id} while in bulk_draft_changes_for()." + ): + with publishing_api.bulk_draft_changes_for(lp1_id): + publishing_api.publish_from_drafts(lp1_id, Draft.objects.filter(entity__learning_package_id=lp1_id)) + + # But we CAN publish if the bulk_draft_changes_for is a different learning package: + with publishing_api.bulk_draft_changes_for(self.learning_package_2.id): + publishing_api.publish_all_drafts(lp1_id)