Fix bad M2M renitialization on wizards (refs #4963)

3 files changed, 107 insertions, 19 deletions

diff --git a/archaeological_finds/tests.py b/archaeological_finds/tests.py
index cfdcb1585..b376a34ff 100644
--- a/archaeological_finds/tests.py
+++ b/archaeological_finds/tests.py
@@ -301,6 +301,79 @@ class FindWizardCreationTest(WizardTest, FindInit, TestCase):
         self.assertEqual(f.datings.count(), 1)
 
 
+class FindWizardModificationTest(WizardTest, FindInit, TestCase):
+    fixtures = WAREHOUSE_FIXTURES
+    url_name = 'find_modification'
+    wizard_name = url_name + '_wizard'
+    steps = views.find_modification_steps
+    redirect_url = "/find_modification/selec-find_modification?open_item=" \
+                   "{last_id}"
+    model = models.Find
+    form_datas = [
+        FormData(
+            'Find modification',
+            form_datas={
+                'selec': {'pk': ''},
+                'selecrecord': {
+                    "get_first_base_find__context_record": ""
+                },
+                'find-find_modification': {
+                    'label': 'hop',
+                    'checked': 'NC',
+                    'check_date': '2016-01-01'
+                },
+                'dating-find_modification': [
+                    {
+                        'period': None,
+                        'start_date': '',
+                        'end_date': '',
+                    },
+                ]
+            },
+            ignored=['preservation-find_modification',
+                     'selecw-find_modification',
+                     'simplefind-find_modification']
+        )
+    ]
+
+    def pre_wizard(self):
+        find, __ = self.get_default_find()
+        self.find = find
+        find.label = "base-hop"
+        find.check_date = "2020-07-01"
+        find.save()
+        data = self.form_datas[0].form_datas
+
+        data['selec']['pk'] = find.pk
+        data['selecrecord']['get_first_base_find__context_record'] = \
+            find.base_finds.all()[0].pk
+
+        self.period = Period.objects.all()[0]
+        self.period2 = Period.objects.all()[1]
+
+        find.datings.add(Dating.objects.create(
+            period=self.period, start_date='0', end_date='200'))
+        find.datings.add(Dating.objects.create(period=self.period2))
+
+        data['dating-find_modification'][0]['period'] = self.period.pk
+        self.find_number = models.Find.objects.count()
+        self.basefind_number = models.BaseFind.objects.count()
+        super(FindWizardModificationTest, self).pre_wizard()
+
+    def post_wizard(self):
+        # no creation
+        self.assertEqual(models.BaseFind.objects.count(),
+                         self.basefind_number)
+        self.assertEqual(models.Find.objects.count(),
+                         self.find_number)
+        f = models.Find.objects.get(pk=self.find.pk)
+        self.assertEqual(f.datings.count(), 1)
+        dating = f.datings.all()[0]
+        self.assertEqual(dating.period, self.period)
+        self.assertEqual(dating.end_date, None)
+        self.assertEqual(dating.start_date, None)
+
+
 class FindWizardDeletionWithWarehouseModTest(WizardTest, FindInit, TestCase):
     fixtures = WAREHOUSE_FIXTURES
     url_name = 'find_deletion'
diff --git a/ishtar_common/tests.py b/ishtar_common/tests.py
index 9b7080387..0eda481b1 100644
--- a/ishtar_common/tests.py
+++ b/ishtar_common/tests.py
@@ -471,10 +471,10 @@ class WizardTest(object):
         try:
             response = client.post(url, data, follow=follow)
         except ValidationError as e:
-            msg = u"Errors: {} on {}. On \"ManagementForm data is " \
-                  u"missing or...\" error verify the wizard_name or " \
-                  u"step name".format(u" - ".join(e.messages),
-                                      current_step)
+            msg = "Errors: {} on {}. On \"ManagementForm data is " \
+                  "missing or...\" error verify the wizard_name or " \
+                  "step name".format(" - ".join(e.messages),
+                                     current_step)
             raise ValidationError(msg)
         return response
 
diff --git a/ishtar_common/wizards.py b/ishtar_common/wizards.py
index 30aa014c4..eac8d1868 100644
--- a/ishtar_common/wizards.py
+++ b/ishtar_common/wizards.py
@@ -447,18 +447,18 @@ class Wizard(IshtarWizard):
                     # regroup each line
                     displayed_values = []
                     for lbl, value, cls in current_form_data:
-                        displayed_value = u""
+                        displayed_value = ""
                         if lbl.strip():
-                            displayed_value = u"<strong>{}{}</strong> ".format(
-                                 lbl, _(u":"))
+                            displayed_value = "<strong>{}{}</strong> ".format(
+                                 lbl, _(":"))
                         displayed_value += str(value)
                         displayed_values.append(displayed_value)
-                    value = u" ; ".join(displayed_values)
+                    value = " ; ".join(displayed_values)
                     if is_deleted:
-                        value = u"<span class='text-danger'>{}</span>".format(
+                        value = "<span class='text-danger'>{}</span>".format(
                             value)
-                    deleted = u"<span class='text-danger'>{}</span>".format(
-                        _(u"Deleted")) if is_deleted else ""
+                    deleted = "<span class='text-danger'>{}</span>".format(
+                        _("Deleted")) if is_deleted else ""
                     form_datas.append((mark_safe(deleted), mark_safe(value),
                                        ''))
                 else:
@@ -502,7 +502,7 @@ class Wizard(IshtarWizard):
                     if multi:
                         assert hasattr(frm, 'base_model') or \
                             hasattr(frm, 'base_models'), \
-                            u"Must define a base_model(s) for " + \
+                            "Must define a base_model(s) for " + \
                             str(frm.__class__)
                 for frm in form.forms:
                     if not frm.is_valid():
@@ -514,9 +514,14 @@ class Wizard(IshtarWizard):
                         frm.cleaned_data.pop('DELETE')
                     for key in frm.cleaned_data:
                         value = frm.cleaned_data[key]
-                        if value is None or value in ['']:
-                            continue
-                        if key in associated_models:
+                        dct_key = key
+                        value_is_empty = value is None or value in ['']
+                        if value_is_empty:
+                            if multi:
+                                dct_key = "__empty-" + key
+                            else:
+                                continue
+                        if not value_is_empty and key in associated_models:
                             if type(value) in (tuple, list):
                                 value = [
                                     associated_models[key].objects.get(pk=v)
@@ -526,7 +531,7 @@ class Wizard(IshtarWizard):
                                 value = associated_models[key].objects.get(
                                     pk=value)
                         if multi:
-                            vals[key] = value
+                            vals[dct_key] = value
                         else:
                             m2m.append((key, value))
                     if multi and vals:
@@ -808,7 +813,7 @@ class Wizard(IshtarWizard):
                     # not m2m -> foreign key
                     if not hasattr(related_model, 'clear'):
                         assert hasattr(model, 'MAIN_ATTR'), \
-                            u"Must define a MAIN_ATTR for " + \
+                            "Must define a MAIN_ATTR for " + \
                             str(model.__class__)
                         value[getattr(model, 'MAIN_ATTR')] = obj
 
@@ -818,11 +823,19 @@ class Wizard(IshtarWizard):
                         for old_item in old_m2ms[key]:
                             is_ok = True
                             for k in value:
+                                new_val = value[k]
+                                val_empty = False
+                                if k.startswith("__empty-"):
+                                    val_empty = True
+                                    k = k[len("__empty-"):]
                                 if not hasattr(old_item, k):
                                     continue
-                                if is_ok and getattr(old_item, k) != value[k]:
+                                old_val = getattr(old_item, k)
+                                if (val_empty and (old_val is None
+                                                   or old_val == "")
+                                        ) or old_val != new_val:
                                     is_ok = False
-                                    continue
+                                    break
                             if is_ok:
                                 my_old_item = old_item
                                 break
@@ -874,6 +887,8 @@ class Wizard(IshtarWizard):
                             if type(value[k]) in (list, tuple):
                                 m2m_values[k] = value.pop(k)
 
+                        value = dict([(k, value[k]) for k in value
+                                      if not k.startswith("__empty-")])
                         if get_or_create:
                             value, created = model.objects.get_or_create(
                                 **value)