#12679 closed Bug (fixed)
In admin, inlines should be allowed to be properties
Reported by: | mariarchi | Owned by: | nobody |
---|---|---|---|
Component: | contrib.admin | Version: | dev |
Severity: | Normal | Keywords: | admin inlines duck typing |
Cc: | admin@… | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Currently, if admin class defines an attribute 'inlines' which is not an instance of tuple or a list, django will raise an error.
I believe that this is wrong and contradicts the duck typing philosophy - we shouldn't care that the attribute 'inlines' is an instance of list/tuple, but rather we should care that it is iterable.
The reason why I believe that behavior is wrong is the following:
one might want to define 'inlines' as a property. This might happen when admin class A inherits from admin class B, and class B adds some inlines (that can be useful when developing a pluggable application). If class A has some inlines of its own, it can be useful to declare inlines as a property and then call 'super' to retrieve the inlines of class B.
Change History (9)
comment:1 by , 15 years ago
comment:2 by , 15 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:3 by , 15 years ago
Component: | Contrib apps → django.contrib.admin |
---|
comment:4 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → Bug |
comment:5 by , 14 years ago
Easy pickings: | unset |
---|
Currently a lot of ModelAdmin
's attributes (list_display
, readonly_fields
, list_filter
, etc.) are expected to be lists or tuples. ModelAdmin.inlines
shouldn't be treated as a special case. If we allow it to be merely an iterable, then the same should be done for all those other attributes. (Hint: check out django.contrib.admin.validation.check_isseq()
)
See also #16089 which is looking at adding flexibility with inlines registration.
comment:7 by , 5 years ago
Turns out this got fixed in 1d8eb0cae57731b481a88dca272b2cb0d645bd8e.
I wrote a pull request to add a test for the specific usecase of the original ticket but I'm not 100% convinced it's necessary.
To me, the test basically checks that python properties work which seems silly. Plus if we're testing ModelAdmin.inlines
then why not check all other potential ModelAdmin
options?
comment:8 by , 5 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Thanks for checking this. I don't think that we need more tests.
Anyway, that behavior is quite inconsistent with the fact that for most attributes in model/admin/url definition django does not check the instance of said attribute