Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle Nested Resource Validation Errors #189

Merged
merged 9 commits into from
Feb 10, 2025

Conversation

JVickery-TBS
Copy link

feat(dev): handle resource validation;

  • Handle nested resource validation.

So we have come across this issue a couple times now, in which users come back to edit a super old dataset in which the schema has changed so much. And they try to add a new resource but some other resources fail validation. Because the resource create and update logic actions just get their own validation errors, and not other resource validation errors, this is what this PR tries to do. So if the current resource does not have any validation errors, it will go check the other ones from the ValidationError object, if it find them, it will make a new error_dict structure which is used in the Resource view functions to output the errors.

image (1)

- Handle nested resource validation.
- Added change log file.
Copy link
Member

@wardi wardi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parsing and reordering resource errors code is repeated 3 times and could be factored out.

We can also have errors in the dataset metadata that prevent updating resources, should that be handled as well?

@JVickery-TBS
Copy link
Author

Reckon so, and will tighten up the reusable code

- Better handling of resource validation errors.
- Handle all dataset errors in resource actions.
@JVickery-TBS
Copy link
Author

JVickery-TBS commented Feb 5, 2025

@wardi okay I THINK this should be good now.

image

So we have a method now which goes through all of the errors in the ValidationError exception and formats them into a new dict if needed. And then the rest is done in jinja templates now which is much better. I did originally just have the new dataset_errors passing into the respective get view methods BUT then ckanext-scheming happened and I had to put a little workaround for that.

I also found out that I needed to handle the Reorder Resource javascript stuff too, so there are some additions in there to handle that.

EDIT: the Resource 1 in the screenshot is not actually the name of the resource, I just adopted how the repeating fields show errors, e.g. Repeating Field x, so I just output the index of the resource in the dataset for the error. But the open in new tab link would help the user fine there. I am seeing about the ckanext-scheming templates in our canada plugin to be able to show the resource name and field labels in the correct languages there.

@JVickery-TBS
Copy link
Author

ckanext-scheming stuff handle in: open-data/ckanext-canada#1556

If you want me to put this one upstream, I can also add the ckanext-scheming template stuff upstream

ckan/logic/action/__init__.py Outdated Show resolved Hide resolved
ckan/logic/action/__init__.py Outdated Show resolved Hide resolved
ckan/logic/action/__init__.py Outdated Show resolved Hide resolved
ckan/logic/action/__init__.py Outdated Show resolved Hide resolved
ckan/logic/action/__init__.py Outdated Show resolved Hide resolved
ckan/logic/action/__init__.py Outdated Show resolved Hide resolved
@wardi
Copy link
Member

wardi commented Feb 7, 2025

This would be a great contribution upstream. Just needs some tests to cover how the errors returned from resource APIs have changed.

JVickery-TBS and others added 3 commits February 7, 2025 11:19
Co-authored-by: Ian Ward <ian@excess.org>
Co-authored-by: Ian Ward <ian@excess.org>
- `dataset_errors` -> `resource_validation_errors`
- Humanize labels.
- Pass pkg_dict into resource confirm delete.
- Pass pkg_dict into resource form.
- Remove try/catch.
- Do not store empty resource validation error dicts.
@JVickery-TBS JVickery-TBS requested a review from wardi February 7, 2025 18:52
@JVickery-TBS
Copy link
Author

@wardi okay all that feedback has been done now

- `h` -> `helpers`
- Humanization fixes.
- `helpers` -> `h`
- Humanization changes.
@JVickery-TBS JVickery-TBS merged commit bb8452e into canada-v2.10 Feb 10, 2025
1 check passed
@JVickery-TBS JVickery-TBS deleted the feature/handle-dataset-resource-validation branch February 10, 2025 17:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants