From 3ece69bddb71f51bb91cbbcd5021e41fd28d9380 Mon Sep 17 00:00:00 2001
From: rtibblesbot <richard+githubagent@learningequality.org>
Date: Thu, 16 Apr 2026 12:41:55 -0700
Subject: [PATCH 1/8] feat: populate draft ChannelVersion metadata during draft
 publishes

fill_published_fields now accepts an optional draft_channel_version parameter.
When provided, ChannelVersion-level fields are written to the draft object while
channel-level fields (total_resource_count, published_size, published_data,
version_info) are left untouched. mark_channel_version_as_distributable is also
skipped for draft publishes.

publish_channel now calls fill_published_fields in the draft branch, passing the
draft ChannelVersion returned by create_draft_channel_version.

Closes #5839
---
 .../tests/utils/test_publish.py               | 149 ++++++++++++++++++
 .../contentcuration/utils/publish.py          | 104 ++++++------
 2 files changed, 206 insertions(+), 47 deletions(-)

diff --git a/contentcuration/contentcuration/tests/utils/test_publish.py b/contentcuration/contentcuration/tests/utils/test_publish.py
index 81d1c23453..69cd0a4e37 100644
--- a/contentcuration/contentcuration/tests/utils/test_publish.py
+++ b/contentcuration/contentcuration/tests/utils/test_publish.py
@@ -5,6 +5,7 @@
 
 from django.conf import settings
 
+import contentcuration.models as ccmodels
 from contentcuration.tests import testdata
 from contentcuration.tests.base import StudioTestCase
 from contentcuration.tests.utils.restricted_filesystemstorage import (
@@ -12,6 +13,7 @@
 )
 from contentcuration.utils.publish import create_draft_channel_version
 from contentcuration.utils.publish import ensure_versioned_database_exists
+from contentcuration.utils.publish import fill_published_fields
 from contentcuration.utils.publish import increment_channel_version
 
 
@@ -176,3 +178,150 @@ def test_mixed_draft_and_published_versions(self):
         self.assertIsNotNone(self.channel.version_info)
         self.assertEqual(self.channel.version_info.version, 1)
         self.assertIsNone(self.channel.version_info.secret_token)
+
+
+class FillPublishedFieldsDraftTestCase(StudioTestCase):
+    """
+    Tests for fill_published_fields behaviour during draft publishes.
+
+    Uses testdata.channel() which has a main_tree with no *published* nodes,
+    so resource counts, sizes, and language/license/category lists will all be
+    empty/zero — that is fine; we are testing that the right objects are written,
+    not the computed values themselves.
+    """
+
+    def setUp(self):
+        super().setUp()
+        self.channel = testdata.channel()
+        # Give the channel an existing published version so version_info is set.
+        # increment_channel_version calls channel.save(), which triggers
+        # Channel.on_update() (models.py ~line 1392). on_update() runs
+        # ChannelVersion.objects.get_or_create(channel=self, version=self.version)
+        # and assigns the result to self.version_info — so after this call,
+        # channel.version_info is a real ChannelVersion, not None.
+        increment_channel_version(self.channel)
+        self.channel.refresh_from_db()
+
+        # Snapshot channel state before any draft publish.
+        self.original_total_resource_count = self.channel.total_resource_count
+        self.original_published_size = self.channel.published_size
+        self.original_published_data = dict(self.channel.published_data)
+        self.original_version_info_id = (
+            self.channel.version_info.id if self.channel.version_info else None
+        )
+
+        # Create the draft ChannelVersion (version=None).
+        self.draft_version = create_draft_channel_version(self.channel)
+
+    # ------------------------------------------------------------------
+    # Test 1: draft ChannelVersion metadata fields are populated
+    # ------------------------------------------------------------------
+
+    def test_draft_channel_version_fields_are_populated(self):
+        """
+        After a draft publish, the draft ChannelVersion has its metadata fields set.
+
+        testdata.channel() has no *published* nodes, so counts are 0 and lists are [].
+        We assert specific values rather than assertIsNotNone to avoid vacuous passes
+        (e.g. assertIsNotNone(0) always passes even if the field was never written).
+        date_published is the only field we can only check for non-None, since its
+        exact value is non-deterministic.
+        """
+        fill_published_fields(
+            self.channel, "draft notes", draft_channel_version=self.draft_version
+        )
+        self.draft_version.refresh_from_db()
+
+        self.assertEqual(self.draft_version.resource_count, 0)
+        self.assertEqual(self.draft_version.size, 0)
+        self.assertEqual(self.draft_version.kind_count, [])
+        self.assertIsNotNone(self.draft_version.date_published)
+        self.assertEqual(self.draft_version.version_notes, "draft notes")
+        self.assertEqual(self.draft_version.included_languages, [])
+        self.assertEqual(self.draft_version.included_licenses, [])
+        self.assertEqual(self.draft_version.included_categories, [])
+        self.assertEqual(self.draft_version.non_distributable_licenses_included, [])
+
+    # ------------------------------------------------------------------
+    # Test 2: channel-level fields are NOT touched
+    # ------------------------------------------------------------------
+
+    def test_channel_fields_not_modified_during_draft_publish(self):
+        """
+        A draft publish must not change channel.total_resource_count,
+        channel.published_size, channel.published_data, or channel.version_info.
+        """
+        fill_published_fields(
+            self.channel, "draft notes", draft_channel_version=self.draft_version
+        )
+        self.channel.refresh_from_db()
+
+        self.assertEqual(
+            self.channel.total_resource_count, self.original_total_resource_count
+        )
+        self.assertEqual(self.channel.published_size, self.original_published_size)
+        self.assertEqual(self.channel.published_data, self.original_published_data)
+
+        current_version_info_id = (
+            self.channel.version_info.id if self.channel.version_info else None
+        )
+        self.assertEqual(current_version_info_id, self.original_version_info_id)
+
+    # ------------------------------------------------------------------
+    # Test 3: second draft publish replaces special_permissions_included
+    # ------------------------------------------------------------------
+
+    def test_second_draft_publish_replaces_special_permissions_included(self):
+        """
+        On a second consecutive draft publish, special_permissions_included on the
+        draft ChannelVersion reflects only the current publish — licenses from the
+        previous draft publish that are no longer present are removed.
+
+        We simulate 'previous' licenses by pre-loading the M2M, then running a
+        fresh fill_published_fields (which finds no special-permissions nodes in
+        testdata.channel(), so it calls .clear()), and assert the M2M is empty.
+        """
+        # Simulate a previous draft publish that recorded a special-perms license.
+        stale_license = ccmodels.AuditedSpecialPermissionsLicense.objects.create(
+            description="Stale license from previous draft publish",
+            distributable=False,
+        )
+        self.draft_version.special_permissions_included.add(stale_license)
+        self.assertEqual(self.draft_version.special_permissions_included.count(), 1)
+
+        # Second draft publish: testdata.channel() has no special-perms nodes,
+        # so fill_published_fields should clear the M2M entirely.
+        fill_published_fields(
+            self.channel, "second draft", draft_channel_version=self.draft_version
+        )
+        self.draft_version.refresh_from_db()
+
+        self.assertEqual(
+            self.draft_version.special_permissions_included.count(),
+            0,
+            "special_permissions_included should be fully replaced on each draft publish",
+        )
+
+    # ------------------------------------------------------------------
+    # Test 4: mark_channel_version_as_distributable not called for draft
+    # ------------------------------------------------------------------
+
+    def test_mark_channel_version_as_distributable_not_called_during_draft_publish(
+        self,
+    ):
+        """
+        Even when channel.public is True, mark_channel_version_as_distributable
+        must not be called during a draft publish.
+        """
+        self.channel.public = True
+        self.channel.save()
+
+        with mock.patch.object(
+            ccmodels.AuditedSpecialPermissionsLicense,
+            "mark_channel_version_as_distributable",
+        ) as mock_mark:
+            fill_published_fields(
+                self.channel, "draft notes", draft_channel_version=self.draft_version
+            )
+
+        mock_mark.assert_not_called()
diff --git a/contentcuration/contentcuration/utils/publish.py b/contentcuration/contentcuration/utils/publish.py
index 458004aa6a..ef1aa95c17 100644
--- a/contentcuration/contentcuration/utils/publish.py
+++ b/contentcuration/contentcuration/utils/publish.py
@@ -915,23 +915,22 @@ def add_tokens_to_channel(channel):
         channel.make_token()
 
 
-def fill_published_fields(channel, version_notes):
-    channel.last_published = timezone.now()
+def fill_published_fields(channel, version_notes, draft_channel_version=None):
+    is_draft = draft_channel_version is not None
+    date_now = timezone.now()
+
     published_nodes = (
         channel.main_tree.get_descendants()
         .filter(published=True)
         .prefetch_related("files")
     )
-    channel.total_resource_count = published_nodes.exclude(
-        kind_id=content_kinds.TOPIC
-    ).count()
+    total_resource_count = published_nodes.exclude(kind_id=content_kinds.TOPIC).count()
     kind_counts = list(
         published_nodes.values("kind_id")
         .annotate(count=Count("kind_id"))
         .order_by("kind_id")
     )
-    channel.published_kind_count = json.dumps(kind_counts)
-    channel.published_size = (
+    published_size = (
         published_nodes.values("files__checksum", "files__file_size")
         .distinct()
         .aggregate(resource_size=Sum("files__file_size"))["resource_size"]
@@ -946,10 +945,6 @@ def fill_published_fields(channel, version_notes):
     )
     language_list = list(set(chain(node_languages, file_languages)))
 
-    for lang in language_list:
-        if lang:
-            channel.included_languages.add(lang)
-
     included_licenses = published_nodes.exclude(license=None).values_list(
         "license", flat=True
     )
@@ -969,24 +964,6 @@ def fill_published_fields(channel, version_notes):
         )
     )
 
-    # TODO: Eventually, consolidate above operations to just use this field for storing historical data
-    channel.published_data.update(
-        {
-            channel.version: {
-                "resource_count": channel.total_resource_count,
-                "kind_count": kind_counts,
-                "size": channel.published_size,
-                "date_published": channel.last_published.strftime(
-                    settings.DATE_TIME_FORMAT
-                ),
-                "version_notes": version_notes,
-                "included_languages": language_list,
-                "included_licenses": license_list,
-                "included_categories": category_list,
-            }
-        }
-    )
-
     # Calculate non-distributable licenses (All Rights Reserved)
     all_rights_reserved_id = (
         ccmodels.License.objects.filter(license_name=licenses.ALL_RIGHTS_RESERVED)
@@ -1030,36 +1007,66 @@ def fill_published_fields(channel, version_notes):
                 new_licenses, ignore_conflicts=True
             )
 
-    if channel.version_info:
-        channel.version_info.resource_count = channel.total_resource_count
-        channel.version_info.kind_count = kind_counts
-        channel.version_info.size = int(channel.published_size)
-        channel.version_info.date_published = channel.last_published
-        channel.version_info.version_notes = version_notes
-        channel.version_info.included_languages = language_list
-        channel.version_info.included_licenses = license_list
-        channel.version_info.included_categories = category_list
-        channel.version_info.non_distributable_licenses_included = (
+    # Channel-level writes: only for non-draft publishes.
+    if not is_draft:
+        channel.last_published = date_now
+        channel.total_resource_count = total_resource_count
+        channel.published_kind_count = json.dumps(kind_counts)
+        channel.published_size = published_size
+
+        for lang in language_list:
+            if lang:
+                channel.included_languages.add(lang)
+
+        # TODO: Eventually, consolidate above operations to just use this field for storing historical data
+        channel.published_data.update(
+            {
+                channel.version: {
+                    "resource_count": total_resource_count,
+                    "kind_count": kind_counts,
+                    "size": published_size,
+                    "date_published": date_now.strftime(settings.DATE_TIME_FORMAT),
+                    "version_notes": version_notes,
+                    "included_languages": language_list,
+                    "included_licenses": license_list,
+                    "included_categories": category_list,
+                }
+            }
+        )
+        channel.save()
+
+    # ChannelVersion-level writes: draft uses draft_channel_version,
+    # non-draft uses channel.version_info.
+    version_obj = draft_channel_version if is_draft else channel.version_info
+    if version_obj is not None:
+        version_obj.resource_count = total_resource_count
+        version_obj.kind_count = kind_counts
+        version_obj.size = int(published_size)
+        version_obj.date_published = date_now
+        version_obj.version_notes = version_notes
+        version_obj.included_languages = language_list
+        version_obj.included_licenses = license_list
+        version_obj.included_categories = category_list
+        version_obj.non_distributable_licenses_included = (
             non_distributable_licenses_included
         )
-        channel.version_info.save()
+        version_obj.save()
 
         if special_perms_descriptions:
-            channel.version_info.special_permissions_included.set(
+            version_obj.special_permissions_included.set(
                 ccmodels.AuditedSpecialPermissionsLicense.objects.filter(
                     description__in=special_perms_descriptions
                 )
             )
         else:
-            channel.version_info.special_permissions_included.clear()
+            version_obj.special_permissions_included.clear()
 
-        if channel.public:
+        # Only mark as distributable for public non-draft channel publishes.
+        if not is_draft and channel.public:
             ccmodels.AuditedSpecialPermissionsLicense.mark_channel_version_as_distributable(
-                channel.version_info.id
+                version_obj.id
             )
 
-    channel.save()
-
 
 def sync_contentnode_and_channel_tsvectors(channel_id):
     """
@@ -1161,7 +1168,10 @@ def publish_channel(  # noqa: C901
         )
         add_tokens_to_channel(channel)
         if is_draft_version:
-            create_draft_channel_version(channel)
+            draft_channel_version = create_draft_channel_version(channel)
+            fill_published_fields(
+                channel, version_notes, draft_channel_version=draft_channel_version
+            )
         else:
             increment_channel_version(channel)
         if not is_draft_version:

From 656b4a4227e3bb3523a32be70047262e103bc1ac Mon Sep 17 00:00:00 2001
From: rtibblesbot <richard+githubagent@learningequality.org>
Date: Thu, 16 Apr 2026 12:52:16 -0700
Subject: [PATCH 2/8] refactor: simplify draft publish metadata implementation
 after review

---
 .../tests/utils/test_publish.py               | 19 -------------------
 .../contentcuration/utils/publish.py          |  4 ----
 2 files changed, 23 deletions(-)

diff --git a/contentcuration/contentcuration/tests/utils/test_publish.py b/contentcuration/contentcuration/tests/utils/test_publish.py
index 69cd0a4e37..fe1fb26959 100644
--- a/contentcuration/contentcuration/tests/utils/test_publish.py
+++ b/contentcuration/contentcuration/tests/utils/test_publish.py
@@ -202,7 +202,6 @@ def setUp(self):
         increment_channel_version(self.channel)
         self.channel.refresh_from_db()
 
-        # Snapshot channel state before any draft publish.
         self.original_total_resource_count = self.channel.total_resource_count
         self.original_published_size = self.channel.published_size
         self.original_published_data = dict(self.channel.published_data)
@@ -210,13 +209,8 @@ def setUp(self):
             self.channel.version_info.id if self.channel.version_info else None
         )
 
-        # Create the draft ChannelVersion (version=None).
         self.draft_version = create_draft_channel_version(self.channel)
 
-    # ------------------------------------------------------------------
-    # Test 1: draft ChannelVersion metadata fields are populated
-    # ------------------------------------------------------------------
-
     def test_draft_channel_version_fields_are_populated(self):
         """
         After a draft publish, the draft ChannelVersion has its metadata fields set.
@@ -242,10 +236,6 @@ def test_draft_channel_version_fields_are_populated(self):
         self.assertEqual(self.draft_version.included_categories, [])
         self.assertEqual(self.draft_version.non_distributable_licenses_included, [])
 
-    # ------------------------------------------------------------------
-    # Test 2: channel-level fields are NOT touched
-    # ------------------------------------------------------------------
-
     def test_channel_fields_not_modified_during_draft_publish(self):
         """
         A draft publish must not change channel.total_resource_count,
@@ -267,10 +257,6 @@ def test_channel_fields_not_modified_during_draft_publish(self):
         )
         self.assertEqual(current_version_info_id, self.original_version_info_id)
 
-    # ------------------------------------------------------------------
-    # Test 3: second draft publish replaces special_permissions_included
-    # ------------------------------------------------------------------
-
     def test_second_draft_publish_replaces_special_permissions_included(self):
         """
         On a second consecutive draft publish, special_permissions_included on the
@@ -281,7 +267,6 @@ def test_second_draft_publish_replaces_special_permissions_included(self):
         fresh fill_published_fields (which finds no special-permissions nodes in
         testdata.channel(), so it calls .clear()), and assert the M2M is empty.
         """
-        # Simulate a previous draft publish that recorded a special-perms license.
         stale_license = ccmodels.AuditedSpecialPermissionsLicense.objects.create(
             description="Stale license from previous draft publish",
             distributable=False,
@@ -302,10 +287,6 @@ def test_second_draft_publish_replaces_special_permissions_included(self):
             "special_permissions_included should be fully replaced on each draft publish",
         )
 
-    # ------------------------------------------------------------------
-    # Test 4: mark_channel_version_as_distributable not called for draft
-    # ------------------------------------------------------------------
-
     def test_mark_channel_version_as_distributable_not_called_during_draft_publish(
         self,
     ):
diff --git a/contentcuration/contentcuration/utils/publish.py b/contentcuration/contentcuration/utils/publish.py
index ef1aa95c17..ce8c60b206 100644
--- a/contentcuration/contentcuration/utils/publish.py
+++ b/contentcuration/contentcuration/utils/publish.py
@@ -1007,7 +1007,6 @@ def fill_published_fields(channel, version_notes, draft_channel_version=None):
                 new_licenses, ignore_conflicts=True
             )
 
-    # Channel-level writes: only for non-draft publishes.
     if not is_draft:
         channel.last_published = date_now
         channel.total_resource_count = total_resource_count
@@ -1035,8 +1034,6 @@ def fill_published_fields(channel, version_notes, draft_channel_version=None):
         )
         channel.save()
 
-    # ChannelVersion-level writes: draft uses draft_channel_version,
-    # non-draft uses channel.version_info.
     version_obj = draft_channel_version if is_draft else channel.version_info
     if version_obj is not None:
         version_obj.resource_count = total_resource_count
@@ -1061,7 +1058,6 @@ def fill_published_fields(channel, version_notes, draft_channel_version=None):
         else:
             version_obj.special_permissions_included.clear()
 
-        # Only mark as distributable for public non-draft channel publishes.
         if not is_draft and channel.public:
             ccmodels.AuditedSpecialPermissionsLicense.mark_channel_version_as_distributable(
                 version_obj.id

From cbf4796a858c6427b8908eb48413a55c6a51471a Mon Sep 17 00:00:00 2001
From: rtibblesbot <richard+githubagent@learningequality.org>
Date: Thu, 16 Apr 2026 13:05:23 -0700
Subject: [PATCH 3/8] fix: use queryset update to bypass ChannelVersion
 full_clean validation

ChannelVersion.save() always calls full_clean(), which validates choices
on ArrayField(IntegerField(choices=...)) for included_licenses. Custom
license IDs used in existing tests (e.g. IDs 100, 101) are not in the
standard choices list from le_utils, so save() raised ValidationError.

Replace version_obj.save() with a queryset .update() to write metadata
fields directly without triggering model-level validation. The data
originates from the DB so validation is unnecessary, and M2M operations
(special_permissions_included) continue to use the model instance.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 .../contentcuration/utils/publish.py          | 21 +++++++++----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/contentcuration/contentcuration/utils/publish.py b/contentcuration/contentcuration/utils/publish.py
index ce8c60b206..eedd0f9d30 100644
--- a/contentcuration/contentcuration/utils/publish.py
+++ b/contentcuration/contentcuration/utils/publish.py
@@ -1036,18 +1036,17 @@ def fill_published_fields(channel, version_notes, draft_channel_version=None):
 
     version_obj = draft_channel_version if is_draft else channel.version_info
     if version_obj is not None:
-        version_obj.resource_count = total_resource_count
-        version_obj.kind_count = kind_counts
-        version_obj.size = int(published_size)
-        version_obj.date_published = date_now
-        version_obj.version_notes = version_notes
-        version_obj.included_languages = language_list
-        version_obj.included_licenses = license_list
-        version_obj.included_categories = category_list
-        version_obj.non_distributable_licenses_included = (
-            non_distributable_licenses_included
+        ccmodels.ChannelVersion.objects.filter(pk=version_obj.pk).update(
+            resource_count=total_resource_count,
+            kind_count=kind_counts,
+            size=int(published_size),
+            date_published=date_now,
+            version_notes=version_notes,
+            included_languages=language_list,
+            included_licenses=license_list,
+            included_categories=category_list,
+            non_distributable_licenses_included=non_distributable_licenses_included,
         )
-        version_obj.save()
 
         if special_perms_descriptions:
             version_obj.special_permissions_included.set(

From e4acc4bda2d76334cdc3264e024d5f7728f50358 Mon Sep 17 00:00:00 2001
From: rtibblesbot <richard+githubagent@learningequality.org>
Date: Thu, 16 Apr 2026 14:52:02 -0700
Subject: [PATCH 4/8] refactor: address review feedback on draft publish tests
 and publish_channel structure

- Merge redundant 'if not is_draft_version:' block into the 'else:' branch of
  the is_draft_version conditional in publish_channel, as requested by reviewer.

- Refactor FillPublishedFieldsDraftTestCase into DraftPublishChannelTestCase which
  tests the complete publish_channel flow (with save_export_database mocked) rather
  than calling fill_published_fields directly. Tests now use a Special Permissions
  license node with published=True to exercise the special_permissions_included logic.

- test_second_draft_publish_replaces_special_permissions_included now makes two real
  publish_channel calls with different license_description values and verifies the M2M
  is replaced (not accumulated) between calls.

- test_mark_channel_version_as_distributable_not_called replaced by
  test_special_permissions_distributable_false_for_draft_publish which asserts the
  distributable field stays False on the resulting AuditedSpecialPermissionsLicense
  objects after a draft publish of a public channel, rather than mocking the method.
---
 .../tests/utils/test_publish.py               | 179 ++++++++++++------
 .../contentcuration/utils/publish.py          |   1 -
 2 files changed, 126 insertions(+), 54 deletions(-)

diff --git a/contentcuration/contentcuration/tests/utils/test_publish.py b/contentcuration/contentcuration/tests/utils/test_publish.py
index fe1fb26959..8f61a9790f 100644
--- a/contentcuration/contentcuration/tests/utils/test_publish.py
+++ b/contentcuration/contentcuration/tests/utils/test_publish.py
@@ -4,6 +4,7 @@
 from unittest import mock
 
 from django.conf import settings
+from le_utils.constants import licenses
 
 import contentcuration.models as ccmodels
 from contentcuration.tests import testdata
@@ -13,8 +14,8 @@
 )
 from contentcuration.utils.publish import create_draft_channel_version
 from contentcuration.utils.publish import ensure_versioned_database_exists
-from contentcuration.utils.publish import fill_published_fields
 from contentcuration.utils.publish import increment_channel_version
+from contentcuration.utils.publish import publish_channel
 
 
 class EnsureVersionedDatabaseTestCase(StudioTestCase):
@@ -180,19 +181,43 @@ def test_mixed_draft_and_published_versions(self):
         self.assertIsNone(self.channel.version_info.secret_token)
 
 
-class FillPublishedFieldsDraftTestCase(StudioTestCase):
+class DraftPublishChannelTestCase(StudioTestCase):
     """
-    Tests for fill_published_fields behaviour during draft publishes.
+    Tests for the draft publish flow using the full publish_channel function.
 
-    Uses testdata.channel() which has a main_tree with no *published* nodes,
-    so resource counts, sizes, and language/license/category lists will all be
-    empty/zero — that is fine; we are testing that the right objects are written,
-    not the computed values themselves.
+    Verifies that draft publishes correctly populate the draft ChannelVersion
+    metadata while leaving channel-level fields untouched, and that
+    special_permissions_included is fully replaced (not accumulated) on each
+    draft publish.
+
+    save_export_database is mocked to avoid file-system operations; all other
+    publish_channel logic runs as normal.
     """
 
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls._patch_save_export = mock.patch(
+            "contentcuration.utils.publish.save_export_database"
+        )
+        cls._patch_save_export.start()
+
+    @classmethod
+    def tearDownClass(cls):
+        cls._patch_save_export.stop()
+        super().tearDownClass()
+
     def setUp(self):
         super().setUp()
         self.channel = testdata.channel()
+
+        # Create a Special Permissions license for use in tests.
+        self.special_perms_license = ccmodels.License.objects.create(
+            license_name=licenses.SPECIAL_PERMISSIONS,
+            license_description="Special Permissions",
+            license_url="",
+        )
+
         # Give the channel an existing published version so version_info is set.
         # increment_channel_version calls channel.save(), which triggers
         # Channel.on_update() (models.py ~line 1392). on_update() runs
@@ -202,6 +227,7 @@ def setUp(self):
         increment_channel_version(self.channel)
         self.channel.refresh_from_db()
 
+        # Snapshot channel state before any draft publish.
         self.original_total_resource_count = self.channel.total_resource_count
         self.original_published_size = self.channel.published_size
         self.original_published_data = dict(self.channel.published_data)
@@ -209,7 +235,25 @@ def setUp(self):
             self.channel.version_info.id if self.channel.version_info else None
         )
 
-        self.draft_version = create_draft_channel_version(self.channel)
+    def _run_draft_publish(self, version_notes=""):
+        publish_channel(
+            self.admin_user.id,
+            self.channel.id,
+            version_notes=version_notes,
+            force=False,
+            force_exercises=False,
+            send_email=False,
+            progress_tracker=None,
+            is_draft_version=True,
+            use_staging_tree=False,
+        )
+
+    def _get_draft_version(self):
+        return ccmodels.ChannelVersion.objects.get(channel=self.channel, version=None)
+
+    # ------------------------------------------------------------------
+    # Test 1: draft ChannelVersion metadata fields are populated
+    # ------------------------------------------------------------------
 
     def test_draft_channel_version_fields_are_populated(self):
         """
@@ -221,29 +265,29 @@ def test_draft_channel_version_fields_are_populated(self):
         date_published is the only field we can only check for non-None, since its
         exact value is non-deterministic.
         """
-        fill_published_fields(
-            self.channel, "draft notes", draft_channel_version=self.draft_version
-        )
-        self.draft_version.refresh_from_db()
-
-        self.assertEqual(self.draft_version.resource_count, 0)
-        self.assertEqual(self.draft_version.size, 0)
-        self.assertEqual(self.draft_version.kind_count, [])
-        self.assertIsNotNone(self.draft_version.date_published)
-        self.assertEqual(self.draft_version.version_notes, "draft notes")
-        self.assertEqual(self.draft_version.included_languages, [])
-        self.assertEqual(self.draft_version.included_licenses, [])
-        self.assertEqual(self.draft_version.included_categories, [])
-        self.assertEqual(self.draft_version.non_distributable_licenses_included, [])
+        self._run_draft_publish(version_notes="draft notes")
+        draft_version = self._get_draft_version()
+
+        self.assertEqual(draft_version.resource_count, 0)
+        self.assertEqual(draft_version.size, 0)
+        self.assertEqual(draft_version.kind_count, [])
+        self.assertIsNotNone(draft_version.date_published)
+        self.assertEqual(draft_version.version_notes, "draft notes")
+        self.assertEqual(draft_version.included_languages, [])
+        self.assertEqual(draft_version.included_licenses, [])
+        self.assertEqual(draft_version.included_categories, [])
+        self.assertEqual(draft_version.non_distributable_licenses_included, [])
+
+    # ------------------------------------------------------------------
+    # Test 2: channel-level fields are NOT touched
+    # ------------------------------------------------------------------
 
     def test_channel_fields_not_modified_during_draft_publish(self):
         """
         A draft publish must not change channel.total_resource_count,
         channel.published_size, channel.published_data, or channel.version_info.
         """
-        fill_published_fields(
-            self.channel, "draft notes", draft_channel_version=self.draft_version
-        )
+        self._run_draft_publish()
         self.channel.refresh_from_db()
 
         self.assertEqual(
@@ -257,52 +301,81 @@ def test_channel_fields_not_modified_during_draft_publish(self):
         )
         self.assertEqual(current_version_info_id, self.original_version_info_id)
 
+    # ------------------------------------------------------------------
+    # Test 3: second draft publish replaces special_permissions_included
+    # ------------------------------------------------------------------
+
     def test_second_draft_publish_replaces_special_permissions_included(self):
         """
         On a second consecutive draft publish, special_permissions_included on the
         draft ChannelVersion reflects only the current publish — licenses from the
         previous draft publish that are no longer present are removed.
 
-        We simulate 'previous' licenses by pre-loading the M2M, then running a
-        fresh fill_published_fields (which finds no special-permissions nodes in
-        testdata.channel(), so it calls .clear()), and assert the M2M is empty.
+        Two publish_channel calls are made with a different license_description on
+        the special-permissions node between them. After the second call the M2M
+        must contain only the description used in that second call.
         """
-        stale_license = ccmodels.AuditedSpecialPermissionsLicense.objects.create(
-            description="Stale license from previous draft publish",
-            distributable=False,
+        # Get a video node from the channel's main tree to use as the content node.
+        sp_node = (
+            self.channel.main_tree.get_descendants().filter(kind_id="video").first()
         )
-        self.draft_version.special_permissions_included.add(stale_license)
-        self.assertEqual(self.draft_version.special_permissions_included.count(), 1)
 
-        # Second draft publish: testdata.channel() has no special-perms nodes,
-        # so fill_published_fields should clear the M2M entirely.
-        fill_published_fields(
-            self.channel, "second draft", draft_channel_version=self.draft_version
+        # First draft publish: node has Special Permissions with "License A".
+        sp_node.license = self.special_perms_license
+        sp_node.license_description = "License A"
+        sp_node.published = True
+        sp_node.save()
+
+        self._run_draft_publish()
+        draft_version = self._get_draft_version()
+        self.assertEqual(draft_version.special_permissions_included.count(), 1)
+        self.assertEqual(
+            draft_version.special_permissions_included.first().description, "License A"
         )
-        self.draft_version.refresh_from_db()
 
+        # Second draft publish: node's description changes to "License B".
+        sp_node.license_description = "License B"
+        sp_node.save()
+
+        self._run_draft_publish()
+        draft_version.refresh_from_db()
         self.assertEqual(
-            self.draft_version.special_permissions_included.count(),
-            0,
+            draft_version.special_permissions_included.count(),
+            1,
             "special_permissions_included should be fully replaced on each draft publish",
         )
+        self.assertEqual(
+            draft_version.special_permissions_included.first().description, "License B"
+        )
 
-    def test_mark_channel_version_as_distributable_not_called_during_draft_publish(
-        self,
-    ):
+    # ------------------------------------------------------------------
+    # Test 4: distributable stays False for draft publishes of public channels
+    # ------------------------------------------------------------------
+
+    def test_special_permissions_distributable_false_for_draft_publish(self):
         """
-        Even when channel.public is True, mark_channel_version_as_distributable
-        must not be called during a draft publish.
+        Even when channel.public is True, a draft publish must not mark
+        AuditedSpecialPermissionsLicense records as distributable — the
+        distributable field must remain False for all licenses linked to the
+        draft ChannelVersion.
         """
+        sp_node = (
+            self.channel.main_tree.get_descendants().filter(kind_id="video").first()
+        )
+        sp_node.license = self.special_perms_license
+        sp_node.license_description = "Custom License"
+        sp_node.published = True
+        sp_node.save()
+
         self.channel.public = True
         self.channel.save()
 
-        with mock.patch.object(
-            ccmodels.AuditedSpecialPermissionsLicense,
-            "mark_channel_version_as_distributable",
-        ) as mock_mark:
-            fill_published_fields(
-                self.channel, "draft notes", draft_channel_version=self.draft_version
-            )
+        self._run_draft_publish()
+        draft_version = self._get_draft_version()
 
-        mock_mark.assert_not_called()
+        self.assertGreater(draft_version.special_permissions_included.count(), 0)
+        for license_obj in draft_version.special_permissions_included.all():
+            self.assertFalse(
+                license_obj.distributable,
+                "distributable must stay False for draft publishes",
+            )
diff --git a/contentcuration/contentcuration/utils/publish.py b/contentcuration/contentcuration/utils/publish.py
index eedd0f9d30..d4313c1e7e 100644
--- a/contentcuration/contentcuration/utils/publish.py
+++ b/contentcuration/contentcuration/utils/publish.py
@@ -1169,7 +1169,6 @@ def publish_channel(  # noqa: C901
             )
         else:
             increment_channel_version(channel)
-        if not is_draft_version:
             ccmodels.ChannelVersion.objects.filter(
                 channel=channel, version=None
             ).delete()

From 067248ec4e2c0fbf2d81f9f364ee7eb574c6467b Mon Sep 17 00:00:00 2001
From: rtibblesbot <richard+githubagent@learningequality.org>
Date: Thu, 16 Apr 2026 14:55:54 -0700
Subject: [PATCH 5/8] fix: replace channel.included_languages on publish
 instead of accumulating

Use .set() instead of .add() so languages removed from a channel are
cleared on subsequent publishes rather than accumulated indefinitely.

Flagged by AlexVelezLl, confirmed as a bug by rtibbles.
---
 contentcuration/contentcuration/utils/publish.py | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/contentcuration/contentcuration/utils/publish.py b/contentcuration/contentcuration/utils/publish.py
index d4313c1e7e..e30ea01225 100644
--- a/contentcuration/contentcuration/utils/publish.py
+++ b/contentcuration/contentcuration/utils/publish.py
@@ -1013,9 +1013,7 @@ def fill_published_fields(channel, version_notes, draft_channel_version=None):
         channel.published_kind_count = json.dumps(kind_counts)
         channel.published_size = published_size
 
-        for lang in language_list:
-            if lang:
-                channel.included_languages.add(lang)
+        channel.included_languages.set([lang for lang in language_list if lang])
 
         # TODO: Eventually, consolidate above operations to just use this field for storing historical data
         channel.published_data.update(

From fd3e20e3dab00bbd9d095329f2d6bdba5a67793a Mon Sep 17 00:00:00 2001
From: rtibblesbot <richard+githubagent@learningequality.org>
Date: Thu, 16 Apr 2026 15:45:12 -0700
Subject: [PATCH 6/8] fix: use get() instead of create() for Special
 Permissions license in tests
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

loadconstants inserts licenses with explicit PKs, leaving the PK sequence
at 1. Calling License.objects.create() in setUp() then collides with the
existing row. Use get() to fetch the pre-existing license — the same
pattern used in test_create_channel_versions.py and test_sync.py.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 .../contentcuration/tests/utils/test_publish.py        | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/contentcuration/contentcuration/tests/utils/test_publish.py b/contentcuration/contentcuration/tests/utils/test_publish.py
index 8f61a9790f..62005a7b01 100644
--- a/contentcuration/contentcuration/tests/utils/test_publish.py
+++ b/contentcuration/contentcuration/tests/utils/test_publish.py
@@ -211,11 +211,11 @@ def setUp(self):
         super().setUp()
         self.channel = testdata.channel()
 
-        # Create a Special Permissions license for use in tests.
-        self.special_perms_license = ccmodels.License.objects.create(
-            license_name=licenses.SPECIAL_PERMISSIONS,
-            license_description="Special Permissions",
-            license_url="",
+        # Fetch the Special Permissions license loaded by loadconstants.
+        # Do NOT call create() — loadconstants inserts licenses with explicit PKs
+        # so the PK sequence is still at 1, and create() would collide with id=1.
+        self.special_perms_license = ccmodels.License.objects.get(
+            license_name=licenses.SPECIAL_PERMISSIONS
         )
 
         # Give the channel an existing published version so version_info is set.

From 6683776fcd89effc97c59c5f2e03d2f908bcb088 Mon Sep 17 00:00:00 2001
From: rtibblesbot <richard+githubagent@learningequality.org>
Date: Thu, 16 Apr 2026 15:48:17 -0700
Subject: [PATCH 7/8] fix: move delete_public_channel_cache_keys into else
 branch of is_draft_version

The previous commit claimed to have merged all 'if not is_draft_version'
blocks into the else branch, but missed the delete_public_channel_cache_keys
call. Move it inside the else block and simplify the condition to 'if
channel.public'.
---
 contentcuration/contentcuration/utils/publish.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/contentcuration/contentcuration/utils/publish.py b/contentcuration/contentcuration/utils/publish.py
index e30ea01225..6f35f4a9a8 100644
--- a/contentcuration/contentcuration/utils/publish.py
+++ b/contentcuration/contentcuration/utils/publish.py
@@ -1182,9 +1182,9 @@ def publish_channel(  # noqa: C901
             base_tree.published = True
             base_tree.save()
 
-        # Delete public channel cache.
-        if not is_draft_version and channel.public:
-            delete_public_channel_cache_keys()
+            # Delete public channel cache.
+            if channel.public:
+                delete_public_channel_cache_keys()
 
         if send_email:
             with override(language):

From 3ab01589b146cacc3a176e1001112c131ba8b6bd Mon Sep 17 00:00:00 2001
From: rtibblesbot <richard+githubagent@learningequality.org>
Date: Fri, 17 Apr 2026 11:57:52 -0700
Subject: [PATCH 8/8] fix: revert queryset update to version_obj.save() for
 ChannelVersion metadata

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 .../contentcuration/utils/publish.py          | 21 ++++++++++---------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/contentcuration/contentcuration/utils/publish.py b/contentcuration/contentcuration/utils/publish.py
index 6f35f4a9a8..ffdb74a2ef 100644
--- a/contentcuration/contentcuration/utils/publish.py
+++ b/contentcuration/contentcuration/utils/publish.py
@@ -1034,17 +1034,18 @@ def fill_published_fields(channel, version_notes, draft_channel_version=None):
 
     version_obj = draft_channel_version if is_draft else channel.version_info
     if version_obj is not None:
-        ccmodels.ChannelVersion.objects.filter(pk=version_obj.pk).update(
-            resource_count=total_resource_count,
-            kind_count=kind_counts,
-            size=int(published_size),
-            date_published=date_now,
-            version_notes=version_notes,
-            included_languages=language_list,
-            included_licenses=license_list,
-            included_categories=category_list,
-            non_distributable_licenses_included=non_distributable_licenses_included,
+        version_obj.resource_count = total_resource_count
+        version_obj.kind_count = kind_counts
+        version_obj.size = int(published_size)
+        version_obj.date_published = date_now
+        version_obj.version_notes = version_notes
+        version_obj.included_languages = language_list
+        version_obj.included_licenses = license_list
+        version_obj.included_categories = category_list
+        version_obj.non_distributable_licenses_included = (
+            non_distributable_licenses_included
         )
+        version_obj.save()
 
         if special_perms_descriptions:
             version_obj.special_permissions_included.set(