Skip to content

feat(new-channels): limit @ user mentions to participants, add back @here support#2116

Merged
synoet merged 3 commits intomainfrom
synoet/new-channels-limit-mentions-to-participants
Mar 23, 2026
Merged

feat(new-channels): limit @ user mentions to participants, add back @here support#2116
synoet merged 3 commits intomainfrom
synoet/new-channels-limit-mentions-to-participants

Conversation

@synoet
Copy link
Copy Markdown
Contributor

@synoet synoet commented Mar 23, 2026

No description provided.

@synoet synoet requested a review from a team as a code owner March 23, 2026 16:45
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 4a9ed885-1a7e-4c3a-953c-67eec5ae2612

📥 Commits

Reviewing files that changed from the base of the PR and between 3b252f7 and e470d92.

📒 Files selected for processing (1)
  • js/app/packages/channel/Input/message-payload.ts

Walkthrough

A new useChannelParticipants hook supplies accessors for channel users and their ids. Channel and thread inputs call this hook and pass participants into ChannelInput. ChannelInput and the configured markdown editor accept an optional users provider to drive mention behavior. buildPostMessageRequest now takes an options object ({ snapshot, threadId?, participantIds? }) and expands group mentions (e.g., here) into per-participant user mentions while de-duplicating users. New tests cover @here expansion and deduplication behavior.

Poem

🐰 Hoppity-hop, I fetch the crowd with care,
Every name unfurls from the basket I bear.
Groups bloom into people, one each, not twice,
Mentions trimmed neat, no echoing slice.
I twitch my nose — messages leap through the air!

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive No description was provided by the author, making it impossible to assess relevance to the changeset. Add a pull request description explaining the changes, rationale, and any testing performed to help reviewers understand the scope and impact.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title 'feat(new-channels): limit @ user mentions to participants, add back @here support' accurately describes the main changes: limiting user mentions to participants and restoring @here mention expansion.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch synoet/new-channels-limit-mentions-to-participants

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 23, 2026

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@js/app/packages/channel/Input/tests/message-payload.test.ts`:
- Around line 11-55: Add a test to verify deduplication when `@here` appears
before an explicit user mention by creating a new it block that calls
buildPostMessageRequest with snapshot: snap([...]) where a group { itemType:
'group', itemId: 'here', groupAlias: 'here' } comes before a { itemType: 'user',
itemId: 'user-a' } entry and participantIds includes ['user-a','user-b']; assert
that result.mentions contains only the expanded users (entity_type: 'user'
entity_id: 'user-a' and 'user-b') and that the explicit user mention is skipped,
matching the other tests' style and expectations.

In `@js/app/packages/channel/Thread/ThreadReplyInput.tsx`:
- Around line 28-35: Duplicate participant query and memo logic
(participantsQuery, channelParticipants, participantIds using
useChannelParticipantsQuery and createMemo) appears in ThreadReplyInput.tsx and
Channel.tsx; extract a reusable hook (e.g.,
useChannelParticipantsData(channelId)) that encapsulates calling
useChannelParticipantsQuery and deriving channelParticipants and participantIds,
return both values from the hook, then replace the inline logic in
ThreadReplyInput.tsx and Channel.tsx with calls to
useChannelParticipantsData(props.channelId) so both components consume the
shared implementation.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c6a23c12-2293-484e-ba02-9e607363c5ba

📥 Commits

Reviewing files that changed from the base of the PR and between ffaf50c and 57f84ed.

📒 Files selected for processing (6)
  • js/app/packages/channel/Channel/Channel.tsx
  • js/app/packages/channel/Input/ChannelInput.tsx
  • js/app/packages/channel/Input/configured-markdown-editor.ts
  • js/app/packages/channel/Input/message-payload.ts
  • js/app/packages/channel/Input/tests/message-payload.test.ts
  • js/app/packages/channel/Thread/ThreadReplyInput.tsx

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@js/app/packages/channel/Input/message-payload.ts`:
- Around line 34-41: The code expands every mention with mention.itemType ===
'group' to all participantIds; restrict this by validating the group alias
before expanding—modify the block that checks mention.itemType in
message-payload.ts to also check mention.groupAlias === 'here' (or a whitelist
of allowed aliases) before iterating participantIds, keeping the existing
seenUserIds and result.push logic unchanged so only `@here` groups are expanded.

In `@js/app/packages/channel/use-channel-participants.ts`:
- Around line 15-23: The memos users and ids only check query.isLoading but
ignore query.isError, so surface or log errors: update the hook that defines
users and ids to detect query.isError and either (a) log the error (e.g.,
console.error or a logger) including query.error for debugging, and/or (b)
expose the error state to consumers by returning it alongside the arrays (e.g.,
return { users: createMemo(...), ids: createMemo(...), error: query.error,
isError: query.isError }), and keep the existing graceful fallback using
(query.data ?? []). Ensure you reference the existing symbols users, ids,
channelParticipantInfo and query when making the changes so consumers can react
to query.isError.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1a9fd7ba-68b3-4206-a592-4a874d5ab72a

📥 Commits

Reviewing files that changed from the base of the PR and between 57f84ed and 3b252f7.

📒 Files selected for processing (7)
  • js/app/packages/channel/Channel/Channel.tsx
  • js/app/packages/channel/Input/ChannelInput.tsx
  • js/app/packages/channel/Input/configured-markdown-editor.ts
  • js/app/packages/channel/Input/message-payload.ts
  • js/app/packages/channel/Input/tests/message-payload.test.ts
  • js/app/packages/channel/Thread/ThreadReplyInput.tsx
  • js/app/packages/channel/use-channel-participants.ts

@synoet synoet merged commit 457ebcd into main Mar 23, 2026
23 checks passed
@synoet synoet deleted the synoet/new-channels-limit-mentions-to-participants branch March 23, 2026 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant