From c49e8ebf1563f633da9d24189dcd0657c5255f61 Mon Sep 17 00:00:00 2001 From: Matthew Foster Date: Fri, 20 Aug 2021 13:25:59 -0700 Subject: [PATCH 01/12] add composite fields separator config setting --- ckanext/scheming/helpers.py | 3 +++ ckanext/scheming/plugins.py | 9 +++++---- .../scheming/form_snippets/repeating_subfields.html | 2 +- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/ckanext/scheming/helpers.py b/ckanext/scheming/helpers.py index f118c747..f9dd922e 100644 --- a/ckanext/scheming/helpers.py +++ b/ckanext/scheming/helpers.py @@ -27,6 +27,9 @@ def lang(): from ckantoolkit import h return h.lang() +@helper +def scheming_composite_separator(): + return config.get('scheming.composite.separator','|') @helper def scheming_language_text(text, prefer_lang=None): diff --git a/ckanext/scheming/plugins.py b/ckanext/scheming/plugins.py index 7c18cdd1..ce19a9d1 100644 --- a/ckanext/scheming/plugins.py +++ b/ckanext/scheming/plugins.py @@ -331,15 +331,16 @@ def expand_form_composite(data, fieldnames): when submitting dataset/resource form composite fields look like "field-0-subfield..." convert these to lists of dicts """ + sep = p.toolkit.h.scheming_composite_separator() # if "field" exists, don't look for "field-0-subfield" fieldnames -= set(data) if not fieldnames: return indexes = {} for key in sorted(data): - if '-' not in key: + if sep not in key: continue - parts = key.split('-') + parts = key.split(sep) if parts[0] not in fieldnames: continue if parts[1] not in indexes: @@ -348,11 +349,11 @@ def expand_form_composite(data, fieldnames): parts[1] = indexes[parts[1]] try: try: - comp[int(parts[1])]['-'.join(parts[2:])] = data[key] + comp[int(parts[1])][sep.join(parts[2:])] = data[key] del data[key] except IndexError: comp.append({}) - comp[int(parts[1])]['-'.join(parts[2:])] = data[key] + comp[int(parts[1])][sep.join(parts[2:])] = data[key] del data[key] except (IndexError, ValueError): pass # best-effort only diff --git a/ckanext/scheming/templates/scheming/form_snippets/repeating_subfields.html b/ckanext/scheming/templates/scheming/form_snippets/repeating_subfields.html index 0107a75a..43e973a2 100644 --- a/ckanext/scheming/templates/scheming/form_snippets/repeating_subfields.html +++ b/ckanext/scheming/templates/scheming/form_snippets/repeating_subfields.html @@ -17,7 +17,7 @@ {% for subfield in field.repeating_subfields %} {% set sf = dict( subfield, - field_name=field.field_name ~ '-' ~ index ~ '-' ~ subfield.field_name) + field_name=field.field_name ~ h.scheming_composite_separator() ~ index ~ h.scheming_composite_separator() ~ subfield.field_name) %} {%- snippet 'scheming/snippets/form_field.html', field=sf, From 3c7bb747e34f49574ec7f99235d592e7e79195a5 Mon Sep 17 00:00:00 2001 From: Matthew Foster Date: Wed, 25 Aug 2021 12:47:32 -0700 Subject: [PATCH 02/12] update flatten subfields --- ckanext/scheming/helpers.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ckanext/scheming/helpers.py b/ckanext/scheming/helpers.py index f9dd922e..ad6f0450 100644 --- a/ckanext/scheming/helpers.py +++ b/ckanext/scheming/helpers.py @@ -424,14 +424,16 @@ def scheming_flatten_subfield(subfield, data): after a validation error) then they are returned as-is. """ flat = dict(data) + sep = toolkit.h.scheming_composite_separator() if subfield['field_name'] not in data: return flat for i, record in enumerate(data[subfield['field_name']]): - prefix = '{field_name}-{index}-'.format( + prefix = '{field_name}{sep}{index}{sep}'.format( field_name=subfield['field_name'], index=i, + sep=sep, ) for k in record: flat[prefix + k] = record[k] From 967baf07006369fca8165953aa1e47f7f705a9e0 Mon Sep 17 00:00:00 2001 From: Matthew Foster Date: Wed, 25 Aug 2021 15:50:48 -0700 Subject: [PATCH 03/12] add support for simple_subfields --- ckanext/scheming/helpers.py | 24 +++++++ ckanext/scheming/plugins.py | 37 +++++++++- .../form_snippets/simple_subfields.html | 69 +++++++++++++++++++ .../scheming/snippets/form_field.html | 2 +- 4 files changed, 130 insertions(+), 2 deletions(-) create mode 100644 ckanext/scheming/templates/scheming/form_snippets/simple_subfields.html diff --git a/ckanext/scheming/helpers.py b/ckanext/scheming/helpers.py index ad6f0450..18bd91b2 100644 --- a/ckanext/scheming/helpers.py +++ b/ckanext/scheming/helpers.py @@ -438,3 +438,27 @@ def scheming_flatten_subfield(subfield, data): for k in record: flat[prefix + k] = record[k] return flat + +@helper +def scheming_flatten_simple_subfield(subfield, data): + """ + Return flattened_data that converts all nested data for this subfield + into {field_name}-{subfield_name} values at the top level, + so that it matches the names of form fields submitted. + + If data already contains flattened subfields (e.g. rendering values + after a validation error) then they are returned as-is. + """ + flat = dict(data) + sep = toolkit.h.scheming_composite_separator() + + if subfield['field_name'] not in data: + return flat + + for i, record in enumerate(data[subfield['field_name']]): + prefix = '{field_name}{sep}'.format( + field_name=subfield['field_name'], + sep=sep, + ) + flat[prefix + record] = data[subfield['field_name']][record] + return flat diff --git a/ckanext/scheming/plugins.py b/ckanext/scheming/plugins.py index ce19a9d1..a4a45b25 100644 --- a/ckanext/scheming/plugins.py +++ b/ckanext/scheming/plugins.py @@ -262,7 +262,7 @@ def validate(self, context, data_dict, schema, action): scheming_schema, convert_this ) - if convert_this and 'repeating_subfields' in f: + if convert_this and ('repeating_subfields' in f or 'simple_subfields' in f): composite_convert_fields.append(f['field_name']) def composite_convert_to(key, data, errors, context): @@ -284,6 +284,14 @@ def composite_convert_to(key, data, errors, context): if ex['key'] not in composite_convert_fields ] else: + dataset_simple_composite = { + f['field_name'] + for f in scheming_schema['dataset_fields'] + if 'simple_subfields' in f + } + if dataset_simple_composite: + expand_form_simple_composite(data_dict, dataset_simple_composite) + dataset_composite = { f['field_name'] for f in scheming_schema['dataset_fields'] @@ -358,7 +366,34 @@ def expand_form_composite(data, fieldnames): except (IndexError, ValueError): pass # best-effort only +def expand_form_simple_composite(data, fieldnames): + """ + when submitting dataset/resource form composite fields look like + "field-subfield..." convert these to a dicts + """ + sep = p.toolkit.h.scheming_composite_separator() + # if "field" exists, don't look for "field-0-subfield" + fieldnames -= set(data) + if not fieldnames: + return + for key in sorted(data): + if sep not in key: + continue + parts = key.split(sep) + if parts[0] not in fieldnames: + continue + comp = data.setdefault(parts[0], {}) + try: + try: + comp[sep.join(parts[1:])] = data[key] + del data[key] + except IndexError: + comp = {} + comp[sep.join(parts[1:])] = data[key] + del data[key] + except (IndexError, ValueError): + pass # best-effort only class SchemingGroupsPlugin(p.SingletonPlugin, _GroupOrganizationMixin, DefaultGroupForm, _SchemingMixin): diff --git a/ckanext/scheming/templates/scheming/form_snippets/simple_subfields.html b/ckanext/scheming/templates/scheming/form_snippets/simple_subfields.html new file mode 100644 index 00000000..2d4f6c6e --- /dev/null +++ b/ckanext/scheming/templates/scheming/form_snippets/simple_subfields.html @@ -0,0 +1,69 @@ +{# A complex field with simple sub-fields #} + + +{% import 'macros/form.html' as form %} +{% include 'scheming/snippets/subfields_asset.html' %} +{% macro repeating_panel(index, index1) %} +
+
+
+ {% for subfield in field.simple_subfields %} + {% set sf = dict( + subfield, + field_name=field.field_name ~ h.scheming_composite_separator() ~ subfield.field_name) + %} + {%- snippet 'scheming/snippets/form_field.html', + field=sf, + data=flat, + errors=flaterr, + licenses=licenses, + entity_type=entity_type, + object_type=object_type + -%} + {% endfor %} +
+ +
+
+{% endmacro %} + +{% set flat = h.scheming_flatten_simple_subfield(field, data) %} +{% set flaterr = h.scheming_flatten_simple_subfield(field, errors) %} + +{% call form.input_block( + 'field-' + field.field_name, + h.scheming_language_text(field.label) or field.field_name, + [], + field.classes if 'classes' in field else ['control-medium'], + dict({"class": "form-control"}, **(field.get('form_attrs', {}))), + is_required=h.scheming_field_required(field)) %} +
+ {% set alert_warning = h.scheming_language_text(field.form_alert_warning) %} + {% if alert_warning %} +
+ {{ alert_warning|safe }} +
+ {% endif %} + +
+ {{ repeating_panel(0, 1) }} +
+
+ {% set help_text = h.scheming_language_text(field.help_text) %} + {% if help_text %} +
+ {{ help_text }} +
+ {% endif %} +
+ +
+{% endcall %} diff --git a/ckanext/scheming/templates/scheming/snippets/form_field.html b/ckanext/scheming/templates/scheming/snippets/form_field.html index 8353f6e2..eaf986d8 100644 --- a/ckanext/scheming/templates/scheming/snippets/form_field.html +++ b/ckanext/scheming/templates/scheming/snippets/form_field.html @@ -1,7 +1,7 @@ {#- master snippet for all scheming form fields -#} {#- render the field the user requested, or use a default field -#} {%- set form_snippet = field.form_snippet|default( - 'repeating_subfields.html' if field.repeating_subfields else 'text.html') -%} + 'repeating_subfields.html' if field.repeating_subfields else 'simple_subfields.html' if field.simple_subfields else 'text.html') -%} {%- if '/' not in form_snippet -%} {%- set form_snippet = 'scheming/form_snippets/' + form_snippet -%} From 75062c42abf219332b4df51bb41452ac3055c60d Mon Sep 17 00:00:00 2001 From: Matthew Foster Date: Thu, 26 Aug 2021 11:01:57 -0700 Subject: [PATCH 04/12] add display fields for simple subfields --- .../display_snippets/simple_subfields.html | 22 +++++++++++++++++++ .../scheming/snippets/display_field.html | 2 ++ 2 files changed, 24 insertions(+) create mode 100644 ckanext/scheming/templates/scheming/display_snippets/simple_subfields.html diff --git a/ckanext/scheming/templates/scheming/display_snippets/simple_subfields.html b/ckanext/scheming/templates/scheming/display_snippets/simple_subfields.html new file mode 100644 index 00000000..59d15071 --- /dev/null +++ b/ckanext/scheming/templates/scheming/display_snippets/simple_subfields.html @@ -0,0 +1,22 @@ +{% set fields = data[field.field_name] %} +{% block subfield_display %} +
+
+
+ {% for subfield in field.simple_subfields %} +
+ {{ h.scheming_language_text(subfield.label) }} +
+
+ {%- snippet 'scheming/snippets/display_field.html', + field=subfield, + data=fields, + entity_type=entity_type, + object_type=object_type + -%} +
+ {% endfor %} +
+
+
+{% endblock %} diff --git a/ckanext/scheming/templates/scheming/snippets/display_field.html b/ckanext/scheming/templates/scheming/snippets/display_field.html index 1ef4d0d9..189f99c7 100644 --- a/ckanext/scheming/templates/scheming/snippets/display_field.html +++ b/ckanext/scheming/templates/scheming/snippets/display_field.html @@ -5,6 +5,8 @@ {%- if not display_snippet -%} {%- if field.repeating_subfields -%} {%- set display_snippet = 'repeating_subfields.html' -%} + {%- elif field.simple_subfields -%} + {%- set display_snippet = 'simple_subfields.html' -%} {%- elif h.scheming_field_choices(field) -%} {%- set display_snippet = 'select.html' -%} {%- else -%} From 3a59dc93d8be58d2c7758471da9aeada510df2ad Mon Sep 17 00:00:00 2001 From: Matthew Foster Date: Fri, 27 Aug 2021 11:38:34 -0700 Subject: [PATCH 05/12] add tests and adjust code to fix bugs --- ckanext/scheming/helpers.py | 11 ++-- ckanext/scheming/plugins.py | 33 ++++++++---- .../templates/scheming/snippets/errors.html | 32 +++++++++++ ckanext/scheming/tests/test_form.py | 52 ++++++++++++++++++ ckanext/scheming/tests/test_subfields.yaml | 12 +++++ ckanext/scheming/tests/test_validation.py | 53 +++++++++++++++++++ 6 files changed, 180 insertions(+), 13 deletions(-) diff --git a/ckanext/scheming/helpers.py b/ckanext/scheming/helpers.py index 18bd91b2..9ffbff32 100644 --- a/ckanext/scheming/helpers.py +++ b/ckanext/scheming/helpers.py @@ -29,7 +29,7 @@ def lang(): @helper def scheming_composite_separator(): - return config.get('scheming.composite.separator','|') + return config.get('scheming.composite.separator','-') @helper def scheming_language_text(text, prefer_lang=None): @@ -423,8 +423,9 @@ def scheming_flatten_subfield(subfield, data): If data already contains flattened subfields (e.g. rendering values after a validation error) then they are returned as-is. """ + from ckantoolkit import h flat = dict(data) - sep = toolkit.h.scheming_composite_separator() + sep = h.scheming_composite_separator() if subfield['field_name'] not in data: return flat @@ -449,8 +450,9 @@ def scheming_flatten_simple_subfield(subfield, data): If data already contains flattened subfields (e.g. rendering values after a validation error) then they are returned as-is. """ + from ckantoolkit import h flat = dict(data) - sep = toolkit.h.scheming_composite_separator() + sep = h.scheming_composite_separator() if subfield['field_name'] not in data: return flat @@ -460,5 +462,6 @@ def scheming_flatten_simple_subfield(subfield, data): field_name=subfield['field_name'], sep=sep, ) - flat[prefix + record] = data[subfield['field_name']][record] + for k in record: + flat[prefix + k] = record[k] return flat diff --git a/ckanext/scheming/plugins.py b/ckanext/scheming/plugins.py index a4a45b25..ccc26c4d 100644 --- a/ckanext/scheming/plugins.py +++ b/ckanext/scheming/plugins.py @@ -369,7 +369,7 @@ def expand_form_composite(data, fieldnames): def expand_form_simple_composite(data, fieldnames): """ when submitting dataset/resource form composite fields look like - "field-subfield..." convert these to a dicts + "field-subfield..." convert these to lists of dicts """ sep = p.toolkit.h.scheming_composite_separator() # if "field" exists, don't look for "field-0-subfield" @@ -383,14 +383,14 @@ def expand_form_simple_composite(data, fieldnames): parts = key.split(sep) if parts[0] not in fieldnames: continue - comp = data.setdefault(parts[0], {}) + comp = data.setdefault(parts[0], []) try: try: - comp[sep.join(parts[1:])] = data[key] + comp[0][sep.join(parts[1:])] = data[key] del data[key] except IndexError: - comp = {} - comp[sep.join(parts[1:])] = data[key] + comp.append({}) + comp[0][sep.join(parts[1:])] = data[key] del data[key] except (IndexError, ValueError): pass # best-effort only @@ -477,7 +477,7 @@ def before_index(self, data_dict): for d in schemas[data_dict['type']]['dataset_fields']: if d['field_name'] not in data_dict: continue - if 'repeating_subfields' in d: + if 'repeating_subfields' in d or 'simple_subfields' in d: data_dict[d['field_name']] = json.dumps(data_dict[d['field_name']]) return data_dict @@ -548,7 +548,12 @@ def _field_output_validators(f, schema, convert_extras, """ Return the output validators for a scheming field f """ - if 'repeating_subfields' in f: + if 'simple_subfields' in f: + validators = { + sf['field_name']: _field_output_validators(sf, schema, False) + for sf in f['simple_subfields'] + } + elif 'repeating_subfields' in f: validators = { sf['field_name']: _field_output_validators(sf, schema, False) for sf in f['repeating_subfields'] @@ -583,7 +588,12 @@ def _field_validators(f, schema, convert_extras): # If this field contains children, we need a special validator to handle # them. - if 'repeating_subfields' in f: + if 'simple_subfields' in f: + validators = { + sf['field_name']: _field_validators(sf, schema, False) + for sf in f['simple_subfields'] + } + elif 'repeating_subfields' in f: validators = { sf['field_name']: _field_validators(sf, schema, False) for sf in f['repeating_subfields'] @@ -611,7 +621,12 @@ def _field_create_validators(f, schema, convert_extras): # If this field contains children, we need a special validator to handle # them. - if 'repeating_subfields' in f: + if 'simple_subfields' in f: + validators = { + sf['field_name']: _field_create_validators(sf, schema, False) + for sf in f['simple_subfields'] + } + elif 'repeating_subfields' in f: validators = { sf['field_name']: _field_create_validators(sf, schema, False) for sf in f['repeating_subfields'] diff --git a/ckanext/scheming/templates/scheming/snippets/errors.html b/ckanext/scheming/templates/scheming/snippets/errors.html index a8298b27..23753bfe 100644 --- a/ckanext/scheming/templates/scheming/snippets/errors.html +++ b/ckanext/scheming/templates/scheming/snippets/errors.html @@ -54,6 +54,38 @@ {%- endif -%} {%- endfor -%} + {%- elif 'simple_subfields' in field %} + {%- for se in errors -%} + {%- if se -%} +
  • {{ + h.scheming_language_text(field.label) }}: +
      + {%- for sf in field.simple_subfields -%} + {%- set se_unprocessed = se.copy() -%} + + {%- if 'error_snippet' in sf -%} + {%- set sfe_snippet = sf.error_snippet -%} + + {%- if '/' not in sfe_snippet -%} + {%- set sfe_snippet = 'scheming/error_snippets/' + + sfe_snippet -%} + {%- endif -%} + + {%- snippet sfe_snippet, unprocessed=se_unprocessed, + field=sf, fields=field.simple_subfileds, + entity_type=entity_type, object_type=object_type -%} + {%- endif -%} + + {%- if sf.field_name in se_unprocessed -%} +
    • {{ + h.scheming_language_text(sf.label) }}: + {{ se_unprocessed[sf.field_name][0] }}
      • + {%- endif -%} + {%- endfor -%} +
    +
    • + {%- endif -%} + {%- endfor -%} {%- else -%}
  • {{ h.scheming_language_text(field.label) }}: diff --git a/ckanext/scheming/tests/test_form.py b/ckanext/scheming/tests/test_form.py index 8b11ed3d..8263fb56 100644 --- a/ckanext/scheming/tests/test_form.py +++ b/ckanext/scheming/tests/test_form.py @@ -379,6 +379,58 @@ def test_dataset_form_update(self, app): assert dataset["contact_address"] == [{'address': 'home'}] +@pytest.mark.usefixtures("clean_db") +class TestSimpleSubfieldDatasetForm(object): + def test_dataset_form_includes_simple_subfields(self, app): + env, response = _get_package_new_page_as_sysadmin(app, 'test-subfields') + form = BeautifulSoup(response.body).select("form")[1] + assert form.select("fieldset[name=scheming-simple-subfields]") + + def test_dataset_form_create_simple_subfields(self, app, sysadmin_env): + data = {"save": "", "_ckan_phase": 1} + + data["name"] = "subfield_dataset_1" + data["temporal_extent-begin"] = '2000-01-23' + data["temporal_extent-end"] = '2021-12-30' + + url = '/test-subfields/new' + try: + app.post(url, environ_overrides=sysadmin_env, data=data, follow_redirects=False) + except TypeError: + app.post(url.encode('ascii'), params=data, extra_environ=sysadmin_env) + + dataset = call_action("package_show", id="subfield_dataset_1") + # TODO: I think the output of package_show for a simple_subfield should be + # a dict not as list but can't seem to find where to change + assert dataset["temporal_extent"] == [{'begin': '2000-01-23', 'end': '2021-12-30'}] + + def test_dataset_form_update_simple_subfield(self, app): + dataset = Dataset( + type="test-subfields", + temporal_extent=[{'begin': '2000-01-23', 'end': '2021-12-30'}]) + + env, response = _get_package_update_page_as_sysadmin( + app, dataset["id"] + ) + form = BeautifulSoup(response.body).select_one("#dataset-edit") + assert form.select_one( + "input[name=temporal_extent-begin]" + ).attrs['value'] == '2000-01-23' + + data = {"save": ""} + data["temporal_extent-begin"] = '1989-04-13' + data["temporal_extent-end"] = '1995-05-15' + data["name"] = dataset["name"] + + url = '/test-subfields/edit/' + dataset["id"] + try: + app.post(url, environ_overrides=env, data=data, follow_redirects=False) + except TypeError: + app.post(url.encode('ascii'), params=data, extra_environ=env) + + dataset = call_action("package_show", id=dataset["id"]) + + assert dataset["temporal_extent"] == [{'begin': '1989-04-13', 'end': '1995-05-15'}] @pytest.mark.usefixtures("clean_db") class TestSubfieldResourceForm(object): diff --git a/ckanext/scheming/tests/test_subfields.yaml b/ckanext/scheming/tests/test_subfields.yaml index f4eaa78d..6490b985 100644 --- a/ckanext/scheming/tests/test_subfields.yaml +++ b/ckanext/scheming/tests/test_subfields.yaml @@ -44,6 +44,18 @@ dataset_fields: - field_name: country label: Country +- field_name: temporal_extent + label: Temporal Extent + simple_subfields: + - field_name: begin + label: Begin + required: true + preset: date + form_placeholder: yyyy-mm-dd + - field_name: end + label: End + preset: date + form_placeholder: yyyy-mm-dd resource_fields: diff --git a/ckanext/scheming/tests/test_validation.py b/ckanext/scheming/tests/test_validation.py index 675ec0ae..15e8aa28 100644 --- a/ckanext/scheming/tests/test_validation.py +++ b/ckanext/scheming/tests/test_validation.py @@ -898,6 +898,59 @@ def test_invalid_bad_date_subfield(self): raise AssertionError("ValidationError not raised") + +@pytest.mark.usefixtures("clean_db") +class TestSimpleSubfieldDatasetValid(object): + def test_valid_simple_subfields(self): + lc = LocalCKAN() + dataset = lc.action.package_create( + type="test-subfields", + name="a_sf_1", + temporal_extent=[{'begin': '2000-01-23', 'end': ''}] + ) + + assert dataset["temporal_extent"] == [{'begin': '2000-01-23'}] + + def test_empty_simple_subfields(self): + lc = LocalCKAN() + dataset = lc.action.package_create( + type="test-subfields", + name="a_sf_1", + temporal_extent=[], + ) + + assert "temporal_extent" not in dataset + +@pytest.mark.usefixtures("clean_db") +class TestSimpleSubfieldDatasetInvalid(object): + def test_invalid_missing_required_simple_subfield(self): + lc = LocalCKAN() + + try: + lc.action.package_create( + type="test-subfields", + name="b_sf_1", + temporal_extent=[{'begin': '', 'end': '2000-01-23'}] + ) + except ValidationError as e: + assert e.error_dict["temporal_extent"][0]["begin"] == ["Missing value"] + else: + raise AssertionError("ValidationError not raised") + + def test_invalid_bad_date_subfield(self): + lc = LocalCKAN() + + try: + lc.action.package_create( + type="test-subfields", + name="b_sf_1", + temporal_extent=[{'begin': '2000-01-23', 'end': 'THEN'}] + ) + except ValidationError as e: + assert e.error_dict["temporal_extent"][0]["end"] == ["Date format incorrect"] + else: + raise AssertionError("ValidationError not raised") + @pytest.mark.usefixtures("clean_db") class TestSubfieldResourceValid(object): def test_simple(self): From ce06bab6a29d080c1dd5c5aefd64de7c43808fbb Mon Sep 17 00:00:00 2001 From: Matthew Foster Date: Wed, 1 Sep 2021 14:57:15 -0700 Subject: [PATCH 06/12] output simple subfields as dictionaries instead of lists --- ckanext/scheming/helpers.py | 5 ++-- ckanext/scheming/plugins.py | 28 ++++++++++++++++++++++- ckanext/scheming/tests/test_form.py | 6 ++--- ckanext/scheming/tests/test_validation.py | 2 +- 4 files changed, 32 insertions(+), 9 deletions(-) diff --git a/ckanext/scheming/helpers.py b/ckanext/scheming/helpers.py index 9ffbff32..5a14c68e 100644 --- a/ckanext/scheming/helpers.py +++ b/ckanext/scheming/helpers.py @@ -457,11 +457,10 @@ def scheming_flatten_simple_subfield(subfield, data): if subfield['field_name'] not in data: return flat - for i, record in enumerate(data[subfield['field_name']]): + for field, value in data[subfield['field_name']].items(): prefix = '{field_name}{sep}'.format( field_name=subfield['field_name'], sep=sep, ) - for k in record: - flat[prefix + k] = record[k] + flat[prefix + field] = value return flat diff --git a/ckanext/scheming/plugins.py b/ckanext/scheming/plugins.py index ccc26c4d..afc896d1 100644 --- a/ckanext/scheming/plugins.py +++ b/ckanext/scheming/plugins.py @@ -372,8 +372,8 @@ def expand_form_simple_composite(data, fieldnames): "field-subfield..." convert these to lists of dicts """ sep = p.toolkit.h.scheming_composite_separator() - # if "field" exists, don't look for "field-0-subfield" + # if "field" exists, don't look for "field-subfield" fieldnames -= set(data) if not fieldnames: return @@ -477,11 +477,37 @@ def before_index(self, data_dict): for d in schemas[data_dict['type']]['dataset_fields']: if d['field_name'] not in data_dict: continue + if 'simple_subfields' in d and isinstance(data_dict[d['field_name']], list): + data_dict[d['field_name']] = data_dict[d['field_name']][0] if 'repeating_subfields' in d or 'simple_subfields' in d: data_dict[d['field_name']] = json.dumps(data_dict[d['field_name']]) return data_dict + def after_search(self, search_results, search_params): + if 'results' not in search_results: + return search_results + schemas = SchemingDatasetsPlugin.instance._expanded_schemas + for data_dict in search_results['results']: + if data_dict['type'] not in schemas: + continue + for d in schemas[data_dict['type']]['dataset_fields']: + if d['field_name'] not in data_dict: + continue + if 'simple_subfields' in d and isinstance(data_dict[d['field_name']], list): + data_dict[d['field_name']] = data_dict[d['field_name']][0] + return search_results + + def after_show(self, context, data_dict): + schemas = SchemingDatasetsPlugin.instance._expanded_schemas + if data_dict['type'] not in schemas: + return data_dict + for d in schemas[data_dict['type']]['dataset_fields']: + if d['field_name'] not in data_dict: + continue + if 'simple_subfields' in d and isinstance(data_dict[d['field_name']], list): + data_dict[d['field_name']] = data_dict[d['field_name']][0] + return data_dict def _load_schemas(schemas, type_field): out = {} diff --git a/ckanext/scheming/tests/test_form.py b/ckanext/scheming/tests/test_form.py index 8263fb56..524377d6 100644 --- a/ckanext/scheming/tests/test_form.py +++ b/ckanext/scheming/tests/test_form.py @@ -400,9 +400,7 @@ def test_dataset_form_create_simple_subfields(self, app, sysadmin_env): app.post(url.encode('ascii'), params=data, extra_environ=sysadmin_env) dataset = call_action("package_show", id="subfield_dataset_1") - # TODO: I think the output of package_show for a simple_subfield should be - # a dict not as list but can't seem to find where to change - assert dataset["temporal_extent"] == [{'begin': '2000-01-23', 'end': '2021-12-30'}] + assert dataset["temporal_extent"] == {'begin': '2000-01-23', 'end': '2021-12-30'} def test_dataset_form_update_simple_subfield(self, app): dataset = Dataset( @@ -430,7 +428,7 @@ def test_dataset_form_update_simple_subfield(self, app): dataset = call_action("package_show", id=dataset["id"]) - assert dataset["temporal_extent"] == [{'begin': '1989-04-13', 'end': '1995-05-15'}] + assert dataset["temporal_extent"] == {'begin': '1989-04-13', 'end': '1995-05-15'} @pytest.mark.usefixtures("clean_db") class TestSubfieldResourceForm(object): diff --git a/ckanext/scheming/tests/test_validation.py b/ckanext/scheming/tests/test_validation.py index 15e8aa28..11746269 100644 --- a/ckanext/scheming/tests/test_validation.py +++ b/ckanext/scheming/tests/test_validation.py @@ -909,7 +909,7 @@ def test_valid_simple_subfields(self): temporal_extent=[{'begin': '2000-01-23', 'end': ''}] ) - assert dataset["temporal_extent"] == [{'begin': '2000-01-23'}] + assert dataset["temporal_extent"] == {'begin': '2000-01-23'} def test_empty_simple_subfields(self): lc = LocalCKAN() From 75543d1746cf8d0226f63534df2ac013ccdf0506 Mon Sep 17 00:00:00 2001 From: Matthew Foster Date: Mon, 9 May 2022 16:10:30 -0700 Subject: [PATCH 07/12] remove converting of simple_subfields to dictionaries in after_show and after_search as it breaks down stream harvesters --- ckanext/scheming/plugins.py | 25 ----------------------- ckanext/scheming/tests/test_form.py | 4 ++-- ckanext/scheming/tests/test_validation.py | 2 +- 3 files changed, 3 insertions(+), 28 deletions(-) diff --git a/ckanext/scheming/plugins.py b/ckanext/scheming/plugins.py index afc896d1..1bca29f3 100644 --- a/ckanext/scheming/plugins.py +++ b/ckanext/scheming/plugins.py @@ -484,31 +484,6 @@ def before_index(self, data_dict): return data_dict - def after_search(self, search_results, search_params): - if 'results' not in search_results: - return search_results - schemas = SchemingDatasetsPlugin.instance._expanded_schemas - for data_dict in search_results['results']: - if data_dict['type'] not in schemas: - continue - for d in schemas[data_dict['type']]['dataset_fields']: - if d['field_name'] not in data_dict: - continue - if 'simple_subfields' in d and isinstance(data_dict[d['field_name']], list): - data_dict[d['field_name']] = data_dict[d['field_name']][0] - return search_results - - def after_show(self, context, data_dict): - schemas = SchemingDatasetsPlugin.instance._expanded_schemas - if data_dict['type'] not in schemas: - return data_dict - for d in schemas[data_dict['type']]['dataset_fields']: - if d['field_name'] not in data_dict: - continue - if 'simple_subfields' in d and isinstance(data_dict[d['field_name']], list): - data_dict[d['field_name']] = data_dict[d['field_name']][0] - return data_dict - def _load_schemas(schemas, type_field): out = {} for n in schemas: diff --git a/ckanext/scheming/tests/test_form.py b/ckanext/scheming/tests/test_form.py index 524377d6..9b8101d3 100644 --- a/ckanext/scheming/tests/test_form.py +++ b/ckanext/scheming/tests/test_form.py @@ -400,7 +400,7 @@ def test_dataset_form_create_simple_subfields(self, app, sysadmin_env): app.post(url.encode('ascii'), params=data, extra_environ=sysadmin_env) dataset = call_action("package_show", id="subfield_dataset_1") - assert dataset["temporal_extent"] == {'begin': '2000-01-23', 'end': '2021-12-30'} + assert dataset["temporal_extent"] == [{'begin': '2000-01-23', 'end': '2021-12-30'}] def test_dataset_form_update_simple_subfield(self, app): dataset = Dataset( @@ -428,7 +428,7 @@ def test_dataset_form_update_simple_subfield(self, app): dataset = call_action("package_show", id=dataset["id"]) - assert dataset["temporal_extent"] == {'begin': '1989-04-13', 'end': '1995-05-15'} + assert dataset["temporal_extent"] == [{'begin': '1989-04-13', 'end': '1995-05-15'}] @pytest.mark.usefixtures("clean_db") class TestSubfieldResourceForm(object): diff --git a/ckanext/scheming/tests/test_validation.py b/ckanext/scheming/tests/test_validation.py index 11746269..15e8aa28 100644 --- a/ckanext/scheming/tests/test_validation.py +++ b/ckanext/scheming/tests/test_validation.py @@ -909,7 +909,7 @@ def test_valid_simple_subfields(self): temporal_extent=[{'begin': '2000-01-23', 'end': ''}] ) - assert dataset["temporal_extent"] == {'begin': '2000-01-23'} + assert dataset["temporal_extent"] == [{'begin': '2000-01-23'}] def test_empty_simple_subfields(self): lc = LocalCKAN() From 61e85c08dc83d740467c3627fedbf042479d093c Mon Sep 17 00:00:00 2001 From: Matthew Foster Date: Mon, 9 May 2022 21:16:32 -0700 Subject: [PATCH 08/12] fix flattening of simple subfields --- ckanext/scheming/helpers.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/ckanext/scheming/helpers.py b/ckanext/scheming/helpers.py index 5a14c68e..7b599484 100644 --- a/ckanext/scheming/helpers.py +++ b/ckanext/scheming/helpers.py @@ -457,6 +457,10 @@ def scheming_flatten_simple_subfield(subfield, data): if subfield['field_name'] not in data: return flat + subdata = data[subfield['field_name']] + if(isinstance(subdata, list) and len(subdata) == 1): + subdata = subdata[0] + for field, value in data[subfield['field_name']].items(): prefix = '{field_name}{sep}'.format( field_name=subfield['field_name'], From e263ea5b23b8c030d733e8d47491702ecd8b1f1c Mon Sep 17 00:00:00 2001 From: Matthew Foster Date: Mon, 9 May 2022 21:26:15 -0700 Subject: [PATCH 09/12] fix flattening of simple subfields again. update subfield templates --- ckanext/scheming/helpers.py | 2 +- .../scheming/display_snippets/repeating_subfields.html | 2 ++ .../scheming/display_snippets/simple_subfields.html | 6 +++++- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/ckanext/scheming/helpers.py b/ckanext/scheming/helpers.py index 7b599484..3bd842c5 100644 --- a/ckanext/scheming/helpers.py +++ b/ckanext/scheming/helpers.py @@ -461,7 +461,7 @@ def scheming_flatten_simple_subfield(subfield, data): if(isinstance(subdata, list) and len(subdata) == 1): subdata = subdata[0] - for field, value in data[subfield['field_name']].items(): + for field, value in subdata.items(): prefix = '{field_name}{sep}'.format( field_name=subfield['field_name'], sep=sep, diff --git a/ckanext/scheming/templates/scheming/display_snippets/repeating_subfields.html b/ckanext/scheming/templates/scheming/display_snippets/repeating_subfields.html index f4948aa9..99756df9 100644 --- a/ckanext/scheming/templates/scheming/display_snippets/repeating_subfields.html +++ b/ckanext/scheming/templates/scheming/display_snippets/repeating_subfields.html @@ -9,6 +9,7 @@
    {% for subfield in field.repeating_subfields %} + {% if subfield.field_name in field_data and field_data[subfield.field_name]|length and subfield.display_snippet is not none %}
    {{ h.scheming_language_text(subfield.label) }}
    @@ -20,6 +21,7 @@ object_type=object_type -%} + {% endif %} {% endfor %}
    diff --git a/ckanext/scheming/templates/scheming/display_snippets/simple_subfields.html b/ckanext/scheming/templates/scheming/display_snippets/simple_subfields.html index 59d15071..ff76f86b 100644 --- a/ckanext/scheming/templates/scheming/display_snippets/simple_subfields.html +++ b/ckanext/scheming/templates/scheming/display_snippets/simple_subfields.html @@ -2,21 +2,25 @@ {% block subfield_display %}
    + {% for field_data in fields %}
    {% for subfield in field.simple_subfields %} + {% if subfield.field_name in field_data and field_data[subfield.field_name]|length and subfield.display_snippet is not none %}
    {{ h.scheming_language_text(subfield.label) }}
    {%- snippet 'scheming/snippets/display_field.html', field=subfield, - data=fields, + data=field_data, entity_type=entity_type, object_type=object_type -%}
    + {% endif %} {% endfor %}
    + {% endfor %}
    {% endblock %} From 40f445bca1ab23673fbdf769ca4620f1d905bcc4 Mon Sep 17 00:00:00 2001 From: Matthew Foster Date: Thu, 12 May 2022 15:11:29 -0700 Subject: [PATCH 10/12] add before validator to check length of all simple_subfields in schema --- ckanext/scheming/plugins.py | 4 +++- ckanext/scheming/tests/test_validation.py | 14 ++++++++++++++ ckanext/scheming/validation.py | 21 +++++++++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/ckanext/scheming/plugins.py b/ckanext/scheming/plugins.py index 1bca29f3..ff755208 100644 --- a/ckanext/scheming/plugins.py +++ b/ckanext/scheming/plugins.py @@ -244,7 +244,9 @@ def validate(self, context, data_dict, schema, action): if before: schema['__before'] = validation.validators_from_string( - before, None, scheming_schema) + before, None, scheming_schema) + validation.validators_from_string('scheming_simple_subfields', None, scheming_schema) + else: + schema['__before'] = validation.validators_from_string('scheming_simple_subfields', None, scheming_schema) if after: schema['__after'] = validation.validators_from_string( after, None, scheming_schema) diff --git a/ckanext/scheming/tests/test_validation.py b/ckanext/scheming/tests/test_validation.py index 15e8aa28..61ad0bcf 100644 --- a/ckanext/scheming/tests/test_validation.py +++ b/ckanext/scheming/tests/test_validation.py @@ -921,6 +921,20 @@ def test_empty_simple_subfields(self): assert "temporal_extent" not in dataset + def test_invalid_simple_subfields_length(self): + lc = LocalCKAN() + try: + dataset = lc.action.package_create( + type="test-subfields", + name="a_sf_1", + temporal_extent=[{'begin': '2000-01-23', 'end': '2021-09-13'}, {'begin': '2022-02-23', 'end': ''}] + ) + except ValidationError as e: + assert e.error_dict["temporal_extent_subfield_length"] == ["Too many items in simple subfield temporal_extent. Found 2 items, expected 1"] + else: + raise AssertionError("ValidationError not raised") + pass + @pytest.mark.usefixtures("clean_db") class TestSimpleSubfieldDatasetInvalid(object): def test_invalid_missing_required_simple_subfield(self): diff --git a/ckanext/scheming/validation.py b/ckanext/scheming/validation.py index b07bdae5..0ca44b99 100644 --- a/ckanext/scheming/validation.py +++ b/ckanext/scheming/validation.py @@ -26,6 +26,7 @@ all_validators = {} + def register_validator(fn): """ collect validator functions into ckanext.scheming.all_helpers dict @@ -45,6 +46,26 @@ def scheming_validator(fn): return fn +@scheming_validator +@register_validator +def scheming_simple_subfields(field, schema): + def validator(key, data, errors, context): + fields = schema.get('dataset_fields', []) + schema.get('resource_fields', []) + key_tuples = data.keys() + for f in fields: + if 'simple_subfields' in f: + iters = [tup[1] for tup in key_tuples if tup[0] == f['field_name']] + if iters and max(iters) > 0: + if (f['field_name'],) not in errors: + errors[(f['field_name'] + '_subfield_length',)] = [] + errors[(f['field_name'] + '_subfield_length',)].extend([ + _('Too many items in simple subfield %s. Found %s items, expected 1') + % (f['field_name'], (max(iters) + 1)) + ]) + raise StopOnError + return validator + + @scheming_validator @register_validator def scheming_subfields(field, schema): From dcca67afe4c684ecc367407899a2f3b83bc13c91 Mon Sep 17 00:00:00 2001 From: Matthew Foster Date: Mon, 16 May 2022 13:01:20 -0700 Subject: [PATCH 11/12] re-add simple_subfield validator --- ckanext/scheming/validation.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/ckanext/scheming/validation.py b/ckanext/scheming/validation.py index 6a6dcaf3..070f20a0 100644 --- a/ckanext/scheming/validation.py +++ b/ckanext/scheming/validation.py @@ -50,6 +50,26 @@ def scheming_validator(fn): register_validator(unicode_safe) +@scheming_validator +@register_validator +def scheming_simple_subfields(field, schema): + def validator(key, data, errors, context): + fields = schema.get('dataset_fields', []) + schema.get('resource_fields', []) + key_tuples = data.keys() + for f in fields: + if 'simple_subfields' in f: + iters = [tup[1] for tup in key_tuples if tup[0] == f['field_name']] + if iters and max(iters) > 0: + if (f['field_name'],) not in errors: + errors[(f['field_name'] + '_subfield_length',)] = [] + errors[(f['field_name'] + '_subfield_length',)].extend([ + _('Too many items in simple subfield %s. Found %s items, expected 1') + % (f['field_name'], (max(iters) + 1)) + ]) + raise StopOnError + return validator + + @scheming_validator @register_validator def scheming_choices(field, schema): From 34d84df3c1e1e5cd84d609a3617f9c05f2c11e0a Mon Sep 17 00:00:00 2001 From: Matthew Foster Date: Mon, 16 May 2022 13:26:49 -0700 Subject: [PATCH 12/12] fix error field name --- ckanext/scheming/validation.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/ckanext/scheming/validation.py b/ckanext/scheming/validation.py index 070f20a0..86164106 100644 --- a/ckanext/scheming/validation.py +++ b/ckanext/scheming/validation.py @@ -58,11 +58,12 @@ def validator(key, data, errors, context): key_tuples = data.keys() for f in fields: if 'simple_subfields' in f: + error_fn = f['field_name'] + '_subfield_length' iters = [tup[1] for tup in key_tuples if tup[0] == f['field_name']] if iters and max(iters) > 0: - if (f['field_name'],) not in errors: - errors[(f['field_name'] + '_subfield_length',)] = [] - errors[(f['field_name'] + '_subfield_length',)].extend([ + if (error_fn,) not in errors: + errors[(error_fn,)] = [] + errors[(error_fn,)].extend([ _('Too many items in simple subfield %s. Found %s items, expected 1') % (f['field_name'], (max(iters) + 1)) ])