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

Fix for issue magento/magento2#18630 #18633

Conversation

dverkade
Copy link
Member

Description

When in the core_config_data table the setting for tax/defaults/postcode is NULL the compare on the frontend is breaking because NULL != " ". This fix resolves that issue.

Fixed Issues

  1. Postcode / Zipcode in checkout form already validated on page load  #18630: Postcode / Zipcode in checkout form already validated on page load

Manual testing scenarios

See steps in issue to reproduce original issue.

@magento-engcom-team
Copy link
Contributor

Hi @dverkade. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me $VERSION instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

Copy link
Contributor

@orlangur orlangur left a comment

Choose a reason for hiding this comment

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

After reading comments from issue - is it actually reproducible under 2.3-develop? If not, please consider backporting to 2.2-develop instead of a custom fix.

@orlangur
Copy link
Contributor

Closing due to inactivity. Feel free to reach me out anytime later if you wish to continue work on this pull request and it will be reopened.
Thanks for collaboration!

@milindsingh
Copy link
Member

@dverkade the labels "community-insider-contribution Community Insider: Cream" are not automatically assigned, but partner-contribution label get auto assigned by magento-engcom-team.
Do you know why ?

@dverkade dverkade reopened this Dec 13, 2018
@dverkade
Copy link
Member Author

@milindsingh the community insider labels will be automatically assigned since last week or so. So this has been resolved already as far as I know.

* @returns boolean.
*/
this.value.equalityComparer = function (oldValue, newValue) {
return !oldValue && !newValue || oldValue === newValue;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to change comparator behavior to achieve

the compare on the frontend is breaking because NULL != " "

?

Copy link
Member Author

Choose a reason for hiding this comment

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

It exactly says what it is for. We get a NULL value, which is not the same as "". So that's why we need this change.

Copy link
Contributor

Choose a reason for hiding this comment

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

@dverkade I mean this looks like kinda low-level change. Is there some other place in client code which may be adjusted instead of changing comparator behavior?

@VladimirZaets VladimirZaets self-assigned this Dec 27, 2018
@VladimirZaets
Copy link
Contributor

@dverkade, I am closing this PR now due to inactivity.
Please reopen and update if you wish to continue.
Thank you for collaboration

@dverkade dverkade reopened this Dec 27, 2018
@dverkade
Copy link
Member Author

This PR can be processed, it fixes the issues mentioned.

@magento-engcom-team magento-engcom-team added this to the Release: 2.3.1 milestone Jan 17, 2019
@magento-engcom-team
Copy link
Contributor

Hi @sivaschenko, thank you for the review.
ENGCOM-3898 has been created to process this Pull Request

@okorshenko okorshenko removed this from the Release: 2.3.1 milestone Jan 28, 2019
…de-Zipcode-in-checkout-form-already-validated-on-page-load
@nmalevanec nmalevanec self-assigned this Mar 14, 2019
@ghost ghost added the Progress: accept label Mar 15, 2019
@magento-engcom-team magento-engcom-team merged commit ac11b34 into magento:2.3-develop Mar 18, 2019
@ghost
Copy link

ghost commented Mar 18, 2019

Hi @dverkade, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants