Skip to content

chore: Move action redesign to GA#38659

Merged
hetunandu merged 12 commits intoreleasefrom
chore/plugin-action-editor-ga
Jan 16, 2025
Merged

chore: Move action redesign to GA#38659
hetunandu merged 12 commits intoreleasefrom
chore/plugin-action-editor-ga

Conversation

@hetunandu
Copy link
Member

@hetunandu hetunandu commented Jan 15, 2025

Description

  • Removes feature flagging for Action Redesign
  • Removes older implementations for API and Query Editor. Moving completely to Plugin Action Editor
  • Misc folder structure changes to simplify Plugin Action Editor

EE PR: https://github.com/appsmithorg/appsmith-ee/pull/5921

Automation

/ok-to-test tags="@tag.All"

🔍 Cypress test results

Caution

🔴 🔴 🔴 Some tests have failed.
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/12784552425
Commit: a85b9e2
Cypress dashboard.
Tags: @tag.All
Spec:
The following are new failures, please fix them before merging the PR:

  1. cypress/e2e/Regression/ClientSide/Binding/Button_Text_WithRecaptcha_spec.js
List of identified flaky tests.
Wed, 15 Jan 2025 10:45:51 UTC

Communication

Should the DevRel and Marketing teams inform users about this change?

  • Yes
  • No

Summary by CodeRabbit

Release Notes

  • Major Refactoring

    • Removed feature flag and components related to action redesign.
    • Simplified component rendering and removed conditional logic.
    • Streamlined import paths and module structure.
  • Component Cleanup

    • Deleted multiple editor components for API, Query, and JS editors.
    • Removed deprecated context providers and utility functions.
    • Simplified toolbar and header components.
  • Feature Flag Removal

    • Eliminated release_actions_redesign_enabled feature flag.
    • Removed associated local storage keys.
  • Type and Interface Updates

    • Added new AutoGeneratedHeader interface.
    • Updated prop types for various components.
    • Removed deprecated type definitions.

These changes represent a significant architectural cleanup, focusing on removing feature flag-related complexity and consolidating the application's component structure.

# Conflicts:
#	app/client/src/entities/Action/index.ts
#	app/client/src/pages/Editor/APIEditor/Editor.tsx
#	app/client/src/pages/Editor/QueryEditor/DatasourceSelector.tsx
#	app/client/src/pages/Editor/QueryEditor/Editor.tsx
#	app/client/src/pages/Editor/QueryEditor/EditorJSONtoForm.tsx
#	app/client/src/pages/Editor/QueryEditor/QueryEditorHeader.tsx
#	app/client/src/pages/Editor/QueryEditor/index.tsx
@hetunandu hetunandu requested a review from ayushpahwa as a code owner January 15, 2025 06:17
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 15, 2025

Walkthrough

This pull request represents a significant refactoring of the application's editor components, focusing on removing deprecated components related to API, Query, and JavaScript editors. The changes involve eliminating feature flag-dependent code, simplifying component structures, and reorganizing import paths. The modifications streamline the codebase by removing old implementations and consolidating the UI components for a more consistent user experience.

Changes

File Path Change Summary
app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/ Removed deprecated component RequestTabs.tsx
app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/utils/ Deleted sortedDatasourcesHandler function and AutoGeneratedHeader interface
app/client/src/PluginActionEditor/components/PluginActionResponse/components/ApiFormatSegmentedResponse.tsx Added new styled component SegmentedControlContainer
app/client/src/PluginActionEditor/components/PluginActionResponse/components/Response/Response.tsx Simplified navigation logic by removing feature flag checks
app/client/src/PluginActionEditor/components/PluginActionResponse/components/Response/components/ActionExecutionInProgressView.tsx Updated import paths for clarity
app/client/src/PluginActionEditor/components/PluginActionSettings/SettingsPopover.tsx Modified import path for ActionSettings component
app/client/src/PluginActionEditor/components/PluginActionToolbar/components/ActionSettings.tsx Updated import paths for FormControl and CenteredWrapper
app/client/src/ce/pages/Editor/IDE/EditorPane/Query/hooks.tsx Removed ApiEditor and QueryEditor components; updated useQueryEditorRoutes function
app/client/src/components/editorComponents/ Deleted ActionNameEditor.tsx and ActionRightPane/index.tsx components
app/client/src/entities/Action/index.ts Added new AutoGeneratedHeader interface and updated ApiActionConfig
app/client/src/pages/Editor/APIEditor/ Deleted entire editor implementation, including Editor.tsx, CommonEditorForm.tsx, and context files
app/client/src/pages/Editor/JSEditor/ Removed old JS editor components and simplified toolbar implementations
app/client/src/pages/Editor/QueryEditor/ Eliminated query editor components and related context management
app/client/src/utils/localStorage.tsx Removed SPLITPANE_ANNOUNCEMENT key from local storage
app/client/src/cypress/support/Objects/FeatureFlags.ts Removed release_actions_redesign_enabled flag from default flags
app/client/src/ce/entities/FeatureFlag.ts Removed release_actions_redesign_enabled feature flag

Possibly related PRs

Suggested Labels

Enhancement, Bug, Task, IDE Product, IDE Pod, ok-to-test

Suggested Reviewers

  • albinAppsmith
  • ankitakinger
  • AmanAgarwal041

Poem

Code's old paths now fade away,
Refactored lines in clean array,
Editors dance with lighter stride,
Legacy components cast aside! 🚀
Simplicity wins the coding day!


📜 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 71f4e6a and a85b9e2.

📒 Files selected for processing (1)
  • app/client/src/entities/Action/index.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • app/client/src/entities/Action/index.ts
⏰ Context from checks skipped due to timeout of 90000ms (5)
  • GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
  • GitHub Check: client-lint / client-lint
  • GitHub Check: client-build / client-build
  • GitHub Check: client-unit-tests / client-unit-tests
  • GitHub Check: client-prettier / prettier-check

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.

@github-actions github-actions bot added the skip-changelog Adding this label to a PR prevents it from being listed in the changelog label Jan 15, 2025
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 (4)
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSEditorToolbar.test.tsx (1)

Line range hint 5-6: Remove unnecessary feature flag imports and mocks.

Since the action redesign feature flag has been removed, we can simplify the test setup by removing the feature flag related code.

-import { useFeatureFlag } from "utils/hooks/useFeatureFlag";
 import { JSObjectFactory } from "test/factories/Actions/JSObject";
-
-jest.mock("utils/hooks/useFeatureFlag");
-
-const mockUseFeatureFlag = useFeatureFlag as jest.Mock;

Also applies to: 8-10

app/client/src/pages/Editor/IDE/EditorPane/Query/QueryRender.test.tsx (1)

Update outdated comment in element count assertion

The comment // Wait until there are exactly 3 elements is outdated as the test now expects 2 elements. Update it to match the actual expectation.

🔗 Analysis chain

Line range hint 144-179: Verify the reduced element count expectation.

The test now expects 2 instances of "Api1" instead of 3. This change seems to align with the removal of feature flagging, but let's verify if this is consistent across all routes.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for similar element count expectations in test files
rg -A 2 "expect\(.*\.toHaveLength\(.*\)" "app/client/src/pages/Editor/IDE/EditorPane"

Length of output: 1488

app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/utils/autoGeneratedHeaders.ts (2)

Line range hint 3-8: Replace any[] with proper type and remove TODO comment.

The helper function uses any[] type which bypasses TypeScript's type checking benefits. Consider using a more specific type.

-// TODO: Fix this the next time the file is edited
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-const isKeyInArray = (arr: any[], key: any) => {
+interface HeaderType {
+  key: string;
+  [key: string]: unknown;
+}
+const isKeyInArray = (arr: HeaderType[], key: string) => {

Line range hint 10-41: Improve type safety and simplify the code.

The function has several areas for improvement:

  1. Uses any type for headers parameter
  2. Has unnecessary optional chaining
  3. Contains repeated object creation patterns
-// TODO: Fix this the next time the file is edited
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-headers: any,
+headers: HeaderType[],
 autoGeneratedHeaders: AutoGeneratedHeader[],
) => {
-  let newAutoGeneratedHeader = [];
+  return autoGeneratedHeaders?.map((autoGenHeader) => {
+    const isInvalid = isKeyInArray(headers, autoGenHeader.key);
+    if (isInvalid !== autoGenHeader.isInvalid) {
+      return {
+        ...autoGenHeader,
+        isInvalid,
+      };
+    }
+    return autoGenHeader;
+  }) ?? [];
-  newAutoGeneratedHeader = autoGeneratedHeaders?.map(
-    (autoGenHeader: AutoGeneratedHeader) => {
-      if (isKeyInArray(headers, autoGenHeader?.key)) {
-        if (!autoGenHeader?.isInvalid) {
-          return {
-            key: autoGenHeader?.key,
-            value: autoGenHeader?.value,
-            isInvalid: true,
-          };
-        }
-      } else {
-        if (autoGenHeader?.isInvalid) {
-          return {
-            key: autoGenHeader?.key,
-            value: autoGenHeader?.value,
-            isInvalid: false,
-          };
-        }
-      }
-
-      return autoGenHeader;
-    },
-  );
-
-  return newAutoGeneratedHeader;
🧹 Nitpick comments (8)
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.tsx (1)

Line range hint 63-66: Update component documentation.

The JSDoc comment still references conditional rendering based on a feature flag, which has been removed. Please update it to reflect the current implementation.

-/**
- * JSFunctionSettings component renders a button and a popover for configuring JS function settings.
- * It conditionally renders the old or new version of the component based on a feature flag.
- */
+/**
+ * JSFunctionSettings component renders a configuration panel for JS function settings,
+ * allowing users to configure execution behavior for JavaScript functions.
+ */
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSEditorToolbar.tsx (2)

32-32: Consider adding JSDoc for the onUpdateSettings prop.

The type signature change improves type safety. Consider adding JSDoc documentation for this prop to describe the expected behavior and parameters.

+  /** Callback to update JS function settings with the provided configuration */
   onUpdateSettings: (props: OnUpdateSettingsProps) => void;

40-42: Enhance component documentation.

While the documentation is cleaner now, consider adding more details about the component's purpose and key features.

 /**
  * JSEditorToolbar component.
  *
  * This component renders a toolbar for the JS editor.
+ * 
+ * @component
+ * @description Provides functionality for running JS functions, updating settings,
+ * and managing JS object names. Includes controls for execution, settings management,
+ * and context menu operations.
  */
app/client/src/ce/pages/Editor/IDE/EditorPane/Query/hooks.tsx (1)

170-170: Consider simplifying the relative import path.

The deep relative path with multiple parent references (../../../../../../) is fragile and hard to maintain. Consider using an absolute import path or moving the component to a more accessible location.

app/client/src/PluginActionEditor/components/PluginActionResponse/components/ApiFormatSegmentedResponse.tsx (1)

14-22: Consider improving scroll behavior indication.

The container has overflow-x: scroll but might not visually indicate horizontal scrollability to users.

const SegmentedControlContainer = styled.div`
  padding: 0 var(--ads-v2-spaces-7);
  padding-top: var(--ads-v2-spaces-4);
  display: flex;
  flex-direction: column;
  gap: var(--ads-v2-spaces-4);
  overflow-y: clip;
-  overflow-x: scroll;
+  overflow-x: auto;
+  scrollbar-width: thin;
+  &::-webkit-scrollbar {
+    height: 4px;
+  }
`;
app/client/src/sagas/ApiPaneSagas.ts (2)

48-48: Consolidate import paths.

The imports for AutoGeneratedHeader and related utilities should be consistently sourced from the new location in entities/Action.

-import type { AutoGeneratedHeader } from "entities/Action";
-import { deriveAutoGeneratedHeaderState } from "../PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/utils/autoGeneratedHeaders";
+import type { AutoGeneratedHeader, deriveAutoGeneratedHeaderState } from "entities/Action";

Also applies to: 64-64


Line range hint 471-472: Consider removing TODO comments.

The TODO comment about fixing eslint issues should be addressed now as part of the refactoring.

app/client/src/pages/Editor/IDE/EditorPane/Query/QueryRender.test.tsx (1)

642-642: Consider using object destructuring for render result.

The test setup could be simplified using object destructuring for commonly used methods.

-      const { getByTestId, getByText } = render(
+      const { getByTestId, getByText, container } = render(
📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between a23bcbf and c94eba2.

📒 Files selected for processing (47)
  • app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/RequestTabs.tsx (0 hunks)
  • app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/utils/autoGeneratedHeaders.ts (1 hunks)
  • app/client/src/PluginActionEditor/components/PluginActionResponse/components/ApiFormatSegmentedResponse.tsx (1 hunks)
  • app/client/src/PluginActionEditor/components/PluginActionResponse/components/Response/Response.tsx (4 hunks)
  • app/client/src/PluginActionEditor/components/PluginActionResponse/components/Response/components/ActionExecutionInProgressView.tsx (1 hunks)
  • app/client/src/PluginActionEditor/components/PluginActionSettings/SettingsPopover.tsx (1 hunks)
  • app/client/src/PluginActionEditor/components/PluginActionToolbar/components/ActionSettings.tsx (1 hunks)
  • app/client/src/ce/pages/Editor/IDE/EditorPane/Query/hooks.tsx (1 hunks)
  • app/client/src/components/editorComponents/ActionNameEditor.tsx (0 hunks)
  • app/client/src/components/editorComponents/ActionRightPane/index.tsx (0 hunks)
  • app/client/src/entities/Action/index.ts (1 hunks)
  • app/client/src/pages/Editor/APIEditor/ApiEditorContext.tsx (0 hunks)
  • app/client/src/pages/Editor/APIEditor/CommonEditorForm.tsx (0 hunks)
  • app/client/src/pages/Editor/APIEditor/Editor.tsx (0 hunks)
  • app/client/src/pages/Editor/APIEditor/GraphQL/GraphQLEditorForm.tsx (0 hunks)
  • app/client/src/pages/Editor/APIEditor/RestAPIForm.tsx (0 hunks)
  • app/client/src/pages/Editor/APIEditor/index.tsx (0 hunks)
  • app/client/src/pages/Editor/IDE/EditorPane/JS/JSRender.test.tsx (5 hunks)
  • app/client/src/pages/Editor/IDE/EditorPane/Query/QueryRender.test.tsx (13 hunks)
  • app/client/src/pages/Editor/IDE/EditorPane/components/Announcement.tsx (0 hunks)
  • app/client/src/pages/Editor/IDE/EditorTabs/Editortabs.test.tsx (0 hunks)
  • app/client/src/pages/Editor/IDE/EditorTabs/index.tsx (0 hunks)
  • app/client/src/pages/Editor/IDE/hooks.ts (1 hunks)
  • app/client/src/pages/Editor/JSEditor/Form.tsx (1 hunks)
  • app/client/src/pages/Editor/JSEditor/JSEditorContextMenu.tsx (1 hunks)
  • app/client/src/pages/Editor/JSEditor/JSEditorForm/JSEditorForm.tsx (0 hunks)
  • app/client/src/pages/Editor/JSEditor/JSEditorForm/old/JSEditorForm.tsx (0 hunks)
  • app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSEditorToolbar.test.tsx (1 hunks)
  • app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSEditorToolbar.tsx (2 hunks)
  • app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSHeader.tsx (0 hunks)
  • app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSObjectNameEditor/JSObjectNameEditor.tsx (0 hunks)
  • app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSObjectNameEditor/old/JSObjectNameEditor.tsx (0 hunks)
  • app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.test.tsx (0 hunks)
  • app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.tsx (0 hunks)
  • app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.tsx (1 hunks)
  • app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/old/JSFunctionRun.tsx (0 hunks)
  • app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/old/JSFunctionSettings.tsx (0 hunks)
  • app/client/src/pages/Editor/QueryEditor/DatasourceSelector.tsx (0 hunks)
  • app/client/src/pages/Editor/QueryEditor/Editor.tsx (0 hunks)
  • app/client/src/pages/Editor/QueryEditor/EditorJSONtoForm.tsx (0 hunks)
  • app/client/src/pages/Editor/QueryEditor/Form.tsx (0 hunks)
  • app/client/src/pages/Editor/QueryEditor/QueryDebuggerTabs.test.tsx (0 hunks)
  • app/client/src/pages/Editor/QueryEditor/QueryEditorContext.tsx (0 hunks)
  • app/client/src/pages/Editor/QueryEditor/QueryEditorHeader.tsx (0 hunks)
  • app/client/src/pages/Editor/QueryEditor/index.tsx (0 hunks)
  • app/client/src/sagas/ApiPaneSagas.ts (2 hunks)
  • app/client/src/utils/localStorage.tsx (0 hunks)
💤 Files with no reviewable changes (30)
  • app/client/src/pages/Editor/QueryEditor/QueryDebuggerTabs.test.tsx
  • app/client/src/utils/localStorage.tsx
  • app/client/src/pages/Editor/QueryEditor/index.tsx
  • app/client/src/pages/Editor/QueryEditor/EditorJSONtoForm.tsx
  • app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.tsx
  • app/client/src/pages/Editor/IDE/EditorTabs/index.tsx
  • app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSHeader.tsx
  • app/client/src/pages/Editor/QueryEditor/DatasourceSelector.tsx
  • app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSObjectNameEditor/old/JSObjectNameEditor.tsx
  • app/client/src/pages/Editor/JSEditor/JSEditorForm/old/JSEditorForm.tsx
  • app/client/src/components/editorComponents/ActionRightPane/index.tsx
  • app/client/src/pages/Editor/QueryEditor/QueryEditorContext.tsx
  • app/client/src/pages/Editor/QueryEditor/QueryEditorHeader.tsx
  • app/client/src/pages/Editor/APIEditor/GraphQL/GraphQLEditorForm.tsx
  • app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionRun.test.tsx
  • app/client/src/pages/Editor/IDE/EditorTabs/Editortabs.test.tsx
  • app/client/src/pages/Editor/APIEditor/ApiEditorContext.tsx
  • app/client/src/pages/Editor/APIEditor/CommonEditorForm.tsx
  • app/client/src/components/editorComponents/ActionNameEditor.tsx
  • app/client/src/pages/Editor/APIEditor/index.tsx
  • app/client/src/pages/Editor/QueryEditor/Editor.tsx
  • app/client/src/pages/Editor/APIEditor/Editor.tsx
  • app/client/src/pages/Editor/QueryEditor/Form.tsx
  • app/client/src/pages/Editor/JSEditor/JSEditorForm/JSEditorForm.tsx
  • app/client/src/pages/Editor/IDE/EditorPane/components/Announcement.tsx
  • app/client/src/pages/Editor/APIEditor/RestAPIForm.tsx
  • app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/old/JSFunctionSettings.tsx
  • app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSObjectNameEditor/JSObjectNameEditor.tsx
  • app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/RequestTabs.tsx
  • app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/old/JSFunctionRun.tsx
✅ Files skipped from review due to trivial changes (2)
  • app/client/src/PluginActionEditor/components/PluginActionResponse/components/Response/components/ActionExecutionInProgressView.tsx
  • app/client/src/PluginActionEditor/components/PluginActionToolbar/components/ActionSettings.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
app/client/src/entities/Action/index.ts

[error] 85-85: Shouldn't redeclare 'AutoGeneratedHeader'. Consider to delete it or rename it.

'AutoGeneratedHeader' is defined here:

(lint/suspicious/noRedeclare)

⏰ Context from checks skipped due to timeout of 90000ms (5)
  • GitHub Check: client-unit-tests / client-unit-tests
  • GitHub Check: client-lint / client-lint
  • GitHub Check: client-build / client-build
  • GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
  • GitHub Check: client-prettier / prettier-check
🔇 Additional comments (21)
app/client/src/pages/Editor/JSEditor/Form.tsx (2)

58-58: LGTM! Type import addition.

The addition of OnUpdateSettingsProps type import improves type safety and maintainability.


65-65: LGTM! Enhanced type safety for onUpdateSettings prop.

The explicit function signature with OnUpdateSettingsProps parameter type replaces the indirect type reference, making the interface more maintainable.

app/client/src/PluginActionEditor/components/PluginActionResponse/components/Response/Response.tsx (3)

18-18: LGTM! Import path changes align with folder restructuring.

The switch to relative imports improves maintainability by making component relationships more explicit.

Also applies to: 34-34


Line range hint 140-149: LGTM! Clean removal of feature flag.

The simplified navigation logic aligns with the PR objective of removing feature flagging for the Action Redesign.


Line range hint 36-266: Well-structured component with robust error handling.

The component demonstrates good practices:

  • Clear state management
  • Proper memoization
  • Comprehensive error handling
  • Type-safe implementation
app/client/src/pages/Editor/JSEditor/JSEditorToolbar/components/JSFunctionSettings.tsx (2)

10-10: LGTM! Clean import of the new type.

The import of OnUpdateSettingsProps type aligns with the removal of the old settings view.


15-15: Props interface update improves type safety.

The explicit function type signature provides better type checking compared to the previous inherited type.

app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSEditorToolbar.tsx (1)

4-4: LGTM! Clean type import addition.

The new type import is well-organized with other type imports.

app/client/src/pages/Editor/JSEditor/JSEditorContextMenu.tsx (1)

56-57: LGTM! Clean simplification of button props.

The removal of feature flag conditional logic aligns with the PR objective of removing the Action Redesign feature flag.

app/client/src/ce/pages/Editor/IDE/EditorPane/Query/hooks.tsx (1)

Line range hint 178-223: LGTM! Clean removal of feature flag logic.

The simplified route configuration aligns with the PR objective of removing the Action Redesign feature flag. The routes now consistently use the new components.

app/client/src/PluginActionEditor/components/PluginActionSettings/SettingsPopover.tsx (1)

3-3: LGTM! Verify import path consistency.

The import path change aligns well with the component reorganization effort.

Let's verify the consistency of this new import path across the codebase:

✅ Verification successful

Import path change verified successfully

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining old import paths
rg "from \"pages/Editor/ActionSettings\"" 

# Verify the new import path usage
rg "from \"\.\./PluginActionToolbar/components/ActionSettings\"" 

Length of output: 271


Script:

#!/bin/bash
# Verify ActionSettings component exists at the new location
fd ActionSettings.tsx -p app/client/src/PluginActionEditor

# Check for any other variations of ActionSettings imports
rg "ActionSettings\"" --type ts

Length of output: 481

app/client/src/pages/Editor/IDE/hooks.ts (2)

214-214: LGTM! Clean simplification of the nudge condition.

The simplified condition makes the code more maintainable while preserving the core functionality.


Line range hint 214-224: Verify the nudge behavior in production scenarios.

Since this feature is moving to GA, ensure the nudge appears correctly for:

  1. New users with widget bindings
  2. Existing users who haven't seen the nudge
  3. Users who have dismissed the nudge previously
✅ Verification successful

Side-by-side nudge implementation is correctly simplified

The removal of the feature flag is safe as:

  • Hook is only used in ScreenModeToggle component
  • Local storage persistence remains unchanged
  • Existing test coverage in Cypress maintained
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for other components that might depend on the old feature flag
rg "FEATURE_FLAG.+ACTION_REDESIGN" -l

# Look for any tests that might need updating
rg "useShowSideBySideNudge" -A 5 -g "**/*.test.{ts,tsx}"

Length of output: 94


Script:

#!/bin/bash
# Search for hook usage and related components
rg "useShowSideBySideNudge" -l
rg "NUDGE_SHOWN_SPLIT_PANE" -l

# Search for widget bindings related code near nudge references
rg "widgetBindings" -A 5 -B 5

# Look for any side-by-side or split pane related components
rg "SplitPane" -l

Length of output: 1672

app/client/src/pages/Editor/IDE/EditorPane/JS/JSRender.test.tsx (3)

Line range hint 133-148: Test stability improvement with waitFor!

Good addition of waitFor with appropriate timeout and interval for checking element presence. This makes the test more reliable by properly handling async rendering.


Line range hint 190-216: LGTM - Consistent with full screen changes!

The split screen test changes maintain consistency with the full screen mode changes, correctly reflecting the new UI structure after Action Redesign.


Line range hint 153-168: Verify complete toolbar element coverage.

The test now focuses on toolbar elements instead of form elements, which aligns with the Action Redesign. Consider adding checks for any other critical toolbar elements that might be part of the new design.

✅ Verification successful

Toolbar element coverage is complete and appropriate

The current test assertions cover all essential toolbar elements: function execution buttons, settings, and additional actions. No evidence of missing toolbar elements was found in the codebase.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for toolbar-related test IDs and roles in the codebase
rg -g '*.tsx' -g '*.ts' 't--js-.*-trigger|role="button".*js' app/client/src/pages/Editor/IDE

Length of output: 203


Script:

#!/bin/bash
# Search for toolbar components in implementation and test files
echo "=== Implementation files ==="
ast-grep --pattern 'data-testid="t--js-' app/client/src/pages/Editor/IDE

echo -e "\n=== Test assertions ==="
rg -g '*.test.tsx' -g '*.test.ts' 'getByTestId\("t--js-|getByRole\("button".*js' app/client/src/pages/Editor/IDE

Length of output: 428


Script:

#!/bin/bash
# Search for toolbar and button components in JS editor implementation
echo "=== JS Editor Components ==="
rg -g '*.tsx' -B 2 -A 2 'className=".*toolbar|Button.*props' app/client/src/pages/Editor/IDE/EditorPane/JS

echo -e "\n=== Component Interfaces ==="
ast-grep --pattern 'interface $_ {
  $$$
}' app/client/src/pages/Editor/IDE/EditorPane/JS/JSEditorToolbar.tsx

Length of output: 473

app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSEditorToolbar.test.tsx (1)

Line range hint 37-58: Remove redundant feature flag mock calls.

The test case no longer needs to set the feature flag value since the feature is now enabled by default.

  it("renders IDEToolbar with JSFunctionRun and JSFunctionSettings", () => {
-   mockUseFeatureFlag.mockReturnValue(true);
    render(<JSEditorToolbar {...defaultProps} />);

Additionally, verify if we need to add test coverage for any new default behaviors introduced after removing the feature flag.

✅ Verification successful

Remove redundant feature flag mocks from all test cases

The feature flag mocks can be safely removed from all test cases as they're redundant. The existing test coverage is comprehensive and already verifies all component behaviors with the feature enabled.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for any new test files or test cases added in other related components
rg -t tsx -t ts "describe\(.*JSEditor" 

Length of output: 68


Script:

#!/bin/bash
# Find all JSEditor related test files
fd "JSEditor.*\.test\.(ts|tsx)$"

# Search for mockUseFeatureFlag usage in test files
rg "mockUseFeatureFlag" -t ts -t typescript -t tsx

# Look for existing test cases in JSEditor files
rg "it\(" --type ts --type typescript --type tsx -g "*JSEditor*.test.*"

Length of output: 307


Script:

#!/bin/bash
# View the test file content
cat app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSEditorToolbar.test.tsx

# Search for mockUseFeatureFlag in this file
grep "mockUseFeatureFlag" app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSEditorToolbar.test.tsx

# Look for test cases in this file
grep "it(" app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSEditorToolbar.test.tsx

Length of output: 4556

app/client/src/pages/Editor/IDE/EditorPane/Query/QueryRender.test.tsx (3)

Line range hint 201-225: Consistent test ID update from action-form to api-editor-form.

The test ID has been updated from t--action-form-API to t--api-editor-form, which is more specific and aligns with the editor type.


Line range hint 555-567: Consistent test updates for Google Sheets routes.

The Google Sheets routes follow the same pattern of updates:

  • Element count reduced to 2
  • Form test ID updated to t--uqi-editor-form

Also applies to: 609-618


Line range hint 358-373: Verify consistent form test IDs across query types.

The Postgres routes now use t--uqi-editor-form instead of the previous action form test ID. This seems to be a standardization across different query types.

Also applies to: 414-421

✅ Verification successful

Test IDs are correctly standardized across query types

The codebase shows consistent usage of test IDs:

  • t--api-editor-form for API queries
  • t--uqi-editor-form for Postgres and other UQI-based queries (Sheets, DB)
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for all editor form test IDs to verify consistency
rg "t--.*-editor-form" "app/client/src/pages/Editor"

Length of output: 718


Script:

#!/bin/bash
# Search for action form test IDs
rg "action.*editor.*form" -i "app/client/src"

# Search for all query-related test files to understand the context better
fd "query.*test" -e tsx -e ts "app/client/src" --exec rg "getByTestId|queryByTestId|findByTestId" {}

Length of output: 25308

app/client/src/PluginActionEditor/components/PluginActionForm/components/CommonEditorForm/utils/autoGeneratedHeaders.ts (1)

1-1: Verify removed code references.

The AI summary indicates removal of sortedDatasourcesHandler function and local AutoGeneratedHeader interface. Let's verify there are no lingering references.

✅ Verification successful

Changes are safe to proceed

The removed code has been properly migrated. The AutoGeneratedHeader interface is now consistently imported from entities/Action, and there are no remaining references to the removed sortedDatasourcesHandler function.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining references to the removed function
rg "sortedDatasourcesHandler" --type ts

# Verify the new interface import is used consistently
ast-grep --pattern 'import { $_ } from "entities/Action"' --lang ts

Length of output: 1192


Script:

#!/bin/bash
# Search for AutoGeneratedHeader usage
rg "AutoGeneratedHeader" --type ts -A 2

# Search for any potential string literals
rg '"AutoGeneratedHeader"' --type ts

Length of output: 9708

@hetunandu hetunandu added the ok-to-test Required label for CI label Jan 15, 2025
@hetunandu hetunandu merged commit 1963a9a into release Jan 16, 2025
85 of 87 checks passed
@hetunandu hetunandu deleted the chore/plugin-action-editor-ga branch January 16, 2025 16:56
github-actions bot pushed a commit to Zeral-Zhang/appsmith that referenced this pull request Feb 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Required label for CI skip-changelog Adding this label to a PR prevents it from being listed in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant