diff --git a/mayan/apps/user_management/api_views.py b/mayan/apps/user_management/api_views.py index 09eadbef79..ec2627efba 100644 --- a/mayan/apps/user_management/api_views.py +++ b/mayan/apps/user_management/api_views.py @@ -56,7 +56,7 @@ class GroupAPIViewSet(MayanAPIModelViewSet): instance = self.get_object() serializer = self.get_serializer(data=request.data) serializer.is_valid(raise_exception=True) - serializer.user_add(instance=instance) + serializer.users_add(instance=instance) headers = self.get_success_headers(data=serializer.data) return Response( serializer.data, status=status.HTTP_200_OK, headers=headers @@ -89,7 +89,7 @@ class GroupAPIViewSet(MayanAPIModelViewSet): instance = self.get_object() serializer = self.get_serializer(data=request.data) serializer.is_valid(raise_exception=True) - serializer.user_remove(instance=instance) + serializer.users_remove(instance=instance) headers = self.get_success_headers(data=serializer.data) return Response( serializer.data, status=status.HTTP_200_OK, headers=headers @@ -100,7 +100,9 @@ class UserAPIViewSet(MayanAPIModelViewSet): lookup_url_kwarg = 'user_id' object_permission_map = { 'destroy': permission_user_delete, - 'group-list': permission_user_view, + 'group_add': permission_user_edit, + 'group_list': permission_user_view, + 'group_remove': permission_user_edit, 'list': permission_user_view, 'partial_update': permission_user_edit, 'retrieve': permission_user_view, @@ -115,13 +117,13 @@ class UserAPIViewSet(MayanAPIModelViewSet): @action( detail=True, lookup_url_kwarg='user_id', methods=('post',), serializer_class=UserGroupAddRemoveSerializer, - url_name='group-add', url_path='group/add' + url_name='group-add', url_path='groups/add' ) def group_add(self, request, *args, **kwargs): instance = self.get_object() serializer = self.get_serializer(data=request.data) serializer.is_valid(raise_exception=True) - serializer.group_add(instance=instance) + serializer.groups_add(instance=instance) headers = self.get_success_headers(data=serializer.data) return Response( serializer.data, status=status.HTTP_200_OK, headers=headers @@ -154,7 +156,7 @@ class UserAPIViewSet(MayanAPIModelViewSet): instance = self.get_object() serializer = self.get_serializer(data=request.data) serializer.is_valid(raise_exception=True) - serializer.group_remove(instance=instance) + serializer.groups_remove(instance=instance) headers = self.get_success_headers(data=serializer.data) return Response( serializer.data, status=status.HTTP_200_OK, headers=headers diff --git a/mayan/apps/user_management/apps.py b/mayan/apps/user_management/apps.py index 3398cd0658..3580a01c74 100644 --- a/mayan/apps/user_management/apps.py +++ b/mayan/apps/user_management/apps.py @@ -34,8 +34,10 @@ from .links import ( separator_user_label ) from .methods import ( - method_group_get_users, method_group_user_add, method_group_user_remove, - method_user_get_absolute_url, method_user_get_groups + get_method_group_save, get_method_user_save, method_group_get_users, + method_group_users_add, method_group_users_remove, + method_user_get_absolute_url, method_user_get_groups, + method_user_groups_add, method_user_groups_remove ) from .permissions import ( permission_group_delete, permission_group_edit, @@ -68,11 +70,12 @@ class UserManagementApp(MayanAppConfig): # Silence UnorderedObjectListWarning # "Pagination may yield inconsistent result" - # Remove on Django 2.x + # TODO: Remove on Django 2.x Group._meta.ordering = ('name',) Group.add_to_class(name='get_users', value=method_group_get_users) - Group.add_to_class(name='user_add', value=method_group_user_add) - Group.add_to_class(name='user_remove', value=method_group_user_remove) + Group.add_to_class(name='save', value=get_method_group_save()) + Group.add_to_class(name='users_add', value=method_group_users_add) + Group.add_to_class(name='users_remove', value=method_group_users_remove) MetadataLookup( description=_('All the groups.'), name='groups', @@ -134,12 +137,23 @@ class UserManagementApp(MayanAppConfig): source=User, widget=TwoStateWidget ) + # Silence UnorderedObjectListWarning + # "Pagination may yield inconsistent result" + # TODO: Remove on Django 2.x + User._meta.ordering = ('pk',) User.add_to_class( name='get_absolute_url', value=method_user_get_absolute_url ) User.add_to_class( name='get_groups', value=method_user_get_groups ) + User.add_to_class( + name='groups_add', value=method_user_groups_add + ) + User.add_to_class( + name='groups_remove', value=method_user_groups_remove + ) + User.add_to_class(name='save', value=get_method_user_save()) menu_list_facet.bind_links( links=( diff --git a/mayan/apps/user_management/methods.py b/mayan/apps/user_management/methods.py index 9f572ac68d..5db28def18 100644 --- a/mayan/apps/user_management/methods.py +++ b/mayan/apps/user_management/methods.py @@ -1,11 +1,15 @@ from __future__ import unicode_literals from django.apps import apps +from django.contrib.auth import get_user_model from django.db import transaction from django.shortcuts import reverse -from .events import event_group_edited, event_user_edited -from .permissions import permission_user_view +from .events import ( + event_group_created, event_group_edited, event_user_created, + event_user_edited +) +from .permissions import permission_group_view, permission_user_view from .querysets import get_user_queryset @@ -20,26 +24,50 @@ def method_group_get_users(self, _user): ) -def method_group_user_add(self, user, _user): +def get_method_group_save(): + Group = apps.get_model(app_label='auth', model_name='Group') + group_save_original = Group.save + + def method_group_save(self, *args, **kwargs): + _group = kwargs.pop('_group', None) + + with transaction.atomic(): + is_new = not self.pk + group_save_original(self, *args, **kwargs) + if is_new: + event_group_created.commit( + actor=_group, target=self + ) + else: + event_group_edited.commit( + actor=_group, target=self + ) + + return method_group_save + + +def method_group_users_add(self, users, _user): with transaction.atomic(): - self.user_set.add(user) event_group_edited.commit( actor=_user, target=self ) - event_user_edited.commit( - actor=_user, target=user - ) + for user in users: + self.user_set.add(user) + event_user_edited.commit( + actor=_user, target=user + ) -def method_group_user_remove(self, user, _user): +def method_group_users_remove(self, users, _user): with transaction.atomic(): - self.user_set.remove(user) event_group_edited.commit( actor=_user, target=self ) - event_user_edited.commit( - actor=_user, target=user - ) + for user in users: + self.user_set.remove(user) + event_user_edited.commit( + actor=_user, target=user + ) def method_user_get_absolute_url(self): @@ -54,6 +82,51 @@ def method_user_get_groups(self, _user): ) return AccessControlList.objects.restrict_queryset( - permission=permission_user_view, queryset=self.groups.all(), + permission=permission_group_view, queryset=self.groups.all(), user=_user ) + + +def method_user_groups_add(self, groups, _user): + with transaction.atomic(): + event_user_edited.commit( + actor=_user, target=self + ) + for group in groups: + self.groups.add(group) + event_group_edited.commit( + actor=_user, target=group + ) + + +def method_user_groups_remove(self, groups, _user): + with transaction.atomic(): + event_user_edited.commit( + actor=_user, target=self + ) + for group in groups: + self.groups.remove(group) + event_group_edited.commit( + actor=_user, target=group + ) + + +def get_method_user_save(): + user_save_original = get_user_model().save + + def method_user_save(self, *args, **kwargs): + _user = kwargs.pop('_user', None) + + with transaction.atomic(): + is_new = not self.pk + user_save_original(self, *args, **kwargs) + if is_new: + event_user_created.commit( + actor=_user, target=self + ) + else: + event_user_edited.commit( + actor=_user, target=self + ) + + return method_user_save diff --git a/mayan/apps/user_management/serializers.py b/mayan/apps/user_management/serializers.py index b1ff9183a0..6c98a01201 100644 --- a/mayan/apps/user_management/serializers.py +++ b/mayan/apps/user_management/serializers.py @@ -46,7 +46,7 @@ class GroupUserAddRemoveSerializer(ExternalObjectListSerializerMixin, serializer 'Primary key of the user that will be added or removed.' ), required=False, write_only=True ) - users_id_list = serializers.CharField( + user_id_list = serializers.CharField( help_text=_( 'Comma separated list of user primary keys that will be added or ' 'removed.' @@ -59,15 +59,17 @@ class GroupUserAddRemoveSerializer(ExternalObjectListSerializerMixin, serializer external_object_list_pk_field = 'user_id' external_object_list_pk_list_field = 'user_id_list' - def user_add(self, instance): - queryset = self.get_external_object_list() - for user in queryset: - instance.user_add(user=user, _user=self.context['request'].user) + def users_add(self, instance): + instance.users_add( + users=self.get_external_object_list(), + _user=self.context['request'].user + ) - def user_remove(self, instance): - queryset = self.get_external_object_list() - for user in queryset: - instance.user_remove(user=user, _user=self.context['request'].user) + def users_remove(self, instance): + instance.users_remove( + users=self.get_external_object_list(), + _user=self.context['request'].user + ) class UserGroupAddRemoveSerializer(ExternalObjectListSerializerMixin, serializers.Serializer): @@ -76,7 +78,7 @@ class UserGroupAddRemoveSerializer(ExternalObjectListSerializerMixin, serializer 'Primary key of the group that will be added or removed.' ), required=False, write_only=True ) - groups_id_list = serializers.CharField( + group_id_list = serializers.CharField( help_text=_( 'Comma separated list of group primary keys that will be added or ' 'removed.' @@ -89,15 +91,17 @@ class UserGroupAddRemoveSerializer(ExternalObjectListSerializerMixin, serializer external_object_list_pk_field = 'group_id' external_object_list_pk_list_field = 'group_id_list' - def group_add(self, instance): - queryset = self.get_external_object_list() - for group in queryset: - instance.group_add(group=group, _group=self.context['request'].group) + def groups_add(self, instance): + instance.groups_add( + groups=self.get_external_object_list(), + _user=self.context['request'].user + ) - def group_remove(self, instance): - queryset = self.get_external_object_list() - for group in queryset: - instance.group_remove(group=group, _group=self.context['request'].group) + def groups_remove(self, instance): + instance.groups_remove( + groups=self.get_external_object_list(), + _user=self.context['request'].user + ) class UserSerializer(serializers.HyperlinkedModelSerializer): diff --git a/mayan/apps/user_management/tests/__init__.py b/mayan/apps/user_management/tests/__init__.py index 8b13789179..e69de29bb2 100644 --- a/mayan/apps/user_management/tests/__init__.py +++ b/mayan/apps/user_management/tests/__init__.py @@ -1 +0,0 @@ - diff --git a/mayan/apps/user_management/tests/mixins.py b/mayan/apps/user_management/tests/mixins.py index 6a5dde7534..625fed83f5 100644 --- a/mayan/apps/user_management/tests/mixins.py +++ b/mayan/apps/user_management/tests/mixins.py @@ -8,7 +8,7 @@ from .literals import ( TEST_CASE_GROUP_NAME, TEST_GROUP_NAME_EDITED, TEST_CASE_USER_EMAIL, TEST_CASE_USER_PASSWORD, TEST_CASE_USER_USERNAME, TEST_GROUP_NAME, TEST_USER_EMAIL, TEST_USER_USERNAME, TEST_USER_USERNAME_EDITED, - TEST_USER_PASSWORD + TEST_USER_PASSWORD, TEST_USER_PASSWORD_EDITED ) __all__ = ('GroupTestMixin', 'UserTestCaseMixin', 'UserTestMixin') @@ -59,12 +59,14 @@ class UserTestCaseMixin(object): username=TEST_CASE_SUPERUSER_USERNAME, email=TEST_CASE_SUPERUSER_EMAIL, password=TEST_CASE_SUPERUSER_PASSWORD ) + self._test_case_superuser.clear_password = TEST_CASE_SUPERUSER_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 ) + self._test_case_user.clear_password = TEST_CASE_USER_PASSWORD def login(self, *args, **kwargs): logged_in = self.client.login(*args, **kwargs) @@ -86,6 +88,67 @@ class UserTestCaseMixin(object): self.client.logout() +class GroupAPITestMixin(object): + def _request_test_group_create_api_view(self): + result = self.post( + viewname='rest_api:group-list', data={ + 'name': TEST_GROUP_NAME + } + ) + if 'id' in result.json(): + self.test_group = Group.objects.get(pk=result.json()['id']) + + return result + + def _request_test_group_delete_api_view(self): + return self.delete( + viewname='rest_api:group-detail', + kwargs={'group_id': self.test_group.pk} + ) + + def _request_test_group_detail_api_view(self): + return self.get( + viewname='rest_api:group-detail', + kwargs={'group_id': self.test_group.pk} + ) + + def _request_test_group_edit_patch_api_view(self): + return self.patch( + viewname='rest_api:group-detail', + kwargs={'group_id': self.test_group.pk}, + data={ + 'name': TEST_GROUP_NAME_EDITED + } + ) + + def _request_test_group_list_api_view(self): + return self.get(viewname='rest_api:group-list') + + def _request_test_group_user_add_api_view(self): + return self.post( + viewname='rest_api:group-user-add', + kwargs={'group_id': self.test_group.pk}, + data={ + 'user_id': self.test_user.pk + } + ) + + def _request_test_group_user_list_api_view(self): + return self.get( + viewname='rest_api:group-user-list', + kwargs={'group_id': self.test_group.pk}, + ) + + def _request_test_group_user_remove_api_view(self): + return self.post( + viewname='rest_api:group-user-remove', + kwargs={'group_id': self.test_group.pk}, + data={ + 'user_id': self.test_user.pk + } + ) + + class GroupTestMixin(object): def _create_test_group(self): self.test_group = Group.objects.create(name=TEST_GROUP_NAME) @@ -94,6 +157,8 @@ class GroupTestMixin(object): self.test_group.name = TEST_GROUP_NAME_EDITED self.test_group.save() + +class GroupViewTestMixin(object): def _request_test_group_create_view(self): reponse = self.post( viewname='user_management:group_create', data={ @@ -135,13 +200,17 @@ class UserTestMixin(object): username=TEST_CASE_SUPERUSER_USERNAME, email=TEST_CASE_SUPERUSER_EMAIL, password=TEST_CASE_SUPERUSER_PASSWORD ) + self.test_superuser.clear_password = TEST_USER_PASSWORD def _create_test_user(self): - self.test_user = get_user_model().objects.create( + self.test_user = get_user_model().objects.create_user( username=TEST_USER_USERNAME, email=TEST_USER_EMAIL, password=TEST_USER_PASSWORD ) + self.test_user.clear_password = TEST_USER_PASSWORD + +class UserViewTestMixin(object): def _request_test_superuser_delete_view(self): return self.post( viewname='user_management:user_delete', @@ -187,3 +256,66 @@ class UserTestMixin(object): viewname='user_management:user_groups', kwargs={'user_id': self.test_user.pk} ) + + +class UserAPITestMixin(object): + def _request_test_user_create_api_view(self): + result = self.post( + viewname='rest_api:user-list', data={ + 'email': TEST_USER_EMAIL, 'password': TEST_USER_PASSWORD, + 'username': TEST_USER_USERNAME, + } + ) + if 'id' in result.json(): + self.test_user = get_user_model().objects.get(pk=result.json()['id']) + + return result + + def _request_test_user_delete_api_view(self): + return self.delete( + viewname='rest_api:user-detail', + kwargs={'user_id': self.test_user.pk} + ) + + def _request_test_user_edit_patch_api_view(self): + return self.patch( + viewname='rest_api:user-detail', + kwargs={'user_id': self.test_user.pk}, + data={'username': TEST_USER_USERNAME_EDITED} + ) + + def _request_test_user_edit_put_api_view(self): + return self.put( + viewname='rest_api:user-detail', + kwargs={'user_id': self.test_user.pk}, + data={'username': TEST_USER_USERNAME_EDITED} + ) + + def _request_test_user_group_list_api_view(self): + return self.get( + viewname='rest_api:user-group-list', + kwargs={'user_id': self.test_user.pk} + ) + + def _request_test_user_group_add_api_view(self): + return self.post( + viewname='rest_api:user-group-add', + kwargs={'user_id': self.test_user.pk}, + data={'group_id_list': '{}'.format(self.test_group.pk)} + ) + + def _request_test_user_group_remove_api_view(self): + return self.post( + viewname='rest_api:user-group-remove', + kwargs={'user_id': self.test_user.pk}, + data={'group_id_list': '{}'.format(self.test_group.pk)} + ) + + def _request_test_user_password_change_api_view(self): + self.test_user.clear_password = TEST_USER_PASSWORD_EDITED + return self.patch( + viewname='rest_api:user-detail', + kwargs={'user_id': self.test_user.pk}, data={ + 'password': TEST_USER_PASSWORD_EDITED, + } + ) diff --git a/mayan/apps/user_management/tests/test_api.py b/mayan/apps/user_management/tests/test_api.py index 1699b6802b..03f62584c8 100644 --- a/mayan/apps/user_management/tests/test_api.py +++ b/mayan/apps/user_management/tests/test_api.py @@ -14,21 +14,12 @@ from ..permissions import ( permission_user_edit, permission_user_view ) -from .literals import ( - TEST_GROUP_NAME, TEST_GROUP_NAME_EDITED, TEST_USER_EMAIL, TEST_USER_USERNAME, - TEST_USER_USERNAME_EDITED, TEST_USER_PASSWORD, TEST_USER_PASSWORD_EDITED +from .mixins import ( + GroupAPITestMixin, GroupTestMixin, UserAPITestMixin, UserTestMixin ) -from .mixins import GroupTestMixin, UserTestMixin -class GroupAPITestCase(UserTestMixin, GroupTestMixin, BaseAPITestCase): - def _request_test_group_create_api_view(self): - return self.post( - viewname='rest_api:group-list', data={ - 'name': TEST_GROUP_NAME - } - ) - +class GroupAPITestCase(GroupAPITestMixin, GroupTestMixin, UserTestMixin, BaseAPITestCase): def test_group_create_api_view_no_permission(self): group_count = Group.objects.count() @@ -46,12 +37,6 @@ class GroupAPITestCase(UserTestMixin, GroupTestMixin, BaseAPITestCase): self.assertNotEqual(group_count, Group.objects.count()) - def _request_test_group_delete_api_view(self): - return self.delete( - viewname='rest_api:group-detail', - kwargs={'group_id': self.test_group.pk} - ) - def test_group_delete_api_view_no_permission(self): self._create_test_group() @@ -75,12 +60,6 @@ class GroupAPITestCase(UserTestMixin, GroupTestMixin, BaseAPITestCase): self.assertNotEqual(group_count, Group.objects.count()) - def _request_test_group_detail_api_view(self): - return self.get( - viewname='rest_api:group-detail', - kwargs={'group_id': self.test_group.pk} - ) - def test_group_detail_api_view_no_permission(self): self._create_test_group() @@ -102,15 +81,6 @@ class GroupAPITestCase(UserTestMixin, GroupTestMixin, BaseAPITestCase): self.assertEqual(self.test_group.name, response.data.get('name', None)) - def _request_test_group_edit_patch_api_view(self): - return self.patch( - viewname='rest_api:group-detail', - kwargs={'group_id': self.test_group.pk}, - data={ - 'name': TEST_GROUP_NAME_EDITED - } - ) - def test_group_edit_patch_api_view_no_permission(self): self._create_test_group() @@ -122,7 +92,7 @@ class GroupAPITestCase(UserTestMixin, GroupTestMixin, BaseAPITestCase): self.test_group.refresh_from_db() self.assertEqual(group_name, self.test_group.name) - def test_group_edit_via_patch_with_access(self): + def test_group_edit_patch_with_access(self): self._create_test_group() group_name = self.test_group.name @@ -136,9 +106,6 @@ class GroupAPITestCase(UserTestMixin, GroupTestMixin, BaseAPITestCase): self.test_group.refresh_from_db() self.assertNotEqual(group_name, self.test_group.name) - def _request_test_group_list_api_view(self): - return self.get(viewname='rest_api:group-list') - def test_group_list_api_view_no_permission(self): self._create_test_group() @@ -160,15 +127,6 @@ class GroupAPITestCase(UserTestMixin, GroupTestMixin, BaseAPITestCase): status_code=status.HTTP_200_OK ) - def _request_test_group_user_add_patch_api_view(self): - return self.post( - viewname='rest_api:group-user-add', - kwargs={'group_id': self.test_group.pk}, - data={ - 'user_id': self.test_user.pk - } - ) - def _setup_group_user_add(self): self._create_test_group() self._create_test_user() @@ -176,7 +134,7 @@ class GroupAPITestCase(UserTestMixin, GroupTestMixin, BaseAPITestCase): def test_group_user_add_api_view_no_permission(self): self._setup_group_user_add() - response = self._request_test_group_user_add_patch_api_view() + response = self._request_test_group_user_add_api_view() self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) self.test_group.refresh_from_db() @@ -188,7 +146,7 @@ class GroupAPITestCase(UserTestMixin, GroupTestMixin, BaseAPITestCase): self.grant_access( obj=self.test_group, permission=permission_group_edit ) - response = self._request_test_group_user_add_patch_api_view() + response = self._request_test_group_user_add_api_view() self.assertEqual(response.status_code, status.HTTP_200_OK) self.test_group.refresh_from_db() @@ -200,7 +158,7 @@ class GroupAPITestCase(UserTestMixin, GroupTestMixin, BaseAPITestCase): self.grant_access( obj=self.test_user, permission=permission_user_edit ) - response = self._request_test_group_user_add_patch_api_view() + response = self._request_test_group_user_add_api_view() self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) self.test_group.refresh_from_db() @@ -215,22 +173,58 @@ class GroupAPITestCase(UserTestMixin, GroupTestMixin, BaseAPITestCase): self.grant_access( obj=self.test_user, permission=permission_user_edit ) - response = self._request_test_group_user_add_patch_api_view() + response = self._request_test_group_user_add_api_view() self.assertEqual(response.status_code, status.HTTP_200_OK) self.test_group.refresh_from_db() self.assertTrue(self.test_user in self.test_group.user_set.all()) - #TODO: group-user-list tests + def _setup_group_user_list(self): + self._create_test_group() + self._create_test_user() + self.test_group.user_set.add(self.test_user) - def _request_test_group_user_remove_patch_api_view(self): - return self.post( - viewname='rest_api:group-user-remove', - kwargs={'group_id': self.test_group.pk}, - data={ - 'user_id': self.test_user.pk - } + def test_group_user_list_api_view_no_permission(self): + self._setup_group_user_list() + + response = self._request_test_group_user_list_api_view() + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertTrue('count' not in response.json()) + + def test_group_user_list_with_group_access(self): + self._setup_group_user_list() + + self.grant_access( + obj=self.test_group, permission=permission_group_view ) + response = self._request_test_group_user_list_api_view() + self.assertEqual(response.status_code, status.HTTP_200_OK) + + self.assertEqual(response.json()['count'], 0) + + def test_group_user_list_with_user_access(self): + self._setup_group_user_list() + + self.grant_access( + obj=self.test_user, permission=permission_user_view + ) + response = self._request_test_group_user_list_api_view() + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + self.assertTrue('count' not in response.json()) + + def test_group_user_list_with_full_access(self): + self._setup_group_user_list() + + self.grant_access( + obj=self.test_group, permission=permission_group_view + ) + self.grant_access( + obj=self.test_user, permission=permission_user_view + ) + response = self._request_test_group_user_list_api_view() + self.assertEqual(response.status_code, status.HTTP_200_OK) + + self.assertEqual(response.json()['count'], 1) def _setup_group_user_remove(self): self._create_test_group() @@ -240,7 +234,7 @@ class GroupAPITestCase(UserTestMixin, GroupTestMixin, BaseAPITestCase): def test_group_user_remove_api_view_no_permission(self): self._setup_group_user_remove() - response = self._request_test_group_user_remove_patch_api_view() + response = self._request_test_group_user_remove_api_view() self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) self.test_group.refresh_from_db() @@ -252,7 +246,7 @@ class GroupAPITestCase(UserTestMixin, GroupTestMixin, BaseAPITestCase): self.grant_access( obj=self.test_group, permission=permission_group_edit ) - response = self._request_test_group_user_remove_patch_api_view() + response = self._request_test_group_user_remove_api_view() self.assertEqual(response.status_code, status.HTTP_200_OK) self.test_group.refresh_from_db() @@ -264,7 +258,7 @@ class GroupAPITestCase(UserTestMixin, GroupTestMixin, BaseAPITestCase): self.grant_access( obj=self.test_user, permission=permission_user_edit ) - response = self._request_test_group_user_remove_patch_api_view() + response = self._request_test_group_user_remove_api_view() self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) self.test_group.refresh_from_db() @@ -279,26 +273,18 @@ class GroupAPITestCase(UserTestMixin, GroupTestMixin, BaseAPITestCase): self.grant_access( obj=self.test_user, permission=permission_user_edit ) - response = self._request_test_group_user_remove_patch_api_view() + response = self._request_test_group_user_remove_api_view() self.assertEqual(response.status_code, status.HTTP_200_OK) self.test_group.refresh_from_db() self.assertTrue(self.test_user not in self.test_group.user_set.all()) -class UserAPITestCase(GroupTestMixin, UserTestMixin, BaseAPITestCase): - def _request_test_user_create_api_view_api_view(self): - return self.post( - viewname='rest_api:user-list', data={ - 'email': TEST_USER_EMAIL, 'password': TEST_USER_PASSWORD, - 'username': TEST_USER_USERNAME, - } - ) - +class UserAPITestCase(UserAPITestMixin, GroupTestMixin, UserTestMixin, BaseAPITestCase): def test_user_create_api_view_no_permission(self): user_count = get_user_model().objects.count() - response = self._request_test_user_create_api_view_api_view() + response = self._request_test_user_create_api_view() self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) self.assertEqual(user_count, get_user_model().objects.count()) @@ -307,11 +293,9 @@ class UserAPITestCase(GroupTestMixin, UserTestMixin, BaseAPITestCase): user_count = get_user_model().objects.count() self.grant_permission(permission=permission_user_create) - response = self._request_test_user_create_api_view_api_view() + response = self._request_test_user_create_api_view() self.assertEqual(response.status_code, status.HTTP_201_CREATED) - user = get_user_model().objects.get(pk=response.data['id']) - self.assertEqual(user.username, TEST_USER_USERNAME) self.assertEqual(user_count + 1, get_user_model().objects.count()) def test_user_create_api_view_login(self): @@ -319,26 +303,19 @@ class UserAPITestCase(GroupTestMixin, UserTestMixin, BaseAPITestCase): self.assertTrue( self.login( - username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD + username=self.test_user.username, + password=self.test_user.clear_password ) ) - def _request_user_password_change(self): - return self.patch( - viewname='rest_api:user-detail', - kwargs={'user_id': self.test_user.pk}, data={ - 'password': TEST_USER_PASSWORD_EDITED, - } - ) - def test_user_create_login_password_change_api_view_no_permission(self): self._create_test_user() - self._request_user_password_change() + self._request_test_user_password_change_api_view() self.assertFalse( - self.client.login( - username=TEST_USER_USERNAME, - password=TEST_USER_PASSWORD_EDITED + self.login( + username=self.test_user.username, + password=self.test_user.clear_password ) ) @@ -346,22 +323,15 @@ class UserAPITestCase(GroupTestMixin, UserTestMixin, BaseAPITestCase): self._create_test_user() self.grant_access(obj=self.test_user, permission=permission_user_edit) - self._request_user_password_change() + self._request_test_user_password_change_api_view() self.assertTrue( - self.client.login( - username=TEST_USER_USERNAME, - password=TEST_USER_PASSWORD_EDITED + self.login( + username=self.test_user.username, + password=self.test_user.clear_password ) ) - def _request_test_user_edit_put_api_view(self): - return self.put( - viewname='rest_api:user-detail', - kwargs={'user_id': self.test_user.pk}, - data={'username': TEST_USER_USERNAME_EDITED} - ) - def test_user_edit_put_api_view_no_permission(self): self._create_test_user() username = self.test_user.username @@ -383,13 +353,6 @@ class UserAPITestCase(GroupTestMixin, UserTestMixin, BaseAPITestCase): self.test_user.refresh_from_db() self.assertNotEqual(username, self.test_user.username) - def _request_test_user_edit_patch_api_view(self): - return self.patch( - viewname='rest_api:user-detail', - kwargs={'user_id': self.test_user.pk}, - data={'username': TEST_USER_USERNAME_EDITED} - ) - def test_user_edit_patch_api_view_no_permission(self): self._create_test_user() username = self.test_user.username @@ -411,12 +374,6 @@ class UserAPITestCase(GroupTestMixin, UserTestMixin, BaseAPITestCase): self.test_user.refresh_from_db() self.assertNotEqual(username, self.test_user.username) - def _request_test_user_delete_api_view(self): - return self.delete( - viewname='rest_api:user-detail', - kwargs={'user_id': self.test_user.pk} - ) - def test_user_delete_api_view_no_permission(self): self._create_test_user() @@ -440,85 +397,70 @@ class UserAPITestCase(GroupTestMixin, UserTestMixin, BaseAPITestCase): get_user_model().objects.filter(pk=self.test_user.pk).exists() ) - def _request_test_user_group_api_view(self): - return self.get( - viewname='rest_api:user-group-list', - kwargs={'user_id': self.test_user.pk} - ) - - def test_user_group_list_api_view_no_permission(self): + def _setup_user_group_list(self): self._create_test_group() self._create_test_user() self.test_user.groups.add(self.test_group) - response = self._request_test_user_group_api_view() + def test_user_group_list_api_view_no_permission(self): + self._setup_user_group_list() + + response = self._request_test_user_group_list_api_view() self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) def test_user_group_list_api_view_with_user_access(self): - self._create_test_group() - self._create_test_user() - self.test_user.groups.add(self.test_group) + self._setup_user_group_list() self.grant_access(obj=self.test_user, permission=permission_user_view) - response = self._request_test_user_group_api_view() + response = self._request_test_user_group_list_api_view() self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.data['count'], 0) def test_user_group_list_api_view_with_group_access(self): - self._create_test_group() - self._create_test_user() - self.test_user.groups.add(self.test_group) + self._setup_user_group_list() self.grant_access( obj=self.test_group, permission=permission_group_view ) - response = self._request_test_user_group_api_view() + response = self._request_test_user_group_list_api_view() self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_user_group_list_api_view_with_access(self): - self._create_test_group() - self._create_test_user() - self.test_user.groups.add(self.test_group) + def test_user_group_list_api_view_with_full_access(self): + self._setup_user_group_list() self.grant_access(obj=self.test_user, permission=permission_user_view) self.grant_access( obj=self.test_group, permission=permission_group_view ) - response = self._request_test_user_group_api_view() + response = self._request_test_user_group_list_api_view() self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(response.data['count'], 1) - def _request_test_user_group_add_api_view(self): - return self.patch( - viewname='rest_api:user-detail', - kwargs={'user_id': self.test_user.pk}, - data={'group_id_list': '{}'.format(self.test_group.pk)} - ) + def _setup_user_group_add(self): + self._create_test_group() + self._create_test_user() def test_user_group_add_api_view_no_permission(self): - self._create_test_group() - self._create_test_user() + self._setup_user_group_add() response = self._request_test_user_group_add_api_view() self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) self.test_user.refresh_from_db() - self.assertEqual(self.test_group.user_set.first(), None) + self.assertTrue(self.test_group not in self.test_user.groups.all()) def test_user_group_add_api_view_with_user_access(self): - self._create_test_group() - self._create_test_user() + self._setup_user_group_add() self.grant_access(obj=self.test_user, permission=permission_user_edit) response = self._request_test_user_group_add_api_view() self.assertEqual(response.status_code, status.HTTP_200_OK) self.test_user.refresh_from_db() - self.assertEqual(self.test_group.user_set.first(), None) + self.assertTrue(self.test_group not in self.test_user.groups.all()) def test_user_group_add_api_view_with_group_access(self): - self._create_test_group() - self._create_test_user() + self._setup_user_group_add() self.grant_access( obj=self.test_group, permission=permission_group_edit @@ -527,11 +469,10 @@ class UserAPITestCase(GroupTestMixin, UserTestMixin, BaseAPITestCase): self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) self.test_user.refresh_from_db() - self.assertEqual(self.test_group.user_set.first(), None) + self.assertTrue(self.test_group not in self.test_user.groups.all()) def test_user_group_add_api_view_with_full_access(self): - self._create_test_group() - self._create_test_user() + self._setup_user_group_add() self.grant_access(obj=self.test_user, permission=permission_user_edit) self.grant_access( @@ -541,4 +482,53 @@ class UserAPITestCase(GroupTestMixin, UserTestMixin, BaseAPITestCase): self.assertEqual(response.status_code, status.HTTP_200_OK) self.test_user.refresh_from_db() - self.assertEqual(self.test_group.user_set.first(), self.test_user) + self.assertTrue(self.test_group in self.test_user.groups.all()) + + def _setup_user_group_remove(self): + self._create_test_group() + self._create_test_user() + self.test_user.groups.add(self.test_group) + + def test_user_group_remove_api_view_no_permission(self): + self._setup_user_group_remove() + + response = self._request_test_user_group_remove_api_view() + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + self.test_user.refresh_from_db() + self.assertTrue(self.test_group in self.test_user.groups.all()) + + def test_user_group_remove_api_view_with_user_access(self): + self._setup_user_group_remove() + + self.grant_access(obj=self.test_user, permission=permission_user_edit) + response = self._request_test_user_group_remove_api_view() + self.assertEqual(response.status_code, status.HTTP_200_OK) + + self.test_user.refresh_from_db() + self.assertTrue(self.test_group in self.test_user.groups.all()) + + def test_user_group_remove_api_view_with_group_access(self): + self._setup_user_group_remove() + + self.grant_access( + obj=self.test_group, permission=permission_group_edit + ) + response = self._request_test_user_group_remove_api_view() + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + self.test_user.refresh_from_db() + self.assertTrue(self.test_group in self.test_user.groups.all()) + + def test_user_group_remove_api_view_with_full_access(self): + self._setup_user_group_remove() + + self.grant_access(obj=self.test_user, permission=permission_user_edit) + self.grant_access( + obj=self.test_group, permission=permission_group_edit + ) + response = self._request_test_user_group_remove_api_view() + self.assertEqual(response.status_code, status.HTTP_200_OK) + + self.test_user.refresh_from_db() + self.assertTrue(self.test_group not in self.test_user.groups.all()) diff --git a/mayan/apps/user_management/tests/test_events.py b/mayan/apps/user_management/tests/test_events.py index e6d630c89f..d2e35cee05 100644 --- a/mayan/apps/user_management/tests/test_events.py +++ b/mayan/apps/user_management/tests/test_events.py @@ -3,6 +3,7 @@ from __future__ import unicode_literals from actstream.models import Action from mayan.apps.common.tests import GenericViewTestCase +from mayan.apps.rest_api.tests import BaseAPITestCase from ..permissions import ( permission_group_create, permission_group_edit, permission_user_create, @@ -14,10 +15,13 @@ from ..events import ( event_user_edited ) -from .mixins import GroupTestMixin, UserTestMixin +from .mixins import ( + GroupAPITestMixin, GroupTestMixin, GroupViewTestMixin, UserAPITestMixin, + UserTestMixin, UserViewTestMixin +) -class GroupEventsTestCase(GroupTestMixin, UserTestMixin, GenericViewTestCase): +class GroupEventsTestCase(GroupTestMixin, GroupViewTestMixin, UserTestMixin, GenericViewTestCase): def test_group_create_event(self): Action.objects.all().delete() @@ -25,6 +29,7 @@ class GroupEventsTestCase(GroupTestMixin, UserTestMixin, GenericViewTestCase): 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) @@ -36,22 +41,49 @@ class GroupEventsTestCase(GroupTestMixin, UserTestMixin, GenericViewTestCase): 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): - def test_user_create_event(self): +class GroupEventsAPITestCase(GroupAPITestMixin, GroupTestMixin, GroupViewTestMixin, BaseAPITestCase): + def test_group_create_event_from_api_view(self): + Action.objects.all().delete() + + self.grant_permission( + permission=permission_group_create + ) + self._request_test_group_create_api_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_from_api_view(self): + self._create_test_group() + Action.objects.all().delete() + + self.grant_access( + obj=self.test_group, permission=permission_group_edit + ) + self._request_test_group_edit_patch_api_view() + + self.assertEqual(Action.objects.last().target, self.test_group) + self.assertEqual(Action.objects.last().verb, event_group_edited.id) + + +class UserEventsTestCase(UserAPITestMixin, UserTestMixin, UserViewTestMixin, GenericViewTestCase): + def test_user_create_event_from_view(self): 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): + def test_user_edit_event_from_view(self): self._create_test_user() Action.objects.all().delete() @@ -59,5 +91,31 @@ class UserEventsTestCase(UserTestMixin, GenericViewTestCase): 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) + + +class UserEventsAPITestCase(UserAPITestMixin, UserTestMixin, UserViewTestMixin, BaseAPITestCase): + def test_user_create_event_from_api_view(self): + Action.objects.all().delete() + + self.grant_permission( + permission=permission_user_create + ) + self._request_test_user_create_api_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_from_api_view(self): + self._create_test_user() + Action.objects.all().delete() + + self.grant_access( + obj=self.test_user, permission=permission_user_edit + ) + self._request_test_user_edit_patch_api_view() + self.assertEqual(Action.objects.last().target, self.test_user) self.assertEqual(Action.objects.last().verb, event_user_edited.id) diff --git a/mayan/apps/user_management/tests/test_views.py b/mayan/apps/user_management/tests/test_views.py index 44e6214138..f1b6ff0a8e 100644 --- a/mayan/apps/user_management/tests/test_views.py +++ b/mayan/apps/user_management/tests/test_views.py @@ -21,12 +21,14 @@ from .literals import ( TEST_GROUP_NAME, TEST_GROUP_NAME_EDITED, TEST_USER_PASSWORD_EDITED, TEST_USER_USERNAME ) -from .mixins import GroupTestMixin, UserTestMixin +from .mixins import ( + GroupTestMixin, GroupViewTestMixin, UserTestMixin, UserViewTestMixin +) TEST_USER_TO_DELETE_USERNAME = 'user_to_delete' -class GroupViewsTestCase(GroupTestMixin, UserTestMixin, GenericViewTestCase): +class GroupViewsTestCase(GroupTestMixin, GroupViewTestMixin, UserTestMixin, GenericViewTestCase): def test_group_create_view_no_permission(self): response = self._request_test_group_create_view() self.assertEqual(response.status_code, 403) @@ -136,7 +138,7 @@ class GroupViewsTestCase(GroupTestMixin, UserTestMixin, GenericViewTestCase): ) -class UserViewsTestCase(GroupTestMixin, UserTestMixin, GenericViewTestCase): +class UserViewsTestCase(GroupTestMixin, UserTestMixin, UserViewTestMixin, GenericViewTestCase): def test_user_create_view_no_permission(self): user_count = get_user_model().objects.count() @@ -343,7 +345,7 @@ class UserViewsTestCase(GroupTestMixin, UserTestMixin, GenericViewTestCase): self.assertEqual(response.status_code, 200) - def _request_user_multiple_delete_view(self): + def _request_test_user_multiple_delete_view(self): return self.post( viewname='user_management:user_multiple_delete', data={ 'id_list': self.test_user.pk @@ -354,7 +356,7 @@ class UserViewsTestCase(GroupTestMixin, UserTestMixin, GenericViewTestCase): self._create_test_user() user_count = get_user_model().objects.count() - response = self._request_user_multiple_delete_view() + response = self._request_test_user_multiple_delete_view() self.assertEqual(response.status_code, 404) self.assertEqual(get_user_model().objects.count(), user_count) @@ -366,7 +368,7 @@ class UserViewsTestCase(GroupTestMixin, UserTestMixin, GenericViewTestCase): self.grant_access( obj=self.test_user, permission=permission_user_delete ) - response = self._request_user_multiple_delete_view() + response = self._request_test_user_multiple_delete_view() self.assertEqual(response.status_code, 302) self.assertEqual(get_user_model().objects.count(), user_count - 1) diff --git a/mayan/apps/user_management/views.py b/mayan/apps/user_management/views.py index 9cdd9fefd0..b27f46c342 100644 --- a/mayan/apps/user_management/views.py +++ b/mayan/apps/user_management/views.py @@ -5,7 +5,6 @@ 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.db import transaction from django.http import HttpResponseRedirect from django.shortcuts import get_object_or_404 from django.template import RequestContext @@ -21,10 +20,6 @@ from mayan.apps.common.generics import ( ) from mayan.apps.common.mixins import ExternalObjectMixin -from .events import ( - event_group_created, event_group_edited, event_user_created, - event_user_edited -) from .forms import UserForm from .icons import icon_group_setup, icon_user_setup from .links import link_group_create, link_user_create @@ -69,14 +64,6 @@ class GroupCreateView(SingleObjectCreateView): post_action_redirect = reverse_lazy(viewname='user_management:group_list') view_permission = permission_group_create - def form_valid(self, form): - with transaction.atomic(): - result = super(GroupCreateView, self).form_valid(form=form) - event_group_created.commit( - actor=self.request.user, target=self.object - ) - return result - class GroupDeleteView(SingleObjectDeleteView): model = Group @@ -98,14 +85,6 @@ class GroupEditView(SingleObjectEditView): pk_url_kwarg = 'group_id' post_action_redirect = reverse_lazy(viewname='user_management:group_list') - def form_valid(self, form): - with transaction.atomic(): - result = super(GroupEditView, self).form_valid(form=form) - event_group_edited.commit( - actor=self.request.user, target=self.object - ) - return result - def get_extra_context(self): return { 'object': self.object, @@ -205,12 +184,7 @@ class UserCreateView(SingleObjectCreateView): view_permission = permission_user_create def form_valid(self, form): - with transaction.atomic(): - super(UserCreateView, self).form_valid(form=form) - event_user_created.commit( - actor=self.request.user, target=self.object - ) - + super(UserCreateView, self).form_valid(form=form) return HttpResponseRedirect( reverse( viewname='user_management:user_set_password', @@ -295,15 +269,6 @@ class UserEditView(SingleObjectEditView): is_superuser=False, is_staff=False ) - def form_valid(self, form): - with transaction.atomic(): - result = super(UserEditView, self).form_valid(form=form) - event_user_edited.commit( - actor=self.request.user, target=self.object - ) - - return result - def get_extra_context(self): return { 'object': self.object,