Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.56k stars 2.9k forks source link

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

Closed isagoico closed 3 years ago

isagoico commented 3 years ago

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?

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

MelvinBot commented 3 years ago

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

akshayasalvi commented 3 years ago

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

MelvinBot commented 3 years ago

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

Hanaffi commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

I'm not able to even view those option. I made a GH here - https://github.com/Expensify/App/issues/5601

MelvinBot commented 3 years ago

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

michaelhaxhiu commented 3 years ago

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 commented 3 years ago

https://www.upwork.com/jobs/~017adafcefa04af78f

MelvinBot commented 3 years ago

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

kadiealexander commented 3 years ago

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 commented 3 years ago

@deetergp quick reminder for the proposal https://github.com/Expensify/App/issues/5541#issuecomment-928203774

deetergp commented 3 years ago

@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 commented 3 years ago

@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

https://user-images.githubusercontent.com/57435789/136850474-155d7c64-1ac4-48d9-bf6c-77687abc9976.mov

deetergp commented 3 years ago

Thanks for the added context @akshayasalvi.

@arielgreen let's hire @akshayasalvi in Upwork.

parasharrajat commented 3 years ago

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 commented 3 years ago

Settle down Melvin, this has already been merged.

arielgreen commented 3 years ago

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.

botify commented 3 years ago

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

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

arielgreen commented 3 years ago

Paid!