From 0c96ea330f235425143c61817da42ae9c961d019 Mon Sep 17 00:00:00 2001 From: Rahul Barwal Date: Mon, 2 Jun 2025 14:56:28 +0530 Subject: [PATCH] fix: Incorrect display of values in table computed value (#40664) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Problem When editing the Computed Value field of a column in the Table widget, a closing round bracket `)` is automatically removed from specific expressions upon losing focus, leading to broken expressions and incorrect widget behavior. Root cause Incorrect handling of double curly braces and nested parentheses in the `computedValue` parsing logic. Solution This PR handles the correction of expression parsing in `ComputeTablePropertyControlV2` by updating the extraction logic to support nested parentheses correctly. It also includes a test to ensure this functionality works as expected. Fixes #40265 _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.Table" ### :mag: Cypress test results > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: 156df279926fc8e39f194d46e868d46c62a1b2bf > Cypress dashboard. > Tags: `@tag.Table` > Spec: >
Fri, 16 May 2025 06:11:17 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **Bug Fixes** - Improved handling of malformed expressions in table computed value inputs, resulting in more consistent error output. - Enhanced default computed value behavior to provide a fallback when table data is empty, preventing errors. - **Tests** - Updated and expanded test cases for computed value extraction, including new input variations and revised expectations for malformed expressions. - **Chores** - Added a migration step to update older computed values for correct display in widgets. - Incremented DSL version to include latest migration updates. --- app/client/packages/dsl/src/migrate/index.ts | 14 +++- .../src/migrate/tests/DSLMigration.test.ts | 9 +++ .../TableComputeValue.test.tsx | 5 +- .../propertyControls/TableComputeValue.tsx | 69 ++----------------- .../widgets/TableWidgetV2/widget/utilities.ts | 4 +- 5 files changed, 31 insertions(+), 70 deletions(-) diff --git a/app/client/packages/dsl/src/migrate/index.ts b/app/client/packages/dsl/src/migrate/index.ts index 3a11b3b9d6..f78a58eab2 100644 --- a/app/client/packages/dsl/src/migrate/index.ts +++ b/app/client/packages/dsl/src/migrate/index.ts @@ -95,7 +95,7 @@ import { migrateTableComputeValueBinding } from "./migrations/090-migrate-table- import { migrateAIChatWidget } from "./migrations/092-update-ai-chat-widget"; import type { DSLWidget } from "./types"; -export const LATEST_DSL_VERSION = 93; +export const LATEST_DSL_VERSION = 94; export const calculateDynamicHeight = () => { const DEFAULT_GRID_ROW_HEIGHT = 10; @@ -636,6 +636,18 @@ const migrateVersionedDSL = async (currentDSL: DSLWidget, newPage = false) => { if (currentDSL.version === 92) { currentDSL = migrateAIChatWidget(currentDSL); + currentDSL.version = 93; + } + + if (currentDSL.version === 93) { + /** + * We are repeating migration(90->91) here. Why? + * Although we updated the computed value logic in the property pane control files, we did not update the default values that the widget assumes upon instantiation. + * While this would not have caused any functional bugs, it results in the DSL saving an older version of the computed value, which the property control does not process or filter correctly for display. + * Consequently, anyone who dropped a table between version 90 and today will see unnecessary computations in their computed value field that should not be visible. + * This migration will update all those values to the latest computed value, ensuring they are displayed correctly by the control. + */ + currentDSL = migrateTableComputeValueBinding(currentDSL); currentDSL.version = LATEST_DSL_VERSION; } diff --git a/app/client/packages/dsl/src/migrate/tests/DSLMigration.test.ts b/app/client/packages/dsl/src/migrate/tests/DSLMigration.test.ts index a8e1a1b0b2..2e4d54a112 100644 --- a/app/client/packages/dsl/src/migrate/tests/DSLMigration.test.ts +++ b/app/client/packages/dsl/src/migrate/tests/DSLMigration.test.ts @@ -955,6 +955,15 @@ const migrations: Migration[] = [ ], version: 92, }, + { + functionLookup: [ + { + moduleObj: m90, + functionName: "migrateTableComputeValueBinding", + }, + ], + version: 93, + }, ]; const mockFnObj: Record = {}; diff --git a/app/client/src/components/propertyControls/TableComputeValue.test.tsx b/app/client/src/components/propertyControls/TableComputeValue.test.tsx index 9f51bda99d..c433600817 100644 --- a/app/client/src/components/propertyControls/TableComputeValue.test.tsx +++ b/app/client/src/components/propertyControls/TableComputeValue.test.tsx @@ -4,7 +4,6 @@ describe("ComputeTablePropertyControlV2.getInputComputedValue", () => { const tableName = "Table1"; const inputVariations = [ "currentRow.price", - `JSObject1.somefunction(currentRow["id"] || 0)`, ` [ { @@ -35,6 +34,8 @@ describe("ComputeTablePropertyControlV2.getInputComputedValue", () => { "currentRow.price * 2", "currentRow.isValid && true", "!currentRow.isDeleted", + "JSObject1.myFun1(currentRow['id'])", + `JSObject1.somefunction(currentRow["id"] || 0)`, ]; it("1. should return the correct computed value", () => { @@ -85,7 +86,7 @@ describe("ComputeTablePropertyControlV2.getInputComputedValue", () => { ComputeTablePropertyControlV2.getInputComputedValue( malformedComputedValue, ), - ).toBe(malformedComputedValue); + ).toBe("{{currentRow.function(unbalanced}}"); }); it("5. should correctly parse complex but valid expressions with multiple nested parentheses", () => { diff --git a/app/client/src/components/propertyControls/TableComputeValue.tsx b/app/client/src/components/propertyControls/TableComputeValue.tsx index 90c9957ef1..0d9cb5de34 100644 --- a/app/client/src/components/propertyControls/TableComputeValue.tsx +++ b/app/client/src/components/propertyControls/TableComputeValue.tsx @@ -161,86 +161,25 @@ class ComputeTablePropertyControlV2 extends BaseControl { const tableData = Table1.processedTableData || []; return tableData.length > 0 ? tableData.map((currentRow, currentIndex) => (currentRow.price * 2)) : currentRow.price * 2 })()}}" const mapSignatureIndex = propertyValue.indexOf(MAP_FUNCTION_SIGNATURE); // Find the actual computation expression between the map parentheses const computationStart = mapSignatureIndex + MAP_FUNCTION_SIGNATURE.length; - - const { endIndex, isValid } = this.findMatchingClosingParenthesis( - propertyValue, - computationStart, - ); - - // Handle case where no matching closing parenthesis is found - if (!isValid) { - // If we can't find the proper closing parenthesis, fall back to returning the original value - // This prevents errors when the expression is malformed - return propertyValue; - } + const computationEnd = propertyValue.lastIndexOf(")) : "); // Extract the computation expression between the map parentheses + // Note: At this point, we're just extracting the raw expression like "currentRow.price * 2" + // The actual removal of "currentRow." prefix happens later in JSToString() const computationExpression = propertyValue.substring( computationStart, - endIndex, + computationEnd, ); return JSToString(computationExpression); }; - /** - * Check if the computed value string looks structurally valid - * This helps catch obviously malformed expressions early - */ - private static isLikelyValidComputedValue(value: string): boolean { - // Check for basic structural elements that should be present - const hasOpeningStructure = value.includes("(() => {"); - const hasTableDataAssignment = value.includes("const tableData ="); - const hasReturnStatement = value.includes("return tableData.length > 0 ?"); - const hasClosingStructure = value.includes("})()}}"); - - return ( - hasOpeningStructure && - hasTableDataAssignment && - hasReturnStatement && - hasClosingStructure - ); - } - - /** - * Utility function to find the matching closing parenthesis - * @param text - The text to search in - * @param startIndex - The index after the opening parenthesis - * @returns Object containing the index of the matching closing parenthesis and whether it was found - */ - private static findMatchingClosingParenthesis( - text: string, - startIndex: number, - ) { - let openParenCount = 1; // Start with 1 for the opening parenthesis - - for (let i = startIndex; i < text.length; i++) { - if (text[i] === "(") { - openParenCount++; - } else if (text[i] === ")") { - openParenCount--; - - if (openParenCount === 0) { - return { endIndex: i, isValid: true }; - } - } - } - - // No matching closing parenthesis found - return { endIndex: text.length, isValid: false }; - } - getComputedValue = (value: string, tableName: string) => { // Return raw value if: // 1. The value is not a dynamic binding (not wrapped in {{...}}) diff --git a/app/client/src/widgets/TableWidgetV2/widget/utilities.ts b/app/client/src/widgets/TableWidgetV2/widget/utilities.ts index 502c852777..c1c17d8160 100644 --- a/app/client/src/widgets/TableWidgetV2/widget/utilities.ts +++ b/app/client/src/widgets/TableWidgetV2/widget/utilities.ts @@ -210,9 +210,9 @@ export function getDefaultColumnProperties( isDiscardVisible: true, computedValue: isDerived ? "" - : `{{${widgetName}.processedTableData.map((currentRow, currentIndex) => ( currentRow["${escapeString( + : `{{(() => { const tableData = ${widgetName}.processedTableData || []; return tableData.length > 0 ? tableData.map((currentRow, currentIndex) => (currentRow["${escapeString( id, - )}"]))}}`, + )}"])) : ${escapeString(id)} })()}}`, sticky: StickyType.NONE, validation: {}, currencyCode: "USD",