Code

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#6358 closed (invalid)

Admin ChangeList bypasses model's custom manager

Reported by: Ionut Ciocirlan <ionut.ciocirlan@…> Owned by: nobody
Component: contrib.admin Version: master
Severity: Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: UI/UX:

Description

When using a custom manager for Model.objects that overrides Manager.get_query_set(), the admin ChangeList disregards it. E.g.:

from django.db import models

class MyManager(models.Manager):
    def get_query_set(self):
        return super(MyManager, self).get_query_set().filter(mybool=False)

class MyModel(models.Model):
    mybool = models.BooleanField()
    objects = MyManager()

To reproduce it, add a MyModel object with mybool set to true. The item will appear in the list. While this might be (quite debatably) a good approach, it is inconsistent as trying to edit the object in admin will lead to a 404: object with primary key u'1' does not exist.

Attachments (1)

django_admin_manager.diff (501 bytes) - added by Ionut Ciocirlan <ionut.ciocirlan@…> 6 years ago.

Download all attachments as: .zip

Change History (5)

Changed 6 years ago by Ionut Ciocirlan <ionut.ciocirlan@…>

comment:1 Changed 6 years ago by Ionut Ciocirlan <ionut.ciocirlan@…>

  • Has patch set
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement set

Created the patch above, which fixes the behaviour.

I checked patch needs improvement, as I didn't read the code to see what model._meta.admin.manager actually does and why it was used here.

However, this does work as expected, regardless of the model.objects having been overwritten, and list filters and pagination are preserved.

comment:2 Changed 6 years ago by brosner

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

model._meta.admin.manager is an undocumented feature of the admin in trunk that is meant for exactly what you are trying to do. Set it up in your model's Admin inner-class. This has been fixed in newforms-admin therefore marking as invalid.

comment:3 Changed 6 years ago by Ionut Ciocirlan <ionut.ciocirlan@…>

If so, then the bug is not in change_list, but in change_stage as it uses the model's manager.

And while I agree that strictly speaking the bug is invalid, and I should add a new bug, having been fixed in a branch is a poor explanation for it. The bug was submitted against trunk. Chances are that the "new" bug exists in newforms-admin as well.

comment:4 Changed 6 years ago by brosner

There is no bug here. The real problem was that the manager option was never documented. Had it been, this would have never been a problem. The current admin application in trunk is no longer supported and will be replaced by newforms-admin. Even if this were a bug then it would have been triaged towards newforms-admin. This problem does not exist because you can define the queryset to use for the ChangeList making this obsolete.

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.