Skip to content

refactor getOptionData and sortReportsToDisplayInLHN to use conciergeReportID from useOnyx #80977

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.

Sign up for GitHub

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

Merged
tgolen merged 10 commits into from
Feb 18, 2026
Merged

refactor getOptionData and sortReportsToDisplayInLHN to use conciergeReportID from useOnyx #80977

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
7243629
refactor getOptionData and sortReportsToDisplayInLHN to use concierge…
dukenv0307 Jan 30, 2026
8f93446
update lint
dukenv0307 Jan 30, 2026
12024bb
update test
dukenv0307 Jan 30, 2026
506de46
merge main
dukenv0307 Feb 2, 2026
f7b21c5
change default value
dukenv0307 Feb 2, 2026
bf276af
add more tests
dukenv0307 Feb 9, 2026
5449591
merge main
dukenv0307 Feb 18, 2026
e4a894a
remove empty string
dukenv0307 Feb 18, 2026
1a1ddd2
update lint
dukenv0307 Feb 18, 2026
408d2bc
update test
dukenv0307 Feb 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/components/LHNOptionsList/OptionRowLHNData.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ function OptionRowLHNData({

const [movedFromReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getMovedReportID(lastAction, CONST.REPORT.MOVE_TYPE.FROM)}`, {canBeMissing: true});
const [movedToReport] = useOnyx(`${ONYXKEYS.COLLECTION.REPORT}${getMovedReportID(lastAction, CONST.REPORT.MOVE_TYPE.TO)}`, {canBeMissing: true});
const [conciergeReportID] = useOnyx(ONYXKEYS.CONCIERGE_REPORT_ID, {canBeMissing: true});
// Check the report errors equality to avoid re-rendering when there are no changes
const prevReportErrors = usePrevious(reportAttributes?.reportErrors);
const areReportErrorsEqual = useMemo(() => deepEqual(prevReportErrors, reportAttributes?.reportErrors), [prevReportErrors, reportAttributes?.reportErrors]);
Expand All @@ -68,6 +69,7 @@ function OptionRowLHNData({
personalDetails,
policy,
parentReportAction,
conciergeReportID,
lastMessageTextFromReport,
invoiceReceiverPolicy,
card,
Expand Down Expand Up @@ -104,6 +106,7 @@ function OptionRowLHNData({
preferredLocale,
policy,
parentReportAction,
conciergeReportID,
iouReportReportActions,
transaction,
receiptTransactions,
Expand Down
6 changes: 4 additions & 2 deletions src/hooks/useSidebarOrderedReports.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ function SidebarOrderedReportsContextProvider({
const [reportNameValuePairs, {sourceValue: reportNameValuePairsUpdates}] = useOnyx(ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS, {canBeMissing: true});
const [reportsDrafts, {sourceValue: reportsDraftsUpdates}] = useOnyx(ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT, {canBeMissing: true});
const [betas] = useOnyx(ONYXKEYS.BETAS, {canBeMissing: true});
const [conciergeReportID] = useOnyx(ONYXKEYS.CONCIERGE_REPORT_ID, {canBeMissing: true});
const reportAttributes = useReportAttributes();
const [currentReportsToDisplay, setCurrentReportsToDisplay] = useState<ReportsToDisplayInLHN>({});
const {shouldUseNarrowLayout} = useResponsiveLayout();
Expand Down Expand Up @@ -230,10 +231,11 @@ function SidebarOrderedReportsContextProvider({
}, [reportsToDisplayInLHN]);

const getOrderedReportIDs = useCallback(
() => SidebarUtils.sortReportsToDisplayInLHN(deepComparedReportsToDisplayInLHN ?? {}, priorityMode, localeCompare, deepComparedReportsDrafts, reportNameValuePairs),
() =>
SidebarUtils.sortReportsToDisplayInLHN(deepComparedReportsToDisplayInLHN ?? {}, priorityMode, localeCompare, deepComparedReportsDrafts, reportNameValuePairs, conciergeReportID),
// Rule disabled intentionally - reports should be sorted only when the reportsToDisplayInLHN changes
// eslint-disable-next-line react-hooks/exhaustive-deps
[deepComparedReportsToDisplayInLHN, localeCompare, deepComparedReportsDrafts],
[deepComparedReportsToDisplayInLHN, localeCompare, deepComparedReportsDrafts, conciergeReportID],
);

const orderedReportIDs = useMemo(() => getOrderedReportIDs(), [getOrderedReportIDs]);
Expand Down
4 changes: 3 additions & 1 deletion src/libs/ReportUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1013,7 +1013,7 @@
};

let conciergeReportIDOnyxConnect: OnyxEntry<string>;
Onyx.connect({

Check warning on line 1016 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.CONCIERGE_REPORT_ID,
callback: (value) => {
conciergeReportIDOnyxConnect = value;
Expand All @@ -1021,7 +1021,7 @@
});

const defaultAvatarBuildingIconTestID = 'SvgDefaultAvatarBuilding Icon';
Onyx.connect({

Check warning on line 1024 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.SESSION,
callback: (value) => {
// When signed out, val is undefined
Expand All @@ -1039,7 +1039,7 @@
let allPersonalDetails: OnyxEntry<PersonalDetailsList>;
let allPersonalDetailLogins: string[];
let currentUserPersonalDetails: OnyxEntry<PersonalDetails>;
Onyx.connect({

Check warning on line 1042 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
callback: (value) => {
if (currentUserAccountID) {
Expand All @@ -1051,7 +1051,7 @@
});

let allReportsDraft: OnyxCollection<Report>;
Onyx.connect({

Check warning on line 1054 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.REPORT_DRAFT,
waitForCollectionCallback: true,
callback: (value) => (allReportsDraft = value),
Expand All @@ -1060,7 +1060,7 @@
let allPolicies: OnyxCollection<Policy>;
let hasPolicies: boolean;
let policiesArray: Policy[] = [];
Onyx.connect({

Check warning on line 1063 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.POLICY,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -1071,7 +1071,7 @@
});

let allPolicyDrafts: OnyxCollection<Policy>;
Onyx.connect({

Check warning on line 1074 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.POLICY_DRAFTS,
waitForCollectionCallback: true,
callback: (value) => (allPolicyDrafts = value),
Expand All @@ -1079,7 +1079,7 @@

let allReports: OnyxCollection<Report>;
let reportsByPolicyID: ReportByPolicyMap;
Onyx.connect({

Check warning on line 1082 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.REPORT,
waitForCollectionCallback: true,
callback: (value) => {
Expand Down Expand Up @@ -1115,14 +1115,14 @@
});

let betaConfiguration: OnyxEntry<BetaConfiguration> = {};
Onyx.connect({

Check warning on line 1118 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.BETA_CONFIGURATION,
callback: (value) => (betaConfiguration = value ?? {}),
});

let allTransactions: OnyxCollection<Transaction> = {};
let reportsTransactions: Record<string, Transaction[]> = {};
Onyx.connect({

Check warning on line 1125 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.TRANSACTION,
waitForCollectionCallback: true,
callback: (value) => {
Expand All @@ -1148,7 +1148,7 @@
});

let allReportActions: OnyxCollection<ReportActions>;
Onyx.connect({

Check warning on line 1151 in src/libs/ReportUtils.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Onyx.connect() is deprecated. Use useOnyx() hook instead and pass the data as parameters to a pure function
key: ONYXKEYS.COLLECTION.REPORT_ACTIONS,
waitForCollectionCallback: true,
callback: (actions) => {
Expand Down Expand Up @@ -5750,6 +5750,7 @@
* Get the title for a report.
* @deprecated Moved to src/libs/ReportNameUtils.ts.
*/
// eslint-disable-next-line @typescript-eslint/max-params
Copy link
Contributor

@github-actions github-actions bot Jan 30, 2026

Choose a reason for hiding this comment

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

❌ CONSISTENCY-5 (docs)

ESLint rule disable lacks justification. While the comment states that max-params is being disabled, it does not explain WHY the function requires more than the maximum allowed parameters or why this exception is acceptable.

Suggested fix: Add a comment explaining the reason:

// eslint-disable-next-line @typescript-eslint/max-params -- getReportName requires many optional parameters for backwards compatibility and full report name generation
function getReportName(

Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.

Copy link
Contributor Author

@dukenv0307 dukenv0307 Feb 2, 2026

Choose a reason for hiding this comment

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

getReportName is used in too many places so I don't want to refactor it here

Copy link
Contributor

@DylanDylann DylanDylann Feb 9, 2026

Choose a reason for hiding this comment

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

@dukenv0307 Hmm, I don't think this is the right approach. We shouldn't allow a function to have too many parameters. That said, I noticed this function is deprecated, so I'm wondering whether it's even worth refactoring

Copy link
Contributor Author

@dukenv0307 dukenv0307 Feb 10, 2026

Choose a reason for hiding this comment

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

What is your plan? @DylanDylann

Copy link
Contributor

@DylanDylann DylanDylann Feb 10, 2026

Choose a reason for hiding this comment

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

How much effort would it take to add a conciergeReportID param to getReportName? If it's significant, I think we should hold it. On the other hand, I'm fine with the current change

Copy link
Contributor Author

@dukenv0307 dukenv0307 Feb 11, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

@DylanDylann A lot of changes for sure, getReportName is used in too many places

Copy link
Contributor

@DylanDylann DylanDylann Feb 16, 2026

Choose a reason for hiding this comment

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

Because this function already been deprecated and should be removed soon, so let's leave this as with disable /max-params rule

function getReportName(
report: OnyxEntry<Report>,
policy?: OnyxEntry<Policy>,
Expand All @@ -5761,6 +5762,7 @@
isReportArchived?: boolean,
reports?: Report[],
policies?: Policy[],
conciergeReportID?: string,
): string {
// Check if we can use report name in derived values - only when we have report but no other params
const canUseDerivedValue =
Expand Down Expand Up @@ -5935,7 +5937,7 @@
});
}

if (isConciergeChatReport(report)) {
if (isConciergeChatReport(report, conciergeReportID)) {
formattedName = CONST.CONCIERGE_DISPLAY_NAME;
}

Expand Down
20 changes: 14 additions & 6 deletions src/libs/SidebarUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,12 @@ function updateReportsToDisplayInLHN({
/**
* Categorizes reports into their respective LHN groups
*/
function categorizeReportsForLHN(reportsToDisplay: ReportsToDisplayInLHN, reportsDrafts: OnyxCollection<string> | undefined, reportNameValuePairs?: OnyxCollection<ReportNameValuePairs>) {
function categorizeReportsForLHN(
reportsToDisplay: ReportsToDisplayInLHN,
reportsDrafts: OnyxCollection<string> | undefined,
conciergeReportID: string | undefined,
reportNameValuePairs?: OnyxCollection<ReportNameValuePairs>,
) {
const pinnedAndGBRReports: MiniReport[] = [];
const errorReports: MiniReport[] = [];
const draftReports: MiniReport[] = [];
Expand All @@ -401,7 +406,7 @@ function categorizeReportsForLHN(reportsToDisplay: ReportsToDisplayInLHN, report

const reportID = report.reportID;
// eslint-disable-next-line @typescript-eslint/no-deprecated
const displayName = getReportName(report);
const displayName = getReportName(report, undefined, undefined, undefined, undefined, undefined, undefined, undefined, undefined, undefined, conciergeReportID);
const miniReport: MiniReport = {
reportID,
displayName,
Expand Down Expand Up @@ -536,7 +541,8 @@ function sortReportsToDisplayInLHN(
priorityMode: OnyxEntry<PriorityMode>,
localeCompare: LocaleContextProps['localeCompare'],
reportsDrafts: OnyxCollection<string> | undefined,
reportNameValuePairs?: OnyxCollection<ReportNameValuePairs>,
reportNameValuePairs: OnyxCollection<ReportNameValuePairs> | undefined,
conciergeReportID: string | undefined,
): string[] {
Performance.markStart(CONST.TIMING.GET_ORDERED_REPORT_IDS);

Expand All @@ -554,7 +560,7 @@ function sortReportsToDisplayInLHN(
// - Sorted by reportDisplayName in GSD (focus) view mode

// Step 1: Categorize reports
const categories = categorizeReportsForLHN(reportsToDisplay, reportsDrafts, reportNameValuePairs);
const categories = categorizeReportsForLHN(reportsToDisplay, reportsDrafts, conciergeReportID, reportNameValuePairs);

// Step 2: Sort each category
const sortedCategories = sortCategorizedReports(categories, isInDefaultMode, localeCompare);
Expand Down Expand Up @@ -660,6 +666,7 @@ function getOptionData({
personalDetails,
policy,
parentReportAction,
conciergeReportID,
invoiceReceiverPolicy,
lastMessageTextFromReport: lastMessageTextFromReportProp,
card,
Expand All @@ -679,6 +686,7 @@ function getOptionData({
personalDetails: OnyxEntry<PersonalDetailsList>;
policy: OnyxEntry<Policy>;
parentReportAction: OnyxEntry<ReportAction> | undefined;
conciergeReportID: string | undefined;
invoiceReceiverPolicy: OnyxEntry<Policy>;
lastMessageTextFromReport?: string;
reportAttributes: OnyxEntry<ReportAttributes>;
Expand Down Expand Up @@ -778,7 +786,7 @@ function getOptionData({
result.tooltipText = getReportParticipantsTitle(visibleParticipantAccountIDs);
result.hasOutstandingChildTask = report.hasOutstandingChildTask;
result.hasParentAccess = report.hasParentAccess;
result.isConciergeChat = isConciergeChatReport(report);
result.isConciergeChat = isConciergeChatReport(report, conciergeReportID);
result.participants = report.participants;

const isExpense = isExpenseReport(report);
Expand Down Expand Up @@ -1115,7 +1123,7 @@ function getOptionData({
}

// eslint-disable-next-line @typescript-eslint/no-deprecated
const reportName = getReportName(report, policy, undefined, undefined, invoiceReceiverPolicy, undefined, undefined, isReportArchived);
const reportName = getReportName(report, policy, undefined, undefined, invoiceReceiverPolicy, undefined, undefined, isReportArchived, undefined, undefined, conciergeReportID);

result.text = reportName;
result.subtitle = subtitle;
Expand Down
1 change: 1 addition & 0 deletions tests/perf-test/SidebarUtils.perf-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ describe('SidebarUtils', () => {
policy,
invoiceReceiverPolicy: undefined,
parentReportAction,
conciergeReportID: '',
oneTransactionThreadReport: undefined,
card: undefined,
lastAction: undefined,
Expand Down
38 changes: 38 additions & 0 deletions tests/unit/ReportUtilsTest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1643,6 +1643,44 @@ describe('ReportUtils', () => {
const reportName = computeReportName(conciergeReport);
expect(reportName).toBe(CONST.CONCIERGE_DISPLAY_NAME);
});

test('should return Concierge display name when explicit conciergeReportID matches report ID', () => {
const explicitConciergeReportID = 'explicit-concierge-456';
const report: Report = {
reportID: explicitConciergeReportID,
participants: buildParticipantsFromAccountIDs([currentUserAccountID, CONST.ACCOUNT_ID.CONCIERGE]),
};

// eslint-disable-next-line @typescript-eslint/no-deprecated
const reportName = getReportNameDeprecated(report, policy, undefined, participantsPersonalDetails, undefined, undefined, [], false, [], [], explicitConciergeReportID);
expect(reportName).toBe(CONST.CONCIERGE_DISPLAY_NAME);
});

test('should not return Concierge display name when explicit conciergeReportID does not match report ID', () => {
const explicitConciergeReportID = 'explicit-concierge-456';
const report: Report = {
reportID: 'different-report-789',
participants: buildParticipantsFromAccountIDs([currentUserAccountID, 1]),
};

// eslint-disable-next-line @typescript-eslint/no-deprecated
const reportName = getReportNameDeprecated(report, policy, undefined, participantsPersonalDetails, undefined, undefined, [], false, [], [], explicitConciergeReportID);
expect(reportName).not.toBe(CONST.CONCIERGE_DISPLAY_NAME);
// Should generate name from participants instead
expect(reportName).toBe('Ragnar Lothbrok');
});

test('should use Onyx-connected conciergeReportID when explicit parameter is undefined', () => {
// conciergeReportID is already set in beforeAll to 'concierge-123'
const report: Report = {
reportID: conciergeReportID,
participants: buildParticipantsFromAccountIDs([currentUserAccountID, CONST.ACCOUNT_ID.CONCIERGE]),
};

// eslint-disable-next-line @typescript-eslint/no-deprecated
const reportName = getReportNameDeprecated(report, policy, undefined, participantsPersonalDetails);
expect(reportName).toBe(CONST.CONCIERGE_DISPLAY_NAME);
});
});

describe('Money Request', () => {
Expand Down
Loading
Loading