Opened 10 years ago

Closed 10 years ago

Last modified 10 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@…> 10 years ago.

Download all attachments as: .zip

Change History (5)

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

Attachment: django_admin_manager.diff added

comment:1 Changed 10 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 10 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 10 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 10 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