fix: Add comprehensive input validation to BasicDetails form(#197) #198
Conversation
WalkthroughAdds client-side form validation and per-field error handling to the BasicDetails multi-step form: introduces validation utilities (phone, email, URL), nested Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Form as BasicDetails Form
participant Validator as validateStep()
participant State as Form State
participant UI as UI Components
User->>Form: Enter or modify field
Form->>State: handleInputChange(field, value)
State->>State: Update nested formData & clear related errors
State->>UI: Re-render field
alt Field invalid
UI->>UI: Show inline error + highlight (Alert icon)
else Field valid
UI->>UI: Clear error styling
end
User->>Form: Click Next
Form->>Validator: validateStep(currentStep, userType)
Validator-->>Form: errors map or success
alt Valid
Form->>Form: Advance step
else Invalid
Form->>State: Set errors map
State->>UI: Re-render showing errors
Form-->>User: Stay on step (block progression)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Frontend/src/pages/BasicDetails.tsx (1)
687-695: Reset logic should operate on component state, not the raw DOM
resetFormcurrently clears native<input>and<select>elements viadocument.querySelectorAll, but all of the form fields here are controlled by component state (formData) and customSelectcomponents:
- Direct DOM mutations are immediately overwritten on the next render.
errorsare never cleared, so old validation messages may persist after “Reset form”.You should reset the React state instead and drop the DOM manipulation, for example:
const resetForm = () => { setStep(0); setAnimationDirection(0); - - document.querySelectorAll("input").forEach((input) => (input.value = "")); - document - .querySelectorAll("select") - .forEach((select) => (select.value = "")); + setFormData({ + influencer: { + firstName: "", + lastName: "", + email: "", + phone: "", + category: "", + instagram: "", + youtube: "", + twitter: "", + tiktok: "", + website: "", + audienceSize: "", + avgEngagement: "", + mainPlatform: "", + audienceAge: "", + }, + brand: { + companyName: "", + website: "", + industry: "", + size: "", + budget: "", + targetAudience: "", + preferredPlatforms: "", + campaignGoals: "", + }, + }); + setErrors({}); };If you’d like to avoid duplicating the initial object, you can also extract it into a shared
initialFormDataconstant and reuse it both inuseStateand here.
🧹 Nitpick comments (1)
Frontend/src/pages/BasicDetails.tsx (1)
162-182: Minor improvement to error clearing inhandleInputChangeThe per‑field error clearing works, but it relies on the outer
errorsclosure:if (errors[field]) { setErrors(prev => { const newErrors = { ...prev }; delete newErrors[field]; return newErrors; }); }You can simplify this and avoid any stale reads by always deleting the key inside the state updater:
- // Clear error for this field - if (errors[field]) { - setErrors(prev => { - const newErrors = { ...prev }; - delete newErrors[field]; - return newErrors; - }); - } + // Clear error for this field + setErrors(prev => { + if (!prev[field]) return prev; + const newErrors = { ...prev }; + delete newErrors[field]; + return newErrors; + });Same behavior, but the logic is fully driven by the latest
prevstate.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Frontend/src/pages/BasicDetails.tsx(15 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Frontend/src/pages/BasicDetails.tsx (3)
Frontend/src/components/ui/label.tsx (1)
Label(25-25)Frontend/src/components/ui/input.tsx (1)
Input(21-21)Frontend/src/components/ui/select.tsx (2)
Select(150-150)SelectTrigger(153-153)
🔇 Additional comments (1)
Frontend/src/pages/BasicDetails.tsx (1)
215-299: Field wiring and per‑field error UX look solidThe way you’ve bound each influencer/brand field to
formDataanderrorsis consistent and matches the requirements:
- Required fields are clearly marked with a red asterisk.
- Error states are surfaced via border color and inline text with the
AlertCircleicon.- Optional fields (e.g., Instagram, YouTube, website) correctly only show errors when present and invalid.
- Audience size, engagement rate, and campaign settings all have clear, specific messages.
This is a good, reusable pattern across all steps and keeps the UX coherent.
Also applies to: 303-381, 383-458, 460-559, 561-625
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Frontend/src/pages/BasicDetails.tsx (1)
180-189: Validation on final step doesn't prevent "Complete" action.The validation guard at line 181 blocks navigation, but on the final step (
step === 2for influencer orstep === 1for brand), clicking "Complete" runsvalidateStep()then immediately returns because the condition on line 183 is false. This means:
- Validation errors are set in state and displayed to the user ✓
- But the "Complete" button performs no action whether validation passes or fails ✗
If "Complete" is meant to submit the form or navigate elsewhere, you need to add that logic after the validation guard.
Add completion logic after validation:
const nextStep = () => { if (!validateStep()) return; // Don't proceed if validation fails - if ((user === "influencer" && step < 2) || (user === "brand" && step < 1)) { + const isLastStep = (user === "influencer" && step === 2) || (user === "brand" && step === 1); + + if (isLastStep) { + // TODO: Submit form data to backend or navigate to next page + console.log("Form completed:", formData); + // e.g., navigate("/dashboard") or submitFormData(formData) + return; + } + + if ((user === "influencer" && step < 2) || (user === "brand" && step < 1)) { setAnimationDirection(1); setTimeout(() => { setStep((prev) => prev + 1); }, 50); } };
🧹 Nitpick comments (1)
Frontend/src/pages/BasicDetails.tsx (1)
114-123: Consider validating Twitter/TikTok handle format for consistency.Instagram handles are validated to start with
@(line 115-116), but Twitter and TikTok handles (which also use @ prefixes) have no validation. This inconsistency might confuse users or allow malformed data.If you want consistent validation, add checks for Twitter and TikTok in Step 1:
} else if (step === 1) { if (influData.instagram && !influData.instagram.startsWith("@")) { newErrors.instagram = "Instagram handle should start with @"; } + if (influData.twitter && !influData.twitter.startsWith("@")) { + newErrors.twitter = "Twitter handle should start with @"; + } + if (influData.tiktok && !influData.tiktok.startsWith("@")) { + newErrors.tiktok = "TikTok username should start with @"; + } if (influData.youtube && !validateURL(influData.youtube)) { newErrors.youtube = "Invalid YouTube URL"; } if (influData.website && !validateURL(influData.website)) { newErrors.website = "Invalid website URL"; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Frontend/src/pages/BasicDetails.tsx(16 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Frontend/src/pages/BasicDetails.tsx (3)
Frontend/src/components/ui/label.tsx (1)
Label(25-25)Frontend/src/components/ui/input.tsx (1)
Input(21-21)Frontend/src/components/ui/select.tsx (2)
Select(150-150)SelectTrigger(153-153)
🔇 Additional comments (5)
Frontend/src/pages/BasicDetails.tsx (5)
58-89: LGTM!The form state structure cleanly separates influencer and brand flows, and the errors map provides flexible field-level tracking for validation feedback.
159-179: LGTM!The input change handler correctly updates nested state and clears field-specific errors, providing immediate feedback when users fix validation issues.
208-622: LGTM!The UI components consistently display validation errors with clear visual indicators (red borders, AlertCircle icons, inline messages) and required-field markers. Error handling is implemented for all validated fields across both user flows.
834-840: Previous review feedback addressed: button no longer disabled on final step.The step-based
disabledlogic has been removed as requested in the previous review. The button is now clickable on all steps, with validation enforced in thenextStephandler. However, see the comment on lines 180-189 regarding missing completion logic for the final step.
38-42: The regex correctly matches the documented placeholder format—no issue found.The test confirms that
"+1 (555) 000-0000"validates successfully. The original analysis incorrectly assumed the regex couldn't handle both the space and opening paren together; however,[-\s.]?consumes the space, and the separate\(?then matches the paren. The regex works as intended for the documented placeholder format.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Frontend/src/pages/BasicDetails.tsx (1)
689-697: Critical:resetFormuses DOM manipulation instead of resetting React state.Lines 693-696 use
querySelectorAllto directly clear input/select DOM values, but this doesn't update theformDatastate. After clicking reset, the inputs appear empty but the React state still contains the old values, which will reappear when the form re-renders (e.g., during step navigation).Reset the React state instead of manipulating the DOM:
const resetForm = () => { setStep(0); setAnimationDirection(0); - - document.querySelectorAll("input").forEach((input) => (input.value = "")); - document - .querySelectorAll("select") - .forEach((select) => (select.value = "")); + setFormData({ + influencer: { + firstName: "", + lastName: "", + email: "", + phone: "", + category: "", + instagram: "", + youtube: "", + twitter: "", + tiktok: "", + website: "", + audienceSize: "", + avgEngagement: "", + mainPlatform: "", + audienceAge: "", + }, + brand: { + companyName: "", + website: "", + industry: "", + size: "", + budget: "", + targetAudience: "", + preferredPlatforms: "", + campaignGoals: "", + }, + }); + setErrors({}); };
♻️ Duplicate comments (1)
Frontend/src/pages/BasicDetails.tsx (1)
839-845: Implement real-time button disabled state per requirements.The PR objectives explicitly state: "Next button disabled until current-step required fields are valid." However, the button is currently always enabled and validation only runs on click. While the validation guard in
nextStep()prevents progression, users can click repeatedly on an invalid form with no feedback until they fix the errors.For better UX that matches the stated requirements, compute validation state and disable the button when the current step is invalid:
+ // Add after line 94 (after errors state) + const [isCurrentStepValid, setIsCurrentStepValid] = useState(false); + + // Add useEffect to recompute validity when formData or step changes + useEffect(() => { + // Run validation without setting errors (silent check) + const newErrors: Record<string, string> = {}; + const data = user === "influencer" ? formData.influencer : formData.brand; + + // Copy the validation logic from validateStep but don't call setErrors + // ... (same logic as lines 103-158) + + setIsCurrentStepValid(Object.keys(newErrors).length === 0); + }, [formData, step, user]); <Button className="bg-purple-700 hover:bg-purple-800 border border-purple-800 transition-all duration-200 transform hover:scale-105 text-white" onClick={nextStep} + disabled={!isCurrentStepValid} >Alternatively, if you prefer validation-on-submit UX, update the PR objectives to reflect the current behavior.
🧹 Nitpick comments (1)
Frontend/src/pages/BasicDetails.tsx (1)
38-43: Phone validation remains US-centric despite international claim.The current regex enforces a 3-3-4 digit grouping that works for US/Canada formats but will reject many valid international phone numbers (e.g., UK: +44 20 1234 5678, India: +91 98765 43210). The comment on line 39 claims "international" support but the pattern is still country-specific.
Consider either:
- Using a dedicated library like
libphonenumber-jsfor true international phone validation, or- Updating the comment to clarify this validates North American formats, or
- Relaxing the pattern to accept variable-length digit groups:
const validatePhone = (phone: string): boolean => { - // Basic pattern: optional country code + local number with common separators - const phoneRegex = - /^\+?[0-9]{1,3}?[-\s.]?\(?[0-9]{3}\)?[-\s.]?[0-9]{3}[-\s.]?[0-9]{4}$/; + // Pattern for international phone: country code + 7-15 digits with common separators + const phoneRegex = /^\+?[0-9]{1,3}[-\s.]?[(]?[0-9]{1,4}[)]?[-\s.]?[0-9]{1,4}[-\s.]?[0-9]{1,9}$/; return phoneRegex.test(phone.trim()); };This approach allows more flexibility for international formats while still enforcing reasonable structure.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Frontend/src/pages/BasicDetails.tsx(16 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
Frontend/src/pages/BasicDetails.tsx (3)
Frontend/src/components/ui/label.tsx (1)
Label(25-25)Frontend/src/components/ui/input.tsx (1)
Input(21-21)Frontend/src/components/ui/select.tsx (2)
Select(150-150)SelectTrigger(153-153)
| if (influData.instagram && !influData.instagram.startsWith("@")) { | ||
| newErrors.instagram = "Instagram handle should start with @"; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Instagram handle validation is too permissive.
Line 120 only verifies the handle starts with @, but doesn't validate the username format. This allows invalid handles like "@", "@ ", "@!@#", or "@123 456".
Instagram usernames must be 1-30 characters, alphanumeric plus periods/underscores, and cannot end with a period:
- if (influData.instagram && !influData.instagram.startsWith("@")) {
- newErrors.instagram = "Instagram handle should start with @";
+ if (influData.instagram) {
+ const handle = influData.instagram.trim();
+ if (!handle.startsWith("@")) {
+ newErrors.instagram = "Instagram handle should start with @";
+ } else if (!/^@[a-zA-Z0-9._]{1,30}$/.test(handle) || handle.endsWith(".")) {
+ newErrors.instagram = "Invalid Instagram handle format";
+ }
}🤖 Prompt for AI Agents
In Frontend/src/pages/BasicDetails.tsx around lines 120 to 122, the Instagram
handle check only ensures the value starts with "@" and allows invalid inputs;
replace this with a stricter validation that enforces Instagram rules (1–30
chars after "@", only letters, numbers, periods and underscores, and not ending
with a period). Implement either a single regex such as
^@(?!.*\.$)[A-Za-z0-9._]{1,30}$ or validate by checking length, allowed
characters, and that the final character is not '.'; update the error message to
reflect the precise format rule when invalid.
| const handleInputChange = (field: string, value: string) => { | ||
| if (user === "influencer") { | ||
| setFormData(prev => ({ | ||
| ...prev, | ||
| influencer: { ...prev.influencer, [field]: value } | ||
| })); | ||
| } else { | ||
| setFormData(prev => ({ | ||
| ...prev, | ||
| brand: { ...prev.brand, [field]: value } | ||
| })); | ||
| } | ||
| // Clear error for this field | ||
| if (errors[field]) { | ||
| setErrors(prev => { | ||
| const newErrors = { ...prev }; | ||
| delete newErrors[field]; | ||
| return newErrors; | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Trim user input before storing to prevent whitespace-only values.
The validation functions call .trim() on values (lines 42, 46, 52), but handleInputChange stores the raw input without trimming. This allows users to enter whitespace-only values (e.g., " ") that pass the isEmpty check but fail validation when trimmed, creating confusing behavior where a "filled" field shows an error.
Trim text inputs at storage time for consistency:
const handleInputChange = (field: string, value: string) => {
+ // Trim text inputs (but not Select values which are controlled)
+ const trimmedValue = typeof value === 'string' && value.includes(' ')
+ ? value.trim()
+ : value;
+
if (user === "influencer") {
setFormData(prev => ({
...prev,
- influencer: { ...prev.influencer, [field]: value }
+ influencer: { ...prev.influencer, [field]: trimmedValue }
}));
} else {
setFormData(prev => ({
...prev,
- brand: { ...prev.brand, [field]: value }
+ brand: { ...prev.brand, [field]: trimmedValue }
}));
}Note: You may want to trim only on blur instead of on every keystroke for better UX, but at minimum trim before validation.
🤖 Prompt for AI Agents
In Frontend/src/pages/BasicDetails.tsx around lines 164-184, handleInputChange
currently stores raw input allowing whitespace-only values; trim the incoming
value before calling setFormData (e.g., compute const trimmed = value.trim() and
use trimmed when updating influencer/brand) so stored state matches validation
expectations, and use the trimmed value when clearing errors (check
errors[field] against the trimmed value if needed); this ensures whitespace-only
inputs are normalized at storage time (or alternatively move trimming to onBlur
if you prefer better UX).
| <Label htmlFor="firstName">First Name <span className="text-red-500">*</span></Label> | ||
| <Input | ||
| id="firstName" | ||
| placeholder="John" | ||
| className="border border-gray-300" | ||
| value={formData.influencer.firstName} | ||
| onChange={(e) => handleInputChange("firstName", e.target.value)} | ||
| className={`border ${errors.firstName ? "border-red-500" : "border-gray-300"}`} | ||
| /> | ||
| {errors.firstName && ( | ||
| <p className="text-red-500 text-sm flex items-center gap-1"> | ||
| <AlertCircle className="h-4 w-4" /> {errors.firstName} | ||
| </p> | ||
| )} |
There was a problem hiding this comment.
Add ARIA attributes for screen reader accessibility.
The error feedback is visual-only (red borders, error text). Screen reader users won't be informed that fields are invalid or what the error messages are. This pattern repeats throughout all form inputs.
Add aria-invalid and aria-describedby to invalid inputs:
<div className="space-y-2">
<Label htmlFor="firstName">First Name <span className="text-red-500">*</span></Label>
<Input
id="firstName"
placeholder="John"
value={formData.influencer.firstName}
onChange={(e) => handleInputChange("firstName", e.target.value)}
- className={`border ${errors.firstName ? "border-red-500" : "border-gray-300"}`}
+ className={`border ${errors.firstName ? "border-red-500" : "border-gray-300"}`}
+ aria-invalid={!!errors.firstName}
+ aria-describedby={errors.firstName ? "firstName-error" : undefined}
/>
{errors.firstName && (
- <p className="text-red-500 text-sm flex items-center gap-1">
+ <p id="firstName-error" className="text-red-500 text-sm flex items-center gap-1" role="alert">
<AlertCircle className="h-4 w-4" /> {errors.firstName}
</p>
)}
</div>Apply this pattern to all validated inputs and Select components throughout lines 217-625. For Select components, pass aria-invalid as a prop to SelectTrigger.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Label htmlFor="firstName">First Name <span className="text-red-500">*</span></Label> | |
| <Input | |
| id="firstName" | |
| placeholder="John" | |
| className="border border-gray-300" | |
| value={formData.influencer.firstName} | |
| onChange={(e) => handleInputChange("firstName", e.target.value)} | |
| className={`border ${errors.firstName ? "border-red-500" : "border-gray-300"}`} | |
| /> | |
| {errors.firstName && ( | |
| <p className="text-red-500 text-sm flex items-center gap-1"> | |
| <AlertCircle className="h-4 w-4" /> {errors.firstName} | |
| </p> | |
| )} | |
| <Label htmlFor="firstName">First Name <span className="text-red-500">*</span></Label> | |
| <Input | |
| id="firstName" | |
| placeholder="John" | |
| value={formData.influencer.firstName} | |
| onChange={(e) => handleInputChange("firstName", e.target.value)} | |
| className={`border ${errors.firstName ? "border-red-500" : "border-gray-300"}`} | |
| aria-invalid={!!errors.firstName} | |
| aria-describedby={errors.firstName ? "firstName-error" : undefined} | |
| /> | |
| {errors.firstName && ( | |
| <p id="firstName-error" className="text-red-500 text-sm flex items-center gap-1" role="alert"> | |
| <AlertCircle className="h-4 w-4" /> {errors.firstName} | |
| </p> | |
| )} |
🤖 Prompt for AI Agents
In Frontend/src/pages/BasicDetails.tsx around lines 217 to 229 (and apply across
lines 217-625), the inputs and Selects only provide visual error feedback; add
accessibility attributes so screen readers announce validation state. For each
validated input: add aria-invalid={Boolean(errors.fieldName)} and
aria-describedby referencing a unique id for the error message (e.g.,
`${fieldName}-error`) and ensure the error <p> has that id. For Select
components, pass aria-invalid to the SelectTrigger and use aria-describedby on
the underlying trigger input (or a wrapper) pointing to the error id. Repeat
this pattern for every validated field and Select between lines 217–625.
1 participant
Closes #197
📝 Description
This PR fixes a critical bug where the BasicDetails form component was accepting invalid user input without any validation or error feedback. Users could submit empty required fields, malformed emails, invalid phone numbers, and incorrect URLs without any warnings or prevention. This compromise data integrity and causes backend errors.
The fix implements comprehensive client-side validation with real-time error feedback, helping users enter correct information before submission.
🔧 Changes Made
Form State Management
Validation Functions
validateEmail()- Validates email format using regex patternvalidatePhone()- Validates international phone number formatvalidateURL()- Validates HTTP/HTTPS URLs using URL constructorvalidateStep()- Step-specific validation based on user type and current stepInput Validation Rules Implemented
Influencer Form - Step 0 (Basic Details):
Influencer Form - Step 1 (Social Media):
Influencer Form - Step 2 (Audience):
Brand Form - Step 0 (Company Info):
Brand Form - Step 1 (Campaign Preferences):
User Experience Improvements
Code Quality
Before:
After:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.