Skip to content

fix(js): scope default connectionIdentifier to subscriberId to prevent 409 conflicts#10896

Merged
djabarovgeorge merged 1 commit intonextfrom
fix/js-per-subscriber-default-connection-identifier
Apr 27, 2026
Merged

fix(js): scope default connectionIdentifier to subscriberId to prevent 409 conflicts#10896
djabarovgeorge merged 1 commit intonextfrom
fix/js-per-subscriber-default-connection-identifier

Conversation

@djabarovgeorge
Copy link
Copy Markdown
Contributor

@djabarovgeorge djabarovgeorge commented Apr 27, 2026

Problem

When SlackConnectButton, MsTeamsConnectButton, SlackLinkUser, or MsTeamsLinkUser are rendered without an explicit connectionIdentifier prop, all subscribers in the same environment shared a single hardcoded default:

  • chconn-slack-default
  • chconn-msteams-default

The channel_connections collection enforces a unique index on { _environmentId, identifier }. This means the second subscriber in an environment to click Connect would receive a 409 Conflict from the API and be silently unable to connect — even though their connection was for a completely different subscriber.

The root cause is that the API's CreateChannelConnection use-case has two separate uniqueness guards:

  1. A semantic check: one connection per (integration + subscriberId + contextKeys) — which correctly allows a second subscriber through
  2. An identifier uniqueness check: { _environmentId, identifier } — which blocks the second subscriber because both default to the same static string

Solution

Derive the default connectionIdentifier as <prefix>-<subscriberId> using a new buildDefaultConnectionIdentifier helper, so each subscriber gets their own scoped default. Falls back to the bare prefix when subscriberId is unavailable (shared mode), preserving existing behaviour for that edge case.

No backend changes, no DB migration, no API breaking change.

Files changed

  • packages/js/src/ui/components/constants.ts — added buildDefaultConnectionIdentifier(prefix, subscriberId)
  • SlackConnectButton.tsx — use per-subscriber default
  • MsTeamsConnectButton.tsx — use per-subscriber default
  • SlackLinkUser.tsx — use per-subscriber default
  • MsTeamsLinkUser.tsx — use per-subscriber default

Behavior after this fix

Scenario Before After
Subscriber A connects Slack (no explicit identifier) Creates chconn-slack-default Creates chconn-slack-default-user-A
Subscriber B connects Slack (no explicit identifier) 409 Conflict Creates chconn-slack-default-user-B
Developer passes explicit connectionIdentifier Works Unchanged — explicit value is always used
Shared mode (no subscriberId) Creates chconn-slack-default Same — falls back to bare prefix

Test plan

  • Connect two different subscribers via SlackConnectButton without providing connectionIdentifier — both should connect successfully
  • Verify each subscriber's connection identifier is chconn-slack-default-<subscriberId>
  • Verify that passing an explicit connectionIdentifier still uses that value unchanged
  • Verify SlackLinkUser and MsTeamsLinkUser behave the same way

…t 409 conflicts

When SlackConnectButton, MsTeamsConnectButton, SlackLinkUser, or MsTeamsLinkUser
are rendered without an explicit connectionIdentifier, all subscribers in the same
environment shared the same hardcoded default (chconn-slack-default /
chconn-msteams-default). Because the channel_connections collection has a unique
index on { _environmentId, identifier }, the second subscriber to connect would
receive a 409 Conflict and be unable to connect.

Fix: derive the default identifier as `<prefix>-<subscriberId>` using the new
buildDefaultConnectionIdentifier helper so each subscriber gets their own unique
default. Falls back to the bare prefix when subscriberId is unavailable (shared
mode), matching existing behaviour for that edge case.

Made-with: Cursor
@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 27, 2026

Deploy Preview for dashboard-v2-novu-staging canceled.

Name Link
🔨 Latest commit 0dd5e64
🔍 Latest deploy log https://app.netlify.com/projects/dashboard-v2-novu-staging/deploys/69efa931fceecc00078f9406

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 27, 2026

📝 Walkthrough

Walkthrough

A new buildDefaultConnectionIdentifier function generates connection identifiers by optionally appending a subscriber ID to a prefix. This function is integrated into MS Teams and Slack connect/link components to produce subscriber-specific identifiers instead of static defaults.

Changes

Cohort / File(s) Summary
Connection Identifier Helper
packages/js/src/ui/components/constants.ts
Added buildDefaultConnectionIdentifier function that dynamically constructs connection identifiers by suffixing a prefix with an optional subscriber ID.
MS Teams Components
packages/js/src/ui/components/msteams-connect-button/MsTeamsConnectButton.tsx, packages/js/src/ui/components/msteams-link-user/MsTeamsLinkUser.tsx
Updated to resolve connectionIdentifier using the new helper, computing resolvedSubscriberId and passing it to build subscriber-specific identifiers. Consolidated duplicate resolver declarations in connect button.
Slack Components
packages/js/src/ui/components/slack-connect-button/SlackConnectButton.tsx, packages/js/src/ui/components/slack-link-user/SlackLinkUser.tsx
Updated to resolve connectionIdentifier via the new helper with subscriber-specific logic. Moved early resolver declarations and removed later duplicates in connect button.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title follows Conventional Commits format with valid type 'fix' and scope 'js', uses lowercase imperative description, and clearly summarizes the main change: scoping default connectionIdentifier to subscriberId.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions
Copy link
Copy Markdown
Contributor

Hey there and thank you for opening this pull request! 👋

We require pull request titles to follow specific formatting rules and it looks like your proposed title needs to be adjusted.

Your PR title is: fix(js): scope default connectionIdentifier to subscriberId to prevent 409 conflicts

Requirements:

  1. Follow the Conventional Commits specification
  2. As a team member, include Linear ticket ID at the end: fixes TICKET-ID or include it in your branch name

Expected format: feat(scope): Add fancy new feature fixes NOV-123

Details:

PR title must end with 'fixes TICKET-ID' (e.g., 'fixes NOV-123') or include ticket ID in branch name

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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
packages/js/src/ui/components/slack-link-user/SlackLinkUser.tsx (1)

35-44: ⚠️ Potential issue | 🟡 Minor

Remove unused subscriberId parameter from hook initialization.

The subscriberId passed to useChannelEndpoint on line 43 is not used by the hook—createResource only depends on endpointIdentifier. The actual generateLinkUserOAuthUrl call correctly receives the resolved subscriberId at invocation time (line 155), and list operations correctly use the derived connectionIdentifier. Passing props.subscriberId to the hook is misleading and should be removed.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/js/src/ui/components/slack-link-user/SlackLinkUser.tsx` around lines
35 - 44, The hook initialization passes an unused subscriberId prop; remove
subscriberId: props.subscriberId from the useChannelEndpoint call and keep the
rest (integrationIdentifier: integrationIdentifier(), connectionIdentifier:
connectionIdentifier()) so generateLinkUserOAuthUrl continues to receive the
resolved subscriber via its invocation; update the call site that constructs the
hook (useChannelEndpoint) and ensure resolvedSubscriberId(),
connectionIdentifier(), buildDefaultConnectionIdentifier,
DEFAULT_SLACK_CONNECTION_IDENTIFIER, and generateLinkUserOAuthUrl remain
unchanged.
packages/js/src/ui/components/msteams-connect-button/MsTeamsConnectButton.tsx (1)

47-59: ⚠️ Potential issue | 🟡 Minor

Pass the resolved subscriber ID to the hook for consistency.

The hook receives subscriberId: props.subscriberId but derives connectionIdentifier from resolvedSubscriberId(). While the hook only uses connectionIdentifier for its API calls and event listeners (not subscriberId), the mismatched values are confusing. Pass resolvedSubscriberId() instead to keep the component's state coherent.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@packages/js/src/ui/components/msteams-connect-button/MsTeamsConnectButton.tsx`
around lines 47 - 59, The component passes props.subscriberId into
useChannelConnection while connectionIdentifier is built from
resolvedSubscriberId(), causing inconsistent state; change the hook call to pass
resolvedSubscriberId() instead of props.subscriberId so useChannelConnection
receives the same subscriber id used by connectionIdentifier (update the
useChannelConnection invocation that destructures connection, loading,
disconnect, mutate, generateConnectOAuthUrl to use resolvedSubscriberId()).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In
`@packages/js/src/ui/components/msteams-connect-button/MsTeamsConnectButton.tsx`:
- Around line 47-59: The component passes props.subscriberId into
useChannelConnection while connectionIdentifier is built from
resolvedSubscriberId(), causing inconsistent state; change the hook call to pass
resolvedSubscriberId() instead of props.subscriberId so useChannelConnection
receives the same subscriber id used by connectionIdentifier (update the
useChannelConnection invocation that destructures connection, loading,
disconnect, mutate, generateConnectOAuthUrl to use resolvedSubscriberId()).

In `@packages/js/src/ui/components/slack-link-user/SlackLinkUser.tsx`:
- Around line 35-44: The hook initialization passes an unused subscriberId prop;
remove subscriberId: props.subscriberId from the useChannelEndpoint call and
keep the rest (integrationIdentifier: integrationIdentifier(),
connectionIdentifier: connectionIdentifier()) so generateLinkUserOAuthUrl
continues to receive the resolved subscriber via its invocation; update the call
site that constructs the hook (useChannelEndpoint) and ensure
resolvedSubscriberId(), connectionIdentifier(),
buildDefaultConnectionIdentifier, DEFAULT_SLACK_CONNECTION_IDENTIFIER, and
generateLinkUserOAuthUrl remain unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c07e3ce4-1b42-4b26-b268-1f884bbb8567

📥 Commits

Reviewing files that changed from the base of the PR and between c0055bd and 0dd5e64.

📒 Files selected for processing (5)
  • packages/js/src/ui/components/constants.ts
  • packages/js/src/ui/components/msteams-connect-button/MsTeamsConnectButton.tsx
  • packages/js/src/ui/components/msteams-link-user/MsTeamsLinkUser.tsx
  • packages/js/src/ui/components/slack-connect-button/SlackConnectButton.tsx
  • packages/js/src/ui/components/slack-link-user/SlackLinkUser.tsx

@djabarovgeorge djabarovgeorge merged commit 18d000e into next Apr 27, 2026
32 checks passed
@djabarovgeorge djabarovgeorge deleted the fix/js-per-subscriber-default-connection-identifier branch April 27, 2026 18:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant