[HOLD Paypal feature update][$1000] Web - It does not show error message if PayPal username has leading or trailing space

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Go to Settings> Payments
2. Click on Add payment method button and select PayPal.me option
3. Enter Paypal username with leading space

Expected Result:

It should show invalid username error (same as previous version) or trim username

Actual Result:

It does not show invalid username error and username shifts left side on click of add Paypal account

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.47-2
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Screen.Recording.2023-07-28.at.12.16.21.PM.mov

Recording.3964.mp4

Expensify/Expensify Issue URL:
Issue reported by: @gadhiyamanan
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1690526881707709

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01caff7fb052f5762f
• Upwork Job ID: 1685298133264203776
• Last Price Increase: 2023-08-05

[melvin-bot]

Triggered auto assignment to @maddylewis (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[akinwale]

Proposal

Please re-state the problem that we are trying to solve in this issue.

When adding a PayPal.me username, leading or trailing spaces as part of the username are accepted as valid.

What is the root cause of that problem?

In the validate method in the AddPayPalMePage component, the value provided by values.payPalMeUsername is automatically trimmed before the validation is performed.

What changes do you think we should make in order to solve the problem?

Get the actual raw value from the input field using a state variable and use that as the value to be validated instead of values.payPalMeUsername.

We can make the following changes to achieve this:

1. Add the useState declaration.

const [payPalMeUsername, setPayPalMeUsername] = useState(lodashGet(props.payPalMeData, 'accountData.username', '')

2. Add the onValueChange prop and update the defaultValue prop for the payPalMeUsername TextInput

onValueChange={value => setPayPalMeUsername(value)}
defaultValue={payPalMeUsername}

3. Update the validate method.

if (!ValidationUtils.isValidPaypalUsername(payPalMeUsername)) {
    errors.payPalMeUsername = 'addPayPalMePage.formatError';
}

4. Update the setPayPalMeData callback.

User.addPaypalMeAddress(paypalMeUsername.trim());

What alternative solutions did you explore? (Optional)

None.

[samh-nl]

Proposal

Please re-state the problem that we are trying to solve in this issue.

It does not show error message if PayPal username has leading or trailing space

What is the root cause of that problem?

During validation, it checks the input against the regular expression CONST.REGEX.PAYPAL_ME_USERNAME which only allows alphanumeric strings.

App/src/pages/settings/Payments/AddPayPalMePage.js

Lines 39 to 46 in ba08d3d

	const validate = (values) => {
	const errors = {};
	if (!ValidationUtils.isValidPaypalUsername(values.payPalMeUsername)) {
	errors.payPalMeUsername = 'addPayPalMePage.formatError';
	}
	return errors;
	};

However, the values passed in the validation function are trimmed.

App/src/components/Form.js

Line 127 in ba08d3d

const validationErrors = validate(trimmedStringValues);

Therefore leading to the regular expression passing.

What changes do you think we should make in order to solve the problem?

Option 1. We pass the trimmed value to User.addPaypalMeAddress
Option 2. We can change Form to also pass the raw non-trimmed values as a second argument:

const validate = (values, rawValues) => {

And then we use rawValues.payPalMeUsername for validation.

What alternative solutions did you explore? (Optional)

Alternatively, we can make the trimming behavior optional via a new prop in Form. Or perhaps it make sense that Form also uses the trimmed values when calling submit, since that's what we're validating. This should be discussed in Slack.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01caff7fb052f5762f

[melvin-bot]

Current assignee @maddylewis is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @sobitneupane (External)

[maddylewis]

this is cause by a regression - https://expensify.slack.com/archives/C049HHMV9SM/p1690526905961579?thread_ts=1690526881.707709&cid=C049HHMV9SM

[samh-nl]

Yes, but I think it's also a symptom of a deeper rooted problem with the Form component: https://app.slack.com/client/T03SC9DTT/C01GTK53T8Q/thread/C01GTK53T8Q-1690552108.022579

[vatsal-git]

Proposal

Go to file CONST.js located in 'https://github.com/Expensify/App/blob/main/src/CONST.js#L1131' path.
Go to line 1146
Original code in the line: PAYPAL_ME_USERNAME: /^[a-zA-Z0-9]{1,20}$/,
Replace the line with this: PAYPAL_ME_USERNAME: /^(?! )(?![a-zA-Z0-9]* $)[a-zA-Z0-9]{1,20}$/,

Done and dusted. It will show error if there are any leading or trailing spaces. If the issue is still not solved, I can provide more guidance of what you exactly need to do. I just need to play around with the Regex here and it will be done.

I will also apply for the job at Upwork!

[melvin-bot]

📣 @vatsal-git! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[vatsal-git]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01f1f8db617204ee96

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[muneebkhan4]

Proposal

Please re-state the problem that we are trying to solve in this issue.

When adding a PayPal.me username, leading or trailing spaces as part of the username are accepted as valid which is wrong. It should not add PayPal.me username which has leading or trailing spaces.

What is the root cause of that problem?

The root cause of the problem in the Regex which is validating PAYPAL_ME_USERNAME.

Here it is:

App/src/CONST.js

Line 1146 in eb02006

PAYPAL_ME_USERNAME: /^[a-zA-Z0-9]{1,20}$/,

What changes do you think we should make in order to solve the problem?

We can solve this problem by updating the Regex for PAYPAL_ME_USERNAME in such a way that it also checks for leading or trailing spaces.

What alternative solutions did you explore? (Optional)

Alternatively, we can look into the onInputChange function here:

App/src/components/Form.js

Line 321 in ba08d3d

onInputChange: (value, key) => {

It is quite possible that the leading and trailing spaces are getting trimmed before the Regex validation.
Like here:

App/src/components/Form.js

Line 127 in ba08d3d

const validationErrors = validate(trimmedStringValues);

We can make a check on this not to trim PAYPAL_ME_USERNAME. This will solve the problem.

[mehvash-Khan]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Paypal.me username is being added and saved(after trimming) even when user adds leading/trailing spaces to the name. Paypal username if entered with leading/trailing spaces should throw error.

What is the root cause of that problem?

Root cause of this problem is that we are trimming the PayPal username and then passing that trimmed value to the regex(where it is checking for string validation). Regex PAYPAL_ME_USERNAME is defined to validate if PayPal name does not have any leading/trailing spaces and special characters which in our case is absolutely correct and needs no change. The root cause here is value being passed for regex validation is being trimmed and therefore regex is returning true and hence not giving error for PayPal username with space.

What changes do you think we should make in order to solve the problem?

If we want error message to be displayed when username has leading/trailing space, we will have to pass that value as it is here and not trim it.

Currently our Form component trims every input value during validation as seen here. We can add an additional prop in Form named disableTrim which will include an array of input IDs of those text fields whose values should not be trimmed during validation. In AddPayPalMePage.js we can add prop disableTrim={["payPalMeUsername"]}. Now in our Form component we update our OnValidate function like below:

 const onValidate = useCallback(
        (values) => {
            const trimmedStringValues = {};
            _.each(values, (inputValue, inputID) => {
                if (_.isString(inputValue)) {
                    if(props.disableTrim.includes(inputID))
                        trimmedStringValues[inputID] = inputValue;
                    else
                     trimmedStringValues[inputID] = inputValue.trim();
                } else {
                    trimmedStringValues[inputID] = inputValue;
                }
            });

Default value for prop disableTrim can be an empty array so that no other component is affected.

What alternative solutions did you explore? (Optional)

[mehvash-Khan]

Adding a video here with the above change.

Screen.Recording.2023-07-30.at.5.07.35.PM.mov

[melvin-bot]

⚠️ Unable to store contributor details cc @thienlnam

[maddylewis]

@sobitneupane - will you review these proposals when you have a chance? thanks!

[sobitneupane]

Thanks for the proposal everyone.

Proposal by @samh-nl looks good to me.

Option 1. We pass the trimmed value to User.addPaypalMeAddress

If we don't mind user adding spaces in the beginning and end for paypal address, we can go with the first option.

Option 2. We can change Form to also pass the raw non-trimmed values as a second argument:

But to align with previous condition before having Form refactor, we can pass the non-trimmed value as 2nd argument from form component itself and use it where ever required.

@samh-nl Are there any other places where rawValues can be useful?

[sobitneupane]

Started a discussion for expected output: https://expensify.slack.com/archives/C01GTK53T8Q/p1690999903863719

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[samh-nl]

@sobitneupane I think arguably all form validation should use the non-trimmed values (or alternatively, submit should pass trimmed values to make it consistent with what is actually validated), and if a field is required ValidationUtils.isRequiredFulfilled should be used, which already does trimming and checks if it's empty.

If we want to limit the scope to PayPal: if there are surrounding spaces this seems like a user typo since PayPal.me usernames cannot contain spaces, so option 1 seems reasonable.

[maddylewis]

@sobitneupane - do we think this is worth fixing based on this comment? https://expensify.slack.com/archives/C01GTK53T8Q/p1691006515807909?thread_ts=1690999903.863719&cid=C01GTK53T8Q

[melvin-bot]

Triggered auto assignment to @Christinadobrzyn (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[maddylewis]

@Christinadobrzyn - i am OOO until ~Aug 20. we are chatting in Slack about the proposed fix for this one - #23791 (comment)

That said, @JmillsExpensify made a comment here that makes me think this might not be worth fixing if we're going to deprecate this feature altogether at some point - https://expensify.slack.com/archives/C01GTK53T8Q/p1691006515807909?thread_ts=1690999903.863719&cid=C01GTK53T8Q

[Christinadobrzyn]

I'll follow this thread over the coming days to see if this should stay open. https://expensify.slack.com/archives/C01GTK53T8Q/p1691006515807909?thread_ts=1690999903.863719&cid=C01GTK53T8Q

[JmillsExpensify]

I don't have a formal proposal at the moment, to be clear. I do think we should do this eventually though.

[melvin-bot]

@sobitneupane, @maddylewis, @Christinadobrzyn Whoops! This issue is 2 days overdue. Let's get this updated quick!

[Christinadobrzyn]

It sounds like this should be on hold for now. Going to move this to monthly for tracking

[sophiepintoraetz]

We are deprecating paypal (decision here, GH here) so closing this out, thanks!