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

Improve address save flow to allow to use custom validators #7552

Conversation

vovayatsyuk
Copy link
Member

@vovayatsyuk vovayatsyuk commented Nov 24, 2016

I'm facing an issue, that I cannot make some fields optional because of hardcoded
validation of address fields. Moreover, validation is made in private method, so I can't use plugin for it.

Hopefully, there is a public validation method analogue, available in address class, that can be extended with plugins.

@slavvka
Copy link
Member

slavvka commented Nov 28, 2016

Hi @vovayatsyuk. Your contribution looks good but would you please also fix failed Travis Builds as well as cover changed functionality with tests?

@vovayatsyuk
Copy link
Member Author

Just to clarify before fixing test with failed messages assertion.

How do you think, what is the best way to accomplish it?

  1. Change messages in asserting rules:

    firstname is a required field. => Please enter the first name.
    
  2. Change messages returned by public validate method:

    Please enter the first name. => firstname is a required field.
    

I'd like to change public validate method (option 2) in order to keep standard phrases for all fields. Additionally, %fieldName is a required field phrase is already used across Magento code.

@vovayatsyuk
Copy link
Member Author

Hi @slavvka. I'm really stuck with following error:

Call to a member function getCountriesWithOptionalZip() on null in 
/home/travis/build/magento/magento2/app/code/Magento/Customer/Model/Address/AbstractAddress.php 
on line 584

The problem is addressModel is created inside save method and we can't call it methods safely, because it does not have any required variables.

Sorry, but I don't have any experience with test writing, so if it could be fixed easily, can you guide me on how to :) ?

For now I'm really think that the save method should be rewritten to make it possible to write a test for it.

@aleksandrosadchiy
Copy link

Hi
@vovayatsyuk.

"2.Change messages returned by public validate method:
Please enter the first name. => firstname is a required field."
In order, not to change the behavior of the system.

Call to a member function getCountriesWithOptionalZip() on null in /home/travis/build/magento/magento2/app/code/Magento/Customer/Model/Address/AbstractAddress.php on line 584
This error started to appear, due to the fact that you suggested to use
$errors = $addressModel->validate();
instead of private function _validate.
I adjusted the unit test. Your pull request has already been prepared and awaiting approval.
Number of internal ticket is MAGETWO-61725.

Thanks.

@vovayatsyuk vovayatsyuk force-pushed the refactoring-address-repository-validate branch from 801c121 to 621f055 Compare December 19, 2016 15:03
@vovayatsyuk vovayatsyuk deleted the refactoring-address-repository-validate branch December 19, 2016 15:12
@vovayatsyuk
Copy link
Member Author

Sorry, I've just screw up something 😕. I will create another one PR for this issue.

@slavvka
Copy link
Member

slavvka commented Dec 20, 2016

@vovayatsyuk, you didn't need to close it since it is already merged and fixed and will be delivered anyway. Please reopen it and don't create another PR.

mmansoor-magento pushed a commit that referenced this pull request Dec 26, 2016
mmansoor-magento pushed a commit that referenced this pull request Dec 26, 2016
mmansoor-magento pushed a commit that referenced this pull request Dec 26, 2016
mmansoor-magento pushed a commit that referenced this pull request Dec 26, 2016
mmansoor-magento pushed a commit that referenced this pull request Dec 26, 2016
mmansoor-magento pushed a commit that referenced this pull request Dec 26, 2016
mmansoor-magento pushed a commit that referenced this pull request Dec 26, 2016
mmansoor-magento pushed a commit that referenced this pull request Dec 26, 2016
mmansoor-magento pushed a commit that referenced this pull request Dec 26, 2016
mmansoor-magento pushed a commit that referenced this pull request Dec 26, 2016
[SOUTH] Bugs:
- MAGETWO-52925 Simple child product without a special price still shown as "was (original price)" #4442 #5097 #6645 - for mainline
- MAGETWO-60098 Configurable product option price is displayed incorrectly per website
- MAGETWO-56793 [GITHUB][PR] Fix Magento\Review\Model\ResourceModel\Rating\Option not instantiable in setup scripts #5465
- MAGETWO-58078 [FT] CreateProductAttributeEntityFromProductPageTest fails
- MAGETWO-61725 [GITHUB] Improve address save flow to allow to use custom validators #7552
magento-devops-reposync-svc pushed a commit that referenced this pull request Jun 20, 2022
…asticsearch-6-module

[Pyrrans] AC-2649: Remove elasticsearch 6 module
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.

4 participants