Show NotFoundPage if report does not exist in sub report views

[s77rt]

Details

• Show NotFoundPage if report does not exist in sub report views
• Renamed HOC withReportOrNavigateHome to withReportOrNotFound

$ #14513
PROPOSAL: #14513 (comment)

Tests

1. Login to any account
2. Open any report
3. Edit the URL by appending any random number
4. Verify that you see "Hmm... it's not here" message
5. Edit the URL again (keep the random added number) by appending /details
6. Verify that you see "Hmm... it's not here" message

• Verify that no errors appear in the JS console

Offline tests

n/a

QA Steps

1. Login to any account
2. Open any report
3. Edit the URL by appending any random number
4. Verify that you see "Hmm... it's not here" message
5. Edit the URL again (keep the random added number) by appending /details
6. Verify that you see "Hmm... it's not here" message

• Verify that no errors appear in the JS console

PR Author Checklist

• I linked the correct issue in the ### Fixed Issues section above
• I wrote clear testing steps that cover the changes made in this PR
  • I added steps for local testing in the Tests section
  • I added steps for the expected offline behavior in the Offline steps section
  • I added steps for Staging and/or Production testing in the QA steps section
  • I added steps to cover 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 tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
• I included screenshots or videos for tests on all platforms
• I ran the tests on all platforms & verified they passed on:
  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop
• I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
• I followed proper code patterns (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
    • If any non-english text was added/modified, I verified the translation was requested/reviewed in #expensify-open-source and it was approved by an internal Expensify engineer. Link to Slack message:
  • 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 followed the guidelines as stated in the Review Guidelines
• I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
• 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
• I verified that if a function's arguments changed that all usages have also been updated correctly
• 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.
• If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
• I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

Screenshots/Videos

Web

web.mp4

Mobile Web - Chrome

mweb-chrome.mp4

Mobile Web - Safari

mweb-safari.mp4

Desktop

desktop.mp4

iOS

Android

[MelvinBot]

@neil-marcellini Please 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]

[s77rt]

cc @AndrewGable @0xmiroslav Tagging you here, not sure why puller bear didn't assign you.

[0xmiros]

@s77rt let's fix these kinds of warnings in all 3 pages

reproducible step:

web-login.mov

[0xmiros]

@s77rt you can also test deep link on desktop app locally and add video by disabling this:

App/src/components/DeeplinkWrapper/index.website.js

Lines 40 to 42 in e6c486c

	if (!this.isMacOSWeb() || CONFIG.ENVIRONMENT === CONST.ENVIRONMENT.DEV) {
	return;
	}

[s77rt]

@0xmiroslav

let's fix these kinds of warnings in all 3 pages

How are those errors related to this PR? They are reproducible in main. Please report on Slack.

[0xmiros]

I know this PR didn't introduce this but I think it should be handled here because these pages are no longer closed automatically and keep open even if personalDetails are not loaded yet.
@AndrewGable what do you suggest?

[s77rt]

@0xmiroslav

you can also test deep link on desktop app locally and add video by disabling this

Do I have to build a production release for desktop? Will deep link work with just npm run desktop?

[0xmiros]

Do I have to build a production release

It would be good to do that. But no need I think if you just comment the code I shared, you can test enough in debug mode.

[0xmiros]

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

web.mov

web-login.mov

Mobile Web - Chrome

mchrome.mov

Mobile Web - Safari

msafari.mp4

Desktop

desktop.mov

iOS

Android

[0xmiros]

@s77rt you can check my video on Desktop section of reviewer checklist

[s77rt]

Updated

[mountiny]

@s77rt please make sure to not change the PR template. Keep the Fixed issues section as its in the PR template so the correct people are assigned for a review. Thanks 🙇

[s77rt]

@mountiny Add @0xmiroslav as a reviwer too.

Actually I was looking into what went wrong with the template, I see it now, I accidentally deleted the ### Fixed Issues header 😅

[0xmiros]

@mountiny as commented on slack, 3 C+s involved in this issue 😄
The correct C+ assigned here is me and I reviewed both proposal and PR.

[0xmiros]

@AndrewGable should we care about this console error in this PR?
This is already happening on production but just asking because it's minor and this PR is a good place to fix this rather than creating a separate issue.

[mountiny]

Updated :D

[neil-marcellini]

Tests well and looks good to me! I didn't see any console errors with your test steps or the ones that @0xmiroslav posted. Did you already fix that? The console error can be fixed in a follow up if needed.

[0xmiros]

ok, console error is very minor and not caused by this PR. It can be fixed in a follow-up if needed.
All yours @AndrewGable

[OSBotify]

✋ 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]

Performance Comparison Report 📊

Significant Changes To Duration

Name	Duration
Open Search Page TTI	617.298 ms → 673.098 ms (+55.799 ms, +9.0%) 🔴

Show details

Name

Duration

Open Search Page TTI

Baseline Mean: 617.298 ms Stdev: 18.064 ms (2.9%) Runs: 588.4184980001301 592.1979569997638 592.7393800001591 594.950072999578 597.5587979997508 598.0697839995846 603.9114580000751 605.368286000099 605.4157710000873 608.3181560002267 608.9034830001183 609.9308680002578 611.7188729997724 614.0209960001521 615.0172129999846 615.2600500001572 617.1312250001356 618.7430830001831 618.8396809999831 619.910726999864 624.5514739998616 624.7634280002676 625.0420329999179 625.9909260000568 627.7772619999014 630.9551190002821 632.3178710001521 634.9971110001206 652.6120609999634 657.6059159999713 663.2115890001878 Current Mean: 673.098 ms Stdev: 25.125 ms (3.7%) Runs: 625.0555010000244 628.0108639998361 632.8667399999686 633.2386879995465 637.5647789998911 643.472453000024 652.290608999785 656.9536139997654 658.8926589996554 661.6483560004272 664.4795330003835 666.3762210002169 666.5479739997536 668.8810229999945 673.6208899999037 673.7154140002094 675.5555429998785 677.1752929999493 677.4728190000169 678.042440999765 679.1735440003686 680.321411000099 681.2401129999198 687.3820799998939 689.0197350000963 692.9708259999752 695.9942219997756 698.3206790001132 699.0485020000488 706.1991779999807 709.1975099998526 714.0633949995972 727.4274090002291

Meaningless Changes To Duration

Show entries

Name	Duration
App start TTI	728.003 ms → 749.693 ms (+21.690 ms, +3.0%)
App start runJsBundle	201.125 ms → 206.438 ms (+5.313 ms, +2.6%)
App start regularAppStart	0.015 ms → 0.015 ms (+0.000 ms, +3.1%)
App start nativeLaunch	19.733 ms → 18.933 ms (-0.800 ms, -4.1%)

Show details

Name	Duration
App start TTI	Baseline Mean: 728.003 ms Stdev: 27.682 ms (3.8%) Runs: 670.703032999998 683.6412410000339 685.0118040000089 686.3371349999215 696.9910059999675 705.1755719999783 706.1768690000754 708.708772999933 709.9200629999395 712.4362629998941 717.2241469998844 718.4147530000191 718.948842999991 719.9043739999179 722.3466509999707 723.6531140001025 726.3566020000726 726.8272859998979 737.8553079999983 737.87770000007 738.6411510000471 742.9625659999438 744.1353150000796 744.2270210001152 748.836222999962 750.096181999892 751.6387219999451 768.1304850000888 768.8223699999508 772.2858959999867 773.5446240000892 778.2674920000136 Current Mean: 749.693 ms Stdev: 24.120 ms (3.2%) Runs: 707.5837659998797 717.6844279998913 724.0652160001919 725.0144110000692 725.4259589998983 726.1473929998465 728.2470669997856 728.9249950000085 731.834309999831 732.4728680001572 736.6745930002071 739.71513399994 741.5265370002016 743.6269919998012 743.8545039999299 745.1014490001835 747.5975700002164 749.7616880000569 750.2351700002328 751.6140530002303 753.6181930000894 762.4275890002027 764.0643139998429 766.6792009999044 767.1267019999214 773.3614150001667 773.4288770002313 781.7019480001181 795.9079200001433 798.0836370000616 806.9874680000357
App start runJsBundle	Baseline Mean: 201.125 ms Stdev: 20.379 ms (10.1%) Runs: 168 173 174 177 178 179 183 184 185 185 187 190 193 195 199 199 200 200 201 202 207 209 217 217 218 220 220 228 235 236 238 239 Current Mean: 206.438 ms Stdev: 21.420 ms (10.4%) Runs: 165 174 178 185 188 188 188 189 191 192 196 196 197 197 197 198 201 205 211 212 213 216 220 223 224 224 225 237 238 240 249 249
App start regularAppStart	Baseline Mean: 0.015 ms Stdev: 0.001 ms (6.8%) Runs: 0.012776999967172742 0.013428000034764409 0.013590000104159117 0.013752999948337674 0.013794000027701259 0.013794000027701259 0.013875999953597784 0.01395700010471046 0.014118999941274524 0.014160000020638108 0.014200999867171049 0.014282000018283725 0.01432300009764731 0.014485999941825867 0.014567000092938542 0.014688999857753515 0.0147299999371171 0.014770000008866191 0.014771000016480684 0.015015000011771917 0.01509599993005395 0.01525900000706315 0.015298999845981598 0.015300000086426735 0.015381000004708767 0.01558400015346706 0.016032000072300434 0.016316999914124608 0.016561000142246485 0.01672299997881055 0.017089999979361892 Current Mean: 0.015 ms Stdev: 0.001 ms (5.0%) Runs: 0.013630999717861414 0.014038000255823135 0.01407900033518672 0.014444999862462282 0.014527000021189451 0.014567000325769186 0.01464799977838993 0.014648000244051218 0.014688999857753515 0.014769999776035547 0.014810999855399132 0.014852000400424004 0.014891999773681164 0.0148930000141263 0.014973999932408333 0.015137000009417534 0.015217999927699566 0.015217999927699566 0.015217999927699566 0.015461999922990799 0.015584000386297703 0.015585000161081553 0.015666000079363585 0.0157880000770092 0.015910000074654818 0.01599099999293685 0.016032000072300434 0.01607300015166402 0.016193999908864498 0.0163569999858737 0.016682999674230814 0.016763999592512846
App start nativeLaunch	Baseline Mean: 19.733 ms Stdev: 1.569 ms (8.0%) Runs: 18 18 18 18 18 18 18 18 18 18 19 19 19 20 20 20 20 20 20 20 20 20 21 21 21 22 22 22 23 23 Current Mean: 18.933 ms Stdev: 1.731 ms (9.1%) Runs: 17 17 17 18 18 18 18 18 18 18 18 18 18 18 18 18 18 19 19 19 19 19 19 20 20 21 21 21 24 24

[github-actions]

@Expensify/mobile-deployers 📣 Please look into this performance regression as it's a deploy blocker.

[OSBotify]

🚀 Deployed to staging by https://github.com/AndrewGable in version: 1.2.78-0 🚀

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

[OSBotify]

🚀 Deployed to production by https://github.com/mountiny in version: 1.2.78-0 🚀

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