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

Integrate custom extra metadata #351

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

pdekraker-epa
Copy link

Add a scheming preset for custom metadata key value pairs

Solution previously mentioned in #14 and #229 has a couple issues:

  • All or nothing (is enabled for all schemas)
  • Incompatible with multi-page forms (fields appear on each page)
  • Cannot control placement within the form

This change adds a preset using the underlying CKAN form element. The associated field must be called "extras" to bypass schema conversion. If there is a better approach please advise.

@GordianDziwis
Copy link

Can this be merged?

@pdekraker-epa
Copy link
Author

I believe if functions properly. The added preset is however not documented in the README.

@pdekraker-epa
Copy link
Author

@wardi could you merge this PR or provide feedback on additional changes that may be required?

@wardi
Copy link
Contributor

wardi commented Nov 7, 2023

I don't use this feature because I don't like how users can accidentally cause the site to break by using the same names for their custom extras that are used in the schema. Once we start storing scheming extra fields in plugin_extras instead of extras this problem will be resolved.

I'm not against merging, though because certainly some sites find this feature useful. I think I would add a warning on the README about users needing to avoid field names used in the schema, at least.

About the name of the feature, what do you think about "free-form extras" or something similar that implies a lack of standardization instead of "custom extras"?

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.

3 participants