Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#29428 closed Bug (fixed)

Add support to admin for expressions like (Lower('myfield'),) in model Meta.ordering

Reported by: Dutch Stiphout Owned by: Tim Graham <timograham@…>
Component: contrib.admin Version: 2.0
Severity: Release blocker Keywords: admin ordering expression AttributeError
Cc: Mariusz Felisiak, Carlton Gibson Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Similar to ticket #26257.

This DOES WORK in the views I've written, but when I try to access /admin/balfor/scheduledtransaction/ I get the same error as in the referenced ticket:

(Note that my app name is balfor, running Django version 2.0.4 final)

AttributeError at /admin/balfor/scheduledtransaction/
'Lower' object has no attribute 'startswith'

models.py:

from django.db import models
from django.db.models.functions import Lower

class ScheduledTransaction(models.Model):
    description = models.CharField(max_length=50, default="New Scheduled Transaction")
    period = models.CharField(max_length=2, default="1S")

    class Meta:
        ordering = ['period', Lower('description')]

admin.py:

admin.site.register(ScheduledTransaction)

Change History (14)

comment:1 by Claude Paroz, 6 years ago

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

I was able to reproduce.

comment:2 by Nicolas Noé, 6 years ago

Owner: changed from nobody to Nicolas Noé
Status: newassigned

comment:3 by Nicolas Noé, 6 years ago

At first look, it seems the issue was previously discovered (#28958), but the fix is incomplete (only solves for F() expressions, while the doc states "query expressions are supported".

Last edited 6 years ago by Tim Graham (previous) (diff)

comment:4 by Mariusz Felisiak, 6 years ago

Cc: Mariusz Felisiak added

comment:5 by Carlton Gibson, 6 years ago

Cc: Carlton Gibson added

A couple of points:

First, I wondered why the existing test from the fix for #28958 didn't fail since it already uses Upper (here):

     Upper(F('name')).asc(),

The key point is the asc() call, which returns an OrderBy (which then gets processed by the block added in the fix).

Remove the asc() call and the test fails with the error raised in get_ordering_field_columns().

The ordering works without the asc() call (or the F for that matter) so maybe get_ordering_field_columns() should call field.asc() (if available, when not already an OrderBy) in order to correct the behaviour here.

Second, whilst you can set an ordering including (say) Lower('description') on the model, if you try to set it on the ModelAdmin, the admin's `_check_ordering` check will error (in LOOKUP_SEP in field_name). This occurs in for the transform or OrderBy case. This check needs adjusting.

@Nicolas Noé Are you good to work on this ticket still?

Last edited 6 years ago by Carlton Gibson (previous) (diff)

comment:6 by Nicolas Noé, 6 years ago

Owner: Nicolas Noé removed
Status: assignednew

Sorry guys, I"ve been much more busy than expected lately. Deassigning the ticket now.

comment:7 by Tim Graham, 6 years ago

Has patch: set

comment:8 by Carlton Gibson, 6 years ago

Triage Stage: AcceptedReady for checkin

comment:9 by Tim Graham <timograham@…>, 6 years ago

Owner: set to Tim Graham <timograham@…>
Resolution: fixed
Status: newclosed

In 0d8e3e6:

Fixed #29428 -- Fixed admin changelist crash when using a query expression without asc()/desc() in the ordering.

comment:10 by Tim Graham <timograham@…>, 6 years ago

In ec2c9c35:

Refs #29428 -- Fixed admin check crash when using a query expression in ModelAdmin.ordering.

comment:11 by Tim Graham <timograham@…>, 6 years ago

In a041161b:

[2.1.x] Fixed #29428 -- Fixed admin changelist crash when using a query expression without asc()/desc() in the ordering.

Backport of 0d8e3e608ee9aab5076d497664aa97e0a29e523e from master

comment:12 by Tim Graham <timograham@…>, 6 years ago

In 531a152:

[2.1.x] Refs #29428 -- Fixed admin check crash when using a query expression in ModelAdmin.ordering.

Backport of ec2c9c353113bb1db6e32ed3f0b6c28bc06ca2eb from master

comment:13 by Tim Graham <timograham@…>, 6 years ago

In 4bccfac3:

[2.0.x] Fixed #29428 -- Fixed admin changelist crash when using a query expression without asc()/desc() in the ordering.

Backport of 0d8e3e608ee9aab5076d497664aa97e0a29e523e from master

comment:14 by Tim Graham <timograham@…>, 6 years ago

In 83986af9:

[2.0.x] Refs #29428 -- Fixed admin check crash when using a query expression in ModelAdmin.ordering.

Backport of ec2c9c353113bb1db6e32ed3f0b6c28bc06ca2eb from master

Note: See TracTickets for help on using tickets.
Back to Top