Update display name front-end validation to match back-end #14873

puneetlath · 2023-02-06T19:25:03Z

Details

Updates the display name validation to match the back-end. These are the changes:

Added maxLength prop to first and last name fields
Added validation that "Expensify" or "Concierge" aren't used in the first name field
Updated some of the personal details related error message copy
Did some general refactoring of the display name validation to make it simpler and more straightforward

Fixed Issues

$ #13779

Tests

Go to Settings > Profile > Display name
Keep typing in the first name or last name fields
Verify that the input stops adding more after 50 characters
Add a semicolon or a comma anywhere in either of the fields
Verify that you get a validation error
Add the word "concierge" or the word "expensify" anywhere in either field
Verify that you get a validation error only for the first name field
Use a name that doesn't result in any errors
Verify that you're able to update the name successfully

Verify that no errors appear in the JS console

Offline tests

Same tests. Everything here should work while offline.

QA Steps

Go to Settings > Profile > Display name
Keep typing in the first name or last name fields
Verify that the input stops adding more after 50 characters
Add a semicolon or a comma anywhere in either of the fields
Verify that you get a validation error
Add the word "concierge" or the word "expensify" anywhere in either field
Verify that you get a validation error only for the first name field
Use a name that doesn't result in any errors
Verify that you're able to update the name successfully

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Web

Mobile Web - Chrome

Mobile Web - Safari

Desktop

iOS

Android

MelvinBot · 2023-02-06T22:04:07Z

@mollfpr @youssef-lr One of you needs to copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

puneetlath · 2023-02-06T22:06:24Z

I'm having trouble getting my iOS simulator to work, but otherwise everything should be ready for review.

src/pages/settings/Profile/DisplayNamePage.js

mollfpr · 2023-02-07T06:29:29Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified tests pass on all platforms & I tested again on:
- Android / native
- Android / Chrome
- iOS / native
- iOS / Safari
- MacOS / Chrome / Safari
- MacOS / Desktop
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that any callback methods that were added or modified are named for what the method does and never what callback they handle (i.e. toggleReport and not onIconClick).
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text shown in the product is localized by adding it to src/languages/* files and using the translation method
- I verified all numbers, amounts, dates and phone numbers shown in the product are using the localization methods
- I verified any copy / text that was added to the app is correct English and approved by marketing by adding the Waiting for Copy label for a copy review on the original GH to get the correct copy.
- I verified proper file naming conventions were followed for any new files or renamed files. All non-platform specific files are named after what they export and are not named "index.js". All platform-specific files are named for the platform the code supports as outlined in the README.
- I verified the JSDocs style guidelines (in STYLE.md) were followed
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
I verified all code is DRY (the PR doesn't include any logic written more than once, with the exception of tests)
I verified any variables that can be defined as constants (ie. in CONST.js or at the top of the file that uses the constant) are defined as such
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately and each prop has a /** comment above it */
- The file is named correctly
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- For Class Components, any internal methods passed to components event handlers are bound to this properly so there are no scoping issues (i.e. for onClick={this.submit} the method this.submit should be bound to this in the constructor)
- Any internal methods bound to this are necessary to be bound (i.e. avoid this.submit = this.submit.bind(this); if this.submit is never passed to a component event handler like onClick)
- All JSX used for rendering exists in the render method
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If any new file was added I verified that:
- The file has a description of what it does and/or why is needed at the top of the file if the code is not self explanatory
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If a new page is added, I verified it's using the ScrollView component to make it scrollable when more elements are added to the page.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

14873.Web.mov

Mobile Web - Chrome

14873.mWeb.Chrome.mov

Mobile Web - Safari

14873.mWeb.Safari.mp4

Desktop

14873.Desktop.mov

iOS

14873.iOS.mp4

Android

14873.Android.mov

src/pages/settings/Profile/DisplayNamePage.js

mollfpr · 2023-02-07T07:24:13Z

Input 0 only on the first name, and submitting the form will reset the user display name to the email, but the first name in personalDetails remains 0. No error was returned from the API. I guess the user should be expected to have the first name 0 displayed if we allow it.

Screen.Recording.2023-02-07.at.14.13.13.mov

src/languages/en.js

puneetlath · 2023-02-07T18:31:29Z

Nice catch @mollfpr. I'll look into it.

neil-marcellini · 2023-02-07T18:50:45Z

I'm going to bow out of this PR review for #focus.

puneetlath · 2023-02-08T22:25:48Z

Thanks for the thorough review so far @mollfpr. Ready for another round!

mollfpr

Looks good to me and test well on the RequestCallPage, LegalNamePage, and DisplayNamePage 👍

Here's the checklist!

puneetlath · 2023-02-09T17:36:36Z

Thank you @mollfpr!

@youssef-lr could you also give this a review please?

neil-marcellini · 2023-02-13T18:22:36Z

I can take a look today.

neil-marcellini

Looking pretty great overall. I like how you have simplified this a lot. There's a few small changes I would like to see. Also I think it would be good to include tests for each page that was modified here, just in case.

neil-marcellini · 2023-02-13T20:41:41Z

src/pages/settings/Profile/DisplayNamePage.js

+        if (doesFirstNameHaveInvalidCharacters) {
+            errors.firstName = this.props.translate('personalDetails.error.hasInvalidCharacter');
+        } else if (ValidationUtils.doesContainReservedWord(values.firstName, CONST.DISPLAY_NAME.RESERVED_FIRST_NAMES)) {
+            errors.firstName = this.props.translate('personalDetails.error.containsReservedWord');


:issue We should display multiple error messages at the same time if multiple validations fail, as we state in the form guidelines. For example, if I set my first name to Concierge; I should see First name cannot contain the words Expensify or Concierge. Name cannot contain a comma or semicolon..

suggestion: We can easily join the error messages together.

suggestion: We can easily join the error messages together.

I'm not sure we do this in any of our other forms?

Agreed, could you give an example of a form that does this? Because that would require copy variations and conditions for each combination of errors. Or I guess appending the error to errors.firstName instead of replacing it.

I don't think we currently do this in any of our forms. I'll ask in Slack if we want to do this. If not then I'm happy with what we have and we should update the form guidelines.

Here's the Slack discussion.

I'll make this a NAB comment. If we decide to make the change I'll create a refactor issue.

Alright sounds good. I'm going to go ahead and merge then. Thanks for starting the slack discussion @neil-marcellini.

src/libs/ValidationUtils.js

youssef-lr · 2023-02-14T09:49:03Z

Looks good to me and tested well! Just a question @puneetlath, Keeping the first name empty and last name as 'Concierge' will still result in a display name that says 'Concierge'. Is this fine?

mollfpr · 2023-02-14T10:57:57Z

@youssef-lr Yup, we already addressed that here.

OSBotify · 2023-02-14T19:47:50Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

github-actions · 2023-02-14T20:14:44Z

Performance Comparison Report 📊

Significant Changes To Duration

There are no entries

Meaningless Changes To Duration

Show entries

Name	Duration
App start TTI	701.139 ms → 721.851 ms (+20.712 ms, +3.0%)
App start runJsBundle	191.194 ms → 196.258 ms (+5.065 ms, +2.6%)
Open Search Page TTI	611.937 ms → 612.790 ms (+0.853 ms, ±0.0%)
App start nativeLaunch	19.871 ms → 19.935 ms (+0.065 ms, ±0.0%)
App start regularAppStart	0.014 ms → 0.015 ms (+0.001 ms, +4.8%)

Show details

Name	Duration
App start TTI	Baseline Mean: 701.139 ms Stdev: 38.170 ms (5.4%) Runs: 627.5586509997956 648.6231269999407 653.5004900000058 662.5699780001305 665.4013450001366 666.4061839999631 666.8986269999295 668.6453499998897 670.9554849998094 671.256490000058 677.8653839998879 678.2209310000762 687.1833009999245 690.8923229998909 694.5230479999445 696.1947940001264 697.4001290001906 701.8032349999994 709.5908050001599 712.7879579998553 719.0251009999774 720.601090000011 721.3308080001734 726.8798219999298 727.1316539999098 733.215299999807 733.8502839999273 737.9294909997843 737.9422789998353 750.5055470000952 766.626021000091 813.1427620002069 Current Mean: 721.851 ms Stdev: 25.363 ms (3.5%) Runs: 676.1864249999635 689.1270480002277 689.9225849998184 691.8273860001937 694.0429980000481 698.4690399998799 701.8609640002251 702.1158429998904 702.5430870000273 703.4250659998506 704.3568790000863 709.327219999861 713.0752389999107 713.5576530001126 713.8197630001232 719.089668000117 721.7701909998432 722.2383909998462 724.2163680000231 724.4607489998452 729.046875 734.0915850000456 740.5075980001129 741.2544169998728 741.436015999876 741.7652830001898 742.8596310000867 744.8739490001462 750.1877689999528 757.2173469997942 765.5318700000644 795.0211029998027
App start runJsBundle	Baseline Mean: 191.194 ms Stdev: 22.405 ms (11.7%) Runs: 159 161 161 165 168 170 173 174 177 178 178 178 179 184 184 186 187 194 197 198 198 200 201 207 210 214 215 220 229 232 250 Current Mean: 196.258 ms Stdev: 16.130 ms (8.2%) Runs: 170 173 175 179 179 180 183 183 184 184 186 191 191 192 195 196 196 196 200 201 203 203 205 208 208 210 212 215 216 231 239
Open Search Page TTI	Baseline Mean: 611.937 ms Stdev: 19.276 ms (3.2%) Runs: 580.804525999818 581.6920980000868 584.5612800000235 586.8015949996188 589.1717530000024 589.4432379999198 590.7444659997709 595.9230549996719 598.8281660000794 601.9893390000798 602.0983080002479 603.65677900007 606.7478439998813 607.0749109997414 609.1887210002169 609.885050999932 610.6902669998817 610.8963219998404 612.3330899998546 612.8577069998719 613.0437830002047 619.4685060000047 620.7049160003662 620.8798839999363 622.3158780001104 622.6413579997607 632.7620040001348 635.4674889999442 637.6037189997733 640.1894939998165 645.2804370000958 646.7051190002821 651.472453000024 Current Mean: 612.790 ms Stdev: 18.036 ms (2.9%) Runs: 581.8181969998404 585.6286619999446 586.8728849999607 587.4799810000695 589.7980150002986 593.7728679999709 594.4903569999151 595.3375249998644 600.0264889998361 603.0832520001568 603.9464520001784 606.2943939999677 608.3620609999634 608.9814860001206 609.1951910001226 614.9837650000118 616.0409750002436 617.6699620001018 618.2917479998432 619.9393719998188 623.035279000178 625.1116140000522 625.9863289999776 626.6190590001643 627.4076340002939 627.5345060001127 628.8662509997375 631.7473960001953 644.6124269999564 644.8991299998015 648.6457929997705
App start nativeLaunch	Baseline Mean: 19.871 ms Stdev: 1.755 ms (8.8%) Runs: 17 17 18 18 18 18 18 18 19 19 19 19 19 19 20 20 20 20 20 20 20 21 21 21 21 22 22 23 23 23 23 Current Mean: 19.935 ms Stdev: 2.139 ms (10.7%) Runs: 18 18 18 18 18 18 18 18 18 18 18 19 19 19 19 19 19 20 20 20 20 21 21 21 21 22 23 23 24 25 25
App start regularAppStart	Baseline Mean: 0.014 ms Stdev: 0.001 ms (5.6%) Runs: 0.012410999741405249 0.012653999961912632 0.012817000038921833 0.01310200011357665 0.013183000031858683 0.013305999804288149 0.013427000027149916 0.013509000185877085 0.013591000344604254 0.013671000022441149 0.01383400009945035 0.013915999792516232 0.013915999792516232 0.013916000258177519 0.013956999871879816 0.014078999869525433 0.01448499970138073 0.014526000246405602 0.014526000246405602 0.014607999939471483 0.014648000244051218 0.014689000323414803 0.014689000323414803 0.014771000016480684 0.014852000400424004 0.014892000239342451 0.0148930000141263 0.015015000011771917 0.01509599993005395 0.015136000234633684 0.015381000004708767 Current Mean: 0.015 ms Stdev: 0.001 ms (6.3%) Runs: 0.013427000027149916 0.013590000104159117 0.013753000181168318 0.013793999794870615 0.013794000260531902 0.013833999633789062 0.013874999713152647 0.013916000258177519 0.014200999867171049 0.01436399994418025 0.014403999783098698 0.014444999862462282 0.014485999941825867 0.014525999780744314 0.014526000246405602 0.014607999939471483 0.01464799977838993 0.014771000016480684 0.014810999855399132 0.014811000321060419 0.0148930000141263 0.014934000093489885 0.01509599993005395 0.015461999922990799 0.01554399961605668 0.015666000079363585 0.0157880000770092 0.0163569999858737 0.0163569999858737 0.016885999590158463 0.0170499999076128

OSBotify · 2023-02-15T18:29:25Z

🚀 Deployed to staging by https://github.com/puneetlath in version: 1.2.72-0 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

OSBotify · 2023-02-16T21:37:28Z

🚀 Deployed to production by https://github.com/francoisl in version: 1.2.72-1 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	success ✅
🕸 web 🕸	success ✅

PauloGasparSv · 2023-03-27T10:34:32Z

Hey everyone, following the BugZero checklist for #15741 here.

We implemented the same validation rules for both the "Display name" and the "Legal Name" in this P.R. but I think the latter has different more strict rules (that don't allow special characters for example).

Because of that, newDot allowed some special characters in the "Legal Name" fields but after saving it the API was stripping them away in the Onyx response.

Video of the bug

Bug5968608_Gravar__2078.mp4

allgandalf · 2024-07-16T07:14:01Z

src/pages/settings/Profile/DisplayNamePage.js

+        // Then we validate the last name field
+        if (!ValidationUtils.isValidDisplayName(values.lastName)) {
+            errors.lastName = this.props.translate('personalDetails.error.hasInvalidCharacter');
+        }


We missed out a case here, we shouldn't had allowed the user to submit empty first name, this caused #44167.

Though i agree this is not actually a bug from this PR, but rather a missed logic.

Thanks for pointing it out!

Add display name constants

afa0442

puneetlath self-assigned this Feb 6, 2023

puneetlath added 8 commits February 6, 2023 14:26

Add maxLength on display name inputs

29dc612

Refactor display name validation to remove assignErrors function

7e8f90c

Refactor findInvalidSymbols to be doesContainCommaOrSemicolon

863a0d1

Update copy for display name errorsg

b4ad412

Add reserved word in first name check

2fc2349

Add copy for reserved word error

411b2bf

Use const on request call page

c281fe0

Remove no longer used imports

33b015e

puneetlath marked this pull request as ready for review February 6, 2023 22:03

puneetlath requested a review from a team as a code owner February 6, 2023 22:03

puneetlath requested a review from neil-marcellini February 6, 2023 22:03

melvin-bot bot requested review from mollfpr and youssef-lr and removed request for a team February 6, 2023 22:04

fedirjh reviewed Feb 6, 2023

View reviewed changes

src/pages/settings/Profile/DisplayNamePage.js Outdated Show resolved Hide resolved

puneetlath added 2 commits February 6, 2023 18:07

Remove unncessary max length validation

0c1546d

Use constants for error copy

eeb046c

mollfpr reviewed Feb 7, 2023

View reviewed changes

src/pages/settings/Profile/DisplayNamePage.js Show resolved Hide resolved

fedirjh reviewed Feb 7, 2023

View reviewed changes

src/languages/en.js Show resolved Hide resolved

Merge main into branch

4c539ea

neil-marcellini removed their request for review February 7, 2023 18:50

Add localize import

9b8ae39

mollfpr previously approved these changes Feb 9, 2023

View reviewed changes

Merge main into branch

745867c

puneetlath dismissed mollfpr’s stale review via 745867c February 13, 2023 16:55

Fully merge es.js

00ae704

neil-marcellini self-requested a review February 13, 2023 18:22

neil-marcellini requested changes Feb 13, 2023

View reviewed changes

Change doesContainCommaOrSemicolon to isValidDisplayName

b31fb63

neil-marcellini approved these changes Feb 14, 2023

View reviewed changes

puneetlath merged commit 023e59e into main Feb 14, 2023

puneetlath deleted the reserved-first-names branch February 14, 2023 19:43

neil-marcellini mentioned this pull request Feb 14, 2023

[HOLD for payment 2023-04-03] [HOLD for payment 2023-03-22] [$1000] [Form Refactor] Show multiple error messages simultaneously using Form.js #15156

Closed

OSBotify mentioned this pull request Feb 15, 2023

Deploy Checklist: New Expensify 2023-02-15 #15175

Closed

22 tasks

kbecciv mentioned this pull request Feb 15, 2023

Settings - Display name- The 3 dots do not appear when a long name is entered #15187

Closed

6 tasks

MelvinBot mentioned this pull request Feb 16, 2023

[HOLD for regression test] [$4000] Bug: Entering 0 as a name causes problems #13779

Closed

9 tasks

PauloGasparSv mentioned this pull request Mar 27, 2023

[HOLD for payment 2023-03-24] [$1000] Personal details - Signs *-+ added to the start of a Legal first name and last name don't save #15741

Closed

6 tasks

francoisl mentioned this pull request May 5, 2023

Personal details - Legal name is not showing error when pasting a name > 150 characters #18107

Closed

6 tasks

fedirjh mentioned this pull request Oct 30, 2023

[HOLD for payment 2023-10-30] Large legal name are not saved and no error message is displayed #19821

Closed

6 tasks

mkhutornyi mentioned this pull request Jan 11, 2024

[HOLD for payment 2024-02-07] [$500] Display Name - User can save Expensify and Concierge as a last name while error is thrown #34304

Closed

6 tasks

allgandalf reviewed Jul 16, 2024

View reviewed changes

allgandalf mentioned this pull request Jul 16, 2024

[HOLD for payment 2024-07-22] [HOLD for payment 2024-07-17] [$250] Able to save empty display first name #44167

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update display name front-end validation to match back-end #14873

Update display name front-end validation to match back-end #14873

puneetlath commented Feb 6, 2023 •

edited

Loading

MelvinBot commented Feb 6, 2023

puneetlath commented Feb 6, 2023

mollfpr commented Feb 7, 2023 •

edited

Loading

mollfpr commented Feb 7, 2023

puneetlath commented Feb 7, 2023

neil-marcellini commented Feb 7, 2023

puneetlath commented Feb 8, 2023

mollfpr left a comment

puneetlath commented Feb 9, 2023

neil-marcellini commented Feb 13, 2023

neil-marcellini left a comment

neil-marcellini Feb 13, 2023

youssef-lr Feb 14, 2023

puneetlath Feb 14, 2023

neil-marcellini Feb 14, 2023

neil-marcellini Feb 14, 2023

neil-marcellini Feb 14, 2023

puneetlath Feb 14, 2023

youssef-lr commented Feb 14, 2023 •

edited

Loading

mollfpr commented Feb 14, 2023

OSBotify commented Feb 14, 2023

github-actions bot commented Feb 14, 2023

OSBotify commented Feb 15, 2023

OSBotify commented Feb 16, 2023

PauloGasparSv commented Mar 27, 2023

allgandalf Jul 16, 2024

puneetlath Jul 16, 2024

Update display name front-end validation to match back-end #14873

Update display name front-end validation to match back-end #14873

Conversation

puneetlath commented Feb 6, 2023 • edited Loading

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

MelvinBot commented Feb 6, 2023

puneetlath commented Feb 6, 2023

mollfpr commented Feb 7, 2023 • edited Loading

Reviewer Checklist

Screenshots/Videos

mollfpr commented Feb 7, 2023

puneetlath commented Feb 7, 2023

neil-marcellini commented Feb 7, 2023

puneetlath commented Feb 8, 2023

mollfpr left a comment

Choose a reason for hiding this comment

puneetlath commented Feb 9, 2023

neil-marcellini commented Feb 13, 2023

neil-marcellini left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

youssef-lr commented Feb 14, 2023 • edited Loading

mollfpr commented Feb 14, 2023

OSBotify commented Feb 14, 2023

github-actions bot commented Feb 14, 2023

Performance Comparison Report 📊

Significant Changes To Duration

Meaningless Changes To Duration

OSBotify commented Feb 15, 2023

OSBotify commented Feb 16, 2023

PauloGasparSv commented Mar 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

puneetlath commented Feb 6, 2023 •

edited

Loading

mollfpr commented Feb 7, 2023 •

edited

Loading

youssef-lr commented Feb 14, 2023 •

edited

Loading