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

Force dni fields required if associated Country needs it #11726

Merged
merged 6 commits into from Jan 3, 2019

Conversation

Projects
None yet
7 participants
@jolelievre
Copy link
Contributor

jolelievre commented Dec 11, 2018

Questions Answers
Branch? develop
Description? Some country needs the dni field in their customer addresses, but you can't force it on every countries So the address validation needs to check this field according to the country settings and for the required field if needed.
Type? bug fix
Category? FO
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #9891
How to test? Go to International > Locations > Countries, select a country and set "Do you need a tax identification number?" to true, and you need to add "dni" in the address format or you won't be able to see (nor validate) the field. Try to enter an address in this country without dni you are blocked. When you try in another country everything is fine.

This change is Reviewable

Show resolved Hide resolved classes/Address.php Outdated
Show resolved Hide resolved classes/Address.php Outdated
Show resolved Hide resolved classes/Address.php Outdated

jolelievre and others added some commits Dec 19, 2018

@marionf

This comment has been minimized.

Copy link
Contributor

marionf commented Jan 2, 2019

@jolelievre

I have set "Do you need a tax identification number?" to Yes for Italy and added "dni" in the address format
When I try to add an address in Italy from the FO, I have this Exception

capture d ecran_836

capture d ecran_835

Also, I can't complete the order while the identification number field is well filled

capture d ecran_837

If I edit this address the identification number is well filled

capture d ecran_840

But when I save I have another exception

capture d ecran_839

If I fill the company field (optional), I will have the same exception for the next optional fields: vat number, address2, other etc ...

@jolelievre
Copy link
Contributor Author

jolelievre left a comment

@PierreRambaud seems good to me, but as an author I can't approve it
either you can do it or someone from @PrestaShop/prestashop-core-developers ?

@matks

matks approved these changes Jan 3, 2019

@eternoendless

This comment has been minimized.

Copy link
Member

eternoendless commented Jan 3, 2019

Thank you @jolelievre and @PierreRambaud

@eternoendless eternoendless merged commit 02d3b8a into PrestaShop:develop Jan 3, 2019

1 of 2 checks passed

Codacy/PR Quality Review Not up to standards. This pull request quality could be better.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment