Ensure OpenReport is always called when deeplinking from an outside source#15994
Conversation
|
|
|
@stitesExpensify Ready to go :D |
Reviewer Checklist
Screenshots/VideosWeb2023-03-15_12-33-45.mp42023-03-15_12-34-07.mp4Mobile Web - Chrome2023-03-15_12-47-37.mp4Mobile Web - Safari2023-03-15_12-35-58.mp42023-03-15_12-39-09.mp4Desktop2023-03-15_12-51-02.mp4iOS2023-03-15_12-35-01.mp4Android2023-03-15_12-41-47.mp42023-03-15_12-45-02.mp4 |
stitesExpensify
left a comment
There was a problem hiding this comment.
LGTM and tests well!
…nDidUpdate Ensure OpenReport is always called when deeplinking from an outside source (cherry picked from commit e9c2e0f)
…-15994 🍒 Cherry pick PR #15994 to staging 🍒
|
🚀 Cherry-picked to staging by https://github.com/jasperhuangg in version: 1.2.85-1 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
|
🚀 Deployed to production by https://github.com/Julesssss in version: 1.2.85-1 🚀
|
|
🚀 Cherry-picked to staging by https://github.com/AndrewGable in version: 1.3.28-2 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
|
🚀 Deployed to production by https://github.com/AndrewGable in version: 1.3.28-5 🚀
|
| // before deciding that we shouldn't call OpenReport. | ||
| const onyxReportID = this.props.report.reportID; | ||
| const routeReportID = getReportID(this.props.route); | ||
| if (routeReportID === prevProps.route.params.reportID && (!onyxReportID || onyxReportID === routeReportID)) { |
There was a problem hiding this comment.
This check caused the OpenReport to be called multiple times when opening a thread. This has been fixed with comparing only the reportIDFromRoute. More info at #51680 .
Details
This fixes an issue where scanning QR codes on Android would cause you to navigate to a "No Access" screen. This was due to a faulty condition in our
componentDidUpdatelogic in the ReportScreen that would prevent OpenReport from being called when you deeplinked to a report on Native platforms from outside the App.This also makes it so I can get rid of a hack I added to get deeplinks within the App to work in the
AnchorRendererFixed Issues
$ N/A
Tests/QA
Deeplinking from within the App
Deeplinking from outside the App
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)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
mweb-chrome.mov
Mobile Web - Safari
mweb-safari.mov
Desktop
desktop.mov
iOS
ios.mp4
Android
android.mov