Feature: Add moveIOUReportToPolicy #58078
Conversation
|
@ikevin127 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] |
|
@luacmartins How do we go about testing this type of PRs of new functions that are not actually used in the app yet, how do we verify that they comply with the doc specs other than reading the code and assuming ? |
|
Ideally, we wait for the backend PR to be available so we can test it end to end. |
|
Waiting on @waterim to address comments |
|
Will address asap! |
|
Waiting on comments to be addressed! |
|
@luacmartins updated! |
|
fixing conflicts now |
|
@luacmartins Ready for a review! |
|
@ikevin127 let's prioritize reviewing this one please when you're online please! |
|
@luacmartins updated! |
|
@ikevin127 let's review this one! Hopefully we can get it merged tomorrow. |
luacmartins
left a comment
There was a problem hiding this comment.
Left a few more comments. Additionally, I think we have a BE bug that's not correctly moving the report preview to the employee's workspace chat. I'll investigate
I found the bug and have a PR in review. |
Reviewer Checklist
Screenshots/Videos
Android: NativeAndroid: mWeb ChromeiOS: NativeiOS: mWeb SafariMacOS: Chrome / SafariMacOS: Desktop |
|
✋ 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/luacmartins in version: 9.1.19-0 🚀
|
|
@waterim Do we need QA steps for this? |
|
🚀 Deployed to production by https://github.com/cristipaval in version: 9.1.19-5 🚀
|
| }); | ||
|
|
||
| // We need to move the report preview action from the DM to the workspace chat. | ||
| const parentReportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${iouReport.parentReportID}`]; |
There was a problem hiding this comment.
Coming from: #64459. We shouldn't add the reportAction prefix when getting data from allReportActions
| return; | ||
| } | ||
| // We do not want to create negative amount expenses | ||
| if (ReportActionsUtils.hasRequestFromCurrentAccount(iouReport.reportID, iouReport.managerID ?? CONST.DEFAULT_NUMBER_ID)) { |
There was a problem hiding this comment.
We should filter out the deleted transactions to derive hasRequestFromCurrentAccount as it caused an issue #65323
| } | ||
|
|
||
| function isPolicyMember(currentUserLogin: string | undefined, policyID: string | undefined): boolean { | ||
| return !!currentUserLogin && !!policyID && !!allPolicies?.[`${ONYXKEYS.COLLECTION.POLICY}${policyID}`]?.employeeList?.[currentUserLogin]; |
There was a problem hiding this comment.
NAB but we improved the function with additional check for policy owner to determine if the user is a policy member. This would help in cases where employeeList is not available as observed in #69320

Details
Fixed Issues
$ #57470
PROPOSAL: N/A
Tests
Pre-requisite: a workspace with an admin and employee
/change-workspaceto the end of it and send it as a message to the reportOffline tests
Same as tests
QA Steps
None
PR 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.