Skip to content

[$500] BA - No validation error when emoji and certain symbols are entered in name field #32291

Description

@lanitochka17

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: 1.4.6.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
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Issue found when executing PR #30674

Action Performed:

  1. Navigate to staging.new.expensify.com
  2. Go to Settings > Workspaces > any workspace
  3. Go to Bank account > Connect manually
  4. Proceed to Step 3
  5. Add @, +, (), [] and emoji in the name field
  6. Submit the form

Expected Result:

Error will show up on the name field with invalid symbols and emoji

Actual Result:

Error does not show up on the name field with invalid symbols and emoji. Only certain symbols pass the validation

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

Add any screenshot/video evidence

Bug6296280_1701365098247.20231130_214539.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01fdad690a07b3c07f
  • Upwork Job ID: 1740328309374783488
  • Last Price Increase: 2024-01-10
  • Automatic offers:
    • 0xmiroslav | Reviewer | 28109979
    • Krishna2323 | Contributor | 28109980

Activity

  1. added
    ExternalAdded to denote the issue can be worked on by a contributor
    BugSomething is broken. Auto assigns a BugZero manager.
    on Nov 30, 2023
  2. melvin-bot commented on Nov 30, 2023

    @melvin-bot

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

  3. melvin-bot commented on Nov 30, 2023

    @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
  4. shubham1206agra commented on Nov 30, 2023

    @shubham1206agra
    Contributor

    Regression from linked PR

  5. 0xmiroslav commented on Dec 3, 2023

    @0xmiroslav
    Contributor

    Not regression. This validation was never happened before #30674.
    Btw, happy to take this as C+

  6. Krishna2323 commented on Dec 3, 2023

    @Krishna2323
    Contributor

    Proposal

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

    BA - No validation error when emoji and certain symbols are entered in name field.

    What is the root cause of that problem?

    We only check validation to determine if the name is empty, there is no validation in place to ensure that the name contains only Latin characters.

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

    We should add an 'if' statement to check whether the name contains only valid firstName & lastName or not. We can use the new function isValidPersonName inside ValidationUtils.

    function isValidPersonName(value: string) {
    return /^[^\d^!#$%*=<>;{}"]+$/.test(value);
    }

    Updated code should look like:

        // Inside `validate` function in `RequestorStep.js`
        if (!ValidationUtils.isValidPersonName(values.firstName)) {
            errors.firstName = 'bankAccount.error.firstName';
        }
    
        if (!ValidationUtils.isValidPersonName(values.lastName)) {
            errors.lastName = 'bankAccount.error.lastName';
        }

    Result

    What alternative solutions did you explore? (Optional)

    Or we can use isValidLegalName instead of isValidPersonName which only permits Latin characters.

    function isValidLegalName(name: string): boolean {
    const hasAccentedChars = Boolean(name.match(CONST.REGEX.ACCENT_LATIN_CHARS));
    return CONST.REGEX.ALPHABETIC_AND_LATIN_CHARS.test(name) && !hasAccentedChars;
    }

  7. trjExpensify commented on Dec 4, 2023

    @trjExpensify
    Contributor

    We only check validation to determine if the name is empty, there is no validation in place to ensure that the name contains only Latin characters.

    So looking at the linked PR we explicitly decided not to exclude special characters in the name field as per here. It's unclear if that matched Onfido's requirements or not? CC: @stitesExpensify as you reviewed that PR as well.

    @0xmiroslav assigning you for the time being.

  8. stitesExpensify commented on Dec 4, 2023

    @stitesExpensify
    Contributor

    We don't check for onfido's requirements on the front end or back end currently. I posted the backend logic on the other issue and we just strip out a specific set of characters. This seems like a nice to have, but honestly if you're putting random random characters and emojis in your name and it fails, I'm not too concerned by that.

  9. stitesExpensify commented on Dec 4, 2023

    @stitesExpensify
    Contributor

    If we want to add @ or + we can, but I'm not aware of any full list of forbidden characters that we have anywhere

  10. 121 remaining items

  11. Krishna2323 commented on Feb 21, 2024

    @Krishna2323
    Contributor

    Raising PR today.

  12. Krishna2323 commented on Feb 21, 2024

    @Krishna2323
    Contributor

    @Ollyws PR ready for review :)

  13. trjExpensify commented on Feb 27, 2024

    @trjExpensify
    Contributor

    PR hit staging 15 hours ago.

  14. Krishna2323 commented on Mar 8, 2024

    @Krishna2323
    Contributor

    @NikkiWines friendly bump for payments here :), PR was deployed to production on 29th Feb.

  15. NikkiWines commented on Mar 8, 2024

    @NikkiWines
    Contributor

    @trjExpensify can you issue payment, please? Thank you 🙇

  16. trjExpensify commented on Mar 11, 2024

    @trjExpensify
    Contributor

    Payment summary:

    @Krishna2323 paid $500 for the fix
    @Ollyws offer sent for $500 for the review

  17. added
    Awaiting PaymentAuto-added when associated PR is deployed to production
    and removed
    ReviewingHas a PR in review
    on Mar 11, 2024
  18. Ollyws commented on Mar 11, 2024

    @Ollyws
    Contributor

    Accepted, thanks.

  19. trjExpensify commented on Mar 11, 2024

    @trjExpensify
    Contributor

    Paid, closing. Thanks, everyone!

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.DailyKSv2EngineeringExternalAdded to denote the issue can be worked on by a contributorWeeklyKSv2

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions