Skip to content

Commit c3d876d

Browse files
fix: more validation of openedx_content to prevent corrupt states (#521)
Tests generated by Claude; fixes implemented by me. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 25bc5d0 commit c3d876d

7 files changed

Lines changed: 349 additions & 98 deletions

File tree

src/openedx_content/applets/backup_restore/zipper.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -780,7 +780,7 @@ def _save(
780780
self._save_subsections(learning_package_obj, containers)
781781
self._save_sections(learning_package_obj, containers)
782782
self._save_collections(learning_package_obj, collections)
783-
publishing_api.publish_all_drafts(learning_package_obj.id)
783+
publishing_api.publish_all_drafts(learning_package_obj.id)
784784

785785
with publishing_api.bulk_draft_changes_for(learning_package_obj.id):
786786
self._save_draft_versions(components, containers, component_static_files)

src/openedx_content/applets/publishing/api.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,14 @@ def create_publishable_entity_version(
237237
created_by_id=created_by,
238238
)
239239
if dependencies:
240+
# Validate that dependencies are from the same learning package:
241+
if (
242+
PublishableEntity.objects.filter(id__in=dependencies)
243+
.exclude(learning_package_id=version.entity.learning_package_id)
244+
.exists()
245+
):
246+
raise ValidationError("Dependencies must be from the same learning package")
247+
# Store dependencies:
240248
set_version_dependencies(version.id, dependencies)
241249

242250
set_draft_version(
@@ -466,6 +474,8 @@ def publish_from_drafts(
466474
By default, this will also publish all dependencies (e.g. unpinned children)
467475
of the Drafts that are passed in.
468476
"""
477+
if DraftChangeLogContext.get_active_draft_change_log(learning_package_id) is not None:
478+
raise ValidationError("Cannot publish while in bulk_draft_changes_for().")
469479
if published_at is None:
470480
published_at = datetime.now(tz=timezone.utc)
471481

@@ -509,6 +519,12 @@ def publish_from_drafts(
509519
# Skip duplicates that we might get from expanding dependencies.
510520
if draft.pk in published_draft_ids:
511521
continue
522+
# Validate Learning Package here where it won't require any extra queries
523+
if draft.entity.learning_package_id != learning_package_id:
524+
raise ValidationError(
525+
f"Draft entity (id={draft.entity.id}) is from learning package "
526+
f"{draft.entity.learning_package_id}; expected learning package {learning_package_id}."
527+
)
512528

513529
try:
514530
old_version = draft.entity.published.version
@@ -925,6 +941,17 @@ def set_draft_version(
925941
# block is bookkeeping in our DraftChangeLog.
926942
draft.version_id = publishable_entity_version_pk
927943

944+
# Validate the entity
945+
if publishable_entity_version_pk is not None:
946+
if draft.entity.id != PublishableEntityVersion.objects.only("entity_id").get(
947+
pk=publishable_entity_version_pk
948+
).entity_id:
949+
invalid_pev = PublishableEntityVersion.objects.get(pk=publishable_entity_version_pk)
950+
raise ValidationError(
951+
f"Entity mismatch - the specified PublishableEntityVersion ({repr(invalid_pev)}) does not match "
952+
f"the PublishableEntity ({repr(draft.entity)})."
953+
)
954+
928955
# Check to see if we're inside a context manager for an active
929956
# DraftChangeLog (i.e. what happens if the caller is using the public
930957
# bulk_draft_changes_for() API call), or if we have to make our own.

tests/openedx_content/applets/containers/test_api.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,10 +373,10 @@ def test_create_container_queries(lp: LearningPackage, child_entity1: TestEntity
373373
"container_cls": TestContainer,
374374
}
375375
# The exact numbers here aren't too important - this is just to alert us if anything significant changes.
376-
with django_assert_num_queries(33):
376+
with django_assert_num_queries(34):
377377
containers_api.create_container_and_version(lp.id, container_code="c1", **base_args)
378378
# And try with a a container that has children:
379-
with django_assert_num_queries(34):
379+
with django_assert_num_queries(37):
380380
containers_api.create_container_and_version(lp.id, container_code="c2", **base_args, entities=[child_entity1])
381381

382382

0 commit comments

Comments
 (0)