Skip to content

Commit

Permalink
Add a contains_unpublished_changes API
Browse files Browse the repository at this point in the history
  • Loading branch information
bradenmacdonald committed Feb 14, 2025
1 parent dd73ee5 commit 4ee8f46
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 10 deletions.
43 changes: 43 additions & 0 deletions openedx_learning/apps/authoring/containers/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"ContainerEntityListEntry",
"get_entities_in_draft_container",
"get_entities_in_published_container",
"contains_unpublished_changes",
]


Expand Down Expand Up @@ -538,3 +539,45 @@ def get_entities_in_published_container(
pinned=row.published_version is not None,
))
return entity_list


def contains_unpublished_changes(
container: ContainerEntity | ContainerEntityMixin,
) -> bool:
"""
Check recursively if a container has any unpublished changes.
Note: container.versioning.has_unpublished_changes only checks if the container
itself has unpublished changes, not if its contents do.
"""
if isinstance(container, ContainerEntityMixin):
container = container.container_entity
assert isinstance(container, ContainerEntity)

if container.versioning.has_unpublished_changes:
return True

# We only care about children that are un-pinned, since published changes to pinned children don't matter
defined_list = container.versioning.draft.defined_list

# TODO: This is a naive inefficient implementation but hopefully correct.
# Once we know it's correct and have a good test suite, then we can optimize.
# We will likely change to a tracking-based approach rather than a "scan for changes" based approach.
for row in defined_list.entitylistrow_set.filter(draft_version=None):
try:
child_container = row.entity.containerentity
except PublishableEntity.containerentity.RelatedObjectDoesNotExist:
child_container = None
if child_container:
child_container = row.entity.containerentity
# This is itself a container - check recursively:
if child_container.versioning.has_unpublished_changes or contains_unpublished_changes(child_container):
return True
else:
# This is not a container:
draft_pk = row.entity.draft.version_id if row.entity.draft else None
published_pk = row.entity.published.version_id if row.entity.published else None
if draft_pk != published_pk:
return True

return False
33 changes: 23 additions & 10 deletions tests/openedx_learning/apps/authoring/units/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ def test_add_component_after_publish(self):
# Publish the empty unit:
authoring_api.publish_all_drafts(self.learning_package.id)
unit.refresh_from_db() # Reloading the unit is necessary
assert unit.versioning.has_unpublished_changes == False
assert unit.versioning.has_unpublished_changes == False # Shallow check for just the unit itself, not children
assert authoring_api.contains_unpublished_changes(unit) == False # Deeper check

# Add a published component (unpinned):
assert self.component_1.versioning.has_unpublished_changes == False
Expand All @@ -128,16 +129,17 @@ def test_add_component_after_publish(self):
)
# Now the unit should have unpublished changes:
unit.refresh_from_db() # Reloading the unit is necessary
assert unit.versioning.has_unpublished_changes == True
assert unit.versioning.has_unpublished_changes == True # Shallow check - adding a child is a change to the unit
assert authoring_api.contains_unpublished_changes(unit) == True # Deeper check
assert unit.versioning.draft == unit_version_v2
assert unit.versioning.published == unit_version

def test_modify_component_after_publish(self):
"""
Modifying a component in a published unit will NOT create a new version
nor show that the unit has unpublished changes. The modifications will
appear in the published version of the unit only after the component is
published.
nor show that the unit has unpublished changes (but it will "contain"
unpublished changes). The modifications will appear in the published
version of the unit only after the component is published.
"""
# Create a unit:
unit, unit_version = authoring_api.create_unit_and_version(
Expand All @@ -164,7 +166,8 @@ def test_modify_component_after_publish(self):
authoring_api.publish_all_drafts(self.learning_package.id)
unit.refresh_from_db() # Reloading the unit is necessary
self.component_1.refresh_from_db()
assert unit.versioning.has_unpublished_changes == False
assert unit.versioning.has_unpublished_changes == False # Shallow check
assert authoring_api.contains_unpublished_changes(unit) == False # Deeper check
assert self.component_1.versioning.has_unpublished_changes == False

# Now modify the component by changing its title (it remains a draft):
Expand All @@ -176,10 +179,11 @@ def test_modify_component_after_publish(self):
created_by=None,
)

# The component now has unpublished changes, but the unit doesn't (⭐️ Is this what we want? ⭐️)
# The component now has unpublished changes; the unit doesn't directly but does contain
unit.refresh_from_db() # Reloading the unit is necessary
self.component_1.refresh_from_db()
assert unit.versioning.has_unpublished_changes == False
assert unit.versioning.has_unpublished_changes == False # Shallow check should be false - unit is unchanged
assert authoring_api.contains_unpublished_changes(unit) == True # But unit DOES contain changes
assert self.component_1.versioning.has_unpublished_changes == True

# Since the component changes haven't been published, they should only appear in the draft unit
Expand All @@ -198,13 +202,22 @@ def test_modify_component_after_publish(self):
assert authoring_api.get_components_in_published_unit(unit) == [
authoring_api.UnitListEntry(component_version=component_1_v2, pinned=False), # new version
]
assert authoring_api.contains_unpublished_changes(unit) == False # No longer contains unpublished changes


# Test query count of contains_unpublished_changes()
# Test that only components can be added to units
# Test that components must be in the same learning package
# Test that _version_pks=[] arguments must be related to publishable_entities_pks
# Test that publishing a unit publishes its components
# Test viewing old snapshots of units and components by passing in a timestamp to some get_historic_unit() API
# Test that publishing a unit publishes its child components (either automatically or throws an exception if you
# don't request the children be published together with the containeer)
# Test that publishing a component does NOT publish changes to its parent unit
# Test that I can get a history of a given unit and all its children, including children that aren't currently in
# the unit and excluding children that are only in other units.
# Test that I can get a history of a given unit and its children, that includes changes made to the child components
# while they were part of the unit but excludes changes made to those children while they were not part of
# the unit. 🫣
# Test viewing old snapshots of units and components by passing in a timestamp to some get_historic_unit() API?


def test_next_version_with_different_different_title(self):
Expand Down

0 comments on commit 4ee8f46

Please sign in to comment.