diff --git a/django_admin_reversefields/mixins.py b/django_admin_reversefields/mixins.py index 34e1bd3..2a832f0 100644 --- a/django_admin_reversefields/mixins.py +++ b/django_admin_reversefields/mixins.py @@ -50,6 +50,7 @@ from django import forms from django.contrib.admin.widgets import FilteredSelectMultiple +from django.core.exceptions import FieldDoesNotExist, ImproperlyConfigured from django.db import models, transaction from django.http import HttpRequest @@ -475,6 +476,41 @@ def get_reverse_relations(self) -> dict[str, ReverseRelationConfig]: """ return self.reverse_relations + def _validate_reverse_relation_configs(self) -> None: + """Validate reverse relation configuration before form construction. + + Raises: + ImproperlyConfigured: If a reverse relation has an invalid ``fk_field`` + (missing, wrong type, or targeting a different parent model). + """ + for field_name, config in self.get_reverse_relations().items(): + try: + fk = config.model._meta.get_field(config.fk_field) + except FieldDoesNotExist as exc: + raise ImproperlyConfigured( + f"Invalid reverse_relations['{field_name}']: fk_field " + f"'{config.fk_field}' does not exist on model '{config.model._meta.label}'." + ) from exc + + if not isinstance(fk, (models.ForeignKey, models.OneToOneField)): + raise ImproperlyConfigured( + f"Invalid reverse_relations['{field_name}']: fk_field " + f"'{config.fk_field}' on model '{config.model._meta.label}' must be " + "a ForeignKey or OneToOneField." + ) + + target_model = fk.remote_field.model + admin_model = self.model + if not ( + issubclass(admin_model, target_model) or issubclass(target_model, admin_model) + ): + raise ImproperlyConfigured( + f"Invalid reverse_relations['{field_name}']: fk_field " + f"'{config.fk_field}' on model '{config.model._meta.label}' points to " + f"'{target_model._meta.label}', but this admin manages " + f"'{admin_model._meta.label}'." + ) + def get_fields(self, request, obj=None): # type: ignore[override] """Ensure virtual reverse field names are part of the rendered fields. @@ -514,6 +550,8 @@ def get_form(self, request: HttpRequest, obj=None, **kwargs): the configured reverse relation fields. """ relations = dict(self.get_reverse_relations()) + if relations: + self._validate_reverse_relation_configs() if relations: provided_fields = kwargs.get("fields") if provided_fields: diff --git a/tests/test_edge_cases.py b/tests/test_edge_cases.py index 80ec01a..caa4e3a 100644 --- a/tests/test_edge_cases.py +++ b/tests/test_edge_cases.py @@ -2,19 +2,71 @@ # Test imports from django.contrib import admin +from django.core.exceptions import ImproperlyConfigured from django.db import transaction from django_admin_reversefields.mixins import ( ReverseRelationAdminMixin, ReverseRelationConfig, ) -from tests.models import Company, CompanySettings, Department, Project +from tests.models import Company, CompanySettings, Department, Employee, Project from tests.shared_test_base import BaseAdminMixinTestCase class EdgeCasesTests(BaseAdminMixinTestCase): """Test suite for edge cases and non-parameterizable scenarios.""" + def test_invalid_fk_field_name_fails_early(self): + """Misconfigured fk_field names should fail during form construction.""" + + class TestAdmin(ReverseRelationAdminMixin, admin.ModelAdmin): + reverse_relations = { + "bad_binding": ReverseRelationConfig( + model=Department, + fk_field="does_not_exist", + ), + } + + admin_inst = TestAdmin(Company, self.site) + request = self.factory.get("/") + + with self.assertRaisesMessage(ImproperlyConfigured, "does not exist on model"): + admin_inst.get_form(request, self.company) + + def test_non_relational_fk_field_fails_early(self): + """fk_field must reference a ForeignKey or OneToOneField.""" + + class TestAdmin(ReverseRelationAdminMixin, admin.ModelAdmin): + reverse_relations = { + "bad_binding": ReverseRelationConfig( + model=Department, + fk_field="name", # CharField, not a relation + ), + } + + admin_inst = TestAdmin(Company, self.site) + request = self.factory.get("/") + + with self.assertRaisesMessage(ImproperlyConfigured, "must be a ForeignKey or OneToOneField"): + admin_inst.get_form(request, self.company) + + def test_fk_field_target_model_mismatch_fails_early(self): + """fk_field target model must match the admin's parent model.""" + + class TestAdmin(ReverseRelationAdminMixin, admin.ModelAdmin): + reverse_relations = { + "bad_binding": ReverseRelationConfig( + model=Employee, + fk_field="department", # Points to Department, not Company + ), + } + + admin_inst = TestAdmin(Company, self.site) + request = self.factory.get("/") + + with self.assertRaisesMessage(ImproperlyConfigured, "but this admin manages"): + admin_inst.get_form(request, self.company) + def test_large_dataset_performance(self): """Test base operations with large datasets.""" # Create a large dataset