♻️ refactoring and optimisation: manage defaults like pre_importer_values - add an explicit field for required value to apply defaults

4 files changed, 126 insertions, 39 deletions

diff --git a/ishtar_common/data_importer.py b/ishtar_common/data_importer.py
index d5e6f966c..8e3c41e62 100644
--- a/ishtar_common/data_importer.py
+++ b/ishtar_common/data_importer.py
@@ -746,7 +746,6 @@ class Importer(object):
     UNICITY_KEYS = []
     # if set only models inside this list can be created
     MODEL_CREATION_LIMIT = []
-    EXTRA_DEFAULTS = {}
     DEFAULTS = {}
     PRE_IMPORT_VALUES = {}  # values from a form before the import
     ERRORS = {
@@ -834,12 +833,6 @@ class Importer(object):
         if import_instance and import_instance.imported_images:
             self.archive = import_instance.imported_images
         self._defaults = self.DEFAULTS.copy()
-        # EXTRA_DEFAULTS are for multiple inheritance
-        if self.EXTRA_DEFAULTS:
-            for k in self.EXTRA_DEFAULTS:
-                if k not in self._defaults:
-                    self._defaults[k] = {}
-                self._defaults[k].update(self.EXTRA_DEFAULTS[k])
         self._pre_import_values = self.PRE_IMPORT_VALUES.copy()
         self.history_modifier = history_modifier
         self.output = output
@@ -1158,6 +1151,20 @@ class Importer(object):
             except:  # nosec
                 pass
 
+        data = update_data(self._pre_import_values, data)
+        # put default values only if relevant
+        for mandatory_keys, defaults in self._defaults:
+            test_dict = data.copy()
+            nok = False
+            for k in mandatory_keys:  # test have keys
+                if k not in test_dict:
+                    nok = True
+                    break
+                test_dict = test_dict[k]
+            if nok or not any(1 for k in test_dict if test_dict[k]):  # test empty dict
+                continue
+            data = update_data(defaults, data)
+
         self.validity.append(c_row)
         if not self.c_errors and (idx_col + 1) < self.min_col_number:
             self.c_errors = True
@@ -1188,7 +1195,6 @@ class Importer(object):
             default_srs = profile.srs if profile.srs else None
 
             if "geodata" in data:
-                geodata = copy.deepcopy(self._defaults.get(("geodata",), {}))
                 geodata.update(data.pop("geodata"))
                 if default_srs and not [
                     1 for k in geodata if k.startswith("spatial_reference_system") and
@@ -1197,7 +1203,6 @@ class Importer(object):
                     geodata["spatial_reference_system"] = default_srs
 
             if "main_geodata" in data:
-                main_geodata = copy.deepcopy(self._defaults.get(("main_geodata",), {}))
                 main_geodata.update(data.pop("main_geodata"))
                 if default_srs and not [
                     1 for k in main_geodata if k.startswith("spatial_reference_system")
@@ -1538,7 +1543,6 @@ class Importer(object):
                         concat=concats[idx],
                         concat_str=concat_str[idx],
                     )
-        data = update_data(self._pre_import_values, data)
         c_row.append(" ; ".join([v for v in c_values]))
         return data
 
@@ -1582,15 +1586,6 @@ class Importer(object):
             # contruct many dict for each values
             default_dict = {}
 
-            # # get default values
-            p = [attribute]
-            if c_path:
-                p = list(c_path) + p
-            p = tuple(p)
-            if p in self._defaults:
-                for k in self._defaults[p]:
-                    default_dict[k] = self._defaults[p][k]
-
             # # init with simple values that will be duplicated
             for key in val.keys():
                 if type(val[key]) not in (list, tuple):
@@ -1896,15 +1891,10 @@ class Importer(object):
             elif type(create_dict[k]) == File:
                 create_dict[k] = copy.copy(data[k])
 
-        # default values
         path = tuple(path)
         defaults = {}
         if hasattr(cls, "get_import_defaults"):
             defaults = cls.get_import_defaults()
-        if path in self._defaults:
-            for k in self._defaults[path]:
-                if (k not in data or not data[k]) and self._defaults[path][k]:
-                    defaults[k] = self._defaults[path][k]
 
         if "history_modifier" in create_dict:
             defaults.update({"history_modifier": create_dict.pop("history_modifier")})
diff --git a/ishtar_common/migrations/0230_auto_20230912_1832.py b/ishtar_common/migrations/0230_auto_20230918_1655.py
index 649e0dccb..76fba744c 100644
--- a/ishtar_common/migrations/0230_auto_20230912_1832.py
+++ b/ishtar_common/migrations/0230_auto_20230918_1655.py
@@ -1,4 +1,4 @@
-# Generated by Django 2.2.24 on 2023-09-12 18:32
+# Generated by Django 2.2.24 on 2023-09-18 16:55
 
 import django.core.validators
 from django.db import migrations, models
@@ -41,6 +41,11 @@ class Migration(migrations.Migration):
             field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='imports', to='ishtar_common.Import', verbose_name='Next import'),
         ),
         migrations.AddField(
+            model_name='importerdefault',
+            name='required_fields',
+            field=models.CharField(blank=True, default='', help_text='Theses defaults values only apply if the designated fields are not empty. Use the "__" notation to pass between models.Leave empty to always apply.', max_length=500, verbose_name='Required fields'),
+        ),
+        migrations.AddField(
             model_name='importertype',
             name='archive_required',
             field=models.BooleanField(default=False, verbose_name='Archive required'),
@@ -71,6 +76,11 @@ class Migration(migrations.Migration):
             field=models.SmallIntegerField(default=1, help_text='Column number in the table. Put 0 or negative number for pre-importer field.', verbose_name='Column number'),
         ),
         migrations.AlterField(
+            model_name='importerdefault',
+            name='target',
+            field=models.CharField(help_text='The target of the default values. Can be set to empty with "-". Use the "__" notation to pass between models.', max_length=500, verbose_name='Target'),
+        ),
+        migrations.AlterField(
             model_name='ishtarsiteprofile',
             name='account_naming_style',
             field=models.CharField(choices=[('NF', 'name.firstname'), ('FN', 'firstname.name')], default='FN', max_length=2, verbose_name='Naming style for accounts'),
diff --git a/ishtar_common/migrations/0231_default_mandatory_keys.py b/ishtar_common/migrations/0231_default_mandatory_keys.py
new file mode 100644
index 000000000..4c5e2ea35
--- /dev/null
+++ b/ishtar_common/migrations/0231_default_mandatory_keys.py
@@ -0,0 +1,38 @@
+# Generated by Django 2.2.24 on 2023-09-18 17:05
+
+from django.db import migrations
+
+EXCLUDE_LIST = [
+    "-",
+    "auto_external_id",
+    "spatial_reference_system",
+    "public_domain",
+]
+
+FULL_COPY_LIST = [
+    "scientist__attached_to",
+]
+
+
+def migrate(apps, __):
+    ImporterDefault = apps.get_model('ishtar_common', 'ImporterDefault')
+    for default in ImporterDefault.objects.all():
+        if default.target not in EXCLUDE_LIST:
+            req = default.target
+            if req not in FULL_COPY_LIST:
+                req = req.split("__")[0]
+            if req.endswith("_type") or req.endswith("_types"):
+                continue
+            default.required_fields = req
+            default.save()
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('ishtar_common', '0230_auto_20230918_1655'),
+    ]
+
+    operations = [
+        migrations.RunPython(migrate),
+    ]
diff --git a/ishtar_common/models_imports.py b/ishtar_common/models_imports.py
index c03dd208b..85b859a5d 100644
--- a/ishtar_common/models_imports.py
+++ b/ishtar_common/models_imports.py
@@ -288,14 +288,23 @@ class ImporterType(models.Model):
         uno.save_calc(calc, dest_filename)
         return dest_filename
 
+    def get_default_values(self) -> list:
+        """
+        Get list of 2-tuple to manage default values.
+        The first item is a tuple of mandatory fields for the values to be relevant. Empty if the
+        default values always applies.
+        The second item is the dict of default values.
+        """
+        return [(default.mandatory_keys, default.values) for default in self.defaults.all()]
+
     def get_importer_class(self, import_instance=None):
-        OBJECT_CLS = import_class(self.associated_models.klass)
-        DEFAULTS = dict(
-            [(default.keys, default.values) for default in self.defaults.all()]
-        )
-        PRE_IMPORT_VALUES = {}
+        object_cls = import_class(self.associated_models.klass)
+        pre_import_values = {}
         if import_instance:
-            PRE_IMPORT_VALUES = import_instance.pre_import_values
+            pre_import_values = import_instance.pre_import_values
+            defaults = import_instance.default_values
+        else:
+            defaults = self.get_default_values()
         LINE_FORMAT = []
         LINE_EXPORT_FORMAT = []
         idx = 0
@@ -356,10 +365,10 @@ class ImporterType(models.Model):
         for modls in self.created_models.all():
             MODEL_CREATION_LIMIT.append(import_class(modls.klass))
         args = {
-            "OBJECT_CLS": OBJECT_CLS,
+            "OBJECT_CLS": object_cls,
             "DESC": self.description,
-            "DEFAULTS": DEFAULTS,
-            "PRE_IMPORT_VALUES": PRE_IMPORT_VALUES,
+            "DEFAULTS": defaults,
+            "PRE_IMPORT_VALUES": pre_import_values,
             "LINE_FORMAT": LINE_FORMAT,
             "UNICITY_KEYS": UNICITY_KEYS,
             "LINE_EXPORT_FORMAT": LINE_EXPORT_FORMAT,
@@ -502,7 +511,19 @@ class ImporterDefault(models.Model):
     importer_type = models.ForeignKey(
         ImporterType, related_name="defaults", on_delete=models.CASCADE
     )
-    target = models.CharField("Target", max_length=500)
+    target = models.CharField(
+        _("Target"), max_length=500,
+        help_text=_('The target of the default values. Can be set to empty with "-". '
+                    'Use the "__" notation to pass between models.')
+    )
+    required_fields = models.CharField(
+        _("Required fields"), max_length=500, blank=True, default="",
+        help_text=_(
+            "Theses defaults values only apply if the designated fields are not empty. "
+            'Use the "__" notation to pass between models.'
+            "Leave empty to always apply."
+        )
+    )
 
     class Meta:
         verbose_name = _("Importer - Default")
@@ -531,13 +552,23 @@ class ImporterDefault(models.Model):
         )
 
     @property
+    def mandatory_keys(self):
+        return tuple(k for k in self.required_fields.split("__") if k)
+
+    @property
     def values(self):
         values = {}
         for default_value in self.default_values.all():
+            targets = list(self.keys)
             target = default_value.target
-            if target == "-":
-                target = ""
-            values[target.split("__")[0]] = default_value.get_value()
+            if target != "-":
+                targets += target.split("__")
+            value = default_value.value
+            # explicit key or id is not set - try to get the value
+            if targets[-1] not in ("srid", "txt_idx", "slug", "id", "pk"):
+                value = default_value.get_value()
+            dct = generate_dict_from_list(targets, value)
+            values = update_data(values, dct)
         return values
 
 
@@ -602,7 +633,7 @@ class ImporterDefaultValues(models.Model):
         if target not in fields:
             return
         field = fields[target]
-        if target in ("srid", "txt_idx"):
+        if target in ("srid", "txt_idx", "slug"):
             try:
                 return parent_model.objects.get(**{target: self.value})
             except (ValueError, parent_model.DoesNotExist):
@@ -1639,6 +1670,17 @@ class Import(BaseImport):
         return errors
 
     @property
+    def default_values(self) -> dict:
+        """
+        Get DB default values for this importer. Cached in memory.
+        :return: defaults values as a dict
+        """
+        if hasattr(self, "_default_values"):
+            return self._default_values
+        self._default_values = self.importer_type.get_default_values()
+        return self._default_values
+
+    @property
     def has_pre_import_form(self) -> bool:
         """
         Check if a pre-import form is available
@@ -1655,6 +1697,12 @@ class Import(BaseImport):
 
     @property
     def pre_import_values(self) -> dict:
+        """
+        Get DB pre import values for this importer. Cached in memory.
+        :return: pre import as a dict
+        """
+        if hasattr(self, "_pre_import_values"):
+            return self._pre_import_values
         values = {}
         for column in self.importer_type.columns.filter(col_number__lte=0):
             q = ImportColumnValue.objects.filter(column=column, import_item=self)
@@ -1697,6 +1745,7 @@ class Import(BaseImport):
                 keys = dup.field_name.split("__")
                 dct = generate_dict_from_list(keys, value)
                 values = update_data(values, dct)
+        self._pre_import_values = values
         return values
 
     def get_number_of_lines(self):