Support the use of boolean attribute on properties in the admin.

[Alexandre Poitevin]

Example (adapted from the polls tutorial):

class Question(models.Model):

    DEFINITION_OF_RECENT = {"days": 1}

    text = models.CharField(max_length=255)
    publication_date = models.DateTimeField('Date of publication')

    def was_published_recently(self):
        timenow = timezone.now()
        recent = timenow - timedelta(**self.DEFINITION_OF_RECENT)
        return recent <= self.publication_date <= timenow

    was_published_recently.admin_order_field = 'publication_date'
    was_published_recently.short_description = 'Published Recently?'
    was_published_recently.boolean = True
    was_published_recently = property(was_published_recently)

There is no problem with short_description nor admin_order_filed, but boolean does not act as expected.

(I hesitated between Bug and New feature for this ticket…)

[Mariusz Felisiak]

Thanks. This is definitely a new feature (see similar ticket #30259 for admin_order_field support). We can support it but when using the property() function and not with decorator, that's why I updated the ticket description.

[Alexandre Poitevin]

Thanks for accept my ticket.
I was hoping to contribute and make the pull request myself, but I’m a new to Django, so…

Anyway I would like to say just 2 things:

1. I don’t understand what is the difference between the decorator and calling explicitly property.

I saw a similar statement in the documentation. But in fact, the effect is the same:

>>> class C: 
...     def m(self): ... 
...     m.attr = 42 
...     m = property(m)
...
>>> C.m.attr                                                                    
Traceback (most recent call last):
 ...
AttributeError: 'property' object has no attribute 'attr'
>>> C.m.fget.attr                                                               
42
>>> class C: 
...     @property 
...     def m(self): ... 
...     m.fget.attr = 42
...
>>> C.m.attr                                                                    
Traceback (most recent call last):
 ...
AttributeError: 'property' object has no attribute 'attr'
>>> C.m.fget.attr                                                               
42

So the original method is stored as a fget attribute of the property, with or without the decorator, so I don’t see why this feature couldn’t be achieve with the decorator.

2. After navigating in the source code and made a little work of debugging, I found two functions that may the ones requiring modification to enable this feature:

contrib.admin.templatetags.items_for_result
contrib.admin.utils.lookup_fields

In the second:

        # For non-field values, the value is either a method, property or
        # returned via a callable.
        if callable(name):
            attr = name
            value = attr(obj)

And in the first :

        try:
            f, attr, value = lookup_field(field_name, result, cl.model_admin)
        except ObjectDoesNotExist:
            result_repr = empty_value_display
        else:
            empty_value_display = getattr(attr, 'empty_value_display', empty_value_display)
            if f is None or f.auto_created:
                if field_name == 'action_checkbox':
                    row_classes = ['action-checkbox']
                boolean = getattr(attr, 'boolean', False)  # <= HERE !
                result_repr = display_for_value(value, empty_value_display, boolean)

By the way, this one (items_for_result) has no unit tests, or at least I didn’t see anyone (grep -nr "items_for_result" tests/).

[Mariusz Felisiak]

fget is a function for getting an attribute value, it's an implementation detail of property(). Setting variable on a function is hacky and cannot be recommended in our docs, IMO.

[Alexandre Poitevin]

Setting a variable function is already what you find in the tutorial…

class Question(models.Model):
    # ...
    def was_published_recently(self):
        now = timezone.now()
        return now - datetime.timedelta(days=1) <= self.pub_date <= now
    was_published_recently.admin_order_field = 'pub_date'
    was_published_recently.boolean = True
    was_published_recently.short_description = 'Published recently?'

See: ​https://docs.djangoproject.com/en/3.0/intro/tutorial07/#id8

And in ​https://docs.djangoproject.com/en/3.0/ref/contrib/admin/ :

Elements of list_display can also be properties. Please note however, that due to the way properties work in Python, setting short_description or admin_order_field on a property is only possible when using the property() function and not with the @property decorator.

For example:

class Person(models.Model):
    first_name = models.CharField(max_length=50)
    last_name = models.CharField(max_length=50)

    def my_property(self):
        return self.first_name + ' ' + self.last_name
    my_property.short_description = "Full name of the person"
    my_property.admin_order_field = 'last_name'

    full_name = property(my_property)

class PersonAdmin(admin.ModelAdmin):
    list_display = ('full_name',)

End of citation.

And this is what I was referring earlier. You can’t set the attribute on property, but you can set it to its fget. There’s no problem about using the decorator. We just have do update this part of the documentation.

[Mariusz Felisiak]

Tutorial doesn't use fget. Again, variables defined in properties must be known before creating a property, that's why setting variable for decorated property raises AttributeError. If you really believe that we should encourage users for override fget you can start a discussion on DevelopersMailingList.

[Alexandre Poitevin]

Not now, because in fact it is a separate debate.
The reason it began, is the modification of my ticket (no grudge intended).

I’m gonna work on a PR. I think the good place to put the corresponding unit test is tests/modeladmin/tests_actions.py.

[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.

[Anvansh Singh]

I'm picking this up, will try to send a patch for this soon.

[Mariusz Felisiak]

​PR

[Mariusz Felisiak]

​New PR

[Mariusz Felisiak <felisiak.mariusz@…>]

In 225328e:

Fixed #31558 -- Added support for boolean attribute on properties in ModelAdmin.list_display.