Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Ticket/16418 #52

Closed
wants to merge 3 commits into from

2 participants

@dstufft
Collaborator

Solves #16418 by explicitly checking for subclasses not assuming anything with a _meta or model attribute is a Model or QuerySet subclass.

@akaariai akaariai commented on the diff
django/views/generic/detail.py
((6 lines not shown))
names.append("%s/%s%s.html" % (
self.object._meta.app_label,
self.object._meta.object_name.lower(),
self.template_name_suffix
))
- elif hasattr(self, 'model') and hasattr(self.model, '_meta'):
+ elif hasattr(self, 'model') and self.model is not None and issubclass(self.model, models.Model):
@akaariai Collaborator

why the is not None and issubclass check here instead of the normal isinstance?

@dstufft Collaborator
dstufft added a note

These views allow passing either a model or a queryset. self.model is not an instance of a model but the actual model class and thus isinstance won't work. The None check is that if self.queryset is used instead of self.model, self.model will be None and the first arg to issubclass must be a class, not None.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@akaariai
Collaborator

Pulled in manually in commit 484fcd3. I removed the checks for isinstance(object_list, QuerySet) and retained the hasattr(object_list, 'model') instead because it seems lists containing attribute model are rare enough to not cause problems.

Thanks to dstufft for the patch! Unfortunately I forgot to thank him in the commit message, sorry for that.

@akaariai akaariai closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
7 django/views/generic/detail.py
@@ -1,4 +1,5 @@
from django.core.exceptions import ImproperlyConfigured, ObjectDoesNotExist
+from django.db import models
from django.http import Http404
from django.utils.encoding import smart_str
from django.utils.translation import ugettext as _
@@ -80,7 +81,7 @@ def get_context_object_name(self, obj):
"""
if self.context_object_name:
return self.context_object_name
- elif hasattr(obj, '_meta'):
+ elif isinstance(obj, models.Model):
return smart_str(obj._meta.object_name.lower())
else:
return None
@@ -127,13 +128,13 @@ def get_template_names(self):
# The least-specific option is the default <app>/<model>_detail.html;
# only use this if the object in question is a model.
- if hasattr(self.object, '_meta'):
+ if isinstance(self.object, models.Model):
names.append("%s/%s%s.html" % (
self.object._meta.app_label,
self.object._meta.object_name.lower(),
self.template_name_suffix
))
- elif hasattr(self, 'model') and hasattr(self.model, '_meta'):
+ elif hasattr(self, 'model') and self.model is not None and issubclass(self.model, models.Model):
@akaariai Collaborator

why the is not None and issubclass check here instead of the normal isinstance?

@dstufft Collaborator
dstufft added a note

These views allow passing either a model or a queryset. self.model is not an instance of a model but the actual model class and thus isinstance won't work. The None check is that if self.queryset is used instead of self.model, self.model will be None and the first arg to issubclass must be a class, not None.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
names.append("%s/%s%s.html" % (
self.model._meta.app_label,
self.model._meta.object_name.lower(),
View
5 django/views/generic/list.py
@@ -1,5 +1,6 @@
from django.core.paginator import Paginator, InvalidPage
from django.core.exceptions import ImproperlyConfigured
+from django.db.models.query import QuerySet
from django.http import Http404
from django.utils.encoding import smart_str
from django.utils.translation import ugettext as _
@@ -76,7 +77,7 @@ def get_context_object_name(self, object_list):
"""
if self.context_object_name:
return self.context_object_name
- elif hasattr(object_list, 'model'):
+ elif isinstance(object_list, QuerySet):
return smart_str('%s_list' % object_list.model._meta.object_name.lower())
else:
return None
@@ -139,7 +140,7 @@ def get_template_names(self):
# app and model name. This name gets put at the end of the template
# name list so that user-supplied names override the automatically-
# generated ones.
- if hasattr(self.object_list, 'model'):
+ if isinstance(self.object_list, QuerySet):
opts = self.object_list.model._meta
names.append("%s/%s%s.html" % (opts.app_label, opts.object_name.lower(), self.template_name_suffix))
View
5 tests/regressiontests/generic_views/detail.py
@@ -92,3 +92,8 @@ def test_invalid_url(self):
def test_invalid_queryset(self):
self.assertRaises(ImproperlyConfigured, self.client.get, '/detail/author/invalid/qs/')
+
+ def test_non_model_object_with_meta(self):
+ res = self.client.get('/detail/nonmodel/1/')
+ self.assertEqual(res.status_code, 200)
+ self.assertEqual(res.context['object'].id, "non_model_1")
View
3  tests/regressiontests/generic_views/list.py
@@ -164,3 +164,6 @@ def _make_authors(self, n):
for i in range(n):
Author.objects.create(name='Author %02i' % i, slug='a%s' % i)
+ def test_non_queryset_list(self):
+ res = self.client.get('/list/nonqueryset/')
+ self.assertEqual(res.status_code, 200)
View
5 tests/regressiontests/generic_views/urls.py
@@ -52,6 +52,8 @@
views.AuthorDetail.as_view()),
(r'^detail/author/invalid/qs/$',
views.AuthorDetail.as_view(queryset=None)),
+ (r'^detail/nonmodel/1/$',
+ views.NonModelDetail.as_view()),
# Create/UpdateView
(r'^edit/artists/create/$',
@@ -142,6 +144,9 @@
views.AuthorList.as_view(paginate_by=5, paginator_class=views.CustomPaginator)),
(r'^list/authors/paginated/custom_constructor/$',
views.AuthorListCustomPaginator.as_view()),
+ (r'^list/nonqueryset/$',
+ views.NonQuerySetList.as_view()),
+
# YearArchiveView
# Mixing keyword and possitional captures below is intentional; the views
View
30 tests/regressiontests/generic_views/views.py
@@ -226,3 +226,33 @@ class BookSigningTodayArchive(BookSigningConfig, generic.TodayArchiveView):
class BookSigningDetail(BookSigningConfig, generic.DateDetailView):
context_object_name = 'book'
+
+
+class NonModel(object):
+ id = "non_model_1"
+
+ _meta = None
+
+
+class NonQuerySet(object):
+ model = NonModel
+
+ def __getitem__(self, i):
+ if i == 0:
+ return self.model()
+ raise IndexError
+
+
+class NonModelDetail(generic.DetailView):
+
+ template_name = 'generic_views/detail.html'
+ model = NonModel
+
+ def get_object(self, queryset=None):
+ return NonModel()
+
+
+class NonQuerySetList(generic.ListView):
+
+ template_name = 'generic_views/list.html'
+ queryset = NonQuerySet()
Something went wrong with that request. Please try again.