App: [HOLD for payment 2024-04-15] [$500] [HIGH] When mentioning a user the "actionable whisper" message shows the full display name

Just to clarify the behavior, all mentions, whether they are login-based or accountID-based should follow what is described here: https://github.com/Expensify/App/issues/32534#issuecomment-1843288493

@brandonhenry I don’t quite understand your question.

Though, re-reading your comment @puneetlath, i wonder if how it working right now is intended? If the user is on the same domain and has personalDetail with login, it renders like email-based mentions, which ultimately leads to rendering If it’s on the same private domain as mine, render as @person. Otherwise render as @person@domain.com.

I don’t think this is how it’s currently working. At least not in the case of the whisper message. It’s rendering with their display name, even though they are on the same domain and I have their email address locally.

@ahmedGaber93 We won’t be using getMentionDisplayText for prioritization.

I believe following code will do the job.

-    displayNameOrLogin = PersonalDetailsUtils.getDisplayNameOrDefault(user, LocalePhoneNumber.formatPhoneNumber(user?.login ?? ''));
+    displayNameOrLogin = LocalePhoneNumber.formatPhoneNumber(lodashGet(user, 'login', '')) || lodashGet(user, 'displayName', '') || translate('common.hidden');
+    displayNameOrLogin = Str.removeSMSDomain(getMentionDisplayText(displayNameOrLogin, htmlAttributeAccountID, lodashGet(user, 'login', '')));

Please let me know what you think.

Started discussion for clarification on the expected output.

https://expensify.slack.com/archives/C02NK2DQWUX/p1711000340700859

@brandonhenry Wouldn’t your proposal recreate https://github.com/Expensify/App/issues/36443 issue? If it doesn’t, can you please explain how it handles the issue. I might not have understand your proposal clearly.

Changes suggested by @puneetlath here was made by https://github.com/Expensify/App/pull/34013 PR. Everything was working fine back then. This should be a regression.

This is probably regression from https://github.com/Expensify/App/pull/36998 PR. The PR made change to prioritize displayName over login. The change was made to show display name instead of login while assigning task.

We should consider https://github.com/Expensify/App/issues/36443 and https://github.com/Expensify/App/issues/34052 issues as well while solving this issue.

cc: @ShridharGoel @nkdengineer @ahmedGaber93 @brandonhenry

Hi there,

Are there any additional solutions? Please inform me if you require assistance.

On Mon, Mar 11, 2024 at 10:42 PM kingsley Osumiri @.***> wrote:

Restated Problem: The issue arises when using a shortened mention in a chat application. Specifically, when mentioning a user who is not a member of the chat room, the full display name is shown in the actionable whisper message. This behavior deviates from the intended functionality, where the login should be displayed instead.

Root Cause: The root cause of the problem is that, in the context of a shortened mention and an actionable whisper message, the display name is being shown even when the user isn’t present in the chat room. In this scenario, the correct behavior should be to display the user’s login.

Proposed Changes: To address this issue, the following changes are proposed:

1. Include context of the current report using withOnyx.

2. Update the PersonalDetailsUtils.getDisplayNameOrDefault function to make it more flexible and easier to use.

interface GetDisplayNameOrDefaultProps { personalDetails?: Partial<PersonalDetails> | null; defaultValue?: string; shouldFallbackToHidden?: boolean; shouldAddCurrentUserPostfix?: boolean; defaultToLogin?: boolean; }

function getDisplayNameOrDefault({ personalDetails, defaultValue = ‘’, shouldFallbackToHidden = true, shouldAddCurrentUserPostfix = false, defaultToLogin = false, }: GetDisplayNameOrDefaultProps): string { if (defaultToLogin && personalDetails?.login) { return personalDetails.login; }

let displayName = personalDetails?.displayName ? personalDetails.displayName.replace(CONST.REGEX.MERGED_ACCOUNT_PREFIX, ‘’) : ‘’; if (shouldAddCurrentUserPostfix && !!displayName) { displayName = ${displayName} (${Localize.translateLocal('common.you').toLowerCase()}); }

const fallbackValue = shouldFallbackToHidden ? Localize.translateLocal(‘common.hidden’) : ‘’;

return displayName || defaultValue || fallbackValue; }

1. Example usage in MentionUserRenderer:

const displayNameOrLogin = PersonalDetailsUtils.getDisplayNameOrDefault({ personalDetails: user, defaultValue: LocalePhoneNumber.formatPhoneNumber(user?.login ?? ‘’), defaultToLogin: !ReportUtils.getReport(currentReportID)?.participantAccountIDs?.includes(Number(htmlAttributeAccountID)), });

These changes aim to provide a more flexible and context-aware solution for handling display names, especially in scenarios where the user is not a member of the chat room. The defaultToLogin option allows for a more explicit control over whether to display the login or the full display name. I think this answer gave you sufficient results.😅😅😅 Best, Kingsley

On Mon, Mar 11, 2024 at 10:27 PM Brandon Henry @.***> wrote:

Proposal Please re-state the problem that we are trying to solve in this issue.

The issue specifically occurs when using a shortened mention. When mentioning a user who is not a member of the chat room, the full display name is shown in the actionable whisper message, instead of the intended behavior of showing the login only. What is the root cause of that problem?

The root cause is that the display name is being shown for a shortened mention, even in an actionable whisper where the user isn’t present in the chat room. In this case, we should not show the full display name and instead rely on the user’s login. What changes do you think we should make in order to solve the problem?

To address this issue, I propose the following changes:

Include context of the current report using withOnyx 2.

Update PersonalDetailsUtils.getDisplayNameOrDefault

interface GetDisplayNameOrDefaultProps { personalDetails?: Partial<PersonalDetails> | null; defaultValue?: string; shouldFallbackToHidden?: boolean; shouldAddCurrentUserPostfix?: boolean; defaultToLogin?: boolean;} function getDisplayNameOrDefault({ personalDetails, defaultValue = ‘’, shouldFallbackToHidden = true, shouldAddCurrentUserPostfix = false, defaultToLogin = false,}: GetDisplayNameOrDefaultProps): string { if (defaultToLogin && personalDetails?.login) { return personalDetails.login; }

let displayName = personalDetails?.displayName ? personalDetails.displayName.replace(CONST.REGEX.MERGED_ACCOUNT_PREFIX, ‘’) : ‘’; if (shouldAddCurrentUserPostfix && !!displayName) { displayName = ${displayName} (${Localize.translateLocal('common.you').toLowerCase()}); }

const fallbackValue = shouldFallbackToHidden ? Localize.translateLocal(‘common.hidden’) : ‘’;

return displayName || defaultValue || fallbackValue;}

Note - This change is motivated by the observation that almost all uses of this function (except for this specific case) do not utilize the second or third arguments. By accepting an object with several properties, we can make the function more flexible and easier to use. Not necessary, but I think we should make the change

Define an interface GetDisplayNameOrDefaultProps that describes the properties of the object passed to the function. This interface includes the existing properties (personalDetails, defaultValue, shouldFallbackToHidden, shouldAddCurrentUserPostfix) and a new property defaultToLogin. 2.

We add a new condition at the beginning of the function to check if defaultToLogin is true and if the login property exists in the personalDetails object. If both conditions are met, the function immediately returns the login value. 3.

The rest of the function remains the same, handling the display name logic based on the other properties.

Example usage in MentionUserRenderer:

const displayNameOrLogin = PersonalDetailsUtils.getDisplayNameOrDefault({ personalDetails: user, defaultValue: LocalePhoneNumber.formatPhoneNumber(user?.login ?? ‘’), defaultToLogin: !ReportUtils.getReport(currentReportID)?.participantAccountIDs?.includes(Number(htmlAttributeAccountID)),});

— Reply to this email directly, view it on GitHub https://github.com/Expensify/App/issues/38079#issuecomment-1990559551, or unsubscribe https://github.com/notifications/unsubscribe-auth/A7BG3UDV64G54WCZV46I3RLYX2N6PAVCNFSM6AAAAABEQ3ZWQKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOJQGU2TSNJVGE . You are receiving this because you were mentioned.Message ID: @.***>

Restated Problem: The issue arises when using a shortened mention in a chat application. Specifically, when mentioning a user who is not a member of the chat room, the full display name is shown in the actionable whisper message. This behavior deviates from the intended functionality, where the login should be displayed instead.

Root Cause: The root cause of the problem is that, in the context of a shortened mention and an actionable whisper message, the display name is being shown even when the user isn’t present in the chat room. In this scenario, the correct behavior should be to display the user’s login.

Proposed Changes: To address this issue, the following changes are proposed:

1. Include context of the current report using withOnyx.

2. Update the PersonalDetailsUtils.getDisplayNameOrDefault function to make it more flexible and easier to use.

interface GetDisplayNameOrDefaultProps { personalDetails?: Partial<PersonalDetails> | null; defaultValue?: string; shouldFallbackToHidden?: boolean; shouldAddCurrentUserPostfix?: boolean; defaultToLogin?: boolean; }

function getDisplayNameOrDefault({ personalDetails, defaultValue = ‘’, shouldFallbackToHidden = true, shouldAddCurrentUserPostfix = false, defaultToLogin = false, }: GetDisplayNameOrDefaultProps): string { if (defaultToLogin && personalDetails?.login) { return personalDetails.login; }

let displayName = personalDetails?.displayName ? personalDetails.displayName.replace(CONST.REGEX.MERGED_ACCOUNT_PREFIX, ‘’) : ‘’; if (shouldAddCurrentUserPostfix && !!displayName) { displayName = ${displayName} (${Localize.translateLocal('common.you').toLowerCase()}); }

const fallbackValue = shouldFallbackToHidden ? Localize.translateLocal(‘common.hidden’) : ‘’;

return displayName || defaultValue || fallbackValue; }

1. Example usage in MentionUserRenderer:

const displayNameOrLogin = PersonalDetailsUtils.getDisplayNameOrDefault({ personalDetails: user, defaultValue: LocalePhoneNumber.formatPhoneNumber(user?.login ?? ‘’), defaultToLogin: !ReportUtils.getReport(currentReportID)?.participantAccountIDs?.includes(Number(htmlAttributeAccountID)), });

These changes aim to provide a more flexible and context-aware solution for handling display names, especially in scenarios where the user is not a member of the chat room. The defaultToLogin option allows for a more explicit control over whether to display the login or the full display name. I think this answer gave you sufficient results.😅😅😅 Best, Kingsley

On Mon, Mar 11, 2024 at 10:27 PM Brandon Henry @.***> wrote:

Proposal Please re-state the problem that we are trying to solve in this issue.

The issue specifically occurs when using a shortened mention. When mentioning a user who is not a member of the chat room, the full display name is shown in the actionable whisper message, instead of the intended behavior of showing the login only. What is the root cause of that problem?

The root cause is that the display name is being shown for a shortened mention, even in an actionable whisper where the user isn’t present in the chat room. In this case, we should not show the full display name and instead rely on the user’s login. What changes do you think we should make in order to solve the problem?

To address this issue, I propose the following changes:

Include context of the current report using withOnyx 2.

Update PersonalDetailsUtils.getDisplayNameOrDefault

interface GetDisplayNameOrDefaultProps { personalDetails?: Partial<PersonalDetails> | null; defaultValue?: string; shouldFallbackToHidden?: boolean; shouldAddCurrentUserPostfix?: boolean; defaultToLogin?: boolean;} function getDisplayNameOrDefault({ personalDetails, defaultValue = ‘’, shouldFallbackToHidden = true, shouldAddCurrentUserPostfix = false, defaultToLogin = false,}: GetDisplayNameOrDefaultProps): string { if (defaultToLogin && personalDetails?.login) { return personalDetails.login; }

let displayName = personalDetails?.displayName ? personalDetails.displayName.replace(CONST.REGEX.MERGED_ACCOUNT_PREFIX, ‘’) : ‘’; if (shouldAddCurrentUserPostfix && !!displayName) { displayName = ${displayName} (${Localize.translateLocal('common.you').toLowerCase()}); }

const fallbackValue = shouldFallbackToHidden ? Localize.translateLocal(‘common.hidden’) : ‘’;

return displayName || defaultValue || fallbackValue;}

Note - This change is motivated by the observation that almost all uses of this function (except for this specific case) do not utilize the second or third arguments. By accepting an object with several properties, we can make the function more flexible and easier to use. Not necessary, but I think we should make the change

Define an interface GetDisplayNameOrDefaultProps that describes the properties of the object passed to the function. This interface includes the existing properties (personalDetails, defaultValue, shouldFallbackToHidden, shouldAddCurrentUserPostfix) and a new property defaultToLogin. 2.

We add a new condition at the beginning of the function to check if defaultToLogin is true and if the login property exists in the personalDetails object. If both conditions are met, the function immediately returns the login value. 3.

The rest of the function remains the same, handling the display name logic based on the other properties.

Example usage in MentionUserRenderer:

const displayNameOrLogin = PersonalDetailsUtils.getDisplayNameOrDefault({ personalDetails: user, defaultValue: LocalePhoneNumber.formatPhoneNumber(user?.login ?? ‘’), defaultToLogin: !ReportUtils.getReport(currentReportID)?.participantAccountIDs?.includes(Number(htmlAttributeAccountID)),});

— Reply to this email directly, view it on GitHub https://github.com/Expensify/App/issues/38079#issuecomment-1990559551, or unsubscribe https://github.com/notifications/unsubscribe-auth/A7BG3UDV64G54WCZV46I3RLYX2N6PAVCNFSM6AAAAABEQ3ZWQKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOJQGU2TSNJVGE . You are receiving this because you were mentioned.Message ID: @.***>