Opened 16 years ago
Closed 9 years ago
#10532 closed Bug (fixed)
An overridden get method on a custom manager passed to get_object_or_404 will not be called.
Reported by: | anonymous | Owned by: | |
---|---|---|---|
Component: | Database layer (models, ORM) | Version: | dev |
Severity: | Normal | Keywords: | |
Cc: | django@…, anubhav9042@… | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
If a manager is passed in as the first argument to get_object_or_404, _get_queryset will convert it into a queryset via manager.all().
This means that an overridden get method on the manager passed to get_object_or_404 will not be called. This is rather unintuitive.
# In django.shortcuts __init__.py def _get_queryset(klass): """ Returns a QuerySet from a Model, Manager, or QuerySet. Created to make get_object_or_404 and get_list_or_404 more DRY. """ if isinstance(klass, QuerySet): return klass elif isinstance(klass, Manager): manager = klass else: manager = klass._default_manager return manager.all() def get_object_or_404(klass, *args, **kwargs): #... queryset = _get_queryset(klass) try: return queryset.get(*args, **kwargs) #...
Perhaps it should just be return manager instead of manager.all().
Change History (13)
comment:1 by , 16 years ago
Component: | Uncategorized → Database layer (models, ORM) |
---|---|
Triage Stage: | Unreviewed → Design decision needed |
comment:2 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → Bug |
comment:3 by , 14 years ago
comment:4 by , 14 years ago
If there were any updates on the bug, you'd see them here.
Since it sounds like you're already working with code to work around this issue, perhaps you'd care to produce a patch and test case? That's the kind of thing that gets things moving.
At the very least, start a discussion on the django-dev mailing list (which is a good idea anyway, since this is sitting in the design decision stage).
comment:7 by , 12 years ago
Triage Stage: | Design decision needed → Accepted |
---|
Sounds like a good change to me. I'd go a bit further: if klass has .get() method then use that. So:
# If it is a model class, or anything else with ._default_manager if hasattr(klass, '_default_manager'): return klass._default_manager.all() return klass # If it is a manager or queryset, then this is OK. Even if it isn't then lets hope it has .get().
This would allow for true duct-typing, anything with .get() is OK, but Model subclasses which have .get() defined for other purposes will still continue to work.
The only question here is if this leads to confusing errors - having a method named .get() is somewhat likely. But I think the risk is worth it.
comment:8 by , 11 years ago
Cc: | added |
---|
comment:9 by , 11 years ago
Cc: | added |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:10 by , 11 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:12 by , 9 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
Any updates on this bug? So far the only resolution I've found is to use my own get_object_or_404() function.