Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[No QA] Actions can have childReportName #29486

Merged
merged 1 commit into from
Oct 12, 2023
Merged

[No QA] Actions can have childReportName #29486

merged 1 commit into from
Oct 12, 2023

Conversation

neil-marcellini
Copy link
Contributor

Details

Just updating a TS type

Fixed Issues

$
PROPOSAL:

Tests

  • Verify that no errors appear in the JS console

Offline tests

QA Steps

  • 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: mWeb Chrome
    • iOS: Native
    • iOS: mWeb 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 the left part of a conditional rendering a React component is a boolean and NOT a string, e.g. myBool && <MyComponent />.
    • 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 grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is 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
    • If we are not using the full Onyx data that we loaded, I've added the proper selector in order to ensure the component only re-renders when the data it is using changes
    • 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 code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • 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 the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • 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

Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop

@neil-marcellini neil-marcellini self-assigned this Oct 12, 2023
@neil-marcellini neil-marcellini requested a review from a team as a code owner October 12, 2023 19:57
@melvin-bot melvin-bot bot requested review from roryabraham and removed request for a team October 12, 2023 19:57
@melvin-bot
Copy link

melvin-bot bot commented Oct 12, 2023

@roryabraham 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]

@cloudflare-workers-and-pages
Copy link

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 07ee6c2
Status: ✅  Deploy successful!
Preview URL: https://fa106cc2.helpdot.pages.dev
Branch Preview URL: https://neil-childreportname.helpdot.pages.dev

View logs

@thienlnam thienlnam merged commit eec9dba into main Oct 12, 2023
14 of 15 checks passed
@thienlnam thienlnam deleted the neil-childReportName branch October 12, 2023 20:32
@OSBotify
Copy link
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 added the DeployBlockerCash This issue or pull request should block deployment label Oct 12, 2023
@github-actions
Copy link
Contributor

Performance Comparison Report 📊

Significant Changes To Duration

Name Duration
App start TTI 1201.618 ms → 1260.127 ms (+58.509 ms, +4.9%)
App start runJsBundle 811.591 ms → 863.989 ms (+52.398 ms, +6.5%) 🔴
Show details
Name Duration
App start TTI Baseline
Mean: 1201.618 ms
Stdev: 41.540 ms (3.5%)
Runs: 1117.0312850000337 1119.7385169998743 1120.5025999997742 1124.7489780001342 1130.9844900001772 1135.17973600002 1140.5959890000522 1144.1926509998739 1153.0225479998626 1154.041890999768 1154.3643060000613 1156.5056050000712 1157.6213879999705 1162.2744939997792 1162.7295369999483 1163.521354000084 1166.3165460000746 1169.2658529998735 1169.9035109998658 1170.5072220000438 1170.7687800000422 1171.3492939998396 1171.4802509997971 1173.270475000143 1173.6704919999465 1174.222821999807 1174.3436030000448 1175.816221000161 1176.5607110001147 1177.0373680000193 1178.9630539999343 1180.5052709998563 1182.7636070000008 1184.7631419999525 1186.5714380000718 1188.540393000003 1189.1280589997768 1189.5986040001735 1189.8837049999274 1191.6082310001366 1192.931317999959 1193.7251169998199 1194.9639739999548 1196.6354109998792 1197.0006389999762 1197.4892299999483 1198.0163190001622 1198.8331329999492 1200.3495100000873 1203.3126219999976 1205.4405020000413 1207.8621459999122 1208.5478500002064 1209.276868000161 1209.3751320000738 1211.210243999958 1214.721092000138 1215.3254180001095 1217.5253559998237 1217.5619839997962 1218.304601999931 1218.3513099998236 1219.4150910000317 1221.5826150001958 1222.0741929998621 1223.0424910001457 1223.2747080000117 1224.6677990001626 1230.2020080001093 1232.4202950000763 1236.127228999976 1236.7049529999495 1239.8793270001188 1241.780673999805 1241.7876909999177 1244.7945750001818 1245.5526290000416 1245.984238000121 1249.8099329997785 1251.2272339998744 1255.1017649997957 1256.6200009998865 1260.99170699995 1261.1505689998157 1271.539090000093 1273.061418000143 1277.05049100006 1277.3446630002 1289.438994999975 1291.4589129998349 1302.5030129998922

Current
Mean: 1260.127 ms
Stdev: 52.120 ms (4.1%)
Runs: 1149.8795719998889 1163.716744999867 1165.8444900000468 1170.466368000023 1176.5682459999807 1178.3708299999125 1181.1353899999522 1185.3376819998957 1186.7793160001747 1187.300675000064 1192.8593339999206 1197.792613999918 1201.226563999895 1209.3950860002078 1210.202709000092 1210.9029959999025 1212.537297999952 1214.3296230002306 1216.4877740000375 1216.995275999885 1217.5157860000618 1218.160166000016 1219.7133220001124 1221.0246350001544 1221.2844279999845 1222.407889999915 1223.7967500002123 1224.6111019998789 1226.404099999927 1229.5108730001375 1230.8915079999715 1232.2245709998533 1240.908650000114 1241.6862420001999 1243.2760209999979 1246.6650249999948 1247.198598000221 1249.7618760000914 1250.379215999972 1250.4228030000813 1250.502789999824 1256.9790380001068 1258.83468300011 1261.9450019998476 1262.0750790000893 1262.3533120001666 1262.432002000045 1265.3178090001456 1265.8221049997956 1266.0179900000803 1268.9299340001307 1269.3470669998787 1269.4009099998511 1269.6475109998137 1270.36569399992 1271.4322199998423 1271.596076999791 1274.4061819999479 1279.262995999772 1282.403518000152 1284.6361929997802 1286.8877420001663 1289.7831520000473 1290.1346920002252 1290.781173999887 1292.3126249997877 1298.1510689998977 1298.7004680000246 1298.8848439999856 1299.7863699998707 1301.6412570001557 1301.8744910000823 1303.2920499998145 1307.7357410001568 1309.1352780000307 1310.4894179999828 1311.6005299999379 1317.2692419998348 1317.5526919998229 1318.2124819997698 1323.7826339998282 1324.742002000101 1326.0157220000401 1331.2134920000099 1333.7802960001864 1339.6982120000757 1341.3143649999984 1349.1028939997777 1366.9919699998572 1384.8446349999867 1396.1321029998362
App start runJsBundle Baseline
Mean: 811.591 ms
Stdev: 29.870 ms (3.7%)
Runs: 735 749 750 752 757 763 763 773 774 780 785 786 788 788 788 789 789 789 790 790 791 791 792 792 793 793 793 797 797 799 801 801 802 803 803 804 804 804 806 807 808 808 809 809 810 812 813 813 813 813 816 817 817 818 819 820 820 821 822 823 825 827 828 829 829 829 831 832 833 836 836 836 837 838 840 845 845 846 847 851 853 861 864 867 868 872 875 888

Current
Mean: 863.989 ms
Stdev: 37.543 ms (4.3%)
Runs: 777 799 802 805 806 808 811 812 814 818 818 819 825 826 827 831 831 831 832 833 837 837 837 840 840 841 841 842 843 843 844 847 848 848 849 850 850 851 852 853 853 853 856 856 857 859 860 860 861 861 862 865 868 870 871 871 872 872 875 876 877 878 878 882 884 886 888 888 893 896 896 896 897 897 899 900 900 902 906 906 906 907 915 917 919 925 928 930 939 944 947 965

Meaningless Changes To Duration

Show entries
Name Duration
App start regularAppStart 0.014 ms → 0.015 ms (+0.001 ms, +7.0%)
App start nativeLaunch 22.393 ms → 21.329 ms (-1.064 ms, -4.8%)
Open Search Page TTI 620.476 ms → 616.434 ms (-4.043 ms, -0.7%)
Show details
Name Duration
App start regularAppStart Baseline
Mean: 0.014 ms
Stdev: 0.001 ms (6.1%)
Runs: 0.012084999587386847 0.012695000041276217 0.01285799965262413 0.012899000197649002 0.0129399998113513 0.012980000115931034 0.013020999729633331 0.013021000195294619 0.013143000192940235 0.013183000031858683 0.013264999724924564 0.013264999724924564 0.013265000190585852 0.013265000190585852 0.013387000188231468 0.013387000188231468 0.013428000267595053 0.01346899988129735 0.013508999720215797 0.013509000185877085 0.01355000026524067 0.013590999878942966 0.013630999717861414 0.013630999717861414 0.013631000183522701 0.013631999958306551 0.013671000022441149 0.013671000022441149 0.013712000101804733 0.013753000181168318 0.013793000020086765 0.013793999794870615 0.013793999794870615 0.013793999794870615 0.013794000260531902 0.01383400009945035 0.0138349998742342 0.0138349998742342 0.0138349998742342 0.013835000339895487 0.013874999713152647 0.013875999953597784 0.013915999792516232 0.013915999792516232 0.013915999792516232 0.013956000097095966 0.01399700017645955 0.014037999790161848 0.014038000255823135 0.014039000030606985 0.014078000094741583 0.014078999869525433 0.014119000174105167 0.014160000253468752 0.014241000171750784 0.01436399994418025 0.014404000248759985 0.014405000023543835 0.014444999862462282 0.014485999941825867 0.014649000018835068 0.014649000018835068 0.014649000018835068 0.0147299999371171 0.014730000402778387 0.014811000321060419 0.014851999934762716 0.014851999934762716 0.014973999932408333 0.015054999850690365 0.015054999850690365 0.015054999850690365 0.015055999625474215 0.015055999625474215 0.015056000091135502 0.015380000229924917 0.015381000004708767 0.015381000004708767 0.015503000002354383 0.015868999995291233 0.01590899983420968 0.015910000074654818 0.016398000065237284 0.0165200000628829

Current
Mean: 0.015 ms
Stdev: 0.001 ms (6.2%)
Runs: 0.012979999650269747 0.013753000181168318 0.01383400009945035 0.0138349998742342 0.013875999953597784 0.013916000258177519 0.013956000097095966 0.014078999869525433 0.01407900033518672 0.01407900033518672 0.01407900033518672 0.014119999948889017 0.014159999787807465 0.014159999787807465 0.014200999867171049 0.014200999867171049 0.014241999946534634 0.014282000251114368 0.01436399994418025 0.014404000248759985 0.014404000248759985 0.014444999862462282 0.014444999862462282 0.014444999862462282 0.01444500032812357 0.01448499970138073 0.014485999941825867 0.014526000246405602 0.014566999860107899 0.014566999860107899 0.014566999860107899 0.014566999860107899 0.014607999939471483 0.014607999939471483 0.01464799977838993 0.01464799977838993 0.014689000323414803 0.01472900016233325 0.0147299999371171 0.0147299999371171 0.014769999776035547 0.014771000016480684 0.014771000016480684 0.014852000400424004 0.014932999853044748 0.014932999853044748 0.014973999932408333 0.014973999932408333 0.01501399977132678 0.015015000011771917 0.015055000316351652 0.015056000091135502 0.01509599993005395 0.01509599993005395 0.01509599993005395 0.015137000009417534 0.015218999702483416 0.0152580002322793 0.015300000086426735 0.01534000039100647 0.015420999843627214 0.015421999618411064 0.015422000084072351 0.015503000002354383 0.015503000002354383 0.015544000081717968 0.015625 0.01570699969306588 0.015746999997645617 0.015787999611347914 0.015868999995291233 0.015910000074654818 0.015910000074654818 0.015910000074654818 0.015910000074654818 0.015910000074654818 0.016193999908864498 0.016195000149309635 0.01635799976065755 0.016398000065237284 0.016398000065237284 0.0165200000628829 0.016642000060528517 0.01680499967187643 0.016927000135183334 0.017090000212192535 0.017210999969393015 0.017212000209838152 0.01733400020748377 0.017700000200420618
App start nativeLaunch Baseline
Mean: 22.393 ms
Stdev: 2.993 ms (13.4%)
Runs: 18 18 18 18 19 19 19 19 19 19 19 19 19 20 20 20 20 20 20 20 20 20 20 20 20 21 21 21 21 21 21 21 21 21 21 21 21 21 21 21 21 22 22 22 22 22 22 22 22 22 22 22 22 22 22 22 23 23 23 23 23 23 23 23 23 24 24 24 24 24 24 24 25 25 26 26 27 27 27 27 28 28 29 29 29 29 29 29 29

Current
Mean: 21.329 ms
Stdev: 2.083 ms (9.8%)
Runs: 18 19 19 19 19 19 19 19 19 19 19 19 19 19 19 19 19 19 19 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 20 21 21 21 21 21 21 21 21 21 21 21 21 21 21 22 22 22 22 22 22 22 22 22 22 22 22 22 22 23 23 23 23 23 23 23 23 24 24 24 25 25 25 25 25 25 26 26 26 27
Open Search Page TTI Baseline
Mean: 620.476 ms
Stdev: 17.324 ms (2.8%)
Runs: 592.5922040003352 596.0419110003859 596.7336019999348 596.9256189996377 598.2041420000605 600.4130049999803 600.550741000101 600.6772059998475 600.6820069998503 601.4195150001906 604.5952960001305 604.725300999824 605.0431320001371 605.1921390001662 605.699421999976 605.750855000224 606.2771410001442 606.284260999877 606.4721679999493 606.6293950001709 607.0238449997269 607.2375900000334 607.9212239999324 608.3582759997807 608.5616460000165 609.0155839999206 609.4966230001301 611.8466389998794 612.2049159999005 612.4626870001666 612.5866700001061 612.9371340000071 613.0405679997057 613.15755299991 613.2057700003497 613.947103000246 614.0343840001151 614.3707279996015 614.7443039999343 615.1720790001564 615.5556650003418 615.6961260000244 615.9299320001155 616.1184089998715 616.4364830004051 616.4885260001756 616.7962650000118 616.9757899995893 617.0701910001226 617.8203529999591 618.0942799998447 618.266480000224 621.6074630003422 622.0624589999206 622.1450189999305 622.2100419998169 623.3306889999658 623.7965500000864 623.9304199996404 624.1848149998114 624.5920009999536 625.0938719999976 625.8406980000436 625.8688559997827 626.7360030002892 628.6581629998982 629.268798999954 629.7133379997686 630.806396999862 630.8915610001422 630.9189049997367 635.4644369999878 640.7661129999906 641.5915120001882 642.7912600003183 644.3068029996939 647.1908780001104 647.562784999609 649.2840990000404 649.3148610000499 655.1643880000338 655.5491539998911 655.8729659998789 656.6403000000864 656.6992190000601 664.0329590002075 674.0740160001442

Current
Mean: 616.434 ms
Stdev: 10.009 ms (1.6%)
Runs: 591.9133299998939 599.8886309997179 604.740722999908 604.8480230001733 604.8694260003977 605.3366700001061 605.3465990000404 605.5186769999564 605.7078040000051 605.728353000246 606.15209900029 606.2723799999803 606.3846839996986 606.4513759999536 606.9644369999878 607.2759190001525 607.719034999609 607.8224690002389 608.2683109999634 608.4650480002165 608.503337000031 608.7725420002826 608.8695479999296 608.9530440000817 609.2939860001206 609.7751059997827 609.9439699999057 610.802896999754 612.1433920003474 612.1732589998282 612.2317300001159 612.366578000132 612.5291749997996 612.5618900000118 612.9700929997489 613.2268479997292 613.2954510003328 613.6758220000193 613.8474129997194 613.8518069996499 613.9215090000071 613.9739180002362 614.5619309996255 615.4098720001057 615.6130369999446 615.8917240002193 615.987914999947 616.1076250001788 616.4606940001249 616.5162349999882 616.5408120001666 616.544311999809 616.5448809997179 616.7755539999343 617.0168460002169 617.362385999877 617.375976999756 617.4565840000287 618.1123049999587 620.4946699999273 620.7548830001615 620.7636309997179 620.9028730001301 621.750326000154 621.9237879998982 622.381184999831 623.3224280001596 624.112875000108 624.2038990003057 624.8908689999953 625.1973480000161 626.6388759999536 627.9178470000625 628.0466320002452 628.081788000185 628.943726000376 630.5463470001705 630.662924000062 630.7206629998982 630.9314379999414 631.7915040003136 633.1048179999925 640.1905110003427 641.3969320002943 644.8798420000821 645.1096200002357

@github-actions
Copy link
Contributor

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

@neil-marcellini
Copy link
Contributor Author

neil-marcellini commented Oct 13, 2023 •
edited
Loading

I really don't think this PR caused that "regression" ^

@OSBotify OSBotify mentioned this pull request Oct 13, 2023
Closed
97 tasks
@OSBotify
Copy link
Contributor

🚀 Deployed to staging by https://github.com/thienlnam in version: 1.3.84-0 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to production by https://github.com/francoisl in version: 1.3.84-10 🚀

platform result
🤖 android 🤖 skipped 🚫
🖥 desktop 🖥 skipped 🚫
🍎 iOS 🍎 skipped 🚫
🕸 web 🕸 skipped 🚫

@OSBotify OSBotify mentioned this pull request Oct 16, 2023
Closed
@OSBotify
Copy link
Contributor

🚀 Deployed to production by https://github.com/francoisl in version: 1.3.84-10 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to staging by https://github.com/thienlnam in version: 1.3.85-0 🚀

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

@OSBotify
Copy link
Contributor

🚀 Deployed to production by https://github.com/francoisl in version: 1.3.85-4 🚀

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

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

@thienlnam thienlnam thienlnam approved these changes

@roryabraham roryabraham Awaiting requested review from roryabraham

Assignees

@neil-marcellini neil-marcellini

Labels
DeployBlockerCash This issue or pull request should block deployment
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@neil-marcellini @OSBotify @thienlnam