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/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/models.py b/src/backend/core/models.py index 91bcae63..132f322a 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -4,13 +4,19 @@ 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 +26,22 @@ 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, + 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 +54,34 @@ 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 and enforce slug immutability. + + - ``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._state.adding: + 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_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") diff --git a/src/backend/core/tests/test_models_services.py b/src/backend/core/tests/test_models_services.py index f7bf6e3c..5821300b 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,18 @@ 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_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 +40,11 @@ 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() 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"