-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Paypal username field validation #5842
Changes from 4 commits
5c9f6e9
cbdad42
c4a4a0f
a1e8ce1
9a0b0b3
592ea97
85444ab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -135,6 +135,15 @@ function isValidSSNLastFour(ssnLast4) { | |
return CONST.REGEX.SSN_LAST_FOUR.test(ssnLast4); | ||
} | ||
|
||
/** | ||
* | ||
* @param {String} paypalUsername | ||
* @returns {Boolean} | ||
*/ | ||
function isValidPaypalUsername(paypalUsername) { | ||
return paypalUsername && CONST.REGEX.PAYPAL_ME_USERNAME.test(paypalUsername); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A minor change as this condition does not always return boolean. when If someone used this in JSX, It will crash the app. Better to convert There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well thinking more about it, should I change it to: Current behavior to remove the username is deleting the content. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well. I think @stitesExpensify can help here with it. Thanks for bringing this up. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I don't think we should add the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the comment @stitesExpensify There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @parasharrajat I've updated the Regex instead by changing * to +. Let me know if that's fine. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup. that's fine. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This one is remaining. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done now. |
||
} | ||
|
||
/** | ||
* Validate that "date" is between 18 and 150 years in the past | ||
* | ||
|
@@ -226,4 +235,5 @@ export { | |
isValidUSPhone, | ||
isValidURL, | ||
validateIdentity, | ||
isValidPaypalUsername, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NAB: Sorry for being Nit-picky. but this does not sound friendly. You can ask this on Slack.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've asked, lets see if we get any response. Any suggestions from your side?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not really good with content but this is not really blocking this so if you don't get any feedback, then it's fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay got it. Thanks for the help.