From 4159380ed59decb6e7f844953baa982b959edae4 Mon Sep 17 00:00:00 2001 From: Rahul Barwal Date: Tue, 20 May 2025 15:41:27 +0530 Subject: [PATCH] fix: email validation in input widget v2 (#40708) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Problem Inbuilt email validation was not functioning correctly for all valid email formats (e.g. `rahul+3@appsmith.com`), leading to incorrect validation failures in the Input widget. Root cause Outdated or overly restrictive email validation regex in `InputWidgetV2`. Solution This PR handles updating the email validation regex in `InputWidgetV2` to support modern and valid email formats. It also refactors and enhances the validation test suite to improve coverage across input types (NUMBER, TEXT, EMAIL, PASSWORD) and validation scenarios (required, custom, and regex), improving accuracy, readability, and maintainability. Fixes #`Issue Number` _or_ Fixes https://github.com/appsmithorg/appsmith-ee/issues/7550 > [!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.Input" ### :mag: Cypress test results > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: 35d061427857620e3c3a4a2b6e5a7d1b532652aa > Cypress dashboard. > Tags: `@tag.Input` > Spec: >
Tue, 20 May 2025 09:29:25 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit - **Bug Fixes** - Improved email validation to support a wider range of valid email formats. - **Tests** - Expanded and reorganized input validation tests to cover more input types and edge cases, ensuring more robust validation across NUMBER, TEXT, EMAIL, and PASSWORD fields. --- .../widgets/InputWidgetV2/widget/derived.js | 2 +- .../InputWidgetV2/widget/derived.test.ts | 775 +++++++++--------- 2 files changed, 404 insertions(+), 373 deletions(-) diff --git a/app/client/src/widgets/InputWidgetV2/widget/derived.js b/app/client/src/widgets/InputWidgetV2/widget/derived.js index ec33f0a365..f32457cae6 100644 --- a/app/client/src/widgets/InputWidgetV2/widget/derived.js +++ b/app/client/src/widgets/InputWidgetV2/widget/derived.js @@ -76,7 +76,7 @@ export default { * https://stackoverflow.com/questions/15017052/understanding-email-validation-using-javascript * */ const emailRegex = new RegExp( - /^\w+([\.-]?\w+)*@\w+([\.-]?\w+)*(\.\w{2,4})+$/, + /^(([^<>()[\]\\.,;:\s@"]+(\.[^<>()[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/, ); if (!emailRegex.test(value)) { diff --git a/app/client/src/widgets/InputWidgetV2/widget/derived.test.ts b/app/client/src/widgets/InputWidgetV2/widget/derived.test.ts index ac6e9c4fc2..ddbf5a6121 100644 --- a/app/client/src/widgets/InputWidgetV2/widget/derived.test.ts +++ b/app/client/src/widgets/InputWidgetV2/widget/derived.test.ts @@ -2,76 +2,386 @@ import _ from "lodash"; import { InputTypes } from "widgets/BaseInputWidget/constants"; import derivedProperty from "./derived"; -describe("Derived property - ", () => { - describe("isValid property", () => { - it("should test isRequired", () => { - //Number input with required false and empty value +describe("Derived property - InputWidgetV2", () => { + describe("Common behaviors across input types", () => { + describe("Required field validation", () => { + it("should validate when field is not required", () => { + // NUMBER - not required, empty + let isValid = derivedProperty.isValid( + { + inputType: InputTypes.NUMBER, + inputText: undefined, + isRequired: false, + }, + null, + _, + ); + + expect(isValid).toBeTruthy(); + + // TEXT - not required, empty + isValid = derivedProperty.isValid( + { + inputType: InputTypes.TEXT, + inputText: "", + isRequired: false, + }, + null, + _, + ); + expect(isValid).toBeTruthy(); + + // EMAIL - not required, empty + isValid = derivedProperty.isValid( + { + inputType: InputTypes.EMAIL, + inputText: "", + isRequired: false, + }, + null, + _, + ); + expect(isValid).toBeTruthy(); + + // PASSWORD - not required, empty + isValid = derivedProperty.isValid( + { + inputType: InputTypes.PASSWORD, + inputText: "", + isRequired: false, + }, + null, + _, + ); + expect(isValid).toBeTruthy(); + }); + + it("should validate when field is required but empty", () => { + // NUMBER - required, empty + let isValid = derivedProperty.isValid( + { + inputType: InputTypes.NUMBER, + inputText: "", + isRequired: true, + }, + null, + _, + ); + + expect(isValid).toBeFalsy(); + + // TEXT - required, empty + isValid = derivedProperty.isValid( + { + inputType: InputTypes.TEXT, + inputText: "", + isRequired: true, + }, + null, + _, + ); + expect(isValid).toBeFalsy(); + + // EMAIL - required, empty + isValid = derivedProperty.isValid( + { + inputType: InputTypes.EMAIL, + inputText: "", + isRequired: true, + }, + null, + _, + ); + expect(isValid).toBeFalsy(); + + // PASSWORD - required, empty + isValid = derivedProperty.isValid( + { + inputType: InputTypes.PASSWORD, + inputText: "", + isRequired: true, + }, + null, + _, + ); + expect(isValid).toBeFalsy(); + }); + + it("should validate when field is required with valid values", () => { + // NUMBER - required, valid + let isValid = derivedProperty.isValid( + { + inputType: InputTypes.NUMBER, + inputText: 1, + isRequired: true, + }, + null, + _, + ); + + expect(isValid).toBeTruthy(); + + // TEXT - required, valid + isValid = derivedProperty.isValid( + { + inputType: InputTypes.TEXT, + inputText: "test", + isRequired: true, + }, + null, + _, + ); + expect(isValid).toBeTruthy(); + + // EMAIL - required, valid + isValid = derivedProperty.isValid( + { + inputType: InputTypes.EMAIL, + inputText: "test@appsmith.com", + isRequired: true, + }, + null, + _, + ); + expect(isValid).toBeTruthy(); + + // PASSWORD - required, valid + isValid = derivedProperty.isValid( + { + inputType: InputTypes.PASSWORD, + inputText: "admin", + isRequired: true, + }, + null, + _, + ); + expect(isValid).toBeTruthy(); + }); + }); + + describe("Custom validation", () => { + it("should respect custom validation across all input types", () => { + // TEXT with custom validation = false + let isValid = derivedProperty.isValid( + { + inputType: InputTypes.TEXT, + inputText: "test", + isRequired: true, + validation: false, + }, + null, + _, + ); + + expect(isValid).toBeFalsy(); + + // TEXT with custom validation = true + isValid = derivedProperty.isValid( + { + inputType: InputTypes.TEXT, + inputText: "test", + isRequired: true, + validation: true, + }, + null, + _, + ); + expect(isValid).toBeTruthy(); + + // NUMBER with custom validation = false + isValid = derivedProperty.isValid( + { + inputType: InputTypes.NUMBER, + inputText: 1, + isRequired: true, + validation: false, + }, + null, + _, + ); + expect(isValid).toBeFalsy(); + + // NUMBER with custom validation = true + isValid = derivedProperty.isValid( + { + inputType: InputTypes.NUMBER, + inputText: 1, + isRequired: true, + validation: true, + }, + null, + _, + ); + expect(isValid).toBeTruthy(); + + // EMAIL with custom validation = false + isValid = derivedProperty.isValid( + { + inputType: InputTypes.EMAIL, + inputText: "test@appsmith.com", + isRequired: true, + validation: false, + }, + null, + _, + ); + expect(isValid).toBeFalsy(); + + // EMAIL with custom validation = true + isValid = derivedProperty.isValid( + { + inputType: InputTypes.EMAIL, + inputText: "test@appsmith.com", + isRequired: true, + validation: true, + }, + null, + _, + ); + expect(isValid).toBeTruthy(); + + // PASSWORD with custom validation = false + isValid = derivedProperty.isValid( + { + inputType: InputTypes.PASSWORD, + inputText: "admin123", + isRequired: true, + validation: false, + }, + null, + _, + ); + expect(isValid).toBeFalsy(); + + // PASSWORD with custom validation = true + isValid = derivedProperty.isValid( + { + inputType: InputTypes.PASSWORD, + inputText: "admin123", + isRequired: true, + validation: true, + }, + null, + _, + ); + expect(isValid).toBeTruthy(); + }); + }); + + describe("Regex validation", () => { + it("should validate against regex for all input types", () => { + // TEXT with matching regex + let isValid = derivedProperty.isValid( + { + inputType: InputTypes.TEXT, + inputText: "test", + isRequired: true, + regex: "^test$", + }, + null, + _, + ); + + expect(isValid).toBeTruthy(); + + // TEXT with non-matching regex + isValid = derivedProperty.isValid( + { + inputType: InputTypes.TEXT, + inputText: "test123", + isRequired: true, + regex: "^test$", + }, + null, + _, + ); + expect(isValid).toBeFalsy(); + + // NUMBER with matching regex + isValid = derivedProperty.isValid( + { + inputType: InputTypes.NUMBER, + inputText: 1, + isRequired: true, + regex: "^1$", + }, + null, + _, + ); + expect(isValid).toBeTruthy(); + + // NUMBER with non-matching regex + isValid = derivedProperty.isValid( + { + inputType: InputTypes.NUMBER, + inputText: 2, + isRequired: true, + regex: "^1$", + }, + null, + _, + ); + expect(isValid).toBeFalsy(); + + // EMAIL with matching regex + isValid = derivedProperty.isValid( + { + inputType: InputTypes.EMAIL, + inputText: "test@appsmith.com", + isRequired: true, + regex: "^test@appsmith.com$", + }, + null, + _, + ); + expect(isValid).toBeTruthy(); + + // EMAIL with non-matching regex + isValid = derivedProperty.isValid( + { + inputType: InputTypes.EMAIL, + inputText: "test123@appsmith.com", + isRequired: true, + regex: "^test@appsmith.com$", + }, + null, + _, + ); + expect(isValid).toBeFalsy(); + + // PASSWORD with matching regex + isValid = derivedProperty.isValid( + { + inputType: InputTypes.PASSWORD, + inputText: "admin123", + isRequired: true, + regex: "^admin123$", + }, + null, + _, + ); + expect(isValid).toBeTruthy(); + + // PASSWORD with non-matching regex + isValid = derivedProperty.isValid( + { + inputType: InputTypes.PASSWORD, + inputText: "admin1234", + isRequired: true, + regex: "^admin123$", + }, + null, + _, + ); + expect(isValid).toBeFalsy(); + }); + }); + }); + + describe("NUMBER input type specific behaviors", () => { + it("should validate different number formats", () => { + // Integer value let isValid = derivedProperty.isValid( - { - inputType: InputTypes.NUMBER, - inputText: undefined, - isRequired: false, - }, - null, - _, - ); - - expect(isValid).toBeTruthy(); - - //Number input with required true and invalid value - isValid = derivedProperty.isValid( - { - inputType: InputTypes.NUMBER, - inputText: "test", - isRequired: true, - }, - null, - _, - ); - - expect(isValid).toBeFalsy(); - - //Number input with required true and invalid value `null` - isValid = derivedProperty.isValid( - { - inputType: InputTypes.NUMBER, - inputText: null, - isRequired: true, - }, - null, - _, - ); - - expect(isValid).toBeFalsy(); - - //Number input with required true and invalid value `undefined` - isValid = derivedProperty.isValid( - { - inputType: InputTypes.NUMBER, - inputText: undefined, - isRequired: true, - }, - null, - _, - ); - - expect(isValid).toBeFalsy(); - - //Number input with required true and invalid value `""` - isValid = derivedProperty.isValid( - { - inputType: InputTypes.NUMBER, - inputText: "", - isRequired: true, - }, - null, - _, - ); - - expect(isValid).toBeFalsy(); - - //Number input with required true and valid value - isValid = derivedProperty.isValid( { inputType: InputTypes.NUMBER, inputText: 1, @@ -83,7 +393,7 @@ describe("Derived property - ", () => { expect(isValid).toBeTruthy(); - //Number input with required true and valid value + // Decimal value isValid = derivedProperty.isValid( { inputType: InputTypes.NUMBER, @@ -93,340 +403,49 @@ describe("Derived property - ", () => { null, _, ); - expect(isValid).toBeTruthy(); - //Text input with required false and empty value + // Invalid number (string) isValid = derivedProperty.isValid( { - inputType: InputTypes.TEXT, - inputText: "", - isRequired: false, - }, - null, - _, - ); - - expect(isValid).toBeTruthy(); - - //Text input with required true and invalid value - isValid = derivedProperty.isValid( - { - inputType: InputTypes.TEXT, - inputText: "", - isRequired: true, - }, - null, - _, - ); - - expect(isValid).toBeFalsy(); - - //Text input with required true and valid value - isValid = derivedProperty.isValid( - { - inputType: InputTypes.TEXT, + inputType: InputTypes.NUMBER, inputText: "test", isRequired: true, }, null, _, ); - - expect(isValid).toBeTruthy(); - - //Email input with required false and empty value - isValid = derivedProperty.isValid( - { - inputType: InputTypes.EMAIL, - inputText: "", - isRequired: false, - }, - null, - _, - ); - - expect(isValid).toBeTruthy(); - - //Email input with required true and invalid value - isValid = derivedProperty.isValid( - { - inputType: InputTypes.EMAIL, - inputText: "", - isRequired: true, - }, - null, - _, - ); - expect(isValid).toBeFalsy(); - //Email input with required true and valid value - isValid = derivedProperty.isValid( - { - inputType: InputTypes.EMAIL, - inputText: "test@appsmith.com", - isRequired: true, - }, - null, - _, - ); - - expect(isValid).toBeTruthy(); - - //Password input with required false and empty value - isValid = derivedProperty.isValid( - { - inputType: InputTypes.PASSWORD, - inputText: "", - isRequired: false, - }, - null, - _, - ); - - expect(isValid).toBeTruthy(); - - //Password input with required true and invalid value - isValid = derivedProperty.isValid( - { - inputType: InputTypes.PASSWORD, - inputText: "", - isRequired: true, - }, - null, - _, - ); - - expect(isValid).toBeFalsy(); - - //Password input with required true and valid value - isValid = derivedProperty.isValid( - { - inputType: InputTypes.PASSWORD, - inputText: "admin", - isRequired: true, - }, - null, - _, - ); - - expect(isValid).toBeTruthy(); - }); - - it("should test validation", () => { - let isValid = derivedProperty.isValid( - { - inputType: InputTypes.TEXT, - inputText: "test", - isRequired: true, - validation: false, - }, - null, - _, - ); - - expect(isValid).toBeFalsy(); - - isValid = derivedProperty.isValid( - { - inputType: InputTypes.TEXT, - inputText: "test", - isRequired: true, - validation: true, - }, - null, - _, - ); - - expect(isValid).toBeTruthy(); - + // Null value isValid = derivedProperty.isValid( { inputType: InputTypes.NUMBER, - inputText: 1, + inputText: null, isRequired: true, - validation: false, }, null, _, ); - expect(isValid).toBeFalsy(); + // Undefined value isValid = derivedProperty.isValid( { inputType: InputTypes.NUMBER, - inputText: 1, + inputText: undefined, isRequired: true, - validation: true, }, null, _, ); - - expect(isValid).toBeTruthy(); - - isValid = derivedProperty.isValid( - { - inputType: InputTypes.EMAIL, - inputText: "test@appsmith.com", - isRequired: true, - validation: false, - }, - null, - _, - ); - - expect(isValid).toBeFalsy(); - - isValid = derivedProperty.isValid( - { - inputType: InputTypes.EMAIL, - inputText: "test@appsmith.com", - isRequired: true, - validation: true, - }, - null, - _, - ); - - expect(isValid).toBeTruthy(); - - isValid = derivedProperty.isValid( - { - inputType: InputTypes.PASSWORD, - inputText: "admin123", - isRequired: true, - validation: false, - }, - null, - _, - ); - - expect(isValid).toBeFalsy(); - - isValid = derivedProperty.isValid( - { - inputType: InputTypes.PASSWORD, - inputText: "admin123", - isRequired: true, - validation: true, - }, - null, - _, - ); - - expect(isValid).toBeTruthy(); - }); - - it("should test regex validation", () => { - let isValid = derivedProperty.isValid( - { - inputType: InputTypes.TEXT, - inputText: "test", - isRequired: true, - regex: "^test$", - }, - null, - _, - ); - - expect(isValid).toBeTruthy(); - - isValid = derivedProperty.isValid( - { - inputType: InputTypes.TEXT, - inputText: "test123", - isRequired: true, - regex: "^test$", - }, - null, - _, - ); - - expect(isValid).toBeFalsy(); - - isValid = derivedProperty.isValid( - { - inputType: InputTypes.NUMBER, - inputText: 1, - isRequired: true, - regex: "^1$", - }, - null, - _, - ); - - expect(isValid).toBeTruthy(); - - isValid = derivedProperty.isValid( - { - inputType: InputTypes.NUMBER, - inputText: 2, - isRequired: true, - regex: "^1$", - }, - null, - _, - ); - - expect(isValid).toBeFalsy(); - - isValid = derivedProperty.isValid( - { - inputType: InputTypes.EMAIL, - inputText: "test@appsmith.com", - isRequired: true, - regex: "^test@appsmith.com$", - }, - null, - _, - ); - - expect(isValid).toBeTruthy(); - - isValid = derivedProperty.isValid( - { - inputType: InputTypes.EMAIL, - inputText: "test123@appsmith.com", - isRequired: true, - regex: "^test@appsmith.com$", - }, - null, - _, - ); - - expect(isValid).toBeFalsy(); - - isValid = derivedProperty.isValid( - { - inputType: InputTypes.PASSWORD, - inputText: "admin123", - isRequired: true, - regex: "^admin123$", - }, - null, - _, - ); - - expect(isValid).toBeTruthy(); - - isValid = derivedProperty.isValid( - { - inputType: InputTypes.PASSWORD, - inputText: "admin1234", - isRequired: true, - regex: "^admin123$", - }, - null, - _, - ); - expect(isValid).toBeFalsy(); }); + }); - it("should test email type built in validation", () => { + describe("EMAIL input type specific behaviors", () => { + it("should validate email format", () => { + // Valid email let isValid = derivedProperty.isValid( { inputType: InputTypes.EMAIL, @@ -439,6 +458,7 @@ describe("Derived property - ", () => { expect(isValid).toBeTruthy(); + // Invalid email (no @ symbol) isValid = derivedProperty.isValid( { inputType: InputTypes.EMAIL, @@ -448,9 +468,9 @@ describe("Derived property - ", () => { null, _, ); - expect(isValid).toBeFalsy(); + // Email with uppercase domain isValid = derivedProperty.isValid( { inputType: InputTypes.EMAIL, @@ -460,7 +480,18 @@ describe("Derived property - ", () => { null, _, ); + expect(isValid).toBeTruthy(); + // Email with special characters + isValid = derivedProperty.isValid( + { + inputType: InputTypes.EMAIL, + inputText: "test+123@appsmith.com", + isRequired: true, + }, + null, + _, + ); expect(isValid).toBeTruthy(); }); });