feat(react): create connect chat channel connections#10711
Merged
djabarovgeorge merged 25 commits intonextfrom Apr 15, 2026
Merged
feat(react): create connect chat channel connections#10711djabarovgeorge merged 25 commits intonextfrom
djabarovgeorge merged 25 commits intonextfrom
Conversation
…controller and service - Implemented new endpoints for listing, creating, retrieving, and deleting channel connections and endpoints in the InboxController. - Updated InboxService to handle channel connection and endpoint operations. - Enhanced the API with new DTOs and commands for channel connections and endpoints. - Integrated feature flags for conditional access to channel features. - Updated module imports to include ChannelConnections and ChannelEndpoints modules.
…nctionality - Added support for Slack integration in the playground with new environment variables for Slack user ID and integration identifier. - Implemented a new DM endpoint creation feature in the Connect Chat page to resolve emails to Slack user IDs. - Updated the LinkUser component to manage channel endpoints more effectively using the Novu context. - Introduced new hooks for channel connections in the React package to streamline integration. - Updated pnpm-lock.yaml to include the internal SDK for Novu API.
- Deleted the `slack-connect.ts` API endpoint as it is no longer needed. - Updated the Slack user ID in the Connect Chat page to reflect the new default value.
- Removed console log statements for API response and data in the Connect Chat page to clean up the code and improve readability.
…hat page - Added a new input field for Slack user ID with a tooltip for user guidance. - Implemented logic to determine the effective Slack user ID for linking, prioritizing user input, environment variable, and a default value. - Updated the LinkUser component to use the dynamically determined Slack user ID.
…rops - Replaced slackUserId prop with type and endpoint props in LinkUser and DefaultLinkUser components for improved flexibility. - Enhanced endpoint matching logic to avoid duplicates based on type and endpoint combination. - Updated related documentation and examples to reflect changes in prop usage.
…to support new parameters - Added optional parameters `channel`, `providerId`, and `contextKeys` to `ListChannelConnectionsArgs` type for enhanced filtering capabilities. - Updated `InboxService` methods to incorporate the new parameters in search queries, allowing for more granular data retrieval. - Modified `LinkUser` component to accept and utilize `contextKeys` for improved endpoint deduplication logic.
- Introduced optional parameters `endpointType` and `endpointData` across various controllers and DTOs to facilitate automatic channel endpoint creation during the OAuth process. - Updated the `GenerateChatOauthUrl` and related use cases to handle new parameters, streamlining the integration process for Slack and Microsoft Teams. - Enhanced the `connectAndLink` method in the ChannelConnections module to support the new functionality, allowing for a single API call to create both channel connections and endpoints. - Modified relevant components and hooks in the React and Next.js applications to utilize the new parameters, improving user experience and reducing the need for additional API calls.
- Introduced the SlackConnectButton component to the UI library, enhancing integration with Slack. - Updated appearance keys and types to include SlackConnectButton properties for better customization. - Modified relevant components and pages to utilize the new SlackConnectButton, improving user experience in the Connect Chat flow. - Removed playground directory from cursorignore to streamline development.
- Eliminated the optional parameters `endpointType` and `endpointData` from various controllers, DTOs, and use cases to simplify the chat OAuth process. - Updated the `GenerateChatOauthUrl` and related methods to remove references to the removed parameters, streamlining the integration for Slack and Microsoft Teams. - Adjusted the `connectAndLink` method in the ChannelConnections module to reflect these changes, ensuring a cleaner API interaction. - Modified relevant components and hooks in the React and Next.js applications to remove dependencies on the removed parameters, enhancing code clarity and maintainability.
- Added `userScope` and `mode` parameters to the chat OAuth flow in the Inbox and Integrations controllers, allowing for more granular control over OAuth requests. - Updated the `GenerateChatOauthUrl` and related DTOs to include the new parameters, facilitating user-level OAuth scope requests. - Implemented logic in the Slack OAuth callback to handle the new `link_user` mode, enabling linking of subscribers to Slack users. - Modified relevant components and hooks in the React and Next.js applications to support the new parameters, improving the user experience during the OAuth process.
…andling - Changed the `subscriberId` prop in `LinkSlackUserProps` to be optional, allowing for more flexible usage. - Updated the `LinkSlackUser` component to use a fallback for `subscriberId` when not provided. - Removed `subscriberId` from `DefaultLinkSlackUserProps` to align with the updated prop definition. - Introduced a context variable in the `ConnectChatPage` to enhance the linking process with contextual information.
- Added the SlackLinkUser component to facilitate linking subscribers via Slack OAuth. - Updated type definitions to replace LinkSlackUser with SlackLinkUser for consistency across the codebase. - Modified relevant imports and component references in the UI library and examples to reflect the new naming convention. - Enhanced the ConnectChatPage to utilize the new SlackLinkUser component, improving the user experience during the linking process.
…ntifiers - Introduced `slack-constants.ts` to define default values for connection and integration identifiers. - Updated `SlackConnectButton` and `SlackLinkUser` components to utilize these constants, improving code maintainability and consistency. - Refactored relevant logic to use the new constants for better clarity in connection handling.
- Added `connectionMode` property to `CreateChannelConnectionRequestDto`, `CreateChannelConnectionCommand`, and related DTOs to support different connection strategies. - Implemented validation logic in the `CreateChannelConnection` use case to enforce rules based on the selected `connectionMode` (either 'subscriber' or 'shared'). - Updated relevant components and hooks across the UI to handle the new `connectionMode` parameter, enhancing flexibility in connection management. - Modified the `SlackConnectButton` and `ConnectChat` components to incorporate `connectionMode`, improving user experience during the connection process.
…d update related types - Renamed `SlackConnectButton` to `ChannelConnectButton` across components and types for consistency. - Updated appearance keys and types to reflect the new naming convention, enhancing clarity in the UI. - Modified the `ConnectChatPage` to utilize the updated `ChannelConnectButton`, improving the integration experience. - Adjusted related imports and context handling to support the new structure, ensuring seamless functionality.
…ser feedback - Changed button text in LinkUser component to 'Connected' and 'Connect your Slack account' for clarity. - Enhanced SlackLinkUser component with visual feedback using icons to indicate connection status. - Updated appearance keys to include 'linkSlackUserButtonIcon' for better styling control.
- Added new types for SlackLinkUser appearance callbacks to improve customization options. - Updated SlackLinkUser component to utilize the new appearance callbacks for dynamic styling based on user link status. - Modified ConnectChatPage to demonstrate the new appearance capabilities, enhancing user feedback during the linking process.
…ditional parameters - Modified the `get` method calls in `InboxService` to include an `undefined` parameter and a `false` flag, enhancing the flexibility of the HTTP requests. - Updated `ConnectChatPage` to comment out the Slack button labels, preparing for potential future customization or changes in the UI.
✅ Deploy preview added
To edit notification comments on pull requests, go to your Netlify project configuration. |
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: Requirements:
Expected format: Details: PR title must end with 'fixes TICKET-ID' (e.g., 'fixes NOV-123') or include ticket ID in branch name |
Contributor
📝 WalkthroughWalkthroughThis PR introduces channel connections and channel endpoints management across the Novu platform. It adds backend API routes for CRUD operations on subscriber-scoped channel connections and endpoints, extends the Slack OAuth flow to support link-user mode with user-level endpoints, introduces new React UI components and hooks for managing these resources, and includes playground examples demonstrating the integration. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ 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. Comment |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 56291a3. Configure here.
- Updated the `GenerateSlackOauthUrl` use case to return a dynamically constructed OAuth URL using the `API_ROOT_URL` environment variable, enhancing flexibility and maintainability. - Refactored the `LinkUser` and `SlackLinkUser` components to utilize `onCleanup` for better resource management during component unmounting.
…hq/novu into chat-new-connect-components
@novu/js
@novu/nextjs
novu
@novu/providers
@novu/react
@novu/react-native
@novu/shared
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-slack-oath-url/generate-slack-oauth-url.usecase.ts (1)
196-204:⚠️ Potential issue | 🔴 CriticalCritical: Hardcoded ngrok URL breaks production OAuth flow.
The redirect URI uses a hardcoded ngrok development URL instead of the environment-based
API_ROOT_URL. This will break OAuth callbacks in all non-development environments and could expose users to unintended redirect behavior.The MS Teams OAuth handler at
apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-msteams-oath-url/generate-msteams-oauth-url.usecase.ts:149-155shows the correct pattern.🐛 Proposed fix
static buildRedirectUri(): string { if (!process.env.API_ROOT_URL) { throw new Error('API_ROOT_URL environment variable is required'); } const baseUrl = process.env.API_ROOT_URL.replace(/\/$/, ''); // Remove trailing slash - return `https://536c-84-110-187-162.ngrok-free.app${CHAT_OAUTH_CALLBACK_PATH}`; - // return `${baseUrl}${CHAT_OAUTH_CALLBACK_PATH}`; + return `${baseUrl}${CHAT_OAUTH_CALLBACK_PATH}`; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-slack-oath-url/generate-slack-oauth-url.usecase.ts` around lines 196 - 204, The buildRedirectUri method currently returns a hardcoded ngrok URL; update GenerateSlackOauthUrlUsecase.buildRedirectUri (the static buildRedirectUri method) to construct the redirect URI from process.env.API_ROOT_URL like the MS Teams handler: keep the existing check that throws if process.env.API_ROOT_URL is missing, strip any trailing slash from API_ROOT_URL into baseUrl, and return `${baseUrl}${CHAT_OAUTH_CALLBACK_PATH}` instead of the hardcoded ngrok value so the OAuth callback uses the environment-configured root URL.
🧹 Nitpick comments (13)
packages/js/scripts/size-limit.mjs (1)
18-18: Please document the bundle-budget increase rationale at Line 18.Raising the UMD minified limit reduces the bundle-size guardrail for
packages/js. Add a short inline note (or PR-linked comment) explaining the expected delta and why213_000is the new baseline to prevent silent size creep.As per coding guidelines, "
packages/js/**: Review with focus on bundle size, browser compatibility, and public API surface."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/js/scripts/size-limit.mjs` at line 18, The bundle-budget increase at limitInBytes: 213_000 removes a guardrail for packages/js and needs an inline rationale; add a short comment next to the limitInBytes entry (or a linked PR comment) that states the expected size delta, why 213_000 is chosen as the new baseline (e.g., new deps, minifier changes, expected gzip/UMD impact), and a reminder to review packages/js for bundle size, browser compatibility, and public API surface; reference the limitInBytes symbol and the packages/js bundle in the comment so future reviewers can trace the decision.packages/js/src/ui/components/Renderer.tsx (1)
158-180: Consider unifying withSubscriptionComponentsRenderer.
ChannelComponentsRendereris structurally identical toSubscriptionComponentsRenderer(lines 134-156). While the duplication is acceptable for clarity, you could extract a sharedSimpleComponentsRendererto reduce repetition.♻️ Optional: Extract shared renderer
const SimpleComponentsRenderer = (props: { elements: MountableElement[]; nodes: Map<MountableElement, NovuComponent>; }) => { return ( <Show when={props.elements.length > 0}> <For each={props.elements}> {(node) => { const novuComponent = () => props.nodes.get(node)!; const Component = novuComponents[novuComponent().name]; return ( <Portal mount={node}> <Root> <Component {...novuComponent().props} /> </Root> </Portal> ); }} </For> </Show> ); }; // Then use for both: // <SimpleComponentsRenderer elements={subscriptionComponents()} nodes={props.nodes} /> // <SimpleComponentsRenderer elements={channelComponents()} nodes={props.nodes} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/js/src/ui/components/Renderer.tsx` around lines 158 - 180, ChannelComponentsRenderer duplicates SubscriptionComponentsRenderer; extract a shared SimpleComponentsRenderer that accepts props { elements: MountableElement[]; nodes: Map<MountableElement, NovuComponent> } and contains the current JSX logic (Show/For mapping to novuComponents lookup, Portal mount, Root and <Component {...novuComponent().props} />). Replace both ChannelComponentsRenderer and SubscriptionComponentsRenderer usages with SimpleComponentsRenderer (pass subscriptionComponents() and channelComponents() as elements and props.nodes as nodes) and remove the duplicated component definitions; keep the same internal symbols novuComponent, Component, Portal, and Root to avoid changing behavior.packages/react/src/components/connect-chat/ConnectChat.tsx (2)
30-32: Add blank line beforereturnstatement.Per coding guidelines, include a blank line before every
returnstatement.Suggested fix
export const ConnectChat = React.memo((props: ConnectChatProps) => { + return <ConnectChatInternal {...props} />; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/components/connect-chat/ConnectChat.tsx` around lines 30 - 32, The ConnectChat component lacks a blank line before its return; update the React.memo wrapper function (ConnectChat) to insert a single empty line immediately before the "return <ConnectChatInternal {...props} />;" statement so the function body follows the project's guideline (locate the ConnectChat arrow function that returns ConnectChatInternal).
20-25: Add blank line beforereturnstatement.Per coding guidelines, include a blank line before every
returnstatement.Suggested fix
}, [container, appearance, novu.options]); + return ( <NovuUI options={options} novu={novu}> <DefaultConnectChat {...defaultProps} /> </NovuUI> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/components/connect-chat/ConnectChat.tsx` around lines 20 - 25, The ConnectChat component's render/functional body should have a blank line immediately before the return statement; update the ConnectChat function (where NovuUI and DefaultConnectChat are returned) to insert a single empty line above the "return (" line so the return is separated from preceding logic per the coding guideline.packages/react/src/components/slack-connect-button/SlackConnectButton.tsx (2)
30-32: Add blank line beforereturnstatement.Per coding guidelines, include a blank line before every
returnstatement.Suggested fix
export const SlackConnectButton = React.memo((props: SlackConnectButtonProps) => { + return <SlackConnectButtonInternal {...props} />; });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/components/slack-connect-button/SlackConnectButton.tsx` around lines 30 - 32, In the SlackConnectButton React.memo wrapper, add a blank line immediately before the return statement so the function body contains a separating empty line prior to "return <SlackConnectButtonInternal {...props} />;"; update the SlackConnectButton declaration (which delegates to SlackConnectButtonInternal) to include that blank line to comply with the coding guideline.
20-25: Add blank line beforereturnstatement.Per coding guidelines, include a blank line before every
returnstatement.Suggested fix
}, [container, appearance, novu.options]); + return ( <NovuUI options={options} novu={novu}> <DefaultSlackConnectButton {...defaultProps} /> </NovuUI> );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/components/slack-connect-button/SlackConnectButton.tsx` around lines 20 - 25, The component in SlackConnectButton.tsx should have a blank line before the return statement; update the SlackConnectButton function body so there is an empty line immediately preceding the "return (" that renders <NovuUI options={options} novu={novu}> and <DefaultSlackConnectButton {...defaultProps} /> to satisfy the coding guideline requiring a blank line before every return.playground/nextjs/src/pages/api/trigger-event.ts (1)
68-70: Consider handling non-JSON upstream responses.If the upstream Novu API returns a non-JSON response (e.g., on certain 5xx errors),
upstream.json()will throw. Since this is playground code, this may be acceptable, but adding a try-catch around the JSON parsing would make the error messages clearer.Suggested improvement
- const data = (await upstream.json()) as ResponseData; - - res.status(upstream.status).json(data); + let data: ResponseData; + try { + data = (await upstream.json()) as ResponseData; + } catch { + data = { error: `Upstream returned non-JSON response (status ${upstream.status})` }; + } + + res.status(upstream.status).json(data);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@playground/nextjs/src/pages/api/trigger-event.ts` around lines 68 - 70, The call to upstream.json() can throw for non-JSON responses; wrap the parsing in a try-catch around the line where you call upstream.json() (the upstream variable and ResponseData cast) and handle failures by calling upstream.text() as a fallback, then return a clear error payload via res.status(upstream.status).json(...) (or res.text for plain text) so non-JSON 5xx responses don't crash the handler and provide informative messages.packages/js/src/ui/components/slack-connect-button/SlackConnectButton.tsx (1)
95-109: Consider validatingsubscriberIdinsubscribermode.When
connectionModeissubscriber(the default),resolvedSubscriberIdfalls back tonovuAccessor().subscriberId. If neitherprops.subscriberIdnor the context subscriber ID is set,undefinedis passed toconnect(). While the backend may handle this, an explicit client-side validation (similar to thesharedmode context check) would provide a clearer error message.💡 Optional: Add subscriber mode validation
if (connectionMode === 'shared' && !resolvedContext) { setActionLoading(false); props.onConnectError?.( new Error( 'context is required when connectionMode is "shared". Provide it on SlackConnectButton or NovuProvider.' ) ); return; } + if (connectionMode === 'subscriber' && !resolvedSubscriberId) { + setActionLoading(false); + props.onConnectError?.( + new Error( + 'subscriberId is required when connectionMode is "subscriber". Provide it on SlackConnectButton or NovuProvider.' + ) + ); + + return; + } + const result = await connect({🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/js/src/ui/components/slack-connect-button/SlackConnectButton.tsx` around lines 95 - 109, When connectionMode === 'subscriber' in SlackConnectButton, validate that resolvedSubscriberId is defined before calling connect(): if props.subscriberId and novuAccessor().subscriberId are both missing, call setActionLoading(false) and invoke props.onConnectError with a clear Error (similar to the existing shared-mode check). Update the branch that computes resolvedSubscriberId to perform this validation and early-return so connect() is never called with undefined subscriber id.packages/js/src/ui/components/link-user/LinkUser.tsx (1)
166-173: Hardcoded Slack-specific label in genericLinkUsercomponent.The component is named
LinkUserand accepts generictype/endpointprops, but the button label is hardcoded to "Connect your Slack account". This limits reusability for other channel types.Consider accepting customizable labels via props or renaming the component to be Slack-specific.
♻️ Proposed fix: Add label props
export type LinkUserProps = { integrationIdentifier: string; connectionIdentifier?: string; subscriberId: string; type: string; endpoint: Record<string, string>; endpointIdentifier?: string; context?: Context; contextKeys?: string[]; + connectLabel?: string; + connectedLabel?: string; onLinkSuccess?: (endpoint: { identifier: string }) => void; onLinkError?: (error: unknown) => void; onUnlinkSuccess?: () => void; onUnlinkError?: (error: unknown) => void; };Then in the render:
- {isLinked() ? 'Connected' : 'Connect your Slack account'} + {isLinked() ? (props.connectedLabel ?? 'Connected') : (props.connectLabel ?? 'Connect')}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/js/src/ui/components/link-user/LinkUser.tsx` around lines 166 - 173, The LinkUser component currently hardcodes the button text "Connect your Slack account" inside the JSX that uses isLinked(), which makes the generic LinkUser (props: type/endpoint) non-reusable; update the component to accept two new optional props (e.g., connectLabel and connectedLabel) and use those in place of the hardcoded strings when rendering the span (fall back to sensible defaults such as "Connect your account" and "Connected" if props are not provided). Ensure the JSX using isLinked() and the span with key 'linkUserButtonLabel' references the new props so other channel types can supply custom labels without renaming the component.packages/js/src/ui/components/connect-chat/ConnectChat.tsx (1)
67-74:onConnectSuccessonly fires whenconnectionIdentifierprop is provided.The success callback is conditionally invoked based on whether
connectionIdentifierwas passed as a prop (line 71). If the user doesn't provide this prop,onConnectSuccesswill never be called even though the OAuth URL was successfully generated and opened. This may be intentional but could surprise consumers.Consider either documenting this behavior or always calling the callback with the identifier from the response if available.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/js/src/ui/components/connect-chat/ConnectChat.tsx` around lines 67 - 74, The onConnectSuccess callback is currently only invoked when the prop connectionIdentifier exists, so consumers without that prop never get success events; update the logic in ConnectChat (around the block handling result.data?.url and window.open) to always call props.onConnectSuccess after opening the OAuth URL, passing props.connectionIdentifier if present or falling back to the identifier returned in the response (e.g., result.data.connectionIdentifier) or null/undefined if none — ensure the call occurs unconditionally after window.open and preserves the existing onConnectError behavior.playground/nextjs/src/lib/slack-dm-endpoint-connect.ts (1)
128-148: Hardcodedlimit: 100may miss endpoints/connections if subscriber has more.Both
channelEndpoints.list(line 131) andchannelConnections.list(line 147) use a fixed limit of 100. If a subscriber has more than 100 endpoints or connections, the existing check at line 134-138 or the connection lookup at lines 150-152 could produce false negatives.For playground/demo code this is likely acceptable, but for production use consider pagination or increasing the limit.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@playground/nextjs/src/lib/slack-dm-endpoint-connect.ts` around lines 128 - 148, The code uses a hardcoded limit of 100 when calling novu.channelEndpoints.list and novu.channelConnections.list which can miss results; update the logic in the functions that call channelEndpoints.list and channelConnections.list to page through results (or remove/parameterize the limit) by iterating with the API's pagination tokens until all pages are fetched, then perform the checks against endpoints.result.data and connections data for SLACK_USER_TYPE and slackUserId using the full dataset (keeping subscriberId and integrationIdentifier in each request) so you never get false negatives when a subscriber has >100 items.apps/api/src/app/inbox/inbox.controller.ts (1)
931-941:NotFoundExceptionmay be misleading for a disabled feature.Throwing
NotFoundException('Feature not enabled')when the feature flag is off could confuse consumers - the resource path exists, but the feature is disabled. Consider usingForbiddenExceptionor a custom exception that more accurately conveys "feature disabled" rather than "not found."Proposed fix
+import { ForbiddenException } from '@nestjs/common'; + private async checkChannelFeatureEnabled(organizationId: string): Promise<void> { const isEnabled = await this.featureFlagsService.getFlag({ key: FeatureFlagsKeysEnum.IS_SLACK_TEAMS_ENABLED, defaultValue: false, organization: { _id: organizationId }, }); if (!isEnabled) { - throw new NotFoundException('Feature not enabled'); + throw new ForbiddenException('Channel connections feature is not enabled for this organization'); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/inbox/inbox.controller.ts` around lines 931 - 941, The method checkChannelFeatureEnabled currently throws NotFoundException('Feature not enabled') which is misleading; update it to throw a more appropriate exception (e.g., ForbiddenException or a custom FeatureDisabledException) when the FeatureFlagsKeysEnum.IS_SLACK_TEAMS_ENABLED flag is false. Locate checkChannelFeatureEnabled in inbox.controller.ts, replace the NotFoundException throw with the chosen exception type (importing it from `@nestjs/common` or your exceptions module), and run callers/tests to ensure any consumers expecting NotFoundException are updated to handle the new error semantics.packages/react/src/index.ts (1)
7-7: New public API exports - ensure minor version bump.These additions expand the React package's public API surface with new hooks, components, and types for channel connections/endpoints. Per semver conventions, this requires a minor version bump.
Also applies to: 42-43, 55-56, 62-73, 80-86, 106-106
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/index.ts` at line 7, The new exports (e.g., ChannelConnectButtonIconKey and the other newly added channel connection hooks/components/types) expand the public API and require a semver minor release; update this package's package.json to bump the minor version (or run the equivalent release command such as npm version minor), and add an entry to the changelog/release notes documenting the new public API exports so the version change is tracked.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-chat-oauth-url.command.ts`:
- Line 5: The import for OAuthMode is a regular value import but is only used as
a type, causing a circular dependency; update the import statement that
currently brings in OAuthMode from generate-slack-oauth-url.usecase to be a
type-only import (use "import type" for OAuthMode) so the compiler treats it as
erased at runtime and avoids the circular module issue while keeping the symbol
OAuthMode referenced correctly.
In `@packages/js/src/ui/api/hooks/useChannelConnection.ts`:
- Around line 53-60: The get handlers (currentNovu.on for
'channel-connection.get.pending' and 'channel-connection.get.resolved') are
currently applied globally and must be guarded by the specific
connectionIdentifier this hook instance manages; update both handlers to inspect
the incoming event payload (e.g., the data or identifier field on the event) and
only call setLoading(true), mutate(...), and setLoading(false) when the
payload's identifier matches the hook's connectionIdentifier; locate the
handlers in useChannelConnection.ts (the currentNovu.on callbacks) and add an
identifier check before performing state updates to mirror the filtering used in
the delete handlers.
In `@packages/js/src/ui/components/slack-connect-button/SlackConnectButton.tsx`:
- Around line 50-79: startPolling can create multiple concurrent intervals
because intervalId is scoped locally and onCleanup only clears on unmount;
modify startPolling (and where handleClick calls it) to use a persistent holder
for the interval (e.g., a module/closure-level variable or a ref named
intervalIdRef) so you first clear any existing interval via
clearInterval(intervalIdRef.current) before creating a new setInterval, assign
the new id to intervalIdRef.current, and keep the existing onCleanup to clear
intervalIdRef.current on unmount; ensure all references to intervalId in
startPolling are replaced with the persistent holder so stale intervals are not
left running.
In `@packages/js/src/ui/components/slack-link-user/SlackLinkUser.tsx`:
- Around line 66-78: onMount's returned function is never executed; instead
register the cleanup with onCleanup inside the onMount callback. Inside the
onMount block where you call novuAccessor(), replace the returned cleanup
pattern with onCleanup(() => cleanupDelete()) so the listener created by
currentNovu.on('channel-endpoint.delete.resolved', ...) is removed properly;
reference the local variables cleanupDelete, currentNovu, endpoint, and
setEndpoint to ensure the same listener is cleaned up.
In `@packages/js/src/ui/icons/CheckCircleFill.tsx`:
- Around line 1-4: Rename the file CheckCircleFill.tsx to use
lowercase-with-dashes (e.g., check-circle-fill.tsx) to match repo naming
conventions, and update any imports that reference it; inside the
CheckCircleFill component add a blank line immediately before the return
statement (i.e., ensure there is an empty line between the end of the function
signature/props line and the line starting with return) so the component's
return follows the project's rule of a blank line before every return.
In `@packages/js/src/ui/icons/SlackColored.tsx`:
- Around line 1-4: The file SlackColored.tsx violates naming and return-spacing
conventions: rename the file to slack-colored.tsx (lowercase-with-dashes) and
update the component SlackColored so there is a blank line immediately before
its return statement (i.e., add an empty line above the "return (" inside the
exported SlackColored function that accepts props?:
JSX.HTMLAttributes<SVGSVGElement>) to satisfy the blank-line-before-return
rule).
In `@packages/react/src/hooks/useChannelConnection.ts`:
- Around line 57-90: The effect in useChannelConnection (the useEffect that
registers novu events and calls fetchConnection) doesn't rerun when the consumer
changes identifier, causing stale data; update the dependency array to include
the identifier so the effect re-registers and refetches when it changes — e.g.
add propsRef.current.identifier (or props.identifier if available) to the
dependency list alongside novu and fetchConnection, ensuring the effect (and the
void fetchConnection({ refetch: true })) runs for new identifiers.
In `@packages/react/src/hooks/useChannelConnections.ts`:
- Around line 47-55: The code calls onSuccess/onError twice because both the
direct list response handling (novu.channelConnections.list) and the later event
listeners invoke the same callbacks; pick a single source of truth—either keep
the immediate response logic (setConnections/setError + onSuccess/onError) and
remove callback invocations from the event listeners, or keep the event
listeners as the only callers and remove onSuccess/onError from the immediate
response block. Update the event listener handlers (the functions registered on
novu.channelConnections events) or the list response branch so only one of them
invokes setConnections/setError and calls onSuccess/onError; ensure
setConnections, setError, onSuccess and onError are only used in that single
location to avoid double side-effects.
In `@packages/react/src/hooks/useChannelEndpoint.ts`:
- Around line 57-99: The useEffect that registers Novu event handlers and calls
fetchEndpoint lacks props.identifier in its dependency array, so when
props.identifier changes the effect (and fetchEndpoint) doesn't rerun; update
the dependency array for the effect that contains useEffect(...) to include
propsRef.current.identifier (or props.identifier) alongside novu and
fetchEndpoint so the handlers and the initial void fetchEndpoint({ refetch: true
}) re-run when identifier changes, ensuring fetchEndpoint (and the state set via
setEndpoint/setError) reflects the new identifier; keep referencing the same
cleanup variables cleanupGetPending, cleanupGetResolved, cleanupCreateResolved,
and cleanupDeleteResolved in the return cleanup.
In `@packages/react/src/hooks/useDeleteChannelEndpoint.ts`:
- Around line 26-44: The async remove function should be wrapped in
try/catch/finally so thrown errors don't leave isDeleting stuck true and onError
uninvoked: call novu.channelEndpoints.delete inside a try, setError and invoke
propsRef.current.onError inside catch (passing the caught error as a NovuError),
and always setIsDeleting(false) in finally; keep the existing response-based
success path (propsRef.current.onSuccess) when delete resolves successfully and
ensure the function still returns an appropriate response/error shape.
In `@playground/nextjs/src/pages/connect-chat/index.tsx`:
- Around line 91-94: Update the stale doc text that references removed props by
removing mentions of endpointType and endpointData and adjust the sentence to
reflect current behavior (e.g., say OAuth can create the ChannelEndpoint and
that the Step 2 Link User flow is optional) so there are no references to the
removed props; edit the paragraph containing <code>endpointType</code>,
<code>endpointData</code>, and <code>ChannelEndpoint</code> accordingly in the
connect-chat page component.
---
Outside diff comments:
In
`@apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-slack-oath-url/generate-slack-oauth-url.usecase.ts`:
- Around line 196-204: The buildRedirectUri method currently returns a hardcoded
ngrok URL; update GenerateSlackOauthUrlUsecase.buildRedirectUri (the static
buildRedirectUri method) to construct the redirect URI from
process.env.API_ROOT_URL like the MS Teams handler: keep the existing check that
throws if process.env.API_ROOT_URL is missing, strip any trailing slash from
API_ROOT_URL into baseUrl, and return `${baseUrl}${CHAT_OAUTH_CALLBACK_PATH}`
instead of the hardcoded ngrok value so the OAuth callback uses the
environment-configured root URL.
---
Nitpick comments:
In `@apps/api/src/app/inbox/inbox.controller.ts`:
- Around line 931-941: The method checkChannelFeatureEnabled currently throws
NotFoundException('Feature not enabled') which is misleading; update it to throw
a more appropriate exception (e.g., ForbiddenException or a custom
FeatureDisabledException) when the FeatureFlagsKeysEnum.IS_SLACK_TEAMS_ENABLED
flag is false. Locate checkChannelFeatureEnabled in inbox.controller.ts, replace
the NotFoundException throw with the chosen exception type (importing it from
`@nestjs/common` or your exceptions module), and run callers/tests to ensure any
consumers expecting NotFoundException are updated to handle the new error
semantics.
In `@packages/js/scripts/size-limit.mjs`:
- Line 18: The bundle-budget increase at limitInBytes: 213_000 removes a
guardrail for packages/js and needs an inline rationale; add a short comment
next to the limitInBytes entry (or a linked PR comment) that states the expected
size delta, why 213_000 is chosen as the new baseline (e.g., new deps, minifier
changes, expected gzip/UMD impact), and a reminder to review packages/js for
bundle size, browser compatibility, and public API surface; reference the
limitInBytes symbol and the packages/js bundle in the comment so future
reviewers can trace the decision.
In `@packages/js/src/ui/components/connect-chat/ConnectChat.tsx`:
- Around line 67-74: The onConnectSuccess callback is currently only invoked
when the prop connectionIdentifier exists, so consumers without that prop never
get success events; update the logic in ConnectChat (around the block handling
result.data?.url and window.open) to always call props.onConnectSuccess after
opening the OAuth URL, passing props.connectionIdentifier if present or falling
back to the identifier returned in the response (e.g.,
result.data.connectionIdentifier) or null/undefined if none — ensure the call
occurs unconditionally after window.open and preserves the existing
onConnectError behavior.
In `@packages/js/src/ui/components/link-user/LinkUser.tsx`:
- Around line 166-173: The LinkUser component currently hardcodes the button
text "Connect your Slack account" inside the JSX that uses isLinked(), which
makes the generic LinkUser (props: type/endpoint) non-reusable; update the
component to accept two new optional props (e.g., connectLabel and
connectedLabel) and use those in place of the hardcoded strings when rendering
the span (fall back to sensible defaults such as "Connect your account" and
"Connected" if props are not provided). Ensure the JSX using isLinked() and the
span with key 'linkUserButtonLabel' references the new props so other channel
types can supply custom labels without renaming the component.
In `@packages/js/src/ui/components/Renderer.tsx`:
- Around line 158-180: ChannelComponentsRenderer duplicates
SubscriptionComponentsRenderer; extract a shared SimpleComponentsRenderer that
accepts props { elements: MountableElement[]; nodes: Map<MountableElement,
NovuComponent> } and contains the current JSX logic (Show/For mapping to
novuComponents lookup, Portal mount, Root and <Component
{...novuComponent().props} />). Replace both ChannelComponentsRenderer and
SubscriptionComponentsRenderer usages with SimpleComponentsRenderer (pass
subscriptionComponents() and channelComponents() as elements and props.nodes as
nodes) and remove the duplicated component definitions; keep the same internal
symbols novuComponent, Component, Portal, and Root to avoid changing behavior.
In `@packages/js/src/ui/components/slack-connect-button/SlackConnectButton.tsx`:
- Around line 95-109: When connectionMode === 'subscriber' in
SlackConnectButton, validate that resolvedSubscriberId is defined before calling
connect(): if props.subscriberId and novuAccessor().subscriberId are both
missing, call setActionLoading(false) and invoke props.onConnectError with a
clear Error (similar to the existing shared-mode check). Update the branch that
computes resolvedSubscriberId to perform this validation and early-return so
connect() is never called with undefined subscriber id.
In `@packages/react/src/components/connect-chat/ConnectChat.tsx`:
- Around line 30-32: The ConnectChat component lacks a blank line before its
return; update the React.memo wrapper function (ConnectChat) to insert a single
empty line immediately before the "return <ConnectChatInternal {...props} />;"
statement so the function body follows the project's guideline (locate the
ConnectChat arrow function that returns ConnectChatInternal).
- Around line 20-25: The ConnectChat component's render/functional body should
have a blank line immediately before the return statement; update the
ConnectChat function (where NovuUI and DefaultConnectChat are returned) to
insert a single empty line above the "return (" line so the return is separated
from preceding logic per the coding guideline.
In `@packages/react/src/components/slack-connect-button/SlackConnectButton.tsx`:
- Around line 30-32: In the SlackConnectButton React.memo wrapper, add a blank
line immediately before the return statement so the function body contains a
separating empty line prior to "return <SlackConnectButtonInternal {...props}
/>;"; update the SlackConnectButton declaration (which delegates to
SlackConnectButtonInternal) to include that blank line to comply with the coding
guideline.
- Around line 20-25: The component in SlackConnectButton.tsx should have a blank
line before the return statement; update the SlackConnectButton function body so
there is an empty line immediately preceding the "return (" that renders <NovuUI
options={options} novu={novu}> and <DefaultSlackConnectButton {...defaultProps}
/> to satisfy the coding guideline requiring a blank line before every return.
In `@packages/react/src/index.ts`:
- Line 7: The new exports (e.g., ChannelConnectButtonIconKey and the other newly
added channel connection hooks/components/types) expand the public API and
require a semver minor release; update this package's package.json to bump the
minor version (or run the equivalent release command such as npm version minor),
and add an entry to the changelog/release notes documenting the new public API
exports so the version change is tracked.
In `@playground/nextjs/src/lib/slack-dm-endpoint-connect.ts`:
- Around line 128-148: The code uses a hardcoded limit of 100 when calling
novu.channelEndpoints.list and novu.channelConnections.list which can miss
results; update the logic in the functions that call channelEndpoints.list and
channelConnections.list to page through results (or remove/parameterize the
limit) by iterating with the API's pagination tokens until all pages are
fetched, then perform the checks against endpoints.result.data and connections
data for SLACK_USER_TYPE and slackUserId using the full dataset (keeping
subscriberId and integrationIdentifier in each request) so you never get false
negatives when a subscriber has >100 items.
In `@playground/nextjs/src/pages/api/trigger-event.ts`:
- Around line 68-70: The call to upstream.json() can throw for non-JSON
responses; wrap the parsing in a try-catch around the line where you call
upstream.json() (the upstream variable and ResponseData cast) and handle
failures by calling upstream.text() as a fallback, then return a clear error
payload via res.status(upstream.status).json(...) (or res.text for plain text)
so non-JSON 5xx responses don't crash the handler and provide informative
messages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 35724a91-00a2-4820-9de6-bfe6ec38e21b
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (67)
apps/api/src/app/channel-connections/channel-connections.controller.tsapps/api/src/app/channel-connections/dtos/create-channel-connection-request.dto.tsapps/api/src/app/channel-connections/usecases/create-channel-connection/create-channel-connection.command.tsapps/api/src/app/channel-connections/usecases/create-channel-connection/create-channel-connection.usecase.tsapps/api/src/app/inbox/inbox.controller.tsapps/api/src/app/inbox/inbox.module.tsapps/api/src/app/integrations/dtos/generate-chat-oauth-url.dto.tsapps/api/src/app/integrations/integrations.controller.tsapps/api/src/app/integrations/usecases/chat-oauth-callback/slack-oauth-callback/slack-oauth-callback.usecase.tsapps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-chat-oauth-url.command.tsapps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-chat-oauth-url.usecase.tsapps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-slack-oath-url/generate-slack-oauth-url.command.tsapps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-slack-oath-url/generate-slack-oauth-url.usecase.tspackages/js/scripts/size-limit.mjspackages/js/src/api/inbox-service.tspackages/js/src/channel-connections/channel-connections.tspackages/js/src/channel-connections/helpers.tspackages/js/src/channel-connections/index.tspackages/js/src/channel-connections/types.tspackages/js/src/channel-endpoints/channel-endpoints.tspackages/js/src/channel-endpoints/helpers.tspackages/js/src/channel-endpoints/index.tspackages/js/src/channel-endpoints/types.tspackages/js/src/event-emitter/types.tspackages/js/src/index.tspackages/js/src/novu.tspackages/js/src/ui/api/hooks/useChannelConnection.tspackages/js/src/ui/api/hooks/useChannelEndpoint.tspackages/js/src/ui/components/Renderer.tsxpackages/js/src/ui/components/connect-chat/ConnectChat.tsxpackages/js/src/ui/components/index.tspackages/js/src/ui/components/link-user/LinkUser.tsxpackages/js/src/ui/components/slack-connect-button/SlackConnectButton.tsxpackages/js/src/ui/components/slack-constants.tspackages/js/src/ui/components/slack-link-user/SlackLinkUser.tsxpackages/js/src/ui/config/appearanceKeys.tspackages/js/src/ui/icons/CheckCircleFill.tsxpackages/js/src/ui/icons/SlackColored.tsxpackages/js/src/ui/icons/index.tspackages/js/src/ui/index.tspackages/js/src/ui/types.tspackages/nextjs/src/app-router/index.tspackages/nextjs/src/pages-router/index.tspackages/react/src/components/NovuUI.tsxpackages/react/src/components/connect-chat/ConnectChat.tsxpackages/react/src/components/connect-chat/DefaultConnectChat.tsxpackages/react/src/components/index.tspackages/react/src/components/slack-connect-button/DefaultSlackConnectButton.tsxpackages/react/src/components/slack-connect-button/SlackConnectButton.tsxpackages/react/src/components/slack-link-user/DefaultSlackLinkUser.tsxpackages/react/src/components/slack-link-user/SlackLinkUser.tsxpackages/react/src/hooks/index.tspackages/react/src/hooks/useChannelConnection.tspackages/react/src/hooks/useChannelConnections.tspackages/react/src/hooks/useChannelEndpoint.tspackages/react/src/hooks/useCreateChannelEndpoint.tspackages/react/src/hooks/useDeleteChannelEndpoint.tspackages/react/src/index.tspackages/react/src/utils/appearance.tspackages/shared/src/types/channel-connection.tsplayground/nextjs/.env.exampleplayground/nextjs/package.jsonplayground/nextjs/src/components/SideNav.tsxplayground/nextjs/src/lib/slack-dm-endpoint-connect.tsplayground/nextjs/src/pages/api/slack-dm-endpoint.tsplayground/nextjs/src/pages/api/trigger-event.tsplayground/nextjs/src/pages/connect-chat/index.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-slack-oath-url/generate-slack-oauth-url.usecase.ts (1)
63-95:⚠️ Potential issue | 🟠 MajorMissing validation for
link_usermode requiringsubscriberId.The callback handler (
linkUserEndpointinslack-oauth-callback.usecase.ts) explicitly requiressubscriberIdforlink_usermode and throwsBadRequestException('subscriberId is required for link_user mode'). However, this validation isn't enforced during URL generation.This creates poor UX: users can generate an OAuth URL for
link_usermode withoutsubscriberId, complete the OAuth flow, and then receive an error at the callback.🔧 Proposed fix: Add link_user mode validation
private validateSubscriberIdOrContext(command: GenerateSlackOauthUrlCommand): void { - const { subscriberId, context, scope, connectionMode } = command; + const { subscriberId, context, scope, connectionMode, mode } = command; + + if (mode === 'link_user' && !subscriberId) { + throw new BadRequestException('subscriberId is required for link_user mode'); + } if (scope?.includes('incoming-webhook')) { if (!subscriberId) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-slack-oath-url/generate-slack-oauth-url.usecase.ts` around lines 63 - 95, The validateSubscriberIdOrContext function must also enforce that when command.mode === 'link_user' a subscriberId is present; update validateSubscriberIdOrContext (used by GenerateSlackOauthUrlCommand handling) to throw BadRequestException('subscriberId is required for link_user mode') if mode === 'link_user' and subscriberId is missing (ensure this check occurs alongside the existing connectionMode and scope checks so URL generation rejects invalid link_user requests before redirecting to linkUserEndpoint).
🧹 Nitpick comments (1)
packages/js/src/ui/components/slack-link-user/SlackLinkUser.tsx (1)
113-152: Consider adding blank lines before earlyreturnstatements.Per coding guidelines, include a blank line before every
returnstatement. The early returns at lines 97 and 144 lack preceding blank lines.♻️ Suggested formatting
if (result.error) { setActionLoading(false); props.onLinkError?.(result.error); + return; }And in startPolling (line 97):
props.onLinkSuccess?.({ identifier: found.identifier }); + return;As per coding guidelines: "Include a blank line before every
returnstatement."🤖 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 113 - 152, The handleClick function has early return statements without a preceding blank line; add a blank line before the `return` inside the identifier check (the `if (!identifier) return;` in the isLinked branch) and add a blank line before the `return` after handling `result.error` in the OAuth URL branch (the `if (result.error) { ... return; }` block) so they follow the project's "blank line before return" formatting rule; adjust only those spots in SlackLinkUser.tsx (inside handleClick) — no logic changes, just insert the blank lines.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-slack-oath-url/generate-slack-oauth-url.usecase.ts`:
- Around line 66-82: The current validation misses an explicit check for the
incompatible combination where scope includes 'incoming-webhook' (which requires
subscriberId) while connectionMode === 'shared' (which forbids subscriberId);
inside the same validation block around the existing checks for scope,
connectionMode, subscriberId, and context (in the generateSlackOAuthUrl use
case), add a guard that detects connectionMode === 'shared' &&
scope?.includes('incoming-webhook') and throw a BadRequestException with a clear
message stating that 'incoming-webhook' cannot be used with shared
connectionMode because it requires a subscriberId.
In `@packages/js/src/ui/components/slack-link-user/SlackLinkUser.tsx`:
- Around line 80-111: startPolling currently creates an intervalId inside the
event handler so the onCleanup call inside startPolling never registers; move
interval tracking to component scope by declaring a component-level variable
(e.g., pollingIntervalId) and assign the setInterval result to it inside
startPolling, replace local clearInterval(intervalId) calls with
clearInterval(pollingIntervalId), and register a proper component cleanup using
onCleanup or onMount at the top-level (not inside startPolling) to clear
pollingIntervalId when the component unmounts; update references in
startPolling, handleClick, and any timeout/found handling to use the new
pollingIntervalId variable so the interval is always cleared.
---
Outside diff comments:
In
`@apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-slack-oath-url/generate-slack-oauth-url.usecase.ts`:
- Around line 63-95: The validateSubscriberIdOrContext function must also
enforce that when command.mode === 'link_user' a subscriberId is present; update
validateSubscriberIdOrContext (used by GenerateSlackOauthUrlCommand handling) to
throw BadRequestException('subscriberId is required for link_user mode') if mode
=== 'link_user' and subscriberId is missing (ensure this check occurs alongside
the existing connectionMode and scope checks so URL generation rejects invalid
link_user requests before redirecting to linkUserEndpoint).
---
Nitpick comments:
In `@packages/js/src/ui/components/slack-link-user/SlackLinkUser.tsx`:
- Around line 113-152: The handleClick function has early return statements
without a preceding blank line; add a blank line before the `return` inside the
identifier check (the `if (!identifier) return;` in the isLinked branch) and add
a blank line before the `return` after handling `result.error` in the OAuth URL
branch (the `if (result.error) { ... return; }` block) so they follow the
project's "blank line before return" formatting rule; adjust only those spots in
SlackLinkUser.tsx (inside handleClick) — no logic changes, just insert the blank
lines.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 424cbe27-8d79-4967-9285-150d717c374b
📒 Files selected for processing (3)
apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-slack-oath-url/generate-slack-oauth-url.usecase.tspackages/js/src/ui/components/link-user/LinkUser.tsxpackages/js/src/ui/components/slack-link-user/SlackLinkUser.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/js/src/ui/components/link-user/LinkUser.tsx
djabarovgeorge
changed the title
…omponents - Updated type imports in `generate-chat-oauth-url.command.ts` for better clarity. - Improved type definitions in `useChannelConnection`, `LinkUser`, and `SlackLinkUser` components to ensure better type safety. - Refactored polling logic in `SlackConnectButton` and `SlackLinkUser` to utilize cleanup functions effectively, preventing potential memory leaks. - Adjusted `useChannelConnections` and `useChannelEndpoint` hooks to include `props.identifier` in dependencies for accurate re-fetching behavior.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
packages/js/src/ui/components/slack-connect-button/SlackConnectButton.tsx (3)
39-43: Hook receives static values instead of reactive accessors.
useChannelConnectionis called withintegrationIdentifier()andconnectionIdentifier()evaluated immediately. Ifprops.integrationIdentifierorprops.connectionIdentifierchange after mount, the hook won't react to those changes. If this is intentional (identifiers are stable), consider adding a comment. Otherwise, the hook should accept accessor functions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/js/src/ui/components/slack-connect-button/SlackConnectButton.tsx` around lines 39 - 43, The hook call currently invokes integrationIdentifier() and connectionIdentifier() immediately so it won't respond to prop changes; update the call to pass accessor functions instead (e.g. integrationIdentifier: () => props.integrationIdentifier and connectionIdentifier: () => props.connectionIdentifier) so useChannelConnection receives reactive accessors, or if those identifiers are guaranteed stable, add a clarifying comment next to the useChannelConnection invocation; reference the useChannelConnection call in SlackConnectButton and the props integrationIdentifier/connectionIdentifier accessors when making the change.
101-151: Missing blank lines beforereturnstatements.Per coding guidelines, include a blank line before every
returnstatement. This applies to lines 104, 128, and 144.♻️ Proposed fix
const identifier = connection()?.identifier; - if (!identifier) return; + if (!identifier) { + return; + } const result = await disconnect(identifier);props.onConnectError?.( new Error( 'context is required when connectionMode is "shared". Provide it on SlackConnectButton or NovuProvider.' ) ); - - return; + + return; }For guard clauses like line 104, consider using a block with the return on its own line, or document if early returns in guards are exempt from this rule.
As per coding guidelines: "Include a blank line before every
returnstatement".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/js/src/ui/components/slack-connect-button/SlackConnectButton.tsx` around lines 101 - 151, In handleClick, add a blank line immediately before each early return to satisfy the style rule: insert a blank line before the guard return after if (!identifier) (inside the isConnected/disconnect branch), before the return inside the connectionMode === 'shared' context-error block, and before the return when connect returns result.error; locate these in the handleClick function around calls to isConnected(), disconnect(), and connect() and ensure the blank line is present just above each return statement.
83-83: Missing blank line beforereturnin polling callback.Line 83 has a
returnstatement without a preceding blank line.♻️ Proposed fix
setActionLoading(false); mutate(response.data); props.onConnectSuccess?.(connectionIdentifier()); + return;As per coding guidelines: "Include a blank line before every
returnstatement".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/js/src/ui/components/slack-connect-button/SlackConnectButton.tsx` at line 83, In the SlackConnectButton component's polling callback (the function used for polling inside SlackConnectButton), add a single blank line immediately before the standalone `return;` so it follows the project's style rule of having a blank line before every return; locate the callback passed to the polling mechanism (e.g., the function inside the useEffect or setInterval in SlackConnectButton) and insert the blank line directly above the `return;`.packages/react/src/hooks/useChannelConnection.ts (2)
79-80: Add a blank line beforereturn.This file misses the required blank line before the early return in the delete handler.
As per coding guidelines:
**/*.{ts,tsx,js,jsx}: Include a blank line before everyreturnstatement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/hooks/useChannelConnection.ts` around lines 79 - 80, In the delete handler inside useChannelConnection (in useChannelConnection.ts) add a blank line immediately before the early return so the block reading "if (!args || args.identifier !== propsRef.current.identifier) { return; }" becomes separated with a blank line before "return"; locate the condition that references args and propsRef.current.identifier and insert one blank line before the return statement to satisfy the repository rule requiring a blank line before every return.
1-1: Rename file to lowercase-with-dashes for guideline compliance.
packages/react/src/hooks/useChannelConnection.tsviolates the repository filename convention.As per coding guidelines:
**/*: File/directory names should use lowercase with dashes (e.g.,components/auth-wizard).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/react/src/hooks/useChannelConnection.ts` at line 1, Rename the file from useChannelConnection.ts to a lowercase-with-dashes name (e.g., use-channel-connection.ts) to match repository conventions, then update all imports/exports and any barrel/index re-exports that reference useChannelConnection to point to the new filename; ensure the exported symbol (useChannelConnection) and the types ChannelConnectionResponse and NovuError remain unchanged so internal references still resolve.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/react/src/hooks/useChannelConnection.ts`:
- Around line 62-76: The get.pending and get.resolved handlers in
useChannelConnection.ts are not scoping events by identifier, so multiple hook
instances can clobber state; update the callbacks for
'channel-connection.get.pending' and 'channel-connection.get.resolved' to first
check the emitted payload's args.identifier against the current hook identifier
(e.g., propsRef.current.identifier) and return early if they don't match, then
only call setIsFetching, setConnection, setError and invoke onSuccess/onError
when identifiers match (mirror the identifier-check pattern used in the existing
'channel-connection.delete.resolved' handler).
- Around line 42-53: The channel-connection.get event listeners in
useChannelConnection (listeners for "channel-connection.get.pending" and
"channel-connection.get.resolved") must check the incoming event identifier and
bail out if it does not match the current identifier to avoid cross-instance
state pollution—add the same guard used in "channel-connection.delete.resolved"
to both get listeners so they only call
setIsFetching/setIsLoading/setConnection/setError/onSuccess/onError when
event.payload.identifier === identifier; rename the file from
useChannelConnection.ts to use-channel-connection.ts to match kebab-case
convention; and insert a blank line immediately before the hook’s return
statement.
In `@packages/react/src/hooks/useChannelConnections.ts`:
- Line 1: Rename the file useChannelConnections.ts to kebab-case
(use-channel-connections.ts) and update any imports that reference it (search
for useChannelConnections in the repo); ensure exported symbol names inside the
module (e.g., the default export or hook name useChannelConnections) remain
unchanged and only the filename/import paths are updated so TypeScript imports
still resolve correctly.
- Around line 47-58: Wrap the call to novu.channelConnections.list inside a
try/catch/finally in the useChannelConnections hook: call listArgs with await
inside try, setConnections and call onSuccess in the try when response.data
exists, handle thrown errors in catch by calling setError and onError with the
caught exception (converted to NovuError), and ensure setIsLoading(false) and
setIsFetching(false) are always executed in finally so loading flags never get
stuck; update the code paths that currently check response.error/response.data
to operate inside the try/catch flow and reference the existing symbols
(novu.channelConnections.list, setConnections, setError, onError, onSuccess,
setIsLoading, setIsFetching).
In `@packages/react/src/hooks/useDeleteChannelEndpoint.ts`:
- Around line 1-60: Rename the module file from useDeleteChannelEndpoint.ts to
kebab-case (use-delete-channel-endpoint.ts) and update all imports that
reference the symbol useDeleteChannelEndpoint to point to the new filename;
ensure the exported hook name useDeleteChannelEndpoint remains unchanged, update
any barrel/index re-exports or story/test imports that reference the old
filename, and run a global search for "useDeleteChannelEndpoint" and the old
filename to replace occurrences so builds and type imports continue to resolve.
---
Nitpick comments:
In `@packages/js/src/ui/components/slack-connect-button/SlackConnectButton.tsx`:
- Around line 39-43: The hook call currently invokes integrationIdentifier() and
connectionIdentifier() immediately so it won't respond to prop changes; update
the call to pass accessor functions instead (e.g. integrationIdentifier: () =>
props.integrationIdentifier and connectionIdentifier: () =>
props.connectionIdentifier) so useChannelConnection receives reactive accessors,
or if those identifiers are guaranteed stable, add a clarifying comment next to
the useChannelConnection invocation; reference the useChannelConnection call in
SlackConnectButton and the props integrationIdentifier/connectionIdentifier
accessors when making the change.
- Around line 101-151: In handleClick, add a blank line immediately before each
early return to satisfy the style rule: insert a blank line before the guard
return after if (!identifier) (inside the isConnected/disconnect branch), before
the return inside the connectionMode === 'shared' context-error block, and
before the return when connect returns result.error; locate these in the
handleClick function around calls to isConnected(), disconnect(), and connect()
and ensure the blank line is present just above each return statement.
- Line 83: In the SlackConnectButton component's polling callback (the function
used for polling inside SlackConnectButton), add a single blank line immediately
before the standalone `return;` so it follows the project's style rule of having
a blank line before every return; locate the callback passed to the polling
mechanism (e.g., the function inside the useEffect or setInterval in
SlackConnectButton) and insert the blank line directly above the `return;`.
In `@packages/react/src/hooks/useChannelConnection.ts`:
- Around line 79-80: In the delete handler inside useChannelConnection (in
useChannelConnection.ts) add a blank line immediately before the early return so
the block reading "if (!args || args.identifier !== propsRef.current.identifier)
{ return; }" becomes separated with a blank line before "return"; locate the
condition that references args and propsRef.current.identifier and insert one
blank line before the return statement to satisfy the repository rule requiring
a blank line before every return.
- Line 1: Rename the file from useChannelConnection.ts to a
lowercase-with-dashes name (e.g., use-channel-connection.ts) to match repository
conventions, then update all imports/exports and any barrel/index re-exports
that reference useChannelConnection to point to the new filename; ensure the
exported symbol (useChannelConnection) and the types ChannelConnectionResponse
and NovuError remain unchanged so internal references still resolve.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 318c5931-a32a-438a-a8d5-c1450bc14909
📒 Files selected for processing (10)
apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-chat-oauth-url.command.tspackages/js/src/ui/api/hooks/useChannelConnection.tspackages/js/src/ui/components/link-user/LinkUser.tsxpackages/js/src/ui/components/slack-connect-button/SlackConnectButton.tsxpackages/js/src/ui/components/slack-link-user/SlackLinkUser.tsxpackages/react/src/hooks/useChannelConnection.tspackages/react/src/hooks/useChannelConnections.tspackages/react/src/hooks/useChannelEndpoint.tspackages/react/src/hooks/useDeleteChannelEndpoint.tsplayground/nextjs/src/pages/connect-chat/index.tsx
✅ Files skipped from review due to trivial changes (1)
- playground/nextjs/src/pages/connect-chat/index.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-chat-oauth-url.command.ts
- packages/js/src/ui/components/link-user/LinkUser.tsx
- packages/react/src/hooks/useChannelEndpoint.ts
- packages/js/src/ui/api/hooks/useChannelConnection.ts
- packages/js/src/ui/components/slack-link-user/SlackLinkUser.tsx
…related use cases - Added a new utility function `validateConnectionMode` to enforce scoping rules for connection modes in channel connections. - Refactored `CreateChannelConnection` and `GenerateSlackOauthUrl` use cases to utilize the new validation function, improving code maintainability and reducing duplication. - Enhanced error handling for connection mode requirements, ensuring proper validation of `subscriberId` and `context` based on the selected mode. - Updated `InboxService` to streamline query parameter construction for channel connections and endpoints.
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/js/src/ui/components/Renderer.tsx (1)
134-139:⚠️ Potential issue | 🟡 MinorAdd a blank line before
returninSimpleComponentsRenderer.The new renderer misses the required blank line before its top-level
return.Suggested fix
const SimpleComponentsRenderer = (props: { elements: MountableElement[]; nodes: Map<MountableElement, NovuComponent>; }) => { + return ( <Show when={props.elements.length > 0}>As per coding guidelines
**/*.{ts,tsx,js,jsx}: Include a blank line before everyreturnstatement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/js/src/ui/components/Renderer.tsx` around lines 134 - 139, The SimpleComponentsRenderer function is missing the required blank line before its top-level return; insert a single blank line immediately before the line starting with "return (" inside the SimpleComponentsRenderer component (the function that accepts props: { elements: MountableElement[]; nodes: Map<MountableElement, NovuComponent>; }) to comply with the repo rule that every return in .ts/.tsx/.js/.jsx files must be preceded by a blank line.
🧹 Nitpick comments (2)
packages/js/src/ui/components/Renderer.tsx (1)
26-30: Use lowercase-with-dashes filenames in component paths.These imports reference PascalCase filenames (
ConnectChat,SlackConnectButton,SlackLinkUser), which conflicts with the repo naming rule.As per coding guidelines
**/*: File/directory names should use lowercase with dashes (e.g.,components/auth-wizard).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/js/src/ui/components/Renderer.tsx` around lines 26 - 30, The imports in Renderer.tsx use PascalCase module paths (ConnectChat, SlackConnectButton, SlackLinkUser) which violates the repo's lowercase-with-dashes naming rule; update the import paths to the dashed-lowercase equivalents (e.g., './connect-chat/ConnectChat' -> './connect-chat/connect-chat', './slack-connect-button/SlackConnectButton' -> './slack-connect-button/slack-connect-button', './slack-link-user/SlackLinkUser' -> './slack-link-user/slack-link-user') and ensure the corresponding files/exports match those filenames (or adjust exports to default named exports) so the module resolution for ConnectChat, SlackConnectButton, and SlackLinkUser continues to work.packages/js/src/ui/types.ts (1)
375-388: Flatten this conditional type before it grows again.This works, but every new appearance group now adds another branch. A keyed lookup type is easier to extend and avoids the nested ternary pattern.
♻️ Proposed refactor
+type AppearanceCallbackFunctionByKey = { + [K in InboxAppearanceCallbackKeys]: InboxAppearanceCallbackFunction<K>; +} & { + [K in SubscriptionAppearanceCallbackKeys]: SubscriptionAppearanceCallbackFunction<K>; +} & { + [K in SlackLinkUserAppearanceCallbackKeys]: SlackLinkUserAppearanceCallbackFunction<K>; +} & { + [K in ChannelConnectButtonAppearanceCallbackKeys]: ChannelConnectButtonAppearanceCallbackFunction<K>; +}; + export type AllAppearanceCallbackKeys = | InboxAppearanceCallbackKeys | SubscriptionAppearanceCallbackKeys | SlackLinkUserAppearanceCallbackKeys | ChannelConnectButtonAppearanceCallbackKeys; -export type AllAppearanceCallbackFunction<K extends AllAppearanceCallbackKeys> = K extends InboxAppearanceCallbackKeys - ? InboxAppearanceCallbackFunction<K> - : K extends SubscriptionAppearanceCallbackKeys - ? SubscriptionAppearanceCallbackFunction<K> - : K extends SlackLinkUserAppearanceCallbackKeys - ? SlackLinkUserAppearanceCallbackFunction<K> - : K extends ChannelConnectButtonAppearanceCallbackKeys - ? ChannelConnectButtonAppearanceCallbackFunction<K> - : never; +export type AllAppearanceCallbackFunction<K extends AllAppearanceCallbackKeys> = + AppearanceCallbackFunctionByKey[K];As per coding guidelines,
**/*.{ts,tsx,js,jsx}: Do not use nested ternaries.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/js/src/ui/types.ts` around lines 375 - 388, The nested conditional type AllAppearanceCallbackFunction is brittle; replace it with a keyed lookup map to avoid nested ternaries: create a mapping type (e.g., AppearanceCallbackMap) that maps each key union member (InboxAppearanceCallbackKeys, SubscriptionAppearanceCallbackKeys, SlackLinkUserAppearanceCallbackKeys, ChannelConnectButtonAppearanceCallbackKeys) to its corresponding function type (InboxAppearanceCallbackFunction, SubscriptionAppearanceCallbackFunction, SlackLinkUserAppearanceCallbackFunction, ChannelConnectButtonAppearanceCallbackFunction) and then redefine AllAppearanceCallbackFunction<K extends AllAppearanceCallbackKeys> = AppearanceCallbackMap[K]; update any references to the old conditional type to use the new lookup type so adding new appearance groups only requires adding an entry to AppearanceCallbackMap.
🤖 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/Renderer.tsx`:
- Around line 134-139: The SimpleComponentsRenderer function is missing the
required blank line before its top-level return; insert a single blank line
immediately before the line starting with "return (" inside the
SimpleComponentsRenderer component (the function that accepts props: { elements:
MountableElement[]; nodes: Map<MountableElement, NovuComponent>; }) to comply
with the repo rule that every return in .ts/.tsx/.js/.jsx files must be preceded
by a blank line.
---
Nitpick comments:
In `@packages/js/src/ui/components/Renderer.tsx`:
- Around line 26-30: The imports in Renderer.tsx use PascalCase module paths
(ConnectChat, SlackConnectButton, SlackLinkUser) which violates the repo's
lowercase-with-dashes naming rule; update the import paths to the
dashed-lowercase equivalents (e.g., './connect-chat/ConnectChat' ->
'./connect-chat/connect-chat', './slack-connect-button/SlackConnectButton' ->
'./slack-connect-button/slack-connect-button', './slack-link-user/SlackLinkUser'
-> './slack-link-user/slack-link-user') and ensure the corresponding
files/exports match those filenames (or adjust exports to default named exports)
so the module resolution for ConnectChat, SlackConnectButton, and SlackLinkUser
continues to work.
In `@packages/js/src/ui/types.ts`:
- Around line 375-388: The nested conditional type AllAppearanceCallbackFunction
is brittle; replace it with a keyed lookup map to avoid nested ternaries: create
a mapping type (e.g., AppearanceCallbackMap) that maps each key union member
(InboxAppearanceCallbackKeys, SubscriptionAppearanceCallbackKeys,
SlackLinkUserAppearanceCallbackKeys, ChannelConnectButtonAppearanceCallbackKeys)
to its corresponding function type (InboxAppearanceCallbackFunction,
SubscriptionAppearanceCallbackFunction, SlackLinkUserAppearanceCallbackFunction,
ChannelConnectButtonAppearanceCallbackFunction) and then redefine
AllAppearanceCallbackFunction<K extends AllAppearanceCallbackKeys> =
AppearanceCallbackMap[K]; update any references to the old conditional type to
use the new lookup type so adding new appearance groups only requires adding an
entry to AppearanceCallbackMap.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: da8a4913-2c60-4329-8307-206a0e0379d5
📒 Files selected for processing (10)
apps/api/src/app/channel-connections/usecases/channel-connection.utils.tsapps/api/src/app/channel-connections/usecases/create-channel-connection/create-channel-connection.usecase.tsapps/api/src/app/integrations/usecases/generate-chat-oath-url/generate-slack-oath-url/generate-slack-oauth-url.usecase.tspackages/js/src/api/inbox-service.tspackages/js/src/channel-endpoints/channel-endpoints.tspackages/js/src/channel-endpoints/helpers.tspackages/js/src/channel-endpoints/index.tspackages/js/src/ui/components/Renderer.tsxpackages/js/src/ui/config/appearanceKeys.tspackages/js/src/ui/types.ts
✅ Files skipped from review due to trivial changes (2)
- packages/js/src/channel-endpoints/channel-endpoints.ts
- packages/js/src/api/inbox-service.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/api/src/app/channel-connections/usecases/create-channel-connection/create-channel-connection.usecase.ts
- packages/js/src/channel-endpoints/helpers.ts
- packages/js/src/channel-endpoints/index.ts
- packages/js/src/ui/config/appearanceKeys.ts
This was referenced Apr 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed? Why was the change needed?
Screenshots
Expand for optional sections
Related enterprise PR
Special notes for your reviewer
Note
High Risk
Adds new subscriber-facing APIs and modifies Slack OAuth state/callback handling, including new validation paths and endpoint creation. Risk is elevated because it touches auth-guarded inbox routes, OAuth flows, and currently hardcodes the Slack OAuth redirect URL to an ngrok domain.
Overview
Enables subscriber-scoped chat setup via the
inboxsurface by adding guarded endpoints to list/get/create/deletechannel-connections, list/get/create/deletechannel-endpoints, and generate chat OAuth URLs (all gated behindIS_SLACK_TEAMS_ENABLED).Extends channel connection creation to accept
connectionMode(subscribervsshared) with stricter validation, and updates Slack OAuth generation/callback to support a newmode(connectvslink_user) plususerScope; the callback can now create aslack_userendpoint fromauthed_user.idand supports shared connections by omittingsubscriberId.Updates the JS + React/Next.js SDKs to expose new
channelConnections/channelEndpointsmodules, events, hooks, and UI components (ConnectChat,SlackConnectButton,SlackLinkUser), and adds a playground demo + server-side helpers for Slack DM endpoint creation and event triggering.Reviewed by Cursor Bugbot for commit 56291a3. Configure here.