From 54aace8abbe93ec0b330a985e5297942fa510a85 Mon Sep 17 00:00:00 2001 From: Bernard Siebens Date: Sun, 12 Jul 2026 21:41:59 +0200 Subject: [PATCH] feat: auto-populate slug fields on save Add clubmanager.base.unique_slugify(instance, value, scope=...): slugify a source value, truncate to the field's max_length, and append -2/-3/... to stay unique within a scope. ClubScopedModel gains a slug_source hook that fills a blank slug (unique per club) on save. Wire it up so every SlugField auto-populates from its natural source when left blank (explicit values are always kept): - shop.Product.slug <- name (per club) - formbuilder.Form.slug <- title (per club) - formbuilder.Field.key <- label (per form) Club.slug already auto-populated; refactor it onto the shared helper. Also fix shop.Product.slug's multi-tenancy bug: it was globally unique (unique=True); make it unique per club like the others. Migrations added. Full suite at 100% coverage. Co-Authored-By: Claude Opus 4.8 --- club/models.py | 15 +------ clubmanager/base.py | 32 +++++++++++++ .../0004_alter_field_key_alter_form_slug.py | 23 ++++++++++ formbuilder/models.py | 13 ++++-- formbuilder/tests.py | 45 +++++++++++++++++++ shop/migrations/0001_initial.py | 29 ++++++++++++ .../0002_alter_product_slug_and_more.py | 23 ++++++++++ shop/models.py | 18 +++++++- shop/tests.py | 40 ++++++++++++++++- 9 files changed, 220 insertions(+), 18 deletions(-) create mode 100644 formbuilder/migrations/0004_alter_field_key_alter_form_slug.py create mode 100644 shop/migrations/0001_initial.py create mode 100644 shop/migrations/0002_alter_product_slug_and_more.py diff --git a/club/models.py b/club/models.py index 8b02009..0a20cb5 100644 --- a/club/models.py +++ b/club/models.py @@ -2,10 +2,9 @@ import datetime from django.db import models from django.utils import timezone -from django.utils.text import slugify from django.utils.translation import gettext_lazy as _ -from clubmanager.base import ClubScopedModel, UUIDModel +from clubmanager.base import ClubScopedModel, UUIDModel, unique_slugify from members.models import Member @@ -33,19 +32,9 @@ class Club(UUIDModel): def save(self, *args, **kwargs): if not self.slug: - self.slug = self._unique_slug() + self.slug = unique_slugify(self, self.name) super().save(*args, **kwargs) - def _unique_slug(self): - base = slugify(self.name) or "club" - slug = base - suffix = 2 - existing = Club.objects.exclude(pk=self.pk) - while existing.filter(slug=slug).exists(): - slug = f"{base}-{suffix}" - suffix += 1 - return slug - class Season(ClubScopedModel): start_date = models.DateField(_("start date")) diff --git a/clubmanager/base.py b/clubmanager/base.py index 3e607e8..a5b2716 100644 --- a/clubmanager/base.py +++ b/clubmanager/base.py @@ -2,6 +2,7 @@ import uuid from typing import TYPE_CHECKING from django.db import models +from django.utils.text import slugify from club.tenancy import require_current_club @@ -9,6 +10,30 @@ if TYPE_CHECKING: from club.models import Club +def unique_slugify(instance, value, *, slug_field="slug", scope=None): + """Return a slug derived from ``value``, unique within ``scope``. + + Truncates to the slug field's ``max_length`` and appends ``-2``, ``-3``, … + on collision. ``scope`` is a dict of field lookups the uniqueness is + checked within (e.g. ``{"club": club}`` for per-club, ``{}``/``None`` for + global). + """ + max_length = instance._meta.get_field(slug_field).max_length + base = slugify(value)[:max_length] or "item" + + queryset = type(instance)._default_manager.exclude(pk=instance.pk) + if scope: + queryset = queryset.filter(**scope) + + slug = base + suffix = 2 + while queryset.filter(**{slug_field: slug}).exists(): + tail = f"-{suffix}" + slug = f"{base[: max_length - len(tail)]}{tail}" + suffix += 1 + return slug + + class TenantQuerySet(models.QuerySet): def for_club(self, club: Club): return self.filter(club=club) @@ -32,6 +57,10 @@ class ClubScopedModel(UUIDModel): club = models.ForeignKey("club.Club", on_delete=models.CASCADE, related_name="%(class)ss") objects = TenantQuerySet.as_manager() + # Subclasses with a ``slug`` field set this to the source field name (e.g. + # "name"/"title") to auto-populate the slug — unique per club — on save. + slug_source = None + class Meta: abstract = True @@ -39,4 +68,7 @@ class ClubScopedModel(UUIDModel): if self.club_id is None: self.club = require_current_club() + if self.slug_source and not self.slug: + self.slug = unique_slugify(self, getattr(self, self.slug_source), scope={"club": self.club}) + super().save(*args, **kwargs) diff --git a/formbuilder/migrations/0004_alter_field_key_alter_form_slug.py b/formbuilder/migrations/0004_alter_field_key_alter_form_slug.py new file mode 100644 index 0000000..2066841 --- /dev/null +++ b/formbuilder/migrations/0004_alter_field_key_alter_form_slug.py @@ -0,0 +1,23 @@ +# Generated by Django 6.0.6 on 2026-07-12 19:39 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('formbuilder', '0003_fix_tenant_uniqueness'), + ] + + operations = [ + migrations.AlterField( + model_name='field', + name='key', + field=models.SlugField(blank=True, verbose_name='key'), + ), + migrations.AlterField( + model_name='form', + name='slug', + field=models.SlugField(blank=True, verbose_name='slug'), + ), + ] diff --git a/formbuilder/models.py b/formbuilder/models.py index 2ec8457..9f8237e 100644 --- a/formbuilder/models.py +++ b/formbuilder/models.py @@ -2,15 +2,17 @@ from django.db import models from django.db.models import UniqueConstraint from django.utils.translation import gettext_lazy as _ -from clubmanager.base import ClubScopedModel, UUIDModel +from clubmanager.base import ClubScopedModel, UUIDModel, unique_slugify from members.models import Member class Form(ClubScopedModel): title = models.CharField(_("title"), max_length=255) - slug = models.SlugField(_("slug")) + slug = models.SlugField(_("slug"), blank=True) description = models.TextField(_("description"), blank=True) + slug_source = "title" + is_active = models.BooleanField(_("is active?"), default=True) login_required = models.BooleanField(_("login required?"), default=False) @@ -43,7 +45,7 @@ class Field(UUIDModel): FILE = "file", _("file") form = models.ForeignKey(Form, on_delete=models.CASCADE, related_name="fields", verbose_name=_("form")) - key = models.SlugField(_("key")) + key = models.SlugField(_("key"), blank=True) label = models.CharField(_("label"), max_length=255) field_type = models.CharField(_("field type"), max_length=255, choices=FieldType.choices, default=FieldType.TEXT) required = models.BooleanField(_("required?"), default=True) @@ -60,6 +62,11 @@ class Field(UUIDModel): UniqueConstraint(fields=["form", "key"], name="unique_field_key_per_form"), ] + def save(self, *args, **kwargs): + if not self.key: + self.key = unique_slugify(self, self.label, slug_field="key", scope={"form": self.form}) + super().save(*args, **kwargs) + def __str__(self): return self.label diff --git a/formbuilder/tests.py b/formbuilder/tests.py index a959654..d0610ca 100644 --- a/formbuilder/tests.py +++ b/formbuilder/tests.py @@ -76,6 +76,51 @@ class ModelTests(FormbuilderTestBase): self.name.delete() +class FormSlugTests(FormbuilderTestBase): + def test_slug_auto_populated_from_title(self): + form = Form.objects.create(club=self.club, title="Registration Form") + + self.assertEqual(form.slug, "registration-form") + + def test_explicit_slug_is_preserved(self): + form = Form.objects.create(club=self.club, title="Registration", slug="reg") + + self.assertEqual(form.slug, "reg") + + def test_slug_is_unique_per_club_with_suffix(self): + first = Form.objects.create(club=self.club, title="Registration") + second = Form.objects.create(club=self.club, title="Registration") + + self.assertEqual(first.slug, "registration") + self.assertEqual(second.slug, "registration-2") + + +class FieldKeyTests(FormbuilderTestBase): + def test_key_auto_populated_from_label(self): + field = Field.objects.create(form=self.form, label="First Name", order=5) + + self.assertEqual(field.key, "first-name") + + def test_explicit_key_is_preserved(self): + field = Field.objects.create(form=self.form, key="fn", label="First Name", order=5) + + self.assertEqual(field.key, "fn") + + def test_key_is_unique_per_form_with_suffix(self): + first = Field.objects.create(form=self.form, label="First Name", order=5) + second = Field.objects.create(form=self.form, label="First Name", order=6) + + self.assertEqual(first.key, "first-name") + self.assertEqual(second.key, "first-name-2") + + def test_same_key_allowed_in_a_different_form(self): + other_form = Form.objects.create(club=self.club, title="Other", slug="other") + here = Field.objects.create(form=self.form, label="First Name", order=5) + there = Field.objects.create(form=other_form, label="First Name", order=1) + + self.assertEqual(here.key, there.key) + + class FieldChoicesTests(FormbuilderTestBase): def test_string_options(self): self.assertEqual(field_choices(self.size), [("S", "S"), ("M", "M"), ("L", "L")]) diff --git a/shop/migrations/0001_initial.py b/shop/migrations/0001_initial.py new file mode 100644 index 0000000..8a5fd72 --- /dev/null +++ b/shop/migrations/0001_initial.py @@ -0,0 +1,29 @@ +# Generated by Django 6.0.6 on 2026-07-12 19:29 + +import django.db.models.deletion +import uuid +from django.db import migrations, models + + +class Migration(migrations.Migration): + + initial = True + + dependencies = [ + ('club', '0008_alter_clubmembership_unique_together_and_more'), + ] + + operations = [ + migrations.CreateModel( + name='Product', + fields=[ + ('id', models.UUIDField(default=uuid.uuid4, editable=False, primary_key=True, serialize=False)), + ('name', models.CharField(max_length=255, verbose_name='name')), + ('slug', models.SlugField(max_length=255, unique=True, verbose_name='slug')), + ('club', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='%(class)ss', to='club.club')), + ], + options={ + 'abstract': False, + }, + ), + ] diff --git a/shop/migrations/0002_alter_product_slug_and_more.py b/shop/migrations/0002_alter_product_slug_and_more.py new file mode 100644 index 0000000..9b865f4 --- /dev/null +++ b/shop/migrations/0002_alter_product_slug_and_more.py @@ -0,0 +1,23 @@ +# Generated by Django 6.0.6 on 2026-07-12 19:39 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('club', '0008_alter_clubmembership_unique_together_and_more'), + ('shop', '0001_initial'), + ] + + operations = [ + migrations.AlterField( + model_name='product', + name='slug', + field=models.SlugField(blank=True, max_length=255, verbose_name='slug'), + ), + migrations.AddConstraint( + model_name='product', + constraint=models.UniqueConstraint(fields=('club', 'slug'), name='unique_product_slug_per_club'), + ), + ] diff --git a/shop/models.py b/shop/models.py index 6b20219..ac6877f 100644 --- a/shop/models.py +++ b/shop/models.py @@ -1 +1,17 @@ -# Create your models here. +from django.db import models +from django.db.models import UniqueConstraint +from django.utils.translation import gettext_lazy as _ + +from clubmanager.base import ClubScopedModel + + +class Product(ClubScopedModel): + name = models.CharField(_("name"), max_length=255) + slug = models.SlugField(_("slug"), max_length=255, blank=True) + + slug_source = "name" + + class Meta: + constraints = [ + UniqueConstraint(fields=["club", "slug"], name="unique_product_slug_per_club"), + ] diff --git a/shop/tests.py b/shop/tests.py index a39b155..bd1b8ec 100644 --- a/shop/tests.py +++ b/shop/tests.py @@ -1 +1,39 @@ -# Create your tests here. +from django.test import TestCase + +from club.models import Club + +from .models import Product + + +class ProductSlugTests(TestCase): + def setUp(self): + self.club = Club.objects.create(name="Ajax United", slug="ajax-united") + + def test_slug_auto_populated_from_name(self): + product = Product.objects.create(club=self.club, name="Home Jersey") + + self.assertEqual(product.slug, "home-jersey") + + def test_explicit_slug_is_preserved(self): + product = Product.objects.create(club=self.club, name="Home Jersey", slug="custom") + + self.assertEqual(product.slug, "custom") + + def test_slug_is_unique_per_club_with_suffix(self): + first = Product.objects.create(club=self.club, name="Home Jersey") + second = Product.objects.create(club=self.club, name="Home Jersey") + + self.assertEqual(first.slug, "home-jersey") + self.assertEqual(second.slug, "home-jersey-2") + + def test_same_slug_allowed_in_a_different_club(self): + other = Club.objects.create(name="Rival FC", slug="rival-fc") + here = Product.objects.create(club=self.club, name="Home Jersey") + there = Product.objects.create(club=other, name="Home Jersey") + + self.assertEqual(here.slug, there.slug) + + def test_unsluggable_name_falls_back(self): + product = Product.objects.create(club=self.club, name="###") + + self.assertEqual(product.slug, "item")