diff --git a/apps/content/models.py b/apps/content/models.py index 81da94f..f9e1d82 100644 --- a/apps/content/models.py +++ b/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) diff --git a/apps/content/services.py b/apps/content/services.py index 9665c28..29fb821 100644 --- a/apps/content/services.py +++ b/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) diff --git a/apps/content/tests/integration/test_services.py b/apps/content/tests/integration/test_services.py index a862dd7..06bfa02 100644 --- a/apps/content/tests/integration/test_services.py +++ b/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()