Skip to content

Export environment and check before deep linking - #14723

Merged
pecanoro merged 3 commits into
mainfrom
stites-fixAnnoyingDeepLink
Feb 1, 2023
Merged

pecanoro merged 3 commits into
mainfrom
stites-fixAnnoyingDeepLink

Conversation

@stitesExpensify

@stitesExpensify stitesExpensify commented Feb 1, 2023 •

Copy link
Copy Markdown
Contributor

Details

Just getting rid of this popup on dev
2023-01-31_19-52-55

Fixed Issues

$ #14722

Tests

  1. Run npm run web
  2. Go to localhost:8080
  3. Make sure you don't get this popup

2023-01-31_19-52-55

4. Update you `.env` to include `ENVIRONMENT=production` 5. Run `npm run web` again 6. Make sure you get the popup like normal and can navigate to the main page 7. 4. Update you `.env` to include `ENVIRONMENT=staging` 5. Run `npm run web` again 6. Make sure you get the popup like normal and can navigate to the main page
  • Verify that no errors appear in the JS console

Offline tests

N/A

QA Steps

  1. Go to staging.new.expensify.com and log in
  2. Open the settings page
  3. Refresh and you should see this popup

2023-01-31_19-52-55

4. Make sure if you click the link you get brought to the desktop app 5. Make sure you can still access the web app by clicking the link shown on the splash page
  • 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.
  • I have checked off every checkbox in the PR author checklist, including those that don't apply to this PR.

Screenshots/Videos

Web
Mobile Web - Chrome
Mobile Web - Safari
Desktop
iOS
Android

@stitesExpensify stitesExpensify self-assigned this Feb 1, 2023

@luacmartins luacmartins left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this!

@stitesExpensify
stitesExpensify marked this pull request as ready for review February 1, 2023 16:16
@stitesExpensify
stitesExpensify requested a review from a team as a code owner February 1, 2023 16:16
@melvin-bot
melvin-bot Bot requested review from parasharrajat and pecanoro and removed request for a team February 1, 2023 16:16
@melvin-bot

melvin-bot Bot commented Feb 1, 2023

Copy link
Copy Markdown

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

@stitesExpensify

Copy link
Copy Markdown
Contributor Author

@parasharrajat removing you since this is a super simple change 😄

@stitesExpensify
stitesExpensify removed the request for review from parasharrajat February 1, 2023 16:17

@Julesssss Julesssss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@pecanoro

pecanoro commented Feb 1, 2023 •

Copy link
Copy Markdown
Contributor

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
Mobile Web - Chrome
Mobile Web - Safari
Desktop
iOS
Android

@pecanoro
pecanoro merged commit ae930fb into main Feb 1, 2023
@pecanoro
pecanoro deleted the stites-fixAnnoyingDeepLink branch February 1, 2023 16:49
@OSBotify

OSBotify commented Feb 1, 2023

Copy link
Copy Markdown
Contributor

✋ 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

github-actions Bot commented Feb 1, 2023

Copy link
Copy Markdown
Contributor

Performance Comparison Report 📊

Significant Changes To Duration

There are no entries

Meaningless Changes To Duration

Show entries
Name Duration
App start TTI 722.503 ms → 749.060 ms (+26.558 ms, +3.7%)
Open Search Page TTI 608.965 ms → 620.005 ms (+11.040 ms, +1.8%)
App start nativeLaunch 20.233 ms → 21.161 ms (+0.928 ms, +4.6%)
App start regularAppStart 0.015 ms → 0.016 ms (+0.002 ms, +10.7%)
App start runJsBundle 202.813 ms → 201.906 ms (-0.906 ms, ±0.0%)
Show details
Name Duration
App start TTI Baseline
Mean: 722.503 ms
Stdev: 40.688 ms (5.6%)
Runs: 625.7681959997863 650.9120780001394 658.9640509998426 682.6938180001453 683.0465990002267 684.9560429998673 687.2480640001595 689.3409669999965 696.0348260002211 699.728269000072 702.0835090000182 710.6656419998035 714.1734409998171 715.2155769998208 722.8528789998963 722.8786320001818 724.4937459998764 728.0094710001722 736.3475770000368 741.073543000035 741.7837720001116 742.9960880000144 747.8195259999484 748.4025940001011 750.1672840001993 750.6141400001943 753.6221250002272 754.200261999853 764.6465480001643 768.5154780000448 802.9927340000868 817.8419789997861

Current
Mean: 749.060 ms
Stdev: 20.727 ms (2.8%)
Runs: 704.4053009999916 720.9688349999487 724.6670180000365 725.3806070000865 726.2817959999666 729.429217999801 729.7082290002145 732.1034980001859 736.6331770000979 737.0354300001636 743.3750140001066 743.580167000182 744.6060440000147 744.7228569998406 745.3346549998969 745.4753140001558 746.6768140001222 747.3687550001778 750.1348020001315 751.8895120001398 755.7344089997932 759.0900779999793 761.3414150001481 762.4590770001523 764.4595139999874 770.6376229999587 772.6668130001053 776.9808629998006 786.751602999866 790.0565209998749 790.9168500001542
Open Search Page TTI Baseline
Mean: 608.965 ms
Stdev: 18.681 ms (3.1%)
Runs: 578.6162510002032 578.9848230001517 579.5137939997949 586.7668059999123 587.4330239999108 587.537760999985 589.6963710002601 593.9730229997076 597.469076000154 598.5109859998338 598.7658290001564 599.5010990002193 599.7545980000868 608.3513179998845 608.6398920002393 610.2902840003371 610.5168050001375 613.2175300000235 616.0168460002169 617.6654460001737 618.224608999677 618.5916749997996 618.8846429996192 619.9613449997269 623.0111090000719 627.1950690001249 631.4331060000695 633.9674889999442 635.2345779999159 638.2693690001033 651.9219969999976

Current
Mean: 620.005 ms
Stdev: 19.074 ms (3.1%)
Runs: 591.197021999862 592.3632000000216 593.9111739997752 595.3003340000287 595.7009279998019 597.3364260001108 597.3600269998424 600.6086019999348 611.7803139998578 611.9244800000452 611.943847999908 614.7920329999179 615.0450850003399 615.7458899999037 615.9269620003179 617.2175699998625 621.6502279997803 623.2422290001996 625.0803629998118 625.318563000299 625.3940019998699 625.5110679999925 627.9207759997807 628.6724449996836 629.544148999732 630.0391029999591 637.4958500000648 640.8921309998259 646.681559999939 647.045167000033 662.7801919998601 664.7297770003788
App start nativeLaunch Baseline
Mean: 20.233 ms
Stdev: 2.261 ms (11.2%)
Runs: 18 18 18 18 18 19 19 19 19 19 19 19 19 19 19 19 20 20 20 20 20 20 21 22 22 23 24 24 25 27

Current
Mean: 21.161 ms
Stdev: 2.172 ms (10.3%)
Runs: 18 18 19 19 19 19 19 19 20 20 20 20 20 20 20 21 21 21 21 22 22 22 22 23 23 23 24 25 25 25 26
App start regularAppStart Baseline
Mean: 0.015 ms
Stdev: 0.001 ms (5.9%)
Runs: 0.01318400027230382 0.013549000024795532 0.013712000101804733 0.013794000260531902 0.013875999953597784 0.014078999869525433 0.014159999787807465 0.014200999867171049 0.014282000251114368 0.014283000025898218 0.0143630001693964 0.01436399994418025 0.014607999939471483 0.014648000244051218 0.014769999776035547 0.014891999773681164 0.01509599993005395 0.015135999768972397 0.015137000009417534 0.015176999848335981 0.0152580002322793 0.01525900000706315 0.015461999922990799 0.015501999761909246 0.0157880000770092 0.015910000074654818 0.01607299968600273 0.01607300015166402 0.0161530002951622 0.016398000065237284 0.016642000060528517

Current
Mean: 0.016 ms
Stdev: 0.001 ms (5.9%)
Runs: 0.014810999855399132 0.014851999934762716 0.015054999850690365 0.015217999927699566 0.015461999922990799 0.015746999997645617 0.01582799991592765 0.01582799991592765 0.015828999690711498 0.015910000074654818 0.015951000154018402 0.01599099999293685 0.016032000072300434 0.01607300015166402 0.016398000065237284 0.01643799990415573 0.016439000144600868 0.016559999901801348 0.016764000058174133 0.01688600005581975 0.01696799974888563 0.01704900013282895 0.017212000209838152 0.0172520000487566 0.017292999662458897 0.017374999821186066 0.01757899997755885 0.01778100011870265 0.017782000359147787 0.01822900027036667 0.018757999874651432
App start runJsBundle Baseline
Mean: 202.813 ms
Stdev: 20.824 ms (10.3%)
Runs: 172 174 179 181 182 184 185 185 188 190 191 191 192 193 193 200 201 204 204 205 205 206 206 216 218 220 227 228 230 236 249 255

Current
Mean: 201.906 ms
Stdev: 13.733 ms (6.8%)
Runs: 179 184 185 186 186 188 189 191 193 194 196 198 198 198 199 200 200 201 201 203 203 204 204 207 209 216 217 218 224 225 230 235

@OSBotify

OSBotify commented Feb 2, 2023

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by https://github.com/pecanoro in version: 1.2.64-0 🚀

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

@OSBotify

OSBotify commented Feb 4, 2023

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by https://github.com/thienlnam in version: 1.2.64-7 🚀

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

@OSBotify

OSBotify commented Feb 4, 2023

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by https://github.com/thienlnam in version: 1.2.64-7 🚀

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

1 similar comment
@OSBotify

OSBotify commented Feb 4, 2023

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by https://github.com/thienlnam in version: 1.2.64-7 🚀

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

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants