Importers: fix user target key management (refs #3725)

6 files changed, 133 insertions, 34 deletions

diff --git a/archaeological_operations/tests.py b/archaeological_operations/tests.py
index 0d17acb8b..60078b7e2 100644
--- a/archaeological_operations/tests.py
+++ b/archaeological_operations/tests.py
@@ -25,6 +25,7 @@ import zipfile
 from django.conf import settings
 from django.core.files.uploadedfile import SimpleUploadedFile
 from django.core.urlresolvers import reverse
+from django.db.models import Q
 from django.test.client import Client
 
 from django.contrib.auth.models import User
@@ -125,6 +126,11 @@ class ImportTest(object):
 
     def init_ope_targetkey(self, imp):
         # doing manually connections
+        q = Q(importer=imp) | Q(user=imp.user)
+        if imp.associated_group:
+            q |= Q(group=imp.associated_group)
+        for ik in ItemKey.objects.filter(q).all():
+            ik.delete()
 
         # target for this import
         target = TargetKey.objects.filter(
@@ -136,39 +142,56 @@ class ImportTest(object):
         target.save()
 
         # target for all users
-        target2 = TargetKey.objects.get(key='gallo-romain')
+        tgs = list(TargetKey.objects.filter(key='gallo-romain').all())
+        for tg in tgs[1:]:
+            tg.delete()
+        target2 = tgs[0]
         gallo = models.Period.objects.get(txt_idx='gallo-roman')
         target2.value = gallo.pk
         target2.is_set = True
         target2.associated_import = None
+        target2.associated_group = None
+        target2.associated_user = None
         target2.save()
 
         # target for this user
-        target3 = TargetKey.objects.get(key='age-du-fer')
+        tgs = list(TargetKey.objects.filter(key='age-du-fer').all())
+        for tg in tgs[1:]:
+            tg.delete()
+        target3 = tgs[0]
         iron = models.Period.objects.get(txt_idx='iron_age')
         target3.value = iron.pk
         target3.is_set = True
         target3.associated_import = None
         target3.associated_user = self.ishtar_user
+        target3.associated_group = None
         target3.save()
 
         # target for another user
         username, password, user = create_user()
         another_user = IshtarUser.objects.get(pk=user.pk)
-        target4 = TargetKey.objects.get(key='neolithique')
+        tgs = list(TargetKey.objects.filter(key='neolithik').all())
+        for tg in tgs[1:]:
+            tg.delete()
+        target4 = tgs[0]
         neo = models.Period.objects.get(txt_idx='neolithic')
         target4.value = neo.pk
         target4.is_set = True
         target4.associated_import = None
+        target4.associated_group = None
         target4.associated_user = another_user
         target4.save()
 
         # target for the current group
-        target5 = TargetKey.objects.get(key='moderne')
+        tgs = list(TargetKey.objects.filter(key='moderne').all())
+        for tg in tgs[1:]:
+            tg.delete()
+        target5 = tgs[0]
         modern = models.Period.objects.get(txt_idx='modern')
         target5.value = modern.pk
         target5.is_set = True
         target5.associated_import = None
+        target5.associated_user = None
         target5.associated_group = imp.associated_group
         target5.save()
 
@@ -271,13 +294,12 @@ class ImportOperationTest(ImportTest, TestCase):
         self.assertEqual(last_ope.code_patriarche, '4200')
         self.assertEqual(last_ope.operation_type.txt_idx, 'prog_excavation')
 
-        # self.assertEqual(last_ope.periods.count(), 3)
-        self.assertEqual(last_ope.periods.count(), 4)
+        self.assertEqual(last_ope.periods.count(), 3)
         periods = [period.txt_idx for period in last_ope.periods.all()]
         self.assertIn('iron_age', periods)
         self.assertIn('gallo-roman', periods)
         # target key set for another user
-        # self.assertNotIn('neolithic', periods)
+        self.assertNotIn('neolithic', periods)
 
         # a second importation will be not possible: no two same patriarche
         # code
@@ -298,12 +320,20 @@ class ImportOperationTest(ImportTest, TestCase):
 
         importer, form = self.init_ope_import()
         other_imp = form.save(self.ishtar_user)
-        # associate with another import
-        for ik in ItemKey.objects.filter(importer=impt).all():
-            ik.importer = other_imp
-            ik.save()
 
-        for tg in TargetKey.objects.filter(associated_import=impt).all():
+        # re-associate with another import
+        q = Q(importer=impt) | Q(user=impt.user)
+        if impt.associated_group:
+            q |= Q(group=impt.associated_group)
+        for ik in ItemKey.objects.filter(q).all():
+            ik.delete()
+
+        q = Q(associated_import=impt) | Q(associated_user=impt.user)
+        if impt.associated_group:
+            q |= Q(associated_group=impt.associated_group)
+        for tg in TargetKey.objects.filter(q).all():
+            tg.associated_user = None
+            tg.associated_group = None
             tg.associated_import = other_imp
             tg.save()
 
diff --git a/archaeological_operations/tests/MCC-operations-example.csv b/archaeological_operations/tests/MCC-operations-example.csv
index 16db46451..bb8fc3084 100644
--- a/archaeological_operations/tests/MCC-operations-example.csv
+++ b/archaeological_operations/tests/MCC-operations-example.csv
@@ -1,3 +1,3 @@
 code OA,region,type operation,intitule operation,operateur,responsable operation,date debut terrain,date fin terrain,chronologie generale,identifiant document georeferencement,notice scientifique
 4201,Bourgogne,Fouille programmée,Oppìdum de Paris 2,L'opérateur,,2000/01/31,2002/12/31,Age du Fer,,
-4200,Bourgogne,Fouille programmée,Oppìdum de Paris,L'opérateur,Jean Sui-Resp'on Sablé,2000/01/22,2002/12/31,Age du Fer & Gallo-Romain & Néolithique & Moderne,,
+4200,Bourgogne,Fouille programmée,Oppìdum de Paris,L'opérateur,Jean Sui-Resp'on Sablé,2000/01/22,2002/12/31,Age du Fer & Gallo-Romain & Néolithik & Moderne,,
diff --git a/ishtar_common/data_importer.py b/ishtar_common/data_importer.py
index 00aa34f4c..b88fbff2f 100644
--- a/ishtar_common/data_importer.py
+++ b/ishtar_common/data_importer.py
@@ -431,7 +431,7 @@ class StrChoiceFormater(Formater, ChoiceChecker):
                 if self.model and v:
                     v = self.model.objects.get(pk=v)
                 self.equiv_dict[value] = v
-                self.add_key(v, value)
+                self.add_key(v, value, import_instance)
                 self.new_keys[value] = v
             elif self.create and res == len(self.choices):
                 self.equiv_dict[value] = self.new(base_value)
@@ -479,7 +479,7 @@ class StrChoiceFormater(Formater, ChoiceChecker):
     def new(self, value):
         return
 
-    def add_key(self, obj, value):
+    def add_key(self, obj, value, importer=None):
         return
 
     def format(self, value):
@@ -509,14 +509,14 @@ class TypeFormater(StrChoiceFormater):
         if self.import_instance:
             for item in model.objects.all():
                 self.choices.append((item.pk, unicode(item)))
-                for key in item.get_keys(importer_id=import_instance.pk):
+                for key in item.get_keys(importer=import_instance):
                     self.equiv_dict[key] = item
 
     def prepare(self, value):
         return slugify(unicode(value).strip())
 
-    def add_key(self, obj, value):
-        obj.add_key(slugify(value), force=True)
+    def add_key(self, obj, value, importer=None):
+        obj.add_key(slugify(value), force=True, importer=importer)
 
     def new(self, value):
         values = copy.copy(self.defaults)
diff --git a/ishtar_common/migrations/0011_auto_20170918_1913.py b/ishtar_common/migrations/0011_auto_20170918_1913.py
new file mode 100644
index 000000000..30ae07fd8
--- /dev/null
+++ b/ishtar_common/migrations/0011_auto_20170918_1913.py
@@ -0,0 +1,50 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11 on 2017-09-18 19:13
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('ishtar_common', '0010_auto_20170829_1716'),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name='targetkey',
+            options={'ordering': ('target', 'key'), 'verbose_name': 'Importer - Target key', 'verbose_name_plural': 'Importer - Targets keys'},
+        ),
+        migrations.AddField(
+            model_name='itemkey',
+            name='group',
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='ishtar_common.TargetKeyGroup'),
+        ),
+        migrations.AddField(
+            model_name='itemkey',
+            name='user',
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to='ishtar_common.IshtarUser'),
+        ),
+        migrations.AlterField(
+            model_name='import',
+            name='associated_group',
+            field=models.ForeignKey(blank=True, help_text='If a group is selected, target key saved in this group will be used.', null=True, on_delete=django.db.models.deletion.CASCADE, to='ishtar_common.TargetKeyGroup'),
+        ),
+        migrations.AlterField(
+            model_name='import',
+            name='conservative_import',
+            field=models.BooleanField(default=False, help_text='If set to true, do not overload existing values.', verbose_name='Conservative import'),
+        ),
+        migrations.AlterField(
+            model_name='import',
+            name='name',
+            field=models.CharField(max_length=500, null=True, verbose_name='Name'),
+        ),
+        migrations.AlterField(
+            model_name='import',
+            name='skip_lines',
+            field=models.IntegerField(default=1, help_text='Number of header lines in your file (can be 0).', verbose_name='Skip lines'),
+        ),
+    ]
diff --git a/ishtar_common/models.py b/ishtar_common/models.py
index a4fe5af7f..53d5c85ac 100644
--- a/ishtar_common/models.py
+++ b/ishtar_common/models.py
@@ -717,33 +717,43 @@ class GeneralType(Cached, models.Model):
         self.generate_key(force=True)
         return obj
 
-    def add_key(self, key, force=False, importer=None):
+    def add_key(self, key, force=False, importer=None, group=None,
+                user=None):
         content_type = ContentType.objects.get_for_model(self.__class__)
         if not importer and not force and ItemKey.objects.filter(
                 key=key, content_type=content_type).count():
             return
+        filtr = {'key': key, 'content_type': content_type}
+        if group:
+            filtr['group'] = group
+        elif user:
+            filtr['user'] = user
+        else:
+            filtr['importer'] = importer
         if force:
-            ItemKey.objects.filter(key=key, content_type=content_type,
-                                   importer=importer)\
-                           .exclude(object_id=self.pk).delete()
-        ItemKey.objects.get_or_create(
-            object_id=self.pk, key=key, content_type=content_type,
-            importer=importer
-        )
+            ItemKey.objects.filter(**filtr).exclude(object_id=self.pk).delete()
+        filtr['object_id'] = self.pk
+        ItemKey.objects.get_or_create(**filtr)
 
     def generate_key(self, force=False):
         for key in (slugify(self.label), self.txt_idx):
             self.add_key(key)
 
-    def get_keys(self, importer_id=None):
+    def get_keys(self, importer):
         keys = [self.txt_idx]
         content_type = ContentType.objects.get_for_model(self.__class__)
-        query = Q(content_type=content_type, object_id=self.pk,
-                  importer__isnull=True)
-        if importer_id:
-            query |= Q(content_type=content_type, object_id=self.pk,
-                       importer__pk=importer_id)
-        q = ItemKey.objects.filter(query)
+        base_q = Q(content_type=content_type, object_id=self.pk)
+        subquery = Q(importer__isnull=True, user__isnull=True,
+                     group__isnull=True)
+        subquery |= Q(user__isnull=True, group__isnull=True,
+                      importer=importer)
+        if importer.user:
+            subquery |= Q(user=importer.user, group__isnull=True,
+                          importer=importer)
+        if importer.associated_group:
+            subquery |= Q(user__isnull=True, group=importer.associated_group,
+                          importer=importer)
+        q = ItemKey.objects.filter(base_q & subquery)
         for ik in q.exclude(key=self.txt_idx).all():
             keys.append(ik.key)
         return keys
@@ -763,6 +773,8 @@ class ItemKey(models.Model):
     importer = models.ForeignKey(
         Import, null=True, blank=True,
         help_text=_(u"Specific key to an import"))
+    user = models.ForeignKey('IshtarUser', blank=True, null=True)
+    group = models.ForeignKey(TargetKeyGroup, blank=True, null=True)
 
     def __unicode__(self):
         return self.key
diff --git a/ishtar_common/models_imports.py b/ishtar_common/models_imports.py
index ccb0f2c57..d82d0580f 100644
--- a/ishtar_common/models_imports.py
+++ b/ishtar_common/models_imports.py
@@ -584,7 +584,14 @@ class TargetKey(models.Model):
                 except self.target.associated_model.DoesNotExist:
                     pass
             if v:
-                v.add_key(self.key, importer=self.associated_import)
+                keys = {}
+                if self.associated_group:
+                    keys['group'] = self.associated_group
+                if self.associated_user:
+                    keys['user'] = self.associated_user
+                else:
+                    keys['importer'] = self.associated_import
+                v.add_key(self.key, **keys)
         return obj
 
 TARGET_MODELS = [