Opened 14 years ago

Closed 14 years ago

Last modified 13 years ago

#7582 closed (fixed)

Sorting by nullable ForeignKey

Reported by: alerades@… Owned by: nobody
Component: contrib.admin Version: dev
Severity: Keywords:
Cc: joel@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


In the admin site (using latest newforms-admin), it is not possible to sort a table by a ForeignKey with null=True.
See this little example:

class Group(models.Model):
    name = models.CharField(max_length=200, primary_key=True)

class Person(models.Model):
    name = models.CharField(max_length=200, primary_key=True)
    group = models.ForeignKey(Group, blank=True, null=True)

You cannot sort Person by group here.
See also

Patch By Karen Tracey <kmtracey@…>

Attachments (1)

nullorder.diff (1.4 KB) - added by Alex Rades 14 years ago.

Download all attachments as: .zip

Change History (10)

Changed 14 years ago by Alex Rades

Attachment: nullorder.diff added

comment:1 Changed 14 years ago by Joel Watts

Triage Stage: UnreviewedAccepted

If I understand correctly, the issue here is that django.contrib.admin isn't making the column re-sortable when a nullable ForeignKey appears in the list_display of a ModelAdmin class.

I've tested the patch and it fixes the above issue by removing the code that explicitly prevents the behavior. It looks to me like that code shouldn't have survived queryset-refactor anyway.

I did notice that the patch as it stands doesn't apply to the top-level directory, so I'll be attaching a version that does, but is otherwise unmodified.

comment:2 Changed 14 years ago by Karen Tracey <kmtracey@…>

Yes, the issue is whether the Admin puts a link in the column header that allows you to sort by the associated field. As mentioned in the list discussion, the restriction has been there since initial import of the code into the current repository. I assume the Admin didn't allow it because the ORM code didn't used to support it. Now it does, I think the Admin restriction can be removed as well. Also mentioned at the tail end of that list discussion is that different DBs may put null values in different places (either first or last), so behavior may not be entirely consistent from DB to DB, but I don't see that as a showstopper for allowing sorting by these fields.

comment:3 Changed 14 years ago by Alex Gaynor

milestone: 1.0 alpha1.0

This is neither backwards incompatible, nor a feature regression, therefore not needed for the nfa merge(and therefore not 1a).

comment:4 Changed 14 years ago by Joel Watts

Nevermind on the new patch; I think the problem was on my end (using git apply instead of patch).

Also, there doesn't seem to be anything in the admin docs indicating that you can't re-sort on nullable ForeignKey fields, so it doesn't look like any changes are required there. In fact, I would argue that if the behavior isn't going to be allowed, that should be documented.

comment:5 Changed 14 years ago by Joel Watts

Cc: joel@… added

comment:6 Changed 14 years ago by James Bennett

milestone: 1.0post-1.0
Version: newforms-adminSVN

This is sort of a minor feature request, and as such could easily be bundled with a 1.x release later on. Plus, the last patch is almost two months old, which means it almost certainly won't work on current trunk.

comment:7 Changed 14 years ago by Karen Tracey

Myself I'd call it removing remnants of an old bug (prohibiting something that wasn't handled correctly before queryset-refactor), not an enhancement. As jpwatts mentions, there's no documentation explaining why these fields are not made sortable in the admin. Thus the current behavior is confusing to the user, since there is no obvious reason for these fields to be treated differently than all the others that are automatically sortable. (Which is why it came up on the user's list in the first place.)

So I'd argue it should be fixed for 1.0, or documented. Fixing it is easier: there's the patch, which does in fact still apply correctly as of [8472].

comment:8 Changed 14 years ago by Karen Tracey

Resolution: fixed
Status: newclosed

(In [9080]) Fixed #7582: Removed checks that prevented null=True ForeignKey fields from being sortable in Admin. Post queryset-refactor there seems no reason to disallow this.

comment:9 Changed 13 years ago by (none)

milestone: post-1.0

Milestone post-1.0 deleted

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