-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
fix: calendly link on schedule a call click, jira and slack integration premium tag #38331
Conversation
WalkthroughThe pull request introduces modifications to several files in the Appsmith client application, focusing on premium datasources and integration management. The changes include adding Slack and Jira to premium integrations, updating component rendering logic for datasource screens, and implementing a schedule call functionality. The modifications aim to enhance the user experience around premium datasource integrations and provide a more streamlined approach to scheduling calls. Changes
Assessment against linked issues
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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
Documentation and Community
|
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12469561314. |
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: 0
🧹 Nitpick comments (1)
app/client/src/pages/Editor/IntegrationEditor/CreateNewDatasourceTab.tsx (1)
Line range hint
182-218
: Premium datasources extraction.
Shifting the premium datasources logic to “CreateNewSaasIntegration” clarifies the architecture and appropriately keeps each component’s responsibility clear.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/client/src/constants/PremiumDatasourcesConstants.ts
(1 hunks)app/client/src/pages/Editor/IntegrationEditor/CreateNewDatasourceTab.tsx
(3 hunks)app/client/src/pages/Editor/IntegrationEditor/DatasourceHome.tsx
(0 hunks)app/client/src/pages/Editor/IntegrationEditor/NewApi.tsx
(3 hunks)app/client/src/pages/Editor/IntegrationEditor/NewQuery.tsx
(1 hunks)app/client/src/utils/PremiumDatasourcesHelpers.ts
(2 hunks)
💤 Files with no reviewable changes (1)
- app/client/src/pages/Editor/IntegrationEditor/DatasourceHome.tsx
🧰 Additional context used
📓 Learnings (1)
app/client/src/pages/Editor/IntegrationEditor/CreateNewDatasourceTab.tsx (1)
Learnt from: sneha122
PR: appsmithorg/appsmith#29377
File: app/client/src/pages/Editor/IntegrationEditor/CreateNewDatasourceTab.tsx:316-329
Timestamp: 2024-11-12T08:11:25.416Z
Learning: The user clarified that the feature being introduced is experimental and that automated tests will be added once the decision to keep the feature is made. This context is important for future reviews as it explains the absence of tests for certain features under development.
🔇 Additional comments (8)
app/client/src/pages/Editor/IntegrationEditor/NewQuery.tsx (1)
59-59
: Confirm that removing children is intentional.
If the DataSourceHome
component no longer needs child elements, this is fine. Otherwise, reintroducing the child rendering logic may be necessary to maintain expected functionality.
✅ Verification successful
Removing children prop is correct - component doesn't use it
Looking at the DatasourceHome
component implementation, while it declares children
in its props interface, the component doesn't render any children in its implementation. The component is designed to render a fixed layout of datasource cards.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Let's first check the DataSourceHome component usage and its children in the file
ast-grep --pattern 'DataSourceHome' NewQuery.tsx
# Let's also check the component definition to understand its props and children handling
fd DataSourceHome --exec cat {}
Length of output: 144
Script:
#!/bin/bash
# Let's try to find the correct path first
fd -e tsx "NewQuery.tsx$"
# Then search for DataSourceHome component files
fd -e tsx -e ts "DataSourceHome"
# Also search for DataSourceHome usage across the codebase
rg "DataSourceHome" -t typescript -t tsx
Length of output: 195
Script:
#!/bin/bash
# Let's examine the NewQuery.tsx file content to understand the DataSourceHome usage
cat "app/client/src/pages/Editor/IntegrationEditor/NewQuery.tsx"
# Also search for DataSourceHome component with proper file extension
rg "DataSourceHome" --type-add 'ts:*.{ts,tsx}' -t ts
Length of output: 2088
Script:
#!/bin/bash
# Let's check the DatasourceHome component implementation
cat "app/client/src/pages/Editor/IntegrationEditor/DatasourceHome.tsx"
Length of output: 11863
app/client/src/constants/PremiumDatasourcesConstants.ts (2)
18-25
: All good for Slack & Jira entries.
The newly added objects for Slack and Jira look correct, and the associated icon references also appear to be accurate.
31-32
: Ensure URL correctness.
The Calendly URL is defined as a constant. Double-check that this link is correct and doesn't require dynamic parameters other than "email."
✅ Verification successful
Let me search for any related usage or configuration of this Calendly URL in the codebase.
URL implementation is correctly handled with dynamic email parameter
The Calendly URL is properly implemented in the codebase. The constant is used in PremiumDatasourcesHelpers.ts
where it's dynamically appended with the email parameter when needed: `${SCHEDULE_CALL_URL}?email=${email}`
. No other instances of Calendly URLs exist in the codebase, confirming this is the intended implementation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for any references to the Calendly URL or the constant
rg -i "calendly.com/carina-neves-fonseca" -A 2 -B 2
# Search for any usage of SCHEDULE_CALL_URL constant
rg "SCHEDULE_CALL_URL" -A 2 -B 2
# Look for any configuration files that might contain Calendly URLs
fd -e json -e yaml -e yml -e config -e env | xargs rg -i "calendly"
# Check if there are any other Calendly URLs defined in the codebase
rg -i "calendly.com" -A 2 -B 2
Length of output: 1796
app/client/src/utils/PremiumDatasourcesHelpers.ts (1)
62-70
: Conditional scheduling flow is well-designed.
The code introduces a useful condition to open the Calendly link only when a valid email is provided and the user is not a business or enterprise user. This approach avoids unnecessary windows in other scenarios.
app/client/src/pages/Editor/IntegrationEditor/NewApi.tsx (2)
145-145
: Using children for composability.
Allowing children to be passed here promotes flexible composition of nested UI elements, such as “PremiumDatasources.” This is a good move toward more modular design.
332-332
: Rendering children in ApiCardsContainer.
Including the new children prop inside the container ensures that additional sections (like PremiumDatasources) can appear seamlessly within the layout. Great for reusability.
app/client/src/pages/Editor/IntegrationEditor/CreateNewDatasourceTab.tsx (2)
174-174
: No issues detected here.
The updated reference to NewQueryScreen remains consistent.
347-347
: Prop usage for premium datasources.
Noting that the new “isPremiumDatasourcesViewEnabled” prop is carried forward simplifies feature toggling. Good approach for progressive feature rollout.
Deploy-Preview-URL: https://ce-38331.dp.appsmith.com |
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12480590221. |
Deploy-Preview-URL: https://ce-38331.dp.appsmith.com |
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12481428723. |
Deploy-Preview-URL: https://ce-38331.dp.appsmith.com |
Description
This PR introduces the change to premium datasources views.
Fixes #38262
or
Fixes
Issue URL
Warning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.Datasource"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12481422641
Commit: 5c4b2a9
Cypress dashboard.
Tags:
@tag.Datasource
Spec:
Tue, 24 Dec 2024 12:50:38 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
NewApiScreen
to accept child components.Bug Fixes
Chores
DatasourceHomeScreen
for improved component structure.PremiumDatasourceContactForm
for better presentation.