workflows: check backoffice fields changes using identifiers (#58241)

tests/workflow/test_all.py

@@ -22,7 +22,7 @@ from pyzbar.pyzbar import decode as zbar_decode_qrcode
 from quixote import cleanup, get_publisher, get_response
 from quixote.http_request import Upload as QuixoteUpload
 
-from wcs import sessions
+from wcs import sessions, sql
 from wcs.blocks import BlockDef
 from wcs.carddef import CardDef
 from wcs.fields import (
@@ -87,6 +87,7 @@ from wcs.workflows import (
     perform_items,
 )
 
+from ..test_sql import column_exists_in_table
 from ..utilities import MockSubstitutionVariables, clean_temporary_pub, create_temporary_pub
 
 
@@ -103,9 +104,8 @@ def pytest_generate_tests(metafunc):
         metafunc.parametrize('two_pubs', ['pickle', 'sql'], indirect=True)
 
 
-@pytest.fixture
-def pub(request):
-    pub = create_temporary_pub()
+def pub_fixture(**kwargs):
+    pub = create_temporary_pub(**kwargs)
     pub.cfg['language'] = {'language': 'en'}
     pub.write_cfg()
     req = HTTPRequest(None, {'SERVER_NAME': 'example.net', 'SCRIPT_NAME': ''})
@@ -117,18 +117,19 @@ def pub(request):
     return pub
 
 
+@pytest.fixture
+def pub(request):
+    return pub_fixture()
+
+
 @pytest.fixture
 def two_pubs(request):
-    pub = create_temporary_pub(sql_mode=(request.param == 'sql'))
-    pub.cfg['language'] = {'language': 'en'}
-    pub.write_cfg()
-    req = HTTPRequest(None, {'SERVER_NAME': 'example.net', 'SCRIPT_NAME': ''})
-    req.response.filter = {}
-    req._user = None
-    pub._set_request(req)
-    req.session = sessions.BasicSession(id=1)
-    pub.set_config(req)
-    return pub
+    return pub_fixture(sql_mode=(request.param == 'sql'))
+
+
+@pytest.fixture
+def sql_pub(request):
+    return pub_fixture(sql_mode=True)
 
 
 def test_get_json_export_dict(pub):
@@ -6387,3 +6388,46 @@ def test_call_external_workflow_manual_targeting(two_pubs):
     assert carddef.data_class().get(2).data['bo0'] is None
     assert carddef.data_class().get(3).data['bo0'] is None
     assert carddef.data_class().get(4).data['bo0'] is None
+
+
+def test_form_update_after_backoffice_fields(sql_pub):
+    wf = Workflow(name='wf with backoffice field')
+    wf.backoffice_fields_formdef = WorkflowBackofficeFieldsFormDef(wf)
+    wf.backoffice_fields_formdef.fields = [
+        StringField(id='bo1', label='bo field 1', type='string'),
+    ]
+    wf.add_status('Status1')
+    wf.store()
+
+    FormDef.wipe()
+    formdef = FormDef()
+    formdef.name = 'baz'
+    formdef.fields = [
+        StringField(id='1', label='field 1', type='string'),
+    ]
+    formdef.workflow_id = wf.id
+    formdef.store()
+
+    conn, cur = sql.get_connection_and_cursor()
+    assert column_exists_in_table(cur, formdef.table_name, 'fbo1')
+
+    wf.backoffice_fields_formdef.fields = [
+        StringField(id='bo1', label='bo field 1', type='string'),
+        StringField(id='bo2', label='bo field 2', type='string'),
+    ]
+    wf.backoffice_fields_formdef.store()
+    get_response().process_after_jobs()
+    assert column_exists_in_table(cur, formdef.table_name, 'fbo2')
+
+    # remove first and add third field
+    wf.backoffice_fields_formdef.fields = [
+        StringField(id='bo2', label='bo field 2', type='string'),
+        StringField(id='bo3', label='bo field 3', type='string'),
+    ]
+    wf.backoffice_fields_formdef.store()
+    get_response().process_after_jobs()
+    assert not column_exists_in_table(cur, formdef.table_name, 'fbo1')
+    assert column_exists_in_table(cur, formdef.table_name, 'fbo3')
+
+    conn.commit()
+    cur.close()

wcs/workflows.py

@@ -522,7 +522,7 @@ class Workflow(StorableObject):
                     new_backoffice_fields = self.backoffice_fields_formdef.fields
                 except AttributeError:
                     new_backoffice_fields = []
-                if len(old_backoffice_fields) != len(new_backoffice_fields):
+                if {x.id for x in old_backoffice_fields} != {x.id for x in new_backoffice_fields}:
                     must_update = True
         elif self.backoffice_fields_formdef:
             must_update = True