Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/openedx_content/applets/publishing/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ class PublishLogRecordTabularInline(admin.TabularInline):
"old_version_num",
"new_version_num",
"dependencies_hash_digest",
"direct",
)
readonly_fields = fields

Expand Down
4 changes: 4 additions & 0 deletions src/openedx_content/applets/publishing/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,9 @@ def publish_from_drafts(
else:
dependency_drafts_qsets = []

# Collect PKs of directly-requested drafts before expanding dependencies.
direct_draft_ids = set(draft_qset.values_list('pk', flat=True))

# One PublishLog for this entire publish operation.
publish_log = PublishLog(
learning_package_id=learning_package_id,
Expand Down Expand Up @@ -484,6 +487,7 @@ def publish_from_drafts(
entity=draft.entity,
old_version=old_version,
new_version=draft.version,
direct=draft.pk in direct_draft_ids,
)
publish_log_record.full_clean()
publish_log_record.save(force_insert=True)
Expand Down
71 changes: 71 additions & 0 deletions src/openedx_content/applets/publishing/models/publish_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,77 @@ class PublishLogRecord(models.Model):
# the values may drift away from each other.
dependencies_hash_digest = hash_field(blank=True, default='', max_length=8)

# The "direct" field captures user intent during the publishing process. It
# is True if the user explicitly requested to publish the entity represented
# by this PublishLogRecord—i.e. they clicked "publish" on this entity or
# selected it for bulk publish.
#
# This field is False if this entity was indirectly published either as a
# child/dependency or side-effect of a directly published entity.
#
# If this field is None, that means that this PublishLogRecord was created
# before we started capturing user intent (pre-Verawood release), and we
# cannot reliably infer what the user clicked on. For example, say we had a
# Subsection > Unit > Component arrangement where the Component had an
# unpublished change. The user is allowed to press the "publish" button at
# the Subsection, Unit, or Component levels in the UI. Before we started
# recording this field, the resulting PublishLogs would have looked
# identical in all three cases: a version change PublishLogRecord for
# Component, and side-effect records for the Unit and Subsection. Therefore,
# we cannot accurately backfill this field.
#
# Here are some examples to illustrate how "direct" gets set and why:
#
# Example 1: The user clicks "publish" on a Component that's in a Unit.
#
# The Component has direct=True, but the side-effect PublishLogRecord for
# the Unit has direct=False. Likewise, any side-effect records at higher
# levels (subsection, section) also have direct=False.
#
# Example 2: The user clicks "publish" on a Unit, where both the Unit and
# Component have unpublished changes:
#
# In this case, the Unit has direct=True, and the Component has
# direct=False. The draft status of the Component is irrelevant. The user
# asked for the Unit to the published, so the Unit's PublishLogRecord is
# the only thing that gets direct=True.
#
# Example 3: The user clicks "publish" on a Unit that has no changes of its
# own (draft version == published version), but the Unit contains a Component
# that has changes.
#
# Again, only the PublishLogRecord for the Unit has direct=True. The
# Component's PublishLogRecord has direct=False. Even though the Unit's
# published version_num does not change (i.e. it is purely a side-effect
# publish), the user intent was to publish the Unit (and anything it
# contains), so the Unit gets direct=True.
#
# Example 4: The user selects multiple entities for bulk publishing.
#
# Those exact entities that the user selected get direct=True. It does not
# matter if some of those entities are children of other selected items or
# not. Other entries like dependencies or side-effects have direct=False.
#
# Example 5: The user selects "publish all".
#
# Selecting "publish all" in our system currently translates into "publish
# all the entities that have a draft version that is different from its
# published version". Those entities would get PublishLogRecords with
# direct=True, while all side-effects would get records with direct=False.
# So if a Unit's draft and published versions match, and one of its
# Components has unpublished changes, then "publish all" would cause the
# Component's record to have direct=True and the Unit's record to have
# direct=False.
#
# All PublishLogRecords in the PublishLog have direct=True. The "publish
# all" operation is indistinguishable from bulk publishing and selecting
# every single item.
direct = models.BooleanField(
null=True,
blank=True,
default=False,
)

class Meta:
constraints = [
# A Publishable can have only one PublishLogRecord per PublishLog.
Expand Down
32 changes: 32 additions & 0 deletions src/openedx_content/migrations/0007_publishlogrecord_direct.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Generated by Django 5.2.13 on 2026-04-13 21:28

from django.db import migrations, models


def backfill_direct_to_none(apps, schema_editor):
"""
Set direct=None for all pre-existing PublishLogRecords so they are treated
as historical records whose user intent cannot be determined retroactively.
New records created after this migration will default to direct=False.
"""
PublishLogRecord = apps.get_model('openedx_content', 'PublishLogRecord')
PublishLogRecord.objects.update(direct=None)


class Migration(migrations.Migration):

dependencies = [
('openedx_content', '0006_typed_ids'),
]

operations = [
migrations.AddField(
model_name='publishlogrecord',
name='direct',
field=models.BooleanField(blank=True, default=False, null=True),
),
migrations.RunPython(
backfill_direct_to_none,
reverse_code=migrations.RunPython.noop,
),
]
8 changes: 4 additions & 4 deletions tests/openedx_content/applets/containers/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -925,7 +925,7 @@ def test_contains_unpublished_changes_queries(
assert containers_api.contains_unpublished_changes(grandparent.id)

# Publish grandparent and all its descendants:
with django_assert_num_queries(135): # TODO: investigate as this seems high!
with django_assert_num_queries(136): # TODO: investigate as this seems high!
publish_entity(grandparent)

# Tests:
Expand Down Expand Up @@ -1244,7 +1244,7 @@ def test_uninstalled_publish(
"""Simple test of publishing a container of uninstalled type, plus its child, and reviewing the publish log"""
# Publish container_of_uninstalled_type (and child_entity1). Should not affect anything else,
# but we should see "child_entity1" omitted from the subsequent publish.
with django_assert_num_queries(49):
with django_assert_num_queries(50):
publish_log = publish_entity(container_of_uninstalled_type)
# Nothing else should have been affected by the publish:
assert list(publish_log.records.order_by("entity__pk").values_list("entity__key", flat=True)) == [
Expand Down Expand Up @@ -1282,7 +1282,7 @@ def test_deep_publish_log(
)
# Publish container_of_uninstalled_type (and child_entity1). Should not affect anything else,
# but we should see "child_entity1" omitted from the subsequent publish.
with django_assert_num_queries(49):
with django_assert_num_queries(50):
publish_log = publish_entity(container_of_uninstalled_type)
# Nothing else should have been affected by the publish:
assert list(publish_log.records.order_by("entity__pk").values_list("entity__key", flat=True)) == [
Expand All @@ -1291,7 +1291,7 @@ def test_deep_publish_log(
]

# Publish great_grandparent. Should publish the whole tree.
with django_assert_num_queries(126):
with django_assert_num_queries(127):
publish_log = publish_entity(great_grandparent)
assert list(publish_log.records.order_by("entity__pk").values_list("entity__key", flat=True)) == [
"child_entity2",
Expand Down
203 changes: 203 additions & 0 deletions tests/openedx_content/applets/publishing/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
LearningPackage,
PublishableEntity,
PublishLog,
PublishLogRecord,
)

User = get_user_model()
Expand Down Expand Up @@ -935,6 +936,56 @@ def test_simple_publish_log(self) -> None:
assert e1_pub_record.old_version == entity1_v1
assert e1_pub_record.new_version == entity1_v2

def test_publish_all_drafts_sets_direct_true(self) -> None:
"""publish_all_drafts() marks every PublishLogRecord as direct=True."""
entity_1 = publishing_api.create_publishable_entity(
self.learning_package_1.id, "direct_entity_1",
created=self.now, created_by=None,
)
publishing_api.create_publishable_entity_version(
entity_1.id, version_num=1, title="Direct Entity 1",
created=self.now, created_by=None,
)
entity_2 = publishing_api.create_publishable_entity(
self.learning_package_1.id, "direct_entity_2",
created=self.now, created_by=None,
)
publishing_api.create_publishable_entity_version(
entity_2.id, version_num=1, title="Direct Entity 2",
created=self.now, created_by=None,
)
publish_log = publishing_api.publish_all_drafts(self.learning_package_1.id)
assert publish_log.records.get(entity=entity_1).direct is True
assert publish_log.records.get(entity=entity_2).direct is True

def test_publish_from_drafts_sets_direct_true(self) -> None:
"""An explicitly selected entity in publish_from_drafts() gets direct=True."""
entity = publishing_api.create_publishable_entity(
self.learning_package_1.id, "explicit_entity",
created=self.now, created_by=None,
)
publishing_api.create_publishable_entity_version(
entity.id, version_num=1, title="Explicit Entity",
created=self.now, created_by=None,
)
publish_log = publishing_api.publish_from_drafts(
self.learning_package_1.id,
Draft.objects.filter(entity=entity),
)
assert publish_log.records.get(entity=entity).direct is True

def test_publish_log_record_direct_defaults_to_false(self) -> None:
"""
New PublishLogRecords default to direct=False (not None).

None is reserved for historical records that pre-date the direct field
(set via the backfill data migration). Records created by the
application—e.g. side-effect records in _create_side_effects_for_change_log()
that don't explicitly set direct—should get False, not None.
"""
field = PublishLogRecord._meta.get_field('direct')
assert field.default is False


class EntitiesQueryTestCase(TestCase):
"""
Expand Down Expand Up @@ -1429,6 +1480,158 @@ def test_publish_all_layers(self) -> None:
# the publish log records.
assert publish_log.records.count() == 3

def test_direct_field_publishing_container_marks_dependencies_indirect(self) -> None:
"""
Publishing a Unit explicitly marks the Unit as direct=True and its
unpublished Component dependency as direct=False.
"""
component = publishing_api.create_publishable_entity(
self.learning_package.id, "direct_component",
created=self.now, created_by=None,
)
publishing_api.create_publishable_entity_version(
component.id, version_num=1, title="Direct Component",
created=self.now, created_by=None,
)
unit = containers_api.create_container(
self.learning_package.id, "direct_unit",
created=self.now, created_by=None, container_cls=TestContainer,
)
containers_api.create_container_version(
unit.id, 1, title="Direct Unit", entities=[component],
created=self.now, created_by=None,
)
publish_log = publishing_api.publish_from_drafts(
self.learning_package.id,
Draft.objects.filter(entity=unit.publishable_entity),
)
assert publish_log.records.get(entity=unit.publishable_entity).direct is True
assert publish_log.records.get(entity=component).direct is False
Comment thread
ChrisChV marked this conversation as resolved.

def test_direct_field_unit_no_version_change_still_direct_true(self) -> None:
"""
Publishing a Unit that has no version change of its own (draft version
== published version) still marks the Unit's record as direct=True.

The user explicitly selected the Unit to publish, so it gets direct=True
even though the only actual change is in its Component child. The Unit's
record has old_version == new_version (pure side-effect in terms of
versioning), but user intent was directed at the Unit.
"""
component = publishing_api.create_publishable_entity(
self.learning_package.id, "no_change_component",
created=self.now, created_by=None,
)
component_v1 = publishing_api.create_publishable_entity_version(
component.id, version_num=1, title="No-change Component",
created=self.now, created_by=None,
)
unit = containers_api.create_container(
self.learning_package.id, "no_change_unit",
created=self.now, created_by=None, container_cls=TestContainer,
)
unit_v1 = containers_api.create_container_version(
unit.id, 1, title="No-change Unit", entities=[component],
created=self.now, created_by=None,
)
# Initial publish so both Unit and Component have a published version.
publishing_api.publish_from_drafts(
self.learning_package.id,
Draft.objects.filter(entity=unit.publishable_entity),
)

# Create a new Component version. The Unit's draft stays at unit_v1,
# but its dependencies_hash_digest now differs from the published state.
publishing_api.create_publishable_entity_version(
component.id, version_num=2, title="No-change Component v2",
created=self.now, created_by=None,
)

# Publish the Unit explicitly. The Unit has no version change of its
# own (old_version == new_version == unit_v1).
publish_log = publishing_api.publish_from_drafts(
self.learning_package.id,
Draft.objects.filter(entity=unit.publishable_entity),
)
unit_record = publish_log.records.get(entity=unit.publishable_entity)
component_record = publish_log.records.get(entity=component)

# User selected the Unit → direct=True despite no version change.
assert unit_record.direct is True
assert unit_record.old_version_id == unit_v1.pk
assert unit_record.new_version_id == unit_v1.pk

# Component was pulled in as a dependency → direct=False.
assert component_record.direct is False
assert component_record.old_version == component_v1
assert component_record.new_version != component_v1

def test_direct_field_publishing_component_marks_parent_indirect(self) -> None:
"""
Publishing a Component directly marks the Component as direct=True.
The parent Unit also gets a PublishLogRecord (because it has an unpinned
reference to the Component and its dependencies_hash_digest now differs
from the published state) with direct=False.
"""
component = publishing_api.create_publishable_entity(
self.learning_package.id, "leaf_component",
created=self.now, created_by=None,
)
publishing_api.create_publishable_entity_version(
component.id, version_num=1, title="Leaf Component",
created=self.now, created_by=None,
)
unit = containers_api.create_container(
self.learning_package.id, "leaf_unit",
created=self.now, created_by=None, container_cls=TestContainer,
)
containers_api.create_container_version(
unit.id, 1, title="Leaf Unit", entities=[component],
created=self.now, created_by=None,
)
# First publish everything to establish a published baseline for the Unit
publishing_api.publish_all_drafts(self.learning_package.id)

# Create a new component version so it has unpublished changes
publishing_api.create_publishable_entity_version(
component.id, version_num=2, title="Leaf Component v2",
created=self.now, created_by=None,
)
publish_log = publishing_api.publish_from_drafts(
self.learning_package.id,
Draft.objects.filter(entity=component),
)
assert publish_log.records.get(entity=component).direct is True
assert publish_log.records.get(entity=unit.publishable_entity).direct is False

def test_direct_field_both_selected_both_direct(self) -> None:
"""
When both a Unit and its Component are explicitly selected, both
get direct=True even though Component is also a dependency of Unit.
"""
component = publishing_api.create_publishable_entity(
self.learning_package.id, "both_component",
created=self.now, created_by=None,
)
publishing_api.create_publishable_entity_version(
component.id, version_num=1, title="Both Component",
created=self.now, created_by=None,
)
unit = containers_api.create_container(
self.learning_package.id, "both_unit",
created=self.now, created_by=None, container_cls=TestContainer,
)
containers_api.create_container_version(
unit.id, 1, title="Both Unit", entities=[component],
created=self.now, created_by=None,
)
publish_log = publishing_api.publish_from_drafts(
self.learning_package.id,
Draft.objects.filter(entity__in=[component, unit.publishable_entity]),
)
assert publish_log.records.get(entity=component).direct is True
assert publish_log.records.get(entity=unit.publishable_entity).direct is True

def test_container_next_version(self) -> None:
"""Test that next_version works for containers."""
child_1 = publishing_api.create_publishable_entity(
Expand Down
Loading