-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
FEATURE: Zendesk chat #1778
FEATURE: Zendesk chat #1778
Conversation
@jwalsh-famkit is attempting to deploy a commit to the Typebot Team on Vercel. A member of the Team first needs to authorize it. |
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…pebot.io into FEAT_ZENDESK_CHAT
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.
That's awesome, thank you so much!!
Can you also provide a quick demo video to this PR? You rock
id: 'zendesk-chat', | ||
name: 'Zendesk Chat', |
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.
Can we rename it to just "Zendesk" I might add more actions in the future that are not tied to Chat
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.
Also change id to zendesk. Make sure to search and replace in the entire project 🙏
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 originally wanted to do exactly that, however, there are two separate API families for Zendesk, the core Zendesk API, https://vorihealth.zendesk.com/admin/apps-integrations/apis/zendesk-api/settings, which covers their Tickets, and other CRM functions and and their Conversations API - https://vorihealth.zendesk.com/admin/apps-integrations/apis/conversations-api, which is the Sunshine conversations product they acquired. They are really two separate apps, and don't share API keys, etc. So, now sure if it would be confusing to do this and manage credentials as one single integration. What do you think?
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 updated it to Zendesk Messaging to be more aligned to the product. If there is a way to manage multiple credentials without confusing the user, happy to move to just Zendesk. Thoughts?
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 honestly think it makes more sense to name it Zendesk and update the credentials form accordingly, explaining that signing key is only useful for authenticating users before opening up the Zendesk Chat widget and that it is optional.
In the future we will add the fields for the other Zendesk APIs all of which will be optional as well.
Does that make sense?
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.
Makes sense to me. What I'm going to do is close this PR so I can generate the block properly as Zendesk. I'll add a new PR for that. Thanks!
emailVerified: option.string.layout({ | ||
label: 'Email Is Verified', | ||
defaultValue: "false" | ||
}), |
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.
emailVerified: option.string.layout({ | |
label: 'Email Is Verified', | |
defaultValue: "false" | |
}), | |
wasEmailVerified: option.boolean.layout({ | |
label: 'Email was verified', | |
}), |
I think it is clearer
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.
Email Verification is a status of the email, so it's probably better to describe it in terms of the current state. How about 'isEmailVerified' and keep the label the same?
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.
Ok, but still convert it to boolean and non-title case? Or does status can be something different than "false" or "true". If so it should be an enum then
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.
@baptisteArno a question on the boolean. If I change it to boolean, it doesn't appear that I can set that value from a variable, which I would need to be able to do as we would be tracking as part of a user profile. Am I missing something? Otherwise, I think we need to leave it as a string so we can do that.
@coderabbitai review |
Actions performedReview triggered.
|
WalkthroughThe pull request introduces a new integration for Zendesk Chat within the Typebot framework. It includes comprehensive documentation, user authentication actions, a logo component, and schemas for managing credentials. Key functionalities such as opening the chat messenger and generating JSON Web Tokens (JWT) for user authentication are implemented. The changes enhance the existing system by allowing the application to recognize and interact with Zendesk Chat services, streamlining the integration process for developers. Changes
Tip Announcements
Recent review detailsConfiguration used: CodeRabbit UI Files ignored due to path filters (7)
Files selected for processing (11)
Additional context usedLanguageTool
Additional comments not posted (17)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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
Outside diff range, codebase verification and nitpick comments (1)
packages/forge/repository/definitions.ts (1)
15-15
: Multiple Exports Detected forzendeskChatBlock
The
zendeskChatBlock
is exported from two different files, which may lead to ambiguity or redundancy in the codebase. Please review the following locations to ensure thatzendeskChatBlock
is exported from a single, consistent source:
packages/forge/blocks/zendeskChat/schemas.ts
packages/forge/blocks/zendeskChat/index.ts
Additionally, the usage of
zendeskChatBlock
inforgedBlocks
is found in:
packages/forge/repository/schemas.ts
packages/forge/repository/definitions.ts
Please verify and consolidate the exports as necessary.
Analysis chain
LGTM!
The code changes are approved.
The import statement correctly imports the
zendeskChatBlock
from@typebot.io/zendesk-chat-block
. ThezendeskChatBlock
is properly added to theforgedBlocks
object using itsid
as the key, following the existing pattern. This makes the Zendesk Chat block available for use within the Typebot framework.To verify that the
zendeskChatBlock
is correctly exported and accessible, you can run the following script:Also applies to: 31-31
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `zendeskChatBlock` is correctly exported and accessible. # Test: Search for the export of `zendeskChatBlock`. Expect: Exactly one occurrence. rg --type typescript -c $'export const zendeskChatBlock' # Test: Search for the usage of `zendeskChatBlock` in `forgedBlocks`. Expect: Exactly one occurrence. rg --type typescript -c $'\\[zendeskChatBlock\\.id\\]: zendeskChatBlock'Length of output: 198
Script:
#!/bin/bash # Description: Verify that the `zendeskChatBlock` is correctly exported and accessible. # Test: Search for the export of `zendeskChatBlock`. Expect: Exactly one occurrence. rg --type ts -c $'export const zendeskChatBlock' # Test: Search for the usage of `zendeskChatBlock` in `forgedBlocks`. Expect: Exactly one occurrence. rg --type ts -c $'\\[zendeskChatBlock\\.id\\]: zendeskChatBlock'Length of output: 286
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (7)
apps/docs/images/blocks/integrations/zendesk-chat/web-widget-config.png
is excluded by!**/*.png
,!**/*.png
apps/docs/images/blocks/integrations/zendesk-chat/zendesk-end-user-auth.png
is excluded by!**/*.png
,!**/*.png
apps/docs/mint.json
is excluded by!**/*.json
packages/forge/blocks/zendeskChat/package.json
is excluded by!**/*.json
packages/forge/blocks/zendeskChat/tsconfig.json
is excluded by!**/*.json
packages/forge/repository/package.json
is excluded by!**/*.json
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
,!**/*.yaml
Files selected for processing (11)
- apps/docs/editor/blocks/integrations/zendesk-chat.mdx (1 hunks)
- packages/forge/blocks/zendeskChat/actions/authenticate.ts (1 hunks)
- packages/forge/blocks/zendeskChat/actions/openMessenger.ts (1 hunks)
- packages/forge/blocks/zendeskChat/auth.ts (1 hunks)
- packages/forge/blocks/zendeskChat/index.ts (1 hunks)
- packages/forge/blocks/zendeskChat/logo.tsx (1 hunks)
- packages/forge/blocks/zendeskChat/schemas.ts (1 hunks)
- packages/forge/repository/constants.ts (1 hunks)
- packages/forge/repository/credentials.ts (2 hunks)
- packages/forge/repository/definitions.ts (2 hunks)
- packages/forge/repository/schemas.ts (2 hunks)
Additional context used
LanguageTool
apps/docs/editor/blocks/integrations/zendesk-chat.mdx
[uncategorized] ~24-~24: Possible missing comma found.
Context: ...hat can be passed as a variable to Open Messeneger you to authenticate a user in Zendesk. ...(AI_HYDRA_LEO_MISSING_COMMA)
Additional comments not posted (17)
packages/forge/repository/constants.ts (1)
18-18
: LGTM!The code change is approved.
packages/forge/blocks/zendeskChat/schemas.ts (1)
1-10
: LGTM!The code is approved.
packages/forge/blocks/zendeskChat/index.ts (1)
8-9
: Address the renaming suggestions from previous reviews.The following comments from previous reviews are still applicable:
baptisteArno: Can we rename it to just "Zendesk" I might add more actions in the future that are not tied to Chat
baptisteArno: Also change id to zendesk. Make sure to search and replace in the entire project 🙏
Please consider renaming the block to "Zendesk" and updating its ID to "zendesk" across the entire project to accommodate potential future actions that may not be specific to chat functionality.
packages/forge/blocks/zendeskChat/auth.ts (1)
1-21
: LGTM!The code changes are approved.
The
auth
object correctly defines the authentication schema for Zendesk Messaging account with the requiredsecretKey
andkeyId
fields. The use ofencryptedCredentials
type ensures the credentials are encrypted. The schema options are configured appropriately.packages/forge/blocks/zendeskChat/logo.tsx (3)
4-15
: Past comment no longer applicable.The suggestion to update the
viewBox
attribute has already been implemented in the current code.
25-25
: Past comment no longer applicable.The unnecessary spaces
{' '}
have been removed in the current code.
1-37
: LGTM!The code changes are approved.
The
ZendeskChatLogo
component correctly renders the Zendesk Chat logo as an SVG. The SVG paths and attributes are defined appropriately. The component acceptsprops
to allow customization of the SVG element.packages/forge/blocks/zendeskChat/actions/authenticate.ts (3)
2-2
: Use named import instead of default import.As suggested in the previous review, use named import for better code readability and to avoid naming conflicts.
Apply this diff:
-import jwt from 'jsonwebtoken'; +import { sign } from 'jsonwebtoken';
21-24
: Change the label to "Email was verified" for clarity.As suggested in the previous review, change the label to "Email was verified" for better clarity.
Apply this diff:
- emailVerified: option.string.layout({ - label: 'Email Is Verified', + wasEmailVerified: option.boolean.layout({ + label: 'Email was verified',
45-46
: Inline thesign
function call for better readability.As suggested in the previous review, inline the
sign
function call to reduce the number of lines and improve readability.Apply this diff:
- var token = jwt.sign({ scope: 'user', external_id: userId, name: name, email: email, email_verified: emailVerified }, secretKey, { algorithm: "HS256", keyid: keyId }); - variables.set(tokenVariableId, token); + variables.set(tokenVariableId, sign({ scope: 'user', external_id: userId, name: name, email: email, email_verified: emailVerified }, secretKey, { algorithm: "HS256", keyid: keyId }));packages/forge/blocks/zendeskChat/actions/openMessenger.ts (3)
9-9
: Use sentence case for the option label.As suggested in the previous review, change the label to "Web widget key" instead of "Web Widget Key" for better consistency and readability.
Apply this diff:
- label: "Web Widget Key", + label: "Web widget key",
12-12
: Change the option type to boolean.As suggested in the previous review, change the option type to boolean for better type safety and to avoid potential issues with string comparisons.
Apply this diff:
- enableAuth: option.string.layout({ + isAuthEnabled: option.boolean.layout({
6-6
: Verify if "Messenger" is the correct term used by Zendesk.As mentioned in the previous review, we should verify if Zendesk uses the term "Messenger" for their chat widget to maintain consistency with their terminology.
Run the following script to search for the term "Messenger" in the Zendesk Chat documentation:
Verification successful
Verified: "Messenger" is the correct term used by Zendesk.
The term "Messenger" is used in the context of Zendesk Chat within the documentation, indicating that it is the correct terminology for their chat widget. The usage of "Open Messenger" in the code is consistent with Zendesk's terminology.
apps/docs/editor/blocks/integrations/zendesk-chat.mdx
: Describes an action to open the Messenger window.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if Zendesk uses the term "Messenger" for their chat widget. # Test: Search for the term "Messenger" in the Zendesk Chat documentation. Expect: Occurrences of the term. rg --type md -C 2 $'Messenger' $(fd --type d --full-path $'/zendesk-chat/')Length of output: 71
Script:
#!/bin/bash # Description: Search for the term "Messenger" across the entire repository to verify its usage. # Test: Search for the term "Messenger" in all files. Expect: Occurrences of the term. rg -i 'Messenger' -A 2Length of output: 11851
packages/forge/repository/credentials.ts (2)
23-24
: LGTM!The import statements for
zendeskChatBlock
andzendeskChatCredentialsSchema
are correctly added.
38-38
: LGTM!The
forgedCredentialsSchemas
object is correctly updated to include the Zendesk chat credentials schema.packages/forge/repository/schemas.ts (2)
28-29
: LGTM!The import statements for
zendeskChatBlock
andzendeskChatBlockSchema
are correctly added.
45-45
: LGTM!The
forgedBlockSchemas
object is correctly updated to include the Zendesk chat block schema.
|
Close in favor of #1794 |
This adds a Zendesk Chat block to allow chat flows to open a Zendesk live chat window, similar to the Chatwoot block.