Pass down url parameters when the native app is opened from browser

[allroundexperts]

Details

The link to the desktop app does not forward query parameters. The DetailsPage component expects route params present. Without it, the app crashes.

Fixed Issues

$ #15059
PROPOSAL: #15059 (comment)

Tests

1. Create .env file in the root folder and add ENVIRONMENT=staging to it.
2. Run both web app (npm run web) and desktop app (npm run desktop).
3. On web app and navigate to a user profile
4. Refresh the browser
5. A prompt should appear asking to 'Open the app in Electron'.
6. Click on it and wait for the desktop app to open.
7. Verify that app opens up correctly without any crashes.

• Verify that no errors appear in the JS console

Offline tests

Not relevant

QA Steps

1. Run both web app (npm run web) and desktop app (npm run desktop).
2. On web app and navigate to a user profile
3. Refresh the browser
4. A prompt should appear asking to 'Open the app in Electron'.
5. Click on it and wait for the desktop app to open.
6. Verify that app opens up correctly without any crashes.

• 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
  • 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.
• I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

Screenshots/Videos

Screen.Recording.2023-02-18.at.2.09.41.AM.mov

Screen.Recording.2023-02-25.at.3.58.57.AM.mov

Screen.Recording.2023-02-25.at.4.05.50.AM.mov

Screen.Recording.2023-02-25.at.4.11.46.AM.mov

Screen.Recording.2023-02-25.at.4.13.30.AM.mov

[MelvinBot]

@marcochavezf @parasharrajat 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]

[parasharrajat]

@allroundexperts As we know hash can be useful too so let's add Hash to the URL as well.

[parasharrajat]

@allroundexperts Can you please improve your commit messages? These messages are useless. Try to explain what changes you did.

[allroundexperts]

Thank you for the feedback @parasharrajat. I have updated the code and commit message. Please let me know if there's anything else that needs to be changed.

[parasharrajat]

Please update the title of this PR as well. Can you also mention offline tests as well?

[allroundexperts]

Please update the title of this PR as well. Can you also mention offline tests as well?

@parasharrajat I have updated the title. As far as I've understood, offline tests are not applicable here since the page won't really load if you're offline. Please correct me if the case is otherwise.

[parasharrajat]

@allroundexperts I am not able to test this PR with your steps. How did you test it on local server?

[allroundexperts]

@allroundexperts I am not able to test this PR with your steps. How did you test it on local server?

I ran the desktop app locally. I also ran web app locally as well. Once both are running, I just open the web app in logged in state. This causes the web app to open a alert that asks to open the app on dekstop. Clicking on it, opens the electron app.

[parasharrajat]

Ok, let me test it with the local build of the desktop app.

[allroundexperts]

Ok, let me test it with the local build of the desktop app.

FYI, I did not run a built version of the app. I ran the app using npm run desktop locally.

[parasharrajat]

Ok, so I am not getting the promot. I decline the prompt on the staging web app. Does it have to do anything with local as well? Is there a way to get it back?

Can you please update the tests and QA steps to add all the instructions? Whether the user has to install the desktop app first or not? etc...

Also, add videos for all platforms.

Did you really run all the checks from the checklist or just checked it all?

[parasharrajat]

I am able to see the prompt on staging app but not on local. I tried both safari and chrome.

Can you please update the video to show the full address bar?

[allroundexperts]

Ok, so I am not getting the promot. I decline the prompt on the staging web app. Does it have to do anything with local as well? Is there a way to get it back?

Can you please update the tests and QA steps to add all the instructions? Whether the user has to install the desktop app first or not? etc...

Also, add videos for all platforms.

Did you really run all the checks from the checklist or just checked it all?

@parasharrajat This is not applicable for mobile. Can you please guide me which screens to test on mobile?
I'll update and add more steps as well. Will also run it locally again and see if the prompt opens up or not.

[parasharrajat]

For the platforms where it does not apply, you can show the working app videos.

[allroundexperts]

For the platforms where it does not apply, you can show the working app videos.

Thanks for letting me know. I'll update these and will let you know here.

[allroundexperts]

@parasharrajat It seems like I had a .env file with ENVIRONMENT set as staging. If you don't have the ENVIRONMENT variable set, the default environment becomes dev. On dev we are not showing the prompt as can be seen here:
https://github.com/allroundexperts/Expensify/blob/151833d2956052a2ba6fe7e85042c31eba3b5c36/src/components/DeeplinkWrapper/index.website.js#L40
So you need to either disable that check in Line 40 or set ENVIRONMENT variable as staging for this to work. I'll also update the description in some time. It's great that we caught this.

[parasharrajat]

Screenshots

🔲 iOS / native

🔲 iOS / Safari

🔲 MacOS / Desktop => MacOS / Chrome

screen-2023-02-25_01.54.16.mp4

🔲 Android / Chrome

No change.

🔲 Android / native

No change.

[parasharrajat]

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.

cc: @marcochavezf

🎀 👀 🎀 C+ reviewed

[parasharrajat]

Things remaining from #15262 (comment) comment.

[allroundexperts]

Things remaining from #15262 (comment) comment.

@parasharrajat Added screen recordings and updated the steps.

[marcochavezf]

Thanks guys! LGTM

I removed point 1. Create .env file in the root folder and add ENVIRONMENT=staging to it. from QA test steps, since that doesn't apply for staging/production.

[github-actions]

Performance Comparison Report 📊

Significant Changes To Duration

Name	Duration
Open Search Page TTI	619.448 ms → 682.479 ms (+63.030 ms, +10.2%) 🔴

Show details

Name

Duration

Open Search Page TTI

Baseline Mean: 619.448 ms Stdev: 16.743 ms (2.7%) Runs: 589.2478430001065 589.5128170000389 591.0936280000024 601.4143480001949 601.5358070000075 605.1678470000625 606.0679529998451 606.6081550000235 610.4167890003882 611.0867110001855 611.6896159998141 613.3551839999855 615.4926760001108 615.6601970000193 616.7290860000066 618.1608879999258 619.5776370000094 619.7590330000967 620.5990809998475 620.9792889999226 621.1826589996926 624.1171059999615 625.7305510002188 627.9550789999776 632.0073250001296 636.1695150001906 636.1771650002338 639.2781980000436 642.9700929997489 643.0180259998888 651.7100829998963 657.8681239997968 Current Mean: 682.479 ms Stdev: 24.397 ms (3.6%) Runs: 635.368978000246 637.588012999855 637.831828000024 641.578288000077 642.3385419994593 649.5920009994879 673.6503100004047 674.8364669997245 676.2421070002019 679.7924410002306 680.5393070001155 680.6929940003902 681.4447839995846 681.5398359997198 682.6807869998738 683.1242680000141 683.993978000246 684.4374190000817 686.8575849998742 690.6844490002841 694.2860110001639 694.5824790000916 697.631347999908 701.2508549997583 701.7955320002511 706.9025879995897 707.5773919997737 708.9814050002024 710.8153079999611 715.8566489992663 732.3398850001395

Meaningless Changes To Duration

Show entries

Name	Duration
App start TTI	740.235 ms → 759.745 ms (+19.510 ms, +2.6%)
App start runJsBundle	205.625 ms → 213.161 ms (+7.536 ms, +3.7%)
App start regularAppStart	0.015 ms → 0.016 ms (+0.001 ms, +4.2%)
App start nativeLaunch	19.903 ms → 19.310 ms (-0.593 ms, -3.0%)

Show details

Name	Duration
App start TTI	Baseline Mean: 740.235 ms Stdev: 21.554 ms (2.9%) Runs: 690.1929680001922 707.0357070001774 717.3007049998268 717.804328000173 721.4555899999104 722.6238400000148 724.6869199997745 725.9112109998241 725.9800229999237 725.9950480000116 727.283505000174 728.5796849997714 728.6047519999556 734.4845079998486 734.9272989998572 737.4131609997712 737.4431830001995 739.7619610000402 742.0021859998815 742.6178850000724 743.4733799998648 744.1296169999987 745.206633000169 760.7932980000041 761.3198649999686 762.2537279999815 762.3583680000156 767.4293700000271 769.0146840000525 776.239475000184 779.0832960000262 784.1117830001749 Current Mean: 759.745 ms Stdev: 18.819 ms (2.5%) Runs: 725.4820149997249 725.6897870004177 727.684949000366 729.1017890004441 735.5774879995733 737.2534889997914 743.5447030002251 749.6151900002733 749.8428969997913 752.8678460000083 753.6398870004341 756.4640330001712 759.6201560003683 759.6828469997272 759.7258550003171 762.1486729998142 764.9058149997145 765.3101559998468 765.3638720000163 765.7346050003543 767.2289829999208 768.7377669997513 771.1478890003636 771.1838610004634 771.4067150000483 772.7670780001208 780.7845099996775 783.877044999972 787.0029490003362 790.3902340000495 798.3207940002903
App start runJsBundle	Baseline Mean: 205.625 ms Stdev: 17.333 ms (8.4%) Runs: 176 176 185 188 189 189 190 195 196 198 198 199 200 200 200 202 202 203 204 205 208 210 214 217 220 223 224 224 226 227 234 258 Current Mean: 213.161 ms Stdev: 14.496 ms (6.8%) Runs: 184 191 194 195 197 199 201 204 204 205 205 206 208 209 212 214 214 215 216 216 217 220 221 223 224 231 232 235 237 238 241
App start regularAppStart	Baseline Mean: 0.015 ms Stdev: 0.001 ms (7.6%) Runs: 0.01293900003656745 0.013183000031858683 0.013956999871879816 0.01411899970844388 0.014403999783098698 0.014485000167042017 0.014485999941825867 0.014485999941825867 0.014525999780744314 0.014567000325769186 0.014606999699026346 0.014688999857753515 0.014688999857753515 0.014891999773681164 0.015056000091135502 0.01509599993005395 0.015137000009417534 0.0152580002322793 0.015299000311642885 0.015339999925345182 0.015544000081717968 0.015544000081717968 0.015705999918282032 0.0157880000770092 0.016031999606639147 0.016195000149309635 0.016276000067591667 0.016478999983519316 0.017253000289201736 0.017537000123411417 0.017659000121057034 0.01769999973475933 Current Mean: 0.016 ms Stdev: 0.001 ms (5.9%) Runs: 0.0139979999512434 0.014649000018835068 0.014811000786721706 0.014851000159978867 0.015056000091135502 0.015219000168144703 0.015258999541401863 0.015300000086426735 0.015381000004708767 0.015422000549733639 0.015461999922990799 0.015542999841272831 0.015544000081717968 0.015625 0.015787999145686626 0.015787999145686626 0.0157880000770092 0.015910000540316105 0.016032000072300434 0.01607299968600273 0.016114000231027603 0.016235999763011932 0.01639800053089857 0.016561000607907772 0.016601000912487507 0.016642000526189804 0.01680499967187643 0.016846000216901302 0.017537000589072704 0.01790399942547083 0.01839200034737587
App start nativeLaunch	Baseline Mean: 19.903 ms Stdev: 2.291 ms (11.5%) Runs: 17 18 18 18 18 18 18 18 18 19 19 19 19 19 19 19 19 19 19 19 20 20 21 21 21 22 23 23 25 25 26 Current Mean: 19.310 ms Stdev: 1.086 ms (5.6%) Runs: 18 18 18 18 18 18 18 18 19 19 19 19 19 19 19 19 19 20 20 20 20 20 20 20 20 21 21 21 22

[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/marcochavezf 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 ✅