diff --git a/HISTORY.rst b/HISTORY.rst index e816e6afc0..172bbcb0e5 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -207,6 +207,7 @@ * Add a bottom margin to list links. * Use copyfileobj to save documents to files * Add user logged in and logged out events. +* Add transaction handling in more places. 3.1.11 (2019-04-XX) =================== diff --git a/docs/releases/3.2.rst b/docs/releases/3.2.rst index ee2156b6a9..40a78a23a2 100644 --- a/docs/releases/3.2.rst +++ b/docs/releases/3.2.rst @@ -239,6 +239,8 @@ Other changes * Add a bottom margin to list links. * Use copyfileobj to save documents to files * Add user logged in and logged out events. +* Add transaction handling in more places: Checkouts, documents, + metadata, tags. Removals -------- diff --git a/mayan/apps/checkouts/managers.py b/mayan/apps/checkouts/managers.py index 349581331a..674a77257e 100644 --- a/mayan/apps/checkouts/managers.py +++ b/mayan/apps/checkouts/managers.py @@ -3,7 +3,7 @@ from __future__ import absolute_import, unicode_literals import logging from django.apps import apps -from django.db import models +from django.db import models, transaction from django.utils.timezone import now from mayan.apps.documents.models import Document @@ -33,17 +33,18 @@ class DocumentCheckoutManager(models.Manager): except self.model.DoesNotExist: raise DocumentNotCheckedOut else: - if user: - if self.get_check_out_info(document=document).user != user: - event_document_forceful_check_in.commit( - actor=user, target=document - ) + with transaction.atomic(): + if user: + if self.get_check_out_info(document=document).user != user: + event_document_forceful_check_in.commit( + actor=user, target=document + ) + else: + event_document_check_in.commit(actor=user, target=document) else: - event_document_check_in.commit(actor=user, target=document) - else: - event_document_auto_check_in.commit(target=document) + event_document_auto_check_in.commit(target=document) - document_check_out.delete() + document_check_out.delete() def check_in_expired_check_outs(self): for document in self.expired_check_outs(): diff --git a/mayan/apps/checkouts/models.py b/mayan/apps/checkouts/models.py index bfc551c06a..c724e21db8 100644 --- a/mayan/apps/checkouts/models.py +++ b/mayan/apps/checkouts/models.py @@ -4,7 +4,7 @@ import logging from django.conf import settings from django.core.exceptions import ValidationError -from django.db import models +from django.db import models, transaction from django.urls import reverse from django.utils.encoding import force_text, python_2_unicode_compatible from django.utils.timezone import now @@ -65,9 +65,9 @@ class DocumentCheckout(models.Model): ) def delete(self, *args, **kwargs): - # TODO: enclose in transaction - NewVersionBlock.objects.unblock(document=self.document) - super(DocumentCheckout, self).delete(*args, **kwargs) + with transaction.atomic(): + NewVersionBlock.objects.unblock(document=self.document) + super(DocumentCheckout, self).delete(*args, **kwargs) def get_absolute_url(self): return reverse( @@ -81,25 +81,25 @@ class DocumentCheckout(models.Model): natural_key.dependencies = ['documents.Document'] def save(self, *args, **kwargs): - # TODO: enclose in transaction new_checkout = not self.pk if not new_checkout or self.document.is_checked_out(): raise DocumentAlreadyCheckedOut - result = super(DocumentCheckout, self).save(*args, **kwargs) - if new_checkout: - event_document_check_out.commit( - actor=self.user, target=self.document - ) - if self.block_new_version: - NewVersionBlock.objects.block(self.document) + with transaction.atomic(): + result = super(DocumentCheckout, self).save(*args, **kwargs) + if new_checkout: + event_document_check_out.commit( + actor=self.user, target=self.document + ) + if self.block_new_version: + NewVersionBlock.objects.block(self.document) - logger.info( - 'Document "%s" checked out by user "%s"', - self.document, self.user - ) + logger.info( + 'Document "%s" checked out by user "%s"', + self.document, self.user + ) - return result + return result class NewVersionBlock(models.Model): diff --git a/mayan/apps/documents/models/document_version_models.py b/mayan/apps/documents/models/document_version_models.py index ca632acbf8..43e2fef5c2 100644 --- a/mayan/apps/documents/models/document_version_models.py +++ b/mayan/apps/documents/models/document_version_models.py @@ -250,9 +250,12 @@ class DocumentVersion(models.Model): self.document, self ) - event_document_version_revert.commit(actor=_user, target=self.document) - for version in self.document.versions.filter(timestamp__gt=self.timestamp): - version.delete() + with transaction.atomic(): + event_document_version_revert.commit( + actor=_user, target=self.document + ) + for version in self.document.versions.filter(timestamp__gt=self.timestamp): + version.delete() def save(self, *args, **kwargs): """ diff --git a/mayan/apps/metadata/models.py b/mayan/apps/metadata/models.py index 39156265b4..20c2321802 100644 --- a/mayan/apps/metadata/models.py +++ b/mayan/apps/metadata/models.py @@ -3,7 +3,7 @@ from __future__ import unicode_literals import shlex from django.core.exceptions import ValidationError -from django.db import models +from django.db import models, transaction from django.template import Context, Template from django.urls import reverse from django.utils.encoding import force_text, python_2_unicode_compatible @@ -97,7 +97,11 @@ class MetadataType(models.Model): return self.label def get_absolute_url(self): - return reverse('metadata:setup_metadata_type_edit', args=(self.pk,)) + return reverse( + viewname='metadata:setup_metadata_type_edit', kwargs={ + 'pk': self.pk + } + ) if PY2: # Python 2 non unicode version @@ -141,21 +145,22 @@ class MetadataType(models.Model): return (self.name,) def save(self, *args, **kwargs): - user = kwargs.pop('_user', None) + _user = kwargs.pop('_user', None) created = not self.pk - result = super(MetadataType, self).save(*args, **kwargs) + with transaction.atomic(): + result = super(MetadataType, self).save(*args, **kwargs) - if created: - event_metadata_type_created.commit( - actor=user, target=self - ) - else: - event_metadata_type_edited.commit( - actor=user, target=self - ) + if created: + event_metadata_type_created.commit( + actor=_user, target=self + ) + else: + event_metadata_type_edited.commit( + actor=_user, target=self + ) - return result + return result def validate_value(self, document_type, value): # Check default @@ -234,13 +239,15 @@ class DocumentMetadata(models.Model): _('Metadata type is required for this document type.') ) - user = kwargs.pop('_user', None) - result = super(DocumentMetadata, self).delete(*args, **kwargs) + _user = kwargs.pop('_user', None) + with transaction.atomic(): + result = super(DocumentMetadata, self).delete(*args, **kwargs) - event_document_metadata_removed.commit( - action_object=self.metadata_type, actor=user, target=self.document, - ) - return result + event_document_metadata_removed.commit( + action_object=self.metadata_type, actor=_user, + target=self.document, + ) + return result def natural_key(self): return self.document.natural_key() + self.metadata_type.natural_key() @@ -262,23 +269,24 @@ class DocumentMetadata(models.Model): _('Metadata type is not valid for this document type.') ) - user = kwargs.pop('_user', None) + _user = kwargs.pop('_user', None) created = not self.pk - result = super(DocumentMetadata, self).save(*args, **kwargs) + with transaction.atomic(): + result = super(DocumentMetadata, self).save(*args, **kwargs) - if created: - event_document_metadata_added.commit( - action_object=self.metadata_type, actor=user, - target=self.document, - ) - else: - event_document_metadata_edited.commit( - action_object=self.metadata_type, actor=user, - target=self.document, - ) + if created: + event_document_metadata_added.commit( + action_object=self.metadata_type, actor=_user, + target=self.document, + ) + else: + event_document_metadata_edited.commit( + action_object=self.metadata_type, actor=_user, + target=self.document, + ) - return result + return result @python_2_unicode_compatible @@ -309,23 +317,27 @@ class DocumentTypeMetadataType(models.Model): return force_text(self.metadata_type) def delete(self, *args, **kwargs): - user = kwargs.pop('_user', None) + _user = kwargs.pop('_user', None) - result = super(DocumentTypeMetadataType, self).delete(*args, **kwargs) + with transaction.atomic(): + result = super(DocumentTypeMetadataType, self).delete(*args, **kwargs) - event_metadata_type_relationship.commit( - action_object=self.document_type, actor=user, target=self.metadata_type, - ) + event_metadata_type_relationship.commit( + action_object=self.document_type, actor=_user, + target=self.metadata_type, + ) - return result + return result def save(self, *args, **kwargs): - user = kwargs.pop('_user', None) + _user = kwargs.pop('_user', None) - result = super(DocumentTypeMetadataType, self).save(*args, **kwargs) + with transaction.atomic(): + result = super(DocumentTypeMetadataType, self).save(*args, **kwargs) - event_metadata_type_relationship.commit( - action_object=self.document_type, actor=user, target=self.metadata_type, - ) + event_metadata_type_relationship.commit( + action_object=self.document_type, actor=_user, + target=self.metadata_type, + ) - return result + return result diff --git a/mayan/apps/tags/models.py b/mayan/apps/tags/models.py index 41a98b213a..9e824e8699 100644 --- a/mayan/apps/tags/models.py +++ b/mayan/apps/tags/models.py @@ -1,6 +1,6 @@ from __future__ import absolute_import, unicode_literals -from django.db import models +from django.db import models, transaction from django.urls import reverse from django.utils.encoding import python_2_unicode_compatible from django.utils.translation import ugettext_lazy as _ @@ -83,17 +83,18 @@ class Tag(models.Model): ) def save(self, *args, **kwargs): - user = kwargs.pop('_user', None) + _user = kwargs.pop('_user', None) created = not self.pk - result = super(Tag, self).save(*args, **kwargs) + with transaction.atomic(): + result = super(Tag, self).save(*args, **kwargs) - if created: - event_tag_created.commit(actor=user, target=self) - else: - event_tag_edited.commit(actor=user, target=self) + if created: + event_tag_created.commit(actor=_user, target=self) + else: + event_tag_edited.commit(actor=_user, target=self) - return result + return result class DocumentTag(Tag):