fix(isAlpha): fix ح character validation in fa-IR language code (#1400) #1455

fakhrip · 2020-10-05T03:49:47Z

I added some Persian alphabet characters retrieved from wikipedia on this line
Also add new validation regarding the language in the validator.js

This PR specifically fix #1400

Checklist

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

codecov · 2020-10-05T03:53:28Z

Codecov Report

Merging #1455 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1455   +/-   ##
=======================================
  Coverage   99.92%   99.92%           
=======================================
  Files          96       96           
  Lines        1275     1276    +1     
=======================================
+ Hits         1274     1275    +1     
  Misses          1        1

Impacted Files	Coverage Δ
src/lib/alpha.js	`100.00% <100.00%> (ø)`
src/lib/isPostalCode.js	`100.00% <0.00%> (ø)`
src/lib/isMobilePhone.js	`100.00% <0.00%> (ø)`
src/lib/isPassportNumber.js	`100.00% <0.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 7916dfd...a83805e. Read the comment docs.

profnandaa · 2020-10-05T11:19:17Z

Thanks for your contribution! Please rebase your branch with master so as to remove the unrelated changes, then we can review.

fakhrip · 2020-10-05T12:55:33Z

I have removed all unnecessary changes, please tell me if i do something wrong here.

profnandaa

LGTM, thanks for your contrib 🎉 // Just update the README and we should be good to go.

fakhrip · 2020-10-06T05:28:09Z

Im pretty sure that there are nothing to be updated in the readme file regarding these changes as its already well-documented there (in the readme) @profnandaa

Edit: I've checked all the boxes so its all clear

profnandaa · 2020-10-06T07:55:33Z

src/lib/alpha.js

@@ -25,6 +25,7 @@ export const alpha = {
  'uk-UA': /^[А-ЩЬЮЯЄIЇҐі]+$/i,
  'vi-VN': /^[A-ZÀÁẠẢÃÂẦẤẬẨẪĂẰẮẶẲẴĐÈÉẸẺẼÊỀẾỆỂỄÌÍỊỈĨÒÓỌỎÕÔỒỐỘỔỖƠỜỚỢỞỠÙÚỤỦŨƯỪỨỰỬỮỲÝỴỶỸ]+$/i,
  'ku-IQ': /^[ئابپتجچحخدرڕزژسشعغفڤقکگلڵمنوۆھەیێيطؤثآإأكضصةظذ]+$/i,
+  'fa-IR': /^[ابپتثجچحخدذرزژسشصضطظعغفقکگلمنوهی]+$/i,


Can move this to L10 so that it's in alphabetic order.

Sure thing!,
ill do it tonight (got to pray first)

profnandaa · 2020-10-06T08:03:35Z

@fakhrip -- my bad, thought it was a new locale.
However, see what you need to fix in L94-103 since it will override what you have added.

fakhrip · 2020-10-06T10:42:36Z

@fakhrip -- my bad, thought it was a new locale.
However, see what you need to fix in L94-103 since it will override what you have added.

Im sorry but i didnt get it, in which file should i see this ?

profnandaa · 2020-10-07T17:32:52Z

That's alpha.js; let me know if there's anything that should be done.

fakhrip · 2020-10-08T02:22:38Z

Can we just add this to line 121 in alpha.js to kind of re-override it ?
alpha['fa-IR'] = alpha['fa-IR'];

profnandaa · 2020-10-08T14:08:31Z

Ok, that works, you can drop in a comment before that.

fakhrip · 2020-10-09T06:32:09Z

Done, if there are anything i should do, please tell me 🙏

profnandaa

LGTM, thanks for your contrib once again! 🎉

fakhrip · 2020-10-10T02:02:35Z

Sure, thank you so much for being good as this is my first legit PR 😃

profnandaa · 2020-10-10T07:40:55Z

Welcome buddy, looking forward to seeing more from you :)

tux-tn · 2020-11-08T10:30:30Z

@profnandaa @fakhrip i don't understand the purpose of the change in L124 😅

fakhrip · 2020-11-08T11:41:23Z

It was overriden by L101 @tux-tn ,
So there i have to "reoverride" it to set the fa-ir to fa-ir (it may sound confusing though, haha)

tux-tn · 2020-11-08T12:29:37Z

I still don't get it, since alpha array has already been mutated alpha['fa-ir'] received alpha['fa'] content, how is re-overriding it is supposed to bring back the old content?
Logging content before and after the line shows the same value

fakhrip · 2020-11-08T16:29:23Z

export const farsiLocales = [
  'IR', 'AF',
];

for (let locale, i = 0; i < farsiLocales.length; i++) {
  locale = `fa-${farsiLocales[i]}`;
  alpha[locale] = alpha.fa;  // This line changes alpha[fa-IR] content to alpha.fa
  alphanumeric[locale] = alphanumeric.fa;
  decimal[locale] = decimal.fa;
}

And because that the content of alpha.fa and alpha[fa-IR] is different (look below)

'fa-IR': /^[ابپتثجچحخدذرزژسشصضطظعغفقکگلمنوهی]+$/i,
fa: /^['0-9آاءأؤئبپتثجچحخدذرزژسشصضطظعغفقکگلمنوهةی۱۲۳۴۵۶۷۸۹۰']+$/i,

It will then be needed to re-override the alpha[fa-IR] that was changed to alpha.fa, to then become alpha[fa-IR] itself

tux-tn · 2020-11-11T16:11:53Z

alpha[locale] = alpha.fa; alpha[fa-IR] has been mutated and contains content of alpha.fa here, right?
Since `alpha[fa_IR] has already been mutated where is stored the old value that you need to reassign?

fakhrip · 2020-11-12T03:19:45Z

Oh wait, i just realized that i was wrong XD, i'm very sorry dude, now i feel like really stupid...

fakhrip · 2020-11-12T03:20:35Z

Do you think i need to fix this in another PR as this one already got merged, or what should i do best ?

tux-tn · 2020-11-12T19:50:04Z

@fakhrip actually i'm asking the question because i'm working on a PR to update dependencies (including eslint who keeps screaming about that line) i fixed it in my branch and i'll probably create a PR by the end of the week.

fakhrip · 2020-11-13T06:59:28Z

@fakhrip actually i'm asking the question because i'm working on a PR to update dependencies (including eslint who keeps screaming about that line) i fixed it in my branch and i'll probably create a PR by the end of the week.

Sure, you're statement is all good, it was my fault, i'm sorry @tux-tn

tux-tn · 2020-11-13T13:53:16Z

@fakhrip no problem my friend, thank you for taking the time to answer my questions. It's not a big deal

profnandaa · 2020-11-15T22:37:42Z

thanks for the due diligence on this @tux-tn

profnandaa added the 🧹 needs-update For PRs that need to be updated before landing label Oct 5, 2020

feat(isAlpha) fix ح character validation in fa-IR language code (#1400)

1829e76

profnandaa reviewed Oct 5, 2020

View reviewed changes

profnandaa reviewed Oct 6, 2020

View reviewed changes

feat: change locale to match lexical order

e37084f

profnandaa removed the 🧹 needs-update For PRs that need to be updated before landing label Oct 7, 2020

feat: reoverride fa-IR language code

a83805e

profnandaa changed the title ~~feat(isAlpha): feat(isAlpha) fix ح character validation in fa-IR language code (#1400)~~ fix(isAlpha): fix ح character validation in fa-IR language code (#1400) Oct 9, 2020

profnandaa approved these changes Oct 9, 2020

View reviewed changes

profnandaa merged commit 0c55656 into validatorjs:master Oct 9, 2020

tux-tn mentioned this pull request Nov 24, 2020

Multiple fixes related to alpha, tests and build #1528

Merged

3 tasks

profnandaa mentioned this pull request Nov 29, 2020

release staging: Nov 2020 release notes #1535

Merged

This was referenced Mar 12, 2021

chore(deps): bump validator from 9.4.1 to 13.5.2 in /api-server Dman247/freeCodeCamp#185

Open

Bump validator from 13.1.17 to 13.5.2 razee-io/Razeedash-api#825

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(isAlpha): fix ح character validation in fa-IR language code (#1400) #1455

fix(isAlpha): fix ح character validation in fa-IR language code (#1400) #1455

fakhrip commented Oct 5, 2020 •

edited

Loading

codecov bot commented Oct 5, 2020 •

edited

Loading

profnandaa commented Oct 5, 2020

fakhrip commented Oct 5, 2020

profnandaa left a comment

fakhrip commented Oct 6, 2020 •

edited

Loading

profnandaa Oct 6, 2020

fakhrip Oct 6, 2020

profnandaa commented Oct 6, 2020

fakhrip commented Oct 6, 2020

profnandaa commented Oct 7, 2020

fakhrip commented Oct 8, 2020 •

edited

Loading

profnandaa commented Oct 8, 2020

fakhrip commented Oct 9, 2020

profnandaa left a comment

fakhrip commented Oct 10, 2020

profnandaa commented Oct 10, 2020

tux-tn commented Nov 8, 2020

fakhrip commented Nov 8, 2020

tux-tn commented Nov 8, 2020

fakhrip commented Nov 8, 2020

tux-tn commented Nov 11, 2020 •

edited

Loading

fakhrip commented Nov 12, 2020

fakhrip commented Nov 12, 2020

tux-tn commented Nov 12, 2020

fakhrip commented Nov 13, 2020

tux-tn commented Nov 13, 2020

profnandaa commented Nov 15, 2020

fix(isAlpha): fix ح character validation in fa-IR language code (#1400) #1455

fix(isAlpha): fix ح character validation in fa-IR language code (#1400) #1455

Conversation

fakhrip commented Oct 5, 2020 • edited Loading

Checklist

codecov bot commented Oct 5, 2020 • edited Loading

Codecov Report

profnandaa commented Oct 5, 2020

fakhrip commented Oct 5, 2020

profnandaa left a comment

Choose a reason for hiding this comment

fakhrip commented Oct 6, 2020 • edited Loading

profnandaa Oct 6, 2020

Choose a reason for hiding this comment

fakhrip Oct 6, 2020

Choose a reason for hiding this comment

profnandaa commented Oct 6, 2020

fakhrip commented Oct 6, 2020

profnandaa commented Oct 7, 2020

fakhrip commented Oct 8, 2020 • edited Loading

profnandaa commented Oct 8, 2020

fakhrip commented Oct 9, 2020

profnandaa left a comment

Choose a reason for hiding this comment

fakhrip commented Oct 10, 2020

profnandaa commented Oct 10, 2020

tux-tn commented Nov 8, 2020

fakhrip commented Nov 8, 2020

tux-tn commented Nov 8, 2020

fakhrip commented Nov 8, 2020

tux-tn commented Nov 11, 2020 • edited Loading

fakhrip commented Nov 12, 2020

fakhrip commented Nov 12, 2020

tux-tn commented Nov 12, 2020

fakhrip commented Nov 13, 2020

tux-tn commented Nov 13, 2020

profnandaa commented Nov 15, 2020

fakhrip commented Oct 5, 2020 •

edited

Loading

codecov bot commented Oct 5, 2020 •

edited

Loading

fakhrip commented Oct 6, 2020 •

edited

Loading

fakhrip commented Oct 8, 2020 •

edited

Loading

tux-tn commented Nov 11, 2020 •

edited

Loading