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

Pass dataset name to resource fields snippets #354

Merged
merged 2 commits into from
Jan 11, 2023

Conversation

amercader
Copy link
Member

@wardi this is just some proposal for discussion

Resource field snippets in the new form have no way of knowing the dataset id/name (if it's an edit form there is data.package_id).

These changes are not the most elegant, because 1) they add a resource-specific key to the generic form_field.html and 2) they pass the dataset name instead of the id.

This is because, annoyingly, upstream CKAN core only passes the name from new_resource.html / new_resource_not_draft.html to resource_form.html, and not pkg_dict, so that's the only thing available to ckanext-scheming to pass on.

Should we patch the linked core templates to pass the whole pkg_dict to resource_form.html?
Is there a simpler approach that I'm missing?

For context, we are using this in ckan/ckanext-validation#83, where we are creating a new frontend component to upload files that targets an endpoint that must know the dataset id to create the resource properly.

Resource field snippets in the new form have no way of knowing the
dataset id/name (if it's an edit form there is `data.package_id`).

These changes are not the most elegant, because 1) they add a
resource-specific key to the generic `form_field.html` and 2) they pass
the dataset name instead of the id.

This is because, annoyingly, upstream CKAN core only passes this from
`new_resource.html` / `new_resource_not_draft.html` to
`resource_form.html`, and not `pkg_dict`
@@ -64,7 +64,8 @@
errors=errors,
licenses=c.licenses,
entity_type='dataset',
object_type=dataset_type
object_type=dataset_type,
package_id=pkg_name
Copy link
Contributor

Choose a reason for hiding this comment

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

a comment here about why we're passing the name as package_id would be good

@wardi
Copy link
Contributor

wardi commented Jan 11, 2023

This is fine. If there's a core change to pass more data to the resource forms it should be driven by a specific need in another extension (or because it simplifies something in core), then we can mirror that change in scheming.

@amercader
Copy link
Member Author

Makes sense @wardi . I added a comment as requested

@wardi wardi merged commit 658fc3f into master Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants