fix: premium and soon datasource display condition changed #38858
Conversation
WalkthroughThe pull request modifies the handling of premium datasources in the application by replacing the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
|
github-actions
bot
added
Bug
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/12997173517. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/src/pages/Editor/IntegrationEditor/PremiumDatasources/index.tsx (1)
64-64: Consider renaming the prop for clarity.The prop name
isBusinessOrEnterpriseno longer reflects its actual usage with the GAC feature flag. Consider renaming it to better match its purpose.-isBusinessOrEnterprise={isGACEnabled} +isGACLicensed={isGACEnabled}Also applies to: 68-68
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/client/src/pages/Editor/IntegrationEditor/PremiumDatasources/ContactForm.tsx(6 hunks)app/client/src/pages/Editor/IntegrationEditor/PremiumDatasources/index.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-build / client-build
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
🔇 Additional comments (5)
app/client/src/pages/Editor/IntegrationEditor/PremiumDatasources/index.tsx (2)
9-10: LGTM! Clean import changes for feature flag implementation.The imports are correctly structured for the new feature flag-based approach.
39-39: Verify consistent feature flag usage across the application.The feature flag implementation looks correct, but let's verify its usage across the codebase.
Also applies to: 41-41
✅ Verification successful
Feature flag implementation is consistent across the codebase
The
license_gac_enabledfeature flag is properly implemented and follows the established pattern used throughout the application.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other occurrences of the GAC feature flag to ensure consistent usage rg -l "license_gac_enabled" --type ts --type tsxLength of output: 76
Script:
#!/bin/bash # Search for feature flag constant and usages rg "license_gac_enabled" --type typescript --type ts # Search for feature flag definition fd -e ts -e tsx FEATURE_FLAG --exec cat {}Length of output: 134
Script:
#!/bin/bash # Search for any occurrence of the feature flag with context rg -A 2 -B 2 "license_gac_enabled" # Search for feature flag enum/constant definition rg "FEATURE_FLAG.*license_gac_enabled"Length of output: 48499
app/client/src/pages/Editor/IntegrationEditor/PremiumDatasources/ContactForm.tsx (3)
20-20: LGTM! Import statements are properly organized.The new imports are correctly placed and necessary for the feature flag implementation.
Also applies to: 32-33
46-46: LGTM! Feature flag is properly integrated with form submission.The implementation correctly handles the feature flag in the submission logic with proper dependency tracking.
Also applies to: 73-74
79-79: LGTM! UI elements consistently use the feature flag.The feature flag is consistently applied across all UI elements and text generation functions.
Also applies to: 86-86, 100-100, 111-111
|
Deploy-Preview-URL: https://ce-38858.dp.appsmith.com |
app/client/src/pages/Editor/IntegrationEditor/PremiumDatasources/ContactForm.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/src/pages/Editor/IntegrationEditor/PremiumDatasources/index.tsx (1)
64-70: Consider adding ARIA attributes for accessibility.While the tag implementation looks correct, consider enhancing accessibility by adding appropriate ARIA attributes to the premium tag.
<PremiumTag isBusinessOrEnterprise={isGACEnabled} isClosable={false} kind={"premium"} + role="status" + aria-label={`Premium feature status: ${getTagText(isGACEnabled)}`} > {getTagText(isGACEnabled)} </PremiumTag>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/client/src/pages/Editor/IntegrationEditor/PremiumDatasources/ContactForm.tsx(6 hunks)app/client/src/pages/Editor/IntegrationEditor/PremiumDatasources/index.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/src/pages/Editor/IntegrationEditor/PremiumDatasources/ContactForm.tsx
⏰ Context from checks skipped due to timeout of 90000ms (6)
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (3)
app/client/src/pages/Editor/IntegrationEditor/PremiumDatasources/index.tsx (3)
9-10: LGTM! Clean import additions.The new feature flag imports are properly structured and necessary for the implementation.
41-44: Verify premium datasource click handler implementation.The click handler update looks correct, but let's verify the implementation of
handlePremiumDatasourceClickto ensure it properly handles the new flag-based logic.✅ Verification successful
Premium datasource click handler implementation verified successfully
The handler correctly uses the flag to determine the appropriate analytics event.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check handlePremiumDatasourceClick implementation ast-grep --pattern 'handlePremiumDatasourceClick($name, $isEnabled) { $$$ }'Length of output: 80
Script:
#!/bin/bash # Search for handlePremiumDatasourceClick with context rg "handlePremiumDatasourceClick" -A 5 # Also try arrow function pattern ast-grep --pattern 'const handlePremiumDatasourceClick = ($name, $isEnabled) => { $$$ }'Length of output: 2166
39-40: Verify feature flag existence and configuration.Good documentation with the Notion link. Let's verify the feature flag setup.
✅ Verification successful
Feature flag implementation verified
The
FEATURE_FLAG.license_gac_enabledis properly defined and consistently used across the codebase. The implementation follows best practices with proper typing and documentation.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify feature flag definition and usage ast-grep --pattern 'FEATURE_FLAG.license_gac_enabled'Length of output: 92499
Script:
#!/bin/bash # Search for feature flag definition echo "=== Searching for feature flag definition ===" rg -i "license_gac_enabled" --type ts --type js -C 2 echo -e "\n=== Searching for feature flag enum/constant ===" ast-grep --pattern 'FEATURE_FLAG = { $$$ license_gac_enabled: $_, $$$ }'Length of output: 39939
Description
Earlier the condition for showing premium and soon datasources were dependent on the tenant instance being enterprise/business or free instance. Now after this PR it depends on the feature flag
license_gac_enabledto show the premium/soon datasources.Fixes #38802
or
Fixes
Issue URLWarning
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/13005514213
Commit: e2f22a4
Cypress dashboard.
Tags:
@tag.DatasourceSpec:
Tue, 28 Jan 2025 08:08:38 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
isGACEnabledto control premium datasource functionality.Refactor
isFreePlanInstancevariable and associated logic.isGACEnabledfor rendering and interaction conditions.