-
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
refactor: move methods to personalDetailsUtils, remove unused one #34657
Merged
puneetlath
merged 12 commits into
Expensify:main
from
koko57:refactor/31312-consolidate-getdisplayname-methods-pt3
Feb 5, 2024
Merged
Changes from 7 commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
fad567c
refactor: move methods to personalDetailsUtils, remove unused one
koko57 ab8dcab
Merge branch 'main' into refactor/31312-consolidate-getdisplayname-me…
koko57 dc76940
fix: run prettier
koko57 23647c7
Merge branch 'main' into refactor/31312-consolidate-getdisplayname-me…
koko57 84fb28a
Merge branch 'main' into refactor/31312-consolidate-getdisplayname-me…
koko57 066bd29
Merge branch 'main' into refactor/31312-consolidate-getdisplayname-me…
koko57 205f9ee
Merge branch 'main' into refactor/31312-consolidate-getdisplayname-me…
koko57 93d8f75
Merge branch 'main' into refactor/31312-consolidate-getdisplayname-me…
koko57 b0970b6
fix: remove params
koko57 c0b1426
Merge branch 'main' into refactor/31312-consolidate-getdisplayname-me…
koko57 c8e9dc1
fix: resolve conflicts
koko57 6fee255
Merge branch 'main' into refactor/31312-consolidate-getdisplayname-me…
koko57 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,3 +1,4 @@ | ||||||||||||
import Str from 'expensify-common/lib/str'; | ||||||||||||
import type {OnyxEntry, OnyxUpdate} from 'react-native-onyx'; | ||||||||||||
import Onyx from 'react-native-onyx'; | ||||||||||||
import CONST from '@src/CONST'; | ||||||||||||
|
@@ -8,6 +9,11 @@ import * as LocalePhoneNumber from './LocalePhoneNumber'; | |||||||||||
import * as Localize from './Localize'; | ||||||||||||
import * as UserUtils from './UserUtils'; | ||||||||||||
|
||||||||||||
type FirstAndLastName = { | ||||||||||||
firstName: string; | ||||||||||||
lastName: string; | ||||||||||||
}; | ||||||||||||
|
||||||||||||
let personalDetails: Array<PersonalDetails | null> = []; | ||||||||||||
let allPersonalDetails: OnyxEntry<PersonalDetailsList> = {}; | ||||||||||||
Onyx.connect({ | ||||||||||||
|
@@ -195,6 +201,66 @@ function getEffectiveDisplayName(personalDetail?: PersonalDetails): string | und | |||||||||||
return undefined; | ||||||||||||
} | ||||||||||||
|
||||||||||||
/** | ||||||||||||
* Creates a new displayName for a user based on passed personal details or login. | ||||||||||||
* | ||||||||||||
* @param login - user's login | ||||||||||||
* @param passedPersonalDetails - details object with firstName and lastName | ||||||||||||
* @returns - The effective display name | ||||||||||||
*/ | ||||||||||||
function createDisplayName(login: string, passedPersonalDetails: 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); | ||||||||||||
|
||||||||||||
if (!passedPersonalDetails) { | ||||||||||||
return userLogin; | ||||||||||||
} | ||||||||||||
|
||||||||||||
const firstName = passedPersonalDetails.firstName ?? ''; | ||||||||||||
const lastName = passedPersonalDetails.lastName ?? ''; | ||||||||||||
const fullName = `${firstName} ${lastName}`.trim(); | ||||||||||||
|
||||||||||||
// It's possible for fullName to be empty string, so we must use "||" to fallback to userLogin. | ||||||||||||
return fullName || userLogin; | ||||||||||||
} | ||||||||||||
|
||||||||||||
/** | ||||||||||||
* Gets the first and last name from the user's personal details. | ||||||||||||
* If the login is the same as the displayName, then they don't exist, | ||||||||||||
* so we return empty strings instead. | ||||||||||||
* | ||||||||||||
* @param login - user's login | ||||||||||||
* @param displayName - user display name | ||||||||||||
* @param firstName | ||||||||||||
* @param lastName | ||||||||||||
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. Same here
Suggested change
|
||||||||||||
*/ | ||||||||||||
function extractFirstAndLastNameFromAvailableDetails({login, displayName, firstName, lastName}: PersonalDetails): FirstAndLastName { | ||||||||||||
// It's possible for firstName to be empty string, so we must use "||" to consider lastName instead. | ||||||||||||
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing | ||||||||||||
if (firstName || lastName) { | ||||||||||||
return {firstName: firstName ?? '', lastName: lastName ?? ''}; | ||||||||||||
} | ||||||||||||
if (login && Str.removeSMSDomain(login) === displayName) { | ||||||||||||
return {firstName: '', lastName: ''}; | ||||||||||||
} | ||||||||||||
|
||||||||||||
if (displayName) { | ||||||||||||
const firstSpaceIndex = displayName.indexOf(' '); | ||||||||||||
const lastSpaceIndex = displayName.lastIndexOf(' '); | ||||||||||||
if (firstSpaceIndex === -1) { | ||||||||||||
return {firstName: displayName, lastName: ''}; | ||||||||||||
} | ||||||||||||
|
||||||||||||
return { | ||||||||||||
firstName: displayName.substring(0, firstSpaceIndex).trim(), | ||||||||||||
lastName: displayName.substring(lastSpaceIndex).trim(), | ||||||||||||
}; | ||||||||||||
} | ||||||||||||
|
||||||||||||
return {firstName: '', lastName: ''}; | ||||||||||||
} | ||||||||||||
|
||||||||||||
export { | ||||||||||||
getDisplayNameOrDefault, | ||||||||||||
getPersonalDetailsByIDs, | ||||||||||||
|
@@ -205,4 +271,6 @@ export { | |||||||||||
getFormattedStreet, | ||||||||||||
getStreetLines, | ||||||||||||
getEffectiveDisplayName, | ||||||||||||
createDisplayName, | ||||||||||||
extractFirstAndLastNameFromAvailableDetails, | ||||||||||||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This is not needed in TS if params are self-explanatory
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.
@situchan but other methods in this file also have params listed
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.
https://github.com/Expensify/App/blob/main/contributingGuides/TS_STYLE.md#jsdoc
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.
@situchan should I remove the params from other methods' docs as well? wdyt?
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.
not a big deal
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.
Let’s remove it from this method though, yeah?