chore: Plugin Action Editor routing #37389
Conversation
WalkthroughThe changes in this pull request involve modifications to several components and hooks within the application. Key updates include enhancements to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
the
skip-changelog
![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
🧹 Outside diff range and nitpick comments (1)
app/client/src/ce/pages/Editor/IDE/EditorPane/Query/hooks.tsx (1)
159-159: Consider lazy loading optimizationGood use of memoized skeleton component. However, consider implementing a loading delay to prevent flash of loading state for fast connections.
- const skeleton = useMemo(() => <Skeleton />, []); + const skeleton = useMemo(() => { + const MinimalLoadingDelay = 100; // ms + return ( + <Suspense fallback={ + <DelayedFallback delay={MinimalLoadingDelay}> + <Skeleton /> + </DelayedFallback> + } /> + ); + }, []);Also applies to: 211-213, 228-230, 253-255, 265-267
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (5)
app/client/src/ce/pages/Editor/IDE/EditorPane/Query/hooks.tsx(6 hunks)app/client/src/ee/pages/Editor/AppPluginActionEditor/AppPluginActionEditor.tsx(0 hunks)app/client/src/pages/Editor/APIEditor/index.tsx(0 hunks)app/client/src/pages/Editor/AppPluginActionEditor/index.ts(0 hunks)app/client/src/pages/Editor/QueryEditor/index.tsx(0 hunks)
💤 Files with no reviewable changes (4)
- app/client/src/ee/pages/Editor/AppPluginActionEditor/AppPluginActionEditor.tsx
- app/client/src/pages/Editor/APIEditor/index.tsx
- app/client/src/pages/Editor/AppPluginActionEditor/index.ts
- app/client/src/pages/Editor/QueryEditor/index.tsx
🔇 Additional comments (3)
app/client/src/ce/pages/Editor/IDE/EditorPane/Query/hooks.tsx (3)
54-57: LGTM: Improved variable declaration
The change from reassignment to const declaration enhances code clarity and follows immutability best practices.
Also applies to: 60-63
155-157: Verify feature flag usage across the codebase
The feature flag implementation for action redesign looks good, but we should ensure consistent usage.
Let's check for other usages of this feature flag:
#!/bin/bash
# Search for other uses of the same feature flag
rg "FEATURE_FLAG.release_actions_redesign_enabled" --type typescriptAlso applies to: 277-281
114-121: Verify chunk loading behavior
The lazy loading implementation looks good and follows the established pattern.
Let's verify other similar lazy-loaded components for consistency:
✅ Verification successful
Let me try a different approach to find lazy-loaded components and verify the chunk loading pattern. The previous command might have failed due to the typescript file type not being recognized, so I'm using explicit type flags.
Let me try without file type restrictions since ripgrep seems to have issues with typescript/tsx file types in this environment.
Chunk loading implementation is consistent with codebase patterns
The implementation follows the established pattern in the codebase:
- Uses proper webpack chunk naming convention (
webpackChunkName: "PluginActionEditor") - Follows the same dynamic import pattern as other editor components (APIEditor, QueryEditor, JSEditor)
- Located alongside other IDE-related lazy-loaded components in the same directory
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other lazy-loaded components with similar patterns
rg -U "lazy\(async \(\) =>\s+retryPromise\(" --type typescript
Length of output: 190
Script:
#!/bin/bash
# Search for lazy loading patterns with retryPromise, accounting for different whitespace
rg -U 'lazy\([^)]*retryPromise' --type ts --type tsx
# Also search for the specific import path to verify consistency
rg -U "import.*AppPluginActionEditor" --type ts --type tsx
Length of output: 171
Script:
#!/bin/bash
# Search for lazy loading patterns without file type restrictions
rg -U 'lazy\([^)]*retryPromise'
# Search for the specific import path
rg -U "import.*AppPluginActionEditor"
# Also look for other webpack chunk names to verify consistency
rg -U "webpackChunkName:.*"
Length of output: 3947
## Description - Earlier we had the new Plugin Action Editor implementation mixed in with the older API and Query Editor components. We have now separated it out so that the eventual removal of older editors is easy - Move AppPluginActionEditor outside the EE split ## Automation /ok-to-test tags="@tag.Datasource" ### 🔍 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/11835509083> > Commit: 3a7fda7 > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11835509083&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Datasource` > Spec: > <hr>Thu, 14 Nov 2024 11:25:19 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Introduced new routes for `PluginActionEditor`, `AddQuery`, and `QueryEmpty` with a standardized fallback UI. - **Bug Fixes** - Removed obsolete feature flag checks that prevented the rendering of the `AppPluginActionEditor`. - **Chores** - Deleted the `AppPluginActionEditor` component and its associated exports, streamlining the codebase. - **Refactor** - Improved the structure and clarity of hooks and component definitions. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
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/11835509083
Commit: 3a7fda7
Cypress dashboard.
Tags:
@tag.DatasourceSpec:
Thu, 14 Nov 2024 11:25:19 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
PluginActionEditor,AddQuery, andQueryEmptywith a standardized fallback UI.Bug Fixes
AppPluginActionEditor.Chores
AppPluginActionEditorcomponent and its associated exports, streamlining the codebase.Refactor