Interrogate (#286)

Co-authored-by: Melanie Crutchfield <[email protected]>

Author: kennethlove

.github/workflows/ci.yml

@@ -34,3 +34,7 @@ jobs:
           flags: unittests
           name: codecov-umbrella
           verbose: true
+      - name: Python Interrogate Check
+        uses: JackMcKew/python-interrogate-check@main
+        with:
+          path: 'braces'

braces/views/_access.py

@@ -58,6 +58,7 @@ def get_redirect_field_name(self):
         return self.redirect_field_name
 
     def handle_no_permission(self, request):
+        """What should happen if the user doesn't have permission?"""
         if self.raise_exception:
             if (
                 self.redirect_unauthenticated_users
@@ -102,6 +103,7 @@ class LoginRequiredMixin(AccessMixin):
     """
 
     def dispatch(self, request, *args, **kwargs):
+        """Call the appropriate method after checking authentication"""
         if not request.user.is_authenticated:
             return self.handle_no_permission(request)
 
@@ -127,6 +129,7 @@ class SomeView(AnonymousRequiredMixin, ListView):
     authenticated_redirect_url = settings.LOGIN_REDIRECT_URL
 
     def dispatch(self, request, *args, **kwargs):
+        """Call the appropriate handler after guaranteeing anonymity"""
         if request.user.is_authenticated:
             return HttpResponseRedirect(self.get_authenticated_redirect_url())
         return super().dispatch(request, *args, **kwargs)
@@ -263,10 +266,12 @@ class SomeView(MultiplePermissionsRequiredMixin, ListView):
     permissions = None  # Default required perms to none
 
     def get_permission_required(self, request=None):
+        """Get which permission is required"""
         self._check_permissions_attr()
         return self.permissions
 
     def check_permissions(self, request):
+        """Get the permissions, both all and any."""
         permissions = self.get_permission_required(request)
         perms_all = permissions.get("all")
         perms_any = permissions.get("any")
@@ -327,6 +332,7 @@ class GroupRequiredMixin(AccessMixin):
     group_required = None
 
     def get_group_required(self):
+        """Get which group's membership is required"""
         if any([
             self.group_required is None,
             not isinstance(self.group_required, (list, tuple, str))
@@ -350,6 +356,7 @@ def check_membership(self, groups):
         return set(groups).intersection(set(user_groups))
 
     def dispatch(self, request, *args, **kwargs):
+        """Call the appropriate handler if the user is a group member"""
         self.request = request
         in_group = False
         if request.user.is_authenticated:
@@ -374,15 +381,18 @@ class UserPassesTestMixin(AccessMixin):
     """
 
     def test_func(self, user):
+        """The function to test the user with"""
         raise NotImplementedError(
             f"{self._class_name} is missing implementation of the "
             "`test_func` method. A function to test the user is required."
         )
 
     def get_test_func(self):
+        """Get the test function"""
         return getattr(self, "test_func")
 
     def dispatch(self, request, *args, **kwargs):
+        """Call the appropriate handler if the users passes the test"""
         user_test_result = self.get_test_func()(request.user)
 
         if not user_test_result:
@@ -397,6 +407,7 @@ class SuperuserRequiredMixin(AccessMixin):
     """
 
     def dispatch(self, request, *args, **kwargs):
+        """Call the appropriate handler if the user is a superuser"""
         if not request.user.is_superuser:
             return self.handle_no_permission(request)
 
@@ -409,6 +420,7 @@ class StaffuserRequiredMixin(AccessMixin):
     """
 
     def dispatch(self, request, *args, **kwargs):
+        """Call the appropriate handler if the user is a staff member"""
         if not request.user.is_staff:
             return self.handle_no_permission(request)
 
@@ -423,6 +435,7 @@ class SSLRequiredMixin:
     raise_exception = False
 
     def dispatch(self, request, *args, **kwargs):
+        """Call the appropriate handler if the connection is secure"""
         if getattr(settings, "DEBUG", False):
             # Don't enforce the check during development
             return super().dispatch(request, *args, **kwargs)
@@ -453,6 +466,7 @@ class RecentLoginRequiredMixin(LoginRequiredMixin):
     max_last_login_delta = 1800  # Defaults to 30 minutes
 
     def dispatch(self, request, *args, **kwargs):
+        """Call the appropriate method if the user's login is recent"""
         resp = super().dispatch(request, *args, **kwargs)
 
         if resp.status_code == 200:

braces/views/_ajax.py

@@ -18,6 +18,7 @@ class JSONResponseMixin:
     json_encoder_class = None
 
     def get_content_type(self):
+        """Get the appropriate content type for the response"""
         if self.content_type is not None and not isinstance(
             self.content_type, str
         ):
@@ -29,11 +30,13 @@ def get_content_type(self):
         return self.content_type or "application/json"
 
     def get_json_dumps_kwargs(self):
+        """Get kwargs for custom JSON compilation"""
         dumps_kwargs = getattr(self, "json_dumps_kwargs", None) or {}
         dumps_kwargs.setdefault("ensure_ascii", False)
         return dumps_kwargs
 
     def get_json_encoder_class(self):
+        """Get the encoder class to use"""
         if self.json_encoder_class is None:
             self.json_encoder_class = DjangoJSONEncoder
         return self.json_encoder_class
@@ -74,6 +77,7 @@ class AjaxResponseMixin:
     """
 
     def dispatch(self, request, *args, **kwargs):
+        """Call the appropriate handler method"""
         if all([
             request.headers.get("x-requested-with") == "XMLHttpRequest",
             request.method.lower() in self.http_method_names
@@ -91,15 +95,19 @@ def dispatch(self, request, *args, **kwargs):
         return super().dispatch(request, *args, **kwargs)
 
     def get_ajax(self, request, *args, **kwargs):
+        """Handle a GET request made with AJAX"""
         return self.get(request, *args, **kwargs)
 
     def post_ajax(self, request, *args, **kwargs):
+        """Handle a POST request made with AJAX"""
         return self.post(request, *args, **kwargs)
 
     def put_ajax(self, request, *args, **kwargs):
+        """Handle a PUT request made with AJAX"""
         return self.get(request, *args, **kwargs)
 
     def delete_ajax(self, request, *args, **kwargs):
+        """Handle a DELETE request made with AJAX"""
         return self.get(request, *args, **kwargs)
 
 
@@ -129,6 +137,7 @@ def post(self, request, *args, **kwargs):
     error_response_dict = {"errors": ["Improperly formatted request"]}
 
     def render_bad_request_response(self, error_dict=None):
+        """Generate errors for bad content"""
         if error_dict is None:
             error_dict = self.error_response_dict
         json_context = json.dumps(
@@ -141,12 +150,14 @@ def render_bad_request_response(self, error_dict=None):
         )
 
     def get_request_json(self):
+        """Get the JSON included in the body"""
         try:
             return json.loads(self.request.body.decode("utf-8"))
         except (json.JSONDecodeError, ValueError):
             return None
 
     def dispatch(self, request, *args, **kwargs):
+        """Trigger the appropriate method"""
         self.request = request
         self.args = args
         self.kwargs = kwargs
@@ -164,4 +175,4 @@ def dispatch(self, request, *args, **kwargs):
 
 
 class JSONRequestResponseMixin(JsonRequestResponseMixin):
-    pass
+    """Convenience alias"""

braces/views/_other.py

@@ -14,11 +14,13 @@ class SetHeadlineMixin:
     headline = None  # Default the headline to none
 
     def get_context_data(self, **kwargs):
+        """Add the headline to the context"""
         kwargs = super().get_context_data(**kwargs)
         kwargs.update({"headline": self.get_headline()})
         return kwargs
 
     def get_headline(self):
+        """Fetch the headline from the instance"""
         if self.headline is None:
             class_name = self.__class__.__name__
             raise ImproperlyConfigured(
@@ -36,6 +38,7 @@ class StaticContextMixin:
     static_context = None
 
     def get_context_data(self, **kwargs):
+        """Update the context to include the static content"""
         kwargs = super().get_context_data(**kwargs)
 
         try:
@@ -49,6 +52,7 @@ def get_context_data(self, **kwargs):
             return kwargs
 
     def get_static_context(self):
+        """Fetch the static content from the view"""
         if self.static_context is None:
             class_name = self.__class__.__name__
             raise ImproperlyConfigured(
@@ -68,28 +72,29 @@ class CanonicalSlugDetailMixin:
     """
 
     def dispatch(self, request, *args, **kwargs):
-        # Set up since we need to super() later instead of earlier.
+        """
+        Redirect to the appropriate URL if necessary.
+        Otherwise, trigger HTTP-method-appropriate handler.
+        """
         self.request = request
         self.args = args
         self.kwargs = kwargs
 
-        # Get the current object, url slug, and
-        # urlpattern name (namespace aware).
+        # Get the current object, url slug, and url name.
         obj = self.get_object()
         slug = self.kwargs.get(self.slug_url_kwarg, None)
         match = resolve(request.path_info)
         url_parts = match.namespaces
         url_parts.append(match.url_name)
         current_urlpattern = ":".join(url_parts)
 
-        # Figure out what the slug is supposed to be.
+        # Find the canonical slug for the object
         if hasattr(obj, "get_canonical_slug"):
             canonical_slug = obj.get_canonical_slug()
         else:
             canonical_slug = self.get_canonical_slug()
 
-        # If there's a discrepancy between the slug in the url and the
-        # canonical slug, redirect to the canonical slug.
+        # Redirect if current slug is not the canonical one
         if canonical_slug != slug:
             params = {
                 self.pk_url_kwarg: obj.pk,
@@ -102,17 +107,14 @@ def dispatch(self, request, *args, **kwargs):
 
     def get_canonical_slug(self):
         """
-        Override this method to customize what slug should be considered
-        canonical.
-
-        Alternatively, define the get_canonical_slug method on this view's
-        object class. In that case, this method will never be called.
+        Provide a method to return the correct slug for this object.
         """
         return self.get_object().slug
 
 
 class AllVerbsMixin:
-    """Call a single method for all HTTP verbs.
+    """
+    Call a single method for all HTTP verbs.
 
     The name of the method should be specified using the class attribute
     `all_handler`. The default value of this attribute is 'all'.
@@ -121,6 +123,7 @@ class AllVerbsMixin:
     all_handler = "all"
 
     def dispatch(self, request, *args, **kwargs):
+        """Call the all handler"""
         if not self.all_handler:
             raise ImproperlyConfigured(
                 f"{self.__class__.__name__} requires the all_handler attribute to be set."
@@ -178,6 +181,7 @@ class CacheControlMixin:
 
     @classmethod
     def get_cachecontrol_options(cls):
+        """Compile a dictionary of selected cache options"""
         opts = (
             'public', 'private', 'no_cache', 'no_transform',
             'must_revalidate', 'proxy_revalidate', 'max_age',
@@ -192,6 +196,7 @@ def get_cachecontrol_options(cls):
 
     @classmethod
     def as_view(cls, *args, **kwargs):
+        """Wrap the view with appropriate cache controls"""
         view_func = super().as_view(*args, **kwargs)
         options = cls.get_cachecontrol_options()
         return cache_control(**options)(view_func)
@@ -204,5 +209,8 @@ class NeverCacheMixin:
     """
     @classmethod
     def as_view(cls, *args, **kwargs):
+        """
+        Wrap the view with the `never_cache` decorator.
+        """
         view_func = super().as_view(*args, **kwargs)
         return never_cache(view_func)

braces/views/_queries.py

@@ -11,6 +11,7 @@ class SelectRelatedMixin:
     select_related = None  # Default related fields to none
 
     def get_queryset(self):
+        """Apply select_related, with appropriate fields, to the queryset"""
         if self.select_related is None:
             # If no fields were provided, raise a configuration error
             raise ImproperlyConfigured(
@@ -43,6 +44,7 @@ class PrefetchRelatedMixin:
     prefetch_related = None  # Default prefetch fields to none
 
     def get_queryset(self):
+        """Apply prefetch_related, with appropriate fields, to the queryset"""
         if self.prefetch_related is None:
             # If no fields were provided, raise a configuration error
             raise ImproperlyConfigured(
@@ -90,20 +92,23 @@ def get_context_data(self, **kwargs):
         return context
 
     def get_orderable_columns(self):
+        """Check that the orderable columns are set and return them"""
         if not self.orderable_columns:
             raise ImproperlyConfigured(
                 f"{self.__class__.__name__} needs the ordering columns defined."
             )
         return self.orderable_columns
 
     def get_orderable_columns_default(self):
+        """Which column(s) should be sorted by, by default?"""
         if not self.orderable_columns_default:
             raise ImproperlyConfigured(
                 f"{self.__class__.__name__} needs the default ordering column defined."
             )
         return self.orderable_columns_default
 
     def get_ordering_default(self):
+        """Which direction should things be sorted?"""
         if not self.ordering_default:
             return "asc"
         else:

conftest.py

@@ -4,5 +4,6 @@
 
 
 def pytest_configure():
+    """Setup Django settings"""
     os.environ.setdefault("DJANGO_SETTINGS_MODULE", "tests.settings")
     settings.configure(default_settings=test_settings)

pyproject.toml

@@ -7,3 +7,22 @@ line-length = 79
 
 [tool.pytest.ini_options]
 addopts = "--cov --nomigrations"
+
+[tool.interrogate]
+ignore-init-method = true
+ignore-init-module = false
+ignore-magic = false
+ignore-semiprivate = false
+ignore-private = false
+ignore-property-decorators = false
+ignore-module = true
+ignore-nested-functions = false
+ignore-nested-classes = true
+fail-under = 75
+exclude = ["setup.py", "conftest.py", "docs", "build"]
+ignore-regex = ["^get$", "^mock_.*"]
+# possible values: 0 (minimal output), 1 (-v), 2 (-vv)
+verbose = 1
+quiet = false
+color = true
+omit-covered-files = true

setup.py

@@ -6,6 +6,7 @@
 
 
 def _add_default(m):
+    """Add on a default"""
     attr_name, attr_value = m.groups()
     return ((attr_name, attr_value.strip("\"'")),)
 

tests/factories.py

@@ -18,27 +18,30 @@ def _get_perm(perm_name):
 
 
 class ArticleFactory(factory.django.DjangoModelFactory):
-    title = factory.Sequence(lambda n: "Article number {0}".format(n))
-    body = factory.Sequence(lambda n: "Body of article {0}".format(n))
+    """Generates Articles"""
+    title = factory.Sequence(lambda n: f"Article number {n}")
+    body = factory.Sequence(lambda n: "Body of article {n}")
 
     class Meta:
         model = Article
         abstract = False
 
 
 class GroupFactory(factory.django.DjangoModelFactory):
-    name = factory.Sequence(lambda n: "group{0}".format(n))
+    """Artificial divides as a service"""
+    name = factory.Sequence(lambda n: f"group{n}")
 
     class Meta:
         model = Group
         abstract = False
 
 
 class UserFactory(factory.django.DjangoModelFactory):
-    username = factory.Sequence(lambda n: "user{0}".format(n))
-    first_name = factory.Sequence(lambda n: "John {0}".format(n))
-    last_name = factory.Sequence(lambda n: "Doe {0}".format(n))
-    email = factory.Sequence(lambda n: "user{0}@example.com".format(n))
+    """The people who make it all possible"""
+    username = factory.Sequence(lambda n: f"user{n}")
+    first_name = factory.Sequence(lambda n: f"John {n}")
+    last_name = factory.Sequence(lambda n: f"Doe {n}")
+    email = factory.Sequence(lambda n: f"user{n}@example.com")
     password = factory.PostGenerationMethodCall("set_password", "asdf1234")
 
     class Meta:
@@ -47,6 +50,7 @@ class Meta:
 
     @factory.post_generation
     def permissions(self, create, extracted, **kwargs):
+        """Give the user some permissions"""
         if create and extracted:
             # We have a saved object and a list of permission names
             self.user_permissions.add(*[_get_perm(pn) for pn in extracted])

tests/forms.py

@@ -6,10 +6,12 @@
 
 
 class FormWithUserKwarg(UserKwargModelFormMixin, forms.Form):
+    """This form will get a `user` kwarg"""
     field1 = forms.CharField()
 
 
 class ArticleForm(forms.ModelForm):
+    """This form represents an Article"""
     class Meta:
         model = Article
         fields = ["author", "title", "body", "slug"]

tests/helpers.py

@@ -3,7 +3,7 @@
 from django.core.serializers.json import DjangoJSONEncoder
 
 
-class TestViewHelper(object):
+class TestViewHelper:
     """
     Helper class for unit-testing class based views.
     """
@@ -12,7 +12,7 @@ class TestViewHelper(object):
     request_factory_class = test.RequestFactory
 
     def setUp(self):
-        super(TestViewHelper, self).setUp()
+        super().setUp()
         self.factory = self.request_factory_class()
 
     def build_request(self, method="GET", path="/test/", user=None, **kwargs):
@@ -61,6 +61,7 @@ class SetJSONEncoder(DjangoJSONEncoder):
     """
 
     def default(self, obj):
+        """Control default methods of encoding data"""
         if isinstance(obj, set):
             return list(obj)
         return super(DjangoJSONEncoder, self).default(obj)

tests/models.py

@@ -2,6 +2,9 @@
 
 
 class Article(models.Model):
+    """
+    A small but useful model for testing most features
+    """
     author = models.ForeignKey(
         "auth.User", null=True, blank=True, on_delete=models.CASCADE
     )
@@ -11,6 +14,9 @@ class Article(models.Model):
 
 
 class CanonicalArticle(models.Model):
+    """
+    Model specifically for testing the canonical slug mixins
+    """
     author = models.ForeignKey(
         "auth.User", null=True, blank=True, on_delete=models.CASCADE
     )
@@ -19,6 +25,7 @@ class CanonicalArticle(models.Model):
     slug = models.SlugField(blank=True)
 
     def get_canonical_slug(self):
+        """Required by mixin to use the model as the source of truth"""
         if self.author:
-            return "{0.author.username}-{0.slug}".format(self)
-        return "unauthored-{0.slug}".format(self)
+            return f"{self.author.username}-{self.slug}"
+        return f"unauthored-{self.slug}"

tests/test_access_mixins.py

@@ -235,31 +235,33 @@ def test_redirect_unauthenticated_false(self):
 
 @pytest.mark.django_db
 class TestLoginRequiredMixin(TestViewHelper, test.TestCase):
-    """
-    Tests for LoginRequiredMixin.
-    """
+    """Scenarios around requiring an authenticated session"""
 
     view_class = LoginRequiredView
     view_url = "/login_required/"
 
     def test_anonymous(self):
+        """Anonymous users should be redirected"""
         resp = self.client.get(self.view_url)
         self.assertRedirects(resp, "/accounts/login/?next=/login_required/")
 
     def test_anonymous_raises_exception(self):
+        """Anonymous users should raise an exception"""
         with self.assertRaises(PermissionDenied):
             self.dispatch_view(
                 self.build_request(path=self.view_url), raise_exception=True
             )
 
     def test_authenticated(self):
+        """Authenticated users should get 'OK'"""
         user = UserFactory()
         self.client.login(username=user.username, password="asdf1234")
         resp = self.client.get(self.view_url)
         assert resp.status_code == 200
         assert force_str(resp.content) == "OK"
 
     def test_anonymous_redirects(self):
+        """Anonymous users are redirected with a 302"""
         resp = self.dispatch_view(
             self.build_request(path=self.view_url),
             raise_exception=True,
@@ -361,7 +363,7 @@ def test_anonymous(self):
     def test_authenticated(self):
         """
         Check that the authenticated user has been successfully directed
-        to the approparite view.
+        to the appropriate view.
         """
         user = UserFactory()
         self.client.login(username=user.username, password="asdf1234")
@@ -372,13 +374,15 @@ def test_authenticated(self):
         self.assertRedirects(resp, "/authenticated_view/")
 
     def test_no_url(self):
+        """View should raise an exception if no URL is provided"""
         self.view_class.authenticated_redirect_url = None
         user = UserFactory()
         self.client.login(username=user.username, password="asdf1234")
         with self.assertRaises(ImproperlyConfigured):
             self.client.get(self.view_url)
 
     def test_bad_url(self):
+        """Redirection can be misconfigured"""
         self.view_class.authenticated_redirect_url = "/epicfailurl/"
         user = UserFactory()
         self.client.login(username=user.username, password="asdf1234")
@@ -388,17 +392,17 @@ def test_bad_url(self):
 
 @pytest.mark.django_db
 class TestPermissionRequiredMixin(_TestAccessBasicsMixin, test.TestCase):
-    """
-    Tests for PermissionRequiredMixin.
-    """
+    """Scenarios around requiring a permission"""
 
     view_class = PermissionRequiredView
     view_url = "/permission_required/"
 
     def build_authorized_user(self):
+        """Create a user with permissions"""
         return UserFactory(permissions=["auth.add_user"])
 
     def build_unauthorized_user(self):
+        """Create a user without permissions"""
         return UserFactory()
 
     def test_invalid_permission(self):
@@ -414,10 +418,12 @@ def test_invalid_permission(self):
 class TestMultiplePermissionsRequiredMixin(
     _TestAccessBasicsMixin, test.TestCase
 ):
+    """Scenarios around requiring multiple permissions"""
     view_class = MultiplePermissionsRequiredView
     view_url = "/multiple_permissions_required/"
 
     def build_authorized_user(self):
+        """Get a user with permissions"""
         return UserFactory(
             permissions=[
                 "tests.add_article",
@@ -427,6 +433,7 @@ def build_authorized_user(self):
         )
 
     def build_unauthorized_user(self):
+        """Get a user without the important permissions"""
         return UserFactory(permissions=["tests.add_article"])
 
     def test_redirects_to_login(self):
@@ -530,49 +537,61 @@ def test_any_permissions_key(self):
 
 @pytest.mark.django_db
 class TestSuperuserRequiredMixin(_TestAccessBasicsMixin, test.TestCase):
+    """Scenarios requiring a superuser"""
     view_class = SuperuserRequiredView
     view_url = "/superuser_required/"
 
     def build_authorized_user(self):
+        """Make a superuser"""
         return UserFactory(is_superuser=True, is_staff=True)
 
     def build_unauthorized_user(self):
+        """Make a non-superuser"""
         return UserFactory()
 
 
 @pytest.mark.django_db
 class TestStaffuserRequiredMixin(_TestAccessBasicsMixin, test.TestCase):
+    """Scenarios requiring a staff user"""
     view_class = StaffuserRequiredView
     view_url = "/staffuser_required/"
 
     def build_authorized_user(self):
+        """Hire a user"""
         return UserFactory(is_staff=True)
 
     def build_unauthorized_user(self):
+        """Get a customer"""
         return UserFactory()
 
 
 @pytest.mark.django_db
 class TestGroupRequiredMixin(_TestAccessBasicsMixin, test.TestCase):
+    """Scenarios requiring membership in a certain group"""
+
     view_class = GroupRequiredView
     view_url = "/group_required/"
 
     def build_authorized_user(self):
+        """Get a user with the right group"""
         user = UserFactory()
         group = GroupFactory(name="test_group")
         user.groups.add(group)
         return user
 
     def build_superuser(self):
+        """Get a superuser"""
         user = UserFactory()
         user.is_superuser = True
         user.save()
         return user
 
     def build_unauthorized_user(self):
+        """Just a normal users, not super and no groups"""
         return UserFactory()
 
     def test_with_string(self):
+        """A group name as a string should restrict access"""
         self.assertEqual("test_group", self.view_class.group_required)
         user = self.build_authorized_user()
         self.client.login(username=user.username, password="asdf1234")
@@ -581,6 +600,7 @@ def test_with_string(self):
         self.assertEqual("OK", force_str(resp.content))
 
     def test_with_group_list(self):
+        """A list of group names should restrict access"""
         group_list = ["test_group", "editors"]
         # the test client will instantiate a new view on request, so we have to
         # modify the class variable (and restore it when the test finished)
@@ -595,13 +615,15 @@ def test_with_group_list(self):
         self.assertEqual("test_group", self.view_class.group_required)
 
     def test_superuser_allowed(self):
+        """Superusers should always be allowed, regardless of group rules"""
         user = self.build_superuser()
         self.client.login(username=user.username, password="asdf1234")
         resp = self.client.get(self.view_url)
         self.assertEqual(200, resp.status_code)
         self.assertEqual("OK", force_str(resp.content))
 
     def test_improperly_configured(self):
+        """No group(s) specified should raise ImproperlyConfigured"""
         view = self.view_class()
         view.group_required = None
         with self.assertRaises(ImproperlyConfigured):
@@ -612,6 +634,7 @@ def test_improperly_configured(self):
             view.get_group_required()
 
     def test_with_unicode(self):
+        """Unicode in group names should restrict access"""
         self.view_class.group_required = "niño"
         self.assertEqual("niño", self.view_class.group_required)
 
@@ -631,21 +654,25 @@ def test_with_unicode(self):
 
 @pytest.mark.django_db
 class TestUserPassesTestMixin(_TestAccessBasicsMixin, test.TestCase):
+    """Scenarios requiring a user to pass a test"""
     view_class = UserPassesTestView
     view_url = "/user_passes_test/"
     view_not_implemented_class = UserPassesTestNotImplementedView
     view_not_implemented_url = "/user_passes_test_not_implemented/"
 
     # for testing with passing and not passsing func_test
     def build_authorized_user(self, is_superuser=False):
+        """Get a test-passing user"""
         return UserFactory(
             is_superuser=is_superuser, is_staff=True, email="user@mydomain.com"
         )
 
     def build_unauthorized_user(self):
+        """Get a blank user"""
         return UserFactory()
 
     def test_with_user_pass(self):
+        """Valid username and password should pass the test"""
         user = self.build_authorized_user()
         self.client.login(username=user.username, password="asdf1234")
         resp = self.client.get(self.view_url)
@@ -654,19 +681,22 @@ def test_with_user_pass(self):
         self.assertEqual("OK", force_str(resp.content))
 
     def test_with_user_not_pass(self):
+        """A failing user should be redirected"""
         user = self.build_authorized_user(is_superuser=True)
         self.client.login(username=user.username, password="asdf1234")
         resp = self.client.get(self.view_url)
 
         self.assertRedirects(resp, "/accounts/login/?next=/user_passes_test/")
 
     def test_with_user_raise_exception(self):
+        """PermissionDenied should be raised"""
         with self.assertRaises(PermissionDenied):
             self.dispatch_view(
                 self.build_request(path=self.view_url), raise_exception=True
             )
 
     def test_not_implemented(self):
+        """NotImplemented should be raised"""
         view = self.view_not_implemented_class()
         with self.assertRaises(NotImplementedError):
             view.dispatch(
@@ -677,11 +707,13 @@ def test_not_implemented(self):
 
 @pytest.mark.django_db
 class TestSSLRequiredMixin(test.TestCase):
+    """Scenarios around requiring SSL"""
     view_class = SSLRequiredView
     view_url = "/sslrequired/"
 
     def test_ssl_redirection(self):
-        self.view_url = "https://testserver" + self.view_url
+        """Should redirect if not SSL"""
+        self.view_url = f"https://testserver{self.view_url}"
         self.view_class.raise_exception = False
         resp = self.client.get(self.view_url)
         self.assertRedirects(resp, self.view_url, status_code=301)
@@ -690,17 +722,20 @@ def test_ssl_redirection(self):
         self.assertEqual("https", resp.request.get("wsgi.url_scheme"))
 
     def test_raises_exception(self):
+        """Should return 404"""
         self.view_class.raise_exception = True
         resp = self.client.get(self.view_url)
         self.assertEqual(404, resp.status_code)
 
     @override_settings(DEBUG=True)
     def test_debug_bypasses_redirect(self):
+        """Debug mode should not require SSL"""
         self.view_class.raise_exception = False
         resp = self.client.get(self.view_url)
         self.assertEqual(200, resp.status_code)
 
     def test_https_does_not_redirect(self):
+        """SSL requests should not redirect"""
         self.view_class.raise_exception = False
         resp = self.client.get(self.view_url, secure=True)
         self.assertEqual(200, resp.status_code)
@@ -709,15 +744,14 @@ def test_https_does_not_redirect(self):
 
 @pytest.mark.django_db
 class TestRecentLoginRequiredMixin(test.TestCase):
-    """
-    Tests for RecentLoginRequiredMixin.
-    """
+    """ Scenarios requiring a recent login"""
 
     view_class = RecentLoginRequiredView
     recent_view_url = "/recent_login/"
     outdated_view_url = "/outdated_login/"
 
     def test_recent_login(self):
+        """A recent login should get a 200"""
         self.view_class.max_last_login_delta = 1800
         last_login = datetime.datetime.now()
         last_login = make_aware(last_login, get_current_timezone())
@@ -728,6 +762,7 @@ def test_recent_login(self):
         assert force_str(resp.content) == "OK"
 
     def test_outdated_login(self):
+        """An outdated login should get a 302"""
         self.view_class.max_last_login_delta = 0
         last_login = datetime.datetime.now() - datetime.timedelta(hours=2)
         last_login = make_aware(last_login, get_current_timezone())
@@ -737,8 +772,8 @@ def test_outdated_login(self):
         assert resp.status_code == 302
 
     def test_not_logged_in(self):
+        """Anonymous requests should be handled appropriately"""
         last_login = datetime.datetime.now()
         last_login = make_aware(last_login, get_current_timezone())
-        user = UserFactory(last_login=last_login)
         resp = self.client.get(self.recent_view_url)
         assert resp.status_code != 200

tests/test_other_mixins.py

@@ -0,0 +1,259 @@
+from unittest import mock
+import pytest
+
+from django.core.exceptions import ImproperlyConfigured
+from django import test
+
+from .helpers import TestViewHelper
+from .models import Article
+from .views import (
+    ArticleListView,
+    ArticleListViewWithCustomQueryset,
+    AuthorDetailView,
+    OrderableListView
+)
+
+
+class TestSelectRelatedMixin(TestViewHelper, test.TestCase):
+    """Scenarios related to adding select_related to queries"""
+    view_class = ArticleListView
+
+    def test_missing_select_related(self):
+        """If select_related is unset, raise ImproperlyConfigured"""
+        with self.assertRaises(ImproperlyConfigured):
+            self.dispatch_view(self.build_request(), select_related=None)
+
+    def test_invalid_select_related(self):
+        """If select_related is not a list or tuple, raise ImproperlyConfigured"""
+        with self.assertRaises(ImproperlyConfigured):
+            self.dispatch_view(self.build_request(), select_related={"a": 1})
+
+    @mock.patch("django.db.models.query.QuerySet.select_related")
+    def test_select_related_called(self, m):
+        """QuerySet.select_related should be called with the correct arguments"""
+        qs = Article.objects.all()
+        m.return_value = qs.select_related("author")
+        qs.select_related = m
+        m.reset_mock()
+
+        resp = self.dispatch_view(self.build_request())
+        self.assertEqual(200, resp.status_code)
+        m.assert_called_once_with("author")
+
+    @mock.patch("django.db.models.query.QuerySet.select_related")
+    def test_select_related_keeps_select_related_from_queryset(self, m):
+        """
+        Checks that an empty select_related attribute does not
+        cancel a select_related provided by queryset.
+        """
+        qs = Article.objects.all()
+        qs.select_related = m
+        m.reset_mock()
+
+        with pytest.warns(UserWarning):
+            resp = self.dispatch_view(
+                self.build_request(),
+                view_class=ArticleListViewWithCustomQueryset,
+            )
+        self.assertEqual(200, resp.status_code)
+        self.assertEqual(0, m.call_count)
+
+
+class TestPrefetchRelatedMixin(TestViewHelper, test.TestCase):
+    """Scenarios related to adding prefetch_related to queries"""
+    view_class = AuthorDetailView
+
+    def test_missing_prefetch_related(self):
+        """If prefetch_related is missing/None, raise ImproperlyConfigured"""
+        with self.assertRaises(ImproperlyConfigured):
+            self.dispatch_view(self.build_request(), prefetch_related=None)
+
+    def test_invalid_prefetch_related(self):
+        """If prefetch_related is not a list or tuple, raise ImproperlyConfigured"""
+        with self.assertRaises(ImproperlyConfigured):
+            self.dispatch_view(self.build_request(), prefetch_related={"a": 1})
+
+    @mock.patch("django.db.models.query.QuerySet.prefetch_related")
+    def test_prefetch_related_called(self, m):
+        """QuerySet.prefetch_related() should be called with correct arguments"""
+        qs = Article.objects.all()
+        m.return_value = qs.prefetch_related("article_set")
+        qs.prefetch_related = m
+        m.reset_mock()
+
+        resp = self.dispatch_view(self.build_request())
+        self.assertEqual(200, resp.status_code)
+        m.assert_called_once_with("article_set")
+
+    @mock.patch("django.db.models.query.QuerySet.prefetch_related")
+    def test_prefetch_related_keeps_select_related_from_queryset(self, m):
+        """
+        Checks that an empty prefetch_related attribute does not
+        cancel a prefetch_related provided by queryset.
+        """
+        qs = Article.objects.all()
+        qs.prefetch_related = m
+        m.reset_mock()
+
+        with pytest.warns(UserWarning):
+            resp = self.dispatch_view(
+                self.build_request(),
+                view_class=ArticleListViewWithCustomQueryset,
+            )
+        self.assertEqual(200, resp.status_code)
+        self.assertEqual(0, m.call_count)
+
+
+class TestOrderableListMixin(TestViewHelper, test.TestCase):
+    """Scenarios involving ordering records"""
+    view_class = OrderableListView
+
+    def __make_test_articles(self):
+        """Generate a couple of articles"""
+        a1 = Article.objects.create(title="Alpha", body="Zet")
+        a2 = Article.objects.create(title="Zet", body="Alpha")
+        return a1, a2
+
+    def test_correct_order(self):
+        """Valid column and order query arguments should order the objects"""
+        a1, a2 = self.__make_test_articles()
+
+        resp = self.dispatch_view(
+            self.build_request(path="?order_by=title&ordering=asc"),
+            orderable_columns=None,
+            get_orderable_columns=lambda: (
+                "id",
+                "title",
+            ),
+        )
+        self.assertEqual(list(resp.context_data["object_list"]), [a1, a2])
+
+        resp = self.dispatch_view(
+            self.build_request(path="?order_by=id&ordering=desc"),
+            orderable_columns=None,
+            get_orderable_columns=lambda: (
+                "id",
+                "title",
+            ),
+        )
+        self.assertEqual(list(resp.context_data["object_list"]), [a2, a1])
+
+    def test_correct_order_with_default_ordering(self):
+        """A valid order_by query argument should sort the default direction"""
+        a1, a2 = self.__make_test_articles()
+
+        resp = self.dispatch_view(
+            self.build_request(path="?order_by=id"),
+            orderable_columns=None,
+            ordering_default=None,
+            get_orderable_columns=lambda: (
+                "id",
+                "title",
+            ),
+        )
+        self.assertEqual(list(resp.context_data["object_list"]), [a1, a2])
+
+        resp = self.dispatch_view(
+            self.build_request(path="?order_by=id"),
+            orderable_columns=None,
+            ordering_default="asc",
+            get_orderable_columns=lambda: (
+                "id",
+                "title",
+            ),
+        )
+        self.assertEqual(list(resp.context_data["object_list"]), [a1, a2])
+
+        resp = self.dispatch_view(
+            self.build_request(path="?order_by=id"),
+            orderable_columns=None,
+            ordering_default="desc",
+            get_orderable_columns=lambda: (
+                "id",
+                "title",
+            ),
+        )
+        self.assertEqual(list(resp.context_data["object_list"]), [a2, a1])
+
+    def test_correct_order_with_param_not_default_ordering(self):
+        """
+        Objects must be properly ordered if requested with valid column names
+        and ordering option in the query params.
+        In this case, the ordering_default will be overwritten.
+        """
+        a1, a2 = self.__make_test_articles()
+
+        resp = self.dispatch_view(
+            self.build_request(path="?order_by=id&ordering=asc"),
+            orderable_columns=None,
+            ordering_default="desc",
+            get_orderable_columns=lambda: (
+                "id",
+                "title",
+            ),
+        )
+        self.assertEqual(list(resp.context_data["object_list"]), [a1, a2])
+
+    def test_correct_order_with_incorrect_default_ordering(self):
+        """
+        Objects must be properly ordered if requested with valid column names
+        and with the default ordering
+        """
+        view = self.view_class()
+        view.ordering_default = "improper_default_value"
+        self.assertRaises(
+            ImproperlyConfigured, lambda: view.get_ordering_default()
+        )
+
+    def test_default_column(self):
+        """
+        When no ordering specified in GET, use
+        View.get_orderable_columns_default()
+        """
+        a1, a2 = self.__make_test_articles()
+
+        resp = self.dispatch_view(self.build_request())
+        self.assertEqual(list(resp.context_data["object_list"]), [a1, a2])
+
+    def test_get_orderable_columns_returns_correct_values(self):
+        """
+        OrderableListMixin.get_orderable_columns() should return
+        View.orderable_columns attribute by default or raise
+        ImproperlyConfigured exception if the attribute is None
+        """
+        view = self.view_class()
+        self.assertEqual(view.get_orderable_columns(), view.orderable_columns)
+        view.orderable_columns = None
+        self.assertRaises(
+            ImproperlyConfigured, lambda: view.get_orderable_columns()
+        )
+
+    def test_get_orderable_columns_default_returns_correct_values(self):
+        """
+        OrderableListMixin.get_orderable_columns_default() should return
+        View.orderable_columns_default attribute by default or raise
+        ImproperlyConfigured exception if the attribute is None
+        """
+        view = self.view_class()
+        self.assertEqual(
+            view.get_orderable_columns_default(),
+            view.orderable_columns_default,
+        )
+        view.orderable_columns_default = None
+        self.assertRaises(
+            ImproperlyConfigured, lambda: view.get_orderable_columns_default()
+        )
+
+    def test_only_allowed_columns(self):
+        """
+        If column is not in Model.Orderable.columns iterable, the objects
+        should be ordered by default column.
+        """
+        a1, a2 = self.__make_test_articles()
+
+        resp = self.dispatch_view(
+            self.build_request(path="?order_by=body&ordering=asc"),
+            orderable_columns_default=None,
+            get_orderable_columns_default=lambda: "title",
+        )
+        self.assertEqual(list(resp.context_data["object_list"]), [a1, a2])

tests/test_queries.py

@@ -1,9 +1,9 @@
-from django.urls import include, re_path
+from django.urls import re_path
 
 from . import views
 
+
 urlpatterns = [
-    # CanonicalSlugDetailMixin namespace tests
     re_path(
         r"^article/(?P<pk>\d+)-(?P<slug>[\w-]+)/$",
         views.CanonicalSlugDetailView.as_view(),

tests/urls_namespaced.py

@@ -26,15 +26,19 @@ class OkView(View):
     """
 
     def get(self, request):
+        """Everything is going to be OK"""
         return HttpResponse("OK")
 
     def post(self, request):
+        """Get it?"""
         return self.get(request)
 
     def put(self, request):
+        """Get it?"""
         return self.get(request)
 
     def delete(self, request):
+        """Get it?"""
         return self.get(request)
 
 
@@ -67,15 +71,19 @@ class AjaxResponseView(views.AjaxResponseMixin, OkView):
     """
 
     def get_ajax(self, request):
+        """Everything will eventually be OK"""
         return HttpResponse("AJAX_OK")
 
     def post_ajax(self, request):
+        """Get it?"""
         return self.get_ajax(request)
 
     def put_ajax(self, request):
+        """Get it?"""
         return self.get_ajax(request)
 
     def delete_ajax(self, request):
+        """Get it?"""
         return self.get_ajax(request)
 
 
@@ -85,6 +93,7 @@ class SimpleJsonView(views.JSONResponseMixin, View):
     """
 
     def get(self, request):
+        """Send back some JSON"""
         object = {"username": request.user.username}
         return self.render_json_response(object)
 
@@ -98,6 +107,7 @@ class CustomJsonEncoderView(views.JSONResponseMixin, View):
     json_encoder_class = SetJSONEncoder
 
     def get(self, request):
+        """Send back some JSON"""
         object = {"numbers": set([1, 2, 3])}
         return self.render_json_response(object)
 
@@ -109,6 +119,7 @@ class SimpleJsonBadRequestView(views.JSONResponseMixin, View):
     """
 
     def get(self, request):
+        """Send back some JSON"""
         object = {"username": request.user.username}
         return self.render_json_response(object, status=400)
 
@@ -120,6 +131,7 @@ class ArticleListJsonView(views.JSONResponseMixin, View):
     """
 
     def get(self, request):
+        """Send back some JSON"""
         queryset = Article.objects.all()
         return self.render_json_object_response(queryset, fields=("title",))
 
@@ -130,6 +142,7 @@ class JsonRequestResponseView(views.JsonRequestResponseMixin, View):
     """
 
     def post(self, request):
+        """Send back some JSON"""
         return self.render_json_response(self.request_json)
 
 
@@ -142,6 +155,7 @@ class JsonBadRequestView(views.JsonRequestResponseMixin, View):
     require_json = True
 
     def post(self, request, *args, **kwargs):
+        """Send back some JSON"""
         return self.render_json_response(self.request_json)
 
 
@@ -152,6 +166,7 @@ class JsonCustomBadRequestView(views.JsonRequestResponseMixin, View):
     """
 
     def post(self, request, *args, **kwargs):
+        """Handle the POST request"""
         if not self.request_json:
             return self.render_bad_request_response({"error": "you messed up"})
         return self.render_json_response(self.request_json)
@@ -229,7 +244,8 @@ class FormWithUserKwargView(views.UserFormKwargsMixin, FormView):
     template_name = "form.html"
 
     def form_valid(self, form):
-        return HttpResponse("username: %s" % form.user.username)
+        """A simple response to watch for"""
+        return HttpResponse(f"username: {form.user.username}")
 
 
 class HeadlineView(views.SetHeadlineMixin, TemplateView):
@@ -265,6 +281,7 @@ class DynamicHeadlineView(views.SetHeadlineMixin, TemplateView):
     template_name = "blank.html"
 
     def get_headline(self):
+        """Return the headline passed in via kwargs"""
         return self.kwargs["s"]
 
 
@@ -286,24 +303,26 @@ class MultiplePermissionsRequiredView(
 
 
 class SuperuserRequiredView(views.SuperuserRequiredMixin, OkView):
-    pass
+    """Require a superuser"""
 
 
 class StaffuserRequiredView(views.StaffuserRequiredMixin, OkView):
-    pass
+    """Require a user marked as `is_staff`"""
 
 
 class CsrfExemptView(views.CsrfExemptMixin, OkView):
-    pass
+    """Ignore CSRF"""
 
 
 class AuthorDetailView(views.PrefetchRelatedMixin, ListView):
+    """A basic detail view to test prefetching"""
     model = User
     prefetch_related = ["article_set"]
     template_name = "blank.html"
 
 
 class OrderableListView(views.OrderableListMixin, ListView):
+    """A basic list view to test ordering the output"""
     model = Article
     orderable_columns = (
         "id",
@@ -313,35 +332,37 @@ class OrderableListView(views.OrderableListMixin, ListView):
 
 
 class CanonicalSlugDetailView(views.CanonicalSlugDetailMixin, DetailView):
+    """A basic detail view to test a canonical slug"""
     model = Article
     template_name = "blank.html"
 
 
-class OverriddenCanonicalSlugDetailView(
-    views.CanonicalSlugDetailMixin, DetailView
-):
+class OverriddenCanonicalSlugDetailView(views.CanonicalSlugDetailMixin, DetailView):
+    """A basic detail view to test an overridden slug"""
     model = Article
     template_name = "blank.html"
 
     def get_canonical_slug(self):
+        """Give back a different, encoded slug. My slug secrets are safe"""
         return codecs.encode(self.get_object().slug, "rot_13")
 
 
-class CanonicalSlugDetailCustomUrlKwargsView(
-    views.CanonicalSlugDetailMixin, DetailView
-):
+class CanonicalSlugDetailCustomUrlKwargsView(views.CanonicalSlugDetailMixin, DetailView):
+    """A basic detail view to test a slug with custom URL stuff"""
     model = Article
     template_name = "blank.html"
     pk_url_kwarg = "my_pk"
     slug_url_kwarg = "my_slug"
 
 
 class ModelCanonicalSlugDetailView(views.CanonicalSlugDetailMixin, DetailView):
+    """A basic detail view to test a model with a canonical slug"""
     model = CanonicalArticle
     template_name = "blank.html"
 
 
 class FormMessagesView(views.FormMessagesMixin, CreateView):
+    """A basic form view to test valid/invalid messages"""
     form_class = ArticleForm
     form_invalid_message = _("Invalid")
     form_valid_message = _("Valid")
@@ -351,10 +372,12 @@ class FormMessagesView(views.FormMessagesMixin, CreateView):
 
 
 class GroupRequiredView(views.GroupRequiredMixin, OkView):
+    """Is everything OK in this group?"""
     group_required = "test_group"
 
 
 class UserPassesTestView(views.UserPassesTestMixin, OkView):
+    """Did I pass a test?"""
     def test_func(self, user):
         return (
             user.is_staff
@@ -366,6 +389,7 @@ def test_func(self, user):
 class UserPassesTestLoginRequiredView(
     views.LoginRequiredMixin, views.UserPassesTestMixin, OkView
 ):
+    """Am I logged in _and_ passing a test?"""
     def test_func(self, user):
         return (
             user.is_staff
@@ -375,15 +399,18 @@ def test_func(self, user):
 
 
 class UserPassesTestNotImplementedView(views.UserPassesTestMixin, OkView):
+    """The test went missing?"""
     pass
 
 
 class AllVerbsView(views.AllVerbsMixin, View):
+    """I know, like, all the verbs"""
     def all(self, request, *args, **kwargs):
         return HttpResponse("All verbs return this!")
 
 
 class SSLRequiredView(views.SSLRequiredMixin, OkView):
+    """Speak friend and enter"""
     pass
 
 
@@ -394,13 +421,15 @@ class RecentLoginRequiredView(views.RecentLoginRequiredMixin, OkView):
 
 
 class AttributeHeaderView(views.HeaderMixin, OkView):
+    """Set headers in an attribute w/o a template render class"""
     headers = {
         "X-DJANGO-BRACES-1": 1,
         "X-DJANGO-BRACES-2": 2,
     }
 
 
 class MethodHeaderView(views.HeaderMixin, OkView):
+    """Set headers in a method w/o a template render class"""
     def get_headers(self, request):
         return {
             "X-DJANGO-BRACES-1": 1,
@@ -409,19 +438,22 @@ def get_headers(self, request):
 
 
 class AuxiliaryHeaderView(View):
+    """A view with a header already set"""
     def dispatch(self, request, *args, **kwargs):
         response = HttpResponse("OK with headers")
         response["X-DJANGO-BRACES-EXISTING"] = "value"
         return response
 
 
 class ExistingHeaderView(views.HeaderMixin, AuxiliaryHeaderView):
+    """A view trying to override a parent's header"""
     headers = {
         'X-DJANGO-BRACES-EXISTING': 'other value'
     }
 
 
 class CacheControlPublicView(views.CacheControlMixin, OkView):
+    """A public-cached page with a 60 second timeout"""
     cachecontrol_public = True
     cachecontrol_max_age = 60