-
Notifications
You must be signed in to change notification settings - Fork 60
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
Better JSON schema support for plugin settings in Rocky #939
Conversation
Move validation to form. Show errors of json schema validation by including "partials/form/form_errors.html" in plugin_settings_add.html.
Nice! functionally this looks like a great step in the right direction. |
Minimum allowed coverage is Generated by 🐒 cobertura-action against 9190bea |
…invws/nl-kat-coordination into feature/full-json-support-for-settings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good I don't have much to say except other than that single linter suppression
Checklist for QA:
What works:What doesn't work: |
@Darwinkel Firefox seems to send empty strings when you pass text in a number field. (Chromium does not allow text, although you can bypass that.) So I guess this is expected behavior? Also because one of the (optional) fields is actually being set? |
Fair point. Would it be much work to add a new error message for when text is somehow passed to a number field (or a similar situation)? |
@Darwinkel When actual text is passed to a number field you do see a message of the kind "Enter a whole number" (also see the test cases). |
Changes
Simplify the settings setup to support JSON schema validation from the plugins. This means no add/edit/delete per key, only an upsert (idempotent
PUT
/create_or_update
) and a delete. Since we validate settings as a whole against the schema it makes more sense to treat it as one entity as well.Issue link
Fixes #464.
Fixes #108.
Proof
Some jsonschema errors in the form:
Code Checklist
Communication
.env
changes files if required and changed the.env-dist
accordingly.Checklist for code reviewers:
Copy-paste the checklist from the docs/source/templates folder into your comment.
Checklist for QA:
Copy-paste the checklist from the docs/source/templates folder into your comment.