-
Notifications
You must be signed in to change notification settings - Fork 0
[CLEAN] Synthetic Benchmark PR #73208 - Use existing selfDM report #21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: base_pr_73208_20251204_5741
Are you sure you want to change the base?
[CLEAN] Synthetic Benchmark PR #73208 - Use existing selfDM report #21
Conversation
PR Compliance Guide 🔍Below is a summary of compliance checks for this PR:
Compliance status legend🟢 - Fully Compliant🟡 - Partial Compliant 🔴 - Not Compliant ⚪ - Requires Further Human Verification 🏷️ - Compliance label |
||||||||||||||||||||||||
PR Code Suggestions ✨Explore these optional code suggestions:
|
||||||||||||
User description
Benchmark PR Expensify#73208
Type: Clean (correct implementation)
Original PR Title: Use existing selfDM report
Original PR Description:
Explanation of Change
SelfDM reports will be created during account creation and will have notification preference equal to
hiddenby default. Optimistically creating selfDM reports during onboarding is causing bugs with new behaviour so this PR removes that logicFixed Issues
$ https://github.com/Expensify/Expensify/issues/556957
PROPOSAL: n/a
Tests
muted.Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
muted.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
android-web.mov
iOS: Native
ios.mov
iOS: mWeb Safari
ios-web.mov
MacOS: Chrome / Safari
web.mov
MacOS: Desktop
Original PR URL: Expensify#73208
PR Type
Enhancement, Bug fix
Description
Store selfDM report ID from OpenApp response in Onyx state
Use cached selfDM ID instead of searching through all reports
Pass selfDM report ID parameter to transaction report changes
Optimize selfDM report lookup with memoization in multiple pages
Diagram Walkthrough
File Walkthrough
1 files
Add SELF_DM_REPORT_ID Onyx key definition9 files
Cache selfDM ID and optimize lookup logicAccept selfDM ID parameter in changeTransactionsReportMemoize and pass selfDM report ID to transaction changesMemoize and pass selfDM report ID to transaction changesMemoize selfDM ID and add eslint disable commentMemoize and pass selfDM ID to all transaction report changesUse named imports and findSelfDMReportID functionMemoize and pass selfDM ID to transaction report changesMemoize and pass selfDM ID to all transaction changes2 files
Pass selfDM ID to changeTransactionsReport in testsPass selfDM ID parameter in transaction test2 files
Add eslint disable comment for restricted importsAdd eslint disable comment for restricted imports