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 custom validation message to international phone number form validation #6650

Merged
merged 21 commits into from Dec 5, 2018

Conversation

Projects
5 participants
@kuzmany
Copy link
Contributor

kuzmany commented Sep 27, 2018

Please be sure you are submitting this against the staging branch.

Q A
Bug fix?
New feature?
Automated tests included?
Related user documentation PR URL
Related developer documentation PR URL mautic/developer-documentation#109
Issues addressed (#s or URLs)
BC breaks?
Deprecations?

Description:

Based on quick fix #6645 and feature added to 2.14.1 #5400 we add option to change validation message for nternational phone number form validation.

This PR also add extend addValidator event with parameter formType, which allow extend validation tab by developers (see docs mautic/developer-documentation#109)

This is example how developers can add options to validator tab and then validate based on setting: https://github.com/mautic/mautic/pull/6650/files#diff-e68b0c198b71013d6f317456841c2abd

image

Steps to test this PR:

1: Create form
2. Create Phone field. See properties tab and option: Validate as internationally-standardized format. Enable it and also setup validation message
3. Try send form with non validated form nubmer (xxx for example)
4. You should see your custom message
5. Then go to phone field. And remove custom validation text message
6. Try send form with non validated form nubmer (xxx for example)
7. You should see default validation message

@escopecz

This comment has been minimized.

Copy link
Member

escopecz commented Oct 3, 2018

@kuzmany as Norman pointed me to this PR from #6645, could you move the validation to the Validation tab as I suggested in this comment?

I still think that the default validation message should be simplified.

@npracht

This comment has been minimized.

Copy link
Member

npracht commented Oct 3, 2018

I'd suggest:

Please use the following international phone number format [+][country code][subscriber number] for this field (eg: ‪+14028650000).

@kuzmany kuzmany added WIP and removed Ready To Test labels Oct 3, 2018

kuzmany added some commits Oct 5, 2018

Extend validation tab of form field
Allow extend by dispatcher for developers

@kuzmany kuzmany referenced this pull request Oct 8, 2018

Merged

Extend validation tab #109

kuzmany added some commits Oct 8, 2018

Merge remote-tracking branch 'upstream/staging' into enh-internationa…
…l-phone-number-validation-custom-text

Resolve conflicts

@kuzmany kuzmany added Ready To Test and removed WIP labels Oct 9, 2018

@npracht npracht added this to To do in Testing 2.15.0 Oct 16, 2018

@Woeler

This comment has been minimized.

Copy link
Member

Woeler commented Nov 6, 2018

I'm not getting through the validator with this imaginary, but true to standards Dutch phone number: ‪+31612345678. I am also getting this instead of the error message mautic.form.submission.phone.invalid.

if (!empty($field->getValidation()['international_validationmsg'])) {
$event->failedValidation($field->getValidation()['international_validationmsg']);
} else {
$event->failedValidation('mautic.form.submission.phone.invalid');

This comment has been minimized.

@Woeler

Woeler Nov 6, 2018

Member

Translation service seems to be missing. See

This comment has been minimized.

@kuzmany

kuzmany Nov 9, 2018

Author Contributor

Good catch. Fixed

@@ -103,7 +103,7 @@ mautic.form.field.type.pagebreak="Page break"
mautic.form.field.type.radiogrp="Radio group"
mautic.form.field.type.textarea="Textarea"
mautic.form.field.type.file="File"
mautic.form.field.type.tel.international="Validate as internationally-standardized format"
mautic.form.field.type.tel.international="Validate as internationally format"

This comment has been minimized.

@Woeler

Woeler Nov 6, 2018

Member

"Validate as international format"

This comment has been minimized.

@npracht
@npracht

npracht approved these changes Nov 7, 2018

Copy link
Member

npracht left a comment

On my side it is working. Just the technical thing from @Woeler to review.

@npracht npracht removed the Ready To Test label Nov 7, 2018

@npracht npracht moved this from To do to Tested once in Testing 2.15.0 Nov 7, 2018

@kuzmany kuzmany added the WIP label Nov 8, 2018

kuzmany added some commits Nov 9, 2018

@dbhurley dbhurley modified the milestone: 2.15.0 Nov 9, 2018

@Woeler

This comment has been minimized.

Copy link
Member

Woeler commented Nov 24, 2018

I still cannot get this to work. I am submitting exactly the number from the error message.
afbeelding

/**
* Class FormValidationSubsriber.
*/
class FormValidationSubsriber extends CommonSubscriber

This comment has been minimized.

@Woeler

Woeler Nov 26, 2018

Member

Class name has a typo FormValidationSubscriber

@kuzmany kuzmany added the WIP label Nov 27, 2018

kuzmany added some commits Nov 27, 2018

@kuzmany kuzmany removed the WIP label Nov 27, 2018

@kuzmany

This comment has been minimized.

Copy link
Contributor Author

kuzmany commented Nov 27, 2018

@Woeler should fixed now. Tested on mautibox

@npracht

This comment has been minimized.

Copy link
Member

npracht commented Nov 29, 2018

@Woeler everything is fine for you now ?

@Woeler

This comment has been minimized.

Copy link
Member

Woeler commented Nov 29, 2018

Mautibox is currently not working with this pr.

@kuzmany

This comment has been minimized.

Copy link
Contributor Author

kuzmany commented Nov 29, 2018

@Woeler @npracht Merge to actual staging, hope helps

@Woeler

Woeler approved these changes Dec 5, 2018

Copy link
Member

Woeler left a comment

This works now.

@Woeler Woeler merged commit dfaed96 into mautic:staging Dec 5, 2018

2 checks passed

Scrutinizer Analysis: 3 new issues, 16 updated code elements – Tests: passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

Testing 2.15.0 automation moved this from Tested once to Merged Dec 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.