Opened 9 years ago

Closed 9 years ago

Last modified 9 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:


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@…> 9 years ago.

Download all attachments as: .zip

Change History (5)

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

Attachment: django_admin_manager.diff added

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

Has patch: set
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 9 years ago by Brian Rosner

Resolution: invalid
Status: newclosed

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 9 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 9 years ago by Brian Rosner

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.

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