Skip to content
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

Merged
merged 13 commits into from
Feb 7, 2025
Merged

Conversation

hetunandu
Copy link
Member

@hetunandu hetunandu commented Feb 6, 2025

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?

  • Yes
  • No

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.

@hetunandu hetunandu requested review from ayushpahwa and a team as code owners February 6, 2025 09:17
Copy link
Contributor

coderabbitai bot commented Feb 6, 2025

Walkthrough

The 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

File(s) Change Summary
app/client/src/IDE/Interfaces/EditorTypes.ts
app/client/src/IDE/Interfaces/UseRoutes.ts
Added new enums (EditorEntityTab, EditorEntityTabState, EditorViewMode) and a new type (UseRoutes) for route configurations.
app/client/src/IDE/constants/SidebarButtons.ts
app/client/src/ce/pages/Editor/IDE/constants/SidebarButtons.ts
Exported individual sidebar button creator functions (e.g., EditorButton, DataButton, etc.) and introduced conditional button composition via BottomButtons and TopButtons.
Multiple IDE modules (hooks, actions, sagas, pages, selectors, tests)
e.g., app/client/src/IDE/hooks/useIsInSideBySideEditor.test.tsx, app/client/src/actions/ideActions.ts, app/client/src/ce/pages/Editor/IDE/EditorPane/JS/hooks.tsx, etc.
Updated numerous import paths from "ee/entities/IDE/constants" to new module paths (such as "IDE/Interfaces/EditorTypes", "IDE/enums", "ee/IDE/Interfaces/EntityItem", "ee/IDE/Interfaces/IDETypes") and re-exported constants/types to reflect the reorganized codebase.
app/client/src/ce/entities/IDE/constants.ts
app/client/src/IDE/enums.ts
Removed legacy constants and types; introduced a new EditorState enum (with members like DATA, EDITOR, etc.) and consolidated re-exports in EE interfaces.

Sequence Diagram(s)

Assessment against linked issues

Objective Addressed Explanation
Refactor Sidebar Buttons to export single button functions rather than grouped arrays [#39050] Grouped exports (TopButtons and BottomButtons) still exist in parts of the code, not exclusively single exports.

Possibly related PRs

  • chore: Housekeeping of List Item and Entity Item #38524: The changes in the main PR, which introduce new enumerations in EditorTypes.ts, are related to the modifications in the retrieved PR that involve updates to the EntityItem component and its props, as both involve adjustments to types and interfaces used within the editor context.
  • chore: Move Sidebar to IDE/Components #34487: The changes in the main PR, which introduce new enumerations in EditorTypes.ts, are related to the modifications in the retrieved PR that involve the Sidebar component, as both involve updates to the structure and organization of constants and types used in the IDE.
  • chore: Transitions for IDE #35714: The changes in the main PR, which introduce new enumerations in EditorTypes.ts, are related to the modifications in the retrieved PR that involve the import of EditorViewMode and EditorEntityTab from the same file, indicating a direct connection at the code level.

Suggested labels

Enhancement

Suggested reviewers

  • AmanAgarwal041
  • alex-golovanov

Poem

In the maze of code we refactor and align,
New enums and types now brightly shine.
Imports migrated to a clearer domain,
Sidebar buttons now sing a refactored refrain.
With every change, our codebase grows fine!
🚀✨


📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e958813 and 0a88e2f.

📒 Files selected for processing (2)
  • app/client/src/pages/Editor/IDE/LeftPane/DataSidePane.test.tsx (1 hunks)
  • app/client/src/pages/Editor/IDE/LeftPane/index.tsx (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • app/client/src/pages/Editor/IDE/LeftPane/DataSidePane.test.tsx
⏰ Context from checks skipped due to timeout of 90000ms (5)
  • 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
🔇 Additional comments (1)
app/client/src/pages/Editor/IDE/LeftPane/index.tsx (1)

20-20: LGTM! Import path update aligns with module reorganization.

The change correctly implements the PR's objective of relocating IDE constants to a dedicated IDE folder structure.


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

# 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
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 tsx

Length 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, the gap="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 with as 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 for userPermissions 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

📥 Commits

Reviewing files that changed from the base of the PR and between 4b20f9f and 2c960da.

📒 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 to IDE/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 10

Length of output: 1258


Import path and enum verification successful.

  • The file located at app/client/src/IDE/Interfaces/EditorState.ts correctly exports the EditorState 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 reuses EntityItem 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 and EditorEntityTab 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 and EditorViewMode 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/EditorTypes
  • IDEType from ee/IDE/Interfaces/IDETypes
  • EntityItem and GenericEntityItem from ee/IDE/Interfaces/EntityItem

app/client/src/ce/entities/IDE/utils.ts Outdated Show resolved Hide resolved
@github-actions github-actions bot added IDE Navigation Issues/feature requests related to IDE navigation, and context switching IDE Pod Issues that new developers face while exploring the IDE IDE Product Issues related to the IDE Product Task A simple Todo labels Feb 6, 2025
@github-actions github-actions bot added the skip-changelog Adding this label to a PR prevents it from being listed in the changelog label Feb 6, 2025
@hetunandu hetunandu added the ok-to-test Required label for CI label Feb 6, 2025
alex-golovanov
alex-golovanov previously approved these changes Feb 6, 2025
app/client/src/IDE/Interfaces/EditorState.ts Outdated Show resolved Hide resolved
app/client/src/ce/IDE/Interfaces/IDETypes.ts Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 and Success 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

📥 Commits

Reviewing files that changed from the base of the PR and between 7e688b5 and 77e2c2e.

📒 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 from IDE/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 from entities/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 to IDE/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:

  1. Adding proper type definitions for the parameters
  2. Using more specific types instead of any

Would you like me to help create proper type definitions for these parameters?

ankitakinger
ankitakinger previously approved these changes Feb 6, 2025
@ankitakinger ankitakinger added ok-to-test Required label for CI and removed ok-to-test Required label for CI labels Feb 6, 2025
# Conflicts:
#	app/client/src/pages/Editor/IDE/EditorPane/Explorer.tsx
@ankitakinger ankitakinger merged commit ac41ad2 into release Feb 7, 2025
83 checks passed
@ankitakinger ankitakinger deleted the chore/update-sidebar-buttons branch February 7, 2025 11:15
github-actions bot pushed a commit to Zeral-Zhang/appsmith that referenced this pull request Feb 12, 2025
## 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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IDE Navigation Issues/feature requests related to IDE navigation, and context switching IDE Pod Issues that new developers face while exploring the IDE IDE Product Issues related to the IDE Product ok-to-test Required label for CI skip-changelog Adding this label to a PR prevents it from being listed in the changelog Task A simple Todo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Task]: Refactor Sidebar Buttons
3 participants