-
-
Notifications
You must be signed in to change notification settings - Fork 300
Auto-discover child models and inlines #582
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
f075aab
d9644fa
d12eccf
308f1d4
4830b30
f8a9302
91b0a47
87e4b95
0de8744
fa69bf7
8da75ce
ada2ef3
b4db7e0
c97c5c6
7a0f54c
4136bd6
d6b2a88
aaa7fa3
0f5f302
b6cac5e
49be2ef
39f1d79
515885d
b56b3cd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -63,6 +63,7 @@ def __iter__(self): | |||||||||||||||||||||||||||||||
| for form in self.formset.extra_forms + self.formset.empty_forms: | ||||||||||||||||||||||||||||||||
| model = form._meta.model | ||||||||||||||||||||||||||||||||
| child_inline = self.opts.get_child_inline_instance(model) | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| yield PolymorphicInlineAdminForm( | ||||||||||||||||||||||||||||||||
| formset=self.formset, | ||||||||||||||||||||||||||||||||
| form=form, | ||||||||||||||||||||||||||||||||
|
|
@@ -141,3 +142,26 @@ def get_inline_formsets(self, request, formsets, inline_instances, obj=None, *ar | |||||||||||||||||||||||||||||||
| admin_formset.request = request | ||||||||||||||||||||||||||||||||
| admin_formset.obj = obj | ||||||||||||||||||||||||||||||||
| return inline_admin_formsets | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| def get_leaf_subclasses(cls, exclude=None): | ||||||||||||||||||||||||||||||||
| "Get leaf subclasses of `cls` class" | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| if exclude is None: | ||||||||||||||||||||||||||||||||
|
Comment on lines
+148
to
+150
|
||||||||||||||||||||||||||||||||
| "Get leaf subclasses of `cls` class" | |
| if exclude is None: | |
| """ | |
| Return a list of all leaf subclasses of the given class. | |
| A leaf subclass is a subclass that does not have any further subclasses. | |
| Args: | |
| cls: The base class to search for leaf subclasses. | |
| exclude: An optional class, list, or tuple of classes to exclude from the results. | |
| Returns: | |
| list: A list of leaf subclasses of `cls`, excluding any specified in `exclude`. | |
| """ |
Copilot
AI
Dec 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The __subclasses__() method only returns direct subclasses, not all descendants. This is correct for a recursive implementation, but be aware that this will only work for classes that have been imported/defined at the time this function is called. If child classes are defined in modules that haven't been imported yet, they won't be discovered. Consider documenting this limitation or ensuring all relevant modules are imported before auto-discovery.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -20,7 +20,7 @@ | |||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||
| from polymorphic.formsets.utils import add_media | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| from .helpers import PolymorphicInlineSupportMixin | ||||||||||||||||||||||||||||||||||||||||||||
| from .helpers import PolymorphicInlineSupportMixin, get_leaf_subclasses | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| class PolymorphicInlineModelAdmin(InlineModelAdmin): | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -53,7 +53,14 @@ class PolymorphicInlineModelAdmin(InlineModelAdmin): | |||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| #: Inlines for all model sub types that can be displayed in this inline. | ||||||||||||||||||||||||||||||||||||||||||||
| #: Each row is a :class:`PolymorphicInlineModelAdmin.Child` | ||||||||||||||||||||||||||||||||||||||||||||
piranna marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||
| child_inlines = () | ||||||||||||||||||||||||||||||||||||||||||||
| child_inlines = None | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| #: The models that should be excluded from the auto-discovered leaf | ||||||||||||||||||||||||||||||||||||||||||||
| #: model sub types that can be displayed in this inline. This can be | ||||||||||||||||||||||||||||||||||||||||||||
| #: a list of models or a single model. It's useful to exclude | ||||||||||||||||||||||||||||||||||||||||||||
| #: non-abstract base models (abstract models are always excluded) | ||||||||||||||||||||||||||||||||||||||||||||
| #: when they don't have defined any child models. | ||||||||||||||||||||||||||||||||||||||||||||
| exclude_children = None | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| def __init__(self, parent_model, admin_site): | ||||||||||||||||||||||||||||||||||||||||||||
| super().__init__(parent_model, admin_site) | ||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -77,12 +84,44 @@ def __init__(self, parent_model, admin_site): | |||||||||||||||||||||||||||||||||||||||||||
| for child_inline in self.child_inline_instances: | ||||||||||||||||||||||||||||||||||||||||||||
| self._child_inlines_lookup[child_inline.model] = child_inline | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| def get_child_inlines(self): | ||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||
| Return the derived inline classes which this admin should handle | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| This should return a list of tuples, exactly like | ||||||||||||||||||||||||||||||||||||||||||||
| :attr:`child_inlines` is. | ||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||
| The inline classes can be retrieved as | ||||||||||||||||||||||||||||||||||||||||||||
| ``base_inline.__subclasses__()``, a setting in a config file, or | ||||||||||||||||||||||||||||||||||||||||||||
| a query of a plugin registration system at your option | ||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+89
to
+96
|
||||||||||||||||||||||||||||||||||||||||||||
| Return the derived inline classes which this admin should handle | |
| This should return a list of tuples, exactly like | |
| :attr:`child_inlines` is. | |
| The inline classes can be retrieved as | |
| ``base_inline.__subclasses__()``, a setting in a config file, or | |
| a query of a plugin registration system at your option | |
| Return the derived inline classes which this admin should handle. | |
| If the :attr:`child_inlines` attribute is set, this method returns its value. | |
| Otherwise, it auto-discovers all leaf subclasses of | |
| :class:`PolymorphicInlineModelAdmin.Child` whose model is a subclass of | |
| this inline's model. The auto-discovery process excludes any models specified | |
| in the :attr:`exclude_children` attribute. | |
| Returns: | |
| tuple: A tuple of inline classes to be used as child inlines. | |
| Raises: | |
| ImproperlyConfigured: If no child inlines are found. |
Copilot
AI
Dec 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The auto-discovery searches for ALL leaf subclasses of PolymorphicInlineModelAdmin.Child across the entire application, then filters by model. This means if you have multiple polymorphic inline configurations, each will scan all Child subclasses application-wide. While the filtering by issubclass(inline.model, self.model) ensures only relevant inlines are used, this approach may be inefficient and could lead to confusion if developers define Child classes that they don't intend to be auto-discovered. Consider documenting this behavior clearly or providing guidance on organizing Child classes to avoid unintended discovery.
Copilot
AI
Dec 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new auto-discovery behavior for child inlines lacks test coverage. Consider adding tests that verify:
- Child inlines are correctly auto-discovered when
child_inlinesis None - The
exclude_childrenparameter works correctly - An
ImproperlyConfiguredexception is raised when no child inlines are found - Only inlines that are subclasses of the current inline's model are included
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -19,6 +19,7 @@ | |||||||||||||||||||||||||||||||||||||||||
| from polymorphic.utils import get_base_polymorphic_model | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| from .forms import PolymorphicModelChoiceForm | ||||||||||||||||||||||||||||||||||||||||||
| from .helpers import get_leaf_subclasses | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| class RegistrationClosed(RuntimeError): | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -51,6 +52,13 @@ class PolymorphicParentModelAdmin(admin.ModelAdmin): | |||||||||||||||||||||||||||||||||||||||||
| #: The child models that should be displayed | ||||||||||||||||||||||||||||||||||||||||||
| child_models = None | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| #: The models that should be excluded from the auto-discovered child | ||||||||||||||||||||||||||||||||||||||||||
| #: leaf models that should be displayed. This can be a list of | ||||||||||||||||||||||||||||||||||||||||||
| #: models or a single model. It's useful to exclude non-abstract | ||||||||||||||||||||||||||||||||||||||||||
| #: base models (abstract models are always excluded) when they don't | ||||||||||||||||||||||||||||||||||||||||||
| #: have defined any child models. | ||||||||||||||||||||||||||||||||||||||||||
| exclude_children = None | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| #: Whether the list should be polymorphic too, leave to ``False`` to optimize | ||||||||||||||||||||||||||||||||||||||||||
| polymorphic_list = False | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -109,24 +117,41 @@ def register_child(self, model, model_admin): | |||||||||||||||||||||||||||||||||||||||||
| def get_child_models(self): | ||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||
| Return the derived model classes which this admin should handle. | ||||||||||||||||||||||||||||||||||||||||||
| This should return a list of tuples, exactly like :attr:`child_models` is. | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| The model classes can be retrieved as ``base_model.__subclasses__()``, | ||||||||||||||||||||||||||||||||||||||||||
| a setting in a config file, or a query of a plugin registration system at your option | ||||||||||||||||||||||||||||||||||||||||||
| This should return a list of tuples, exactly like | ||||||||||||||||||||||||||||||||||||||||||
| :attr:`child_models` is. | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| The model classes can be retrieved as | ||||||||||||||||||||||||||||||||||||||||||
| ``base_model.__subclasses__()``, a setting in a config file, or | ||||||||||||||||||||||||||||||||||||||||||
| a query of a plugin registration system at your option | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+121
to
+126
|
||||||||||||||||||||||||||||||||||||||||||
| This should return a list of tuples, exactly like | |
| :attr:`child_models` is. | |
| The model classes can be retrieved as | |
| ``base_model.__subclasses__()``, a setting in a config file, or | |
| a query of a plugin registration system at your option | |
| If the :attr:`child_models` attribute is set, its value is returned. | |
| If :attr:`child_models` is None (the default), this method will | |
| automatically discover all concrete (non-abstract) leaf subclasses | |
| of the base model using :func:`get_leaf_subclasses`. | |
| The :attr:`exclude_children` attribute can be set to a model or a list | |
| of models to exclude from the auto-discovered child models. Abstract | |
| models are always excluded automatically. | |
| Returns: | |
| list: A list of child model classes (or tuples, as in :attr:`child_models`). | |
| Raises: | |
| ImproperlyConfigured: If no child models are found. |
Copilot
AI
Dec 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new auto-discovery behavior for child models lacks test coverage. Consider adding tests that verify:
- Child models are correctly auto-discovered when
child_modelsis None - The
exclude_childrenparameter works correctly - An
ImproperlyConfiguredexception is raised when no child models are found - Abstract models are properly excluded from auto-discovery
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function is missing proper documentation including parameter descriptions and return type. Following the codebase convention, add a proper docstring with parameter descriptions, return type (
:rtype: list), and explanation of behavior (e.g., "Recursively finds all leaf subclasses ofclsthat are not abstract and not in the exclude list.").