diff --git a/HISTORY.rst b/HISTORY.rst index 26b4be0f83..0ff65ca33d 100644 --- a/HISTORY.rst +++ b/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) ================== diff --git a/mayan/apps/tags/api_views.py b/mayan/apps/tags/api_views.py index 54d82b2bbc..e4d109c08f 100644 --- a/mayan/apps/tags/api_views.py +++ b/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, - WritableTagSerializer + DocumentTagSerializer, TagSerializer, 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( - permissions=permission_tag_view, user=self.request.user, obj=tag - ) - - return tag.documents.all() + def get_tag(self): + return self.get_external_object() -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() - - AccessControlList.objects.check_access( - permissions=permission_document_view, user=self.request.user, - obj=document - ) - - return document.get_tags().all() + return self.get_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 - elif self.request.method == 'POST': - return NewDocumentTagSerializer + return DocumentTagSerializer 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(generics.RetrieveDestroyAPIView): +class APIDocumentTagView(ExternalObjectViewMixin, 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( - permissions=permission_document_view, user=self.request.user, - obj=document - ) - return document + def get_external_object_permission(self): + if self.request.method == 'DELETE': + return permission_tag_remove + else: + return permission_tag_view def get_queryset(self): return self.get_document().get_tags().all() diff --git a/mayan/apps/tags/models.py b/mayan/apps/tags/models.py index 1e734f4477..0e49ae2f27 100644 --- a/mayan/apps/tags/models.py +++ b/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): """ diff --git a/mayan/apps/tags/serializers.py b/mayan/apps/tags/serializers.py index 64c3c01ded..5048d10324 100644 --- a/mayan/apps/tags/serializers.py +++ b/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',) - read_only_fields = TagSerializer.Meta.fields + fields = TagSerializer.Meta.fields + ('document_tag_url', 'tag_pk') + read_only_fields = TagSerializer.Meta.fields + ('document_tag_url',) def get_document_tag_url(self, instance): return reverse( - 'rest_api:document-tag-detail', args=( - self.context['document'].pk, instance.pk - ), request=self.context['request'], format=self.context['format'] + viewname='rest_api:document-tag-detail', kwargs={ + 'document_pk': self.context['document'].pk, + '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: - tag = Tag.objects.get(pk=validated_data['tag_pk']) - - try: - AccessControlList.objects.check_access( - permissions=permission_tag_attach, - user=self.context['request'].user, obj=tag - ) - except PermissionDenied: - pass - else: - tag.documents.add(validated_data['document']) - except Exception as exception: - raise ValidationError(exception) - - return {'tag_pk': tag.pk} + queryset = AccessControlList.objects.filter_by_access( + permission=permission_tag_attach, queryset=Tag.objects.all(), + user=self.context['request'].user + ) + tag = get_object_or_404(queryset=queryset, pk=validated_data['tag_pk']) + tag.documents.add(self.context['document']) + return tag diff --git a/mayan/apps/tags/tests/mixins.py b/mayan/apps/tags/tests/mixins.py index d12ef43176..d93041edb5 100644 --- a/mayan/apps/tags/tests/mixins.py +++ b/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, } diff --git a/mayan/apps/tags/tests/test_api.py b/mayan/apps/tags/tests/test_api.py index 625c9be6f1..ec55335692 100644 --- a/mayan/apps/tags/tests/test_api.py +++ b/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): - auto_upload_document = False - +class TagAPITestCase(TagTestMixin, DocumentTestMixin, BaseAPITestCase): 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() - response = self._request_tag_delete_view() + self._create_tag() + 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.grant_access(permission=permission_tag_delete, obj=self.tag) - response = self._request_tag_delete_view() + self._create_tag() + self.grant_access(obj=self.tag, permission=permission_tag_delete) + 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() - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + response = self._request_api_tag_document_list_view() + 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) - response = self._request_tag_document_list_view() + self.grant_access(obj=self.tag, permission=permission_tag_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() - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + response = self._request_api_tag_document_list_view() + 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): - 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): + def _request_api_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() - self.assertEqual(response.status_code, status.HTTP_201_CREATED) - self.assertFalse(self.tag in self.document.tags.all()) + 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_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) - response = self._request_document_attach_tag_view() - self.assertEqual(response.status_code, status.HTTP_201_CREATED) - self.assertFalse(self.tag in self.document.tags.all()) + self.grant_access(obj=self.document, permission=permission_tag_attach) + 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_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) - response = self._request_document_attach_tag_view() + 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_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): - 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): + def _request_api_document_tag_detail_view(self): return self.get( - viewname='rest_api:document-tag-detail', args=( - self.document.pk, self.tag.pk - ) + viewname='rest_api:document-tag-detail', kwargs={ + 'document_pk': self.document.pk, 'tag_pk': self.tag.pk + } ) - def test_document_tag_detail_view_no_access(self): - self.tag = self._create_tag() + def test_document_tag_detail_view_no_permission(self): + self._create_tag() self.document = self.upload_document() self.tag.documents.add(self.document) - response = self._request_document_tag_detail_view() - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + 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_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) - response = self._request_document_tag_detail_view() + self.grant_access( + 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) - response = self._request_document_tag_detail_view() - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.grant_access(obj=self.tag, permission=permission_tag_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_access(self): - self.tag = self._create_tag() + def test_document_tag_detail_view_with_full_access(self): + 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(permission=permission_document_view, obj=self.document) - response = self._request_document_tag_detail_view() + self.grant_access(obj=self.tag, permission=permission_tag_view) + self.grant_access( + 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() - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + response = self._request_api_document_tag_list_view() + 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) - response = self._request_document_tag_list_view() + self.grant_access(obj=self.document, permission=permission_tag_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) - response = self._request_document_tag_list_view() - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.grant_access(obj=self.tag, permission=permission_tag_view) + response = self._request_api_document_tag_list_view() + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) - def test_document_tag_list_view_with_access(self): - self.tag = self._create_tag() + def test_document_tag_list_view_with_full_access(self): + 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(permission=permission_tag_view, obj=self.tag) - response = self._request_document_tag_list_view() + self.grant_access(obj=self.document, permission=permission_tag_view) + self.grant_access(obj=self.tag, permission=permission_tag_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=( - self.document.pk, self.tag.pk - ) + viewname='rest_api:document-tag-detail', kwargs={ + '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() - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + response = self._request_api_document_tag_remove_view() + 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) - response = self._request_document_tag_remove_view() - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.grant_access(obj=self.document, permission=permission_tag_remove) + response = self._request_api_document_tag_remove_view() + 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) - response = self._request_document_tag_remove_view() - self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + 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_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): - self.tag = self._create_tag() + def test_document_tag_remove_view_with_full_access(self): + 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(permission=permission_tag_remove, obj=self.tag) - response = self._request_document_tag_remove_view() + self.grant_access( + obj=self.document, permission=permission_tag_remove + ) + 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) diff --git a/mayan/apps/tags/tests/test_events.py b/mayan/apps/tags/tests/test_events.py index fd63cb0533..33fa2bf95d 100644 --- a/mayan/apps/tags/tests/test_events.py +++ b/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( diff --git a/mayan/apps/tags/tests/test_views.py b/mayan/apps/tags/tests/test_views.py index 850946e054..7f699357ca 100644 --- a/mayan/apps/tags/tests/test_views.py +++ b/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) diff --git a/mayan/apps/tags/tests/test_wizard_steps.py b/mayan/apps/tags/tests/test_wizard_steps.py index c82470c52c..edc1732ce5 100644 --- a/mayan/apps/tags/tests/test_wizard_steps.py +++ b/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, diff --git a/mayan/apps/tags/urls.py b/mayan/apps/tags/urls.py index c477e49756..67857d8a46 100644 --- a/mayan/apps/tags/urls.py +++ b/mayan/apps/tags/urls.py @@ -13,61 +13,70 @@ from .views import ( ) urlpatterns = [ - url(r'^list/$', TagListView.as_view(), name='tag_list'), - url(r'^create/$', TagCreateView.as_view(), name='tag_create'), + url(regex=r'^tags/list/$', name='tag_list', view=TagListView.as_view()), url( - r'^(?P\d+)/delete/$', TagDeleteActionView.as_view(), - name='tag_delete' - ), - url(r'^(?P\d+)/edit/$', TagEditView.as_view(), name='tag_edit'), - url( - r'^(?P\d+)/documents/$', TagTaggedItemListView.as_view(), - name='tag_tagged_item_list' + regex=r'^tags/create/$', name='tag_create', + view=TagCreateView.as_view() ), url( - r'^multiple/delete/$', TagDeleteActionView.as_view(), - name='tag_multiple_delete' + regex=r'^tags/(?P\d+)/delete/$', name='tag_delete', + view=TagDeleteActionView.as_view() + ), + url( + regex=r'^tags/(?P\d+)/edit/$', name='tag_edit', + view=TagEditView.as_view() + ), + url( + regex=r'^tags/(?P\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\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\d+)/$', - TagRemoveActionView.as_view(), - name='single_document_multiple_tag_remove' + regex=r'^documents/(?P\d+)/attach/$', + name='tag_attach', view=TagAttachActionView.as_view() ), url( - r'^multiple/remove/document/multiple/$', - TagRemoveActionView.as_view(), - name='multiple_documents_selection_tag_remove' + regex=r'^documents/multiple/attach//$', + name='multiple_documents_tag_attach', + view=TagAttachActionView.as_view() ), url( - r'^selection/attach/document/(?P\d+)/$', - TagAttachActionView.as_view(), name='tag_attach' - ), - url( - r'^selection/attach/document/multiple/$', - TagAttachActionView.as_view(), name='multiple_documents_tag_attach' - ), - - url( - r'^document/(?P\d+)/tags/$', DocumentTagListView.as_view(), - name='document_tags' + regex=r'^documents/(?P\d+)/tags/$', name='document_tags', + view=DocumentTagListView.as_view(), ), ] api_urls = [ url( - r'^tags/(?P[0-9]+)/documents/$', APITagDocumentListView.as_view(), - name='tag-document-list' - ), - url(r'^tags/(?P[0-9]+)/$', APITagView.as_view(), name='tag-detail'), - url(r'^tags/$', APITagListView.as_view(), name='tag-list'), - url( - r'^documents/(?P[0-9]+)/tags/$', - APIDocumentTagListView.as_view(), name='document-tag-list' + regex=r'^tags/(?P\d+)/documents/$', + name='tag-document-list', view=APITagDocumentListView.as_view(), ), url( - r'^documents/(?P[0-9]+)/tags/(?P[0-9]+)/$', - APIDocumentTagView.as_view(), name='document-tag-detail' + regex=r'^tags/(?P\d+)/$', name='tag-detail', + view=APITagView.as_view() + ), + url(regex=r'^tags/$', name='tag-list', view=APITagListView.as_view()), + url( + regex=r'^documents/(?P\d+)/tags/$', + name='document-tag-list', view=APIDocumentTagListView.as_view() + ), + url( + regex=r'^documents/(?P\d+)/tags/(?P[0-9]+)/$', + name='document-tag-detail', view=APIDocumentTagView.as_view() ), ] diff --git a/mayan/apps/tags/views.py b/mayan/apps/tags/views.py index e0bb6d30c2..1c0e8050f4 100644 --- a/mayan/apps/tags/views.py +++ b/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()