[TS migration] Migrate 'IOU.js' lib to TypeScript#34092
[TS migration] Migrate 'IOU.js' lib to TypeScript#34092roryabraham merged 97 commits intoExpensify:mainfrom
Conversation
# Conflicts: # src/libs/ReportUtils.ts
…pantsFromReport functions
# Conflicts: # src/libs/IOUUtils.ts # src/libs/ReportUtils.ts # src/libs/TransactionUtils.ts # src/libs/actions/IOU.ts # src/types/onyx/Transaction.ts # src/types/onyx/index.ts
…st, updateMoneyRequestAmountAndCurrency functions
# Conflicts: # src/libs/actions/IOU.ts
# Conflicts: # src/ONYXKEYS.ts # src/components/OfflineWithFeedback.tsx # src/libs/actions/IOU.ts # src/types/onyx/Transaction.ts
|
@aimane-chnaif @roryabraham Conflicts are resolved! 🙂 |
| payeeAccountID = userAccountID, | ||
| payeeEmail = currentUserEmail, |
There was a problem hiding this comment.
These 2 params don't exist on main. Not sure why git diff doesn't indicate that.
There was a problem hiding this comment.
oh ok, so you changed order of theses params
|
Re-tested all possible cases of money request flow. No issues found. @roryabraham all yours for merge |
# Conflicts: # src/libs/actions/IOU.ts
|
@aimane-chnaif @roryabraham I've resolved comments one more time, let's try to merge it today |
|
@VickyStash please fix conflict again |
|
Hope it would be the last one. @roryabraham is ready to merge immediately once fixed |
# Conflicts: # src/libs/actions/IOU.ts
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Cherry-picked to staging by https://github.com/thienlnam in version: 1.4.36-5 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
1 similar comment
|
🚀 Cherry-picked to staging by https://github.com/thienlnam in version: 1.4.36-5 🚀
@Expensify/applauseleads please QA this PR and check it off on the deploy checklist if it passes. |
| @@ -3103,7 +3073,7 @@ function getSendMoneyParams(report, amount, currency, comment, paymentMethodType | |||
| paymentMethodType, | |||
| transactionID: optimisticTransaction.transactionID, | |||
| newIOUReportDetails, | |||
| createdReportActionID: isNewChat ? optimisticCreatedAction.reportActionID : 0, | |||
| createdReportActionID: isNewChat ? optimisticCreatedAction.reportActionID : '', | |||
There was a problem hiding this comment.
This created a warning on the server, polluting the logs. It should be '0' instead of an empty string, as the server expects an integer. Here's a PR that fixes it
There was a problem hiding this comment.
Got it, thank you!
There was a problem hiding this comment.
@VickyStash As PR already raised thanks to @cristipaval, can you please co-review #35918?
|
This PR is failing because of issue #35942 The issue is reproducible in: Android, mWeb, IOS Bug6369674_1707243456539.az_recorder_20240206_180530.1.mp4Bug6369674_1707243456496.az_recorder_20240206_175443.1.mp4 |
yes, it's caused by another PR #34564 and the fix is being CPed |
|
🚀 Deployed to production by https://github.com/Julesssss in version: 1.4.37-7 🚀
|
| const diff = TransactionUtils.getAmount(transaction, true) - TransactionUtils.getAmount(updatedTransaction, true); | ||
| if (ReportUtils.isExpenseReport(iouReport)) { | ||
| if (ReportUtils.isExpenseReport(iouReport) && typeof updatedMoneyRequestReport.total === 'number') { |
There was a problem hiding this comment.
Looks like the number type check here led to the following issue (more details in #59267 (comment)):
which we fixed by changing the check to !Number.isNaN(iouReport.total) && iouReport.total !== undefined, using iouReport.total !== undefined instead of !!iouReport.total because its value can be 0.
Details
[TS migration] Migrate 'IOU.js' lib to TypeScript
Fixed Issues
$ #24926
PROPOSAL: N/A
Tests
Make sure the Money Request / Split bill functionality works the same way as before. You can use next flows to test it.
First flows: Using FAB button
Manual Money Request
Requestbutton.ReplaceandDeleteworks as expected.Scan Money Request
Distance Money Request
Second flows: Chat with another user:
Manual Money Request
Requestbutton.Scan Money Request
Third flows: Workspace chat
Split bill -> Manual
ManualoptionSplitbutton.Split bill -> Scan
ScanoptionSplitbutton.Split bill -> Distance
DistanceoptionSplitbuttonMoney Request-> Manual
Requestbutton.Money Request -> Scan
Money Request -> Distance
Offline tests
N/A
QA Steps
Same as in the Tests section
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)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel so 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_iou1.mp4
Android: mWeb Chrome
android_web_iou1.mp4
iOS: Native
ios1.mp4
iOS: mWeb Safari
ios_web1.mp4
MacOS: Chrome / Safari
web1.mp4
MacOS: Desktop
desktop1.mp4