From 0ef2b979165049d9665574f782877df2db9302d4 Mon Sep 17 00:00:00 2001 From: Jon Froehlich Date: Tue, 23 Jun 2026 17:24:06 -0700 Subject: [PATCH] Redirect renamed project slugs to current page (#944) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Renaming a project changes its short_name (URL slug), which previously made the old /project// URL hard-404 — breaking external links and search-engine results. This surfaced in the #1142 SEO audit, where /project/mapoutloud/ and /project/mixed-ability-art/ sat in Google's "Not found (404)" bucket. Add a ProjectAlias model (retired slug -> Project). Project.save() captures the old slug as an alias on rename (and clears a reclaimed slug so it can't self-redirect); the project view 301-redirects a retired slug to the current canonical /project//. clean() keeps the live-slug + alias namespace unique so a slug always resolves to one destination. A ProjectAliasInline lets editors see/add aliases, and the idempotent seed_project_aliases command (wired into docker-entrypoint.sh) backfills the historical renames done before auto-capture existed: mapoutloud->geovisally, mixed-ability-art-> artinsight, smarthomedhh->homesound. Tests: website/tests/test_project_aliases.py (13) covering auto-capture, 301 redirect, slug reclaim, namespace uniqueness, visibility, seed idempotency, and 404 on unknown slugs. Co-Authored-By: Claude Opus 4.8 (1M context) --- docker-entrypoint.sh | 5 + website/admin/project_admin.py | 17 +- .../commands/seed_project_aliases.py | 67 ++++++++ website/models/__init__.py | 1 + website/models/project.py | 41 +++++ website/models/project_alias.py | 82 ++++++++++ website/tests/test_project_aliases.py | 145 ++++++++++++++++++ website/views/project.py | 29 +++- 8 files changed, 382 insertions(+), 5 deletions(-) create mode 100644 website/management/commands/seed_project_aliases.py create mode 100644 website/models/project_alias.py create mode 100644 website/tests/test_project_aliases.py diff --git a/docker-entrypoint.sh b/docker-entrypoint.sh index b06254bd..e6aa2d0e 100755 --- a/docker-entrypoint.sh +++ b/docker-entrypoint.sh @@ -143,6 +143,11 @@ echo "4.8 Running 'python manage.py recompute_url_names' to de-collide historica echo "******************************************" python manage.py recompute_url_names +echo "****************** STEP 4.8b/5: docker-entrypoint.sh ************************" +echo "4.8b Running 'python manage.py seed_project_aliases' to redirect renamed project slugs (#944)" +echo "******************************************" +python manage.py seed_project_aliases + echo "****************** STEP 4.9/5: docker-entrypoint.sh ************************" echo "4.9 Running 'python manage.py propagate_publication_projects' to link talks/videos/posters to their publication's projects (#649)" echo "******************************************" diff --git a/website/admin/project_admin.py b/website/admin/project_admin.py index 3777f39f..79e7a9bd 100644 --- a/website/admin/project_admin.py +++ b/website/admin/project_admin.py @@ -1,6 +1,6 @@ from django.contrib import admin from django.contrib.admin import widgets -from website.models import (Project, Banner, Photo, ProjectRole, Grant, +from website.models import (Project, ProjectAlias, Banner, Photo, ProjectRole, Grant, Publication, Talk, Video, Person) from website.models.project import PROJECT_THUMBNAIL_SIZE from website.admin_list_filters import ActiveProjectsFilter @@ -65,12 +65,25 @@ class GrantInline(admin.TabularInline): model = Grant.projects.through extra = 1 +class ProjectAliasInline(admin.TabularInline): + """Former URL slugs that 301-redirect to this project (#944). + + Rows appear automatically when a project's short_name is changed (see + Project.save()); editors can also add historical aliases by hand here so old + links keep resolving.""" + model = ProjectAlias + extra = 0 + fields = ['slug', 'created'] + readonly_fields = ['created'] + verbose_name = "Former slug (redirect)" + verbose_name_plural = "Former slugs (redirect to this project)" + @admin.register(Project, site=ml_admin_site) class ProjectAdmin(ImageCroppingMixin, admin.ModelAdmin): # Search by name plus the research-area facets editors think in (umbrella, keyword). search_fields = ['name', 'short_name', 'project_umbrellas__name', 'keywords__name'] ordering = ('name',) # deterministic alphabetical sort (matched the autocomplete already) - inlines = [GrantInline, BannerInline, PhotoInline, ProjectRoleInline] + inlines = [GrantInline, BannerInline, PhotoInline, ProjectRoleInline, ProjectAliasInline] # The list display lets us control what is shown in the Project table at Home > Website > Project # info on displaying multiple entries comes from http://stackoverflow.com/questions/9164610/custom-columns-using-django-admin diff --git a/website/management/commands/seed_project_aliases.py b/website/management/commands/seed_project_aliases.py new file mode 100644 index 00000000..b436a587 --- /dev/null +++ b/website/management/commands/seed_project_aliases.py @@ -0,0 +1,67 @@ +import logging +from django.core.management.base import BaseCommand +from website.models import Project, ProjectAlias + +_logger = logging.getLogger(__name__) + +# Historical project renames that predate the auto-capture in Project.save() +# (#944). Each entry maps a *former* slug to the project's *current* slug. Going +# forward, renames record their own aliases automatically; this list only +# backfills the ones that already happened. +# +# Keep keys/values lowercase (slugs are matched case-insensitively). The command +# is idempotent and safe to run on every deploy: it skips any entry whose target +# project doesn't exist or whose old slug is already a live project. +# +# Current slugs confirmed against prod 2026-06-23. +# +# smarthomedhh -> homesound was renamed in the admin *before* the auto-capture in +# Project.save() shipped, so no alias was recorded and /project/smarthomedhh/ +# 404s. It's backfilled here. (Renames done after this feature deploys capture +# their own aliases automatically and don't need an entry.) +HISTORICAL_ALIASES = { + 'mapoutloud': 'geovisally', + 'mixed-ability-art': 'artinsight', + 'smarthomedhh': 'homesound', +} + + +class Command(BaseCommand): + help = ("Idempotently backfills ProjectAlias rows for known historical project " + "renames so their old /project// URLs 301-redirect (#944).") + + def handle(self, *args, **options): + created, skipped = 0, 0 + for old_slug, current_slug in HISTORICAL_ALIASES.items(): + old_slug = old_slug.strip().lower() + current_slug = current_slug.strip().lower() + + # Don't shadow a live project: if the old slug now resolves to a real + # project, an alias for it would be dead (the live project always wins). + if Project.objects.filter(short_name__iexact=old_slug).exists(): + _logger.info(f"seed_project_aliases: '{old_slug}' is a live slug; skipping.") + skipped += 1 + continue + + project = Project.objects.filter(short_name__iexact=current_slug).first() + if project is None: + _logger.warning( + f"seed_project_aliases: target project '{current_slug}' not found; " + f"skipping alias '{old_slug}'.") + skipped += 1 + continue + + alias, was_created = ProjectAlias.objects.get_or_create( + slug=old_slug, defaults={'project': project}) + if was_created: + _logger.info(f"seed_project_aliases: created alias {old_slug} → {current_slug}") + created += 1 + else: + # Repoint if an existing alias drifted to the wrong project. + if alias.project_id != project.id: + alias.project = project + alias.save() + _logger.info(f"seed_project_aliases: repointed alias {old_slug} → {current_slug}") + skipped += 1 + + self.stdout.write(f"seed_project_aliases: {created} created, {skipped} skipped.") diff --git a/website/models/__init__.py b/website/models/__init__.py index 8f892126..7a1b16a2 100644 --- a/website/models/__init__.py +++ b/website/models/__init__.py @@ -7,6 +7,7 @@ from .position import Position from .poster import Poster from .project import Project +from .project_alias import ProjectAlias from .project_role import ProjectRole from .project_umbrella import ProjectUmbrella from .publication import Publication diff --git a/website/models/project.py b/website/models/project.py index e9505f1c..4a25afde 100644 --- a/website/models/project.py +++ b/website/models/project.py @@ -143,6 +143,23 @@ def clean(self): ) }) + # The slug namespace also includes *retired* slugs (ProjectAlias), each + # of which 301-redirects to its project (#944). Reject a short_name that + # collides with another project's alias, or the alias's redirect and this + # new project would fight over the same URL. Reclaiming this project's own + # former slug is fine — save() clears that alias. + from website.models.project_alias import ProjectAlias + alias_clash = (ProjectAlias.objects + .filter(slug__iexact=self.short_name) + .exclude(project=self if self.pk else None)) + if alias_clash.exists(): + raise ValidationError({ + 'short_name': ( + f'The slug "{self.short_name}" is a former slug of another project ' + f'and still redirects there. Please choose a different short name.' + ) + }) + def save(self, *args, **kwargs): """ This method overrides the default save method for the Project model. @@ -154,6 +171,16 @@ def save(self, *args, **kwargs): """ _logger.debug("Running Project.save() method...") + # Capture the slug as it currently stands in the DB *before* saving, so we + # can detect a rename below and record the old slug as a redirecting alias + # (#944). None for a brand-new project (no row yet). + old_short_name = None + if self.pk is not None: + old_short_name = (Project.objects + .filter(pk=self.pk) + .values_list('short_name', flat=True) + .first()) + # New projects are private by default (issue #1300). We set this at the # model layer (rather than via a field default) so it applies to every # creation path — admin, shell, seeds, tests — while leaving pre-existing @@ -164,6 +191,20 @@ def save(self, *args, **kwargs): super(Project, self).save(*args, **kwargs) # Save the Project instance first + # Auto-capture renames as redirecting aliases (#944). When short_name + # changes, the previous slug becomes a ProjectAlias pointing at this + # project so its old URL 301-redirects instead of 404ing. + if old_short_name and self.short_name and old_short_name.lower() != self.short_name.lower(): + from website.models.project_alias import ProjectAlias + # This project just vacated old_short_name, so it's the rightful owner + # of that alias (update_or_create repoints any stale alias to us). + ProjectAlias.objects.update_or_create( + slug=old_short_name.lower(), defaults={'project': self}) + # If this project reclaimed a slug that was previously an alias, drop + # that alias so it can't self-redirect (slug now resolves to a live + # project). + ProjectAlias.objects.filter(slug__iexact=self.short_name).delete() + if self.end_date: # Get ProjectRoles related to the Project that have a null end_date project_roles_to_close = ProjectRole.objects.filter(project=self, end_date__isnull=True) diff --git a/website/models/project_alias.py b/website/models/project_alias.py new file mode 100644 index 00000000..7004759a --- /dev/null +++ b/website/models/project_alias.py @@ -0,0 +1,82 @@ +from django.db import models +from django.core.exceptions import ValidationError + +from .project import Project + +import logging + +_logger = logging.getLogger(__name__) + + +class ProjectAlias(models.Model): + """A retired URL slug that should redirect to a project's current page. + + Projects occasionally get renamed to a *completely different* name (e.g. + MapOutLoud -> GeoVisA11y), which also changes ``Project.short_name`` (the URL + slug). Without a record of the old slug, the previous URL hard-404s and any + external links / search-engine results to it break (#944, surfaced in the + #1142 SEO audit). + + Each ``ProjectAlias`` maps one old slug -> one project. The project view + (``website/views/project.py``) consults this table when a slug doesn't match + a live project and issues a permanent (301) redirect to the current slug. + Rows are created automatically by ``Project.save()`` when a slug changes, and + can also be added by hand via the admin inline or seeded by the + ``seed_project_aliases`` management command for historical renames. + + Uniqueness: the slug namespace spans *both* live ``Project.short_name`` values + and these aliases. ``clean()`` enforces that a slug collides with neither, so + a slug always resolves to exactly one destination (no ambiguous redirects). + The live project always wins resolution, so an alias equal to a live slug + would be dead — hence it's rejected. + """ + + slug = models.CharField(max_length=255, db_index=True) + slug.help_text = ("A former URL slug for this project (lowercase, no spaces). Visiting " + "/project// will 301-redirect to the project's current page.") + + project = models.ForeignKey(Project, related_name='aliases', on_delete=models.CASCADE) + + created = models.DateField(auto_now_add=True) + + class Meta: + verbose_name = "Project slug alias" + verbose_name_plural = "Project slug aliases" + + def __str__(self): + return f"{self.slug} → {self.project.short_name}" + + def save(self, *args, **kwargs): + # Normalize so lookups (always done via __iexact, but stored lowercase for + # consistency) and the redirect target are predictable. + if self.slug: + self.slug = self.slug.strip().lower() + super().save(*args, **kwargs) + + def clean(self): + """Keep the combined (live slug + alias) namespace unique, case-insensitively. + + Rejects a slug that (a) matches any live ``Project.short_name`` — the live + project would always win resolution, making the alias dead — or (b) matches + another ``ProjectAlias`` for a different destination. + """ + super().clean() + if not self.slug: + raise ValidationError({'slug': 'A slug is required.'}) + + slug = self.slug.strip().lower() + + if Project.objects.filter(short_name__iexact=slug).exists(): + raise ValidationError({ + 'slug': (f'"{slug}" is already a live project slug, so an alias for it would ' + f'never be used. Aliases are only for *former* slugs.') + }) + + clash = ProjectAlias.objects.filter(slug__iexact=slug) + if self.pk: + clash = clash.exclude(pk=self.pk) + clash = clash.exclude(project=self.project) + if clash.exists(): + raise ValidationError({ + 'slug': f'The alias "{slug}" already points to a different project.' + }) diff --git a/website/tests/test_project_aliases.py b/website/tests/test_project_aliases.py new file mode 100644 index 00000000..4fad21d3 --- /dev/null +++ b/website/tests/test_project_aliases.py @@ -0,0 +1,145 @@ +"""Tests for project slug renames → 301 redirects via ProjectAlias (#944). + +Covers the four moving parts: + - Project.save() auto-captures the old slug as a ProjectAlias on rename. + - The project view 301-redirects a retired slug to the project's current URL. + - Reclaiming a slug clears its alias (no self-redirect loop). + - clean() keeps the live-slug + alias namespace unique. + - The seed_project_aliases backfill command is idempotent. +""" + +from django.core.exceptions import ValidationError +from django.core.management import call_command +from django.urls import reverse + +from website.models import Project, ProjectAlias +from website.tests.base import DatabaseTestCase + + +class ProjectRenameAutoCaptureTests(DatabaseTestCase): + """Project.save() records the previous slug as a redirecting alias (#944).""" + + def test_rename_creates_alias_for_old_slug(self): + project = self.make_project(name="Map Out Loud", short_name="mapoutloud") + project.short_name = "geovisally" + project.save() + + aliases = ProjectAlias.objects.filter(project=project).values_list('slug', flat=True) + self.assertIn("mapoutloud", aliases) + # The current slug must NOT be an alias of itself. + self.assertNotIn("geovisally", aliases) + + def test_rename_is_case_insensitive_noop(self): + """Changing only the case of the slug is not a rename — no alias created.""" + project = self.make_project(name="Sidewalk", short_name="projectsidewalk") + project.short_name = "ProjectSidewalk" + project.save() + self.assertEqual(ProjectAlias.objects.filter(project=project).count(), 0) + + def test_creating_a_project_makes_no_alias(self): + project = self.make_project(name="Fresh", short_name="fresh") + self.assertEqual(ProjectAlias.objects.filter(project=project).count(), 0) + + +class ProjectAliasRedirectViewTests(DatabaseTestCase): + """The project view 301-redirects retired slugs to the current page.""" + + def test_old_slug_301_redirects_to_current(self): + project = self.make_project(name="Map Out Loud", short_name="mapoutloud", + is_visible=True) + project.short_name = "geovisally" + project.save() + + resp = self.client.get("/project/mapoutloud/") + self.assertRedirects(resp, "/project/geovisally/", + status_code=301, target_status_code=200) + + def test_unknown_slug_still_404s(self): + resp = self.client.get("/project/doesnotexistanywhere/") + self.assertEqual(resp.status_code, 404) + + def test_live_slug_wins_over_alias(self): + """A reclaimed slug serves the live project (200), not a redirect.""" + # A: foo -> bar, leaving alias 'foo' -> A + a = self.make_project(name="Alpha", short_name="foo", is_visible=True) + a.short_name = "bar" + a.save() + # B reclaims 'foo' + b = self.make_project(name="Beta", short_name="baz", is_visible=True) + b.short_name = "foo" + b.save() + + self.assertFalse(ProjectAlias.objects.filter(slug="foo").exists()) + resp = self.client.get("/project/foo/") + self.assertEqual(resp.status_code, 200) # serves B, no redirect + # B's previous slug now redirects to foo + resp_baz = self.client.get("/project/baz/") + self.assertRedirects(resp_baz, "/project/foo/", + status_code=301, target_status_code=200) + + def test_alias_to_private_project_redirects_then_404s_for_anon(self): + """Resolution still honors visibility: the redirect lands, but a private + target 404s for anonymous visitors (#1300).""" + project = self.make_project(name="Secret", short_name="secretnow", is_visible=False) + ProjectAlias.objects.create(slug="secretold", project=project) + + resp = self.client.get("/project/secretold/") + self.assertEqual(resp.status_code, 301) + self.assertEqual(resp["Location"], "/project/secretnow/") + self.assertEqual(self.client.get("/project/secretnow/").status_code, 404) + + +class SlugNamespaceUniquenessTests(DatabaseTestCase): + """clean() keeps live slugs and aliases from colliding (#944).""" + + def test_project_short_name_cannot_collide_with_other_projects_alias(self): + a = self.make_project(name="Alpha", short_name="foo") + a.short_name = "bar" + a.save() # alias 'foo' -> A now exists + + dupe = Project(name="Gamma", short_name="foo") + with self.assertRaises(ValidationError) as ctx: + dupe.full_clean() + self.assertIn("short_name", ctx.exception.message_dict) + + def test_project_may_reclaim_its_own_former_slug(self): + a = self.make_project(name="Alpha", short_name="foo") + a.short_name = "bar" + a.save() # alias 'foo' -> A + a.short_name = "foo" + a.full_clean() # reclaiming our own former slug must not raise + + def test_alias_cannot_equal_a_live_slug(self): + live = self.make_project(name="Live", short_name="liveslug") + other = self.make_project(name="Other", short_name="otherslug") + bad = ProjectAlias(slug="liveslug", project=other) + with self.assertRaises(ValidationError) as ctx: + bad.full_clean() + self.assertIn("slug", ctx.exception.message_dict) + + def test_alias_cannot_duplicate_another_projects_alias(self): + a = self.make_project(name="Alpha", short_name="alpha") + b = self.make_project(name="Beta", short_name="beta") + ProjectAlias.objects.create(slug="shared", project=a) + dupe = ProjectAlias(slug="shared", project=b) + with self.assertRaises(ValidationError) as ctx: + dupe.full_clean() + self.assertIn("slug", ctx.exception.message_dict) + + +class SeedProjectAliasesCommandTests(DatabaseTestCase): + """seed_project_aliases backfills historical renames idempotently (#944).""" + + def test_seed_creates_known_alias_and_is_idempotent(self): + # Matches the confirmed HISTORICAL_ALIASES entry mapoutloud -> geovisally. + self.make_project(name="GeoVisA11y", short_name="geovisally") + + call_command("seed_project_aliases") + call_command("seed_project_aliases") # second run must not duplicate + + self.assertEqual(ProjectAlias.objects.filter(slug="mapoutloud").count(), 1) + + def test_seed_skips_when_target_project_missing(self): + # No 'geovisally' project exists → nothing to point at, so no alias. + call_command("seed_project_aliases") + self.assertFalse(ProjectAlias.objects.filter(slug="mapoutloud").exists()) diff --git a/website/views/project.py b/website/views/project.py index 444639b0..d1e66b34 100644 --- a/website/views/project.py +++ b/website/views/project.py @@ -1,11 +1,11 @@ from django.conf import settings # for access to settings variables, see https://docs.djangoproject.com/en/4.0/topics/settings/#using-settings-in-python-code -from website.models import Project, Position, ProjectRole, Grant +from website.models import Project, Position, ProjectRole, Grant, ProjectAlias from website.models.project_role import LeadProjectRoleTypes from website.models.position import MemberClassification import website.utils.ml_utils as ml_utils from website.utils.metadata import meta_description from django.urls import reverse -from django.shortcuts import render, get_object_or_404, redirect +from django.shortcuts import render, redirect from operator import attrgetter from django.template.loader import render_to_string from django.http import HttpResponse, Http404 @@ -34,7 +34,30 @@ def project(request, project_name): func_start_time = time.perf_counter() _logger.debug(f"Starting views/project {project_name} at {func_start_time:0.4f}") - project = get_object_or_404(Project, short_name__iexact=project_name) + # Resolve the slug to a live project. On a miss, fall back to the retired-slug + # table (ProjectAlias) and 301-redirect renamed projects to their current URL + # (#944) rather than 404ing. A live project always wins over an alias. + try: + project = Project.objects.get(short_name__iexact=project_name) + except Project.DoesNotExist: + alias = (ProjectAlias.objects + .filter(slug__iexact=project_name) + .select_related('project') + .first()) + if alias: + # Permanent redirect so search engines consolidate the old URL onto the + # canonical singular /project// (reverse resolves to it). + return redirect(reverse('website:project', args=[alias.project.short_name]), + permanent=True) + raise Http404("Project not found") + except Project.MultipleObjectsReturned: + # short_name uniqueness is enforced case-insensitively in Project.clean() + # (#1156), so this should be unreachable. Fail with a clean 404 rather than + # a 500 if duplicate slugs ever recur; the fix is to de-dupe the data. + _logger.error( + f"Multiple projects share short_name={project_name!r} (case-insensitively) — " + f"slug uniqueness is broken. Returning 404.") + raise Http404("No unique project matches the given query.") # Private projects (is_visible False/None) are hidden from the public but # remain previewable by logged-in staff so they can build a project before