Feat/track whispers #38709
Feat/track whispers #38709
Conversation
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, Desktop, and Web. Happy testing! 🧪🧪 |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
| DELETE: 'delete', | ||
| APPROVE: 'approve', | ||
| TRACK: 'track', | ||
| MOVE: 'move', |
There was a problem hiding this comment.
Why was this constant added? I can't find it referenced anywhere so I am thinking of removing it in this PR.
There was a problem hiding this comment.
Yes this can be removed, Sorry I didn't remember to remove it when i no longer need it : (
There was a problem hiding this comment.
OK, thank you! I'll move forward with removing it.
|
🚀 Deployed to staging by https://github.com/thienlnam in version: 1.4.63-0 🚀
|
| </View> | ||
| )} | ||
| { | ||
| {(!isMovingTransactionFromTrackExpense || !hasRoute) && |
There was a problem hiding this comment.
Just use !isDistanceRequest here instead.
|
🚀 Deployed to production by https://github.com/mountiny in version: 1.4.63-21 🚀
|
| } | ||
| } | ||
| if (action === CONST.IOU.ACTION.SHARE) { | ||
| Navigation.navigate(ROUTES.ROOM_INVITE.getRoute(activeReportID ?? '', CONST.IOU.SHARE.ROLE.ACCOUNTANT)); |
| waypoints: validWaypoints ? JSON.stringify(validWaypoints) : undefined, | ||
| }; | ||
|
|
||
| API.write(WRITE_COMMANDS.TRACK_EXPENSE, parameters, onyxData); |
There was a problem hiding this comment.
Coming from #40664, we missed adding an optimistic actionable whisper while creating track expense for self-DM.
| transactionID, | ||
| reportActionID: reportAction.reportActionID, | ||
| }; | ||
| const {parameters, optimisticData, successData, failureData, shouldDeleteTransactionThread} = getDeleteTrackExpenseInformation(chatReportID, transactionID, reportAction); |
There was a problem hiding this comment.
When we introduced track expense whisper, we missed handling the whisper when the track expense gets deleted thereby resulting in #43560
| linkedTrackedExpenseReportID: reportID, | ||
| amount: transactionAmount, | ||
| created: transactionCreated, | ||
| } as Transaction); |
There was a problem hiding this comment.
| function getAmount(transaction: OnyxEntry<Transaction>, isFromExpenseReport = false, isFromTrackedExpense = false): number { | ||
| // IOU requests cannot have negative values, but they can be stored as negative values, let's return absolute value | ||
| if (!isFromExpenseReport) { | ||
| if (!isFromExpenseReport || isFromTrackedExpense) { |
There was a problem hiding this comment.
Anyone have any idea why the || isFromTrackedExpense check was added here ? Asking because we have this issue where the expected result states:
after moving expense with negative amount to self DM, the negative sign should be retained.
and the above check is causing this actual result:
In Step 9, after moving expense with negative amount to self DM, the negative sign is not retained.
In Step 11, the same unreported expense shows negative sign on Old Dot, but not on New Dot.
In Step 18, after adding the unreported expense to a report, the negative sign reappears.
Before moving forward with the existing proposal which suggests changing the condition to !isFromTrackedExpense (falsy), I'd want to confirm the purpose / role of this change and whether the selfDM case it's expected behaviour or if the suggested fix would break any logic this PR implemented.
cc @ishpaul777 @shubham1206agra @tgolen @thienlnam if any of you know, please let us know
There was a problem hiding this comment.
I don't have any context or knowledge of this change. Sorry! FWIW, I'd agree that it definitely sounds like the negative sign should be retained when moving the expense to the self DM.
| failureData?.push(...moveTransactionFailureData); | ||
|
|
||
| const parameters = { | ||
| amount, |
There was a problem hiding this comment.
Come from #74641, we are missing to pass distance to param
11 participants
Details
Fixed Issues
$ #38533
PROPOSAL:
Tests
Same as QA
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop