Fix external id generation for finds

2 files changed, 35 insertions, 28 deletions

diff --git a/archaeological_finds/models_finds.py b/archaeological_finds/models_finds.py
index 8a7dbaf08..be563f34e 100644
--- a/archaeological_finds/models_finds.py
+++ b/archaeological_finds/models_finds.py
@@ -33,8 +33,8 @@ from ishtar_common.utils import cached_label_changed, post_save_point
 
 from ishtar_common.models import GeneralType, ImageModel, BaseHistorizedItem, \
     ShortMenuItem, LightHistorizedItem, HistoricalRecords, OwnPerms, Source, \
-    Person, Basket, get_external_id, post_save_cache, ValueGetter, \
-    get_current_profile
+    Person, Basket, post_save_cache, ValueGetter, \
+    get_current_profile, ExternalIdManager
 
 from archaeological_operations.models import AdministrativeAct
 from archaeological_context_records.models import ContextRecord, Dating
@@ -150,7 +150,10 @@ class BFBulkView(object):
     """
 
 
-class BaseFind(BulkUpdatedItem, BaseHistorizedItem, OwnPerms):
+class BaseFind(ExternalIdManager, BulkUpdatedItem, BaseHistorizedItem,
+               OwnPerms):
+    EXTERNAL_ID_KEY = 'base_find_external_id'
+    EXTERNAL_ID_DEPENDENCIES = ['find']
     label = models.TextField(_(u"Free ID"))
     external_id = models.TextField(_(u"External ID"), blank=True, null=True)
     auto_external_id = models.BooleanField(
@@ -327,21 +330,6 @@ class BaseFind(BulkUpdatedItem, BaseHistorizedItem, OwnPerms):
     def name(self):
         return self.label
 
-    def save(self, *args, **kwargs):
-        returned = super(BaseFind, self).save(*args, **kwargs)
-
-        updated = False
-        if not self.external_id or self.auto_external_id:
-            external_id = get_external_id('base_find_external_id', self)
-            if external_id != self.external_id:
-                updated = True
-                self.auto_external_id = True
-                self.external_id = external_id
-        if updated:
-            self._cached_label_checked = False
-            self.save()
-        return returned
-
     @classmethod
     def cached_label_bulk_update(cls, operation_id=None, parcel_id=None,
                                  context_record_id=None, transaction_id=None):
@@ -506,8 +494,9 @@ class FBulkView(object):
     """
 
 
-class Find(BulkUpdatedItem, ValueGetter, BaseHistorizedItem, ImageModel,
-           OwnPerms, ShortMenuItem):
+class Find(ExternalIdManager, BulkUpdatedItem, ValueGetter,
+           BaseHistorizedItem, ImageModel, OwnPerms, ShortMenuItem):
+    EXTERNAL_ID_KEY = 'find_external_id'
     CHECK_DICT = dict(CHECK_CHOICES)
     SHOW_URL = 'show-find'
     SLUG = 'find'
@@ -1161,14 +1150,8 @@ class Find(BulkUpdatedItem, ValueGetter, BaseHistorizedItem, ImageModel,
     def save(self, *args, **kwargs):
         super(Find, self).save(*args, **kwargs)
 
-        updated = False
         self.skip_history_when_saving = True
-        if not self.external_id or self.auto_external_id:
-            external_id = get_external_id('find_external_id', self)
-            if external_id != self.external_id:
-                updated = True
-                self.auto_external_id = True
-                self.external_id = external_id
+        updated = self.update_external_id(save=False)
         if updated:
             self._cached_label_checked = False
             self.save()
diff --git a/archaeological_finds/tests.py b/archaeological_finds/tests.py
index 3e89273a0..05eef2c96 100644
--- a/archaeological_finds/tests.py
+++ b/archaeological_finds/tests.py
@@ -29,7 +29,7 @@ from ishtar_common.models import ImporterType, IshtarUser, ImporterColumn,\
     FormaterType, ImportTarget, IshtarSiteProfile
 
 from ishtar_common.models import Person, get_current_profile
-from archaeological_context_records.models import Period, Dating
+from archaeological_context_records.models import Period, Dating, ContextRecord
 from archaeological_finds import models, views
 from archaeological_warehouse.models import Warehouse, WarehouseType
 
@@ -356,6 +356,30 @@ class FindTest(FindInit, TestCase):
             u"{}-{}".format(
                 base_find.context_record.external_id,
                 base_find.label))
+        base_find.label = "New label"
+        base_find.save()
+        base_find = models.BaseFind.objects.get(pk=base_find.pk)
+        self.assertEqual(
+            base_find.external_id,
+            u"{}-{}".format(
+                base_find.context_record.external_id,
+                "New label"))
+        cr = ContextRecord.objects.get(pk=base_find.context_record.pk)
+        cr.label = "new-label-too"
+        cr.save()
+        base_find = models.BaseFind.objects.get(pk=base_find.pk)
+        find = models.Find.objects.get(pk=find.pk)
+        cr = ContextRecord.objects.get(pk=cr.pk)
+        self.assertIn("new-label-too", find.external_id)
+        self.assertIn("new-label-too", base_find.external_id)
+
+        cr.operation.code_patriarche = "PAT"
+        cr.operation.save()
+        base_find = models.BaseFind.objects.get(pk=base_find.pk)
+        find = models.Find.objects.get(pk=find.pk)
+        cr = ContextRecord.objects.get(pk=cr.pk)
+        self.assertIn("PAT", find.external_id)
+        self.assertIn("PAT", base_find.external_id)
 
     def testIndex(self):
         profile = get_current_profile()