fix: email validation in input widget v2 (#40708)
## Description <ins>Problem</ins> 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. <ins>Root cause</ins> Outdated or overly restrictive email validation regex in `InputWidgetV2`. <ins>Solution</ins> 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" ### 🔍 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/15133467577> > Commit: 35d061427857620e3c3a4a2b6e5a7d1b532652aa > <a href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=15133467577&attempt=1" target="_blank">Cypress dashboard</a>. > Tags: `@tag.Input` > Spec: > <hr>Tue, 20 May 2025 09:29:25 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 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. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
Rahul Barwal
GitHub
parent
338357d504
commit
4159380ed5
|
|
@ -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)) {
|
||||
|
|
|
|||
|
|
@ -2,10 +2,11 @@ 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,
|
||||
|
|
@ -18,47 +19,46 @@ describe("Derived property - ", () => {
|
|||
|
||||
expect(isValid).toBeTruthy();
|
||||
|
||||
//Number input with required true and invalid value
|
||||
// TEXT - not required, empty
|
||||
isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.NUMBER,
|
||||
inputText: "test",
|
||||
isRequired: true,
|
||||
inputType: InputTypes.TEXT,
|
||||
inputText: "",
|
||||
isRequired: false,
|
||||
},
|
||||
null,
|
||||
_,
|
||||
);
|
||||
expect(isValid).toBeTruthy();
|
||||
|
||||
expect(isValid).toBeFalsy();
|
||||
|
||||
//Number input with required true and invalid value `null`
|
||||
// EMAIL - not required, empty
|
||||
isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.NUMBER,
|
||||
inputText: null,
|
||||
isRequired: true,
|
||||
inputType: InputTypes.EMAIL,
|
||||
inputText: "",
|
||||
isRequired: false,
|
||||
},
|
||||
null,
|
||||
_,
|
||||
);
|
||||
expect(isValid).toBeTruthy();
|
||||
|
||||
expect(isValid).toBeFalsy();
|
||||
|
||||
//Number input with required true and invalid value `undefined`
|
||||
// PASSWORD - not required, empty
|
||||
isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.NUMBER,
|
||||
inputText: undefined,
|
||||
isRequired: true,
|
||||
inputType: InputTypes.PASSWORD,
|
||||
inputText: "",
|
||||
isRequired: false,
|
||||
},
|
||||
null,
|
||||
_,
|
||||
);
|
||||
expect(isValid).toBeTruthy();
|
||||
});
|
||||
|
||||
expect(isValid).toBeFalsy();
|
||||
|
||||
//Number input with required true and invalid value `""`
|
||||
isValid = derivedProperty.isValid(
|
||||
it("should validate when field is required but empty", () => {
|
||||
// NUMBER - required, empty
|
||||
let isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.NUMBER,
|
||||
inputText: "",
|
||||
|
|
@ -70,8 +70,46 @@ describe("Derived property - ", () => {
|
|||
|
||||
expect(isValid).toBeFalsy();
|
||||
|
||||
//Number input with required true and valid value
|
||||
// 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,
|
||||
|
|
@ -83,46 +121,7 @@ describe("Derived property - ", () => {
|
|||
|
||||
expect(isValid).toBeTruthy();
|
||||
|
||||
//Number input with required true and valid value
|
||||
isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.NUMBER,
|
||||
inputText: 1.1,
|
||||
isRequired: true,
|
||||
},
|
||||
null,
|
||||
_,
|
||||
);
|
||||
|
||||
expect(isValid).toBeTruthy();
|
||||
|
||||
//Text input with required false and empty value
|
||||
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
|
||||
// TEXT - required, valid
|
||||
isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.TEXT,
|
||||
|
|
@ -132,36 +131,9 @@ describe("Derived property - ", () => {
|
|||
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
|
||||
// EMAIL - required, valid
|
||||
isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.EMAIL,
|
||||
|
|
@ -171,36 +143,9 @@ describe("Derived property - ", () => {
|
|||
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
|
||||
// PASSWORD - required, valid
|
||||
isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.PASSWORD,
|
||||
|
|
@ -210,11 +155,13 @@ describe("Derived property - ", () => {
|
|||
null,
|
||||
_,
|
||||
);
|
||||
|
||||
expect(isValid).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
it("should test validation", () => {
|
||||
describe("Custom validation", () => {
|
||||
it("should respect custom validation across all input types", () => {
|
||||
// TEXT with custom validation = false
|
||||
let isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.TEXT,
|
||||
|
|
@ -228,6 +175,7 @@ describe("Derived property - ", () => {
|
|||
|
||||
expect(isValid).toBeFalsy();
|
||||
|
||||
// TEXT with custom validation = true
|
||||
isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.TEXT,
|
||||
|
|
@ -238,9 +186,9 @@ describe("Derived property - ", () => {
|
|||
null,
|
||||
_,
|
||||
);
|
||||
|
||||
expect(isValid).toBeTruthy();
|
||||
|
||||
// NUMBER with custom validation = false
|
||||
isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.NUMBER,
|
||||
|
|
@ -251,9 +199,9 @@ describe("Derived property - ", () => {
|
|||
null,
|
||||
_,
|
||||
);
|
||||
|
||||
expect(isValid).toBeFalsy();
|
||||
|
||||
// NUMBER with custom validation = true
|
||||
isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.NUMBER,
|
||||
|
|
@ -264,9 +212,9 @@ describe("Derived property - ", () => {
|
|||
null,
|
||||
_,
|
||||
);
|
||||
|
||||
expect(isValid).toBeTruthy();
|
||||
|
||||
// EMAIL with custom validation = false
|
||||
isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.EMAIL,
|
||||
|
|
@ -277,9 +225,9 @@ describe("Derived property - ", () => {
|
|||
null,
|
||||
_,
|
||||
);
|
||||
|
||||
expect(isValid).toBeFalsy();
|
||||
|
||||
// EMAIL with custom validation = true
|
||||
isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.EMAIL,
|
||||
|
|
@ -290,9 +238,9 @@ describe("Derived property - ", () => {
|
|||
null,
|
||||
_,
|
||||
);
|
||||
|
||||
expect(isValid).toBeTruthy();
|
||||
|
||||
// PASSWORD with custom validation = false
|
||||
isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.PASSWORD,
|
||||
|
|
@ -303,9 +251,9 @@ describe("Derived property - ", () => {
|
|||
null,
|
||||
_,
|
||||
);
|
||||
|
||||
expect(isValid).toBeFalsy();
|
||||
|
||||
// PASSWORD with custom validation = true
|
||||
isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.PASSWORD,
|
||||
|
|
@ -316,11 +264,13 @@ describe("Derived property - ", () => {
|
|||
null,
|
||||
_,
|
||||
);
|
||||
|
||||
expect(isValid).toBeTruthy();
|
||||
});
|
||||
});
|
||||
|
||||
it("should test regex validation", () => {
|
||||
describe("Regex validation", () => {
|
||||
it("should validate against regex for all input types", () => {
|
||||
// TEXT with matching regex
|
||||
let isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.TEXT,
|
||||
|
|
@ -334,6 +284,7 @@ describe("Derived property - ", () => {
|
|||
|
||||
expect(isValid).toBeTruthy();
|
||||
|
||||
// TEXT with non-matching regex
|
||||
isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.TEXT,
|
||||
|
|
@ -344,9 +295,9 @@ describe("Derived property - ", () => {
|
|||
null,
|
||||
_,
|
||||
);
|
||||
|
||||
expect(isValid).toBeFalsy();
|
||||
|
||||
// NUMBER with matching regex
|
||||
isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.NUMBER,
|
||||
|
|
@ -357,9 +308,9 @@ describe("Derived property - ", () => {
|
|||
null,
|
||||
_,
|
||||
);
|
||||
|
||||
expect(isValid).toBeTruthy();
|
||||
|
||||
// NUMBER with non-matching regex
|
||||
isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.NUMBER,
|
||||
|
|
@ -370,9 +321,9 @@ describe("Derived property - ", () => {
|
|||
null,
|
||||
_,
|
||||
);
|
||||
|
||||
expect(isValid).toBeFalsy();
|
||||
|
||||
// EMAIL with matching regex
|
||||
isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.EMAIL,
|
||||
|
|
@ -383,9 +334,9 @@ describe("Derived property - ", () => {
|
|||
null,
|
||||
_,
|
||||
);
|
||||
|
||||
expect(isValid).toBeTruthy();
|
||||
|
||||
// EMAIL with non-matching regex
|
||||
isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.EMAIL,
|
||||
|
|
@ -396,9 +347,9 @@ describe("Derived property - ", () => {
|
|||
null,
|
||||
_,
|
||||
);
|
||||
|
||||
expect(isValid).toBeFalsy();
|
||||
|
||||
// PASSWORD with matching regex
|
||||
isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.PASSWORD,
|
||||
|
|
@ -409,9 +360,9 @@ describe("Derived property - ", () => {
|
|||
null,
|
||||
_,
|
||||
);
|
||||
|
||||
expect(isValid).toBeTruthy();
|
||||
|
||||
// PASSWORD with non-matching regex
|
||||
isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.PASSWORD,
|
||||
|
|
@ -422,11 +373,79 @@ describe("Derived property - ", () => {
|
|||
null,
|
||||
_,
|
||||
);
|
||||
|
||||
expect(isValid).toBeFalsy();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
it("should test email type built in validation", () => {
|
||||
describe("NUMBER input type specific behaviors", () => {
|
||||
it("should validate different number formats", () => {
|
||||
// Integer value
|
||||
let isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.NUMBER,
|
||||
inputText: 1,
|
||||
isRequired: true,
|
||||
},
|
||||
null,
|
||||
_,
|
||||
);
|
||||
|
||||
expect(isValid).toBeTruthy();
|
||||
|
||||
// Decimal value
|
||||
isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.NUMBER,
|
||||
inputText: 1.1,
|
||||
isRequired: true,
|
||||
},
|
||||
null,
|
||||
_,
|
||||
);
|
||||
expect(isValid).toBeTruthy();
|
||||
|
||||
// Invalid number (string)
|
||||
isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.NUMBER,
|
||||
inputText: "test",
|
||||
isRequired: true,
|
||||
},
|
||||
null,
|
||||
_,
|
||||
);
|
||||
expect(isValid).toBeFalsy();
|
||||
|
||||
// Null value
|
||||
isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.NUMBER,
|
||||
inputText: null,
|
||||
isRequired: true,
|
||||
},
|
||||
null,
|
||||
_,
|
||||
);
|
||||
expect(isValid).toBeFalsy();
|
||||
|
||||
// Undefined value
|
||||
isValid = derivedProperty.isValid(
|
||||
{
|
||||
inputType: InputTypes.NUMBER,
|
||||
inputText: undefined,
|
||||
isRequired: true,
|
||||
},
|
||||
null,
|
||||
_,
|
||||
);
|
||||
expect(isValid).toBeFalsy();
|
||||
});
|
||||
});
|
||||
|
||||
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();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user