Code

Opened 6 years ago

Closed 3 years ago

#7309 closed Bug (fixed)

NFA: Don't override order_by if no default ordering is specified

Reported by: lukas@… Owned by: nobody
Component: contrib.admin Version: master
Severity: Normal Keywords: order_by nfa-someday
Cc: simon@…, zerok@… Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

newforms-admin has this incredibly useful feature where it allowas you to override the modelAdmin's queryset method like this:

    def queryset(self, request):
        return Chain.objects.all().order_by('treeid', 'level')

And there, you already see the problem: Newforms-admin always changes the order_by clause, even if nothing else was specified in model.Meta or in it's modelAdmin class, or via a GET parameter. This makes it impossible to have the queryset from the aformentioned method presorted.

The reason I need it is quite simple, I need to sort with two or more fields, and nfa only support sorting by one single field :/

So far, I have no idea how to overcome this.

Attachments (0)

Change History (11)

comment:1 Changed 6 years ago by Karen Tracey <kmtracey@…>

  • Keywords nfa-someday added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 6 years ago by anonymous

  • Cc simon@… added

comment:3 Changed 6 years ago by Horst Gutmann <zerok@…>

  • Cc zerok@… added

Is this even valid anymore? From what I can see in the ModelAdmin class, ordering is now handled using the ordering class-element, which can be any list/tuple (I think this happened somewhere around #7484):

{{

def queryset(self, request):

"""
Returns a QuerySet of all model instances that can be edited by the
admin site. This is used by changelist_view.
"""
qs = self.model._default_manager.get_query_set()
# TODO: this should be handled by some parameter to the ChangeList.
ordering = self.ordering or () # otherwise we might try to *None, which is bad ;)
if ordering:

qs = qs.order_by(*ordering)

return qs

}}

comment:4 Changed 6 years ago by lukas@…

I just tried it again with newforms-admin as of commit 7875, but it still doesn't do the ordering properly in the admin interface.

comment:5 Changed 6 years ago by ericholscher

  • milestone set to 1.0
  • Triage Stage changed from Unreviewed to Design decision needed

comment:6 Changed 6 years ago by ubernostrum

  • milestone changed from 1.0 to post-1.0
  • Version changed from newforms-admin to SVN

Since you can override queryset() to get initial multi-field ordering, I'm punting this to at least post-1.0.

comment:7 Changed 6 years ago by erik

The problem seems to be that in ChangeList.get_query_set an ordering is added to the queryset, ignoring an already applied one. This just clears the previous ordering.

class QuerySet(object):
    ...
    def order_by(self, *field_names):
        """
        Returns a new QuerySet instance with the ordering changed.
        """
        assert self.query.can_filter(), \
                "Cannot reorder a query once a slice has been taken."
        obj = self._clone()
        obj.query.clear_ordering()
        obj.query.add_ordering(*field_names)
        return obj

comment:8 Changed 5 years ago by anonymous

  • milestone post-1.0 deleted

Milestone post-1.0 deleted

comment:9 Changed 3 years ago by julien

#10212 is a dupe and has a patch.

comment:10 Changed 3 years ago by lukeplant

  • Severity set to Normal
  • Type set to Bug

comment:11 Changed 3 years ago by lukeplant

  • Resolution set to fixed
  • Status changed from new to closed

In [16316]:

Fixed #11868 - Multiple sort in admin changelist.

Many thanks to bendavis78 for the initial patch, and for input from others.

Also fixed #7309. If people were relying on the undocumented default ordering
applied by the admin before, they will need to add 'ordering = -pk?' to
their ModelAdmin.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.