Update tag views to use updated ACL filter_by_access. Add better tag app views tests.

mayan/apps/tags/tests/test_api.py

@@ -16,8 +16,8 @@ from user_management.tests.literals import (
 from ..models import Tag
 
 from .literals import (
-    TEST_TAG_COLOR, TEST_TAG_COLOR_ALTERNATE, TEST_TAG_LABEL,
+    TEST_TAG_COLOR, TEST_TAG_COLOR_EDITED, TEST_TAG_LABEL,
-    TEST_TAG_LABEL_ALTERNATE
+    TEST_TAG_LABEL_EDITED
 )
 
 
@@ -63,15 +63,15 @@ class TagAPITestCase(APITestCase):
         self.client.put(
             reverse('rest_api:tag-detail', args=(tag.pk,)),
             {
-                'label': TEST_TAG_LABEL_ALTERNATE,
+                'label': TEST_TAG_LABEL_EDITED,
-                'color': TEST_TAG_COLOR_ALTERNATE
+                'color': TEST_TAG_COLOR_EDITED
             }
         )
 
         tag = Tag.objects.first()
 
-        self.assertEqual(tag.label, TEST_TAG_LABEL_ALTERNATE)
+        self.assertEqual(tag.label, TEST_TAG_LABEL_EDITED)
-        self.assertEqual(tag.color, TEST_TAG_COLOR_ALTERNATE)
+        self.assertEqual(tag.color, TEST_TAG_COLOR_EDITED)
 
     @override_settings(OCR_AUTO_OCR=False)
     def test_tag_add_document(self):

mayan/apps/tags/tests/test_views.py

@@ -1,134 +1,200 @@
-from __future__ import absolute_import, unicode_literals
+from __future__ import unicode_literals
 
-from django.contrib.auth import get_user_model
-from django.contrib.auth.models import Group
-from django.core.files import File
-from django.core.urlresolvers import reverse
-from django.test.client import Client
-from django.test import TestCase
-
-from documents.models import DocumentType
 from documents.permissions import permission_document_view
-from documents.tests import TEST_DOCUMENT_TYPE, TEST_SMALL_DOCUMENT_PATH
+from documents.tests.test_views import GenericDocumentViewTestCase
-from permissions import Permission
+from user_management.tests import TEST_USER_PASSWORD, TEST_USER_USERNAME
-from permissions.models import Role
-from permissions.tests.literals import TEST_ROLE_LABEL
-from user_management.tests import (
-    TEST_ADMIN_EMAIL, TEST_ADMIN_PASSWORD, TEST_ADMIN_USERNAME, TEST_GROUP,
-    TEST_USER_PASSWORD, TEST_USER_USERNAME
-)
 
 from ..models import Tag
-from ..permissions import permission_tag_attach, permission_tag_view
+from ..permissions import (
+    permission_tag_attach, permission_tag_create, permission_tag_delete,
+    permission_tag_edit, permission_tag_remove, permission_tag_view
+)
 
-from .literals import TEST_TAG_COLOR, TEST_TAG_LABEL
+from .literals import (
+    TEST_TAG_COLOR, TEST_TAG_COLOR_EDITED, TEST_TAG_LABEL,
+    TEST_TAG_LABEL_EDITED
+)
 
 
-class TagViewTestCase(TestCase):
+class TagViewTestCase(GenericDocumentViewTestCase):
     def setUp(self):
-        self.admin_user = get_user_model().objects.create_superuser(
+        super(TagViewTestCase, self).setUp()
-            email=TEST_ADMIN_EMAIL, password=TEST_ADMIN_PASSWORD,
-            username=TEST_ADMIN_USERNAME
-        )
-
-        self.user = get_user_model().objects.create_user(
-            username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD
-        )
-        self.group = Group.objects.create(name=TEST_GROUP)
-        self.role = Role.objects.create(label=TEST_ROLE_LABEL)
-        self.group.user_set.add(self.user)
-        self.role.groups.add(self.group)
-        Permission.invalidate_cache()
-
-        self.document_type = DocumentType.objects.create(
-            label=TEST_DOCUMENT_TYPE
-        )
-
-        with open(TEST_SMALL_DOCUMENT_PATH) as file_object:
-            self.document = self.document_type.new_document(
-                file_object=File(file_object)
-            )
-
-        self.client = Client()
-        # Login the admin user
 
         self.tag = Tag.objects.create(
             color=TEST_TAG_COLOR, label=TEST_TAG_LABEL
         )
 
     def tearDown(self):
-        self.admin_user.delete()
+        super(TagViewTestCase, self).tearDown()
-        self.document_type
+        if self.tag.pk:
-        self.group.delete()
+            self.tag.delete()
-        self.role.delete()
+
+    def test_tag_create_view_no_permissions(self):
+        self.login(username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD)
+
         self.tag.delete()
-        self.user.delete()
+        self.assertEqual(Tag.objects.count(), 0)
+
+        response = self.post(
+            'tags:tag_create', data={
+                'label': TEST_TAG_LABEL,
+                'color': TEST_TAG_COLOR
+            }
+        )
+
+        self.assertEqual(response.status_code, 403)
+        self.assertEqual(Tag.objects.count(), 0)
+
+    def test_tag_create_view_with_permissions(self):
+        self.login(username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD)
+
+        self.tag.delete()
+        self.assertEqual(Tag.objects.count(), 0)
+
+        self.role.permissions.add(permission_tag_create.stored_permission)
+
+        response = self.post(
+            'tags:tag_create', data={
+                'label': TEST_TAG_LABEL,
+                'color': TEST_TAG_COLOR
+            }, follow=True
+        )
+
+        self.assertContains(response, text='created', status_code=200)
+
+        self.assertEqual(Tag.objects.count(), 1)
+        tag = Tag.objects.first()
+        self.assertEqual(tag.label, TEST_TAG_LABEL)
+        self.assertEqual(tag.color, TEST_TAG_COLOR)
+
+    def test_tag_delete_view_no_permissions(self):
+        self.login(username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD)
+
+        self.assertEqual(Tag.objects.count(), 1)
+
+        response = self.post(
+            'tags:tag_delete', args=(self.tag.pk,)
+        )
+
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(Tag.objects.count(), 1)
+
+    def test_tag_delete_view_with_permissions(self):
+        self.login(username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD)
+
+        self.assertEqual(Tag.objects.count(), 1)
+
+        self.role.permissions.add(permission_tag_delete.stored_permission)
+
+        response = self.post(
+            'tags:tag_delete', args=(self.tag.pk,), follow=True
+        )
+
+        self.assertContains(response, text='deleted', status_code=200)
+
+        self.assertEqual(Tag.objects.count(), 0)
+
+    def test_tag_multiple_delete_view_no_permissions(self):
+        self.login(username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD)
+
+        self.assertEqual(Tag.objects.count(), 1)
+
+        response = self.post(
+            'tags:tag_multiple_delete', data={'id_list': self.tag.pk}
+        )
+
+        self.assertEqual(response.status_code, 302)
+        self.assertEqual(Tag.objects.count(), 1)
+
+    def test_tag_multiple_delete_view_with_permissions(self):
+        self.login(username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD)
+
+        self.assertEqual(Tag.objects.count(), 1)
+
+        self.role.permissions.add(permission_tag_delete.stored_permission)
+
+        response = self.post(
+            'tags:tag_multiple_delete', data={'id_list': self.tag.pk},
+            follow=True
+        )
+
+        self.assertContains(response, text='deleted', status_code=200)
+
+        self.assertEqual(Tag.objects.count(), 0)
+
+    def test_tag_edit_view_no_permissions(self):
+        self.login(username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD)
+
+        response = self.post(
+            'tags:tag_edit', args=(self.tag.pk,), data={
+                'label': TEST_TAG_LABEL_EDITED, 'color': TEST_TAG_COLOR_EDITED
+            }
+        )
+
+        self.assertEqual(response.status_code, 403)
+        tag = Tag.objects.get(pk=self.tag.pk)
+        self.assertEqual(tag.label, TEST_TAG_LABEL)
+        self.assertEqual(tag.color, TEST_TAG_COLOR)
+
+    def test_tag_edit_view_with_permissions(self):
+        self.login(username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD)
+
+        self.role.permissions.add(permission_tag_edit.stored_permission)
+
+        response = self.post(
+            'tags:tag_edit', args=(self.tag.pk,), data={
+                'label': TEST_TAG_LABEL_EDITED, 'color': TEST_TAG_COLOR_EDITED
+            }, follow=True
+        )
+
+        self.assertContains(response, text='saved', status_code=200)
+        tag = Tag.objects.get(pk=self.tag.pk)
+        self.assertEqual(tag.label, TEST_TAG_LABEL_EDITED)
+        self.assertEqual(tag.color, TEST_TAG_COLOR_EDITED)
 
     def test_document_tags_widget_no_permissions(self):
-        logged_in = self.client.login(
+        self.login(username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD)
-            username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD
-        )
-        self.assertTrue(logged_in)
-        self.assertTrue(self.user.is_authenticated())
 
         self.tag.documents.add(self.document)
-        response = self.client.get(reverse('documents:document_list'))
+        response = self.get('documents:document_list')
         self.assertNotContains(response, text=TEST_TAG_LABEL, status_code=200)
 
     def test_document_tags_widget_with_permissions(self):
-        logged_in = self.client.login(
+        self.login(username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD)
-            username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD
-        )
-        self.assertTrue(logged_in)
-        self.assertTrue(self.user.is_authenticated())
 
         self.tag.documents.add(self.document)
         self.role.permissions.add(permission_tag_view.stored_permission)
         self.role.permissions.add(permission_document_view.stored_permission)
-        response = self.client.get(reverse('documents:document_list'))
+        response = self.get('documents:document_list')
 
-        self.assertContains(
+        self.assertContains(response, text=TEST_TAG_LABEL, status_code=200)
-            response, text=TEST_TAG_LABEL.replace(' ', ' '),
-            status_code=200
-        )
 
-    def test_document_attach_tag_user_view(self):
+    def test_document_attach_tag_user_view_no_permission(self):
-        logged_in = self.client.login(
+        self.login(username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD)
-            username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD
-        )
-        self.assertTrue(logged_in)
-        self.assertTrue(self.user.is_authenticated())
 
-        self.assertEqual(
+        self.assertEqual(self.document.tags.count(), 0)
-            self.document.tags.count(), 0
-        )
 
-        response = self.client.post(
+        response = self.post(
-            reverse(
+            'tags:tag_attach', args=(self.document.pk,), data={
-                'tags:tag_attach',
-                args=(self.document.pk,)
-            ), data={
                 'tag': self.tag.pk,
                 'user': self.user.pk
-            }, follow=True
+            }
         )
 
         self.assertEqual(response.status_code, 200)
 
-        self.assertEqual(
+    def test_document_attach_tag_user_view_with_permission(self):
-            self.document.tags.count(), 0
+        self.login(username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD)
-        )
+
+        self.assertEqual(self.document.tags.count(), 0)
 
         self.role.permissions.add(permission_tag_attach.stored_permission)
         # permission_tag_view is needed because the form filters the
         # choices
         self.role.permissions.add(permission_tag_view.stored_permission)
 
-        response = self.client.post(
+        response = self.post(
-            reverse(
+            'tags:tag_attach', args=(self.document.pk,), data={
-                'tags:tag_attach',
-                args=(self.document.pk,)
-            ), data={
                 'tag': self.tag.pk,
                 'user': self.user.pk
             }, follow=True
@@ -140,31 +206,25 @@ class TagViewTestCase(TestCase):
             self.document.tags.all(), (repr(self.tag),)
         )
 
-    def test_document_multiple_attach_tag_user_view(self):
+    def test_document_multiple_attach_tag_view_no_permission(self):
-        logged_in = self.client.login(
+        self.login(username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD)
-            username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD
-        )
-        self.assertTrue(logged_in)
-        self.assertTrue(self.user.is_authenticated())
 
-        self.assertEqual(
+        self.assertEqual(self.document.tags.count(), 0)
-            self.document.tags.count(), 0
+        self.role.permissions.add(permission_tag_view.stored_permission)
-        )
 
-        response = self.client.post(
+        response = self.post(
-            reverse(
+            'tags:multiple_documents_tag_attach', data={
-                'tags:multiple_documents_tag_attach',
-            ), data={
                 'id_list': self.document.pk, 'tag': self.tag.pk,
                 'user': self.user.pk
-            }, follow=True
+            }
         )
 
-        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.status_code, 302)
 
-        self.assertEqual(
+    def test_document_multiple_attach_tag_view_with_permission(self):
-            self.document.tags.count(), 0
+        self.login(username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD)
-        )
+
+        self.assertEqual(self.document.tags.count(), 0)
 
         self.role.permissions.add(permission_tag_attach.stored_permission)
 
@@ -172,10 +232,8 @@ class TagViewTestCase(TestCase):
         # choices
         self.role.permissions.add(permission_tag_view.stored_permission)
 
-        response = self.client.post(
+        response = self.post(
-            reverse(
+            'tags:multiple_documents_tag_attach', data={
-                'tags:multiple_documents_tag_attach',
-            ), data={
                 'id_list': self.document.pk, 'tag': self.tag.pk,
                 'user': self.user.pk
             }, follow=True
@@ -186,3 +244,77 @@ class TagViewTestCase(TestCase):
         self.assertQuerysetEqual(
             self.document.tags.all(), (repr(self.tag),)
         )
+
+    def test_single_document_multiple_tag_remove_view_no_permissions(self):
+        self.login(username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD)
+
+        self.document.tags.add(self.tag)
+        self.assertQuerysetEqual(self.document.tags.all(), (repr(self.tag),))
+        self.role.permissions.add(permission_tag_view.stored_permission)
+
+        response = self.post(
+            'tags:single_document_multiple_tag_remove',
+            args=(self.document.pk,), data={
+                'id_list': self.tag.pk,
+            }
+        )
+
+        self.assertEqual(response.status_code, 302)
+        self.assertQuerysetEqual(self.document.tags.all(), (repr(self.tag),))
+
+    def test_single_document_multiple_tag_remove_view_with_permission(self):
+        self.login(username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD)
+
+        self.document.tags.add(self.tag)
+        self.assertQuerysetEqual(self.document.tags.all(), (repr(self.tag),))
+
+        self.role.permissions.add(permission_tag_remove.stored_permission)
+        self.role.permissions.add(permission_tag_view.stored_permission)
+
+        response = self.post(
+            'tags:single_document_multiple_tag_remove',
+            args=(self.document.pk,), data={
+                'id_list': self.tag.pk,
+            }, follow=True
+        )
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(self.document.tags.count(), 0)
+
+    def test_multiple_documents_selection_tag_remove_view_no_permissions(self):
+        self.login(username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD)
+
+        self.document.tags.add(self.tag)
+        self.assertQuerysetEqual(self.document.tags.all(), (repr(self.tag),))
+        self.role.permissions.add(permission_tag_view.stored_permission)
+
+        response = self.post(
+            'tags:multiple_documents_selection_tag_remove',
+            data={
+                'id_list': self.document.pk,
+                'tag': self.tag.pk,
+            }
+        )
+
+        self.assertEqual(response.status_code, 302)
+        self.assertQuerysetEqual(self.document.tags.all(), (repr(self.tag),))
+
+    def test_multiple_documents_selection_tag_remove_view_with_permission(self):
+        self.login(username=TEST_USER_USERNAME, password=TEST_USER_PASSWORD)
+
+        self.document.tags.add(self.tag)
+        self.assertQuerysetEqual(self.document.tags.all(), (repr(self.tag),))
+
+        self.role.permissions.add(permission_tag_remove.stored_permission)
+        self.role.permissions.add(permission_tag_view.stored_permission)
+
+        response = self.post(
+            'tags:multiple_documents_selection_tag_remove',
+            data={
+                'id_list': self.document.pk,
+                'tag': self.tag.pk,
+            }, follow=True
+        )
+
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(self.document.tags.count(), 0)

mayan/apps/tags/views.py

@@ -62,10 +62,14 @@ def tag_attach(request, document_id=None, document_id_list=None):
 
     post_action_redirect = None
     previous = request.POST.get(
-        'previous', request.GET.get('previous', request.META.get('HTTP_REFERER', reverse(settings.LOGIN_REDIRECT_URL)))
+        'previous', request.GET.get(
+            'previous', request.META.get('HTTP_REFERER', reverse(settings.LOGIN_REDIRECT_URL))
+        )
     )
     next = request.POST.get(
-        'next', request.GET.get('next', post_action_redirect if post_action_redirect else request.META.get('HTTP_REFERER', reverse(settings.LOGIN_REDIRECT_URL)))
+        'next', request.GET.get(
+            'next', post_action_redirect if post_action_redirect else request.META.get('HTTP_REFERER', reverse(settings.LOGIN_REDIRECT_URL))
+        )
     )
 
     if request.method == 'POST':
@@ -144,13 +148,12 @@ def tag_delete(request, tag_id=None, tag_id_list=None):
     post_action_redirect = None
 
     if tag_id:
-        tags = [get_object_or_404(Tag, pk=tag_id)]
+        queryset = Tag.objects.filter(pk=tag_id)
         post_action_redirect = reverse('tags:tag_list')
     elif tag_id_list:
-        tags = [
+        queryset = Tag.objects.filter(pk__in=tag_id_list)
-            get_object_or_404(Tag, pk=tag_id) for tag_id in tag_id_list.split(',')
+
-        ]
+    if not queryset:
-    else:
         messages.error(request, _('Must provide at least one tag.'))
         return HttpResponseRedirect(
             request.META.get(
@@ -161,15 +164,15 @@ def tag_delete(request, tag_id=None, tag_id_list=None):
     try:
         Permission.check_permissions(request.user, (permission_tag_delete,))
     except PermissionDenied:
-        tags = AccessControlList.objects.filter_by_access(
+        queryset = AccessControlList.objects.filter_by_access(
-            permission_tag_delete, request.user, tags
+            permission_tag_delete, request.user, queryset
         )
 
     previous = request.POST.get('previous', request.GET.get('previous', request.META.get('HTTP_REFERER', reverse(settings.LOGIN_REDIRECT_URL))))
     next = request.POST.get('next', request.GET.get('next', post_action_redirect if post_action_redirect else request.META.get('HTTP_REFERER', reverse(settings.LOGIN_REDIRECT_URL))))
 
     if request.method == 'POST':
-        for tag in tags:
+        for tag in queryset:
             try:
                 tag.delete()
                 messages.success(
@@ -192,12 +195,12 @@ def tag_delete(request, tag_id=None, tag_id_list=None):
         'title': ungettext(
             'Delete the selected tag?',
             'Delete the selected tags?',
-            len(tags)
+            queryset.count()
         )
     }
 
-    if len(tags) == 1:
+    if queryset.count() == 1:
-        context['object'] = tags[0]
+        context['object'] = queryset.first()
 
     return render_to_response(
         'appearance/generic_confirm.html', context,
@@ -207,7 +210,9 @@ def tag_delete(request, tag_id=None, tag_id_list=None):
 
 def tag_multiple_delete(request):
     return tag_delete(
-        request, tag_id_list=request.GET.get('id_list', [])
+        request, tag_id_list=request.GET.get(
+            'id_list', request.POST.get('id_list', '')
+        ).split(',')
     )
 
 
@@ -269,12 +274,11 @@ class DocumentTagListView(TagListView):
 
 def tag_remove(request, document_id=None, document_id_list=None, tag_id=None, tag_id_list=None):
     if document_id:
-        documents = [get_object_or_404(Document, pk=document_id)]
+        documents = Document.objects.filter(pk=document_id)
     elif document_id_list:
-        documents = [
+        documents = Document.objects.filter(pk__in=document_id_list)
-            get_object_or_404(Document, pk=document_id) for document_id in document_id_list.split(',')
+
-        ]
+    if not documents:
-    else:
         messages.error(
             request, _('Must provide at least one tagged document.')
         )
@@ -303,28 +307,24 @@ def tag_remove(request, document_id=None, document_id_list=None, tag_id=None, ta
 
     template = 'appearance/generic_confirm.html'
     if tag_id:
-        tags = [get_object_or_404(Tag, pk=tag_id)]
+        tags = Tag.objects.filter(pk=tag_id)
     elif tag_id_list:
-        tags = [
+        tags = Tag.objects.filter(pk__in=tag_id_list)
-            get_object_or_404(
-                Tag, pk=tag_id
-            ) for tag_id in tag_id_list.split(',')
-        ]
     else:
         template = 'appearance/generic_form.html'
 
         if request.method == 'POST':
             form = TagListForm(request.POST, user=request.user)
             if form.is_valid():
-                tags = [form.cleaned_data['tag']]
+                tags = Tag.objects.filter(pk=form.cleaned_data['tag'].pk)
         else:
             if not tag_id and not tag_id_list:
                 form = TagListForm(user=request.user)
-                tags = None
+                tags = Tag.objects.none()
 
         context['form'] = form
         if len(documents) == 1:
-            context['object'] = documents[0]
+            context['object'] = documents.first()
             context['title'] = _(
                 'Remove tag from document: %s.'
             ) % ', '.join([unicode(d) for d in documents])
@@ -334,9 +334,9 @@ def tag_remove(request, document_id=None, document_id_list=None, tag_id=None, ta
             ) % ', '.join([unicode(d) for d in documents])
 
     if tags:
-        if len(tags) == 1:
+        if tags.count() == 1:
-            if len(documents) == 1:
+            if documents.count() == 1:
-                context['object'] = documents[0]
+                context['object'] = documents.first()
                 context['title'] = _(
                     'Remove the tag "%(tag)s" from the document: %(document)s?'
                 ) % {
@@ -350,9 +350,9 @@ def tag_remove(request, document_id=None, document_id_list=None, tag_id=None, ta
                     'tag': ', '.join([unicode(d) for d in tags]),
                     'documents': ', '.join([unicode(d) for d in documents])
                 }
-        elif len(tags) > 1:
+        elif tags.count() > 1:
-            if len(documents) == 1:
+            if documents.count() == 1:
-                context['object'] = documents[0]
+                context['object'] = documents.first()
                 context['title'] = _(
                     'Remove the tags: %(tags)s from the document: %(document)s?'
                 ) % {
@@ -398,9 +398,15 @@ def tag_remove(request, document_id=None, document_id_list=None, tag_id=None, ta
 def single_document_multiple_tag_remove(request, document_id):
     return tag_remove(
         request, document_id=document_id,
-        tag_id_list=request.GET.get('id_list', [])
+        tag_id_list=request.GET.get(
+            'id_list', request.POST.get('id_list', '')
+        ).split(',')
     )
 
 
 def multiple_documents_selection_tag_remove(request):
-    return tag_remove(request, document_id_list=request.GET.get('id_list', []))
+    return tag_remove(
+        request, document_id_list=request.GET.get(
+            'id_list', request.POST.get('id_list', '')
+        ).split(',')
+    )