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

Add missing field type DNI in brand address form #16153

Merged
merged 5 commits into from Oct 30, 2019

Conversation

@jolelievre
Copy link
Contributor

jolelievre commented Oct 28, 2019

Questions Answers
Branch? 1.7.6.x
Description? Some countries require a DNI field, but it was missing in the Brand address form, so we add it in the form along with validation when it's required
Type? bug fix
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #16121
How to test? Try adding a brand address on a country which requires DNI (Spain for example) you can't add/update it if the field is empty (see issue for more details)

This change is Reviewable

@jolelievre jolelievre added this to the 1.7.6.2 milestone Oct 28, 2019
@jolelievre jolelievre requested a review from PrestaShop/prestashop-core-developers as a code owner Oct 28, 2019
jolelievre added 2 commits Oct 29, 2019
…ntries with DNI, add a js component to toggle the input requirement
Copy link
Contributor Author

jolelievre left a comment

Feedback addressed

@Robin-Fischer-PS

This comment has been minimized.

Copy link

Robin-Fischer-PS commented Oct 29, 2019

Hi @jolelievre !

We've got a bug !

It's impossible to empty DNI field when it was filled before.

See video :
https://drive.google.com/file/d/1AHRBt8pQtXdDfdxUhpof7TF2e3LmUUZT/view?usp=sharing

Thanks :)

…turn by the form, because they are ignore by the handler
@jolelievre

This comment has been minimized.

Copy link
Contributor Author

jolelievre commented Oct 29, 2019

Hi @Robin-Fischer-PS

Thank you, nice catch! Actually there was the same problem for other non-required fields (phone numbers, state, ...)

So I fixed it as well The internal problem is that the symfony form transforms empty string into null which was then ignored by the handler I detail this bug here because I'm sure similar bugs are present on other forms

@Robin-Fischer-PS

This comment has been minimized.

Copy link

Robin-Fischer-PS commented Oct 30, 2019

Thanks for the fix @jolelievre , it's OK now ! 😄

@Progi1984 Progi1984 merged commit 5f9a3a3 into PrestaShop:1.7.6.x Oct 30, 2019
2 checks passed
2 checks passed
PrettyCI Code formatting
Details
Travis CI - Pull Request Build Passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.