diff --git a/HISTORY.rst b/HISTORY.rst index cac3a110f6..b833082cde 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -32,7 +32,8 @@ - Use the literal 'System' instead of the target name when the action user in unknown. - Remove the view to submit all document for OCR. - +- When changing document types, don't delete the old metadata that is + also found in the new document type. GitLab issue #421. 2.6.4 (2017-07-26) ================== diff --git a/mayan/apps/metadata/apps.py b/mayan/apps/metadata/apps.py index 20a24c0367..b34d36ce17 100644 --- a/mayan/apps/metadata/apps.py +++ b/mayan/apps/metadata/apps.py @@ -26,7 +26,7 @@ from .classes import DocumentMetadataHelper from .handlers import ( handler_index_document, post_document_type_metadata_type_add, post_document_type_metadata_type_delete, - post_post_document_type_change_metadata + post_document_type_change_metadata ) from .links import ( link_metadata_add, link_metadata_edit, link_metadata_multiple_add, @@ -236,17 +236,17 @@ class MetadataApp(MayanAppConfig): post_delete.connect( post_document_type_metadata_type_delete, - dispatch_uid='post_document_type_metadata_type_delete', + dispatch_uid='metadata_post_document_type_metadata_type_delete', sender=DocumentTypeMetadataType ) post_document_type_change.connect( - post_post_document_type_change_metadata, - dispatch_uid='post_post_document_type_change_metadata', + post_document_type_change_metadata, + dispatch_uid='metadata_post_document_type_change_metadata', sender=Document ) post_save.connect( post_document_type_metadata_type_add, - dispatch_uid='post_document_type_metadata_type_add', + dispatch_uid='metadata_post_document_type_metadata_type_add', sender=DocumentTypeMetadataType ) @@ -254,11 +254,11 @@ class MetadataApp(MayanAppConfig): post_delete.connect( handler_index_document, - dispatch_uid='handler_index_document_delete', + dispatch_uid='metadata_handler_index_document_delete', sender=DocumentMetadata ) post_save.connect( handler_index_document, - dispatch_uid='handler_index_document_save', + dispatch_uid='metadata_handler_index_document_save', sender=DocumentMetadata ) diff --git a/mayan/apps/metadata/handlers.py b/mayan/apps/metadata/handlers.py index 7419426d8d..0bba99d1ce 100644 --- a/mayan/apps/metadata/handlers.py +++ b/mayan/apps/metadata/handlers.py @@ -33,23 +33,43 @@ def post_document_type_metadata_type_delete(sender, instance, **kwargs): ) -def post_post_document_type_change_metadata(sender, instance, **kwargs): +def post_document_type_change_metadata(sender, instance, **kwargs): logger.debug('received post_document_type_change') logger.debug('instance: %s', instance) - # Delete existing document metadata - for metadata in instance.metadata.all(): + + # Delete existing document metadata types not found in the new document + # type + + # First get the existing metadata types not found in the new document + # type + unneeded_metadata = instance.metadata.exclude( + metadata_type__in=instance.document_type.metadata.values( + 'metadata_type' + ) + ) + + # Remove the document metadata whose types are not found in the new + # document type + for metadata in unneeded_metadata: metadata.delete(enforce_required=False) DocumentMetadata = apps.get_model( app_label='metadata', model_name='DocumentMetadata' ) - # Add new document type metadata types to document - for document_type_metadata_type in instance.document_type.metadata.filter(required=True): + # Add the metadata types of the new document type to the document + # excluding exisitng document metadata + # get_or_create is not used to avoid a possible triggering of indexes + # or workflow on document change by metadata save signal + new_document_type_metadata_types = instance.document_type.metadata.filter( + required=True + ).exclude(metadata_type__in=instance.metadata.values('metadata_type')) + + for document_type_metadata_type in new_document_type_metadata_types: DocumentMetadata.objects.create( document=instance, metadata_type=document_type_metadata_type.metadata_type, - value=None + value=document_type_metadata_type.metadata_type.default ) diff --git a/mayan/apps/metadata/managers.py b/mayan/apps/metadata/managers.py index bff9196770..68775323ab 100644 --- a/mayan/apps/metadata/managers.py +++ b/mayan/apps/metadata/managers.py @@ -1,5 +1,6 @@ from __future__ import unicode_literals +from django.apps import apps from django.db import models @@ -20,3 +21,16 @@ class MetadataTypeManager(models.Manager): 'metadata_type', flat=True ) ) + + +class DocumentTypeMetadataTypeManager(models.Manager): + def get_metadata_types_for(self, document_type): + DocumentType = apps.get_model( + app_label='metadata', model_name='MetadataType' + ) + + return DocumentType.objects.filter( + pk__in=self.filter( + document_type=document_type + ).values_list('metadata_type__pk', flat=True) + ) diff --git a/mayan/apps/metadata/models.py b/mayan/apps/metadata/models.py index 9211673950..b58b4dcd9f 100644 --- a/mayan/apps/metadata/models.py +++ b/mayan/apps/metadata/models.py @@ -12,7 +12,7 @@ from django.utils.translation import ugettext_lazy as _ from documents.models import Document, DocumentType from .classes import MetadataLookup -from .managers import MetadataTypeManager +from .managers import DocumentTypeMetadataTypeManager, MetadataTypeManager from .settings import setting_available_parsers, setting_available_validators @@ -166,6 +166,11 @@ class DocumentMetadata(models.Model): return force_text(self.metadata_type) def delete(self, enforce_required=True, *args, **kwargs): + """ + enforce_required prevents deletion of required metadata at the + model level. It used set to False when deleting document metadata + on document type change. + """ if enforce_required and self.metadata_type.pk in self.document.document_type.metadata.filter(required=True).values_list('metadata_type', flat=True): raise ValidationError( _('Metadata type is required for this document type.') @@ -212,6 +217,8 @@ class DocumentTypeMetadataType(models.Model): ) required = models.BooleanField(default=False, verbose_name=_('Required')) + objects = DocumentTypeMetadataTypeManager() + def __str__(self): return force_text(self.metadata_type) diff --git a/mayan/apps/metadata/tests/test_models.py b/mayan/apps/metadata/tests/test_models.py index b7244a2c55..7f538442ff 100644 --- a/mayan/apps/metadata/tests/test_models.py +++ b/mayan/apps/metadata/tests/test_models.py @@ -6,7 +6,10 @@ from django.test import override_settings from common.tests import BaseTestCase from documents.models import DocumentType -from documents.tests import TEST_SMALL_DOCUMENT_PATH, TEST_DOCUMENT_TYPE_LABEL +from documents.tests import ( + TEST_DOCUMENT_TYPE_2_LABEL, TEST_SMALL_DOCUMENT_PATH, + TEST_DOCUMENT_TYPE_LABEL +) from ..models import MetadataType, DocumentMetadata @@ -190,3 +193,107 @@ class MetadataTestCase(BaseTestCase): self.metadata_type.lookup = 'test1,test2' self.metadata_type.save() self.metadata_type.validate_value(document_type=None, value='test1') + + def test_add_new_metadata_type_on_document_type_change(self): + """ + When switching document types, add the required metadata of the new + document type, the value to the default of the metadata type. + """ + self.metadata_type.default = TEST_DEFAULT_VALUE + self.metadata_type.save() + + self.document_type_2 = DocumentType.objects.create( + label=TEST_DOCUMENT_TYPE_2_LABEL + ) + + self.document_type_2.metadata.create( + metadata_type=self.metadata_type, required=True + ) + + self.document.set_document_type(document_type=self.document_type_2) + + self.assertEqual(self.document.metadata.count(), 1) + self.assertEqual( + self.document.metadata.first().value, TEST_DEFAULT_VALUE + ) + + def test_preserve_metadata_value_on_document_type_change(self): + """ + Preserve the document metadata that is present in the + old and new document types + """ + document_metadata = DocumentMetadata( + document=self.document, metadata_type=self.metadata_type, + value=TEST_DEFAULT_VALUE + ) + + document_metadata.full_clean() + document_metadata.save() + + self.document_type_2 = DocumentType.objects.create( + label=TEST_DOCUMENT_TYPE_2_LABEL + ) + + self.document_type_2.metadata.create(metadata_type=self.metadata_type) + + self.document.set_document_type(document_type=self.document_type_2) + + self.assertEqual(self.document.metadata.count(), 1) + self.assertEqual( + self.document.metadata.first().value, TEST_DEFAULT_VALUE + ) + self.assertEqual( + self.document.metadata.first().metadata_type, self.metadata_type + ) + + def test_delete_metadata_value_on_document_type_change(self): + """ + Delete the old document metadata whose types are not present in the + new document type + """ + document_metadata = DocumentMetadata( + document=self.document, metadata_type=self.metadata_type, + value=TEST_DEFAULT_VALUE + ) + + document_metadata.full_clean() + document_metadata.save() + + self.document_type_2 = DocumentType.objects.create( + label=TEST_DOCUMENT_TYPE_2_LABEL + ) + + self.document.set_document_type(document_type=self.document_type_2) + + self.assertEqual(self.document.metadata.count(), 0) + + def test_duplicate_metadata_value_on_document_type_change(self): + """ + Delete the old document metadata whose types are not present in the + new document type + """ + document_metadata = DocumentMetadata( + document=self.document, metadata_type=self.metadata_type, + value=TEST_DEFAULT_VALUE + ) + + document_metadata.full_clean() + document_metadata.save() + + self.document_type_2 = DocumentType.objects.create( + label=TEST_DOCUMENT_TYPE_2_LABEL + ) + + self.document_type_2.metadata.create( + metadata_type=self.metadata_type, required=True + ) + + self.document.set_document_type(document_type=self.document_type_2) + + self.assertEqual(self.document.metadata.count(), 1) + self.assertEqual( + self.document.metadata.first().value, TEST_DEFAULT_VALUE + ) + self.assertEqual( + self.document.metadata.first().metadata_type, self.metadata_type + )