Reduce mapToTransactionItemWithAdditionalInfo invocations - #76339
Conversation
346fef0 to
2e4e6c4
Compare
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
2e4e6c4 to
09f2f8d
Compare
|
@dominictb 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] |
In my opinion it's ok, but if we need more test coverage I can prepare test file. |
JmillsExpensify
left a comment
There was a problem hiding this comment.
No product review required.
|
@szymonzalarski98 I think all the |
| let allNonDeletedSelected = true; | ||
| let hasNonDeletedTransactions = false; |
There was a problem hiding this comment.
I guess these are intentional for performance reason, right? Because we can directly filter the pending delete then count the selected.
There was a problem hiding this comment.
I did some improvements, thank you for the comment
|
I remember that we used to show the expense reports list in an collapsible format (the expense report row shows all of its expenses) but now we only show the expense report, then need to tap that row to view the expenses. @szymonzalarski98 Do you have any idea where it goes or did we get rid of it 😄 ? |
…ng-map-to-transaction-item-computations
…ng-map-to-transaction-item-computations
Sorry for late response, I'm quite new to the project, so sorry but I cannot give you a good answer :D |
…ng-map-to-transaction-item-computations
db0d6d3 to
4ee7673
Compare
|
@szymonzalarski98 You mistakenly pushed |
6fc1cdb to
1b1e5a4
Compare
Thank you, I've removed it |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2025-12-29.at.17.38.01.movAndroid: mWeb ChromeiOS: HybridAppScreen.Recording.2025-12-29.at.17.29.18-compressed.moviOS: mWeb SafariScreen.Recording.2025-12-29.at.17.33.20.movMacOS: Chrome / SafariScreen.Recording.2025-12-29.at.17.10.56.movScreen.Recording.2025-12-29.at.17.11.11.movScreen.Recording.2025-12-29.at.17.13.43.mov |
dominictb
left a comment
There was a problem hiding this comment.
🎀
@szymonzalarski98 Can you add brief details to the Explanation of Change section so Product guys can have more context?
| const mappedTransactions = item.transactions.map((transaction) => { | ||
| const isTransactionSelected = !!(transaction.keyForList && selectedTransactions[transaction.keyForList]?.isSelected); | ||
|
|
||
| if (transaction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE) { |
There was a problem hiding this comment.
| if (transaction.pendingAction !== CONST.RED_BRICK_ROAD_PENDING_ACTION.DELETE) { | |
| if (isTransactionPendingDelete(transaction)) { |
|
✋ 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/JS00001 in version: 9.2.88-0 🚀
|
|
🚀 Deployed to production by https://github.com/mountiny in version: 9.2.88-7 🚀
|
Explanation of Change
Fixed Issues
$ #76417
PROPOSAL:
Tests
Offline tests
QA Steps
Same as tests
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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari