Do not mask AttributeErrors from CBV get_object

dfunckt · Jul 21, 2018 · 24e04ed · 24e04ed
1 parent c560b96
commit 24e04ed
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 3 deletions.
diff --git a/rules/contrib/views.py b/rules/contrib/views.py
@@ -39,10 +39,10 @@ def get_permission_object(self):
         ``SingleObjectMixin``. Returns None if there's no ``get_object``
         method.
         """
-        try:
+        if hasattr(self, 'get_object') and callable(self.get_object):
             # Requires SingleObjectMixin or equivalent ``get_object`` method
             return self.get_object()
-        except AttributeError:  # pragma: no cover
+        else:  # pragma: no cover
             return None
 
     def has_permission(self):

diff --git a/tests/testapp/urls.py b/tests/testapp/urls.py
@@ -3,7 +3,8 @@
 
 from .views import (change_book, delete_book,
     view_that_raises, view_with_object, view_with_permission_list,
-    BookUpdateView, BookDeleteView, ViewThatRaises, ViewWithPermissionList)
+    BookUpdateView, BookDeleteView, ViewThatRaises, ViewWithPermissionList,
+    BookUpdateErrorView)
 
 admin.autodiscover()
 
@@ -22,4 +23,6 @@
     url(r'^cbv/(?P<book_id>\d+)/delete/$', BookDeleteView.as_view(), name='cbv.delete_book'),
     url(r'^cbv/(?P<book_id>\d+)/raise/$', ViewThatRaises.as_view(), name='cbv.view_that_raises'),
     url(r'^cbv/(?P<book_id>\d+)/list/$', ViewWithPermissionList.as_view(), name='cbv.view_with_permission_list'),
+
+    url(r'^cbv/(?P<book_id>\d+)/change-error/$', BookUpdateErrorView.as_view(), name='cbv.change_book_error'),
 ]
diff --git a/tests/testapp/views.py b/tests/testapp/views.py
@@ -14,6 +14,11 @@ def get_object(self):
         return Book.objects.get(pk=self.kwargs['book_id'])
 
 
+class BookMixinWithError(object):
+    def get_object(self):
+        raise AttributeError('get_object')
+
+
 @permission_required('testapp.change_book', fn=objectgetter(Book, 'book_id'))
 def change_book(request, book_id):
     return HttpResponse('OK')
@@ -25,6 +30,12 @@ class BookUpdateView(LoginRequiredMixin, PermissionRequiredMixin, BookMixin, Upd
     permission_required = 'testapp.change_book'
 
 
+class BookUpdateErrorView(LoginRequiredMixin, PermissionRequiredMixin, BookMixinWithError, UpdateView):
+    fields = ['title']
+    template_name = 'empty.html'
+    permission_required = 'testapp.change_book'
+
+
 @permission_required('testapp.delete_book', fn=objectgetter(Book, 'book_id'))
 def delete_book(request, book_id):
     return HttpResponse('OK')

diff --git a/tests/testsuite/contrib/test_views.py b/tests/testsuite/contrib/test_views.py
@@ -93,6 +93,11 @@ def test_permission_required(self):
 
 
 class CBVMixinTests(TestCase):
+    def test_get_object_error(self):
+        self.assertTrue(self.client.login(username='adrian', password='secr3t'))
+        with self.assertRaises(AttributeError):
+            self.client.get(reverse('cbv.change_book_error', args=(1,)))
+
     def test_permission_required_mixin(self):
         # Adrian can change his book
         self.assertTrue(self.client.login(username='adrian', password='secr3t'))