[$500] Chat - Including any capital letter in mention displays wrong tooltip and mention with all capital letters is not displayed as mention

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Open the app
2. Open any report
3. Write '@' to trigger mentions
4. Select any one mention, make any letter of mention as capital and send
5. Hover on mention to observe that tooltip doesn't display the right display name and avatar
6. For same mention as step 4 or select different mention and capitalize all letters and send
7. Observe that now it is not displayed as mention

Expected Result:

Mention tooltip should display correct name and avatar irrespective of any capital letter in email and while making all letters capital, app should display the result as mention

Actual Result:

App does not display result as mention for all capital letters and if any one letter is made capital in mention, it does not display correct name and avatar in tooltip

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.57-5

Reproducible in staging?: Yes

Reproducible in production?: Yes

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

Notes/Photos/Videos: Any additional supporting documentation

capital.letters.affect.mention.results.mp4

Recording.2993.mp4

Expensify/Expensify Issue URL:

Issue reported by: @dhanashree-sawant

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1692338072767669

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01922c54c131acf6fe
• Upwork Job ID: 1697001421253230592
• Last Price Increase: 2023-09-20

Issue Owner

Current Issue Owner: @adelekennedy

Issue Owner

Current Issue Owner: @neil-marcellini

[melvin-bot]

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

[melvin-bot]

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

[nikhildewoolkar]

Proposal

What is the problem

Mention is case sensitive.

Root cause

Mention is considering case of the email. Email can't have upper case characters, but the app is treating it as a new user when showing tooltip.

But when we click on the mention, the correct user is shown in the details page.

What changes do i propose

step 1: Mail regex is case sensitive. In below code link, "userMentions'' regex rules block should contain 'gmi' instead of 'gm' in second parameter in "expensify-commons/lib/ExpensiMark.js" to make it case insensitive. So that tooltip will show exact user instead of treating it like new user when any capital letter in mention included.
https://github.com/Expensify/expensify-common/blob/cb1c6b75cb593990ed18381ba7dba8e8ceb9f16b/lib/ExpensiMark.js#L113

These changes in "expensify-commons" won't make any regression.

step 2: the tooltip should convert the email to lowercase so that it finds the correct user. But we don't change the message sent by user.

convert to lowercase here -

App/src/components/HTMLEngineProvider/HTMLRenderers/MentionUserRenderer.js

Line 35 in be55d5e

const loginWithoutLeadingAt = props.tnode.data ? props.tnode.data.slice(1) : '';

[melvin-bot]

📣 @nikhildewoolkar! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[nikhildewoolkar]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01acda9a473ca15e27

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[melvin-bot]

@adelekennedy Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01922c54c131acf6fe

[melvin-bot]

Current assignee @adelekennedy is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[adelekennedy]

I think the likelihood that a user makes this mistake is low - but the outcome is inconvenient enough that we should fix

[tienifr]

Proposal

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

Chat - Including any capital letter in mention displays wrong tooltip and mention with all capital letters is not displayed as mention

What is the root cause of that problem?

We don't have the logic to convert the capital text to the correct mention.
We already have the logic to convert text to emoji here

App/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js

Line 178 in c20c387

const {text: newComment, emojis} = EmojiUtils.replaceAndExtractEmojis(commentValue, preferredSkinTone, preferredLocale);

We should do the same with mention

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

We should create the new function named replaceAndExtractMention.

• lowerCase the mention
• find the matched login or displayName in personalDetails
• return the matched value

here is the sample code

function replaceAndExtractMention(text) {
    const newComment = text.replace(CONST.REGEX.MENTION_REPLACER,(match)=>{
        const matchLowerCase = match.substring(1).toLowerCase()

        const mention = personalDetails.find(m=>(m.displayName?.toLowerCase()===matchLowerCase || m.login?.toLowerCase()===matchLowerCase));

        if(mention) return `@${mention.login || mention.displayName}`;
        return match
    })

    return newComment
}

Then add newComment = replaceAndExtractMention(commentValue) below this line

App/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js

Line 178 in c20c387

const {text: newComment, emojis} = EmojiUtils.replaceAndExtractEmojis(commentValue, preferredSkinTone, preferredLocale);

Result

Screen.Recording.2023-08-31.at.15.48.41.mov

[adelekennedy]

@sobitneupane will review the proposals - not overdue!

[sobitneupane]

Let's first finalize the expected output. We have two probable options.

1. Convert the input with small letters.

Screen.Recording.2023-08-31.at.15.48.41.mov

2. Keep the input as it is and only replace it to small letter for processing.

Screen.Recording.2023-09-05.at.20.29.34.mov

@adelekennedy which one is the expected output?

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[sobitneupane]

#26016 (comment)

@adelekennedy We have two options here. What should be the expected output?

[adelekennedy]

2 in my opinion

[adelekennedy]

Still pending update

[nikhildewoolkar]

It's been so many days now... I have made PR @sobitneupane has already reviewed the PR. Only one review by @youssef-lr is remaining. Let's complete this asap. Kindly check and review it. As well as merge it. @neil-marcellini @sobitneupane @adelekennedy

PR link:- #33539

[youssef-lr]

@nikhildewoolkar the issue is still reproducible, see my last comment please.

[melvin-bot]

This issue has not been updated in over 15 days. @sobitneupane, @neil-marcellini, @nikhildewoolkar, @adelekennedy eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[nikhildewoolkar]

@nikhildewoolkar the issue is still reproducible, see my last comment please.

Actually that latest reproducible bug issue isn't related to my problem statement. It is happening because of some other thing. Let me know, earlier it wasn't like that. Let me know further updates on this. @sobitneupane @youssef-lr

[nikhildewoolkar]

due to slow review, I constantly got new conflicts and had to re-write many times which was challenging. so I think $1000 is a fair compensation. the underlying code was refactored and changed completely, so i had to test it again and again for new code.
CC: @adelekennedy

[youssef-lr]

@nikhildewoolkar Your PR did not fully fix the issue. Here's when I mentioned that, @sobitneupane was able to reproduce as well. The issue I linked was fixed in a different PR and I can't reproduce it currently in staging/production. Here's a recording from staging.

Screen.Recording.2024-02-14.at.01.37.59.mov

due to slow review, I constantly got new conflicts and had to re-write many times which was challenging. so I think $1000 is a fair compensation. the underlying code was refactored and changed completely, so i had to test it again and again for new code.

I would agree if this was a complex PR, but it's a one line change that adds .toLowerCase(), fixing conflicts IMO should not be challenging in this case, no?

I've left one last comment in the PR if you could check it out so we can get the PR merged. Thanks for your patience here.

[nikhildewoolkar]

@nikhildewoolkar Your PR did not fully fix the issue. Here's when I mentioned that, @sobitneupane was able to reproduce as well. The issue I linked was fixed in a different PR and I can't reproduce it currently in staging/production. Here's a recording from staging.

Screen.Recording.2024-02-14.at.01.37.59.mov

due to slow review, I constantly got new conflicts and had to re-write many times which was challenging. so I think $1000 is a fair compensation. the underlying code was refactored and changed completely, so i had to test it again and again for new code.

I would agree if this was a complex PR, but it's a one line change that adds .toLowerCase(), fixing conflicts IMO should not be challenging in this case, no?

I've left one last comment in the PR if you could check it out so we can get the PR merged. Thanks for your patience here.

actually i have attached the evidence below that. It was working fine.
@sobitneupane @youssef-lr Prior to this i have made changes to Expensify/common and wrote test cases there as well. andding @toLowerCase isn't the only thing i did if you check it properly. Check my proposal its written there. that work has been done so much time ago. because of lot of code changes in expensify/App and no proper timely review. I had to stick to this issue again and again.
thats why i think 1000$ is fair compensation. This issue must have solved few months ago. Even for fast responses i requested for slack network with the mail. It didn't work. But i hope you understand my concern.
Thats why i feel due to slow review, I constantly got new conflicts and had to re-write many times which was challenging. so I think $1000 is a fair compensation. the underlying code was refactored and changed completely, so i had to test it again and again for new code..

[nikhildewoolkar]

This issue has been resolved. Kindly generate an upwork job for this particular issue. Kindly read the earlier message .
I think$ 1000 is fair compensation for the task. I have already provided a reason for this in the above comment. Kindly review it.

[youssef-lr]

This issue must have solved few months ago. Even for fast responses i requested for slack network with the mail. It didn't work. But i hope you understand my concern.
Thats why i feel due to slow review, I constantly got new conflicts and had to re-write many times which was challenging.

Hi @nikhildewoolkar. I have not been involved with your previous PRs so I'm not sure how complex were they. But speaking for the one I reviewed, I really don't see how it can be challenging to fix conflicts when we're just adding toLowerCase, worst case you can just grab the code from main and add back toLowerCase to it, no?

As for the delays, here's how many times we had to bump you in this issue:

Please note that your response time will also affect how long an issue can drag. With that being said, I still think $500 is a fair price for the kind of changes we made here and the priority of the bug.

[sobitneupane]

I agree with @youssef-lr. I also believe $500 is fair price for the work.

[adelekennedy]

agreed - looking at the scope of the bug and the delays on both sides I think $500 is fair. @nikhildewoolkar I've sent you a $500 offer on Upwork

@sobitneupane should be paid $500 via NewDot

[nikhildewoolkar]

I have seen the job on upwork and accepted it as well. Kindly check ✅

[nikhildewoolkar]

Kindly accept the submission on upwork. @adelekennedy @neil-marcellini @sobitneupane

[dhanashree-sawant]

Hi @adelekennedy, @neil-marcellini , @sobitneupane sorry I am not sure if issue was worked on or not so can you let me know if this issue is eligible for reporting bonus or not?

[nikhildewoolkar]

Hi @adelekennedy, @neil-marcellini , @sobitneupane sorry I am not sure if issue was worked on or not so can you let me know if this issue is eligible for reporting bonus or not?

Kindly process this fast because it's been 6 months, this issue is open. That's why i had to update code and pr again and again and complete it. PR had been merged long back and everything is done properly. @adelekennedy @neil-marcellini @dhanashree-sawant

[adelekennedy]

sorry about that! The monthly label made me miss these I've just remitted payment and ended the contract. @dhanashree-sawant this was opened right before we changed our reporting bounty, I've sent you a contract on Upwork

[dhanashree-sawant]

Thanks @adelekennedy, I have accepted the offer.

[adelekennedy]

payment has been remitted - closing this!

[sobitneupane]

Requested payment in newDot.

Payment Summary: #26016 (comment)

[JmillsExpensify]

$500 approved for @sobitneupane based on summary.