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

Validate phone and email for Organization #2112

Merged
merged 8 commits into from
Feb 17, 2017
Merged

Conversation

snittoor
Copy link

Added semantic tags for organization from schema.org.
Validated email and phone number using regular expression while adding/updating organization.

Regular Expression Explanation for phone no:

// can start with +/# ( 2 digits with or without ()) , can have -/./slash/or space between nos
//eg. +(XX)-XXXX-XXXX , +XX XXXX XXXXX , #X/XXXX/XXXX/XXXX  , XX.XXXX.XXXX ,     XXXXXXXXXXXXX
regEx: /^\+|#?(|[(][0-9]{2})|[)]\)?[-|.|/|\s ]?([0-9]{4})[-|.|/|\s ]?([0-9]{4})$/, 
  1. +|# : may or maynot have +or # in beginning.
  2. ?(|[(][0-9]{2})|[)]) : may or maynot have () , no for 0-9 , two digits
  3. ?[-|.|/|\s ]?([0-9]{4})[-|.|/|\s ]?([0-9]{4})$ : the no can have - or . or / or space between then and 4 digits each.
  4. can enter just digits.
    Issue 'Email' and 'phone' fields not validated in 'Add Organization' form #2034

{{> afQuickField name='url' }}
{{> afQuickField name='description' }}
<span itemprop="name"> {{> afQuickField name='name' }} </span>
<span itemprop="url"> {{> afQuickField name='url' }}</span>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use consistent indentation.

{{> afQuickField name='name' }}
{{> afQuickField name='url' }}
{{> afQuickField name='description' }}
<span itemprop="name"> {{> afQuickField name='name' }} </span>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To improve readability, nest HTML tags like so:

<span itemprop="name">
  {{> afQuickField name='name' }}
</span>

@brylie
Copy link
Contributor

brylie commented Feb 13, 2017

Requested small improvements for readability.

@@ -13,6 +13,9 @@ Organizations.schema = new SimpleSchema({
},
'contact.phone': {
type: String,
// can start with +/# ( 2 digits with or without ()) , can have -/./slash/or space between nos
//eg. +(XX)-XXXX-XXXX , +XX XXXX XXXXX , #X/XXXX/XXXX/XXXX , XX.XXXX.XXXX , XXXXXXXXXXXXX
regEx: /^\+|#?(|[(][0-9]{2})|[)]\)?[-|.|/|\s ]?([0-9]{4})[-|.|/|\s ]?([0-9]{4})$/,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering if this RegEx is too complicated.

@brylie brylie self-assigned this Feb 13, 2017
@snittoor
Copy link
Author

@brylie : I made the RegEx changes as discussed with Taija. These include having + ,( ) , space , + , . ,/ while entering the phone. Right now # and * are not included. Please have a review and let me know the changes.

@@ -13,6 +13,9 @@ Organizations.schema = new SimpleSchema({
},
'contact.phone': {
type: String,
// can start with +/# ( 2 digits with or without ()) , can have -/./slash/or space between
// nos eg. +(XX)-XXXX-XXXX , +XX XXXX XXXXX , XX.XXXX.XXXX , XXXXXXXXXXXXX
regEx: /^\+?(|[(][0-9]{2})|[)]\)?[-|.|/|\s ]?([0-9]{4})[-|.|/|\s ]?([0-9]{4})$/,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Sri,

Just wanted to make sure you understood my earlier comment. I would like you to control only which characters are allowed in the field, but not their order or grouping. But this is really hard for me to read, so I'm not sure what you are doing.

The reason why we do not want to control e.g. grouping is that the grouping of phone numbers depends on the locale (in this case country, not so much language). So just keep it simple.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bajiat : Hi Taija : Ok , will rework on the expression again.

// can start with +/# ( 2 digits with or without ()) , can have -/./slash/or space between
// nos eg. +(XX)-XXXX-XXXX , +XX XXXX XXXXX , XX.XXXX.XXXX , XXXXXXXXXXXXX
regEx: /^\+?(|[(][0-9]{2})|[)]\)?[-|.|/|\s ]?([0-9]{4})[-|.|/|\s ]?([0-9]{4})$/,
regEx: /^[0-9-+()/\s]+$/,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember to add a comment describing why this regular expression is written this way. What characters are you intending to allow?

@brylie brylie modified the milestone: Sprint 36 Feb 17, 2017
@@ -13,6 +13,9 @@ Organizations.schema = new SimpleSchema({
},
'contact.phone': {
type: String,
// all numbers (0-9) , + , - , space ,/ are allowed and can appear anywhere in the phone numbers
//e.g. 754-3010,(541) 754-3010,1-541-754-3010,1-541-754-3010,191 541 754 3010,(089) / 636-48018
regEx: /^[0-9-+()/\s]+$/,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember to allow dot character '.'

@brylie brylie merged commit a5451ed into develop Feb 17, 2017
@brylie brylie deleted the orgEmailPhoneValidate branch February 17, 2017 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants