Opened 3 months ago

Closed 2 months ago

Last modified 2 months 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 3 months ago by Claude Paroz

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

I was able to reproduce.

comment:2 Changed 3 months ago by Nicolas Noé

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

comment:3 Changed 3 months 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 3 months ago by Tim Graham (previous) (diff)

comment:4 Changed 3 months ago by felixxm

Cc: felixxm added

comment:5 Changed 2 months 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 months ago by Carlton Gibson (previous) (diff)

comment:6 Changed 2 months 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 2 months ago by Tim Graham

Has patch: set

comment:8 Changed 2 months ago by Carlton Gibson

Triage Stage: AcceptedReady for checkin

comment:9 Changed 2 months 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 2 months ago by Tim Graham <timograham@…>

In ec2c9c35:

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

comment:11 Changed 2 months 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 2 months 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 2 months 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 2 months 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