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

[$500] LHN shows tag as @email@domain.com instead of @person #36284

Closed
1 of 6 tasks
m-natarajan opened this issue Feb 9, 2024 · 13 comments
Closed
1 of 6 tasks

[$500] LHN shows tag as @email@domain.com instead of @person #36284

m-natarajan opened this issue Feb 9, 2024 · 13 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@m-natarajan
Copy link

m-natarajan commented Feb 9, 2024

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: 1.4.39-0
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @coleaeason
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1707498095462179

Action Performed:

  1. Go to any public room
  2. Mention any user in the message

Expected Result:

LHN should show @person

Actual Result:

Shows as @email@domain.com

Workaround:

unknown

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence
Screenshot 2024-02-09 at 9 01 38 AM

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~017c3d81a0688971b8
  • Upwork Job ID: 1756047410853744640
  • Last Price Increase: 2024-02-09
@m-natarajan m-natarajan added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Feb 9, 2024
Copy link

melvin-bot bot commented Feb 9, 2024

Job added to Upwork: https://www.upwork.com/jobs/~017c3d81a0688971b8

@melvin-bot melvin-bot bot changed the title LHN shows tag as @email@domain.com instead of @person [$500] LHN shows tag as @email@domain.com instead of @person Feb 9, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 9, 2024
Copy link

melvin-bot bot commented Feb 9, 2024

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

Copy link

melvin-bot bot commented Feb 9, 2024

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

@jeremy-croff
Copy link
Contributor

jeremy-croff commented Feb 9, 2024

Is this a bug?
I found client side support for the @person feature here: #26877
But no record of the api sending the account_id or the client setting the account_id to map to @person

@jeremy-croff
Copy link
Contributor

jeremy-croff commented Feb 9, 2024

Proposal

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

Show @person mentions in LHN

What is the root cause of that problem?

Seems we have no accountID on the html object we send and retrieve.

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

We can start sending the accountid of the mentioned user inside the node so that #26877 works as expected.

It requires us to pass down the account id from the suggestion to the ExpensiMark parser which converts it to the suggestion html.
We would need to update the parser to accept additional data like accountID as an html attribute.

What alternative solutions did you explore? (Optional)

@melvin-bot melvin-bot bot added the Overdue label Feb 12, 2024
@joekaufmanexpensify
Copy link
Contributor

Not overdue. Will look at this more today

@melvin-bot melvin-bot bot removed the Overdue label Feb 12, 2024
@suneox
Copy link
Contributor

suneox commented Feb 12, 2024

Proposal

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

LHN shows tag as @email@domain.com instead of @person

What is the root cause of that problem?

On LHN show the lastMessageText including email mentioned without a username

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

We will detect email mention with the pattern @<email> and replace by username from the list PersonalDetails

    let lastMessageText = lastMessageTextFromReport;

+   const mentionRegex = /[^@\s]+@[^@\s]+\.[^@\s]+/g;
+   const matchedEmail = lastMessageText.match(mentionRegex);
+   if (matchedEmail && matchedEmail.length > 0) {
+       matchedEmail.forEach((email) => {
+           const pDetail = UserUtils.getPersonalDetailByEmail(email);
+           if (pDetail?.displayName) {
+               lastMessageText = lastMessageText.replace(email, pDetail.displayName);
+           }
+       });
+   }

Or update at OptionsListUtils.getLastMessageTextForReport function

Add function getPersonalDetailByEmail to UserUtils

    function getPersonalDetailByEmail(email?: string)?: PersonalDetails | undefined | null {
        return Object.values(allPersonalDetails ?? {}).find((personalDetails) => personalDetails?.login === email);
    }

POC

Screen.Recording.2024-02-13.at.02.12.05.mov

What alternative solutions did you explore? (Optional)

@joekaufmanexpensify
Copy link
Contributor

Discussing

@joekaufmanexpensify
Copy link
Contributor

Confirmed in Slack that this should be fixed by https://github.com/Expensify/Expensify/issues/313934 , closing in favor of that issue!

@suneox
Copy link
Contributor

suneox commented Feb 15, 2024

Confirmed in Slack that this should be fixed by https://github.com/Expensify/Expensify/issues/313934 , closing in favor of that issue!

Hi, @joekaufmanexpensify Your reference issue is incorrect, could you please update it?

Discussing

and how can I access this thread? Thank you!

@joekaufmanexpensify
Copy link
Contributor

Hi, @joekaufmanexpensify Your reference issue is incorrect, could you please update it?

@suneox It's actually just an internal issue, that's why you're unable to view it. The Slack discussion I linked is internal as well. Our team is fixing this one internally. LMK if you have any other questions.

@suneox
Copy link
Contributor

suneox commented Feb 15, 2024

@joekaufmanexpensify thank you for your feedback, I've got it

LMK if you have any other questions.

Due to the issue will be resolved internal so I have a bit confuse if my proposal will be applied

@joekaufmanexpensify
Copy link
Contributor

Ah, got it. Nope we're refactoring this feature entirely, which will fix this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
None yet
Development

No branches or pull requests

5 participants