{{ _('Are you sure you want to delete resource - {name}?').format(name=h.resource_display_name(resource_dict)) }}
diff --git a/changes/189.canada.feature b/changes/189.canada.feature new file mode 100644 index 00000000000..9567d69a4ed --- /dev/null +++ b/changes/189.canada.feature @@ -0,0 +1 @@ +Now handles nested resource validation errors. Creating, updating, and deleting a resource can catch and handle validation errors from other resources in the dataset. diff --git a/ckan/lib/helpers.py b/ckan/lib/helpers.py index 19347aba70f..5cdf2dd9e08 100644 --- a/ckan/lib/helpers.py +++ b/ckan/lib/helpers.py @@ -1088,6 +1088,11 @@ def humanize_entity_type(entity_type: str, object_type: str, u'search placeholder': _(u'Search {object_type}s...'), u'you not member': _(u'You are not a member of any {object_type}s.'), u'update label': _(u"Update {object_type}"), + # (canada fork only): handle all errors in resource actions + # TODO: upstream contrib?? + 'other errors': _('Errors in {object_type}'), + 'other errors package': _('The {object_type} contains errors:'), + 'other errors resources': _('The {object_type} contains invalid resources:') } type_label = object_type.replace(u"_", u" ").capitalize() diff --git a/ckan/logic/action/__init__.py b/ckan/logic/action/__init__.py index 7196d357401..024e0df4882 100644 --- a/ckan/logic/action/__init__.py +++ b/ckan/logic/action/__init__.py @@ -3,7 +3,9 @@ import re from copy import deepcopy -from typing import Any, Mapping, cast +# (canada fork only): handle all errors in resource actions +# TODO: upstream contrib?? +from typing import Any, Mapping, cast, Tuple, Dict from ckan.logic import NotFound @@ -73,3 +75,77 @@ def prettify(field_name: str): else: summary[_(prettify(key))] = error[0] return summary + + +# (canada fork only): handle all errors in resource actions +# TODO: upstream contrib?? +def resource_validation_errors( + error_dict: ErrorDict, + action: str, + pkg_dict: Dict[str, Any], + resource_index: int = -1) -> Tuple[Dict[str, Any], str]: + """ + Returns a modified (error_dict, error_summary) with errors from the + resource_index resource fields in error_dict, errors from the dataset + fields under error_dict['dataset'] and errors from other resource + fields under error_dict['resources'][resource_id]. + """ + new_error_dict = dict(error_dict) + # define out own error summaries so we can have + # a more customized structure to the ErrorDict + error_summaries = [] + if action == 'delete': + # special case for deleting as there is no index + # for a non-existent resource in the pkg_dict. + current_res_error_dict = False + else: + current_res_error_dict = cast("list[ErrorDict]", error_dict['resources'])[resource_index] + if current_res_error_dict: + # if there are errors for the current resource + # let's raise them to the user first. + new_error_dict = dict(current_res_error_dict) + if not current_res_error_dict and 'resources' in error_dict and isinstance(error_dict['resources'], list): + # compile the other resource errors + new_error_dict = {'resources': {}} + for key, res_error_dict in enumerate(error_dict['resources']): + if not res_error_dict: + continue + if key <= len(pkg_dict['resources']): # case for resource_delete + errored_resource = pkg_dict['resources'][key] + if errored_resource.get('id'): + new_error_dict['resources'][errored_resource.get('id')] = res_error_dict + if res_error_dict: + if action == 'create' or action == 'update': + error_summaries.append( + _('Could not create or update resource because another resource in ' + 'this dataset has errors: {}').format(errored_resource['id'])) + elif action == 'delete': + error_summaries.append( + _('Could not delete resource because another resource in ' + 'this dataset has errors: {}').format(errored_resource['id'])) + elif action == 'reorder': + error_summaries.append( + _('Could not reorder resources because a resource in ' + 'this dataset has errors: {}').format(errored_resource['id'])) + if error_dict: + # compile the dataset errors + for key, errors in error_dict.items(): + if key == 'resources': + continue + if 'dataset' not in new_error_dict: + new_error_dict['dataset'] = {} + new_error_dict['dataset'][key] = errors + if 'dataset' in new_error_dict: + if action == 'create' or action == 'update': + error_summaries.append( + _('Could not create or update resource because ' + 'the dataset has errors')) + elif action == 'delete': + error_summaries.append( + _('Could not delete resource because ' + 'the dataset has errors')) + elif action == 'reorder': + error_summaries.append( + _('Could not reorder resources because ' + 'the dataset has errors')) + return new_error_dict, _('; ').join(error_summaries) diff --git a/ckan/logic/action/create.py b/ckan/logic/action/create.py index 2f88b358879..8984d218f52 100644 --- a/ckan/logic/action/create.py +++ b/ckan/logic/action/create.py @@ -33,6 +33,9 @@ import ckan.authz as authz import ckan.model +# (canada fork only): handle all errors in resource actions +# TODO: upstream contrib?? +from . import resource_validation_errors from ckan.common import _ from ckan.types import Context, DataDict, ErrorDict, Schema @@ -329,11 +332,11 @@ def resource_create(context: Context, _get_action('package_update')(context, pkg_dict) context.pop('defer_commit') except ValidationError as e: - try: - error_dict = cast("list[ErrorDict]", e.error_dict['resources'])[-1] - except (KeyError, IndexError): - error_dict = e.error_dict - raise ValidationError(error_dict) + # (canada fork only): handle all errors in resource actions + # TODO: upstream contrib?? + error_dict, error_summary = resource_validation_errors( + e.error_dict, action='create', pkg_dict=pkg_dict) + raise ValidationError(error_dict, error_summary=error_summary) # Get out resource_id resource from model as it will not appear in # package_show until after commit diff --git a/ckan/logic/action/delete.py b/ckan/logic/action/delete.py index dc3ce8ec282..f257706ec74 100644 --- a/ckan/logic/action/delete.py +++ b/ckan/logic/action/delete.py @@ -19,6 +19,9 @@ from ckan.lib.navl.dictization_functions import validate from ckan.model.follower import ModelFollowingModel +# (canada fork only): handle all errors in resource actions +# TODO: upstream contrib?? +from . import resource_validation_errors from ckan.common import _ from ckan.types import Context, DataDict, ErrorDict, Schema @@ -195,8 +198,11 @@ def resource_delete(context: Context, data_dict: DataDict) -> ActionResult.Resou try: pkg_dict = _get_action('package_update')(context, pkg_dict) except ValidationError as e: - errors = cast("list[ErrorDict]", e.error_dict['resources'])[-1] - raise ValidationError(errors) + # (canada fork only): handle all errors in resource actions + # TODO: upstream contrib?? + error_dict, error_summary = resource_validation_errors( + e.error_dict, action='delete', pkg_dict=pkg_dict) + raise ValidationError(error_dict, error_summary=error_summary) for plugin in plugins.PluginImplementations(plugins.IResourceController): plugin.after_resource_delete(context, pkg_dict.get('resources', [])) diff --git a/ckan/logic/action/update.py b/ckan/logic/action/update.py index d0c1400e241..ebb6ed1f07f 100644 --- a/ckan/logic/action/update.py +++ b/ckan/logic/action/update.py @@ -28,6 +28,9 @@ #TODO: upstream contrib!! import ckan.lib.search.jobs as search_jobs +# (canada fork only): handle all errors in resource actions +# TODO: upstream contrib?? +from . import resource_validation_errors from ckan.common import _, config from ckan.types import Context, DataDict, ErrorDict @@ -114,11 +117,11 @@ def resource_update(context: Context, data_dict: DataDict) -> ActionResult.Resou context['use_cache'] = False updated_pkg_dict = _get_action('package_update')(context, pkg_dict) except ValidationError as e: - try: - error_dict = cast("list[ErrorDict]", e.error_dict['resources'])[n] - except (KeyError, IndexError): - error_dict = e.error_dict - raise ValidationError(error_dict) + # (canada fork only): handle all errors in resource actions + # TODO: upstream contrib?? + error_dict, error_summary = resource_validation_errors( + e.error_dict, action='update', pkg_dict=pkg_dict, resource_index=n) + raise ValidationError(error_dict, error_summary=error_summary) resource = _get_action('resource_show')(context, {'id': id}) @@ -579,9 +582,18 @@ def package_resource_reorder( package_dict['resources'] = new_resources _check_access('package_resource_reorder', context, package_dict) - _get_action('package_update')(context, package_dict) + # (canada fork only): handle all errors in resource actions + # TODO: upstream contrib?? + try: + _get_action('package_update')(context, package_dict) + except ValidationError as e: + error_dict, error_summary = resource_validation_errors( + e.error_dict, action='reorder', pkg_dict=package_dict) + return {'id': id, 'order': [resource['id'] for resource in new_resources], + 'errors': error_dict, 'error_summary': error_summary} - return {'id': id, 'order': [resource['id'] for resource in new_resources]} + return {'id': id, 'order': [resource['id'] for resource in new_resources], + 'errors': None, 'error_summary': None} def _update_package_relationship( diff --git a/ckan/public/base/javascript/modules/resource-reorder.js b/ckan/public/base/javascript/modules/resource-reorder.js index 83be73e7833..aa590968a6b 100644 --- a/ckan/public/base/javascript/modules/resource-reorder.js +++ b/ckan/public/base/javascript/modules/resource-reorder.js @@ -30,6 +30,14 @@ this.ckan.module('resource-reorder', function($) { '', '', '' + ].join('\n'), + // (canada fork only): handle all errors in resource actions + // TODO: upstream contrib?? + errors: [ + '
{{ _('The form contains invalid entries:') }}
-{{ _('The form contains invalid entries:') }}
+{{ h.humanize_entity_type('package', dataset_type, 'other errors package') or _('The dataset contains errors:') }}
+{{ h.humanize_entity_type('package', dataset_type, 'other errors resources') or _('The dataset contains invalid resources:') }}
+{{ _('Are you sure you want to delete resource - {name}?').format(name=h.resource_display_name(resource_dict)) }}