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 international phone number form validation #6645

Merged
merged 3 commits into from Oct 3, 2018

Conversation

Projects
5 participants
@kuzmany
Copy link
Contributor

kuzmany commented Sep 26, 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
Issues addressed (#s or URLs) #6633
BC breaks?
Deprecations?

Description:

Enhancement validation for phone number form field in Mautic 2.14.1 didn't work properly.
#5400
This PR fixed the condition When validation is skipped (everytime If validaton is disabled from properties tab).

Steps to reproduce the bug:

  1. Create from and phone field type with disable required and validation from properties tab
  2. Go to to form (mauticurl.com/form/idForm) and try send it.
  3. Form should return validation error event the option for validation is disabled

Steps to test this PR:

  1. Repeat all steps
  2. See If validation is use just when is enabled
@npracht
Copy link
Member

npracht left a comment

Does not work, we still have the issue.

kuzmany added some commits Sep 26, 2018

@dgopinionpublica

This comment has been minimized.

Copy link

dgopinionpublica commented Sep 26, 2018

with the last change , is working fine! thanks a lot!

@kuzmany kuzmany changed the title Fix international form validation Fix international phone number form validation Sep 26, 2018

@npracht
Copy link
Member

npracht left a comment

Now working. Good job 👍

@YosuCadilla

This comment has been minimized.

Copy link

YosuCadilla commented Sep 28, 2018

Trying to replicate for test confirmation, unable to understand steps. Please elaborate and make it understandable for testing newbies.

@npracht

This comment has been minimized.

Copy link
Member

npracht commented Oct 2, 2018

@YosuCadilla

  1. Create a form with a phone type field.
  2. Go to field properties tab and enable the international validation
  3. Try to submit the form with a wrong number ---> see the control
  4. Try to submit with a correct number ---> see it works
@YosuCadilla

This comment has been minimized.

Copy link

YosuCadilla commented Oct 2, 2018

Thank you Norman,

I've had partial success only, depending on how the form is added to the LP.

When I add the "automatic" Form code to the LP it gets duplicated after hitting "Apply".
The first Submit button does nothing, the second returns an error 500.

When I use the {form#} method form seems to simply not work.

@escopecz escopecz added this to Tested Once in Testing 2.14.2 Oct 2, 2018

@escopecz escopecz self-assigned this Oct 2, 2018

@escopecz

This comment has been minimized.

Copy link
Member

escopecz commented Oct 2, 2018

I was able to replicate the error. When I checked out this PR I was able to submit the form properly. But I wasn't able to submit it if I enable the validation. I get this message:

The phone number is not in internationally-standardized E.164 format. E.164 numbers can have a maximum of fifteen digits and are usually written as follows: [+][country code][subscriber number including area code].

It's very long and not really user friendly. It causes problems in the UI where the most important part is hidden behind the submit button:
screen shot 2018-10-02 at 22 40 33

@dbhurley can you assist with more user friendly message?

Maybe something like

Phone number must be in format [+][country code][subscriber number including area code] like +100123456789.

Another UX issue is that the validation configuration is in a wrong tab:
edit form phone validation mautic

@kuzmany could you please move it into the validation tab?

I was able to submit with my number successfully.

@escopecz escopecz moved this from Tested Once to Pending Feedback in Testing 2.14.2 Oct 2, 2018

@npracht

This comment has been minimized.

Copy link
Member

npracht commented Oct 3, 2018

@escopecz we're improving that to give the opportunity to people to write a custom validation message (like a mandatory field), please see #6650

@escopecz

This comment has been minimized.

Copy link
Member

escopecz commented Oct 3, 2018

@npracht thanks for the link. I'll approve this bug fix then as the UI/UX is handled by #6650.

@escopecz escopecz merged commit 2122fd8 into mautic:staging Oct 3, 2018

2 checks passed

Scrutinizer Analysis: No new issues – Tests: passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@escopecz escopecz removed the Code Review label Oct 3, 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.