Fix statement download crash and invalid date by thienlnam · Pull Request #14454 · Expensify/App

thienlnam · 2023-01-21T00:48:49Z

Details

Fixed Issues

Tests

There isn't an easy way to test this on devices that are not web. You'll have to overwrite a button to navigate you to the statement. On web, you can just access the statement by changing the URL to http://localhost:8080/statements/202211/

This is what I do:

App/src/pages/home/sidebar/SidebarScreen/FloatingActionButtonAndPopover.js

Lines 172 to 176 in 9489c1b

    
           { 
        
               icon: Expensicons.ChatBubble, 
        
               text: this.props.translate('sidebarScreen.newChat'), 
        
               onSelected: () => Navigation.navigate(ROUTES.NEW_CHAT), 
        
           },

Update to
onSelected: () => Navigation.navigate(ROUTES.getWalletStatementWithDateRoute('202211')),

This will override the new chat button to open up the statement for November, 2022

Open up the statement
Press the download button, and notice that you see a growl

WebView to production from staging/dev doesn't work right now - so as a note just test this on Web and I'll have all platforms tested on production

Offline tests

QA Steps

INTERNAL QA ONLY

Open up the statement
Press the download button, and notice that you see a growl
Press the download button again, and notice that it downloaded correctly

PR Author Checklist

Screenshots/Videos

Web

Screen.Recording.2023-01-27.at.12.15.08.PM.mov

Mobile Web - Chrome

Screen.Recording.2023-01-27.at.12.16.14.PM.mov

Mobile Web - Safari

Desktop

Screen.Recording.2023-01-27.at.1.40.39.PM.mov

iOS

Screen.Recording.2023-01-27.at.12.27.56.PM.mov

Android

Emulator not working

melvin-bot · 2023-01-21T00:49:00Z

Looks like you modified deprecatedAPI.js! To be clear, you should not be adding any code to this file.

Instead, all new API commands should use API.js, and follow our guidelines for writing new API commands.

Unsure if your change is okay? Drop a note in #expensify-open-source!

thienlnam · 2023-01-27T00:47:42Z

Changes complete, just need to run through checklist

melvin-bot · 2023-01-27T21:48:30Z

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

thesahindia · 2023-01-30T17:54:43Z

This is not related to the PR but I am not sure why at mWeb the page is blank.

Screen.Recording.2023-01-30.at.11.10.12.PM.mov

When we use a url with wrong date format (e.g. 20221) the RHN will open and close. I think we can improve this by not opening the RHN if we can or by showing a hint on that page asking the user to use the right format but It is an edge case and not related to the PR

Screen.Recording.2023-01-30.at.8.55.59.PM.mov

thesahindia · 2023-01-30T18:03:07Z

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

Screen.Recording.2023-01-30.at.11.08.37.PM.mov

Mobile Web - Chrome

Screen.Recording.2023-01-30.at.11.11.23.PM.mov

Mobile Web - Safari

Screen.Recording.2023-01-30.at.11.10.12.PM.mov

Desktop

Screen.Recording.2023-01-30.at.11.02.41.PM.mov

iOS

Screen.Recording.2023-01-30.at.11.12.52.PM.mov

Android

Screen.Recording.2023-01-30.at.11.12.12.PM.mov

thesahindia

Looks good to me

cc: @marcochavezf

thesahindia · 2023-02-01T15:13:40Z

Gentle bump @marcochavezf

marcochavezf · 2023-02-01T16:09:54Z

        }

-        Growl.show(this.props.translate('statementPage.generatingPDF'), CONST.GROWL.NOTICE, 5000);
+        Growl.show(this.props.translate('statementPage.generatingPDF'), CONST.GROWL.SUCCESS, 3000);


I suppose we'll get rid of the Growl alerts eventually (maybe by using an optimistic message?). But since it's fixing a crash I don't think it's a blocker

OSBotify · 2023-02-01T16:14:41Z

✋ 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-01T16:44:26Z

Performance Comparison Report 📊

Significant Changes To Duration

There are no entries

Meaningless Changes To Duration

Show entries

Name	Duration
App start TTI	685.655 ms → 707.206 ms (+21.551 ms, +3.1%)
Open Search Page TTI	607.442 ms → 611.776 ms (+4.334 ms, +0.7%)
App start runJsBundle	192.813 ms → 196.594 ms (+3.781 ms, +2.0%)
App start regularAppStart	0.014 ms → 0.015 ms (+0.001 ms, +7.2%)
App start nativeLaunch	20.031 ms → 19.688 ms (-0.344 ms, -1.7%)

Show details

Name	Duration
App start TTI	Baseline Mean: 685.655 ms Stdev: 31.170 ms (4.5%) Runs: 645.4267459996045 647.9453630000353 651.1312659997493 655.4840070009232 655.8976550009102 655.9335709996521 657.390008000657 657.8355059996247 662.9599779993296 664.6032940000296 666.815923999995 668.5437429994345 669.955714000389 670.6218289993703 671.5057989992201 678.000060999766 678.0421360004693 678.1644400004297 681.9355389997363 687.7912869993597 689.6528489999473 690.190246000886 690.7183619998395 708.0815159995109 711.8713230006397 714.9490510001779 716.4722949992865 720.6487050000578 737.2090380005538 746.0698080006987 749.583696000278 759.5293049998581 Current Mean: 707.206 ms Stdev: 30.080 ms (4.3%) Runs: 652.1374719999731 663.1274399999529 668.6836980003864 672.6690820008516 675.5240100007504 677.24615399912 678.0868570003659 683.9727159999311 688.6356259994209 689.7488960009068 690.8526879996061 692.7420150004327 697.7435950003564 698.9241860006005 701.8475860003382 703.5295160003006 704.7366870008409 708.736486999318 712.2454020008445 712.8242450002581 713.7990039996803 714.5020109992474 720.8194379992783 720.9149650000036 723.0601039994508 733.5540640000254 735.4455660004169 741.641888000071 743.2095359992236 760.3570029996336 772.7905529998243 776.4767949990928
Open Search Page TTI	Baseline Mean: 607.442 ms Stdev: 21.741 ms (3.6%) Runs: 571.8173420000821 573.2246909998357 579.0563149992377 581.0924070011824 582.8065189998597 587.9447029996663 588.1313879992813 590.6808270011097 594.5662439987063 595.6688650008291 596.5941160004586 597.8224280010909 598.6096190009266 599.4197599999607 602.5710039995611 603.7921549994498 606.5821939986199 606.8675130009651 607.6916100010276 613.1583670005202 613.7238769996911 614.0268550012261 616.4672449994832 617.1437989994884 617.2999680005014 619.9077959991992 631.6458750013262 635.1732589993626 638.0875650011003 650.5770269986242 651.9471840001643 654.0308839995414 Current Mean: 611.776 ms Stdev: 25.098 ms (4.1%) Runs: 564.6777350008488 570.7742919996381 584.6510419994593 586.2219249997288 586.4633390009403 588.1144619993865 588.4307050015777 589.5480960011482 593.0442709997296 594.3120530005544 595.6955570001155 603.6591389998794 605.3059080000967 606.1830649990588 606.7794189993292 607.1073819994926 607.2525639999658 608.6335450001061 613.6281739994884 615.7836099993438 616.4630950000137 618.2413739990443 619.1060390006751 620.28133200109 620.9961749985814 625.6335450001061 634.988323001191 635.9785159993917 641.7319339998066 642.9128009993583 659.9588220007718 667.0444740001112 669.0066330004483
App start runJsBundle	Baseline Mean: 192.813 ms Stdev: 24.230 ms (12.6%) Runs: 156 167 167 169 171 172 172 172 173 174 174 177 181 182 183 183 188 189 192 195 199 199 199 209 211 215 220 223 225 238 246 249 Current Mean: 196.594 ms Stdev: 25.072 ms (12.8%) Runs: 158 167 171 172 173 173 173 174 174 175 178 182 182 182 185 187 191 191 199 199 200 201 218 223 227 227 228 231 233 234 235 248
App start regularAppStart	Baseline Mean: 0.014 ms Stdev: 0.001 ms (7.4%) Runs: 0.012531999498605728 0.012654000893235207 0.012899000197649002 0.013183001428842545 0.013225000351667404 0.013467999175190926 0.013508999720215797 0.013590000569820404 0.013631999492645264 0.013671999797224998 0.013712000101804733 0.013875000178813934 0.013916000723838806 0.013996999710798264 0.013996999710798264 0.014038000255823135 0.014038000255823135 0.014039000496268272 0.014118999242782593 0.014159999787807465 0.014241000637412071 0.014567000791430473 0.01464799977838993 0.014810999855399132 0.014932999387383461 0.014972999691963196 0.015177998691797256 0.015625 0.01607299968600273 0.0168869998306036 0.01688700169324875 Current Mean: 0.015 ms Stdev: 0.001 ms (7.6%) Runs: 0.013020999729633331 0.013467999175190926 0.013590998947620392 0.013875000178813934 0.013955999165773392 0.014038000255823135 0.014322999864816666 0.0143630001693964 0.01440499909222126 0.014527000486850739 0.014607999473810196 0.014730000868439674 0.014934001490473747 0.015178000554442406 0.015178000554442406 0.01521800085902214 0.015300000086426735 0.015463000163435936 0.015584001317620277 0.015665000304579735 0.015786999836564064 0.015829000622034073 0.01586899906396866 0.015869000926613808 0.01590999960899353 0.01635800115764141 0.016764000058174133 0.016805000603199005 0.0174150001257658 0.017455998808145523 0.017456000670790672
App start nativeLaunch	Baseline Mean: 20.031 ms Stdev: 2.099 ms (10.5%) Runs: 17 17 18 18 18 18 18 18 18 19 19 19 19 19 19 19 20 20 20 20 21 21 21 21 21 22 22 23 23 24 24 25 Current Mean: 19.688 ms Stdev: 1.944 ms (9.9%) Runs: 17 17 17 17 18 18 18 18 18 18 19 19 19 19 19 19 20 20 20 20 20 20 20 21 21 21 22 22 22 23 23 25

OSBotify · 2023-02-02T00:44:45Z

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

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

OSBotify · 2023-02-04T00:17:56Z

🚀 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 · 2023-02-04T01:55:57Z

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

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

OSBotify · 2023-02-04T02:02:04Z

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

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

thienlnam added 3 commits January 20, 2023 16:31

remove old api call

489f915

add check for if it's a valid statement date

7e31abf

fix the growl, clean up the dismiss modal logic

baffb62

thienlnam self-assigned this Jan 21, 2023

thienlnam added 3 commits January 26, 2023 16:03

Merge branch 'main' into jack-fixstatementcrash

ea53586

lint

94b695b

remove unnecessary return

32dec36

thienlnam marked this pull request as ready for review January 27, 2023 21:47

thienlnam requested a review from a team as a code owner January 27, 2023 21:47

melvin-bot Bot requested review from marcochavezf and thesahindia and removed request for a team January 27, 2023 21:48

thesahindia approved these changes Jan 30, 2023

View reviewed changes

marcochavezf reviewed Feb 1, 2023

View reviewed changes

marcochavezf approved these changes Feb 1, 2023

View reviewed changes

marcochavezf merged commit d84dfa1 into main Feb 1, 2023

marcochavezf deleted the jack-fixstatementcrash branch February 1, 2023 16:10

thienlnam mentioned this pull request Feb 2, 2023

Deploy Checklist: New Expensify 2023-02-02 #14771

Closed

30 tasks

melvin-bot Bot mentioned this pull request Feb 3, 2023

[HOLD for payment 2023-02-10] Crash when trying to download invalid statements #14395

Closed

6 tasks

	{
	icon: Expensicons.ChatBubble,
	text: this.props.translate('sidebarScreen.newChat'),
	onSelected: () => Navigation.navigate(ROUTES.NEW_CHAT),
	},

Uh oh!

Conversation

thienlnam commented Jan 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Details

Fixed Issues

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

melvin-bot Bot commented Jan 21, 2023

Uh oh!

thienlnam commented Jan 27, 2023

Uh oh!

melvin-bot Bot commented Jan 27, 2023

Uh oh!

thesahindia commented Jan 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thesahindia commented Jan 30, 2023

Reviewer Checklist

Screenshots/Videos

Uh oh!

thesahindia left a comment

Choose a reason for hiding this comment

Uh oh!

thesahindia commented Feb 1, 2023

Uh oh!

marcochavezf Feb 1, 2023

Choose a reason for hiding this comment

Uh oh!

OSBotify commented Feb 1, 2023

Uh oh!

github-actions Bot commented Feb 1, 2023

Performance Comparison Report 📊

Significant Changes To Duration

Meaningless Changes To Duration

Uh oh!

OSBotify commented Feb 2, 2023

Uh oh!

OSBotify commented Feb 4, 2023

Uh oh!

OSBotify commented Feb 4, 2023

Uh oh!

OSBotify commented Feb 4, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

thienlnam commented Jan 21, 2023 •

edited

Loading

thesahindia commented Jan 30, 2023 •

edited

Loading