-
Notifications
You must be signed in to change notification settings - Fork 261
fix: enterprise form validation (#2189) #2190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix: enterprise form validation (#2189) #2190
Conversation
appwrite.ioProject ID: Note Cursor pagination performs better than offset pagination when loading further pages. |
hi @sara-k-48 @tomer @stnguyen90 👋, This PR addresses issue #2189 by improving the validation logic in the enterprise form to handle Inputs Please review the changes when you get a chance. Thanks for your time and feedback! 🙌 |
The preview deployment failed. 🔴 Last updated at: 2025-07-29 14:37:54 CET |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes form validation in the enterprise contact form to prevent submission of empty or whitespace-only fields and ensures proper data sanitization. Previously, the form had no validation and would accept whitespace as legitimate data.
- Added comprehensive validation logic to check for required fields and text-only constraints
- Implemented trimming of input values before validation and submission
- Added specific error messages for empty fields and numeric-only inputs in text fields
|
||
for (const field of validationRules.textOnly) { | ||
const trimmedValue = field.value.trim(); | ||
if (/^\d+$/.test(trimmedValue)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The regex /^\d+$/
only validates that the field contains only digits, but this would also reject valid company names like '3M' or '7-Eleven'. Consider using a more specific validation or clarifying the business requirement for this constraint.
if (/^\d+$/.test(trimmedValue)) { | |
if (!/\D/.test(trimmedValue)) { |
Copilot uses AI. Check for mistakes.
const validationRules = { | ||
required: [ | ||
{ value: firstName, name: 'First name' }, | ||
{ value: lastName, name: 'Last name' }, | ||
{ value: email, name: 'Email' }, | ||
{ value: companyName, name: 'Company name' }, | ||
{ value: companyWebsite, name: 'Company website' }, | ||
{ value: useCase, name: 'Use case' } | ||
], | ||
textOnly: [ | ||
{ value: firstName, name: 'First name' }, | ||
{ value: lastName, name: 'Last name' }, | ||
{ value: companyName, name: 'Company name' }, | ||
{ value: useCase, name: 'Use case' } | ||
] | ||
}; | ||
|
||
for (const field of validationRules.required) { | ||
const trimmedValue = field.value.trim(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same field values are being trimmed multiple times in different validation loops. Consider trimming all values once at the beginning and storing them in variables to avoid redundant operations.
const validationRules = { | |
required: [ | |
{ value: firstName, name: 'First name' }, | |
{ value: lastName, name: 'Last name' }, | |
{ value: email, name: 'Email' }, | |
{ value: companyName, name: 'Company name' }, | |
{ value: companyWebsite, name: 'Company website' }, | |
{ value: useCase, name: 'Use case' } | |
], | |
textOnly: [ | |
{ value: firstName, name: 'First name' }, | |
{ value: lastName, name: 'Last name' }, | |
{ value: companyName, name: 'Company name' }, | |
{ value: useCase, name: 'Use case' } | |
] | |
}; | |
for (const field of validationRules.required) { | |
const trimmedValue = field.value.trim(); | |
const trimmedFirstName = firstName.trim(); | |
const trimmedLastName = lastName.trim(); | |
const trimmedEmail = email.trim(); | |
const trimmedCompanyName = companyName.trim(); | |
const trimmedCompanyWebsite = companyWebsite.trim(); | |
const trimmedUseCase = useCase.trim(); | |
const validationRules = { | |
required: [ | |
{ value: trimmedFirstName, name: 'First name' }, | |
{ value: trimmedLastName, name: 'Last name' }, | |
{ value: trimmedEmail, name: 'Email' }, | |
{ value: trimmedCompanyName, name: 'Company name' }, | |
{ value: trimmedCompanyWebsite, name: 'Company website' }, | |
{ value: trimmedUseCase, name: 'Use case' } | |
], | |
textOnly: [ | |
{ value: trimmedFirstName, name: 'First name' }, | |
{ value: trimmedLastName, name: 'Last name' }, | |
{ value: trimmedCompanyName, name: 'Company name' }, | |
{ value: trimmedUseCase, name: 'Use case' } | |
] | |
}; | |
for (const field of validationRules.required) { | |
const trimmedValue = field.value; |
Copilot uses AI. Check for mistakes.
{ value: companyName, name: 'Company name' }, | ||
{ value: useCase, name: 'Use case' } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Including 'Use case' in the textOnly validation seems inappropriate as use cases could legitimately contain numbers (e.g., 'Support for 1000+ users'). Consider removing this field from the textOnly validation rules.
{ value: companyName, name: 'Company name' }, | |
{ value: useCase, name: 'Use case' } | |
{ value: companyName, name: 'Company name' } |
Copilot uses AI. Check for mistakes.
@Mohit5Upadhyay copilot has suggested some good changes, can you please take a look |
hi @ChiragAgg5k , |
@Mohit5Upadhyay also dont forget to run format command, tests are failing |
Apologies @ChiragAgg5k 🙏
I run the format command and push the changes. Let me know if anything else needed. |
What does this PR do?
This PR fixes: #2189 the validation logic in the
enterprise form
to ensure that input values are properlytrimmed before validation
checks. Previously, there was NO data validation, even the whitespace is also consider as legitimate data.Test Plan
Changes included in this PR:
+page.svelte
for the enterprise form to provide form validationManually tested the enterprise form by:
Update
form-validation.mp4
Related PRs and Issues
Closes #2189
Have you read the Contributing Guidelines on issues?
Yes