Skip to content

[WIP] Better admin config form validation #2801

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

Closed
wants to merge 3 commits into from
Closed

[WIP] Better admin config form validation #2801

wants to merge 3 commits into from

Conversation

sreichel
Copy link
Contributor

@sreichel sreichel commented Dec 11, 2022

Description (*)

This is just an idea for better input validations ...

  1. a lot of fields have no <validate> node in system.xml
  2. no php-side validation for most fields

This PR takes <validation> node to test on php-side.

(supports required-entry, valdidate-digit, validate-number for now)

Note:
<validate-digit> (or others) shoud test against empty input, but this not seem to work (anymore?). So i'd to add required-entry ...

Manual testing scenarios (*)

  1. remove validation class from html to bypass js-validation
  2. ...

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All automated tests passed successfully (all builds are green)
  • Add yourself to contributors list

@sreichel sreichel marked this pull request as draft December 11, 2022 20:26
@github-actions github-actions bot added Component: Catalog Relates to Mage_Catalog Component: CatalogSearch Relates to Mage_CatalogSearch Component: ConfigurableSwatches Relates to Mage_ConfigurableSwatches Component: Core Relates to Mage_Core Component: Downloadable Relates to Mage_Downloadable Component: Reports Relates to Mage_Reports labels Dec 11, 2022
@sreichel sreichel marked this pull request as ready for review December 30, 2022 00:42
@sreichel
Copy link
Contributor Author

Not complete, but ready for test.

@sreichel sreichel closed this by deleting the head repository Jan 8, 2023
@addison74 addison74 reopened this Jan 9, 2023
@sreichel sreichel closed this Jan 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Catalog Relates to Mage_Catalog Component: CatalogSearch Relates to Mage_CatalogSearch Component: ConfigurableSwatches Relates to Mage_ConfigurableSwatches Component: Core Relates to Mage_Core Component: Downloadable Relates to Mage_Downloadable Component: Reports Relates to Mage_Reports Don't forget this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants