[HOLD for payment 2024-02-07] MEDIUM: [$500] Drop domain name in mentions if chat members are on the same domain #32534

NikkiWines · 2023-12-05T23:56:29Z

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Version Number: v1.4.8-1
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): all customers on private domains
Logs: N/A
Expensify/Expensify Issue URL: N/A
Issue reported by: N/A
Slack conversation: https://expensify.slack.com/archives/C066HJM2CAZ/p1701743829149999

Action Performed:

Create a room with 4 members: 2 on the same private domain, 1 on another private domain, and 1 on a public domain
As each member, send a message mentioning each other member

Expected Result:

If the user looking at the chat is on the same domain as the person being mentioned, they should not see the domain (e.g. @user@expensify.com) in the mention. They should see a shortened mention containing only the email prefix (e.g. @user)

If the user looking at the chat is not on the same domain as the person being mentioned, or if the user being mentioned is on a public domain, then they should see the full mention (e.g. @user@gmail.com)

E.g. viewing as nikki@meep.com, which has a common domain

E.g. viewing as nikki@notification.com, no common domain

Actual Result:

Regardless of whether or not users share a domain, the mention is always the full mention (e.g. @user@expensify.com)

E.g. viewing as nikki@meep.com, which has a common domain

E.g. viewing as nikki@notification.com, no common domain

Workaround:

N/A

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

N/A

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~0173ab1d6ef6e3a31a
Upwork Job ID: 1732819861042143232
Last Price Increase: 2023-12-29

The text was updated successfully, but these errors were encountered:

melvin-bot · 2023-12-05T23:56:34Z

Triggered auto assignment to @stephanieelliott (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

melvin-bot · 2023-12-05T23:56:36Z

Bug0 Triage Checklist (Main S/O)

This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
This bug is not a duplicate report (check E/App issues and #expensify-bugs)
- If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
This bug is reproducible using the reproduction steps in the OP. S/O
- If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
- If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
This issue is filled out as thoroughly and clearly as possible
- Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

puneetlath · 2023-12-06T16:55:32Z

A few things to add here:

This should only be the case for private domains (e.g. expensify.com). For public domains (e.g. gmail.com) should always use the full email. We have a list of public domains in expensify-common here. So if my email address is puneet@gmail.com and I'm viewing a mention of nikki@gmail.com it should show as @nikki@gmail.com not @nikki
The app supports both email based mentions - <mention-user>@person@gmail.com</mention-user> - and accountID-based mentions - <mention-user accountID=1234></mention-here>. We should make sure that the solution works for both of them. So I would think that maybe it's as follows.

For email-based mentions:

Look at the email address in the mention
If it's on the same private domain as mine, render as @person. Otherwise render as @person@domain.com

For accountID-based mentions:

Look at the personalDetails object for the accountID
If we don't have a personalDetail for that accountID, render as @Hidden
If we have a personalDetail, but we don't have a display name or login in there, render as @Hidden
If we have a personalDetail with display name, but no login, render as @Display Name
if we have a personalDetail with login, render the same way as email-based mentions

Does that sound good?

melvin-bot · 2023-12-07T17:50:26Z

Job added to Upwork: https://www.upwork.com/jobs/~0173ab1d6ef6e3a31a

melvin-bot · 2023-12-07T17:50:31Z

Triggered auto assignment to Contributor-plus team member for initial proposal review - @sobitneupane (External)

tienifr · 2023-12-07T18:24:42Z

Proposal

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

Drop domain name in mentions if chat members are on the same domain

What is the root cause of that problem?

This is new feature

What changes do you think we should make in order to solve the problem?

We need to modify the MentionUserRenderer to accomodate the rules here, it's very clearly listed out so can just follow that.

Create a common method getMentionDisplayText that accepts account id/email (and original domain/display name), and run through the rules here and return the final display text.
In here, use the getMentionDisplayText to transform the displayNameOrLogin or props.tnode properly so it displays the mention text according to the rules

More details on implementation, there could be minor variations/fixes required but the below should be the main changes:

Define a areEmailsFromSamePrivateDomain method in LoginUtils to validate if 2 emails belong to the same private domain. This can be reused for various use cases later on.

function areEmailsFromSamePrivateDomain(email1: string, email2: string): boolean {
    if (isEmailPublicDomain(email1) || isEmailPublicDomain(email2)) {
        return false;
    }
    const emailDomain1 = Str.extractEmailDomain(email1).toLowerCase();
    const emailDomain2 = Str.extractEmailDomain(email2).toLowerCase();
    return emailDomain1 === emailDomain2;
};

getMentionDisplayText method to get the actual text to display, based on these rules

const getMentionDisplayText = (displayText, accountId, userLogin = '') => {
    if (accountId && userLogin !== displayText) {
        return displayText;
    }

    if (!LoginUtils.areEmailsFromSamePrivateDomain(displayText, props.currentUserPersonalDetails.login)) {
        return displayText;
    }

    return displayText.split('@')[0];
}

The accountId && userLogin !== displayName is to check, if the accountId does not exist, this is email-based mention so the displayName must be an email. If the accountId exists but userLogin is different from displayName, this means the displayName is either user display name, Hidden, or phone number, in which case we should return it as is.

If the emails are not in the same private domain, we also return the displayText as is.

Otherwise, the emails must be of the same private domain, so we should remove the domain part and return the user "handle".

Then use it here for the accountId-based mention

displayNameOrLogin = getMentionDisplayText(displayNameOrLogin, htmlAttribAccountID, lodashGet(user, 'login', ''));

And here for the case of email-based mention

const tnode = props.tnode; // outside
...
tnode.data = tnode.data.replace(displayNameOrLogin, getMentionDisplayText(displayNameOrLogin, htmlAttribAccountID));

// optional
displayNameOrLogin = getMentionDisplayText(displayNameOrLogin, htmlAttribAccountID);

We need to use tnode.data.replace because we want to preserve the LTR character and the leading @ of the tnode.data (see here)

Then here, change props.tnode to tnode.

What alternative solutions did you explore? (Optional)

The above is in the mention renderer, if we want it to display the same way in LHN as well, we need to make additional changes and potentially some back-end changes as well.

b4s36t4 · 2023-12-07T18:36:41Z

Proposal

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

MEDIUM: Drop domain name in mentions if chat members are on the same domain

What is the root cause of that problem?

NA - Feature Request

What changes do you think we should make in order to solve the problem?

We just need to render through the rules of things first flow.

Checking if the rendering is mention is a email or not.

// Checking if the rendering mention is login i.e email
    const isDisplyingLogin = displayNameOrLogin === lodashGet(user, 'login', '');
    // Check if the email is valid, if so extract the email
    const mentionUserEmailDomain = isDisplyingLogin && Str.isValidEmail(displayNameOrLogin) && Str.extractEmailDomain(displayNameOrLogin);

Extracting domains form the emails

    const currentUserEmailDomian = Str.extractEmailDomain(props.currentUserPersonalDetails.login);

If the the domain is from public domains

    const isPublicDomain = StrConst.PUBLIC_DOMAINS.includes(mentionUserEmailDomain.toLowerCase());

if the both users (mentioned and viewing) are having same domains

 const isSameOriginEmail = mentionUserEmailDomain === currentUserEmailDomian;

Update the rendering value.

if (isSameOriginEmail && isDisplyingLogin && !isOurMention && !isPublicDomain) {
        displayNameOrLogin = Str.replaceAll(`@${currentUserEmailDomian}`, '');
    }

This fills all the conditions.

Conditions based on rendering based upon on accountID param, because we're validating the value based on displayNameOrLogin which plays vital role on things we render on.
We're checking if we're rendering login i.e Not the display name and we're also validating the email.
We're extracting the email i.e Str.extractEmailDomain and checking the domains.

What alternative solutions did you explore? (Optional)

NA

gijoe0295 · 2023-12-07T18:38:22Z

Proposal

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

Drop domain name in mentions if chat members are on the same domain

What is the root cause of that problem?

Feature request

What changes do you think we should make in order to solve the problem?

In MentionUserRenderer:

Check if displayNameOrLogin is email or display name
Extract domain of the mentioned user and current user
Check if the current user's email domain is private
Check if they are equal
If so, extract the email part only to display

We already have built-in functions for above tasks.

let displayMention = displayNameOrLogin;
if (displayNameOrLogin && Str.isValidEmail(displayNameOrLogin) && !LoginUtils.isEmailPublicDomain(props.currentUserPersonalDetails.login)) {
    currentUserDomain = Str.extractEmailDomain(props.currentUserPersonalDetails.login);
    if (currentUserDomain === Str.extractEmailDomain(displayNameOrLogin)) {
        displayMention = Str.extractEmail(displayNameOrLogin);
    }
}

And use it to render mention.

What alternative solutions did you explore? (Optional)

NA

quinthar · 2023-12-08T00:51:13Z

FYI, before doing anything on this, please read this entire @mentions design doc to make sure you understand the full context. Also, be sure to call out anything inaccurate. If we're going to do something different than the doc says, or do it in a different order, let's update the doc FIRST to reflect the real plan -- and THEN do the coding. Thanks!

b4s36t4 · 2023-12-08T07:05:37Z

@quinthar design doc is protected, can you please share the link with read permission? thanks

quinthar · 2023-12-08T07:06:47Z

Ah, what's your email address so I can share it with you?

gijoe0295 · 2023-12-08T07:33:39Z

@quinthar Can you share gijoe0295@gmail.com? Thanks!

b4s36t4 · 2023-12-08T07:49:49Z

@quinthar it's maheshvagicherla99438@gmail.com

b4s36t4 · 2023-12-08T07:51:16Z

Also I have requested the read or (maybe edit) from docs itself.

quinthar · 2023-12-08T08:00:04Z

Shared!

b4s36t4 · 2023-12-08T08:03:25Z

Got it, thanks :)

quinthar · 2023-12-08T08:14:14Z

Hm, reading the design doc, and I'm not actually sure it ever finished the design for "simplified handles". @NikkiWines are you sure this is ready for a contributor to pick up? I think we should first finish the design doc for this to make sure we all agree on how it's going to work, before taking it out for proposals.

sobitneupane · 2024-01-22T10:18:13Z

I am reviewing the PR now.

stephanieelliott · 2024-01-23T03:23:23Z

C+ review has been completed, PR is now just waiting on a few minor change requests to be addressed by @tienifr before merge.

stephanieelliott · 2024-01-24T00:03:11Z

PR is updated, just awaiting a final review

stephanieelliott · 2024-01-25T20:06:28Z

PR is merged and undergoing QA

stephanieelliott · 2024-01-30T04:41:13Z

This PR is on staging 👏

melvin-bot · 2024-01-31T02:22:35Z

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot · 2024-01-31T02:22:38Z

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.33-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

fix: Drop domain name in mentions if chat members are on the same domain #34013

If no regressions arise, payment will be issued on 2024-02-07. 🎊

For reference, here are some details about the assignees on this issue:

@sobitneupane requires payment through NewDot Manual Requests
@tienifr requires payment (Needs manual offer from BZ)

melvin-bot · 2024-01-31T02:22:39Z

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[@sobitneupane] The PR that introduced the bug has been identified. Link to the PR:
[@sobitneupane] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
[@sobitneupane] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
[@sobitneupane] Determine if we should create a regression test for this bug.
[@sobitneupane] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
[@stephanieelliott] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

stephanieelliott · 2024-02-07T07:26:26Z

Hey @sobitneupane can you complete the BZ checklist when you get a chance?

melvin-bot · 2024-02-07T15:08:25Z

Payment Summary

Upwork Job

Reviewer: @sobitneupane owed $500 via NewDot
ROLE: @tienifr owed $500 via Upwork https://www.upwork.com/jobs/~0103a481ee7d9180a4

BugZero Checklist (@stephanieelliott)

I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1732819861042143232/hired)
I have paid out the Upwork contracts or cancelled the ones that are incorrect
I have verified the payment summary above is correct

stephanieelliott · 2024-02-08T21:12:06Z

Bump @sobitneupane, can you complete the BZ checklist?

sobitneupane · 2024-02-09T05:23:12Z

It's a feature request not a bug. So, I don't think the checklist applies here.

sobitneupane · 2024-02-09T05:29:12Z

Regression test proposal

Create a room with 4 members: 2 on the same private domain, 1 on another private domain, and 1 on a public domain
As each member, send a message mentioning each other member
If the user looking at the chat is on the same domain as the person being mentioned, they should not see the domain (e.g. @user@expensify.com) in the mention. They should see a shortened mention containing only the email prefix (e.g. @user)
If the user looking at the chat is not on the same domain as the person being mentioned, or if the user being mentioned is on a public domain, then they should see the full mention (e.g. @user@gmail.com)

Do we agree 👍 or 👎

stephanieelliott · 2024-02-09T19:42:02Z

Testrail issue created: https://github.com/Expensify/Expensify/issues/368461

@tienifr offer extended in Upwork, please accept when you get a chance!

stephanieelliott · 2024-02-13T07:14:24Z

All paid, thanks!

JmillsExpensify · 2024-03-12T03:02:58Z

$500 approved for @sobitneupane based on this summary.

NikkiWines added Daily KSv2 Improvement Item broken or needs improvement. Bug Something is broken. Auto assigns a BugZero manager. labels Dec 5, 2023

melvin-bot bot assigned stephanieelliott Dec 5, 2023

NikkiWines self-assigned this Dec 6, 2023

puneetlath mentioned this issue Dec 6, 2023

HIGH - [$2000] Make mentions auto-suggestions work with phone number accounts #18729

Closed

quinthar added this to [#whatsnext] #vip-vsb Dec 7, 2023

quinthar changed the title ~~[$500] HIGH: Drop domain name in mentions if chat members are on the same domain~~ [$500] MEDIUM: Drop domain name in mentions if chat members are on the same domain Dec 7, 2023

quinthar moved this to MEDIUM in [#whatsnext] #vip-vsb Dec 7, 2023

NikkiWines added the External Added to denote the issue can be worked on by a contributor label Dec 7, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 7, 2023

melvin-bot bot assigned sobitneupane Dec 7, 2023

quinthar changed the title ~~[$500] MEDIUM: Drop domain name in mentions if chat members are on the same domain~~ MEDIUM: [$500] Drop domain name in mentions if chat members are on the same domain Dec 8, 2023

melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jan 31, 2024

melvin-bot bot changed the title ~~MEDIUM: [$500] Drop domain name in mentions if chat members are on the same domain~~ [HOLD for payment 2024-02-07] MEDIUM: [$500] Drop domain name in mentions if chat members are on the same domain Jan 31, 2024

melvin-bot bot removed the Reviewing Has a PR in review label Jan 31, 2024

melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels Feb 6, 2024

melvin-bot bot removed the Overdue label Feb 7, 2024

melvin-bot bot added the Overdue label Feb 12, 2024

stephanieelliott closed this as completed Feb 13, 2024

github-project-automation bot moved this from MEDIUM to CRITICAL in [#whatsnext] #vip-vsb Feb 13, 2024

melvin-bot bot removed the Overdue label Feb 13, 2024

puneetlath mentioned this issue Mar 12, 2024

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

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2024-02-07] MEDIUM: [$500] Drop domain name in mentions if chat members are on the same domain #32534

[HOLD for payment 2024-02-07] MEDIUM: [$500] Drop domain name in mentions if chat members are on the same domain #32534

NikkiWines commented Dec 5, 2023 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Dec 5, 2023

melvin-bot bot commented Dec 5, 2023

puneetlath commented Dec 6, 2023

melvin-bot bot commented Dec 7, 2023

melvin-bot bot commented Dec 7, 2023

tienifr commented Dec 7, 2023 •

edited

Loading

b4s36t4 commented Dec 7, 2023 •

edited

Loading

gijoe0295 commented Dec 7, 2023 •

edited

Loading

quinthar commented Dec 8, 2023

b4s36t4 commented Dec 8, 2023

quinthar commented Dec 8, 2023

gijoe0295 commented Dec 8, 2023

b4s36t4 commented Dec 8, 2023

b4s36t4 commented Dec 8, 2023

quinthar commented Dec 8, 2023

b4s36t4 commented Dec 8, 2023

quinthar commented Dec 8, 2023

sobitneupane commented Jan 22, 2024

stephanieelliott commented Jan 23, 2024

stephanieelliott commented Jan 24, 2024

stephanieelliott commented Jan 25, 2024

stephanieelliott commented Jan 30, 2024

melvin-bot bot commented Jan 31, 2024

melvin-bot bot commented Jan 31, 2024

melvin-bot bot commented Jan 31, 2024 •

edited by stephanieelliott

Loading

stephanieelliott commented Feb 7, 2024

melvin-bot bot commented Feb 7, 2024 •

edited by stephanieelliott

Loading

stephanieelliott commented Feb 8, 2024

sobitneupane commented Feb 9, 2024

sobitneupane commented Feb 9, 2024

stephanieelliott commented Feb 9, 2024

stephanieelliott commented Feb 13, 2024

JmillsExpensify commented Mar 12, 2024

[HOLD for payment 2024-02-07] MEDIUM: [$500] Drop domain name in mentions if chat members are on the same domain #32534

[HOLD for payment 2024-02-07] MEDIUM: [$500] Drop domain name in mentions if chat members are on the same domain #32534

Comments

NikkiWines commented Dec 5, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Dec 5, 2023

melvin-bot bot commented Dec 5, 2023

Bug0 Triage Checklist (Main S/O)

puneetlath commented Dec 6, 2023

melvin-bot bot commented Dec 7, 2023

melvin-bot bot commented Dec 7, 2023

tienifr commented Dec 7, 2023 • edited Loading

Proposal

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

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

b4s36t4 commented Dec 7, 2023 • edited Loading

Proposal

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

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

gijoe0295 commented Dec 7, 2023 • edited Loading

Proposal

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

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

quinthar commented Dec 8, 2023

b4s36t4 commented Dec 8, 2023

quinthar commented Dec 8, 2023

gijoe0295 commented Dec 8, 2023

b4s36t4 commented Dec 8, 2023

b4s36t4 commented Dec 8, 2023

quinthar commented Dec 8, 2023

b4s36t4 commented Dec 8, 2023

quinthar commented Dec 8, 2023

sobitneupane commented Jan 22, 2024

stephanieelliott commented Jan 23, 2024

stephanieelliott commented Jan 24, 2024

stephanieelliott commented Jan 25, 2024

stephanieelliott commented Jan 30, 2024

melvin-bot bot commented Jan 31, 2024

melvin-bot bot commented Jan 31, 2024

melvin-bot bot commented Jan 31, 2024 • edited by stephanieelliott Loading

stephanieelliott commented Feb 7, 2024

melvin-bot bot commented Feb 7, 2024 • edited by stephanieelliott Loading

Payment Summary

BugZero Checklist (@stephanieelliott)

stephanieelliott commented Feb 8, 2024

sobitneupane commented Feb 9, 2024

sobitneupane commented Feb 9, 2024

stephanieelliott commented Feb 9, 2024

stephanieelliott commented Feb 13, 2024

JmillsExpensify commented Mar 12, 2024

NikkiWines commented Dec 5, 2023 •

edited by melvin-bot bot

Loading

tienifr commented Dec 7, 2023 •

edited

Loading

b4s36t4 commented Dec 7, 2023 •

edited

Loading

gijoe0295 commented Dec 7, 2023 •

edited

Loading

melvin-bot bot commented Jan 31, 2024 •

edited by stephanieelliott

Loading

melvin-bot bot commented Feb 7, 2024 •

edited by stephanieelliott

Loading