Update tags app to comply with MERCs 5 and 6

Addionally the permission workflow is updated to work in a
reciprocal manner. In order to attach a tag, the user's role
will need the tag attach permissions for both, the document
and the tag.

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

HISTORY.rst

@@ -213,7 +213,10 @@
 - Event handler to highlight panels when selected.
 - Improve duplicated document display.
 - Filter document duplicted count by access.
-
+- Updated the tags app to comply with MERCs 5 and 6.
+- The tags app permission workflow is now reciprocal. In order
+  to attach a tag, the user's role will need the tag attach
+  permissions for both, the document and the tag.
 
 3.1.9 (2018-11-01)
 ==================

mayan/apps/tags/api_views.py

@@ -1,12 +1,10 @@
 from __future__ import absolute_import, unicode_literals
 
-from django.shortcuts import get_object_or_404
-
 from rest_framework import generics
 from rest_framework.exceptions import ValidationError
 from rest_framework.response import Response
 
-from mayan.apps.acls.models import AccessControlList
+from mayan.apps.common.mixins import ExternalObjectViewMixin
 from mayan.apps.documents.models import Document
 from mayan.apps.documents.permissions import permission_document_view
 from mayan.apps.documents.serializers import DocumentSerializer
@@ -19,8 +17,7 @@ from .permissions import (
     permission_tag_edit, permission_tag_remove, permission_tag_view
 )
 from .serializers import (
-    DocumentTagSerializer, NewDocumentTagSerializer, TagSerializer,
+    DocumentTagSerializer, TagSerializer, WritableTagSerializer
-    WritableTagSerializer
 )
 
 
@@ -56,6 +53,7 @@ class APITagView(generics.RetrieveUpdateDestroyAPIView):
     put: Edit the selected tag.
     """
     filter_backends = (MayanObjectPermissionsFilter,)
+    lookup_url_kwarg = 'tag_pk'
     mayan_object_permissions = {
         'DELETE': (permission_tag_delete,),
         'GET': (permission_tag_view,),
@@ -77,29 +75,31 @@ class APITagView(generics.RetrieveUpdateDestroyAPIView):
             return WritableTagSerializer
 
 
-class APITagDocumentListView(generics.ListAPIView):
+class APITagDocumentListView(ExternalObjectViewMixin, generics.ListAPIView):
     """
     get: Returns a list of all the documents tagged by a particular tag.
     """
+    external_object_class = Tag
+    external_object_pk_url_kwarg = 'tag_pk'
+    external_object_permission = permission_tag_view
     filter_backends = (MayanObjectPermissionsFilter,)
     mayan_object_permissions = {'GET': (permission_document_view,)}
     serializer_class = DocumentSerializer
 
     def get_queryset(self):
-        tag = get_object_or_404(klass=Tag, pk=self.kwargs['pk'])
+        return self.get_tag().documents.all()
 
-        AccessControlList.objects.check_access(
+    def get_tag(self):
-            permissions=permission_tag_view, user=self.request.user, obj=tag
+        return self.get_external_object()
-        )
-
-        return tag.documents.all()
 
 
-class APIDocumentTagListView(generics.ListCreateAPIView):
+class APIDocumentTagListView(ExternalObjectViewMixin, generics.ListCreateAPIView):
     """
     get: Returns a list of all the tags attached to a document.
     post: Attach a tag to a document.
     """
+    external_object_class = Document
+    external_object_pk_url_kwarg = 'document_pk'
     filter_backends = (MayanObjectPermissionsFilter,)
     mayan_object_permissions = {
         'GET': (permission_tag_view,),
@@ -107,17 +107,16 @@ class APIDocumentTagListView(generics.ListCreateAPIView):
     }
 
     def get_document(self):
-        return get_object_or_404(klass=Document, pk=self.kwargs['document_pk'])
+        return self.get_external_object()
+
+    def get_external_object_permission(self):
+        if self.request.method == 'POST':
+            return permission_tag_attach
+        else:
+            return permission_tag_view
 
     def get_queryset(self):
-        document = self.get_document()
+        return self.get_document().get_tags().all()
-
-        AccessControlList.objects.check_access(
-            permissions=permission_document_view, user=self.request.user,
-            obj=document
-        )
-
-        return document.get_tags().all()
 
     def get_serializer(self, *args, **kwargs):
         if not self.request:
@@ -126,10 +125,7 @@ class APIDocumentTagListView(generics.ListCreateAPIView):
         return super(APIDocumentTagListView, self).get_serializer(*args, **kwargs)
 
     def get_serializer_class(self):
-        if self.request.method == 'GET':
+        return DocumentTagSerializer
-            return DocumentTagSerializer
-        elif self.request.method == 'POST':
-            return NewDocumentTagSerializer
 
     def get_serializer_context(self):
         """
@@ -145,16 +141,16 @@ class APIDocumentTagListView(generics.ListCreateAPIView):
 
         return context
 
-    def perform_create(self, serializer):
-        serializer.save(document=self.get_document())
 
-
+class APIDocumentTagView(ExternalObjectViewMixin, generics.RetrieveDestroyAPIView):
-class APIDocumentTagView(generics.RetrieveDestroyAPIView):
     """
     delete: Remove a tag from the selected document.
     get: Returns the details of the selected document tag.
     """
+    external_object_class = Document
+    external_object_pk_url_kwarg = 'document_pk'
     filter_backends = (MayanObjectPermissionsFilter,)
+    lookup_url_kwarg = 'tag_pk'
     mayan_object_permissions = {
         'GET': (permission_tag_view,),
         'DELETE': (permission_tag_remove,)
@@ -162,13 +158,13 @@ class APIDocumentTagView(generics.RetrieveDestroyAPIView):
     serializer_class = DocumentTagSerializer
 
     def get_document(self):
-        document = get_object_or_404(klass=Document, pk=self.kwargs['document_pk'])
+        return self.get_external_object()
 
-        AccessControlList.objects.check_access(
+    def get_external_object_permission(self):
-            permissions=permission_document_view, user=self.request.user,
+        if self.request.method == 'DELETE':
-            obj=document
+            return permission_tag_remove
-        )
+        else:
-        return document
+            return permission_tag_view
 
     def get_queryset(self):
         return self.get_document().get_tags().all()

mayan/apps/tags/models.py

@@ -55,7 +55,9 @@ class Tag(models.Model):
         )
 
     def get_absolute_url(self):
-        return reverse('tags:tag_tagged_item_list', args=(str(self.pk),))
+        return reverse(
+            viewname='tags:tag_tagged_item_list', kwargs={'tag_pk': str(self.pk)}
+        )
 
     def get_document_count(self, user):
         """

mayan/apps/tags/serializers.py

@@ -1,10 +1,9 @@
 from __future__ import absolute_import, unicode_literals
 
-from django.core.exceptions import PermissionDenied
 from django.utils.translation import ugettext_lazy as _
 
 from rest_framework import serializers
-from rest_framework.exceptions import ValidationError
+from rest_framework.generics import get_object_or_404
 from rest_framework.reverse import reverse
 
 from mayan.apps.acls.models import AccessControlList
@@ -16,13 +15,17 @@ from .permissions import permission_tag_attach
 
 class TagSerializer(serializers.HyperlinkedModelSerializer):
     documents_url = serializers.HyperlinkedIdentityField(
+        lookup_field='pk', lookup_url_kwarg='tag_pk',
         view_name='rest_api:tag-document-list'
     )
     documents_count = serializers.SerializerMethodField()
 
     class Meta:
         extra_kwargs = {
-            'url': {'view_name': 'rest_api:tag-detail'},
+            'url': {
+                'lookup_field': 'pk', 'lookup_url_kwarg': 'tag_pk',
+                'view_name': 'rest_api:tag-detail'
+            }
         }
         fields = (
             'color', 'documents_count', 'documents_url', 'id', 'label', 'url'
@@ -88,38 +91,27 @@ class DocumentTagSerializer(TagSerializer):
             'tag URL.'
         )
     )
+    tag_pk = serializers.IntegerField(
+        help_text=_('Primary key of the tag to be added.'), write_only=True
+    )
 
     class Meta(TagSerializer.Meta):
-        fields = TagSerializer.Meta.fields + ('document_tag_url',)
+        fields = TagSerializer.Meta.fields + ('document_tag_url', 'tag_pk')
-        read_only_fields = TagSerializer.Meta.fields
+        read_only_fields = TagSerializer.Meta.fields + ('document_tag_url',)
 
     def get_document_tag_url(self, instance):
         return reverse(
-            'rest_api:document-tag-detail', args=(
+            viewname='rest_api:document-tag-detail', kwargs={
-                self.context['document'].pk, instance.pk
+                'document_pk': self.context['document'].pk,
-            ), request=self.context['request'], format=self.context['format']
+                'tag_pk': instance.pk
+            }, request=self.context['request'], format=self.context['format']
         )
 
-
-class NewDocumentTagSerializer(serializers.Serializer):
-    tag_pk = serializers.IntegerField(
-        help_text=_('Primary key of the tag to be added.')
-    )
-
     def create(self, validated_data):
-        try:
+        queryset = AccessControlList.objects.filter_by_access(
-            tag = Tag.objects.get(pk=validated_data['tag_pk'])
+            permission=permission_tag_attach, queryset=Tag.objects.all(),
-
+            user=self.context['request'].user
-            try:
+        )
-                AccessControlList.objects.check_access(
+        tag = get_object_or_404(queryset=queryset, pk=validated_data['tag_pk'])
-                    permissions=permission_tag_attach,
+        tag.documents.add(self.context['document'])
-                    user=self.context['request'].user, obj=tag
+        return tag
-                )
-            except PermissionDenied:
-                pass
-            else:
-                tag.documents.add(validated_data['document'])
-        except Exception as exception:
-            raise ValidationError(exception)
-
-        return {'tag_pk': tag.pk}

mayan/apps/tags/tests/mixins.py

@@ -14,6 +14,34 @@ class TagTestMixin(object):
             color=TEST_TAG_COLOR, label=TEST_TAG_LABEL
         )
 
+    def _request_api_tag_create_view(self):
+        return self.post(
+            viewname='rest_api:tag-list', data={
+                'label': TEST_TAG_LABEL, 'color': TEST_TAG_COLOR
+            }
+        )
+
+    def _request_api_tag_delete_view(self):
+        return self.delete(
+            viewname='rest_api:tag-detail', kwargs={'tag_pk': self.tag.pk}
+        )
+
+    def _request_api_tag_edit_via_patch_view(self):
+        return self.patch(
+            viewname='rest_api:tag-detail', kwargs={'tag_pk': self.tag.pk}, data={
+                'label': TEST_TAG_LABEL_EDITED,
+                'color': TEST_TAG_COLOR_EDITED
+            }
+        )
+
+    def _request_api_tag_edit_via_put_view(self):
+        return self.put(
+            viewname='rest_api:tag-detail', kwargs={'tag_pk': self.tag.pk}, data={
+                'label': TEST_TAG_LABEL_EDITED,
+                'color': TEST_TAG_COLOR_EDITED
+            }
+        )
+
     def _request_tag_create_view(self):
         return self.post(
             viewname='tags:tag_create', data={
@@ -24,12 +52,12 @@ class TagTestMixin(object):
 
     def _request_tag_delete_view(self):
         return self.post(
-            viewname='tags:tag_delete', args=(self.tag.pk,)
+            viewname='tags:tag_delete', kwargs={'tag_pk': self.tag.pk}
         )
 
     def _request_tag_edit_view(self):
         return self.post(
-            viewname='tags:tag_edit', args=(self.tag.pk,), data={
+            viewname='tags:tag_edit', kwargs={'tag_pk': self.tag.pk}, data={
                 'label': TEST_TAG_LABEL_EDITED, 'color': TEST_TAG_COLOR_EDITED
             }
         )
@@ -42,7 +70,7 @@ class TagTestMixin(object):
 
     def _request_edit_tag_view(self):
         return self.post(
-            viewname='tags:tag_edit', args=(self.tag.pk,), data={
+            viewname='tags:tag_edit', kwargs={'tag_pk': self.tag.pk}, data={
                 'label': TEST_TAG_LABEL_EDITED, 'color': TEST_TAG_COLOR_EDITED
             }
         )
@@ -57,7 +85,8 @@ class TagTestMixin(object):
 
     def _request_attach_tag_view(self):
         return self.post(
-            viewname='tags:tag_attach', args=(self.document.pk,), data={
+            viewname='tags:tag_attach',
+            kwargs={'document_pk': self.document.pk}, data={
                 'tags': self.tag.pk,
                 'user': self.user.pk
             }
@@ -74,7 +103,7 @@ class TagTestMixin(object):
     def _request_single_document_multiple_tag_remove_view(self):
         return self.post(
             viewname='tags:single_document_multiple_tag_remove',
-            args=(self.document.pk,), data={
+            kwargs={'document_pk': self.document.pk}, data={
                 'id_list': self.document.pk,
                 'tags': self.tag.pk,
             }

mayan/apps/tags/tests/test_api.py

@@ -18,35 +18,22 @@ from .literals import (
     TEST_TAG_COLOR, TEST_TAG_COLOR_EDITED, TEST_TAG_LABEL,
     TEST_TAG_LABEL_EDITED
 )
+from .mixins import TagTestMixin
 
 
-class TagAPITestCase(DocumentTestMixin, BaseAPITestCase):
+class TagAPITestCase(TagTestMixin, DocumentTestMixin, BaseAPITestCase):
-    auto_upload_document = False
-
     def setUp(self):
         super(TagAPITestCase, self).setUp()
         self.login_user()
 
-    def _create_tag(self):
-        return Tag.objects.create(
-            color=TEST_TAG_COLOR, label=TEST_TAG_LABEL
-        )
-
-    def _request_tag_create_view(self):
-        return self.post(
-            viewname='rest_api:tag-list', data={
-                'label': TEST_TAG_LABEL, 'color': TEST_TAG_COLOR
-            }
-        )
-
     def test_tag_create_view_no_permission(self):
-        response = self._request_tag_create_view()
+        response = self._request_api_tag_create_view()
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
         self.assertEqual(Tag.objects.count(), 0)
 
     def test_tag_create_view_with_permission(self):
         self.grant_permission(permission=permission_tag_create)
-        response = self._request_tag_create_view()
+        response = self._request_api_tag_create_view()
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
 
         tag = Tag.objects.first()
@@ -58,276 +45,285 @@ class TagAPITestCase(DocumentTestMixin, BaseAPITestCase):
         self.assertEqual(tag.label, TEST_TAG_LABEL)
         self.assertEqual(tag.color, TEST_TAG_COLOR)
 
-    def _request_tag_delete_view(self):
-        return self.delete(
-            viewname='rest_api:tag-detail', args=(self.tag.pk,)
-        )
-
     def test_tag_delete_view_no_access(self):
-        self.tag = self._create_tag()
+        self._create_tag()
-        response = self._request_tag_delete_view()
+        response = self._request_api_tag_delete_view()
         self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
         self.assertTrue(self.tag in Tag.objects.all())
+        self.assertEqual(Tag.objects.all().count(), 1)
 
     def test_tag_delete_view_with_access(self):
-        self.tag = self._create_tag()
+        self._create_tag()
-        self.grant_access(permission=permission_tag_delete, obj=self.tag)
+        self.grant_access(obj=self.tag, permission=permission_tag_delete)
-        response = self._request_tag_delete_view()
+        response = self._request_api_tag_delete_view()
         self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
-        self.assertFalse(self.tag in Tag.objects.all())
+        self.assertEqual(Tag.objects.all().count(), 0)
 
-    def _request_tag_document_list_view(self):
+    def test_tag_edit_via_patch_no_access(self):
+        self._create_tag()
+        response = self._request_api_tag_edit_via_patch_view()
+        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+        self.tag.refresh_from_db()
+        self.assertEqual(self.tag.label, TEST_TAG_LABEL)
+        self.assertEqual(self.tag.color, TEST_TAG_COLOR)
+        self.assertEqual(Tag.objects.all().count(), 1)
+
+    def test_tag_edit_via_patch_with_access(self):
+        self._create_tag()
+        self.grant_access(obj=self.tag, permission=permission_tag_edit)
+        response = self._request_api_tag_edit_via_patch_view()
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.tag.refresh_from_db()
+        self.assertEqual(self.tag.label, TEST_TAG_LABEL_EDITED)
+        self.assertEqual(self.tag.color, TEST_TAG_COLOR_EDITED)
+        self.assertEqual(Tag.objects.all().count(), 1)
+
+    def test_tag_edit_via_put_no_access(self):
+        self._create_tag()
+        response = self._request_api_tag_edit_via_put_view()
+        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+        self.tag.refresh_from_db()
+        self.assertEqual(self.tag.label, TEST_TAG_LABEL)
+        self.assertEqual(self.tag.color, TEST_TAG_COLOR)
+        self.assertEqual(Tag.objects.all().count(), 1)
+
+    def test_tag_edit_via_put_with_access(self):
+        self._create_tag()
+        self.grant_access(obj=self.tag, permission=permission_tag_edit)
+        response = self._request_api_tag_edit_via_put_view()
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.tag.refresh_from_db()
+        self.assertEqual(self.tag.label, TEST_TAG_LABEL_EDITED)
+        self.assertEqual(self.tag.color, TEST_TAG_COLOR_EDITED)
+        self.assertEqual(Tag.objects.all().count(), 1)
+
+
+class DocumentAPITestCase(TagTestMixin, DocumentTestMixin, BaseAPITestCase):
+    auto_upload_document = False
+
+    def setUp(self):
+        super(DocumentAPITestCase, self).setUp()
+        self.login_user()
+
+    def _request_api_tag_document_list_view(self):
         return self.get(
-            viewname='rest_api:tag-document-list', args=(self.tag.pk,)
+            viewname='rest_api:tag-document-list',
+            kwargs={'tag_pk': self.tag.pk}
         )
 
     def test_tag_document_list_view_no_access(self):
-        self.tag = self._create_tag()
+        self._create_tag()
         self.document = self.upload_document()
         self.tag.documents.add(self.document)
-        response = self._request_tag_document_list_view()
+        response = self._request_api_tag_document_list_view()
-        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
 
     def test_tag_document_list_view_with_tag_access(self):
-        self.tag = self._create_tag()
+        self._create_tag()
         self.document = self.upload_document()
         self.tag.documents.add(self.document)
-        self.grant_access(permission=permission_tag_view, obj=self.tag)
+        self.grant_access(obj=self.tag, permission=permission_tag_view)
-        response = self._request_tag_document_list_view()
+        response = self._request_api_tag_document_list_view()
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(response.data['count'], 0)
 
     def test_tag_document_list_view_with_document_access(self):
-        self.tag = self._create_tag()
+        self._create_tag()
         self.document = self.upload_document()
         self.tag.documents.add(self.document)
         self.grant_access(
-            permission=permission_document_view, obj=self.document
+            obj=self.document, permission=permission_document_view
         )
-        response = self._request_tag_document_list_view()
+        response = self._request_api_tag_document_list_view()
-        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
 
     def test_tag_document_list_view_with_access(self):
-        self.tag = self._create_tag()
+        self._create_tag()
         self.document = self.upload_document()
         self.tag.documents.add(self.document)
-        self.grant_access(permission=permission_tag_view, obj=self.tag)
+        self.grant_access(obj=self.tag, permission=permission_tag_view)
         self.grant_access(
-            permission=permission_document_view, obj=self.document
+            obj=self.document, permission=permission_document_view
         )
-        response = self._request_tag_document_list_view()
+        response = self._request_api_tag_document_list_view()
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(
             response.data['results'][0]['uuid'],
             force_text(self.document.uuid)
         )
 
-    def _request_tag_edit_via_patch_view(self):
+    def _request_api_document_attach_tag_view(self):
-        return self.patch(
-            viewname='rest_api:tag-detail', args=(self.tag.pk,), data={
-                'label': TEST_TAG_LABEL_EDITED,
-                'color': TEST_TAG_COLOR_EDITED
-            }
-        )
-
-    def test_tag_edit_via_patch_no_access(self):
-        self.tag = self._create_tag()
-        response = self._request_tag_edit_via_patch_view()
-        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
-        self.tag.refresh_from_db()
-        self.assertEqual(self.tag.label, TEST_TAG_LABEL)
-        self.assertEqual(self.tag.color, TEST_TAG_COLOR)
-
-    def test_tag_edit_via_patch_with_access(self):
-        self.tag = self._create_tag()
-        self.grant_access(permission=permission_tag_edit, obj=self.tag)
-        response = self._request_tag_edit_via_patch_view()
-        self.assertEqual(response.status_code, status.HTTP_200_OK)
-        self.tag.refresh_from_db()
-        self.assertEqual(self.tag.label, TEST_TAG_LABEL_EDITED)
-        self.assertEqual(self.tag.color, TEST_TAG_COLOR_EDITED)
-
-    def _request_tag_edit_via_put_view(self):
-        return self.put(
-            viewname='rest_api:tag-detail', args=(self.tag.pk,), data={
-                'label': TEST_TAG_LABEL_EDITED,
-                'color': TEST_TAG_COLOR_EDITED
-            }
-        )
-
-    def test_tag_edit_via_put_no_access(self):
-        self.tag = self._create_tag()
-        response = self._request_tag_edit_via_put_view()
-        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
-        self.tag.refresh_from_db()
-        self.assertEqual(self.tag.label, TEST_TAG_LABEL)
-        self.assertEqual(self.tag.color, TEST_TAG_COLOR)
-
-    def test_tag_edit_via_put_with_access(self):
-        self.tag = self._create_tag()
-        self.grant_access(permission=permission_tag_edit, obj=self.tag)
-        response = self._request_tag_edit_via_put_view()
-        self.assertEqual(response.status_code, status.HTTP_200_OK)
-        self.tag.refresh_from_db()
-        self.assertEqual(self.tag.label, TEST_TAG_LABEL_EDITED)
-        self.assertEqual(self.tag.color, TEST_TAG_COLOR_EDITED)
-
-    def _request_document_attach_tag_view(self):
         return self.post(
-            viewname='rest_api:document-tag-list', args=(self.document.pk,),
+            viewname='rest_api:document-tag-list',
+            kwargs={'document_pk': self.document.pk},
             data={'tag_pk': self.tag.pk}
         )
 
     def test_document_attach_tag_view_no_access(self):
-        self.tag = self._create_tag()
+        self._create_tag()
         self.document = self.upload_document()
-        response = self._request_document_attach_tag_view()
+        response = self._request_api_document_attach_tag_view()
-        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
-        self.assertFalse(self.tag in self.document.tags.all())
+        self.assertTrue(self.tag not in self.document.tags.all())
+        self.assertEqual(Tag.objects.all().count(), 1)
 
     def test_document_attach_tag_view_with_document_access(self):
-        self.tag = self._create_tag()
+        self._create_tag()
         self.document = self.upload_document()
-        self.grant_access(permission=permission_tag_attach, obj=self.document)
+        self.grant_access(obj=self.document, permission=permission_tag_attach)
-        response = self._request_document_attach_tag_view()
+        response = self._request_api_document_attach_tag_view()
-        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
-        self.assertFalse(self.tag in self.document.tags.all())
+        self.assertTrue(self.tag not in self.document.tags.all())
+        self.assertEqual(Tag.objects.all().count(), 1)
 
     def test_document_attach_tag_view_with_tag_access(self):
-        self.tag = self._create_tag()
+        self._create_tag()
         self.document = self.upload_document()
-        self.grant_access(permission=permission_tag_attach, obj=self.tag)
+        self.grant_access(obj=self.tag, permission=permission_tag_attach)
-        response = self._request_document_attach_tag_view()
+        response = self._request_api_document_attach_tag_view()
+        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+        self.assertTrue(self.tag not in self.document.tags.all())
+        self.assertEqual(Tag.objects.all().count(), 1)
+
+    def test_document_attach_tag_view_with_full_access(self):
+        self._create_tag()
+        self.document = self.upload_document()
+        self.grant_access(obj=self.document, permission=permission_tag_attach)
+        self.grant_access(obj=self.tag, permission=permission_tag_attach)
+        response = self._request_api_document_attach_tag_view()
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
         self.assertTrue(self.tag in self.document.tags.all())
+        self.assertEqual(Tag.objects.all().count(), 1)
 
-    def test_document_attach_tag_view_with_access(self):
+    def _request_api_document_tag_detail_view(self):
-        self.tag = self._create_tag()
-        self.document = self.upload_document()
-        self.grant_access(permission=permission_tag_attach, obj=self.document)
-        self.grant_access(permission=permission_tag_attach, obj=self.tag)
-        response = self._request_document_attach_tag_view()
-        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
-        self.assertTrue(self.tag in self.document.tags.all())
-
-    def _request_document_tag_detail_view(self):
         return self.get(
-            viewname='rest_api:document-tag-detail', args=(
+            viewname='rest_api:document-tag-detail', kwargs={
-                self.document.pk, self.tag.pk
+                'document_pk': self.document.pk, 'tag_pk': self.tag.pk
-            )
+            }
         )
 
-    def test_document_tag_detail_view_no_access(self):
+    def test_document_tag_detail_view_no_permission(self):
-        self.tag = self._create_tag()
+        self._create_tag()
         self.document = self.upload_document()
         self.tag.documents.add(self.document)
-        response = self._request_document_tag_detail_view()
+        response = self._request_api_document_tag_detail_view()
-        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
 
     def test_document_tag_detail_view_with_document_access(self):
-        self.tag = self._create_tag()
+        self._create_tag()
         self.document = self.upload_document()
         self.tag.documents.add(self.document)
-        self.grant_access(permission=permission_document_view, obj=self.document)
+        self.grant_access(
-        response = self._request_document_tag_detail_view()
+            obj=self.document, permission=permission_document_view
+        )
+        response = self._request_api_document_tag_detail_view()
         self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
 
     def test_document_tag_detail_view_with_tag_access(self):
-        self.tag = self._create_tag()
+        self._create_tag()
         self.document = self.upload_document()
         self.tag.documents.add(self.document)
-        self.grant_access(permission=permission_tag_view, obj=self.tag)
+        self.grant_access(obj=self.tag, permission=permission_tag_view)
-        response = self._request_document_tag_detail_view()
+        response = self._request_api_document_tag_detail_view()
-        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
 
-    def test_document_tag_detail_view_with_access(self):
+    def test_document_tag_detail_view_with_full_access(self):
-        self.tag = self._create_tag()
+        self._create_tag()
         self.document = self.upload_document()
         self.tag.documents.add(self.document)
-        self.grant_access(permission=permission_tag_view, obj=self.tag)
+        self.grant_access(obj=self.tag, permission=permission_tag_view)
-        self.grant_access(permission=permission_document_view, obj=self.document)
+        self.grant_access(
-        response = self._request_document_tag_detail_view()
+            obj=self.document, permission=permission_tag_view
+        )
+        response = self._request_api_document_tag_detail_view()
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(response.data['label'], self.tag.label)
 
-    def _request_document_tag_list_view(self):
+    def _request_api_document_tag_list_view(self):
         return self.get(
-            viewname='rest_api:document-tag-list', args=(self.document.pk,)
+            viewname='rest_api:document-tag-list',
+            kwargs={'document_pk': self.document.pk}
         )
 
     def test_document_tag_list_view_no_access(self):
-        self.tag = self._create_tag()
+        self._create_tag()
         self.document = self.upload_document()
         self.tag.documents.add(self.document)
-        response = self._request_document_tag_list_view()
+        response = self._request_api_document_tag_list_view()
-        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
 
     def test_document_tag_list_view_with_document_access(self):
-        self.tag = self._create_tag()
+        self._create_tag()
         self.document = self.upload_document()
         self.tag.documents.add(self.document)
-        self.grant_access(permission=permission_document_view, obj=self.document)
+        self.grant_access(obj=self.document, permission=permission_tag_view)
-        response = self._request_document_tag_list_view()
+        response = self._request_api_document_tag_list_view()
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(response.data['count'], 0)
 
     def test_document_tag_list_view_with_tag_access(self):
-        self.tag = self._create_tag()
+        self._create_tag()
         self.document = self.upload_document()
         self.tag.documents.add(self.document)
-        self.grant_access(permission=permission_tag_view, obj=self.tag)
+        self.grant_access(obj=self.tag, permission=permission_tag_view)
-        response = self._request_document_tag_list_view()
+        response = self._request_api_document_tag_list_view()
-        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
 
-    def test_document_tag_list_view_with_access(self):
+    def test_document_tag_list_view_with_full_access(self):
-        self.tag = self._create_tag()
+        self._create_tag()
         self.document = self.upload_document()
         self.tag.documents.add(self.document)
-        self.grant_access(permission=permission_document_view, obj=self.document)
+        self.grant_access(obj=self.document, permission=permission_tag_view)
-        self.grant_access(permission=permission_tag_view, obj=self.tag)
+        self.grant_access(obj=self.tag, permission=permission_tag_view)
-        response = self._request_document_tag_list_view()
+        response = self._request_api_document_tag_list_view()
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         self.assertEqual(response.data['results'][0]['label'], self.tag.label)
 
-    def _request_document_tag_remove_view(self):
+    def _request_api_document_tag_remove_view(self):
         return self.delete(
-            viewname='rest_api:document-tag-detail', args=(
+            viewname='rest_api:document-tag-detail', kwargs={
-                self.document.pk, self.tag.pk
+                'document_pk': self.document.pk, 'tag_pk': self.tag.pk
-            )
+            }
         )
 
     def test_document_tag_remove_view_no_access(self):
-        self.tag = self._create_tag()
+        self._create_tag()
         self.document = self.upload_document()
         self.tag.documents.add(self.document)
-        response = self._request_document_tag_remove_view()
+        response = self._request_api_document_tag_remove_view()
-        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
         self.assertTrue(self.tag in self.document.tags.all())
+        self.assertEqual(Tag.objects.all().count(), 1)
 
     def test_document_tag_remove_view_with_document_access(self):
-        self.tag = self._create_tag()
+        self._create_tag()
         self.document = self.upload_document()
         self.tag.documents.add(self.document)
-        self.grant_access(permission=permission_tag_remove, obj=self.document)
+        self.grant_access(obj=self.document, permission=permission_tag_remove)
-        response = self._request_document_tag_remove_view()
+        response = self._request_api_document_tag_remove_view()
-        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
         self.assertTrue(self.tag in self.document.tags.all())
+        self.assertEqual(Tag.objects.all().count(), 1)
 
     def test_document_tag_remove_view_with_tag_access(self):
-        self.tag = self._create_tag()
+        self._create_tag()
         self.document = self.upload_document()
         self.tag.documents.add(self.document)
-        self.grant_access(permission=permission_tag_remove, obj=self.tag)
+        self.grant_access(obj=self.tag, permission=permission_tag_remove)
-        response = self._request_document_tag_remove_view()
+        response = self._request_api_document_tag_remove_view()
-        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
         self.assertTrue(self.tag in self.document.tags.all())
+        self.assertEqual(Tag.objects.all().count(), 1)
 
-    def test_document_tag_remove_view_with_access(self):
+    def test_document_tag_remove_view_with_full_access(self):
-        self.tag = self._create_tag()
+        self._create_tag()
         self.document = self.upload_document()
         self.tag.documents.add(self.document)
-        self.grant_access(permission=permission_document_view, obj=self.document)
+        self.grant_access(
-        self.grant_access(permission=permission_tag_remove, obj=self.tag)
+            obj=self.document, permission=permission_tag_remove
-        response = self._request_document_tag_remove_view()
+        )
+        self.grant_access(obj=self.tag, permission=permission_tag_remove)
+        response = self._request_api_document_tag_remove_view()
         self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
         self.assertFalse(self.tag in self.document.tags.all())
+        self.assertEqual(Tag.objects.all().count(), 1)

mayan/apps/tags/tests/test_events.py

@@ -12,9 +12,11 @@ from .mixins import TagTestMixin
 
 
 class TagEventsTestCase(TagTestMixin, GenericDocumentViewTestCase):
-    def test_tag_create_event_no_permissions(self):
+    def setUp(self):
+        super(TagEventsTestCase, self).setUp()
         self.login_user()
 
+    def test_tag_create_event_no_permissions(self):
         Action.objects.all().delete()
 
         response = self._request_tag_create_view()
@@ -22,8 +24,6 @@ class TagEventsTestCase(TagTestMixin, GenericDocumentViewTestCase):
         self.assertEqual(Action.objects.count(), 0)
 
     def test_tag_create_event_with_permissions(self):
-        self.login_user()
-
         Action.objects.all().delete()
 
         self.grant_permission(permission=permission_tag_create)
@@ -42,20 +42,14 @@ class TagEventsTestCase(TagTestMixin, GenericDocumentViewTestCase):
 
     def test_tag_edit_event_no_permissions(self):
         self._create_tag()
-
-        self.login_user()
-
         Action.objects.all().delete()
 
         response = self._request_tag_edit_view()
-        self.assertEqual(response.status_code, 403)
+        self.assertEqual(response.status_code, 404)
         self.assertEqual(Action.objects.count(), 0)
 
     def test_tag_edit_event_with_access(self):
         self._create_tag()
-
-        self.login_user()
-
         Action.objects.all().delete()
 
         self.grant_access(

mayan/apps/tags/tests/test_views.py

@@ -81,7 +81,7 @@ class TagViewTestCase(TagTestMixin, GenericViewTestCase):
         self._create_tag()
 
         response = self._request_edit_tag_view()
-        self.assertEqual(response.status_code, 403)
+        self.assertEqual(response.status_code, 404)
         tag = Tag.objects.get(pk=self.tag.pk)
         self.assertEqual(tag.label, TEST_TAG_LABEL)
         self.assertEqual(tag.color, TEST_TAG_COLOR)

mayan/apps/tags/tests/test_wizard_steps.py

@@ -29,7 +29,8 @@ class TaggedDocumentUploadTestCase(GenericDocumentViewTestCase):
     def _request_upload_interactive_document_create_view(self):
         with open(TEST_SMALL_DOCUMENT_PATH, mode='rb') as file_object:
             return self.post(
-                viewname='sources:upload_interactive', args=(self.source.pk,),
+                viewname='sources:upload_interactive',
+                kwargs={'source_id': self.source.pk},
                 data={
                     'document_type_id': self.document_type.pk,
                     'source-file': file_object,

mayan/apps/tags/urls.py

@@ -13,61 +13,70 @@ from .views import (
 )
 
 urlpatterns = [
-    url(r'^list/$', TagListView.as_view(), name='tag_list'),
+    url(regex=r'^tags/list/$', name='tag_list', view=TagListView.as_view()),
-    url(r'^create/$', TagCreateView.as_view(), name='tag_create'),
     url(
-        r'^(?P<pk>\d+)/delete/$', TagDeleteActionView.as_view(),
+        regex=r'^tags/create/$', name='tag_create',
-        name='tag_delete'
+        view=TagCreateView.as_view()
-    ),
-    url(r'^(?P<pk>\d+)/edit/$', TagEditView.as_view(), name='tag_edit'),
-    url(
-        r'^(?P<pk>\d+)/documents/$', TagTaggedItemListView.as_view(),
-        name='tag_tagged_item_list'
     ),
     url(
-        r'^multiple/delete/$', TagDeleteActionView.as_view(),
+        regex=r'^tags/(?P<tag_pk>\d+)/delete/$', name='tag_delete',
-        name='tag_multiple_delete'
+        view=TagDeleteActionView.as_view()
+    ),
+    url(
+        regex=r'^tags/(?P<tag_pk>\d+)/edit/$', name='tag_edit',
+        view=TagEditView.as_view()
+    ),
+    url(
+        regex=r'^tags/(?P<tag_pk>\d+)/documents/$',
+        name='tag_tagged_item_list', view=TagTaggedItemListView.as_view()
+    ),
+    url(
+        regex=r'^tags/multiple/delete/$', name='tag_multiple_delete',
+        view=TagDeleteActionView.as_view()
+    ),
+    url(
+        regex=r'^tags/multiple/remove/document/(?P<document_pk>\d+)/$',
+        name='single_document_multiple_tag_remove',
+        view=TagRemoveActionView.as_view()
+    ),
+    url(
+        regex=r'^tags/multiple/remove/document/multiple/$',
+        name='multiple_documents_selection_tag_remove',
+        view=TagRemoveActionView.as_view()
     ),
 
     url(
-        r'^multiple/remove/document/(?P<pk>\d+)/$',
+        regex=r'^documents/(?P<document_pk>\d+)/attach/$',
-        TagRemoveActionView.as_view(),
+        name='tag_attach', view=TagAttachActionView.as_view()
-        name='single_document_multiple_tag_remove'
     ),
     url(
-        r'^multiple/remove/document/multiple/$',
+        regex=r'^documents/multiple/attach//$',
-        TagRemoveActionView.as_view(),
+        name='multiple_documents_tag_attach',
-        name='multiple_documents_selection_tag_remove'
+        view=TagAttachActionView.as_view()
     ),
 
     url(
-        r'^selection/attach/document/(?P<pk>\d+)/$',
+        regex=r'^documents/(?P<document_pk>\d+)/tags/$', name='document_tags',
-        TagAttachActionView.as_view(), name='tag_attach'
+        view=DocumentTagListView.as_view(),
-    ),
-    url(
-        r'^selection/attach/document/multiple/$',
-        TagAttachActionView.as_view(), name='multiple_documents_tag_attach'
-    ),
-
-    url(
-        r'^document/(?P<pk>\d+)/tags/$', DocumentTagListView.as_view(),
-        name='document_tags'
     ),
 ]
 
 api_urls = [
     url(
-        r'^tags/(?P<pk>[0-9]+)/documents/$', APITagDocumentListView.as_view(),
+        regex=r'^tags/(?P<tag_pk>\d+)/documents/$',
-        name='tag-document-list'
+        name='tag-document-list', view=APITagDocumentListView.as_view(),
-    ),
-    url(r'^tags/(?P<pk>[0-9]+)/$', APITagView.as_view(), name='tag-detail'),
-    url(r'^tags/$', APITagListView.as_view(), name='tag-list'),
-    url(
-        r'^documents/(?P<document_pk>[0-9]+)/tags/$',
-        APIDocumentTagListView.as_view(), name='document-tag-list'
     ),
     url(
-        r'^documents/(?P<document_pk>[0-9]+)/tags/(?P<pk>[0-9]+)/$',
+        regex=r'^tags/(?P<tag_pk>\d+)/$', name='tag-detail',
-        APIDocumentTagView.as_view(), name='document-tag-detail'
+        view=APITagView.as_view()
+    ),
+    url(regex=r'^tags/$', name='tag-list', view=APITagListView.as_view()),
+    url(
+        regex=r'^documents/(?P<document_pk>\d+)/tags/$',
+        name='document-tag-list', view=APIDocumentTagListView.as_view()
+    ),
+    url(
+        regex=r'^documents/(?P<document_pk>\d+)/tags/(?P<tag_pk>[0-9]+)/$',
+        name='document-tag-detail', view=APIDocumentTagView.as_view()
     ),
 ]

mayan/apps/tags/views.py

@@ -34,6 +34,7 @@ logger = logging.getLogger(__name__)
 class TagAttachActionView(MultipleObjectFormActionView):
     form_class = TagMultipleSelectionForm
     model = Document
+    pk_url_kwarg = 'document_pk'
     object_permission = permission_tag_attach
     success_message = _('Tag attach request performed on %(count)d document')
     success_message_plural = _(
@@ -75,7 +76,9 @@ class TagAttachActionView(MultipleObjectFormActionView):
         if queryset.count() == 1:
             result.update(
                 {
-                    'queryset': Tag.objects.exclude(pk__in=queryset.first().tags.all())
+                    'queryset': Tag.objects.exclude(
+                        pk__in=queryset.first().tags.all()
+                    )
                 }
             )
 
@@ -84,7 +87,11 @@ class TagAttachActionView(MultipleObjectFormActionView):
     def get_post_action_redirect(self):
         queryset = self.get_queryset()
         if queryset.count() == 1:
-            return reverse('tags:document_tags', args=(queryset.first().pk,))
+            return reverse(
+                viewname='tags:document_tags', kwargs={
+                    'document_pk': queryset.first().pk
+                }
+            )
         else:
             return super(TagAttachActionView, self).get_post_action_redirect()
 
@@ -132,6 +139,7 @@ class TagCreateView(SingleObjectCreateView):
 
 class TagDeleteActionView(MultipleObjectConfirmActionView):
     model = Tag
+    pk_url_kwarg = 'tag_pk'
     post_action_redirect = reverse_lazy('tags:tag_list')
     object_permission = permission_tag_delete
     success_message = _('Tag delete request performed on %(count)d tag')
@@ -181,6 +189,8 @@ class TagEditView(SingleObjectEditView):
     fields = ('label', 'color')
     model = Tag
     object_permission = permission_tag_edit
+    object_permission_raise_404 = True
+    pk_url_kwarg = 'tag_pk'
     post_action_redirect = reverse_lazy('tags:tag_list')
 
     def get_extra_context(self):
@@ -221,7 +231,7 @@ class TagListView(SingleObjectListView):
 
 class TagTaggedItemListView(DocumentListView):
     def get_tag(self):
-        return get_object_or_404(klass=Tag, pk=self.kwargs['pk'])
+        return get_object_or_404(klass=Tag, pk=self.kwargs['tag_pk'])
 
     def get_document_queryset(self):
         return self.get_tag().documents.all()
@@ -240,7 +250,9 @@ class TagTaggedItemListView(DocumentListView):
 
 class DocumentTagListView(TagListView):
     def dispatch(self, request, *args, **kwargs):
-        self.document = get_object_or_404(klass=Document, pk=self.kwargs['pk'])
+        self.document = get_object_or_404(
+            klass=Document, pk=self.kwargs['document_pk']
+        )
 
         AccessControlList.objects.check_access(
             permissions=permission_document_view, user=request.user,
@@ -256,12 +268,12 @@ class DocumentTagListView(TagListView):
         context.update(
             {
                 'hide_link': True,
-                'no_results_title': _('Document has no tags attached'),
                 'no_results_main_link': link_tag_attach.resolve(
                     context=RequestContext(
                         self.request, {'object': self.document}
                     )
                 ),
+                'no_results_title': _('Document has no tags attached'),
                 'object': self.document,
                 'title': _('Tags for document: %s') % self.document,
             }
@@ -300,7 +312,9 @@ class TagRemoveActionView(MultipleObjectFormActionView):
             result.update(
                 {
                     'object': queryset.first(),
-                    'title': _('Remove tags from document: %s') % queryset.first()
+                    'title': _(
+                        'Remove tags from document: %s'
+                    ) % queryset.first()
                 }
             )
 
@@ -326,7 +340,11 @@ class TagRemoveActionView(MultipleObjectFormActionView):
     def get_post_action_redirect(self):
         queryset = self.get_queryset()
         if queryset.count() == 1:
-            return reverse('tags:document_tags', args=(queryset.first().pk,))
+            return reverse(
+                viewname='tags:document_tags', kwargs={
+                    'document_pk': queryset.first().pk
+                }
+            )
         else:
             return super(TagRemoveActionView, self).get_post_action_redirect()