Improve document comment app
Add keyword arguments to URL definitions and reverse resolution. Raise HTTP error 404 instead of 403 to reduce the information divulged. Add view tests. Signed-off-by: Roberto Rosario <Roberto.Rosario@mayan-edms.com>
This commit is contained in:
@@ -4,8 +4,8 @@ from django.conf import settings
|
||||
from django.contrib import messages
|
||||
from django.core.exceptions import PermissionDenied
|
||||
from django.db.models.query import QuerySet
|
||||
from django.http import HttpResponseRedirect
|
||||
from django.shortcuts import resolve_url
|
||||
from django.http import Http404, HttpResponseRedirect
|
||||
from django.shortcuts import get_object_or_404, resolve_url
|
||||
from django.utils.translation import ugettext_lazy as _
|
||||
from django.utils.translation import ungettext
|
||||
|
||||
@@ -64,6 +64,33 @@ class ExtraContextMixin(object):
|
||||
return context
|
||||
|
||||
|
||||
class ExternalObjectViewMixin(object):
|
||||
external_object_pk_url_kwarg = 'pk'
|
||||
external_object_class = None
|
||||
external_object_permission = None
|
||||
|
||||
def get_external_object(self, klass=None, permission=None):
|
||||
klass = klass or self.external_object_class
|
||||
permission = permission or self.external_object_permission
|
||||
|
||||
return get_object_or_404(
|
||||
klass=self.get_external_object_queryset(
|
||||
klass=klass, permission=permission
|
||||
), pk=self.kwargs[self.external_object_pk_url_kwarg]
|
||||
)
|
||||
|
||||
def get_external_object_queryset(self, klass, permission=None):
|
||||
queryset = klass.objects.all()
|
||||
|
||||
if permission:
|
||||
queryset = AccessControlList.objects.filter_by_access(
|
||||
permission=permission, queryset=queryset,
|
||||
user=self.request.user
|
||||
)
|
||||
|
||||
return queryset
|
||||
|
||||
|
||||
class FormExtraKwargsMixin(object):
|
||||
"""
|
||||
Mixin that allows a view to pass extra keyword arguments to forms
|
||||
@@ -247,8 +274,8 @@ class ObjectActionMixin(object):
|
||||
|
||||
class ObjectListPermissionFilterMixin(object):
|
||||
"""
|
||||
access_object_retrieve_method is have the entire view check against
|
||||
an object permission and not the individual secondary items.
|
||||
access_object_retrieve_method is used to have the entire view check
|
||||
against an object permission and not the individual secondary items.
|
||||
"""
|
||||
access_object_retrieve_method = None
|
||||
object_permission = None
|
||||
@@ -289,18 +316,29 @@ class ObjectNameMixin(object):
|
||||
|
||||
|
||||
class ObjectPermissionCheckMixin(object):
|
||||
"""
|
||||
If object_permission_raise_404 is True an HTTP 404 error will be raised
|
||||
instead of the normal 403.
|
||||
"""
|
||||
object_permission = None
|
||||
object_permission_raise_404 = False
|
||||
|
||||
def get_permission_object(self):
|
||||
return self.get_object()
|
||||
|
||||
def dispatch(self, request, *args, **kwargs):
|
||||
if self.object_permission:
|
||||
AccessControlList.objects.check_access(
|
||||
permissions=self.object_permission, user=request.user,
|
||||
obj=self.get_permission_object(),
|
||||
related=getattr(self, 'object_permission_related', None)
|
||||
)
|
||||
try:
|
||||
AccessControlList.objects.check_access(
|
||||
permissions=self.object_permission, user=request.user,
|
||||
obj=self.get_permission_object(),
|
||||
related=getattr(self, 'object_permission_related', None)
|
||||
)
|
||||
except PermissionDenied:
|
||||
if self.object_permission_raise_404:
|
||||
raise Http404
|
||||
else:
|
||||
raise
|
||||
|
||||
return super(
|
||||
ObjectPermissionCheckMixin, self
|
||||
|
||||
@@ -25,7 +25,9 @@ class APICommentListView(generics.ListCreateAPIView):
|
||||
else:
|
||||
permission_required = permission_comment_create
|
||||
|
||||
document = get_object_or_404(klass=Document, pk=self.kwargs['document_pk'])
|
||||
document = get_object_or_404(
|
||||
klass=Document, pk=self.kwargs['document_pk']
|
||||
)
|
||||
|
||||
AccessControlList.objects.check_access(
|
||||
permissions=permission_required, user=self.request.user,
|
||||
@@ -78,7 +80,9 @@ class APICommentView(generics.RetrieveDestroyAPIView):
|
||||
else:
|
||||
permission_required = permission_comment_delete
|
||||
|
||||
document = get_object_or_404(klass=Document, pk=self.kwargs['document_pk'])
|
||||
document = get_object_or_404(
|
||||
klass=Document, pk=self.kwargs['document_pk']
|
||||
)
|
||||
|
||||
AccessControlList.objects.check_access(
|
||||
permissions=permission_required, user=self.request.user,
|
||||
|
||||
@@ -53,6 +53,10 @@ class DocumentCommentsApp(MayanAppConfig):
|
||||
)
|
||||
)
|
||||
|
||||
ModelPermission.register_inheritance(
|
||||
model=Comment, related='document',
|
||||
)
|
||||
|
||||
SourceColumn(source=Comment, label=_('Date'), attribute='submit_date')
|
||||
SourceColumn(
|
||||
source=Comment, label=_('User'),
|
||||
|
||||
@@ -24,16 +24,16 @@ class CommentSerializer(serializers.HyperlinkedModelSerializer):
|
||||
|
||||
def get_document_comments_url(self, instance):
|
||||
return reverse(
|
||||
'rest_api:comment-list', args=(
|
||||
instance.document.pk,
|
||||
), request=self.context['request'], format=self.context['format']
|
||||
viewname='rest_api:comment-list', kwargs={
|
||||
'document_pk': instance.document.pk,
|
||||
}, request=self.context['request'], format=self.context['format']
|
||||
)
|
||||
|
||||
def get_url(self, instance):
|
||||
return reverse(
|
||||
'rest_api:comment-detail', args=(
|
||||
instance.document.pk, instance.pk
|
||||
), request=self.context['request'], format=self.context['format']
|
||||
viewname='rest_api:comment-detail', kwargs={
|
||||
'document_pk': instance.document.pk, 'comment_pk': instance.pk
|
||||
}, request=self.context['request'], format=self.context['format']
|
||||
)
|
||||
|
||||
|
||||
@@ -58,14 +58,14 @@ class WritableCommentSerializer(serializers.ModelSerializer):
|
||||
|
||||
def get_document_comments_url(self, instance):
|
||||
return reverse(
|
||||
'rest_api:comment-list', args=(
|
||||
instance.document.pk,
|
||||
), request=self.context['request'], format=self.context['format']
|
||||
viewname='rest_api:comment-list', kwargs={
|
||||
'document_pk': instance.document.pk
|
||||
}, request=self.context['request'], format=self.context['format']
|
||||
)
|
||||
|
||||
def get_url(self, instance):
|
||||
return reverse(
|
||||
'rest_api:comment-detail', args=(
|
||||
instance.document.pk, instance.pk
|
||||
), request=self.context['request'], format=self.context['format']
|
||||
viewname='rest_api:comment-detail', kwargs={
|
||||
'document_pk': instance.document.pk, 'comment_pk': instance.pk
|
||||
}, request=self.context['request'], format=self.context['format']
|
||||
)
|
||||
|
||||
@@ -26,7 +26,7 @@ class CommentAPITestCase(DocumentTestMixin, BaseAPITestCase):
|
||||
|
||||
def _request_comment_create_view(self):
|
||||
return self.post(
|
||||
viewname='rest_api:comment-list', args=(self.document.pk,),
|
||||
viewname='rest_api:comment-list', kwargs={'document_pk': self.document.pk},
|
||||
data={
|
||||
'comment': TEST_COMMENT_TEXT
|
||||
}
|
||||
@@ -47,9 +47,10 @@ class CommentAPITestCase(DocumentTestMixin, BaseAPITestCase):
|
||||
|
||||
def _request_comment_delete_view(self):
|
||||
return self.delete(
|
||||
viewname='rest_api:comment-detail', args=(
|
||||
self.document.pk, self.comment.pk,
|
||||
)
|
||||
viewname='rest_api:comment-detail', kwargs={
|
||||
'document_pk': self.document.pk,
|
||||
'comment_pk': self.comment.pk
|
||||
}
|
||||
)
|
||||
|
||||
def test_comment_delete_view_no_access(self):
|
||||
@@ -69,9 +70,10 @@ class CommentAPITestCase(DocumentTestMixin, BaseAPITestCase):
|
||||
|
||||
def _request_comment_view(self):
|
||||
return self.get(
|
||||
viewname='rest_api:comment-detail', args=(
|
||||
self.document.pk, self.comment.pk,
|
||||
)
|
||||
viewname='rest_api:comment-detail', kwargs={
|
||||
'document_pk': self.document.pk,
|
||||
'comment_pk': self.comment.pk
|
||||
}
|
||||
)
|
||||
|
||||
def test_comment_detail_view_no_access(self):
|
||||
@@ -90,7 +92,8 @@ class CommentAPITestCase(DocumentTestMixin, BaseAPITestCase):
|
||||
|
||||
def _request_comment_list_view(self):
|
||||
return self.get(
|
||||
viewname='rest_api:comment-list', args=(self.document.pk,)
|
||||
viewname='rest_api:comment-list',
|
||||
kwargs={'document_pk': self.document.pk}
|
||||
)
|
||||
|
||||
def test_comment_list_view_no_access(self):
|
||||
|
||||
89
mayan/apps/document_comments/tests/test_views.py
Normal file
89
mayan/apps/document_comments/tests/test_views.py
Normal file
@@ -0,0 +1,89 @@
|
||||
from __future__ import unicode_literals
|
||||
|
||||
from mayan.apps.documents.tests import GenericDocumentViewTestCase
|
||||
|
||||
from ..permissions import (
|
||||
permission_comment_create, permission_comment_delete,
|
||||
permission_comment_view
|
||||
)
|
||||
|
||||
from .literals import TEST_COMMENT_TEXT
|
||||
|
||||
|
||||
class CommentsViewsTestCase(GenericDocumentViewTestCase):
|
||||
def setUp(self):
|
||||
super(CommentsViewsTestCase, self).setUp()
|
||||
self.login_user()
|
||||
|
||||
def _request_document_comment_add_view(self):
|
||||
return self.post(
|
||||
viewname='comments:comment_add',
|
||||
kwargs={'document_pk': self.document.pk},
|
||||
data={'comment': TEST_COMMENT_TEXT}
|
||||
)
|
||||
|
||||
def test_document_comment_add_view_no_permission(self):
|
||||
response = self._request_document_comment_add_view()
|
||||
self.assertEqual(response.status_code, 404)
|
||||
self.assertEqual(self.document.comments.all().count(), 0)
|
||||
|
||||
def test_document_comment_add_view_with_access(self):
|
||||
self.grant_access(
|
||||
obj=self.document, permission=permission_comment_create
|
||||
)
|
||||
response = self._request_document_comment_add_view()
|
||||
self.assertEqual(response.status_code, 302)
|
||||
self.assertEqual(self.document.comments.all().count(), 1)
|
||||
|
||||
def _create_test_comment(self):
|
||||
self.test_comment = self.document.comments.create(
|
||||
user=self.user, comment=TEST_COMMENT_TEXT
|
||||
)
|
||||
|
||||
def _request_document_comment_delete_view(self):
|
||||
return self.post(
|
||||
viewname='comments:comment_delete',
|
||||
kwargs={'comment_pk': self.test_comment.pk},
|
||||
)
|
||||
|
||||
def test_document_comment_delete_view_no_permission(self):
|
||||
self._create_test_comment()
|
||||
|
||||
response = self._request_document_comment_delete_view()
|
||||
self.assertEqual(response.status_code, 404)
|
||||
self.assertEqual(self.document.comments.all().count(), 1)
|
||||
|
||||
def test_document_comment_delete_view_with_access(self):
|
||||
self._create_test_comment()
|
||||
|
||||
self.grant_access(
|
||||
obj=self.document, permission=permission_comment_delete
|
||||
)
|
||||
response = self._request_document_comment_delete_view()
|
||||
self.assertEqual(response.status_code, 302)
|
||||
self.assertEqual(self.document.comments.all().count(), 0)
|
||||
|
||||
def _request_document_comment_list_view(self):
|
||||
return self.get(
|
||||
viewname='comments:comments_for_document',
|
||||
kwargs={'document_pk': self.document.pk}
|
||||
)
|
||||
|
||||
def test_document_comment_list_view_no_permissions(self):
|
||||
self._create_test_comment()
|
||||
|
||||
response = self._request_document_comment_list_view()
|
||||
self.assertNotContains(
|
||||
response=response, status_code=404, text=TEST_COMMENT_TEXT
|
||||
)
|
||||
|
||||
def test_document_comment_list_view_with_access(self):
|
||||
self._create_test_comment()
|
||||
|
||||
self.grant_access(
|
||||
obj=self.document, permission=permission_comment_view
|
||||
)
|
||||
response = self._request_document_comment_list_view()
|
||||
self.assertContains(
|
||||
response=response, status_code=200, text=TEST_COMMENT_TEXT
|
||||
)
|
||||
@@ -10,26 +10,26 @@ from .views import (
|
||||
|
||||
urlpatterns = [
|
||||
url(
|
||||
r'^comment/(?P<pk>\d+)/delete/$', DocumentCommentDeleteView.as_view(),
|
||||
name='comment_delete'
|
||||
regex=r'^comments/(?P<comment_pk>\d+)/delete/$', name='comment_delete',
|
||||
view=DocumentCommentDeleteView.as_view()
|
||||
),
|
||||
url(
|
||||
r'^(?P<pk>\d+)/comment/add/$', DocumentCommentCreateView.as_view(),
|
||||
name='comment_add'
|
||||
regex=r'^documents/(?P<document_pk>\d+)/comments/add/$',
|
||||
name='comment_add', view=DocumentCommentCreateView.as_view()
|
||||
),
|
||||
url(
|
||||
r'^(?P<pk>\d+)/comment/list/$',
|
||||
DocumentCommentListView.as_view(), name='comments_for_document'
|
||||
regex=r'^documents/(?P<document_pk>\d+)/comments/$',
|
||||
name='comments_for_document', view=DocumentCommentListView.as_view()
|
||||
),
|
||||
]
|
||||
|
||||
api_urls = [
|
||||
url(
|
||||
r'^documents/(?P<document_pk>[0-9]+)/comments/$',
|
||||
APICommentListView.as_view(), name='comment-list'
|
||||
regex=r'^documents/(?P<document_pk>[0-9]+)/comments/$',
|
||||
name='comment-list', view=APICommentListView.as_view()
|
||||
),
|
||||
url(
|
||||
r'^documents/(?P<document_pk>[0-9]+)/comments/(?P<comment_pk>[0-9]+)/$',
|
||||
APICommentView.as_view(), name='comment-detail'
|
||||
regex=r'^documents/(?P<document_pk>[0-9]+)/comments/(?P<comment_pk>[0-9]+)/$',
|
||||
name='comment-detail', view=APICommentView.as_view()
|
||||
),
|
||||
]
|
||||
|
||||
@@ -1,14 +1,13 @@
|
||||
from __future__ import absolute_import, unicode_literals
|
||||
|
||||
from django.shortcuts import get_object_or_404
|
||||
from django.template import RequestContext
|
||||
from django.urls import reverse
|
||||
from django.utils.translation import ugettext_lazy as _
|
||||
|
||||
from mayan.apps.acls.models import AccessControlList
|
||||
from mayan.apps.common.generics import (
|
||||
SingleObjectCreateView, SingleObjectDeleteView, SingleObjectListView
|
||||
)
|
||||
from mayan.apps.common.mixins import ExternalObjectViewMixin
|
||||
from mayan.apps.documents.models import Document
|
||||
|
||||
from .icons import icon_comments_for_document
|
||||
@@ -20,22 +19,15 @@ from .permissions import (
|
||||
)
|
||||
|
||||
|
||||
class DocumentCommentCreateView(SingleObjectCreateView):
|
||||
class DocumentCommentCreateView(ExternalObjectViewMixin, SingleObjectCreateView):
|
||||
fields = ('comment',)
|
||||
external_object_pk_url_kwarg = 'document_pk'
|
||||
external_object_class = Document
|
||||
external_object_permission = permission_comment_create
|
||||
model = Comment
|
||||
|
||||
def dispatch(self, request, *args, **kwargs):
|
||||
AccessControlList.objects.check_access(
|
||||
permissions=permission_comment_create, user=request.user,
|
||||
obj=self.get_document()
|
||||
)
|
||||
|
||||
return super(
|
||||
DocumentCommentCreateView, self
|
||||
).dispatch(request, *args, **kwargs)
|
||||
|
||||
def get_document(self):
|
||||
return get_object_or_404(klass=Document, pk=self.kwargs['pk'])
|
||||
return self.get_external_object()
|
||||
|
||||
def get_extra_context(self):
|
||||
return {
|
||||
@@ -50,7 +42,9 @@ class DocumentCommentCreateView(SingleObjectCreateView):
|
||||
|
||||
def get_post_action_redirect(self):
|
||||
return reverse(
|
||||
'comments:comments_for_document', args=(self.kwargs['pk'],)
|
||||
viewname='comments:comments_for_document', kwargs={
|
||||
'document_pk': self.kwargs['document_pk']
|
||||
}
|
||||
)
|
||||
|
||||
def get_save_extra_data(self):
|
||||
@@ -61,16 +55,9 @@ class DocumentCommentCreateView(SingleObjectCreateView):
|
||||
|
||||
class DocumentCommentDeleteView(SingleObjectDeleteView):
|
||||
model = Comment
|
||||
|
||||
def dispatch(self, request, *args, **kwargs):
|
||||
AccessControlList.objects.check_access(
|
||||
permissions=permission_comment_delete, user=request.user,
|
||||
obj=self.get_object().document
|
||||
)
|
||||
|
||||
return super(
|
||||
DocumentCommentDeleteView, self
|
||||
).dispatch(request, *args, **kwargs)
|
||||
pk_url_kwarg = 'comment_pk'
|
||||
object_permission = permission_comment_delete
|
||||
object_permission_raise_404 = True
|
||||
|
||||
def get_delete_extra_data(self):
|
||||
return {'_user': self.request.user}
|
||||
@@ -83,36 +70,36 @@ class DocumentCommentDeleteView(SingleObjectDeleteView):
|
||||
|
||||
def get_post_action_redirect(self):
|
||||
return reverse(
|
||||
'comments:comments_for_document',
|
||||
args=(self.get_object().document.pk,)
|
||||
viewname='comments:comments_for_document', kwargs={
|
||||
'document_pk': self.get_object().document.pk
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
class DocumentCommentListView(SingleObjectListView):
|
||||
class DocumentCommentListView(ExternalObjectViewMixin, SingleObjectListView):
|
||||
external_object_pk_url_kwarg = 'document_pk'
|
||||
external_object_class = Document
|
||||
external_object_permission = permission_comment_view
|
||||
|
||||
def get_document(self):
|
||||
return get_object_or_404(klass=Document, pk=self.kwargs['pk'])
|
||||
return self.get_external_object()
|
||||
|
||||
def get_extra_context(self):
|
||||
return {
|
||||
'hide_link': True,
|
||||
'hide_object': True,
|
||||
'no_results_icon': icon_comments_for_document,
|
||||
'no_results_external_link': link_comment_add.resolve(
|
||||
RequestContext(self.request, {'object': self.get_document()})
|
||||
),
|
||||
'no_results_text': _(
|
||||
'Document comments are timestamped text entries from users. '
|
||||
'They are great for collaboration.'
|
||||
),
|
||||
'no_results_main_link': link_comment_add.resolve(
|
||||
RequestContext(self.request, {'object': self.get_document()})
|
||||
),
|
||||
'no_results_title': _('There are no comments'),
|
||||
'object': self.get_document(),
|
||||
'title': _('Comments for document: %s') % self.get_document(),
|
||||
}
|
||||
|
||||
def get_object_list(self):
|
||||
AccessControlList.objects.check_access(
|
||||
permissions=permission_comment_view, user=self.request.user,
|
||||
obj=self.get_document()
|
||||
)
|
||||
|
||||
return self.get_document().comments.all()
|
||||
|
||||
Reference in New Issue
Block a user