From 9b012ef62e8a568b06d76eadfeb85173421be75d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20P=C3=A9ters?= <fpeters@entrouvert.com>
Date: Mon, 1 Nov 2021 12:58:50 +0100
Subject: [PATCH] misc: auto-enable geolocation when there's such an action in
 workflow (#13670)

---
 tests/admin_pages/test_form.py | 26 ----------------
 tests/test_snapshots.py        |  3 +-
 tests/workflow/test_all.py     | 55 ++++++++++++++++++++++++++++++++++
 wcs/admin/forms.py             | 45 ++++++----------------------
 wcs/backoffice/cards.py        |  6 ----
 wcs/formdef.py                 |  2 ++
 wcs/workflows.py               | 19 ++++++++++--
 7 files changed, 84 insertions(+), 72 deletions(-)

diff --git a/tests/admin_pages/test_form.py b/tests/admin_pages/test_form.py
index 81437b5a7..b36e32153 100644
--- a/tests/admin_pages/test_form.py
+++ b/tests/admin_pages/test_form.py
@@ -321,32 +321,6 @@ def test_forms_edit_publication(pub, formdef):
     assert_option_display(resp, 'Online Status', 'Inactive by date')
 
 
-def test_forms_edit_geolocation(pub, formdef):
-    create_superuser(pub)
-    create_role(pub)
-
-    app = login(get_app(pub))
-    resp = app.get('/backoffice/forms/1/')
-
-    # enable geolocation
-    resp = resp.click('Geolocation')
-    resp.forms[0]['geoloc_label'] = 'Foobar'
-    resp = resp.forms[0].submit()
-    assert resp.location == 'http://example.net/backoffice/forms/1/'
-    resp = resp.follow()
-    assert_option_display(resp, 'Geolocation', 'Enabled')
-    assert FormDef.get(formdef.id).geolocations == {'base': 'Foobar'}
-
-    # and disable it
-    resp = resp.click('Geolocation')
-    resp.forms[0]['geoloc_label'] = ''
-    resp = resp.forms[0].submit()
-    assert resp.location == 'http://example.net/backoffice/forms/1/'
-    resp = resp.follow()
-    assert_option_display(resp, 'Geolocation', 'Disabled')
-    assert FormDef.get(formdef.id).geolocations is None
-
-
 def test_form_title_change(pub):
     create_superuser(pub)
     create_role(pub)
diff --git a/tests/test_snapshots.py b/tests/test_snapshots.py
index 94e820ead..dec157926 100644
--- a/tests/test_snapshots.py
+++ b/tests/test_snapshots.py
@@ -345,7 +345,8 @@ def test_card_snapshot_browse(pub):
     resp = resp.click(href='%s/view/' % snapshot.id)
     assert 'This card model is readonly' in resp.text
     assert '<p>%s</p>' % localstrftime(snapshot.timestamp) in resp.text
-    resp = resp.click('Geolocation')
+    # check option dialogs only have a cancel button
+    resp = resp.click('User support')
     assert [x[0].name for x in resp.form.fields.values() if x[0].tag == 'button'] == ['cancel']
     assert pub.custom_view_class.count() == 0  # custom views are not restore on preview
 
diff --git a/tests/workflow/test_all.py b/tests/workflow/test_all.py
index c529247fe..4b88db58f 100644
--- a/tests/workflow/test_all.py
+++ b/tests/workflow/test_all.py
@@ -3115,6 +3115,61 @@ def test_criticality(pub):
     assert formdata.get_criticality_level_object().name == 'green'
 
 
+def test_geolocate_action_enable_geolocation(two_pubs):
+    # switch to a workflow with geolocation
+    formdef = FormDef()
+    formdef.name = 'foo'
+    formdef.fields = []
+    formdef.store()
+
+    workflow = Workflow(name='wf')
+    st1 = workflow.add_status('Status1', 'st1')
+    item = GeolocateWorkflowStatusItem()
+    item.method = 'address_string'
+    item.address_string = '{{form_var_string}}, paris, france'
+    item.parent = st1
+    st1.items.append(item)
+    workflow.store()
+
+    formdef.change_workflow(workflow)
+    assert formdef.geolocations
+
+    if two_pubs.is_using_postgresql():
+        conn, cur = sql.get_connection_and_cursor()
+        assert column_exists_in_table(cur, formdef.table_name, 'geoloc_base')
+        conn.commit()
+        cur.close()
+
+    # change to current workflow
+    workflow = Workflow(name='wf2')
+    st1 = workflow.add_status('Status1', 'st1')
+    workflow.store()
+
+    formdef = FormDef()
+    formdef.name = 'bar'
+    formdef.fields = []
+    formdef.workflow = workflow
+    formdef.store()
+    assert not formdef.geolocations
+
+    item = GeolocateWorkflowStatusItem()
+    item.method = 'address_string'
+    item.address_string = '{{form_var_string}}, paris, france'
+    item.parent = st1
+    st1.items.append(item)
+    workflow.store()
+    get_response().process_after_jobs()
+
+    formdef.refresh_from_storage()
+    assert formdef.geolocations
+
+    if two_pubs.is_using_postgresql():
+        conn, cur = sql.get_connection_and_cursor()
+        assert column_exists_in_table(cur, formdef.table_name, 'geoloc_base')
+        conn.commit()
+        cur.close()
+
+
 def test_geolocate_address(two_pubs):
     if two_pubs.is_using_postgresql():
         two_pubs.loggederror_class.wipe()
diff --git a/wcs/admin/forms.py b/wcs/admin/forms.py
index 0087d4eb3..ba5b7822f 100644
--- a/wcs/admin/forms.py
+++ b/wcs/admin/forms.py
@@ -211,7 +211,6 @@ class OptionsDirectory(Directory):
         'keywords',
         'category',
         'management',
-        'geolocations',
         'appearance',
         'templates',
         'user_support',
@@ -371,19 +370,6 @@ class OptionsDirectory(Directory):
         )
         return self.handle(form, _('Category'))
 
-    def geolocations(self):
-        form = Form(enctype='multipart/form-data')
-        geoloc_label = (self.formdef.geolocations or {}).get('base')
-        form.add(
-            StringWidget,
-            'geoloc_label',
-            title=_('Geolocation Label'),
-            value=geoloc_label,
-            size=50,
-            hint=_('Location label (empty to disable geolocation)'),
-        )
-        return self.handle(form, _('Geolocation'))
-
     def appearance(self):
         form = Form(enctype='multipart/form-data')
         form.add(
@@ -465,7 +451,6 @@ class OptionsDirectory(Directory):
                 'keywords',
                 'category_id',
                 'skip_from_360_view',
-                'geoloc_label',
                 'appearance_keywords',
                 'include_download_all_button',
                 'digest_template',
@@ -481,22 +466,16 @@ class OptionsDirectory(Directory):
                         has_error = getattr(self, 'clean_%s' % attr)(form)
                         if has_error:
                             continue
-                    if attr == 'geoloc_label':
-                        if widget.parse():
-                            self.formdef.geolocations = {'base': widget.parse()}
-                        else:
-                            self.formdef.geolocations = None
+                    new_value = widget.parse()
+                    if attr == 'digest_template':
+                        if self.formdef.default_digest_template != new_value:
+                            self.changed = True
+                            if not self.formdef.digest_templates:
+                                self.formdef.digest_templates = {}
+                            self.formdef.digest_templates['default'] = new_value
                     else:
-                        new_value = widget.parse()
-                        if attr == 'digest_template':
-                            if self.formdef.default_digest_template != new_value:
-                                self.changed = True
-                                if not self.formdef.digest_templates:
-                                    self.formdef.digest_templates = {}
-                                self.formdef.digest_templates['default'] = new_value
-                        else:
-                            if getattr(self.formdef, attr, None) != new_value:
-                                setattr(self.formdef, attr, new_value)
+                        if getattr(self.formdef, attr, None) != new_value:
+                            setattr(self.formdef, attr, new_value)
             if not form.has_errors():
                 self.formdef.store(comment=_('Changed "%s" parameters') % title)
                 return redirect('..')
@@ -800,12 +779,6 @@ class FormDefPage(Directory):
             or C_('tracking code|Disabled'),
         )
 
-        r += self.add_option_line(
-            'options/geolocations',
-            _('Geolocation'),
-            self.formdef.geolocations and C_('geolocation|Enabled') or C_('geolocation|Disabled'),
-        )
-
         if get_publisher().has_site_option('formdef-captcha-option'):
             r += self.add_option_line(
                 'options/captcha',
diff --git a/wcs/backoffice/cards.py b/wcs/backoffice/cards.py
index e2b087187..8d7126f9d 100644
--- a/wcs/backoffice/cards.py
+++ b/wcs/backoffice/cards.py
@@ -157,12 +157,6 @@ class CardDefPage(FormDefPage):
         r += htmltext('<h3>%s</h3>') % _('Options')
         r += htmltext('<ul class="biglist optionslist">')
 
-        r += self.add_option_line(
-            'options/geolocations',
-            _('Geolocation'),
-            self.formdef.geolocations and C_('geolocation|Enabled') or C_('geolocation|Disabled'),
-        )
-
         if (
             self.formdef.default_digest_template
             or self.formdef.lateral_template
diff --git a/wcs/formdef.py b/wcs/formdef.py
index 8cd203b06..c0580fd0d 100644
--- a/wcs/formdef.py
+++ b/wcs/formdef.py
@@ -1721,6 +1721,8 @@ class FormDef(StorableObject):
                         formdata.store()
 
         self.workflow = new_workflow
+        if new_workflow.has_action('geolocate') and not self.geolocations:
+            self.geolocations = {'base': str(_('Geolocation'))}
         self.store(comment=_('Workflow change'))
         if formdata_count:
             # instruct formdef to update its security rules
diff --git a/wcs/workflows.py b/wcs/workflows.py
index ca3961097..8f13b34a7 100644
--- a/wcs/workflows.py
+++ b/wcs/workflows.py
@@ -505,6 +505,7 @@ class Workflow(StorableObject):
     def store(self, comment=None, *args, **kwargs):
         assert not self.is_readonly()
         must_update = False
+        has_geolocation = False
         if self.id:
             old_self = self.get(self.id, ignore_errors=True, ignore_migration=True)
             if old_self:
@@ -524,6 +525,12 @@ class Workflow(StorableObject):
                     new_backoffice_fields = []
                 if {x.id for x in old_backoffice_fields} != {x.id for x in new_backoffice_fields}:
                     must_update = True
+
+                # if a geolocation action has been added tables may have to be updated
+                if self.has_action('geolocate') and not old_self.has_action('geolocate'):
+                    must_update = True
+                    has_geolocation = True
+
         elif self.backoffice_fields_formdef:
             must_update = True
 
@@ -533,13 +540,16 @@ class Workflow(StorableObject):
 
         def update(job=None):
             # instruct all related carddefs/formdefs to update.
-            for form in itertools.chain(
+            for formdef in itertools.chain(
                 self.formdefs(ignore_migration=True, order_by='id'),
                 self.carddefs(ignore_migration=True, order_by='id'),
             ):
                 if must_update:
-                    form.update_storage()
-                form.data_class().rebuild_security()
+                    if has_geolocation and not formdef.geolocations:
+                        formdef.geolocations = {'base': str(_('Geolocation'))}
+                        formdef.store(comment=_('Geolocation enabled by workflow'))
+                    formdef.update_storage()
+                formdef.data_class().rebuild_security()
 
         if get_response():
             get_response().add_after_job(_('Reindexing cards and forms after workflow change'), update)
@@ -595,6 +605,9 @@ class Workflow(StorableObject):
         for action in self.global_actions or []:
             yield from action.items or []
 
+    def has_action(self, action_type):
+        return any(x.key == action_type for x in self.get_all_items())
+
     def add_global_action(self, name, id=None):
         if [x for x in self.global_actions if x.name == name]:
             raise DuplicateGlobalActionNameError()
-- 
2.33.1

