When changing document types, don't delete the old metadata that is

also found in the new document type. GitLab issue #421.

Signed-off-by: Roberto Rosario <roberto.rosario.gonzalez@gmail.com>

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)
 ==================

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
         )

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
         )
 
 

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)
+        )

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)
 

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
+        )