Handle Nested Resource Validation Errors

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.

ckan/logic/action/create.py

@@ -329,11 +329,24 @@ def resource_create(context: Context,
         _get_action('package_update')(context, pkg_dict)
         context.pop('defer_commit')
     except ValidationError as e:
+        # (canada fork only): handle other "broken" resources
+        # TODO: upstream contrib??
+        error_summary = ''
         try:
             error_dict = cast("list[ErrorDict]", e.error_dict['resources'])[-1]
+            if not error_dict and 'resources' in e.error_dict and isinstance(e.error_dict['resources'], list):
+                error_dict = {'resources': {}}
+                for key, res_error_dict in enumerate(e.error_dict['resources']):
+                    if key <= len(pkg_dict['resources']):
+                        errored_resource = pkg_dict['resources'][key]
+                        if errored_resource.get('id'):
+                            error_dict['resources'][errored_resource.get('id')] = res_error_dict
+                            if res_error_dict:
+                                error_summary += _('Could not create or update resource because another resource in '
+                                                   'this dataset has errors: {}; ').format(errored_resource['id'])
         except (KeyError, IndexError):
             error_dict = e.error_dict
-        raise ValidationError(error_dict)
+        raise ValidationError(error_dict, error_summary)
 
     # Get out resource_id resource from model as it will not appear in
     # package_show until after commit

ckan/logic/action/delete.py

@@ -195,8 +195,24 @@ 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 other "broken" resources
+        # TODO: upstream contrib??
+        error_dict = e.error_dict
+        error_summary = ''
+        try:
+            if 'resources' in e.error_dict and isinstance(e.error_dict['resources'], list):
+                error_dict = {'resources': {}}
+                for key, res_error_dict in enumerate(e.error_dict['resources']):
+                    if key <= len(pkg_dict['resources']):
+                        errored_resource = pkg_dict['resources'][key]
+                        if errored_resource.get('id'):
+                            error_dict['resources'][errored_resource.get('id')] = res_error_dict
+                            if res_error_dict:
+                                error_summary += _('Could not delete resource because another resource in '
+                                                   'this dataset has errors: {}; ').format(errored_resource['id'])
+        except (KeyError, IndexError):
+            error_dict = e.error_dict
+        raise ValidationError(error_dict, error_summary)
 
     for plugin in plugins.PluginImplementations(plugins.IResourceController):
         plugin.after_resource_delete(context, pkg_dict.get('resources', []))

ckan/logic/action/update.py

@@ -114,11 +114,24 @@ 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:
+        # (canada fork only): handle other "broken" resources
+        # TODO: upstream contrib??
+        error_summary = ''
         try:
             error_dict = cast("list[ErrorDict]", e.error_dict['resources'])[n]
+            if not error_dict and 'resources' in e.error_dict and isinstance(e.error_dict['resources'], list):
+                error_dict = {'resources': {}}
+                for key, res_error_dict in enumerate(e.error_dict['resources']):
+                    if key <= len(pkg_dict['resources']):
+                        errored_resource = pkg_dict['resources'][key]
+                        if errored_resource.get('id'):
+                            error_dict['resources'][errored_resource.get('id')] = res_error_dict
+                            if res_error_dict:
+                                error_summary += _('Could not create or update resource because another resource in '
+                                                   'this dataset has errors: {}; ').format(errored_resource['id'])
         except (KeyError, IndexError):
             error_dict = e.error_dict
-        raise ValidationError(error_dict)
+        raise ValidationError(error_dict, error_summary)
 
     resource = _get_action('resource_show')(context, {'id': id})
 

ckan/templates/macros/form/errors.html

@@ -12,7 +12,17 @@
 
 #}
 
-{% macro errors(errors={}, type="error", classes=[]) %}
+
+{# (canada fork only): actually error summary and error dict handling #}
+{# TODO: upstream contrib?? #}
+{# (canada fork only): error -> danger BS version #}
+{% macro errors(errors={}, type="danger", classes=[], error_summary="") %}
+{% if error_summary %}
+<div class="error-explanation alert alert-{{ type }}{{ " " ~ classes | join(' ') }}">
+  <h3>{{_('Errors')}}</h3>
+  <p>{{ error_summary }}</p>
+</div>
+{% endif %}
 {% if errors %}
 <div class="error-explanation alert alert-{{ type }}{{ " " ~ classes | join(' ') }}">
 <p>{{ _('The form contains invalid entries:') }}</p>
@@ -23,4 +33,4 @@
 </ul>
 </div>
 {% endif %}
-{% endmacro %}
+{% endmacro %}

ckan/templates/package/confirm_delete_resource.html

@@ -1,5 +1,9 @@
 {% extends "page.html" %}
 
+{# (canada fork only): handle validation errors #}
+{# TODO: upstream contrib?? #}
+{% import 'macros/form.html' as form %}
+
 {% block subtitle %}{{ _("Confirm Delete") }}{% endblock %}
 
 {% block maintag %}<div class="row" role="main">{% endblock %}
@@ -8,6 +12,9 @@
   <section class="module col-md-6 col-md-offset-3">
     <div class="module-content">
       {% block form %}
+        {# (canada fork only): handle validation errors #}
+        {# TODO: upstream contrib?? #}
+        {% block errors %}{{ form.errors(errors=errors, error_summary=error_summary) }}{% endblock %}
         <p>{{ _('Are you sure you want to delete resource - {name}?').format(name=h.resource_display_name(resource_dict)) }}</p>
         <p class="form-actions">
           <form id='confirm-resource-delete-form' action="{% url_for 'resource.delete', resource_id=resource_dict.id, id=pkg_id %}" method="post">

ckan/templates/package/snippets/resource_form.html

@@ -6,15 +6,17 @@
 
 
 <form id="resource-edit" class="dataset-form dataset-resource-form" method="post" action="{{ action }}" data-module="basic-form resource-form" enctype="multipart/form-data">
-  {{ h.csrf_input() }} 
+  {{ h.csrf_input() }}
   {% block stages %}
     {# An empty stages variable will not show the stages #}
     {% if stage %}
       {{ h.snippet('package/snippets/stages.html', stages=stage, pkg_name=pkg_name, dataset_type=dataset_type) }}
     {% endif %}
   {% endblock %}
 
-  {% block errors %}{{ form.errors(error_summary) }}{% endblock %}
+  {# (canada fork only): error_summary param #}
+  {# TODO: upstream contrib?? #}
+  {% block errors %}{{ form.errors(errors=errors, error_summary=error_summary) }}{% endblock %}
 
   <input name="id" value="{{ data.id }}" type="hidden"/>
 

ckan/views/resource.py

@@ -254,6 +254,22 @@ def post(self, package_type: str, id: str) -> Union[str, Response]:
         except ValidationError as e:
             errors = e.error_dict
             error_summary = e.error_summary
+            # (canada fork only): handle other "broken" resources
+            # TODO: upstream contrib??
+            if 'resources' in errors and isinstance(errors['resources'], dict):
+                error_summary = _('Could not create or update resource because other resources '
+                                  'in this dataset have errors:') + '\n'
+                for err_res_id, res_errors in errors['resources'].items():
+                    if res_errors:
+                        errored_resource = get_action('resource_show')(context, {'id': err_res_id})
+                        error_summary += '\n- [{}]({})'.format(
+                            h.get_translated(errored_resource, 'name'),
+                            h.url_for('resource.read', id=errored_resource['package_id'],
+                                      resource_id=errored_resource['id']))
+                        for field_key, field_errs in res_errors.items():
+                            error_summary += '\n    - {}{} {}'.format(field_key, _(':'), ', '.join([_(err) for err in field_errs]))
+                errors = None
+                error_summary = h.render_markdown(error_summary, allow_html=True)
             if data.get(u'url_type') == u'upload' and data.get(u'url'):
                 data[u'url'] = u''
                 data[u'url_type'] = u''
@@ -380,6 +396,22 @@ def post(self, package_type: str, id: str,
         except ValidationError as e:
             errors = e.error_dict
             error_summary = e.error_summary
+            # (canada fork only): handle other "broken" resources
+            # TODO: upstream contrib??
+            if 'resources' in errors and isinstance(errors['resources'], dict):
+                error_summary = _('Could not create or update resource because other resources '
+                                  'in this dataset have errors:') + '\n'
+                for err_res_id, res_errors in errors['resources'].items():
+                    if res_errors:
+                        errored_resource = get_action('resource_show')(context, {'id': err_res_id})
+                        error_summary += '\n- [{}]({})'.format(
+                            h.get_translated(errored_resource, 'name'),
+                            h.url_for('resource.read', id=errored_resource['package_id'],
+                                      resource_id=errored_resource['id']))
+                        for field_key, field_errs in res_errors.items():
+                            error_summary += '\n    - {}{} {}'.format(field_key, _(':'), ', '.join([_(err) for err in field_errs]))
+                errors = None
+                error_summary = h.render_markdown(error_summary, auto_link=False)
             return self.get(
                 package_type, id, resource_id, data, errors, error_summary
             )
@@ -478,6 +510,30 @@ def post(self, package_type: str, id: str, resource_id: str) -> Response:
                 )
             else:
                 return h.redirect_to(u'{}.read'.format(package_type), id=id)
+        # (canada fork only): handle validation errors
+        # TODO: upstream contrib??
+        except ValidationError as e:
+            errors = e.error_dict
+            error_summary = e.error_summary
+            # (canada fork only): handle other "broken" resources
+            # TODO: upstream contrib??
+            if 'resources' in errors and isinstance(errors['resources'], dict):
+                error_summary = _('Could not delete resource because other resources '
+                                  'in this dataset have errors:') + '\n'
+                for err_res_id, res_errors in errors['resources'].items():
+                    if res_errors:
+                        errored_resource = get_action('resource_show')(context, {'id': err_res_id})
+                        error_summary += '\n- [{}]({})'.format(
+                            h.get_translated(errored_resource, 'name'),
+                            h.url_for('resource.read', id=errored_resource['package_id'],
+                                      resource_id=errored_resource['id']))
+                        for field_key, field_errs in res_errors.items():
+                            error_summary += '\n    - {}{} {}'.format(field_key, _(':'), ', '.join([_(err) for err in field_errs]))
+                errors = None
+                error_summary = h.render_markdown(error_summary, auto_link=False)
+            return self.get(
+                package_type, id, resource_id, errors, error_summary
+            )
         except NotAuthorized:
             return base.abort(
                 403,
@@ -486,7 +542,11 @@ def post(self, package_type: str, id: str, resource_id: str) -> Response:
         except NotFound:
             return base.abort(404, _(u'Resource not found'))
 
-    def get(self, package_type: str, id: str, resource_id: str) -> str:
+    # (canada fork only): handle validation errors
+    # TODO: upstream contrib??
+    def get(self, package_type: str, id: str, resource_id: str,
+            errors: Optional[dict[str, Any]] = None,
+            error_summary: Optional[dict[str, Any]] = None) -> str:
         context = self._prepare(id)
         try:
             resource_dict = get_action(u'resource_show')(
@@ -509,6 +569,10 @@ def get(self, package_type: str, id: str, resource_id: str) -> str:
 
         return base.render(
             u'package/confirm_delete_resource.html', {
+                # (canada fork only): handle validation errors
+                # TODO: upstream contrib??
+                'errors': errors,
+                'error_summary': error_summary,
                 u'dataset_type': _get_package_type(id),
                 u'resource_dict': resource_dict,
                 u'pkg_id': pkg_id