Skip to content

Commit

Permalink
Merge pull request #20371 from BeeMargarida/migration-OptionsListUtils
Browse files Browse the repository at this point in the history 
fix: fixing failing tests for `OptionsListUtils` and `ReportUtils` suites
  • Loading branch information
@Beamanator
Beamanator authored Jun 7, 2023
2 parents e8b7f86 + 3ca4e0d commit fde3aff
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 59 deletions.
4 changes: 4 additions & 0 deletions src/CONST.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -872,6 +872,10 @@ const CONST = {
GUIDES_DOMAIN: 'team.expensify.com',
},

ACCOUNT_ID: {
CONCIERGE: '8392101',
},

ENVIRONMENT: {
DEV: 'development',
STAGING: 'staging',
Expand Down
79 changes: 41 additions & 38 deletions src/libs/OptionsListUtils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,36 +161,36 @@ function getAvatarsForLogins(logins, personalDetails) {
}

/**
* Returns the personal details for an array of logins
* Returns the personal details for an array of accountIDs
*
* @param {Array} logins
* @param {Array} accountIDs
* @param {Object} personalDetails
* @returns {Object} – keys of the object are emails, values are PersonalDetails objects.
*/
function getPersonalDetailsForLogins(logins, personalDetails) {
function getPersonalDetailsForAccountIDs(accountIDs, personalDetails) {
const personalDetailsForLogins = {};
if (!personalDetails) {
return personalDetailsForLogins;
}
_.chain(logins)

// Somehow it's possible for the logins coming from report.participants to contain undefined values so we use compact to remove them.
_.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.
.compact()
.each((login) => {
let personalDetail = personalDetails[login];
.each((accountID) => {
let personalDetail = personalDetails[accountID];
if (!personalDetail) {
// NOTE: this can possibly be changed to 'hidden'
personalDetail = {
login,
displayName: LocalePhoneNumber.formatPhoneNumber(login),
avatar: UserUtils.getDefaultAvatar(login),
accountID,
avatar: UserUtils.getDefaultAvatar(accountID),
};
}

if (login === CONST.EMAIL.CONCIERGE) {
if (accountID === CONST.ACCOUNT_ID.CONCIERGE) {
personalDetail.avatar = CONST.CONCIERGE_ICON_URL;
}

personalDetailsForLogins[login] = personalDetail;
personalDetailsForLogins[accountID] = personalDetail;
});
return personalDetailsForLogins;
}
Expand All @@ -201,7 +201,7 @@ function getPersonalDetailsForLogins(logins, personalDetails) {
* @returns {Boolean}
*/
function isPersonalDetailsReady(personalDetails) {
return !_.isEmpty(personalDetails) && _.some(_.keys(personalDetails), (key) => personalDetails[key].login);
return !_.isEmpty(personalDetails) && _.some(_.keys(personalDetails), (key) => personalDetails[key].accountID);
}

/**
Expand All @@ -212,7 +212,7 @@ function isPersonalDetailsReady(personalDetails) {
*/
function getParticipantsOptions(report, personalDetails) {
const participants = lodashGet(report, 'participants', []);
return _.map(getPersonalDetailsForLogins(participants, personalDetails), (details) => ({
return _.map(getPersonalDetailsForAccountIDs(participants, personalDetails), (details) => ({
keyForList: details.login,
login: details.login,
text: details.displayName,
Expand Down Expand Up @@ -302,10 +302,12 @@ function getSearchText(report, reportName, personalDetailList, isChatRoomOrPolic
for (let i = 0; i < personalDetailList.length; i++) {
const personalDetail = personalDetailList[i];

// 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 (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) {
Expand Down Expand Up @@ -362,7 +364,7 @@ function getAllReportErrors(report, reportActions) {
/**
* Creates a report list option
*
* @param {Array<String>} logins
* @param {Array<String>} accountIDs
* @param {Object} personalDetails
* @param {Object} report
* @param {Object} reportActions
Expand All @@ -371,7 +373,7 @@ function getAllReportErrors(report, reportActions) {
* @param {Boolean} [options.forcePolicyNamePreview]
* @returns {Object}
*/
function createOption(logins, personalDetails, report, reportActions = {}, {showChatPreviewLine = false, forcePolicyNamePreview = false}) {
function createOption(accountIDs, personalDetails, report, reportActions = {}, {showChatPreviewLine = false, forcePolicyNamePreview = false}) {
const result = {
text: null,
alternateText: null,
Expand Down Expand Up @@ -402,7 +404,7 @@ function createOption(logins, personalDetails, report, reportActions = {}, {show
isPolicyExpenseChat: false,
};

const personalDetailMap = getPersonalDetailsForLogins(logins, personalDetails);
const personalDetailMap = getPersonalDetailsForAccountIDs(accountIDs, personalDetails);
const personalDetailList = _.values(personalDetailMap);
const personalDetail = personalDetailList[0] || {};
let hasMultipleParticipants = personalDetailList.length > 1;
Expand Down Expand Up @@ -443,7 +445,7 @@ function createOption(logins, personalDetails, report, reportActions = {}, {show
lastMessageTextFromReport = report ? report.lastMessageText || '' : '';
}

const lastActorDetails = personalDetailMap[report.lastActorEmail] || null;
const lastActorDetails = personalDetailMap[report.lastActorAccountID] || null;
let lastMessageText = hasMultipleParticipants && lastActorDetails && lastActorDetails.login !== currentUserLogin ? `${lastActorDetails.displayName}: ` : '';
lastMessageText += report ? lastMessageTextFromReport : '';

Expand All @@ -466,10 +468,9 @@ function createOption(logins, personalDetails, report, reportActions = {}, {show
}
reportName = ReportUtils.getReportName(report);
} else {
const login = logins[0];
reportName = ReportUtils.getDisplayNameForParticipant(login);
result.keyForList = login;
result.alternateText = LocalePhoneNumber.formatPhoneNumber(login);
reportName = ReportUtils.getDisplayNameForParticipant(accountIDs[0]);
result.keyForList = accountIDs[0];
result.alternateText = '';
}

result.isIOUReportOwner = ReportUtils.isIOUOwnedByCurrentUser(result, iouReports);
Expand All @@ -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.login));
result.icons = ReportUtils.getIcons(report, personalDetails, UserUtils.getAvatar(personalDetail.avatar, personalDetail.accountID));
result.subtitle = subtitle;

return result;
Expand Down Expand Up @@ -572,7 +573,7 @@ function getOptions(

let recentReportOptions = [];
let personalDetailsOptions = [];
const reportMapForLogins = {};
const reportMapForAccountIDs = {};
const parsedPhoneNumber = parsePhoneNumber(LoginUtils.appendCountryCode(searchInputValue));
const searchValue = parsedPhoneNumber.possible ? parsedPhoneNumber.number.e164 : searchInputValue;

Expand Down Expand Up @@ -603,7 +604,7 @@ function getOptions(
const isChatRoom = ReportUtils.isChatRoom(report);
const isTaskReport = ReportUtils.isTaskReport(report);
const isPolicyExpenseChat = ReportUtils.isPolicyExpenseChat(report);
const logins = report.participants || [];
const accountIDs = report.participantAccountIDs || [];

if (isPolicyExpenseChat && report.isOwnPolicyExpenseChat && !includeOwnedWorkspaceChats) {
return;
Expand All @@ -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 (logins.length <= 1 && !isPolicyExpenseChat && !isChatRoom) {
reportMapForLogins[logins[0]] = report;
if (accountIDs.length <= 1 && !isPolicyExpenseChat && !isChatRoom) {
reportMapForAccountIDs[accountIDs[0]] = report;
}
const isSearchingSomeonesPolicyExpenseChat = !report.isOwnPolicyExpenseChat && searchValue !== '';

Expand All @@ -630,15 +631,15 @@ function getOptions(
const isPolicyChatAdmin = ReportUtils.isPolicyExpenseChatAdmin(report, policies);

allReportOptions.push(
createOption(logins, personalDetails, report, reportActions, {
createOption(accountIDs, personalDetails, report, reportActions, {
showChatPreviewLine,
forcePolicyNamePreview: isPolicyExpenseChat ? isSearchingSomeonesPolicyExpenseChat || isPolicyChatAdmin : forcePolicyNamePreview,
}),
);
});

let allPersonalDetailsOptions = _.map(personalDetails, (personalDetail) =>
createOption([personalDetail.login], personalDetails, reportMapForLogins[personalDetail.login], reportActions, {
createOption([personalDetail.accountID], personalDetails, reportMapForAccountIDs[personalDetail.accountID], reportActions, {
showChatPreviewLine,
forcePolicyNamePreview,
}),
Expand Down Expand Up @@ -727,15 +728,17 @@ function getOptions(
!_.find(loginOptionsToExclude, (loginOptionToExclude) => loginOptionToExclude.login === addSMSDomainIfPhoneNumber(searchValue).toLowerCase()) &&
(searchValue !== CONST.EMAIL.CHRONOS || Permissions.canUseChronos(betas))
) {
userToInvite = createOption([searchValue], personalDetails, null, reportActions, {
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('', searchValue),
name: searchValue,
source: UserUtils.getAvatar('', fakeAccountID),
login: searchValue,
type: CONST.ICON_TYPE_AVATAR,
},
];
Expand Down Expand Up @@ -960,7 +963,7 @@ export {
getShareDestinationOptions,
getMemberInviteOptions,
getHeaderMessage,
getPersonalDetailsForLogins,
getPersonalDetailsForAccountIDs,
getIOUConfirmationOptionsFromPayeePersonalDetail,
getIOUConfirmationOptionsFromParticipants,
getSearchText,
Expand Down
39 changes: 20 additions & 19 deletions src/libs/ReportUtils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,17 @@ import DateUtils from './DateUtils';
import linkingConfig from './Navigation/linkingConfig';
import isReportMessageAttachment from './isReportMessageAttachment';
import * as defaultWorkspaceAvatars from '../components/Icon/WorkspaceDefaultAvatars';
import * as LocalePhoneNumber from './LocalePhoneNumber';
import * as CurrencyUtils from './CurrencyUtils';
import * as UserUtils from './UserUtils';

let sessionEmail;
let sessionAccountID;
Onyx.connect({
key: ONYXKEYS.SESSION,
callback: (val) => (sessionEmail = val ? val.email : null),
callback: (val) => {
sessionEmail = val ? val.email : null;
sessionAccountID = val ? val.accountID : null;
},
});

let preferredLocale = CONST.LOCALES.DEFAULT;
Expand Down Expand Up @@ -829,34 +832,32 @@ function getIcons(report, personalDetails, defaultIcon = null, isPayer = false)
/**
* Gets the personal details for a login by looking in the ONYXKEYS.PERSONAL_DETAILS_LIST Onyx key (stored in the local variable, allPersonalDetails). If it doesn't exist in Onyx,
* then a default object is constructed.
* @param {String} login
* @param {String} accountID
* @returns {Object}
*/
function getPersonalDetailsForLogin(login) {
if (!login) {
function getPersonalDetailsForAccountID(accountID) {
if (!accountID) {
return {};
}
return (
(allPersonalDetails && allPersonalDetails[login]) || {
login,
displayName: LocalePhoneNumber.formatPhoneNumber(login),
avatar: UserUtils.getDefaultAvatar(login),
(allPersonalDetails && allPersonalDetails[accountID]) || {
avatar: UserUtils.getDefaultAvatar(accountID),
}
);
}

/**
* Get the displayName for a single report participant.
*
* @param {String} login
* @param {String} accountID
* @param {Boolean} [shouldUseShortForm]
* @returns {String}
*/
function getDisplayNameForParticipant(login, shouldUseShortForm = false) {
if (!login) {
function getDisplayNameForParticipant(accountID, shouldUseShortForm = false) {
if (!accountID) {
return '';
}
const personalDetails = getPersonalDetailsForLogin(login);
const personalDetails = getPersonalDetailsForAccountID(accountID);

const longName = personalDetails.displayName;

Expand All @@ -872,7 +873,7 @@ function getDisplayNameForParticipant(login, shouldUseShortForm = false) {
*/
function getDisplayNamesWithTooltips(participants, isMultipleParticipantReport) {
return _.map(participants, (participant) => {
const displayName = getDisplayNameForParticipant(participant.login, isMultipleParticipantReport);
const displayName = getDisplayNameForParticipant(participant.accountID, isMultipleParticipantReport);
const tooltip = participant.login ? Str.removeSMSDomain(participant.login) : '';

let pronouns = participant.pronouns;
Expand Down Expand Up @@ -943,7 +944,7 @@ function getMoneyRequestTotal(report, moneyRequestReports = {}) {
* @returns {String}
*/
function getPolicyExpenseChatName(report) {
const reportOwnerDisplayName = getDisplayNameForParticipant(report.ownerEmail) || report.ownerEmail || report.reportName;
const reportOwnerDisplayName = getDisplayNameForParticipant(report.ownerAccountID) || report.ownerEmail || report.reportName;

// If the policy expense chat is owned by this user, use the name of the policy as the report name.
if (report.isOwnPolicyExpenseChat) {
Expand Down Expand Up @@ -974,7 +975,7 @@ function getPolicyExpenseChatName(report) {
*/
function getMoneyRequestReportName(report) {
const formattedAmount = CurrencyUtils.convertToDisplayString(getMoneyRequestTotal(report), report.currency);
const payerName = isExpenseReport(report) ? getPolicyName(report) : getDisplayNameForParticipant(report.managerEmail);
const payerName = isExpenseReport(report) ? getPolicyName(report) : getDisplayNameForParticipant(report.managerID);

return Localize.translateLocal(report.hasOutstandingIOU ? 'iou.payerOwesAmount' : 'iou.payerPaidAmount', {payer: payerName, amount: formattedAmount});
}
Expand Down Expand Up @@ -1034,11 +1035,11 @@ function getReportName(report) {
}

// Not a room or PolicyExpenseChat, generate title from participants
const participants = (report && report.participants) || [];
const participantsWithoutCurrentUser = _.without(participants, sessionEmail);
const participants = (report && report.participantAccountIDs) || [];
const participantsWithoutCurrentUser = _.without(participants, sessionAccountID);
const isMultipleParticipantReport = participantsWithoutCurrentUser.length > 1;

return _.map(participantsWithoutCurrentUser, (login) => getDisplayNameForParticipant(login, isMultipleParticipantReport)).join(', ');
return _.map(participantsWithoutCurrentUser, (accountID) => getDisplayNameForParticipant(accountID, isMultipleParticipantReport)).join(', ');
}

/**
Expand Down
11 changes: 11 additions & 0 deletions src/libs/UserUtils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,16 @@ function getSmallSizeAvatar(avatarURL, accountID) {
return `${source.substring(0, lastPeriodIndex)}_128${source.substring(lastPeriodIndex)}`;
}

/**
* Generate a random accountID.
* Uses the same approach of 'generateReportID'.
*
* @returns {String}
*/
function generateAccountID() {
return (Math.floor(Math.random() * 2 ** 21) * 2 ** 32 + Math.floor(Math.random() * 2 ** 32)).toString();
}

export {
hashText,
hasLoginListError,
Expand All @@ -213,4 +223,5 @@ export {
getAvatarUrl,
getSmallSizeAvatar,
getFullSizeAvatar,
generateAccountID,
};
Loading

0 comments on commit fde3aff

Please sign in to comment.