Skip to content

feat: (isMobilePhone) add El Salvador validation#1766

Closed
hereje wants to merge 2 commits into
validatorjs:masterfrom
hereje:feat/add-validator-on-isMobilePhone-for-El_Salvador
Closed

feat: (isMobilePhone) add El Salvador validation#1766
hereje wants to merge 2 commits into
validatorjs:masterfrom
hereje:feat/add-validator-on-isMobilePhone-for-El_Salvador

Conversation

@hereje

@hereje hereje commented Oct 9, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor

feat: (isMobilePhone) add El Salvador validation

Add mobile phone validation for El Salvador

  • country code with ISO: 'es-SV'
  • add entry to README.md file
  • add test case fixture entry for valid and invalid scenarios

Related to issue: #1761

Checklist

  • PR contains only changes related; no stray files, etc.
  • README updated (where applicable)
  • Tests written (where applicable)

@codecov

codecov Bot commented Oct 9, 2021 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #1766 (6c1a29a) into master (13651ea) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master     #1766   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          102       102           
  Lines         2029      2029           
  Branches       457       457           
=========================================
  Hits          2029      2029
Impacted Files Coverage Δ
src/lib/isMobilePhone.js 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 13651ea...6c1a29a. Read the comment docs.

tux-tn
tux-tn requested changes Oct 9, 2021
View reviewed changes

@tux-tn tux-tn left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hello @hereje and thank you for your PR!

The regex should only validate mobile phone numbers, looks like yours is also validating landline numbers. Please make the necessary changes to allow only mobile phones.

You can use this document from ITU or the Wikipieda article to know how mobile phones work in El Salvador

@hereje
fix: (isMobilePhone) add El Salvador validation
6c1a29a 
* Solve issue with wrong validation on El Salvador mobile phones
* update tests accordingly

in order to define the right regEx, the following site was
consulted.

https://www.siget.gob.sv/guia-de-servicios/consulta-el-plan-de-numeracion/numeros-moviles/
@hereje hereje requested a review from tux-tn October 10, 2021 03:27
@hereje

hereje commented Oct 13, 2021

Copy link
Copy Markdown
Contributor Author

Done

tux-tn
tux-tn approved these changes Oct 14, 2021
View reviewed changes

@tux-tn tux-tn left a comment •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for making the necessary changes @hereje ! LGTM 🎉
Is your contribution related to hacktoberfest?

@tux-tn tux-tn added ready-to-land For PRs that are reviewed and ready to be landed and removed 🧹 needs-update For PRs that need to be updated before landing labels Oct 14, 2021
@hereje

hereje commented Oct 18, 2021

Copy link
Copy Markdown
Contributor Author

@tux-tn Yes it is!
I would appreciate if you mark my PR as hacktoberfest-accepted

@tux-tn

tux-tn commented Oct 18, 2021

Copy link
Copy Markdown
Member

Yes sir ! That's why i asked 😃

profnandaa
profnandaa approved these changes Oct 30, 2021
View reviewed changes

@profnandaa profnandaa left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks for your contrib! 🎉

@profnandaa

profnandaa commented Oct 30, 2021

Copy link
Copy Markdown
Member

@hereje -- please fix the merge conflict on README and we should be good.

@profnandaa profnandaa added the mc-to-land Just merge-conflict standing between the PR and landing. label Oct 30, 2021
@profnandaa

profnandaa commented Oct 31, 2021

Copy link
Copy Markdown
Member

Will be merged in #1851, no worries your commit contribution will be preserved. Thanks!

@profnandaa profnandaa closed this Oct 31, 2021
@profnandaa profnandaa mentioned this pull request Oct 31, 2021
Closed
@hereje hereje deleted the feat/add-validator-on-isMobilePhone-for-El_Salvador branch February 13, 2022 04:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@profnandaa profnandaa profnandaa approved these changes

@tux-tn tux-tn tux-tn approved these changes

Assignees

No one assigned

Labels

🎉 first-pr hacktoberfest-accepted mc-to-land Just merge-conflict standing between the PR and landing. ready-to-land For PRs that are reviewed and ready to be landed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hereje @tux-tn @profnandaa