test: add tests for several places where our content APIs lack validation

bradenmacdonald · claude · bradenmacdonald · commit 5c69fc13e2e5 · 2026-03-30T13:57:21.000-07:00
Co-Authored-By: Claude Opus 4.6 &lt;noreply@anthropic.com&gt;
diff --git a/tests/openedx_content/applets/components/test_api.py b/tests/openedx_content/applets/components/test_api.py
@@ -5,7 +5,7 @@
 
 from django.contrib.auth import get_user_model
 from django.contrib.auth.models import User as UserType  # pylint: disable=imported-auth-user
-from django.core.exceptions import ObjectDoesNotExist
+from django.core.exceptions import ObjectDoesNotExist, ValidationError
 from django.test import TestCase
 
 from openedx_content.applets.collections import api as collection_api
@@ -724,3 +724,61 @@ def test_get_or_create_component_type_by_entity_key_invalid_format(self):
             components_api.get_or_create_component_type_by_entity_key("not-enough-parts")
 
         self.assertIn("Invalid entity_key format", str(ctx.exception))
+
+
+class CrossPackageMediaTestCase(ComponentTestCase):
+    """
+    Tests for validation gaps around cross-LearningPackage media references.
+    """
+    learning_package_2: LearningPackage
+
+    @classmethod
+    def setUpTestData(cls) -> None:
+        super().setUpTestData()
+        cls.learning_package_2 = publishing_api.create_learning_package(
+            key="CrossPackageMediaTestCase-lp2",
+            title="Second Learning Package for Cross-Package Media Test",
+        )
+
+    def test_create_component_version_media_rejects_cross_package_media(self) -> None:
+        """
+        create_component_version_media() should reject Media that belongs
+        to a different LearningPackage than the ComponentVersion.
+
+        If this validation is missing, a ComponentVersion in LP 1 can
+        reference Media data from LP 2. This violates the documented invariant
+        that Media is associated with a specific LearningPackage and breaks
+        the assumption that all content within a LearningPackage is
+        self-contained (e.g. for import/export, deletion, storage accounting).
+        """
+        # Create a component + version in LP 1
+        _component, component_version = components_api.create_component_and_version(
+            self.learning_package.id,
+            component_type=self.problem_type,
+            local_key="component_in_lp1",
+            title="Component in LP 1",
+            created=self.now,
+            created_by=None,
+        )
+
+        # Create media in LP 2
+        text_media_type = media_api.get_or_create_media_type("text/plain")
+        media_in_lp2 = media_api.get_or_create_text_media(
+            self.learning_package_2.id,
+            text_media_type.id,
+            text="This media belongs to LP 2",
+            created=self.now,
+        )
+
+        # Confirm the media is in LP 2, not LP 1
+        assert media_in_lp2.learning_package_id == self.learning_package_2.id
+        assert media_in_lp2.learning_package_id != self.learning_package.id
+
+        # This should raise an error because media_in_lp2 is from a different
+        # LearningPackage than the ComponentVersion.
+        with self.assertRaises((ValidationError, ValueError)):
+            components_api.create_component_version_media(
+                component_version.pk,
+                media_in_lp2.pk,
+                key="cross_package_file.txt",
+            )
diff --git a/tests/openedx_content/applets/containers/test_api.py b/tests/openedx_content/applets/containers/test_api.py
@@ -1646,3 +1646,59 @@ def test_soft_delete_container(lp: LearningPackage, parent_of_two: TestContainer
     child_entity1.refresh_from_db()
     assert child_entity1.versioning.draft == child_entity1.versioning.published
     assert child_entity1.versioning.draft is not None
+
+
+def test_publish_container_without_children_should_fail(lp: LearningPackage):
+    """
+    Publishing a container with publish_dependencies=False when its unpinned
+    children have never been published should either fail at publish time or
+    produce a readable published state.
+
+    If this validation is missing, the published container references child
+    entities that have no Published row. Reading the published container's
+    contents via get_entities_in_container(published=True) will crash with
+    RelatedObjectDoesNotExist because row.entity.published doesn't exist for
+    never-published children.
+    """
+    # Create child entities (draft-only, never published)
+    child_1 = create_test_entity(lp, key="unpublished_child_1", title="Unpublished Child 1")
+    child_2 = create_test_entity(lp, key="unpublished_child_2", title="Unpublished Child 2")
+
+    # Create a container with unpinned references to these children
+    container = create_test_container(
+        lp,
+        key="container_with_unpublished_children",
+        title="Container with Unpublished Children",
+        entities=[child_1, child_2],  # unpinned references
+    )
+
+    # Publish ONLY the container, skipping its dependencies (children).
+    # This should either:
+    # (a) raise an error at publish time (preventing the bad state), or
+    # (b) produce a published state where get_entities_in_container works
+    #     gracefully (e.g. returns an empty list for unpublished children).
+    container_drafts = publishing_api.get_all_drafts(lp.id).filter(
+        entity=container.publishable_entity,
+    )
+    publish_log = publishing_api.publish_from_drafts(
+        lp.id,
+        container_drafts,
+        publish_dependencies=False,
+    )
+    assert publish_log is not None
+
+    # The children were never published, so reading the published container
+    # should not crash. It should either raise a clear error, or gracefully
+    # handle the missing children.
+    container.refresh_from_db()
+    assert container.versioning.published is not None
+
+    # This is the call that currently crashes with RelatedObjectDoesNotExist
+    # because the unpinned children have no Published row.
+    entries = containers_api.get_entities_in_container(
+        container,
+        published=True,
+    )
+    # If we get here without crashing, the children should be excluded since
+    # they were never published.
+    assert len(entries) == 0
diff --git a/tests/openedx_content/applets/publishing/test_api.py b/tests/openedx_content/applets/publishing/test_api.py
@@ -19,7 +19,9 @@
     DraftSideEffect,
     LearningPackage,
     PublishableEntity,
+    PublishableEntityVersion,
     PublishLog,
+    Published,
 )
 
 User = get_user_model()
@@ -1483,3 +1485,169 @@ def test_container_next_version(self) -> None:
 # Test that I can get a [PublishLog] history of a given container and its children, that includes changes made to the
 #     child components while they were part of the container but excludes changes made to those children while they were
 #     not part of the container. 🫣
+
+
+class CrossEntityValidationTestCase(TestCase):
+    """
+    Tests for validation gaps where API calls can corrupt state by mixing
+    entities/versions/packages that shouldn't be combined.
+    """
+    now: datetime
+    learning_package_1: LearningPackage
+    learning_package_2: LearningPackage
+
+    @classmethod
+    def setUpTestData(cls) -> None:
+        cls.now = datetime(2024, 6, 15, 12, 0, 0, tzinfo=timezone.utc)
+        cls.learning_package_1 = publishing_api.create_learning_package(
+            "cross_entity_validation_lp_1",
+            "Cross-Entity Validation LP 1",
+            created=cls.now,
+        )
+        cls.learning_package_2 = publishing_api.create_learning_package(
+            "cross_entity_validation_lp_2",
+            "Cross-Entity Validation LP 2",
+            created=cls.now,
+        )
+
+    def test_set_draft_version_rejects_version_from_different_entity(self) -> None:
+        """
+        set_draft_version() should reject a PublishableEntityVersion that
+        belongs to a different PublishableEntity.
+
+        If this validation is missing, entity_a's Draft will point to a version
+        that was defined for entity_b. This corrupts the publishing state:
+        component_a.versioning.draft would return component_b's data, and
+        publishing would propagate the wrong content.
+        """
+        entity_a = publishing_api.create_publishable_entity(
+            self.learning_package_1.id,
+            "entity_a",
+            created=self.now,
+            created_by=None,
+        )
+        entity_b = publishing_api.create_publishable_entity(
+            self.learning_package_1.id,
+            "entity_b",
+            created=self.now,
+            created_by=None,
+        )
+
+        # Create v1 for entity_a (draft_a -> v1)
+        publishing_api.create_publishable_entity_version(
+            entity_a.id,
+            version_num=1,
+            title="Entity A v1",
+            created=self.now,
+            created_by=None,
+        )
+
+        # Create v1 and v2 for entity_b. After v2 is created, entity_b's
+        # draft points to v2, so v1 is "free" (no Draft points to it) and
+        # won't trigger a OneToOne constraint violation.
+        version_b_v1 = publishing_api.create_publishable_entity_version(
+            entity_b.id,
+            version_num=1,
+            title="Entity B v1",
+            created=self.now,
+            created_by=None,
+        )
+        publishing_api.create_publishable_entity_version(
+            entity_b.id,
+            version_num=2,
+            title="Entity B v2",
+            created=self.now,
+            created_by=None,
+        )
+
+        # Confirm version_b_v1 belongs to entity_b, not entity_a.
+        assert version_b_v1.entity_id == entity_b.id
+        assert version_b_v1.entity_id != entity_a.id
+
+        # This should raise an error because version_b_v1 belongs to entity_b,
+        # not entity_a. Without validation, this silently corrupts entity_a's
+        # draft to point to entity_b's content.
+        with pytest.raises((ValidationError, ValueError)):
+            publishing_api.set_draft_version(entity_a.id, version_b_v1.pk)
+
+    def test_publish_from_drafts_rejects_cross_package_drafts(self) -> None:
+        """
+        publish_from_drafts() should reject drafts that don't belong to the
+        specified LearningPackage.
+
+        If this validation is missing, a PublishLog is created for LP 1 but
+        with PublishLogRecords referencing entities from LP 2. The Published
+        rows for LP 2's entities would point to records in LP 1's PublishLog,
+        corrupting the publish history for both packages.
+        """
+        # Create an entity in LP 2
+        entity_in_lp2 = publishing_api.create_publishable_entity(
+            self.learning_package_2.id,
+            "entity_in_lp2",
+            created=self.now,
+            created_by=None,
+        )
+        publishing_api.create_publishable_entity_version(
+            entity_in_lp2.id,
+            version_num=1,
+            title="Entity in LP2",
+            created=self.now,
+            created_by=None,
+        )
+
+        # Get drafts from LP 2
+        drafts_from_lp2 = Draft.objects.filter(
+            entity__learning_package_id=self.learning_package_2.id
+        )
+        assert drafts_from_lp2.exists()
+
+        # This should raise an error because we're trying to publish LP 2's
+        # drafts under LP 1's PublishLog.
+        with pytest.raises((ValidationError, ValueError)):
+            publishing_api.publish_from_drafts(
+                self.learning_package_1.id,
+                drafts_from_lp2,
+            )
+
+    def test_create_version_rejects_cross_package_dependencies(self) -> None:
+        """
+        create_publishable_entity_version() should reject dependencies that
+        are from a different LearningPackage.
+
+        If this validation is missing, PublishableEntityVersionDependency rows
+        are created linking entities across packages. The side-effect machinery
+        would then propagate draft/publish changes across LearningPackage
+        boundaries, creating DraftChangeLogRecords and PublishLogRecords in the
+        wrong package's logs.
+        """
+        entity_in_lp1 = publishing_api.create_publishable_entity(
+            self.learning_package_1.id,
+            "entity_in_lp1",
+            created=self.now,
+            created_by=None,
+        )
+        entity_in_lp2 = publishing_api.create_publishable_entity(
+            self.learning_package_2.id,
+            "dep_entity_in_lp2",
+            created=self.now,
+            created_by=None,
+        )
+        publishing_api.create_publishable_entity_version(
+            entity_in_lp2.id,
+            version_num=1,
+            title="Dependency in LP2",
+            created=self.now,
+            created_by=None,
+        )
+
+        # This should raise an error because entity_in_lp2 is from a
+        # different LearningPackage than entity_in_lp1.
+        with pytest.raises((ValidationError, ValueError)):
+            publishing_api.create_publishable_entity_version(
+                entity_in_lp1.id,
+                version_num=1,
+                title="Entity in LP1 with cross-package dep",
+                created=self.now,
+                created_by=None,
+                dependencies=[entity_in_lp2.id],
+            )
