Skip to content
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

Merged
Merged
40 changes: 36 additions & 4 deletions src/libs/OptionsListUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

/**
Expand Down Expand Up @@ -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,
[
Expand All @@ -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;
}
Expand All @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// For Submit Expense flow, prioritze the most recent expense reports and then policy expense chats (without expense requests)
// For Submit Expense flow, prioritize 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'],
);
}

Expand Down Expand Up @@ -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) {
Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think we should use it only for CREATE.

Copy link
Contributor

Choose a reason for hiding this comment

The 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 preferRecentExpenseReports, seeing as we're defining that as action === CONST.IOU.ACTION.CREATE.

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});
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -2391,6 +2422,7 @@ function filterOptions(options: Options, searchInputValue: string, config?: Filt
excludeLogins = [],
preferChatroomsOverThreads = false,
preferPolicyExpenseChat = false,
action,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps it would be better to define preferRecentExpenseReports here and then we can put the action === CONST.IOU.ACTION.CREATE condition in MoneyRequestParticipantsSelector

} = config ?? {};
if (searchInputValue.trim() === '' && maxRecentReportsToShow > 0) {
return {...options, recentReports: options.recentReports.slice(0, maxRecentReportsToShow)};
Expand Down Expand Up @@ -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: [],
Expand Down
1 change: 1 addition & 0 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,7 @@ type OptionData = {
tabIndex?: 0 | -1;
isConciergeChat?: boolean;
isBold?: boolean;
lastIOUTime?: string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
lastIOUTime?: string;
lastIOUCreationDate?: string;

Just trying to see if we can make this a bit more descriptive

} & Report;

type OnyxDataTaskAssigneeChat = {
Expand Down
1 change: 1 addition & 0 deletions src/pages/home/ReportScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,7 @@ function ReportScreen({route, currentReportID = '', navigation}: ReportScreenPro

const lastRoute = usePrevious(route);
const lastReportActionIDFromRoute = usePrevious(reportActionIDFromRoute);

// Define here because reportActions are recalculated before mount, allowing data to display faster than useEffect can trigger.
// If we have cached reportActions, they will be shown immediately.
// We aim to display a loader first, then fetch relevant reportActions, and finally show them.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,10 @@ function MoneyRequestParticipantsSelector({participants = CONST.EMPTY_ARRAY, onF
excludeLogins: CONST.EXPENSIFY_EMAILS,
maxRecentReportsToShow: CONST.IOU.MAX_RECENT_REPORTS_TO_SHOW,
preferPolicyExpenseChat: isPaidGroupPolicy,
action,
});
return newOptions;
}, [areOptionsInitialized, defaultOptions, debouncedSearchTerm, participants, isPaidGroupPolicy, canUseP2PDistanceRequests, iouRequestType, isCategorizeOrShareAction]);
}, [areOptionsInitialized, defaultOptions, debouncedSearchTerm, participants, isPaidGroupPolicy, canUseP2PDistanceRequests, iouRequestType, isCategorizeOrShareAction, action]);

/**
* Returns the sections needed for the OptionsSelector
Expand Down
Loading