fix: Disable custom widgets for airgapped environments#34540
fix: Disable custom widgets for airgapped environments#34540rahulbarwal merged 2 commits intoreleasefrom
Conversation
WalkthroughThe set of changes enhances the Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
app/client/src/widgets/CustomWidget/widget/index.tsx (1)
Line range hint
397-397: ReplacehasOwnPropertywithObject.hasOwn().Using
Object.hasOwn()is recommended overhasOwnPropertyfor checking property existence due to improved safety and alignment with modern JavaScript practices.- if (this.props.hasOwnProperty(eventName)) { + if (Object.hasOwn(this.props, eventName)) {
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- app/client/src/widgets/CustomWidget/widget/index.tsx (2 hunks)
Additional context used
Biome
app/client/src/widgets/CustomWidget/widget/index.tsx
[error] 106-108: This else clause can be omitted because previous branches break early.
Unsafe fix: Omit the else clause.
(lint/style/noUselessElse)
[error] 397-397: Do not access Object.prototype method 'hasOwnProperty' from target object.
It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.(lint/suspicious/noPrototypeBuiltins)
Additional comments not posted (3)
app/client/src/widgets/CustomWidget/widget/index.tsx (3)
37-37: Import statement looks correct.The import of
isAirgappedfrom"@appsmith/utils/airgapHelpers"is correctly placed and used in the configuration to dynamically set thehideCardproperty based on the environment.
59-59: Dynamic setting ofhideCardproperty is a smart implementation.Setting
hideCardbased onisAirgapped()aligns with the PR's objective to disable custom widgets in airgapped environments. Consider adding a comment explaining why this property is set dynamically for clarity and maintainability.
Line range hint
106-108: Static analysis hint about else clause cannot be addressed.The hint regarding the else clause cannot be addressed as the specific code segment is not visible in the provided code.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/Custom/CustomWidgetDefaultComponent_spec.ts (1 hunks)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/Custom/CustomWidgetEditorPropertyPane_spec.ts (1 hunks)
Files skipped from review due to trivial changes (2)
- app/client/cypress/e2e/Regression/ClientSide/Widgets/Custom/CustomWidgetDefaultComponent_spec.ts
- app/client/cypress/e2e/Regression/ClientSide/Widgets/Custom/CustomWidgetEditorPropertyPane_spec.ts
## Description Since custom widgets rely on react delievered from `https://cdn.jsdelivr.net`, it can't work in airgapped edition. Fixes #`Issue Number` _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.Widget" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/9692258525> > Commit: bf3b40a > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=9692258525&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Widget` <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Custom widgets will now hide specific cards if the environment is air-gapped. - **Tests** - Updated test tags for custom widgets to exclude tests in air-gapped environments. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…t removal from Airgapped (#34612) ## Description This [PR](#34540) removed custom widgets from Airgapped versions and this caused the widget sidebar tests to fail. This PR updated the required tests to account for this change. Fixes #`Issue Number` _or_ Fixes `Issue URL` ## Automation /ok-to-test tags="@tag.Widget" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/9757281541> > Commit: bd1e5ed > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=9757281541&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Widget` <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Updated widgets catalog to display different widgets based on the airgapped environment. - Adjusted widget search behavior to align with the airgapped environment, enhancing search accuracy and display. - **Bug Fixes** - Fixed widget card display consistency when the `AIRGAPPED` environment variable is enabled. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…4540) ## Description Since custom widgets rely on react delievered from `https://cdn.jsdelivr.net`, it can't work in airgapped edition. Fixes #`Issue Number` _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.Widget" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/9692258525> > Commit: bf3b40a > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=9692258525&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Widget` <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Custom widgets will now hide specific cards if the environment is air-gapped. - **Tests** - Updated test tags for custom widgets to exclude tests in air-gapped environments. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…t removal from Airgapped (appsmithorg#34612) ## Description This [PR](appsmithorg#34540) removed custom widgets from Airgapped versions and this caused the widget sidebar tests to fail. This PR updated the required tests to account for this change. Fixes #`Issue Number` _or_ Fixes `Issue URL` ## Automation /ok-to-test tags="@tag.Widget" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/9757281541> > Commit: bd1e5ed > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=9757281541&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Widget` <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Updated widgets catalog to display different widgets based on the airgapped environment. - Adjusted widget search behavior to align with the airgapped environment, enhancing search accuracy and display. - **Bug Fixes** - Fixed widget card display consistency when the `AIRGAPPED` environment variable is enabled. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
Since custom widgets rely on react delievered from
https://cdn.jsdelivr.net, it can't work in airgapped edition.Fixes #34504
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.Widget"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9692258525
Commit: bf3b40a
Cypress dashboard.
Tags:
@tag.WidgetCommunication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Tests