Call OpenDraftWorkspaceRequest when mounting the MoneyRequestConfirmPage for policyExpenseChats#24640
Conversation
…age for policyExpenseChats
|
The Web-E PR is on staging now so C+ can start testing. I'm asking for a C+ in Slack. I'll review the code now. |
|
@jasperhuangg for some reason the author checklist is failing |
|
@jasperhuangg I am seeing the OpenDraftWorkspaceRequest API call being made multiple times in the network tab. And crash when i choose a different chat in the money selection flow Screen.Recording.2023-08-18.at.12.56.19.AM.mp4On a side note: Is there any specific reason to add log lines? I think checking network tab should be fine IMO. And I am not sure on which cases we want the OpenDraftWorkspaceRequest API to be called |
I initially added the logs so we could test on iOS/Android as well (since there's no network tab there). |
neil-marcellini
left a comment
There was a problem hiding this comment.
Thanks for the updates
abdulrahuman5196
left a comment
There was a problem hiding this comment.
@jasperhuangg I am still see the issue. Could you kindly fix the same
|
@jasperhuangg The steps in the authors checklist also needs to be updated. Steps 6 and 11 won't work. Since we call the api on component mounts, it will call every we go to the flow. I don't it will have to do anything with signout or sign in
|
Reviewer Checklist
Screenshots/VideosWebScreen.Recording.2023-08-18.at.2.48.09.AM.mp4Mobile Web - ChromeMobile Web - SafariDesktopScreen.Recording.2023-08-18.at.2.59.18.AM.mp4iOSScreen.Recording.2023-08-18.at.3.01.58.AM.mp4Android |
|
Android build is taking too much time to build. Since this is a time sensitive PR(just API) and I have already tested in ios, I think that should be fine. |
abdulrahuman5196
left a comment
There was a problem hiding this comment.
Changes looks good and works well. Reviewers checklist is also complete.
All yours. @neil-marcellini
🎀 👀 🎀
C+ Reviewed
|
🎯 @abdulrahuman5196, thanks for reviewing and testing this PR! 🎉 An E/App issue has been created to issue payment here: #25440. |
neil-marcellini
left a comment
There was a problem hiding this comment.
Good to go thanks!
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/neil-marcellini in version: 1.3.56-0 🚀
|
|
🚀 Deployed to production by https://github.com/roryabraham in version: 1.3.56-24 🚀
|
Fixed Issues
$ https://github.com/Expensify/Expensify/issues/299210
Tests
Add the following log here at the top of
logRequestDetailsQA Steps
OpenDraftWorkspaceMoneyRequestcallPR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodWaiting for Copylabel for a copy review on the original GH to get the correct copy.STYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)/** comment above it */thisproperly so there are no scoping issues (i.e. foronClick={this.submit}the methodthis.submitshould be bound tothisin the constructor)thisare necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);ifthis.submitis never passed to a component event handler likeonClick)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Web
web.mov
Mobile Web - Chrome
N/A
Mobile Web - Safari
N/A
Desktop
desktop.mov
iOS
Android