Fix history M2M save

3 files changed, 27 insertions, 38 deletions

diff --git a/archaeological_finds/models_finds.py b/archaeological_finds/models_finds.py
index f8cc34e61..9e377d1f6 100644
--- a/archaeological_finds/models_finds.py
+++ b/archaeological_finds/models_finds.py
@@ -32,7 +32,7 @@ from django.utils.translation import ugettext_lazy as _, pgettext_lazy, \
 
 from ishtar_common.data_importer import post_importer_action, ImporterError
 from ishtar_common.utils import cached_label_changed, post_save_point, \
-    m2m_historization_changed, HISTORY_M2M_SPLIT
+    m2m_historization_changed
 
 from ishtar_common.alternative_configs import ALTERNATE_CONFIGS
 
@@ -41,7 +41,7 @@ from ishtar_common.models import Document, GeneralType, \
     HistoricalRecords, OwnPerms, Person, Basket, post_save_cache, \
     ValueGetter, get_current_profile, IshtarSiteProfile, PRIVATE_FIELDS, \
     SpatialReferenceSystem, BulkUpdatedItem, ExternalIdManager, QuickAction, \
-    MainItem, document_attached_changed
+    MainItem, document_attached_changed, HistoryModel
 
 from archaeological_operations.models import AdministrativeAct, Operation
 from archaeological_context_records.models import ContextRecord, Dating
@@ -1199,7 +1199,7 @@ class Find(BulkUpdatedItem, ValueGetter, BaseHistorizedItem, OwnPerms,
         help_text=_(u"Related treatments when no new find is created"))
     cached_label = models.TextField(_(u"Cached name"), null=True, blank=True,
                                     db_index=True)
-    history = HistoricalRecords()
+    history = HistoricalRecords(bases=[HistoryModel])
     BASKET_MODEL = FindBasket
 
     class Meta:
@@ -1233,16 +1233,6 @@ class Find(BulkUpdatedItem, ValueGetter, BaseHistorizedItem, OwnPerms,
         return u" ; ".join([unicode(dating) for dating in self.datings.all()])
 
     @property
-    def dating_list(self):
-        if self.historical_datings:
-            return [
-                Dating.history_decompress(v)
-                for v in self.historical_datings.split(HISTORY_M2M_SPLIT)
-            ]
-        else:
-            return self.datings.all()
-
-    @property
     def excavation_ids(self):
         return u" - ".join(
             [base_find['excavation_id']
diff --git a/archaeological_finds/templates/ishtar/sheet_find.html b/archaeological_finds/templates/ishtar/sheet_find.html
index 2d234b2d9..ac2df607d 100644
--- a/archaeological_finds/templates/ishtar/sheet_find.html
+++ b/archaeological_finds/templates/ishtar/sheet_find.html
@@ -19,7 +19,7 @@
 {% with permission_view_document=permission_view_document %}
 {% with permission_view_own_document=permission_view_own_document %}
 
-{% with display_identification=item.integrities.count|or_:item.remarkabilities.count|or_:item.conservatory_state|or_:item.conservatory_comment|or_:item.alterations.count|or_:item.alteration_causes.count|or_:item.preservation_to_considers.count|or_:item.appraisal_date|or_:item.treatment_emergency|or_:item.insurance_value|or_:item.estimated_value|or_:item.datings.count|or_:item.dating_comment %}
+{% with display_datings=item.integrities.count|or_:item.remarkabilities.count|or_:item.conservatory_state|or_:item.conservatory_comment|or_:item.alterations.count|or_:item.alteration_causes.count|or_:item.preservation_to_considers.count|or_:item.appraisal_date|or_:item.treatment_emergency|or_:item.insurance_value|or_:item.estimated_value|or_:item.historical_datings|or_:item.datings.count|or_:item.dating_comment %}
 {% with display_warehouse_treatments=item.container|or_:item.container_ref|or_:item.upstream_treatment|or_:item.downstream_treatment|or_:item.treatments.count %}
 {% with can_view_documents=permission_view_own_document|or_:permission_view_document %}
 {% with display_documents=can_view_documents|and_:item.documents.count %}
@@ -39,7 +39,7 @@
             {% trans "Identification / Description / Dimensions" %}
         </a>
     </li>
-    {% if display_identification %}
+    {% if display_datings %}
     <li class="nav-item">
         <a class="nav-link" id="{{window_id}}-preservation-tab"
            data-toggle="tab" href="#{{window_id}}-preservation" role="tab"
@@ -180,7 +180,7 @@
           {% include "ishtar/blocks/sheet_creation_section.html" %}
       </div>
   </div>
-  {% if display_identification %}
+  {% if display_datings %}
   <div class="tab-pane fade" id="{{window_id}}-preservation"
        role="tabpanel" aria-labelledby="{{window_id}}-preservation-tab">
       {% if item.integrities.count or item.remarkabilities.count or item.conservatory_state or item.conservatory_comment or item.alterations.count or item.alteration_causes.count or item.preservation_to_considers.count or item.appraisal_date or item.treatment_emergency or item.insurance_value or item.estimated_value %}
@@ -202,7 +202,6 @@
 
       {% if item.historical_datings or item.datings.count or item.dating_comment %}
       <h3>{% trans "Dating" %}</h3>
-      {% if item.datings.count %}
       <table id='{{window_id}}-datings' class="table table-striped">
           <tr>
               <th>{% trans "Period" %}</th>
@@ -212,7 +211,7 @@
               <th>{% trans "Quality" %}</th>
               <th>{% trans "Precise dating" %}</th>
           </tr>
-          {% for dating in item.dating_list %}
+          {% for dating in item|m2m_listing:'datings' %}
           <tr>
               <td>
                   {{dating.period}}
@@ -235,7 +234,6 @@
           </tr>
           {% endfor %}
       </table>
-      {% endif %}
       {% field_flex_full "Comment on dating" item.dating_comment "<pre>" "</pre>" %}
       {% endif %}
   </div>
diff --git a/archaeological_finds/tests.py b/archaeological_finds/tests.py
index cefeb1649..231ac30ad 100644
--- a/archaeological_finds/tests.py
+++ b/archaeological_finds/tests.py
@@ -861,15 +861,19 @@ class FindHistoryTest(FindInit, TestCase):
     def test_m2m_history_save(self):
         find = self.finds[0]
         user = self.get_default_user()
-        nb_hist = find.history.count()
 
         ceram = models.MaterialType.objects.get(txt_idx='ceramic').pk
         glass = models.MaterialType.objects.get(txt_idx='glass').pk
+
+        find = models.Find.objects.get(pk=find.pk)
+        nb_hist = find.history.count()
+        find.label = "hop hop hop1"
+        find.history_modifier = user
+        find._force_history = True
+        find.save()
         find.material_types.add(ceram)
         find.material_types.add(glass)
         d1_txt, d2_txt = self._add_datings(find)
-
-        find = models.Find.objects.get(pk=find.pk)
         self.assertIn(
             find.historical_material_types,
             [HISTORY_M2M_SPLIT.join(['ceramic', 'glass']),   # order do not
@@ -880,20 +884,16 @@ class FindHistoryTest(FindInit, TestCase):
              HISTORY_M2M_SPLIT.join([d2_txt, d1_txt])])  # matter
 
         historical_material_types = find.historical_material_types
-        find.label = "hop hop hop1"
-        find.history_modifier = user
-        find._force_history = True
-        find.save()
 
         find = models.Find.objects.get(pk=find.pk)
-        find.material_types.remove(ceram)
-        find.datings.clear()
         find.label = "hop hop hop2"
         find.history_modifier = user
         if hasattr(find, 'skip_history_when_saving'):
             delattr(find, 'skip_history_when_saving')
         find._force_history = True
         find.save()
+        find.material_types.remove(ceram)
+        find.datings.clear()
 
         find = models.Find.objects.get(pk=find.pk)
         self.assertEqual(find.historical_material_types, 'glass')
@@ -908,33 +908,33 @@ class FindHistoryTest(FindInit, TestCase):
         c = Client()
         user = self.get_default_user()
         find = self.finds[0]
-
-        find = models.Find.objects.get(pk=find.pk)
         ceram = models.MaterialType.objects.get(txt_idx='ceramic').pk
         glass = models.MaterialType.objects.get(txt_idx='glass').pk
-        find.material_types.add(ceram)
-        find.material_types.add(glass)
-        self._add_datings(find)
+
+        find = models.Find.objects.get(pk=find.pk)
         find.history_modifier = user
         find.label = "hop hop hop1"
         find._force_history = True
         if hasattr(find, 'skip_history_when_saving'):
             delattr(find, 'skip_history_when_saving')
         find.save()
+        find.material_types.add(ceram)
+        find.material_types.add(glass)
+        self._add_datings(find)
 
         find = models.Find.objects.get(pk=find.pk)
-        find.material_types.remove(ceram)
         find.history_modifier = user
         find.label = "hop hop hop2"
-        find.datings.clear()
         find._force_history = True
         if hasattr(find, 'skip_history_when_saving'):
             delattr(find, 'skip_history_when_saving')
         find.save()
+        find.datings.clear()
+        find.material_types.remove(ceram)
 
         find = models.Find.objects.get(pk=find.pk)
-        history_date = find.history.all()[1].history_date.strftime(
-            '%Y-%m-%dT%H:%M:%S.%f')
+        history_date = find.history.order_by('-history_date').all()[
+            1].history_date.strftime('%Y-%m-%dT%H:%M:%S.%f')
 
         c.login(username=self.username, password=self.password)
         response = c.get(reverse('show-find',
@@ -957,7 +957,8 @@ class FindHistoryTest(FindInit, TestCase):
         content = response.content.decode('utf-8')
         self.assertIn(
             models.MaterialType.objects.get(txt_idx='ceramic').label, content)
-        self.assertIn("5001", content)
+        self.assertIn("5001", content, msg=u"5001 not found in historical "
+                                           u"sheet")
         self.assertIn(
             Period.objects.get(txt_idx='neolithic').label, content)