WIP: Remove allReports usages from OptionsListUtils utility functions ||#83778
WIP: Remove allReports usages from OptionsListUtils utility functions ||#83778abzokhattab wants to merge 12 commits intoExpensify:mainfrom
Conversation
Thread explicit report/chatReport params through getAlternateText, getReportOption, and getLastMessageTextForReport instead of relying on the module-level allReports Onyx.connect variable. For loop-based functions (getValidOptions, prepareReportOptionsForDisplay, formatSectionsFromSearchTerm), accept the reports collection as a required param since each iteration needs a different chatReport. Simplify getIOUReportIDOfLastAction to return the ID directly without the redundant getReportOrDraftReport existence check.
abzokhattab
changed the title
- Extract repeated report+chatReport lookup pattern into getReportOptionFromCollection in OptionsListUtils - Replace duplicated 3-line pattern in MoneyRequest.ts, IOURequestStepAmount, and SubmitDetailsPage - Add missing reports param to getSearchOptions/getValidOptions callers - Fix test calls for updated getOptionData and MoneyRequest signatures
…port lookups - Create useParticipantReport hook to subscribe to only the specific participant report needed instead of the full reports collection - Refactor MoneyRequest action params to accept participantReport and participantChatReport instead of OnyxCollection<Report> - Remove getReportOptionFromCollection helper (no longer needed) - Update all IOU step pages to use the targeted hook
…-optionslistutils-followup
… mocks - Fix object-shorthand lint error in useSearchSelector.base.ts - Extract MemberInviteConfig type and consolidate getMemberInviteOptions params into config object to satisfy max-params rule - Remove unused draftReports variable in OptionsListUtilsTest - Pass report to getReportOption test calls instead of undefined - Add getMoneyRequestParticipantsFromReport mock in useReceiptScan test - Pass fakeReport as participantReport in MoneyRequestTest - Update all callers of getMemberInviteOptions
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@DylanDylann 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] |
melvin-bot
bot
requested review from
joekaufmanexpensify
and removed request for
a team
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2bb59c4ac6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| () => getMoneyRequestParticipantsFromReport(report, currentUserAccountID).find((p) => !(p.accountID ?? CONST.DEFAULT_NUMBER_ID))?.reportID, | ||
| [report, currentUserAccountID], | ||
| ); | ||
| const [participantReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${participantReportID}`); |
There was a problem hiding this comment.
This hook now replaces the old getReportOrDraftReport path but only subscribes to ONYXKEYS.COLLECTION.REPORT. getMoneyRequestParticipantsFromReport() can return a reportID that exists only in COLLECTION.REPORT_DRAFT, so participantReport stays undefined; downstream callers (e.g., the updated IOU step flows) pass that into getReportOption, which drops report metadata (including reportID) and can mis-handle workspace participants during skip-confirmation paths. Please add a draft-report fallback when resolving the participant report.
Useful? React with 👍 / 👎.
| options: optionsWithContacts, | ||
| draftComments, | ||
| nvpDismissedProductTraining, | ||
| reports, |
There was a problem hiding this comment.
baseOptions now depends on reports (it is passed into getSearchOptions/getValidOptions), but reports was not added to the useMemo dependency array. When report collection data changes without another dependency changing, this memo can return stale search options until a separate re-render trigger happens. Add reports to the dependency list to keep selector output in sync.
Useful? React with 👍 / 👎.
|
No product review needed |
…ports-optionslistutils-followup
…ports-optionslistutils-followup
2 participants
Explanation of Change
allReportsusage from utility functions inOptionsListUtils—getAlternateText,getReportOption,getLastMessageTextForReport,prepareReportOptionsForDisplay,getValidOptionsfiltering, andformatSectionsFromSearchTermno longer read from the module-levelallReportsvariablereport+chatReportparams to single-item functions (getAlternateText,getReportOption) so callers resolve the specific reports neededreportscollection param to loop-based functions (getValidOptions,prepareReportOptionsForDisplay,formatSectionsFromSearchTerm,getSearchOptions,getMemberInviteOptions) since each iteration needs a differentchatReportgetIOUReportIDOfLastAction— returns the ID directly from the report action instead of round-tripping throughgetReportOrDraftReportchatReportthrough SidebarUtils —getOptionDataandOptionRowLHNDatanow resolve and passchatReporttogetLastMessageTextForReportuseOnyx(ONYXKEYS.COLLECTION.REPORT)Fixed Issues
$ #66378
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand 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