From e1988a4980d18ed30f3fbd22574941d3cfcc2d8d Mon Sep 17 00:00:00 2001 From: Jakub Korytko Date: Wed, 23 Apr 2025 11:23:21 +0200 Subject: [PATCH 1/4] Fix participants array in option object --- src/libs/ReportUtils.ts | 40 +++++++++++++++++++++++++++++++++------- src/libs/SidebarUtils.ts | 2 +- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 2cf2f7c6bb43..0123a6169581 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -814,6 +814,7 @@ type GetReportNameParams = { draftReports?: OnyxCollection; reportNameValuePairs?: OnyxCollection; policies?: SearchPolicy[]; + participantPersonalDetailList?: PersonalDetails[]; }; type ReportByPolicyMap = Record; @@ -4556,18 +4557,41 @@ function getInvoicesChatName({ /** * Get the title for a report using only participant names. This may be used for 1:1 DMs and other non-categorized chats. */ -function buildReportNameFromParticipantNames({report, personalDetails}: {report: OnyxEntry; personalDetails?: Partial}) { +function buildReportNameFromParticipantNames({ + report, + personalDetails, + participantPersonalDetailList, +}: { + report: OnyxEntry; + personalDetails?: Partial; + participantPersonalDetailList?: PersonalDetails[]; +}) { const participantsWithoutCurrentUser: number[] = []; - Object.keys(report?.participants ?? {}).forEach((accountID) => { + const currentParticipants = participantPersonalDetailList ?? report?.participants ?? {}; + Object.keys(currentParticipants).forEach((accountID) => { const accID = Number(accountID); if (accID !== currentUserAccountID && participantsWithoutCurrentUser.length < 5) { participantsWithoutCurrentUser.push(accID); } }); + const isMultipleParticipantReport = participantsWithoutCurrentUser.length > 1; - return participantsWithoutCurrentUser - .map((accountID) => getDisplayNameForParticipant({accountID, shouldUseShortForm: isMultipleParticipantReport, personalDetailsData: personalDetails})) - .join(', '); + const displayNames = participantsWithoutCurrentUser + .map((accountID) => ({ + accountID, + value: getDisplayNameForParticipant({accountID, shouldUseShortForm: isMultipleParticipantReport, personalDetailsData: personalDetails}), + })) + .filter((name) => name.value); + + if (!displayNames || displayNames.length === 0) { + return ''; + } + + if (displayNames.length > 1) { + return displayNames.map((name) => name.value).join(', '); + } + + return getDisplayNameForParticipant({accountID: displayNames.at(0)?.accountID, shouldUseShortForm: false, personalDetailsData: personalDetails}); } function generateReportName(report: OnyxEntry): string { @@ -4586,6 +4610,7 @@ function getReportName( parentReportActionParam?: OnyxInputOrEntry, personalDetails?: Partial, invoiceReceiverPolicy?: OnyxEntry, + participantPersonalDetailList?: PersonalDetails[], ): string { // Check if we can use report name in derived values - only when we have report but no other params const canUseDerivedValue = report && policy === undefined && parentReportActionParam === undefined && personalDetails === undefined && invoiceReceiverPolicy === undefined; @@ -4593,7 +4618,7 @@ function getReportName( if (canUseDerivedValue && reportAttributes?.[report.reportID]) { return reportAttributes[report.reportID].reportName; } - return getReportNameInternal({report, policy, parentReportActionParam, personalDetails, invoiceReceiverPolicy}); + return getReportNameInternal({report, policy, parentReportActionParam, personalDetails, invoiceReceiverPolicy, participantPersonalDetailList}); } function getSearchReportName(props: GetReportNameParams): string { @@ -4620,6 +4645,7 @@ function getReportNameInternal({ reports, reportNameValuePairs, policies, + participantPersonalDetailList, }: GetReportNameParams): string { const reportID = report?.reportID; @@ -4821,7 +4847,7 @@ function getReportNameInternal({ } // Not a room or PolicyExpenseChat, generate title from first 5 other participants - formattedName = buildReportNameFromParticipantNames({report, personalDetails}); + formattedName = buildReportNameFromParticipantNames({report, personalDetails, participantPersonalDetailList}); return formattedName; } diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index e4b1ba6ef8c7..62613b877d6f 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -688,7 +688,7 @@ function getOptionData({ result.phoneNumber = personalDetail?.phoneNumber ?? ''; } - const reportName = getReportName(report, policy, undefined, undefined, invoiceReceiverPolicy); + const reportName = getReportName(report, policy, undefined, undefined, invoiceReceiverPolicy, participantPersonalDetailList); result.text = reportName; result.subtitle = subtitle; From 5ae00d4de63e66bf22cadc70f7529907a854f070 Mon Sep 17 00:00:00 2001 From: Jakub Korytko Date: Wed, 23 Apr 2025 14:58:56 +0200 Subject: [PATCH 2/4] Fix tests & improve the logic --- src/libs/ReportUtils.ts | 67 +++++++++++++++++----------------------- src/libs/SidebarUtils.ts | 2 +- 2 files changed, 29 insertions(+), 40 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 0123a6169581..00a03a9cbbb3 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -814,7 +814,6 @@ type GetReportNameParams = { draftReports?: OnyxCollection; reportNameValuePairs?: OnyxCollection; policies?: SearchPolicy[]; - participantPersonalDetailList?: PersonalDetails[]; }; type ReportByPolicyMap = Record; @@ -4555,44 +4554,36 @@ function getInvoicesChatName({ } /** - * Get the title for a report using only participant names. This may be used for 1:1 DMs and other non-categorized chats. + * Generates a report title using the names of participants, excluding the current user. + * This function is useful in contexts such as 1:1 direct messages (DMs) or other group chats. + * It limits to a maximum of 5 participants for the title and uses short names unless there is only one participant. */ -function buildReportNameFromParticipantNames({ - report, - personalDetails, - participantPersonalDetailList, -}: { - report: OnyxEntry; - personalDetails?: Partial; - participantPersonalDetailList?: PersonalDetails[]; -}) { - const participantsWithoutCurrentUser: number[] = []; - const currentParticipants = participantPersonalDetailList ?? report?.participants ?? {}; - Object.keys(currentParticipants).forEach((accountID) => { - const accID = Number(accountID); - if (accID !== currentUserAccountID && participantsWithoutCurrentUser.length < 5) { - participantsWithoutCurrentUser.push(accID); - } - }); - - const isMultipleParticipantReport = participantsWithoutCurrentUser.length > 1; - const displayNames = participantsWithoutCurrentUser +const buildReportNameFromParticipantNames = ({report, personalDetails: personalDetailsData}: {report: OnyxEntry; personalDetails?: Partial}) => + Object.keys(report?.participants ?? {}) + .map(Number) + .filter((id) => id !== currentUserAccountID) // Exclude the current user's ID + .slice(0, 5) // Limit the number of participants to 5 .map((accountID) => ({ + // Retrieve the display name for each participant accountID, - value: getDisplayNameForParticipant({accountID, shouldUseShortForm: isMultipleParticipantReport, personalDetailsData: personalDetails}), + name: getDisplayNameForParticipant({ + accountID, + shouldUseShortForm: true, + personalDetailsData, + }), })) - .filter((name) => name.value); - - if (!displayNames || displayNames.length === 0) { - return ''; - } - - if (displayNames.length > 1) { - return displayNames.map((name) => name.value).join(', '); - } - - return getDisplayNameForParticipant({accountID: displayNames.at(0)?.accountID, shouldUseShortForm: false, personalDetailsData: personalDetails}); -} + .filter((participant) => participant.name) // Filter out any participants without a name + .reduce((formattedNames, {name, accountID}, _, array) => { + // If there is only one participant (if it is 0 or less the function will return empty string), return their full name + if (array.length < 2) { + return getDisplayNameForParticipant({ + accountID, + personalDetailsData, + }); + } + // Concatenate names for a comma-separated list + return formattedNames ? `${formattedNames}, ${name}` : name; + }, ''); function generateReportName(report: OnyxEntry): string { if (!report) { @@ -4610,7 +4601,6 @@ function getReportName( parentReportActionParam?: OnyxInputOrEntry, personalDetails?: Partial, invoiceReceiverPolicy?: OnyxEntry, - participantPersonalDetailList?: PersonalDetails[], ): string { // Check if we can use report name in derived values - only when we have report but no other params const canUseDerivedValue = report && policy === undefined && parentReportActionParam === undefined && personalDetails === undefined && invoiceReceiverPolicy === undefined; @@ -4618,7 +4608,7 @@ function getReportName( if (canUseDerivedValue && reportAttributes?.[report.reportID]) { return reportAttributes[report.reportID].reportName; } - return getReportNameInternal({report, policy, parentReportActionParam, personalDetails, invoiceReceiverPolicy, participantPersonalDetailList}); + return getReportNameInternal({report, policy, parentReportActionParam, personalDetails, invoiceReceiverPolicy}); } function getSearchReportName(props: GetReportNameParams): string { @@ -4645,7 +4635,6 @@ function getReportNameInternal({ reports, reportNameValuePairs, policies, - participantPersonalDetailList, }: GetReportNameParams): string { const reportID = report?.reportID; @@ -4847,7 +4836,7 @@ function getReportNameInternal({ } // Not a room or PolicyExpenseChat, generate title from first 5 other participants - formattedName = buildReportNameFromParticipantNames({report, personalDetails, participantPersonalDetailList}); + formattedName = buildReportNameFromParticipantNames({report, personalDetails}); return formattedName; } diff --git a/src/libs/SidebarUtils.ts b/src/libs/SidebarUtils.ts index 62613b877d6f..e4b1ba6ef8c7 100644 --- a/src/libs/SidebarUtils.ts +++ b/src/libs/SidebarUtils.ts @@ -688,7 +688,7 @@ function getOptionData({ result.phoneNumber = personalDetail?.phoneNumber ?? ''; } - const reportName = getReportName(report, policy, undefined, undefined, invoiceReceiverPolicy, participantPersonalDetailList); + const reportName = getReportName(report, policy, undefined, undefined, invoiceReceiverPolicy); result.text = reportName; result.subtitle = subtitle; From 0abd3882fe1876c34c0318e0e3dc79b75318cc00 Mon Sep 17 00:00:00 2001 From: Jakub Korytko Date: Mon, 28 Apr 2025 13:09:04 +0200 Subject: [PATCH 3/4] Remove redundant comments --- src/libs/ReportUtils.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/libs/ReportUtils.ts b/src/libs/ReportUtils.ts index 041e25a61c83..edf086b4302c 100644 --- a/src/libs/ReportUtils.ts +++ b/src/libs/ReportUtils.ts @@ -4570,10 +4570,9 @@ function getInvoicesChatName({ const buildReportNameFromParticipantNames = ({report, personalDetails: personalDetailsData}: {report: OnyxEntry; personalDetails?: Partial}) => Object.keys(report?.participants ?? {}) .map(Number) - .filter((id) => id !== currentUserAccountID) // Exclude the current user's ID - .slice(0, 5) // Limit the number of participants to 5 + .filter((id) => id !== currentUserAccountID) + .slice(0, 5) .map((accountID) => ({ - // Retrieve the display name for each participant accountID, name: getDisplayNameForParticipant({ accountID, @@ -4581,7 +4580,7 @@ const buildReportNameFromParticipantNames = ({report, personalDetails: personalD personalDetailsData, }), })) - .filter((participant) => participant.name) // Filter out any participants without a name + .filter((participant) => participant.name) .reduce((formattedNames, {name, accountID}, _, array) => { // If there is only one participant (if it is 0 or less the function will return empty string), return their full name if (array.length < 2) { @@ -4590,7 +4589,6 @@ const buildReportNameFromParticipantNames = ({report, personalDetails: personalD personalDetailsData, }); } - // Concatenate names for a comma-separated list return formattedNames ? `${formattedNames}, ${name}` : name; }, ''); From c38f4bdc4029382ce5cda8ee514cb952c9da614b Mon Sep 17 00:00:00 2001 From: Jakub Korytko Date: Mon, 5 May 2025 13:28:10 +0200 Subject: [PATCH 4/4] Add tests for buildReportNameFromParticipantNames --- tests/unit/ReportUtilsTest.ts | 67 +++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/tests/unit/ReportUtilsTest.ts b/tests/unit/ReportUtilsTest.ts index aa95baf25374..7391cbfe6ec7 100644 --- a/tests/unit/ReportUtilsTest.ts +++ b/tests/unit/ReportUtilsTest.ts @@ -1,4 +1,5 @@ /* eslint-disable @typescript-eslint/naming-convention */ +import {beforeAll} from '@jest/globals'; import {renderHook} from '@testing-library/react-native'; import {addDays, format as formatDate} from 'date-fns'; import type {OnyxCollection, OnyxEntry} from 'react-native-onyx'; @@ -13,6 +14,7 @@ import { buildOptimisticExpenseReport, buildOptimisticIOUReportAction, buildParticipantsFromAccountIDs, + buildReportNameFromParticipantNames, buildTransactionThread, canDeleteReportAction, canEditWriteCapability, @@ -49,7 +51,9 @@ import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type {Beta, OnyxInputOrEntry, PersonalDetailsList, Policy, PolicyEmployeeList, Report, ReportAction, Transaction} from '@src/types/onyx'; import type {ErrorFields, Errors} from '@src/types/onyx/OnyxCommon'; +import type {Participant} from '@src/types/onyx/Report'; import {toCollectionDataSet} from '@src/types/utils/CollectionDataSet'; +import {chatReportR14932 as mockedChatReport} from '../../__mocks__/reportData/reports'; import * as NumberUtils from '../../src/libs/NumberUtils'; import {convertedInvoiceChat} from '../data/Invoice'; import createRandomPolicy from '../utils/collections/policies'; @@ -2456,4 +2460,67 @@ describe('ReportUtils', () => { expect(isArchivedNonExpenseReportWithID(archivedChatReport, isReportArchived.current)).toBe(true); }); }); + + describe('buildReportNameFromParticipantNames', () => { + /** + * Generates a fake report and matching personal details for specified number of participants. + * Participants in the report are directly linked with their personal details. + */ + const generateFakeReportAndParticipantsPersonalDetails = ({count, start = 0}: {count: number; start?: number}): {report: Report; personalDetails: PersonalDetailsList} => { + const data = { + report: { + ...mockedChatReport, + participants: Object.keys(fakePersonalDetails) + .slice(start, count) + .reduce>((acc, cur) => { + acc[cur] = {notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.ALWAYS}; + return acc; + }, {}), + }, + personalDetails: Object.fromEntries(Object.entries(fakePersonalDetails).slice(start, count)), + }; + + data.personalDetails[currentUserAccountID] = { + accountID: currentUserAccountID, + displayName: 'CURRENT USER', + firstName: 'CURRENT', + }; + + return data; + }; + + it('excludes the current user from the report title', () => { + const result = buildReportNameFromParticipantNames(generateFakeReportAndParticipantsPersonalDetails({count: currentUserAccountID + 2})); + expect(result).not.toContain('CURRENT'); + }); + + it('limits to a maximum of 5 participants in the title', () => { + const result = buildReportNameFromParticipantNames(generateFakeReportAndParticipantsPersonalDetails({count: 10})); + expect(result.split(',').length).toBeLessThanOrEqual(5); + }); + + it('returns full name if only one participant is present (excluding current user)', () => { + const result = buildReportNameFromParticipantNames(generateFakeReportAndParticipantsPersonalDetails({count: 1})); + const {displayName} = fakePersonalDetails[1] ?? {}; + expect(result).toEqual(displayName); + }); + + it('returns an empty string if there are no participants or all are excluded', () => { + const result = buildReportNameFromParticipantNames(generateFakeReportAndParticipantsPersonalDetails({start: currentUserAccountID - 1, count: 1})); + expect(result).toEqual(''); + }); + + it('handles partial or missing personal details correctly', () => { + const {report} = generateFakeReportAndParticipantsPersonalDetails({count: 6}); + + const secondUser = fakePersonalDetails[2]; + const fourthUser = fakePersonalDetails[4]; + + const incompleteDetails = {2: secondUser, 4: fourthUser}; + const result = buildReportNameFromParticipantNames({report, personalDetails: incompleteDetails}); + const expectedNames = [secondUser?.firstName, fourthUser?.firstName].sort(); + const resultNames = result.split(', ').sort(); + expect(resultNames).toEqual(expect.arrayContaining(expectedNames)); + }); + }); });