Early return in isPayer function when policy does not exist#76510
Early return in isPayer function when policy does not exist#76510mountiny merged 12 commits intoExpensify:mainfrom
Conversation
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@rojiphil, the bot recommendation was nice, I applied. Please take another look when you get a chance. Thanks |
JmillsExpensify
left a comment
There was a problem hiding this comment.
Agree with the PR changes, GBR shouldn't show on the workspace chat if approver isn't part of the workspace.
|
@rojiphil, friendly bump on PR review. thanks |
Reviewer Checklist
Screenshots/VideosAndroid: HybridApp76510-android-hybrid-001.mp4Android: mWeb Chrome76510-mweb-chrome-001.mp4iOS: HybridApp76510-ios-hybrid-001.mp4iOS: mWeb Safari76510-mweb-safari-001.mp4MacOS: Chrome / Safari76510-web-chrome-002.mp4 |
src/libs/actions/IOU.ts
Outdated
|
|
||
| function canCancelPayment(iouReport: OnyxEntry<OnyxTypes.Report>, session: OnyxEntry<OnyxTypes.Session>) { | ||
| return isPayerReportUtils(session, iouReport) && (isSettled(iouReport) || iouReport?.isWaitingOnBankAccount) && isExpenseReport(iouReport); | ||
| function canCancelPayment(iouReport: OnyxEntry<OnyxTypes.Report>, session: OnyxEntry<OnyxTypes.Session>, reportPolicy?: OnyxTypes.OnyxInputOrEntry<OnyxTypes.Policy>) { |
There was a problem hiding this comment.
@marufsharifi Not sure what led you here but isn't canCancelPayment a redundant code as I don't see it been used anywhere?
There was a problem hiding this comment.
@rojiphil, you are right: this function is only used for the tests, so the tests failed. I added this to get it working again.
There was a problem hiding this comment.
Then, let's clean that up as well and remove the redundant code.
There was a problem hiding this comment.
@rojiphil Thanks for the clarification. Would you like me to remove the function entirely, along with all test steps where it’s currently being used?
|
@rojiphil Thanks for the review — the feedback has been addressed. the failure test is not related. |
rojiphil
left a comment
There was a problem hiding this comment.
Thanks @marufsharifi for the updates.
@mountiny Changes LGTM and works well too.
All yours. Thanks.
|
@mountiny Quick bump. 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/mountiny in version: 9.2.79-0 🚀
|
|
🚀 Deployed to staging by https://github.com/mountiny in version: 9.2.81-0 🚀
|
|
@marufsharifi @rojiphil The log has been actually causing issues with the queue too, too many logs for heavy accounts, that should be fixed independently 😢 |
|
🚀 Deployed to production by https://github.com/AndrewGable in version: 9.2.81-5 🚀
|
|
🚀 Deployed to production by https://github.com/AndrewGable in version: 9.2.81-5 🚀
|
Explanation of Change
Fixed Issues
$ #75329
PROPOSAL: #75329 (Comment)
Tests
Precondition: Owner create workspace and invite employee and approver. Submission frequency in Workflows needs to be disabled
Offline tests
Same as Tests
QA Steps
Same as Tests
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.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
Android: Native
Screen.Recording.1404-09-11.at.3.00.18.PM.mov
Android: mWeb Chrome
Screen.Recording.1404-09-11.at.3.03.40.PM.mov
iOS: Native
Screen.Recording.1404-09-11.at.2.53.53.PM.mov
iOS: mWeb Safari
Screen.Recording.1404-09-11.at.2.57.06.PM.mov
MacOS: Chrome / Safari
Screen.Recording.1404-09-11.at.2.33.38.PM.mov