-
Notifications
You must be signed in to change notification settings - Fork 4k
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
feat: added custom action config control type for paragon integrations #39764
Conversation
WalkthroughThis PR introduces a new React component, Changes
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (8)
🔇 Additional comments (4)
🪧 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
|
/build-deploy-preview skip-tests=true |
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/13919194610. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/src/components/formControls/CustomActionsConfigControl/index.tsx (1)
72-75
: Update JSDoc comment to match component purposeThe JSDoc comment incorrectly describes this component as being used for "function calling" when it's actually for "custom actions" configuration.
/** - * This component is used to configure the function calling for the AI assistant. - * It allows the user to add, edit and delete functions that can be used by the AI assistant. + * This component is used to configure custom actions for Paragon integrations. + * It allows the user to define HTTP endpoints with methods, paths, headers, parameters and body. */
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/client/src/components/formControls/CustomActionsConfigControl/index.tsx
(1 hunks)app/client/src/utils/formControl/FormControlRegistry.tsx
(2 hunks)app/client/src/utils/formControl/formControlTypes.ts
(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
app/client/src/utils/formControl/FormControlRegistry.tsx (1)
app/client/src/components/formControls/CustomActionsConfigControl/index.tsx (1) (1)
CustomActionsControl
(76:119)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: client-lint / client-lint
- 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-build / client-build
🔇 Additional comments (7)
app/client/src/utils/formControl/formControlTypes.ts (1)
27-27
: LGTM: New form control type added correctlyThe new constant
CUSTOM_ACTIONS_CONFIG_FORM
follows the existing naming convention and pattern. This addition is clean and will enable registration of the custom actions configuration form control.app/client/src/utils/formControl/FormControlRegistry.tsx (2)
50-50
: Import added correctlyThe import for the
CustomActionsControl
component is properly placed with the other component imports.
244-251
: Correctly registered the new control builderThe registration of the new control builder follows the established pattern in this file. The implementation is clean and passes the control props correctly to the component.
app/client/src/components/formControls/CustomActionsConfigControl/index.tsx (4)
10-14
: Good use of enum for tab organizationThe enum provides a clear structure for organizing the tabs in the form control.
16-70
: Well-structured tabbed interface implementationThe
TabbedControls
component is well-organized and follows a clean pattern for creating the tabbed interface. Each tab is properly configured with the appropriate form control type.Notable points:
- Good reuse of existing form controls
- Proper configuration of properties for each tab
- Clear tab structure with appropriate labels from the message system
The TypeScript error suppressions are necessary due to incomplete type definitions in the
FormControl
component.
76-80
: Control type implementation is correctThe
getControlType
method correctly returns the string constant that matches the new form control type added toformControlTypes.ts
.
83-117
: Well-structured form layout with appropriate controlsThe component's render method creates a clean grid layout with:
- A dropdown for HTTP methods
- A dynamic input text field for the path
- The tabbed controls for headers, params, and body
The structure is intuitive and will provide a good user experience for configuring custom actions.
Deploy-Preview-URL: https://ce-39764.dp.appsmith.com |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (6)
app/client/src/components/formControls/CustomActionsConfigControl/index.tsx (6)
16-70
: Consider addressing the TypeScript definition issues.The component uses
@ts-expect-error
comments to suppress TypeScript errors for theshowHeader
property. While this works as a temporary solution, it would be better to properly fix the TypeScript definitions in theFormControl
component.Also, you're passing
formName
twice toFormControl
- both inconfig
and as a direct prop. This redundancy could be eliminated.
27-40
: Consider adding validation for headers.The headers tab currently has
isValid: true
set statically. Consider implementing proper validation for header keys and values, especially for standard HTTP headers.
41-54
: Consider adding validation for URL parameters.Similar to headers, the params tab has
isValid: true
set statically. URL parameters might benefit from validation to ensure they follow correct format (no spaces, special characters handled properly).
55-67
: Body editor might need content-type awareness.The body editor uses a generic "QUERY_DYNAMIC_TEXT" control type. Consider enhancing this to be aware of different content types (JSON, form data, etc.) and provide appropriate editing capabilities for each.
83-116
: Consider adding accessibility attributes.The form controls would benefit from proper accessibility attributes such as aria-labels. This would improve usability for screen readers and other assistive technologies.
Additionally, there's another instance of TypeScript definition issues being suppressed with
@ts-expect-error
on line 95. Consider fixing the underlying type definition issue.
110-110
: Consider making the placeholder more descriptive.The placeholder "/v1/users" is a good example, but you might want to add a comment or make it more descriptive to indicate the expected format or constraints for the path.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
app/client/src/components/formControls/CustomActionsConfigControl/index.tsx
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-build / client-build
- GitHub Check: client-lint / client-lint
🔇 Additional comments (3)
app/client/src/components/formControls/CustomActionsConfigControl/index.tsx (3)
1-9
: Import organization looks good.The necessary imports for React, form controls, UI components, and constants are present and appropriately organized.
10-14
: Well-structured enum for tab types.Good use of an enum to define the tab types, which ensures type safety and maintainability.
72-79
: Good documentation and type implementation.The component has clear documentation that explains its purpose. The
getControlType
method properly implements the required interface.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/client/src/components/formControls/CustomActionsConfigControl/NoSearchCommandFound.tsx (1)
30-39
: Potential improvement in option selection logic.The current implementation uses string includes to find an option with "custom action" in its label. This might be fragile if label text changes.
Consider using a more specific identifier or exact match for the custom action option:
- const customActionOption = options.find((option) => - option.label.toLowerCase().includes("custom action"), - ); + const customActionOption = options.find((option) => + option.label.toLowerCase() === "custom action" + ); - if (customActionOption?.value) { - onSelectOptions(customActionOption?.value); + if (customActionOption?.value) { + onSelectOptions(customActionOption.value);app/client/src/components/formControls/CustomActionsConfigControl/index.tsx (1)
81-116
: Good form layout and configuration.The component uses a grid layout to organize the method dropdown and path input side by side, and places the tabbed controls below them. The form controls are appropriately configured.
Consider setting a default value for the HTTP method dropdown to improve user experience:
config={{ controlType: "DROP_DOWN", configProperty: `${props.configProperty}.method`, formName: props.formName, id: `${props.configProperty}.method`, label: "", isValid: true, + initialValue: HTTP_METHOD.GET, // @ts-expect-error FormControl component has incomplete TypeScript definitions for some valid properties options: Object.values(HTTP_METHOD).map((method) => ({ label: method, value: method, })), }}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (4)
app/client/src/ce/constants/messages.ts
(1 hunks)app/client/src/components/formControls/CustomActionsConfigControl/NoSearchCommandFound.tsx
(1 hunks)app/client/src/components/formControls/CustomActionsConfigControl/index.tsx
(1 hunks)app/client/src/components/formControls/DropDownControl.tsx
(2 hunks)
🧰 Additional context used
🧬 Code Definitions (3)
app/client/src/components/formControls/DropDownControl.tsx (1)
app/client/src/components/formControls/CustomActionsConfigControl/NoSearchCommandFound.tsx (1) (1)
NoSearchCommandFound
(11:60)
app/client/src/components/formControls/CustomActionsConfigControl/index.tsx (1)
app/client/src/ce/constants/messages.ts (1) (1)
API_EDITOR_TAB_TITLES
(1396:1403)
app/client/src/components/formControls/CustomActionsConfigControl/NoSearchCommandFound.tsx (1)
app/client/src/ce/constants/messages.ts (2) (2)
NO_SEARCH_COMMAND_FOUND_EXTERNAL_SAAS
(2636:2637)ADD_CUSTOM_ACTION
(2639:2639)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / server-build / server-unit-tests
- 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-prettier / prettier-check
- GitHub Check: client-build / client-build
🔇 Additional comments (12)
app/client/src/components/formControls/DropDownControl.tsx (2)
31-31
: Import for NoSearchCommandFound component added.The import adds support for a custom component that will be shown when no search results are found in a dropdown, specifically for external SaaS actions.
466-473
: Enhanced dropdown with custom "not found" content.The implementation nicely adds user feedback when no dropdown options match a search. Passing the necessary props (
configProperty
,onSelectOptions
,options
, andpluginId
) allows the component to conditionally render a helpful UI for external SaaS plugins.app/client/src/ce/constants/messages.ts (1)
2636-2639
: Added message constants for NoSearchCommandFound component.New internationalized message constants for the dropdown search experience when no results are found. These follow the established pattern of the codebase.
app/client/src/components/formControls/CustomActionsConfigControl/NoSearchCommandFound.tsx (5)
1-9
: Imports and dependencies look good.The component properly imports React, UI components from the design system, Redux hooks, and the necessary selectors and type definitions.
11-24
: Component props and plugin retrieval look good.The component has well-defined props with TypeScript types and uses Redux's
useSelector
to retrieve the plugin information.
26-29
: Clean conditional check for rendering.Good approach to check both the plugin type and the configProperty to determine when to show this component.
41-57
: Well-structured UI rendering.The component renders a clean, focused UI with appropriate spacing, color, and icon usage. Using the
createMessage
utility ensures proper localization.
59-60
: Appropriate fallback rendering.Returning null when conditions aren't met is appropriate and prevents UI clutter when the component isn't needed.
app/client/src/components/formControls/CustomActionsConfigControl/index.tsx (4)
1-9
: Imports and dependencies are appropriate.The component imports the necessary React, UI, and form components needed for its functionality.
10-15
: Good use of enum for tab identifiers.Using an enum for tab identifiers provides type safety and makes the code more maintainable.
16-70
: Well-structured TabbedControls component.The component effectively organizes different configuration sections into tabs, making the interface more user-friendly. The form controls are appropriately configured for each tab.
Note: There are multiple
@ts-expect-error
comments due to incomplete TypeScript definitions in the FormControl component. Consider updating the type definitions when possible.Are there plans to update the FormControl component's TypeScript definitions to avoid these errors in the future?
72-80
: CustomActionsControl class with clear purpose.The component is well-documented with JSDoc comments explaining its purpose. The control type is correctly defined.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (4)
app/client/src/components/formControls/CustomActionsConfigControl/index.tsx (4)
17-38
: Consider more maintainable CSS selectors in styled componentThe styled component uses deep nesting and specific child selectors that may be fragile to DOM structure changes. Consider using more targeted classnames or data attributes instead of relying on child selectors.
const TabbedWrapper = styled(Tabs)` .t--form-control-KEYVALUE_ARRAY { & > div { - margin-bottom: var(--ads-v2-spaces-3); + margin-bottom: var(--ads-v2-spaces-3); & > * { flex-grow: 1; } - & > *:first-child { + & > .key-field { max-width: 184px; } - & > *:nth-child(2) { + & > .value-field { margin-left: var(--ads-v2-spaces-3); } & > .t--delete-field { max-width: 34px; } } & .t--add-field { height: 24px; } } `;
40-94
: Add JSDoc comment for TabbedControls componentThe
TabbedControls
component lacks documentation. Consider adding a JSDoc comment explaining its purpose and props, similar to what you've done for the mainCustomActionsControl
component.+/** + * This component renders a tabbed interface for configuring headers, params, and body + * of a custom action. It's used as part of the CustomActionsControl. + */ const TabbedControls = (props: ControlProps) => { return ( <TabbedWrapper defaultValue={CUSTOM_ACTION_TABS.HEADERS}>
51-64
: Consider refactoring duplicate FormControl configurationsThe HEADERS and PARAMS tabs have almost identical FormControl configurations. Consider extracting this pattern to reduce duplication.
+const createKeyValueFormControl = ( + configProperty: string, + formName: string, + tabType: CUSTOM_ACTION_TABS.HEADERS | CUSTOM_ACTION_TABS.PARAMS +) => { + return ( + <FormControl + config={{ + controlType: "KEYVALUE_ARRAY", + configProperty: `${configProperty}.${tabType.toLowerCase()}`, + formName: formName, + id: `${configProperty}.${tabType.toLowerCase()}`, + isValid: true, + // @ts-expect-error FormControl component has incomplete TypeScript definitions for some valid properties + showHeader: true, + }} + formName={formName} + /> + ); +}; // Then in the TabPanel sections: <TabPanel value={CUSTOM_ACTION_TABS.HEADERS}> - <FormControl - config={{ - controlType: "KEYVALUE_ARRAY", - configProperty: `${props.configProperty}.headers`, - formName: props.formName, - id: `${props.configProperty}.headers`, - isValid: true, - // @ts-expect-error FormControl component has incomplete TypeScript definitions for some valid properties - showHeader: true, - }} - formName={props.formName} - /> + {createKeyValueFormControl(props.configProperty, props.formName, CUSTOM_ACTION_TABS.HEADERS)} </TabPanel>Also applies to: 65-78
134-135
: Consider adding validation for path inputThe path input field has a placeholder, but there's no validation to ensure the path follows the correct format. Consider adding validation to ensure it starts with a forward slash and follows API path conventions.
config={{ controlType: "QUERY_DYNAMIC_INPUT_TEXT", configProperty: `${props.configProperty}.path`, formName: props.formName, id: `${props.configProperty}.path`, label: "", isValid: true, placeholderText: "/v1/users", + validationRegex: "^\/.*", + validationMessage: "Path must start with a forward slash", }}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
app/client/src/components/formControls/CustomActionsConfigControl/index.tsx
(1 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
app/client/src/components/formControls/CustomActionsConfigControl/index.tsx (1)
app/client/src/ce/constants/messages.ts (1) (1)
API_EDITOR_TAB_TITLES
(1396-1403)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / server-build / server-unit-tests
- 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 (2)
app/client/src/components/formControls/CustomActionsConfigControl/index.tsx (2)
100-143
: The CustomActionsControl implementation looks goodThe main component implementation is well structured with a clear separation of UI elements. The HTTP method dropdown and path input are appropriately placed in a grid layout above the tabbed interface.
101-103
:✅ Verification successful
Verify control type registration
The control type "CUSTOM_ACTIONS_CONFIG_FORM" is returned in the getControlType method, but I don't see where this is registered in FormControlRegistry. Ensure this control type is properly registered for the component to be usable.
🏁 Script executed:
#!/bin/bash # Check if the control type is registered rg -p "CUSTOM_ACTIONS_CONFIG_FORM" --type tsLength of output: 395
Verification Complete: Control Type Registration Confirmed
I've verified that the control type
"CUSTOM_ACTIONS_CONFIG_FORM"
is correctly registered in bothapp/client/src/utils/formControl/formControlTypes.ts
(line 27) andapp/client/src/utils/formControl/FormControlRegistry.tsx
(line 245). No further changes are necessary.
// @ts-expect-error FormControl component has incomplete TypeScript definitions for some valid properties | ||
showHeader: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Address TypeScript errors properly
There are multiple @ts-expect-error
comments indicating incomplete type definitions for FormControl. Consider creating proper type interfaces or contributing to the existing ones instead of suppressing these errors.
Also applies to: 72-74
🏁 Script executed:
#!/bin/bash
# Check FormControl type definitions to understand what's missing
fd FormControl --type file --exec grep -l "interface.*FormControlProps" {}
Length of output: 124
Update FormControl types to remove the need for error suppression
It looks like the FormControl component’s TypeScript definitions (defined in app/client/src/pages/Editor/FormControl.tsx
) are missing the showHeader
property. Please update the FormControlProps
interface to include showHeader
(and any other missing props), then remove the accompanying @ts-expect-error
comments in:
app/client/src/components/formControls/CustomActionsConfigControl/index.tsx
(lines 59–60)app/client/src/components/formControls/CustomActionsConfigControl/index.tsx
(lines 72–74)
This change will ensure TypeScript accurately validates the component without suppressing errors.
import React from "react"; | ||
import type { ControlType } from "constants/PropertyControlConstants"; | ||
import FormControl from "pages/Editor/FormControl"; | ||
import { Grid, Tabs, TabPanel, TabsList, Tab, Flex } from "@appsmith/ads"; | ||
import BaseControl, { type ControlProps } from "../BaseControl"; | ||
import { HTTP_METHOD } from "PluginActionEditor/constants/CommonApiConstants"; | ||
import { API_EDITOR_TAB_TITLES } from "ee/constants/messages"; | ||
import { createMessage } from "ee/constants/messages"; | ||
import styled from "styled-components"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Verify import path for API_EDITOR_TAB_TITLES
The import statement uses ee/constants/messages
for API_EDITOR_TAB_TITLES
, but the provided context shows this constant is defined in app/client/src/ce/constants/messages.ts
. Ensure the import path is correct.
🏁 Script executed:
#!/bin/bash
# Check where API_EDITOR_TAB_TITLES is actually defined
rg -p "API_EDITOR_TAB_TITLES.*=.*{" --type ts
Length of output: 165
Below is a revised review comment after noting that the initial regex search failed. I've generated an updated shell script to re-check the definition (please run it to obtain additional details), and based on the inconclusive automated output, I'm flagging this for manual verification.
Action Required: Verify the Import Path for API_EDITOR_TAB_TITLES
The current code imports API_EDITOR_TAB_TITLES
from ee/constants/messages
. However, context suggests it may be defined in app/client/src/ce/constants/messages.ts
. Please verify the actual definition location and update the import path if necessary.
To assist further, please run the script below:
- Verify if
API_EDITOR_TAB_TITLES
is actually defined inapp/client/src/ce/constants/messages.ts
. - Update the import in
app/client/src/components/formControls/CustomActionsConfigControl/index.tsx
accordingly if needed.
🏁 Script executed:
#!/bin/bash
echo "Searching for API_EDITOR_TAB_TITLES definition across the repository:"
rg "API_EDITOR_TAB_TITLES\s*=" --type ts || true
echo ""
echo "Searching in app/client/src/ce/constants/messages.ts for API_EDITOR_TAB_TITLES:"
grep -R "API_EDITOR_TAB_TITLES" app/client/src/ce/constants/messages.ts || true
Length of output: 683
Action Required: Correct Import Path for API_EDITOR_TAB_TITLES
- The constant is defined in
app/client/src/ce/constants/messages.ts
(confirmed via grep). - The current import in
app/client/src/components/formControls/CustomActionsConfigControl/index.tsx
uses"ee/constants/messages"
. Update this to reference the correct path.
app/client/src/components/formControls/CustomActionsConfigControl/NoSearchCommandFound.tsx
Outdated
Show resolved
Hide resolved
@@ -462,6 +463,14 @@ function renderDropdown( | |||
isLoading={props.isLoading} | |||
isMultiSelect={isMultiSelect} | |||
maxTagCount={props.maxTagCount} | |||
notFoundContent={ | |||
<NoSearchCommandFound |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AmanAgarwal041 Is it possible to make this config driven? Asking because this notContentFound is not very generic, it's very specific to external saas plugin but DropdownControl is actually generic
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sneha122 If you see the implementation of this : https://github.com/appsmithorg/appsmith/pull/39764/files#diff-dec54d51495715bd2faf9ea73dc09419fc8e14da8577fadab3fa09878df6bdc5R40
It is specific to external saas plugin. Any ideas on how we can do it via config ? Because it requires some functionalities as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that's true, even passing this component in notContentFound is specific to external saas plugin. Doing it via config would also be tricky, we can probably have component name in the config but the options and what happens on select etc can't be passed through config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hence, going ahead with the current implementation. @sneha122
Description
Adding Custom Action Control type for external integrations to enable custom actions.
Fixes #39650
or
Fixes
Issue URL
Warning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.Datasource, @tag.IDE"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/14028489940
Commit: aacec72
Cypress dashboard.
Tags:
@tag.Datasource, @tag.IDE
Spec:
Mon, 24 Mar 2025 07:05:03 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Summary by CodeRabbit