From b596cf7abd3bdb6ee90a06cec1c6318a3a6d713a Mon Sep 17 00:00:00 2001 From: Stephan Meijer Date: Mon, 1 Jun 2026 15:28:48 +0200 Subject: [PATCH 1/6] =?UTF-8?q?=E2=99=BB=EF=B8=8F(backend)=20introduce=20S?= =?UTF-8?q?ervice.slug=20as=20immutable=20index=20identifier?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stephan Meijer --- src/backend/core/factories.py | 3 +- .../0003_service_slug_and_editable_name.py | 73 +++++++++++++++++++ src/backend/core/models.py | 50 +++++++++++-- .../core/tests/test_models_services.py | 52 +++++++++++-- src/backend/demo/defaults.py | 9 ++- .../demo/tests/test_commands_create_demo.py | 6 +- 6 files changed, 172 insertions(+), 21 deletions(-) create mode 100644 src/backend/core/migrations/0003_service_slug_and_editable_name.py diff --git a/src/backend/core/factories.py b/src/backend/core/factories.py index ddb13636..10470f68 100644 --- a/src/backend/core/factories.py +++ b/src/backend/core/factories.py @@ -50,7 +50,8 @@ class ServiceFactory(factory.django.DjangoModelFactory): A factory for generating service instances for testing and development purposes. """ - name = factory.Sequence(lambda n: f"test-index-{n!s}") + slug = factory.Sequence(lambda n: f"testidx{n!s}") + name = factory.LazyAttribute(lambda o: f"Test Service {o.slug}") created_at = factory.Faker("date_time_this_year", tzinfo=None) is_active = True client_id = "some_client_id" diff --git a/src/backend/core/migrations/0003_service_slug_and_editable_name.py b/src/backend/core/migrations/0003_service_slug_and_editable_name.py new file mode 100644 index 00000000..05026a00 --- /dev/null +++ b/src/backend/core/migrations/0003_service_slug_and_editable_name.py @@ -0,0 +1,73 @@ +import re + +import django.core.validators +from django.db import migrations, models + + +def backfill_slug_from_name(apps, schema_editor): + Service = apps.get_model("core", "Service") # noqa: N806 + seen = set() + for service in Service.objects.all(): + derived = re.sub(r"[^a-zA-Z0-9]", "", service.name or "").lower() + if not derived: + raise RuntimeError( + f"Cannot derive slug for Service id={service.pk!r} " + f"name={service.name!r}: name contains no alphanumeric characters." + ) + if derived in seen: + raise RuntimeError( + f"Slug collision while backfilling: name={service.name!r} -> " + f"slug={derived!r} already used by another service." + ) + seen.add(derived) + service.slug = derived + service.save(update_fields=["slug"]) + + +class Migration(migrations.Migration): + dependencies = [ + ("core", "0002_service_client_id_service_services"), + ] + + operations = [ + migrations.AddField( + model_name="service", + name="slug", + field=models.CharField(max_length=20, null=True), + ), + migrations.RunPython( + backfill_slug_from_name, + reverse_code=migrations.RunPython.noop, + ), + migrations.AlterField( + model_name="service", + name="slug", + field=models.SlugField( + editable=False, + help_text=( + "Stable identifier used in the OpenSearch index name. " + "Lowercase alphanumeric only. Set on creation, immutable thereafter." + ), + max_length=20, + unique=True, + validators=[ + django.core.validators.RegexValidator( + message="Slug must contain only lowercase letters and digits.", + regex="^[a-z0-9]+$", + ) + ], + ), + ), + migrations.AlterField( + model_name="service", + name="name", + field=models.CharField(max_length=255), + ), + migrations.AddConstraint( + model_name="service", + constraint=models.CheckConstraint( + condition=models.Q(("slug__regex", "^[a-z0-9]+$")), + name="slug_alphanumeric_only", + ), + ), + ] diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 91bcae63..7f797391 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -1,16 +1,23 @@ """Models for find's core app""" +import re import secrets import string from django.contrib.auth.models import AbstractUser +from django.core.exceptions import ValidationError +from django.core.validators import RegexValidator from django.db import models from django.db.models.functions import Length -from django.utils.text import slugify from django.utils.translation import gettext_lazy as _ models.CharField.register_lookup(Length) TOKEN_LENGTH = 50 +SLUG_REGEX = r"^[a-z0-9]+$" +SLUG_VALIDATOR = RegexValidator( + regex=SLUG_REGEX, + message=_("Slug must contain only lowercase letters and digits."), +) class User(AbstractUser): @@ -20,15 +27,23 @@ class User(AbstractUser): class Service(models.Model): """Service registered to index its documents to our find""" - name = models.SlugField(max_length=20, unique=True) + name = models.CharField(max_length=255) + slug = models.SlugField( + max_length=20, + unique=True, + editable=False, + validators=[SLUG_VALIDATOR], + help_text=_( + "Stable identifier used in the OpenSearch index name. " + "Lowercase alphanumeric only. Set on creation, immutable thereafter." + ), + ) token = models.CharField(max_length=TOKEN_LENGTH) created_at = models.DateTimeField(auto_now_add=True) is_active = models.BooleanField(default=True) client_id = models.CharField(blank=True, null=True) services = models.ManyToManyField( - "self", - verbose_name=_("Allowed services for search"), - blank=True, + "self", blank=True, verbose_name=_("Allowed services for search") ) class Meta: @@ -41,14 +56,35 @@ class Meta: condition=models.Q(token__length=TOKEN_LENGTH), name="token_length_exact_50", ), + models.CheckConstraint( + condition=models.Q(slug__regex=SLUG_REGEX), + name="slug_alphanumeric_only", + ), ] def __str__(self): return self.name def save(self, *args, **kwargs): - """Automatically slugify the service name and generate a token on creation""" - self.name = slugify(self.name) + """Generate token, auto-derive slug on creation, enforce slug immutability. + + - ``slug`` is auto-derived from ``name`` if not provided (strip + non-alphanumeric, lowercase). It is immutable after creation. + - ``name`` is a free-form display field and can be edited. + - ``token`` is generated once on creation if missing. + """ + if not self.slug: + self.slug = re.sub(r"[^a-zA-Z0-9]", "", self.name or "").lower() + if self.pk is not None: + stored_slug = ( + Service.objects.filter(pk=self.pk) + .values_list("slug", flat=True) + .first() + ) + if self.slug != stored_slug: + raise ValidationError( + {"slug": _("Service.slug is immutable after creation.")} + ) if not self.token: self.token = self.generate_secure_token() super().save(*args, **kwargs) diff --git a/src/backend/core/tests/test_models_services.py b/src/backend/core/tests/test_models_services.py index f7bf6e3c..20c25c40 100644 --- a/src/backend/core/tests/test_models_services.py +++ b/src/backend/core/tests/test_models_services.py @@ -1,5 +1,6 @@ """Tests Service model for find's core app.""" +from django.core.exceptions import ValidationError from django.db import DataError, IntegrityError import pytest @@ -9,18 +10,37 @@ pytestmark = pytest.mark.django_db -def test_models_services_name_unique(): - """The name field should be unique across services.""" +def test_models_services_slug_unique(): + """The slug field must be unique across services.""" service = factories.ServiceFactory() with pytest.raises(IntegrityError): - factories.ServiceFactory(name=service.name) + factories.ServiceFactory(slug=service.slug) -def test_models_services_name_slugified(): - """The name field should be slugified.""" - service = factories.ServiceFactory(name="My service name") - assert service.name == "my-service-name" +def test_models_services_name_not_required_unique(): + """Two services may share the same display name.""" + factories.ServiceFactory(slug="aa", name="Same Name") + factories.ServiceFactory(slug="bb", name="Same Name") + + +def test_models_services_slug_auto_derived_from_name(): + """When no slug is provided, it is auto-derived from name (alphanumeric, lowercase).""" + service = factories.ServiceFactory(slug=None, name="My Service Name") + assert service.slug == "myservicename" + assert service.name == "My Service Name" + + +def test_models_services_slug_auto_derivation_strips_special_chars(): + """Slug derivation strips hyphens, underscores, spaces and punctuation.""" + service = factories.ServiceFactory(slug=None, name="docs-service_v2!") + assert service.slug == "docsservicev2" + + +def test_models_services_slug_rejects_non_alphanumeric(): + """Explicit non-alphanumeric slugs are rejected by the DB check constraint.""" + with pytest.raises(IntegrityError): + factories.ServiceFactory(slug="has-hyphen") def test_models_services_token_50_characters_exact(): @@ -39,3 +59,21 @@ def test_models_services_token_50_characters_more(): """The token field should be 50 characters long.""" with pytest.raises(DataError): factories.ServiceFactory(token="a" * 51) + + +def test_service_slug_immutable_after_creation(): + """The slug field must be immutable after creation.""" + service = factories.ServiceFactory(slug="originalname") + service.slug = "differentname" + with pytest.raises(ValidationError): + service.save() + + +def test_service_name_editable_after_creation(): + """The name field is freely editable after creation.""" + service = factories.ServiceFactory(slug="myslug", name="Original") + service.name = "New Display Name" + service.save() + service.refresh_from_db() + assert service.name == "New Display Name" + assert service.slug == "myslug" diff --git a/src/backend/demo/defaults.py b/src/backend/demo/defaults.py index d0115bf3..92721614 100644 --- a/src/backend/demo/defaults.py +++ b/src/backend/demo/defaults.py @@ -4,17 +4,20 @@ DEV_SERVICES = ( { - "name": "docs", + "slug": "docs", + "name": "Docs", "client_id": "impress", "token": "find-api-key-for-docs-with-exactly-50-chars-length", }, { - "name": "drive", + "slug": "drive", + "name": "Drive", "client_id": "drive", "token": "find-api-key-for-driv-with-exactly-50-chars-length", }, { - "name": "conversations", + "slug": "conversations", + "name": "Conversations", "client_id": "conversations", "token": "find-api-key-for-conv-with-exactly-50-chars-length", }, diff --git a/src/backend/demo/tests/test_commands_create_demo.py b/src/backend/demo/tests/test_commands_create_demo.py index bd1169d6..dc24b787 100644 --- a/src/backend/demo/tests/test_commands_create_demo.py +++ b/src/backend/demo/tests/test_commands_create_demo.py @@ -26,11 +26,11 @@ def test_commands_create_demo(settings): """The create_demo management command should create objects as expected.""" call_command("create_demo") - assert models.Service.objects.exclude(name="docs").count() == 4 + assert models.Service.objects.exclude(slug="docs").count() == 4 assert opensearch_client().count(index=settings.OPENSEARCH_INDEX)["count"] == 4 - docs = models.Service.objects.get(name="docs") + docs = models.Service.objects.get(slug="docs") assert docs.client_id == "impress" - drive = models.Service.objects.get(name="drive") + drive = models.Service.objects.get(slug="drive") assert drive.client_id == "drive" From 6b3bd0c9cabad56601c436862081e81f5cdc7ba5 Mon Sep 17 00:00:00 2001 From: Stephan Meijer Date: Mon, 1 Jun 2026 16:05:59 +0200 Subject: [PATCH 2/6] =?UTF-8?q?fixup!=20=E2=99=BB=EF=B8=8F(backend)=20intr?= =?UTF-8?q?oduce=20Service.slug=20as=20immutable=20index=20identifier?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stephan Meijer --- src/backend/core/models.py | 12 +++++------- src/backend/core/tests/test_models_services.py | 13 ------------- 2 files changed, 5 insertions(+), 20 deletions(-) diff --git a/src/backend/core/models.py b/src/backend/core/models.py index 7f797391..da2b0178 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -1,6 +1,5 @@ """Models for find's core app""" -import re import secrets import string @@ -66,16 +65,15 @@ def __str__(self): return self.name def save(self, *args, **kwargs): - """Generate token, auto-derive slug on creation, enforce slug immutability. + """Generate token and enforce slug immutability. - - ``slug`` is auto-derived from ``name`` if not provided (strip - non-alphanumeric, lowercase). It is immutable after creation. + - ``slug`` must be provided explicitly on creation and is immutable + thereafter. The DB check constraint enforces the allowed character + set; no auto-derivation is performed. - ``name`` is a free-form display field and can be edited. - ``token`` is generated once on creation if missing. """ - if not self.slug: - self.slug = re.sub(r"[^a-zA-Z0-9]", "", self.name or "").lower() - if self.pk is not None: + if not self._state.adding: stored_slug = ( Service.objects.filter(pk=self.pk) .values_list("slug", flat=True) diff --git a/src/backend/core/tests/test_models_services.py b/src/backend/core/tests/test_models_services.py index 20c25c40..de18bf93 100644 --- a/src/backend/core/tests/test_models_services.py +++ b/src/backend/core/tests/test_models_services.py @@ -24,19 +24,6 @@ def test_models_services_name_not_required_unique(): factories.ServiceFactory(slug="bb", name="Same Name") -def test_models_services_slug_auto_derived_from_name(): - """When no slug is provided, it is auto-derived from name (alphanumeric, lowercase).""" - service = factories.ServiceFactory(slug=None, name="My Service Name") - assert service.slug == "myservicename" - assert service.name == "My Service Name" - - -def test_models_services_slug_auto_derivation_strips_special_chars(): - """Slug derivation strips hyphens, underscores, spaces and punctuation.""" - service = factories.ServiceFactory(slug=None, name="docs-service_v2!") - assert service.slug == "docsservicev2" - - def test_models_services_slug_rejects_non_alphanumeric(): """Explicit non-alphanumeric slugs are rejected by the DB check constraint.""" with pytest.raises(IntegrityError): From 5b2a938d5045816f45d58556ea3f4e8d4947b037 Mon Sep 17 00:00:00 2001 From: Stephan Meijer Date: Mon, 8 Jun 2026 09:49:39 +0200 Subject: [PATCH 3/6] =?UTF-8?q?fixup!=20=E2=99=BB=EF=B8=8F(backend)=20intr?= =?UTF-8?q?oduce=20Service.slug=20as=20immutable=20index=20identifier?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stephan Meijer --- .../0003_service_slug_and_editable_name.py | 37 ++++++------------- 1 file changed, 11 insertions(+), 26 deletions(-) diff --git a/src/backend/core/migrations/0003_service_slug_and_editable_name.py b/src/backend/core/migrations/0003_service_slug_and_editable_name.py index 05026a00..6feffcd8 100644 --- a/src/backend/core/migrations/0003_service_slug_and_editable_name.py +++ b/src/backend/core/migrations/0003_service_slug_and_editable_name.py @@ -1,27 +1,17 @@ -import re - import django.core.validators from django.db import migrations, models -def backfill_slug_from_name(apps, schema_editor): +def wipe_services(apps, schema_editor): + """Drop pre-existing services before introducing the required ``slug`` column. + + No production data exists, so there is no value-preserving backfill to do. + Wiping ``Service`` rows up front lets ``AddField`` introduce ``slug`` with + its final ``NOT NULL UNIQUE`` shape in a single operation. Going through + the ORM cascades to the ``services`` self-referential M2M join table. + """ Service = apps.get_model("core", "Service") # noqa: N806 - seen = set() - for service in Service.objects.all(): - derived = re.sub(r"[^a-zA-Z0-9]", "", service.name or "").lower() - if not derived: - raise RuntimeError( - f"Cannot derive slug for Service id={service.pk!r} " - f"name={service.name!r}: name contains no alphanumeric characters." - ) - if derived in seen: - raise RuntimeError( - f"Slug collision while backfilling: name={service.name!r} -> " - f"slug={derived!r} already used by another service." - ) - seen.add(derived) - service.slug = derived - service.save(update_fields=["slug"]) + Service.objects.all().delete() class Migration(migrations.Migration): @@ -30,16 +20,11 @@ class Migration(migrations.Migration): ] operations = [ - migrations.AddField( - model_name="service", - name="slug", - field=models.CharField(max_length=20, null=True), - ), migrations.RunPython( - backfill_slug_from_name, + wipe_services, reverse_code=migrations.RunPython.noop, ), - migrations.AlterField( + migrations.AddField( model_name="service", name="slug", field=models.SlugField( From 2b711f78ac2fbda56b39a2348cd50e2b0c0407a3 Mon Sep 17 00:00:00 2001 From: Stephan Meijer Date: Mon, 8 Jun 2026 09:50:33 +0200 Subject: [PATCH 4/6] =?UTF-8?q?fixup!=20=E2=99=BB=EF=B8=8F(backend)=20intr?= =?UTF-8?q?oduce=20Service.slug=20as=20immutable=20index=20identifier?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stephan Meijer --- src/backend/core/tests/test_models_services.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/backend/core/tests/test_models_services.py b/src/backend/core/tests/test_models_services.py index de18bf93..276fd12a 100644 --- a/src/backend/core/tests/test_models_services.py +++ b/src/backend/core/tests/test_models_services.py @@ -18,12 +18,6 @@ def test_models_services_slug_unique(): factories.ServiceFactory(slug=service.slug) -def test_models_services_name_not_required_unique(): - """Two services may share the same display name.""" - factories.ServiceFactory(slug="aa", name="Same Name") - factories.ServiceFactory(slug="bb", name="Same Name") - - def test_models_services_slug_rejects_non_alphanumeric(): """Explicit non-alphanumeric slugs are rejected by the DB check constraint.""" with pytest.raises(IntegrityError): From ac59ee64ce9675f87fd448146c2ce4444a253039 Mon Sep 17 00:00:00 2001 From: Stephan Meijer Date: Mon, 8 Jun 2026 09:50:59 +0200 Subject: [PATCH 5/6] =?UTF-8?q?fixup!=20=E2=99=BB=EF=B8=8F(backend)=20intr?= =?UTF-8?q?oduce=20Service.slug=20as=20immutable=20index=20identifier?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Stephan Meijer --- src/backend/core/tests/test_models_services.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/backend/core/tests/test_models_services.py b/src/backend/core/tests/test_models_services.py index 276fd12a..5821300b 100644 --- a/src/backend/core/tests/test_models_services.py +++ b/src/backend/core/tests/test_models_services.py @@ -48,13 +48,3 @@ def test_service_slug_immutable_after_creation(): service.slug = "differentname" with pytest.raises(ValidationError): service.save() - - -def test_service_name_editable_after_creation(): - """The name field is freely editable after creation.""" - service = factories.ServiceFactory(slug="myslug", name="Original") - service.name = "New Display Name" - service.save() - service.refresh_from_db() - assert service.name == "New Display Name" - assert service.slug == "myslug" From 33af6ed8b5e14ed3aa96793c638881264e4e06c2 Mon Sep 17 00:00:00 2001 From: Stephan Meijer Date: Mon, 8 Jun 2026 15:31:20 +0200 Subject: [PATCH 6/6] =?UTF-8?q?fixup!=20=E2=99=BB=EF=B8=8F(backend)=20intr?= =?UTF-8?q?oduce=20Service.slug=20as=20immutable=20index=20identifier?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/backend/core/admin.py | 15 +++++ .../0003_service_slug_and_editable_name.py | 58 ------------------- src/backend/core/models.py | 1 - src/backend/core/tests/test_admin_selftest.py | 18 ++++++ 4 files changed, 33 insertions(+), 59 deletions(-) delete mode 100644 src/backend/core/migrations/0003_service_slug_and_editable_name.py diff --git a/src/backend/core/admin.py b/src/backend/core/admin.py index 5be02cdd..79309180 100644 --- a/src/backend/core/admin.py +++ b/src/backend/core/admin.py @@ -1,5 +1,6 @@ """Admin config for find's core app""" +from django import forms from django.contrib import admin from django.shortcuts import render from django.urls import path @@ -9,10 +10,24 @@ from .selftests import registry +class ServiceAdminForm(forms.ModelForm): + """Admin form keeping the slug immutable after creation.""" + + class Meta: + model = Service + fields = ("name", "slug", "is_active", "client_id", "services") + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + if self.instance and self.instance.pk: + self.fields["slug"].disabled = True + + @admin.register(Service) class ServiceAdmin(admin.ModelAdmin): """Register the serivce model for the admin site""" + form = ServiceAdminForm list_display = ("name", "created_at", "is_active") search_fields = ("name",) list_filter = ("is_active", "created_at") diff --git a/src/backend/core/migrations/0003_service_slug_and_editable_name.py b/src/backend/core/migrations/0003_service_slug_and_editable_name.py deleted file mode 100644 index 6feffcd8..00000000 --- a/src/backend/core/migrations/0003_service_slug_and_editable_name.py +++ /dev/null @@ -1,58 +0,0 @@ -import django.core.validators -from django.db import migrations, models - - -def wipe_services(apps, schema_editor): - """Drop pre-existing services before introducing the required ``slug`` column. - - No production data exists, so there is no value-preserving backfill to do. - Wiping ``Service`` rows up front lets ``AddField`` introduce ``slug`` with - its final ``NOT NULL UNIQUE`` shape in a single operation. Going through - the ORM cascades to the ``services`` self-referential M2M join table. - """ - Service = apps.get_model("core", "Service") # noqa: N806 - Service.objects.all().delete() - - -class Migration(migrations.Migration): - dependencies = [ - ("core", "0002_service_client_id_service_services"), - ] - - operations = [ - migrations.RunPython( - wipe_services, - reverse_code=migrations.RunPython.noop, - ), - migrations.AddField( - model_name="service", - name="slug", - field=models.SlugField( - editable=False, - help_text=( - "Stable identifier used in the OpenSearch index name. " - "Lowercase alphanumeric only. Set on creation, immutable thereafter." - ), - max_length=20, - unique=True, - validators=[ - django.core.validators.RegexValidator( - message="Slug must contain only lowercase letters and digits.", - regex="^[a-z0-9]+$", - ) - ], - ), - ), - migrations.AlterField( - model_name="service", - name="name", - field=models.CharField(max_length=255), - ), - migrations.AddConstraint( - model_name="service", - constraint=models.CheckConstraint( - condition=models.Q(("slug__regex", "^[a-z0-9]+$")), - name="slug_alphanumeric_only", - ), - ), - ] diff --git a/src/backend/core/models.py b/src/backend/core/models.py index da2b0178..132f322a 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -30,7 +30,6 @@ class Service(models.Model): slug = models.SlugField( max_length=20, unique=True, - editable=False, validators=[SLUG_VALIDATOR], help_text=_( "Stable identifier used in the OpenSearch index name. " diff --git a/src/backend/core/tests/test_admin_selftest.py b/src/backend/core/tests/test_admin_selftest.py index c1deffcb..c16ae567 100644 --- a/src/backend/core/tests/test_admin_selftest.py +++ b/src/backend/core/tests/test_admin_selftest.py @@ -7,6 +7,8 @@ import pytest +from core.admin import ServiceAdminForm +from core.models import Service from core.selftests import SelfTestResult pytestmark = pytest.mark.django_db @@ -27,6 +29,22 @@ def _override_storage_settings(settings): } +def test_service_admin_form_enables_slug_on_creation(): + """The service slug must be explicit when adding a service in admin.""" + form = ServiceAdminForm() + + assert not form.fields["slug"].disabled + + +@patch("django.forms.models.model_to_dict", return_value={}) +def test_service_admin_form_disables_slug_after_creation(_mock_model_to_dict): + """The service slug must be read-only when editing an existing service in admin.""" + service = Service(pk=1, name="Test Service", slug="testidx") + form = ServiceAdminForm(instance=service) + + assert form.fields["slug"].disabled + + def test_selftest_requires_authentication(client): """Test that the selftest page requires authentication.""" selftest_url = reverse("admin:selftest")