Paypal username field validation #5842

akshayasalvi · 2021-10-14T04:09:19Z

cc - @parasharrajat

Details

Added validations for Paypal.me username

Fixed Issues

$ #5541

Tests

Tested invalid with special characters
Tested invalid with spaces
Tested valid with alphanumeric characters in

QA Steps

Go to Sidebar -> Payments
Select Paypal to add a username
Try updating with the special characters, spaces. Paypal username allows only alphabets and numbers only.

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

deetergp

Code tests well, but there are two typos that need to be addressed.

deetergp · 2021-10-14T18:57:22Z

src/languages/en.js

@@ -282,6 +282,7 @@ export default {
        addPayPalAccount: 'Add PayPal account',
        editPayPalAccount: 'Update PayPal account',
        growlMessageOnSave: 'Your PayPal username was successfully added',
+        formatError: 'Invalid PayPa.me username',


Missing an l — should be PayPal.me

deetergp · 2021-10-14T18:58:09Z

src/languages/es.js

@@ -282,6 +282,7 @@ export default {
        addPayPalAccount: 'Agregar cuenta de PayPal',
        growlMessageOnSave: 'Su nombre de usuario de PayPal se agregó correctamente',
        editPayPalAccount: 'Actualizar cuenta de PayPal',
+        formatError: 'Usuario PayPa.me no válido',


Also missing the l in PayPal

akshayasalvi · 2021-10-14T19:06:47Z

Code tests well, but there are two typos that need to be addressed.

@deetergp PR updated. Typos fixed. Sorry about that.

deetergp · 2021-10-14T19:09:53Z

@akshayasalvi Typos happen — no big deal!

parasharrajat

Few changes. @deetergp Please wait before the merge.

parasharrajat · 2021-10-14T19:14:33Z

src/languages/en.js

@@ -282,6 +282,7 @@ export default {
        addPayPalAccount: 'Add PayPal account',
        editPayPalAccount: 'Update PayPal account',
        growlMessageOnSave: 'Your PayPal username was successfully added',
+        formatError: 'Invalid PayPal.me username',


NAB: Sorry for being Nit-picky. but this does not sound friendly. You can ask this on Slack.

I've asked, lets see if we get any response. Any suggestions from your side?

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.

Okay got it. Thanks for the help.

parasharrajat · 2021-10-14T19:15:36Z

src/pages/settings/Payments/AddPayPalMePage.js

+     *
+     * @returns {Boolean}
+     */
+    validatePaypalMeUsername() {


Let's move this to ValidationUtils.js

parasharrajat · 2021-10-14T19:16:24Z

src/pages/settings/Payments/AddPayPalMePage.js

@@ -92,6 +114,9 @@ class AddPayPalMePage extends React.Component {
                                placeholder={this.props.translate('addPayPalMePage.yourPayPalUsername')}
                                onChangeText={text => this.setState({payPalMeUsername: text})}
                                returnKeyType="done"
+                                hasError={this.state.payPalMeUsernameError}
+                                errorText={this.state.payPalMeUsernameError ? this.props.translate('addPayPalMePage.formatError') : ''}
+


We can remove this empty line.

parasharrajat · 2021-10-14T19:21:19Z

src/pages/settings/Payments/AddPayPalMePage.js

@@ -92,6 +114,9 @@ class AddPayPalMePage extends React.Component {
                                placeholder={this.props.translate('addPayPalMePage.yourPayPalUsername')}
                                onChangeText={text => this.setState({payPalMeUsername: text})}


Please clear the error on the onChangeText

parasharrajat · 2021-10-14T19:21:56Z

@akshayasalvi Could you please add [HOLD] on the title?

akshayasalvi · 2021-10-14T19:41:06Z

@parasharrajat @deetergp PR updated

parasharrajat · 2021-10-14T20:46:39Z

src/libs/ValidationUtils.js

+ * @returns {Boolean}
+ */
+function isValidPaypalUsername(paypalUsername) {
+    return paypalUsername && CONST.REGEX.PAYPAL_ME_USERNAME.test(paypalUsername);


A minor change as this condition does not always return boolean. when paypalUsername is ''.

If someone used this in JSX, It will crash the app. Better to convert paypalUsername in boolean.

Well thinking more about it, should I change it to:
!paypalUsername || CONST.REGEX.PAYPAL_ME_USERNAME.test(paypalUsername)

Current behavior to remove the username is deleting the content.

Well. I think @stitesExpensify can help here with it. Thanks for bringing this up.

Yeah I don't think we should add the !paypalUsername since I'm adding a way to delete it right now

Thanks for the comment @stitesExpensify

@parasharrajat I've updated the Regex instead by changing * to +. Let me know if that's fine.

Yup. that's fine.

This one is remaining.

akshayasalvi · 2021-10-17T13:41:47Z

@parasharrajat Any changes required in this PR?

parasharrajat

LGTM. Tests well.

Ready for final review and possibly merge cc: @deetergp

deetergp

Looks good, thanks for the changes. There was a merge conflict on the list of exports from src/libs/ValidationUtils.js which I resolved. It dismissed @parasharrajat's review, but since that export list is the only thing that changed since his last approval, I am going to go ahead and merge before we end up with another merge conflict.

Edit: There's also a [HOLD] on it. That needs to be removed before I can merge.

akshayasalvi · 2021-10-19T22:34:59Z

@deetergp Thank you for resolving the conflict. The Hold was added for n6-hold and hence I've removed it from the title. Can we merge this?

OSBotify · 2021-10-19T22:46:52Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

OSBotify · 2021-10-23T01:32:24Z

🚀 Deployed to staging by @deetergp in version: 1.1.8-10 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2021-10-28T20:57:16Z

🚀 Deployed to production by @roryabraham in version: 1.1.10-2 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

akshayasalvi added 2 commits October 13, 2021 23:59

Added paypal.me username validation

5c9f6e9

Fixed typos and flags for paypal validation

cbdad42

akshayasalvi requested a review from a team as a code owner October 14, 2021 04:09

MelvinBot requested review from deetergp and removed request for a team October 14, 2021 04:09

deetergp requested changes Oct 14, 2021

View reviewed changes

Corrected typo for paypal

c4a4a0f

deetergp previously approved these changes Oct 14, 2021

View reviewed changes

parasharrajat suggested changes Oct 14, 2021

View reviewed changes

Moved validate func to ValidationUtils, removed error on text change

a1e8ce1

akshayasalvi dismissed deetergp’s stale review via a1e8ce1 October 14, 2021 19:39

akshayasalvi changed the title ~~Paypal username field validation~~ [Hold] Paypal username field validation Oct 14, 2021

parasharrajat suggested changes Oct 14, 2021

View reviewed changes

Changed regex for Paypal username

9a0b0b3

Made the check boolean

592ea97

parasharrajat previously approved these changes Oct 18, 2021

View reviewed changes

Merge branch 'main' into paypal-field-validation

85444ab

deetergp dismissed parasharrajat’s stale review via 85444ab October 19, 2021 21:38

deetergp approved these changes Oct 19, 2021

View reviewed changes

akshayasalvi changed the title ~~[Hold] Paypal username field validation~~ Paypal username field validation Oct 19, 2021

deetergp merged commit 33768ca into Expensify:main Oct 19, 2021

OSBotify mentioned this pull request Oct 23, 2021

Deploy Checklist: New Expensify 2021-10-23 #6017

Closed

botify mentioned this pull request Oct 28, 2021

[HOLD for payment 2021-11-04] PayPal.me - Don't allow spaces or special characters when adding a PayPal.me #5541

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Paypal username field validation #5842

Paypal username field validation #5842

akshayasalvi commented Oct 14, 2021 •

edited

Loading

deetergp left a comment

deetergp Oct 14, 2021

deetergp Oct 14, 2021

akshayasalvi commented Oct 14, 2021

deetergp commented Oct 14, 2021

parasharrajat left a comment

parasharrajat Oct 14, 2021

akshayasalvi Oct 14, 2021

parasharrajat Oct 14, 2021

akshayasalvi Oct 14, 2021

parasharrajat Oct 14, 2021

akshayasalvi Oct 14, 2021

parasharrajat Oct 14, 2021

akshayasalvi Oct 14, 2021

parasharrajat Oct 14, 2021

akshayasalvi Oct 14, 2021

parasharrajat commented Oct 14, 2021

akshayasalvi commented Oct 14, 2021

parasharrajat Oct 14, 2021

akshayasalvi Oct 14, 2021

parasharrajat Oct 14, 2021

stitesExpensify Oct 15, 2021

akshayasalvi Oct 15, 2021

akshayasalvi Oct 15, 2021

parasharrajat Oct 15, 2021

parasharrajat Oct 17, 2021

akshayasalvi Oct 17, 2021

akshayasalvi commented Oct 17, 2021

parasharrajat left a comment

deetergp left a comment •

edited

Loading

akshayasalvi commented Oct 19, 2021

OSBotify commented Oct 19, 2021

OSBotify commented Oct 23, 2021

OSBotify commented Oct 28, 2021

		@@ -92,6 +114,9 @@ class AddPayPalMePage extends React.Component {
		placeholder={this.props.translate('addPayPalMePage.yourPayPalUsername')}
		onChangeText={text => this.setState({payPalMeUsername: text})}

Paypal username field validation #5842

Paypal username field validation #5842

Conversation

akshayasalvi commented Oct 14, 2021 • edited Loading

Details

Fixed Issues

Tests

QA Steps

Tested On

Screenshots

Web

Mobile Web

Desktop

iOS

Android

deetergp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akshayasalvi commented Oct 14, 2021

deetergp commented Oct 14, 2021

parasharrajat left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

parasharrajat commented Oct 14, 2021

akshayasalvi commented Oct 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akshayasalvi commented Oct 17, 2021

parasharrajat left a comment

Choose a reason for hiding this comment

deetergp left a comment • edited Loading

Choose a reason for hiding this comment

akshayasalvi commented Oct 19, 2021

OSBotify commented Oct 19, 2021

OSBotify commented Oct 23, 2021

OSBotify commented Oct 28, 2021

akshayasalvi commented Oct 14, 2021 •

edited

Loading

deetergp left a comment •

edited

Loading