Skip to content

Commit

Permalink
fix: Enhance error handling in ComputeTablePropertyControlV2 binding …
Browse files Browse the repository at this point in the history
…methods (#38205)
  • Loading branch information
rahulbarwal authored Jan 27, 2025
1 parent da449ee commit 08cd433
Show file tree
Hide file tree
Showing 9 changed files with 163 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ const commonlocators = require("../../../../../locators/commonlocators.json");
import {
agHelper,
apiPage,
entityExplorer,
propPane,
} from "../../../../../support/Objects/ObjectsCore";

Expand Down Expand Up @@ -116,6 +115,38 @@ describe(
// verify customColumn is visible in the table
cy.get(".draggable-header:contains('CustomColumn')").should("be.visible");
cy.closePropertyPane();
cy.deleteColumn("customColumn1");
});

it("5. Verify computed value with try-catch blocks handles missing nested properties", function () {
cy.openPropertyPane("tablewidgetv2");

// Set table data with nested object and missing property
propPane.UpdatePropertyFieldValue(
"Table data",
`{{[{"name": "Rahul", age: {value: 31}}, {"name": "Jacq", age: {}}, {"name": "John"}]}}`,
);

cy.editColumn("age");
propPane.UpdatePropertyFieldValue(
"Computed value",
"{{currentRow.age.value}}",
);

cy.readTableV2data(0, 1).then((val) => {
expect(val).to.equal("31");
});

cy.readTableV2data(1, 1).then((val) => {
expect(val).to.equal("");
});

cy.readTableV2data(2, 1).then((val) => {
expect(val).to.equal("");
});

// Clean up
cy.backFromPropertyPanel();
});
},
);
8 changes: 7 additions & 1 deletion app/client/packages/dsl/src/migrate/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,10 @@ import { migrateTableServerSideFiltering } from "./migrations/086-migrate-table-
import { migrateChartwidgetCustomEchartConfig } from "./migrations/087-migrate-chart-widget-customechartdata";
import { migrateCustomWidgetDynamicHeight } from "./migrations/088-migrate-custom-widget-dynamic-height";
import { migrateTableWidgetV2CurrentRowInValidationsBinding } from "./migrations/089-migrage-table-widget-v2-currentRow-binding";
import { migrateTableWidgetV2ValidationTryCatch } from "./migrations/090-migrate-table-widget-v2-validation-try-catch";
import type { DSLWidget } from "./types";

export const LATEST_DSL_VERSION = 91;
export const LATEST_DSL_VERSION = 92;

export const calculateDynamicHeight = () => {
const DEFAULT_GRID_ROW_HEIGHT = 10;
Expand Down Expand Up @@ -624,6 +625,11 @@ const migrateVersionedDSL = async (currentDSL: DSLWidget, newPage = false) => {
* What we missed was that, the auto-commit does not handle clientVersion, which lead to this bug: https://github.com/appsmithorg/appsmith/issues/38511
* We are bumping this version to make sure that the auto-commit will handle this version bump.
*/
currentDSL.version = 91;
}

if (currentDSL.version === 91) {
currentDSL = migrateTableWidgetV2ValidationTryCatch(currentDSL);
currentDSL.version = LATEST_DSL_VERSION;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import type { ColumnProperties, DSLWidget, WidgetProps } from "../types";
import { isDynamicValue, traverseDSLAndMigrate } from "../utils";

const oldBindingPrefix = (tableName: string) => {
return `{{${tableName}.processedTableData.map((currentRow, currentIndex) => (`;
};

const newBindingPrefix = (tableName: string) => {
return `{{${tableName}.processedTableData.map((currentRow, currentIndex) => { try { return (`;
};

const oldBindingSuffix = `))}}`;
const newBindingSuffix = `); } catch (e) { return null; }})}}`;

export const migrateTableWidgetV2ValidationTryCatch = (
currentDSL: DSLWidget,
) => {
return traverseDSLAndMigrate(currentDSL, (widget: WidgetProps) => {
if (widget.type !== "TABLE_WIDGET_V2") return;

const primaryColumns: Record<string, ColumnProperties> =
widget.primaryColumns as Record<string, ColumnProperties>;

Object.values(primaryColumns).forEach((colProperties) => {
if (!colProperties.computedValue) return;

const value = colProperties.computedValue;

if (!isDynamicValue(value)) return;

const tableName = widget.widgetName;
const oldPrefix = oldBindingPrefix(tableName);

// Only update if it matches the old format
if (!value.startsWith(oldPrefix)) return;

// Replace old prefix/suffix with new ones
const computation = value
.replace(oldPrefix, "")
.replace(oldBindingSuffix, "");

// Add the new prefix and suffix with try-catch
colProperties.computedValue = `${newBindingPrefix(tableName)}${computation}${newBindingSuffix}`;
});
});
};
10 changes: 10 additions & 0 deletions app/client/packages/dsl/src/migrate/tests/DSLMigration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ import * as m86 from "../migrations/086-migrate-table-server-side-filtering";
import * as m87 from "../migrations/087-migrate-chart-widget-customechartdata";
import * as m88 from "../migrations/088-migrate-custom-widget-dynamic-height";
import * as m89 from "../migrations/089-migrage-table-widget-v2-currentRow-binding";
import * as m91 from "../migrations/090-migrate-table-widget-v2-validation-try-catch";

interface Migration {
functionLookup: {
Expand Down Expand Up @@ -934,6 +935,15 @@ const migrations: Migration[] = [
functionLookup: [],
version: 90,
},
{
functionLookup: [
{
moduleObj: m91,
functionName: "migrateTableWidgetV2ValidationTryCatch",
},
],
version: 91,
},
];

const mockFnObj: Record<number, any> = {};
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import type { DSLWidget } from "../../../types";

const oldBindingPrefix = (tableName: string) => {
return `{{${tableName}.processedTableData.map((currentRow, currentIndex) => (`;
};

const newBindingPrefix = (tableName: string) => {
return `{{${tableName}.processedTableData.map((currentRow, currentIndex) => { try { return (`;
};

const oldBindingSuffix = `))}}`;
const newBindingSuffix = `); } catch (e) { return null; }})}}`;

const computation = "currentRow.id + '_' + currentIndex";

export const validationTryCatchInput = {
children: [
{
widgetName: "Table1",
type: "TABLE_WIDGET_V2",
primaryColumns: {
customColumn1: {
computedValue: `${oldBindingPrefix("Table1")}${computation}${oldBindingSuffix}`,
},
customColumn2: {
computedValue: "static value", // Should not be modified
},
},
},
],
} as any as DSLWidget;

export const validationTryCatchOutput = {
children: [
{
widgetName: "Table1",
type: "TABLE_WIDGET_V2",
primaryColumns: {
customColumn1: {
computedValue: `${newBindingPrefix("Table1")}${computation}${newBindingSuffix}`,
},
customColumn2: {
computedValue: "static value", // Not modified
},
},
},
],
} as any as DSLWidget;
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { migrateTableWidgetV2ValidationTryCatch } from "../../migrations/090-migrate-table-widget-v2-validation-try-catch";
import {
validationTryCatchInput,
validationTryCatchOutput,
} from "./DSLs/ValidationTryCatchDSLs";

describe("migrateTableWidgetV2ValidationTryCatch", () => {
it("should add try-catch blocks to table compute value bindings", () => {
const migratedDSL = migrateTableWidgetV2ValidationTryCatch(
validationTryCatchInput,
);

expect(migratedDSL).toEqual(validationTryCatchOutput);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,10 @@ function InputText(props: InputTextProp) {

class ComputeTablePropertyControlV2 extends BaseControl<ComputeTablePropertyControlPropsV2> {
static getBindingPrefix(tableName: string) {
return `{{${tableName}.processedTableData.map((currentRow, currentIndex) => ( `;
return `{{${tableName}.processedTableData.map((currentRow, currentIndex) => { try { return ( `;
}

static bindingSuffix = `))}}`;
static bindingSuffix = `); } catch (e) { return null; }})}}`;

render() {
const {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@ export const transformDataPureFn = (
): tableData => {
if (isArray(tableData)) {
return tableData.map((row, rowIndex) => {
// TODO: Fix this the next time the file is edited
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const newRow: { [key: string]: any } = {};
const newRow: { [key: string]: unknown } = {};

columns.forEach((column) => {
const { alias } = column;
Expand Down
4 changes: 2 additions & 2 deletions app/client/src/widgets/TableWidgetV2/widget/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,9 +210,9 @@ export function getDefaultColumnProperties(
isDiscardVisible: true,
computedValue: isDerived
? ""
: `{{${widgetName}.processedTableData.map((currentRow, currentIndex) => ( currentRow["${escapeString(
: `{{${widgetName}.processedTableData.map((currentRow, currentIndex) => { try { return ( currentRow["${escapeString(
id,
)}"]))}}`,
)}"]); } catch (e) { return null; }})}}`,
sticky: StickyType.NONE,
validation: {},
currencyCode: "USD",
Expand Down

0 comments on commit 08cd433

Please sign in to comment.