fix: edge case to ignore parsing very large numbers (#37104)
## Description <ins>Problem</ins> The defaultOptionValueValidation function in select.ts is not properly handling different input values, resulting in inconsistent behavior. <ins>Root cause</ins> This function does not account for edge cases where the input value is a very large number and is resulting in a rounded off number, leading to incorrect validation results. <ins>Solution</ins> This PR changes the behavior to parsing only when required. Fixes #27881 _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.JSONForm" ### 🔍 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/11517642696> > Commit: d31373a95fbf67429be8ce6a8717d74c3827098c > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=11517642696&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.JSONForm` > Spec: > <hr>Fri, 25 Oct 2024 12:16:10 UTC <!-- end of auto-generated comment: Cypress test results --> ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [x] No <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced type safety for various functions, improving overall reliability. - Streamlined validation logic for option values, allowing for more robust input handling. - **Bug Fixes** - Improved test coverage for `defaultOptionValueValidation`, ensuring accurate handling of both truthy and falsy values. - **Documentation** - Added comments indicating areas for future improvements in the codebase. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
parent
5bca179116
commit
5e46804698
|
|
@ -226,9 +226,7 @@ export const normalizeArrayValue = (data: any[]) => {
|
||||||
return data[0];
|
return data[0];
|
||||||
};
|
};
|
||||||
|
|
||||||
// TODO: Fix this the next time the file is edited
|
export const fieldTypeFor = (value: unknown): FieldType => {
|
||||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
|
||||||
export const fieldTypeFor = (value: any): FieldType => {
|
|
||||||
const dataType = dataTypeFor(value);
|
const dataType = dataTypeFor(value);
|
||||||
const potentialFieldType = DATA_TYPE_POTENTIAL_FIELD[dataType];
|
const potentialFieldType = DATA_TYPE_POTENTIAL_FIELD[dataType];
|
||||||
const subDataType = subDataTypeFor(value);
|
const subDataType = subDataTypeFor(value);
|
||||||
|
|
|
||||||
|
|
@ -4,98 +4,120 @@ import type { JSONFormWidgetProps } from "../..";
|
||||||
import { defaultOptionValueValidation } from "./select";
|
import { defaultOptionValueValidation } from "./select";
|
||||||
|
|
||||||
describe(".defaultOptionValueValidation", () => {
|
describe(".defaultOptionValueValidation", () => {
|
||||||
it("return undefined when input is undefined", () => {
|
describe("handling falsey values", () => {
|
||||||
const input = undefined;
|
it("return undefined when input is undefined", () => {
|
||||||
const expectedOutput = {
|
const input = undefined;
|
||||||
isValid: true,
|
const expectedOutput = {
|
||||||
parsed: undefined,
|
isValid: true,
|
||||||
messages: [{ name: "", message: "" }],
|
parsed: undefined,
|
||||||
};
|
messages: [{ name: "", message: "" }],
|
||||||
|
};
|
||||||
|
|
||||||
const response = defaultOptionValueValidation(
|
const response = defaultOptionValueValidation(
|
||||||
input,
|
input,
|
||||||
{} as JSONFormWidgetProps,
|
{} as JSONFormWidgetProps,
|
||||||
_,
|
_,
|
||||||
);
|
);
|
||||||
|
|
||||||
expect(response).toEqual(expectedOutput);
|
expect(response).toEqual(expectedOutput);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("return null when input is null", () => {
|
||||||
|
const input = null;
|
||||||
|
const expectedOutput = {
|
||||||
|
isValid: true,
|
||||||
|
parsed: null,
|
||||||
|
messages: [{ name: "", message: "" }],
|
||||||
|
};
|
||||||
|
|
||||||
|
const response = defaultOptionValueValidation(
|
||||||
|
input,
|
||||||
|
{} as JSONFormWidgetProps,
|
||||||
|
_,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(response).toEqual(expectedOutput);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("return empty string with empty string", () => {
|
||||||
|
const input = "";
|
||||||
|
const expectedOutput = {
|
||||||
|
isValid: true,
|
||||||
|
parsed: "",
|
||||||
|
messages: [{ name: "", message: "" }],
|
||||||
|
};
|
||||||
|
|
||||||
|
const response = defaultOptionValueValidation(
|
||||||
|
input,
|
||||||
|
{} as JSONFormWidgetProps,
|
||||||
|
_,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(response).toEqual(expectedOutput);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
it("return null when input is null", () => {
|
describe("handling truthy values", () => {
|
||||||
const input = null;
|
it("return value with string", () => {
|
||||||
const expectedOutput = {
|
const input = "green";
|
||||||
isValid: true,
|
const expectedOutput = {
|
||||||
parsed: null,
|
isValid: true,
|
||||||
messages: [{ name: "", message: "" }],
|
parsed: "green",
|
||||||
};
|
messages: [{ name: "", message: "" }],
|
||||||
|
};
|
||||||
|
|
||||||
const response = defaultOptionValueValidation(
|
const response = defaultOptionValueValidation(
|
||||||
input,
|
input,
|
||||||
{} as JSONFormWidgetProps,
|
{} as JSONFormWidgetProps,
|
||||||
_,
|
_,
|
||||||
);
|
);
|
||||||
|
|
||||||
expect(response).toEqual(expectedOutput);
|
expect(response).toEqual(expectedOutput);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("return empty string with empty string", () => {
|
it("return value with stringified json", () => {
|
||||||
const input = "";
|
const input = `
|
||||||
const expectedOutput = {
|
|
||||||
isValid: true,
|
|
||||||
parsed: "",
|
|
||||||
messages: [{ name: "", message: "" }],
|
|
||||||
};
|
|
||||||
|
|
||||||
const response = defaultOptionValueValidation(
|
|
||||||
input,
|
|
||||||
{} as JSONFormWidgetProps,
|
|
||||||
_,
|
|
||||||
);
|
|
||||||
|
|
||||||
expect(response).toEqual(expectedOutput);
|
|
||||||
});
|
|
||||||
|
|
||||||
it("return value with string", () => {
|
|
||||||
const input = "green";
|
|
||||||
const expectedOutput = {
|
|
||||||
isValid: true,
|
|
||||||
parsed: "green",
|
|
||||||
messages: [{ name: "", message: "" }],
|
|
||||||
};
|
|
||||||
|
|
||||||
const response = defaultOptionValueValidation(
|
|
||||||
input,
|
|
||||||
{} as JSONFormWidgetProps,
|
|
||||||
_,
|
|
||||||
);
|
|
||||||
|
|
||||||
expect(response).toEqual(expectedOutput);
|
|
||||||
});
|
|
||||||
|
|
||||||
it("return value with stringified json", () => {
|
|
||||||
const input = `
|
|
||||||
{
|
{
|
||||||
"label": "green",
|
"label": "green",
|
||||||
"value": "green"
|
"value": "green"
|
||||||
}
|
}
|
||||||
`;
|
`;
|
||||||
|
|
||||||
const expectedOutput = {
|
const expectedOutput = {
|
||||||
isValid: true,
|
isValid: true,
|
||||||
parsed: {
|
parsed: {
|
||||||
label: "green",
|
label: "green",
|
||||||
value: "green",
|
value: "green",
|
||||||
},
|
},
|
||||||
messages: [{ name: "", message: "" }],
|
messages: [{ name: "", message: "" }],
|
||||||
};
|
};
|
||||||
|
|
||||||
const response = defaultOptionValueValidation(
|
const response = defaultOptionValueValidation(
|
||||||
input,
|
input,
|
||||||
{} as JSONFormWidgetProps,
|
{} as JSONFormWidgetProps,
|
||||||
_,
|
_,
|
||||||
);
|
);
|
||||||
|
|
||||||
expect(response).toEqual(expectedOutput);
|
expect(response).toEqual(expectedOutput);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("Edge Case: For very long numbers passed as string, don't parse it as number", () => {
|
||||||
|
const input =
|
||||||
|
"123456789012345678901234567890123456789012345678901234567890";
|
||||||
|
const expectedOutput = {
|
||||||
|
isValid: true,
|
||||||
|
parsed: "123456789012345678901234567890123456789012345678901234567890",
|
||||||
|
messages: [{ name: "", message: "" }],
|
||||||
|
};
|
||||||
|
|
||||||
|
const response = defaultOptionValueValidation(
|
||||||
|
input,
|
||||||
|
{} as JSONFormWidgetProps,
|
||||||
|
_,
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(response).toEqual(expectedOutput);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
it("should return isValid false with invalid values", () => {
|
it("should return isValid false with invalid values", () => {
|
||||||
|
|
|
||||||
|
|
@ -7,21 +7,13 @@ import type { ValidationResponse } from "constants/WidgetValidation";
|
||||||
import { ValidationTypes } from "constants/WidgetValidation";
|
import { ValidationTypes } from "constants/WidgetValidation";
|
||||||
import { EvaluationSubstitutionType } from "entities/DataTree/dataTreeFactory";
|
import { EvaluationSubstitutionType } from "entities/DataTree/dataTreeFactory";
|
||||||
import { AutocompleteDataType } from "utils/autocomplete/AutocompleteDataType";
|
import { AutocompleteDataType } from "utils/autocomplete/AutocompleteDataType";
|
||||||
|
import type { LoDashStatic } from "lodash";
|
||||||
|
|
||||||
export function defaultOptionValueValidation(
|
export function defaultOptionValueValidation(
|
||||||
inputValue: unknown,
|
value: unknown,
|
||||||
props: JSONFormWidgetProps,
|
props: JSONFormWidgetProps,
|
||||||
// TODO: Fix this the next time the file is edited
|
_: LoDashStatic,
|
||||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
|
||||||
_: any,
|
|
||||||
): ValidationResponse {
|
): ValidationResponse {
|
||||||
const DEFAULT_ERROR_MESSAGE = {
|
|
||||||
name: "TypeError",
|
|
||||||
message:
|
|
||||||
'value should match: string | { "label": "label1", "value": "value1" }',
|
|
||||||
};
|
|
||||||
let value = inputValue;
|
|
||||||
|
|
||||||
const hasLabelValueProperties = (
|
const hasLabelValueProperties = (
|
||||||
// TODO: Fix this the next time the file is edited
|
// TODO: Fix this the next time the file is edited
|
||||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||||
|
|
@ -38,22 +30,7 @@ export function defaultOptionValueValidation(
|
||||||
|
|
||||||
// If input value is empty string then we can fairly assume that the input
|
// If input value is empty string then we can fairly assume that the input
|
||||||
// was cleared out and can be treated as undefined.
|
// was cleared out and can be treated as undefined.
|
||||||
if (inputValue === undefined || inputValue === null || inputValue === "") {
|
if (value === undefined || value === null || value === "") {
|
||||||
return {
|
|
||||||
isValid: true,
|
|
||||||
parsed: inputValue,
|
|
||||||
messages: [{ name: "", message: "" }],
|
|
||||||
};
|
|
||||||
}
|
|
||||||
|
|
||||||
if (typeof inputValue === "string") {
|
|
||||||
try {
|
|
||||||
value = JSON.parse(inputValue);
|
|
||||||
} catch (e) {}
|
|
||||||
}
|
|
||||||
|
|
||||||
if (_.isString(value) || _.isFinite(value)) {
|
|
||||||
// When value is "", "green", 444
|
|
||||||
return {
|
return {
|
||||||
isValid: true,
|
isValid: true,
|
||||||
parsed: value,
|
parsed: value,
|
||||||
|
|
@ -61,8 +38,22 @@ export function defaultOptionValueValidation(
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
if (hasLabelValueProperties(value)) {
|
if (typeof value === "string") {
|
||||||
// When value is {label: "green", value: "green"}
|
try {
|
||||||
|
const parsedValue = JSON.parse(value);
|
||||||
|
|
||||||
|
if (_.isObject(parsedValue)) {
|
||||||
|
value = parsedValue;
|
||||||
|
}
|
||||||
|
} catch (e) {}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (
|
||||||
|
_.isString(value) ||
|
||||||
|
_.isFinite(value) ||
|
||||||
|
hasLabelValueProperties(value)
|
||||||
|
) {
|
||||||
|
// When value is "", "green", 444
|
||||||
return {
|
return {
|
||||||
isValid: true,
|
isValid: true,
|
||||||
parsed: value,
|
parsed: value,
|
||||||
|
|
@ -73,7 +64,13 @@ export function defaultOptionValueValidation(
|
||||||
return {
|
return {
|
||||||
isValid: false,
|
isValid: false,
|
||||||
parsed: {},
|
parsed: {},
|
||||||
messages: [DEFAULT_ERROR_MESSAGE],
|
messages: [
|
||||||
|
{
|
||||||
|
name: "TypeError",
|
||||||
|
message:
|
||||||
|
'value should match: string | { "label": "label1", "value": "value1" }',
|
||||||
|
},
|
||||||
|
],
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user