From 423b7eb176b87643649397e3e42f5b30e1e9ea60 Mon Sep 17 00:00:00 2001 From: Trisha Anand Date: Tue, 15 Sep 2020 15:22:35 +0530 Subject: [PATCH 1/2] Added `confirmBeforeExecute` field in action view dto which is returned only during view mode. (#547) * Added `confirmBeforeExecute` field in action view dto which is returned only during view mode. * Added test for fetch actions in view mode. --- .../appsmith/server/dtos/ActionViewDTO.java | 1 + .../server/services/ActionServiceImpl.java | 1 + .../server/services/ActionServiceTest.java | 31 +++++++++++++++++++ 3 files changed, 33 insertions(+) diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/ActionViewDTO.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/ActionViewDTO.java index d0fad649a7..143ea29aa8 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/ActionViewDTO.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/dtos/ActionViewDTO.java @@ -16,6 +16,7 @@ public class ActionViewDTO { String name; String pageId; Integer timeoutInMillisecond; + Boolean confirmBeforeExecute; Set jsonPathKeys; // Overriding the getter to ensure that for actions missing action configuration, the timeout is diff --git a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java index cac5bc61f1..b0133d389b 100644 --- a/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java +++ b/app/server/appsmith-server/src/main/java/com/appsmith/server/services/ActionServiceImpl.java @@ -571,6 +571,7 @@ public class ActionServiceImpl extends BaseService jsonPathKeys; jsonPathKeys = new HashSet<>(); diff --git a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionServiceTest.java b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionServiceTest.java index 3959bc853d..40494b44b0 100644 --- a/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionServiceTest.java +++ b/app/server/appsmith-server/src/test/java/com/appsmith/server/services/ActionServiceTest.java @@ -582,4 +582,35 @@ public class ActionServiceTest { Mono actionExecutionResultMono = actionService.executeAction(executeActionDTO); return actionExecutionResultMono; } + + @Test + @WithUserDetails(value = "api_user") + public void getActionInViewMode() { + Mockito.when(pluginExecutorHelper.getPluginExecutor(Mockito.any())).thenReturn(Mono.just(new MockPluginExecutor())); + + Action action = new Action(); + action.setName("view-mode-action-test"); + action.setPageId(testPage.getId()); + ActionConfiguration actionConfiguration = new ActionConfiguration(); + actionConfiguration.setHttpMethod(HttpMethod.GET); + actionConfiguration.setPath("{{mustache}}"); + action.setActionConfiguration(actionConfiguration); + action.setDatasource(datasource); + + Mono createActionMono = actionService.create(action); + Mono> actionViewModeListMono = createActionMono + .then(actionService.getActionsForViewMode(testApp.getId()).collectList()); + + StepVerifier.create(actionViewModeListMono) + .assertNext(actions -> { + assertThat(actions.size()).isGreaterThan(0); + ActionViewDTO actionViewDTO = actions.get(0); + assertThat(actionViewDTO.getId()).isNotNull(); + assertThat(actionViewDTO.getTimeoutInMillisecond()).isNotNull(); + assertThat(actionViewDTO.getPageId()).isNotNull(); + assertThat(actionViewDTO.getConfirmBeforeExecute()).isNotNull(); + assertThat(actionViewDTO.getJsonPathKeys().size()).isEqualTo(1); + }) + .verifyComplete(); + } } From 499570667974b07379f1504b8fbb7bdd4d573149 Mon Sep 17 00:00:00 2001 From: Nikhil Nandagopal Date: Tue, 15 Sep 2020 22:24:15 +0530 Subject: [PATCH 2/2] Fix chart data evaluation (#548) * changed chart data to behave as an array and not use json stringify to update added a generic logic to flag all objects with internal bindings as dynamic moved unescape to only handle at the time of eval and removed new lines before eval * added a migration for older charts that have their data as strings * flagged column actions as an action trigger to avoid evaluating it Co-authored-by: Nikhil Nandagopal --- app/client/cypress/fixtures/ChartTextDsl.json | 2 +- .../components/formControls/SwitchControl.tsx | 2 +- .../propertyControls/ChartDataControl.tsx | 85 +++++++++++-------- .../src/entities/DataTree/dataTreeFactory.ts | 9 +- app/client/src/sagas/WidgetOperationSagas.tsx | 9 +- app/client/src/utils/DynamicBindingUtils.ts | 25 ++---- app/client/src/widgets/TableWidget.tsx | 1 + 7 files changed, 75 insertions(+), 58 deletions(-) diff --git a/app/client/cypress/fixtures/ChartTextDsl.json b/app/client/cypress/fixtures/ChartTextDsl.json index 043995369b..5d9202bb30 100644 --- a/app/client/cypress/fixtures/ChartTextDsl.json +++ b/app/client/cypress/fixtures/ChartTextDsl.json @@ -58,7 +58,7 @@ "chartType": "LINE_CHART", "chartName": "Sales on working days", "allowHorizontalScroll": false, - "chartData": "[{\"seriesName\":\"\",\"data\":\"\"}]", + "chartData": [{"seriesName":"Sales","data":""}], "xAxisName": "Last Week", "yAxisName": "Total Order Revenue $", "type": "CHART_WIDGET", diff --git a/app/client/src/components/formControls/SwitchControl.tsx b/app/client/src/components/formControls/SwitchControl.tsx index 0946f856e2..10a1b3658d 100644 --- a/app/client/src/components/formControls/SwitchControl.tsx +++ b/app/client/src/components/formControls/SwitchControl.tsx @@ -61,7 +61,7 @@ class SwitchControl extends BaseControl { } getControlType(): ControlType { - return "FILE_PICKER"; + return "SWITCH"; } } diff --git a/app/client/src/components/propertyControls/ChartDataControl.tsx b/app/client/src/components/propertyControls/ChartDataControl.tsx index 2c4fff78a9..d6a53ac732 100644 --- a/app/client/src/components/propertyControls/ChartDataControl.tsx +++ b/app/client/src/components/propertyControls/ChartDataControl.tsx @@ -12,6 +12,7 @@ import { EditorTheme, TabBehaviour, } from "components/editorComponents/CodeEditor/EditorConfig"; +import * as Sentry from "@sentry/react"; const StyledOptionControlWrapper = styled(ControlWrapper)` display: flex; @@ -138,6 +139,11 @@ function DataControlComponent(props: RenderComponentProps) { } class ChartDataControl extends BaseControl { + chartData: Array<{ + seriesName: string; + data: string; + }> = []; + getValidations = (message: string, isValid: boolean, len: number) => { const validations: Array<{ isValid: boolean; @@ -166,16 +172,36 @@ class ChartDataControl extends BaseControl { return validations; }; - render() { - const chartData: Array<{ - seriesName: string; - data: Array<{ x: string; y: string }> | string; - }> = - this.props.propertyValue && _.isString(this.props.propertyValue) - ? JSON.parse(this.props.propertyValue) - : this.props.propertyValue; + componentDidMount() { + const chartData = this.props.propertyValue; + // Added a migration script for older chart data that was strings + // deprecate after enough charts have moved to the new format + if (_.isString(chartData)) { + try { + const parsedData: Array<{ + seriesName: string; + data: string; + }> = JSON.parse(chartData); + this.updateProperty("chartData", parsedData); + this.chartData = parsedData; + } catch (error) { + Sentry.captureException({ + message: "Chart Migration Failed", + oldData: this.props.propertyValue, + }); + } + } else { + this.chartData = this.props.propertyValue; + } + } - const dataLength = chartData.length; + componentDidUpdate() { + this.chartData = this.props.propertyValue; + } + + render() { + const chartData = this.chartData; + const dataLength = this.chartData.length; const { validationMessage, isValid } = this.props; const validations: Array<{ isValid: boolean; @@ -187,7 +213,7 @@ class ChartDataControl extends BaseControl { ); return ( - {chartData.map((data, index) => { + {this.chartData.map((data, index) => { return ( { } deleteOption = (index: number) => { - const chartData: object[] = - this.props.propertyValue && _.isString(this.props.propertyValue) - ? JSON.parse(this.props.propertyValue) - : this.props.propertyValue; + const chartData: Array<{ + seriesName: string; + data: string; + }> = this.props.propertyValue; chartData.splice(index, 1); - this.updateProperty(this.props.propertyName, JSON.stringify(chartData)); + this.updateProperty(this.props.propertyName, chartData); }; updateOption = ( @@ -229,41 +255,28 @@ class ChartDataControl extends BaseControl { ) => { const chartData: Array<{ seriesName: string; - data: Array<{ x: string; y: string }> | any; - }> = - this.props.propertyValue && _.isString(this.props.propertyValue) - ? JSON.parse(this.props.propertyValue) - : this.props.propertyValue; + data: string; + }> = this.props.propertyValue; const updatedChartData = chartData.map((item, i) => { if (index === i) { if (propertyName === "seriesName") { item.seriesName = updatedValue; } else { - try { - item.data = JSON.parse(updatedValue); - } catch (err) { - item.data = updatedValue; - } + item.data = updatedValue; } } return item; }); - this.updateProperty( - this.props.propertyName, - JSON.stringify(updatedChartData), - ); + this.updateProperty(this.props.propertyName, updatedChartData); }; addOption = () => { const chartData: Array<{ seriesName: string; - data: Array<{ x: string; y: string }> | any; - }> = - this.props.propertyValue && _.isString(this.props.propertyValue) - ? JSON.parse(this.props.propertyValue) - : this.props.propertyValue; - chartData.push({ seriesName: "", data: [{ x: "", y: "" }] }); - this.updateProperty(this.props.propertyName, JSON.stringify(chartData)); + data: string; + }> = this.props.propertyValue; + chartData.push({ seriesName: "", data: '[{ x: "", y: "" }]' }); + this.updateProperty(this.props.propertyName, chartData); }; static getControlType() { diff --git a/app/client/src/entities/DataTree/dataTreeFactory.ts b/app/client/src/entities/DataTree/dataTreeFactory.ts index 4e8afc6774..f629b8cfb9 100644 --- a/app/client/src/entities/DataTree/dataTreeFactory.ts +++ b/app/client/src/entities/DataTree/dataTreeFactory.ts @@ -10,6 +10,7 @@ import { PageListPayload } from "constants/ReduxActionConstants"; import WidgetFactory from "utils/WidgetFactory"; import { ActionConfig, PluginType, Property } from "entities/Action"; import { AppDataState } from "reducers/entityReducers/appReducer"; +import _ from "lodash"; export type ActionDescription = { type: string; @@ -129,7 +130,7 @@ export class DataTreeFactory { dataTree.actionPaths && dataTree.actionPaths.push(); }); Object.keys(widgets).forEach(w => { - const widget = widgets[w]; + const widget = { ...widgets[w] }; const widgetMetaProps = widgetsMeta[w]; const defaultMetaProps = WidgetFactory.getWidgetMetaPropertiesMap( widget.type, @@ -139,6 +140,12 @@ export class DataTreeFactory { ); const derivedProps: any = {}; const dynamicBindings = widget.dynamicBindings || {}; + Object.keys(dynamicBindings).forEach(propertyName => { + if (_.isObject(widget[propertyName])) { + // Stringify this because composite controls may have bindings in the sub controls + widget[propertyName] = JSON.stringify(widget[propertyName]); + } + }); Object.keys(derivedPropertyMap).forEach(propertyName => { derivedProps[propertyName] = derivedPropertyMap[propertyName].replace( /this./g, diff --git a/app/client/src/sagas/WidgetOperationSagas.tsx b/app/client/src/sagas/WidgetOperationSagas.tsx index 060dec6182..b3d3841125 100644 --- a/app/client/src/sagas/WidgetOperationSagas.tsx +++ b/app/client/src/sagas/WidgetOperationSagas.tsx @@ -298,9 +298,14 @@ function* updateDynamicTriggers( function* updateDynamicBindings( widget: WidgetProps, propertyName: string, - propertyValue: string, + propertyValue: any, ) { - const isDynamic = isDynamicValue(propertyValue); + let stringProp = propertyValue; + if (_.isObject(propertyValue)) { + // Stringify this because composite controls may have bindings in the sub controls + stringProp = JSON.stringify(propertyValue); + } + const isDynamic = isDynamicValue(stringProp); let dynamicBindings: Record = widget.dynamicBindings || {}; if (!isDynamic && propertyName in dynamicBindings) { dynamicBindings = _.omit(dynamicBindings, propertyName); diff --git a/app/client/src/utils/DynamicBindingUtils.ts b/app/client/src/utils/DynamicBindingUtils.ts index 9285b5bef6..b7084ecf44 100644 --- a/app/client/src/utils/DynamicBindingUtils.ts +++ b/app/client/src/utils/DynamicBindingUtils.ts @@ -123,7 +123,12 @@ export const evaluateDynamicBoundValue = ( path: string, callbackData?: any, ): JSExecutorResult => { - return JSExecutionManagerSingleton.evaluateSync(path, data, callbackData); + const unescapedJS = unescapeJS(path).replace(/(\r\n|\n|\r)/gm, ""); + return JSExecutionManagerSingleton.evaluateSync( + unescapedJS, + data, + callbackData, + ); }; // For creating a final value where bindings could be in a template format @@ -332,14 +337,7 @@ export const createDependencyTree = ( if (entity.dynamicBindings) { Object.keys(entity.dynamicBindings).forEach(propertyName => { // using unescape to remove new lines from bindings which interfere with our regex extraction - let unevalPropValue = _.get(entity, propertyName); - if ( - _.isString(unevalPropValue) && - isDynamicValue(unevalPropValue) - ) { - unevalPropValue = unescapeJS(unevalPropValue); - } - _.set(entity, propertyName, unevalPropValue); + const unevalPropValue = _.get(entity, propertyName); const { jsSnippets } = getDynamicBindings(unevalPropValue); const existingDeps = dependencyMap[`${entityKey}.${propertyName}`] || []; @@ -358,14 +356,7 @@ export const createDependencyTree = ( if (entity.dynamicBindingPathList.length) { entity.dynamicBindingPathList.forEach(prop => { // using unescape to remove new lines from bindings which interfere with our regex extraction - let unevalPropValue = _.get(entity, prop.key); - if ( - _.isString(unevalPropValue) && - isDynamicValue(unevalPropValue) - ) { - unevalPropValue = unescapeJS(unevalPropValue); - } - _.set(entity, prop.key, unevalPropValue); + const unevalPropValue = _.get(entity, prop.key); const { jsSnippets } = getDynamicBindings(unevalPropValue); const existingDeps = dependencyMap[`${entityKey}.${prop.key}`] || []; diff --git a/app/client/src/widgets/TableWidget.tsx b/app/client/src/widgets/TableWidget.tsx index 5a6e9a3613..28a03ae5cd 100644 --- a/app/client/src/widgets/TableWidget.tsx +++ b/app/client/src/widgets/TableWidget.tsx @@ -117,6 +117,7 @@ class TableWidget extends BaseWidget { onRowSelected: true, onPageChange: true, onSearchTextChanged: true, + columnActions: true, }; }