Skip to content

[HOLD for payment 2024-08-09] [$250] Bank account - No error message is displayed for incorrect validation amounts #45246

Closed

Description

@izarutskaya

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


Version Number: v9.0.6-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: https://expensify.testrail.io/index.php?/tests/view/4709268
Email or phone of affected tester (no customers): applausetester+vd_ios071024@applause.expensifail.com
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

Pre-requisite: user must have created a Workspace and have enabled Workflows.

  1. Go to Settings > Workspaces > Workspace > Workflows.
  2. Tap on "Connect bank account".
  3. Select "Connect manually".
  4. Go trough the add bank account process using the testing credentials.
  5. Once you reach the "Validate your bank account" page, enter 3 random amounts and tap on "Validate" button.
  6. Submit many times.

Expected Result:

"The validate code you entered is incorrect, please try again" error should be displayed. Also, the validation should be disabled due to many tries.

Actual Result:

The error message for incorrect validation amounts is not displayed. Also, the validation is not disabled due to many tries.

Workaround:

Unknown

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Bug6538320_1720642110665.Bexr3345_1_.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~016e89ad61491fffbc
  • Upwork Job ID: 1811487532517648861
  • Last Price Increase: 2024-07-11
Issue OwnerCurrent Issue Owner: @abekkala

Activity

  1. added
    BugSomething is broken. Auto assigns a BugZero manager.
    on Jul 11, 2024
  2. melvin-bot commented on Jul 11, 2024

    @melvin-bot

    Triggered auto assignment to @abekkala (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

  3. izarutskaya commented on Jul 11, 2024

    @izarutskaya
    Author

    We think this issue might be related to the #collect project.

  4. added
    ExternalAdded to denote the issue can be worked on by a contributor
    on Jul 11, 2024
  5. changed the title [-]Bank account - No error message is displayed for incorrect validation amounts[/-] [+][$250] Bank account - No error message is displayed for incorrect validation amounts[/+] on Jul 11, 2024
  6. melvin-bot commented on Jul 11, 2024

    @melvin-bot
  7. added
    Help WantedApply this label when an issue is open to proposals by contributors
    on Jul 11, 2024
  8. melvin-bot commented on Jul 11, 2024

    @melvin-bot

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

  9. Krishna2323 commented on Jul 11, 2024

    @Krishna2323
    Contributor

    Proposal

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

    Bank account - No error message is displayed for incorrect validation amounts

    What is the root cause of that problem?

    No validation is added for amount limit.

    const validate = (values: FormOnyxValues<typeof ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM>): FormInputErrors<typeof ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM> => {
    const errors: FormInputErrors<typeof ONYXKEYS.FORMS.REIMBURSEMENT_ACCOUNT_FORM> = {};
    const amountValues = getAmountValues(values);
    const outputCurrency = policy?.outputCurrency ?? CONST.CURRENCY.USD;
    const amountRegex = RegExp(String.raw`^-?\d{0,8}([${permittedDecimalSeparator}]\d{0,${CurrencyUtils.getCurrencyDecimals(outputCurrency)}})?$`, 'i');
    Object.keys(amountValues).forEach((key) => {
    const value = amountValues[key as keyof AmountValues];
    const filteredValue = filterInput(value, amountRegex, permittedDecimalSeparator);
    if (ValidationUtils.isRequiredFulfilled(filteredValue.toString())) {
    return;
    }
    errors[key as keyof AmountValues] = translate('common.error.invalidAmount');
    });
    return errors;
    };

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

    Add a if block which will compare max/min amount with filteredValue and add error using errors[key as keyof AmountValues] = translate('common.error.invalidAmount');. Error message can be created or can be used from any other form.

    What alternative solutions did you explore? (Optional)

  10. Krishna2323 commented on Jul 11, 2024

    @Krishna2323
    Contributor

    @izarutskaya @abekkala, is there any min/max limit for these inputs? When we should throw an error? I don't understand the bug exactly.

  11. tienifr commented on Jul 12, 2024

    @tienifr
    Contributor

    Proposal

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

    The error message for incorrect validation amounts is not displayed. Also, the validation is not disabled due to many tries.

    What is the root cause of that problem?

    In failure data of VALIDATE_BANK_ACCOUNT_WITH_TRANSACTIONS in

    We don't set the errors, so when the data returned by the back-end does not have the errors set, like in the case of 402 error code (back-end temporary issue) here, no errors will be shown to the users and it looks like the app is not responsive and broken.

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

    Add optimistic errors for code validation similar to the approach here https://github.com/Expensify/App/blob/main/src/libs/actions/BankAccounts.ts#L165

    In this add

    errors: ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('validationStep.validateCodeFailure'),
    

    What alternative solutions did you explore? (Optional)

    Store only the error translation key in Onyx, and use translate to translate it when we display the error

  12. 26 remaining items

  13. melvin-bot commented on Aug 2, 2024

    @melvin-bot

    The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.15-9 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 2024-08-09. 🎊

    For reference, here are some details about the assignees on this issue:

    • @hungvu193 requires payment through NewDot Manual Requests
    • @tienifr requires payment through NewDot Manual Requests
  14. melvin-bot commented on Aug 2, 2024

    @melvin-bot

    BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

    • [@hungvu193] The PR that introduced the bug has been identified. Link to the PR:
    • [@hungvu193] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
    • [@hungvu193] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
    • [@hungvu193] Determine if we should create a regression test for this bug.
    • [@hungvu193] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
    • [@abekkala] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:
  15. abekkala commented on Aug 2, 2024

    @abekkala
    Contributor

    PAYMENT SUMMARY FOR AUG 09, if no regressions

    • Fix: @tienifr [$250] payment via NewDot
    • PR Review: @hungvu193 [$250] payment via NewDot
      please complete checklist
  16. abekkala commented on Aug 8, 2024

    @abekkala
    Contributor

    not overdue payment is tomorrow

  17. hungvu193 commented on Aug 9, 2024

    @hungvu193
    Contributor
    • The PR that introduced the bug has been identified. Link to the PR: Improve VBA logic #6230
    • The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: https://github.com/Expensify/App/pull/6230/files#r1710967043
    • A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
    • Determine if we should create a regression test for this bug: Yes

    Regression test:

    1. Go to Settings > Workspaces > Workspace > Workflows.
    2. Tap on "Connect bank account".
    3. Select "Connect manually" or "Connect with Plaid".
    4. Go through the add bank account process using the testing credentials.
    5. Once you reach the "Validate your bank account" page, enter 3 random amounts and click on the "Validate" button.
    6. Submit many times.
    7. Verify that the error message display at the bottom of the page.

    Do we 👍 or 👎 ?

  18. abekkala commented on Aug 9, 2024

    @abekkala
    Contributor

    PAYMENT SUMMARY FOR AUG 09, if no regressions

  19. JmillsExpensify commented on Aug 14, 2024

    @JmillsExpensify
    Contributor

    $250 approved for @hungvu193

  20. garrettmknight commented on Sep 4, 2024

    @garrettmknight
    Contributor

    $250 approved for @tienifr

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Awaiting PaymentAuto-added when associated PR is deployed to productionBugSomething is broken. Auto assigns a BugZero manager.DailyKSv2ExternalAdded to denote the issue can be worked on by a contributor

Type

No type

Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions