Repository navigation
fix: fixing failing tests for OptionsListUtils and ReportUtils suites
#20371
There are no files selected for viewing
This file contains hidden or 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 hidden or 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 |
|---|---|---|
|
|
@@ -161,36 +161,36 @@ function getAvatarsForLogins(logins, personalDetails) { | |
| } | ||
|
|
||
| /** | ||
| * Returns the personal details for an array of accountIDs | ||
| * | ||
| * @param {Array} accountIDs | ||
| * @param {Object} personalDetails | ||
| * @returns {Object} – keys of the object are emails, values are PersonalDetails objects. | ||
| */ | ||
| function getPersonalDetailsForAccountIDs(accountIDs, personalDetails) { | ||
| const personalDetailsForLogins = {}; | ||
| if (!personalDetails) { | ||
| return personalDetailsForLogins; | ||
| } | ||
| _.chain(accountIDs) | ||
| // NOTE: this comment is possibly legacy, we need to verify if it's still true | ||
| // Somehow it's possible for the accountIDs coming from report.participantAccountIDs to contain undefined values so we use compact to remove them. | ||
|
BeeMargarida marked this conversation as resolved.
|
||
| .compact() | ||
| .each((accountID) => { | ||
| let personalDetail = personalDetails[accountID]; | ||
| if (!personalDetail) { | ||
| // NOTE: this can possibly be changed to 'hidden' | ||
| personalDetail = { | ||
| accountID, | ||
|
Contributor
There was a problem hiding this comment. One thing @puneetlath and I are thinking is: If we don't have a display name to show we maybbbe want to just show "hidden" everywhere, OR we want to hide this - this is something we'll have to keep in mind when doing final testing
Contributor
Author
There was a problem hiding this comment. Ah, have the displayName be 'hidden'? Do we want to change this right now, or is it something still being discussed?
Contributor
There was a problem hiding this comment. Well we will plan to show "hidden" on the the profile page that we're updating (PR: #20144) - IFF you don't have that person's personal details in Onyx. So we were just "thinking" this may apply to other pages in the app, but I'd say let's just see what happens as we go - I think it would be better if we try to figure out "why" personal details may or may not be available in this scenario, and handle it from there, not just add "hidden" everywhere this happens and gloss over the why 😅 |
||
| avatar: UserUtils.getDefaultAvatar(accountID), | ||
| }; | ||
| } | ||
|
|
||
| if (accountID === CONST.ACCOUNT_ID.CONCIERGE) { | ||
| personalDetail.avatar = CONST.CONCIERGE_ICON_URL; | ||
| } | ||
|
|
||
| personalDetailsForLogins[accountID] = personalDetail; | ||
| }); | ||
| return personalDetailsForLogins; | ||
| } | ||
|
|
@@ -201,7 +201,7 @@ function getPersonalDetailsForLogins(logins, personalDetails) { | |
| * @returns {Boolean} | ||
| */ | ||
| function isPersonalDetailsReady(personalDetails) { | ||
| return !_.isEmpty(personalDetails) && _.some(_.keys(personalDetails), (key) => personalDetails[key].accountID); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -212,7 +212,7 @@ function isPersonalDetailsReady(personalDetails) { | |
| */ | ||
| function getParticipantsOptions(report, personalDetails) { | ||
| const participants = lodashGet(report, 'participants', []); | ||
| return _.map(getPersonalDetailsForAccountIDs(participants, personalDetails), (details) => ({ | ||
| keyForList: details.login, | ||
| login: details.login, | ||
| text: details.displayName, | ||
|
|
@@ -302,10 +302,12 @@ function getSearchText(report, reportName, personalDetailList, isChatRoomOrPolic | |
| for (let i = 0; i < personalDetailList.length; i++) { | ||
| const personalDetail = personalDetailList[i]; | ||
|
|
||
| if (personalDetail.login) { | ||
| // The regex below is used to remove dots only from the local part of the user email (local-part@domain) | ||
| // so that we can match emails that have dots without explicitly writing the dots (e.g: fistlast@domain will match first.last@domain) | ||
| // More info https://github.com/Expensify/App/issues/8007 | ||
| searchTerms = searchTerms.concat([personalDetail.displayName, personalDetail.login, personalDetail.login.replace(/\.(?=[^\s@]*@)/g, '')]); | ||
| } | ||
| } | ||
| } | ||
| if (report) { | ||
|
|
@@ -362,7 +364,7 @@ function getAllReportErrors(report, reportActions) { | |
| /** | ||
| * Creates a report list option | ||
| * | ||
| * @param {Array<String>} accountIDs | ||
| * @param {Object} personalDetails | ||
| * @param {Object} report | ||
| * @param {Object} reportActions | ||
|
|
@@ -371,7 +373,7 @@ function getAllReportErrors(report, reportActions) { | |
| * @param {Boolean} [options.forcePolicyNamePreview] | ||
| * @returns {Object} | ||
| */ | ||
| function createOption(accountIDs, personalDetails, report, reportActions = {}, {showChatPreviewLine = false, forcePolicyNamePreview = false}) { | ||
| const result = { | ||
| text: null, | ||
| alternateText: null, | ||
|
|
@@ -402,7 +404,7 @@ function createOption(logins, personalDetails, report, reportActions = {}, {show | |
| isPolicyExpenseChat: false, | ||
| }; | ||
|
|
||
| const personalDetailMap = getPersonalDetailsForAccountIDs(accountIDs, personalDetails); | ||
| const personalDetailList = _.values(personalDetailMap); | ||
| const personalDetail = personalDetailList[0] || {}; | ||
| let hasMultipleParticipants = personalDetailList.length > 1; | ||
|
|
@@ -443,7 +445,7 @@ function createOption(logins, personalDetails, report, reportActions = {}, {show | |
| lastMessageTextFromReport = report ? report.lastMessageText || '' : ''; | ||
| } | ||
|
|
||
| const lastActorDetails = personalDetailMap[report.lastActorAccountID] || null; | ||
| let lastMessageText = hasMultipleParticipants && lastActorDetails && lastActorDetails.login !== currentUserLogin ? `${lastActorDetails.displayName}: ` : ''; | ||
| lastMessageText += report ? lastMessageTextFromReport : ''; | ||
|
|
||
|
|
@@ -466,10 +468,9 @@ function createOption(logins, personalDetails, report, reportActions = {}, {show | |
| } | ||
| reportName = ReportUtils.getReportName(report); | ||
| } else { | ||
| reportName = ReportUtils.getDisplayNameForParticipant(accountIDs[0]); | ||
| result.keyForList = accountIDs[0]; | ||
| result.alternateText = ''; | ||
| } | ||
|
|
||
| result.isIOUReportOwner = ReportUtils.isIOUOwnedByCurrentUser(result, iouReports); | ||
|
|
@@ -483,7 +484,7 @@ function createOption(logins, personalDetails, report, reportActions = {}, {show | |
|
|
||
| result.text = reportName; | ||
| result.searchText = getSearchText(report, reportName, personalDetailList, result.isChatRoom || result.isPolicyExpenseChat, result.isThread); | ||
| result.icons = ReportUtils.getIcons(report, personalDetails, UserUtils.getAvatar(personalDetail.avatar, personalDetail.accountID)); | ||
| result.subtitle = subtitle; | ||
|
|
||
| return result; | ||
|
|
@@ -572,7 +573,7 @@ function getOptions( | |
|
|
||
| let recentReportOptions = []; | ||
| let personalDetailsOptions = []; | ||
| const reportMapForAccountIDs = {}; | ||
| const parsedPhoneNumber = parsePhoneNumber(LoginUtils.appendCountryCode(searchInputValue)); | ||
| const searchValue = parsedPhoneNumber.possible ? parsedPhoneNumber.number.e164 : searchInputValue; | ||
|
|
||
|
|
@@ -603,7 +604,7 @@ function getOptions( | |
| const isChatRoom = ReportUtils.isChatRoom(report); | ||
| const isTaskReport = ReportUtils.isTaskReport(report); | ||
| const isPolicyExpenseChat = ReportUtils.isPolicyExpenseChat(report); | ||
| const accountIDs = report.participantAccountIDs || []; | ||
|
|
||
| if (isPolicyExpenseChat && report.isOwnPolicyExpenseChat && !includeOwnedWorkspaceChats) { | ||
| return; | ||
|
|
@@ -620,8 +621,8 @@ function getOptions( | |
| // Save the report in the map if this is a single participant so we can associate the reportID with the | ||
| // personal detail option later. Individuals should not be associated with single participant | ||
| // policyExpenseChats or chatRooms since those are not people. | ||
| if (accountIDs.length <= 1 && !isPolicyExpenseChat && !isChatRoom) { | ||
| reportMapForAccountIDs[accountIDs[0]] = report; | ||
| } | ||
| const isSearchingSomeonesPolicyExpenseChat = !report.isOwnPolicyExpenseChat && searchValue !== ''; | ||
|
|
||
|
|
@@ -630,15 +631,15 @@ function getOptions( | |
| const isPolicyChatAdmin = ReportUtils.isPolicyExpenseChatAdmin(report, policies); | ||
|
|
||
| allReportOptions.push( | ||
| createOption(accountIDs, personalDetails, report, reportActions, { | ||
| showChatPreviewLine, | ||
| forcePolicyNamePreview: isPolicyExpenseChat ? isSearchingSomeonesPolicyExpenseChat || isPolicyChatAdmin : forcePolicyNamePreview, | ||
| }), | ||
| ); | ||
| }); | ||
|
|
||
| let allPersonalDetailsOptions = _.map(personalDetails, (personalDetail) => | ||
| createOption([personalDetail.accountID], personalDetails, reportMapForAccountIDs[personalDetail.accountID], reportActions, { | ||
| showChatPreviewLine, | ||
| forcePolicyNamePreview, | ||
| }), | ||
|
|
@@ -727,15 +728,17 @@ function getOptions( | |
| !_.find(loginOptionsToExclude, (loginOptionToExclude) => loginOptionToExclude.login === addSMSDomainIfPhoneNumber(searchValue).toLowerCase()) && | ||
| (searchValue !== CONST.EMAIL.CHRONOS || Permissions.canUseChronos(betas)) | ||
| ) { | ||
| const fakeAccountID = UserUtils.generateAccountID(); | ||
| userToInvite = createOption([fakeAccountID], personalDetails, null, reportActions, { | ||
| showChatPreviewLine, | ||
| }); | ||
| userToInvite.login = searchValue; | ||
|
|
||
| // If user doesn't exist, use a default avatar | ||
| userToInvite.icons = [ | ||
| { | ||
| source: UserUtils.getAvatar('', fakeAccountID), | ||
| login: searchValue, | ||
| type: CONST.ICON_TYPE_AVATAR, | ||
| }, | ||
| ]; | ||
|
|
@@ -960,7 +963,7 @@ export { | |
| getShareDestinationOptions, | ||
| getMemberInviteOptions, | ||
| getHeaderMessage, | ||
| getPersonalDetailsForAccountIDs, | ||
| getIOUConfirmationOptionsFromPayeePersonalDetail, | ||
| getIOUConfirmationOptionsFromParticipants, | ||
| getSearchText, | ||
|
|
||
This file contains hidden or 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 hidden or 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.
I thinkkkk this is one thing we'll have to modify - because in expensify employee dev environments this account ID is definitely different - so we'll have to read the account ID from the
.envfile.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.
Is this being extracted anywhere? I tried looking at
CONFIG.jsto see the name of the variable, but there's no mentioned of a concierge account idThere 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 yet, I just think we'll need to do that in our migration since the email will always stay the same, but account IDs won't 😅
It issss possible we can have AppInit send the concierge accountID instead and store that in Onyx or something 🤔
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.
Oh right, it makes sense. Hum, I'll add a note here, so that this not get forgotten.
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.
That sounds fantastic, thanks!