Handle Nested Resource Validation Errors

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,76 @@ 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]:
+    """
+    Checks through the error_dict to find all errors in
+    the Dataset and its Resources.
+    """
+    new_error_dict = dict(error_dict)
+    # define out own error summaries so we can have
+    # a more customized structure to the ErrorDict
+    error_summaries = []
+    try:
+        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 = 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 key <= len(pkg_dict['resources']):
+                    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'))
+    except (KeyError, IndexError):
+        new_error_dict = dict(error_dict)
+    return new_error_dict, '; '.join(error_summaries)

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,24 +332,11 @@ 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
+        # (canada fork only): handle all errors in resource actions
         # 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, error_summary)
+        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

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,24 +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:
-        # (canada fork only): handle other "broken" resources
+        # (canada fork only): handle all errors in resource actions
         # 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)
+        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', []))

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,24 +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:
-        # (canada fork only): handle other "broken" resources
+        # (canada fork only): handle all errors in resource actions
         # 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, error_summary)
+        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})
 
@@ -592,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(

ckan/public/base/javascript/modules/resource-reorder.js

@@ -30,6 +30,14 @@ this.ckan.module('resource-reorder', function($) {
         '<i class="fa fa-spinner fa-spin"></i>',
         '<span></span>',
         '</span>'
+      ].join('\n'),
+      // (canada fork only): handle all errors in resource actions
+      // TODO: upstream contrib??
+      errors: [
+        '<div class="error-explanation alert alert-danger">',
+        '<h3></h3>',
+        '<p></p>',
+        '</div>'
       ].join('\n')
     },
     is_reordering: false,
@@ -47,6 +55,13 @@ this.ckan.module('resource-reorder', function($) {
         .insertBefore(this.el)
         .hide();
 
+      // (canada fork only): handle all errors in resource actions
+      // TODO: upstream contrib??
+      this.html_errors = $(this.template.errors)
+        .insertBefore(this.el)
+        .hide();
+      $(this.html_errors).find('h3').text(this._('Errors in dataset'));
+
       this.html_help_text = $(this.template.help_text)
         .text(helpText)
         .insertBefore(this.el)
@@ -126,7 +141,13 @@ this.ckan.module('resource-reorder', function($) {
         module.sandbox.client.call('POST', 'package_resource_reorder', {
           id: module.options.id,
           order: order
-        }, function() {
+        }, function(data) {
+          // (canada fork only): handle all errors in resource actions
+          // TODO: upstream contrib??
+          if( typeof data.result !== 'undefined' && typeof data.result.error_summary !== 'undefined' ){
+            $(module.html_errors).find('p').text(data.result.error_summary);
+            $(module.html_errors).show();
+          }
           module.html_saving.hide();
           $('.save, .cancel', module.html_form_actions).removeClass('disabled');
           module.cache = module.el.html();

ckan/templates/macros/form.html

@@ -13,6 +13,9 @@
 {% from "macros/form/prepend.html" import prepend %}
 {% from "macros/form/custom.html" import custom %}
 {% from "macros/form/errors.html" import errors %}
+{# (canada fork only): handle all errors in resource actions #}
+{# TODO: upstream contrib?? #}
+{% from "macros/form/dataset_errors.html" import dataset_errors %}
 {% from "macros/form/info.html" import info %}
 {% from "macros/form/hidden.html" import hidden %}
 {% from "macros/form/hidden_from_list.html" import hidden_from_list %}
@@ -29,6 +32,9 @@
 {% set prepend = prepend %}
 {% set custom = custom %}
 {% set errors = errors %}
+{# (canada fork only): handle all errors in resource actions #}
+{# TODO: upstream contrib?? #}
+{% set dataset_errors = dataset_errors %}
 {% set info = info %}
 {% set hidden = hidden %}
 {% set hidden_from_list = hidden_from_list %}

ckan/templates/macros/form/dataset_errors.html

@@ -0,0 +1,51 @@
+{# (canada fork only): handle all errors in resource actions #}
+{# TODO: upstream contrib?? #}
+
+{#
+Builds a list of errors for the current form.
+
+errors  - A dict of field/message pairs.
+type    - The alert-* class that should be applied (default: "error")
+classes - A list of classes to apply to the wrapper (default: [])
+
+Example:
+
+{% import 'macros/form.html' as form %}
+{{ form.errors(error_summary, type="warning") }}
+
+#}
+
+
+{# (canada fork only): error -> danger BS version #}
+{% macro dataset_errors(errors={}, type="danger", classes=[], pkg_name='', helpers={}) %}
+  {% if errors %}
+    <div class="error-explanation alert alert-{{ type }}{{ " " ~ classes | join(' ') }}">
+      <h3>{{_('Errors in dataset')}}</h3>
+      {% if 'dataset' in errors and errors.dataset %}
+        {% set dataset_link = helpers.url_for('dataset.read', id=pkg_name) %}
+        <p>{{ _('The <a href="{}" target="_blank">dataset</a> contains errors:').format(dataset_link) }}</p>
+        <ul>
+          {% for field_id, field_errs in errors.dataset.items() %}
+            <li data-field-label="{{ field_id }}">{% if field_id %}{{ field_id }}{{_(':')}} {% endif %}{{ field_errs|join(', ') }}</li>
+          {% endfor %}
+        </ul>
+      {% endif %}
+      {% if 'resources' in errors and errors.resources %}
+        <p>{{ _('The dataset contains invalid resources:') }}</p>
+        <ul>
+          {% for res_id, errs in errors.resources.items() %}
+            {% if not errs %}
+              {% continue %}
+            {% endif %}
+            <li><a href="{{ helpers.url_for('resource.read', id=pkg_name, resource_id=res_id) }}" target="_blank">{{ _('Resource') ~ ' ' ~ loop.index }}</a></li>
+            <ul>
+              {% for field_id, field_errs in errs.items() %}
+                <li data-field-label="{{ field_id }}">{% if field_id %}{{ field_id }}{{_(':')}} {% endif %}{{ field_errs|join(', ') }}</li>
+              {% endfor %}
+            </ul>
+          {% endfor %}
+        </ul>
+      {% endif %}
+    </div>
+  {% endif %}
+{% endmacro %}

ckan/templates/macros/form/errors.html

@@ -12,25 +12,16 @@
 
 #}
 
-
-{# (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 %}
+{% macro errors(errors={}, type="danger", classes=[]) %}
 {% if errors %}
 <div class="error-explanation alert alert-{{ type }}{{ " " ~ classes | join(' ') }}">
-<p>{{ _('The form contains invalid entries:') }}</p>
-<ul>
-    {% for key, error in errors.items() %}
-    <li data-field-label="{{ key }}">{% if key %}{{ key }}: {% endif %}{{ error }}</li>
-    {% endfor %}
-</ul>
+  <p>{{ _('The form contains invalid entries:') }}</p>
+  <ul>
+      {% for key, error in errors.items() %}
+      <li data-field-label="{{ key }}">{% if key %}{{ key }}: {% endif %}{{ error }}</li>
+      {% endfor %}
+  </ul>
 </div>
 {% endif %}
 {% endmacro %}

ckan/templates/package/confirm_delete_resource.html

@@ -14,7 +14,15 @@
       {% block form %}
         {# (canada fork only): handle validation errors #}
         {# TODO: upstream contrib?? #}
-        {% block errors %}{{ form.errors(errors=errors, error_summary=error_summary) }}{% endblock %}
+        {# (canada fork only): handle all errors in resource actions #}
+        {# TODO: upstream contrib?? #}
+        {% block errors %}
+          {% if dataset_errors %}
+            {{ form.dataset_errors(errors=dataset_errors, pkg_name=pkg_id, helpers=h) }}
+          {% else %}
+            {{ form.errors(errors=errors) }}
+          {% endif %}
+        {% 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">