-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
order options by the last expense request time in "Submit Expense" flow #49568
Changes from 5 commits
9659570
6bd112c
1e18107
0d6b776
39b2172
3fa7a8f
277bea7
3a3cfe5
9c9634e
c8a3caf
46840cd
b76fb45
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -222,6 +222,8 @@ type PreviewConfig = {showChatPreviewLine?: boolean; forcePolicyNamePreview?: bo | |
type FilterOptionsConfig = Pick<GetOptionsConfig, 'sortByReportTypeInSearch' | 'canInviteUser' | 'selectedOptions' | 'excludeUnknownUsers' | 'excludeLogins' | 'maxRecentReportsToShow'> & { | ||
preferChatroomsOverThreads?: boolean; | ||
preferPolicyExpenseChat?: boolean; | ||
preferRecentExpenseReports?: boolean; | ||
action?: IOUAction; | ||
}; | ||
|
||
/** | ||
|
@@ -1571,7 +1573,11 @@ function createOptionFromReport(report: Report, personalDetails: OnyxEntry<Perso | |
* @param searchValue - search string | ||
* @returns a sorted list of options | ||
*/ | ||
function orderOptions(options: ReportUtils.OptionData[], searchValue: string | undefined, {preferChatroomsOverThreads = false, preferPolicyExpenseChat = false} = {}) { | ||
function orderOptions( | ||
options: ReportUtils.OptionData[], | ||
searchValue: string | undefined, | ||
{preferChatroomsOverThreads = false, preferPolicyExpenseChat = false, preferRecentExpenseReports = false} = {}, | ||
) { | ||
return lodashOrderBy( | ||
options, | ||
[ | ||
|
@@ -1583,6 +1589,12 @@ function orderOptions(options: ReportUtils.OptionData[], searchValue: string | u | |
if (option.isSelfDM) { | ||
return 0; | ||
} | ||
if (preferRecentExpenseReports && !!option?.lastIOUTime) { | ||
return 1; | ||
} | ||
if (preferRecentExpenseReports && option.isPolicyExpenseChat) { | ||
return 1; | ||
} | ||
if (preferChatroomsOverThreads && option.isThread) { | ||
return 4; | ||
} | ||
|
@@ -1599,8 +1611,11 @@ function orderOptions(options: ReportUtils.OptionData[], searchValue: string | u | |
// When option.login is an exact match with the search value, returning 0 puts it at the top of the option list | ||
return 0; | ||
}, | ||
// For Submit Expense flow, prioritze the most recent expense reports and then policy expense chats (without expense requests) | ||
preferRecentExpenseReports ? (option) => option?.lastIOUTime ?? '' : '', | ||
preferRecentExpenseReports ? (option) => option?.isPolicyExpenseChat : 0, | ||
], | ||
['asc'], | ||
['asc', 'desc', 'desc'], | ||
); | ||
} | ||
|
||
|
@@ -1979,6 +1994,22 @@ function getOptions( | |
recentReportOptions.push(reportOption); | ||
} | ||
|
||
// Add a field to sort the recent reports by the time of last IOU request | ||
if (action) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A query I have is: Do we want to apply this ordering just to the create action? Because with at the moment this will also apply to sharing and categorizing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I think we should use it only for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps it would be a little cleaner if we made this condition dependent on |
||
const reportPreviewAction = allSortedReportActions[reportOption.reportID]?.find((reportAction) => | ||
ReportActionUtils.isActionOfType(reportAction, CONST.REPORT.ACTIONS.TYPE.REPORT_PREVIEW), | ||
); | ||
|
||
if (reportPreviewAction) { | ||
const iouReportID = ReportActionUtils.getIOUReportIDFromReportActionPreview(reportPreviewAction); | ||
const iouReportActions = allSortedReportActions[iouReportID] ?? []; | ||
const lastIOUAction = iouReportActions.find((iouAction) => iouAction.actionName === CONST.REPORT.ACTIONS.TYPE.IOU); | ||
if (lastIOUAction) { | ||
reportOption.lastIOUTime = lastIOUAction.lastModified; | ||
} | ||
} | ||
} | ||
|
||
// Add this login to the exclude list so it won't appear when we process the personal details | ||
if (reportOption.login) { | ||
optionsToExclude.push({login: reportOption.login}); | ||
|
@@ -2027,7 +2058,7 @@ function getOptions( | |
recentReportOptions.push(...personalDetailsOptions); | ||
personalDetailsOptions = []; | ||
} | ||
recentReportOptions = orderOptions(recentReportOptions, searchValue, {preferChatroomsOverThreads: true, preferPolicyExpenseChat: !!action}); | ||
recentReportOptions = orderOptions(recentReportOptions, searchValue, {preferChatroomsOverThreads: true, preferPolicyExpenseChat: !!action, preferRecentExpenseReports: !!action}); | ||
} | ||
|
||
return { | ||
|
@@ -2391,6 +2422,7 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt | |
excludeLogins = [], | ||
preferChatroomsOverThreads = false, | ||
preferPolicyExpenseChat = false, | ||
action, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. perhaps it would be better to define |
||
} = config ?? {}; | ||
if (searchInputValue.trim() === '' && maxRecentReportsToShow > 0) { | ||
return {...options, recentReports: options.recentReports.slice(0, maxRecentReportsToShow)}; | ||
|
@@ -2472,7 +2504,7 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt | |
|
||
return { | ||
personalDetails, | ||
recentReports: orderOptions(recentReports, searchValue, {preferChatroomsOverThreads, preferPolicyExpenseChat}), | ||
recentReports: orderOptions(recentReports, searchValue, {preferChatroomsOverThreads, preferPolicyExpenseChat, preferRecentExpenseReports: !!action}), | ||
userToInvite, | ||
currentUserOption: matchResults.currentUserOption, | ||
categoryOptions: [], | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -459,6 +459,7 @@ type OptionData = { | |||||
tabIndex?: 0 | -1; | ||||||
isConciergeChat?: boolean; | ||||||
isBold?: boolean; | ||||||
lastIOUTime?: string; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Just trying to see if we can make this a bit more descriptive |
||||||
} & Report; | ||||||
|
||||||
type OnyxDataTaskAssigneeChat = { | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.