Skip to content
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

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

Closed
isagoico opened this issue Sep 27, 2021 · 21 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Weekly KSv2

Comments

@isagoico
Copy link

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Navigate to settings
  2. Click on payments
  3. Click on add new payment method
  4. Click on the Paypal option
  5. Enter a PayPal.me user name with spaces and special characters

Expected Result:

User should not be able to save a username with spaces and special characters

Actual Result:

User is able to save the PayPal.me username with spaces and special characters

Workaround:

None needed.

Platform:

Where is this issue occurring?

  • Web
  • iOS
  • Android
  • Desktop App
  • Mobile Web

Version Number: 1.1.2-5

Reproducible in staging?: yes
Reproducible in production?: yes

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation
image

Expensify/Expensify Issue URL:
Issue reported by: @akshayasalvi
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1632504499064300

View all open jobs on GitHub

@isagoico isagoico added Engineering Daily KSv2 Improvement Item broken or needs improvement. labels Sep 27, 2021
@MelvinBot
Copy link

Triggered auto assignment to @pecanoro (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@isagoico isagoico removed the Improvement Item broken or needs improvement. label Sep 27, 2021
@akshayasalvi
Copy link
Contributor

Solution:

We add a validate function and error in AddPayPalMePage.js.

  constructor(props) {
        this.state = {
            payPalMeUsername: props.payPalMeUsername,
            payPalMeUsernameError: false,
        };
    }


    validatePaypalMeUsername() {
        const regex = '/^([a-zA-Z0-9 _-]*)$/';
        if(this.state.payPalMeUsername && regex.test(this.state.payPalMeUsername)) {
            this.setState({ payPalMeUsernameError: true });
            return true;
        }
        this.setState({ payPalMeUsernameError: false});
        return false;
    }

    setPayPalMeUsername() {
        const isValid = validatePaypalMeUsername();
        if(!isValid) {
            return;
        }

        // Rest of the code
    }



    render() {

        // Other code


        <ExpensiTextInput
            label={this.props.translate('addPayPalMePage.payPalMe')}
            error={this.state.payPalMeUsernameError ? this.props.translate('addPayPalMePage.formatError')}
        />

    }

and then accordingly add translations in en.js and es.js

@pecanoro pecanoro added the Improvement Item broken or needs improvement. label Sep 28, 2021
@pecanoro pecanoro removed their assignment Sep 28, 2021
@pecanoro pecanoro added Weekly KSv2 External Added to denote the issue can be worked on by a contributor and removed Daily KSv2 labels Sep 28, 2021
@MelvinBot
Copy link

Triggered auto assignment to @michaelhaxhiu (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@MelvinBot MelvinBot added Daily KSv2 and removed Weekly KSv2 labels Sep 28, 2021
@Hanaffi
Copy link

Hanaffi commented Sep 28, 2021

Proposal

class AddPayPalMePage extends React.Component {
    constructor(props) {
        super(props);

        this.state = {
            payPalMeUsername: props.payPalMeUsername,
            paypalError:false // NEW
        };
        this.setPayPalMeUsername = this.setPayPalMeUsername.bind(this);
        this.paypalUsernameInputRef = null;
    }

    // Rest of code ...

    handleTextChange(text){ // NEW
        if(/\s/.test(text)){
            this.setState({error:true})
        }
        this.setState({payPalMeUsername: text})
    }

    render() {
        return (
             // Rest of code ...
        
                            <ExpensiTextInput
                                label={this.props.translate('addPayPalMePage.payPalMe')}
                                ref={el => this.paypalUsernameInputRef = el}
                                autoCompleteType="off"
                                autoCorrect={false}
                                value={this.state.payPalMeUsername}
                                placeholder={this.props.translate('addPayPalMePage.yourPayPalUsername')}
                                onChangeText={this.handleTextChange}  // NEW
                                returnKeyType="done"
                            />
                    //Rest of code
                        <Button
                            success
                            isDisabled={error}  // NEW
                            onPress={this.setPayPalMeUsername}
                            pressOnEnter
                            style={[styles.mt3]}
                            text={this.props.payPalMeUsername
                                ? this.props.translate('addPayPalMePage.editPayPalAccount')
                                : this.props.translate('addPayPalMePage.addPayPalAccount')}
                        />

        );
    }
}

@michaelhaxhiu
Copy link
Contributor

michaelhaxhiu commented Sep 29, 2021

I'm not able to reproduce this on web or iOS. When I tap "Payments" I get stuck on this white page.

Web:
image
iOS:

@isagoico were you able to reproduce this one on your side?

@isagoico
Copy link
Author

Weird, not able to reproduce that blank screen on my side 🤔 Do you have a bank account added? looks like somethingis wrong with the bankName
image

@michaelhaxhiu
Copy link
Contributor

I'm not able to even view those option. I made a GH here - #5601

@michaelhaxhiu michaelhaxhiu removed their assignment Oct 5, 2021
@MelvinBot MelvinBot removed the Overdue label Oct 5, 2021
@michaelhaxhiu michaelhaxhiu added Overdue External Added to denote the issue can be worked on by a contributor and removed External Added to denote the issue can be worked on by a contributor labels Oct 5, 2021
@MelvinBot
Copy link

Triggered auto assignment to @arielgreen (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@michaelhaxhiu
Copy link
Contributor

I'm re-applying the External label because I'm blocked from reproducing this myself. It seems to be working for most others.

Associated bug is filed already

@arielgreen
Copy link
Contributor

@MelvinBot MelvinBot removed the Overdue label Oct 6, 2021
@MelvinBot MelvinBot added Weekly KSv2 Help Wanted Apply this label when an issue is open to proposals by contributors and removed Daily KSv2 labels Oct 6, 2021
@MelvinBot
Copy link

Triggered auto assignment to @deetergp (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@kadiealexander
Copy link
Contributor

Please refer to this post for updated information on the n6 hold, we've raised the bonus to $250 for all issues where a PR is created before the N6 hold is lifted.

@akshayasalvi
Copy link
Contributor

@deetergp quick reminder for the proposal #5541 (comment)

@deetergp
Copy link
Contributor

@akshayasalvi's solution looks good for the most part. I'd love some reference that proves [a-zA-Z0-9 _-] are the only permissible characters for a PayPal.me link. And since we are not doing anything with the results of the capture group, we don't really need it do we?

Screen Shot 2021-10-11 at 1 00 03 PM

@akshayasalvi
Copy link
Contributor

akshayasalvi commented Oct 11, 2021

@deetergp You're right can remove the capture group.

Here's the screencast while adding any space or a special character it doesn't allow and surrounds with a red border. Only numbers and characters work. So it seems the regex will get updates to remove _ and -.

Updated regex
/^[a-zA-Z0-9]+$/

Screencast

Screen.Recording.2021-10-12.at.1.44.09.AM.mov

@deetergp
Copy link
Contributor

Thanks for the added context @akshayasalvi.

@arielgreen let's hire @akshayasalvi in Upwork.

@parasharrajat
Copy link
Member

I will be reviewing the PR and I will let you know @deetergp when this is ready for final review and possibly for merge. See More

@deetergp
Copy link
Contributor

Settle down Melvin, this has already been merged.

@MelvinBot MelvinBot removed the Overdue label Oct 20, 2021
@arielgreen
Copy link
Contributor

Apologies for the delay @akshayasalvi ! I've sent over the offer. You'll also be paid the $250 bonus for reporting this, as well as the $250 n6-hold bonus.

@MelvinBot MelvinBot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 27, 2021
@botify botify added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Oct 28, 2021
@botify
Copy link

botify commented Oct 28, 2021

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.10-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2021-11-04. 🎊

@botify botify changed the title PayPal.me - Don't allow spaces or special characters when adding a PayPal.me [HOLD for payment 2021-11-04] PayPal.me - Don't allow spaces or special characters when adding a PayPal.me Oct 28, 2021
@arielgreen
Copy link
Contributor

Paid!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Engineering External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement. Weekly KSv2
Projects
None yet
Development

No branches or pull requests