#31558: Support the use of boolean attribute on properties in the admin.
-------------------------------------+-------------------------------------
Reporter: Alexandre Poitevin | Owner: Alexandre
| Poitevin
Type: New feature | Status: assigned
Component: contrib.admin | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Alexandre Poitevin):
So I do really think I’ve tracked down the reason why it works with
`short_description` and `admin_order_field`, but not with `boolean`.
I don’t know if this is the good place to make some kind of code review
(maybe in the dev mailist?), but anyway as a reference (at least for me):
For `short_description`, the code is in
`contrib.admin.utils.label_for_fields()`:
{{{
if hasattr(attr, "short_description"):
label = attr.short_description
elif (isinstance(attr, property) and
hasattr(attr, "fget") and
hasattr(attr.fget, "short_description")):
label = attr.fget.short_description
elif callable(attr):
if attr.__name__ == "<lambda>":
label = "--"
else:
label = pretty_name(attr.__name__)
else:
label = pretty_name(name)
}}}
The `property` case is properly handled.
For `admin_order_field`, it’s in
`contrib.admin.views.main.get_ordering_fields()`:
{{{
if callable(field_name):
attr = field_name
elif hasattr(self.model_admin, field_name):
attr = getattr(self.model_admin, field_name)
else:
attr = getattr(self.model, field_name)
if isinstance(attr, property) and hasattr(attr, 'fget'):
attr = attr.fget
return getattr(attr, 'admin_order_field', None)
}}}
Same thing.
But for `boolean`, it’s in
`contrib.admin.templatetags.admin_list.items_for_result()`:
{{{
boolean = getattr(attr, 'boolean', False)
result_repr = display_for_value(value,
empty_value_display, boolean)
}}}
No special handling for properties…
And `items_for_result()` has no test to begin with.
The implementation of the feature seems easy, but I’m not sure about how
to write a good test for that.
I also think that the retrieving of these models’ fields “metadata” should
be refactored to appear in one_place only.
There is already a `lookup_field()` in `contrib.admin.utils` but it seems
it’s not enough for `property` (and again no unit tests):
{{{
def lookup_field(name, obj, model_admin=None):
opts = obj._meta
try:
f = _get_non_gfk_field(opts, name)
except (FieldDoesNotExist, FieldIsAForeignKeyColumnName):
# For non-field values, the value is either a method, property or
# returned via a callable.
if callable(name):
attr = name
value = attr(obj)
elif hasattr(model_admin, name) and name != '__str__':
attr = getattr(model_admin, name)
value = attr(obj)
else:
attr = getattr(obj, name)
if callable(attr):
value = attr()
else:
value = attr
f = None
else:
attr = None
value = getattr(obj, name)
return f, attr, value
}}}
I think (lots of “I think”) that I need to write tests for this before
anything else.
I just need some orientations in order to take care of this.
--
Ticket URL: <https://code.djangoproject.com/ticket/31558#comment:10>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/074.2b6d4fd20d34bb4886aa674f6671e507%40djangoproject.com.
