Improve validation error messages

.gitignore

@@ -1,5 +1,7 @@
 /.idea
 /.ve
+/.pytest_cache
+/libcovebods.egg-info
 __pycache__
 /.pytest_cache
 /libcovebods.egg-info

CHANGELOG.md

@@ -13,7 +13,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 - Statistic: Count of ownership-or-control statements by year
 - Statistic: Count of subjects of ownership-or-control statements by year
 - Statistic: Count of different kinds of interested parties by year
-
+- Improve validation error messages (including oneOf)
+  - https://github.com/openownership/cove-bods/issues/16
+  - https://github.com/openownership/lib-cove-bods/pull/5
+  - This adds translation issues, human messages and a requirement for Django; it was not intended that these should be in this library but as discussed in the pull request we added it hopefully temporarily.
+
 ## [0.4.0] - 2019-04-02
 
 ### Added

libcovebods/common_checks.py

@@ -1,15 +1,105 @@
+import json
+from collections import OrderedDict
 from libcove.lib.common import common_checks_context
 from libcovebods.lib.common_checks import get_statistics, RunAdditionalChecks
+from django.utils.html import format_html
 from libcovebods.config import LibCoveBODSConfig
 
 
+validation_error_template_lookup = {
+    'date': 'Date is not in the correct format. The correct format is YYYY-MM-DD.',
+    'date-time': 'Date is not in the correct format. The correct format is YYYY-MM-DDThh:mm:ssZ.',
+    'uri': 'Invalid uri found',
+    'string': '\'{}\' should be a string. Check that the value {} has quotes at the start and end. Escape any quotes in the value with \'\\\'', # noqa
+    'integer': '\'{}\' should be an integer. Check that the value {} doesn’t contain decimal points or any characters other than 0-9. Integer values should not be in quotes. ', # noqa
+    'number': '\'{}\' should be a number. Check that the value {} doesn’t contain any characters other than 0-9 and dot (\'.\'). Number values should not be in quotes. ', # noqa
+    'boolean': '\'{}\' should be a JSON boolean, \'true\' or \'false\'.', # noqa
+    'object': '\'{}\' should be a JSON object',
+    'array': '\'{}\' should be a JSON array. Check that value(s) appear within square brackets, [...]'}
+# These are "safe" html that we trust
+# Don't insert any values into these strings without ensuring escaping
+# e.g. using django's format_html function.
+validation_error_template_lookup_safe = {
+    'date': 'Date is not in the correct format. The correct format is YYYY-MM-DD.',
+    'date-time': 'Date is not in the correct format. The correct format is YYYY-MM-DDT00:00:00Z.',
+    'uri': 'Invalid uri found',
+    'string': '<code>{}</code> should be a string. Check that the value {} has quotes at the start and end. Escape any quotes in the value with <code>\</code>', # noqa
+    'integer': '<code>{}</code> should be an integer. Check that the value {} doesn’t contain decimal points or any characters other than 0-9. Integer values should not be in quotes. ', # noqa
+    'number': '<code>{}</code> should be a number. Check that the value {} doesn’t contain any characters other than 0-9 and dot (<code>.</code>). Number values should not be in quotes. ', # noqa
+    'boolean': '<code>{}</code> should be a JSON boolean, <code>true</code> or <code>false</code>.', # noqa
+    'object': '<code>{}</code> should be a JSON object',
+    'array': '<code>{}</code> should be a JSON array. Check that value(s) appear within square brackets, [...]'}
+
+
 def common_checks_bods(context, upload_dir, json_data, schema_obj, lib_cove_bods_config=None):
 
     if not lib_cove_bods_config:
         lib_cove_bods_config = LibCoveBODSConfig()
 
     common_checks = common_checks_context(upload_dir, json_data, schema_obj, 'bods-schema.json', context)
 
+    # Rewrite validation errors
+    # We do something similar for OCDS
+    # https://github.com/open-contracting/lib-cove-ocds/blob/74c459c06136af45db76487f39408664fd2d4854/libcoveocds/common_checks.py#L32
+    validation_errors = common_checks['context']['validation_errors']
+    new_validation_errors = []
+    for (json_key, values) in validation_errors:
+        error = json.loads(json_key, object_pairs_hook=OrderedDict)
+
+        e_validator = error['validator']
+        e_validator_value = error['validator_value']
+        validator_type = error['message_type']
+        null_clause = error['null_clause']
+        header = error['header_extra']
+
+        message = None
+        message_safe = None
+
+        if e_validator in ('format', 'type'):
+            message_template = validation_error_template_lookup.get(validator_type)
+            message_safe_template = validation_error_template_lookup_safe.get(validator_type)
+
+            if message_template:
+                message = message_template.format(header, null_clause)
+            if message_safe_template:
+                message_safe = format_html(message_safe_template, header, null_clause)
+
+        if e_validator == 'required':
+            extra_message = ". Check that the field is included and correctly spelled."
+            error['message'] += extra_message
+            error['message_safe'] += extra_message
+
+        if e_validator == 'enum':
+            message = "'{}' contains an unrecognised value. Check the related codelist for allowed code values.".format(header) # noqa
+            message_safe = format_html("<code>{}</code> contains an unrecognised value. Check the related codelist for allowed code values.", header) # noqa
+
+        if e_validator == 'minItems' and e_validator_value == 1:
+            message_safe = format_html('<code>{}</code> is too short. You must supply at least one value, or remove the item entirely (unless it’s required).', header) # noqa
+
+        if e_validator == 'minLength':
+            if e_validator_value == 1:
+                message_safe = format_html('<code>"{}"</code> is too short. Strings must be at least one character. This error typically indicates a missing value.', header) # noqa
+            else:
+                message_safe = format_html('<code>{}</code> is too short. It should be at least {} characters.', header, e_validator_value) # noqa 
+
+        if e_validator == 'maxLength':
+            message_safe = format_html('<code>{}</code> is too long. It should not exceed {} characters.', header, e_validator_value) # noqa
+
+        if e_validator == 'minimum':
+            message_safe = format_html('<code>{}</code> is too small. The minimum allowed value is {}.', header, e_validator_value) # noqa
+
+        if e_validator == 'maximum':
+            message_safe = format_html('<code>{}</code> is too large. The maximum allowed value is {}.', header, e_validator_value) # noqa
+
+        if message is not None:
+            error['message'] = message
+        if message_safe is not None:
+            error['message_safe'] = message_safe
+
+        new_validation_errors.append([json.dumps(error), values])
+    new_validation_errors.sort()
+    common_checks['context']['validation_errors'] = new_validation_errors
+
     context.update(common_checks['context'])
 
     additional_checks = RunAdditionalChecks(json_data, lib_cove_bods_config).run()

libcovebods/lib/common_checks.py

@@ -77,7 +77,7 @@ def get_statistics(json_data):
                 count_person_statements_types[statement['personType']] += 1
         elif statement_type == 'ownershipOrControlStatement':
             try:
-                year = int(statement['statementDate'].split('-')[0])
+                year = int(statement.get('statementDate', '').split('-')[0])
             except ValueError:
                 year = None
             count_ownership_or_control_statement += 1

requirements.in

@@ -1,3 +1,3 @@
 -e git+https://github.com/OpenDataServices/[email protected]#egg=flattentool
--e git+https://github.com/OpenDataServices/lib-cove.git@v0.5.0#egg=libcove
+-e git+https://github.com/OpenDataServices/lib-cove.git@v0.6.0#egg=libcove
 -e .

requirements.txt

@@ -1,29 +1,30 @@
 -e git+https://github.com/OpenDataServices/flatten-tool.git@4c13ef0b32a59e810919a3de09bc8f64ce8f9392#egg=flattentool
--e git+https://github.com/OpenDataServices/lib-cove.git@v0.5.0#egg=libcove
+-e git+https://github.com/OpenDataServices/lib-cove.git@52acb2f0dcd296b9d952ff2adbae61dc4f8dd296#egg=libcove
 ## The following requirements were added by pip freeze:
 bleach==3.1.0
 cached-property==1.5.1
 certifi==2019.3.9
 chardet==3.0.4
 commonmark==0.8.1
 contextlib2==0.5.5
-Django==2.1.7
+Django==2.2
 et-xmlfile==1.0.1
 future==0.17.1
 idna==2.8
-jdcal==1.4
+jdcal==1.4.1
 json-merge-patch==0.2
 jsonref==0.2
 jsonschema==2.6.0
-lxml==4.3.2
-openpyxl==2.6.1
+lxml==4.3.3
+openpyxl==2.6.2
 python-dateutil==2.8.0
-pytz==2018.9
+pytz==2019.1
 requests==2.21.0
 rfc3987==1.3.8
 schema==0.7.0
 six==1.12.0
+sqlparse==0.3.0
 strict-rfc3339==0.7
-urllib3==1.24.1
+urllib3==1.24.2
 webencodings==0.5.1
 xmltodict==0.12.0

requirements_dev.txt

@@ -1,5 +1,6 @@
--r requirements.in
-pytest==4.3.1
+-e git+https://github.com/OpenDataServices/flatten-tool.git@4c13ef0b32a59e810919a3de09bc8f64ce8f9392#egg=flattentool
+-e git+https://github.com/OpenDataServices/lib-cove.git@52acb2f0dcd296b9d952ff2adbae61dc4f8dd296#egg=libcove
+pytest==4.4.1
 flake8==3.7.7
 ## The following requirements were added by pip freeze:
 atomicwrites==1.3.0
@@ -10,32 +11,31 @@ certifi==2019.3.9
 chardet==3.0.4
 commonmark==0.8.1
 contextlib2==0.5.5
-Django==2.1.7
+Django==2.2
 entrypoints==0.3
 et-xmlfile==1.0.1
--e git+https://github.com/OpenDataServices/flatten-tool.git@4c13ef0b32a59e810919a3de09bc8f64ce8f9392#egg=flattentool
 future==0.17.1
 idna==2.8
-jdcal==1.4
+jdcal==1.4.1
 json-merge-patch==0.2
 jsonref==0.2
 jsonschema==2.6.0
--e git+https://github.com/OpenDataServices/[email protected]#egg=libcove
-lxml==4.3.2
+lxml==4.3.3
 mccabe==0.6.1
-more-itertools==6.0.0
-openpyxl==2.6.1
+more-itertools==7.0.0
+openpyxl==2.6.2
 pluggy==0.9.0
 py==1.8.0
 pycodestyle==2.5.0
 pyflakes==2.1.1
 python-dateutil==2.8.0
-pytz==2018.9
+pytz==2019.1
 requests==2.21.0
 rfc3987==1.3.8
 schema==0.7.0
 six==1.12.0
+sqlparse==0.3.0
 strict-rfc3339==0.7
-urllib3==1.24.1
+urllib3==1.24.2
 webencodings==0.5.1
 xmltodict==0.12.0

tests/fixtures/api/badfile_all_validation_errors.json

@@ -0,0 +1,24 @@
+[
+{},
+{"statementType": "bad statement type"},
+{"statementType": "personStatement"},
+{"statementType": {}},
+{"statementType": "personStatement", "statementID": 100},
+{"statementType": "personStatement", "statementID": "tooshort"},
+{"statementType": "personStatement", "statementID": "too long long long long long long long long long long long long long long long long"},
+{"statementType": "personStatement", "replacesStatements": ["tooshort"]},
+{"statementType": "personStatement", "replacesStatements": ["too long long long long long long long long long long long long long long long long"]},
+{"statementType": "personStatement", "replacesStatements": "not an array"},
+{"statementType": "personStatement", "statementDate": "not a date"},
+{"statementType": "personStatement", "statementID": "2f957a72-caaa-4363-bf45-1257f1b57db6", "personType": "bad person type"},
+{"statementType": "personStatement", "statementID": "2f957a72-caaa-4363-bf45-1257f1b57db6", "birthDate": "not a date"},
+{"statementType": "personStatement", "statementID": "2f957a72-caaa-4363-bf45-1257f1b57db6", "source":{"retrievedAt": "not a date-time"}},
+{"statementType": "personStatement", "statementID": "2f957a72-caaa-4363-bf45-1257f1b57db6", "missingPersonReason": "no missingPersonType"},
+{"statementType": "personStatement", "statementID": "2f957a72-caaa-4363-bf45-1257f1b57db6", "identifiers":[{}]},
+{"statementType": "ownershipOrControlStatement", "statementID": "772099ea-419f-418b-b6b8-98492e5516bb", "interests": [{"share": {"exact": "not a number", "minimum":-1, "maximum":101}}]},
+{"statementType": "ownershipOrControlStatement", "statementID": "772099ea-419f-418b-b6b8-98492e5516bb", "interests": {"share": {"exact": "not a number", "minimum":-1, "maximum":101}}},
+{"statementType": "ownershipOrControlStatement", "statementID": "772099ea-419f-418b-b6b8-98492e5516bb", "interests": ["not an object"]},
+{"statementType": "ownershipOrControlStatement", "statementID": "772099ea-419f-418b-b6b8-98492e5516bb", "interests": [{"share": {"exclusiveMinimum": "not a bool"}}]},
+{"statementType": "ownershipOrControlStatement", "statementID": "772099ea-419f-418b-b6b8-98492e5516bb", "annotations":[{"motivation": "not on open list"}]},
+{"statementType": "entityStatement", "statementID": "a484ec70-ef24-45ca-ae2a-52453ca2fe9b", "uri": "not a uri"}
+]