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

Chat search should prioritise a perfectly matching email above a partially matching one #8763

Closed
Julesssss opened this issue Apr 25, 2022 · 16 comments
Assignees
Labels
Engineering Improvement Item broken or needs improvement. Reviewing Has a PR in review Weekly KSv2

Comments

@Julesssss
Copy link
Contributor

Julesssss commented Apr 25, 2022

Action Performed:

  • Have some accounts with similar names, in my case user2@expensify.com & newuser2@expensify.com
  • Sign in to two accounts, in my case user7@expensify.com & user2@expensify.com
  • As one user, attempt to search for the other, using their EMAIL ADDRESS
  • Enter the full emal address and hit enter

Expected Result:

  • I would expect the first result to be the user which matches the email

Actual Result:

  • A completely different user chat was opened, one which is similarly named

Workaround:

  • Manually select the user which matches the email search
NewDot-SearchShouldPrioritiseFullMatches.mov

Platform:

  • Web
  • Desktop App

Version Number: v1.1.56-0
Reproducible in staging?: Yep
Reproducible in production?: Yep
Email or phone of affected tester (no customers): N/A
Logs: https://stackoverflow.com/c/expensify/questions/4856 N/A
Notes/Photos/Videos: See above
Expensify/Expensify Issue URL: N/A
Issue reported by: @Julesssss
Slack conversation: N/A

View all open jobs on GitHub

@Julesssss Julesssss added AutoAssignerTriage Auto assign issues for triage to an available triage team member Daily KSv2 labels Apr 25, 2022
@Julesssss Julesssss self-assigned this Apr 25, 2022
@melvin-bot
Copy link

melvin-bot bot commented Apr 25, 2022

Triggered auto assignment to @laurenreidexpensify (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

@melvin-bot melvin-bot bot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Apr 25, 2022
@Julesssss Julesssss added Internal Requires API changes or must be handled by Expensify staff and removed Internal Requires API changes or must be handled by Expensify staff labels Apr 25, 2022
@laurenreidexpensify laurenreidexpensify removed their assignment Apr 26, 2022
@melvin-bot
Copy link

melvin-bot bot commented Apr 26, 2022

Current assignee @Julesssss is eligible for the Engineering assigner, not assigning anyone new.

@laurenreidexpensify
Copy link
Contributor

@Julesssss wanna apply the rest of the labels to get this on upwork? add external and we're in business

@Julesssss Julesssss added the External Added to denote the issue can be worked on by a contributor label Apr 26, 2022
@melvin-bot
Copy link

melvin-bot bot commented Apr 26, 2022

Triggered auto assignment to @jliexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@Julesssss Julesssss added the Improvement Item broken or needs improvement. label Apr 26, 2022
@Julesssss
Copy link
Contributor Author

Out of interest Lauren, how come you unassigned yourself? Auto-assigner would have kept you on in that case right? Like it did for me

@jliexpensify
Copy link
Contributor

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Apr 27, 2022
@melvin-bot
Copy link

melvin-bot bot commented Apr 27, 2022

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Apr 27, 2022
@melvin-bot
Copy link

melvin-bot bot commented Apr 27, 2022

Current assignee @Julesssss is eligible for the Exported assigner, not assigning anyone new.

@melvin-bot melvin-bot bot changed the title Chat search should prioritise a perfectly matching email above a partially matching one [$250] Chat search should prioritise a perfectly matching email above a partially matching one Apr 27, 2022
@tgolen
Copy link
Contributor

tgolen commented Apr 27, 2022

I'm actually gonna grab this and work on it myself

@tgolen tgolen assigned tgolen and unassigned mananjadhav and Julesssss Apr 27, 2022
@tgolen tgolen removed 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 labels Apr 27, 2022
@tgolen tgolen changed the title [$250] Chat search should prioritise a perfectly matching email above a partially matching one Chat search should prioritise a perfectly matching email above a partially matching one Apr 27, 2022
@Julesssss
Copy link
Contributor Author

Nice, thanks Tim. I honestly wasn't sure if this was worth fixing. But it will inevitably catch users out so let's get it resolved.

Feel free to tag me as a reviewer!

@tgolen
Copy link
Contributor

tgolen commented Apr 27, 2022

Thanks @Julesssss!

I'm having some issues reproducing the issue. I created three users:

When I search for "user1@" it selects the correct person:
image

When I search for "newuser1@" it also selects the correct person:
image

It appears to be working correctly, but maybe there is something specific about your setup that is causing the issue you're seeing. Do you think you could help me narrow down the exact case to reproduce this?

@Julesssss
Copy link
Contributor Author

Ah curious. I'll take another look at my env first thing tomorrow morning and get back to you. It's even more of an edge case than I originally thought :D

@jliexpensify
Copy link
Contributor

Cancelling Upworks job.

@Julesssss
Copy link
Contributor Author

@tgolen

I tried again today and noticed that newuser2@... was showing above user2@ because I had sent messages to that user recently. I still think there's an argument that a literal email match should take priority over recent chat history, but I'll leave it up to you to decide whether this is worth changing.

@tgolen
Copy link
Contributor

tgolen commented Apr 28, 2022

Aha! Maybe there is something there. I see we are setting sortByLastMessageTimestamp: false, when doing a search, so I will look into that a little more.

@tgolen
Copy link
Contributor

tgolen commented Apr 28, 2022

Yep, I was able to reproduce it after having a recent message sent to newuser1@expensify.com:

image

Looking into it more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering Improvement Item broken or needs improvement. Reviewing Has a PR in review Weekly KSv2
Projects
None yet
Development

No branches or pull requests

5 participants