chore: Refactoring usage of editorType to ideType to remove code duplication
#38778
Conversation
WalkthroughThis pull request introduces a comprehensive refactoring of the editor type handling across multiple components in the application. The primary focus is on replacing the Changes
Sequence DiagramsequenceDiagram
participant URL
participant getIDETypeByUrl
participant Component
participant useParentEntityInfo
URL->>getIDETypeByUrl: Determine IDE Type
getIDETypeByUrl-->>Component: Return ideType
Component->>useParentEntityInfo: Pass ideType
useParentEntityInfo-->>Component: Return parent entity info
Possibly related PRs
Suggested Labels
Suggested Reviewers
Poem
✨ Finishing Touches
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
ankitakinger
added
ok-to-test
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: 2
🔭 Outside diff range comments (2)
app/client/src/ce/actions/helpers.ts (2)
Line range hint
67-83: Consider removing redundantparentEntityTypeparameter.The function still uses
parentEntityTypewhile being refactored to useideType. This seems inconsistent with the overall refactoring strategy.export const createNewApiActionBasedOnIdeType = ( ideType: IDEType, editorId: string, parentEntityId: string, - parentEntityType: ActionParentEntityTypeInterface, apiType: string, ) => { if (parentEntityId) { return createNewAPIBasedOnParentEntity( parentEntityId, "API_PANE", apiType, - parentEntityType, ); } };
The TODO comment should be addressed in this PR since we're editing the file
The TODO comment specifically states "Fix this the next time the file is edited". Since we're currently editing this file, this is the appropriate time to properly type the function return value instead of using
any.🔗 Analysis chain
Line range hint
73-73: Address the TODO comment in the next iteration.The TODO comment indicates there might be more refactoring needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for other TODO comments related to this refactoring rg -l "TODO.*next.*time.*file.*edited" --type tsLength of output: 56233
Script:
#!/bin/bash # Get TODO comments with context from the specific file rg -A 3 -B 3 "TODO" app/client/src/ce/actions/helpers.tsLength of output: 308
🧹 Nitpick comments (9)
app/client/src/pages/Editor/DatasourceInfo/HideGeneratePageButton.test.tsx (1)
120-121: LGTM! Consider extracting the URL structure.The pathname change aligns with the IDE type refactoring. Consider extracting this URL structure into a constant or utility function to maintain consistency across tests.
+ // Add to test utils or constants + const generateDatasourceUrl = (appName: string, pageId: string, datasourceId: string) => + `/app/${appName}/${pageId}/edit/datasource/${datasourceId}`; const mockHistoryLocation = { - pathname: "/app/untitled-application-1/page1-678a356f18346f618bc2c80a/edit/datasource/users-ds-id", + pathname: generateDatasourceUrl("untitled-application-1", "page1-678a356f18346f618bc2c80a", "users-ds-id"), search: "", hash: "", state: {}, };app/client/src/pages/Editor/IDE/EditorPane/JS/List.tsx (1)
80-112: Consider memoizing the mapped components for better performance.The mapping operation inside the render could benefit from memoization, especially when dealing with large lists.
+ const MemoizedJSListItem = React.memo(JSListItem); // ... inside the mapping function - <JSListItem + <MemoizedJSListItem isActive={item.key === activeActionBaseId} item={item} key={item.key} parentEntityId={pageId} />app/client/src/ce/pages/Editor/IDE/EditorPane/Query/utils/getQueryEntityItemUrl.ts (1)
4-15: LGTM! Consider enhancing error handling.The implementation is clean and follows single responsibility principle. Consider using a custom error class for better error handling downstream.
+class QueryEntityUrlError extends Error { + constructor(pluginType: string) { + super(`Cannot find url of plugin type ${pluginType}`); + this.name = 'QueryEntityUrlError'; + } +} export const getQueryEntityItemUrl = ( item: EntityItem, basePageId: string, ): string => { const config = getActionConfig(item.type); if (!config) { - throw Error(`Cannot find url of plugin type ${item.type}`); + throw new QueryEntityUrlError(item.type); } return config.getURL(basePageId, item.key, item.type); };app/client/src/ce/IDE/hooks/useParentEntityInfo.ts (1)
14-19: Consider memoizing the return object.The return object is recreated on every render. Consider using useMemo to optimize performance.
+import { useMemo } from "react"; export const useParentEntityInfo = (ideType: IDEType) => { const appId = useSelector(getCurrentApplicationId); const basePageId = useSelector(getCurrentBasePageId); - return { + return useMemo(() => ({ editorId: appId || "", parentEntityId: basePageId || "", parentEntityType: ActionParentEntityType.PAGE, - }; + }), [appId, basePageId]); };app/client/src/ce/entities/IDE/hooks/useCreateActionsPermissions.ts (1)
8-20: Add unit tests for the new hookThis new hook requires test coverage to ensure correct permission handling across different IDE types.
Would you like me to generate unit test cases for this hook?
app/client/src/pages/Editor/Explorer/JSActions/JSActionEntity.tsx (1)
37-41: Consider memoizing the base parent entity ID selector.The selector could benefit from memoization to prevent unnecessary recalculations.
+const baseParentEntityIdSelector = useMemo( + () => convertToBaseParentEntityIdSelector(state, parentEntityId), + [state, parentEntityId] +); -const baseParentEntityId = useSelector((state) => - convertToBaseParentEntityIdSelector(state, parentEntityId), -);app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.tsx (1)
46-46: Type safety improvement in permission checks.The refactoring improves type safety by:
- Using
IDETypeinstead of string- Using
IDE_TYPE.Appconstant instead of string literalConsider adding JSDoc comments to document the parameters and return type.
Also applies to: 170-175, 179-182
app/client/src/plugins/Linting/utils/getLintingErrors.ts (1)
84-84: Consider using type assertion for default ideType.While the initialization and usage are correct, consider using type assertion for better type safety:
- let ideType: IDEType = IDE_TYPE.App; + let ideType = IDE_TYPE.App as IDEType;Also applies to: 143-143
app/client/src/ce/selectors/entitiesSelector.ts (1)
1705-1715: Consider consistent string template usage.While the type changes are good, the string interpolation could be more consistent:
- actionDsMap[ds.id] = `No queries in this ${ideType.toLowerCase()}`; - actionDsMap[dsId] = - `${actionCount[dsId]} queries in this ${ideType.toLowerCase()}`; + const contextName = ideType.toLowerCase(); + actionDsMap[ds.id] = `No queries in this ${contextName}`; + actionDsMap[dsId] = `${actionCount[dsId]} queries in this ${contextName}`;This change:
- Avoids repeated toLowerCase() calls
- Makes the string templates more readable
- Maintains consistent formatting
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (60)
app/client/src/PluginActionEditor/components/PluginActionResponse/components/DatasourceTab/DatasourceInfo.tsx(2 hunks)app/client/src/PluginActionEditor/components/PluginActionResponse/components/DatasourceTab/DatasourceTab.tsx(3 hunks)app/client/src/ce/IDE/hooks/useParentEntityInfo.ts(1 hunks)app/client/src/ce/actions/helpers.ts(2 hunks)app/client/src/ce/entities/IDE/hooks/useCreateActionsPermissions.ts(1 hunks)app/client/src/ce/hooks/datasourceEditorHooks.tsx(3 hunks)app/client/src/ce/hooks/hooks.test.ts(0 hunks)app/client/src/ce/hooks/index.ts(0 hunks)app/client/src/ce/navigation/FocusSetters.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/utils/getJSEntityItemUrl.test.ts(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/JS/utils/getJSUrl.ts(0 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(1 hunks)app/client/src/ce/pages/Editor/IDE/EditorPane/Query/utils/getQueryEntityItemUrl.test.ts(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/EditorPane/Query/utils/getQueryUrl.ts(1 hunks)app/client/src/ce/pages/Editor/gitSync/useReconnectModalData.ts(2 hunks)app/client/src/ce/selectors/appIDESelectors.ts(1 hunks)app/client/src/ce/selectors/entitiesSelector.ts(2 hunks)app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.tsx(2 hunks)app/client/src/ce/utils/lintRulesHelpers.ts(1 hunks)app/client/src/components/editorComponents/form/fields/DropdownWrapper.tsx(1 hunks)app/client/src/ee/IDE/hooks/useParentEntityInfo.ts(1 hunks)app/client/src/ee/entities/IDE/hooks/useCreateActionsPermissions.ts(1 hunks)app/client/src/ee/pages/Editor/IDE/EditorPane/JS/utils.ts(0 hunks)app/client/src/ee/pages/Editor/IDE/EditorPane/JS/utils/getJSEntityItemUrl.ts(1 hunks)app/client/src/ee/pages/Editor/IDE/EditorPane/JS/utils/getJSUrl.ts(1 hunks)app/client/src/ee/pages/Editor/IDE/EditorPane/Query/utils.ts(0 hunks)app/client/src/ee/pages/Editor/IDE/EditorPane/Query/utils/getQueryEntityItemUrl.ts(1 hunks)app/client/src/ee/pages/Editor/IDE/EditorPane/Query/utils/getQueryUrl.ts(1 hunks)app/client/src/pages/Editor/CustomWidgetBuilder/useCustomBuilder.tsx(2 hunks)app/client/src/pages/Editor/DataSourceEditor/DSFormHeader.tsx(3 hunks)app/client/src/pages/Editor/DataSourceEditor/hooks.ts(3 hunks)app/client/src/pages/Editor/DatasourceInfo/DatasourceStructure.tsx(2 hunks)app/client/src/pages/Editor/DatasourceInfo/DatasourceViewModeSchema.tsx(3 hunks)app/client/src/pages/Editor/DatasourceInfo/GoogleSheetSchema.tsx(3 hunks)app/client/src/pages/Editor/DatasourceInfo/HideGeneratePageButton.test.tsx(1 hunks)app/client/src/pages/Editor/Explorer/Actions/ActionEntity.tsx(3 hunks)app/client/src/pages/Editor/Explorer/Files/index.tsx(0 hunks)app/client/src/pages/Editor/Explorer/JSActions/JSActionEntity.tsx(2 hunks)app/client/src/pages/Editor/IDE/EditorPane/JS/List.tsx(1 hunks)app/client/src/pages/Editor/IDE/EditorPane/Query/List.tsx(0 hunks)app/client/src/pages/Editor/IDE/EditorTabs/constants.ts(1 hunks)app/client/src/pages/Editor/IDE/LeftPane/DataSidePane.test.tsx(2 hunks)app/client/src/pages/Editor/IDE/LeftPane/DataSidePane.tsx(2 hunks)app/client/src/pages/Editor/IDE/hooks.ts(2 hunks)app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx(6 hunks)app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx(9 hunks)app/client/src/pages/Editor/IntegrationEditor/DatasourceCard.tsx(3 hunks)app/client/src/pages/Editor/gitSync/ReconnectDatasourceModal.tsx(2 hunks)app/client/src/plugins/Linting/constants.ts(3 hunks)app/client/src/plugins/Linting/tests/generateLintingGlobalData.test.ts(0 hunks)app/client/src/plugins/Linting/utils/getLintingErrors.ts(5 hunks)app/client/src/sagas/IDESaga.tsx(1 hunks)app/client/src/selectors/jsPaneSelectors.ts(1 hunks)
💤 Files with no reviewable changes (8)
- app/client/src/ee/pages/Editor/IDE/EditorPane/JS/utils.ts
- app/client/src/ee/pages/Editor/IDE/EditorPane/Query/utils.ts
- app/client/src/ce/hooks/hooks.test.ts
- app/client/src/plugins/Linting/tests/generateLintingGlobalData.test.ts
- app/client/src/pages/Editor/Explorer/Files/index.tsx
- app/client/src/ce/pages/Editor/IDE/EditorPane/JS/utils/getJSUrl.ts
- app/client/src/pages/Editor/IDE/EditorPane/Query/List.tsx
- app/client/src/ce/hooks/index.ts
✅ Files skipped from review due to trivial changes (15)
- app/client/src/ee/entities/IDE/hooks/useCreateActionsPermissions.ts
- app/client/src/ee/IDE/hooks/useParentEntityInfo.ts
- app/client/src/ee/pages/Editor/IDE/EditorPane/JS/utils/getJSUrl.ts
- app/client/src/ee/pages/Editor/IDE/EditorPane/Query/utils/getQueryUrl.ts
- app/client/src/ee/pages/Editor/IDE/EditorPane/JS/utils/getJSEntityItemUrl.ts
- app/client/src/ce/navigation/FocusSetters.ts
- app/client/src/sagas/IDESaga.tsx
- app/client/src/ce/pages/Editor/IDE/EditorPane/JS/hooks.tsx
- app/client/src/ee/pages/Editor/IDE/EditorPane/Query/utils/getQueryEntityItemUrl.ts
- app/client/src/ce/selectors/appIDESelectors.ts
- app/client/src/pages/Editor/IDE/LeftPane/DataSidePane.test.tsx
- app/client/src/pages/Editor/IDE/EditorTabs/constants.ts
- app/client/src/components/editorComponents/form/fields/DropdownWrapper.tsx
- app/client/src/selectors/jsPaneSelectors.ts
- app/client/src/ce/pages/Editor/IDE/EditorPane/Query/hooks.tsx
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: client-unit-tests / client-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 (40)
app/client/src/pages/Editor/IDE/EditorPane/JS/List.tsx (3)
74-79: LGTM! Well-structured container with proper data-testid.The Flex container is properly configured with appropriate flex direction and overflow properties.
113-120: LGTM! Proper handling of empty search results.The empty state handling is well implemented with appropriate messaging.
93-109: Verify the impact of FilesContextProvider nesting.The FilesContextProvider has been moved inside the group mapping. While this provides more granular context, verify that this doesn't cause unnecessary context re-renders for each group.
✅ Verification successful
FilesContextProvider nesting is implemented correctly
The provider implementation uses proper memoization and the context value only updates when props change. The nesting inside the mapping won't cause unnecessary re-renders.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other usages of FilesContextProvider to understand the impact rg -A 3 "FilesContextProvider" --type tsxLength of output: 69
Script:
#!/bin/bash # Search for FilesContextProvider in TypeScript files rg -l "FilesContextProvider" --type ts --type tsx # Find context definition and usage ast-grep --pattern 'const $_ = createContext<$_>($_)' # Look for the FilesContextProvider component definition ast-grep --pattern 'function FilesContextProvider($_) { $$$ }' # Search for useContext hooks using this provider rg "useContext.*FilesContext" --type ts --type tsxLength of output: 3513
Script:
#!/bin/bash # Get FilesContextProvider implementation cat app/client/src/pages/Editor/Explorer/Files/FilesContextProvider.tsx # Find JSListItem implementation and context usage rg -g "*.{ts,tsx}" "JSListItem" # Look for useMemo or useCallback patterns in FilesContextProvider rg -g "*.{ts,tsx}" "useMemo|useCallback" app/client/src/pages/Editor/Explorer/Files/FilesContextProvider.tsxLength of output: 3080
Script:
#!/bin/bash # Get JSListItem implementation from both CE and EE versions cat app/client/src/ce/pages/Editor/IDE/EditorPane/JS/ListItem.tsx cat app/client/src/ee/pages/Editor/IDE/EditorPane/JS/ListItem.tsxLength of output: 937
app/client/src/pages/Editor/IDE/LeftPane/DataSidePane.tsx (2)
33-33: LGTM! Import aligns with refactoring goals.
62-64: Verify the IDE type determination across different routes.The refactoring from
editorTypetoideTypelooks correct. However, we should verify thatgetIDETypeByUrlhandles all possible pathname patterns correctly.Run this script to check IDE type determination across different routes:
✅ Verification successful
IDE type determination is correctly implemented and handles all routes
The function
getIDETypeByUrlsafely handles all application routes through theIDEBasePathsmapping and provides a fallback toIDE_TYPE.None. The refactoring fromeditorTypetoideTypeis consistent across the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify IDE type determination logic across different routes # Search for route definitions and URL patterns rg -g '*.{ts,tsx}' -A 3 'export const.*URL.*=.*{.*}.*=>' ./app/client # Search for existing usages of getIDETypeByUrl to compare patterns ast-grep --pattern 'getIDETypeByUrl($path)'Length of output: 253
Script:
#!/bin/bash # Find implementation of getIDETypeByUrl ast-grep --pattern 'function getIDETypeByUrl($_) { $$$ }' # Find route definitions rg -g '*.{ts,tsx}' 'const.*Routes.*=.*\[.*\]' -A 5 # Find all usages of getIDETypeByUrl rg -g '*.{ts,tsx}' 'getIDETypeByUrl'Length of output: 6549
Script:
#!/bin/bash # Get the implementation of getIDETypeByUrl cat app/client/src/ce/entities/IDE/utils.ts # Find IDEType enum definition rg -g '*.{ts,tsx}' 'enum.*IDEType' # Find route constants rg -g '*.{ts,tsx}' 'export const.*(PATH|URL).*=' -A 1Length of output: 29350
app/client/src/ce/pages/Editor/IDE/EditorPane/Query/utils/getQueryUrl.ts (1)
Line range hint
8-31: RefactoredgetQueryUrlfunction looks goodThe updated
getQueryUrlfunction effectively generates the correct URLs based onFocusEntityInfo. The conditional logic is clear and maintains consistency across different scenarios.app/client/src/ce/pages/Editor/IDE/EditorPane/JS/utils/getJSEntityItemUrl.ts (1)
1-12: Implementation ofgetJSEntityItemUrlis appropriateThe new
getJSEntityItemUrlfunction correctly constructs the URL for JavaScript entities usingjsCollectionIdURL. The code is straightforward and aligns with the refactoring goals.app/client/src/ce/utils/lintRulesHelpers.ts (1)
1-4: Update toContextTypeinterface is aligned with refactoringReplacing
editorTypewithideTypein theContextTypeinterface reflects the intended shift towards usingideType. This change maintains the integrity of the interface.app/client/src/ce/pages/Editor/IDE/EditorPane/Query/ListItem.tsx (1)
12-12: LGTM! Clean prop removal.The removal of parentEntityType aligns with the IDE refactoring objectives while maintaining component simplicity.
app/client/src/ce/pages/Editor/IDE/EditorPane/JS/ListItem.tsx (1)
13-13: Verify ExplorerJSCollectionEntity functionality without parentEntityTypeThe removal of
parentEntityTypealigns with the refactoring goal, but we should verify thatExplorerJSCollectionEntityfunctions correctly without this prop.✅ Verification successful
Removal of parentEntityType prop is safe
The
parentEntityTypeprop is not used by theExplorerJSCollectionEntitycomponent, and other instances of this component in the codebase don't pass this prop.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usages of parentEntityType in ExplorerJSCollectionEntity ast-grep --pattern 'class ExplorerJSCollectionEntity { $$$ parentEntityType $$$ }' # Search for any remaining references to parentEntityType rg "parentEntityType" "app/client/src/pages/Editor/Explorer/JSActions/JSActionEntity"Length of output: 276
Script:
#!/bin/bash # Find JSCollection-related files fd -e tsx -e ts "JSCollection|Explorer.*Entity" # Search for ExplorerJSCollectionEntity usage and parentEntityType rg -A 2 "ExplorerJSCollectionEntity" --type ts --type tsx # Search for any remaining parentEntityType references rg "parentEntityType" --type ts --type tsxLength of output: 279
Script:
#!/bin/bash # Check JSCollectionLink.tsx content cat app/client/src/components/editorComponents/Debugger/JSCollectionLink.tsx # Search for ExplorerJSCollectionEntity usage rg -A 2 "ExplorerJSCollectionEntity" # Search for any remaining parentEntityType references rg "parentEntityType"Length of output: 7625
app/client/src/ce/entities/IDE/hooks/useCreateActionsPermissions.ts (1)
16-18: Review default permission handlingThe default case returns
true, which might be too permissive. Consider if other IDE types should have explicit permission checks.app/client/src/ce/pages/Editor/gitSync/useReconnectModalData.ts (1)
39-39: LGTM!The change from
editorTypetoideTypeis consistent with the refactoring objective.app/client/src/ce/pages/Editor/IDE/EditorPane/JS/utils/getJSUrl.test.ts (1)
Line range hint
1-70: LGTM!The test cases are comprehensive and well-structured, covering all necessary scenarios for URL generation.
app/client/src/PluginActionEditor/components/PluginActionResponse/components/DatasourceTab/DatasourceInfo.tsx (1)
11-12: LGTM! Clean refactoring of editor type to IDE type.The changes maintain the component's functionality while aligning with the new IDE-type approach.
Also applies to: 28-29
app/client/src/ce/pages/Editor/IDE/EditorPane/Query/utils/getQueryUrl.test.ts (1)
1-5: LGTM! Well-structured test cases.The tests comprehensively cover all URL generation scenarios.
app/client/src/pages/Editor/Explorer/JSActions/JSActionEntity.tsx (1)
102-104: LGTM! Clean implementation of the IDE-type approach.The name saving functionality has been properly updated to use the new IDE-type pattern.
app/client/src/pages/Editor/Explorer/Actions/ActionEntity.tsx (2)
24-24: LGTM! Import statements are properly updated.The changes correctly replace the old parent entity imports with the new IDE-related imports.
Also applies to: 26-26
50-51: Verify the IDE type determination and action name saving.The implementation looks correct, but we should ensure that
getIDETypeByUrlreturns the same type of values that were previously used for action name saving.Also applies to: 117-117
✅ Verification successful
The IDE type determination and action name saving implementation is type-safe and correct.
The implementation maintains type consistency between
getIDETypeByUrlandsaveActionNameBasedOnIdeType, both using the sameIDETypetype definition. The default value ensures backward compatibility.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any other implementations that might expect different values rg -A 2 "saveActionNameBasedOn(Parent|Ide)"Length of output: 888
Script:
#!/bin/bash # Check the implementation of getIDETypeByUrl ast-grep --pattern 'export const getIDETypeByUrl = $_' # Check the implementation of saveActionNameBasedOnIdeType ast-grep --pattern 'export const saveActionNameBasedOnIdeType = $_' # Look for type definitions rg -g '*.ts' -g '*.tsx' "type.*IDE.*Type" -A 2Length of output: 76770
Script:
#!/bin/bash # Check the implementation in the utils file rg -A 5 "getIDETypeByUrl" "app/client/src/ee/entities/IDE/utils.ts" # Check the implementation of saveActionNameBasedOnIdeType rg -A 5 "saveActionNameBasedOnIdeType" "app/client/src/ce/actions/helpers.ts" # Look for IDE type definitions in IDE related files rg -g "**/IDE/**/*.ts" "type.*IDE" -A 2Length of output: 4590
app/client/src/ce/hooks/datasourceEditorHooks.tsx (1)
36-36: Type safety improvement with IDEType.The change from string to IDEType enum improves type safety and prevents potential errors from invalid editor type values.
Also applies to: 75-75
app/client/src/pages/Editor/DataSourceEditor/hooks.ts (1)
144-144: Verify permission check behavior with IDE type.The change from editorType to ideType in permission checks looks correct, but we should verify that the permission logic remains consistent.
Also applies to: 154-154
✅ Verification successful
Permission check behavior with IDE type is correct
The ideType parameter is properly integrated into the permission check logic, maintaining the correct permission validation for both App and non-App IDE contexts.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any changes in permission check implementations rg -A 3 "hasCreateDSActionPermissionInApp.*ideType"Length of output: 53
Script:
#!/bin/bash # Search for permission check function definition ast-grep --pattern 'function hasCreateDSActionPermissionInApp($_) { $$$ }' # Search for any usage of the function rg "hasCreateDSActionPermissionInApp" # Search for files containing both editorType and ideType rg -l "editorType.*ideType|ideType.*editorType"Length of output: 1742
Script:
#!/bin/bash # Get the permission check function implementation cat app/client/src/ce/utils/BusinessFeatures/permissionPageHelpers.tsx # Get the context of usage in hooks.ts rg -B 2 -A 2 "hasCreateDSActionPermissionInApp" app/client/src/pages/Editor/DataSourceEditor/hooks.tsLength of output: 8000
app/client/src/pages/Editor/DatasourceInfo/DatasourceStructure.tsx (1)
69-69: LGTM! Consistent implementation of IDE type.The changes follow the same pattern as other files, maintaining consistency across the codebase.
Also applies to: 75-75
app/client/src/pages/Editor/CustomWidgetBuilder/useCustomBuilder.tsx (1)
19-20: LGTM! Clean refactoring of editor type handling.The changes align with the PR objective of standardizing IDE type handling across the codebase.
Let's verify that all usages of
parentEntityIdare compatible with the new IDE type:Also applies to: 32-33
✅ Verification successful
✓ IDE type handling verification complete
The new IDE type handling is correctly integrated. All usages of
parentEntityIdare type-agnostic string operations that work seamlessly with the standardized IDE type system.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for parentEntityId usages in the component rg -A 2 "parentEntityId" "app/client/src/pages/Editor/CustomWidgetBuilder/"Length of output: 2191
app/client/src/pages/Editor/DataSourceEditor/DSFormHeader.tsx (1)
28-30: LGTM! Consistent refactoring of header actions.The changes maintain the component's functionality while standardizing IDE type usage.
Also applies to: 119-120, 168-172
app/client/src/PluginActionEditor/components/PluginActionResponse/components/DatasourceTab/DatasourceTab.tsx (1)
26-26: LGTM! Clean transition to IDE type in datasource handling.The changes maintain consistency with the codebase-wide refactoring effort.
Also applies to: 36-36, 61-62
app/client/src/plugins/Linting/constants.ts (1)
6-6: Improved type safety in linting options.The change from string to
IDETypeenhances type checking and prevents potential errors.Let's verify that all consumers of
lintOptionsare updated to use the new type:Also applies to: 16-16, 46-48
✅ Verification successful
Type safety changes verified successfully.
The change from string to IDEType is properly implemented and all consumers are using the type correctly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for lintOptions usage rg "lintOptions\(" --type tsLength of output: 125
Script:
#!/bin/bash # Find lintOptions implementation and IDE type usage ast-grep --pattern 'function lintOptions' ast-grep --pattern 'import { IDEType }' # Get content of the getLintingErrors.ts cat app/client/src/plugins/Linting/utils/getLintingErrors.tsLength of output: 85
Script:
#!/bin/bash # Search for lintOptions declarations and exports ast-grep --pattern 'const lintOptions' ast-grep --pattern 'export const lintOptions' # Get content of both files cat app/client/src/plugins/Linting/constants.ts cat app/client/src/plugins/Linting/utils/getLintingErrors.tsLength of output: 44275
app/client/src/pages/Editor/IDE/hooks.ts (2)
30-30: Import changes align with the refactoring objective.The replacement of editor-related imports with IDE-specific ones maintains consistency with the PR's objective of standardizing IDE type handling.
Also applies to: 34-34
66-67: Verify the impact of ideType usage.The switch from
editorTypetoideTypeinuseSegmentNavigationhook affects how parent entity information is retrieved.✅ Verification successful
The switch to ideType is consistent with codebase patterns
The codebase consistently uses
ideTypeandgetIDETypeByUrlfor IDE type determination, with no remainingeditorTypeusages that would need updating.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any remaining editorType usages that might need updating rg -l 'useEditorType|editorType' --type ts --type tsxLength of output: 83
Script:
#!/bin/bash # Search for editorType and useEditorType in ts files rg 'editorType|useEditorType' -t ts --context 2 # Search for ideType usage rg 'ideType|getIDETypeByUrl' -t ts --context 2 # Find hook usage patterns ast-grep --pattern 'const $_ = useEditorType($$$)' ast-grep --pattern 'const $_ = useSegmentNavigation($$$)'Length of output: 49813
app/client/src/pages/Editor/DatasourceInfo/DatasourceViewModeSchema.tsx (1)
54-54: Consistent usage of IDE type in datasource view.The changes maintain consistency with the new IDE type pattern and improve code clarity.
Also applies to: 85-85, 96-96
app/client/src/pages/Editor/IntegrationEditor/APIOrSaasPlugins.tsx (2)
23-24: Function signature update improves type safety.The changes to
createNewApiActionBasedOnIdeTypeimprove type safety and maintain consistency with the IDE type refactoring.Also applies to: 50-51, 66-71
91-93: Verify the IDE type usage in action creation.The changes to action creation logic need verification to ensure proper handling of IDE types in all scenarios.
Also applies to: 118-126
✅ Verification successful
IDE type usage in action creation is properly implemented
The changes follow the established patterns in the codebase and properly handle IDE types in action creation flows.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any remaining references to the old action creation method rg "createNewApiActionBasedOnEditorType" --type ts --type tsxLength of output: 89
Script:
#!/bin/bash # Search for IDE type and action creation related patterns rg -t typescript -t javascriptreact -t typescriptreact "getIDETypeByUrl|createNewApiAction|newApiAction.*IDE" -A 2 # Find files related to action creation fd -e ts -e tsx "Action" --exec grep -l "createNewApi\|getIDEType" # Look for IDE type related constants and utilities rg -t typescript -t javascriptreact -t typescriptreact "IDE_TYPE|ideType.*=|getIDEType" -A 1Length of output: 820
Script:
#!/bin/bash # Check content of action-related files cat app/client/src/sagas/ActionSagas.ts cat app/client/src/actions/pluginActionActions.ts # Search with simpler pattern matching rg --type-add 'ts:*.{ts,tsx}' -t ts "getIDETypeByUrl|createNewApiAction" -A 3Length of output: 66432
app/client/src/pages/Editor/IntegrationEditor/DBOrMostPopularPlugins.tsx (3)
30-30: LGTM! Import statements are properly organized.The new imports for IDE-related utilities are correctly placed and maintain good code organization.
Also applies to: 41-41, 53-54
77-77: LGTM! Type definitions are properly updated.The interface and function signature changes correctly reflect the transition from
editorTypetoideType.Also applies to: 94-99
257-259: Verify the IDE type determination logic.The IDE type is now determined using
getIDETypeByUrlinstead of a hook. Ensure this change doesn't affect the component's behavior during route changes.✅ Verification successful
The IDE type determination change is safe and consistent with the codebase
The
getIDETypeByUrlutility is already widely used across the application for route-based IDE type determination, including in navigation and state management code. This change aligns with the existing pattern.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any other usages of useEditorType that might need updating rg "useEditorType|editorType" --type ts --type tsxLength of output: 80
Script:
#!/bin/bash # Search for IDE type related functions and hooks rg "useEditorType|getIDETypeByUrl|editorType" --type ts # Search for the specific utility function implementation ast-grep --pattern 'function getIDETypeByUrl($_) { $$$ }' # Look for route/location related usage with IDE type rg "location.*(?:IDE|editorType)" --type tsLength of output: 6262
app/client/src/pages/Editor/IntegrationEditor/DatasourceCard.tsx (1)
63-63: LGTM! IDE type handling is properly implemented.The transition from
editorTypetoideTypeis consistent with the new pattern, and the IDE type is correctly determined from the URL.Also applies to: 182-182, 193-193
app/client/src/pages/Editor/DatasourceInfo/GoogleSheetSchema.tsx (1)
54-54: LGTM! IDE type integration is properly implemented.The changes correctly implement the new IDE type pattern while maintaining the component's functionality.
Also applies to: 363-363, 374-374
app/client/src/pages/Editor/gitSync/ReconnectDatasourceModal.tsx (1)
328-328: LGTM! Analytics integration is properly updated.The changes correctly integrate the new IDE type with analytics event logging while maintaining the existing functionality.
Also applies to: 357-357
app/client/src/ce/pages/Editor/IDE/EditorPane/Query/utils/getQueryEntityItemUrl.test.ts (1)
1-26: LGTM! Well-structured test cases.The test suite effectively covers both error and success scenarios for the getQueryEntityItemUrl function.
app/client/src/plugins/Linting/utils/getLintingErrors.ts (2)
48-49: LGTM! Clean import additions.The new imports for IDE types are well-organized and properly scoped to the enterprise edition.
160-160: LGTM! Clean integration with linting system.The ideType is properly integrated into both the return value and linting options.
Also applies to: 376-381
app/client/src/ce/selectors/entitiesSelector.ts (1)
67-67: LGTM! Clean type import.The IDEType import is properly placed within the existing imports section.
app/client/src/ce/pages/Editor/IDE/EditorPane/JS/utils/getJSEntityItemUrl.test.ts
Show resolved
Hide resolved
github-actions
bot
added
IDE Pod
…duplication (appsmithorg#38778) ## Description Refactoring usage of `editorType` to `ideType` to remove code duplication Fixes [appsmithorg#38286](appsmithorg#38286) [appsmithorg#39289](appsmithorg#38289) ## 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/12878729200> > Commit: dfe8f5c > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=12878729200&attempt=2" target="_blank">Cypress dashboard</a>. > Tags: `@tag.All` > Spec: > <hr>Tue, 21 Jan 2025 06:50:04 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 ## Release Notes - **New Features** - Introduced a new IDE type context to replace the previous editor type system - Enhanced type safety for IDE-related operations - **Improvements** - Refactored editor type handling across multiple components - Updated utility functions to work with new IDE type - Simplified permission and context management - **Code Quality** - Removed deprecated editor type hooks and constants - Standardized IDE type determination across the application - Improved import and module organization - **Performance** - Streamlined type checking and context retrieval mechanisms <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Description
Refactoring usage of
editorTypetoideTypeto remove code duplicationFixes #38286 #39289
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/12878729200
Commit: dfe8f5c
Cypress dashboard.
Tags:
@tag.AllSpec:
Tue, 21 Jan 2025 06:50:04 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Release Notes
New Features
Improvements
Code Quality
Performance