Refactor user management app

Add keyword arguments.

Update view resolutions and URL parameters to the '_id' form.

Remove code from create and edit subclasses and user
the super class error checking.

Cache the view object instead of using .get_object()
every time.

Movernize tests.

Update views to comply with MERCs 5 and 6.

Split UserTestMixin into mixins for Groups and Users tests.

Add super delete and detail tests.

Remove redundant superuser filtering from views.

Add transactions to views that also commit events.

Signed-off-by: Roberto Rosario <Roberto.Rosario@mayan-edms.com>

mayan/apps/user_management/api_views.py

@@ -7,6 +7,7 @@ from django.shortcuts import get_object_or_404
 from rest_framework import generics
 
 from mayan.apps.acls.models import AccessControlList
+from mayan.apps.common.mixins import ExternalObjectMixin
 from mayan.apps.rest_api.filters import MayanObjectPermissionsFilter
 from mayan.apps.rest_api.permissions import MayanPermission
 
@@ -16,7 +17,7 @@ from .permissions import (
     permission_user_edit, permission_user_view
 )
 from .serializers import (
-    GroupSerializer, UserSerializer, UserGroupListSerializer
+    GroupSerializer, UserSerializer#, UserGroupListSerializer
 )
 
 
@@ -53,6 +54,7 @@ class APIGroupView(generics.RetrieveUpdateDestroyAPIView):
     patch: Partially edit the selected group.
     put: Edit the selected group.
     """
+    lookup_url_kwarg = 'group_pk'
     mayan_object_permissions = {
         'GET': (permission_group_view,),
         'PUT': (permission_group_edit,),
@@ -84,6 +86,7 @@ class APIUserView(generics.RetrieveUpdateDestroyAPIView):
     patch: Partially edit the selected user.
     put: Edit the selected user.
     """
+    lookup_url_kwarg = 'user_pk'
     mayan_object_permissions = {
         'GET': (permission_user_view,),
         'PUT': (permission_user_edit,),
@@ -95,16 +98,28 @@ class APIUserView(generics.RetrieveUpdateDestroyAPIView):
     serializer_class = UserSerializer
 
 
-class APIUserGroupList(generics.ListCreateAPIView):
+class APIUserGroupList(ExternalObjectMixin, generics.ListCreateAPIView):
     """
     get: Returns a list of all the groups to which an user belongs.
     post: Add a user to a list of groups.
     """
+    external_object_pk_url_kwarg = 'user_pk'
+    filter_backends = (MayanObjectPermissionsFilter,)
     mayan_object_permissions = {
-        'GET': (permission_user_view,),
+        'GET': (permission_group_view,),
-        'POST': (permission_user_edit,)
+        'POST': (permission_group_edit,)
     }
-    permission_classes = (MayanPermission,)
+
+    def get_external_object_permission(self):
+        if self.request.method == 'POST':
+            return permission_user_edit
+        else:
+            return permission_user_view
+
+    def get_external_object_queryset(self):
+        return get_user_model().objects.exclude(is_staff=True).exclude(
+            is_superuser=True
+        )
 
     def get_serializer(self, *args, **kwargs):
         if not self.request:
@@ -113,10 +128,10 @@ class APIUserGroupList(generics.ListCreateAPIView):
         return super(APIUserGroupList, self).get_serializer(*args, **kwargs)
 
     def get_serializer_class(self):
-        if self.request.method == 'GET':
+        if self.request.method == 'POST':
+            return UserSerializer
+        else:
             return GroupSerializer
-        elif self.request.method == 'POST':
-            return UserGroupListSerializer
 
     def get_serializer_context(self):
         """
@@ -133,26 +148,10 @@ class APIUserGroupList(generics.ListCreateAPIView):
         return context
 
     def get_queryset(self):
-        user = self.get_user()
+        return self.get_user().groups.order_by('id')
-
-        return AccessControlList.objects.filter_by_access(
-            permission_group_view, self.request.user,
-            queryset=user.groups.order_by('id')
-        )
 
     def get_user(self):
-        if self.request.method == 'GET':
+        return self.get_external_object()
-            permission = permission_user_view
-        else:
-            permission = permission_user_edit
-
-        user = get_object_or_404(klass=get_user_model(), pk=self.kwargs['pk'])
-
-        AccessControlList.objects.check_access(
-            permissions=(permission,), user=self.request.user,
-            obj=user
-        )
-        return user
 
     def perform_create(self, serializer):
-        serializer.save(user=self.get_user(), _user=self.request.user)
+        return serializer.save(user=self.get_object(), _user=self.request.user)

mayan/apps/user_management/apps.py

@@ -40,7 +40,7 @@ from .permissions import (
     permission_user_view
 )
 from .search import *  # NOQA
-from .utils import get_groups, get_users
+from .utils import lookup_get_groups, lookup_get_users
 
 
 class UserManagementApp(MayanAppConfig):
@@ -65,15 +65,15 @@ class UserManagementApp(MayanAppConfig):
 
         MetadataLookup(
             description=_('All the groups.'), name='groups',
-            value=get_groups
+            value=lookup_get_groups
         )
         MetadataLookup(
             description=_('All the users.'), name='users',
-            value=get_users
+            value=lookup_get_users
         )
 
         ModelEventType.register(
-            model=User, event_types=(event_user_edited,)
+            event_types=(event_user_edited,), model=User
         )
 
         ModelPermission.register(
@@ -129,7 +129,9 @@ class UserManagementApp(MayanAppConfig):
 
         menu_list_facet.bind_links(
             links=(
-                link_acl_list, link_group_members,
+                link_acl_list, link_events_for_object,
+                link_object_event_types_user_subcriptions_list,
+                link_group_members,
             ), sources=(Group,)
         )
 
@@ -138,8 +140,7 @@ class UserManagementApp(MayanAppConfig):
                 link_acl_list, link_events_for_object,
                 link_object_event_types_user_subcriptions_list,
                 link_user_groups,
-            ),
+            ), sources=(User,)
-            sources=(User,)
         )
 
         menu_multi_item.bind_links(
@@ -147,12 +148,10 @@ class UserManagementApp(MayanAppConfig):
             sources=('user_management:user_list',)
         )
         menu_object.bind_links(
-            links=(link_group_edit,),
+            links=(link_group_edit,), sources=(Group,)
-            sources=(Group,)
         )
         menu_object.bind_links(
-            links=(link_group_delete,), position=99,
+            links=(link_group_delete,), position=99, sources=(Group,)
-            sources=(Group,)
         )
         menu_object.bind_links(
             links=(

mayan/apps/user_management/events.py

@@ -5,7 +5,7 @@ from django.utils.translation import ugettext_lazy as _
 from mayan.apps.events import EventTypeNamespace
 
 namespace = EventTypeNamespace(
-    name='user_management', label=_('User management')
+    label=_('User management'), name='user_management'
 )
 
 event_group_created = namespace.add_event_type(

mayan/apps/user_management/icons.py

@@ -16,7 +16,7 @@ icon_user_delete = Icon(driver_name='fontawesome', symbol='times')
 icon_user_edit = Icon(driver_name='fontawesome', symbol='pencil-alt')
 icon_user_list = Icon(driver_name='fontawesome', symbol='user')
 icon_user_multiple_delete = icon_user_delete
+icon_user_multiple_set_password = Icon(driver_name='fontawesome', symbol='key')
 icon_user_set_options = Icon(driver_name='fontawesome', symbol='cog')
 icon_user_set_password = Icon(driver_name='fontawesome', symbol='key')
 icon_user_setup = Icon(driver_name='fontawesome', symbol='user')
-icon_user_multiple_set_password = icon_user_set_password

mayan/apps/user_management/links.py

@@ -38,24 +38,23 @@ link_group_create = Link(
     text=_('Create new group'), view='user_management:group_create'
 )
 link_group_delete = Link(
-    args='object.id', icon_class=icon_group_delete,
+    icon_class=icon_group_delete, kwargs={'group_id': 'object.pk'},
     permission=permission_group_delete, tags='dangerous',
-    text=_('Delete'), view='user_management:group_delete',
+    text=_('Delete'), view='user_management:group_delete'
 )
 link_group_edit = Link(
-    args='object.id', icon_class=icon_group_edit,
+    icon_class=icon_group_edit, kwargs={'group_id': 'object.pk'},
     permission=permission_group_edit, text=_('Edit'),
-    view='user_management:group_edit',
+    view='user_management:group_edit'
 )
 link_group_list = Link(
     icon_class=icon_group_list, permission=permission_group_view,
-    text=_('Groups'),
+    text=_('Groups'), view='user_management:group_list'
-    view='user_management:group_list'
 )
 link_group_members = Link(
-    args='object.id', icon_class=icon_group_members,
+    icon_class=icon_group_members, kwargs={'group_id': 'object.pk'},
     permission=permission_group_edit, text=_('Users'),
-    view='user_management:group_members',
+    view='user_management:group_members'
 )
 link_group_setup = Link(
     icon_class=icon_group_setup, permission=permission_group_view,
@@ -66,19 +65,19 @@ link_user_create = Link(
     text=_('Create new user'), view='user_management:user_create'
 )
 link_user_delete = Link(
-    args='object.id', icon_class=icon_user_delete,
+    icon_class=icon_user_delete, kwargs={'user_id': 'object.pk'},
     permission=permission_user_delete, tags='dangerous', text=_('Delete'),
-    view='user_management:user_delete',
+    view='user_management:user_delete'
 )
 link_user_edit = Link(
-    args='object.id', icon_class=icon_user_edit,
+    icon_class=icon_user_edit, kwargs={'user_id': 'object.pk'},
     permission=permission_user_edit, text=_('Edit'),
-    view='user_management:user_edit',
+    view='user_management:user_edit'
 )
 link_user_groups = Link(
-    args='object.id', condition=condition_is_not_superuser,
+    condition=condition_is_not_superuser, icon_class=icon_group,
-    icon_class=icon_group, permission=permission_user_edit,
+    kwargs={'user_id': 'object.pk'}, permission=permission_user_edit,
-    text=_('Groups'), view='user_management:user_groups',
+    text=_('Groups'), view='user_management:user_groups'
 )
 link_user_list = Link(
     icon_class=icon_user_list, permission=permission_user_view,
@@ -95,14 +94,14 @@ link_user_multiple_set_password = Link(
     view='user_management:user_multiple_set_password'
 )
 link_user_set_options = Link(
-    args='object.id', icon_class=icon_user_set_options,
+    icon_class=icon_user_set_options, kwargs={'user_id': 'object.pk'},
     permission=permission_user_edit, text=_('User options'),
-    view='user_management:user_options',
+    view='user_management:user_options'
 )
 link_user_set_password = Link(
-    args='object.id', icon_class=icon_user_set_password,
+    icon_class=icon_user_set_password, kwargs={'user_id': 'object.pk'},
     permission=permission_user_edit, text=_('Set password'),
-    view='user_management:user_set_password',
+    view='user_management:user_set_password'
 )
 link_user_setup = Link(
     icon_class=icon_user_setup, permission=permission_user_view,

mayan/apps/user_management/methods.py

@@ -4,4 +4,6 @@ from django.shortcuts import reverse
 
 
 def method_get_absolute_url(self):
-    return reverse(viewname='user_management:user_details', args=(self.pk,))
+    return reverse(
+        viewname='user_management:user_details', kwargs={'user_id': self.pk}
+    )

mayan/apps/user_management/models.py

@@ -18,8 +18,9 @@ class UserOptions(models.Model):
         to=settings.AUTH_USER_MODEL, unique=True, verbose_name=_('User')
     )
     block_password_change = models.BooleanField(
-        default=False,
+        default=False, verbose_name=_(
-        verbose_name=_('Forbid this user from changing their password.')
+            'Forbid this user from changing their password.'
+        )
     )
 
     objects = UserOptionsManager()

mayan/apps/user_management/permissions.py

@@ -4,29 +4,31 @@ from django.utils.translation import ugettext_lazy as _
 
 from mayan.apps.permissions import PermissionNamespace
 
-namespace = PermissionNamespace(label=_('User management'), name='user_management')
+namespace = PermissionNamespace(
+    label=_('User management'), name='user_management'
+)
 
 permission_group_create = namespace.add_permission(
-    name='group_create', label=_('Create new groups')
+    label=_('Create new groups'), name='group_create'
 )
 permission_group_delete = namespace.add_permission(
-    name='group_delete', label=_('Delete existing groups')
+    label=_('Delete existing groups'), name='group_delete'
 )
 permission_group_edit = namespace.add_permission(
-    name='group_edit', label=_('Edit existing groups')
+    label=_('Edit existing groups'), name='group_edit'
 )
 permission_group_view = namespace.add_permission(
-    name='group_view', label=_('View existing groups')
+    label=_('View existing groups'), name='group_view'
 )
 permission_user_create = namespace.add_permission(
-    name='user_create', label=_('Create new users')
+    label=_('Create new users'), name='user_create'
 )
 permission_user_delete = namespace.add_permission(
-    name='user_delete', label=_('Delete existing users')
+    label=_('Delete existing users'), name='user_delete'
 )
 permission_user_edit = namespace.add_permission(
-    name='user_edit', label=_('Edit existing users')
+    label=_('Edit existing users'), name='user_edit'
 )
 permission_user_view = namespace.add_permission(
-    name='user_view', label=_('View existing users')
+    label=_('View existing users'), name='user_view'
 )

mayan/apps/user_management/search.py

@@ -32,8 +32,7 @@ user_search.add_model_field(
 )
 
 group_search = SearchModel(
-    app_label='auth', model_name='Group',
+    app_label='auth', model_name='Group', permission=permission_group_view,
-    permission=permission_group_view,
     serializer_path='user_management.serializers.GroupSerializer'
 )
 

mayan/apps/user_management/serializers.py

@@ -11,7 +11,7 @@ from rest_framework.exceptions import ValidationError
 
 from mayan.apps.acls.models import AccessControlList
 
-from .permissions import permission_group_view
+from .permissions import permission_group_edit, permission_group_view
 
 
 class GroupSerializer(serializers.HyperlinkedModelSerializer):
@@ -19,7 +19,10 @@ class GroupSerializer(serializers.HyperlinkedModelSerializer):
 
     class Meta:
         extra_kwargs = {
-            'url': {'view_name': 'rest_api:group-detail'}
+            'url': {
+                'lookup_field': 'pk', 'lookup_url_kwarg': 'group_pk',
+                'view_name': 'rest_api:group-detail'
+            }
         }
         fields = ('id', 'name', 'url', 'users_count')
         model = Group
@@ -28,41 +31,13 @@ class GroupSerializer(serializers.HyperlinkedModelSerializer):
         return instance.user_set.count()
 
 
-class UserGroupListSerializer(serializers.Serializer):
+class UserSerializer(serializers.HyperlinkedModelSerializer):
-    group_pk_list = serializers.CharField(
+    groups = GroupSerializer(many=True, read_only=True, required=False)
+    groups_pk_list = serializers.CharField(
         help_text=_(
             'Comma separated list of group primary keys to assign this '
             'user to.'
-        )
+        ), required=False, write_only=True
-    )
-
-    def create(self, validated_data):
-        validated_data['user'].groups.clear()
-        try:
-            pk_list = validated_data['group_pk_list'].split(',')
-
-            for group in Group.objects.filter(pk__in=pk_list):
-                try:
-                    AccessControlList.objects.check_access(
-                        permissions=(permission_group_view,),
-                        user=self.context['request'].user, obj=group
-                    )
-                except PermissionDenied:
-                    pass
-                else:
-                    validated_data['user'].groups.add(group)
-        except Exception as exception:
-            raise ValidationError(exception)
-
-        return {'group_pk_list': validated_data['group_pk_list']}
-
-
-class UserSerializer(serializers.HyperlinkedModelSerializer):
-    groups = GroupSerializer(many=True, read_only=True)
-    groups_pk_list = serializers.CharField(
-        help_text=_(
-            'List of group primary keys to which to add the user.'
-        ), required=False
     )
     password = serializers.CharField(
         required=False, style={'input_type': 'password'}, write_only=True
@@ -70,7 +45,10 @@ class UserSerializer(serializers.HyperlinkedModelSerializer):
 
     class Meta:
         extra_kwargs = {
-            'url': {'view_name': 'rest_api:user-detail'}
+            'url': {
+                'lookup_field': 'pk', 'lookup_url_kwarg': 'user_pk',
+                'view_name': 'rest_api:user-detail'
+            }
         }
         fields = (
             'first_name', 'date_joined', 'email', 'groups', 'groups_pk_list',
@@ -81,13 +59,19 @@ class UserSerializer(serializers.HyperlinkedModelSerializer):
         read_only_fields = ('groups', 'is_active', 'last_login', 'date_joined')
         write_only_fields = ('password', 'group_pk_list')
 
-    def _add_groups(self, instance):
+    def _add_groups(self, instance, groups_pk_list):
-        instance.groups.add(
+        instance.groups.clear()
-            *Group.objects.filter(pk__in=self.groups_pk_list.split(','))
+
+        queryset = AccessControlList.objects.restrict_queryset(
+            permission=permission_group_edit,
+            queryset=Group.objects.filter(pk__in=groups_pk_list.split(',')),
+            user=self.context['request'].user
         )
 
+        instance.groups.add(*queryset)
+
     def create(self, validated_data):
-        self.groups_pk_list = validated_data.pop('groups_pk_list', '')
+        groups_pk_list = validated_data.pop('groups_pk_list', '')
         password = validated_data.pop('password', None)
         instance = super(UserSerializer, self).create(validated_data)
 
@@ -95,13 +79,13 @@ class UserSerializer(serializers.HyperlinkedModelSerializer):
             instance.set_password(password)
             instance.save()
 
-        if self.groups_pk_list:
+        if groups_pk_list:
-            self._add_groups(instance=instance)
+            self._add_groups(instance=instance, groups_pk_list=groups_pk_list)
 
         return instance
 
     def update(self, instance, validated_data):
-        self.groups_pk_list = validated_data.pop('groups_pk_list', '')
+        groups_pk_list = validated_data.pop('groups_pk_list', '')
 
         if 'password' in validated_data:
             instance.set_password(validated_data['password'])
@@ -109,9 +93,8 @@ class UserSerializer(serializers.HyperlinkedModelSerializer):
 
         instance = super(UserSerializer, self).update(instance, validated_data)
 
-        if self.groups_pk_list:
+        if groups_pk_list:
-            instance.groups.clear()
+            self._add_groups(instance=instance, groups_pk_list=groups_pk_list)
-            self._add_groups(instance=instance)
 
         return instance
 

mayan/apps/user_management/tests/__init__.py

@@ -1 +1,2 @@
 from .literals import *  # NOQA
+from .mixins import *  # NOQA

mayan/apps/user_management/tests/literals.py

@@ -5,26 +5,28 @@ __all__ = (
     'TEST_USER_PASSWORD', 'TEST_USER_PASSWORD_EDITED', 'TEST_USER_USERNAME'
 )
 
-TEST_CASE_ADMIN_EMAIL = 'admin@example.com'
+TEST_CASE_ADMIN_EMAIL = 'case_admin@example.com'
-TEST_CASE_ADMIN_PASSWORD = 'test admin password'
+TEST_CASE_ADMIN_PASSWORD = 'test case admin password'
-TEST_CASE_ADMIN_USERNAME = 'test_admin'
+TEST_CASE_ADMIN_USERNAME = 'test_case_admin'
 
-TEST_CASE_GROUP_NAME = 'test group'
+TEST_CASE_GROUP_NAME = 'test case group'
-TEST_CASE_USER_EMAIL = 'user@example.com'
+
-TEST_CASE_USER_PASSWORD = 'test user password'
+TEST_CASE_USER_EMAIL = 'test_case_user@example.com'
-TEST_CASE_USER_USERNAME = 'test_user'
+TEST_CASE_USER_PASSWORD = 'test case user password'
+TEST_CASE_USER_USERNAME = 'test_case_user'
 
 TEST_GROUP_NAME = 'test group'
 TEST_GROUP_NAME_EDITED = 'test group edited'
 TEST_GROUP_2_NAME = 'test group 2'
 TEST_GROUP_2_NAME_EDITED = 'test group 2 edited'
-TEST_USER_EMAIL = 'user@example.com'
+
+TEST_USER_EMAIL = 'testuser@example.com'
 TEST_USER_PASSWORD = 'test user password'
 TEST_USER_PASSWORD_EDITED = 'test user password edited'
 TEST_USER_USERNAME = 'test_user'
 TEST_USER_USERNAME_EDITED = 'test_user_edited'
 
-TEST_USER_2_EMAIL = 'user2@example.com'
+TEST_USER_2_EMAIL = 'testuser2@example.com'
 TEST_USER_2_PASSWORD = 'test user 2 password'
 TEST_USER_2_PASSWORD_EDITED = 'test user 2 password edited'
 TEST_USER_2_USERNAME = 'test_user_2'

mayan/apps/user_management/tests/mixins.py

@@ -5,97 +5,117 @@ from django.contrib.auth.models import Group
 
 from .literals import (
     TEST_CASE_ADMIN_EMAIL, TEST_CASE_ADMIN_PASSWORD, TEST_CASE_ADMIN_USERNAME,
-    TEST_CASE_GROUP_NAME, TEST_CASE_USER_EMAIL, TEST_CASE_USER_PASSWORD,
+    TEST_CASE_GROUP_NAME, TEST_GROUP_NAME_EDITED, TEST_CASE_USER_EMAIL,
-    TEST_CASE_USER_USERNAME, TEST_GROUP_NAME, TEST_GROUP_2_NAME,
+    TEST_CASE_USER_PASSWORD, TEST_CASE_USER_USERNAME, TEST_GROUP_NAME,
-    TEST_GROUP_2_NAME_EDITED, TEST_USER_2_EMAIL, TEST_USER_2_PASSWORD,
+    TEST_USER_EMAIL, TEST_USER_USERNAME, TEST_USER_USERNAME_EDITED,
-    TEST_USER_EMAIL, TEST_USER_USERNAME, TEST_USER_PASSWORD,
+    TEST_USER_PASSWORD
-    TEST_USER_2_USERNAME, TEST_USER_2_USERNAME_EDITED
 )
 
+__all__ = ('GroupTestMixin', 'UserTestCaseMixin', 'UserTestMixin')
+
 
 class UserTestCaseMixin(object):
-    auto_login_admin = False
+    """
+    This TestCaseMixin is used to create an user and group to execute the
+    test case, these are used to just create an identity which is required by
+    most of the code in the project, these are not meant to be acted upon
+    (edited, deleted, etc). To create a test users or groups to modify, use
+    the UserTestMixin instead and the respective test_user and test_group.
+    The user and group created by this mixin will be prepended with
+    _test_case_{...}. The _test_case_user and _test_case_group are meant
+    to be used by other test case mixins like the ACLs test case mixin which
+    adds shorthand methods to create ACL entries to test access control.
+    """
+    auto_login_superuser = False
     auto_login_user = True
+    create_test_case_superuser = False
+    create_test_case_user = True
 
     def setUp(self):
         super(UserTestCaseMixin, self).setUp()
-        if self.auto_login_user:
+        if self.create_test_case_user:
-            self._test_case_user = get_user_model().objects.create_user(
+            self._create_test_case_user()
-                username=TEST_CASE_USER_USERNAME, email=TEST_CASE_USER_EMAIL,
+            self._create_test_case_group()
-                password=TEST_CASE_USER_PASSWORD
-            )
-            self.login_user()
-            self._test_case_group = Group.objects.create(name=TEST_GROUP_NAME)
             self._test_case_group.user_set.add(self._test_case_user)
-        elif self.auto_login_admin:
+
-            self._test_case_admin_user = get_user_model().objects.create_superuser(
+            if self.auto_login_user:
-                username=TEST_CASE_ADMIN_USERNAME, email=TEST_CASE_ADMIN_EMAIL,
+                self.login_user()
-                password=TEST_CASE_ADMIN_PASSWORD
+
-            )
+        elif self.create_test_case_superuser:
-            self.login_admin_user()
+            self._create_test_case_superuser()
+
+            if self.auto_login_superuser:
+                self.login_superuser()
 
     def tearDown(self):
         self.client.logout()
         super(UserTestCaseMixin, self).tearDown()
 
+    def _create_test_case_group(self):
+        self._test_case_group = Group.objects.create(name=TEST_CASE_GROUP_NAME)
+
+    def _create_test_case_superuser(self):
+        self._test_case_superuser = get_user_model().objects.create_superuser(
+            username=TEST_CASE_ADMIN_USERNAME, email=TEST_CASE_ADMIN_EMAIL,
+            password=TEST_CASE_ADMIN_PASSWORD
+        )
+
+    def _create_test_case_user(self):
+        self._test_case_user = get_user_model().objects.create_user(
+            username=TEST_CASE_USER_USERNAME, email=TEST_CASE_USER_EMAIL,
+            password=TEST_CASE_USER_PASSWORD
+        )
+
     def login(self, *args, **kwargs):
         logged_in = self.client.login(*args, **kwargs)
 
         return logged_in
 
-    def login_user(self):
+    def login_superuser(self):
-        self.login(
-            username=TEST_CASE_USER_USERNAME, password=TEST_CASE_USER_PASSWORD
-        )
-
-    def login_admin_user(self):
         self.login(
             username=TEST_CASE_ADMIN_USERNAME,
             password=TEST_CASE_ADMIN_PASSWORD
         )
 
+    def login_user(self):
+        self.login(
+            username=TEST_CASE_USER_USERNAME, password=TEST_CASE_USER_PASSWORD
+        )
+
     def logout(self):
         self.client.logout()
 
 
-class UserTestMixin(object):
+class GroupTestMixin(object):
     def _create_test_group(self):
-        self.test_group = Group.objects.create(name=TEST_GROUP_2_NAME)
+        self.test_group = Group.objects.create(name=TEST_GROUP_NAME)
 
     def _edit_test_group(self):
-        self.test_group.name = TEST_GROUP_2_NAME_EDITED
+        self.test_group.name = TEST_GROUP_NAME_EDITED
         self.test_group.save()
 
-    def _create_test_user(self):
-        self.test_user = get_user_model().objects.create(
-            username=TEST_USER_2_USERNAME, email=TEST_USER_2_EMAIL,
-            password=TEST_USER_2_PASSWORD
-        )
-
-    # Group views
-
     def _request_test_group_create_view(self):
         reponse = self.post(
             viewname='user_management:group_create', data={
-                'name': TEST_GROUP_2_NAME
+                'name': TEST_GROUP_NAME
             }
         )
-        self.test_group = Group.objects.filter(name=TEST_GROUP_2_NAME).first()
+        self.test_group = Group.objects.filter(name=TEST_GROUP_NAME).first()
         return reponse
 
     def _request_test_group_delete_view(self):
         return self.post(
             viewname='user_management:group_delete', kwargs={
-                'group_pk': self.test_group.pk
+                'group_id': self.test_group.pk
             }
         )
 
     def _request_test_group_edit_view(self):
         return self.post(
             viewname='user_management:group_edit', kwargs={
-                'group_pk': self.test_group.pk
+                'group_id': self.test_group.pk
             }, data={
-                'name': TEST_GROUP_2_NAME_EDITED
+                'name': TEST_GROUP_NAME_EDITED
             }
         )
 
@@ -105,41 +125,65 @@ class UserTestMixin(object):
     def _request_test_group_members_view(self):
         return self.get(
             viewname='user_management:group_members',
-            kwargs={'group_pk': self.test_group.pk}
+            kwargs={'group_id': self.test_group.pk}
         )
 
-    # User views
+
+class UserTestMixin(object):
+    def _create_test_superuser(self):
+        self.test_superuser = get_user_model().objects.create_superuser(
+            username=TEST_CASE_ADMIN_USERNAME, email=TEST_CASE_ADMIN_EMAIL,
+            password=TEST_CASE_ADMIN_PASSWORD
+        )
+
+    def _create_test_user(self):
+        self.test_user = get_user_model().objects.create(
+            username=TEST_USER_USERNAME, email=TEST_USER_EMAIL,
+            password=TEST_USER_PASSWORD
+        )
+
+    def _request_test_superuser_delete_view(self):
+        return self.post(
+            viewname='user_management:user_delete',
+            kwargs={'user_id': self.test_superuser.pk}
+        )
+
+    def _request_test_superuser_detail_view(self):
+        return self.get(
+            viewname='user_management:user_details',
+            kwargs={'user_id': self.test_superuser.pk}
+        )
 
     def _request_test_user_create_view(self):
         reponse = self.post(
             viewname='user_management:user_create', data={
-                'username': TEST_USER_2_USERNAME,
+                'username': TEST_USER_USERNAME,
-                'password': TEST_USER_2_PASSWORD
+                'password': TEST_USER_PASSWORD
             }
         )
 
         self.test_user = get_user_model().objects.filter(
-            username=TEST_USER_2_USERNAME
+            username=TEST_USER_USERNAME
         ).first()
         return reponse
 
     def _request_test_user_delete_view(self):
         return self.post(
             viewname='user_management:user_delete',
-            kwargs={'user_pk': self.test_user.pk}
+            kwargs={'user_id': self.test_user.pk}
         )
 
     def _request_test_user_edit_view(self):
         return self.post(
             viewname='user_management:user_edit', kwargs={
-                'user_pk': self.test_user.pk
+                'user_id': self.test_user.pk
             }, data={
-                'username': TEST_USER_2_USERNAME_EDITED
+                'username': TEST_USER_USERNAME_EDITED
             }
         )
 
     def _request_test_user_groups_view(self):
         return self.get(
             viewname='user_management:user_groups',
-            kwargs={'user_pk': self.test_user.pk}
+            kwargs={'user_id': self.test_user.pk}
         )

mayan/apps/user_management/tests/test_events.py

@@ -4,47 +4,60 @@ from actstream.models import Action
 
 from mayan.apps.common.tests import GenericViewTestCase
 
+from ..permissions import (
+    permission_group_create, permission_group_edit, permission_user_create,
+    permission_user_edit
+)
+
 from ..events import (
     event_group_created, event_group_edited, event_user_created,
     event_user_edited
 )
 
-from .mixins import UserTestMixin
+from .mixins import GroupTestMixin, UserTestMixin
 
 
-class GroupEventsTestCase(UserTestMixin, GenericViewTestCase):
+class GroupEventsTestCase(GroupTestMixin, UserTestMixin, GenericViewTestCase):
-    auto_create_group = False
-
     def test_group_create_event(self):
-        self.login_admin_user()
         Action.objects.all().delete()
+
+        self.grant_permission(
+            permission=permission_group_create
+        )
         self._request_test_group_create_view()
         self.assertEqual(Action.objects.last().target, self.test_group)
         self.assertEqual(Action.objects.last().verb, event_group_created.id)
 
     def test_group_edit_event(self):
-        self.login_admin_user()
         self._create_test_group()
         Action.objects.all().delete()
+
+        self.grant_access(
+            obj=self.test_group, permission=permission_group_edit
+        )
         self._request_test_group_edit_view()
         self.assertEqual(Action.objects.last().target, self.test_group)
         self.assertEqual(Action.objects.last().verb, event_group_edited.id)
 
 
 class UserEventsTestCase(UserTestMixin, GenericViewTestCase):
-    auto_create_group = False
-
     def test_user_create_event(self):
-        self.login_admin_user()
         Action.objects.all().delete()
+
+        self.grant_permission(
+            permission=permission_user_create
+        )
         self._request_test_user_create_view()
         self.assertEqual(Action.objects.last().target, self.test_user)
         self.assertEqual(Action.objects.last().verb, event_user_created.id)
 
     def test_user_edit_event(self):
-        self.login_admin_user()
         self._create_test_user()
         Action.objects.all().delete()
+
+        self.grant_access(
+            obj=self.test_user, permission=permission_user_edit
+        )
         self._request_test_user_edit_view()
         self.assertEqual(Action.objects.last().target, self.test_user)
         self.assertEqual(Action.objects.last().verb, event_user_edited.id)

mayan/apps/user_management/tests/test_models.py

@@ -7,5 +7,5 @@ from .mixins import UserTestMixin
 
 class UserTestCase(UserTestMixin, BaseTestCase):
     def test_natural_keys(self):
-        self._create_user()
+        self._create_test_user()
         self._test_database_conversion('auth', 'user_management')

mayan/apps/user_management/tests/test_views.py

@@ -6,70 +6,253 @@ from django.contrib.auth.models import Group
 from mayan.apps.common.tests import GenericViewTestCase
 from mayan.apps.documents.tests import GenericDocumentViewTestCase
 from mayan.apps.metadata.models import MetadataType
-from mayan.apps.metadata.permissions import permission_metadata_document_edit
+from mayan.apps.metadata.permissions import permission_document_metadata_edit
 from mayan.apps.metadata.tests.literals import (
     TEST_METADATA_TYPE_LABEL, TEST_METADATA_TYPE_NAME,
 )
 
 from ..permissions import (
-    permission_user_create, permission_user_delete, permission_user_edit
+    permission_group_create, permission_group_delete, permission_group_edit,
+    permission_group_view, permission_user_create, permission_user_delete,
+    permission_user_edit, permission_user_view
 )
 
 from .literals import (
-    TEST_USER_PASSWORD_EDITED, TEST_USER_USERNAME, TEST_USER_2_USERNAME
+    TEST_GROUP_NAME, TEST_GROUP_NAME_EDITED, TEST_USER_PASSWORD_EDITED,
+    TEST_USER_USERNAME
 )
-from .mixins import UserTestMixin
+from .mixins import GroupTestMixin, UserTestMixin
 
 TEST_USER_TO_DELETE_USERNAME = 'user_to_delete'
 
 
-class UserManagementViewTestCase(UserTestMixin, GenericViewTestCase):
+class GroupViewsTestCase(GroupTestMixin, UserTestMixin, GenericViewTestCase):
-    def setUp(self):
+    def test_group_create_view_no_permission(self):
-        super(UserManagementViewTestCase, self).setUp()
+        response = self._request_test_group_create_view()
-        self.login_user()
+        self.assertEqual(response.status_code, 403)
+        self.assertEqual(Group.objects.count(), 1)
 
-    #def _request_test_user_create_view(self):
+    def test_group_create_view_with_permission(self):
-    #    return self.post(
+        self.grant_permission(permission=permission_group_create)
-    #        viewname='user_management:user_create', data={
+        response = self._request_test_group_create_view()
-    #            'username': TEST_USER_2_USERNAME
+        self.assertEqual(response.status_code, 302)
-    #        }
+        self.assertEqual(Group.objects.count(), 2)
-    #    )
 
+    def test_group_delete_view_no_permission(self):
+        self._create_test_group()
+        response = self._request_test_group_delete_view()
+        self.assertEqual(response.status_code, 404)
+        self.assertEqual(Group.objects.count(), 2)
+
+    def test_group_delete_view_with_access(self):
+        self._create_test_group()
+        self.grant_access(
+            obj=self.test_group, permission=permission_group_delete
+        )
+        response = self._request_test_group_delete_view()
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(Group.objects.count(), 1)
+
+    def test_group_edit_view_no_permission(self):
+        self._create_test_group()
+        response = self._request_test_group_edit_view()
+        self.assertEqual(response.status_code, 404)
+        self.test_group.refresh_from_db()
+        self.assertEqual(self.test_group.name, TEST_GROUP_NAME)
+
+    def test_group_edit_view_with_access(self):
+        self._create_test_group()
+        self.grant_access(
+            obj=self.test_group, permission=permission_group_edit
+        )
+        response = self._request_test_group_edit_view()
+        self.assertEqual(response.status_code, 302)
+        self.test_group.refresh_from_db()
+        self.assertEqual(self.test_group.name, TEST_GROUP_NAME_EDITED)
+
+    def test_group_list_view_no_permission(self):
+        self._create_test_group()
+        response = self._request_test_group_list_view()
+        self.assertNotContains(
+            response=response, text=self.test_group.name, status_code=200
+        )
+
+    def test_group_list_view_with_permission(self):
+        self._create_test_group()
+        self.grant_access(
+            obj=self.test_group, permission=permission_group_view
+        )
+        response = self._request_test_group_list_view()
+        self.assertContains(
+            response=response, text=self.test_group.name, status_code=200
+        )
+
+    def test_group_members_view_no_permission(self):
+        self._create_test_user()
+        self._create_test_group()
+        self.test_user.groups.add(self.test_group)
+        response = self._request_test_group_members_view()
+        self.assertEqual(response.status_code, 404)
+
+    def test_group_members_view_with_group_access(self):
+        self._create_test_user()
+        self._create_test_group()
+        self.test_user.groups.add(self.test_group)
+        self.grant_access(
+            obj=self.test_group, permission=permission_group_edit
+        )
+        response = self._request_test_group_members_view()
+        self.assertContains(
+            response=response, text=self.test_group.name, status_code=200
+        )
+        self.assertNotContains(
+            response=response, text=self.test_user.username, status_code=200
+        )
+
+    def test_group_members_view_with_user_access(self):
+        self._create_test_user()
+        self._create_test_group()
+        self.test_user.groups.add(self.test_group)
+        self.grant_access(obj=self.test_user, permission=permission_user_edit)
+        response = self._request_test_group_members_view()
+        self.assertNotContains(
+            response=response, text=self.test_group.name, status_code=404
+        )
+
+    def test_group_members_view_with_full_access(self):
+        self._create_test_user()
+        self._create_test_group()
+        self.test_user.groups.add(self.test_group)
+        self.grant_access(
+            obj=self.test_group, permission=permission_group_edit
+        )
+        self.grant_access(obj=self.test_user, permission=permission_user_edit)
+        response = self._request_test_group_members_view()
+        self.assertContains(
+            response=response, text=self.test_user.username, status_code=200
+        )
+        self.assertContains(
+            response=response, text=self.test_group.name, status_code=200
+        )
+
+
+class UserViewsTestCase(GroupTestMixin, UserTestMixin, GenericViewTestCase):
     def test_user_create_view_no_permission(self):
+        user_count = get_user_model().objects.count()
+
         response = self._request_test_user_create_view()
         self.assertEqual(response.status_code, 403)
-        self.assertEqual(get_user_model().objects.count(), 2)
+
-        self.assertFalse(TEST_USER_2_USERNAME in get_user_model().objects.values_list('username', flat=True))
+        self.assertEqual(get_user_model().objects.count(), user_count)
+        self.assertFalse(TEST_USER_USERNAME in get_user_model().objects.values_list('username', flat=True))
 
     def test_user_create_view_with_permission(self):
+        user_count = get_user_model().objects.count()
+
         self.grant_permission(permission=permission_user_create)
         response = self._request_test_user_create_view()
         self.assertEqual(response.status_code, 302)
-        self.assertEqual(get_user_model().objects.count(), 3)
-        self.assertTrue(TEST_USER_2_USERNAME in get_user_model().objects.values_list('username', flat=True))
 
-    def _request_user_groups_view(self):
+        self.assertEqual(get_user_model().objects.count(), user_count + 1)
-        return self.post(
+        self.assertTrue(TEST_USER_USERNAME in get_user_model().objects.values_list('username', flat=True))
-            viewname='user_management:user_groups', args=(self.test_user.pk,)
+
+    def test_user_delete_view_no_access(self):
+        self._create_test_user()
+        user_count = get_user_model().objects.count()
+
+        response = self._request_test_user_delete_view()
+        self.assertEqual(response.status_code, 404)
+
+        self.assertEqual(get_user_model().objects.count(), user_count)
+
+    def test_user_delete_view_with_access(self):
+        self._create_test_user()
+        user_count = get_user_model().objects.count()
+
+        self.grant_access(
+            obj=self.test_user, permission=permission_user_delete
         )
+        response = self._request_test_user_delete_view()
+        self.assertEqual(response.status_code, 302)
+
+        self.assertEqual(get_user_model().objects.count(), user_count - 1)
+
+    def test_superuser_delete_view_with_access(self):
+        self._create_test_superuser()
+
+        superuser_count = get_user_model().objects.filter(is_superuser=True).count()
+        self.grant_access(
+            obj=self.test_superuser, permission=permission_user_delete
+        )
+        response = self._request_test_superuser_delete_view()
+        self.assertEqual(response.status_code, 404)
+        self.assertEqual(
+            get_user_model().objects.filter(is_superuser=True).count(),
+            superuser_count
+        )
+
+    def test_superuser_detail_view_with_access(self):
+        self._create_test_superuser()
+
+        self.grant_access(
+            obj=self.test_superuser, permission=permission_user_view
+        )
+        response = self._request_test_superuser_detail_view()
+        self.assertEqual(response.status_code, 404)
 
     def test_user_groups_view_no_permission(self):
         self._create_test_user()
-        response = self._request_user_groups_view()
+        self._create_test_group()
-        self.assertEqual(response.status_code, 403)
+        self.test_user.groups.add(self.test_group)
+        response = self._request_test_user_groups_view()
+        self.assertEqual(response.status_code, 404)
 
-    def test_user_groups_view_with_access(self):
+    def test_user_groups_view_with_group_access(self):
         self._create_test_user()
-        self.grant_access(permission=permission_user_edit, obj=self.test_user)
+        self._create_test_group()
+        self.test_user.groups.add(self.test_group)
+        self.grant_access(
+            obj=self.test_group, permission=permission_group_edit
+        )
+        response = self._request_test_user_groups_view()
+        self.assertNotContains(
+            response=response, text=self.test_user.username, status_code=404
+        )
 
-        response = self._request_user_groups_view()
+    def test_user_groups_view_with_user_access(self):
+        self._create_test_user()
+        self._create_test_group()
+        self.test_user.groups.add(self.test_group)
+        self.grant_access(obj=self.test_user, permission=permission_user_edit)
+        response = self._request_test_user_groups_view()
         self.assertContains(
             response=response, text=self.test_user.username, status_code=200
         )
+        self.assertNotContains(
+            response=response, text=self.test_group.name, status_code=200
+        )
+
+    def test_user_groups_view_with_full_access(self):
+        self._create_test_user()
+        self._create_test_group()
+        self.test_user.groups.add(self.test_group)
+        self.grant_access(
+            obj=self.test_group, permission=permission_group_edit
+        )
+        self.grant_access(obj=self.test_user, permission=permission_user_edit)
+        response = self._request_test_user_groups_view()
+
+        self.assertContains(
+            response=response, text=self.test_user.username, status_code=200
+        )
+        self.assertContains(
+            response=response, text=self.test_group.name, status_code=200
+        )
 
     def _request_set_password_view(self, password):
         return self.post(
-            viewname='user_management:user_set_password', args=(self.test_user.pk,),
+            viewname='user_management:user_set_password',
+            kwargs={'user_id': self.test_user.pk},
             data={
                 'new_password1': password, 'new_password2': password
             }
@@ -81,13 +264,14 @@ class UserManagementViewTestCase(UserTestMixin, GenericViewTestCase):
             password=TEST_USER_PASSWORD_EDITED
         )
 
-        self.assertEqual(response.status_code, 403)
+        self.assertEqual(response.status_code, 404)
 
         self.logout()
 
         with self.assertRaises(AssertionError):
             self.login(
-                username=TEST_USER_2_USERNAME, password=TEST_USER_PASSWORD_EDITED
+                username=self.test_user.username,
+                password=TEST_USER_PASSWORD_EDITED
             )
 
         response = self.get(viewname='user_management:current_user_details')
@@ -96,7 +280,7 @@ class UserManagementViewTestCase(UserTestMixin, GenericViewTestCase):
 
     def test_user_set_password_view_with_access(self):
         self._create_test_user()
-        self.grant_access(permission=permission_user_edit, obj=self.test_user)
+        self.grant_access(obj=self.test_user, permission=permission_user_edit)
 
         response = self._request_set_password_view(
             password=TEST_USER_PASSWORD_EDITED
@@ -106,7 +290,7 @@ class UserManagementViewTestCase(UserTestMixin, GenericViewTestCase):
 
         self.logout()
         self.login(
-            username=TEST_USER_2_USERNAME, password=TEST_USER_PASSWORD_EDITED
+            username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD_EDITED
         )
         response = self.get(viewname='user_management:current_user_details')
 
@@ -128,13 +312,14 @@ class UserManagementViewTestCase(UserTestMixin, GenericViewTestCase):
             password=TEST_USER_PASSWORD_EDITED
         )
 
-        self.assertEqual(response.status_code, 403)
+        self.assertEqual(response.status_code, 404)
 
         self.logout()
 
         with self.assertRaises(AssertionError):
             self.login(
-                username=TEST_USER_2_USERNAME, password=TEST_USER_PASSWORD_EDITED
+                username=self.test_user.username,
+                password=TEST_USER_PASSWORD_EDITED
             )
 
         response = self.get(viewname='user_management:current_user_details')
@@ -142,7 +327,7 @@ class UserManagementViewTestCase(UserTestMixin, GenericViewTestCase):
 
     def test_user_multiple_set_password_view_with_access(self):
         self._create_test_user()
-        self.grant_access(permission=permission_user_edit, obj=self.test_user)
+        self.grant_access(obj=self.test_user, permission=permission_user_edit)
 
         response = self._request_multiple_user_set_password_view(
             password=TEST_USER_PASSWORD_EDITED
@@ -152,30 +337,12 @@ class UserManagementViewTestCase(UserTestMixin, GenericViewTestCase):
 
         self.logout()
         self.login(
-            username=TEST_USER_2_USERNAME, password=TEST_USER_PASSWORD_EDITED
+            username=self.test_user.username, password=TEST_USER_PASSWORD_EDITED
         )
         response = self.get(viewname='user_management:current_user_details')
 
         self.assertEqual(response.status_code, 200)
 
-    def _request_user_delete_view(self):
-        return self.post(
-            viewname='user_management:user_delete', args=(self.test_user.pk,)
-        )
-
-    def test_user_delete_view_no_access(self):
-        self._create_test_user()
-        response = self._request_user_delete_view()
-        self.assertEqual(response.status_code, 302)
-        self.assertEqual(get_user_model().objects.count(), 3)
-
-    def test_user_delete_view_with_access(self):
-        self._create_test_user()
-        self.grant_access(permission=permission_user_delete, obj=self.test_user)
-        response = self._request_user_delete_view()
-        self.assertEqual(response.status_code, 302)
-        self.assertEqual(get_user_model().objects.count(), 2)
-
     def _request_user_multiple_delete_view(self):
         return self.post(
             viewname='user_management:user_multiple_delete', data={
@@ -185,16 +352,24 @@ class UserManagementViewTestCase(UserTestMixin, GenericViewTestCase):
 
     def test_user_multiple_delete_view_no_access(self):
         self._create_test_user()
+        user_count = get_user_model().objects.count()
+
         response = self._request_user_multiple_delete_view()
-        self.assertEqual(response.status_code, 302)
+        self.assertEqual(response.status_code, 404)
-        self.assertEqual(get_user_model().objects.count(), 3)
+
+        self.assertEqual(get_user_model().objects.count(), user_count)
 
     def test_user_multiple_delete_view_with_access(self):
         self._create_test_user()
-        self.grant_access(permission=permission_user_delete, obj=self.test_user)
+        user_count = get_user_model().objects.count()
+
+        self.grant_access(
+            obj=self.test_user, permission=permission_user_delete
+        )
         response = self._request_user_multiple_delete_view()
         self.assertEqual(response.status_code, 302)
-        self.assertEqual(get_user_model().objects.count(), 2)
+
+        self.assertEqual(get_user_model().objects.count(), user_count - 1)
 
 
 class MetadataLookupIntegrationTestCase(GenericDocumentViewTestCase):
@@ -212,13 +387,13 @@ class MetadataLookupIntegrationTestCase(GenericDocumentViewTestCase):
         self.metadata_type.lookup = '{{ users }}'
         self.metadata_type.save()
         self.document.metadata.create(metadata_type=self.metadata_type)
-        self.role.permissions.add(
+        self.grant_access(
-            permission_metadata_document_edit.stored_permission
+            obj=self.document, permission=permission_document_metadata_edit
         )
 
         response = self.get(
             viewname='metadata:document_metadata_edit',
-            kwargs={'pk': self.document.pk}
+            kwargs={'document_id': self.document.pk}
         )
         self.assertContains(
             response=response, text='<option value="{}">{}</option>'.format(
@@ -230,13 +405,13 @@ class MetadataLookupIntegrationTestCase(GenericDocumentViewTestCase):
         self.metadata_type.lookup = '{{ groups }}'
         self.metadata_type.save()
         self.document.metadata.create(metadata_type=self.metadata_type)
-        self.role.permissions.add(
+        self.grant_access(
-            permission_metadata_document_edit.stored_permission
+            obj=self.document, permission=permission_document_metadata_edit
         )
 
         response = self.get(
             viewname='metadata:document_metadata_edit',
-            kwargs={'pk': self.document.pk}
+            kwargs={'document_id': self.document.pk}
         )
 
         self.assertContains(

mayan/apps/user_management/urls.py

@@ -14,76 +14,93 @@ from .views import (
 )
 
 urlpatterns = [
-    url(r'^groups/list/$', GroupListView.as_view(), name='group_list'),
-    url(r'^groups/create/$', GroupCreateView.as_view(), name='group_create'),
     url(
-        r'^groups/(?P<pk>\d+)/edit/$', GroupEditView.as_view(),
+        regex=r'^groups/$', name='group_list', view=GroupListView.as_view()
-        name='group_edit'
     ),
     url(
-        r'^groups/(?P<pk>\d+)/delete/$', GroupDeleteView.as_view(),
+        regex=r'^groups/create/$', name='group_create',
-        name='group_delete'
+        view=GroupCreateView.as_view()
     ),
     url(
-        r'^groups/(?P<pk>\d+)/members/$', GroupMembersView.as_view(),
+        regex=r'^groups/(?P<group_id>\d+)/delete/$', name='group_delete',
-        name='group_members'
+        view=GroupDeleteView.as_view()
-    ),
-
-    url(
-        r'^users/current/$', CurrentUserDetailsView.as_view(),
-        name='current_user_details'
     ),
     url(
-        r'^users/current/edit/$', CurrentUserEditView.as_view(),
+        regex=r'^groups/(?P<group_id>\d+)/edit/$', name='group_edit',
-        name='current_user_edit'
+        view=GroupEditView.as_view()
-    ),
-    url(r'^users/list/$', UserListView.as_view(), name='user_list'),
-    url(r'^users/create/$', UserCreateView.as_view(), name='user_create'),
-    url(
-        r'^users/(?P<pk>\d+)/delete/$', UserDeleteView.as_view(),
-        name='user_delete'
-    ),
-    url(r'^users/(?P<pk>\d+)/edit/$', UserEditView.as_view(), name='user_edit'),
-    url(
-        r'^users/(?P<pk>\d+)/$', UserDetailsView.as_view(),
-        name='user_details'
     ),
     url(
-        r'^users/multiple/delete/$', UserDeleteView.as_view(),
+        regex=r'^groups/(?P<group_id>\d+)/members/$', name='group_members',
-        name='user_multiple_delete'
+        view=GroupMembersView.as_view()
     ),
     url(
-        r'^users/(?P<pk>\d+)/set_password/$', UserSetPasswordView.as_view(),
+        regex=r'^user/$', name='current_user_details',
-        name='user_set_password'
+        view=CurrentUserDetailsView.as_view()
     ),
     url(
-        r'^users/multiple/set_password/$', UserSetPasswordView.as_view(),
+        regex=r'^user/edit/$', name='current_user_edit',
-        name='user_multiple_set_password'
+        view=CurrentUserEditView.as_view()
     ),
     url(
-        r'^users/(?P<pk>\d+)/groups/$', UserGroupsView.as_view(),
+        regex=r'^users/$', name='user_list', view=UserListView.as_view()
-        name='user_groups'
     ),
     url(
-        r'^users/(?P<pk>\d+)/options/$',
+        regex=r'^users/create/$', name='user_create',
-        UserOptionsEditView.as_view(),
+        view=UserCreateView.as_view()
-        name='user_options'
     ),
+    url(
+        regex=r'^users/(?P<user_id>\d+)/$', name='user_details',
+        view=UserDetailsView.as_view()
+    ),
+    url(
+        regex=r'^users/(?P<user_id>\d+)/delete/$', name='user_delete',
+        view=UserDeleteView.as_view(),
+    ),
+    url(
+        regex=r'^users/(?P<user_id>\d+)/edit/$', name='user_edit',
+        view=UserEditView.as_view()
+    ),
+    url(
+        regex=r'^users/(?P<user_id>\d+)/groups/$', name='user_groups',
+        view=UserGroupsView.as_view()
+    ),
+    url(
+        regex=r'^users/(?P<user_id>\d+)/options/$', name='user_options',
+        view=UserOptionsEditView.as_view()
+    ),
+    url(
+        regex=r'^users/(?P<user_id>\d+)/set_password/$',
+        name='user_set_password', view=UserSetPasswordView.as_view()
+    ),
+    url(
+        regex=r'^users/multiple/delete/$', name='user_multiple_delete',
+        view=UserDeleteView.as_view()
+    ),
+    url(
+        regex=r'^users/multiple/set_password/$',
+        name='user_multiple_set_password', view=UserSetPasswordView.as_view()
+    )
 ]
 
 api_urls = [
-    url(r'^groups/$', APIGroupListView.as_view(), name='group-list'),
     url(
-        r'^groups/(?P<pk>[0-9]+)/$', APIGroupView.as_view(),
+        regex=r'^groups/$', name='group-list', view=APIGroupListView.as_view()
-        name='group-detail'
-    ),
-    url(r'^users/$', APIUserListView.as_view(), name='user-list'),
-    url(r'^users/(?P<pk>[0-9]+)/$', APIUserView.as_view(), name='user-detail'),
-    url(
-        r'^users/current/$', APICurrentUserView.as_view(), name='user-current'
     ),
     url(
-        r'^users/(?P<pk>[0-9]+)/groups/$', APIUserGroupList.as_view(),
+        regex=r'^groups/(?P<group_id>\d+)/$', name='group-detail',
-        name='users-group-list'
+        view=APIGroupView.as_view(),
+    ),
+    url(
+        regex=r'^user/$', name='user-current',
+        view=APICurrentUserView.as_view()
+    ),
+    url(regex=r'^users/$', name='user-list', view=APIUserListView.as_view()),
+    url(
+        regex=r'^users/(?P<user_id>\d+)/$', name='user-detail',
+        view=APIUserView.as_view()
+    ),
+    url(
+        regex=r'^users/(?P<user_id>\d+)/groups/$', name='users-group-list',
+        view=APIUserGroupList.as_view()
     ),
 ]

mayan/apps/user_management/utils.py

@@ -5,22 +5,22 @@ from django.contrib.auth import get_user_model
 from django.utils.translation import ugettext_lazy as _
 
 
-def get_groups():
+def get_user_label_text(context):
+    if not context['request'].user.is_authenticated:
+        return _('Anonymous')
+    else:
+        return context['request'].user.get_full_name() or context['request'].user
+
+
+def lookup_get_groups():
     Group = apps.get_model(app_label='auth', model_name='Group')
     return ','.join([group.name for group in Group.objects.all()])
 
 
-def get_users():
+def lookup_get_users():
     return ','.join(
         [
             user.get_full_name() or user.username
             for user in get_user_model().objects.all()
         ]
     )
-
-
-def get_user_label_text(context):
-    if not context['request'].user.is_authenticated:
-        return _('Anonymous')
-    else:
-        return context['request'].user.get_full_name() or context['request'].user

mayan/apps/user_management/views.py

@@ -5,19 +5,21 @@ from django.contrib.auth import get_user_model
 from django.contrib.auth.forms import SetPasswordForm
 from django.contrib.auth.models import Group
 from django.contrib.contenttypes.models import ContentType
-from django.core.exceptions import PermissionDenied
+from django.db import transaction
 from django.http import HttpResponseRedirect
 from django.shortcuts import get_object_or_404
 from django.template import RequestContext
 from django.urls import reverse, reverse_lazy
 from django.utils.translation import ungettext, ugettext_lazy as _
 
-from mayan.apps.common.views import (
+from mayan.apps.acls.models import AccessControlList
+from mayan.apps.common.generics import (
     AssignRemoveView, MultipleObjectConfirmActionView,
     MultipleObjectFormActionView, SingleObjectCreateView,
     SingleObjectDeleteView, SingleObjectDetailView, SingleObjectEditView,
     SingleObjectListView
 )
+from mayan.apps.common.mixins import ExternalObjectMixin
 
 from .events import (
     event_group_created, event_group_edited, event_user_created,
@@ -52,7 +54,9 @@ class CurrentUserDetailsView(SingleObjectDetailView):
 class CurrentUserEditView(SingleObjectEditView):
     extra_context = {'object': None, 'title': _('Edit current user details')}
     form_class = UserForm
-    post_action_redirect = reverse_lazy('user_management:current_user_details')
+    post_action_redirect = reverse_lazy(
+        viewname='user_management:current_user_details'
+    )
 
     def get_object(self):
         return self.request.user
@@ -62,38 +66,50 @@ class GroupCreateView(SingleObjectCreateView):
     extra_context = {'title': _('Create new group')}
     fields = ('name',)
     model = Group
-    post_action_redirect = reverse_lazy('user_management:group_list')
+    post_action_redirect = reverse_lazy(viewname='user_management:group_list')
     view_permission = permission_group_create
 
     def form_valid(self, form):
-        group = form.save()
+        with transaction.atomic():
+            result = super(GroupCreateView, self).form_valid(form=form)
+            event_group_created.commit(
+                actor=self.request.user, target=self.object
+            )
+        return result
 
-        event_group_created.commit(
-            actor=self.request.user, target=group
-        )
 
-        messages.success(
+class GroupDeleteView(SingleObjectDeleteView):
-            self.request, _('Group "%s" created successfully.') % group
+    model = Group
-        )
+    object_permission = permission_group_delete
-        return super(GroupCreateView, self).form_valid(form=form)
+    pk_url_kwarg = 'group_id'
+    post_action_redirect = reverse_lazy(viewname='user_management:group_list')
+
+    def get_extra_context(self):
+        return {
+            'object': self.object,
+            'title': _('Delete the group: %s?') % self.object,
+        }
 
 
 class GroupEditView(SingleObjectEditView):
     fields = ('name',)
     model = Group
     object_permission = permission_group_edit
-    post_action_redirect = reverse_lazy('user_management:group_list')
+    pk_url_kwarg = 'group_id'
+    post_action_redirect = reverse_lazy(viewname='user_management:group_list')
 
     def form_valid(self, form):
-        event_group_edited.commit(
+        with transaction.atomic():
-            actor=self.request.user, target=self.get_object()
+            result = super(GroupEditView, self).form_valid(form=form)
-        )
+            event_group_edited.commit(
-        return super(GroupEditView, self).form_valid(form=form)
+                actor=self.request.user, target=self.object
+            )
+        return result
 
     def get_extra_context(self):
         return {
-            'object': self.get_object(),
+            'object': self.object,
-            'title': _('Edit group: %s') % self.get_object(),
+            'title': _('Edit group: %s') % self.object,
         }
 
 
@@ -120,29 +136,20 @@ class GroupListView(SingleObjectListView):
         }
 
 
-class GroupDeleteView(SingleObjectDeleteView):
+class GroupMembersView(ExternalObjectMixin, AssignRemoveView):
-    model = Group
-    object_permission = permission_group_delete
-    post_action_redirect = reverse_lazy('user_management:group_list')
-
-    def get_extra_context(self):
-        return {
-            'object': self.get_object(),
-            'title': _('Delete the group: %s?') % self.get_object(),
-        }
-
-
-class GroupMembersView(AssignRemoveView):
     decode_content_type = True
+    external_object_class = Group
+    external_object_permission = permission_group_edit
+    external_object_pk_url_kwarg = 'group_id'
     left_list_title = _('Available users')
-    right_list_title = _('Users in group')
     object_permission = permission_group_edit
+    right_list_title = _('Users in group')
 
     @staticmethod
     def generate_choices(choices):
         results = []
         for choice in choices:
-            ct = ContentType.objects.get_for_model(choice)
+            ct = ContentType.objects.get_for_model(model=choice)
             label = choice.get_full_name() if choice.get_full_name() else choice
 
             results.append(('%s,%s' % (ct.model, choice.pk), '%s' % (label)))
@@ -151,31 +158,43 @@ class GroupMembersView(AssignRemoveView):
         return sorted(results, key=lambda x: x[1])
 
     def add(self, item):
-        self.get_object().user_set.add(item)
+        self.object.user_set.add(item)
+
+    def dispatch(self, *args, **kwargs):
+        self.object = self.get_object()
+        return super(GroupMembersView, self).dispatch(*args, **kwargs)
 
     def get_extra_context(self):
         return {
-            'object': self.get_object(),
+            'object': self.object,
-            'title': _('Users of group: %s') % self.get_object()
+            'title': _('Users of group: %s') % self.object
         }
 
     def get_object(self):
-        return get_object_or_404(klass=Group, pk=self.kwargs['pk'])
+        return self.get_external_object()
 
     def left_list(self):
-        return GroupMembersView.generate_choices(
+        queryset = AccessControlList.objects.restrict_queryset(
-            get_user_model().objects.exclude(
+            permission=permission_user_edit,
-                groups=self.get_object()
+            queryset=get_user_model().objects.exclude(
-            ).exclude(is_staff=True).exclude(is_superuser=True)
+                groups=self.object
+            ).exclude(is_staff=True).exclude(is_superuser=True),
+            user=self.request.user
         )
 
+        return GroupMembersView.generate_choices(choices=queryset)
+
     def right_list(self):
-        return GroupMembersView.generate_choices(
+        queryset = AccessControlList.objects.restrict_queryset(
-            self.get_object().user_set.all()
+            permission=permission_user_edit,
+            queryset=self.object.user_set.all(),
+            user=self.request.user
         )
 
+        return GroupMembersView.generate_choices(choices=queryset)
+
     def remove(self, item):
-        self.get_object().user_set.remove(item)
+        self.object.user_set.remove(item)
 
 
 class UserCreateView(SingleObjectCreateView):
@@ -186,25 +205,24 @@ class UserCreateView(SingleObjectCreateView):
     view_permission = permission_user_create
 
     def form_valid(self, form):
-        user = form.save(commit=False)
+        with transaction.atomic():
-        user.set_unusable_password()
+            super(UserCreateView, self).form_valid(form=form)
-        user.save()
+            event_user_created.commit(
+                actor=self.request.user, target=self.object
+            )
 
-        event_user_created.commit(
-            actor=self.request.user, target=user
-        )
-
-        messages.success(
-            self.request, _('User "%s" created successfully.') % user
-        )
         return HttpResponseRedirect(
-            reverse('user_management:user_set_password', args=(user.pk,))
+            reverse(
+                viewname='user_management:user_set_password',
+                kwargs={'user_id': self.object.pk}
+            )
         )
 
 
 class UserDeleteView(MultipleObjectConfirmActionView):
     object_permission = permission_user_delete
-    queryset = get_user_model().objects.filter(
+    pk_url_kwarg = 'user_id'
+    source_queryset = get_user_model().objects.filter(
         is_superuser=False, is_staff=False
     )
     success_message = _('User delete request performed on %(count)d user')
@@ -213,13 +231,13 @@ class UserDeleteView(MultipleObjectConfirmActionView):
     )
 
     def get_extra_context(self):
-        queryset = self.get_queryset()
+        queryset = self.get_object_list()
 
         result = {
             'title': ungettext(
-                'Delete user',
+                singular='Delete user',
-                'Delete users',
+                plural='Delete users',
-                queryset.count()
+                number=queryset.count()
             )
         }
 
@@ -235,26 +253,18 @@ class UserDeleteView(MultipleObjectConfirmActionView):
 
     def object_action(self, form, instance):
         try:
-            if instance.is_superuser or instance.is_staff:
+            instance.delete()
-                messages.error(
+            messages.success(
-                    self.request,
+                message=_(
-                    _(
+                    'User "%s" deleted successfully.'
-                        'Super user and staff user deleting is not '
+                ) % instance, request=self.request
-                        'allowed, use the admin interface for these cases.'
+            )
-                    )
-                )
-            else:
-                instance.delete()
-                messages.success(
-                    self.request, _(
-                        'User "%s" deleted successfully.'
-                    ) % instance
-                )
         except Exception as exception:
             messages.error(
-                self.request, _(
+                message=_(
                     'Error deleting user "%(user)s": %(error)s'
-                ) % {'user': instance, 'error': exception}
+                ) % {'user': instance, 'error': exception},
+                request=self.request
             )
 
 
@@ -264,7 +274,8 @@ class UserDetailsView(SingleObjectDetailView):
         'date_joined', 'groups',
     )
     object_permission = permission_user_view
-    queryset = get_user_model().objects.filter(
+    pk_url_kwarg = 'user_id'
+    source_queryset = get_user_model().objects.filter(
         is_superuser=False, is_staff=False
     )
 
@@ -278,58 +289,72 @@ class UserDetailsView(SingleObjectDetailView):
 class UserEditView(SingleObjectEditView):
     fields = ('username', 'first_name', 'last_name', 'email', 'is_active',)
     object_permission = permission_user_edit
-    post_action_redirect = reverse_lazy('user_management:user_list')
+    pk_url_kwarg = 'user_id'
-    queryset = get_user_model().objects.filter(
+    post_action_redirect = reverse_lazy(viewname='user_management:user_list')
+    source_queryset = get_user_model().objects.filter(
         is_superuser=False, is_staff=False
     )
 
     def form_valid(self, form):
-        event_user_edited.commit(
+        with transaction.atomic():
-            actor=self.request.user, target=self.get_object()
+            result = super(UserEditView, self).form_valid(form=form)
-        )
+            event_user_edited.commit(
-        return super(UserEditView, self).form_valid(form=form)
+                actor=self.request.user, target=self.object
+            )
+
+        return result
 
     def get_extra_context(self):
         return {
-            'object': self.get_object(),
+            'object': self.object,
-            'title': _('Edit user: %s') % self.get_object(),
+            'title': _('Edit user: %s') % self.object,
         }
 
 
-class UserGroupsView(AssignRemoveView):
+class UserGroupsView(ExternalObjectMixin, AssignRemoveView):
     decode_content_type = True
+    external_object_queryset = get_user_model().objects.filter(
+        is_staff=False, is_superuser=False
+    )
+    external_object_permission = permission_user_edit
+    external_object_pk_url_kwarg = 'user_id'
     left_list_title = _('Available groups')
     right_list_title = _('Groups joined')
-    object_permission = permission_user_edit
 
     def add(self, item):
-        item.user_set.add(self.get_object())
+        item.user_set.add(self.object)
+
+    def dispatch(self, *args, **kwargs):
+        self.object = self.get_object()
+        return super(UserGroupsView, self).dispatch(*args, **kwargs)
 
     def get_extra_context(self):
         return {
-            'object': self.get_object(),
+            'object': self.object,
-            'title': _('Groups of user: %s') % self.get_object()
+            'title': _('Groups of user: %s') % self.object
         }
 
     def get_object(self):
-        return get_object_or_404(
+        return self.get_external_object()
-            klass=get_user_model().objects.filter(
-                is_superuser=False, is_staff=False
-            ), pk=self.kwargs['pk']
-        )
 
     def left_list(self):
-        return AssignRemoveView.generate_choices(
+        queryset = AccessControlList.objects.restrict_queryset(
-            Group.objects.exclude(user=self.get_object())
+            permission=permission_group_edit,
+            queryset=Group.objects.exclude(user=self.object),
+            user=self.request.user
         )
+        return AssignRemoveView.generate_choices(choices=queryset)
 
     def right_list(self):
-        return AssignRemoveView.generate_choices(
+        queryset = AccessControlList.objects.restrict_queryset(
-            Group.objects.filter(user=self.get_object())
+            permission=permission_group_edit,
+            queryset=Group.objects.filter(user=self.object),
+            user=self.request.user
         )
+        return AssignRemoveView.generate_choices(choices=queryset)
 
     def remove(self, item):
-        item.user_set.remove(self.get_object())
+        item.user_set.remove(self.object)
 
 
 class UserListView(SingleObjectListView):
@@ -350,7 +375,7 @@ class UserListView(SingleObjectListView):
             'title': _('Users'),
         }
 
-    def get_object_list(self):
+    def get_source_queryset(self):
         return get_user_model().objects.exclude(
             is_superuser=True
         ).exclude(is_staff=True).order_by('last_name', 'first_name')
@@ -372,13 +397,13 @@ class UserOptionsEditView(SingleObjectEditView):
         return self.get_user().user_options
 
     def get_post_action_redirect(self):
-        return reverse('user_management:user_list')
+        return reverse(viewname='user_management:user_list')
 
     def get_user(self):
         return get_object_or_404(
             klass=get_user_model().objects.filter(
                 is_superuser=False, is_staff=False
-            ), pk=self.kwargs['pk']
+            ), pk=self.kwargs['user_id']
         )
 
 
@@ -386,66 +411,57 @@ class UserSetPasswordView(MultipleObjectFormActionView):
     form_class = SetPasswordForm
     model = get_user_model()
     object_permission = permission_user_edit
+    pk_url_kwarg = 'user_id'
+    source_queryset = get_user_model().objects.filter(
+        is_superuser=False, is_staff=False
+    )
     success_message = _('Password change request performed on %(count)d user')
     success_message_plural = _(
         'Password change request performed on %(count)d users'
     )
 
     def get_extra_context(self):
-        queryset = self.get_queryset()
+        queryset = self.get_object_list()
 
         result = {
             'submit_label': _('Submit'),
             'title': ungettext(
-                'Change user password',
+                singular='Change the password of the %(count)d selected user',
-                'Change users passwords',
+                plural='Change the password of the %(count)d selected users',
-                queryset.count()
+                number=queryset.count()
-            )
+            ) % {'count': queryset.count()}
         }
 
         if queryset.count() == 1:
             result.update(
                 {
                     'object': queryset.first(),
-                    'title': _('Change password for user: %s') % queryset.first()
+                    'title': _(
+                        'Change the password of user: %s'
+                    ) % queryset.first()
                 }
             )
 
         return result
 
     def get_form_extra_kwargs(self):
-        queryset = self.get_queryset()
+        queryset = self.get_object_list()
-        result = {}
+        return {'user': queryset.first()}
-        if queryset:
-            result['user'] = queryset.first()
-            return result
-        else:
-            raise PermissionDenied
 
     def object_action(self, form, instance):
         try:
-            if instance.is_superuser or instance.is_staff:
+            instance.set_password(form.cleaned_data['new_password1'])
-                messages.error(
+            instance.save()
-                    self.request,
+            messages.success(
-                    _(
+                message=_(
-                        'Super user and staff user password '
+                    'Successful password reset for user: %s.'
-                        'reseting is not allowed, use the admin '
+                ) % instance, request=self.request
-                        'interface for these cases.'
+            )
-                    )
-                )
-            else:
-                instance.set_password(form.cleaned_data['new_password1'])
-                instance.save()
-                messages.success(
-                    self.request, _(
-                        'Successful password reset for user: %s.'
-                    ) % instance
-                )
         except Exception as exception:
             messages.error(
-                self.request, _(
+                message=_(
                     'Error reseting password for user "%(user)s": %(error)s'
                 ) % {
                     'user': instance, 'error': exception
-                }
+                }, request=self.request
             )