[$500] Chat - After selecting "here" from suggestion list, the suggestion box is opened

[lanitochka17]

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

---

Version Number: 1.4.6-2
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: Applause - Internal Team
Slack conversation:

Action Performed:

Pre-condition : Must have user with 'here' in the display name.

1. Go to https://staging.new.expensify.com/
2. Tap on a report
3. Enter @ in compose box
   4.Select "here" from suggestion list

Expected Result:

After selecting "here" from suggestion list, the suggestion box must be closed

Actual Result:

After selecting "here" from suggestion list, the suggestion box is opened showing contact with "here" display name

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

Bug6296214_1701362159518.az_recorder_20231130_210049.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~010fc871e1309cdbb5
• Upwork Job ID: 1740328328014270464
• Last Price Increase: 2024-02-14

[melvin-bot]

Triggered auto assignment to @sakluger (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

[paultsimura]

Proposal

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

When entering @here, the suggestions are reopened.

What is the root cause of that problem?

If this is not an expected behavior – it looks like a regression from #30217
We now allow searching the suggestion by 2 words and do not handle a specific reserved keyword – @here.

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

To solve it, we should skip the multi-word calculation in case the first word is @here:

} else if (secondToLastWord && secondToLastWord !== "@here" && secondToLastWord.startsWith('@') && secondToLastWord.length > 1) {

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

Line 215 in 7f00bff

} else if (secondToLastWord && secondToLastWord.startsWith('@') && secondToLastWord.length > 1) {

What alternative solutions did you explore? (Optional)

[sakluger]

@graylewis @allroundexperts, the above proposal says that your PR for #30217 may have caused this regression, do you think that's right?

@situchan this also seems related (or at least very similar) to the issue you just fixed for #32214, and your PR there didn't fix this one. Are they related, or just similar?

[allroundexperts]

I think the PR that @situchan did should have fixed this as well. @situchan Can you confirm?

[situchan]

Seems like this is another edge case where user display name includes "here"

[allroundexperts]

Hm... But the root cause is the same as previous bug, right?

[situchan]

yes, both issues are regression from #30217

[graylewis]

The first issue is definitely caused by my solution (#32214) (to me this seems more like a UX nit-pick than a bug. If you start typing after selecting the mention the suggestion goes away). However this issue seems like either an edge case that wasn't considered in the solution for #32214, or just a duplicate of that issue.

[situchan]

Ideally, #31435 should have been reverted at the time of deploy blocker.
But we applied patch (more like UX improvement) instead of revert in #32227. This actually didn't fix the root cause of regression caused by #31435.
So I'd say bug demoted to NAB by fixing non-display name cases. Still remaining in this edge case of "here" being contained in display name.

[allroundexperts]

It's a weird situation tbh. @situchan Can you maybe raise a PR for this since you applied the previous patch as well?

[situchan]

yes will do if @graylewis doesn't want to.

[puneetlath]

Another example of this bug: https://expensify.slack.com/archives/C049HHMV9SM/p1701454107049459

[sakluger]

Just to confirm, should this have been fixed in any of the past PRs? Or is this a separate edge case?

[allroundexperts]

I think it should have been fixed in the past PRs. @graylewis can you please let us know if you'd have time for creating a fix? If no, then that is fine as well and @situchan can take over.

[graylewis]

I can pick up the ticket but I can't work on it until the weekend @situchan @allroundexperts

[sakluger]

That's fine, we can wait a few days. @graylewis please create a PR as soon as you have a chance, thanks!

[graylewis]

@sakluger @allroundexperts Unfortunately things came up the past couple days. Going to try to get a PR in this week. Sorry for the delay

[sakluger]

@graylewis it's important that we prioritize fixing regressions, can you please prioritize this one today?

[graylewis]

@sakluger I'm not being compensated for this work so I simply can't prioritize it over my job and personal obligations. It's also already 7PM here in the Netherlands. I'll do my best to get this done tomorrow.

I was docked $250 for this UX nitpick so being asked to make the solution for free and then being put under time pressure for free work feels inappropriate.

[graylewis]

Trying to build a fix for this issue now but I'm unable to reproduce. Was a patch for this already pushed? Also tested this on Android chrome with the same results

@sakluger @allroundexperts @situchan

Screen.Recording.2023-12-13.at.10.55.40.AM.mov

[melvin-bot]

Triggered auto assignment to @arosiclair, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[arosiclair]

I'm thinking the root problem is that we are still showing suggestions after a suggestion has already been selected. I also think this would be an issue for any matching prefix not just "here". For example this would also reproduce the issue:

1. Type in @prefix
2. Select "Prefix" from suggestions
3. Suggestion still displays for "Prefix Suffix".

So this

I was thinking of having a flag if the suggestion was chosen, but it would need to be stored somewhere (ideally in Onyx, I wonder if it should also be sent to the BE - I'll check it) and then toggled when we start to type another mention and I wonder if it would affect the overall performance and is worth implementing.

sounds like a better direction to me.

What's tricky is when to start showing suggestions again. Maybe whenever the @text is changed or when another @ is input?

[koko57]

I'm thinking the root problem is that we are still showing suggestions after a suggestion has already been selected. I also think this would be an issue for any matching prefix not just "here". For example this would also reproduce the issue:

1. Type in @prefix
2. Select "Prefix" from suggestions
3. Suggestion still displays for "Prefix Suffix".

Responding to your concern - no it won't. If you choose @prefix from a selection list the composer will be filled with the handle and a space - the lastWord will be an empty string and secondToLast will be '@Prefix'. If we check for '@Prefix' in the condition the suggestion list re-calculation won't be triggered and the list will be reset. The suggestion will reappear only if you start typing "Suffix".

When we select a user from the suggestion list the name we typed is replaced with the handle with the user's email login (even for phone numbers) the only exception is the "here" mention - it stays as it is "@here". If some other handles like this will be added in the future, that should be added to the condition. We could have a util function to check for all the possible default mentions.

[arosiclair]

If we check for '@Prefix' in the condition the suggestion list re-calculation won't be triggered and the list will be reset. The suggestion will reappear only if you start typing "Suffix".

You mean this regarding your proposal, right? I'm referring to just the current (broken) behavior.

My concern is that with your proposal, we have to hardcode any conflicting @text. And once we add proper @handle support (similar thing coming in this issue), it won't work anymore. So it seems that this part that you mentioned:

As calculateMentionSugestion is also triggered when we choose a suggestion

is the real problem and we just need to stop that from happening with a flag or maybe something else.

[koko57]

@arosiclair Ah, ok, yes I meant my proposal. You're right - if we're going to change the handle behavior, it makes sense to investigate the other solution. Thanks for pointing this out!

[koko57]

Working on the alternative solution

[koko57]

Ok, so another solution that would be ready for the mention handle changes:

Proposal

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

same as here

What is the root cause of that problem?

same as here

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

Just like we save the draft only in Onyx (we don’t sync the draft with other devices, we don’t send it to the BE), we can do the same for the mentions. Let’s save the mention after it was chosen from the suggestion list as reportDraftLastMention for each chat (just like the drafts). After it was saved we check if the lastWord (ot the secondToLast, when last word is an empty string) is the same as reportDraftLastMention - if so, reset the suggestion list and return early from the calculateMentionSuggestion function. When the message is sent, or we the mention removed from the message, reportDraftLastMention would be cleared.

What alternative solutions did you explore? (Optional)

[arosiclair]

Do we need to use Onyx for that or can we just use local state in SuggestionMention? I see we already have a shouldBlockCalc flag there. Maybe we can just use that?

[koko57]

local state won't work after refreshing the page and leaving the chat and returning back to draft - the suggestion will be shown again

[arosiclair]

Ah okay good points. Let's move forward with your proposal 👍. Last thing: can we just name the Onyx key something more clear like lastSelectedMentionSuggestion? Thanks!

[koko57]

Thanks! 🙌🏻 Yeah, I'll try to come up with another name 😃

[koko57]

Hi! I opened the PR last week 🙂 I'm ooo this week, so if any changes will be required or there will be any conflicts to resolve, please ping Callstack team - someone will take it over

[koko57]

PR: #37262

[koko57]

probably fixed by this: #38361

[arosiclair]

@sobitneupane can you retest to verify this is no longer an issue with those changes?

[sobitneupane]

@arosiclair Yup. The issue is no longer reproducible.

Screen.Recording.2024-03-20.at.12.17.52.mov

[arosiclair]

@arosiclair Yup. The issue is no longer reproducible.

Alright that's good news. I appreciate the efforts from you guys. @sobitneupane still did most of the work so I think they should still get payment. What do you think @sakluger?

[sakluger]

Sounds good to me! Thanks, everyone, for sticking with this one.

Summarizing payment on this issue:

Contributor: @koko57 - no payment necessary (from CallStack)
Contributor+: @sobitneupane $500, please request on Newdot

[JmillsExpensify]

$500 approved for @sobitneupane based on summary.