fix: Incorrect display of values in table computed value (#40664)
## Description <ins>Problem</ins> 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. <ins>Root cause</ins> Incorrect handling of double curly braces and nested parentheses in the `computedValue` parsing logic. <ins>Solution</ins> 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" ### 🔍 Cypress test results <!-- This is an auto-generated comment: Cypress test results --> > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: <https://github.com/appsmithorg/appsmith/actions/runs/15061326484> > Commit: 156df279926fc8e39f194d46e868d46c62a1b2bf > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=15061326484&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Table` > Spec: > <hr>Fri, 16 May 2025 06:11:17 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## 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. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
parent
afc9e4e9b4
commit
0c96ea330f
|
|
@ -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;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -955,6 +955,15 @@ const migrations: Migration[] = [
|
|||
],
|
||||
version: 92,
|
||||
},
|
||||
{
|
||||
functionLookup: [
|
||||
{
|
||||
moduleObj: m90,
|
||||
functionName: "migrateTableComputeValueBinding",
|
||||
},
|
||||
],
|
||||
version: 93,
|
||||
},
|
||||
];
|
||||
|
||||
const mockFnObj: Record<number, any> = {};
|
||||
|
|
|
|||
|
|
@ -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", () => {
|
||||
|
|
|
|||
|
|
@ -161,86 +161,25 @@ class ComputeTablePropertyControlV2 extends BaseControl<ComputeTablePropertyCont
|
|||
|
||||
if (!isComputedValue) return propertyValue;
|
||||
|
||||
// Check if the entire structure of the expression looks valid before attempting to parse
|
||||
if (!this.isLikelyValidComputedValue(propertyValue)) {
|
||||
return propertyValue;
|
||||
}
|
||||
|
||||
// Extract the computation logic from the full binding string
|
||||
// Input example: "{{(() => { 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 {{...}})
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user