Opened 4 weeks ago

Closed 6 days ago

Last modified 6 days 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: felixxm, 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 Changed 4 weeks ago by Claude Paroz

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

I was able to reproduce.

comment:2 Changed 4 weeks ago by Nicolas Noé

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

comment:3 Changed 4 weeks ago by Nicolas Noé

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 4 weeks ago by Tim Graham (previous) (diff)

comment:4 Changed 3 weeks ago by felixxm

Cc: felixxm added

comment:5 Changed 2 weeks ago by Carlton Gibson

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 2 weeks ago by Carlton Gibson (previous) (diff)

comment:6 Changed 2 weeks ago by Nicolas Noé

Owner: Nicolas Noé deleted
Status: assignednew

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

comment:7 Changed 7 days ago by Tim Graham

Has patch: set

comment:8 Changed 7 days ago by Carlton Gibson

Triage Stage: AcceptedReady for checkin

comment:9 Changed 6 days ago by Tim Graham <timograham@…>

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 Changed 6 days ago by Tim Graham <timograham@…>

In ec2c9c35:

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

comment:11 Changed 6 days ago by Tim Graham <timograham@…>

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 Changed 6 days ago by Tim Graham <timograham@…>

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 Changed 6 days ago by Tim Graham <timograham@…>

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 Changed 6 days ago by Tim Graham <timograham@…>

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