mage-os / mageos-magento2

Work in progress.
Open Software License 3.0
214 stars 45 forks source link

Fix type when validating field using attribute rules #96

Closed dvrgoc closed 4 months ago

dvrgoc commented 6 months ago

Description (*)

If attribute based rules are used to specify validation of a field in HTML, incorrect type is used when validating. For example, validation rule is ipv4 and has assigned a value of true and is processed as String, but should be Boolean instead.

<input name="field" ipv4="true" ... />

Related Pull Requests

N/A

Fixed Issues (if relevant)

N/A

Manual testing scenarios (*)

  1. create new form (for example on Homepage, custom-form.phtml)
  2. paste the following code:
    <form class="form" action="#" id="custom-form" method="post">
    <fieldset class="fieldset">
    <div class="field required">
            <label class="label" for="custom-field"><span>Attribute Rule</span></label>
            <div class="control">
                <input name="custom-field" id="custom-field" title="Attribute Rule" value="ipv4" ipv4="true" class="input-text" type="text" />
            </div>
        </div>
    </fieldset>
    </form>
    <script type="text/x-magento-init">
    {
        "*": {
            "Magento_Customer/js/block-submit-on-send": {
                "formId": "custom-form"
            }
        }
    }
    </script>
  3. submit the form, error will be displayed on the field. When debugging the applied rule using the following command in rules method:
    
    rules: function( command, argument ) {
    ...
              data = $.validator.normalizeRules(
                $.extend(
                    {},
                    $.validator.metadataRules(element),
                    $.validator.classRules( element ),
                    $.validator.attributeRules( element ),
                    $.validator.dataRules( element ),
                    $.validator.staticRules( element )
                ), element );

// debug the value if (!!$.validator.attributeRules(element) && Object.keys($.validator.attributeRules(element)).length > 0) { console.log($.validator.attributeRules(element)); } ...



- Actual value: String
`{ipv4: 'true'}`
- Expected value: boolean
`{ipv4: true}`

### Questions or comments
The PR has been elaborated in my article which describes the issue in more details: [Deep Dive into Validation Rules](https://inchoo.net/dev-talk/deep-dive-into-validation-rules/)

### Contribution checklist (*)
 - [x] Pull request has a meaningful description of its purpose
 - [x] All commits are accompanied by meaningful commit messages
 - [ ] All new or changed code is covered with unit/integration tests (if applicable)
 - [ ] README.md files for modified modules are updated and included in the pull request if any [README.md predefined sections](https://github.com/magento/devdocs/wiki/Magento-module-README.md) require an update
 - [ ] All automated tests passed successfully (all builds are green)
rhoerr commented 5 months ago

Thanks for your PR!

I'm concerned in that this is a change to a library file, jquery.validate.js. Current version of it is 1.19.5. Latest is 1.20.1. Is it fixed in the latest validate version?

Ultimately I think it should either be fixed there (and this file brought up to date), or fixed outside in a way that overrides the library behavior. We shouldn't be changing third party library code directly unless there's a very strong reason. Is there here?

rhoerr commented 4 months ago

Closing as out of scope for this project. It should be addressed in jQuery Validate.