Handle mention suggestions for third party users

[acelaya]

Part of #6886

This PR does two things:

• When building mention suggestions, include only their display name if they are third party users.
• When applying a mention suggestion, use the format @[Display Name] instead of @username if they are a third party user.

Note

This PR does not cover rendering those mentions. This will come on a follow-up PR.

For third party users:

third-party-mentions-2025-03-13_12.59.59.mp4

For first party users everything should continue working the same:

first-party-mentions-2025-03-13_13.00.46.mp4

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.42%. Comparing base (3eea6b9) to head (ff0b8fe).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6889   +/-   ##
=======================================
  Coverage   99.42%   99.42%           
=======================================
  Files         273      273           
  Lines       10509    10523   +14     
  Branches     2523     2529    +6     
=======================================
+ Hits        10449    10463   +14     
  Misses         60       60           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[acelaya]

This type was unused. It was likely something used at some point but then moved to a different module.

[robertknight]

Rather than plumbing down the default authority through several levels of the tree, we could simplify this by deciding in AnnotationEditor whether to use usernames or display names for all mentions, and passing that policy down to MarkdownEditor and children (as a boolean, or enum). That way if we need to change the policy in future (eg. if there is a new third-party that does want us to show usernames), it will be easier to support.

[acelaya]

Rather than plumbing down the default authority through several levels of the tree, we could simplify this by deciding in AnnotationEditor whether to use usernames or display names for all mentions, and passing that policy down to MarkdownEditor and children (as a boolean, or enum). That way if we need to change the policy in future (eg. if there is a new third-party that does want us to show usernames), it will be easier to support.

Yeah, I considered this option. I ended up passing the authority down so that we could do the check for every individually suggested user.

If we do it "globally" at the AnnotationEditor we need to decide how to check that condition. I guess could check if the logged-in user is a third party user and propagate the type based on that.

In future we could choose to change the logic to determine this.

Do you have a better suggestion?

[robertknight]

If we do it "globally" at the AnnotationEditor we need to decide how to check that condition. I guess could check if the logged-in user is a third party user and propagate the type based on that.

You could decide "globally" based on the logged-in user, and that would work fine. You could also decide based on the author of the annotation, using the logged-in user ID as the author of new annotations.

[acelaya]

If we do it "globally" at the AnnotationEditor we need to decide how to check that condition. I guess could check if the logged-in user is a third party user and propagate the type based on that.

You could decide "globally" based on the logged-in user, and that would work fine. You could also decide based on the author of the annotation, using the logged-in user ID as the author of new annotations.

Yeah, I just realized annotation.user always matches the logged-in user in the AnnotationEditor, assuming you cannot edit anyone else's annotations.

I see it's also always set. I was not sure if it would be for new unsaved annotations.

[robertknight]

Looks good with a few corrections. I also had a query about how we are going to handle escaping [ and ] in display names. If you want to handle that it a follow-up PR that would be fine, but I'd make sure it is tracked somewhere.

[robertknight]

There is no escaping of [ or ] in display names here. Do you want to address that here or in a follow up? If in a follow-up I would add a brief comment about it here.

[robertknight]

"Escaping" could also involve replacing or stripping these characters. We don't need to be able to recover (ie. unescape) to get the original text in this context.

[acelaya]

Hmm, I didn't think a user could use square brackets in their display name, but that's definitely a possibility.

[acelaya]

I created an issue to investigate this #6891