From 88b987fd184c174cbddd096fc539f36c842377d7 Mon Sep 17 00:00:00 2001 From: Nilansh Bansal Date: Thu, 24 Oct 2024 12:18:26 +0530 Subject: [PATCH 1/6] chore: removed unwanted metrics (#37052) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description > This PR removes the extra metrics that were added which clouded up the newrelic. Old PR https://github.com/appsmithorg/appsmith/pull/37010 is to be closed because extra indentation was added due to the IDE configuration. Fixes #37051 ## Automation /ok-to-test tags="@tag.Sanity" ### :mag: Cypress test results > [!IMPORTANT] > 🟣 🟣 🟣 Your tests are running. > Tests running at: > Commit: 51ecb28956b5c7fa6b12e18428161614017407ba > Workflow: `PR Automation test suite` > Tags: `@tag.Sanity` > Spec: `` >
Thu, 24 Oct 2024 06:16:14 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No --- .../constants/spans/ce/OnLoadSpanCE.java | 2 -- .../internal/OnLoadExecutablesUtilCEImpl.java | 18 ++---------------- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/OnLoadSpanCE.java b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/OnLoadSpanCE.java index 5b7dcc7ea20d..138aa40f93f2 100644 --- a/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/OnLoadSpanCE.java +++ b/app/server/appsmith-interfaces/src/main/java/com/appsmith/external/constants/spans/ce/OnLoadSpanCE.java @@ -11,10 +11,8 @@ public class OnLoadSpanCE { public static final String EXECUTABLE_IN_CREATOR_CONTEXT = APPSMITH_SPAN_PREFIX + "executablesInCreatorContext"; public static final String ADD_DIRECTLY_REFERENCED_EXECUTABLES_TO_GRAPH = APPSMITH_SPAN_PREFIX + "addDirectlyReferencedExecutablesToGraph"; - public static final String GET_POSSIBLE_ENTITY_REFERENCES = APPSMITH_SPAN_PREFIX + "getPossibleEntityReferences"; public static final String UPDATE_EXECUTABLE_SELF_REFERENCING_PATHS = APPSMITH_SPAN_PREFIX + "updateExecutableSelfReferencingPaths"; - public static final String GET_POSSIBLE_ENTITY_PARENTS_MAP = APPSMITH_SPAN_PREFIX + "getPossibleEntityParentsMap"; public static final String ADD_EXPLICIT_USER_SET_ON_LOAD_EXECUTABLES_TO_GRAPH = APPSMITH_SPAN_PREFIX + "addExplicitUserSetOnLoadExecutablesToGraph"; public static final String GET_UNPUBLISHED_ON_LOAD_EXECUTABLES_EXPLICIT_SET_BY_USER_IN_CREATOR_CONTEXT = diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java index 7f8cc405cfae..08c6840bc62c 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/onload/internal/OnLoadExecutablesUtilCEImpl.java @@ -56,8 +56,6 @@ import static com.appsmith.external.constants.spans.OnLoadSpan.ADD_EXPLICIT_USER_SET_ON_LOAD_EXECUTABLES_TO_GRAPH; import static com.appsmith.external.constants.spans.OnLoadSpan.EXECUTABLE_NAME_TO_EXECUTABLE_MAP; import static com.appsmith.external.constants.spans.OnLoadSpan.GET_ALL_EXECUTABLES_BY_CREATOR_ID; -import static com.appsmith.external.constants.spans.OnLoadSpan.GET_POSSIBLE_ENTITY_PARENTS_MAP; -import static com.appsmith.external.constants.spans.OnLoadSpan.GET_POSSIBLE_ENTITY_REFERENCES; import static com.appsmith.external.constants.spans.OnLoadSpan.GET_UNPUBLISHED_ON_LOAD_EXECUTABLES_EXPLICIT_SET_BY_USER_IN_CREATOR_CONTEXT; import static com.appsmith.external.constants.spans.OnLoadSpan.UPDATE_EXECUTABLE_SELF_REFERENCING_PATHS; import static com.appsmith.external.helpers.MustacheHelper.EXECUTABLE_ENTITY_REFERENCES; @@ -505,9 +503,7 @@ private Mono>> filterAndTransformSchedulingOrderToDTO */ private Mono> getPossibleEntityReferences( Mono> executableNameToExecutableMapMono, Set bindings, int evalVersion) { - return getPossibleEntityReferences(executableNameToExecutableMapMono, bindings, evalVersion, null) - .name(GET_POSSIBLE_ENTITY_REFERENCES) - .tap(Micrometer.observation(observationRegistry)); + return getPossibleEntityReferences(executableNameToExecutableMapMono, bindings, evalVersion, null); } /** @@ -530,9 +526,7 @@ private Mono> getPossibleEntityReferences( final int entityTypes = EXECUTABLE_ENTITY_REFERENCES | WIDGET_ENTITY_REFERENCES; return executableNameToExecutableMono - .zipWith(getPossibleEntityParentsMap(bindings, entityTypes, evalVersion) - .name(GET_POSSIBLE_ENTITY_PARENTS_MAP) - .tap(Micrometer.observation(observationRegistry))) + .zipWith(getPossibleEntityParentsMap(bindings, entityTypes, evalVersion)) .map(tuple -> { Map executableMap = tuple.getT1(); // For each binding, here we receive a set of possible references to global entities @@ -647,8 +641,6 @@ private Mono> addDirectlyReferencedExecutablesToGr bindingsInWidget, evalVersion, executableBindingsInDslRef) - .name(GET_POSSIBLE_ENTITY_REFERENCES) - .tap(Micrometer.observation(observationRegistry)) .flatMapMany(Flux::fromIterable) // Add dependencies of the executables found in the DSL in the graph // We are ignoring the widget references at this point @@ -939,8 +931,6 @@ private Mono> recursivelyAddExecutablesAndTheirDep // First fetch all the executables in the page whose name matches the words found in all the dynamic bindings Mono> findAndAddExecutablesInBindingsMono = getPossibleEntityReferences( executableNameToExecutableMapMono, dynamicBindings, evalVersion) - .name(GET_POSSIBLE_ENTITY_REFERENCES) - .tap(Micrometer.observation(observationRegistry)) .flatMapMany(Flux::fromIterable) // Add dependencies of the executables found in the DSL in the graph. .flatMap(possibleEntity -> { @@ -1115,8 +1105,6 @@ private Mono extractAndSetExecutableBindingsInGraphEdges( executableBindingsMap.get(bindingPath), evalVersion, bindingsInDsl) - .name(GET_POSSIBLE_ENTITY_REFERENCES) - .tap(Micrometer.observation(observationRegistry)) .flatMapMany(Flux::fromIterable) .map(relatedDependencyNode -> { bindingsFromExecutables.add(relatedDependencyNode.getReferenceString()); @@ -1147,8 +1135,6 @@ private Mono> addWidgetRelationshipToGraph( return Flux.fromIterable(widgetBindingMap.entrySet()) .flatMap(widgetBindingEntries -> getPossibleEntityParentsMap( widgetBindingEntries.getValue(), entityTypes, evalVersion) - .name(GET_POSSIBLE_ENTITY_PARENTS_MAP) - .tap(Micrometer.observation(observationRegistry)) .map(possibleParentsMap -> { possibleParentsMap.entrySet().stream().forEach(entry -> { if (entry.getValue() == null || entry.getValue().isEmpty()) { From 23a577285f71ddb9c8579d6464db88ea924bd27c Mon Sep 17 00:00:00 2001 From: Pawan Kumar Date: Thu, 24 Oct 2024 13:19:30 +0530 Subject: [PATCH 2/6] chore: Create query selector (#37018) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit /ok-to-test tags="@tag.All" This PR updates the WidgetQueryGeneratorForm to only allow queries so that it looks like a query selector. This is needed for chat widget where the widget can only bind Appsmith AI queries. ![CleanShot 2024-10-23 at 12 30 32](https://github.com/user-attachments/assets/c1237435-3e9e-4cfd-8c2e-c762c76089bd) PS: The above image is an example where TableWidget had allowedDatasourcestypes as "Mysql" ( just for testing ) ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Introduced a new dropdown variant "Connect to query" for improved datasource selection. - Enhanced handling of dynamic query options and datasource filtering based on context. - **Improvements** - Updated the Widget Query Generator Form to support new properties for better data management. - Improved control flow in dropdown options to ensure appropriate options are displayed based on the current context. - Enhanced the One Click Binding Control to utilize new properties for better interaction with the Widget Query Generator Form. - **Bug Fixes** - Refined logic for displaying datasource options, preventing unnecessary options from appearing. > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: d6e322c649ef07c5fa58cd16fa0a5d97e55f7129 > Cypress dashboard. > Tags: `@tag.All` > Spec: >
Thu, 24 Oct 2024 06:53:37 UTC --- app/client/packages/dsl/src/index.ts | 1 + app/client/src/actions/datasourceActions.ts | 7 +-- .../DatasourceDropdown/types.ts | 1 + .../useSource/useConnectToOptions.tsx | 38 ++++++++++--- .../useSource/useDatasourceOptions.tsx | 11 ++-- .../useSource/useOtherOptions.tsx | 54 ++++++++++--------- .../WidgetQueryGeneratorForm/index.tsx | 15 +++++- .../OneClickBindingControl.tsx | 10 +++- 8 files changed, 97 insertions(+), 40 deletions(-) diff --git a/app/client/packages/dsl/src/index.ts b/app/client/packages/dsl/src/index.ts index 6696835075ac..65b82c1f4bf7 100644 --- a/app/client/packages/dsl/src/index.ts +++ b/app/client/packages/dsl/src/index.ts @@ -11,3 +11,4 @@ export type { export { migrateDSL, LATEST_DSL_VERSION } from "./migrate"; export type { DSLWidget } from "./migrate/types"; +export { isDynamicValue } from "./migrate/utils"; diff --git a/app/client/src/actions/datasourceActions.ts b/app/client/src/actions/datasourceActions.ts index 250b6cc5b874..965f37bee77c 100644 --- a/app/client/src/actions/datasourceActions.ts +++ b/app/client/src/actions/datasourceActions.ts @@ -32,9 +32,10 @@ export const createDatasourceFromForm = ( }; }; -export const createTempDatasourceFromForm = ( - payload: CreateDatasourceConfig | Datasource, -) => { +export const createTempDatasourceFromForm = (payload: { + pluginId: string; + type: PluginType; +}) => { return { type: ReduxActionTypes.CREATE_TEMP_DATASOURCE_FROM_FORM_SUCCESS, payload, diff --git a/app/client/src/components/editorComponents/WidgetQueryGeneratorForm/CommonControls/DatasourceDropdown/types.ts b/app/client/src/components/editorComponents/WidgetQueryGeneratorForm/CommonControls/DatasourceDropdown/types.ts index e17856a4fbd6..3a4e4f49977f 100644 --- a/app/client/src/components/editorComponents/WidgetQueryGeneratorForm/CommonControls/DatasourceDropdown/types.ts +++ b/app/client/src/components/editorComponents/WidgetQueryGeneratorForm/CommonControls/DatasourceDropdown/types.ts @@ -1,4 +1,5 @@ export enum DROPDOWN_VARIANT { CONNECT_TO_DATASOURCE = "Connect to datasource", CREATE_OR_EDIT_RECORDS = "Create or edit records", + CONNECT_TO_QUERY = "Connect to query", } diff --git a/app/client/src/components/editorComponents/WidgetQueryGeneratorForm/CommonControls/DatasourceDropdown/useSource/useConnectToOptions.tsx b/app/client/src/components/editorComponents/WidgetQueryGeneratorForm/CommonControls/DatasourceDropdown/useSource/useConnectToOptions.tsx index ceedd1fda2b5..8701bd31c7d9 100644 --- a/app/client/src/components/editorComponents/WidgetQueryGeneratorForm/CommonControls/DatasourceDropdown/useSource/useConnectToOptions.tsx +++ b/app/client/src/components/editorComponents/WidgetQueryGeneratorForm/CommonControls/DatasourceDropdown/useSource/useConnectToOptions.tsx @@ -4,6 +4,7 @@ import { getCurrentActions, getCurrentPageWidgets, getPluginIdPackageNamesMap, + getPlugins, getQueryModuleInstances, } from "ee/selectors/entitiesSelector"; import WidgetFactory from "WidgetProvider/factory"; @@ -25,6 +26,7 @@ import type { import type { Module } from "ee/constants/ModuleConstants"; import { getAllModules } from "ee/selectors/modulesSelector"; import { getModuleIcon } from "pages/Editor/utils"; +import { isDynamicValue } from "@shared/dsl"; enum SortingWeights { alphabetical = 1, @@ -74,20 +76,26 @@ export function sortQueries( }); } +export const getDefaultQueryBindingValue = ( + query: ActionData | ModuleInstanceData, +) => `{{${query.config.name}.data}}`; + export function getBindingValue( widget: WidgetProps, query: ActionData | ModuleInstanceData, + getQueryBindingValue: ( + query: ActionData | ModuleInstanceData, + ) => string = getDefaultQueryBindingValue, ) { - const defaultBindingValue = `{{${query.config.name}.data}}`; const querySuggestedWidgets = query.data?.suggestedWidgets; - if (!querySuggestedWidgets) return defaultBindingValue; + if (!querySuggestedWidgets) return getQueryBindingValue(query); const suggestedWidget = querySuggestedWidgets.find( (suggestedWidget) => suggestedWidget.type === widget.type, ); - if (!suggestedWidget) return defaultBindingValue; + if (!suggestedWidget) return getQueryBindingValue(query); return `{{${query.config.name}.${suggestedWidget.bindingQuery}}}`; } @@ -157,7 +165,9 @@ export const getAnalyticsInfo = ( function useConnectToOptions(props: ConnectToOptionsProps) { const { addBinding, + allowedDatasourceTypes, expectedType, + getQueryBindingValue, isConnectableToWidget, propertyName, updateConfig, @@ -165,6 +175,7 @@ function useConnectToOptions(props: ConnectToOptionsProps) { const queries = useSelector(getCurrentActions); const pluginsPackageNamesMap = useSelector(getPluginIdPackageNamesMap); + const plugins = useSelector(getPlugins); const { pluginImages, widget } = props; @@ -182,6 +193,19 @@ function useConnectToOptions(props: ConnectToOptionsProps) { }); } + // filter query based on allowedDatasourceType + if (allowedDatasourceTypes) { + const allowedPluginIds = plugins + .filter((plugin) => allowedDatasourceTypes.includes(plugin.name)) + .map((plugin) => plugin.id); + + if (allowedPluginIds.length) { + filteredQueries = filteredQueries.filter((query) => + allowedPluginIds.includes(query.config.pluginId), + ); + } + } + filteredQueries = [...filteredQueries, ...queryModuleInstances] as | ActionDataState | ModuleInstanceDataState; @@ -190,10 +214,12 @@ function useConnectToOptions(props: ConnectToOptionsProps) { return sortQueries(filteredQueries, expectedType).map((query) => ({ id: query.config.id, label: query.config.name, - value: getBindingValue(widget, query), + value: getBindingValue(widget, query, getQueryBindingValue), icon: getQueryIcon(query, pluginImages, modules), - onSelect: function (value?: string, valueOption?: DropdownOptionType) { - addBinding(valueOption?.value, true); + onSelect: function (value: string, valueOption?: DropdownOptionType) { + const isDynamic = isDynamicValue(value); + + addBinding(valueOption?.value, isDynamic); updateConfig({ datasource: "", diff --git a/app/client/src/components/editorComponents/WidgetQueryGeneratorForm/CommonControls/DatasourceDropdown/useSource/useDatasourceOptions.tsx b/app/client/src/components/editorComponents/WidgetQueryGeneratorForm/CommonControls/DatasourceDropdown/useSource/useDatasourceOptions.tsx index 01c690cfcaa9..cedf9653feba 100644 --- a/app/client/src/components/editorComponents/WidgetQueryGeneratorForm/CommonControls/DatasourceDropdown/useSource/useDatasourceOptions.tsx +++ b/app/client/src/components/editorComponents/WidgetQueryGeneratorForm/CommonControls/DatasourceDropdown/useSource/useDatasourceOptions.tsx @@ -34,6 +34,7 @@ import type { WidgetProps } from "widgets/BaseWidget"; import { WidgetQueryGeneratorFormContext } from "components/editorComponents/WidgetQueryGeneratorForm/index"; import { getAssetUrl } from "ee/utils/airgapHelpers"; import { getDatasourceConnectionMode } from "components/editorComponents/WidgetQueryGeneratorForm/utils"; +import { DROPDOWN_VARIANT } from "../types"; interface DatasourceOptionsProps { widget: WidgetProps; @@ -41,9 +42,8 @@ interface DatasourceOptionsProps { } function useDatasourceOptions(props: DatasourceOptionsProps) { - const { config, propertyName, updateConfig } = useContext( - WidgetQueryGeneratorFormContext, - ); + const { config, datasourceDropdownVariant, propertyName, updateConfig } = + useContext(WidgetQueryGeneratorFormContext); const { pluginImages, widget } = props; const dispatch = useDispatch(); const datasources: Datasource[] = useSelector(getDatasources); @@ -58,6 +58,11 @@ function useDatasourceOptions(props: DatasourceOptionsProps) { useState(false); const [actualDatasourceOptions, mockDatasourceOptions] = useMemo(() => { + // we don't want to show the datasource options for connect to query variant + if (datasourceDropdownVariant === DROPDOWN_VARIANT.CONNECT_TO_QUERY) { + return [[], []]; + } + const availableDatasources = datasources.filter(({ pluginId }) => WidgetQueryGeneratorRegistry.has(pluginsPackageNamesMap[pluginId]), ); diff --git a/app/client/src/components/editorComponents/WidgetQueryGeneratorForm/CommonControls/DatasourceDropdown/useSource/useOtherOptions.tsx b/app/client/src/components/editorComponents/WidgetQueryGeneratorForm/CommonControls/DatasourceDropdown/useSource/useOtherOptions.tsx index 8fac0f697776..e28f1fe53e3c 100644 --- a/app/client/src/components/editorComponents/WidgetQueryGeneratorForm/CommonControls/DatasourceDropdown/useSource/useOtherOptions.tsx +++ b/app/client/src/components/editorComponents/WidgetQueryGeneratorForm/CommonControls/DatasourceDropdown/useSource/useOtherOptions.tsx @@ -35,35 +35,39 @@ function useOtherOptions(props: OtherOptionsProps) { datasourceDropdownVariant === DROPDOWN_VARIANT.CREATE_OR_EDIT_RECORDS; const { widget } = props; const otherOptions = useMemo(() => { - const options = [ - { - icon: , - id: "Connect new datasource", - label: "Connect new datasource", - value: "Connect new datasource", - onSelect: () => { - history.push( - integrationEditorURL({ - basePageId, - selectedTab: INTEGRATION_TABS.NEW, - }), - ); + // we don't want to show the options for connect to query variant + if (datasourceDropdownVariant === DROPDOWN_VARIANT.CONNECT_TO_QUERY) + return []; - AnalyticsUtil.logEvent("BIND_OTHER_ACTIONS", { - widgetName: widget.widgetName, - widgetType: widget.type, - propertyName: propertyName, - selectedAction: "Connect new datasource", - }); + const options = []; - const entryPoint = DatasourceCreateEntryPoints.ONE_CLICK_BINDING; + options.push({ + icon: , + id: "Connect new datasource", + label: "Connect new datasource", + value: "Connect new datasource", + onSelect: () => { + history.push( + integrationEditorURL({ + basePageId, + selectedTab: INTEGRATION_TABS.NEW, + }), + ); - AnalyticsUtil.logEvent("NAVIGATE_TO_CREATE_NEW_DATASOURCE_PAGE", { - entryPoint, - }); - }, + AnalyticsUtil.logEvent("BIND_OTHER_ACTIONS", { + widgetName: widget.widgetName, + widgetType: widget.type, + propertyName: propertyName, + selectedAction: "Connect new datasource", + }); + + const entryPoint = DatasourceCreateEntryPoints.ONE_CLICK_BINDING; + + AnalyticsUtil.logEvent("NAVIGATE_TO_CREATE_NEW_DATASOURCE_PAGE", { + entryPoint, + }); }, - ]; + }); if (sampleData) { options.push({ diff --git a/app/client/src/components/editorComponents/WidgetQueryGeneratorForm/index.tsx b/app/client/src/components/editorComponents/WidgetQueryGeneratorForm/index.tsx index bc1a394bb363..aba8a7770462 100644 --- a/app/client/src/components/editorComponents/WidgetQueryGeneratorForm/index.tsx +++ b/app/client/src/components/editorComponents/WidgetQueryGeneratorForm/index.tsx @@ -16,10 +16,10 @@ import { import { updateOneClickBindingOptionsVisibility } from "actions/oneClickBindingActions"; import type { AlertMessage, Alias, OtherField } from "./types"; import { CONNECT_BUTTON_TEXT, createMessage } from "ee/constants/messages"; - import { DROPDOWN_VARIANT } from "./CommonControls/DatasourceDropdown/types"; +import type { getDefaultQueryBindingValue } from "./CommonControls/DatasourceDropdown/useSource/useConnectToOptions"; -interface WidgetQueryGeneratorFormContextType { +export interface WidgetQueryGeneratorFormContextType { widgetId: string; propertyValue: string; propertyName: string; @@ -53,6 +53,8 @@ interface WidgetQueryGeneratorFormContextType { datasourceDropdownVariant: DROPDOWN_VARIANT; alertMessage?: AlertMessage | null; showEditFieldsModal?: boolean; + allowedDatasourceTypes?: string[]; + getQueryBindingValue?: typeof getDefaultQueryBindingValue; } const DEFAULT_CONFIG_VALUE = { @@ -110,6 +112,8 @@ interface Props { datasourceDropdownVariant: DROPDOWN_VARIANT; actionButtonCtaText?: string; alertMessage?: AlertMessage; + allowedDatasourceTypes?: WidgetQueryGeneratorFormContextType["allowedDatasourceTypes"]; + getQueryBindingValue?: WidgetQueryGeneratorFormContextType["getQueryBindingValue"]; } function WidgetQueryGeneratorForm(props: Props) { @@ -121,10 +125,12 @@ function WidgetQueryGeneratorForm(props: Props) { actionButtonCtaText = createMessage(CONNECT_BUTTON_TEXT), alertMessage, aliases, + allowedDatasourceTypes, datasourceDropdownVariant, errorMsg, excludePrimaryColumnFromQueryGeneration, expectedType, + getQueryBindingValue, isConnectableToWidget, onUpdate, otherFields = [], @@ -247,6 +253,8 @@ function WidgetQueryGeneratorForm(props: Props) { datasourceDropdownVariant, alertMessage, showEditFieldsModal, + allowedDatasourceTypes, + getQueryBindingValue, }; }, [ config, @@ -266,6 +274,9 @@ function WidgetQueryGeneratorForm(props: Props) { datasourceDropdownVariant, alertMessage, showEditFieldsModal, + allowedDatasourceTypes, + getQueryBindingValue, + expectedType, ]); useEffect(() => { diff --git a/app/client/src/components/propertyControls/OneClickBindingControl.tsx b/app/client/src/components/propertyControls/OneClickBindingControl.tsx index a442b898a1d4..38f271173cf4 100644 --- a/app/client/src/components/propertyControls/OneClickBindingControl.tsx +++ b/app/client/src/components/propertyControls/OneClickBindingControl.tsx @@ -1,4 +1,6 @@ -import WidgetQueryGeneratorForm from "components/editorComponents/WidgetQueryGeneratorForm"; +import WidgetQueryGeneratorForm, { + type WidgetQueryGeneratorFormContextType, +} from "components/editorComponents/WidgetQueryGeneratorForm"; import type { AlertMessage, Alias, @@ -70,6 +72,9 @@ class OneClickBindingControl extends BaseControl { actionButtonCtaText={this.props.controlConfig?.actionButtonCtaText} alertMessage={this.props.controlConfig?.alertMessage} aliases={this.props.controlConfig.aliases} + allowedDatasourceTypes={ + this.props.controlConfig?.allowedDatasourceTypes + } datasourceDropdownVariant={ this.props.controlConfig?.datasourceDropdownVariant || DROPDOWN_VARIANT.CONNECT_TO_DATASOURCE @@ -79,6 +84,7 @@ class OneClickBindingControl extends BaseControl { this.props.controlConfig?.excludePrimaryColumnFromQueryGeneration } expectedType={this.props.expected?.autocompleteDataType || ""} + getQueryBindingValue={this.props.controlConfig?.getQueryBindingValue} isConnectableToWidget={this.props.controlConfig?.isConnectableToWidget} onUpdate={this.onUpdatePropertyValue} otherFields={this.props.controlConfig.otherFields} @@ -107,5 +113,7 @@ export type OneClickBindingControlProps = ControlProps & { actionButtonCtaText: string; datasourceDropdownVariant: DROPDOWN_VARIANT; alertMessage: AlertMessage; + allowedDatasourceTypes?: WidgetQueryGeneratorFormContextType["allowedDatasourceTypes"]; + getQueryBindingValue?: WidgetQueryGeneratorFormContextType["getQueryBindingValue"]; }; }; From 0b937687d5fcdd5ef29c665c39129ba0d16225ab Mon Sep 17 00:00:00 2001 From: Ayush Pahwa Date: Thu, 24 Oct 2024 16:31:00 +0800 Subject: [PATCH 3/6] chore: refactor query duplication flow (#36915) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description This PR refactors the action duplication saga and action calls to remove dependency on pageID. As far as CE code is concerned, this PR doesn't change any functionality for the end user. Those changes are done for workflows editor in EE. Fixes https://github.com/appsmithorg/appsmith/issues/36886 ## Automation /test all ### :mag: Cypress test results > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: 04dfbfbf910c789f4da4892497ce97be952bd2cd > Cypress dashboard. > Tags: `@tag.All` > Spec: >
Tue, 22 Oct 2024 16:21:32 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No ## Summary by CodeRabbit - **New Features** - Introduced a new hook, `useHandleDuplicateClick`, enhancing action duplication functionality. - Added a new interface and function for generating destination ID information. - **Bug Fixes** - Updated action request structure to use `destinationEntityId` for consistency across components. - **Documentation** - Enhanced success message flexibility for action copy notifications. - **Tests** - Added unit tests for the `ActionEntityContextMenu` component to ensure proper functionality and rendering. - **Refactor** - Improved context menu handling based on entity types for better user experience. --- .../src/PluginActionEditor/hooks/index.ts | 1 + .../hooks/useHandleDuplicateClick.ts | 26 +++ app/client/src/actions/pluginActionActions.ts | 5 +- app/client/src/ce/constants/messages.ts | 3 +- .../components/ToolbarMenu/Copy.tsx | 2 +- app/client/src/ce/sagas/helpers.ts | 17 ++ .../Actions/ActionEntityContextMenu.test.tsx | 170 ++++++++++++++++++ .../Actions/ActionEntityContextMenu.tsx | 40 +++-- .../Explorer/Actions/MoreActionsMenu.tsx | 2 +- app/client/src/sagas/ActionSagas.ts | 81 ++++++--- 10 files changed, 304 insertions(+), 43 deletions(-) create mode 100644 app/client/src/PluginActionEditor/hooks/useHandleDuplicateClick.ts create mode 100644 app/client/src/pages/Editor/Explorer/Actions/ActionEntityContextMenu.test.tsx diff --git a/app/client/src/PluginActionEditor/hooks/index.ts b/app/client/src/PluginActionEditor/hooks/index.ts index c61530a08d8c..c6a483d54fc1 100644 --- a/app/client/src/PluginActionEditor/hooks/index.ts +++ b/app/client/src/PluginActionEditor/hooks/index.ts @@ -1,5 +1,6 @@ export { useActionSettingsConfig } from "ee/PluginActionEditor/hooks/useActionSettingsConfig"; export { useHandleDeleteClick } from "ee/PluginActionEditor/hooks/useHandleDeleteClick"; +export { useHandleDuplicateClick } from "./useHandleDuplicateClick"; export { useHandleRunClick } from "ee/PluginActionEditor/hooks/useHandleRunClick"; export { useBlockExecution } from "ee/PluginActionEditor/hooks/useBlockExecution"; export { useAnalyticsOnRunClick } from "ee/PluginActionEditor/hooks/useAnalyticsOnRunClick"; diff --git a/app/client/src/PluginActionEditor/hooks/useHandleDuplicateClick.ts b/app/client/src/PluginActionEditor/hooks/useHandleDuplicateClick.ts new file mode 100644 index 000000000000..6551d7b9852b --- /dev/null +++ b/app/client/src/PluginActionEditor/hooks/useHandleDuplicateClick.ts @@ -0,0 +1,26 @@ +import { copyActionRequest } from "actions/pluginActionActions"; +import { usePluginActionContext } from "PluginActionEditor/PluginActionContext"; +import { useCallback } from "react"; +import { useDispatch } from "react-redux"; + +function useHandleDuplicateClick() { + const { action } = usePluginActionContext(); + const dispatch = useDispatch(); + + const handleDuplicateClick = useCallback( + (destinationEntityId: string) => { + dispatch( + copyActionRequest({ + id: action.id, + destinationEntityId, + name: action.name, + }), + ); + }, + [action.id, action.name, dispatch], + ); + + return { handleDuplicateClick }; +} + +export { useHandleDuplicateClick }; diff --git a/app/client/src/actions/pluginActionActions.ts b/app/client/src/actions/pluginActionActions.ts index ca6c9988d52c..461a29d51722 100644 --- a/app/client/src/actions/pluginActionActions.ts +++ b/app/client/src/actions/pluginActionActions.ts @@ -21,6 +21,7 @@ import type { ApiResponse } from "api/ApiResponses"; import type { JSCollection } from "entities/JSCollection"; import type { ErrorActionPayload } from "sagas/ErrorSagas"; import type { EventLocation } from "ee/utils/analyticsUtilTypes"; +import type { GenerateDestinationIdInfoReturnType } from "ee/sagas/helpers"; export const createActionRequest = (payload: Partial) => { return { @@ -225,7 +226,7 @@ export const moveActionError = ( export const copyActionRequest = (payload: { id: string; - destinationPageId: string; + destinationEntityId: string; name: string; }) => { return { @@ -244,7 +245,7 @@ export const copyActionSuccess = (payload: Action) => { export const copyActionError = ( payload: { id: string; - destinationPageId: string; + destinationEntityIdInfo: GenerateDestinationIdInfoReturnType; } & ErrorActionPayload, ) => { return { diff --git a/app/client/src/ce/constants/messages.ts b/app/client/src/ce/constants/messages.ts index 46e2c4aa91be..7ba25c3cb172 100644 --- a/app/client/src/ce/constants/messages.ts +++ b/app/client/src/ce/constants/messages.ts @@ -337,7 +337,7 @@ export const ACTION_MOVE_SUCCESS = (actionName: string, pageName: string) => export const ERROR_ACTION_MOVE_FAIL = (actionName: string) => `Error while moving action ${actionName}`; export const ACTION_COPY_SUCCESS = (actionName: string, pageName: string) => - `${actionName} action copied to page ${pageName} successfully`; + `${actionName} action copied ${pageName.length > 0 ? "to page " + pageName : ""} successfully`; export const ERROR_ACTION_COPY_FAIL = (actionName: string) => `Error while copying action ${actionName}`; export const ERROR_ACTION_RENAME_FAIL = (actionName: string) => @@ -1731,6 +1731,7 @@ export const CONTEXT_RENAME = () => "Rename"; export const CONTEXT_SHOW_BINDING = () => "Show bindings"; export const CONTEXT_MOVE = () => "Move to page"; export const CONTEXT_COPY = () => "Copy to page"; +export const CONTEXT_DUPLICATE = () => "Duplicate"; export const CONTEXT_DELETE = () => "Delete"; export const CONFIRM_CONTEXT_DELETE = () => "Are you sure?"; export const CONFIRM_CONTEXT_DELETING = () => "Deleting"; diff --git a/app/client/src/ce/pages/Editor/AppPluginActionEditor/components/ToolbarMenu/Copy.tsx b/app/client/src/ce/pages/Editor/AppPluginActionEditor/components/ToolbarMenu/Copy.tsx index de4037213c46..7ade07c38535 100644 --- a/app/client/src/ce/pages/Editor/AppPluginActionEditor/components/ToolbarMenu/Copy.tsx +++ b/app/client/src/ce/pages/Editor/AppPluginActionEditor/components/ToolbarMenu/Copy.tsx @@ -17,7 +17,7 @@ export const Copy = () => { dispatch( copyActionRequest({ id: action.id, - destinationPageId: pageId, + destinationEntityId: pageId, name: action.name, }), ), diff --git a/app/client/src/ce/sagas/helpers.ts b/app/client/src/ce/sagas/helpers.ts index 6a8e70d2c47f..6a57f1cf322e 100644 --- a/app/client/src/ce/sagas/helpers.ts +++ b/app/client/src/ce/sagas/helpers.ts @@ -10,6 +10,23 @@ export interface ResolveParentEntityMetadataReturnType { parentEntityKey?: CreateNewActionKeyInterface; } +// This function is extended in EE. Please check the EE implementation before any modification. +export interface GenerateDestinationIdInfoReturnType { + pageId?: string; +} + +// This function is extended in EE. Please check the EE implementation before any modification. +export function generateDestinationIdInfoForQueryDuplication( + destinationEntityId: string, + parentEntityKey: CreateNewActionKeyInterface, +): GenerateDestinationIdInfoReturnType { + if (parentEntityKey === CreateNewActionKey.PAGE) { + return { pageId: destinationEntityId }; + } + + return {}; +} + // This function is extended in EE. Please check the EE implementation before any modification. export const resolveParentEntityMetadata = ( action: Partial, diff --git a/app/client/src/pages/Editor/Explorer/Actions/ActionEntityContextMenu.test.tsx b/app/client/src/pages/Editor/Explorer/Actions/ActionEntityContextMenu.test.tsx new file mode 100644 index 000000000000..4a5258016c8e --- /dev/null +++ b/app/client/src/pages/Editor/Explorer/Actions/ActionEntityContextMenu.test.tsx @@ -0,0 +1,170 @@ +import React from "react"; +import { fireEvent, render, waitFor } from "@testing-library/react"; +import "@testing-library/jest-dom/extend-expect"; +import { Provider } from "react-redux"; +import { lightTheme } from "selectors/themeSelectors"; +import { ThemeProvider } from "styled-components"; +import configureStore from "redux-mock-store"; +import { PluginType } from "entities/Action"; +import { + ActionEntityContextMenu, + type EntityContextMenuProps, +} from "./ActionEntityContextMenu"; +import { FilesContextProvider } from "../Files/FilesContextProvider"; +import { ActionParentEntityType } from "ee/entities/Engine/actionHelpers"; +import { act } from "react-dom/test-utils"; +import { + CONTEXT_COPY, + CONTEXT_DELETE, + CONTEXT_MOVE, + CONTEXT_RENAME, + CONTEXT_SHOW_BINDING, + createMessage, +} from "ee/constants/messages"; +import { + ReduxActionTypes, + type ReduxAction, +} from "ee/constants/ReduxActionConstants"; + +const mockStore = configureStore([]); + +const page1Id = "605c435a91dea93f0eaf91ba"; +const page2Id = "605c435a91dea93f0eaf91bc"; +const basePage2Id = "605c435a91dea93f0eaf91bc"; +const defaultState = { + ui: { + selectedWorkspace: { + workspace: {}, + }, + workspaces: { + packagesList: [], + }, + users: { + featureFlag: { + data: {}, + }, + }, + }, + entities: { + actions: [], + pageList: { + pages: [ + { + pageId: page1Id, + basePageId: page2Id, + pageName: "Page1", + isDefault: true, + slug: "page-1", + }, + { + pageId: page2Id, + basePageId: basePage2Id, + pageName: "Page2", + isDefault: false, + slug: "page-2", + }, + ], + }, + }, +}; + +const defaultProps: EntityContextMenuProps = { + id: "test-action-id", + name: "test-action", + canManageAction: true, + canDeleteAction: true, + pluginType: PluginType.DB, +}; + +const defaultContext = { + editorId: "test-editor-id", + canCreateActions: true, + parentEntityId: "test-parent-entity-id", + parentEntityType: ActionParentEntityType.PAGE, +}; + +describe("ActionEntityContextMenu", () => { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let store: any; + + beforeEach(() => { + jest.clearAllMocks(); + }); + + it("renders context menu with correct options for application editor", async () => { + store = mockStore(defaultState); + const { findByText, getByRole } = render( + + + + + + + , + ); + const triggerToOpenMenu = getByRole("button"); + + act(() => { + fireEvent.click(triggerToOpenMenu); + }); + + await waitFor(() => { + expect(triggerToOpenMenu.parentNode).toHaveAttribute( + "aria-expanded", + "true", + ); + }); + + // In context of pages, the copy to page option will be shown + const copyQueryToPageOptions = await findByText( + createMessage(CONTEXT_COPY), + ); + + expect(await findByText(createMessage(CONTEXT_RENAME))).toBeInTheDocument(); + expect(await findByText(createMessage(CONTEXT_DELETE))).toBeInTheDocument(); + expect(await findByText(createMessage(CONTEXT_MOVE))).toBeInTheDocument(); + expect( + await findByText(createMessage(CONTEXT_SHOW_BINDING)), + ).toBeInTheDocument(); + expect(copyQueryToPageOptions).toBeInTheDocument(); + + // Now we click on the copy to page option + act(() => { + fireEvent.click(copyQueryToPageOptions); + }); + + // Now a menu with the list of pages will show up + const copyQueryToPageSubOptionPage1 = await findByText("Page1"); + + expect(copyQueryToPageSubOptionPage1).toBeInTheDocument(); + expect(await findByText("Page2")).toBeInTheDocument(); + + // Clicking on the page will trigger the correct action + act(() => { + fireEvent.click(copyQueryToPageSubOptionPage1); + }); + + let actions: Array< + ReduxAction<{ + payload: { + id: string; + destinationEntityId: string; + name: string; + }; + }> + > = []; + + await waitFor(() => { + actions = store.getActions(); + }); + + expect(actions.length).toBe(1); + expect(actions[0].type).toBe(ReduxActionTypes.COPY_ACTION_INIT); + expect(actions[0].payload).toEqual({ + destinationEntityId: page1Id, + id: "test-action-id", + name: "test-action", + }); + }); + // TODO: add tests for all options rendered in the context menu +}); diff --git a/app/client/src/pages/Editor/Explorer/Actions/ActionEntityContextMenu.tsx b/app/client/src/pages/Editor/Explorer/Actions/ActionEntityContextMenu.tsx index e17e37c3c3fd..7e23af50883c 100644 --- a/app/client/src/pages/Editor/Explorer/Actions/ActionEntityContextMenu.tsx +++ b/app/client/src/pages/Editor/Explorer/Actions/ActionEntityContextMenu.tsx @@ -20,6 +20,7 @@ import { CONTEXT_NO_PAGE, CONTEXT_SHOW_BINDING, createMessage, + CONTEXT_DUPLICATE, } from "ee/constants/messages"; import { builderURL } from "ee/RouteBuilder"; @@ -33,8 +34,9 @@ import { useConvertToModuleOptions } from "ee/pages/Editor/Explorer/hooks"; import { MODULE_TYPE } from "ee/constants/ModuleConstants"; import { PluginType } from "entities/Action"; import { convertToBaseParentEntityIdSelector } from "selectors/pageListSelectors"; +import { ActionParentEntityType } from "ee/entities/Engine/actionHelpers"; -interface EntityContextMenuProps { +export interface EntityContextMenuProps { id: string; name: string; className?: string; @@ -45,7 +47,7 @@ interface EntityContextMenuProps { export function ActionEntityContextMenu(props: EntityContextMenuProps) { // Import the context const context = useContext(FilesContext); - const { menuItems, parentEntityId } = context; + const { menuItems, parentEntityId, parentEntityType } = context; const baseParentEntityId = useSelector((state) => convertToBaseParentEntityIdSelector(state, parentEntityId), ); @@ -53,12 +55,12 @@ export function ActionEntityContextMenu(props: EntityContextMenuProps) { const { canDeleteAction, canManageAction } = props; const dispatch = useDispatch(); const [confirmDelete, setConfirmDelete] = useState(false); - const copyActionToPage = useCallback( - (actionId: string, actionName: string, pageId: string) => + const copyAction = useCallback( + (actionId: string, actionName: string, destinationEntityId: string) => dispatch( copyActionRequest({ id: actionId, - destinationPageId: pageId, + destinationEntityId, name: actionName, }), ), @@ -129,14 +131,26 @@ export function ActionEntityContextMenu(props: EntityContextMenuProps) { menuItems.includes(ActionEntityContextMenuItemsEnum.COPY) && canManageAction && { value: "copy", - onSelect: noop, - label: createMessage(CONTEXT_COPY), - children: menuPages.map((page) => { - return { - ...page, - onSelect: () => copyActionToPage(props.id, props.name, page.id), - }; - }), + onSelect: + parentEntityType === ActionParentEntityType.PAGE + ? noop + : () => { + copyAction(props.id, props.name, parentEntityId); + }, + label: createMessage( + parentEntityType === ActionParentEntityType.PAGE + ? CONTEXT_COPY + : CONTEXT_DUPLICATE, + ), + children: + parentEntityType === ActionParentEntityType.PAGE && + menuPages.length > 0 && + menuPages.map((page) => { + return { + ...page, + onSelect: () => copyAction(props.id, props.name, page.id), + }; + }), }, menuItems.includes(ActionEntityContextMenuItemsEnum.MOVE) && canManageAction && { diff --git a/app/client/src/pages/Editor/Explorer/Actions/MoreActionsMenu.tsx b/app/client/src/pages/Editor/Explorer/Actions/MoreActionsMenu.tsx index 4311feaf459b..b87fbe468a13 100644 --- a/app/client/src/pages/Editor/Explorer/Actions/MoreActionsMenu.tsx +++ b/app/client/src/pages/Editor/Explorer/Actions/MoreActionsMenu.tsx @@ -63,7 +63,7 @@ export function MoreActionsMenu(props: EntityContextMenuProps) { dispatch( copyActionRequest({ id: actionId, - destinationPageId: pageId, + destinationEntityId: pageId, name: actionName, }), ), diff --git a/app/client/src/sagas/ActionSagas.ts b/app/client/src/sagas/ActionSagas.ts index 2dc50983aa6b..60c375e76948 100644 --- a/app/client/src/sagas/ActionSagas.ts +++ b/app/client/src/sagas/ActionSagas.ts @@ -134,7 +134,10 @@ import { sendAnalyticsEventSaga } from "./AnalyticsSaga"; import { EditorModes } from "components/editorComponents/CodeEditor/EditorConfig"; import { updateActionAPICall } from "ee/sagas/ApiCallerSagas"; import FocusRetention from "./FocusRetentionSaga"; -import { resolveParentEntityMetadata } from "ee/sagas/helpers"; +import { + generateDestinationIdInfoForQueryDuplication, + resolveParentEntityMetadata, +} from "ee/sagas/helpers"; import { handleQueryEntityRedirect } from "./IDESaga"; import { EditorViewMode, IDE_TYPE } from "ee/entities/IDE/constants"; import { getIDETypeByUrl } from "ee/entities/IDE/utils"; @@ -144,7 +147,8 @@ import { } from "actions/ideActions"; import { getIsSideBySideEnabled } from "selectors/ideSelectors"; import { CreateNewActionKey } from "ee/entities/Engine/actionHelpers"; -import { convertToBasePageIdSelector } from "selectors/pageListSelectors"; +import { objectKeys } from "@appsmith/utils"; +import { convertToBaseParentEntityIdSelector } from "selectors/pageListSelectors"; export const DEFAULT_PREFIX = { QUERY: "Query", @@ -745,17 +749,35 @@ function* moveActionSaga( } function* copyActionSaga( - action: ReduxAction<{ id: string; destinationPageId: string; name: string }>, + action: ReduxAction<{ + id: string; + destinationEntityId: string; + name: string; + }>, ) { - let actionObject: Action = yield select(getAction, action.payload.id); + const { destinationEntityId, id, name } = action.payload; + let actionObject: Action = yield select(getAction, id); + + const { parentEntityId, parentEntityKey } = + resolveParentEntityMetadata(actionObject); + + if (!parentEntityId || !parentEntityKey) return; + const newName: string = yield select(getNewEntityName, { - prefix: action.payload.name, - parentEntityId: action.payload.destinationPageId, - parentEntityKey: CreateNewActionKey.PAGE, + prefix: name, + parentEntityId: destinationEntityId, + parentEntityKey, suffix: "Copy", startWithoutIndex: true, }); + const destinationEntityIdInfo = generateDestinationIdInfoForQueryDuplication( + destinationEntityId, + parentEntityKey, + ); + + if (objectKeys(destinationEntityIdInfo).length === 0) return; + try { if (!actionObject) throw new Error("Could not find action to copy"); @@ -768,7 +790,7 @@ function* copyActionSaga( const copyAction = Object.assign({}, actionObject, { name: newName, - pageId: action.payload.destinationPageId, + ...destinationEntityIdInfo, }) as Partial; // Indicates that source of action creation is copy action @@ -781,11 +803,15 @@ function* copyActionSaga( const datasources: Datasource[] = yield select(getDatasources); const isValidResponse: boolean = yield validateResponse(response); - const pageName: string = yield select( - getPageNameByPageId, - // @ts-expect-error: pageId not present on ActionCreateUpdateResponse - response.data.pageId, - ); + let pageName: string = ""; + + if (parentEntityKey === CreateNewActionKey.PAGE) { + pageName = yield select( + getPageNameByPageId, + // @ts-expect-error: pageId not present on ActionCreateUpdateResponse + response.data.pageId, + ); + } if (isValidResponse) { toast.show( @@ -801,12 +827,14 @@ function* copyActionSaga( const originalActionId = get( actionObject, `${RequestPayloadAnalyticsPath}.originalActionId`, - action.payload.id, + id, ); AnalyticsUtil.logEvent("DUPLICATE_ACTION", { // @ts-expect-error: name not present on ActionCreateUpdateResponse actionName: response.data.name, + parentEntityId, + parentEntityKey, pageName: pageName, actionId: response.data.id, originalActionId, @@ -836,7 +864,8 @@ function* copyActionSaga( yield put( copyActionError({ - ...action.payload, + id, + destinationEntityIdInfo, show: true, error: { message: errorMessage, @@ -1039,21 +1068,23 @@ function* toggleActionExecuteOnLoadSaga( } function* handleMoveOrCopySaga(actionPayload: ReduxAction) { - const { - baseId: baseActionId, - pageId, - pluginId, - pluginType, - } = actionPayload.payload; + const { baseId: baseActionId, pluginId, pluginType } = actionPayload.payload; const isApi = pluginType === PluginType.API; const isQuery = pluginType === PluginType.DB; const isSaas = pluginType === PluginType.SAAS; - const basePageId: string = yield select(convertToBasePageIdSelector, pageId); + const { parentEntityId } = resolveParentEntityMetadata(actionPayload.payload); + + if (!parentEntityId) return; + + const baseParentEntityId: string = yield select( + convertToBaseParentEntityIdSelector, + parentEntityId, + ); if (isApi) { history.push( apiEditorIdURL({ - basePageId, + baseParentEntityId, baseApiId: baseActionId, }), ); @@ -1062,7 +1093,7 @@ function* handleMoveOrCopySaga(actionPayload: ReduxAction) { if (isQuery) { history.push( queryEditorIdURL({ - basePageId, + baseParentEntityId, baseQueryId: baseActionId, }), ); @@ -1076,7 +1107,7 @@ function* handleMoveOrCopySaga(actionPayload: ReduxAction) { history.push( saasEditorApiIdURL({ - basePageId, + baseParentEntityId, pluginPackageName: plugin.packageName, baseApiId: baseActionId, }), From 170230490585060e4a63534fbefc6451fc2f9d56 Mon Sep 17 00:00:00 2001 From: Diljit Date: Thu, 24 Oct 2024 16:36:10 +0530 Subject: [PATCH 4/6] chore: remove an extra eval cycle caused by the FETCH_FORM_CONFIG_SUCCESS action (#37013) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description `FETCH_FORM_CONFIG_SUCCESS` is part of the EVAL trigger action list and is used to trigger evaluation in other entities such as templates, packages, and modules. This was part of the pre-consolidated API implementation, where fetching form configurations was a separate API call, and we needed to re-trigger the evaluation once the form configurations were fetched. With the consolidated API now including form configurations, there is no longer a need to re-trigger evaluation upon the dispatch of the `FETCH_FORM_CONFIG_SUCCESS` action. This change ensures that `FETCH_ALL_PAGE_ENTITY_COMPLETION` is dispatched after the `FETCH_FORM_CONFIG_SUCCESS` action. This update eliminates an unnecessary evaluation cycle during the initial page load. Fixes #`Issue Number` _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.All" ### :mag: Cypress test results > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: 23ec5017c53169568ce5035533136a5adf98b78f > Cypress dashboard. > Tags: `@tag.All` > Spec: >
Wed, 23 Oct 2024 09:05:06 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **New Features** - Improved loading process for plugins and datasources, now integrated within the page themes and actions loading. - **Bug Fixes** - Enhanced initialization sequence for the editor setup, ensuring timely server reset after setup completion. --- app/client/src/entities/Engine/AppEditorEngine.ts | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/app/client/src/entities/Engine/AppEditorEngine.ts b/app/client/src/entities/Engine/AppEditorEngine.ts index 8b63d0f333bc..d2ff14faf778 100644 --- a/app/client/src/entities/Engine/AppEditorEngine.ts +++ b/app/client/src/entities/Engine/AppEditorEngine.ts @@ -192,6 +192,13 @@ export default class AppEditorEngine extends AppEngine { yield call(waitForFetchEnvironments); endSpan(waitForFetchEnvironmentsSpan); + yield call( + this.loadPluginsAndDatasources, + allResponses, + rootSpan, + applicationId, + ); + yield put(fetchAllPageEntityCompletion([executePageLoadActions()])); endSpan(loadPageThemesAndActionsSpan); } @@ -265,12 +272,6 @@ export default class AppEditorEngine extends AppEngine { allResponses, rootSpan, ); - yield call( - this.loadPluginsAndDatasources, - allResponses, - rootSpan, - applicationId, - ); } public *completeChore(rootSpan: Span) { From 3d9d08a6c8544aeb0a6356ea59c8cd5206987d46 Mon Sep 17 00:00:00 2001 From: Ankita Kinger Date: Thu, 24 Oct 2024 17:04:03 +0530 Subject: [PATCH 5/6] chore: Adding new name editor for JS object in toolbar (#37056) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Adding new name editor for JS object in toolbar under modularised flow. Fixes [#36964](https://github.com/appsmithorg/appsmith/issues/36964) ## Automation /ok-to-test tags="@tag.Sanity, @tag.JS" ### :mag: Cypress test results > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: 7ae7ec74dbe45be237b20a07b56b1c32aa2dbba5 > Cypress dashboard. > Tags: `@tag.Sanity, @tag.JS` > Spec: >
Thu, 24 Oct 2024 10:42:17 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **New Features** - Introduced a new optional name editor in the JSEditor, allowing users to edit JavaScript object names dynamically. - Enhanced the JSEditorToolbar to conditionally render the name editor based on user permissions. - **Bug Fixes** - Simplified the props for the JSObjectNameEditor component by removing unnecessary properties. - **Documentation** - Updated export statements to ensure accessibility of the new JSObjectNameEditor component across modules. --- app/client/src/pages/Editor/JSEditor/Form.tsx | 5 +- .../JSEditorToolbar/JSEditorToolbar.tsx | 11 +- .../JSEditor/JSEditorToolbar/JSHeader.tsx | 2 +- .../JSObjectNameEditor/JSObjectNameEditor.tsx | 232 ++++++++++++++++++ .../JSObjectNameEditor/index.tsx | 4 + .../old}/JSObjectNameEditor.tsx | 7 - 6 files changed, 251 insertions(+), 10 deletions(-) create mode 100644 app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSObjectNameEditor/JSObjectNameEditor.tsx create mode 100644 app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSObjectNameEditor/index.tsx rename app/client/src/pages/Editor/JSEditor/{ => JSEditorToolbar/JSObjectNameEditor/old}/JSObjectNameEditor.tsx (90%) diff --git a/app/client/src/pages/Editor/JSEditor/Form.tsx b/app/client/src/pages/Editor/JSEditor/Form.tsx index 6eabdd909634..68028eb4ebaf 100644 --- a/app/client/src/pages/Editor/JSEditor/Form.tsx +++ b/app/client/src/pages/Editor/JSEditor/Form.tsx @@ -3,7 +3,7 @@ import React, { useCallback, useEffect, useMemo, useState } from "react"; import type { JSAction } from "entities/JSCollection"; import type { DropdownOnSelect } from "@appsmith/ads-old"; import { EditorTheme } from "components/editorComponents/CodeEditor/EditorConfig"; -import type { JSObjectNameEditorProps } from "./JSObjectNameEditor"; +import type { JSObjectNameEditorProps } from "./JSEditorToolbar/JSObjectNameEditor"; import { setActiveJSAction, setJsPaneConfigSelectedTab, @@ -68,6 +68,7 @@ interface JSFormProps { hideContextMenuOnEditor?: boolean; hideEditIconOnEditor?: boolean; notification?: React.ReactNode; + showNameEditor?: boolean; } type Props = JSFormProps; @@ -108,6 +109,7 @@ function JSEditorForm({ notification, onUpdateSettings, saveJSObjectName, + showNameEditor = false, showSettings = true, }: Props) { const theme = EditorTheme.LIGHT; @@ -353,6 +355,7 @@ function JSEditorForm({ onUpdateSettings={onUpdateSettings} saveJSObjectName={saveJSObjectName} selected={selectedJSActionOption} + showNameEditor={showNameEditor} showSettings={showSettings} /> {notification && ( diff --git a/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSEditorToolbar.tsx b/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSEditorToolbar.tsx index 06f3df58cd2f..d2c6237ebf3c 100644 --- a/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSEditorToolbar.tsx +++ b/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSEditorToolbar.tsx @@ -13,6 +13,7 @@ import { JSHeader } from "./JSHeader"; import { JSFunctionSettings } from "./components/JSFunctionSettings"; import type { JSFunctionSettingsProps } from "./components/old/JSFunctionSettings"; import { convertJSActionsToDropdownOptions } from "./utils"; +import { JSObjectNameEditor } from "./JSObjectNameEditor"; interface Props { changePermitted: boolean; @@ -33,6 +34,7 @@ interface Props { jsActions: JSAction[]; selected: JSActionDropdownOption; onUpdateSettings: JSFunctionSettingsProps["onUpdateSettings"]; + showNameEditor?: boolean; showSettings: boolean; } @@ -59,7 +61,14 @@ export const JSEditorToolbar = (props: Props) => { // Render the IDEToolbar with JSFunctionRun and JSFunctionSettings components return ( - + + {props.showNameEditor && ( + + )} +
ReduxAction; +} + +export const NameWrapper = styled(Flex)` + height: 100%; + position: relative; + font-size: 12px; + color: var(--ads-v2-colors-text-default); + cursor: pointer; + gap: var(--ads-v2-spaces-2); + align-items: center; + justify-content: center; + padding: var(--ads-v2-spaces-3); +`; + +export const IconContainer = styled.div` + height: 12px; + width: 12px; + display: flex; + align-items: center; + justify-content: center; + flex-shrink: 0; + img { + width: 12px; + } +`; + +export const Text = styled(ADSText)` + min-width: 3ch; + padding: 0 var(--ads-v2-spaces-1); + font-weight: 500; +`; + +export const JSObjectNameEditor = (props: JSObjectNameEditorProps) => { + const params = useParams<{ + baseCollectionId?: string; + baseQueryId?: string; + }>(); + + const currentJSObjectConfig = useSelector((state: AppState) => + getJsCollectionByBaseId(state, params.baseCollectionId || ""), + ); + + const currentPlugin = useSelector((state: AppState) => + getPlugin(state, currentJSObjectConfig?.pluginId || ""), + ); + + const isLoading = useSelector( + (state) => + getSavingStatusForJSObjectName(state, currentJSObjectConfig?.id || "") + .isSaving, + ); + + const title = currentJSObjectConfig?.name || ""; + const previousTitle = usePrevious(title); + const [editableTitle, setEditableTitle] = useState(title); + const [validationError, setValidationError] = useState(null); + const inputRef = useRef(null); + + const { handleNameSave, normalizeName, validateName } = useNameEditor({ + entityId: params?.baseCollectionId || "", + entityName: title, + nameSaveAction: props.saveJSObjectName, + }); + + const { + setFalse: exitEditMode, + setTrue: enterEditMode, + value: isEditing, + } = useBoolean(false); + + const currentTitle = + isEditing || isLoading || title !== editableTitle ? editableTitle : title; + + const handleKeyUp = useEventCallback( + (e: React.KeyboardEvent) => { + if (e.key === "Enter") { + const nameError = validateName(editableTitle); + + if (nameError === null) { + exitEditMode(); + handleNameSave(editableTitle); + } else { + setValidationError(nameError); + } + } else if (e.key === "Escape") { + exitEditMode(); + setEditableTitle(title); + setValidationError(null); + } else { + setValidationError(null); + } + }, + ); + + const handleTitleChange = useEventCallback( + (e: React.ChangeEvent) => { + setEditableTitle(normalizeName(e.target.value)); + }, + ); + + const handleEnterEditMode = useEventCallback(() => { + setEditableTitle(title); + enterEditMode(); + }); + + const handleDoubleClick = props.disabled ? noop : handleEnterEditMode; + + const inputProps = useMemo( + () => ({ + onKeyUp: handleKeyUp, + onChange: handleTitleChange, + autoFocus: true, + style: { + paddingTop: 0, + paddingBottom: 0, + left: -1, + top: -1, + }, + }), + [handleKeyUp, handleTitleChange], + ); + + useEventListener( + "focusout", + function handleFocusOut() { + if (isEditing) { + const nameError = validateName(editableTitle); + + exitEditMode(); + + if (nameError === null) { + handleNameSave(editableTitle); + } else { + setEditableTitle(title); + setValidationError(null); + } + } + }, + inputRef, + ); + + useEffect( + function syncEditableTitle() { + if (!isEditing && previousTitle !== title) { + setEditableTitle(title); + } + }, + [title, previousTitle, isEditing], + ); + + useEffect( + function recaptureFocusInEventOfFocusRetention() { + const input = inputRef.current; + + if (isEditing && input) { + setTimeout(() => { + input.focus(); + }, 200); + } + }, + [isEditing], + ); + + const isActionRedesignEnabled = useFeatureFlag( + FEATURE_FLAG.release_actions_redesign_enabled, + ); + + if (!isActionRedesignEnabled) { + return ( + + ); + } + + return ( + + {currentPlugin && !isLoading ? ( + + {currentPlugin.name} + + ) : null} + {isLoading && } + + + + {currentTitle} + + + + ); +}; diff --git a/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSObjectNameEditor/index.tsx b/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSObjectNameEditor/index.tsx new file mode 100644 index 000000000000..6dd2a6a8ae90 --- /dev/null +++ b/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSObjectNameEditor/index.tsx @@ -0,0 +1,4 @@ +export { + JSObjectNameEditor, + type JSObjectNameEditorProps, +} from "./JSObjectNameEditor"; diff --git a/app/client/src/pages/Editor/JSEditor/JSObjectNameEditor.tsx b/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSObjectNameEditor/old/JSObjectNameEditor.tsx similarity index 90% rename from app/client/src/pages/Editor/JSEditor/JSObjectNameEditor.tsx rename to app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSObjectNameEditor/old/JSObjectNameEditor.tsx index 0ce1112c3c1f..53e1e920850a 100644 --- a/app/client/src/pages/Editor/JSEditor/JSObjectNameEditor.tsx +++ b/app/client/src/pages/Editor/JSEditor/JSEditorToolbar/JSObjectNameEditor/old/JSObjectNameEditor.tsx @@ -28,13 +28,6 @@ import type { ReduxAction } from "ee/constants/ReduxActionConstants"; import type { SaveActionNameParams } from "PluginActionEditor"; export interface JSObjectNameEditorProps { - /* - This prop checks if page is API Pane or Query Pane or Curl Pane - So, that we can toggle between ads editable-text component and existing editable-text component - Right now, it's optional so that it doesn't impact any other pages other than API Pane. - In future, when default component will be ads editable-text, then we can remove this prop. - */ - page?: string; disabled?: boolean; saveJSObjectName: ( params: SaveActionNameParams, From abc064bef306a6a3602ca45312695e07178d29b9 Mon Sep 17 00:00:00 2001 From: Sagar Khalasi Date: Fri, 25 Oct 2024 10:45:07 +0530 Subject: [PATCH 6/6] chore: Update dispatch option (#37066) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description We’ve often faced challenges in running server-specific tests efficiently for individual PRs. To streamline this process and improve testing speed, I’ve implemented the workflow dispatch feature, enabling quick and on-demand execution of server tests. Fixes #`37060` ## Automation /ok-to-test tags="@tag.IDE" ### :mag: Cypress test results > [!WARNING] > Workflow run: > Commit: 48c3cd9618bc4a4dc61d0ecdb612cc1798f07442 > Cypress dashboard. > Tags: @tag.IDE > Spec: > It seems like **no tests ran** 😔. We are not able to recognize it, please check workflow here. >
Fri, 25 Oct 2024 05:13:19 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No --- .github/workflows/server-build.yml | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/.github/workflows/server-build.yml b/.github/workflows/server-build.yml index 56f7086f075d..68c439738348 100644 --- a/.github/workflows/server-build.yml +++ b/.github/workflows/server-build.yml @@ -23,6 +23,27 @@ on: type: string default: "false" + workflow_dispatch: + inputs: + pr: + description: "PR number for the workflow" + required: false + type: number + skip-tests: + description: "Skip tests flag" + required: false + type: string + default: "false" + branch: + description: "Branch for the build" + required: false + type: string + is-pg-build: + description: "Flag for PG build" + required: false + type: string + default: "false" + # Change the working directory for all the jobs in this workflow defaults: run: