perf: transactions and violations by reports derived value#63655
Conversation
…transactions-derived-value
…transactions-derived-value
…transactions-derived-value
1e87642 to
976143e
Compare
| const [policies] = useOnyx(ONYXKEYS.COLLECTION.POLICY, {allowStaleData: true, initialValue: {}, canBeMissing: false}); | ||
| const policy = policies?.[`${ONYXKEYS.COLLECTION.POLICY}${report?.policyID}`]; | ||
| const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP, {canBeMissing: true}); | ||
| const [transactionsAndViolationsByReport = {}] = useOnyx(ONYXKEYS.DERIVED.REPORT_TRANSACTIONS_AND_VIOLATIONS, {canBeMissing: true}); |
There was a problem hiding this comment.
let's not default to a new object on each render
src/pages/home/ReportScreen.tsx
Outdated
|
|
||
| const [reportTransactions = []] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION, { | ||
| selector: (allTransactions): OnyxTypes.Transaction[] => selectAllTransactionsForReport(allTransactions, reportIDFromRoute, reportActions), | ||
| const [transactionsAndViolationsByReport = {}] = useOnyx(ONYXKEYS.DERIVED.REPORT_TRANSACTIONS_AND_VIOLATIONS, { |
There was a problem hiding this comment.
same here, default of {}
|
@TMisiukiewicz you've got conflicts |
…transactions-derived-value
|
conflicts resolved ✅ |
…transactions-derived-value
|
sorry @TMisiukiewicz, you've got conflicts again |
…transactions-derived-value
|
resolved 😭 |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
@TMisiukiewicz I see the expense(create, delete) isn't reflected in real-time for the other user. Can you plz test that once? It seems to be bcoz Screen.Recording.2025-06-26.at.13.36.49.mov |
| transactionThreadReport?: OnyxEntry<OnyxTypes.Report>; | ||
|
|
||
| /** All transactions grouped by reportID */ | ||
| transactionsAndViolationsByReport?: OnyxTypes.ReportTransactionsAndViolationsDerivedValue; |
|
FYI we're reverting this PR because it broke reactivity on the MoneyRequestView page. Whenever we split a transaction, we kept the original transaction in the view, even though the reportID is updated to -2 Steps to reproduce:
|
|
@Pujan92 @luacmartins thanks for your findings, I'll work on the improved solution |
|
🚀 Deployed to staging by https://github.com/roryabraham in version: 9.1.72-0 🚀
|
1 similar comment
|
🚀 Deployed to staging by https://github.com/roryabraham in version: 9.1.72-0 🚀
|
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.1.72-10 🚀
|


Explanation of Change
Creating a derived value which holds all transactions and transaction violations related to given reports.
Tested on iOS, navigating from Create expense -> Report screen:
Before: 7.5s
After: 5.9s
Fixed Issues
$ #64163
$ #63436
PROPOSAL:
Tests
+and select Create expenseOffline tests
n/a
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as tests
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.mov
Android: mWeb Chrome
iOS: Native
ios.mp4
iOS: mWeb Safari
ios-web.mp4
MacOS: Chrome / Safari
web.mov
MacOS: Desktop
desktop.mov