Skip to content

[HOLD for payment 2024-03-13] MEDIUM: [$500] Drop domain names when searching for users and auto-filling mentions #35532

Closed

Description

@NikkiWines

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.35-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: N/A
Email or phone of affected tester (no customers): N/A
Logs: N/A
Expensify/Expensify Issue URL: N/A
Issue reported by: N/A
Slack conversation: N/A

Action Performed:

  1. Create a room with 4 members: 2 on the same private domain, 1 on another private domain, and 1 on a public domain
  2. Draft a message that includes mentions for each user.

Expected Result:

When mentioning a user on the same private domain, the search result for the mention should not include the domain for the user's email. For example, given a message between two users - nikkiwines@expensify.com and puneet@expensify.com - the search and auto-complete behavior should look like this:

When mentioning a user on a public domain or on a different private domain, the behavior should remain largely as it is, though we will now prepend an @ to the front of the login details (e.g. Nikki Wines nikkiwines@expensifail.com becomes Nikki Wines @nikkiwines@expensifail.com) when showing the mention options in search.

Actual Result:

The search result and mention auto-complete a same-private-domain mention contains the full email domain, even though the domain name gets removed after sending the message.

Screen.Recording.2024-01-31.at.17.31.02.mov

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

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

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01c84c193620b0a5db
  • Upwork Job ID: 1754684823991881728
  • Last Price Increase: 2024-02-06
  • Automatic offers:
    • tienifr | Contributor | 0

Activity

  1. added
    BugSomething is broken. Auto assigns a BugZero manager.
    on Feb 1, 2024
  2. melvin-bot commented on Feb 1, 2024

    @melvin-bot

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

  3. puneetlath commented on Feb 1, 2024

    @puneetlath
    Contributor

    @NikkiWines what would you think about including the @ symbol in the auto-selection dialog. So instead of having it be:

    Puneet Expensicorp puneet@expensicorp.com
    Puneet Lath puneet

    To have it be:

    Puneet Expensicorp @puneet@expensicorp.com
    Puneet Lath @puneet

    So that we really reinforce this idea of the handle

  4. ayoung19 commented on Feb 1, 2024

    @ayoung19

    Proposal

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

    Users on the same private domain should have their domain hidden in the list of suggestions.

    What is the root cause of that problem?

    Currently the suggested mentions computed are displayed as is.

    mentions={suggestionValues.suggestedMentions}

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

    Like already done in the CompanyStep component, the SuggestionMention component should get the user and session object from Onyx to figure out whether the user is a from a public domain. If not, the domain can be extracted with Str.extractEmailDomain from the session object and this value can be used to transform alternateText in suggestionValues.suggestedMentions to hide the private domain before it is passed into MentionSuggestions.

    We can then modify

    const mentionCode = mentionObject.text === CONST.AUTO_COMPLETE_SUGGESTER.HERE_TEXT ? CONST.AUTO_COMPLETE_SUGGESTER.HERE_TEXT : `@${mentionObject.login}`;
    to also strip the private domain when a suggestion has been selected so that the autofilled value matches what is being displayed in the suggestion list.

    What alternative solutions did you explore? (Optional)

    The above proposal has a little bit of duplicate logic that's spread out in an attempt to lower the risk of regressions, as the underlying data (which may be used in many other places) remains the same. Only alternateText in suggestionValues.suggestedMentions and behavior of insertSelectedMention are changed, both of which are self contained changes that won't propagate too far.

    An alternative solution is to modify how the suggestionValues state is set. getMentionOptions can be modified to correctly compute the correct alternateText and login values of a suggestion. This may have a higher chance of causing a regression.

  5. NikkiWines commented on Feb 1, 2024

    @NikkiWines
    ContributorAuthor

    I'm fine with it! I based this issue on what we have currently in product and also what Slack does.

    image

    But, since the autocomplete mention in the message draft prepends @, and we see it in the actual mention, I think it'd be fine to display it in the search as well. I'll update the screenshots + details in the OP

  6. NikkiWines commented on Feb 1, 2024

    @NikkiWines
    ContributorAuthor

    @puneetlath do you think we should also highlight the @ in the search or do you think it's fine as it is in this screenshot

  7. puneetlath commented on Feb 2, 2024

    @puneetlath
    Contributor

    I think that screenshot looks good! We can get some design team feedback too if you'd like.

  8. alexpensify commented on Feb 2, 2024

    @alexpensify
    Contributor

    @NikkiWines - let me know if you want me to run this over to Design and then I can assign external. Thanks!

  9. NikkiWines commented on Feb 2, 2024

    @NikkiWines
    ContributorAuthor

    Always get to get a once over by the experts! Let's grab someone from design to give things a look just to be certain

  10. alexpensify commented on Feb 6, 2024

    @alexpensify
    Contributor
  11. 59 remaining items

  12. alexpensify commented on Mar 13, 2024

    @alexpensify
    Contributor

    I'm going to work on the payment process tomorrow-- I need to review the payment details here.

  13. alexpensify commented on Mar 15, 2024

    @alexpensify
    Contributor

    Here is the payment summary:

    • External issue reporter - N/A
    • Contributor that fixed the issue @tienifr $500 + $125 = $625
    • Contributor+ that helped on the issue and/or PR @allroundexperts $250 + $125 = $375

    Upwork Job: https://www.upwork.com/jobs/~01c84c193620b0a5db

    Extra Notes regarding payment: There is regression, so I've accounted for that in the payment summary. Closing since everyone who is paid in Upwork has been sent one there.

    There is an additional amount for the extra work as discussed here: #35532 (comment)

  14. allroundexperts commented on Mar 17, 2024

    @allroundexperts
    Contributor

    Hi @alexpensify!
    I think you forgot to account for #35532 (comment) in the payment summary?

  15. alexpensify commented on Mar 18, 2024

    @alexpensify
    Contributor

    @allroundexperts - thanks for the reminder. I'll work on this update later today.

  16. moved this from CRITICAL to MEDIUM in [#whatsnext] #vip-vsbon Mar 19, 2024
  17. alexpensify commented on Mar 19, 2024

    @alexpensify
    Contributor

    I've updated the payment summary:

    #35532 (comment)

  18. alexpensify commented on Mar 19, 2024

    @alexpensify
    Contributor

    Alright, closing again - the summary and the payment in Upwork has been updated accordingly.

  19. JmillsExpensify commented on Mar 22, 2024

    @JmillsExpensify
    Contributor

    $375 approved for @allroundexperts based on summary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

Awaiting PaymentAuto-added when associated PR is deployed to productionBugSomething is broken. Auto assigns a BugZero manager.DailyKSv2ExternalAdded to denote the issue can be worked on by a contributor

Type

No type

Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions