[change] Improved code quality

Author: ManishShah120

openwisp_users/api/permissions.py

@@ -1,5 +1,8 @@
 from django.utils.translation import gettext_lazy as _
-from rest_framework.permissions import BasePermission, DjangoModelPermissions
+from rest_framework.permissions import BasePermission
+from rest_framework.permissions import (
+    DjangoModelPermissions as BaseDjangoModelPermissions,
+)
 from swapper import load_model
 
 Organization = load_model('openwisp_users', 'Organization')
@@ -68,7 +71,7 @@ def validate_membership(self, user, org):
         return org and (user.is_superuser or user.is_owner(org))
 
 
-class ViewDjangoModelPermissions(DjangoModelPermissions):
+class DjangoModelPermissions(BaseDjangoModelPermissions):
     perms_map = {
         'GET': ['%(app_label)s.view_%(model_name)s'],
         'OPTIONS': [],
@@ -94,9 +97,6 @@ def has_permission(self, request, view):
         change_perm = self.get_required_permissions('PUT', queryset.model)
 
         if request.method == 'GET':
-            if user.has_perms(perms) or user.has_perms(change_perm):
-                return True
-            else:
-                return False
+            return user.has_perms(perms) or user.has_perms(change_perm)
 
         return user.has_perms(perms)

tests/testapp/tests/test_permission_classes.py

@@ -127,18 +127,20 @@ def test_organization_field_with_errored_parent(self):
             self.client.get(reverse('test_error_field_view'), **auth)
         self.assertIn('Organization not found', str(error.exception))
 
-    def test_view_permission_with_operator(self):
-        user = User.objects.create_user(
-            username='operator', password='tester', email='operator@test.com'
-        )
-        operator_group = Group.objects.filter(name='Operator')
-        user.groups.set(operator_group)
-        org1 = self._get_org()
+    def _get_auth_template(self, user, org1):
         OrganizationUser.objects.create(user=user, organization=org1, is_admin=True)
         self.client.force_login(user)
-        token = self._obtain_auth_token()
+        token = self._obtain_auth_token(user)
         auth = dict(HTTP_AUTHORIZATION=f'Bearer {token}')
         t1 = self._create_template(organization=org1)
+        return (auth, t1)
+
+    def test_view_permission_with_operator(self):
+        user = self._get_user()
+        operator_group = Group.objects.filter(name='Operator')
+        user.groups.set(operator_group)
+        org1 = self._get_org()
+        auth, t1 = self._get_auth_template(user, org1)
         with self.subTest('Get Template List'):
             response = self.client.get(reverse('test_template_list'), **auth)
             self.assertEqual(response.status_code, 403)
@@ -149,19 +151,13 @@ def test_view_permission_with_operator(self):
             self.assertEqual(response.status_code, 403)
 
     def test_view_permission_with_administrator(self):
-        user = User.objects.create_user(
-            username='operator', password='tester', email='operator@test.com'
-        )
+        user = self._get_user()
         administrator_group = Group.objects.get(name='Administrator')
         change_perm = Permission.objects.get(codename='change_template')
         administrator_group.permissions.add(change_perm)
         user.groups.add(administrator_group)
         org1 = self._get_org()
-        OrganizationUser.objects.create(user=user, organization=org1, is_admin=True)
-        self.client.force_login(user)
-        token = self._obtain_auth_token()
-        auth = dict(HTTP_AUTHORIZATION=f'Bearer {token}')
-        t1 = self._create_template(organization=org1)
+        auth, t1 = self._get_auth_template(user, org1)
         with self.subTest('Get Template List'):
             response = self.client.get(reverse('test_template_list'), **auth)
             self.assertEqual(response.status_code, 200)
@@ -175,19 +171,13 @@ def test_view_permission_with_administrator(self):
         self.assertTrue('change_template' in permissions)
 
     def test_view_permission_with_operator_having_view_perm(self):
-        user = User.objects.create_user(
-            username='operator', password='tester', email='operator@test.com'
-        )
+        user = self._get_user()
         operator_group = Group.objects.get(name='Operator')
         view_perm = Permission.objects.get(codename='view_template')
         operator_group.permissions.add(view_perm)
         user.groups.add(operator_group)
         org1 = self._get_org()
-        OrganizationUser.objects.create(user=user, organization=org1, is_admin=True)
-        self.client.force_login(user)
-        token = self._obtain_auth_token()
-        auth = dict(HTTP_AUTHORIZATION=f'Bearer {token}')
-        t1 = self._create_template(organization=org1)
+        auth, t1 = self._get_auth_template(user, org1)
         with self.subTest('Get Template List'):
             response = self.client.get(reverse('test_template_list'), **auth)
             self.assertEqual(response.status_code, 200)
@@ -209,18 +199,12 @@ def test_view_permission_with_operator_having_view_perm(self):
             self.assertEqual(response.status_code, 403)
 
     def test_view_django_model_permission_with_view_perm(self):
-        user = User.objects.create_user(
-            username='operator', password='tester', email='operator@test.com'
-        )
+        user = self._get_user()
         user_permissions = Permission.objects.filter(codename='view_template')
         user.user_permissions.add(*user_permissions)
         user.organizations_dict  # force caching
         org1 = self._get_org()
-        OrganizationUser.objects.create(user=user, organization=org1, is_admin=True)
-        self.client.force_login(user)
-        token = self._obtain_auth_token()
-        auth = dict(HTTP_AUTHORIZATION=f'Bearer {token}')
-        t1 = self._create_template(organization=org1)
+        auth, t1 = self._get_auth_template(user, org1)
         with self.subTest('Get Template List'):
             response = self.client.get(reverse('test_template_list'), **auth)
             self.assertEqual(response.status_code, 200)
@@ -231,18 +215,12 @@ def test_view_django_model_permission_with_view_perm(self):
             self.assertEqual(response.status_code, 200)
 
     def test_view_django_model_permission_with_change_perm(self):
-        user = User.objects.create_user(
-            username='operator', password='tester', email='operator@test.com'
-        )
+        user = self._get_user()
         user_permissions = Permission.objects.filter(codename='change_template')
         user.user_permissions.add(*user_permissions)
         user.organizations_dict  # force caching
         org1 = self._get_org()
-        OrganizationUser.objects.create(user=user, organization=org1, is_admin=True)
-        self.client.force_login(user)
-        token = self._obtain_auth_token()
-        auth = dict(HTTP_AUTHORIZATION=f'Bearer {token}')
-        t1 = self._create_template(organization=org1)
+        auth, t1 = self._get_auth_template(user, org1)
         with self.subTest('Get Template List'):
             response = self.client.get(reverse('test_template_list'), **auth)
             self.assertEqual(response.status_code, 200)

tests/testapp/views.py

@@ -18,10 +18,10 @@
 )
 from openwisp_users.api.permissions import (
     BaseOrganizationPermission,
+    DjangoModelPermissions,
     IsOrganizationManager,
     IsOrganizationMember,
     IsOrganizationOwner,
-    ViewDjangoModelPermissions,
 )
 
 from .models import Book, Config, Shelf, Template
@@ -176,7 +176,7 @@ class TemplateListCreateView(FilterByOrganizationManaged, ListCreateAPIView):
     authentication_classes = (BearerAuthentication,)
     permission_classes = (
         IsOrganizationMember,
-        ViewDjangoModelPermissions,
+        DjangoModelPermissions,
     )
     queryset = Template.objects.all()
 
@@ -186,7 +186,7 @@ class TemplateDetailView(FilterByOrganizationManaged, RetrieveUpdateDestroyAPIVi
     authentication_classes = (BearerAuthentication,)
     permission_classes = (
         IsOrganizationMember,
-        ViewDjangoModelPermissions,
+        DjangoModelPermissions,
     )
     queryset = Template.objects.all()