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

[$1000] Routing Number shows an error before typing anything #24044

Closed
6 tasks done
kavimuru opened this issue Aug 2, 2023 · 21 comments
Closed
6 tasks done

[$1000] Routing Number shows an error before typing anything #24044

kavimuru opened this issue Aug 2, 2023 · 21 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Reviewing Has a PR in review Weekly KSv2

Comments

@kavimuru
Copy link

kavimuru commented Aug 2, 2023

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- Click on your avatar
2- click on Worspaces option
3- select any workspace
4- Click on bank account option
5- click on Connect manually
6- Close the keyboard without typing anything

Expected Result:

Routing Number shouldn't show any error before typing ( Account Number works perfectly)

Actual Result:

Routing Number shows an error before typing anything

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

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.49-0
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_20230730-163338.mp4
Recording.1430.mp4

Expensify/Expensify Issue URL:
Issue reported by: @mejed-alkoutaini
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1690724240140599

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01c0ac78d2c9b90a4f
  • Upwork Job ID: 1686753426700144640
  • Last Price Increase: 2023-08-02
@kavimuru kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Aug 2, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 2, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Aug 2, 2023

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

@ygshbht
Copy link
Contributor

ygshbht commented Aug 2, 2023

Proposal

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

The issue is that the Routing Number field in the form shows a validation error prematurely when the keyboard is closed without entering any input.

What is the root cause of that problem?

The root cause is the inconsistency between the initial values of inputValues and lastValidatedValues.current. When the form is rendered, inputValues for a field is set to an empty string if it is undefined, but lastValidatedValues.current for that field remains undefined.

App/src/components/Form.js

Lines 98 to 104 in 577152d

const [inputValues, setInputValues] = useState({...props.draftValues});
const formRef = useRef(null);
const formContentRef = useRef(null);
const inputRefs = useRef({});
const touchedInputs = useRef({});
const isFirstRender = useRef(true);
const lastValidatedValues = useRef({...props.draftValues});

App/src/components/Form.js

Lines 268 to 271 in 577152d

// We want to initialize the input value if it's undefined
if (_.isUndefined(inputValues[inputID])) {
inputValues[inputID] = defaultValue || '';
}

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

One solution is to set lastValidatedValues.current to an empty string as well if it is undefined for a field. This can be done in the childrenWrapperWithProps function like so:

if (_.isUndefined(inputValues[inputID])) {
    inputValues[inputID] = defaultValue || '';
    if (_.isUndefined(lastValidatedValues.current[inputID])) {
        lastValidatedValues.current[inputID] = defaultValue || '';
    }
}

This change ensures that inputValues and lastValidatedValues.current are consistent when the form is initially rendered, which will prevent the form from showing a premature validation error for the Routing Number field.

This logic can also be done out of this callback function when the values are initially being set

@flaviadefaria flaviadefaria added the External Added to denote the issue can be worked on by a contributor label Aug 2, 2023
@melvin-bot melvin-bot bot changed the title Routing Number shows an error before typing anything [$1000] Routing Number shows an error before typing anything Aug 2, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 2, 2023

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 2, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 2, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Aug 2, 2023

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

@hanzalahsamana
Copy link

Proposal,

I am submitting my proposal to resolve the "Routing Number" error display issue in the FormAlertWrapper component. The current problem displays an error message prematurely, even before any user input. To address this, I will make the following changes:

Conditional Error Rendering: I will update the FormAlertWrapper to show error messages only when there are validation issues or when users attempt to submit incomplete forms.

Enhanced Offline Handling: The component will handle offline scenarios gracefully, displaying appropriate messages to inform users about their connection status.

Accessibility & Localization: I will ensure that the error messages are accessible and support multi-language localization.

My goal is to deliver a polished solution that improves the user experience by providing accurate error feedback and seamless form interactions.

I am excited to collaborate on this project and welcome any specific preferences or additional requirements you may have.

Thank you for considering my proposal. I am eager to contribute to your project's success.

Best regards,
Hanzalah Imran

@melvin-bot
Copy link

melvin-bot bot commented Aug 2, 2023

📣 @hanzalahsamana! 📣
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.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@hanzalahsamana
Copy link

Expensify account email: hanzalahsamana789@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~019b0a755f7bda1754

@ares-dev05
Copy link

Proposal

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

The issue is that the Routing Number field in the form shows a validation error prematurely when the keyboard is closed without entering any input.

What is the root cause of that problem?

When the keyboard is closed without entering any input, the Routing Number field is automatically filled with an empty string. Subsequently, when you select another field or lose focus on the Routing Number field, the validation process is triggered. However, since the Routing Number is empty, it is currently displaying an error.

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

If the Routing Number field is an empty string, we need to display an error message stating "Please input the Routing Number field." If you prefer not to show this error message, we can prevent the error from being displayed when the Routing Number field is empty. However, this approach may introduce potential new issues, so it is recommended to display the error message as "Please input the Routing Number field."

In BankAccountManualStep.js, you can find validate function and then we can update the error message.

const validate = useCallback(
    (values) => {
        const errorFields = {};
        const routingNumber = values.routingNumber && values.routingNumber.trim();

        if (
            !values.accountNumber ||
            (!CONST.BANK_ACCOUNT.REGEX.US_ACCOUNT_NUMBER.test(values.accountNumber.trim()) && !CONST.BANK_ACCOUNT.REGEX.MASKED_US_ACCOUNT_NUMBER.test(values.accountNumber.trim()))
        ) {
            errorFields.accountNumber = 'bankAccount.error.accountNumber';
        } else if (values.accountNumber === routingNumber) {
            errorFields.accountNumber = translate('bankAccount.error.routingAndAccountNumberCannotBeSame');
        }
        if (!routingNumber) {
            errorFields.routingNumber = 'bankAccount.error.routingNumberEmpty';
        }
        if (!CONST.BANK_ACCOUNT.REGEX.SWIFT_BIC.test(routingNumber) || !ValidationUtils.isValidRoutingNumber(routingNumber)) {
            errorFields.routingNumber = 'bankAccount.error.routingNumber';
        }
        if (!values.acceptTerms) {
            errorFields.acceptTerms = 'common.error.acceptTerms';
        }

        return errorFields;
    },
    [translate],
);

And in en.js, add the error message.
image

What alternative solutions did you explore? (Optional)

We can ignore the empty string error in the validate function.
image
const validate = useCallback(
(values) => {
const errorFields = {};
const routingNumber = values.routingNumber && values.routingNumber.trim();

    if (
        !values.accountNumber ||
        (!CONST.BANK_ACCOUNT.REGEX.US_ACCOUNT_NUMBER.test(values.accountNumber.trim()) && !CONST.BANK_ACCOUNT.REGEX.MASKED_US_ACCOUNT_NUMBER.test(values.accountNumber.trim()))
    ) {
        errorFields.accountNumber = 'bankAccount.error.accountNumber';
    } else if (values.accountNumber === routingNumber) {
        errorFields.accountNumber = translate('bankAccount.error.routingAndAccountNumberCannotBeSame');
    }
    if (routingNumber && (!CONST.BANK_ACCOUNT.REGEX.SWIFT_BIC.test(routingNumber) || !ValidationUtils.isValidRoutingNumber(routingNumber))) {
        errorFields.routingNumber = 'bankAccount.error.routingNumber';
    }
    if (!values.acceptTerms) {
        errorFields.acceptTerms = 'common.error.acceptTerms';
    }

    return errorFields;
},
[translate],

);

@mudassar69
Copy link

mudassar69 commented Aug 3, 2023

Proposal About Bank Account Manual Step - Form Validation Enhancement

Problem Statement:

In the Bank Account Manual Step component, there is an issue with form validation where error messages persist even after the user closes the input field without entering any data. This creates a confusing user experience as the errors should disappear when the user starts typing.

Root Cause:

The issue arises from how the error state is managed in the component. The current implementation does not handle the scenario when the user closes the input field without entering any data, resulting in the error message remaining visible.

Proposed Solution:

Please check my fixed solution in the below video now no errors are shown initially only errors are shown on continue button pressed:

Fixed Video Demo

Expensify.Bug.Solution.mp4

To fix the form validation issue and ensure a smooth user experience, the following changes are recommended:

  • Introduce isFormSubmitted State: Add a new state property isFormSubmitted to track whether the form has been submitted.

  • Modiied the validate Method: I have adjusted the validate method to set errors for the input fields only when the form is submitted (isFormSubmitted is true). If the form has not been submitted, the method will return an empty error object, ensuring that errors disappear when the user closes the input field without entering any data.

  • Modified the submit Method: In the submit method, I have set the isFormSubmitted state to true when the form is submitted. This will enable the validation to occur based on the user's interaction with the form.

  • Removed onFocus and onBlur: I have temporarily remove the onFocus and onBlur event handlers from the input fields. These event handlers were causing issues and will be re-evaluated for a better solution in the future.

Updated Code:

Relative Path of file: src/pages/ReimbursementAccount/BankAccountManualStep.js

import React from 'react';
import {Image} from 'react-native';
import lodashGet from 'lodash/get';
import HeaderWithBackButton from '../../components/HeaderWithBackButton';
import CONST from '../../CONST';
import * as BankAccounts from '../../libs/actions/BankAccounts';
import Text from '../../components/Text';
import TextInput from '../../components/TextInput';
import styles from '../../styles/styles';
import CheckboxWithLabel from '../../components/CheckboxWithLabel';
import TextLink from '../../components/TextLink';
import withLocalize from '../../components/withLocalize';
import * as ValidationUtils from '../../libs/ValidationUtils';
import ONYXKEYS from '../../ONYXKEYS';
import exampleCheckImage from './exampleCheckImage';
import Form from '../../components/Form';
import shouldDelayFocus from '../../libs/shouldDelayFocus';
import ScreenWrapper from '../../components/ScreenWrapper';
import StepPropTypes from './StepPropTypes';

const propTypes = {
    ...StepPropTypes,
};

class BankAccountManualStep extends React.Component {
    constructor(props) {
        super(props);
        this.state = {
            errors: {},
            isFormSubmitted: false, // New state property to track form submission
        };
        this.submit = this.submit.bind(this);
        this.validate = this.validate.bind(this);
        this.handleInputChange = this.handleInputChange.bind(this);
    }

    handleInputChange(inputID, value) {
        // Clear the error for the specific input when the user starts typing
        this.setState((prevState) => ({
            errors: {
                ...prevState.errors,
                [inputID]: undefined,
            },
        }));
    }

    validate(values) {
        // Only set the errors if the form is submitted
        if (this.state.isFormSubmitted) {
            const errorFields = {};
            const routingNumber = values.routingNumber && values.routingNumber.trim();

            if (
                !values.accountNumber ||
                (!CONST.BANK_ACCOUNT.REGEX.US_ACCOUNT_NUMBER.test(values.accountNumber.trim()) && !CONST.BANK_ACCOUNT.REGEX.MASKED_US_ACCOUNT_NUMBER.test(values.accountNumber.trim()))
            ) {
                errorFields.accountNumber = this.props.translate('bankAccount.error.accountNumber');
            } else if (values.accountNumber === routingNumber) {
                errorFields.accountNumber = this.props.translate('bankAccount.error.routingAndAccountNumberCannotBeSame');
            }
            if (!routingNumber || !CONST.BANK_ACCOUNT.REGEX.SWIFT_BIC.test(routingNumber) || !ValidationUtils.isValidRoutingNumber(routingNumber)) {
                errorFields.routingNumber = this.props.translate('bankAccount.error.routingNumber');
            }
            if (!values.acceptTerms) {
                errorFields.acceptTerms = this.props.translate('common.error.acceptTerms');
            }

            this.setState({errors: errorFields});
            return errorFields;
        }
        return {};
    }

    submit(values) {
        // Set isFormSubmitted to true when the form is submitted
        this.setState({isFormSubmitted: true});

        // If values of routing number and account number are empty give error
        if (!values.routingNumber || !values.accountNumber) {
            // alert('Please enter routing number and account number');
            const errorFields = {};
            const routingNumber = values.routingNumber && values.routingNumber.trim();

            if (
                !values.accountNumber ||
                (!CONST.BANK_ACCOUNT.REGEX.US_ACCOUNT_NUMBER.test(values.accountNumber.trim()) && !CONST.BANK_ACCOUNT.REGEX.MASKED_US_ACCOUNT_NUMBER.test(values.accountNumber.trim()))
            ) {
                errorFields.accountNumber = 'bankAccount.error.accountNumber';
            } else if (values.accountNumber === routingNumber) {
                errorFields.accountNumber = this.props.translate('bankAccount.error.routingAndAccountNumberCannotBeSame');
            }
            if (!routingNumber || !CONST.BANK_ACCOUNT.REGEX.SWIFT_BIC.test(routingNumber) || !ValidationUtils.isValidRoutingNumber(routingNumber)) {
                errorFields.routingNumber = 'bankAccount.error.routingNumber';
            }
            if (!values.acceptTerms) {
                errorFields.acceptTerms = 'common.error.acceptTerms';
            }

            this.setState({errors: errorFields});
            return errorFields;
        } else {
            BankAccounts.connectBankAccountManually(
                lodashGet(this.props.reimbursementAccount, 'achData.bankAccountID') || 0,
                values.accountNumber,
                values.routingNumber,
                lodashGet(this.props, ['reimbursementAccountDraft', 'plaidMask']),
            );
        }
    }

    render() {
        const shouldDisableInputs = Boolean(lodashGet(this.props.reimbursementAccount, 'achData.bankAccountID'));

        return (
            <ScreenWrapper includeSafeAreaPaddingBottom={false}>
                <HeaderWithBackButton
                    title={this.props.translate('workspace.common.connectBankAccount')}
                    stepCounter={{step: 1, total: 5}}
                    shouldShowGetAssistanceButton
                    guidesCallTaskID={CONST.GUIDES_CALL_TASK_IDS.WORKSPACE_BANK_ACCOUNT}
                    onBackButtonPress={this.props.onBackButtonPress}
                />
                <Form
                    formID={ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM}
                    onSubmit={this.submit}
                    validate={this.validate}
                    // Remove onFocus/onBlur when we have a better solution for
                    submitButtonText={this.props.translate('common.continue')}
                    style={[styles.mh5, styles.flexGrow1]}
                >
                    <Text style={[styles.mb5]}>{this.props.translate('bankAccount.checkHelpLine')}</Text>
                    <Image
                        resizeMode="contain"
                        style={[styles.exampleCheckImage, styles.mb5]}
                        source={exampleCheckImage(this.props.preferredLocale)}
                    />
                    <TextInput
                        autoFocus
                        shouldDelayFocus={shouldDelayFocus}
                        inputID="routingNumber"
                        label={this.props.translate('bankAccount.routingNumber')}
                        defaultValue={this.props.getDefaultStateForField('routingNumber', '')}
                        keyboardType={CONST.KEYBOARD_TYPE.NUMBER_PAD}
                        disabled={shouldDisableInputs}
                        shouldSaveDraft
                        shouldUseDefaultValue={shouldDisableInputs}
                        error={this.state.errors.routingNumber} // Pass the error message to the TextInput component
                        onChangeText={(value) => this.handleInputChange('routingNumber', value)}
                    />
                    <TextInput
                        inputID="accountNumber"
                        containerStyles={[styles.mt4]}
                        label={this.props.translate('bankAccount.accountNumber')}
                        defaultValue={this.props.getDefaultStateForField('accountNumber', '')}
                        keyboardType={CONST.KEYBOARD_TYPE.NUMBER_PAD}
                        disabled={shouldDisableInputs}
                        shouldSaveDraft
                        shouldUseDefaultValue={shouldDisableInputs}
                        error={this.state.errors.accountNumber} // Pass the error message to the TextInput component
                        onChangeText={(value) => this.handleInputChange('accountNumber', value)}
                    />

                    <CheckboxWithLabel
                        style={styles.mt4}
                        inputID="acceptTerms"
                        LabelComponent={() => (
                            <Text>
                                {this.props.translate('common.iAcceptThe')}
                                <TextLink href={CONST.TERMS_URL}>{this.props.translate('common.expensifyTermsOfService')}</TextLink>
                            </Text>
                        )}
                        defaultValue={this.props.getDefaultStateForField('acceptTerms', false)}
                        shouldSaveDraft
                        error={this.state.errors.acceptTerms} // Pass the error message to the CheckboxWithLabel component
                    />
                </Form>
            </ScreenWrapper>
        );
    }
}

BankAccountManualStep.propTypes = propTypes;
export default withLocalize(BankAccountManualStep);

Please review these changes in which I have ensured that error messages are displayed correctly during form submission and cleared appropriately when the user interacts with the form. It will improve the overall user experience and streamline the validation process in the Bank Account Manual Step component.

Looking forward to fixing it for production as well!
Thanks
Regards

@melvin-bot
Copy link

melvin-bot bot commented Aug 3, 2023

📣 @mudassar69! 📣
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.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@mudassar69
Copy link

Contributor details
Expensify account email: razamudassar867@gmail.com
Upwork Profile Link: https://www.upwork.com/freelancers/~012f6c0781bee2ce23

@melvin-bot melvin-bot bot added the Overdue label Aug 4, 2023
@chiragxarora
Copy link
Contributor

Regression from #23306

@joh42 @eVoloshchak @techievivek

@chiragxarora
Copy link
Contributor

expected behaviour here should be that upon touching account no. field and blurring it, error on account no. should also be shown, like it happens on routing field

@hungvu193 does your proposal solve #21838 and keep the touched errors too? If yes, then i think we should revert this PR and proceed with your proposal

@flaviadefaria
Copy link
Contributor

@thesahindia what do you think about @chiragxarora's comment above?

@melvin-bot melvin-bot bot removed the Overdue label Aug 7, 2023
@joh42
Copy link
Contributor

joh42 commented Aug 7, 2023

As the person who authored #23306, I agree with @chiragxarora. I don't think we can fix this regression while keeping the solution we implemented in that PR.

CC @eVoloshchak @techievivek

@thesahindia
Copy link
Member

@flaviadefaria, since the regression period isn't over we should remove the External and Help wanted label. The original PR needs to be reverted and we will be able to close this issue.

cc: @eVoloshchak @techievivek

@techievivek
Copy link
Contributor

techievivek commented Aug 8, 2023

I think we can put this on HOLD for now until we have decided the next step here, we can remove the external and help wanted label.

@flaviadefaria flaviadefaria removed External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors labels Aug 8, 2023
@flaviadefaria flaviadefaria added Weekly KSv2 and removed Daily KSv2 labels Aug 8, 2023
@flaviadefaria flaviadefaria changed the title [$1000] Routing Number shows an error before typing anything [HOLD][$1000] Routing Number shows an error before typing anything Aug 8, 2023
@flaviadefaria
Copy link
Contributor

Ok, I removed the 2 labels, put the issue on hold, and switched the priority to weekly. We'll wait for the next steps from you @techievivek.

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Weekly KSv2 labels Aug 9, 2023
@techievivek techievivek changed the title [HOLD][$1000] Routing Number shows an error before typing anything [$1000] Routing Number shows an error before typing anything Aug 10, 2023
@techievivek
Copy link
Contributor

We have reverted the original fix #21838 (comment) that caused this regression, so we can close this now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

10 participants