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

Refactor/31312 consolidate getdisplayname methods pt2 #33930

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 7 additions & 15 deletions src/libs/GroupChatUtils.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,18 @@
import type {OnyxEntry} from 'react-native-onyx';
import Onyx from 'react-native-onyx';
import ONYXKEYS from '@src/ONYXKEYS';
import type {PersonalDetailsList, Report} from '@src/types/onyx';
import * as OptionsListUtils from './OptionsListUtils';
import type {Report} from '@src/types/onyx';
import * as ReportUtils from './ReportUtils';

let allPersonalDetails: OnyxEntry<PersonalDetailsList> = {};
Onyx.connect({
key: ONYXKEYS.PERSONAL_DETAILS_LIST,
callback: (val) => (allPersonalDetails = val),
});

/**
* Returns the report name if the report is a group chat
*/
function getGroupChatName(report: Report): string | undefined {
const participants = report.participantAccountIDs ?? [];
const isMultipleParticipantReport = participants.length > 1;
const participantPersonalDetails = OptionsListUtils.getPersonalDetailsForAccountIDs(participants, allPersonalDetails ?? {});
// @ts-expect-error Error will gone when OptionsListUtils will be migrated to Typescript
const displayNamesWithTooltips = ReportUtils.getDisplayNamesWithTooltips(participantPersonalDetails, isMultipleParticipantReport);
return ReportUtils.getDisplayNamesStringFromTooltips(displayNamesWithTooltips);

return participants
.map((participant) => ReportUtils.getDisplayNameForParticipant(participant, isMultipleParticipantReport))
.sort((first, second) => first?.localeCompare(second ?? '') ?? 0)
.filter(Boolean)
.join(', ');
}

export {
Expand Down
13 changes: 0 additions & 13 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1488,7 +1488,6 @@ function getDisplayNameForParticipant(accountID?: number, shouldUseShortForm = f
}

const personalDetails = getPersonalDetailsForAccountID(accountID);
// console.log(personalDetails);
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
const formattedLogin = LocalePhoneNumber.formatPhoneNumber(personalDetails.login || '');
// This is to check if account is an invite/optimistically created one
Expand Down Expand Up @@ -1550,17 +1549,6 @@ function getDisplayNamesWithTooltips(
});
}

/**
* Gets a joined string of display names from the list of display name with tooltip objects.
*
*/
function getDisplayNamesStringFromTooltips(displayNamesWithTooltips: DisplayNameWithTooltips | undefined) {
return displayNamesWithTooltips
?.map(({displayName}) => displayName)
.filter(Boolean)
.join(', ');
}

/**
* For a deleted parent report action within a chat report,
* let us return the appropriate display message
Expand Down Expand Up @@ -4406,7 +4394,6 @@ export {
getIcons,
getRoomWelcomeMessage,
getDisplayNamesWithTooltips,
getDisplayNamesStringFromTooltips,
getReportName,
getReport,
getReportNotificationPreference,
Expand Down
15 changes: 7 additions & 8 deletions src/libs/actions/PersonalDetails.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,20 +43,19 @@ Onyx.connect({
});

/**
* Returns the displayName for a user
* Creates a new displayName for a user based on passed personal details or login.
*/
function getDisplayName(login: string, personalDetail: Pick<PersonalDetails, 'firstName' | 'lastName'> | null): string {
function createDisplayName(login: string, personalDetails: Pick<PersonalDetails, 'firstName' | 'lastName'> | OnyxEntry<PersonalDetails>): string {
// If we have a number like +15857527441@expensify.sms then let's remove @expensify.sms and format it
// so that the option looks cleaner in our UI.
const userLogin = LocalePhoneNumber.formatPhoneNumber(login);
const userDetails = personalDetail ?? allPersonalDetails?.[login];
Copy link
Contributor

Choose a reason for hiding this comment

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

- ?? allPersonalDetails?.[login];

For 2nd case, this is major change. Others are just code refactor, not affecting logic, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@situchan could you explain what do you mean by major change? We're always passing personalDetails object that should not be empty, so I removed this check

Copy link
Contributor

Choose a reason for hiding this comment

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

logic change I mean. Others are just variable re-naming, etc


if (!userDetails) {
if (!personalDetails) {
return userLogin;
}

const firstName = userDetails.firstName ?? '';
const lastName = userDetails.lastName ?? '';
const firstName = personalDetails.firstName ?? '';
const lastName = personalDetails.lastName ?? '';
const fullName = `${firstName} ${lastName}`.trim();

// It's possible for fullName to be empty string, so we must use "||" to fallback to userLogin.
Expand Down Expand Up @@ -150,7 +149,7 @@ function updateDisplayName(firstName: string, lastName: string) {
[currentUserAccountID]: {
firstName,
lastName,
displayName: getDisplayName(currentUserEmail ?? '', {
displayName: createDisplayName(currentUserEmail ?? '', {
firstName,
lastName,
}),
Expand Down Expand Up @@ -566,7 +565,7 @@ export {
deleteAvatar,
extractFirstAndLastNameFromAvailableDetails,
getCountryISO,
getDisplayName,
createDisplayName,
getPrivatePersonalDetails,
openPersonalDetailsPage,
openPublicProfilePage,
Expand Down
2 changes: 1 addition & 1 deletion src/libs/actions/User.ts
Original file line number Diff line number Diff line change
Expand Up @@ -717,7 +717,7 @@ function setContactMethodAsDefault(newDefaultContactMethod: string) {
value: {
[currentUserAccountID]: {
login: newDefaultContactMethod,
displayName: PersonalDetails.getDisplayName(newDefaultContactMethod, myPersonalDetails),
displayName: PersonalDetails.createDisplayName(newDefaultContactMethod, myPersonalDetails),
},
},
},
Expand Down
Loading