diff --git a/CHANGES.md b/CHANGES.md index 1e64d3b..811cc79 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,10 @@ # Changelog +**11.5.0** (2024-09-29) + +* Added system check to ensure that all model relation fields have a related name, either directly set or via the model + meta-option "default_related_name". + **11.4.0** (2024-09-24) * Added system check to enforce naming conventions for DateFields and DateTimeFields diff --git a/ambient_toolbox/__init__.py b/ambient_toolbox/__init__.py index 0697442..0ef23cb 100644 --- a/ambient_toolbox/__init__.py +++ b/ambient_toolbox/__init__.py @@ -1,3 +1,3 @@ """Python toolbox of Ambient Digital containing an abundance of useful tools and gadgets.""" -__version__ = "11.4.0" +__version__ = "11.5.0" diff --git a/ambient_toolbox/system_checks/model_relation_conventions.py b/ambient_toolbox/system_checks/model_relation_conventions.py new file mode 100644 index 0000000..1ff1b23 --- /dev/null +++ b/ambient_toolbox/system_checks/model_relation_conventions.py @@ -0,0 +1,41 @@ +from django.apps import apps +from django.conf import settings +from django.core import checks +from django.db.models import ForeignKey, ManyToManyField, OneToOneField + + +def check_model_related_names_for_related_name(*args, **kwargs): + """ + Checks all model relation fields ('ForeignKey', 'ManyToManyField', "OneToOneField") for having a defined + "related_name". Either directly via the field or otherwise via the model meta-option "default_related_name". + """ + + project_apps = [ + app.split(".")[-1] for app in settings.INSTALLED_APPS if app.startswith(settings.ROOT_URLCONF.split(".")[0]) + ] + issue_list = [] + + # Iterate all registered models... + for model in apps.get_models(): + # Check if the model is from your project... + if model._meta.app_label in project_apps: + # If the model has a related name, this will be inherited to all relation fields and we're OK + if model._meta.default_related_name: + continue + + # Iterate over all fields... + for field in model._meta.get_fields(): + # Check relation field types... + if isinstance(field, (ForeignKey, ManyToManyField, OneToOneField)): + # Check if the field has a related name set... + if not field._related_name: + issue_list.append( + checks.Warning( + f"'{model.__name__}.{field.name}' doesn't have a related name set and neither does the " + "model define a default related name.", + obj=field, + id="ambient_toolbox.W003", + ) + ) + + return issue_list diff --git a/docs/features/system_checks.md b/docs/features/system_checks.md index 55367ce..269bffa 100644 --- a/docs/features/system_checks.md +++ b/docs/features/system_checks.md @@ -4,8 +4,7 @@ Inspired by [Luke Plants article](https://lukeplant.me.uk/blog/posts/enforcing-conventions-in-django-projects-with-introspection/), -this package implements a system check to ensure that all custom DateField and DateTimeField are named in a uniform -manner. +this package implements a system check to ensure that all custom DateField and DateTimeField are named uniformly. By default, it requires for DateFields to end on `_date` and DateTimeFields on `_at`. @@ -35,3 +34,29 @@ You can configure which field name endings are allowed by setting these variable ALLOWED_MODEL_DATETIME_FIELD_ENDINGS = ["_at"] ALLOWED_MODEL_DATE_FIELD_ENDINGS = ["_date"] ```` + +## Model relation naming conventions + +If you create a relation between two models (ForeignKey, ManyToMany, OneToOne), Django will name this relation with +the somehow obscure `*_set` name. + +Since a well-chosen related name, either on the field itself or on the model meta-option "default_related_name", this +system check encourages you to set one of these attributes. Explicit is better than implicit. + +It's straightforward to register this system check in your project. + +````python +# apps/common/apps.py +from ambient_toolbox.system_checks.model_relation_conventions import check_model_related_names_for_related_name + +from django.apps import AppConfig +from django.core.checks import register + + +class CommonConfig(AppConfig): + name = "apps.common" + verbose_name = "Common" + + def ready(self): + register(check_model_related_names_for_related_name) +```` diff --git a/testapp/migrations/0002_modelwithoutrelatednameonfieldandmeta_and_more.py b/testapp/migrations/0002_modelwithoutrelatednameonfieldandmeta_and_more.py new file mode 100644 index 0000000..5b639b0 --- /dev/null +++ b/testapp/migrations/0002_modelwithoutrelatednameonfieldandmeta_and_more.py @@ -0,0 +1,61 @@ +# Generated by Django 5.0.7 on 2024-09-29 06:46 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("testapp", "0001_initial"), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name="ModelWithoutRelatedNameOnFieldAndMeta", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "relation_field", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to=settings.AUTH_USER_MODEL, + ), + ), + ], + ), + migrations.CreateModel( + name="ModelWithoutRelatedNameOnFieldButWithMeta", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "relation_field", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + to=settings.AUTH_USER_MODEL, + ), + ), + ], + options={ + "default_related_name": "model_without_related_name_on_field_but_with_metas", + }, + ), + ] diff --git a/testapp/models.py b/testapp/models.py index f875a82..ed573bf 100644 --- a/testapp/models.py +++ b/testapp/models.py @@ -131,3 +131,20 @@ class ModelNameTimeBasedFieldTest(models.Model): def __str__(self): return self.id + + +class ModelWithoutRelatedNameOnFieldAndMeta(models.Model): + relation_field = models.ForeignKey("auth.User", related_name=None, on_delete=models.CASCADE) + + def __str__(self): + return self.id + + +class ModelWithoutRelatedNameOnFieldButWithMeta(models.Model): + relation_field = models.ForeignKey("auth.User", related_name=None, on_delete=models.CASCADE) + + class Meta: + default_related_name = "model_without_related_name_on_field_but_with_metas" + + def __str__(self): + return self.id diff --git a/tests/system_checks/test_model_relation_conventions.py b/tests/system_checks/test_model_relation_conventions.py new file mode 100644 index 0000000..1d3c01e --- /dev/null +++ b/tests/system_checks/test_model_relation_conventions.py @@ -0,0 +1,23 @@ +from django.core import checks +from django.test import SimpleTestCase + +from ambient_toolbox.system_checks.model_relation_conventions import check_model_related_names_for_related_name +from testapp.models import ModelWithoutRelatedNameOnFieldAndMeta + + +class CheckModelRelationConventionsTestCase(SimpleTestCase): + def test_check_model_related_names_for_related_name_regular(self): + # Create expected warning + relation_warning = checks.Warning( + "'ModelWithoutRelatedNameOnFieldAndMeta.relation_field' doesn't have a related name set and neither " + "does the model define a default related name.", + obj=ModelWithoutRelatedNameOnFieldAndMeta.relation_field.field, + id="ambient_toolbox.W003", + ) + + # Call system check + error_list = check_model_related_names_for_related_name() + + # Assert warngins + self.assertEqual(len(error_list), 1) + self.assertIn(relation_warning, error_list)