[POLY-91] Add required field indicators and validation to listing creation#104
[POLY-91] Add required field indicators and validation to listing creation#104
Conversation
…creation (POLY-91)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 46 minutes and 2 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe changes introduce form field validation for listings, adding a validation module with field-specific validators and error management utilities, integrating this into the form component with inline feedback UI and preventing submission until all required fields pass validation. Changes
Sequence DiagramsequenceDiagram
participant User
participant Form as Form Component
participant Handler as Change Handler
participant Validator as Validation Module
participant State as Component State
participant UI as Rendered Output
User->>Form: Enters/Changes Field Value
Form->>Handler: Triggers onChange Event
Handler->>State: Updates Field Value
Handler->>Validator: Calls Validator for Field
Validator->>Handler: Returns Error (if any)
Handler->>State: Updates fieldErrors via upsertFieldError
State->>UI: Re-renders with Error/Success State
UI->>User: Displays Inline Error or Valid Indicator
User->>Form: Submits Form
Form->>Validator: Calls validateListingFields
Validator->>Form: Returns All Field Errors
Form->>State: Stores fieldErrors & hasAttemptedSubmit
alt Validation Fails
State->>UI: Displays Form-Level Error Banner
UI->>User: Prevents Submission
else Validation Passes
Form->>Form: Proceeds with Create/Update Logic
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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: 1
🧹 Nitpick comments (2)
frontend/app/listings/__tests__/newListingValidation.test.ts (1)
11-65: Tests are solid; consider adding boundary coverage.The suite covers the happy path and the key
upsertFieldErrorremoval semantics cleanly. Consider adding assertions for the other documented bounds that aren't exercised today:
validateTitlewith a 101+ char string →'Title must be 100 characters or less.'validateImageswith a 9+ entry array →'Maximum 8 photos allowed.'These branches are implemented but currently untested, so regressions (e.g., flipping
</>) would slip through.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/listings/__tests__/newListingValidation.test.ts` around lines 11 - 65, Add two boundary tests in the same test suite to cover the upper limits: call validateTitle with a 101-character string and assert it returns 'Title must be 100 characters or less.'; call validateImages with an array of 9 entries (e.g., nine distinct strings) and assert it returns 'Maximum 8 photos allowed.' Place these as new it(...) cases alongside existing tests so validateTitle and validateImages branches for the 100-char and 8-photo limits are exercised.frontend/app/listings/new.tsx (1)
163-198: Minor:setHasAttemptedSubmit(true)fires before the profile/uploads guards.Once
onSubmitsetshasAttemptedSubmit(Line 168) and then returns early becauseprofileis still loading or uploads are pending (Lines 170–186), every subsequent keystroke will start surfacing inline errors even though the user never actually failed validation. Consider movingsetHasAttemptedSubmit(true)down to just beforevalidateListingFieldsso the "attempted" state only latches when a real validation pass was made.♻️ Suggested move
async function onSubmit() { if (submittingRef.current) { return; } - setHasAttemptedSubmit(true); - if (profile === undefined) { ... return; } if (profile === null) { ... return; } if (hasPendingUploads) { ... return; } + setHasAttemptedSubmit(true); const errors = validateListingFields({ title, description, price, images }); setFieldErrors(errors); - if (Object.keys(errors).length > 0) { + if (hasFieldErrors(errors)) { return; }Also swapping
Object.keys(errors).length > 0forhasFieldErrors(errors)keeps the gate consistent with the module's public API.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/app/listings/new.tsx` around lines 163 - 198, Move the setHasAttemptedSubmit(true) call in onSubmit so it runs only after passing the early guards (profile/loading and hasPendingUploads) and immediately before calling validateListingFields; this ensures hasAttemptedSubmit only flips when a real validation attempt occurs. Update the post-validation gate to use the public helper hasFieldErrors(errors) instead of Object.keys(errors).length > 0 and continue to call setFieldErrors(errors) and bail out if hasFieldErrors(errors) returns true. Ensure references are to onSubmit, setHasAttemptedSubmit, validateListingFields, setFieldErrors, hasFieldErrors, profile, and hasPendingUploads.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/app/listings/new.tsx`:
- Around line 153-161: In handleImagesChange, don't call setFieldErrors from
inside the setImages updater; compute nextImages first (using the incoming
newImages or invoking it with prevImages), then call setImages(nextImages) and,
if hasAttemptedSubmit, call setFieldErrors(prev => upsertFieldError(prev,
'images', validateImages(nextImages))). This keeps the setImages updater pure
and matches the pattern used in handlePriceChange; reference functions/idents:
handleImagesChange, setImages, setFieldErrors, upsertFieldError, validateImages,
hasAttemptedSubmit.
---
Nitpick comments:
In `@frontend/app/listings/__tests__/newListingValidation.test.ts`:
- Around line 11-65: Add two boundary tests in the same test suite to cover the
upper limits: call validateTitle with a 101-character string and assert it
returns 'Title must be 100 characters or less.'; call validateImages with an
array of 9 entries (e.g., nine distinct strings) and assert it returns 'Maximum
8 photos allowed.' Place these as new it(...) cases alongside existing tests so
validateTitle and validateImages branches for the 100-char and 8-photo limits
are exercised.
In `@frontend/app/listings/new.tsx`:
- Around line 163-198: Move the setHasAttemptedSubmit(true) call in onSubmit so
it runs only after passing the early guards (profile/loading and
hasPendingUploads) and immediately before calling validateListingFields; this
ensures hasAttemptedSubmit only flips when a real validation attempt occurs.
Update the post-validation gate to use the public helper hasFieldErrors(errors)
instead of Object.keys(errors).length > 0 and continue to call
setFieldErrors(errors) and bail out if hasFieldErrors(errors) returns true.
Ensure references are to onSubmit, setHasAttemptedSubmit, validateListingFields,
setFieldErrors, hasFieldErrors, profile, and hasPendingUploads.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f100d84e-6c53-4d10-9761-e07778c38afe
📒 Files selected for processing (3)
frontend/app/listings/__tests__/newListingValidation.test.tsfrontend/app/listings/new.tsxfrontend/app/listings/newListingValidation.ts
2 participants
Summary
Improves the Create Listing form with visible required field indicators and inline validation so users know exactly what needs to be fixed before submitting.
Linear: POLY-91
Changes
frontend/app/listings/new.tsxValidation rules:
Testing
Created with Kiro
Summary by CodeRabbit
New Features
Tests