Move new document version blocking logic to the checkouts app. Remove PERMISSION_DOCUMENT_RESTRICTIONS_OVERRIDE, overriding checkout restrictions even for admin users has the potential to confuse or corrupt data, removing it. Even admins must now checkin a document before trying to perform a restricted operation.

This commit is contained in:
Roberto Rosario
2015-06-16 20:38:45 -04:00
parent dc35ed6948
commit c8268b1a5b
9 changed files with 60 additions and 55 deletions

View File

@@ -3,14 +3,16 @@ from __future__ import absolute_import, unicode_literals
from datetime import timedelta from datetime import timedelta
from django import apps from django import apps
from django.db.models.signals import pre_save
from django.utils.translation import ugettext_lazy as _ from django.utils.translation import ugettext_lazy as _
from acls.api import class_permissions from acls.api import class_permissions
from common.menus import menu_facet, menu_main, menu_sidebar from common.menus import menu_facet, menu_main, menu_sidebar
from documents.models import Document from documents.models import Document, DocumentVersion
from mayan.celery import app from mayan.celery import app
from rest_api.classes import APIEndPoint from rest_api.classes import APIEndPoint
from .handlers import check_if_new_versions_allowed
from .links import ( from .links import (
link_checkin_document, link_checkout_document, link_checkout_info, link_checkin_document, link_checkout_document, link_checkout_info,
link_checkout_list link_checkout_list
@@ -18,7 +20,7 @@ from .links import (
from .models import DocumentCheckout from .models import DocumentCheckout
from .permissions import ( from .permissions import (
PERMISSION_DOCUMENT_CHECKIN, PERMISSION_DOCUMENT_CHECKIN_OVERRIDE, PERMISSION_DOCUMENT_CHECKIN, PERMISSION_DOCUMENT_CHECKIN_OVERRIDE,
PERMISSION_DOCUMENT_CHECKOUT, PERMISSION_DOCUMENT_RESTRICTIONS_OVERRIDE PERMISSION_DOCUMENT_CHECKOUT
) )
CHECK_EXPIRED_CHECK_OUTS_INTERVAL = 60 # Lowest check out expiration allowed CHECK_EXPIRED_CHECK_OUTS_INTERVAL = 60 # Lowest check out expiration allowed
@@ -33,7 +35,6 @@ class CheckoutsApp(apps.AppConfig):
Document.add_to_class('check_in', lambda document, user=None: DocumentCheckout.objects.check_in_document(document, user)) Document.add_to_class('check_in', lambda document, user=None: DocumentCheckout.objects.check_in_document(document, user))
Document.add_to_class('checkout_info', lambda document: DocumentCheckout.objects.document_checkout_info(document)) Document.add_to_class('checkout_info', lambda document: DocumentCheckout.objects.document_checkout_info(document))
Document.add_to_class('checkout_state', lambda document: DocumentCheckout.objects.document_checkout_state(document)) Document.add_to_class('checkout_state', lambda document: DocumentCheckout.objects.document_checkout_state(document))
Document.add_to_class('is_new_versions_allowed', lambda document, user=None: DocumentCheckout.objects.is_document_new_versions_allowed(document, user))
app.conf.CELERYBEAT_SCHEDULE.update({ app.conf.CELERYBEAT_SCHEDULE.update({
'check_expired_check_outs': { 'check_expired_check_outs': {
@@ -47,11 +48,12 @@ class CheckoutsApp(apps.AppConfig):
PERMISSION_DOCUMENT_CHECKOUT, PERMISSION_DOCUMENT_CHECKOUT,
PERMISSION_DOCUMENT_CHECKIN, PERMISSION_DOCUMENT_CHECKIN,
PERMISSION_DOCUMENT_CHECKIN_OVERRIDE, PERMISSION_DOCUMENT_CHECKIN_OVERRIDE,
PERMISSION_DOCUMENT_RESTRICTIONS_OVERRIDE
]) ])
menu_facet.bind_links(links=[link_checkout_info], sources=[Document]) menu_facet.bind_links(links=[link_checkout_info], sources=[Document])
menu_main.bind_links(links=[link_checkout_list]) menu_main.bind_links(links=[link_checkout_list])
menu_sidebar.bind_links(links=[link_checkout_document, link_checkin_document], sources=['checkouts:checkout_info', 'checkouts:checkout_document', 'checkouts:checkin_document']) menu_sidebar.bind_links(links=[link_checkout_document, link_checkin_document], sources=['checkouts:checkout_info', 'checkouts:checkout_document', 'checkouts:checkin_document'])
pre_save.connect(check_if_new_versions_allowed, dispatch_uid='document_index_delete', sender=DocumentVersion)
APIEndPoint('checkouts') APIEndPoint('checkouts')

View File

@@ -3,16 +3,38 @@ from __future__ import unicode_literals
from django.utils.translation import ugettext from django.utils.translation import ugettext
class DocumentNotCheckedOut(Exception): class DocumentCheckoutError(Exception):
"""
Base checkout exception
"""
pass
class DocumentCheckoutWarning(Warning):
"""
Base checkout warning
"""
pass
class DocumentNotCheckedOut(DocumentCheckoutError):
""" """
Raised when trying to checkin a document that is not checkedout Raised when trying to checkin a document that is not checkedout
""" """
pass pass
class DocumentAlreadyCheckedOut(Exception): class DocumentAlreadyCheckedOut(DocumentCheckoutError):
""" """
Raised when trying to checkout an already checkedout document Raised when trying to checkout an already checkedout document
""" """
def __unicode__(self): def __unicode__(self):
return ugettext('Document already checked out.') return ugettext('Document already checked out.')
class NewDocumentVersionNotAllowed(DocumentCheckoutWarning):
"""
Uploading new versions for this document is not allowed
Current reasons: Document is in checked out state
"""
pass

View File

@@ -0,0 +1,11 @@
from __future__ import unicode_literals
from django.utils.translation import ugettext_lazy as _
from .exceptions import NewDocumentVersionNotAllowed
from .models import DocumentCheckout
def check_if_new_versions_allowed(sender, **kwargs):
if not DocumentCheckout.objects.are_document_new_versions_allowed(kwargs['instance'].document):
raise NewDocumentVersionNotAllowed(_('New versions not allowed for the checkedout document: %s' % kwargs['instance'].document))

View File

@@ -16,12 +16,14 @@ from .events import (
) )
from .exceptions import DocumentNotCheckedOut from .exceptions import DocumentNotCheckedOut
from .literals import STATE_CHECKED_OUT, STATE_CHECKED_IN from .literals import STATE_CHECKED_OUT, STATE_CHECKED_IN
from .permissions import PERMISSION_DOCUMENT_RESTRICTIONS_OVERRIDE
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
class DocumentCheckoutManager(models.Manager): class DocumentCheckoutManager(models.Manager):
def checkout_document(self, document, expiration_datetime, user, block_new_version=True):
self.create(document=document, expiration_datetime=expiration_datetime, user_object=user, block_new_version=block_new_version)
def checked_out_documents(self): def checked_out_documents(self):
return Document.objects.filter(pk__in=self.model.objects.all().values_list('document__pk', flat=True)) return Document.objects.filter(pk__in=self.model.objects.all().values_list('document__pk', flat=True))
@@ -68,33 +70,10 @@ class DocumentCheckoutManager(models.Manager):
else: else:
return STATE_CHECKED_IN return STATE_CHECKED_IN
def is_document_new_versions_allowed(self, document, user=None): def are_document_new_versions_allowed(self, document, user=None):
try: try:
checkout_info = self.document_checkout_info(document) checkout_info = self.document_checkout_info(document)
except DocumentNotCheckedOut: except DocumentNotCheckedOut:
return True return True
else: else:
if not user: return not checkout_info.block_new_version
return not checkout_info.block_new_version
else:
if user.is_staff or user.is_superuser:
# Allow anything to superusers and staff
return True
if user == checkout_info.user_object:
# Allow anything to the user who checked out this document
return True
else:
# If not original user check to see if user has global or this document's PERMISSION_DOCUMENT_RESTRICTIONS_OVERRIDE permission
try:
Permission.objects.check_permissions(user, [PERMISSION_DOCUMENT_RESTRICTIONS_OVERRIDE])
except PermissionDenied:
try:
AccessEntry.objects.check_accesses([PERMISSION_DOCUMENT_RESTRICTIONS_OVERRIDE], user, document)
except PermissionDenied:
# Last resort check if original user enabled restriction
return not checkout_info.block_new_version
else:
return True
else:
return True

View File

@@ -9,4 +9,3 @@ namespace = PermissionNamespace('checkouts', _('Document checkout'))
PERMISSION_DOCUMENT_CHECKOUT = Permission.objects.register(namespace, 'checkout_document', _('Check out documents')) PERMISSION_DOCUMENT_CHECKOUT = Permission.objects.register(namespace, 'checkout_document', _('Check out documents'))
PERMISSION_DOCUMENT_CHECKIN = Permission.objects.register(namespace, 'checkin_document', _('Check in documents')) PERMISSION_DOCUMENT_CHECKIN = Permission.objects.register(namespace, 'checkin_document', _('Check in documents'))
PERMISSION_DOCUMENT_CHECKIN_OVERRIDE = Permission.objects.register(namespace, 'checkin_document_override', _('Forcefully check in documents')) PERMISSION_DOCUMENT_CHECKIN_OVERRIDE = Permission.objects.register(namespace, 'checkin_document_override', _('Forcefully check in documents'))
PERMISSION_DOCUMENT_RESTRICTIONS_OVERRIDE = Permission.objects.register(namespace, 'checkout_restrictions_override', _('Allow overriding check out restrictions'))

View File

@@ -74,10 +74,12 @@ def checkout_document(request, document_pk):
form = DocumentCheckoutForm(data=request.POST, initial={'document': document}) form = DocumentCheckoutForm(data=request.POST, initial={'document': document})
if form.is_valid(): if form.is_valid():
try: try:
document_checkout = form.save(commit=False) DocumentCheckout.objects.checkout_document(
document_checkout.user_object = request.user document=document,
document_checkout.document = document expiration_datetime=form.cleaned_data['expiration_datetime'],
document_checkout.save() user=request.user,
block_new_version=form.cleaned_data['block_new_version'],
)
except DocumentAlreadyCheckedOut: except DocumentAlreadyCheckedOut:
messages.error(request, _('Document already checked out.')) messages.error(request, _('Document already checked out.'))
return HttpResponseRedirect(reverse('checkouts:checkout_info', args=[document.pk])) return HttpResponseRedirect(reverse('checkouts:checkout_info', args=[document.pk]))

View File

@@ -1,9 +0,0 @@
from __future__ import unicode_literals
class NewDocumentVersionNotAllowed(Exception):
"""
Uploading new versions for this document is not allowed
Current reasons: Document is in checked out state
"""
pass

View File

@@ -29,7 +29,6 @@ from converter.models import Transformation
from mimetype.api import get_mimetype from mimetype.api import get_mimetype
from .events import event_document_create from .events import event_document_create
from .exceptions import NewDocumentVersionNotAllowed
from .managers import ( from .managers import (
DocumentManager, DocumentTypeManager, RecentDocumentManager DocumentManager, DocumentTypeManager, RecentDocumentManager
) )
@@ -138,10 +137,7 @@ class Document(models.Model):
return self.latest_version.size return self.latest_version.size
def new_version(self, file_object, user=None, comment=None): def new_version(self, file_object, user=None, comment=None):
logger.debug('creating new document version') logger.info('Creating a new document version for document: %s', self)
# TODO: move this restriction to a signal processor of the checkouts app
if not self.is_new_versions_allowed(user=user):
raise NewDocumentVersionNotAllowed
new_version = DocumentVersion.objects.create( new_version = DocumentVersion.objects.create(
document=self, document=self,
@@ -149,7 +145,7 @@ class Document(models.Model):
comment=comment or '', comment=comment or '',
) )
logger.debug('new_version saved') logger.info('New document version created for document: %s', self)
# TODO: new HISTORY for version updates # TODO: new HISTORY for version updates

View File

@@ -71,6 +71,9 @@ def task_upload_new_version(document_id, shared_uploaded_file_id, user_id, comme
user = None user = None
with File(file=shared_file.file) as file_object: with File(file=shared_file.file) as file_object:
document.new_version(comment=comment, file_object=file_object, user=user) try:
document.new_version(comment=comment, file_object=file_object, user=user)
shared_file.delete() except Warning as warning:
logger.info('Warning during attempt to create new document version for document:%s ; %s', document, warning)
finally:
shared_file.delete()