Protect published snapshots and concurrent edits

apps/content/models.py

@@ -31,6 +31,9 @@ class MediaAsset(models.Model):
     approved = models.BooleanField(default=False)
     created_at = models.DateTimeField(auto_now_add=True)
 
+    def __str__(self):
+        return self.file.name or f"Media asset {self.pk}"
+
     def clean(self):
         super().clean()
         for field in ("alt", "source", "license"):
@@ -60,6 +63,11 @@ class MediaAsset(models.Model):
         self.bytes = size
 
     def save(self, *args, **kwargs):
+        if self.pk:
+            previous = type(self).objects.get(pk=self.pk)
+            protected = ("file", "alt", "source", "license", "width", "height", "bytes")
+            if previous.approved and (not self.approved or any(getattr(self, field) != getattr(previous, field) for field in protected)):
+                raise ValidationError("Approved media is immutable.")
         self.full_clean()
         return super().save(*args, **kwargs)
 
@@ -77,6 +85,9 @@ class SiteIdentity(models.Model):
             ("restore_content", "Can restore published content"),
         ]
 
+    def __str__(self):
+        return "Association site identity"
+
     def clean(self):
         if self.published_revision_id and self.published_revision.site_id != self.pk:
             raise ValidationError("Published revision belongs to another site.")
@@ -126,6 +137,8 @@ class SiteRevision(models.Model):
             protected = ("site_id", "number", "official_name", "short_name", "faculty_subline", "description", "logo_id", "logo_alt", "navigation", "contacts", "footer_copy", "seo_title", "seo_description", "seo_image_id", "created_by_id", "source_seed_key", "source_seed_version", "content_checksum")
             if any(getattr(self, field) != getattr(previous, field) for field in protected):
                 raise ValidationError("Site revision content is immutable.")
+            if previous.status == "published" and (self.status != previous.status or self.published_by_id != previous.published_by_id or self.published_at != previous.published_at):
+                raise ValidationError("Published site revision metadata is immutable.")
         return super().save(*args, **kwargs)
 
 
@@ -134,6 +147,9 @@ class EditorialPage(models.Model):
     page_kind = models.CharField(max_length=20, choices=[("sections", "Sections"), ("listing_intro", "Listing intro")])
     published_revision = models.ForeignKey("PageRevision", null=True, blank=True, on_delete=models.PROTECT, related_name="published_for")
 
+    def __str__(self):
+        return self.slug
+
     def clean(self):
         super().clean()
         if self.slug not in PAGE_KINDS or self.page_kind != PAGE_KINDS[self.slug]:
@@ -177,6 +193,8 @@ class PageRevision(models.Model):
                 raise ValidationError("Page revision content is immutable.")
             if self.content_checksum != previous.content_checksum and previous.content_checksum:
                 raise ValidationError("Page revision checksum is immutable.")
+            if previous.status == "published" and (self.status != previous.status or self.published_by_id != previous.published_by_id or self.published_at != previous.published_at):
+                raise ValidationError("Published page revision metadata is immutable.")
         return super().save(*args, **kwargs)
 
 
@@ -218,7 +236,8 @@ class PageSection(models.Model):
         if self.key == "contact" and not self.visible:
             raise ValidationError("About contact must be visible.")
         validators.text(self.heading, 120 if self.key in {"hero", "identity"} else 90, required=True)
-        validators.text(self.body, 1200 if slug == "about" else 600)
+        body_limit = 500 if self.key == "hero" else 600 if self.key in {"participation", "identity"} else 1200
+        validators.text(self.body, body_limit)
         validators.text(self.intro, 300)
         validators.text(self.all_link_label, 40)
         validators.text(self.image_alt, 180)

apps/content/services.py

@@ -109,6 +109,8 @@ def save_page_draft(actor, page: EditorialPage, fields: dict, sections: list[dic
         base_revision = PageRevision.objects.get(pk=base_revision.pk)
         if base_revision.page_id != page.pk:
             raise ValidationError("Base revision belongs to another page.")
+        if base_revision.pk != page.revisions.order_by("-number").values_list("pk", flat=True).first():
+            raise PublishConflict("A newer revision exists. Reload the draft before editing.")
     if not isinstance(sections, list):
         raise ValidationError("Sections must be a list.")
     revision = PageRevision(page=page, number=_next_number(page), created_by=actor, **_fields(fields, PAGE_FIELDS))
@@ -132,6 +134,8 @@ def save_site_draft(actor, site: SiteIdentity, fields: dict, *, base_revision=No
         base_revision = SiteRevision.objects.get(pk=base_revision.pk)
         if base_revision.site_id != site.pk:
             raise ValidationError("Base revision belongs to another site.")
+        if base_revision.pk != site.revisions.order_by("-number").values_list("pk", flat=True).first():
+            raise PublishConflict("A newer revision exists. Reload the draft before editing.")
     revision = SiteRevision(site=site, number=_next_number(site), created_by=actor, **_fields(fields, SITE_FIELDS))
     revision.full_clean()
     revision.content_checksum = _site_checksum(revision)

apps/content/tests/integration/test_services.py

@@ -1,10 +1,13 @@
 import io
+from concurrent.futures import ThreadPoolExecutor
+from threading import Barrier
 
 import pytest
 from django.contrib.auth.models import Group, Permission
 from django.core.exceptions import ValidationError
 from django.core.exceptions import PermissionDenied
 from django.core.files.uploadedfile import SimpleUploadedFile
+from django.db import connection, connections, close_old_connections
 from PIL import Image
 
 from apps.content.models import EditorialPage, MediaAsset, PageRevision, PublicationAudit, SiteIdentity
@@ -103,6 +106,34 @@ def test_stale_publish_is_rejected_without_swapping_pointer():
     assert second.status == "draft"
 
 
+@pytest.mark.django_db(transaction=True)
+@pytest.mark.skipif(connection.vendor != "postgresql", reason="SQLite has no row-level select_for_update locking")
+def test_concurrent_publish_only_one_revision_wins():
+    page = EditorialPage.objects.create(slug="home", page_kind="sections")
+    user = actor("chief", superuser=True)
+    drafts = [save_page_draft(user, page, page_fields(title), home_sections()) for title in ("First", "Second")]
+    barrier = Barrier(3)
+
+    def attempt(revision_id):
+        close_old_connections()
+        barrier.wait()
+        try:
+            publish_page(user, PageRevision.objects.get(pk=revision_id), expected_published_id=None)
+            return "published"
+        except PublishConflict:
+            return "conflict"
+        finally:
+            connections.close_all()
+
+    with ThreadPoolExecutor(max_workers=2) as pool:
+        futures = [pool.submit(attempt, draft.pk) for draft in drafts]
+        barrier.wait()
+        outcomes = [future.result(timeout=15) for future in futures]
+    assert sorted(outcomes) == ["conflict", "published"]
+    page.refresh_from_db()
+    assert page.published_revision_id in {draft.pk for draft in drafts}
+
+
 def test_publish_rejects_incomplete_page_and_leaves_no_public_content():
     page = EditorialPage.objects.create(slug="home", page_kind="sections")
     user = actor("chief", superuser=True)
@@ -181,3 +212,7 @@ def test_revision_sections_and_audit_are_immutable():
     audit.operation = "publish"
     with pytest.raises(ValidationError):
         audit.save()
+    published = publish_page(user, PageRevision.objects.get(pk=draft.pk), expected_published_id=None)
+    published.status = "draft"
+    with pytest.raises(ValidationError):
+        published.save()