Add @-mentions in comments#104
Conversation
Type @username in any comment textarea to open a typeahead picker. On submit, plain @username is rewritten to canonical [@user](mention:user) markdown for round-trip-safe storage. Renders as a styled chip; mentioned users get an inbox notification. Generated with Amp Amp-Thread-ID: https://ampcode.com/threads/T-019de464-3784-7138-9110-2c69513f8c7c Co-authored-by: Amp <amp@ampcode.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dbfa86378a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| async fetchResults(query) { | ||
| try { | ||
| const response = await fetch(`/users/search?q=${encodeURIComponent(query)}`, { |
There was a problem hiding this comment.
Avoid hard-coding "/users/search" here, because this controller lives in the mountable CoPlan engine and host apps can mount the engine under a subpath (for example /coplan). In that setup, mention lookup calls the wrong URL and the picker silently fails for all comments. Build the endpoint from a server-provided route (data attribute) or use a relative URL so it respects the engine mount point.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — fixed in 2c457b3. The Stimulus controller now reads the URL from a searchUrlValue Stimulus value populated server-side via coplan.search_users_path, wired into both _new_comment_form.html.erb and _reply_form.html.erb. Default falls back to /users/search for hosts mounting at root.
| users = if CoPlan.configuration.user_search | ||
| CoPlan.configuration.user_search.call(query) |
There was a problem hiding this comment.
When CoPlan.configuration.user_search is configured, this endpoint returns hook results directly, but mention persistence/notification only works for usernames that exist in CoPlan::User (RewriteMentions resolves against local users). That means the picker can offer selectable users that will be downgraded to plain text on save (no mention chip, no notification). Restricting results to resolvable local usernames (or resolving via the same source used by the hook) prevents this broken suggestion flow.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2c457b3. When the user_search hook is configured, results are now filtered to only include candidates whose username also exists in coplan_users — same set RewriteMentions can resolve. Added a request spec covering the filter behavior.
- Move mention chip rendering from pre-Commonmarker source rewrite to post-Commonmarker Nokogiri replacement so mentions inside fenced code blocks and inline code stay as literal text. - ProcessMentions: use find_or_create_by so editing a comment to add a new mention notifies the new user without duplicating existing rows. Hook moved from after_create_commit to after_save_commit. - Explicit human-only author exclusion when computing badge updates (avoids any chance of an API token UUID colliding with a user UUID). - Picker keydown now ignores Enter/Tab during IME composition. - Remove duplicate /api/v1/users/search endpoint and its spec — the new session-authenticated /users/search is the only user-typeahead surface. Generated with Amp Amp-Thread-ID: https://ampcode.com/threads/T-019de464-3784-7138-9110-2c69513f8c7c Co-authored-by: Amp <amp@ampcode.com>
- comment_form_controller: read /users/search URL from a Stimulus value populated from coplan.search_users_path so the picker respects the engine's mount point. Both new-comment and reply forms updated. - users_controller: when CoPlan.configuration.user_search hook is set, filter results to only include candidates whose username exists in coplan_users — RewriteMentions can only resolve local users, so surfacing external-only candidates would let the picker offer mentions that silently fall through to plain text on save. Generated with Amp Amp-Thread-ID: https://ampcode.com/threads/T-019de464-3784-7138-9110-2c69513f8c7c Co-authored-by: Amp <amp@ampcode.com>
1 participant
Why
Get humans into plan reviews via lightweight @-mention. The reviewer/approver roster we'd been building is metadata without consequences — assignment doesn't tell anyone what to do. Mentions are where collaboration actually starts: tag a person in a comment, they get notified, they show up.
What
@usernametypeahead picker in every comment textarea (new threads + replies). Hits a new session-authenticated/users/searchendpoint.Comments::RewriteMentionsrunsbefore_saveand converts plain@username→ canonical[@username](mention:username)markdown for resolved users only. Textarea stays clean while editing; storage is round-trip-safe.MarkdownHelperrenders the canonical form as a styled chip (Google-Docs blue) — zero DB lookups at render time.Comments::ProcessMentionsfires aNotificationper mentioned user (skips self, dedupes, ignores typos and email addresses). Newmentionreason added toNotification::REASONS.--color-primary,--color-surface, etc.) so dark mode works automatically.Risk Assessment
Low — additive only. New endpoint, new service objects, new model reason; no existing behavior changes. 791 specs pass, including 11 new for the mention rewrite/process flow.
References
coplan_users.usernamewas already populated by OIDC sync — no migration needed.Generated with Amp