-
-
Notifications
You must be signed in to change notification settings - Fork 312
feat: add subscription and org details to Chatwoot support widget #3277
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughReplaces HelpMenu's inline Chatwoot bootstrap/toggle logic with a new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant HelpMenu
participant Hook as useChatwoot
participant Chatwoot as window.$chatwoot
User->>HelpMenu: Click "Open chat"
HelpMenu->>Hook: call openChatwoot()
Note over Hook,Chatwoot: Hook ensures SDK loaded (shared promise)
Hook->>Chatwoot: init/configure (user/org attributes)
Hook->>Chatwoot: show/open widget
Chatwoot-->>User: chat UI appears
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
webapp/src/component/HelpMenu.tsx(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (24)
- GitHub Check: E2E testing 🔎 (15, 12)
- GitHub Check: E2E testing 🔎 (15, 14)
- GitHub Check: E2E testing 🔎 (15, 9)
- GitHub Check: E2E testing 🔎 (15, 13)
- GitHub Check: E2E testing 🔎 (15, 11)
- GitHub Check: E2E testing 🔎 (15, 7)
- GitHub Check: E2E testing 🔎 (15, 10)
- GitHub Check: E2E testing 🔎 (15, 4)
- GitHub Check: E2E testing 🔎 (15, 6)
- GitHub Check: E2E testing 🔎 (15, 8)
- GitHub Check: E2E testing 🔎 (15, 3)
- GitHub Check: E2E testing 🔎 (15, 5)
- GitHub Check: E2E testing 🔎 (15, 0)
- GitHub Check: E2E testing 🔎 (15, 2)
- GitHub Check: E2E testing 🔎 (15, 1)
- GitHub Check: BT 🔎 (server-app:runWithoutEeTests)
- GitHub Check: BT 🔎 (data:test)
- GitHub Check: BT 🔎 (ktlint:test)
- GitHub Check: BT 🔎 (security:test)
- GitHub Check: BT 🔎 (server-app:runContextRecreatingTests)
- GitHub Check: BT 🔎 (server-app:runWebsocketTests)
- GitHub Check: BT 🔎 (server-app:runStandardTests)
- GitHub Check: BT 🔎 (ee-test:test)
- GitHub Check: Frontend static check 🪲
🔇 Additional comments (1)
webapp/src/component/HelpMenu.tsx (1)
129-139: Implementation looks correct with appropriate fallbacks.The custom attributes are properly set with defensive fallbacks:
- Optional chaining prevents errors if
activeCloudSubscriptionis undefined- Fallback values ('free', 'inactive') handle edge cases where support features exist but no active subscription is present
- Direct access to
enabledFeaturesis safe given the type guarantees (consistent with usage at lines 154, 157-158)The timing is correct—attributes are set after user initialization and before toggling the widget.
webapp/src/component/HelpMenu.tsx
Outdated
| if (preferredOrganization) { | ||
| const subscription = preferredOrganization.activeCloudSubscription; | ||
| // @ts-ignore | ||
| window.$chatwoot.setCustomAttributes({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, this looks like potential issue when the $chatwoot is not defined. I wouldn't use @ts-ignore ignore here.
Also I would move this to separate hook usePrepareChatwootAttributes. This component us already long enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ended up moving the entire Chatwoot setup to a separate hook.
The ts-ignore must stay because it's needed to access the property, but I wonder how to handle the case where the property is missing — for example, if Chatwoot loading fails. Currently, it just fails silently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or, I guess, what Coderabbit suggested: "type declaration file for the Window interface".
|
Sorry for all the tasks, but can you also finish this? @Anty0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (3)
webapp/src/hooks/useChatwoot.ts (3)
58-65: Remove unnecessary non-null assertions.The non-null assertions (
!) on lines 61-62 are unnecessary sinceuseris guaranteed to exist when this function is called (checked by theavailableguard inopenChatwoot). TheUsertype from the API schema should already define these fields as required.Apply this diff:
function setChatwootUser(user: User) { // @ts-ignore window.$chatwoot?.setUser(user.id, { - email: user!.username, - name: user!.name, + email: user.username, + name: user.name, url: window.location, }); }
105-109: Consider loading Chatwoot only when the feature is available.The
useEffectloads the Chatwoot script whenever atokenexists, even for users without support features. This wastes resources loading scripts for users who can't access the chat feature.Apply this diff to defer loading until the user actually opens the chat:
- useEffect(() => { - if (token) { - loadChatwootOnce(token, darkMode); - } - }, [token]); - const openChatwoot = async () => { if (!available) { return; } await loadChatwootOnce(token, darkMode);Alternatively, if you want to pre-load for eligible users:
useEffect(() => { - if (token) { + if (available) { loadChatwootOnce(token, darkMode); } - }, [token]); + }, [available, token, darkMode]);
111-123: Address type safety for token parameter.At line 116,
tokencould beundefined(fromconfig?.chatwootToken), butloadChatwootOnceexpects astring. While theavailablecheck at line 112 ensurestokenis truthy, TypeScript can't infer this narrowing.Apply this diff to make the type explicit:
const openChatwoot = async () => { - if (!available) { + if (!available || !token) { return; } await loadChatwootOnce(token, darkMode);Alternatively, add a type assertion after verifying
available:const openChatwoot = async () => { if (!available) { return; } + + // Token is guaranteed to exist when available is true + const websiteToken = token!; - await loadChatwootOnce(token, darkMode); + await loadChatwootOnce(websiteToken, darkMode); setChatwootUser(user);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
webapp/src/component/HelpMenu.tsx(4 hunks)webapp/src/hooks/useChatwoot.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
webapp/src/hooks/useChatwoot.ts (2)
webapp/src/component/UserAccount.tsx (1)
User(10-15)webapp/src/globalContext/helpers.tsx (3)
useUser(13-13)usePreferredOrganization(31-43)useConfig(10-11)
webapp/src/component/HelpMenu.tsx (1)
webapp/src/hooks/useChatwoot.ts (1)
useChatwoot(85-129)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: E2E testing 🔎 (15, 1)
- GitHub Check: E2E testing 🔎 (15, 12)
- GitHub Check: E2E testing 🔎 (15, 7)
- GitHub Check: E2E testing 🔎 (15, 14)
- GitHub Check: E2E testing 🔎 (15, 13)
- GitHub Check: E2E testing 🔎 (15, 4)
- GitHub Check: E2E testing 🔎 (15, 0)
- GitHub Check: E2E testing 🔎 (15, 2)
- GitHub Check: BT 🔎 (security:test)
- GitHub Check: BT 🔎 (ktlint:test)
- GitHub Check: BT 🔎 (ee-test:test)
- GitHub Check: BT 🔎 (server-app:runWebsocketTests)
- GitHub Check: BT 🔎 (server-app:runStandardTests)
- GitHub Check: BT 🔎 (data:test)
- GitHub Check: BT 🔎 (server-app:runWithoutEeTests)
- GitHub Check: BT 🔎 (server-app:runContextRecreatingTests)
- GitHub Check: Frontend static check 🪲
🔇 Additional comments (5)
webapp/src/component/HelpMenu.tsx (1)
1-1: LGTM! Clean refactoring to use the useChatwoot hook.The component successfully delegates all Chatwoot logic to the new
useChatwoothook as suggested in previous reviews. The changes simplify the component by removing direct script loading and attribute management, making it more maintainable.Also applies to: 28-28, 31-31, 44-44, 144-159
webapp/src/hooks/useChatwoot.ts (4)
10-14: LGTM! Good use of generated types and singleton pattern.Using the API schema types ensures type safety, and the singleton promise pattern for
chatwootLoadPromiseprevents duplicate script loads.
16-32: LGTM! Standard dynamic script loading implementation.The function correctly implements asynchronous script loading with proper promise handling and script element configuration.
50-56: LGTM! Correct singleton pattern implementation.The function properly ensures the Chatwoot script is loaded only once by caching the promise.
67-78: LGTM! Proper fallback values for subscription attributes.The function correctly provides fallback values for
planandsubscriptionStatuswhen subscription data is unavailable, which aligns with the requirement to set attributes for all users (not just paying customers as noted in previous reviews).
| async function loadChatwoot(websiteToken: string, darkMode: boolean) { | ||
| // @ts-ignore | ||
| window.chatwootSettings = { | ||
| darkMode: darkMode ? 'auto' : 'light', | ||
| hideMessageBubble: true, | ||
| }; | ||
|
|
||
| await loadScript(document, BASE_URL + '/packs/js/sdk.js'); | ||
|
|
||
| // @ts-ignore | ||
| window.chatwootSDK?.run({ | ||
| websiteToken, | ||
| baseUrl: BASE_URL, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid @ts-ignore by declaring proper Window interface augmentation.
The @ts-ignore comments on lines 35 and 43 directly contradict the previous review feedback from JanCizmar who stated "I wouldn't use @ts-ignore ignore here." Instead, declare proper TypeScript types for the Chatwoot properties.
Add a type declaration file or augment the Window interface at the top of this file:
+declare global {
+ interface Window {
+ chatwootSettings?: {
+ darkMode: 'auto' | 'light';
+ hideMessageBubble: boolean;
+ };
+ chatwootSDK?: {
+ run: (config: { websiteToken: string; baseUrl: string }) => void;
+ };
+ $chatwoot?: {
+ setUser: (id: number, data: { email: string; name: string; url: Location }) => void;
+ setCustomAttributes: (attributes: Record<string, unknown>) => void;
+ toggle: () => void;
+ };
+ }
+}
+
type User = components['schemas']['PrivateUserAccountModel'];
type Organization = components['schemas']['PrivateOrganizationModel'];Then remove all @ts-ignore comments throughout the file.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In webapp/src/hooks/useChatwoot.ts around lines 34 to 48, remove the two
@ts-ignore comments and instead add proper TypeScript declarations for the
Chatwoot properties on the Window interface (either in a dedicated .d.ts file or
via a local declare global block at the top of this file). Declare
window.chatwootSettings with the expected shape (darkMode and hideMessageBubble)
and window.chatwootSDK with a run method that accepts the websiteToken/baseUrl
options, ensure the file picks up the declaration (export {} if using local
augmentation), then delete the @ts-ignore lines so the compiler recognizes the
properties.
| useEffect(() => { | ||
| if (token) { | ||
| loadChatwootOnce(token, darkMode); | ||
| } | ||
| }, [token]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add darkMode to the dependency array.
The useEffect depends on darkMode (used in loadChatwootOnce) but only includes [token] in the dependency array. If the user switches between light and dark themes, the Chatwoot widget settings won't be updated.
Apply this diff:
useEffect(() => {
if (token) {
loadChatwootOnce(token, darkMode);
}
- }, [token]);
+ }, [token, darkMode]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| useEffect(() => { | |
| if (token) { | |
| loadChatwootOnce(token, darkMode); | |
| } | |
| }, [token]); | |
| useEffect(() => { | |
| if (token) { | |
| loadChatwootOnce(token, darkMode); | |
| } | |
| }, [token, darkMode]); |
🤖 Prompt for AI Agents
In webapp/src/hooks/useChatwoot.ts around lines 105 to 109, the useEffect calls
loadChatwootOnce(token, darkMode) but only lists [token] as dependencies; add
darkMode to the dependency array so the effect re-runs when the theme changes
(i.e., change the dependencies to [token, darkMode]) to ensure the Chatwoot
widget updates when switching between light and dark modes.
Summary by CodeRabbit
New Features
Bug Fixes