diff --git a/tests/backoffice_pages/test_all.py b/tests/backoffice_pages/test_all.py index da5f26d1f..6723a8ef0 100644 --- a/tests/backoffice_pages/test_all.py +++ b/tests/backoffice_pages/test_all.py @@ -4447,6 +4447,16 @@ def test_inspect_page(pub, local_user): assert 'form_user_f3' not in resp.text assert 'form_user_f_' not in resp.text + # make sure workflow data itself is not displayed, as it's available in + # expanded variables + assert not pq('[title="form_workflow_data"]') + + # but do display it if it has some invalid contents + formdata.workflow_data['invalid key'] = 'foobar' + formdata.store() + resp = login(get_app(pub)).get(resp.request.url) + assert resp.pyquery('[title="form_workflow_data"]') + # check functions assert re.findall('Recipient.*foobar', resp.text) diff --git a/tests/test_formdata.py b/tests/test_formdata.py index 204b38efb..00bd97048 100644 --- a/tests/test_formdata.py +++ b/tests/test_formdata.py @@ -546,6 +546,24 @@ def test_workflow_data_file_url(pub, formdef): assert substvars['foo_var_file_url'] +def test_workflow_data_invalid_keys(pub, formdef): + formdata = formdef.data_class()() + formdata.store() + formdata.workflow_data = { + 'valid_key': {'invalid key': 'foo', 'valid_key': 'bar'}, + 'invalid key': 'baz', + } + substvars = formdata.get_substitution_variables() + assert 'form_workflow_data_valid_key' in substvars + assert 'form_workflow_data_invalid key' not in substvars + assert 'form_workflow_data_valid_key_valid_key' in substvars + assert 'form_workflow_data_valid_key_invalid key' not in substvars + assert substvars['form_workflow_data_valid_key_valid_key'] == 'bar' + with pytest.raises(KeyError): + # noqa pylint: disable=pointless-statement + substvars['form_workflow_data_invalid key'] + + def test_evolution_get_status(pub): Workflow.wipe() workflow = Workflow(name='test') diff --git a/wcs/backoffice/management.py b/wcs/backoffice/management.py index 198bc5bac..3f52e536a 100644 --- a/wcs/backoffice/management.py +++ b/wcs/backoffice/management.py @@ -3279,10 +3279,16 @@ class FormBackOfficeStatusPage(FormStatusPage): 'inspect_url': v['form'].backoffice_url + 'inspect', 'display_name': v['form_display_name'], } - elif hasattr(v, 'inspect_keys') or isinstance(v, dict): - # skip expanded + elif hasattr(v, 'inspect_keys'): + # skip as there are expanded identifiers continue else: + if isinstance(v, dict): + # only display dictionaries if they have invalid keys + # (otherwise the expanded identifiers are a better way + # to get to the values). + if all(CompatibilityNamesDict.valid_key_regex.match(k) for k in v): + continue r += htmltext('
  • %s') % (k, k) r += htmltext('
    %s') % ellipsize(safe(v), 10000) if not isinstance(v, str): diff --git a/wcs/formdata.py b/wcs/formdata.py index f0065777d..8fea86fac 100644 --- a/wcs/formdata.py +++ b/wcs/formdata.py @@ -29,7 +29,7 @@ from .qommon import N_, _, misc from .qommon.evalutils import make_datetime from .qommon.publisher import get_cfg from .qommon.storage import Contains, Intersects, Null, StorableObject -from .qommon.substitution import Substitutions, invalidate_substitution_cache +from .qommon.substitution import CompatibilityNamesDict, Substitutions, invalidate_substitution_cache from .qommon.template import Template @@ -908,8 +908,6 @@ class FormData(StorableObject): def get_substitution_variables(self, minimal=False): from wcs.workflows import AttachmentsSubstitutionProxy - from .qommon.substitution import CompatibilityNamesDict - variables = CompatibilityNamesDict( { 'form': self.get_as_lazy(), @@ -949,7 +947,7 @@ class FormData(StorableObject): d = copy.deepcopy(d) flatten_dict(d) - variables.update(d) + variables.update({k: v for k, v in d.items() if CompatibilityNamesDict.valid_key_regex.match(k)}) return variables diff --git a/wcs/qommon/substitution.py b/wcs/qommon/substitution.py index 09aea0e0c..d06ca95d0 100644 --- a/wcs/qommon/substitution.py +++ b/wcs/qommon/substitution.py @@ -14,6 +14,7 @@ # You should have received a copy of the GNU General Public License # along with this program; if not, see . +import re from contextlib import contextmanager from quixote import get_publisher @@ -161,6 +162,9 @@ class Substitutions: class CompatibilityNamesDict(dict): # custom dictionary that provides automatic fallback to legacy variable # names (namespaced with underscores) + + valid_key_regex = re.compile(r'^[a-zA-Z][a-zA-Z0-9_]*$') + def get(self, key, default=None): try: return self.__getitem__(key) @@ -178,7 +182,7 @@ class CompatibilityNamesDict(dict): if hasattr(item, 'inspect_keys'): sub_keys = item.inspect_keys() elif isinstance(item, dict): - sub_keys = item.keys() + sub_keys = [x for x in item.keys() if self.valid_key_regex.match(x)] else: return for sub_key in sub_keys: @@ -226,6 +230,8 @@ class CompatibilityNamesDict(dict): raise KeyError def __getitem__(self, key): + if not self.valid_key_regex.match(key): + raise KeyError(key) parts = key.split('_') try: return self.get_path(self, parts)