Fix history management - fix archaeological files history tests

* fix rollback
* fix history duplicates

3 files changed, 40 insertions, 25 deletions

diff --git a/archaeological_files/tests.py b/archaeological_files/tests.py
index 8cd53d946..58291bc04 100644
--- a/archaeological_files/tests.py
+++ b/archaeological_files/tests.py
@@ -23,26 +23,29 @@ Unit tests
 import json
 
 from django.conf import settings
+from django.contrib.auth.models import User
 from django.test import TestCase
 
+from ishtar_common.models import PersonType
 import models
 
 class FileTest(TestCase):
-    fixtures = [settings.ROOT_PATH + 'ishtar_common/fixtures/test-users.json']
+    fixtures = [settings.ROOT_PATH + \
+                '../ishtar_common/fixtures/initial_data-fr.json']
     model = models.File
 
     def setUp(self):
         self.extra_models, self.model_list = {}, []
-
-        user = models.IshtarUser.objects.get(pk=1)
-        person_type = models.PersonType(label=u'Test person type',
-                                        txt_idx='test_person', available=True)
+        self.user, created = User.objects.get_or_create(username='username')
+        o_user, created = User.objects.get_or_create(username='ousername')
+        person_type = PersonType(label=u'Test person type',
+                                 txt_idx='test_person', available=True)
         person_type.save()
         self.extra_models['person_type'] = person_type
         self.model_list.append(person_type)
 
         person = models.Person(title='Mr', surname='Surname', name='Name',
-                               person_type=person_type, history_modifier=user)
+                               history_modifier=o_user)
         person.save()
         self.extra_models['person'] = person
         self.model_list.append(person)
@@ -55,7 +58,7 @@ class FileTest(TestCase):
 
         dct = {'year':2010, 'numeric_reference':1000, 'file_type':file_type,
                'internal_reference':u'UNIT_testÉ ?', 'in_charge':person,
-               'history_modifier':user, 'total_surface':10000}
+               'history_modifier':o_user, 'total_surface':10000}
         self.item = self.model(**dct)
         self.item.save()
 
@@ -72,6 +75,7 @@ class FileTest(TestCase):
         self.assertTrue(self.item.history.count() >= 1)
         base_label = self.item.internal_reference
         self.item.internal_reference = u"Unité_Test"
+        self.item.history_modifier = self.user
         self.item.save()
         self.failUnlessEqual(self.item.history.count(), nb_hist+1)
         self.failUnlessEqual(self.item.history.all()[1].internal_reference,
@@ -84,6 +88,7 @@ class FileTest(TestCase):
         """
         nb_hist = self.item.history.count()
         self.item.internal_reference = u"Unité_Test"
+        self.item.history_modifier = self.user
         self.item.save()
         self.failUnlessEqual(self.item.history.count(), nb_hist+1)
         nb_hist = self.item.history.count()
@@ -95,13 +100,16 @@ class FileTest(TestCase):
         initial_values = self.item.values()
         backup_date = self.item.history.all()[0].history_date
         self.item.internal_reference = u"Unité_Test"
+        self.item.history_modifier = self.user
         self.item.save()
         self.item.rollback(backup_date)
         self.failUnlessEqual(self.item.history.count(), nb_hist)
         new_values = self.item.values()
         for k in initial_values.keys():
             self.assertTrue(k in new_values)
-            self.assertEqual(new_values[k], initial_values[k])
+            self.assertEqual(new_values[k], initial_values[k],
+                        msg=u"for %s: %s != %s" % (k, unicode(new_values[k]),
+                                                   unicode(initial_values[k])))
 
     def testRESTGetFile(self):
         response = self.client.post('/get-file/',
@@ -116,6 +124,7 @@ class FileTest(TestCase):
         new_ref = u"Unité_Test_old_file"
         new_ref = initial_ref != new_ref and new_ref or new_ref + u"extra"
         self.item.internal_reference = new_ref
+        self.item.history_modifier = self.user
         self.item.save()
         response = self.client.post('/get-file/',
                      {'numeric_reference':self.item.numeric_reference, 'old':1})
diff --git a/archaeological_operations/tests.py b/archaeological_operations/tests.py
index 983165968..0a48f4d4a 100644
--- a/archaeological_operations/tests.py
+++ b/archaeological_operations/tests.py
@@ -31,9 +31,9 @@ import models
 from ishtar_common.models import OrganizationType, Organization, Town
 
 class ImportOperationTest(TestCase):
-    fixtures = ['../ishtar_common/fixtures/initial_data.json',
-                '../archaeological_files/fixtures/initial_data.json',
-                '../archaeological_operations/fixtures/initial_data-fr.json']
+    fixtures = [settings.ROOT_PATH + '../ishtar_common/fixtures/initial_data.json',
+                settings.ROOT_PATH + '../archaeological_files/fixtures/initial_data.json',
+                settings.ROOT_PATH + '../archaeological_operations/fixtures/initial_data-fr.json']
 
     def setUp(self):
         user = User.objects.create_user('username')
diff --git a/ishtar_common/models.py b/ishtar_common/models.py
index ee5ad0c8d..54381598e 100644
--- a/ishtar_common/models.py
+++ b/ishtar_common/models.py
@@ -59,31 +59,31 @@ post_save.connect(post_save_user, sender=User)
 
 class HistoricalRecords(BaseHistoricalRecords):
     def create_historical_record(self, instance, type):
-        history_user = getattr(instance, '_history_user', None)
+        history_modifier = getattr(instance, 'history_modifier', None)
         manager = getattr(instance, self.manager_name)
         attrs = {}
         for field in instance._meta.fields:
             attrs[field.attname] = getattr(instance, field.attname)
-        q_history = instance.history.order_by('-history_date',
-                                              '-history_id')
+        q_history = instance.history.filter(
+                                   history_modifier_id=history_modifier.pk
+                                   ).order_by('-history_date', '-history_id')
         if not q_history.count():
-            manager.create(history_type=type, history_user=history_user,
-                           **attrs)
+            manager.create(history_type=type, **attrs)
             return
-        old_instance = q_history.all()[0]
         # multiple saving by the same user in a very short time are generaly
         # caused by post_save signals it is not relevant to keep them
-        history_date = datetime.datetime.now()
-        if old_instance.history_date and \
-           history_date - old_instance.history_date \
-                                           < datetime.timedelta(seconds=20):
+        min_history_date = datetime.datetime.now() \
+                           - datetime.timedelta(seconds=5)
+        q = q_history.filter(history_date__isnull=False,
+                             history_date__gt=min_history_date
+                     ).order_by('-history_date', '-history_id')
+        if q.count():
             return
 
         # record a new version only if data have been changed
         for field in instance._meta.fields:
             if getattr(old_instance, field.attname) != attrs[field.attname]:
-                manager.create(history_type=type, history_user=history_user,
-                               **attrs)
+                manager.create(history_type=type, **attrs)
                 return
 
 # valid ID validator for models
@@ -407,10 +407,10 @@ class BaseHistorizedItem(models.Model):
         """
         to_del, new_item = [], None
         for item in self.history.all():
-            to_del.append(item)
             if item.history_date == date:
                 new_item = item
                 break
+            to_del.append(item)
         if not new_item:
             raise HistoryError(u"The date to rollback to doesn't exist.")
         try:
@@ -420,6 +420,11 @@ class BaseHistorizedItem(models.Model):
                     if not hasattr(new_item, k):
                         k = k + "_id"
                     setattr(self, k, getattr(new_item, k))
+            try:
+                self.history_modifier = User.objects.get(
+                           pk=new_item.history_modifier_id)
+            except User.ObjectDoesNotExist:
+                pass
             self.save()
         except:
             raise HistoryError(u"The rollback has failed.")
@@ -740,7 +745,8 @@ class IshtarUser(User):
                                        name=name, email=email,
                                        history_modifier=user)
         person.person_types.add(person_type)
-        return IshtarUser.objects.create(user_ptr=user, person=person)
+        return IshtarUser.objects.create(user_ptr=user, username=default,
+                                         person=person)
 
     def has_right(self, right_name):
        return self.person.has_right(right_name)