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

fix: mentions are not shown in issued card page, add/remove user page #55699

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
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
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,8 @@ import type {WithCurrentUserPersonalDetailsProps} from '@components/withCurrentU
import useStyleUtils from '@hooks/useStyleUtils';
import useThemeStyles from '@hooks/useThemeStyles';
import {formatPhoneNumber} from '@libs/LocalePhoneNumber';
import {areEmailsFromSamePrivateDomain} from '@libs/LoginUtils';
import Navigation from '@libs/Navigation/Navigation';
import {getAccountIDsByLogins, getDisplayNameOrDefault} from '@libs/PersonalDetailsUtils';
import {getAccountIDsByLogins, getDisplayNameOrDefault, getShortMentionIfFound} from '@libs/PersonalDetailsUtils';
import {isArchivedNonExpenseReport} from '@libs/ReportUtils';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
Expand All @@ -41,33 +40,20 @@ function MentionUserRenderer({style, tnode, TDefaultRenderer, currentUserPersona

const tnodeClone = cloneDeep(tnode);

const getShortMentionIfFound = (displayText: string, userAccountID: string, userLogin = '') => {
// If the userAccountID does not exist, this is an email-based mention so the displayText must be an email.
// If the userAccountID exists but userLogin is different from displayText, this means the displayText is either user display name, Hidden, or phone number, in which case we should return it as is.
if (userAccountID && userLogin !== displayText) {
return displayText;
}

// If the emails are not in the same private domain, we also return the displayText
if (!areEmailsFromSamePrivateDomain(displayText, currentUserPersonalDetails.login ?? '')) {
return displayText;
}

// Otherwise, the emails must be of the same private domain, so we should remove the domain part
return displayText.split('@').at(0);
};

if (!isEmpty(htmlAttribAccountID) && personalDetails?.[htmlAttribAccountID]) {
const user = personalDetails[htmlAttribAccountID];
accountID = parseInt(htmlAttribAccountID, 10);
mentionDisplayText = formatPhoneNumber(user?.login ?? '') || getDisplayNameOrDefault(user);
mentionDisplayText = getShortMentionIfFound(mentionDisplayText, htmlAttributeAccountID, user?.login ?? '') ?? '';
mentionDisplayText = getShortMentionIfFound(mentionDisplayText, htmlAttributeAccountID, currentUserPersonalDetails, user?.login ?? '') ?? '';
navigationRoute = ROUTES.PROFILE.getRoute(accountID, Navigation.getReportRHPActiveRoute());
} else if ('data' in tnodeClone && !isEmptyObject(tnodeClone.data)) {
// We need to remove the LTR unicode and leading @ from data as it is not part of the login
mentionDisplayText = tnodeClone.data.replace(CONST.UNICODE.LTR, '').slice(1);
// We need to replace tnode.data here because we will pass it to TNodeChildrenRenderer below
asMutable(tnodeClone).data = tnodeClone.data.replace(mentionDisplayText, Str.removeSMSDomain(getShortMentionIfFound(mentionDisplayText, htmlAttributeAccountID) ?? ''));
asMutable(tnodeClone).data = tnodeClone.data.replace(
mentionDisplayText,
Str.removeSMSDomain(getShortMentionIfFound(mentionDisplayText, htmlAttributeAccountID, currentUserPersonalDetails) ?? ''),
);

accountID = getAccountIDsByLogins([mentionDisplayText])?.at(0) ?? -1;
navigationRoute = ROUTES.PROFILE.getRoute(accountID, Navigation.getReportRHPActiveRoute(), mentionDisplayText);
Expand Down
38 changes: 28 additions & 10 deletions src/libs/PersonalDetailsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ import type {OnyxInputOrEntry, PersonalDetails, PersonalDetailsList, PrivatePers
import type {Address} from '@src/types/onyx/PrivatePersonalDetails';
import type {OnyxData} from '@src/types/onyx/Request';
import {isEmptyObject} from '@src/types/utils/EmptyObject';
import * as LocalePhoneNumber from './LocalePhoneNumber';
import * as Localize from './Localize';
import * as UserUtils from './UserUtils';
import {formatPhoneNumber} from './LocalePhoneNumber';
import {translateLocal} from './Localize';
import {areEmailsFromSamePrivateDomain} from './LoginUtils';
import {generateAccountID} from './UserUtils';

type FirstAndLastName = {
firstName: string;
Expand Down Expand Up @@ -42,8 +43,8 @@ Onyx.connect({
if (!value) {
return;
}
hiddenTranslation = Localize.translateLocal('common.hidden');
youTranslation = Localize.translateLocal('common.you').toLowerCase();
hiddenTranslation = translateLocal('common.hidden');
youTranslation = translateLocal('common.you').toLowerCase();
},
});

Expand Down Expand Up @@ -118,7 +119,7 @@ function getPersonalDetailsByIDs({
if (shouldChangeUserDisplayName && currentUserAccountID === detail.accountID) {
return {
...detail,
displayName: Localize.translateLocal('common.you'),
displayName: translateLocal('common.you'),
};
}

Expand All @@ -143,7 +144,7 @@ function getAccountIDsByLogins(logins: string[]): number[] {
const currentDetail = personalDetails.find((detail) => detail?.login === login?.toLowerCase());
if (!currentDetail) {
// generate an account ID because in this case the detail is probably new, so we don't have a real accountID yet
foundAccountIDs.push(UserUtils.generateAccountID(login));
foundAccountIDs.push(generateAccountID(login));
} else {
foundAccountIDs.push(Number(currentDetail.accountID));
}
Expand Down Expand Up @@ -197,7 +198,7 @@ function getPersonalDetailsOnyxDataForOptimisticUsers(newLogins: string[], newAc
personalDetailsNew[accountID] = {
login,
accountID,
displayName: LocalePhoneNumber.formatPhoneNumber(login),
displayName: formatPhoneNumber(login),
isOptimisticPersonalDetail: true,
};

Expand Down Expand Up @@ -294,7 +295,7 @@ function getFormattedAddress(privatePersonalDetails: OnyxEntry<PrivatePersonalDe
*/
function getEffectiveDisplayName(personalDetail?: PersonalDetails): string | undefined {
if (personalDetail) {
return LocalePhoneNumber.formatPhoneNumber(personalDetail?.login ?? '') || personalDetail.displayName;
return formatPhoneNumber(personalDetail?.login ?? '') || personalDetail.displayName;
}

return undefined;
Expand All @@ -306,7 +307,7 @@ function getEffectiveDisplayName(personalDetail?: PersonalDetails): string | und
function createDisplayName(login: string, passedPersonalDetails: Pick<PersonalDetails, 'firstName' | 'lastName'> | OnyxInputOrEntry<PersonalDetails>): string {
// If we have a number like [email protected] then let's remove @expensify.sms and format it
// so that the option looks cleaner in our UI.
const userLogin = LocalePhoneNumber.formatPhoneNumber(login);
const userLogin = formatPhoneNumber(login);

if (!passedPersonalDetails) {
return userLogin;
Expand Down Expand Up @@ -370,6 +371,22 @@ function getUserNameByEmail(email: string, nameToDisplay: 'firstName' | 'display
return email;
}

const getShortMentionIfFound = (displayText: string, userAccountID: string, currentUserPersonalDetails: OnyxEntry<PersonalDetails>, userLogin = '') => {
// If the userAccountID does not exist, this is an email-based mention so the displayText must be an email.
// If the userAccountID exists but userLogin is different from displayText, this means the displayText is either user display name, Hidden, or phone number, in which case we should return it as is.
if (userAccountID && userLogin !== displayText) {
return displayText;
}

// If the emails are not in the same private domain, we also return the displayText
if (!areEmailsFromSamePrivateDomain(displayText, currentUserPersonalDetails?.login ?? '')) {
return displayText;
}

// Otherwise, the emails must be of the same private domain, so we should remove the domain part
return displayText.split('@').at(0);
};

export {
isPersonalDetailsEmpty,
getDisplayNameOrDefault,
Expand All @@ -388,4 +405,5 @@ export {
getNewAccountIDsAndLogins,
getPersonalDetailsLength,
getUserNameByEmail,
getShortMentionIfFound,
};
11 changes: 2 additions & 9 deletions src/libs/ReportActionsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import CONST from '@src/CONST';
import type {TranslationPaths} from '@src/languages/types';
import ONYXKEYS from '@src/ONYXKEYS';
import ROUTES from '@src/ROUTES';
import type {Locale, OnyxInputOrEntry, PersonalDetailsList, PrivatePersonalDetails} from '@src/types/onyx';
import type {Locale, OnyxInputOrEntry, PrivatePersonalDetails} from '@src/types/onyx';
import type {JoinWorkspaceResolution, OriginalMessageChangeLog, OriginalMessageExportIntegration} from '@src/types/onyx/OriginalMessage';
import type Report from '@src/types/onyx/Report';
import type ReportAction from '@src/types/onyx/ReportAction';
Expand Down Expand Up @@ -1834,13 +1834,11 @@ function getCardIssuedMessage({
shouldRenderHTML = false,
policyID = '-1',
shouldDisplayLinkToCard = false,
personalDetails,
}: {
reportAction: OnyxEntry<ReportAction>;
shouldRenderHTML?: boolean;
policyID?: string;
shouldDisplayLinkToCard?: boolean;
personalDetails?: Partial<PersonalDetailsList>;
}) {
const cardIssuedActionOriginalMessage = isActionOfType(
reportAction,
Expand All @@ -1854,13 +1852,8 @@ function getCardIssuedMessage({

const assigneeAccountID = cardIssuedActionOriginalMessage?.assigneeAccountID ?? CONST.DEFAULT_NUMBER_ID;
const cardID = cardIssuedActionOriginalMessage?.cardID ?? CONST.DEFAULT_NUMBER_ID;
const assigneeDetails = getPersonalDetailsByIDs({
accountIDs: [assigneeAccountID],
currentUserAccountID: currentUserAccountID ?? CONST.DEFAULT_NUMBER_ID,
personalDetailsParam: personalDetails,
}).at(0);
const isPolicyAdmin = isPolicyAdminPolicyUtils(getPolicy(policyID));
const assignee = shouldRenderHTML ? `<mention-user accountID="${assigneeAccountID}"/>` : assigneeDetails?.firstName ?? assigneeDetails?.login ?? '';
const assignee = shouldRenderHTML ? `<mention-user accountID="${assigneeAccountID}"/>` : Parser.htmlToText(`<mention-user accountID="${assigneeAccountID}"/>`);
const navigateRoute = isPolicyAdmin ? ROUTES.EXPENSIFY_CARD_DETAILS.getRoute(policyID, String(cardID)) : ROUTES.SETTINGS_DOMAINCARD_DETAIL.getRoute(String(cardID));
const expensifyCardLink =
shouldRenderHTML && shouldDisplayLinkToCard
Expand Down
23 changes: 18 additions & 5 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,15 @@ import Navigation from './Navigation/Navigation';
import {rand64} from './NumberUtils';
import Parser from './Parser';
import Permissions from './Permissions';
import {getAccountIDsByLogins, getDisplayNameOrDefault, getEffectiveDisplayName, getLoginsByAccountIDs, getPersonalDetailByEmail, getPersonalDetailsByIDs} from './PersonalDetailsUtils';
import {
getAccountIDsByLogins,
getDisplayNameOrDefault,
getEffectiveDisplayName,
getLoginsByAccountIDs,
getPersonalDetailByEmail,
getPersonalDetailsByIDs,
getShortMentionIfFound,
} from './PersonalDetailsUtils';
import {addSMSDomainIfPhoneNumber} from './PhoneNumber';
import {
arePaymentsEnabled,
Expand Down Expand Up @@ -4209,7 +4217,7 @@ function parseReportActionHtmlToText(reportAction: OnyxEntry<ReportAction>, repo
return text ?? '';
}

const mentionReportRegex = /<mention-report reportID="(\d+)" *\/>/gi;
const mentionReportRegex = /<mention-report reportID="?(\d+)"?(?: *\/>|><\/mention-report>)/gi;
const matches = html.matchAll(mentionReportRegex);

const reportIDToName: Record<string, string> = {};
Expand All @@ -4220,11 +4228,16 @@ function parseReportActionHtmlToText(reportAction: OnyxEntry<ReportAction>, repo
}
}

const mentionUserRegex = /<mention-user accountID="(\d+)" *\/>/gi;
const mentionUserRegex = /(?:<mention-user accountID="?(\d+)"?(?: *\/>|><\/mention-user>))/gi;
const accountIDToName: Record<string, string> = {};
const accountIDs = Array.from(html.matchAll(mentionUserRegex), (mention) => Number(mention[1]));
const logins = getLoginsByAccountIDs(accountIDs);
accountIDs.forEach((id, index) => (accountIDToName[id] = logins.at(index) ?? ''));
accountIDs.forEach((id, index) => {
const login = logins.at(index);
const user = allPersonalDetails?.[id];
const displayName = formatPhoneNumber(login ?? '') || getDisplayNameOrDefault(user);
accountIDToName[id] = getShortMentionIfFound(displayName, id.toString(), currentUserPersonalDetails, login) ?? '';
});

const textMessage = Str.removeSMSDomain(Parser.htmlToText(html, {reportIDToName, accountIDToName}));
parsedReportActionMessageCache[key] = textMessage;
Expand Down Expand Up @@ -4470,7 +4483,7 @@ function getReportNameInternal({
}

if (isCardIssuedAction(parentReportAction)) {
return getCardIssuedMessage({reportAction: parentReportAction, personalDetails});
return getCardIssuedMessage({reportAction: parentReportAction});
}
return reportActionMessage;
}
Expand Down