-
Notifications
You must be signed in to change notification settings - Fork 3.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
chore: Move entities/IDE/constants #39064
Conversation
WalkthroughThe changes introduce new enumerations in the editor types and a new route configuration type, while updating many import paths and re-export statements across IDE modules. Several files have been refactored to remove legacy constants and reorganize types, including refactoring the Sidebar buttons. The modifications span hooks, actions, selectors, sagas, components, and tests to align with the new module structure. Changes
Sequence Diagram(s)Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (5)
🔇 Additional comments (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
|
# Conflicts: # app/client/src/pages/Editor/IDE/EditorTabs/AddButton.tsx # app/client/src/pages/Editor/IDE/EditorTabs/AddTab.tsx # app/client/src/pages/Editor/IDE/EditorTabs/EditableTab.tsx # app/client/src/pages/Editor/IDE/EditorTabs/index.tsx # app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx # app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx
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 comments (2)
app/client/src/ce/pages/Editor/IDE/EditorPane/Query/old/ListItem.tsx (1)
1-26
:🧩 Analysis chain
Consider addressing the legacy code indicator in the file path.
The file is located in an "old" directory, which typically indicates legacy code. Consider whether this component should be migrated or deprecated.
Let's check if this is the only remaining file in the "old" directory and if there are any newer versions:
🏁 Script executed:
#!/bin/bash # Check for files in the old directory echo "Files in old directory:" fd . "app/client/src/ce/pages/Editor/IDE/EditorPane/Query/old" # Check for potential newer versions echo -e "\nPotential newer versions:" fd "ListItem.tsx" "app/client/src/ce/pages/Editor/IDE/EditorPane/Query" -E "*/old/*"Length of output: 472
Legacy Code Alert: Duplicate Implementation Found
We verified that there are two versions of ListItem.tsx:
- The legacy file at
app/client/src/ce/pages/Editor/IDE/EditorPane/Query/old/ListItem.tsx
- A newer version at
app/client/src/ce/pages/Editor/IDE/EditorPane/Query/ListItem.tsx
If the new version is fully replacing the legacy behavior, consider removing or deprecating the old file and updating any references to use the newer implementation.
app/client/src/pages/Editor/IDE/EditorPane/index.tsx (1)
24-25
:🧩 Analysis chain
Address ts-expect-error comment.
The comment indicates a TypeScript error that needs to be fixed. Consider prioritizing this technical debt.
🏁 Script executed:
#!/bin/bash # Check for other ts-expect-error instances that might need attention rg "@ts-expect-error" --type ts --type tsxLength of output: 70
🏁 Script executed:
#!/bin/bash # Search for all occurrences of "@ts-expect-error" in .ts and .tsx files rg "@ts-expect-error" -g "*.{ts,tsx}"Length of output: 34034
Action: Address the ts-expect-error suppressions by fixing the underlying type issues.
In the file
app/client/src/pages/Editor/IDE/EditorPane/index.tsx
, thegap="spacing-2"
prop triggers a type error that is currently being ignored via ts-expect-error. Our investigation also shows that similar suppressions are used broadly across the codebase, indicating a pattern of bypassed type errors. Please revise the type definitions for the affected component so that the gap property (and others, if applicable) accepts the intended values without needing suppression. It might be worth opening a ticket to gradually improve these definitions across the project.
🧹 Nitpick comments (6)
app/client/src/ce/IDE/Interfaces/IDETypes.ts (1)
1-4
: LGTM! Consider adding JSDoc comments.The implementation of
IDE_TYPE
withas const
assertion is type-safe and follows best practices.Add JSDoc comments to improve documentation:
+/** Defines the available IDE types in the application */ export const IDE_TYPE = { None: "None", App: "App", } as const;
app/client/src/IDE/Interfaces/UseRoutes.ts (1)
3-8
: LGTM! Consider adding JSDoc comments.The
UseRoutes
type is well-structured for route configuration. Consider adding JSDoc comments to document the purpose of each property.+/** + * Type definition for route configuration + */ export type UseRoutes = Array<{ + /** Unique identifier for the route */ key: string; + /** React component to render for this route */ component: ComponentType; + /** Array of URL paths that match this route */ path: string[]; + /** Whether the path should match exactly */ exact: boolean; }>;app/client/src/ce/IDE/Interfaces/EntityItem.ts (1)
4-11
: LGTM! Consider adding type constraint for userPermissions.The
EntityItem
interface is well-structured. Consider adding a type constraint foruserPermissions
to be more specific than just string[].+/** Represents a specific entity item with plugin type */ export interface EntityItem { title: string; type: PluginType; key: string; icon?: ReactNode; group?: string; - userPermissions?: string[]; + userPermissions?: Array<'read' | 'write' | 'execute'>; }app/client/src/ce/navigation/FocusStrategy/index.ts (1)
5-15
: LGTM! Consider adding exhaustive type checking.The switch case handles IDE types appropriately. Consider adding exhaustive type checking to catch missing cases at compile time.
export const getIDEFocusStrategy = (type: IDEType) => { switch (type) { case IDE_TYPE.None: return NoIDEFocusStrategy; case IDE_TYPE.App: return AppIDEFocusStrategy; // Add EE cases below default: { + const _exhaustiveCheck: never = type; throw Error("Ide focus strategy not defined"); } } };
app/client/src/ce/pages/Editor/IDE/constants/SidebarButtons.ts (1)
20-24
: Consider adding type annotation for the return array.The BottomButtons function implementation is correct, but could benefit from explicit return type annotation.
-export const BottomButtons = (datasourcesExist: boolean) => [ +export const BottomButtons = (datasourcesExist: boolean): IDESidebarButton[] => [app/client/src/IDE/constants/SidebarButtons.ts (1)
11-41
: Add JSDoc documentation for exported functions.The exported button creator functions would benefit from JSDoc documentation describing their parameters and return values.
Example documentation:
+/** + * Creates an Editor button configuration + * @param urlSuffix - The URL suffix for the button + * @returns An IDESidebarButton configuration + */ export const EditorButton = (urlSuffix: string): IDESidebarButton => ({
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (82)
app/client/src/IDE/Interfaces/EditorState.ts
(1 hunks)app/client/src/IDE/Interfaces/EditorTypes.ts
(1 hunks)app/client/src/IDE/Interfaces/UseRoutes.ts
(1 hunks)app/client/src/IDE/constants/SidebarButtons.ts
(1 hunks)app/client/src/IDE/hooks/useIsInSideBySideEditor.test.tsx
(1 hunks)app/client/src/actions/ideActions.ts
(1 hunks)app/client/src/ce/IDE/Interfaces/EntityItem.ts
(1 hunks)app/client/src/ce/IDE/Interfaces/IDETypes.ts
(1 hunks)app/client/src/ce/IDE/constants/routes.ts
(1 hunks)app/client/src/ce/IDE/hooks/useParentEntityInfo.ts
(1 hunks)app/client/src/ce/PluginActionEditor/components/PluginActionResponse/hooks/usePluginActionResponseTabs.tsx
(1 hunks)app/client/src/ce/actions/helpers.ts
(1 hunks)app/client/src/ce/entities/IDE/constants.ts
(0 hunks)app/client/src/ce/entities/IDE/hooks/useCreateActionsPermissions.ts
(1 hunks)app/client/src/ce/entities/IDE/utils.ts
(2 hunks)app/client/src/ce/hooks/datasourceEditorHooks.tsx
(1 hunks)app/client/src/ce/navigation/FocusStrategy/AppIDEFocusStrategy.ts
(1 hunks)app/client/src/ce/navigation/FocusStrategy/index.ts
(1 hunks)app/client/src/ce/pages/Editor/IDE/EditorPane/JS/ListItem.tsx
(1 hunks)app/client/src/ce/pages/Editor/IDE/EditorPane/JS/hooks.tsx
(1 hunks)app/client/src/ce/pages/Editor/IDE/EditorPane/JS/old/ListItem.tsx
(1 hunks)app/client/src/ce/pages/Editor/IDE/EditorPane/JS/utils/getJSContextMenuByIdeType.tsx
(1 hunks)app/client/src/ce/pages/Editor/IDE/EditorPane/JS/utils/getJSEntityItemUrl.ts
(1 hunks)app/client/src/ce/pages/Editor/IDE/EditorPane/JS/utils/getJSUrl.test.ts
(1 hunks)app/client/src/ce/pages/Editor/IDE/EditorPane/Query/ListItem.tsx
(1 hunks)app/client/src/ce/pages/Editor/IDE/EditorPane/Query/hooks.tsx
(2 hunks)app/client/src/ce/pages/Editor/IDE/EditorPane/Query/old/ListItem.tsx
(1 hunks)app/client/src/ce/pages/Editor/IDE/EditorPane/Query/utils/getQueryContextMenuByIdeType.tsx
(1 hunks)app/client/src/ce/pages/Editor/IDE/EditorPane/Query/utils/getQueryEntityItemUrl.ts
(1 hunks)app/client/src/ce/pages/Editor/IDE/EditorPane/Query/utils/getQueryUrl.test.ts
(1 hunks)app/client/src/ce/pages/Editor/IDE/constants/SidebarButtons.ts
(1 hunks)app/client/src/ce/pages/Editor/gitSync/useReconnectModalData.ts
(1 hunks)app/client/src/ce/sagas/DatasourcesSagas.ts
(1 hunks)app/client/src/ce/sagas/JSActionSagas.ts
(1 hunks)app/client/src/ce/sagas/NavigationSagas.ts
(1 hunks)app/client/src/ce/selectors/appIDESelectors.test.ts
(1 hunks)app/client/src/ce/selectors/appIDESelectors.ts
(1 hunks)app/client/src/ce/selectors/entitiesSelector.ts
(1 hunks)app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.tsx
(1 hunks)app/client/src/ce/utils/lintRulesHelpers.ts
(1 hunks)app/client/src/components/editorComponents/ApiResponseView.test.tsx
(1 hunks)app/client/src/components/editorComponents/ApiResponseView.tsx
(1 hunks)app/client/src/components/editorComponents/Debugger/DebuggerTabs.tsx
(1 hunks)app/client/src/components/editorComponents/Debugger/StateInspector/hooks/useStateInspectorItems.ts
(1 hunks)app/client/src/components/editorComponents/Debugger/hooks/useDebuggerTriggerClick.ts
(1 hunks)app/client/src/components/editorComponents/JSResponseView.test.tsx
(1 hunks)app/client/src/components/editorComponents/JSResponseView.tsx
(1 hunks)app/client/src/ee/IDE/Interfaces/EntityItem.ts
(1 hunks)app/client/src/ee/IDE/Interfaces/IDETypes.ts
(1 hunks)app/client/src/ee/IDE/constants/routes.ts
(1 hunks)app/client/src/ee/pages/Editor/IDE/constants/SidebarButtons.ts
(1 hunks)app/client/src/layoutSystems/common/dropTarget/OnBoarding/OnBoarding.test.tsx
(1 hunks)app/client/src/navigation/FocusEntity.test.ts
(1 hunks)app/client/src/navigation/FocusEntity.ts
(1 hunks)app/client/src/pages/Editor/CanvasLayoutConversion/hooks/useShowSnapShotBanner.ts
(1 hunks)app/client/src/pages/Editor/DataSourceEditor/Debugger.test.tsx
(1 hunks)app/client/src/pages/Editor/Explorer/Entity/EntityProperties.tsx
(1 hunks)app/client/src/pages/Editor/IDE/EditorPane/Editor.tsx
(1 hunks)app/client/src/pages/Editor/IDE/EditorPane/Explorer.tsx
(1 hunks)app/client/src/pages/Editor/IDE/EditorPane/JS/Add.tsx
(1 hunks)app/client/src/pages/Editor/IDE/EditorPane/JS/EntityItem/JSEntityItem.tsx
(1 hunks)app/client/src/pages/Editor/IDE/EditorPane/JS/Explorer.tsx
(1 hunks)app/client/src/pages/Editor/IDE/EditorPane/JS/JSRender.test.tsx
(1 hunks)app/client/src/pages/Editor/IDE/EditorPane/JS/List.tsx
(1 hunks)app/client/src/pages/Editor/IDE/EditorPane/Query/Add.tsx
(1 hunks)app/client/src/pages/Editor/IDE/EditorPane/Query/EntityItem/QueryEntityItem.tsx
(1 hunks)app/client/src/pages/Editor/IDE/EditorPane/Query/Explorer.tsx
(1 hunks)app/client/src/pages/Editor/IDE/EditorPane/Query/List.tsx
(1 hunks)app/client/src/pages/Editor/IDE/EditorPane/Query/QueryRender.test.tsx
(1 hunks)app/client/src/pages/Editor/IDE/EditorPane/UI/UIRender.test.tsx
(1 hunks)app/client/src/pages/Editor/IDE/EditorPane/components/SegmentSwitcher.tsx
(1 hunks)app/client/src/pages/Editor/IDE/EditorPane/index.tsx
(1 hunks)app/client/src/pages/Editor/IDE/EditorTabs/AddButton.tsx
(1 hunks)app/client/src/pages/Editor/IDE/EditorTabs/AddTab.tsx
(1 hunks)app/client/src/pages/Editor/IDE/EditorTabs/EditableTab.tsx
(1 hunks)app/client/src/pages/Editor/IDE/EditorTabs/Editortabs.test.tsx
(1 hunks)app/client/src/pages/Editor/IDE/EditorTabs/List.tsx
(1 hunks)app/client/src/pages/Editor/IDE/EditorTabs/ScreenModeToggle.tsx
(1 hunks)app/client/src/pages/Editor/IDE/EditorTabs/constants.ts
(2 hunks)app/client/src/pages/Editor/IDE/EditorTabs/index.tsx
(2 hunks)app/client/src/pages/Editor/IDE/Header/index.tsx
(1 hunks)app/client/src/pages/Editor/IDE/Layout/hooks/useEditorStateLeftPaneWidth.ts
(1 hunks)
⛔ Files not processed due to max files limit (26)
- app/client/src/pages/Editor/IDE/Layout/hooks/useGridLayoutTemplate.ts
- app/client/src/pages/Editor/IDE/Sidebar.tsx
- app/client/src/pages/Editor/IDE/hooks.ts
- app/client/src/pages/Editor/IDE/hooks/useCurrentAppState.ts
- app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx
- app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx
- app/client/src/pages/Editor/QueryEditor/QueryDebuggerTabs.tsx
- app/client/src/pages/Editor/WidgetsEditor/WidgetEditorContainer.tsx
- app/client/src/pages/Editor/WidgetsEditor/components/CodeModeTooltip.tsx
- app/client/src/pages/Editor/WidgetsEditor/components/NavigationAdjustedPageViewer.tsx
- app/client/src/pages/Editor/WidgetsEditor/components/NavigationPreview.tsx
- app/client/src/pages/Editor/WidgetsEditor/components/WidgetEditorNavigation.tsx
- app/client/src/pages/Editor/utils.tsx
- app/client/src/plugins/Linting/constants.ts
- app/client/src/plugins/Linting/utils/getLintingErrors.ts
- app/client/src/reducers/uiReducers/ideReducer.ts
- app/client/src/sagas/ActionSagas.ts
- app/client/src/sagas/AnalyticsSaga.ts
- app/client/src/sagas/DebuggerSagas.ts
- app/client/src/sagas/FocusRetentionSaga.ts
- app/client/src/sagas/IDESaga.tsx
- app/client/src/sagas/JSPaneSagas.ts
- app/client/src/sagas/getNextEntityAfterRemove.test.ts
- app/client/src/selectors/ideSelectors.tsx
- app/client/src/utils/hooks/useHoverToFocusWidget.ts
- app/client/src/utils/storage.ts
💤 Files with no reviewable changes (1)
- app/client/src/ce/entities/IDE/constants.ts
✅ Files skipped from review due to trivial changes (47)
- app/client/src/ee/IDE/constants/routes.ts
- app/client/src/ee/pages/Editor/IDE/constants/SidebarButtons.ts
- app/client/src/ee/IDE/Interfaces/IDETypes.ts
- app/client/src/IDE/Interfaces/EditorState.ts
- app/client/src/pages/Editor/IDE/Header/index.tsx
- app/client/src/ee/IDE/Interfaces/EntityItem.ts
- app/client/src/pages/Editor/Explorer/Entity/EntityProperties.tsx
- app/client/src/pages/Editor/IDE/EditorTabs/ScreenModeToggle.tsx
- app/client/src/components/editorComponents/Debugger/hooks/useDebuggerTriggerClick.ts
- app/client/src/components/editorComponents/Debugger/StateInspector/hooks/useStateInspectorItems.ts
- app/client/src/pages/Editor/IDE/EditorPane/Query/EntityItem/QueryEntityItem.tsx
- app/client/src/ce/sagas/JSActionSagas.ts
- app/client/src/ce/pages/Editor/IDE/EditorPane/JS/utils/getJSEntityItemUrl.ts
- app/client/src/ce/pages/Editor/IDE/EditorPane/Query/utils/getQueryEntityItemUrl.ts
- app/client/src/ce/PluginActionEditor/components/PluginActionResponse/hooks/usePluginActionResponseTabs.tsx
- app/client/src/ce/navigation/FocusStrategy/AppIDEFocusStrategy.ts
- app/client/src/ce/utils/lintRulesHelpers.ts
- app/client/src/pages/Editor/IDE/EditorTabs/Editortabs.test.tsx
- app/client/src/ce/IDE/hooks/useParentEntityInfo.ts
- app/client/src/ce/actions/helpers.ts
- app/client/src/ce/pages/Editor/IDE/EditorPane/Query/utils/getQueryUrl.test.ts
- app/client/src/pages/Editor/IDE/EditorPane/Query/List.tsx
- app/client/src/ce/selectors/appIDESelectors.test.ts
- app/client/src/ce/pages/Editor/IDE/EditorPane/Query/ListItem.tsx
- app/client/src/components/editorComponents/JSResponseView.test.tsx
- app/client/src/pages/Editor/IDE/EditorPane/UI/UIRender.test.tsx
- app/client/src/pages/Editor/IDE/EditorPane/JS/Explorer.tsx
- app/client/src/actions/ideActions.ts
- app/client/src/components/editorComponents/JSResponseView.tsx
- app/client/src/ce/entities/IDE/hooks/useCreateActionsPermissions.ts
- app/client/src/components/editorComponents/ApiResponseView.tsx
- app/client/src/pages/Editor/IDE/EditorPane/JS/Add.tsx
- app/client/src/IDE/hooks/useIsInSideBySideEditor.test.tsx
- app/client/src/ce/pages/Editor/IDE/EditorPane/JS/old/ListItem.tsx
- app/client/src/pages/Editor/IDE/EditorPane/JS/EntityItem/JSEntityItem.tsx
- app/client/src/pages/Editor/IDE/EditorPane/Query/QueryRender.test.tsx
- app/client/src/ce/selectors/appIDESelectors.ts
- app/client/src/components/editorComponents/Debugger/DebuggerTabs.tsx
- app/client/src/ce/pages/Editor/IDE/EditorPane/JS/utils/getJSUrl.test.ts
- app/client/src/ce/pages/Editor/IDE/EditorPane/JS/utils/getJSContextMenuByIdeType.tsx
- app/client/src/ce/pages/Editor/IDE/EditorPane/Query/hooks.tsx
- app/client/src/ce/pages/Editor/IDE/EditorPane/JS/ListItem.tsx
- app/client/src/ce/sagas/DatasourcesSagas.ts
- app/client/src/ce/pages/Editor/IDE/EditorPane/Query/utils/getQueryContextMenuByIdeType.tsx
- app/client/src/navigation/FocusEntity.ts
- app/client/src/ce/pages/Editor/IDE/EditorPane/JS/hooks.tsx
- app/client/src/pages/Editor/IDE/EditorPane/Query/Add.tsx
🔇 Additional comments (33)
app/client/src/ce/IDE/Interfaces/IDETypes.ts (1)
6-6
: LGTM! Type definition is correct.The type definition using
keyof typeof
creates a union type of the literal keys.app/client/src/ce/pages/Editor/gitSync/useReconnectModalData.ts (1)
1-1
: LGTM! Import path updated correctly.The import path change aligns with the PR objective of moving IDE constants.
app/client/src/ce/hooks/datasourceEditorHooks.tsx (1)
26-26
: LGTM! Import path updated correctly.The import path changes align with the PR objective of moving IDE constants.
app/client/src/ce/sagas/NavigationSagas.ts (1)
24-25
: LGTM! Import paths updated correctly.The import path changes align with the PR objective of moving IDE constants.
app/client/src/navigation/FocusEntity.test.ts (2)
3-3
: LGTM! Import path update aligns with code reorganization.The import path change from
ee/entities/IDE/constants
toIDE/Interfaces/EditorState
correctly reflects the new location of IDE constants.
13-382
: Comprehensive test coverage maintained.The test suite thoroughly covers all URL patterns and entity identification scenarios. The unchanged test cases continue to validate the correct behavior after the refactoring.
Run this script to verify that all test cases pass with the new import:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Verify that the test file compiles and all tests pass # Note: Only verifying import resolution as build tools aren't available # Test: Check if the new import path exists fd --type f "EditorState.ts" "app/client/src/IDE/Interfaces"Length of output: 103
Test suite and import verification successful.
- The new import at
app/client/src/IDE/Interfaces/EditorState.ts
was confirmed to exist.- The comprehensive test coverage in
FocusEntity.test.ts
validates that URL patterns and entity identification continue to work as expected.app/client/src/ce/pages/Editor/IDE/EditorPane/Query/old/ListItem.tsx (1)
3-3
: LGTM! Import path update aligns with codebase reorganization.The updated import path for
EntityItem
follows the new structure, moving IDE-related types to a dedicated folder.app/client/src/pages/Editor/CanvasLayoutConversion/hooks/useShowSnapShotBanner.ts (1)
2-2
: Verify the new import path and enum values.The import path change aligns with the PR's objective of reorganizing IDE constants. Let's verify the new structure.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Verify the new EditorState interface exists and contains the required SETTINGS value ast-grep --pattern 'enum EditorState { $$$ SETTINGS $$$ }'Length of output: 66
I'm not seeing evidence of the expected enum declaration. Let's dig a bit deeper to confirm the existence and content of the file that should export the
EditorState
enum.
🏁 Script executed:
#!/bin/bash # 1. Locate any TypeScript file related to EditorState. echo "Searching for EditorState.ts files..." fd --glob '*EditorState.ts' # 2. If found, display its contents (preview first 25 lines). found=$(fd --glob '*EditorState.ts' | head -n 1) if [ -z "$found" ]; then echo "No EditorState.ts file found." else echo "Displaying content of $found:" head -n 25 "$found" fi # 3. Also, search the repository for the enum declaration specifically. echo "Searching for 'export enum EditorState' declarations..." rg "export enum EditorState" -A 10Length of output: 1258
Import path and enum verification successful.
- The file located at
app/client/src/IDE/Interfaces/EditorState.ts
correctly exports theEditorState
enum.- The expected
SETTINGS
value is present.app/client/src/IDE/Interfaces/EditorTypes.ts (1)
1-16
: LGTM! Well-structured enums.The enums are well-organized with descriptive values. Using string literals is a good choice for serialization compatibility.
app/client/src/ce/IDE/Interfaces/EntityItem.ts (1)
13-13
: LGTM! Good use of Omit utility type.The
GenericEntityItem
interface effectively reusesEntityItem
structure while omitting the type property.app/client/src/ce/pages/Editor/IDE/constants/SidebarButtons.ts (1)
12-16
: LGTM! Well-structured button configuration with warning state.The DataButtonWithWarning configuration properly extends the base DataButton with warning condition and tooltip.
app/client/src/pages/Editor/IDE/EditorPane/Query/Explorer.tsx (1)
7-7
: LGTM! Import path updated correctly.The EditorViewMode import path has been properly updated to align with the new IDE constants organization.
app/client/src/pages/Editor/IDE/EditorTabs/List.tsx (1)
6-6
: LGTM! Import path updated correctly.The EditorEntityTab import path has been properly updated to align with the new IDE constants organization.
app/client/src/pages/Editor/IDE/EditorTabs/AddButton.tsx (1)
5-5
: LGTM! Import path updated correctly.The EditorEntityTabState import path has been properly updated to align with the new IDE constants organization.
app/client/src/pages/Editor/IDE/EditorTabs/AddTab.tsx (1)
5-8
: LGTM! Import path update aligns with PR objectives.The import path change from 'ee/entities/IDE/constants' to 'IDE/Interfaces/EditorTypes' is consistent with the goal of relocating IDE constants.
app/client/src/pages/Editor/IDE/EditorPane/components/SegmentSwitcher.tsx (1)
2-3
: LGTM! Consistent import path update.Import path change follows the same pattern as other files, maintaining consistency across the codebase.
app/client/src/IDE/constants/SidebarButtons.ts (1)
4-9
: LGTM! Well-structured constants.Good use of a centralized object for button titles, making maintenance easier.
app/client/src/pages/Editor/IDE/EditorPane/index.tsx (1)
7-7
: LGTM! Consistent import path update.Import path change aligns with the codebase reorganization.
app/client/src/ce/IDE/constants/routes.ts (2)
25-40
: LGTM! Well-organized consolidation of entity paths.The
EntityPaths
array provides a centralized location for all editor-related paths, making it easier to maintain and track routing configurations.
41-44
: Consider future extensibility of IDEBasePaths.The
IDEBasePaths
constant currently only handles 'None' and 'App' types. Consider documenting if other IDE types will be added in the future.app/client/src/pages/Editor/IDE/EditorPane/Editor.tsx (1)
13-13
: LGTM! Import path updated correctly.The import path change aligns with the new module structure.
app/client/src/pages/Editor/IDE/EditorTabs/constants.ts (1)
1-1
: LGTM! Import paths updated consistently.The import path changes align with the new module structure and maintain consistency across the codebase.
Also applies to: 13-13
app/client/src/pages/Editor/IDE/Layout/hooks/useEditorStateLeftPaneWidth.ts (1)
11-11
: LGTM! Import path updated correctly.The import path change aligns with the new module structure.
app/client/src/pages/Editor/IDE/EditorPane/Explorer.tsx (1)
21-21
: LGTM! Import path update aligns with the IDE reorganization.app/client/src/pages/Editor/DataSourceEditor/Debugger.test.tsx (1)
9-9
: LGTM! Import path update maintains consistency.app/client/src/pages/Editor/IDE/EditorTabs/EditableTab.tsx (1)
18-18
: LGTM! Import path update is consistent with the IDE reorganization.app/client/src/components/editorComponents/ApiResponseView.test.tsx (1)
10-10
: LGTM!The import path update aligns with the PR's objective to reorganize IDE constants.
app/client/src/pages/Editor/IDE/EditorPane/JS/List.tsx (1)
28-28
: LGTM!The import path update for EntityItem type is consistent with the codebase reorganization.
app/client/src/pages/Editor/IDE/EditorTabs/index.tsx (1)
6-9
: LGTM!The import path updates for editor types are consistent with the codebase reorganization.
Also applies to: 29-29
app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.tsx (1)
46-46
: LGTM!The import path update for IDE types is consistent with the codebase reorganization.
app/client/src/layoutSystems/common/dropTarget/OnBoarding/OnBoarding.test.tsx (1)
2-3
: LGTM! Import paths correctly updated.The import paths for
EditorState
andEditorEntityTab
have been properly updated to reflect their new locations in the IDE interfaces directory.app/client/src/pages/Editor/IDE/EditorPane/JS/JSRender.test.tsx (1)
8-8
: LGTM! Import path correctly updated.The import path for
EditorEntityTab
andEditorViewMode
has been properly updated to reflect their new location in the IDE interfaces directory.app/client/src/ce/selectors/entitiesSelector.ts (1)
63-68
: LGTM! Import paths correctly updated.The import paths have been properly updated to reflect the new locations of IDE-related types:
EditorEntityTab
from IDE/Interfaces/EditorTypesIDEType
from ee/IDE/Interfaces/IDETypesEntityItem
andGenericEntityItem
from ee/IDE/Interfaces/EntityItem
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/IDE/enums.ts (1)
1-5
: Remove commented out code.The commented out enum members
Error
andSuccess
should be removed if they are no longer needed.export enum Condition { Warn = "Warn", - // Error = "Error", - // Success = "Success", }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (23)
app/client/src/IDE/constants/SidebarButtons.ts
(1 hunks)app/client/src/IDE/enums.ts
(1 hunks)app/client/src/ce/entities/IDE/utils.ts
(2 hunks)app/client/src/ce/navigation/FocusStrategy/AppIDEFocusStrategy.ts
(1 hunks)app/client/src/ce/pages/Editor/IDE/EditorPane/JS/utils/getJSUrl.test.ts
(1 hunks)app/client/src/ce/pages/Editor/IDE/EditorPane/Query/utils/getQueryUrl.test.ts
(1 hunks)app/client/src/ce/pages/Editor/IDE/constants/SidebarButtons.ts
(1 hunks)app/client/src/layoutSystems/common/dropTarget/OnBoarding/OnBoarding.test.tsx
(1 hunks)app/client/src/navigation/FocusEntity.test.ts
(1 hunks)app/client/src/navigation/FocusEntity.ts
(1 hunks)app/client/src/pages/Editor/CanvasLayoutConversion/hooks/useShowSnapShotBanner.ts
(1 hunks)app/client/src/pages/Editor/IDE/Header/index.tsx
(1 hunks)app/client/src/pages/Editor/IDE/Layout/hooks/useGridLayoutTemplate.ts
(1 hunks)app/client/src/pages/Editor/IDE/Sidebar.tsx
(2 hunks)app/client/src/pages/Editor/IDE/hooks/useCurrentAppState.ts
(1 hunks)app/client/src/pages/Editor/WidgetsEditor/WidgetEditorContainer.tsx
(1 hunks)app/client/src/pages/Editor/WidgetsEditor/components/CodeModeTooltip.tsx
(1 hunks)app/client/src/pages/Editor/WidgetsEditor/components/NavigationAdjustedPageViewer.tsx
(1 hunks)app/client/src/pages/Editor/WidgetsEditor/components/NavigationPreview.tsx
(1 hunks)app/client/src/pages/Editor/WidgetsEditor/components/WidgetEditorNavigation.tsx
(1 hunks)app/client/src/pages/Editor/utils.tsx
(1 hunks)app/client/src/sagas/getNextEntityAfterRemove.test.ts
(1 hunks)app/client/src/utils/hooks/useHoverToFocusWidget.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (7)
- app/client/src/pages/Editor/WidgetsEditor/components/NavigationAdjustedPageViewer.tsx
- app/client/src/utils/hooks/useHoverToFocusWidget.ts
- app/client/src/pages/Editor/WidgetsEditor/components/WidgetEditorNavigation.tsx
- app/client/src/pages/Editor/WidgetsEditor/components/CodeModeTooltip.tsx
- app/client/src/pages/Editor/WidgetsEditor/components/NavigationPreview.tsx
- app/client/src/pages/Editor/IDE/hooks/useCurrentAppState.ts
- app/client/src/pages/Editor/IDE/Layout/hooks/useGridLayoutTemplate.ts
🚧 Files skipped from review as they are similar to previous changes (10)
- app/client/src/pages/Editor/IDE/Header/index.tsx
- app/client/src/ce/pages/Editor/IDE/EditorPane/JS/utils/getJSUrl.test.ts
- app/client/src/ce/navigation/FocusStrategy/AppIDEFocusStrategy.ts
- app/client/src/layoutSystems/common/dropTarget/OnBoarding/OnBoarding.test.tsx
- app/client/src/ce/pages/Editor/IDE/EditorPane/Query/utils/getQueryUrl.test.ts
- app/client/src/pages/Editor/CanvasLayoutConversion/hooks/useShowSnapShotBanner.ts
- app/client/src/ce/entities/IDE/utils.ts
- app/client/src/navigation/FocusEntity.test.ts
- app/client/src/navigation/FocusEntity.ts
- app/client/src/IDE/constants/SidebarButtons.ts
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
🔇 Additional comments (9)
app/client/src/pages/Editor/WidgetsEditor/WidgetEditorContainer.tsx (1)
8-8
: LGTM! Import path update aligns with the code reorganization.The updated import path for
EditorState
fromIDE/enums
matches the PR's objective of moving IDE constants to a common IDE folder.Let's verify that all other files using
EditorState
have been updated consistently:✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check for any remaining imports of EditorState from the old path rg "import.*EditorState.*from.*entities/IDE/constants"Length of output: 56
LGTM! All instances of the old import have been removed.
The search for any remaining imports of
EditorState
fromentities/IDE/constants
produced no results, which indicates that the refactoring has been applied consistently.app/client/src/IDE/enums.ts (1)
7-12
: LGTM!The
EditorState
enum follows good naming conventions and has clear, descriptive values.app/client/src/ce/pages/Editor/IDE/constants/SidebarButtons.ts (2)
13-17
: LGTM!Good use of spread operator to extend the base button configuration with warning condition.
21-26
: LGTM!Clean implementation of conditional button configuration based on datasource existence.
app/client/src/sagas/getNextEntityAfterRemove.test.ts (1)
1-6
: LGTM!Import paths have been correctly updated to reflect the new module organization.
app/client/src/pages/Editor/IDE/Sidebar.tsx (2)
11-14
: LGTM!The
TopButtons
has been correctly moved toIDE/constants/SidebarButtons
as suggested in the previous review.
24-27
: LGTM!Clean refactor of bottom buttons configuration using the new
BottomButtons
function.app/client/src/pages/Editor/utils.tsx (2)
36-40
: LGTM! Import paths updated correctly.The import paths have been updated to reflect the new organizational structure, aligning with the PR objective of moving IDE constants to a common IDE folder.
46-55
: Address TODO comments for eslint issues.There are multiple TODO comments about fixing eslint issues related to
@typescript-eslint/no-explicit-any
. Consider addressing these by:
- Adding proper type definitions for the parameters
- Using more specific types instead of
any
Would you like me to help create proper type definitions for these parameters?
…o chore/update-sidebar-buttons
…o chore/update-sidebar-buttons
# Conflicts: # app/client/src/pages/Editor/IDE/EditorPane/Explorer.tsx
…to chore/update-sidebar-buttons
## Description Remove the IDE constants from the entities folder and move it to the common IDE folder Create separate buttons exports for sidebar buttons Update the button usages Fixes appsmithorg#39050 ## Automation /ok-to-test tags="@tag.All" ### 🔍 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/13191919670> > Commit: 5055b6e > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=13191919670&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Fri, 07 Feb 2025 05:19:56 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 editor state definitions and enhanced sidebar configurations with conditional messaging for data source availability. - **Refactor** - Reorganized internal module structures and consolidated type definitions to improve maintainability, consistency, and type safety. - **Tests** - Updated test references to align with the new module organization. These changes streamline the codebase and lay the groundwork for future improvements while maintaining the existing end-user experience. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Ankita Kinger <[email protected]>
Description
Remove the IDE constants from the entities folder and move it to the common IDE folder
Create separate buttons exports for sidebar buttons
Update the button usages
Fixes #39050
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13191919670
Commit: 5055b6e
Cypress dashboard.
Tags:
@tag.All
Spec:
Fri, 07 Feb 2025 05:19:56 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Refactor
Tests
These changes streamline the codebase and lay the groundwork for future improvements while maintaining the existing end-user experience.